Problem/Motivation
Paid checkout can abort with a fatal PDOException from drupal_write_record() when a course_enrollment (nid_uid) or
course_outline_fulfillment (coid_uid) row already
exists for the user.
Two distinct causes:
course_enroll()guards its insert withcourse_enrollment_check(), which only matches active (status = 1) rows. The{course_enrollment}nid_uid unique key, however, ignores status.
A pre-existing inactive row (for example a status = 0course_relationshipsenrollment) slips past the guard and collides on insert.CourseObjectFulfillment::getFulfillment()does a non-atomic load-or-create. A concurrent request or a double-clicked checkout submit can insert the row between the load and the save,
so the second save
collides on the coid_uid unique key.CourseEnrollment::save()has the same race on nid_uid.
In all cases the uncaught PDOException is fatal and aborts the checkout the user has already paid for.
Steps to reproduce
- Have a course product that enrolls the user on purchase.
- Ensure a pre-existing inactive
{course_enrollment}row exists for the user/node (or double-submit the checkout to trigger the concurrent race). - Complete paid checkout.
- Observe a fatal
PDOException(integrity constraint violation, duplicate entry for the nid_uid / coid_uid unique key) and an aborted checkout.
Proposed resolution
- Detect a pre-existing enrollment with the status-agnostic
course_enrollment_load()rather than the status-onlycourse_enrollment_check(), socourse_enroll()no longer walks into a collision on the nid_uid unique key. - Where that row is a placeholder - one a module created at status 0 to record a relationship without granting access - activate it, since the unique key means no second row can be inserted. Record the method and code
the caller passed, so the row stops describing how the placeholder came to exist rather than how the user enrolled. - Do not confuse a placeholder with an enrollment an administrator deliberately switched off through the Edit enrollment bulk operation, which sets status and leaves
enrollmenttypealone. Nothing stored on the row separates the two, and the convention that does belongs to whichever module invented it, so addhook_course_enrollment_is_placeholder()and let modules claim their own rows. An unclaimed inactive row is left alone and the refusal is logged as a warning. See #3618084 for the Course
relationships implementation. - Refuse before money moves. Declining inside
course_enroll()is not enough on its own: checkout still completed and charged the card, the access code form still printed "registered",
and the admin Enroll user action still said "Enrolled X in Y". Block theenrolloperation while a deactivated row exists, which closes every self-service route at once because they all
consult enroll access -course_uc_form_alter()hides the add to cart form,course_uc_init()drops the product from a cart it is already in, andcourse_relationships_bulk_add_to_cart()refuses the set. The access code form checks explicitly, since it never consulted access. Administrative enrollment is untouched, so Edit
enrollment remains the way to restore someone. - In
CourseEnrollment::save()andCourseObjectFulfillment::save(), catch a duplicate-keyPDOException(SQLSTATE
23000), adopt the existing row (preserving the fulfillment uuid, the enrollment's lifecycle dates and a NULLenroll_end), and re-save as an update. This covers the concurrent /
double-submit race that a pre-check cannot.
The happy path is unchanged; recovery only runs on an actual collision.
Remaining tasks
- Review.
- Confirm the SQLSTATE 23000 catch is appropriate across supported database drivers.
User interface changes
- A user whose enrollment was deactivated now sees "Your enrollment in this course is not active. Please contact us if you need your access restored." in place of an enroll or purchase option, rather than being allowed
to pay for access they will not receive. - Redeeming an access code for such a course fails validation with the same message, instead of reporting a successful registration.
- The admin Enroll user operation shows a warning naming the Edit enrollment operation when it declines, instead of reporting an enrollment that did not happen.
API changes
- New hook:
hook_course_enrollment_is_placeholder(). A module that creates enrollments at status 0 to record a relationship should implement it and return TRUE for its own rows. Only
inactive enrollments are passed to it. A module that does not implement it will find its inactive rows treated as deactivated and never activated. - New function:
course_enrollment_is_placeholder($enrollment), which asks the above and returns FALSE for anything unclaimed. - New enroll blocker key
course_enrollment_inactive, returned fromhook_course_access()for theenrollop. Anything
inspecting enroll blockers by key can match on it. course_enroll()now returns the stored row untouched when a deactivated enrollment already exists. Getting an enrollment back has never guaranteed the user is enrolled; callers that
report success to the user should checkstatuson the returned object.CourseEnrollment::$eidis now declared on the class, so the duplicate-key recovery no longer creates a dynamic property (deprecated as of PHP 8.2).
Data model changes
None. No schema change and no hook_update_N.
Issue fork course-3612572
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
kyoder commentedComment #4
kyoder commented