Thursday, October 26, 2023

Code refactoring

With the code gradually expanding, it is necessary to stop once in a while and find ways to make the code more DRY and also follow the Model-View-Controller approach to building a web app. So, in terms of DRY, a few lines of code were being continuously repeated in the view method and therefore, it seemed like something that could be inserted into a function. The first is returning a HTTP error response when the serializer throws an error:

def serializer_error_response(serializer):
    '''Return 400 HTTP error if serializer has errors'''
    error_list = [serializer.errors[e][0].title()
                  for e in serializer.errors]
    return Response(
        data=error_list[0],
        status=status.HTTP_400_BAD_REQUEST
    )

The next is when a HTTP error response is returned when a JWT is either invalid or expired:

def token_error_response(token):
    '''Return 400 HTTP error if JWT has errors'''
    error_list = [token.errors[e][0].title()
                  for e in token.errors]
    return Response(
        data=error_list[0],
        status=status.HTTP_400_BAD_REQUEST
    )

The next is when the change password request is being processed. Initially, I had placed the code in the view function itself. Which is probably not the best place as what we are doing is an update of the existing User model instance. Therefore, the code for updating the password is better placed in the create() method of the ChangePasswordSerializer:

def update(self, instance, validated_data):
    '''Update user password'''
    if validated_data.get('password') is None:
        raise serializers.ValidationError('New password missing')
    if not instance.is_active:
        raise serializers.ValidationError('User not found')
    instance.set_password(validated_data.get('password'))
    instance.save()
    return instance

With that the code in the view ChangePasswordView becomes much simpler:

user_obj = User.objects.get_user_by_token(verification_token)
user_form = ChangePasswordSerializer(
    user_obj,
    data=self.request.data
)
# Check for password match
if not user_form.is_valid():
    return serializer_error_response(user_form)
else:
    user_form.save()

Another refactoring that was done is the extraction of the user object from the JWT. This was also done in the view function, but this would be better placed in queryset in a manager for the User model.

class UserManager(BaseUserManager):
    '''Manager for the User model'''

    def get_user_by_token(self, token, *args, **kwargs):
        '''Return a user object from JWT'''
        user_data = RefreshToken(token)
        try:
            user_obj = self.get_queryset().filter(
                id=int(user_data['user_id'])
            )[0]
        except:
            raise ValidationError('User not found')
        return user_obj

    def activate_user_by_token(self, token, *args, **kwargs):
        '''Activate a user from JWT'''
        try:
            user_obj = self.get_user_by_token(token)
            user_obj.is_active = True
            user_obj.save()
            return user_obj
        except:
            raise ValidationError(
            	'User could not be activated'
            )

There is a base query get_user_by_token which attempts to extract a user object from the JWT. This could fail for many reasons – the token is not genuine and does not result in a sensible user_data object or does not have the user_id claim. In this case, a User not found validation error will be generated. The other problem is that the user with that ID may not be found, which again will produce the same error.

The other query activate_user_by_token references the get_user_by_token query to fetch a user object and sets the is_active attribute to True. These two queries also make the view function cleaner as handling the database can now be moved into the manager which deals with the User table.

No comments:

Post a Comment