[HOLD for payment 2024-08-19] [$250] Xero - Onboarding modal shows up after refreshing page with 2FA setup prompt

[lanitochka17]

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.11-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• Account is connected to Xero
• 2FA is disabled after Xero is set up
• Expensifail account is used as it does not require onboarding

1. Go to staging.new.expensify.com
2. Log out if logged in
3. Log in to the account with Xero connected and 2FA disabled
4. After login, 2FA setup prompt will show up
5. Without setting up 2FA, refresh the page

Expected Result:

Onboarding modal will not show up after refreshing the page

Actual Result:

Onboarding modal shows up after refreshing the page

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6550548_1721742752377.20240723_203738.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0186166f403ad15af9
• Upwork Job ID: 1815869402034012020
• Last Price Increase: 2024-07-23

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 2

[mountiny]

This is most likely not a backend issue. cc @rushatgabhane. This seems to be mixing your changes with the ones from @filip-solecki and @WojtekBoman. Can you guys have a look, please?

[francoisl]

Either way, sounds like an edge case, going to demote to a daily issue for now.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0186166f403ad15af9

[melvin-bot]

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

[melvin-bot]

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

[tushar-a-b]

Proposal

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

Onboarding modal shows up along with 2FA setup prompt after logging in and refreshing the page when Xero is connected.

What is the root cause of that problem?

The root cause of this issue is the missing required nvp_onboarding key data for the logged-in user in ONYX. The absence of the nvp_onboarding key data in ONYX is because the server is not sending the nvp_onboarding data when Xero is connected and 2FA is disabled, which results in server's response with error that user need to complete their 2FA setup.

Explanation:

App/src/libs/Navigation/NavigationRoot.tsx

Lines 90 to 121 in 750c34e

	const initialState = useMemo(() => {
	if (!user || user.isFromPublicDomain) {
	return;
	}
	// If the user haven't completed the flow, we want to always redirect them to the onboarding flow.
	// We also make sure that the user is authenticated.
	if (!hasCompletedGuidedSetupFlow && authenticated) {
	const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT, linkingConfig.config);
	return adaptedState;
	}
	// If there is no lastVisitedPath, we can do early return. We won't modify the default behavior.
	if (!lastVisitedPath) {
	return undefined;
	}
	const path = initialUrl ? getPathFromURL(initialUrl) : null;
	// If the user opens the root of app "/" it will be parsed to empty string "".
	// If the path is defined and different that empty string we don't want to modify the default behavior.
	if (path) {
	return;
	}
	// Otherwise we want to redirect the user to the last visited path.
	const {adaptedState} = getAdaptedStateFromPath(lastVisitedPath, linkingConfig.config);
	return adaptedState;
	// The initialState value is relevant only on the first render.
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, []);

In the above code, we are calculating initialState to provide it to the NavigationContainer, which will redirect the user to the calculated initial route. Refer to the code below:

App/src/libs/Navigation/NavigationRoot.tsx

Lines 169 to 178 in 750c34e

	<NavigationContainer
	initialState={initialState}
	onStateChange={handleStateChange}
	onReady={onReady}
	theme={navigationTheme}
	ref={navigationRef}
	linking={linkingConfig}
	documentTitle={{
	enabled: false,
	}}

But the problem here is we are getting incorrect initialState, i.e. onboarding route, since below condition while calculating initialState is satisfied.

App/src/libs/Navigation/NavigationRoot.tsx

Lines 95 to 100 in 750c34e

	// If the user haven't completed the flow, we want to always redirect them to the onboarding flow.
	// We also make sure that the user is authenticated.
	if (!hasCompletedGuidedSetupFlow && authenticated) {
	const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT, linkingConfig.config);
	return adaptedState;
	}

In the above condition, we should get hasCompletedGuidedSetupFlow as true for users who have already completed the onboarding process. However, we end up getting false, which satisfies the condition and results in an incorrect initialState that is the onboarding route, thereby triggering the onboarding modal.

The reason for getting an incorrect hasCompletedGuidedSetupFlow value is that we do not have the nvp_onboarding key data in ONYX yet.

The main reason for not having nvp_onboarding data in ONYX is due to receiving an error from the server with code=666 and message=Your organization requires Two Factor Authentication. Go to your Inbox to get set up for the OpenApp API. This error occurs because of a pending 2FA setup, which is required when Xero is connected.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 257 to 264 in 7af1d37

	// If we are on this screen then we are "logged in", but the user might not have "just logged in". They could be reopening the app
	// or returning from background. If so, we'll assume they have some app data already and we can call reconnectApp() instead of openApp().
	if (SessionUtils.didUserLogInDuringSession()) {
	App.openApp();
	} else {
	Log.info('[AuthScreens] Sending ReconnectApp');
	App.reconnectApp(initialLastUpdateIDAppliedToClient);
	}

