Tuesday, January 30, 2024

Code refactor to simplify view classes

After having written a few view classes for endpoints, it is quite clear that there is way too much repetition. The reason for the repetition is the need to generate clear and different error messages from the backend so that the frontend will not have to do much. My first thought was that I would have to define a base view class that would define a wrapper method that would include these exception blocks and then all other view classes would inherit this base class. But this seemed like a very basic requirement that a million other Django developers might have wanted and so the chances that this would not somehow already be built into DRF seemed unlikely. After a little bit of googling, the answer was in the APIView class in DRF.

The GenericAPIView has only a few method such as get_queryset, get_serializer_class, get_object etc. What I was looking for was a base method similar to how the View class in Django had the dispatch method and a few others that were called under the hood. The GenericAPIView inherits the APIView class. The APIView class has a number of methods that get called under the hood, and one of them is the handle_exception method. The documentation says that any exception that is thrown by any handler method (get, post, patch etc) is passed to this method which either returns the appropriate Response or re-raises the exception if it can't handle it.

To fully appreciate what is going on, one really has to read the source code in Django Rest Framework. This is the second time I found myself browsing DRF source code and in my opinion, every one should regularly do so, as reading the source code gives you an understanding of DRF that is far deeper than just reading the documentation.

The handle_exception method is in the views.py file inside rest_framework folder of the Github repo:

def handle_exception(self, exc):
    """
    Handle any exception that occurs, by returning an appropriate response,
    or re-raising the error.
    """
    if isinstance(exc, (exceptions.NotAuthenticated,
                        exceptions.AuthenticationFailed)):
        # WWW-Authenticate header for 401 responses, else coerce to 403
        auth_header = self.get_authenticate_header(self.request)

        if auth_header:
            exc.auth_header = auth_header
        else:
            exc.status_code = status.HTTP_403_FORBIDDEN

    exception_handler = self.get_exception_handler()

    context = self.get_exception_handler_context()
    response = exception_handler(exc, context)

    if response is None:
        self.raise_uncaught_exception(exc)

    response.exception = True
    return response

There is a special handling for errors related to a user not being authenticated or authentication failed.  But the actual exception handling is a bit dynamic and the method is returned by the get_exception_handler method:

def get_exception_handler(self):
    """
    Returns the exception handler that this view uses.
    """
    return self.settings.EXCEPTION_HANDLER

The exception handler to be used is defined in the settings, and this is probably in case you want to choose a custom exception handler. But, the EXCEPTION_HANDLER is the settings is merely the method defined in the same file:

def exception_handler(exc, context):
    """
    Returns the response that should be used for any given exception.

    By default we handle the REST framework `APIException`, and also
    Django's built-in `Http404` and `PermissionDenied` exceptions.

    Any unhandled exceptions may return `None`, which will cause a 500 error
    to be raised.
    """
    if isinstance(exc, Http404):
        exc = exceptions.NotFound(*(exc.args))
    elif isinstance(exc, PermissionDenied):
        exc = exceptions.PermissionDenied(*(exc.args))

    if isinstance(exc, exceptions.APIException):
        headers = {}
        if getattr(exc, 'auth_header', None):
            headers['WWW-Authenticate'] = exc.auth_header
        if getattr(exc, 'wait', None):
            headers['Retry-After'] = '%d' % exc.wait

        if isinstance(exc.detail, (list, dict)):
            data = exc.detail
        else:
            data = {'detail': exc.detail}

        set_rollback()
        return Response(data, status=exc.status_code, headers=headers)

    return None

There is special handling for 404 and permission denied errors (403). But other than all it does it checks if the error is of type APIException.

class APIException(Exception):
    """
    Base class for REST framework exceptions.
    Subclasses should provide `.status_code` and `.default_detail` properties.
    """
    status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
    default_detail = _('A server error occurred.')
    default_code = 'error'

    def __init__(self, detail=None, code=None):
        if detail is None:
            detail = self.default_detail
        if code is None:
            code = self.default_code

        self.detail = _get_error_details(detail, code)

    def __str__(self):
        return str(self.detail)

    def get_codes(self):
        """
        Return only the code part of the error details.

        Eg. {"name": ["required"]}
        """
        return _get_codes(self.detail)

    def get_full_details(self):
        """
        Return both the message & code parts of the error details.

        Eg. {"name": [{"message": "This field is required.", "code": "required"}]}
        """
        return _get_full_details(self.detail)

