Skip to content

SF-3772 Prevent duplicate onboarding requests #3784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Nateowami merged 1 commit into from
Jun 3, 2026
Merged

SF-3772 Prevent duplicate onboarding requests #3784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...pp/serval-administration/onboarding-request-detail/onboarding-request-detail.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ import { NoticeComponent } from '../../shared/notice/notice.component';
import { projectLabel } from '../../shared/utils';
import { normalizeLanguageCodeToISO639_3 } from '../../translate/draft-generation/draft-utils';
import {
DraftingSignupFormData,
ONBOARDING_REQUEST_RESOLUTION_OPTIONS,
OnboardingRequest,
OnboardingRequestFormData,
OnboardingRequestResolutionKey,
OnboardingRequestResolutionMetadata,
OnboardingRequestService
Expand Down Expand Up @@ -258,7 +258,7 @@ export class OnboardingRequestDetailComponent extends DataLoadingComponent imple
return this.request?.resolution != null && this.request.resolution !== 'unresolved';
}

get formData(): DraftingSignupFormData {
get formData(): OnboardingRequestFormData {
return this.request!.submission.formData;
}

Expand Down
4 changes: 2 additions & 2 deletions ...orge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On
private readonly nllbService: NllbLanguageService,
protected readonly i18n: I18nService,
private readonly onlineStatusService: OnlineStatusService,
private readonly draftingSignupService: OnboardingRequestService,
private readonly onboardingRequestService: OnboardingRequestService,
protected readonly noticeService: NoticeService,
protected readonly urlService: ExternalUrlService,
protected readonly draftOptionsService: DraftOptionsService,
Expand Down Expand Up @@ -265,7 +265,7 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On
// Check if user has already submitted a signup for this project
if (this.activatedProject.projectId != null) {
try {
this.onboardingRequest = await this.draftingSignupService.getOpenOnboardingRequest(
this.onboardingRequest = await this.onboardingRequestService.getOpenOnboardingRequest(
this.activatedProject.projectId
);
} catch {
Expand Down
47 changes: 38 additions & 9 deletions ...p/src/app/translate/draft-generation/draft-signup-form/draft-onboarding-form.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Canon } from '@sillsdev/scripture';
import { User } from 'realtime-server/lib/esm/common/models/user';
import { ActivatedProjectService } from 'xforge-common/activated-project.service';
import { DataLoadingComponent } from 'xforge-common/data-loading-component';
import { DialogService } from 'xforge-common/dialog.service';
import { I18nService } from 'xforge-common/i18n.service';
import { NoticeService } from 'xforge-common/notice.service';
import { UserService } from 'xforge-common/user.service';
Expand All @@ -28,15 +29,15 @@ import { DevOnlyComponent } from '../../../shared/dev-only/dev-only.component';
import { JsonViewerComponent } from '../../../shared/json-viewer/json-viewer.component';
import { compareProjectsForSorting, projectLabel } from '../../../shared/utils';
import {
DraftingSignupFormData,
OnboardingRequestFormData,
OnboardingRequestService,
PARTNER_ORGANIZATION_OPTIONS,
PartnerOrganization
} from '../onboarding-request.service';

export const DRAFT_SIGNUP_RESPONSE_DAYS = { min: 1, max: 3 } as const;

type DraftOnboardingFormUiState = 'editing' | 'submitting' | 'submitted';
type OnboardingFormUiState = 'editing' | 'submitting' | 'submitted';

/**
* Component for the in-app draft signup form.
Expand Down Expand Up @@ -132,7 +133,7 @@ export class DraftOnboardingFormComponent extends DataLoadingComponent implement

submittedData?: any;

uiState: DraftOnboardingFormUiState = 'editing';
uiState: OnboardingFormUiState = 'editing';

readonly responseDays = DRAFT_SIGNUP_RESPONSE_DAYS;

Expand All @@ -141,8 +142,9 @@ export class DraftOnboardingFormComponent extends DataLoadingComponent implement
private readonly activatedProject: ActivatedProjectService,
private readonly userService: UserService,
private readonly paratextService: ParatextService,
private readonly draftingSignupService: OnboardingRequestService,
private readonly onboardingRequestService: OnboardingRequestService,
protected readonly noticeService: NoticeService,
protected readonly dialogService: DialogService,
private readonly destroyRef: DestroyRef,
private readonly cd: ChangeDetectorRef,
private readonly i18n: I18nService
Expand All @@ -153,6 +155,8 @@ export class DraftOnboardingFormComponent extends DataLoadingComponent implement
ngOnInit(): void {
this.loadingStarted();

void this.checkAndWarnIfAlreadySubmitted();

// Get the current user and pre-fill the form
this.userService
.getCurrentUser()
Expand Down Expand Up @@ -215,18 +219,23 @@ export class DraftOnboardingFormComponent extends DataLoadingComponent implement

async onSubmit(): Promise<void> {
const projectId = this.activatedProject.projectId;
if (projectId == null || this.uiState === 'submitting') {
return;
}
if (projectId == null || this.uiState === 'submitting') return;

if (await this.checkAndWarnIfAlreadySubmitted()) return;
Comment on lines 220 to +224

@devin-ai-integration devin-ai-integration Bot Apr 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Race condition double-click guard is effective but adds a redundant network call

The race condition guard at line 219 (if ((this.uiState as OnboardingFormUiState) === 'submitting') return;) correctly handles the double-click scenario. In JavaScript's single-threaded event loop: Click 1 awaits checkAndWarnIfAlreadySubmitted, then proceeds to set uiState = 'submitting'. Click 2, which also awaited checkAndWarnIfAlreadySubmitted concurrently, reaches line 219 and finds uiState is now 'submitting', so it exits. However, both clicks trigger a network call to getOpenOnboardingRequest, which is wasteful. A simpler approach would be to set a flag (or check uiState) before entering the async check, but the current approach is functionally correct.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@Nateowami Nateowami Apr 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Devin had a good point here, which I've addressed. However, note that the type system is overly optimistic and assumes the type of this.uiState can't be 'submitting' because we already added a guard for that. This is one of many instances where the type checker is overly permissive.

@marksvc marksvc Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting.


// Handle race condition when submit is double clicked and the second click happens before the form state is updated
// to 'submitting'. The type of this.uiState has to be widened after TS narrowed it to `"editing" | "submitted"`
// due to the check at the beginning of the function.
if ((this.uiState as OnboardingFormUiState) === 'submitting') return;

if (this.signupForm.valid === true) {
this.uiState = 'submitting';
this.cd.markForCheck();

const formData: DraftingSignupFormData = this.signupForm.getRawValue() as DraftingSignupFormData;
const formData = this.signupForm.getRawValue() as OnboardingRequestFormData;

try {
const requestId = await this.draftingSignupService.submitOnboardingRequest(projectId, formData);
const requestId = await this.onboardingRequestService.submitOnboardingRequest(projectId, formData);

// For testing purposes, store and display the submitted data
this.submittedData = { requestId, projectId, formData };
Expand Down Expand Up @@ -325,6 +334,26 @@ export class DraftOnboardingFormComponent extends DataLoadingComponent implement
}
}

/**
* If a request has already been submitted:
* - Informs user with a message dialog
* - Redirects to drafting page when user closes dialog
* - Resolves to true
*
* Otherwise, resolves to false
*/
private async checkAndWarnIfAlreadySubmitted(): Promise<boolean> {
if (this.activatedProject.projectId == null) return false;

if ((await this.onboardingRequestService.getOpenOnboardingRequest(this.activatedProject.projectId)) == null) {
return false;
} else {
await this.dialogService.message('draft_sources.request_already_submitted', undefined, true);
this.cancel();
return true;
}
}

private async loadProjectsAndResources(): Promise<void> {
try {
const [projects, resources] = await Promise.all([
Expand Down
13 changes: 3 additions & 10 deletions ...orge.Scripture/ClientApp/src/app/translate/draft-generation/onboarding-request.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@ import { CommandService } from 'xforge-common/command.service';
import { ONBOARDING_REQUESTS_URL } from 'xforge-common/url-constants';
import { SFProjectService } from '../../core/sf-project.service';

export interface OnboardingRequestComment {
id: string;
userId: string;
text: string;
dateCreated: string;
}

/** The available partner organization options for the onboarding form. */
export const PARTNER_ORGANIZATION_OPTIONS = ['Bolshoi Group', 'Seed Company', 'none'] as const;
/** A valid partner organization selection from the onboarding form. */
export type PartnerOrganization = (typeof PARTNER_ORGANIZATION_OPTIONS)[number];

export interface DraftingSignupFormData {
export interface OnboardingRequestFormData {
name: string;
email: string;
organization: string;
Expand Down Expand Up @@ -50,7 +43,7 @@ export interface OnboardingRequest {
projectId: string;
userId: string;
timestamp: string;
formData: DraftingSignupFormData;
formData: OnboardingRequestFormData;
};
assigneeId: string;
status: OnboardingRequestStatusOption;
Expand Down Expand Up @@ -119,7 +112,7 @@ export class OnboardingRequestService {
}

/** Submits a new onboarding request. */
async submitOnboardingRequest(projectId: string, formData: DraftingSignupFormData): Promise<string> {
async submitOnboardingRequest(projectId: string, formData: OnboardingRequestFormData): Promise<string> {
return (await this.onlineInvoke<string>('submitOnboardingRequest', { projectId, formData }))!;
}

Expand Down
1 change: 1 addition & 0 deletions src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@
"some_projects_use_back_translation": "Some projects get better results by adding a back translation as another reference.",
"source_language": "the source language",
"source_side_language_codes_differ": "All source and reference projects must be in the same language. Please select different source or reference projects.",
"request_already_submitted": "A request to activate drafting on this project has already been submitted. Please wait for a response from the team.",
"state_connecting": "Connecting",
"state_sync_failed": "There was an error syncing",
"state_sync_successful": "Sync successful",
Expand Down
Loading