fix: create CEA object when enrolling using a license flow #18
Conversation
| @@ -2419,7 +2420,4 @@ def ensure_course_enrollment_is_allowed(course_id, email, enrollment_api_client) | |||
|
|
|||
| course_details = enrollment_api_client.get_course_details(course_id) | |||
| if course_details["invite_only"]: | |||
| CourseEnrollmentAllowed.objects.update_or_create( | |||
There was a problem hiding this comment.
I tried to re-use this helper as is, but it was locking a DB row causing enrollment to fail here, because the LMS was trying to modify the locked row. That's why I had to use this API to create CEA in a separate transaction. If this approach is viable, we'll have to backport this endpoint to Palm.
There was a problem hiding this comment.
@0x29a It's surprising that we haven't run into this race condition before. Any idea why it might be occuring now?
Irrespective of the reasons, I think using the API is a fine approach to take, given that we get the course details in the previous statement using the API and not an import. It looks like the enrollment_allowed is a recent API addition (added 6/7 months ago). We would have probably used it if it existed earlier, as this allows creating CEA objects from other services like an admin MFE as well.
There was a problem hiding this comment.
Thanks for checking this, @tecoholic.
Any idea why it might be occuring now?
This was bothering me as well, but I was unable to figure this out back then. It turns out that the view modified in openedx#1813 is not atomic. And the view modified in #14 doesn't send any internal HTTP requests in fact, so CEA objects are manipulated within a single transaction. In this PR we modify a view that is both atomic and sends a request to another atomic view, and they both try to modify CEA.
We can also change View with NonAtomicView here, and this will simplify things a bit, but I'm not sure how risky this is.
0x29a
force-pushed
the
0x29a/bb8808/cea-license-flow
branch
from
5927941
to
2381ecf
Compare
0x29a
force-pushed
the
0x29a/bb8808/cea-license-flow
branch
from
2381ecf
to
7d010cb
Compare
|
@0x29a I am merging this into the target branch to simplify the PRs.
|
tecoholic
merged commit d77191a
into
0x29a/bb8626/enroll-enterprise-learners-in-invite-only-courses
* fix: respect allow_enrollment_in_invite_only_courses Fixes `enroll_learners_in_courses` endpoint (and `enroll_subsidy_users_in_courses` utility function) to respect enterprise customer's `allow_enrollment_in_invite_only_courses` flag. * fix: create CEA object when enrolling using a license flow (#18)
This is a port of #19.