So APIException inherits Exception but defines status_code and detail. The exception_handler method merely extracts the detail attribute along with the status_code in the APIException object and returns Response with the data being a dictionary containing the detail message. So, this exception_handler in reality is doing what I have been trying to do manually with different except blocks.

So, the first is to define a CustomAPIError that subclasses this APIException:

class CustomAPIError(APIException):
    def __init__(self, status_code, detail):
        self.status_code = status_code
        self.detail = detail

And all the other error classes like Http400Error etc can all be deleted. Where I was throwing these specific errors, I now throw the CustomAPIError. And the view classes can be much simpler, as once these errors are thrown, the handle_exception will call the exception_handler which will return the error Response. So, now the POST method handler for courses will be:

def post(self, request, *args, **kwargs):
    '''Create a new course - POST request'''
    self.authenticate(request)
    serializer = self.get_serializer(data=request.data)
    self.perform_create(serializer)
    return Response(
        serializer.data,
        status=status.HTTP_201_CREATED
    )

No need to enclose it in a try/except with different excepts throwing different errors - all taken care of by handle_exception. The only problem is that now logging is a bit broken. That will need that base class with a custom handle_exception method that uses logging.

Friday, January 26, 2024

Starting with the lecture app

A new app called lectures was created and the following very basic model just as a starting point:

class Lecture(models.Model):
    '''Lecture model'''

    course = models.ForeignKey(
        'courses.Course',
        models.SET_NULL,
        null=True
    )
    title = models.CharField(max_length=300)
    description = models.TextField(blank=True, null=True)
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

    def __str__(self):
        return self.title

Each lecture has to belong to a course, has to have a title and a description. For now, I have not created a ForeignKey pointing to User though it might be useful to know which instructor created the course and which one updated it. But, again, since this app is mainly for small-time instructors (such as myself) to host their courses rather than become a full-scale MOOC platform, there may not be that many instructors and so just logs might be good enough to keep track of what's going on. It can be added without too much fuss.

All URLs for lectures will be with respect to courses, and this implies, the structure will be /api/courses/<course-slug>/lectures/<lecture-urls>. For this reason, I did not add a placeholder for lectures in the main urls.py file, but rather inside the urls.py file of the courses app. In the urls.py file of the lectures app, I started off with a basic create view. Using the learning from the views in courses, I created a base view that inherits UserAuthentication and will later implement a get_object which will determine if the user has the authority to view the lecture.

class LectureBaseView(GenericAPIView, UserAuthentication):
    '''Basic lecture view'''

    serializer_class = LectureSerializer
    user_model = User
    lookup_field = 'id'


class LectureView(LectureBaseView, CreateModelMixin):
    '''Basic lecture view'''

    def get(self, request, *args, **kwargs):
        return Response('TODO')

    def post(self, request, *args, **kwargs):
        course_slug = self.kwargs.get('slug', None)
        try:
            course_obj = Course.objects.get_course_by_slug(course_slug)
            self.authenticate(request)
            serializer = LectureSerializer(data=request.data)
            serializer.save(
                user=self.request.user,
                course=course_obj
            )
            return Response(serializer.data)
        except Http400Error as e:
            return Response(
                data=str(e),
                status=status.HTTP_400_BAD_REQUEST
            )
        except Http403Error as e:
            return Response(
                data=str(e),
                status=status.HTTP_403_FORBIDDEN
            )
        except Http404Error as e:
            return Response(
                data=str(e),
                status=status.HTTP_404_NOT_FOUND
            )
        except Exception:
            return Response(
                data=DEFAULT_ERROR_RESPONSE,
                status=status.HTTP_400_BAD_REQUEST
            )

Though I am inheriting the CreateModelMixin, I am not really using it as I would like to throw custom exceptions rather than ValidationErrors by rest_framework. Maybe I will remove it later. But other than that the usual stuff, make sure user is authenticated and as an instructor (which means admin). Once course has been extracted from the slug and the user from the JWT token, the lecture can be created from the LectureSerializer:

