Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

유저 가입 승인 api 작성 #22

Merged
merged 4 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions users/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import random
import string

from django.contrib.auth import get_user_model, password_validation
from rest_framework import serializers

from users.models import UserConfirmCode


class UserSerializer(serializers.ModelSerializer):
class Meta:
Expand All @@ -15,3 +20,39 @@ def validate_password(self, data):
def create(self, validated_data):
user = get_user_model().objects.create_user(validated_data["email"], validated_data["username"], validated_data["password"])
return user


class UserConfirmCodeSerializer(UserSerializer):
def create(self, validated_data):
user_serializer = UserSerializer(data=validated_data)
user_serializer.is_valid(raise_exception=True)
user = user_serializer.save()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserSerializer를 상속받았기 때문에 user = super().create(validated_data)로 대체할 수 있을 것 같습니다.


confirm_code = "".join(random.choice(string.ascii_letters + string.digits) for i in range(6))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 기능으로는 랜덤한 스트링 값을 할당을 해주는 것으로 제가 확인이됩니다. 혹시 common/utils.py에 generate_random_string 함수명으로 따로 모듈 분리 후 불러오는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 것 같습니다! 수정할게요

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다

user_confirm_code = UserConfirmCode.objects.create(code=confirm_code, user=user)
return user_confirm_code


class UserConfirmSerializer(serializers.Serializer):
username = serializers.CharField(max_length=128)
password = serializers.CharField(max_length=128, write_only=True)
code = serializers.CharField(max_length=32)

def validate(self, data):
user = self.instance
if not user.check_password(data["password"]):
raise serializers.ValidationError("Password is incorrect")

if user.is_confirmed:
raise serializers.ValidationError("User is already confirmed")

confirm_code = UserConfirmCode.objects.get(user=user).code
if confirm_code != data["code"]:
raise serializers.ValidationError("Confirmation code is incorrect")

return data

def update(self, user, validated_data):
user.is_confirmed = True
user.save()
return user
93 changes: 93 additions & 0 deletions users/tests/views/test_confirm_user_view.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import json

from django.urls import reverse
from rest_framework import status
from rest_framework.test import APITestCase

from users.models import User, UserConfirmCode


class SignupViewTest(APITestCase):
@classmethod
def setUpTestData(cls):
cls.user = User.objects.create_user(
email="testuser1@example.com",
username="testusername1",
password="testpassword",
)

cls.userconfirmcode = UserConfirmCode.objects.create(
code="abcdef",
user=cls.user,
)