As per the above code, we call the OpenApp API to fetch the initial data of the app when the user successfully logs in. Let's take a look at the data received from the server through the OpenApp API in the scenarios where 2FA is enabled and disabled with Xero connected. Refer to the screenshots below for the server responses.

• API OpenApp response when 2FA disabled and Xero connected.

• API OpenApp response when 2FA enabled and Xero connected.

In the first screenshot, we can clearly notice that the server is responding with an error: when 2FA is disabled, we are receiving an empty onyxData, resulting in no required initial user data such as nvp_onboarding in ONYX on the client side.

On the other hand, in the second screenshot, when 2FA is enabled, we are receiving all the onyxData for the logged-in user, including the nvp_onboarding key, which is required in this case.

Since there is no nvp_onboarding in ONYX, we are incorrectly getting the value of hasCompletedGuidedSetupFlow as false here:

App/src/libs/Navigation/NavigationRoot.tsx

Lines 86 to 88 in b751d4e

	const [hasCompletedGuidedSetupFlow] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {
	selector: hasCompletedGuidedSetupFlowSelector,
	});

App/src/libs/hasCompletedGuidedSetupFlowSelector.ts

Lines 4 to 10 in b751d4e

	function hasCompletedGuidedSetupFlowSelector(onboarding: OnyxValue<typeof ONYXKEYS.NVP_ONBOARDING>): boolean {
	// onboarding is an array for old accounts and accounts created from olddot
	if (Array.isArray(onboarding)) {
	return true;
	}
	return onboarding?.hasCompletedGuidedSetupFlow ?? false;
	}

which will satisfies the below condition and result in opening of onboarding modal along with 2FA setup.

App/src/libs/Navigation/NavigationRoot.tsx

Lines 97 to 100 in b7dabee

	if (!hasCompletedGuidedSetupFlow && authenticated) {
	const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT.route, linkingConfig.config);
	return adaptedState;
	}

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

We can achieve the expected results by making changes either on the frontend or the server. This solution focuses on making changes on the frontend:

Since there is no availability of nvp_onboarding data in ONYX when 2FA is disabled with Xero, We can add check for shouldShowRequire2FAModal while calculating initialState and prevent onboarding when shouldShowRequire2FAModal is true.
We can update the condition from below in NavigationRoot.tsx

App/src/libs/Navigation/NavigationRoot.tsx

Lines 95 to 100 in e997502

	// If the user haven't completed the flow, we want to always redirect them to the onboarding flow.
	// We also make sure that the user is authenticated.
	if (!hasCompletedGuidedSetupFlow && authenticated) {
	const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT.route, linkingConfig.config);
	return adaptedState;
	}

to

if (!hasCompletedGuidedSetupFlow && authenticated && !shouldShowRequire2FAModal) {
    const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT, linkingConfig.config);
    return adaptedState;
}

We can get shouldShowRequire2FAModal as prop from parent component that is Expensify to child component NavigationRoot:

App/src/Expensify.tsx

Lines 107 to 114 in 67a3378

	const [shouldShowRequire2FAModal, setShouldShowRequire2FAModal] = useState(false);
	useEffect(() => {
	if (!account?.needsTwoFactorAuthSetup || account.requiresTwoFactorAuth) {
	return;
	}
	setShouldShowRequire2FAModal(true);
	}, [account?.needsTwoFactorAuthSetup, account?.requiresTwoFactorAuth]);

What alternative solutions did you explore? (Optional)

This solution focuses on making changes on the backend:
When user completes the authentication we are calling the API SigninUser which responds with authenticated user's data (Refer below screenshot). Here, in SigninUser API's response we should add additional data of nvp_onboarding so that we will have availability of nvp_onboarding data in ONYX, no need to rely on API OpenApp since it will not send the onyxData as 2FA is disabled.

Solution Conclusion

To address the root cause of the issue, I propose two potential solutions:

• Ensure the nvp_onboarding data is available to the application initially. This will prevent the incorrect calculation of the hasCompletedGuidedSetupFlow value, thereby avoiding unintended navigation to the onboarding route.

• Implement logic to prevent triggering the onboarding process when the 2FA prompt needs to be displayed to the user. This will ensure that users are correctly guided to complete their 2FA setup without encountering the onboarding flow.

By adopting either of these solutions, we can effectively resolve the issue and improve the user experience.

Results by making changes on frontend:

With.Undefined.mov

In the video above, we resolved the issue of the onboarding modal opening during the 2FA setup. However, this introduced a new problem where undefined is automatically appended to the URL. This causes the Hmm... Not Found view to appear in a report because the app treats undefined as a reportID. Consequently, no report is found since there is no report with an ID of undefined.

Note: Even after making changes to server and getting correct data for `nvp_onboarding` may not fix above issue unless instead of error, server responds with all the `onyxData` even if 2FA is disabled with `Xero` connected.

