REST API: "enrollment" vs. "enrollments", and how they deal with deleted courses

Development
1

Hi everyone,

following up on this, I poked around the Git history for two similarly named endpoints in the enrollment app’s REST API, namely

Apparently EnrollmentListView has been accessible via the REST API since 2015 (see PR 7193, submitted by @Diana_Huang and reviewed by Renzo Lucioni), whereas CourseEnrollmentsApiListView is more recent, from 2019 (PR 18838, submitted by @guruprasad and reviewed by @jill and @dave).

It looks as though the instruction “give me all enrollments of a specific user” can be expressed via a GET request to either endpoint, namely

  • api/enrollment/v1/enrollment?user=<username>
  • api/enrollment/v1/enrollments?username=<username>

These two endpoints behave quite differently if a user at some point enrolled in a course that was subsequently deleted from the CMS:

  • With the enrollment URL (using the older view), the deleted courses are simply omitted in the response. This is the behavior that I would intuitively consider “correct”.

  • With the enrollments URL, we get an HTTP 500, triggered by the CourseEnrollmentsApiListSerializer class:

Exception Type: AttributeError at /api/enrollment/v1/enrollments
Exception Value: Got AttributeError when attempting to get a value for field `course_id` on serializer `CourseEnrollmentsApiListSerializer`.
The serializer field might be named incorrectly and not match any attribute or key on the `CourseEnrollment` instance.
Original exception text was: 'NoneType' object has no attribute 'id'.

This appears to come from:

I suppose what this means is that since we don’t have a matching course (that course having been deleted), the course_overview object reference is None and hence we get the AttributeError. I think that the uncaught error might be fixed by simply adding required=False to the kwargs for CharField — but I’m by no means a DRF expert, so please take this with a big pinch of salt.

But I’d really like to understand the respective purposes of the enrollment and enrollments endpoints in the first place. Is one of them preferred over the other? Is or was one of them meant to replace the other at some point?

And also, is my assessment of what’s causing the HTTP 500 correct?

Thanks in advance for your thoughts!

2

Hello @fghaas

The difference I can see is that api/enrollment/v1/enrollments newer endpoint is capable of filtering enrollments by course_id and multiple usernames, which is geared towards Staff users.

The original endpoint is more restrictive in that aspect: it can get all enrollments for any user if the querying user is Staff, or the user making the request.

One use is exemplified in the PR that added that endpoint:

/api/enrollment/v1/enrollments?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course&username=audit,staff

This can be useful to fetch enrollments for multiple users at the same time or check if a given user is enrolled in a course without fetching all the enrollments for a user, for example.

On the 500 issue you mentioned, can you provide more information on how you triggered it? Information such as how you deleted the course and how the request was performed.

Cheers!

3

Hi @viadanna, thanks for the reply!

The difference I can see is that api/enrollment/v1/enrollments newer endpoint is capable of filtering enrollments by course_id and multiple usernames, which is geared towards Staff users.

Yes, I understand that. What I don’t quite understand is why the enrollments endpoint has a rather overly simplistic approach toward enrollment in deleted courses (it seems to pretend that deleted courses do not exist), whereas enrollment makes a pretty substantial effort for filtering those out.

On the 500 issue you mentioned, can you provide more information on how you triggered it? Information such as how you deleted the course and how the request was performed.

Sure. Here’s how you can reproduce this.

  1. Enroll a user in a course. Suppose their username is alex. It doesn’t matter how the enrollment actually happens: they could self-enroll, be enrolled by an instructor, be enrolled via this REST API or via the bulk enrollment API.
  2. Delete the course (manage.py cms delete_course <courseid>). Since this is a cms subcommand, it only concerns itself with deleting the course content, and doesn’t touch enrollments pertaining to that course. At least I think that’s the rationale.
  3. Then, try to fetch enrollments for that user, via the REST API. If you do it with enrollment?user=alex, what you’ll get is a list of enrollments for alex that excludes the deleted course. Use enrollments?username=alex (aside: why is the parameter named user for one view, username for the other?), and you get the 500 that I quoted in the original post.

Does this clarify things? And also, do you have thoughts on the idea of adding required=False to CourseEnrollmentsApiListSerializer’s course_id field?

One other observation on EnrollmentListView (i.e. the enrollment endpoint): unless I’m misunderstanding something, its docstring appears to be wrong and misleading:

            **POST Parameters**
              A POST request can include the following parameters.
              * user: Optional. The username of the currently logged in user.
                You cannot use the command to enroll a different user.

That appears to be at odds with this bit, from the post() method:

        username = request.data.get('user', request.user.username)

        # [...]

        has_api_key_permissions = self.has_api_key_permissions(request)

        # Check that the user specified is either the same user, or this is a server-to-server request.
        if not username:
            username = request.user.username
        if username != request.user.username and not has_api_key_permissions \
                and not GlobalStaff().has_user(request.user):
            # Return a 404 instead of a 403 (Unauthorized). If one user is looking up
            # other users, do not let them deduce the existence of an enrollment.
            return Response(status=status.HTTP_404_NOT_FOUND)

So, if a POST request comes properly authenticated with an API key, and it is linked to a user with the global Staff flag set, then the request should be able to enroll a different user, no?