def test_post_signup_success(self):
response = self.client.post(
path=reverse("confirm"),
data=json.dumps(
{
"username": "testusername1",
"password": "testpassword",
"code": "abcdef",
}
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_post_signup_fail_user_not_found(self):
response = self.client.post(
path=reverse("confirm"),
data=json.dumps(
{
"username": "testusername2",
"password": "testpassword",
"code": "abcdef",
}
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)

def test_post_signup_fail_invalid_password(self):
response = self.client.post(
path=reverse("signup"),
data=json.dumps(
{
"username": "testusername1",
"password": "testpassword2",
"code": "abcdef",
}
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

def test_post_signup_fail_invalid_code(self):
response = self.client.post(
path=reverse("signup"),
data=json.dumps(
{
"username": "testusername1",
"password": "testpassword",
"code": "aaaaaa",
}
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

def test_post_signup_fail_already_confirmed(self):
self.user.is_confirmed = True
response = self.client.post(
path=reverse("signup"),
data=json.dumps(
{
"username": "testusername1",
"password": "testpassword",
"code": "abcdef",
}
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
6 changes: 2 additions & 4 deletions users/urls.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from django.urls import path

from users.views import SignupView
from users.views import ConfirmUserView, SignupView

urlpatterns = [
path("signup/", SignupView.as_view(), name="signup"),
]
urlpatterns = [path("signup/", SignupView.as_view(), name="signup"), path("confirm/", ConfirmUserView.as_view(), name="confirm")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url패턴이 정확하게 확인이 어려운 것 같습니다
아래와 같이 작성해보시는 것은 어떠신가요?

urlpatterns = [
    path("signup/", SignupView.as_view(), name="signup"), 
    path("confirm/", ConfirmUserView.as_view(), name="confirm")
]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇게 작성했었는데 flake, black 등에서 저렇게 바뀐 것 같네요 수정해보겠습니다

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 pre-commit 을 많이 써보지 않아 이번 과제에 적용하는데 어려움이 많군요... 이번 과제로 인해서 다음 과제에는 실수 없이 설정 처리할 수 있도록 노력하겠습니다 :)

53 changes: 45 additions & 8 deletions users/views.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,69 @@
from django.contrib.auth import get_user_model
from django.shortcuts import get_object_or_404
from drf_yasg.utils import swagger_auto_schema
from rest_framework import status
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.views import APIView

from users.serializers import UserSerializer
from users.serializers import UserConfirmCodeSerializer, UserConfirmSerializer, UserSerializer


class SignupView(APIView):
@swagger_auto_schema(
operation_summary="유저 회원가입",
request_body=UserSerializer,
request_body=UserConfirmCodeSerializer,
responses={
status.HTTP_201_CREATED: UserSerializer,
status.HTTP_201_CREATED: UserConfirmCodeSerializer,
},
)
def post(self, request: Request) -> Response:
"""
username, email, paswword를 받아 유저 계정을 생성합니다.
username, email, paswword를 받아 유저 계정과 인증 코드를 생성합니다.
Args:
email: 이메일
username: 이름
password: 비밀번호
Returns:
email: 생성된 계정 이메일
username: 생성된 계정 이름
code: 생성된 인증 코드
"""
serializer = UserSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(serializer.data, status=status.HTTP_201_CREATED)
user_confirm_code_serializer = UserConfirmCodeSerializer(data=request.data)
user_confirm_code_serializer.is_valid(raise_exception=True)
user_confirm_code = user_confirm_code_serializer.save()
Comment on lines +36 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 사람들의 코드 통일성을 위해 user_confirm_code_serializer보다 serialzier로 통힐해보시는 것은 어떨까 싶습니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 아래도 수정하겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 감사합니다!


response_data = UserSerializer(user_confirm_code.user).data
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인코드를 메일로 보내는데 django에 내장되어 있는 smtp 라이브러리 함수를 사용하면 어떨까요?

from django.core.mail import send_mail

추가로 당장 적용하긴 시간상 어려움이 있지만, 메일링 같은 시간이 많이 쓰여지는 작업은 메시지큐(MQ)를 사용하여 비동기 태스크로 작업한다고 알고 있습니다.
비동기 태스크 패캐지 Celery와 메세지 큐 브로커 (RabbitMq 혹은 Redis)를 연결해보는 것도 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메일로 발송은 생략한다고 해서 추후에 시간이 남으면 해보도록 하겠습니다!

response_data["confirm_code"] = user_confirm_code.code
return Response(response_data, status=status.HTTP_201_CREATED)


class ConfirmUserView(APIView):
@swagger_auto_schema(
operation_summary="유저 가입 승인",
request_body=UserConfirmSerializer,
responses={
status.HTTP_200_OK: UserConfirmSerializer,
},
)
def post(self, request: Request) -> Response:
"""
username, paswword, code를 받아 code가 user의 인증코드와 같을 경우 회원가입을 승인합니다.
Args:
username: 이름
password: 비밀번호
code: 인증 코드
Returns:
username: 이름
is_confirmed: 인증 여부
"""
user = get_object_or_404(get_user_model(), username=request.data["username"])
user_confirm_serializer = UserConfirmSerializer(user, data=request.data)
user_confirm_serializer.is_valid(raise_exception=True)
user_confirm_serializer.save()
Comment on lines +65 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 코드 역시 serialzer로 통일하시면 어떨까 싶습니다 :)


response_data = {}
response_data["username"] = user.username
response_data["is_confirmed"] = user.is_confirmed

return Response(response_data, status=status.HTTP_200_OK)
Comment on lines +69 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약 저였으면 아래와 같이 작성했을 것 같은데
해당 코드를 명시하신 이유가 궁금한데 알려주실 수 있을까요!

return Response(serialzier.data, status=status.HTTP_200_OK)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serializer에 is_confirmed 항목이 없어서 추가했습니다. 저도 시리얼라이저로 처리할 수 있는 방법이 없나 생각했는데 혹시 있을까요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 그렇군요 그러면 상속 받으신 serialzier에 필드를 추가해보시는 것은 어떨까요??

class UserConfirmCodeSerializer(UserSerializer):
    username = serializer.CharFiled()
    is_confirmed = serialzier.BooleanFiled()

Loading