class LectureSerializer(serializers.ModelSerializer):
    '''Serializer for Lecture model'''

    def save(self, *args, **kwargs):
        if self.is_valid():
            return super().save(*args, **kwargs)
        else:
            raise Http400Error(extract_serializer_error(self.errors))

    def check_user_is_instructor(self, course, user):
        if user is None:
            raise Http403Error(
                'Must be logged in as an instructor to create lectures'
            )
        if not course.check_user_is_instructor(user):
            raise Http403Error(
                'Must be an instructor of the course to create lectures'
            )
        return True

    def create(self, validated_data):
        user = validated_data.get('user', None)
        course = validated_data.get('course', None)
        if self.check_user_is_instructor(course, user):
            del validated_data['user']
            del validated_data['course']
            return Lecture.objects.create(
                **validated_data,
                course=course
            )

    class Meta:
        model = Lecture
        fields = ['title', 'description']
        extra_kwargs = {
            'title': {
                'error_messages': {
                    'required': 'The title of a lecture is required',
                    'blank': 'The title of a lecture is required'
                }
            }
        }

The create method checks if the user is an instructor and only then creates the course or else throws an exception.

Tried it out a bit through Postman and it works. A little more tweaking required to ensure that two lectures with the same title do not exist in the same course. That will probably a lecture manager method. Tests will make the code better.

Wednesday, January 24, 2024

Completing tests for basic course and user apps

Following the code refactor, I wrote tests for the three apps so far - user_auth, courses and registration. The tests cover almost all the non-trivial logic written so far, and so now the time has come to move on to the next part - course lectures.

A course can be created by an admin user and other admins can be added as instructors. Maybe, an option can be created for a teaching assistant at a later stage, in case an instructor does not want to add other co-instructors as admins but rather just other users as helpers. This I will figure out later along with the logic for deleting courses.

A course will have many lectures, so this is a simple case of a foreign key. The lectures can be video or audio or just text. Rather than having the content directly in the lectures, it might be better to create another table for that so that the content can be given metadata such as text translations for other languages or just merely audio in different languages.

In terms of a lecture, it should have a title, a number in a sequence, an optional description, content and optional attachments. Basic operations should be CRUD, except bulk delete should not be allowed in case a malicious instructor tries to delete a course. It should also be possible to change the sequence of lectures in a course. Of course, only an instructor should be able to perform any of these actions.

In terms of student registration, currently any student can register for any course paid or free. The payment processing will come later. In terms of student registrations, my plan is to allow any student to watch around 40% of a course for free and only then ask for payment for those courses which are paid. The reasoning - after watching 40% of the course, a student can judge whether the course is truly relevant and useful, and this way there will be no possibility of refunds later. The way to do this would be to update the CourseStudentRegistration model to include a paid field which can initially be False and will turn to True only when payment is processed. This can be used to check for authorization on whether a student can watch a particular video - if the lecture number is greater than the free quota, he will be asked to pay.

Thursday, January 18, 2024

Refactoring code

After having a few endpoints, before continuing, it might be best to stop and look at ways in which the code can be a bit more efficient with less repetition. For now, I refactored only the user_auth and courses endpoints, as these two had tests and the tests can be used to verify the code refactor.

To begin with, the view functions were handling too many exceptions. While it is necessary to produce explicit error messages along with appropriate status codes so that the frontend does not have to do much processing, I found several except blocks producing the same error. For example, in the user register view POST method:

def post(self, *args, **kwargs):
    '''Create new user from API request'''
    try:
        user = RegisterUserSerializer(data=self.request.data)
    except Exception as e:
        logger.critical(
            'Error in registering new user - {error}'.format(error=str(e))
        )
        return Response(
            data=DEFAULT_ERROR_RESPONSE,
            status=status.HTTP_400_BAD_REQUEST
        )
    new_user = None
    if user.is_valid():
        try:
            new_user = user.save()
            send_verification_link_email(new_user)
            logger.info('New user {} created'.format(new_user.username))
            return Response(user.data, status=status.HTTP_201_CREATED)
        except Http400Error as e:
            logger.error(
                'Error in registering new user {username} - {error}'.format(
                    username=user.data['username'],
                    error=str(e)
                )
            )
            return Response(
                data=str(e),
                status=status.HTTP_400_BAD_REQUEST
            )
        except Exception as e:
            logger.critical(
                'Error in registering new user - {error}'.format(
                    error=str(e)
                )
            )
            return Response(
                data=DEFAULT_ERROR_RESPONSE,
                status=status.HTTP_400_BAD_REQUEST
            )
    else:
        return serializer_error_response(user, 'Could not register user')

