[Due for payment 2025-05-28] Ensure CompleteGuidedSetup is always called after ND onboarding redirects to OldDot

[chiragsalian]

Part of the New Dot Onboarding for 1-50 Companies project

Main issue: https://github.com/Expensify/Expensify/issues/476894
Doc section: N/A
Project: https://github.com/orgs/Expensify/projects/193/views/1

Steps

1. Sign into newDot with a new account.
2. Open network tab. Enable preserve log.
3. For onboarding select "manage my team", size "51-100", accounting package "None".
4. You'll be redirected to oldDot.

Expected Result:

The API command CompleteGuidedSetup should always be called.

Actual Result:

From the network log, after being redirected to oldDot the command CompleteGuidedSetup is not always called.
It seems like there is a race condition here becomes sometimes locally i see we're getting CompleteGuidedSetup called while sometimes we're not.

Solution

Investigate why CompleteGuidedSetup is not always called and ensure its always called after newDot onboarding regardless if we remain in newdot or route to oldDot.
The relevant code is here - https://github.com/Expensify/App/blob/main/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx#L216.

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @RachCHopkins

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

[melvin-bot]

Triggered auto assignment to @RachCHopkins (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Ensure CompleteGuidedSetup is always called after ND onboarding redirects to OldDot

What is the root cause of that problem?

completeOnboarding is asynchronous (it makes an API call), but we're not waiting for it to complete before potentially redirecting to OldDot. This can cause a race condition where the redirect happens before the CompleteGuidedSetup command is processed.

App/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx

Lines 216 to 232 in 1af42e2

	completeOnboarding({
	engagementChoice: onboardingPurposeSelected,
	onboardingMessage: CONST.ONBOARDING_MESSAGES[onboardingPurposeSelected],
	adminsChatReportID,
	onboardingPolicyID: policyID,
	companySize: onboardingCompanySize,
	userReportedIntegration,
	});
	const isSupportedIntegration = supportedIntegrationsInNewDot.includes(userReportedIntegration) || userReportedIntegration === null;
	const shouldRedirectToOldDot = getPlatform() !== CONST.PLATFORM.DESKTOP && (!isSupportedIntegration || onboardingCompanySize !== CONST.ONBOARDING_COMPANY_SIZE.MICRO);
	if (shouldRedirectToOldDot) {
	if (CONFIG.IS_HYBRID_APP) {
	return;
	}
	openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);
	}

What changes do you think we should make in order to solve the problem?

We should add InteractionManager.runAfterInteractions to wrap openOldDotLink

            InteractionManager.runAfterInteractions(() => {
                openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);
            });

App/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx

Lines 227 to 232 in 1af42e2

	if (shouldRedirectToOldDot) {
	if (CONFIG.IS_HYBRID_APP) {
	return;
	}
	openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);
	}

OR we can create a function waitForUserCompleteOnboarding like we did with waitForUserSignIn to make sure that hasCompletedGuidedSetupFlow in ONYXKEYS.NVP_ONBOARDING is true

    function waitForUserCompleteOnboarding(): Promise<boolean> {
        return new Promise<boolean>((resolve) => {
            if (onboardingComplete) {
                resolve(true);
            } else {
                onboardingCompletePromiseResolver = resolve;
            }
        });
    }

App/src/libs/actions/Session/index.ts

Lines 1186 to 1207 in bfc4b86

	/**
	* Waits for a user to sign in.
	*
	* If the user is already signed in (`authToken` is truthy), the promise resolves immediately.
	* Otherwise, the promise will resolve when the `authToken` in `ONYXKEYS.SESSION` becomes truthy via the Onyx callback.
	* The promise will not reject on failed login attempt.
	*
	* @returns A promise that resolves to `true` once the user is signed in.
	* @example
	* waitForUserSignIn().then(() => {
	* console.log('User is signed in!');
	* });
	*/
	function waitForUserSignIn(): Promise<boolean> {
	return new Promise<boolean>((resolve) => {
	if (session.authToken) {
	resolve(true);
	} else {
	authPromiseResolver = resolve;
	}
	});
	}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[thelullabyy]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• Ensure that CompleteGuidedSetup is always called before navigation to OldDot after ND onboarding redirects.

What is the root cause of that problem?

• Before reaching the following code:
  

  App/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx

  Line 231 in 23537ae

  openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);

  
  there are two persisted requests queued: ["CreateWorkspace", "CompleteGuidedSetup"]. Meanwhile, immediately after the OpenOldDotLink API call succeeds, the logic to navigate to OldDot is triggered:
  

  App/src/libs/actions/Link.ts

  Lines 71 to 79 in 23537ae

  	asyncOpenURL(
  	// eslint-disable-next-line rulesdir/no-api-side-effects-method
  	API.makeRequestWithSideEffects(SIDE_EFFECT_REQUEST_COMMANDS.OPEN_OLD_DOT_LINK, {}, {})
  	.then((response) => (response ? buildOldDotURL(url, response.shortLivedAuthToken) : buildOldDotURL(url)))
  	.catch(() => buildOldDotURL(url)),
  	(oldDotURL) => oldDotURL,
  	undefined,
  	shouldOpenInSameTab,
  	);

• This creates a race condition where the navigation to OldDot may occur before all persisted requests, specifically CompleteGuidedSetup, have completed successfully.

What changes do you think we should make in order to solve the problem?

• We need to ensure that all persisted requests are completed before initiating the OldDot navigation logic. To achieve this, we can update the logic at the following line:
  

  App/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx

  Line 231 in 23537ae

  openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);

• Proposed change:

SequentialQueue.waitForIdle().then(() => {
    openOldDotLink(CONST.OLDDOT_URLS.INBOX, true);
});

• SequentialQueue.waitForIdle() ensures all persisted actions, including CompleteGuidedSetup, have completed before proceeding.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

• N/A

What alternative solutions did you explore? (Optional)

[chiragsalian]

@fedirjh, any update here? Can you look at the proposals when you get a chance. I'd like to have this issue resolved soon if possible 🙂