From b5d5fe4cf6581ac84c13e0f65a90d3fc3503c652 Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Thu, 26 Mar 2026 04:18:44 +0000 Subject: [PATCH 1/7] Preserve deep-linked report route during onboarding guard redirect When the OnboardingGuard fires a REDIRECT, handleNavigationGuards was replacing the entire navigation state with [Home, OnboardingModalNavigator]. This discarded any existing fullscreen routes, including a deep-linked report that the user was trying to access before signing in. Now we separate the redirect state into fullscreen (base) and modal routes. If the current state already has fullscreen routes and the redirect adds modal routes, we preserve the existing fullscreen routes and layer the modal routes on top. This means the navigation state becomes [ReportsSplitNavigator(deep-linked report), OnboardingModalNavigator] instead of [Home, OnboardingModalNavigator], so when onboarding completes and the modal is dismissed, the user lands on their deep-linked report. Co-authored-by: Bernhard Owen Josephus --- .../RootStackRouter.ts | 24 ++- .../handleNavigationGuardRedirect.test.ts | 189 ++++++++++++++++++ 2 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts diff --git a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts index fc70a1464801..e41ed17ed863 100644 --- a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts +++ b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts @@ -1,5 +1,5 @@ import {CommonActions, StackRouter} from '@react-navigation/native'; -import type {RouterConfigOptions, StackActionType, StackNavigationState} from '@react-navigation/native'; +import type {NavigationState, PartialState, RouterConfigOptions, StackActionType, StackNavigationState} from '@react-navigation/native'; import type {ParamListBase} from '@react-navigation/routers'; import {createGuardContext, evaluateGuards} from '@libs/Navigation/guards'; import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath'; @@ -94,10 +94,26 @@ function handleNavigationGuards( return null; } + const redirectRoute = redirectState.routes.at(-1); + + // If the focused route is already the redirect target (e.g., multiple actions triggered + // on fresh app open all fire the same guard), don't add it again. + if (redirectRoute && state.routes[state.index]?.name === redirectRoute.name) { + return state; + } + + const hasExistingFullScreenRoute = state.routes.some((route) => isFullScreenName(route.name)); + + // When the current stack already has a fullscreen route (e.g., a deep-linked report), + // append only the redirect target on top of the existing routes so the user returns + // to them after the redirect screen is dismissed. Otherwise (fresh app with no stack), + // use the full redirect state which includes the base route (e.g., Home). + const routes = hasExistingFullScreenRoute && redirectRoute ? [...state.routes, redirectRoute] : redirectState.routes; + const resetAction = CommonActions.reset({ - index: redirectState.index ?? redirectState.routes.length - 1, - routes: redirectState.routes, - }); + index: routes.length - 1, + routes, + } as PartialState); return stackRouter.getStateForAction(state, resetAction, configOptions); } diff --git a/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts b/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts new file mode 100644 index 000000000000..17edec9a1fec --- /dev/null +++ b/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts @@ -0,0 +1,189 @@ +import type {StackNavigationState} from '@react-navigation/native'; +import type {ParamListBase} from '@react-navigation/routers'; +import RootStackRouter from '@libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter'; +import {evaluateGuards} from '@libs/Navigation/guards'; +import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath'; +import NAVIGATORS from '@src/NAVIGATORS'; +import SCREENS from '@src/SCREENS'; + +jest.mock('@libs/Navigation/guards', () => ({ + // eslint-disable-next-line @typescript-eslint/naming-convention + __esModule: true, + createGuardContext: jest.fn(() => ({ + isAuthenticated: true, + isLoading: false, + currentUrl: '', + })), + evaluateGuards: jest.fn(() => ({type: 'ALLOW'})), + registerGuard: jest.fn(), + clearGuards: jest.fn(), + getRegisteredGuards: jest.fn(() => []), +})); + +jest.mock('@libs/Navigation/helpers/getAdaptedStateFromPath', () => ({ + // eslint-disable-next-line @typescript-eslint/naming-convention + __esModule: true, + default: jest.fn(), +})); + +const mockedEvaluateGuards = evaluateGuards as jest.Mock; +const mockedGetAdaptedStateFromPath = getAdaptedStateFromPath as jest.Mock; + +const routeNames = [SCREENS.HOME, NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR]; + +describe('handleNavigationGuards - REDIRECT stack preservation', () => { + const router = RootStackRouter({}); + + const configOptions = { + routeNames, + routeParamList: {} as ParamListBase, + routeGetIdList: {} as Record) => string) | undefined>, + }; + + const mockAction = { + type: 'NAVIGATE' as const, + payload: {name: SCREENS.HOME}, + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockedEvaluateGuards.mockReturnValue({type: 'ALLOW'}); + }); + + it('should preserve existing fullscreen routes and append redirect target on top', () => { + // Given the current stack has a deep-linked report (a fullscreen route) + const state: StackNavigationState = { + key: 'root', + index: 0, + routeNames, + routes: [{key: 'report-1', name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, params: undefined}], + stale: false, + type: 'stack', + preloadedRoutes: [], + }; + + // When the guard returns REDIRECT to onboarding and getAdaptedStateFromPath returns a state with Home + OnboardingModalNavigator + mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'}); + mockedGetAdaptedStateFromPath.mockReturnValue({ + routes: [ + {name: SCREENS.HOME, key: 'home-1'}, + {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-1'}, + ], + }); + + const result = router.getStateForAction(state, mockAction, configOptions); + + // Then the deep-linked report should be preserved and onboarding should be appended on top + expect(result).not.toBeNull(); + expect(result?.routes).toHaveLength(2); + expect(result?.routes[0].name).toBe(NAVIGATORS.REPORTS_SPLIT_NAVIGATOR); + expect(result?.routes[1].name).toBe(NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR); + expect(result?.index).toBe(1); + }); + + it('should use the full redirect state when no existing fullscreen route is present', () => { + // Given a fresh app state with no fullscreen routes (e.g., only a non-fullscreen route) + const state: StackNavigationState = { + key: 'root', + index: 0, + routeNames: [...routeNames, 'SomeNonFullScreenRoute'], + routes: [{key: 'other-1', name: 'SomeNonFullScreenRoute', params: undefined}], + stale: false, + type: 'stack', + preloadedRoutes: [], + }; + + // When the guard returns REDIRECT to onboarding + mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'}); + mockedGetAdaptedStateFromPath.mockReturnValue({ + routes: [ + {name: SCREENS.HOME, key: 'home-1'}, + {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-1'}, + ], + }); + + const result = router.getStateForAction(state, mockAction, configOptions); + + // Then the full redirect state (Home + Onboarding) should be used since there's no fullscreen route to preserve + expect(result).not.toBeNull(); + expect(result?.routes).toHaveLength(2); + expect(result?.routes[0].name).toBe(SCREENS.HOME); + expect(result?.routes[1].name).toBe(NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR); + expect(result?.index).toBe(1); + }); + + it('should not duplicate the redirect target when it is already the focused route', () => { + // Given the current stack already has the onboarding route as the focused route + const state: StackNavigationState = { + key: 'root', + index: 1, + routeNames, + routes: [ + {key: 'home-1', name: SCREENS.HOME, params: undefined}, + {key: 'onboarding-1', name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, params: undefined}, + ], + stale: false, + type: 'stack', + preloadedRoutes: [], + }; + + // When the guard fires the same REDIRECT again + mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'}); + mockedGetAdaptedStateFromPath.mockReturnValue({ + routes: [ + {name: SCREENS.HOME, key: 'home-2'}, + {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-2'}, + ], + }); + + const result = router.getStateForAction(state, mockAction, configOptions); + + // Then the state should be unchanged (no duplicate onboarding route added) + expect(result).toEqual(state); + }); + + it('should not process redirect when guard allows navigation', () => { + // Given the guard allows navigation + const state: StackNavigationState = { + key: 'root', + index: 0, + routeNames, + routes: [{key: 'home-1', name: SCREENS.HOME, params: undefined}], + stale: false, + type: 'stack', + preloadedRoutes: [], + }; + + mockedEvaluateGuards.mockReturnValue({type: 'ALLOW'}); + + // Normal routing may error with minimal config — we only care that redirect logic was not triggered + try { + router.getStateForAction(state, mockAction, configOptions); + } catch { + // Expected: StackRouter needs full config for normal routing + } + + // Then getAdaptedStateFromPath should NOT have been called (no redirect processing) + expect(mockedGetAdaptedStateFromPath).not.toHaveBeenCalled(); + }); + + it('should return unchanged state when guard blocks navigation', () => { + // Given the guard blocks navigation + const state: StackNavigationState = { + key: 'root', + index: 0, + routeNames, + routes: [{key: 'home-1', name: SCREENS.HOME, params: undefined}], + stale: false, + type: 'stack', + preloadedRoutes: [], + }; + + mockedEvaluateGuards.mockReturnValue({type: 'BLOCK', reason: 'Test block'}); + + const result = router.getStateForAction(state, mockAction, configOptions); + + // Then the state should be returned unchanged + expect(result).toEqual(state); + }); +}); From 71716c73e6672f59bdfbb9c931a9f11b6c77e7d7 Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Tue, 7 Apr 2026 09:13:34 +0000 Subject: [PATCH 2/7] Refactor redirect logic to only preserve routes for modal guard targets Co-authored-by: bernhardoj Co-authored-by: Bernhard Owen Josephus --- .../RootStackRouter.ts | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts index e41ed17ed863..8948b4bf329d 100644 --- a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts +++ b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts @@ -64,6 +64,12 @@ function isPreloadAction(action: RootStackNavigatorAction): action is PreloadAct return action.type === CONST.NAVIGATION.ACTION_TYPE.PRELOAD; } +const MODAL_GUARD_REDIRECT_TARGETS = new Set([NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, NAVIGATORS.MIGRATED_USER_MODAL_NAVIGATOR]); + +function isModalGuardRedirectTarget(name: string) { + return MODAL_GUARD_REDIRECT_TARGETS.has(name); +} + /** * Evaluates navigation guards and handles BLOCK/REDIRECT results * @@ -94,25 +100,22 @@ function handleNavigationGuards( return null; } - const redirectRoute = redirectState.routes.at(-1); - - // If the focused route is already the redirect target (e.g., multiple actions triggered - // on fresh app open all fire the same guard), don't add it again. - if (redirectRoute && state.routes[state.index]?.name === redirectRoute.name) { - return state; + const isModalRedirect = redirectState.routes.some((route) => isModalGuardRedirectTarget(route.name)); + + let resetRoutes: typeof redirectState.routes = redirectState.routes; + if (isModalRedirect) { + const redirectRoute = redirectState.routes.at(-1); + const existingFullScreenRoute = state.routes.find((route) => isFullScreenName(route.name)); + // When the current stack already has a fullscreen route (e.g., a deep-linked report), + // append only the redirect target on top of the existing routes so the user returns + // to them after the redirect screen is dismissed. Otherwise (fresh app with no stack), + // use the full redirect state which includes the base route (e.g., Home). + resetRoutes = existingFullScreenRoute && redirectRoute ? ([existingFullScreenRoute, redirectRoute] as typeof redirectState.routes) : redirectState.routes; } - const hasExistingFullScreenRoute = state.routes.some((route) => isFullScreenName(route.name)); - - // When the current stack already has a fullscreen route (e.g., a deep-linked report), - // append only the redirect target on top of the existing routes so the user returns - // to them after the redirect screen is dismissed. Otherwise (fresh app with no stack), - // use the full redirect state which includes the base route (e.g., Home). - const routes = hasExistingFullScreenRoute && redirectRoute ? [...state.routes, redirectRoute] : redirectState.routes; - const resetAction = CommonActions.reset({ - index: routes.length - 1, - routes, + index: resetRoutes.length - 1, + routes: resetRoutes, } as PartialState); return stackRouter.getStateForAction(state, resetAction, configOptions); From 4b4ac60feaf24493831a9758aaa9dbce70b81ae9 Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Tue, 7 Apr 2026 09:28:56 +0000 Subject: [PATCH 3/7] Update unit test to match modal-only redirect preservation logic Co-authored-by: bernhardoj Co-authored-by: Bernhard Owen Josephus --- .../handleNavigationGuardRedirect.test.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts b/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts index 17edec9a1fec..dfdc3e3f6a9a 100644 --- a/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts +++ b/tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts @@ -112,34 +112,31 @@ describe('handleNavigationGuards - REDIRECT stack preservation', () => { expect(result?.index).toBe(1); }); - it('should not duplicate the redirect target when it is already the focused route', () => { - // Given the current stack already has the onboarding route as the focused route + it('should use the full redirect state for non-modal redirects even when fullscreen routes exist', () => { + // Given the current stack has a deep-linked report (a fullscreen route) const state: StackNavigationState = { key: 'root', - index: 1, + index: 0, routeNames, - routes: [ - {key: 'home-1', name: SCREENS.HOME, params: undefined}, - {key: 'onboarding-1', name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, params: undefined}, - ], + routes: [{key: 'report-1', name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, params: undefined}], stale: false, type: 'stack', preloadedRoutes: [], }; - // When the guard fires the same REDIRECT again - mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'}); + // When the guard returns a non-modal REDIRECT (e.g., to SettingsSplitNavigator) + mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'settings'}); mockedGetAdaptedStateFromPath.mockReturnValue({ - routes: [ - {name: SCREENS.HOME, key: 'home-2'}, - {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-2'}, - ], + routes: [{name: NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR, key: 'settings-1'}], }); const result = router.getStateForAction(state, mockAction, configOptions); - // Then the state should be unchanged (no duplicate onboarding route added) - expect(result).toEqual(state); + // Then the full redirect state should be used (no route preservation for non-modal redirects) + expect(result).not.toBeNull(); + expect(result?.routes).toHaveLength(1); + expect(result?.routes[0].name).toBe(NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR); + expect(result?.index).toBe(0); }); it('should not process redirect when guard allows navigation', () => { From a9d22dc25e338973a19b19f400cdff3c935e8f49 Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Tue, 7 Apr 2026 09:29:59 +0000 Subject: [PATCH 4/7] Fix: skip redirect when target is already the focused route Prevent duplicate navigation state when the guard fires a REDIRECT to a route that is already focused (e.g. OnboardingModalNavigator). Return the current state unchanged instead of creating a new route. Co-authored-by: Bernhard Owen Josephus --- .../createRootStackNavigator/RootStackRouter.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts index 8948b4bf329d..095c4a9192fc 100644 --- a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts +++ b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts @@ -100,6 +100,14 @@ function handleNavigationGuards( return null; } + const redirectTarget = redirectState.routes.at(-1); + const focusedRoute = state.routes[state.index]; + + // If the redirect target is already the focused route, skip the redirect to avoid duplicating it. + if (redirectTarget && focusedRoute && redirectTarget.name === focusedRoute.name) { + return state; + } + const isModalRedirect = redirectState.routes.some((route) => isModalGuardRedirectTarget(route.name)); let resetRoutes: typeof redirectState.routes = redirectState.routes; From 6e5d049d2cdfd510d3812787bd162180997ef023 Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Wed, 8 Apr 2026 08:54:00 +0000 Subject: [PATCH 5/7] Revert "Fix: skip redirect when target is already the focused route" This reverts commit a9d22dc25e338973a19b19f400cdff3c935e8f49. --- .../createRootStackNavigator/RootStackRouter.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts index 095c4a9192fc..8948b4bf329d 100644 --- a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts +++ b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts @@ -100,14 +100,6 @@ function handleNavigationGuards( return null; } - const redirectTarget = redirectState.routes.at(-1); - const focusedRoute = state.routes[state.index]; - - // If the redirect target is already the focused route, skip the redirect to avoid duplicating it. - if (redirectTarget && focusedRoute && redirectTarget.name === focusedRoute.name) { - return state; - } - const isModalRedirect = redirectState.routes.some((route) => isModalGuardRedirectTarget(route.name)); let resetRoutes: typeof redirectState.routes = redirectState.routes; From c58257cbd4acf70a45f2170fb8efe877d657cbea Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Wed, 8 Apr 2026 10:02:24 +0000 Subject: [PATCH 6/7] Remove HOME navigation fallback from navigateAfterOnboarding When no specific report is found after onboarding, stop navigating to HOME and let the modal dismissal handle the default behavior. Co-authored-by: Bernhard Owen Josephus --- src/libs/navigateAfterOnboarding.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libs/navigateAfterOnboarding.ts b/src/libs/navigateAfterOnboarding.ts index cfb8de469374..67087e1d7890 100644 --- a/src/libs/navigateAfterOnboarding.ts +++ b/src/libs/navigateAfterOnboarding.ts @@ -69,9 +69,6 @@ function navigateAfterOnboarding( ); if (reportID) { Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID)); - } else { - // Navigate to home to trigger guard evaluation - Navigation.navigate(ROUTES.HOME); } } From 36a1718fdd2bec052ba2863c082bf6dc9aac38ef Mon Sep 17 00:00:00 2001 From: "Bernhard Owen Josephus (via MelvinBot)" Date: Fri, 10 Apr 2026 04:33:38 +0000 Subject: [PATCH 7/7] Fix: use last fullscreen route instead of first for modal redirects When multiple fullscreen routes exist in the stack, find() picks the first one which may be stale. Using findLast() preserves the most recently active fullscreen route so dismissing the redirect modal returns users to the correct screen. Co-authored-by: Bernhard Owen Josephus --- .../AppNavigator/createRootStackNavigator/RootStackRouter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts index 8948b4bf329d..ccea9c2e5e74 100644 --- a/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts +++ b/src/libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter.ts @@ -105,7 +105,7 @@ function handleNavigationGuards( let resetRoutes: typeof redirectState.routes = redirectState.routes; if (isModalRedirect) { const redirectRoute = redirectState.routes.at(-1); - const existingFullScreenRoute = state.routes.find((route) => isFullScreenName(route.name)); + const existingFullScreenRoute = state.routes.findLast((route) => isFullScreenName(route.name)); // When the current stack already has a fullscreen route (e.g., a deep-linked report), // append only the redirect target on top of the existing routes so the user returns // to them after the redirect screen is dismissed. Otherwise (fresh app with no stack),