I am returning a 400 error response in four different places - the first in case the RegisterUserSerializer fails maybe due to corrupt request object, the second when there is a problem with the user model instance creation, the third is if there is an unexpected error and the last is if there is a serializer error such as missing username or password etc. All I need is two errors - the first is if there is a missing field or wrong API request and the second is if there is an unexpected error. 

So, a better way to write this would be:

def post(self, *args, **kwargs):
    '''Create new user from API request'''
    try:
        user = RegisterUserSerializer(data=self.request.data)
        new_user = user.save()
        send_verification_link_email(new_user)
        logger.info('New user {} created'.format(new_user.username))
        return Response(user.data, status=status.HTTP_201_CREATED)
    except Http400Error as e:
        logger.error(
            'Error in registering new user {}'.format(str(e))
        )
        return Response(
            data=str(e),
            status=status.HTTP_400_BAD_REQUEST
        )
    except Exception as e:
        logger.critical(
            'Error in registering new user - {}'.format(str(e))
        )
        return Response(
            data=DEFAULT_ERROR_RESPONSE,
            status=status.HTTP_400_BAD_REQUEST
        )

This would mean that some of the exception handling would move into the serializer as the .save() method which was returning a serializer error will now have to return a custom Http400Error. For this reason, the .save() method will need to be overloaded:

def save(self):
    if self.is_valid():
        return super().save()
    else:
        raise Http400Error(
            extract_serializer_error(self.errors)
        )

The is_valid() which was before in the view method is now in the serializer method which seems a better place for this to occur. A new method extract_serializer_error has to be defined:

def extract_serializer_error(error, default_msg=DEFAULT_ERROR_RESPONSE):
    '''Return error string as generic error'''
    try:
        err_message = [error[e][0]
                       for e in error][0]
    except:
        err_message = default_msg
    return err_message

It merely extracts the error string from the serializer Validation error.

In refactoring the Course view class, there was a greater deal of complication. In the create() method which I was using since I was inheriting the CreateAPIView:

def create(self, request, *args, **kwargs):
    '''Create a new course - POST request'''
    try:
        self.authenticate(request)
        return super().create(request, *args, **kwargs)
    except Http400Error as e:
        logger.error('Error creating course - {}'.format(str(e)))
        return Response(
            data=str(e),
            status=status.HTTP_400_BAD_REQUEST
        )
    except Http403Error as e:
        logger.critical('Course creation by non admin attempted')
        return Response(
            data=str(e),
            status=status.HTTP_403_FORBIDDEN
        )
    except ValidationError as e:
        logger.error('Error creating course - {}'.format(str(e)))
        return rest_framework_validation_error(e, 'Course could not be created')
    except Exception as e:
        logger.critical('Error creating course - {}'.format(str(e)))
        return Response(
            data=DEFAULT_ERROR_RESPONSE,
            status=status.HTTP_400_BAD_REQUEST
        )

Here, there is an unnecessary except block checking for ValidationError and which also returns a 400 error response. But, moving the validation into the overloaded save() method of the course serializer does not solve the problem:

def save(self, *args, **kwargs):
    if self.is_valid():
        return super().save(*args, **kwargs)
    else:
        raise Http400Error(extract_serializer_error(self.errors))

The reason this will not work is because of the way the create() method provided by the CreateAPIView works. Looking into the source code in the Django Rest Framework Github repository, I found that the create method is as:

def create(self, request, *args, **kwargs):
    serializer = self.get_serializer(data=request.data)
    serializer.is_valid(raise_exception=True)
    self.perform_create(serializer)
    headers = self.get_success_headers(serializer.data)
    return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

The create() method runs is_valid() under the hood and therefore will not wait for the overloaded save() method of the serializer to be called. The result is that exceptions will be of type serializers.ValidationError. The only way forward is to repeat some of the code:

def post(self, request, *args, **kwargs):
    '''Create a new course - POST request'''
    try:
        self.authenticate(request)
        serializer = self.get_serializer(data=request.data)
        self.perform_create(serializer)
        return Response(
            serializer.data,
            status=status.HTTP_201_CREATED
        )
    except Http400Error as e:
        logger.error('Error creating course - {}'.format(str(e)))
        return Response(
            data=str(e),
            status=status.HTTP_400_BAD_REQUEST
        )
    except Http403Error as e:
        logger.critical('Course creation by non admin attempted')
        return Response(
            data=str(e),
            status=status.HTTP_403_FORBIDDEN
        )
    except Exception as e:
        logger.critical('Error creating course - {}'.format(str(e)))
        return Response(
            data=DEFAULT_ERROR_RESPONSE,
            status=status.HTTP_400_BAD_REQUEST
        )

