diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx index 2ce48fa2bba1..f649e4aeb6fa 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx +++ b/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 = {}; + + // 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(); 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(); - 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]); diff --git a/tests/ui/MoneyRequestReportTransactionsNavigationTest.tsx b/tests/ui/MoneyRequestReportTransactionsNavigationTest.tsx index 453ecba4ef86..533a314e0765 100644 --- a/tests/ui/MoneyRequestReportTransactionsNavigationTest.tsx +++ b/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; + +// 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('@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 ( + + } + 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} + /> + + ); +} + +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 => item.type === 'transaction').map((item) => item.transaction.transactionID); +} + +async function getCarouselTransactionIDs(): Promise { + 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); + }); +});