Let an explicit column sort win over the RBR ordering and the alphabetical group order

src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

@@ -26,19 +26,10 @@ import {navigationRef} from '@libs/Navigation/Navigation';
 import {isPolicyTaxEnabled} from '@libs/PolicyUtils';
 import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils';
 import {groupTransactionsByCategory, groupTransactionsByTag} from '@libs/ReportLayoutUtils';
-import {
-    getActionErrorsByTransaction,
-    getMoneyRequestSpendBreakdown,
-    getReportOfflinePendingActionAndErrors,
-    getTransactionSortValue,
-    isExpenseReport,
-    isIOUReport,
-    isSortableColumnName,
-} from '@libs/ReportUtils';
+import {getMoneyRequestSpendBreakdown, getReportOfflinePendingActionAndErrors, getTransactionSortValue, isExpenseReport, isIOUReport, isSortableColumnName} from '@libs/ReportUtils';
 import type {SortableColumnName} from '@libs/ReportUtils';
 import {compareValues, getColumnsToShow, getTableMinWidth, isTransactionAmountTooLong, isTransactionTaxAmountTooLong} from '@libs/SearchUIUtils';
 import {getPendingSubmitFollowUpAction} from '@libs/telemetry/submitFollowUpAction';
-import {transactionHasRBR} from '@libs/TransactionPreviewUtils';
 import {getTransactionPendingAction, getVisibleTransactionViolations, hasNonReimbursableTransactions, isTransactionPendingDelete} from '@libs/TransactionUtils';
 import shouldShowTransactionPostedYear from '@libs/TransactionUtils/shouldShowTransactionPostedYear';
 import shouldShowTransactionYear from '@libs/TransactionUtils/shouldShowTransactionYear';
@@ -373,18 +364,13 @@ function MoneyRequestReportTransactionList({
     });
 
     const {sortBy, sortOrder} = sortConfig;