I explicitly create the serializer from the request and then call perform_create() which has not changed. Only, I do not call is_valid() and I let the perform_create() method call the save() method:

def perform_create(self, serializer):
    '''Create a new course with authenticated user as instructor'''
    course = serializer.save(user=self.request.user)
    logger.info(
        'Creating course {course} by user {user}'.format(
            course=course.title,
            user=self.request.user.username
        )
    )

With this refactor, the code repetitions have decreased. The tests are still passing which implies on the whole the code should still work.

Monday, January 15, 2024

Course registration for students and instructors

To complete the student registration endpoint, the add_students method was removed from the Course model as now an instance of the CourseStudentRegistration model needs to be created. To create this, I created a ModelManager for the CourseStudentRegistration model:

class CourseStudentRegistrationManager(models.Manager):
    '''Student registration manager'''

    def register_student(self, user, course):
        '''Register student for course'''

        register_obj = None
        try:
            register_obj = self.get(user=user, course=course)
        except:
            pass
        if register_obj is not None:
            raise CourseGenericError('User is already registered')

        return self.create(user=user, course=course)

Might seem like a roundabout way to do this, but I figure it is a direct query on the database. The get will throw an error if no object is returned and this error needs to be suppressed as that means the student has not yet registered for the course.

Besides this change, I changed the student registration into a POST request as it is not a modification of the Course model instance anymore. Moreover from a conceptual point of view, when a student registers for a course, it is a new create request and so POST seems best.

Following this, an endpoint is needed to add new instructors. The request is again a POST request and can be made only by someone who is already an instructor for the course.

class CourseInstructorAddView(GenericAPIView, UserAuthentication):
    '''Add an instructor to a course'''

    serializer_class = CourseSerializer
    lookup_field = 'slug'
    user_model = User

    def get_queryset(self, *args, **kwargs):
        '''Return published courses'''
        if self.request.user is not None and self.request.user.is_staff:
            return Course.objects.all()
        else:
            raise CourseForbiddenError('Must be logged in as an instructor')

    def post(self, request, *args, **kwargs):
        try:
            user = self.authenticate(request)
            course_obj = self.get_object()
            if user is not None and course_obj.check_user_is_instructor(user):
                new_user = User.objects.get_user_by_email(
                    request.data.get('email'))
                course_obj.add_instructor(new_user)
                return Response()
            else:
                return Response(
                    data='Must be logged in as an instructor',
                    status=status.HTTP_403_FORBIDDEN
                )
        except InvalidToken as e:
            return Response(
                data='Must be logged in as instructor',
                status=status.HTTP_403_FORBIDDEN
            )
        except CourseForbiddenError as e:
            return Response(
                data=str(e),
                status=status.HTTP_403_FORBIDDEN
            )
        except CourseGenericError as e:
            return Response(
                data=str(e),
                status=status.HTTP_400_BAD_REQUEST
            )
        except Exception as e:
            return Response(
                data=DEFAULT_ERROR_RESPONSE,
                status=status.HTTP_400_BAD_REQUEST
            )

The get_queryset() method will return courses only if the request comes from an admin. At this stage, no need to check for whether the user is an instructor as this is merely returning the superset against which the get_object filters with the slug field.

The add_instructor method in the Course model has to be modified:

def add_instructor(self, user):
    '''Add instructors to the course'''
    if self.check_user_is_instructor(user):
        raise CourseGenericError('Already an instructor')
    if user.is_staff:
        self.instructors.add(user)
    else:
        raise CourseForbiddenError('Instructors have to be administrators')

This is to ensure that the user has not already been added as instructor. At this point, I will keep the instructors field on the Course model as a simple ManyToManyField without a through table, as a course will not have a large number of instructors, and it does not seem very critical to keep track of when each instructor was added.

