Merge pull request Expensify#87622 from software-mansion-labs/collectioneur/fix-flag-comment-page-in-thread

Author: mjasikowski

src/libs/Navigation/helpers/dynamicRoutesUtils/isDynamicRouteScreen.ts

@@ -0,0 +1,21 @@
+import {normalizedConfigs} from '@libs/Navigation/linkingConfig/config';
+import type {DynamicRouteSuffix} from '@src/ROUTES';
+import type {Screen} from '@src/SCREENS';
+import {dynamicRoutePaths} from './isDynamicRouteSuffix';
+
+/**
+ * Checks if a screen name is a dynamic route screen
+ * @param screenName - The name of the screen to check.
+ * @returns True if the screen name is a dynamic route screen, false otherwise.
+ */
+function isDynamicRouteScreen(screenName: Screen): boolean {
+    const screenPath = normalizedConfigs[screenName]?.path;
+
+    if (!screenPath) {
+        return false;
+    }
+
+    return dynamicRoutePaths.has(screenPath as DynamicRouteSuffix);
+}
+
+export default isDynamicRouteScreen;

src/libs/Navigation/helpers/getAdaptedStateFromPath.ts

@@ -10,8 +10,10 @@ import NAVIGATORS from '@src/NAVIGATORS';
 import type {Route as RoutePath} from '@src/ROUTES';
 import ROUTES from '@src/ROUTES';
 import SCREENS from '@src/SCREENS';
+import type {Screen} from '@src/SCREENS';
 import findMatchingDynamicSuffix from './dynamicRoutesUtils/findMatchingDynamicSuffix';
 import getPathWithoutDynamicSuffix from './dynamicRoutesUtils/getPathWithoutDynamicSuffix';
+import isDynamicRouteScreen from './dynamicRoutesUtils/isDynamicRouteScreen';
 import findFocusedRouteWithOnyxTabGuard from './findFocusedRouteWithOnyxTabGuard';
 import getMatchingNewRoute from './getMatchingNewRoute';
 import getParamsFromRoute from './getParamsFromRoute';
