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:

  1. course_enroll() guards its insert with course_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 = 0 course_relationships enrollment) slips past the guard and collides on insert.
  2. 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

  1. Have a course product that enrolls the user on purchase.
  2. Ensure a pre-existing inactive {course_enrollment} row exists for the user/node (or double-submit the checkout to trigger the concurrent race).
  3. Complete paid checkout.
  4. 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-only course_enrollment_check(), so course_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 enrollmenttype alone. Nothing stored on the row separates the two, and the convention that does belongs to whichever module invented it, so add hook_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 the enroll operation 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, and course_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() and CourseObjectFulfillment::save(), catch a duplicate-key PDOException (SQLSTATE
    23000), adopt the existing row (preserving the fulfillment uuid, the enrollment's lifecycle dates and a NULL enroll_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 from hook_course_access() for the enroll op. 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 check status on the returned object.
  • CourseEnrollment::$eid is 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

Command icon 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

kyoder created an issue. See original summary.

kyoder’s picture

kyoder’s picture

Issue summary: View changes