-    const isDefaultSort = sortBy === CONST.SEARCH.TABLE_COLUMNS.DATE && sortOrder === CONST.SEARCH.SORT_ORDER.ASC;
-
-    // In a single pass over reportActions, build:
-    // - reportActionsMap: keyed by reportActionID for transactionHasRBR.
-    // - transactionThreadReportIDByTransactionID: transactionID → transaction-thread report ID, so each row can pass it
-    //   to the RBR, letting rows without RBR content early-return instead of mounting the heavy RBR inner (6 Onyx
-    //   subscriptions). Without this, the per-row alternative would re-scan every report action (O(transactions × actions)).
-    const {reportActionsMap, transactionThreadReportIDByTransactionID} = useMemo(() => {
-        const actionsMap: Record<string, OnyxTypes.ReportAction> = {};
+
+    // Maps each transactionID to its transaction-thread report ID, so each row can pass it to the RBR, letting rows
+    // without RBR content early-return instead of mounting the heavy RBR inner (6 Onyx subscriptions). Without this,
+    // the per-row alternative would re-scan every report action (O(transactions x actions)).
+    const transactionThreadReportIDByTransactionID = useMemo(() => {
         const threadReportIDByTransactionID = new Map<string, string>();
         for (const action of reportActions) {
-            actionsMap[action.reportActionID] = action;
             if (!isMoneyRequestAction(action)) {
                 continue;
             }
@@ -394,49 +380,21 @@ function MoneyRequestReportTransactionList({
                 threadReportIDByTransactionID.set(iouTransactionID, action.childReportID);
             }
         }
-        return {reportActionsMap: actionsMap, transactionThreadReportIDByTransactionID: threadReportIDByTransactionID};
+        return threadReportIDByTransactionID;
     }, [reportActions]);
 
-    // Precompute the set of RBR-flagged transaction IDs
-    const rbrTransactionIDs = useMemo(() => {
-        if (!isDefaultSort || !allTransactionViolations) {
-            return null;
-        }
-        const login = currentUserDetails?.login ?? '';
-        const accountID = currentUserDetails?.accountID ?? CONST.DEFAULT_NUMBER_ID;
-        // Precompute report-action errors once so each transaction's RBR check is an O(1) lookup instead of
-        // re-scanning every report action (O(transactions × actions)).
-        const actionErrors = getActionErrorsByTransaction(report?.reportID, reportActionsMap);
-        const ids = new Set<string>();
-        for (const transaction of transactions) {
-            const violations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`] ?? [];
-            if (transactionHasRBR(transaction, violations, login, accountID, report, ownerLogin, policy, reportActionsMap, actionErrors)) {
-                ids.add(transaction.transactionID);
-            }
-        }
-        return ids;
-    }, [isDefaultSort, allTransactionViolations, currentUserDetails?.login, currentUserDetails?.accountID, transactions, report, ownerLogin, policy, reportActionsMap]);
-
     const sortedTransactions: TransactionWithOptionalHighlight[] = useMemo(() => {
-        return [...transactions].sort((a, b) => {
-            // When on default sort (Date/ASC), prioritize RBR-flagged transactions
-            if (rbrTransactionIDs) {
-                const aHasRBR = rbrTransactionIDs.has(a.transactionID);
-                const bHasRBR = rbrTransactionIDs.has(b.transactionID);
-                if (aHasRBR !== bHasRBR) {
-                    return aHasRBR ? -1 : 1;
-                }
-            }
-            return compareValues(
+        return [...transactions].sort((a, b) =>
+            compareValues(
                 getTransactionSortValue(a, sortBy, report, policy, policyCategories, policyTagLists),
                 getTransactionSortValue(b, sortBy, report, policy, policyCategories, policyTagLists),
                 sortOrder,
                 sortBy,
                 localeCompare,
                 true,
-            );
-        });
-    }, [sortBy, sortOrder, transactions, localeCompare, report, policy, policyCategories, policyTagLists, rbrTransactionIDs]);
+            ),
+        );
+    }, [sortBy, sortOrder, transactions, localeCompare, report, policy, policyCategories, policyTagLists]);
 
     const resolvedTransactions = useMemo(() => resolveTransactionCardFields(sortedTransactions, cardList, translate), [sortedTransactions, cardList, translate]);
 

tests/ui/MoneyRequestReportTransactionsNavigationTest.tsx

@@ -1,38 +1,101 @@
-import {fireEvent, render, screen, waitFor} from '@testing-library/react-native';
+import {act, fireEvent, render, screen, waitFor} from '@testing-library/react-native';
 
+import ComposeProviders from '@components/ComposeProviders';
+import {LocaleContextProvider} from '@components/LocaleContextProvider';
+import type {MoneyRequestReportTransactionListController, TransactionListItemData} from '@components/MoneyRequestReportView/MoneyRequestReportTransactionList';
+import MoneyRequestReportTransactionList from '@components/MoneyRequestReportView/MoneyRequestReportTransactionList';
 import MoneyRequestReportTransactionsNavigation from '@components/MoneyRequestReportView/MoneyRequestReportTransactionsNavigation';
+import OnyxListItemProvider from '@components/OnyxListItemProvider';
+import type * as SearchContext from '@components/Search/SearchContext';
+import type {SortOrder} from '@components/Search/types';
 
 import * as ReportActions from '@libs/actions/Report';
+import {clearActiveTransactionIDs} from '@libs/actions/TransactionThreadNavigation';
 
 import Navigation from '@navigation/Navigation';
 
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
+import SCREENS from '@src/SCREENS';
+import type {StableReport} from '@src/selectors/Report';
 import type {ReportActions as OnyxReportActions, Transaction} from '@src/types/onyx';
 
 import React from 'react';
+import {View} from 'react-native';
 import Onyx from 'react-native-onyx';
 
 import createRandomReportAction from '../utils/collections/reportActions';
 import createRandomTransaction from '../utils/collections/transaction';
 import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
 
+/** The slice of the transaction list's controller the tests below read. */
+type CapturedController = Pick<MoneyRequestReportTransactionListController, 'tableColumnHeader' | 'transactionListItems'>;
+
+// Drives the RHP-open check both the transaction list and the navigation component make through findFocusedRoute().
+// Undefined reproduces the real module's behaviour while the navigation ref isn't ready, which is what the existing
+// tests below run against.
+const mockFocusedRoute: {value: {name: string; key: string} | undefined} = {value: undefined};
+
+// Populated by the mocked unified list below with the controller the transaction list renders it with.
+const mockUnifiedList: {controller: CapturedController | undefined} = {controller: undefined};
+
 jest.mock('@react-navigation/native', () => {
     // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- jest.requireActual() returns the real module for partial mocking
     const actualNavigation = jest.requireActual('@react-navigation/native');
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-return -- returning the real module plus one overridden hook is the standard Jest partial-mock pattern
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-return -- returning the real module plus a few overridden hooks is the standard Jest partial-mock pattern
     return {
         ...actualNavigation,
         useIsFocused: () => true,
+        useFocusEffect: () => {},
+        findFocusedRoute: () => mockFocusedRoute.value,
     };
 });
 
+// The unified list is a FlashList. The tests below only need the controller it is handed, which carries the rendered
+// row order and the real column-header element.
+jest.mock('@components/MoneyRequestReportView/MoneyRequestReportUnifiedList', () => ({
+    __esModule: true,
+    default: ({controller}: {controller: CapturedController}) => {
+        mockUnifiedList.controller = controller;
+        return null;
+    },
+}));
+
+jest.mock('@hooks/useResponsiveLayout', () => ({
+    __esModule: true,
+    default: () => ({
+        shouldUseNarrowLayout: false,
+        isSmallScreenWidth: false,
+        isMediumScreenWidth: false,
+        isLargeScreenWidth: true,
+        isExtraLargeScreenWidth: true,
+        onboardingIsMediumOrLargerScreenWidth: true,
+    }),
+}));
+
+jest.mock('@hooks/useResponsiveLayoutOnWideRHP', () => ({
+    __esModule: true,
+    default: () => ({shouldUseNarrowLayout: false}),
+}));
+
+// The transaction list only reads the selection slice of the search context, and the real provider needs a
+// NavigationContainer, so the two selection hooks are stubbed instead of mounting the whole provider tree.
+jest.mock('@components/Search/SearchContext', () => ({
+    ...jest.requireActual<typeof SearchContext>('@components/Search/SearchContext'),
+    useSearchSelectionContext: () => ({selectedTransactionIDs: []}),
+    useSearchSelectionActions: () => ({setSelectedTransactions: () => {}, clearSelectedTransactions: () => {}}),
+}));
+
 jest.mock('@components/WideRHPContextProvider', () => ({
     useWideRHPActions: () => ({markReportRHPWidth: jest.fn(), unmarkReportRHPWidth: jest.fn()}),
 }));
 
 jest.mock('@components/OnyxListItemProvider', () => ({
+    __esModule: true,
+    // The provider itself only seeds context the components under test read through the mocked hook below, so it is a
+    // passthrough here. It still has to be a component: the transaction-list tests compose it as a provider.
+    default: ({children}: {children: React.ReactNode}) => children,
     usePersonalDetails: () => ({}),
 }));
 
@@ -177,3 +240,136 @@ describe('MoneyRequestReportTransactionsNavigation', () => {
         expect(openReportSpy).not.toHaveBeenCalled();
     });
 });
+
+const EXPENSE_REPORT_ID = 'expense1';
+const POLICY_ID = 'policy1';
+
+function buildExpenseReportTransaction(transactionID: string, created: string, category: string, index: number): Transaction {
+    return {...createRandomTransaction(index), transactionID, reportID: EXPENSE_REPORT_ID, created, category, currency: 'USD', amount: -1000};
+}
+
+function buildTransactionListElement(transactions: Transaction[], reportID: string) {
+    const report = {
+        reportID,
+        policyID: POLICY_ID,
+        type: CONST.REPORT.TYPE.EXPENSE,
+        currency: 'USD',
+        ownerAccountID: 1,
+    } as StableReport;
+
+    return (
+        <ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider]}>
+            <MoneyRequestReportTransactionList
+                report={report}
+                transactions={transactions}
+                newTransactions={[]}
+                reportActions={[]}
+                hasComments={false}
+                visibleReportActions={[]}
+                renderReportAction={() => <View />}
+                reportActionsExtraData={undefined}
+                linkedReportActionID={undefined}
+                listRef={null}
+                accessibilityLabel="transactions"
+                onListLayout={jest.fn()}
+                onScroll={jest.fn()}
+                onScrollBeginDrag={jest.fn()}
+                onContentSizeChange={jest.fn()}
+                onViewableItemsChanged={jest.fn()}
+                onEndReached={jest.fn()}
+                onStartReached={jest.fn()}
+                contentContainerStyle={undefined}
+                isLoadingInitialActions={false}
+            />
+        </ComposeProviders>
+    );
+}
+
+function pressDateHeader(sortOrder: SortOrder) {
+    // The controller types the header as a plain ReactElement, so the one prop these tests drive is narrowed here
+    // rather than by duplicating the controller's own type.
+    const columnHeader = mockUnifiedList.controller?.tableColumnHeader;
+    if (!React.isValidElement<{onSortPress?: (sortBy: string, sortOrder: SortOrder) => void}>(columnHeader)) {
+        throw new Error('the sortable column header did not render');
+    }
+    const onSortPress = columnHeader.props.onSortPress;
+    if (!onSortPress) {
+        throw new Error('the sortable column header rendered without onSortPress');
+    }
+    act(() => {
+        onSortPress(CONST.SEARCH.TABLE_COLUMNS.DATE, sortOrder);
+    });
+}
+
+function getRenderedTransactionIDs(): string[] {
+    const listItems = mockUnifiedList.controller?.transactionListItems ?? [];
+    return listItems.filter((item): item is Extract<TransactionListItemData, {type: 'transaction'}> => item.type === 'transaction').map((item) => item.transaction.transactionID);
+}
+
+async function getCarouselTransactionIDs(): Promise<string[] | undefined> {
+    return new Promise((resolve) => {
+        const connection = Onyx.connectWithoutView({
+            key: ONYXKEYS.TRANSACTION_THREAD_NAVIGATION_TRANSACTION_IDS,
+            callback: (ids) => {
+                Onyx.disconnect(connection);
+                resolve(ids);
+            },
+        });
+    });
+}
+
+// The RHP prev/next arrows walk the transaction IDs this list seeds, so the seed has to stay equal to the rows the
+// user is actually looking at. Sorting is the case that breaks it: the rows move within their groups, and if the seed
+// is built from anything other than the rendered groups (a flat date sort, say) "next" jumps to a row that isn't
+// below the current one on screen.
+describe('MoneyRequestReportTransactionList - RHP arrow order', () => {
+    // The newest expense is in the alphabetically-last category, so a flat date sort and the rendered order disagree:
+    // under Date DESC the newest expense (4, Travel) is not the first row, because the alphabetical Meals group still
+    // renders first.
+    const transactions = [
+        buildExpenseReportTransaction('1', '2026-09-15', 'Meals', 0),
+        buildExpenseReportTransaction('2', '2026-09-16', 'Travel', 1),
+        buildExpenseReportTransaction('3', '2026-09-17', 'Meals', 2),
+        buildExpenseReportTransaction('4', '2026-09-18', 'Travel', 3),
+    ];
+
+    beforeAll(() => {
+        Onyx.init({keys: ONYXKEYS});
+    });
+
+    beforeEach(async () => {
+        jest.clearAllMocks();
+        mockIsOffline.value = false;
+        mockUnifiedList.controller = undefined;
+        // The list only seeds the carousel while a transaction thread is open in the RHP.
+        mockFocusedRoute.value = {name: SCREENS.RIGHT_MODAL.SEARCH_REPORT, key: 'rhp'};
+        await Onyx.clear();
+        await clearActiveTransactionIDs();
+        await Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.DEFAULT);
+        await waitForBatchedUpdates();
+    });
+
+    afterEach(() => {
+        mockFocusedRoute.value = undefined;
+    });
+
+    it('seeds the arrows with the rendered row order, and re-seeds it when a column is sorted', async () => {
+        render(buildTransactionListElement(transactions, EXPENSE_REPORT_ID));
+        await waitForBatchedUpdates();
+
+        // Date ascending, bucketed into alphabetical groups: Meals (1, 3) then Travel (2, 4). The arrows must follow
+        // that rendered order rather than the flat date order the rows were sorted into.
+        const initialRenderedOrder = getRenderedTransactionIDs();
+        expect(initialRenderedOrder).toEqual(['1', '3', '2', '4']);
+        expect(await getCarouselTransactionIDs()).toEqual(initialRenderedOrder);
+
+        pressDateHeader(CONST.SEARCH.SORT_ORDER.DESC);
+        await waitForBatchedUpdates();
+
+        const sortedRenderedOrder = getRenderedTransactionIDs();
+        // The rows reverse inside each group while the group headers stay alphabetical, so Meals (3, 1) still renders
+        // before Travel (4, 2). Group order is a separate axis from the column sort and is deliberately unaffected.
+        expect(sortedRenderedOrder).toEqual(['3', '1', '4', '2']);
+        expect(await getCarouselTransactionIDs()).toEqual(sortedRenderedOrder);
+    });
+});