Skip to content

fix: ignore a repeated submit of an already submitted attempt - #1365

Merged
feanil merged 2 commits into
openedx:masterfrom
mitodl:anas/idempotent-submit
Nov 6, 2026
Merged

feanil merged 2 commits into
openedx:masterfrom
mitodl:anas/idempotent-submit

Conversation

@Anas12091101

@Anas12091101 Anas12091101 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

Related: https://github.com/mitodl/hq/issues/14274

update_attempt_status treats submitted → submitted like a first submission: it moves completed_at to now, sets the credit requirement again, calls the backend's stop_exam_attempt and sends another "Proctoring attempt submitted" email. So every repeated submit request means another email to the learner.

Repeated submits happen in normal use: when the exam timer runs out, the server has usually already timed the attempt out (which submits it) by the time the browser's submit arrives, and every other tab showing the exam submits once more. Since frontend-lib-special-exams v5, a learner with a page of another course open when their time runs out sends that request several times a second until they close the tab (fixed separately in openedx/frontend-lib-special-exams#181). In production some learners got thousands of emails.

This PR makes update_attempt_status return early, logging it, when an attempt that is already submitted is submitted again. The request still answers with the attempt id. Nothing changes for any other transition. The one visible difference outside the bug: re-saving an already submitted attempt in Django admin no longer re-sends the email or moves completed_at.

hq-14274-expire-loop.mp4

Testing

Automated: pytest src/edx_proctoring/tests/test_email.py src/edx_proctoring/tests/test_views.py -k repeated_submit. Both fail on master (a second email, a moved completed_at) and pass here.

Manual:

  1. As a learner, start a proctored or timed exam and submit it (or let the time run out). In Django admin → Proctored exam student attempts, note the attempt's id and completed_at.
  2. In an LMS shell, submit it again:
    from edx_proctoring.api import update_attempt_status
    update_attempt_status(<ATTEMPT_ID>, 'submitted')
    → The LMS logs Attempt attempt_id=<ATTEMPT_ID> for exam_id=... is already submitted. Ignoring the repeated submission. and completed_at is unchanged. On master the same call moves completed_at and, for a proctored exam, sends a second "Proctoring attempt submitted" email.
  3. Optional, the browser loop: with a learning MFE that does not have frontend-lib-special-exams#181, follow that PR's manual steps (a timed exam whose time runs out on a page of another course). The browser keeps sending submit (277 times in about 35 seconds locally), and each one now logs "Ignoring the repeated submission". set_credit_requirement_status is logged only once, for the real submission.

Pre-Merge Checklist:

  • Confirmed Github reports all automated tests/checks are passing
  • Approved by at least one additional reviewer

update_attempt_status treated submitted -> submitted like a first
submission: it reset completed_at, set the credit requirement again,
called the provider's stop_exam_attempt and sent another "Proctoring
attempt submitted" email. A browser that repeats the submit request
(frontend-lib-special-exams v5 can loop on it) sent learners hundreds or
thousands of identical emails. Return early instead.
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @Anas12091101!

This repository is currently maintained by @openedx/openedx-unmaintained.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

…t id

Also return the attempt's own id from the early return, so callers get
the same integer as for any other transition.
@Anas12091101 Anas12091101 moved this from Needs Triage to Ready for Review in Contributions Nov 6, 2026
@Anas12091101
Anas12091101 requested a balanced review from Copilot November 6, 2026 13:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A provider timeout after the attempt is saved as submitted leaves submission side effects incomplete, while the new guard reports success on retries without reattempting the failed provider notification.

1 open issue
What changed in this PR

This PR makes repeated submissions of an already-submitted exam attempt return early, preventing duplicate learner emails and repeated submission side effects.

Changes:

  • Logs and ignores repeated submissions in update_attempt_status.
  • Adds a regression test checking that repeated submissions send only one email and preserve the completion timestamp.
File Description
src/​edx_proctoring/​tests/​test_email.py Tests email deduplication and completion-time preservation.
src/​edx_proctoring/​api.py Adds an early return for already-submitted attempts.

Review effort: Balanced

Comment thread src/edx_proctoring/api.py Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the change preventing duplicate exam-submission side effects.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@Anas12091101
Anas12091101 requested a review from feanil November 6, 2026 13:51
@feanil
feanil merged commit 082b0cc into openedx:master Nov 6, 2026
14 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Contributions Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants