유저 가입 승인 api 작성 #22
유저 가입 승인 api 작성 #22
Conversation
- 유저 회원가입 시 인증 코드 생성하도록 시리얼라이저, signupview 수정 - 인증 코드 확인하는 confirmuserview 작성 - 테스트코드 작성
users/serializers.py
Outdated
| user_serializer = UserSerializer(data=validated_data) | ||
| user_serializer.is_valid(raise_exception=True) | ||
| user = user_serializer.save() |
There was a problem hiding this comment.
UserSerializer를 상속받았기 때문에 user = super().create(validated_data)로 대체할 수 있을 것 같습니다.
| user_confirm_code_serializer.is_valid(raise_exception=True) | ||
| user_confirm_code = user_confirm_code_serializer.save() | ||
|
|
||
| response_data = UserSerializer(user_confirm_code.user).data |
There was a problem hiding this comment.
확인코드를 메일로 보내는데 django에 내장되어 있는 smtp 라이브러리 함수를 사용하면 어떨까요?
from django.core.mail import send_mail
추가로 당장 적용하긴 시간상 어려움이 있지만, 메일링 같은 시간이 많이 쓰여지는 작업은 메시지큐(MQ)를 사용하여 비동기 태스크로 작업한다고 알고 있습니다.
비동기 태스크 패캐지 Celery와 메세지 큐 브로커 (RabbitMq 혹은 Redis)를 연결해보는 것도 좋을 것 같습니다.
There was a problem hiding this comment.
메일로 발송은 생략한다고 해서 추후에 시간이 남으면 해보도록 하겠습니다!
| user_serializer.is_valid(raise_exception=True) | ||
| user = user_serializer.save() | ||
|
|
||
| confirm_code = "".join(random.choice(string.ascii_letters + string.digits) for i in range(6)) |
There was a problem hiding this comment.
해당 기능으로는 랜덤한 스트링 값을 할당을 해주는 것으로 제가 확인이됩니다. 혹시 common/utils.py에 generate_random_string 함수명으로 따로 모듈 분리 후 불러오는 것은 어떨까요?
There was a problem hiding this comment.
좋은 것 같습니다! 수정할게요
| 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() |
There was a problem hiding this comment.
다른 사람들의 코드 통일성을 위해 user_confirm_code_serializer보다 serialzier로 통힐해보시는 것은 어떨까 싶습니다 :)
| urlpatterns = [ | ||
| path("signup/", SignupView.as_view(), name="signup"), | ||
| ] | ||
| urlpatterns = [path("signup/", SignupView.as_view(), name="signup"), path("confirm/", ConfirmUserView.as_view(), name="confirm")] |
There was a problem hiding this comment.
url패턴이 정확하게 확인이 어려운 것 같습니다
아래와 같이 작성해보시는 것은 어떠신가요?
urlpatterns = [
path("signup/", SignupView.as_view(), name="signup"),
path("confirm/", ConfirmUserView.as_view(), name="confirm")
]
There was a problem hiding this comment.
그렇게 작성했었는데 flake, black 등에서 저렇게 바뀐 것 같네요 수정해보겠습니다
There was a problem hiding this comment.
저도 pre-commit 을 많이 써보지 않아 이번 과제에 적용하는데 어려움이 많군요... 이번 과제로 인해서 다음 과제에는 실수 없이 설정 처리할 수 있도록 노력하겠습니다 :)
| response_data = {} | ||
| response_data["username"] = user.username | ||
| response_data["is_confirmed"] = user.is_confirmed | ||
|
|
||
| return Response(response_data, status=status.HTTP_200_OK) |
There was a problem hiding this comment.
만약 저였으면 아래와 같이 작성했을 것 같은데
해당 코드를 명시하신 이유가 궁금한데 알려주실 수 있을까요!
return Response(serialzier.data, status=status.HTTP_200_OK)
There was a problem hiding this comment.
serializer에 is_confirmed 항목이 없어서 추가했습니다. 저도 시리얼라이저로 처리할 수 있는 방법이 없나 생각했는데 혹시 있을까요?
There was a problem hiding this comment.
앗 그렇군요 그러면 상속 받으신 serialzier에 필드를 추가해보시는 것은 어떨까요??
class UserConfirmCodeSerializer(UserSerializer):
username = serializer.CharFiled()
is_confirmed = serialzier.BooleanFiled()
| user_confirm_serializer = UserConfirmSerializer(user, data=request.data) | ||
| user_confirm_serializer.is_valid(raise_exception=True) | ||
| user_confirm_serializer.save() |
There was a problem hiding this comment.
해당 코드 역시 serialzer로 통일하시면 어떨까 싶습니다 :)
PR 체크리스트
아래 항목을 확인해 주세요:
PR 유형
이 PR은 어떤 종류의 변경을 가져오나요?
현재 동작은 무엇인가요?
이슈 번호: #7
새로운 동작은 무엇인가요?
이 PR은 호환성 변경을 도입하나요?
기타 정보