Skip to content

Commit c491e7e

Browse files
MelvinBotmkhutornyi
andcommitted
Use route truthiness check instead of hasAuthToken to fix both issues
The hasAuthToken() guard didn't fix Expensify#90880 because the real problem is navigating to an empty route ('') when the user visits the root URL and signs in. After login, hasAuthToken() returns true, and the selector returns true for empty onboarding objects, so Navigation.navigate('') fires and shows "not found". The fix checks that route is truthy (non-empty) before allowing deeplink navigation for non-anonymous users. This prevents empty-route navigation (fixing Expensify#90880) while preserving deeplink-then-login flows like /concierge (preserving the Expensify#82818 fix from PR Expensify#85740). Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
1 parent c164857 commit c491e7e

2 files changed

Lines changed: 18 additions & 12 deletions

File tree

src/libs/actions/Link.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import SCREENS from '@src/SCREENS';
3232
import {hasCompletedGuidedSetupFlowSelector} from '@src/selectors/Onboarding';
3333
import type {Beta, IntroSelected, Report} from '@src/types/onyx';
3434
import {doneCheckingPublicRoom, navigateToConciergeChat, openReport} from './Report';
35-
import {canAnonymousUserAccessRoute, hasAuthToken, isAnonymousUser, signOutAndRedirectToSignIn, waitForUserSignIn} from './Session';
35+
import {canAnonymousUserAccessRoute, isAnonymousUser, signOutAndRedirectToSignIn, waitForUserSignIn} from './Session';
3636
import {setOnboardingErrorMessage} from './Welcome';
3737

3838
let currentUserEmail = '';
@@ -364,11 +364,11 @@ function openReportFromDeepLink(
364364
}
365365
};
366366

367-
// We must check hasAuthToken() dynamically (not the stale `isAuthenticated` closure value)
368-
// before hasCompletedGuidedSetupFlowSelector because the selector returns true for empty
369-
// onboarding objects (the pre-login default state), which would cause premature deeplink
370-
// navigation before authentication completes.
371-
if ((hasAuthToken() && hasCompletedGuidedSetupFlowSelector(val)) || isAnonymousUser()) {
367+
// Guard deeplink navigation with a route truthiness check to prevent navigating to an
368+
// empty route when the user visits the root URL (e.g. new.expensify.com) and then signs in.
369+
// Without this, hasCompletedGuidedSetupFlowSelector returns true for empty onboarding objects
370+
// and Navigation.navigate('') fires, showing a "not found" page.
371+
if ((route && hasCompletedGuidedSetupFlowSelector(val)) || isAnonymousUser()) {
372372
handleDeeplinkNavigation();
373373
}
374374
});

tests/unit/OnboardingSelectorsTest.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,24 @@ describe('onboardingSelectors', () => {
88
// the onboarding flow is only showed to the users with `hasCompletedGuidedSetupFlow` set to false
99
describe('hasCompletedGuidedSetupFlowSelector', () => {
1010
// Regression test: hasCompletedGuidedSetupFlowSelector returns true for empty onboarding objects (the pre-login default state).
11-
// The deeplink guard in Link.ts must combine this with a dynamic hasAuthToken() check to prevent premature navigation.
12-
it('Should return true for empty onboarding (pre-login default), confirming the need for an auth guard in deeplink navigation', () => {
11+
// The deeplink guard in Link.ts must combine this with a route truthiness check to prevent navigating to an empty route
12+
// when the user visits the root URL and signs in (issue #90880).
13+
it('Should return true for empty onboarding (pre-login default), confirming the need for a route check in deeplink navigation', () => {
1314
const emptyOnboarding = {} as OnyxValue<typeof ONYXKEYS.NVP_ONBOARDING>;
14-
const hasToken = false; // simulates hasAuthToken() returning false before login
1515
const selectorResult = hasCompletedGuidedSetupFlowSelector(emptyOnboarding);
1616

1717
// The selector returns true for empty objects (old/migrated accounts), which is correct for its own purpose.
1818
expect(selectorResult).toBe(true);
1919

20-
// But the deeplink guard must NOT navigate when the user has no auth token, even if the selector returns true.
21-
// This is the condition from openReportFromDeepLink: (hasAuthToken() && hasCompletedGuidedSetupFlowSelector(val))
22-
expect(hasToken && selectorResult).toBe(false);
20+
// The deeplink guard must NOT navigate when the route is empty (root URL produces ''),
21+
// even if the selector returns true. This is the condition from openReportFromDeepLink:
22+
// (route && hasCompletedGuidedSetupFlowSelector(val))
23+
const emptyRoute = ''; // root URL produces empty string via getRouteFromLink
24+
expect(emptyRoute && selectorResult).toBeFalsy();
25+
26+
// But when a real deeplink route exists, navigation should proceed
27+
const deeplinkRoute = 'concierge';
28+
expect(deeplinkRoute && selectorResult).toBeTruthy();
2329
});
2430

2531
it('Should return true if onboarding NVP is an empty object', () => {

0 commit comments

Comments
 (0)