[HOLD for payment 2024-10-30] [HOLD for payment 2024-10-29] Onboarding - Onboarding does not show up when creating new account for the second time

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.51-1
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh081006@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com on incognito mode.
2. Sign up with a new Gmail account.
3. Complete the onboarding.
4. Log out.
5. Sign up with another new Gmail account.

Expected Result:

Onboarding will show up for every new account created (production behavior).

Actual Result:

Onboarding does not show up for the second new Gmail account.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Uploading Bug6639192_1729287472033!20241019_053558.mp4…

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

Triggered auto assignment to @abekkala (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.

[melvin-bot]

Triggered auto assignment to @Beamanator (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[Kalydosos]

Edited by proposal-police: This proposal was edited at 2024-10-20 20:49:16 UTC.

Proposal

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

Onboarding does not show up when creating new account for the second time

What is the root cause of that problem?

when signing up for the second account, the fetch statuses of onyx keys nvp_onboarding, nvp_trynewdot and is_single_new_dot_entry are "loading" even though their values are undefined. The keys nvp_onboarding, nvp_trynewdot values are updated to false in the signup process but the key is_single_new_dot_entry is not updated which leads to its fetch status to be locked at "loading". That affects the triggering of the onboarding process which is stuck in the line below

App/src/hooks/useOnboardingFlow.ts

Lines 27 to 29 in d6f883a

	if (isLoadingOnyxValue(isOnboardingCompletedMetadata, isHybridAppOnboardingCompletedMetadata, isSingleNewDotEntryMetadata)) {
	return;
	}

as isSingleNewDotEntryMetadata status keep returning "loading"

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

the onyx key is_single_new_dot_entry value is used for NativeModules.HybridAppModule, see line below

App/src/hooks/useOnboardingFlow.ts

Lines 33 to 35 in d6f883a

	if (isHybridAppOnboardingCompleted === false && !isSingleNewDotEntry) {
	Navigation.navigate(ROUTES.EXPLANATION_MODAL_ROOT);
	}

we will consider the onyx key only in the context of NativeModules.HybridAppModule for the onboarding process (that key fetch status is always "loaded" during the first account creation) by changing the lines

App/src/hooks/useOnboardingFlow.ts

Lines 27 to 29 in d6f883a

	if (isLoadingOnyxValue(isOnboardingCompletedMetadata, isHybridAppOnboardingCompletedMetadata, isSingleNewDotEntryMetadata)) {
	return;
	}

into this

if (isLoadingOnyxValue(isOnboardingCompletedMetadata, isHybridAppOnboardingCompletedMetadata, (isSingleNewDotEntry !== undefined ||NativeModules.HybridAppModule)?isSingleNewDotEntryMetadata:{status :'loaded' as FetchStatus} as ResultMetadata)) {
return;
}

RESULT

it works like a charm

2024-10-20.20-48-33.mp4

What alternative solutions did you explore? (Optional)

we could also change the lines

App/src/hooks/useOnboardingFlow.ts

Lines 27 to 29 in d6f883a

	if (isLoadingOnyxValue(isOnboardingCompletedMetadata, isHybridAppOnboardingCompletedMetadata, isSingleNewDotEntryMetadata)) {
	return;
	}

into this

if (isLoadingOnyxValue(isOnboardingCompletedMetadata, isHybridAppOnboardingCompletedMetadata, NativeModules.HybridAppModule?isSingleNewDotEntryMetadata:{status :'loaded' as FetchStatus} as ResultMetadata)) {
return;
}

as a shorter version of the main changes proposed but i think the main changes taking extra precautions (isSingleNewDotEntry !== undefined ||NativeModules.HybridAppModule) is a good thing

[Beamanator]

@Kalydosos - thanks for your proposal! Since this is a Deploy Blocker, we also want to figure out where this came from (since this is most likely due to a very recent PR). Can you help find which PR caused this?

[Beamanator]

I will test to see if this has to do with #49602

[Beamanator]

Tested & revert did fix this issue - i'll move forward with the revert - @Kalydosos i'll try to get you compensated for the help since you pointed me to the file where the issue was, which made it very easy to find the problem PR 🙏

[Kalydosos]

@Beamanator thx, i would love that indeed 👍

[Beamanator]

@Beamanator it seems to be this PR #50015

@Kalydosos the reason I don't think it's that PR is b/c that PR was deployed to production 3 weeks ago, whereas the current bug in this issue doesn't exist in production yet... Though it is possible that recent changes showed this bug, which was caused by that PR!

[Kalydosos]

@Beamanator it seems to be this PR #50015

@Kalydosos the reason I don't think it's that PR is b/c that PR was deployed to production 3 weeks ago, whereas the current bug in this issue doesn't exist in production yet... Though it is possible that recent changes showed this bug, which was caused by that PR!

@Beamanator yes you are right, that was not the offending PR indeed, it is missing a part in the test.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.51-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Revert "[HybridApp] Allow classic experience users to use NewDot travel page" #51155

If no regressions arise, payment will be issued on 2024-10-29. 🎊

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@Beamanator] The PR that introduced the bug has been identified. Link to the PR:
• [@Beamanator] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@Beamanator] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@Beamanator] Determine if we should create a regression test for this bug.
• [@Beamanator] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.52-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Revert "[HybridApp] Allow classic experience users to use NewDot travel page" #51155

If no regressions arise, payment will be issued on 2024-10-30. 🎊

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@Beamanator] The PR that introduced the bug has been identified. Link to the PR:
• [@Beamanator] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@Beamanator] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@Beamanator] Determine if we should create a regression test for this bug.
• [@Beamanator] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: