From f10366a8f5670798ec7d070b0e03932283e3bbb9 Mon Sep 17 00:00:00 2001 From: "Kevin Brian Bader (via MelvinBot)" Date: Tue, 13 Oct 2026 19:52:19 +0000 Subject: [PATCH 1/2] Guard search snapshot data reads that crash while a search is loading A new search's snapshot has only `search` until its results arrive, so `currentSearchResults?.data[...]` throws. Guard the remaining unguarded reads and cover the bulk actions case with a test. Co-authored-by: Kevin Brian Bader --- .../ReportActionItem/MoneyRequestView.tsx | 2 +- src/hooks/useMergeTransactions.ts | 2 +- .../Search/SearchReportsMergeReports.tsx | 2 +- tests/unit/hooks/useSearchBulkActionsTest.ts | 31 ++++++++++++++++++- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index fa9f09ab9d60..a9063417711a 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -229,7 +229,7 @@ function MoneyRequestView({ // When this component is used when merging from the search page, we might not have the parent report stored in the main collection const [parentReportFromOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`); - const parentReport = parentReportFromOnyx ?? currentSearchResults?.data[`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`]; + const parentReport = parentReportFromOnyx ?? currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`]; const [iouReportOwnerLogin] = usePersonalDetail(parentReport?.ownerAccountID, loginSelector); const [reportPolicyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${getNonEmptyStringOnyxID(parentReport?.policyID)}`); diff --git a/src/hooks/useMergeTransactions.ts b/src/hooks/useMergeTransactions.ts index 51d4230c0875..50c1b2d00af5 100644 --- a/src/hooks/useMergeTransactions.ts +++ b/src/hooks/useMergeTransactions.ts @@ -56,7 +56,7 @@ function getTransaction( return transaction; } - return currentSearchResults?.data[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? onyxTransaction; + return currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? onyxTransaction; } function useMergeTransactions({mergeTransaction}: UseMergeTransactionsProps): UseMergeTransactionsReturn { diff --git a/src/pages/Search/SearchReportsMergeReports.tsx b/src/pages/Search/SearchReportsMergeReports.tsx index 1f7fff4c4167..33cb628dbf47 100644 --- a/src/pages/Search/SearchReportsMergeReports.tsx +++ b/src/pages/Search/SearchReportsMergeReports.tsx @@ -223,7 +223,7 @@ function SearchMergeReports() { isLoadingOnyxData || selectedReportIDs?.some((reportID) => { const reportKey = `${ONYXKEYS.COLLECTION.REPORT}${reportID}` as const; - return !allReports?.[reportKey] && !!currentSearchResults?.data[reportKey]; + return !allReports?.[reportKey] && !!currentSearchResults?.data?.[reportKey]; }); if (!isReportMergeBetaEnabled) { diff --git a/tests/unit/hooks/useSearchBulkActionsTest.ts b/tests/unit/hooks/useSearchBulkActionsTest.ts index 369ca3dfc0d2..441409888e8e 100644 --- a/tests/unit/hooks/useSearchBulkActionsTest.ts +++ b/tests/unit/hooks/useSearchBulkActionsTest.ts @@ -168,7 +168,7 @@ let mockSelectedTransactions: SelectedTransactions = {}; let mockExcludedTransactions: SelectedTransactions = {}; let mockSelectedReports: SelectedReports[] = []; let mockAreAllMatchingItemsSelected = false; -let mockCurrentSearchResults: {search: {type: string}; data: Record} | undefined; +let mockCurrentSearchResults: {search: {type: string}; data?: Record} | undefined; jest.mock('@components/Search/SearchContext', () => ({ useSearchSelectionContext: () => ({ @@ -515,6 +515,35 @@ describe('useSearchBulkActions - CSV export flow', () => { }); }); + it('does not crash when selected reports remain while the snapshot has no data', async () => { + // Given a report is still selected while a new search's snapshot only has `search` (its optimistic loading state), + // because the selection is cleared from an effect that runs after this render + mockSelectedTransactions = {tx1: makeSelectedTransaction({reportID: 'report1'})}; + mockSelectedReports = [ + { + reportID: 'report1', + policyID: 'policy1', + action: CONST.SEARCH.ACTION_TYPES.VIEW, + canPay: false, + canApprove: false, + canSubmit: false, + canChangeApprover: false, + total: 100, + chatReportID: undefined, + }, + ]; + mockCurrentSearchResults = {search: {type: CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT}}; + + // When the bulk actions are created for an expense-report search + const {result} = renderHook(() => useSearchBulkActions({queryJSON: expenseReportQueryJSON}), {wrapper: OnyxListItemProvider}); + + // Then the hook renders its options instead of throwing, and Merge stays hidden until the results load + await waitFor(() => { + expect(result.current.headerButtonsOptions.length).toBeGreaterThan(0); + }); + expect(result.current.headerButtonsOptions.some((option) => option.value === CONST.SEARCH.BULK_ACTION_TYPES.MERGE_REPORTS)).toBe(false); + }); + it('handleBasicExport with manual selection does not track any export', async () => { mockAreAllMatchingItemsSelected = false; mockSelectedTransactions = {tx1: makeSelectedTransaction()}; From c30a0625d080a00f912bb836403aa17e7c3582b3 Mon Sep 17 00:00:00 2001 From: "Kevin Brian Bader (via MelvinBot)" Date: Wed, 14 Oct 2026 04:07:02 +0000 Subject: [PATCH 2/2] Add useMergeTransactions test for a snapshot that has no data yet Co-authored-by: Kevin Brian Bader --- tests/unit/hooks/useMergeTransactionsTest.ts | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/unit/hooks/useMergeTransactionsTest.ts diff --git a/tests/unit/hooks/useMergeTransactionsTest.ts b/tests/unit/hooks/useMergeTransactionsTest.ts new file mode 100644 index 000000000000..b5fc82f4d9d0 --- /dev/null +++ b/tests/unit/hooks/useMergeTransactionsTest.ts @@ -0,0 +1,59 @@ +import {renderHook, waitFor} from '@testing-library/react-native'; + +import useMergeTransactions from '@hooks/useMergeTransactions'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {MergeTransaction, SearchResults} from '@src/types/onyx'; + +import Onyx from 'react-native-onyx'; + +import createRandomTransaction from '../../utils/collections/transaction'; +import createMock from '../../utils/createMock'; + +let mockCurrentSearchResults: SearchResults | undefined; + +jest.mock('@components/Search/SearchContext', () => ({ + useSearchQueryContext: () => ({ + currentSearchHash: 12345, + }), + useSearchResultsContext: () => ({ + currentSearchResults: mockCurrentSearchResults, + }), +})); + +jest.mock('@hooks/usePolicyForMovingExpenses', () => ({ + __esModule: true, + default: () => ({policyForMovingExpensesID: undefined}), +})); + +describe('useMergeTransactions', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + await Onyx.clear(); + mockCurrentSearchResults = undefined; + }); + + it('falls back to the Onyx transactions while the search snapshot has no data yet', async () => { + // Given a merge started from search while the new search's snapshot only has `search` (optimistic loading state), + // with transactions that exist in Onyx but not in the merge draft, so the hook looks them up in the snapshot first + mockCurrentSearchResults = createMock({search: {type: CONST.SEARCH.DATA_TYPES.EXPENSE}}); + const onyxTransaction1 = createRandomTransaction(1); + const onyxTransaction2 = createRandomTransaction(2); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}1`, onyxTransaction1); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}2`, onyxTransaction2); + const mergeTransaction = createMock({targetTransactionID: '1', sourceTransactionID: '2'}); + + // When the hook resolves the target and source transactions + const {result} = renderHook(() => useMergeTransactions({mergeTransaction})); + + // Then it falls back to the Onyx transactions instead of throwing on the missing snapshot data + await waitFor(() => { + expect(result.current.targetTransaction).toEqual(onyxTransaction1); + expect(result.current.sourceTransaction).toEqual(onyxTransaction2); + }); + }); +});