@@ -54,8 +56,13 @@ function getSearchScreenNameForRoute(route: NavigationPartialRoute): string {
 }
 
 function getMatchingFullScreenRoute(route: NavigationPartialRoute) {
+    const isDynamicScreen = isDynamicRouteScreen(route.name as Screen);
+
     // Check for backTo param. One screen with different backTo value may need different screens visible under the overlay.
-    if (isRouteWithBackToParam(route)) {
+    // Dynamic screens are skipped here because they never carry their own backTo - they only
+    // inherit it from the screen underneath. Letting backTo dictate the full-screen route for
+    // a dynamic screen would resolve the wrong page.
+    if (isRouteWithBackToParam(route) && !isDynamicScreen) {
         const stateForBackTo = getStateFromPath(route.params.backTo as RoutePath);
 
         // This may happen if the backTo url is invalid.

src/libs/Navigation/helpers/getPathFromState.ts

@@ -1,27 +1,13 @@
 import {findFocusedRoute, getPathFromState as RNGetPathFromState} from '@react-navigation/native';
 import type {NavigationState, PartialState} from '@react-navigation/routers';
 import {config, normalizedConfigs} from '@libs/Navigation/linkingConfig/config';
-import type {DynamicRouteSuffix} from '@src/ROUTES';
 import type {Screen} from '@src/SCREENS';
 import getDynamicRouteQueryParams from './dynamicRoutesUtils/getDynamicRouteQueryParams';
-import {dynamicRoutePaths} from './dynamicRoutesUtils/isDynamicRouteSuffix';
+import isDynamicRouteScreen from './dynamicRoutesUtils/isDynamicRouteScreen';
 import splitPathAndQuery from './dynamicRoutesUtils/splitPathAndQuery';
 
 type State = NavigationState | Omit<PartialState<NavigationState>, 'stale'>;
 
-/**
- * Checks if a screen name is a dynamic route screen
- */
-function isDynamicRouteScreen(screenName: Screen): boolean {
-    const screenPath = normalizedConfigs[screenName]?.path;
-
-    if (!screenPath) {
-        return false;
-    }
-
-    return dynamicRoutePaths.has(screenPath as DynamicRouteSuffix);
-}
-
 /**
  * Resolves a single path segment: if it's a `:param` placeholder, replaces it
  * with the URL-encoded value from `params`; otherwise returns the segment as-is.

tests/navigation/getMatchingFullScreenRouteTests.ts

@@ -4,7 +4,9 @@ import getStateFromPath from '@libs/Navigation/helpers/getStateFromPath';
 import SCREENS from '@src/SCREENS';
 
 jest.mock('@libs/Navigation/linkingConfig/config', () => ({
-    normalizedConfigs: {},
+    normalizedConfigs: {
+        DynamicScreen: {path: 'suffix-a'},
+    },
     screensWithOnyxTabNavigator: new Set(),
 }));
 
@@ -180,4 +182,30 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => {
         expect(mockGetStateFromPath).toHaveBeenCalledWith('/broken/path/suffix-a');
         expect(result).toBeUndefined();
     });
+
+    it('should ignore backTo for a dynamic screen and resolve full screen route via dynamic suffix instead', () => {
+        const route = {
+            name: 'DynamicScreen',
+            path: '/base/suffix-a',
+            params: {backTo: '/some/other/path'},
+        };
+        const fullScreenRoute = {name: SCREENS.HOME};
+        const basePathState = {
+            routes: [{name: 'BaseScreen'}, fullScreenRoute],
+            index: 1,
+        };
+
+        mockGetStateFromPath.mockImplementation((path: string) => {
+            if (path === '/base') {
+                return basePathState;
+            }
+            return {routes: [{name: 'WrongScreen'}], index: 0};
+        });
+
+        const result = getMatchingFullScreenRoute(route);
+
+        expect(mockGetStateFromPath).toHaveBeenCalledWith('/base');
+        expect(mockGetStateFromPath).not.toHaveBeenCalledWith('/some/other/path');
+        expect(result).toEqual(fullScreenRoute);
+    });
 });

tests/navigation/isDynamicRouteScreenTests.ts

@@ -0,0 +1,41 @@
+import isDynamicRouteScreen from '@libs/Navigation/helpers/dynamicRoutesUtils/isDynamicRouteScreen';
+import type {Screen} from '@src/SCREENS';
+import SCREENS from '@src/SCREENS';
+
+describe('isDynamicRouteScreen', () => {
+    it('should return true for a static dynamic route screen (DYNAMIC_VERIFY_ACCOUNT)', () => {
+        expect(isDynamicRouteScreen(SCREENS.SETTINGS.DYNAMIC_VERIFY_ACCOUNT)).toBe(true);
+    });
+
+    it('should return true for a multi-segment dynamic route screen (DYNAMIC_ADD_BANK_ACCOUNT_VERIFY_ACCOUNT)', () => {
+        expect(isDynamicRouteScreen(SCREENS.SETTINGS.DYNAMIC_ADD_BANK_ACCOUNT_VERIFY_ACCOUNT)).toBe(true);
+    });
+
+    it('should return true for a parametric dynamic route screen (DYNAMIC_FLAG_COMMENT)', () => {
+        expect(isDynamicRouteScreen(SCREENS.DYNAMIC_FLAG_COMMENT)).toBe(true);
+    });
+
+    it('should return true for DYNAMIC_KEYBOARD_SHORTCUTS', () => {
+        expect(isDynamicRouteScreen(SCREENS.SETTINGS.DYNAMIC_KEYBOARD_SHORTCUTS)).toBe(true);
+    });
+
+    it('should return true for DYNAMIC_ADDRESS_COUNTRY', () => {
+        expect(isDynamicRouteScreen(SCREENS.SETTINGS.PROFILE.DYNAMIC_ADDRESS_COUNTRY)).toBe(true);
+    });
+
+    it('should return false for a regular screen (HOME)', () => {
+        expect(isDynamicRouteScreen(SCREENS.HOME)).toBe(false);
+    });
+
+    it('should return false for a regular screen (REPORT)', () => {
+        expect(isDynamicRouteScreen(SCREENS.REPORT)).toBe(false);
+    });
+
+    it('should return false for a regular settings screen (Settings_Root)', () => {
+        expect(isDynamicRouteScreen(SCREENS.SETTINGS.ROOT)).toBe(false);
+    });
+
+    it('should return false for a screen name not present in normalizedConfigs', () => {
+        expect(isDynamicRouteScreen('NonExistentScreen_12345' as Screen)).toBe(false);
+    });
+});