From 1fce3b05d4e9f70bc8733707d979ab8b6a410c81 Mon Sep 17 00:00:00 2001 From: Tony Kim Date: Thu, 10 Aug 2023 15:53:18 +0900 Subject: [PATCH] REVIEW: review ver 1 proposed --- server/account/models.py | 1 + server/account/views.py | 3 +++ server/vote/views.py | 19 +++++++++++++------ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/account/models.py b/server/account/models.py index 5d355cb..1ffa4ad 100644 --- a/server/account/models.py +++ b/server/account/models.py @@ -9,6 +9,7 @@ class User(AbstractUser): ) gender = models.CharField(verbose_name="성별", max_length=1, choices=GENDERS) nickname = models.CharField(max_length=20) + # Review : MBTI 셋 변수가 다른 파일에서도 사용되고 있는데, 통합 관리가 필요할 것 같습니다. MBTI_set = ( ("INFP", "INFP"), ("ENFP", "ENFP"), diff --git a/server/account/views.py b/server/account/views.py index fe3664a..18afdf4 100644 --- a/server/account/views.py +++ b/server/account/views.py @@ -43,11 +43,13 @@ def login(request): auth.login(request, user) return redirect("/") else: + # Review : 아이디가 틀리는 경우는 없는건가요? wrong_password = True # 비밀번호가 틀렸을 때 변수를 설정하여 템플릿으로 전달 context = { "form": form, "wrong_password": wrong_password, # 변수를 템플릿으로 전달 } + # Review : 위에서 ctx를 쓰고 여기서 context를 쓰는데, 변수명 통일이 필요합니다. return render(request, "account/login.html", context=context) else: form = AuthenticationForm() @@ -76,6 +78,7 @@ def change_password(request): #회원탈퇴 class UserDeleteView(DeleteView): + # Review : 갑자기 탈퇴 부분만 DRF가 된 이유가 있나요?? model = User template_name = "account/delete.html" success_url = reverse_lazy("vote:mypage") diff --git a/server/vote/views.py b/server/vote/views.py index 063f358..7dc591b 100644 --- a/server/vote/views.py +++ b/server/vote/views.py @@ -15,12 +15,16 @@ def main(request): polls = Poll.objects.all() sort = request.GET.get("sort") - if sort == "popular": - polls = polls.order_by("-views_count") # 인기순 - elif sort == "latest": - polls = polls.order_by("-id") # 최신순 - elif sort == "oldest": - polls = polls.order_by("id") # 등록순 + # Review : 정렬 방식이 많아지면 관리가 까다로워질 수 있으니 아래처럼 딕셔너리 사용하는 것을 추천드립니다. + + SORT_KEY_ORDER_BY_MAPPING = { + "popular": "-views_count", + "latest": "-id", + "oldest": "id", + "default": "-id" + } + + polls = pools.order_by(SORT_KEY_ORDER_BY_MAPPING.get(sort, "default")) page = request.GET.get("page") @@ -78,9 +82,11 @@ def poll_like(request): if request.user.is_authenticated: user = request.user + # Review : login_required 인데 이 부분으로 들어올 수 있나요? else: user = AnonymousUser() + # Review : login_required 인데 is_authenticated는 항상 참 아닌가요? if user.is_authenticated and user.is_active: # 인증된 사용자 중 활성화된 사용자만 고려 if poll.poll_like.filter(id=user.id).exists(): poll.poll_like.remove(user) @@ -162,6 +168,7 @@ def classifyuser(request, poll_id): # 회원/비회원 투표 통계 계산 및 결과 페이지 def calcstat(request, poll_id): + # Review : 반복되는 부분은 반복문 사용해서 없앨 수 있을 것 같습니다. poll = get_object_or_404(Poll, pk=poll_id) mbtis = [ "ISTJ",