[CP Staging] fix: Expense - Arrow navigation is not automatically displayed when opening expense via growl

src/libs/Navigation/helpers/navigateToCreatedExpense.ts

@@ -1,6 +1,8 @@
 import {setActiveTransactionIDs} from '@libs/actions/TransactionThreadNavigation';
 import getIsNarrowLayout from '@libs/getIsNarrowLayout';
 import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
+import {isMoneyRequestReport} from '@libs/ReportUtils';
+import {getCreated} from '@libs/TransactionUtils';
 
 import CONST from '@src/CONST';
 import ROUTES from '@src/ROUTES';
@@ -25,6 +27,16 @@ type NavigateToCreatedExpenseParams = {
     reportTransactions: Transaction[];
 };
 
+// Created dates are ISO strings, so comparing them as strings orders them chronologically
+function compareByCreated(a: Transaction, b: Transaction) {
+    const createdA = getCreated(a);
+    const createdB = getCreated(b);
+    if (createdA === createdB) {
+        return 0;
+    }
+    return createdA < createdB ? -1 : 1;
+}
+
 function getCurrentRouteBackTo() {
     const params = navigationRef.current?.getCurrentRoute()?.params;
     if (typeof params !== 'object' || params === null || !('backTo' in params) || typeof params.backTo !== 'string') {
@@ -38,11 +50,16 @@ function getCurrentRouteBackTo() {
  * switched tabs while the growl was up, so the destination follows wherever they are now.
  */
 function navigateToCreatedExpense({threadReportID, transactionID, iouReportID, reportTransactions}: NavigateToCreatedExpenseParams) {
-    // Don't reopen an expense the user is already looking at
-    const hasMultipleReportTransactions = iouReportID
-        ? reportTransactions.filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length > 1
-        : false;
-    // getState() can miss a just-opened RHP's nested state, so read the full tree.
+    // Oldest first, so the prev/next arrows follow the expense dates. Grouped layouts (e.g. by category) list them
+    // differently, which only the report view knows.
+    const openableTransactionIDs = reportTransactions
+        .filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
+        .sort(compareByCreated)
+        .map((transaction) => transaction.transactionID);
+    const hasMultipleReportTransactions = iouReportID ? openableTransactionIDs.length > 1 : false;
+
+    // Don't reopen an expense the user is already looking at. getState() can miss a just-opened RHP's nested state,
+    // so read the full tree.
     const focusedReportID = Navigation.getFocusedReportId(navigationRef.getRootState());
     if (focusedReportID === threadReportID || (!hasMultipleReportTransactions && !!iouReportID && focusedReportID === iouReportID)) {
         return;
@@ -62,6 +79,37 @@ function navigateToCreatedExpense({threadReportID, transactionID, iouReportID, r
         return;
     }
 
+    // Same as opening the expense from its report preview: the thread goes in the RHP on top of its report, so the
+    // prev/next arrows show.
+    if (getIsNarrowLayout() && iouReportID && hasMultipleReportTransactions) {
+        // The expense report is already open full screen, with or without an RHP above it, so the thread goes on top of it.
+        if ((!forceReplace && focusedReportID === iouReportID) || (forceReplace && Navigation.getTopmostReportId() === iouReportID)) {
+            setActiveTransactionIDs(openableTransactionIDs);
+            const threadBackTo = backTo ?? ROUTES.REPORT_WITH_ID.getRoute(iouReportID);
+            Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: threadReportID, backTo: threadBackTo}), {forceReplace});
+            return;
+        }
+
+        const openThreadOnExpenseReport = () => {
+            // Expense reports replace each other rather than stack, so another one open full screen is swapped out
+            const openReportID = Navigation.getTopmostReportId(navigationRef.getRootState());
+            const shouldReplaceOpenReport = !!openReportID && isMoneyRequestReport(openReportID);
+            const reportBackTo = shouldReplaceOpenReport ? getCurrentRouteBackTo() : Navigation.getActiveRoute();
+            const reportRoute = ROUTES.REPORT_WITH_ID.getRoute(iouReportID, undefined, undefined, reportBackTo);
+            Navigation.navigate(reportRoute, {forceReplace: shouldReplaceOpenReport});
+            setActiveTransactionIDs(openableTransactionIDs);
+            Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: threadReportID, backTo: reportRoute}));
+        };
+
+        // A full screen report would be pushed above the open RHP instead of replacing it, so close the RHP first.
+        if (forceReplace) {
+            Navigation.dismissModal({afterTransition: openThreadOnExpenseReport});
+            return;
+        }
+        openThreadOnExpenseReport();
+        return;
+    }
+
     if (getIsNarrowLayout()) {
         Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(threadReportID, undefined, undefined, backTo), {forceReplace});
         return;
