Skip to content

fix: resolved race condition for get certificate button#2409

Open
cap-code wants to merge 1 commit into
frappe:developfrom
cap-code:fix/certificate_race_condition
Open

fix: resolved race condition for get certificate button#2409
cap-code wants to merge 1 commit into
frappe:developfrom
cap-code:fix/certificate_race_condition

Conversation

@cap-code

Copy link
Copy Markdown
  1. Add server-side locking (SELECT ... FOR UPDATE) in create_certificate to serialize concurrent requests for the same user+course, preventing duplicate certificates from being created when the "Get Certificate" button is clicked rapidly

  2. Add a composite unique index on (member, course) via on_doctype_update as a database-level safety net, with a DuplicateEntryError handler that gracefully returns the existing certificate.

  3. Add a frontend loading guard and disable the "Get Certificate" button while a request is in flight to prevent unnecessary duplicate submissions.

FIXES #2408

@raizasafeel

Copy link
Copy Markdown
Contributor

Could you go through contribution guidelines? We want to ideally use direct sql as little as possible. Could you check out alternatives such as frappe.db.add_unique or simply look into frappe query builder?

For the duplicates: the real fix is a DB-level unique constraint on member, course (think frappe.db.add_unique()), since that's what actually stops two simultaneous request from both creating a certificate (an if exists check alone can't, because both requests pass the check before either inserts). Keep an if certificate exists, return it as a fast path, but also wrap the insert so that if the unique constraint trips on a race, you catch it and return the existing one instead of erroring.

We'll also need a data patch here, the unique constraint won't apply on sites that already have duplicate certificates. Since that one deletes data, I'm happy to add it as a co author once your part is in.

Also could you resolve the conflicts (with develop branch) and add a simple test (insert a certificate, attempt a second for the same member+course, assert it's rejected / count stays 1)?

Let me know if you need any help with it. Thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Race condition in create_certificate allows duplicate certificates per user per course

3 participants