This issue is happing in below code:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 214 to 229 in 1908bd1

	let initialReportID: string | undefined;
	const isInitialRender = useRef(true);
	if (isInitialRender.current) {
	Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER);
	const currentURL = getCurrentUrl();
	if (currentURL) {
	initialReportID = new URL(currentURL).pathname.match(CONST.REGEX.REPORT_ID_FROM_PATH)?.at(1);
	}
	if (!initialReportID) {
	initialReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID)?.reportID;
	}
	isInitialRender.current = false;
	}

In above code we are trying to get the initialReportID to pass it to the below:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 487 to 498 in 1908bd1

	{Object.entries(CENTRAL_PANE_SCREENS).map(([screenName, componentGetter]) => {
	const centralPaneName = screenName as CentralPaneName;
	return (
	<RootStack.Screen
	key={centralPaneName}
	name={centralPaneName}
	initialParams={getCentralPaneScreenInitialParams(centralPaneName, initialReportID)}
	getComponent={componentGetter}
	options={CentralPaneScreenOptions}
	/>
	);
	})}

Since we do not have any :reportID to our url and also there is no LastAccessedReport data since we are just logged-in and due to required 2FA setup we are not getting any data for ONYX for initialization we will end up getting initialReportID as undefined below.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 220 to 226 in 1908bd1

	if (currentURL) {
	initialReportID = new URL(currentURL).pathname.match(CONST.REGEX.REPORT_ID_FROM_PATH)?.at(1);
	}
	if (!initialReportID) {
	initialReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID)?.reportID;
	}

Since we keep initialReportID as undefined below code leads us to /r/undefined.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 493 in 1908bd1

initialParams={getCentralPaneScreenInitialParams(centralPaneName, initialReportID)}

Solution for above issue is that we should not keep initialReportID as undefined instead we can set initialReportID="" when initialReportID is undefined by changing below code:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 224 to 226 in 1908bd1

	if (!initialReportID) {
	initialReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID)?.reportID;
	}

to

if (!initialReportID) {
   const initialReport = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID);
   initialReportID = initialReport?.reportID ?? "";
}

This keeps initialReportID as "" if it is undefined so now below code will lead us to the route /r instead of /r/undefined.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 493 in 1908bd1

initialParams={getCentralPaneScreenInitialParams(centralPaneName, initialReportID)}

• Final Result:

Fixed.Undefined.mov

[melvin-bot]

📣 @tushar-a-b! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[tushar-a-b]

Contributor details
Your Expensify account email: onmactab@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01c3775ee22f439793?mp_source=share

[tushar-a-b]

Thank you for assigning this job to me!

I have applied to the Upwork job, and you can expect the PR to be ready for review by 2nd of August.

[tushar-a-b]

@Julesssss I have applied for the job on Upwork but have not been accepted yet. Should I wait for the acceptance before starting to work on the PR, or should I begin working on it now? Additionally, is there a specific template for the PR that I should follow?

[fedirjh]

@tushar-a-b You should start to work on the PR. The payment will be handled later (7 days after PR is deployed to prod). You should follow these steps begin-coding-your-solution-in-a-pull-request to start your first pull request (make sure to sign your commits)

[tushar-a-b]

okay got it.
Thank you @fedirjh

[tushar-a-b]

PR is ready for review

[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]

⚠️ 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]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix: Onboarding modal shows up after refreshing page with 2FA setup when xero connected and 2FA disabled #46750

If no regressions arise, payment will be issued on 2024-08-19. 🎊

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment through NewDot Manual Requests
• @tushar-a-b requires payment (Needs manual offer from BZ)

[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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[fedirjh]

BugZero Checklist:

• Link to the PR: [Wave Collect][Xero] Enforce 2FA for xero #45702
• Link to comment: [Wave Collect][Xero] Enforce 2FA for xero #45702 (comment)
• Link to discussion: N/A, missed case, it should have been caught during testing
• Determine if we should create a regression test for this bug: N/A

---

Regression Test Proposal

Precondition:

- Account is connected to Xero
- 2FA is disabled after Xero is set up

1. Open app
2. Log out if logged in
3. Log in to the account with Xero connected and 2FA disabled
4. After login, 2FA setup prompt will show up
5. Without setting up 2FA, refresh the page (For native apps, close the application and reopen it)
6. Verify that the onboarding modal does not shows up behind the 2FA setup prompt

• Do we agree 👍 or 👎

[Julesssss]

@fedirjh sounds good to me

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @fedirjh owed $250 via NewDot
• ROLE: @tushar-a-b paid $250 via Upwork

BugZero Checklist (@lschurr)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1815869402034012020/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[lschurr]

@tushar-a-b - Offer sent in Upwork (https://www.upwork.com/nx/wm/offer/103591411)

[tushar-a-b]

@lschurr Thank you! I've accepted the offer👍🏻

[lschurr]

All set - Payment summary is correct: #46026 (comment)

[garrettmknight]

$250 approved for @fedirjh