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."
No comments:
Post a Comment