With this it seems like the basic User and Course model are now in working state. Before continuing, I need to address the repetition of code, especially in the view classes. On one hand, it is important to provide clear error messages and error status codes. However, instead of having GenericError and ForbiddenError exceptions for each app, it might be better to create one class of error definitions and use them all over the app. The idea is merely to provide a placeholder to determine the status code for a particular type of error and pass on the error message as it is. There will still be the last uncaught exception that returns the default error message of "Something unexpected happened. Please try again later."

Saturday, January 13, 2024

Registration app and model

After creating a basic student registration end-point, now expanding on this registration feature as registration will not just be the user but also time of registration, price paid etc. For now, let us just have the time of registration and handle the discount options later.

Since the registration may be a fairly complex logic at a later stage, I created another app called registration. Within this app, I created a model CourseStudentRegistration:

class CourseStudentRegistration(models.Model):
    '''Registration of a student in a course'''
    user = models.ForeignKey(
        'user_auth.User',
        null=True,
        on_delete=models.SET_NULL
    )
    course = models.ForeignKey(
        'courses.Course',
        null=True,
        on_delete=models.SET_NULL
    )
    registered_at = models.DateTimeField(auto_now_add=True)

Back to the Course model, the students field is changed to point to this model as a through table, but for now the instructors field is kept the same:

instructors = models.ManyToManyField(
    'user_auth.User', related_name='courses_taught')
students = models.ManyToManyField(
    'user_auth.User',
    blank=True,
    through='registration.CourseStudentRegistration'
)

At this point, there was a quite a lot of confusion. Before, all the models in the ForeignKey and ManyToMany fields were specified as Python objects rather than strings with the format '<app_label>.<model_name>'. So before, there was just User instead of 'user_auth.User' and CourseStudentRegistration rather than 'registration.CourseStudentRegistration'. But specifying Python objects results in circular dependency errors as we have a model that references another model which in turn references the original model. So the conflict is understandable.

But, quite strangely, getting the migrations right even with the above relative strings was a bit tricky and not much in the documentation to help. In the documentation, all models are in a single file and so running migrations works. For that matter, if I put the CourseStudentRegistration in the same file as Course, migrations worked. The problem comes with having them in different apps and files.

When there are multiple apps, one would think that migrations have to be run successively:

python manage.py makemigrations app1
python manage.py makemigrations app2
...

If I ran makemigrations on the three apps - user_auth, courses and registration - one after the other, I was getting circular dependency errors. Turns out when there are such interconnections between models, one should run all the migrations in a single command and let Django figure out how they are connected:

python manage.py makemigrations app1 app2 ...

After this, the circular dependencies were resolved and the server can be launched. The tests related to course registration failed because I had commented out the part related to adding students:

def add_students(self, user):
    '''Add students to the course'''
    # TODO - registering students logic will 
    # need to be separated from Course model
    # CourseStudentRegistration.objects.create(
    #     course=self,
    #     user=user
    # )
    pass
    # if user not in self.students.all():
    #     self.students.add(user)
    # else:
    #     raise CourseGenericError('User is already registered')

With a through table now being defined in the ManyToMany field, one cannot simply add the user, but rather a new CourseStudentRegistration model instance has to be created pointing to the Course and the User. This now implies that this method will go, as it makes little sense accessing another model manager method - CourseStudentRegistration.objects.create - from another model method. The sensible thing to do is to create a model Manager for the CourseStudentRegistration that can be called directly from the view class.

Thursday, January 11, 2024

Changes to student registration and database change

Testing the register-student API endpoint brought out a few changes and also made me think about how this registration needs to be handled to include a few more details.

To begin with, only a student logged in with an active account should be able to register for courses. So this means any API requests without credentials or with credentials of an inactive user should not be allowed. 

user = self.authenticate(request, check_admin=False)
if user is not None:
    course_obj = self.get_object()
    course_obj.add_students(user)
    return Response(
        data=CourseSerializer(
            user.course_set.all(),
            many=True
        ).data
    )
else:
    return Response(
        data='Must be logged in to register for course',
        status=status.HTTP_403_FORBIDDEN
    )

Before, the response was outside the if check whether user is not None. This would throw a 400 error as then I was accessing course_set of user which is None. The else condition handles this case specifically.

In the case of an inactive account, it is handled by the authenticate function which is an extension of the authenticate method provided by JWTAuthentication. If the user is inactive, it raises an exception. This exception seems generic and is difficult to catch in a specific except block. Rather than let the authenticate method raise an exception, for now, it seems better to let it return None if a valid user is not found.

