refactor: cleaner view code for many endpoints (#81)

* chore: add resend confirmation mail endpoint to bruno project

* fix: remove internal error from response message in unhandled exception handler.

* refactor: remove unneeded complexity from ResendAccountSerializer

* refactor: simplified ResendAccountConfirmationView

* refactor: simplified Login endpoints

* refactor: removed unused custom exception

* refactor: refactored register account endpoint. It also won't return a login token with the response anymore.

* fix: fixed token lifespan for confirmation being in minutes instead of hours

* refactor: refactored Serializer so the validated data is the data coming in body

This is normally how they behave, so changing it can confuse other contributors who are familiar with Django rest framework.

* refactor: Modified RequestPasswordResetView so that its serializer only validates incoming data

Same as the other refactor, this is how serializers are usually used. The logic for doing stuff is done on the view itself.

* refactor: modified LoginWithCredentialsView to have all logic in the view itself.

As stated before, the serializer only task is to validate incoming data.

* chore: update unit test

api-collection/Auth/ResendConfirmationMail/Valid.bru

@@ -0,0 +1,17 @@
+meta {
+  name: Valid
+  type: http
+  seq: 1
+}
+
+post {
+  url: {{baseUrl}}/accounts/resend-account-confirmation
+  body: json
+  auth: none
+}
+
+body:json {
+  {
+    "email": "[email protected]"
+  }
+}

src/accounts/api/serializers.py

@@ -1,11 +1,8 @@
-import secrets
-
 from django.conf import settings
-from django.contrib.auth import authenticate
 from django.contrib.auth.password_validation import validate_password
 from rest_framework import serializers
 
-from ..models import Account, AccountConfirmation, PasswordResetRequestModel
+from ..models import Account, AccountConfirmation
 
 
 class PublicAccountDataSerializer(serializers.ModelSerializer):
@@ -37,16 +34,6 @@ class LoginWithCredentialsSerializer(serializers.Serializer):
     email = serializers.EmailField()
     password = serializers.CharField(style={"input_type": "password"})
 
-    def validate(self, data):
-        account: Account | None = authenticate(username=data["email"], password=data["password"])  # type: ignore[assignment]
-        if account is None:
-            raise serializers.ValidationError("Unable to login with provided credentials.")
-        if not account.is_confirmed:
-            raise serializers.ValidationError(
-                "This account hasn't done the mail confirmation step or has been disabled."
-            )
-        return account
-
 
 class UpdateAccountSerializer(serializers.ModelSerializer):
     class Meta:
@@ -85,40 +72,15 @@ def validate(self, data):
         return account
 
 
-class ResetPasswordSerializer(serializers.ModelSerializer):
-    class Meta:
-        model = Account
-        fields = ("password",)
-        extra_kwargs = {"password": {"write_only": True}}
+class ResetPasswordSerializer(serializers.Serializer):
+    password = serializers.CharField(style={"input_type": "password"})
 
     def validate_password(self, value):
         # Validate the password using Django's built-in validators
         validate_password(value)
         return value
 
 
-class ResetPasswordRequestSerializer(serializers.ModelSerializer):
-    class Meta:
-        model = PasswordResetRequestModel
-        fields = ("email",)
-
-    email = serializers.EmailField()
-
-    def validate(self, data):
-        email = data["email"]
-        account = Account.objects.get(email=email)
-        if account is None:
-            raise serializers.ValidationError("Account with this email doesn't exist.")
-
-        return {
-            "token": secrets.token_urlsafe(32),
-            "account": account,
-        }
-
-    def create(self, validated_data):
-        return PasswordResetRequestModel.objects.create(**validated_data)
-
-
 class ConfirmAccountSerializer(serializers.Serializer):
     token = serializers.CharField()
 
@@ -130,16 +92,8 @@ def validate(self, data):
 
         if not account_confirmation.is_token_valid():
             raise serializers.ValidationError("Token is invalid or expired.")
-        return account_confirmation
+        return {"token": data["token"]}
 
 
-class ResendAccountSerializer(serializers.Serializer):
+class EmailSerializer(serializers.Serializer):
     email = serializers.EmailField()
-
-    def validate(self, data):
-        email = data["email"]
-        account = Account.objects.get(email=email)
-        if account is None:
-            raise Account.DoesNotExist("Account with this email doesn't exist.")
-
-        return account

src/accounts/api/views.py

@@ -1,11 +1,13 @@
 import logging
+import secrets
 
 from urllib.parse import urljoin
 from uuid import uuid4
 
 from commons.error_response import ErrorResponse
 from commons.mail_wrapper import send_email_with_template
 from django.conf import settings
+from django.contrib.auth import authenticate
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
 from drf_spectacular.utils import extend_schema
 from knox.models import AuthToken
@@ -17,16 +19,13 @@
 from rest_framework.serializers import ValidationError
 from rest_framework.views import APIView
 
-from ..exceptions import MissingMailConfirmationError
-from ..models import Account
+from ..models import Account, AccountConfirmation, PasswordResetRequestModel
 from .serializers import (
     ConfirmAccountSerializer,
+    EmailSerializer,
     LoginWithCredentialsSerializer,
-    PasswordResetRequestModel,
     PublicAccountDataSerializer,
     RegisterAccountSerializer,
-    ResendAccountSerializer,
-    ResetPasswordRequestSerializer,
     ResetPasswordSerializer,
     UpdateAccountSerializer,
     VerifyAccountSerializer,
@@ -54,11 +53,8 @@ def post(self, request, format=None):
     def get_post_response_data(self, request, token, instance):
         user: Account = request.user
 
-        try:
-            if not user.is_confirmed:
-                raise MissingMailConfirmationError()
-        except MissingMailConfirmationError as e:
-            return ErrorResponse(str(e), MissingMailConfirmationError.status_code)
+        if not user.is_confirmed:
+            return ErrorResponse("You must confirm your email before attempting to login.", status.HTTP_400_BAD_REQUEST)
 
         serializer = self.get_user_serializer_class()
 
@@ -84,19 +80,22 @@ class LoginWithCredentialsView(GenericAPIView):
     def post(self, request):
         serializer = self.serializer_class(data=request.data)
 
-        try:
-            serializer.is_valid(raise_exception=True)
-            account = Account.objects.get(email=serializer.data["email"])
-            if not account.is_active:
-                raise MissingMailConfirmationError()
-        except ObjectDoesNotExist:
-            return ErrorResponse("Account does not exist.", status.HTTP_404_NOT_FOUND)
-        except MissingMailConfirmationError as e:
-            return ErrorResponse(str(e), status.HTTP_400_BAD_REQUEST)
-        except ValidationError as e:
-            return ErrorResponse(str(e), status.HTTP_400_BAD_REQUEST)
+        if not serializer.is_valid():
+            return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)
 
-        account = serializer.validated_data
+        email = serializer.validated_data["email"]
+        password = serializer.validated_data["password"]
+
+        account: Account | None = authenticate(email=email, password=password)  # type: ignore[assignment]
+
+        if account is None:
+            return ErrorResponse("Unable to login with provided credentials.", status.HTTP_400_BAD_REQUEST)
+
+        if not account.is_confirmed:
+            return ErrorResponse("You must confirm your email before attempting to login.", status.HTTP_400_BAD_REQUEST)
+
+        if not account.is_active:
+            return ErrorResponse("Account is suspended.", status.HTTP_400_BAD_REQUEST)
 
         return Response(
             {
@@ -119,17 +118,14 @@ class RegisterAccountView(GenericAPIView):
 
     def post(self, request):
         serializer = self.get_serializer(data=request.data)
-        try:
-            serializer.is_valid(raise_exception=True)
-        except ValidationError as e:
-            return Response(data={"error": str(e)}, status=e.status_code)
 
-        account = serializer.save()
+        if not serializer.is_valid():
+            return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)
 
+        account = serializer.save()
         return Response(
             {
-                "account": RegisterAccountSerializer(account, context=self.get_serializer_context()).data,
-                "token": AuthToken.objects.create(account)[1],
+                "account": PublicAccountDataSerializer(account, context=self.get_serializer_context()).data,
             },
             status=status.HTTP_200_OK,
         )
@@ -232,20 +228,21 @@ class ResetPasswordView(GenericAPIView):
     def post(self, request, reset_token):
         serializer = self.serializer_class(data=request.data)
 
+        if not serializer.is_valid():
+            return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)
+
         try:
-            serializer.is_valid(raise_exception=True)
-            reset_request = PasswordResetRequestModel.objects.get(token=reset_token)
-            if not reset_request.is_token_valid():
+            reset_token = PasswordResetRequestModel.objects.get(token=reset_token)
+            if not reset_token.is_token_valid():
                 raise PasswordResetRequestModel.DoesNotExist
-        except ValidationError as e:
-            return ErrorResponse(str(e), e.status_code)
         except PasswordResetRequestModel.DoesNotExist:
             return ErrorResponse("Invalid link or expired.", status.HTTP_400_BAD_REQUEST)
 
-        account = reset_request.account
+        account = reset_token.account
         account.set_password(serializer.validated_data["password"])
         account.save()
-        reset_request.delete()
+        reset_token.delete()
+
         return Response(status=status.HTTP_200_OK)
 
 
@@ -257,24 +254,29 @@ class RequestPasswordResetView(GenericAPIView):
     """
 
     permission_classes = (AllowAny,)
-    serializer_class = ResetPasswordRequestSerializer
+    serializer_class = EmailSerializer
 
     def post(self, request):
         serializer = self.serializer_class(data=request.data)
 
+        if not serializer.is_valid():
+            return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)
+
         try:
-            serializer.is_valid(raise_exception=True)
-        except ValidationError:
+            account = Account.objects.get(email=serializer.validated_data["email"])
+        except Account.DoesNotExist:
             logger.warning(
                 "Attempted to reset password for non-existing account: %s", serializer.validated_data["email"]
             )
             return Response(status=status.HTTP_200_OK)
 
-        serializer.save()
-        link = urljoin(settings.PASS_RESET_URL, serializer.validated_data["token"])
+        token = secrets.token_urlsafe(32)
+        PasswordResetRequestModel.objects.create(account=account, token=token)
+
+        link = urljoin(settings.PASS_RESET_URL, token)
 
         send_email_with_template(
-            recipient=serializer.validated_data["account"].email,
+            recipient=account.email,
             subject="Reset your password",
             template="password_reset.html",
             context={"link": link},
@@ -302,17 +304,12 @@ def post(self, request, confirm_token):
         except ValidationError as e:
             return ErrorResponse(str(e), e.status_code)
 
-        account_id = serializer.validated_data.account.unique_identifier
+        account_confirmation = AccountConfirmation.objects.get(token=serializer.validated_data["token"])
+        account = account_confirmation.account
 
-        try:
-            account = Account.objects.get(unique_identifier=account_id)
-        except Account.DoesNotExist:
-            return ErrorResponse("Account for this confirmation link does not exist.", status.HTTP_400_BAD_REQUEST)
-
-        account.is_active = True
         account.is_confirmed = True
+        account_confirmation.delete()
         account.save()
-        serializer.validated_data.delete()
 
         return Response(status=status.HTTP_200_OK)
 
@@ -325,23 +322,24 @@ class ResendAccountConfirmationView(GenericAPIView):
     """
 
     permission_classes = (AllowAny,)
-    serializer_class = ResendAccountSerializer
+    serializer_class = EmailSerializer
 
-    def post(self, request):
-        serializer = self.serializer_class(data=request.data)
+    def post(self, request, *args, **kwargs):
+        serializer: EmailSerializer = self.serializer_class(data=request.data)
 
-        try:
-            serializer.is_valid(raise_exception=True)
-        except ValidationError as e:
-            return ErrorResponse(str(e), e.status_code)
-        except Account.DoesNotExist:
-            logger.warning(
-                "Attempted to resend account confirmation for non-existing account: %s",
-                serializer.validated_data["email"],
-            )
-            return Response(status=status.HTTP_200_OK)
+        if serializer.is_valid():
+            email = serializer.validated_data["email"]
+            try:
+                account = Account.objects.get(email=email)
+            except Account.DoesNotExist:
+                logger.warning("Attempted to resend confirmation mail for non-existing account: %s", email)
+                return Response(status=status.HTTP_200_OK)
 
-        account = serializer.validated_data
-        account.send_confirmation_mail()
+            if account.is_confirmed:
+                logger.warning("Attempted to resend confirmation mail for already confirmed account: %s", email)
+                return Response(status=status.HTTP_200_OK)
 
-        return Response(status=status.HTTP_200_OK)
+            account.send_confirmation_mail()
+            return Response(status=status.HTTP_200_OK)
+        else:
+            return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)

src/accounts/models.py

@@ -115,7 +115,7 @@ def __str__(self):
     def is_token_valid(self):
         if self.created_at is None:
             return False
-        return (self.created_at + timedelta(minutes=settings.ACCOUNT_CONFIRMATION_TOKEN_TTL)) > timezone.now()
+        return (self.created_at + timedelta(hours=settings.ACCOUNT_CONFIRMATION_TOKEN_TTL)) > timezone.now()
 
 
 class PasswordResetRequestModel(models.Model):

src/commons/error_response.py

@@ -23,6 +23,6 @@ def custom_exception_handler(exc, context):
     else:
         logger.error("An unhandled error occurred: %s", exc)
         logger.error("Context: %s", context)
-        return ErrorResponse(f"An unhandled error occurred: {exc}")
+        return ErrorResponse("Something went wrong on our end. Please try again later.")
 
     return response

src/tests/test_unhandled_exceptions.py

@@ -20,5 +20,5 @@ def test_custom_exception_handler(self):
         response = ExceptionView.as_view()(request)
 
         self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR)
-        self.assertEqual(response.data["error"], "An unhandled error occurred: This is a test exception")
         self.assertIn("error", response.data)
+        self.assertEqual(response.data["error"], "Something went wrong on our end. Please try again later.")