@@ -77,7 +125,8 @@ function navigateToCreatedExpense({threadReportID, transactionID, iouReportID, r
             // Defer so the thread RHP stacks on top of the expense report navigation above. This is always a
             // push (never a replace) - it stacks on the report we just opened, not on the previously-open one.
             setNavigationActionToMicrotaskQueue(() => {
-                setActiveTransactionIDs([transactionID]).then(() => {
+                // Seed every expense on the report, not just this one, so the thread shows the prev/next arrows.
+                setActiveTransactionIDs(openableTransactionIDs).then(() => {
                     Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: threadReportID, backTo: Navigation.getActiveRoute()}));
                 });
             });

tests/unit/navigateToCreatedExpenseTest.ts

@@ -1,3 +1,4 @@
+import {setActiveTransactionIDs} from '@libs/actions/TransactionThreadNavigation';
 import type isReportOpenInRHP from '@libs/Navigation/helpers/isReportOpenInRHP';
 import navigateToCreatedExpense from '@libs/Navigation/helpers/navigateToCreatedExpense';
 import Navigation from '@libs/Navigation/Navigation';
@@ -15,6 +16,8 @@ const mockGetIsNarrowLayout = jest.fn<boolean, []>();
 const mockGetCurrentRoute = jest.fn<{params?: Record<string, unknown>} | undefined, []>();
 const mockGetFocusedReportId = jest.fn<string | undefined, [unknown]>();
 const mockGetRootState = jest.fn<unknown, []>(() => ({routes: []}));
+const mockGetTopmostReportId = jest.fn<string | undefined, []>();
+const mockIsMoneyRequestReport = jest.fn<boolean, [string]>();
 
 function buildTransaction(transactionID: string): Transaction {
     return {transactionID, reportID: 'iou-1', amount: 0, created: '', currency: CONST.CURRENCY.USD, merchant: '', comment: {}};
@@ -27,14 +30,19 @@ jest.mock('@libs/Navigation/helpers/setNavigationActionToMicrotaskQueue', () =>
     callback();
 });
 jest.mock('@libs/getIsNarrowLayout', () => () => mockGetIsNarrowLayout());
+jest.mock('@libs/ReportUtils', () => ({
+    isMoneyRequestReport: (reportID: string) => mockIsMoneyRequestReport(reportID),
+}));
 jest.mock('@libs/actions/TransactionThreadNavigation', () => ({
     setActiveTransactionIDs: jest.fn(() => Promise.resolve()),
 }));
 
 jest.mock('@libs/Navigation/Navigation', () => ({
     navigate: jest.fn(),
+    dismissModal: jest.fn(),
     getActiveRoute: jest.fn(() => ''),
     getFocusedReportId: (state: unknown) => mockGetFocusedReportId(state),
+    getTopmostReportId: () => mockGetTopmostReportId(),
     navigationRef: {
         getRootState: () => mockGetRootState(),
         current: {
@@ -54,6 +62,8 @@ describe('navigateToCreatedExpense', () => {
         mockGetCurrentRoute.mockReturnValue(undefined);
         mockGetFocusedReportId.mockReturnValue(undefined);
         mockGetRootState.mockReturnValue({routes: []});
+        mockGetTopmostReportId.mockReturnValue(undefined);
+        mockIsMoneyRequestReport.mockReturnValue(false);
     });
 
     it('should do nothing when the user already has the transaction thread open', async () => {
@@ -102,8 +112,157 @@ describe('navigateToCreatedExpense', () => {
         });
         await waitForBatchedUpdates();
 
-        // Then the transaction thread still opens, since the list does not show the expense itself
-        expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute('thread-1', undefined, undefined, ''), {forceReplace: false});
+        // Then the transaction thread opens on top of the report already open, since the list does not show the expense itself
+        expect(Navigation.navigate).toHaveBeenCalledTimes(1);
+        expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: ''}), {forceReplace: false});
+    });
+
+    it('should open the expense report then the thread RHP with the prev/next arrows on a narrow layout when the report has multiple transactions', async () => {
+        // Given the user is on a chat in the Inbox tab on a narrow layout, and the report holds several expenses
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockIsSearchTopmostFullScreenRoute.mockReturnValue(false);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('chat-1');
+
+        // When they press "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the thread opens in the RHP on top of its report, the same as opening it from the report preview
+        const reportRoute = ROUTES.REPORT_WITH_ID.getRoute('iou-1', undefined, undefined, '');
+        expect(setActiveTransactionIDs).toHaveBeenCalledWith(['txn-1', 'txn-2']);
+        expect(Navigation.navigate).toHaveBeenCalledTimes(2);
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(1, reportRoute, {forceReplace: false});
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(2, ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: reportRoute}));
+    });
+
+    it('should replace the open RHP with the thread on a narrow layout when the expense report is under it', async () => {
+        // Given another expense of the same report is open in the RHP, on top of the full screen expense report
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockIsReportOpenInRHP.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('other-thread');
+        mockGetTopmostReportId.mockReturnValue('iou-1');
+        mockGetCurrentRoute.mockReturnValue({params: {backTo: 'r/iou-1'}});
+
+        // When the user presses "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the thread replaces the open RHP, so going back lands on the expense report under it
+        expect(Navigation.dismissModal).not.toHaveBeenCalled();
+        expect(Navigation.navigate).toHaveBeenCalledTimes(1);
+        expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: 'r/iou-1'}), {forceReplace: true});
+    });
+
+    it('should close the open RHP before opening the expense report and thread on a narrow layout when the RHP is over another report', async () => {
+        // Given an expense of a different report is open in the RHP, on top of that other report
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockIsReportOpenInRHP.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('other-thread');
+        mockGetTopmostReportId.mockReturnValue('other-iou');
+
+        // When the user presses "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the RHP closes first, since a full screen report would be pushed above it instead of replacing it
+        expect(Navigation.navigate).not.toHaveBeenCalled();
+        const afterTransition = jest.mocked(Navigation.dismissModal).mock.calls.at(0)?.at(0)?.afterTransition;
+        afterTransition?.();
+
+        // And then the expense report opens with the thread on top of it
+        const reportRoute = ROUTES.REPORT_WITH_ID.getRoute('iou-1', undefined, undefined, '');
+        expect(setActiveTransactionIDs).toHaveBeenCalledWith(['txn-1', 'txn-2']);
+        expect(Navigation.navigate).toHaveBeenCalledTimes(2);
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(1, reportRoute, {forceReplace: false});
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(2, ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: reportRoute}));
+    });
+
+    it('should replace another expense report open under the RHP instead of stacking on it on a narrow layout', async () => {
+        // Given an expense of another expense report is open in the RHP, on top of that report opened from its chat
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockIsReportOpenInRHP.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('other-thread');
+        mockGetTopmostReportId.mockReturnValue('other-iou');
+        mockIsMoneyRequestReport.mockImplementation((reportID) => reportID === 'other-iou');
+
+        // When the user presses "View" and the RHP has closed, leaving the other expense report on top
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+        mockGetCurrentRoute.mockReturnValue({params: {backTo: 'r/chat-1'}});
+        jest.mocked(Navigation.dismissModal).mock.calls.at(0)?.at(0)?.afterTransition?.();
+
+        // Then the new expense report replaces the other one and takes over its backTo, so going back skips it
+        const reportRoute = ROUTES.REPORT_WITH_ID.getRoute('iou-1', undefined, undefined, 'r/chat-1');
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(1, reportRoute, {forceReplace: true});
+        expect(Navigation.navigate).toHaveBeenNthCalledWith(2, ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: reportRoute}));
+    });
+
+    it('should fall back to the expense report as the thread backTo when the replaced RHP had none', async () => {
+        // Given an expense of the same report is open in the RHP without a backTo, on top of the full screen expense report
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockIsReportOpenInRHP.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('other-thread');
+        mockGetTopmostReportId.mockReturnValue('iou-1');
+        mockGetCurrentRoute.mockReturnValue({params: {}});
+
+        // When the user presses "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the thread still points back at its expense report, so a reload keeps the user in the Inbox
+        expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: ROUTES.REPORT_WITH_ID.getRoute('iou-1')}), {forceReplace: true});
+    });
+
+    it('should close the RHP showing the expense report itself before stacking the thread on a narrow layout', async () => {
+        // Given the expense report itself is open in the RHP, on top of a chat
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(true);
+        mockIsReportOpenInRHP.mockReturnValue(true);
+        mockGetFocusedReportId.mockReturnValue('iou-1');
+        mockGetTopmostReportId.mockReturnValue('chat-1');
+
+        // When the user presses "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2')],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the RHP closes instead of being replaced by the thread, so going back still lands on the expense report
+        expect(Navigation.navigate).not.toHaveBeenCalled();
+        expect(Navigation.dismissModal).toHaveBeenCalledTimes(1);
     });
 
     it('should still navigate when the focused report is a different one', async () => {
@@ -182,6 +341,47 @@ describe('navigateToCreatedExpense', () => {
         expect(Navigation.navigate).toHaveBeenNthCalledWith(2, ROUTES.SEARCH_REPORT.getRoute({reportID: 'thread-1', backTo: ''}));
     });
 
+    it('should seed every non-deleted expense on the report so the thread shows the prev/next arrows', async () => {
+        // Given the user is on the Inbox tab on a wide layout and the expense report holds several transactions, one pending delete
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockIsSearchTopmostFullScreenRoute.mockReturnValue(false);
+        mockGetIsNarrowLayout.mockReturnValue(false);
+        const deletedTransaction = {...buildTransaction('txn-3'), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE};
+
+        // When they press "View"
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-1',
+            iouReportID: 'iou-1',
+            reportTransactions: [buildTransaction('txn-1'), buildTransaction('txn-2'), deletedTransaction],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the arrows cover the report's other expenses, but skip the one being deleted
+        expect(setActiveTransactionIDs).toHaveBeenCalledWith(['txn-1', 'txn-2']);
+    });
+
+    it('should seed the expenses oldest first rather than in creation order', async () => {
+        // Given the expenses were created in a different order than their dates
+        mockIsReportTopmostSplitNavigator.mockReturnValue(true);
+        mockGetIsNarrowLayout.mockReturnValue(false);
+        const octoberExpense = {...buildTransaction('txn-oct'), created: '2026-10-01'};
+        const septemberExpense = {...buildTransaction('txn-sep'), created: '2026-09-15'};
+        const newExpense = {...buildTransaction('txn-new'), created: '2026-09-20'};
+
+        // When the user presses "View" on the newest expense
+        navigateToCreatedExpense({
+            threadReportID: 'thread-1',
+            transactionID: 'txn-new',
+            iouReportID: 'iou-1',
+            reportTransactions: [octoberExpense, septemberExpense, newExpense],
+        });
+        await waitForBatchedUpdates();
+
+        // Then the arrows follow the dates, so "Next" from the new expense opens the October one
+        expect(setActiveTransactionIDs).toHaveBeenCalledWith(['txn-sep', 'txn-new', 'txn-oct']);
+    });
+
     it('should open the expense report without the replaced RHP backTo, so deleting the report falls back to its chat', async () => {
         // Given the user is on the Inbox tab on a wide layout with another report already open in the RHP
         mockIsReportTopmostSplitNavigator.mockReturnValue(true);