def authenticate(self, request,
                check_admin=True,
                *args, **kwargs
    ):
    try:
        user = super().authenticate(
            request,
            *args,
            **kwargs
        )
        if user is not None:
            if check_admin and not user[0].is_staff:
                return None
            request.user = user[0]
            return user[0]
    except Exception as e:
        pass
    return None

For now, an exception is not thrown, but I could later throw a UserForbiddenException if necessary - something that can be explicitly caught in the view method.

Coming to the next topic - how should additional details be handled in student registration? These details include the time and date of registration, the price paid, the actual price (in case of discount applied) and also any association (such as a group in the case of group study). Django handles ManyToMany fields using a through table. Found a nice blog post on this:

https://www.sankalpjonna.com/learn-django/the-right-way-to-use-a-manytomanyfield-in-django 

All that needs to be done is to specify this through table explicitly with the additional fields. For now it will only be the time of registration and the price.

Tuesday, January 9, 2024

Endpoint for registering users for courses

Coming back to this project after a month as I get back from the holiday season. I left off where I had written the endpoint and the tests for publishing and unpublishing a course. The next endpoint will be to register a logged in student for the course.

I created a new class for this as for now I can't see how this endpoint can be multiplexed with another one in the manner in which CRU for the basic course creation was done. This endpoint will add the logged in user to the list of students in the course and then return all the courses which the student has registered so far. Conceptually, a logged in user will visit a course page, go through the overview and maybe watch a few videos, and if interesting will click on the register button. If the registration is successful, the user will get back a list of all the courses he has registered for maybe with this latest course first on the list.

The class is:

class CourseRegisterView(UpdateAPIView, UserAuthentication):
    '''
    Register a student for a course and
    return list of courses for the student.
    '''

    serializer_class = CourseSerializer
    lookup_field = 'slug'
    user_model = User

    def get_queryset(self, *args, **kwargs):
        '''Return published courses'''
        return Course.objects.fetch_courses()

    def partial_update(self, request, *args, **kwargs):
        try:
            user = self.authenticate(
            		request,
                        check_admin=False
                   )
            if user is not None:
                course_obj = self.get_object()
                course_obj.add_students(user)
            return Response(
                data=CourseSerializer(
                	user.course_set.all(),
                    	many=True
                     ).data
            )
        except CourseGenericError as e:
            return Response(
                data=str(e),
                status=status.HTTP_400_BAD_REQUEST
            )
        except InvalidToken as e:
            return Response(
                data='Must be logged in to register for course',
                status=status.HTTP_403_FORBIDDEN
            )
        except Exception as e:
            return Response(
                data=str(e),
                status=status.HTTP_400_BAD_REQUEST
            )

Some of it is a duplicate of the previous CourseView class. The first few properties of the class like serializer_class, lookup_field and user_model seem basic and so not sure if a base class can be created to not repeat these lines.

The get_queryset() method has been simplified to return only published courses as no one (admin or normal user) should be able to register for an unpublished course. The class uses the UpdateAPIView which is apt for editing a detail and provides a PUT and PATCH request. Theoretically, a register should be a POST, but then this would mean the extraction of the course object would need to be done manually. Since a user is being added to the "students" ManyToManyField, this can be interpreted as a PATCH request.

The UpdateAPIView handles a put or a patch request and provides an update() or partial_update method(). The UpdateModelMixin was used for the CourseView class as well. The logged in user is the user to be added to the students list. And so the view class inherits the UserAuthentication class which overloads the authenticate() method provided by JWTAuthentication.

To extract the course, I had initially overloaded the get_object() method. This get_object method is provided by GenericAPIView from which the UpdateAPIView inherits. This was my original overloaded method:

def get_object(self):
    try:
        return get_object_or_404(
            self.get_queryset(),
            slug=self.kwargs['slug']
        )
    except:
        raise Exception('Course not found')

Then I thought - what for? If the lookup_field has been specified as 'slug' and the get_queryset() method has been defined as all the published courses, the default get_object which filters the queryset with the value of the slug should be good enough. And so getting rid of this get_object() method makes no difference to the working of the code.

Trying to register a user for a course if the user has already been registered will through a 400 error. This can be displayed as a modal in the frontend. Will write tests for the this endpoint and check if any changes need to be made.