Skip to content

SF-3772 Prevent duplicate onboarding requests#3784

Open
Nateowami wants to merge 1 commit intomasterfrom
feature/SF-3772-prevent-duplicate-onboarding-requests
Open

SF-3772 Prevent duplicate onboarding requests#3784
Nateowami wants to merge 1 commit intomasterfrom
feature/SF-3772-prevent-duplicate-onboarding-requests

Conversation

@Nateowami
Copy link
Copy Markdown
Collaborator

@Nateowami Nateowami commented Apr 8, 2026

Before this change, the UI already informs the user when a request has already been submitted and doesn't allow submitting another. However, the check is only done when the drafting component loads, so by using multiple tabs, or possibly by using browser history, it's possible to submit the form twice. This change adds additional checks to prevent duplicate submissions.

EDIT: I think the actual reason we were getting duplicate requests was network errors when submitting the form:
Screenshot from 2026-04-08 12-27-44

The request did however get saved, but the client didn't know it, so the user was able to re-submit.

EDIT 2: I think the network errors are timeouts. I've created a PR to fix them: #3785. However, I think this PR is still a good change.


Open with Devin

This change is Reviewable

@Nateowami Nateowami added the will require testing PR should not be merged until testers confirm testing is complete label Apr 8, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 13.33333% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.32%. Comparing base (797889b) to head (dec8b56).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...aft-signup-form/draft-onboarding-form.component.ts 0.00% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3784      +/-   ##
==========================================
- Coverage   81.34%   81.32%   -0.02%     
==========================================
  Files         622      622              
  Lines       39202    39210       +8     
  Branches     6401     6403       +2     
==========================================
  Hits        31889    31889              
- Misses       6326     6334       +8     
  Partials      987      987              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nateowami Nateowami force-pushed the feature/SF-3772-prevent-duplicate-onboarding-requests branch from 1c4ac76 to dec8b56 Compare April 8, 2026 14:39
@marksvc marksvc self-assigned this Apr 10, 2026
Copy link
Copy Markdown
Collaborator

@marksvc marksvc left a comment

Choose a reason for hiding this comment

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

@marksvc reviewed 5 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on Nateowami).


src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 406 at r2 (raw file):
Hmm. If they hear back from the team, can they submit another request?

If not, perhaps this might say

A request to activate drafting on this project has already been submitted. Thank you for your patience in waiting for a response from the team.

Also, we might consider mentioning where they should be looking for a response. An email? A new ability/button present on the Draft Generation page?


src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-signup-form/draft-onboarding-form.component.ts line 373 at r2 (raw file):

  private async hasRequestAlreadyBeenSubmitted(): Promise<boolean> {
    return (await this.onboardingRequestService.getOpenOnboardingRequest(this.activatedProject.projectId!)) != null;

I think we will do ourselves good to continue to let the type system help us. Rather than introduce the non null assertion operator here, what do you think about using

    if (this.activatedProject.projectId == null) return false;
    return (await this.onboardingRequestService.getOpenOnboardingRequest(this.activatedProject.projectId)) != null;

or

    const projectId: string | undefined = this.activatedProject.projectId;
    return (projectId == null || (await this.onboardingRequestService.getOpenOnboardingRequest(projectId))) != null;

?

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

Labels

will require testing PR should not be merged until testers confirm testing is complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants