From 42dc3ab6fbc0fbfff73d3cad6278584132b1b126 Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Tue, 14 Jul 2026 14:24:27 -0500 Subject: [PATCH 1/7] fix: Total spend footer does not always appear when all expenses are selected --- src/components/Search/index.tsx | 4 +- src/hooks/useSearchShouldCalculateTotals.ts | 32 ++++++++-- src/libs/actions/Search.ts | 2 +- .../Search/searchTotalsLoadingDataTest.ts | 25 ++++++++ .../useSearchShouldCalculateTotals.test.ts | 63 +++++++++++++++++++ 5 files changed, 120 insertions(+), 6 deletions(-) diff --git a/src/components/Search/index.tsx b/src/components/Search/index.tsx index d827d2157fa2..6506f3c32e3a 100644 --- a/src/components/Search/index.tsx +++ b/src/components/Search/index.tsx @@ -180,7 +180,9 @@ function Search({ const [, cardFeedsResult] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); const searchDataType = useMemo(() => (shouldUseLiveData ? CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT : searchResults?.search?.type), [shouldUseLiveData, searchResults?.search?.type]); - const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, hash, offset === 0, areAllMatchingItemsSelected); + // This drives the search request, so it latches until the query changes — deselecting must not re-run the + // search effect below with totals off and wipe the totals we already have. + const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, hash, offset === 0, areAllMatchingItemsSelected, true); const previousReportActions = usePrevious(reportActions); const {translate} = useLocalize(); diff --git a/src/hooks/useSearchShouldCalculateTotals.ts b/src/hooks/useSearchShouldCalculateTotals.ts index bfbac7546488..506220db2a6a 100644 --- a/src/hooks/useSearchShouldCalculateTotals.ts +++ b/src/hooks/useSearchShouldCalculateTotals.ts @@ -3,19 +3,43 @@ import type {SearchKey} from '@libs/SearchUIUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import {useMemo} from 'react'; +import {useMemo, useState} from 'react'; import useOnyx from './useOnyx'; -function useSearchShouldCalculateTotals(searchKey: SearchKey | undefined, searchHash: number | undefined, enabled: boolean, areAllMatchingItemsSelected = false) { +/** + * @param shouldKeepTotalsUntilQueryChanges Opt-in for callers that drive the search *request* rather than + * just rendering totals. See the latch below for why the request side needs it and the display side doesn't. + */ +function useSearchShouldCalculateTotals( + searchKey: SearchKey | undefined, + searchHash: number | undefined, + enabled: boolean, + areAllMatchingItemsSelected = false, + shouldKeepTotalsUntilQueryChanges = false, +) { const [savedSearches] = useOnyx(ONYXKEYS.SAVED_SEARCHES); + const [latchedHash, setLatchedHash] = useState(undefined); + let nextLatchedHash: number | undefined; + if (areAllMatchingItemsSelected) { + nextLatchedHash = searchHash; + } else if (latchedHash === searchHash) { + nextLatchedHash = latchedHash; + } + // Leaving `nextLatchedHash` undefined above drops the latch once we move to a different query, so an + // unrelated ad-hoc search doesn't inherit it. + if (shouldKeepTotalsUntilQueryChanges && nextLatchedHash !== latchedHash) { + setLatchedHash(nextLatchedHash); + } + const wasAllMatchingItemsSelectedForQuery = shouldKeepTotalsUntilQueryChanges && nextLatchedHash !== undefined && nextLatchedHash === searchHash; + const shouldCalculateTotals = useMemo(() => { // When the user selects all matching items we always want the server-computed count/total, // even for an ad-hoc query that isn't a suggested or saved search. This must bypass the // `enabled` (offset === 0) gate so totals are still requested when more results were loaded // before select-all was triggered. - if (areAllMatchingItemsSelected) { + if (areAllMatchingItemsSelected || wasAllMatchingItemsSelectedForQuery) { return true; } @@ -46,7 +70,7 @@ function useSearchShouldCalculateTotals(searchKey: SearchKey | undefined, search const isSavedSearch = searchHash !== undefined && savedSearches && !!savedSearches[searchHash]; return isSuggestedSearchWithTotals || isSavedSearch; - }, [enabled, savedSearches, searchKey, searchHash, areAllMatchingItemsSelected]); + }, [enabled, savedSearches, searchKey, searchHash, areAllMatchingItemsSelected, wasAllMatchingItemsSelectedForQuery]); return shouldCalculateTotals ?? false; } diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index a4c17be3fcfd..11b7a89df73c 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -912,7 +912,7 @@ function search({ return; } - const dedupeKey = `${queryJSON.hash}_${offset ?? 0}`; + const dedupeKey = `${queryJSON.hash}_${offset ?? 0}_${shouldCalculateTotals}`; if (inFlightSearchRequests.has(dedupeKey)) { return; } diff --git a/tests/unit/Search/searchTotalsLoadingDataTest.ts b/tests/unit/Search/searchTotalsLoadingDataTest.ts index 3bb2877c8852..0246086e017e 100644 --- a/tests/unit/Search/searchTotalsLoadingDataTest.ts +++ b/tests/unit/Search/searchTotalsLoadingDataTest.ts @@ -134,4 +134,29 @@ describe('search loading totals handling', () => { expect(loadingSearchData?.total).toBeUndefined(); expect(loadingSearchData?.currency).toBeUndefined(); }); + + describe('in-flight request deduping', () => { + // Both calls run before any microtask, so the first request is still registered as in flight when the + // second one is made. + function searchTwiceConcurrently(firstShouldCalculateTotals: boolean, secondShouldCalculateTotals: boolean) { + const queryJSON = getQueryJSON(); + const params = {queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false}; + + return Promise.all([search({...params, shouldCalculateTotals: firstShouldCalculateTotals}), search({...params, shouldCalculateTotals: secondShouldCalculateTotals})]); + } + + it('drops a request that duplicates an in-flight one for the same page', async () => { + await searchTwiceConcurrently(false, false); + + expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(1); + }); + + it('does not drop a totals request that overlaps an in-flight request which did not ask for totals', async () => { + // The overlapping request asks for strictly more data, so deduping it away would leave the totals + // unfetched with nothing to trigger a retry. + await searchTwiceConcurrently(false, true); + + expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts b/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts index 4ec2528a44a3..14c01b635b5a 100644 --- a/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts +++ b/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts @@ -88,4 +88,67 @@ describe('useSearchShouldCalculateTotals', () => { expect(result.current).toBe(true); }); + + describe('shouldKeepTotalsUntilQueryChanges', () => { + it('stays true after all matching items are deselected, so the search is not re-run with totals off', () => { + const {result, rerender} = renderHook( + ({areAllMatchingItemsSelected}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, areAllMatchingItemsSelected, true), + { + initialProps: {areAllMatchingItemsSelected: true}, + }, + ); + + expect(result.current).toBe(true); + + rerender({areAllMatchingItemsSelected: false}); + + expect(result.current).toBe(true); + }); + + it('does not latch when the caller has not opted in, so the footer gate still follows the selection', () => { + const {result, rerender} = renderHook( + ({areAllMatchingItemsSelected}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, areAllMatchingItemsSelected), + { + initialProps: {areAllMatchingItemsSelected: true}, + }, + ); + + expect(result.current).toBe(true); + + rerender({areAllMatchingItemsSelected: false}); + + expect(result.current).toBe(false); + }); + + it('drops the latch when the query changes, so a new ad-hoc search does not inherit it', () => { + const {result, rerender} = renderHook(({searchHash}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, searchHash, true, searchHash === 123, true), { + initialProps: {searchHash: 123}, + }); + + expect(result.current).toBe(true); + + rerender({searchHash: 456}); + + expect(result.current).toBe(false); + }); + + it('does not keep totals on for paginated loads of an eligible search, so the latch cannot defeat the offset gate', () => { + const {result, rerender} = renderHook(({enabled}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.SUBMIT, 123, enabled, false, true), { + initialProps: {enabled: true}, + }); + + expect(result.current).toBe(true); + + // `enabled` is `offset === 0`, so this is the user loading a second page. + rerender({enabled: false}); + + expect(result.current).toBe(false); + }); + + it('does not latch a query that never asked for totals', () => { + const {result} = renderHook(() => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, false, true)); + + expect(result.current).toBe(false); + }); + }); }); From b9f2f4cc9e8c88888bad28e7be8026fd929d11cf Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Tue, 14 Jul 2026 14:36:54 -0500 Subject: [PATCH 2/7] fix: spell --- tests/unit/Search/searchTotalsLoadingDataTest.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/Search/searchTotalsLoadingDataTest.ts b/tests/unit/Search/searchTotalsLoadingDataTest.ts index 0246086e017e..346ba73d13c5 100644 --- a/tests/unit/Search/searchTotalsLoadingDataTest.ts +++ b/tests/unit/Search/searchTotalsLoadingDataTest.ts @@ -152,8 +152,6 @@ describe('search loading totals handling', () => { }); it('does not drop a totals request that overlaps an in-flight request which did not ask for totals', async () => { - // The overlapping request asks for strictly more data, so deduping it away would leave the totals - // unfetched with nothing to trigger a retry. await searchTwiceConcurrently(false, true); expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(2); From 030a4212707b86397c69ad2daad8f964e0fdc3a4 Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Tue, 28 Jul 2026 15:43:49 +0700 Subject: [PATCH 3/7] chore: remove lazy logic --- src/components/Search/index.tsx | 4 +- src/hooks/useSearchShouldCalculateTotals.ts | 32 ++-------- .../useSearchShouldCalculateTotals.test.ts | 63 ------------------- 3 files changed, 5 insertions(+), 94 deletions(-) diff --git a/src/components/Search/index.tsx b/src/components/Search/index.tsx index f20b82265ae1..429c8b9434b6 100644 --- a/src/components/Search/index.tsx +++ b/src/components/Search/index.tsx @@ -180,9 +180,7 @@ function Search({ const [, cardFeedsResult] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); const searchDataType = useMemo(() => (shouldUseLiveData ? CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT : searchResults?.search?.type), [shouldUseLiveData, searchResults?.search?.type]); - // This drives the search request, so it latches until the query changes — deselecting must not re-run the - // search effect below with totals off and wipe the totals we already have. - const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, hash, offset === 0, areAllMatchingItemsSelected, true); + const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, hash, offset === 0, areAllMatchingItemsSelected); const previousReportActions = usePrevious(reportActions); const {translate} = useLocalize(); diff --git a/src/hooks/useSearchShouldCalculateTotals.ts b/src/hooks/useSearchShouldCalculateTotals.ts index 506220db2a6a..bfbac7546488 100644 --- a/src/hooks/useSearchShouldCalculateTotals.ts +++ b/src/hooks/useSearchShouldCalculateTotals.ts @@ -3,43 +3,19 @@ import type {SearchKey} from '@libs/SearchUIUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import {useMemo, useState} from 'react'; +import {useMemo} from 'react'; import useOnyx from './useOnyx'; -/** - * @param shouldKeepTotalsUntilQueryChanges Opt-in for callers that drive the search *request* rather than - * just rendering totals. See the latch below for why the request side needs it and the display side doesn't. - */ -function useSearchShouldCalculateTotals( - searchKey: SearchKey | undefined, - searchHash: number | undefined, - enabled: boolean, - areAllMatchingItemsSelected = false, - shouldKeepTotalsUntilQueryChanges = false, -) { +function useSearchShouldCalculateTotals(searchKey: SearchKey | undefined, searchHash: number | undefined, enabled: boolean, areAllMatchingItemsSelected = false) { const [savedSearches] = useOnyx(ONYXKEYS.SAVED_SEARCHES); - const [latchedHash, setLatchedHash] = useState(undefined); - let nextLatchedHash: number | undefined; - if (areAllMatchingItemsSelected) { - nextLatchedHash = searchHash; - } else if (latchedHash === searchHash) { - nextLatchedHash = latchedHash; - } - // Leaving `nextLatchedHash` undefined above drops the latch once we move to a different query, so an - // unrelated ad-hoc search doesn't inherit it. - if (shouldKeepTotalsUntilQueryChanges && nextLatchedHash !== latchedHash) { - setLatchedHash(nextLatchedHash); - } - const wasAllMatchingItemsSelectedForQuery = shouldKeepTotalsUntilQueryChanges && nextLatchedHash !== undefined && nextLatchedHash === searchHash; - const shouldCalculateTotals = useMemo(() => { // When the user selects all matching items we always want the server-computed count/total, // even for an ad-hoc query that isn't a suggested or saved search. This must bypass the // `enabled` (offset === 0) gate so totals are still requested when more results were loaded // before select-all was triggered. - if (areAllMatchingItemsSelected || wasAllMatchingItemsSelectedForQuery) { + if (areAllMatchingItemsSelected) { return true; } @@ -70,7 +46,7 @@ function useSearchShouldCalculateTotals( const isSavedSearch = searchHash !== undefined && savedSearches && !!savedSearches[searchHash]; return isSuggestedSearchWithTotals || isSavedSearch; - }, [enabled, savedSearches, searchKey, searchHash, areAllMatchingItemsSelected, wasAllMatchingItemsSelectedForQuery]); + }, [enabled, savedSearches, searchKey, searchHash, areAllMatchingItemsSelected]); return shouldCalculateTotals ?? false; } diff --git a/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts b/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts index 14c01b635b5a..4ec2528a44a3 100644 --- a/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts +++ b/tests/unit/hooks/useSearchShouldCalculateTotals.test.ts @@ -88,67 +88,4 @@ describe('useSearchShouldCalculateTotals', () => { expect(result.current).toBe(true); }); - - describe('shouldKeepTotalsUntilQueryChanges', () => { - it('stays true after all matching items are deselected, so the search is not re-run with totals off', () => { - const {result, rerender} = renderHook( - ({areAllMatchingItemsSelected}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, areAllMatchingItemsSelected, true), - { - initialProps: {areAllMatchingItemsSelected: true}, - }, - ); - - expect(result.current).toBe(true); - - rerender({areAllMatchingItemsSelected: false}); - - expect(result.current).toBe(true); - }); - - it('does not latch when the caller has not opted in, so the footer gate still follows the selection', () => { - const {result, rerender} = renderHook( - ({areAllMatchingItemsSelected}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, areAllMatchingItemsSelected), - { - initialProps: {areAllMatchingItemsSelected: true}, - }, - ); - - expect(result.current).toBe(true); - - rerender({areAllMatchingItemsSelected: false}); - - expect(result.current).toBe(false); - }); - - it('drops the latch when the query changes, so a new ad-hoc search does not inherit it', () => { - const {result, rerender} = renderHook(({searchHash}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, searchHash, true, searchHash === 123, true), { - initialProps: {searchHash: 123}, - }); - - expect(result.current).toBe(true); - - rerender({searchHash: 456}); - - expect(result.current).toBe(false); - }); - - it('does not keep totals on for paginated loads of an eligible search, so the latch cannot defeat the offset gate', () => { - const {result, rerender} = renderHook(({enabled}) => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.SUBMIT, 123, enabled, false, true), { - initialProps: {enabled: true}, - }); - - expect(result.current).toBe(true); - - // `enabled` is `offset === 0`, so this is the user loading a second page. - rerender({enabled: false}); - - expect(result.current).toBe(false); - }); - - it('does not latch a query that never asked for totals', () => { - const {result} = renderHook(() => useSearchShouldCalculateTotals(CONST.SEARCH.SEARCH_KEYS.EXPENSES, 123, true, false, true)); - - expect(result.current).toBe(false); - }); - }); }); From 8cbe7b00fcc457797e450aa9442011a37d4cacb6 Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Tue, 28 Jul 2026 17:14:52 +0700 Subject: [PATCH 4/7] fix: glitch in loading indicator --- .../Search/SearchBulkActionsButton.tsx | 3 +- src/libs/actions/Search.ts | 40 ++++++++++++---- tests/unit/Search/searchSnapshotStateTest.ts | 6 ++- .../Search/searchTotalsLoadingDataTest.ts | 46 +++++++++++++++++++ 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/components/Search/SearchBulkActionsButton.tsx b/src/components/Search/SearchBulkActionsButton.tsx index 6a99cc26620e..a9d662739445 100644 --- a/src/components/Search/SearchBulkActionsButton.tsx +++ b/src/components/Search/SearchBulkActionsButton.tsx @@ -126,7 +126,8 @@ function SearchBulkActionsButton({queryJSON}: SearchBulkActionsButtonProps) { }, [selectedTransactions, selectedTransactionsKeys, isExpenseReportType, searchData]); const allMatchingItemsCount = currentSearchResults?.search?.count; - const isAllMatchingItemsCountLoading = areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !!currentSearchResults?.search?.isLoading; + const hasSearchErrors = Object.keys(currentSearchResults?.errors ?? {}).length > 0; + const isAllMatchingItemsCountLoading = areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !hasSearchErrors; let selectionButtonText: string; if (areAllMatchingItemsSelected) { selectionButtonText = diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index cd1147636c54..2e9358e17020 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -957,11 +957,25 @@ function openBulkChangeApproverPage(reportIDList: OpenBulkChangeApproverPagePara write(WRITE_COMMANDS.OPEN_BULK_CHANGE_APPROVER_PAGE, {reportIDList}, {optimisticData, successData}); } -// Tracks in-flight search requests by hash+offset to prevent duplicate API calls +// Tracks in-flight search requests by hash+offset+totals to prevent duplicate API calls // when both page-level (useSearchPageSetup) and Search-internal (handleSearch) effects // fire for the same query. Cleared when the request completes. const inFlightSearchRequests = new Set(); +/** + * Every request for a given hash writes the same snapshot, so `search.isLoading` belongs to the snapshot + * rather than to any single request. Used to keep it set until the last overlapping request settles. + */ +function hasInFlightSearchRequestForHash(hash: number) { + const prefix = `${hash}_`; + for (const key of inFlightSearchRequests) { + if (key.startsWith(prefix)) { + return true; + } + } + return false; +} + let shouldPreventSearchAPI = false; function handlePreventSearchAPI(hash: number | undefined) { if (typeof hash === 'undefined') { @@ -1016,13 +1030,14 @@ function search({ return; } - const dedupeKey = `${queryJSON.hash}_${offset ?? 0}_${shouldCalculateTotals}`; - if (inFlightSearchRequests.has(dedupeKey)) { + const pageKey = `${queryJSON.hash}_${offset ?? 0}`; + const dedupeKey = `${pageKey}_${shouldCalculateTotals ? 'totals' : 'noTotals'}`; + if (inFlightSearchRequests.has(dedupeKey) || (!shouldCalculateTotals && inFlightSearchRequests.has(`${pageKey}_totals`))) { return; } inFlightSearchRequests.add(dedupeKey); - const {optimisticData, successData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals); + const {optimisticData, successData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals); const {exactMatchFilterKeys, flatFilters, limit, ...queryJSONWithoutFlatFilters} = queryJSON; const backendQueryJSON = shouldUseBackendDateSortFallback(queryJSON.sortBy) ? { @@ -1057,7 +1072,7 @@ function search({ } const startRequest = () => - makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, successData, finallyData, failureData}) + makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, successData, failureData}) .then((result) => { const response = result?.onyxData?.[0]?.value as OnyxSearchResponse; @@ -1103,11 +1118,16 @@ function search({ // this still rejects for any caller relying on that. Onyx.update(failureData ?? []); throw error; - }) - .finally(() => { - inFlightSearchRequests.delete(dedupeKey); }); + const releaseRequest = () => { + inFlightSearchRequests.delete(dedupeKey); + + if (!hasInFlightSearchRequestForHash(queryJSON.hash)) { + Onyx.merge(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}`, {search: {isLoading: false}}); + } + }; + // Catch here so every caller (the page-load fire in useSearchPageSetup and the re-search handlers // in SearchPage/SearchPageNarrow) is covered without a separate catch each. Failure state is already // applied via failureData, so this only prevents the rejection from floating into the browser's @@ -1119,10 +1139,10 @@ function search({ }; if (skipWaitForWrites) { - return startRequest().catch(handleSearchError); + return startRequest().catch(handleSearchError).finally(releaseRequest); } - return waitForWrites(READ_COMMANDS.SEARCH).then(startRequest).catch(handleSearchError); + return waitForWrites(READ_COMMANDS.SEARCH).then(startRequest).catch(handleSearchError).finally(releaseRequest); } function submitMoneyRequestOnSearch( diff --git a/tests/unit/Search/searchSnapshotStateTest.ts b/tests/unit/Search/searchSnapshotStateTest.ts index 8d1fea243e57..e4f6cad8be04 100644 --- a/tests/unit/Search/searchSnapshotStateTest.ts +++ b/tests/unit/Search/searchSnapshotStateTest.ts @@ -44,6 +44,10 @@ function getCapturedSearchOnyxData(): NonNullable>}) { const {optimisticData, successData, failureData, finallyData} = getCapturedSearchOnyxData(); @@ -113,7 +117,7 @@ describe('search snapshot terminal state', () => { await simulateResolvedRequest({jsonCode: FAILURE_JSON_CODE}); const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const); - // finallyData runs after failureData; the error state must survive it. + // The error state is terminal and must survive the rest of the response application order. expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.ERROR); expect(snapshot?.search?.hash).toBe(queryJSON.hash); }); diff --git a/tests/unit/Search/searchTotalsLoadingDataTest.ts b/tests/unit/Search/searchTotalsLoadingDataTest.ts index 346ba73d13c5..469c6a08436f 100644 --- a/tests/unit/Search/searchTotalsLoadingDataTest.ts +++ b/tests/unit/Search/searchTotalsLoadingDataTest.ts @@ -6,6 +6,8 @@ import {buildSearchQueryJSON} from '@libs/SearchQueryUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import Onyx from 'react-native-onyx'; + jest.mock('@libs/API', () => ({ makeRequestWithSideEffects: jest.fn(), waitForWrites: jest.fn(), @@ -61,6 +63,16 @@ function getSearchLoadingUpdateForHash(hash: number) { return optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}` && !!update.value?.search?.isLoading)?.value?.search; } +/** Narrows an Onyx.merge payload to the `{search: {isLoading: false}}` write that settles a snapshot. */ +function isLoadingClear(value: unknown) { + if (typeof value !== 'object' || value === null || !('search' in value)) { + return false; + } + + const {search: searchValue} = value; + return typeof searchValue === 'object' && searchValue !== null && 'isLoading' in searchValue && searchValue.isLoading === false; +} + describe('search loading totals handling', () => { beforeEach(() => { jest.clearAllMocks(); @@ -156,5 +168,39 @@ describe('search loading totals handling', () => { expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(2); }); + + it('drops a non-totals request while a totals request for the same page is in flight', async () => { + await searchTwiceConcurrently(true, false); + + expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(1); + }); + + it('drops a totals request that duplicates an in-flight totals request', async () => { + await searchTwiceConcurrently(true, true); + + expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(1); + }); + + it('does not hand the API layer a finallyData that would settle the snapshot mid-flight', async () => { + const queryJSON = getQueryJSON(); + + await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, shouldCalculateTotals: true, isLoading: false}); + + const [, , requestData] = getMakeRequestWithSideEffectsMock().mock.calls.at(-1) ?? []; + expect(requestData).not.toHaveProperty('finallyData'); + }); + + it('clears the snapshot loading state once the last overlapping request settles', async () => { + const queryJSON = getQueryJSON(); + const onyxMergeSpy = jest.spyOn(Onyx, 'merge'); + + await searchTwiceConcurrently(false, true); + + const loadingClears = onyxMergeSpy.mock.calls.filter(([key, value]) => key === `${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` && isLoadingClear(value)); + + // Two requests overlap on one snapshot, but only the one that finishes last may report it as settled. + expect(loadingClears).toHaveLength(1); + onyxMergeSpy.mockRestore(); + }); }); }); From 43a2c44fcdb9afb078f45c2525ab0592ba747ed4 Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Sun, 2 Aug 2026 13:21:06 +0700 Subject: [PATCH 5/7] chore: remove copies --- .../Search/SearchBulkActionsButton.tsx | 12 ++--- src/languages/de.ts | 1 - src/languages/en.ts | 1 - src/languages/es.ts | 1 - src/languages/fr.ts | 1 - src/languages/it.ts | 1 - src/languages/ja.ts | 2 +- src/languages/nl.ts | 1 - src/languages/pl.ts | 1 - src/languages/pt-BR.ts | 1 - src/languages/zh-hans.ts | 2 +- src/libs/actions/Search.ts | 40 +++------------ tests/unit/Search/searchSnapshotStateTest.ts | 4 -- .../Search/searchTotalsLoadingDataTest.ts | 50 +++---------------- 14 files changed, 20 insertions(+), 98 deletions(-) diff --git a/src/components/Search/SearchBulkActionsButton.tsx b/src/components/Search/SearchBulkActionsButton.tsx index 0b3d249d4955..43c4d1a898de 100644 --- a/src/components/Search/SearchBulkActionsButton.tsx +++ b/src/components/Search/SearchBulkActionsButton.tsx @@ -135,14 +135,12 @@ function SearchBulkActionsButton({queryJSON}: SearchBulkActionsButtonProps) { const allMatchingItemsCount = currentSearchResults?.search?.count; const hasSearchErrors = Object.keys(currentSearchResults?.errors ?? {}).length > 0; + // The server count is the only source for how many items "select all" covers, so keep the button loading until it + // arrives. Offline or on error it never will, so fall back to the count of the items we do have selected. const isAllMatchingItemsCountLoading = areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !hasSearchErrors; - let selectionButtonText: string; - if (areAllMatchingItemsSelected) { - selectionButtonText = - typeof allMatchingItemsCount !== 'number' ? translate('search.exportAll.allMatchingItemsSelected') : translate('workspace.common.selected', {count: allMatchingItemsCount}); - } else { - selectionButtonText = translate('workspace.common.selected', {count: selectedItemsCount}); - } + const selectionButtonText = translate('workspace.common.selected', { + count: areAllMatchingItemsSelected && typeof allMatchingItemsCount === 'number' ? allMatchingItemsCount : selectedItemsCount, + }); return ( <> diff --git a/src/languages/de.ts b/src/languages/de.ts index 7985462fac08..6846c1653d99 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -9144,7 +9144,6 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc exportedTo: 'Exportiert nach', exportAll: { selectAllMatchingItems: 'Alle passenden Einträge auswählen', - allMatchingItemsSelected: 'Alle passenden Elemente ausgewählt', selectAllOnThisPage: 'Alle auf dieser Seite auswählen', }, errors: { diff --git a/src/languages/en.ts b/src/languages/en.ts index 076f253dd641..db300d7d3f1d 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -9289,7 +9289,6 @@ const translations = { exportedTo: 'Exported to', exportAll: { selectAllMatchingItems: 'Select all matching items', - allMatchingItemsSelected: 'All matching items selected', selectAllOnThisPage: 'Select all on this page', }, errors: { diff --git a/src/languages/es.ts b/src/languages/es.ts index 8aafd38e5a39..440c8c431280 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -8953,7 +8953,6 @@ El plan Controlar empieza en 9 $ por miembro activo al mes.`, exportedTo: 'Exported to', exportAll: { selectAllMatchingItems: 'Seleccionar todos los elementos coincidentes', - allMatchingItemsSelected: 'Todos los elementos coincidentes seleccionados', selectAllOnThisPage: 'Seleccionar todo en esta página', }, errors: { diff --git a/src/languages/fr.ts b/src/languages/fr.ts index 03fc902ca84c..8d0ff0a1b6f3 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -9180,7 +9180,6 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e exportedTo: 'Exporté vers', exportAll: { selectAllMatchingItems: 'Sélectionnez tous les éléments correspondants', - allMatchingItemsSelected: 'Tous les éléments correspondants sont sélectionnés', selectAllOnThisPage: 'Tout sélectionner sur cette page', }, errors: { diff --git a/src/languages/it.ts b/src/languages/it.ts index d5d143f1be5f..f7efd92c6823 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -9121,7 +9121,6 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`, exportedTo: 'Esportato in', exportAll: { selectAllMatchingItems: 'Seleziona tutti gli elementi corrispondenti', - allMatchingItemsSelected: 'Tutti gli elementi corrispondenti selezionati', selectAllOnThisPage: 'Seleziona tutto in questa pagina', }, errors: { diff --git a/src/languages/ja.ts b/src/languages/ja.ts index 22bdedf0bf98..f89758874338 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -8997,7 +8997,7 @@ ${reportName}`, description: 'おっと、アイテムがたくさんありますね！まとめて整理して、間もなくConciergeからファイルをお送りします。', }, exportedTo: 'エクスポート先', - exportAll: {selectAllMatchingItems: '一致する項目をすべて選択', allMatchingItemsSelected: '一致する項目をすべて選択済み', selectAllOnThisPage: 'このページのすべてを選択'}, + exportAll: {selectAllMatchingItems: '一致する項目をすべて選択', selectAllOnThisPage: 'このページのすべてを選択'}, errors: { pleaseSelectDatesForBothFromAndTo: '開始日と終了日の両方を選択してください', }, diff --git a/src/languages/nl.ts b/src/languages/nl.ts index 2b7e77d15664..262bb06cdffa 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -9080,7 +9080,6 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`, exportedTo: 'Geëxporteerd naar', exportAll: { selectAllMatchingItems: 'Selecteer alle overeenkomende items', - allMatchingItemsSelected: 'Alle overeenkomende items geselecteerd', selectAllOnThisPage: 'Selecteer alles op deze pagina', }, errors: { diff --git a/src/languages/pl.ts b/src/languages/pl.ts index bb4db669b8d7..c864f2a2196f 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -9069,7 +9069,6 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`, exportedTo: 'Wyeksportowano do', exportAll: { selectAllMatchingItems: 'Zaznacz wszystkie pasujące elementy', - allMatchingItemsSelected: 'Zaznaczono wszystkie pasujące elementy', selectAllOnThisPage: 'Zaznacz wszystko na tej stronie', }, errors: { diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index 550ac8da144d..acb5be4c0203 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -9069,7 +9069,6 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`, exportedTo: 'Exportado para', exportAll: { selectAllMatchingItems: 'Selecionar todos os itens correspondentes', - allMatchingItemsSelected: 'Todos os itens correspondentes selecionados', selectAllOnThisPage: 'Selecionar tudo nesta página', }, chartTitles: { diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index bd140d2ad548..154712d7d79f 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -8774,7 +8774,7 @@ ${reportName}`, description: '哇，项目真不少！我们会把它们打包好，Concierge 很快就会给你发送一个文件。', }, exportedTo: '已导出到', - exportAll: {selectAllMatchingItems: '选择所有匹配的项目', allMatchingItemsSelected: '已选择所有匹配的项目', selectAllOnThisPage: '选择本页全部内容'}, + exportAll: {selectAllMatchingItems: '选择所有匹配的项目', selectAllOnThisPage: '选择本页全部内容'}, errors: { pleaseSelectDatesForBothFromAndTo: '请选择起始和结束日期', }, diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index 297c87fc0941..00c37b811730 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -680,11 +680,6 @@ function getOnyxLoadingData( isOffline?: boolean, isSearchAPI = false, shouldCalculateTotals?: boolean, - /** - * Whether finallyData may clear `search.isLoading`. search() opts out because several requests can share one - * snapshot, so it settles the loading flag itself once the last of them finishes. - */ - shouldSettleLoading = true, ): OnyxData { const shouldClearTotals = isSearchAPI && shouldCalculateTotals === false && offset === 0; @@ -733,7 +728,7 @@ function getOnyxLoadingData( key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`, value: { search: { - ...(isSearchAPI && shouldSettleLoading && {isLoading: false}), + ...(isSearchAPI && {isLoading: false}), ...(isSearchRequest && {state: CONST.SEARCH.SNAPSHOT_STATE.LOADED, type, hash}), }, }, @@ -971,20 +966,6 @@ function openBulkChangeApproverPage(reportIDList: OpenBulkChangeApproverPagePara // fire for the same query. Cleared when the request completes. const inFlightSearchRequests = new Set(); -/** - * Every request for a given hash writes the same snapshot, so `search.isLoading` belongs to the snapshot - * rather than to any single request. Used to keep it set until the last overlapping request settles. - */ -function hasInFlightSearchRequestForHash(hash: number) { - const prefix = `${hash}_`; - for (const key of inFlightSearchRequests) { - if (key.startsWith(prefix)) { - return true; - } - } - return false; -} - let shouldPreventSearchAPI = false; function handlePreventSearchAPI(hash: number | undefined) { if (typeof hash === 'undefined') { @@ -1068,9 +1049,7 @@ function search({ } inFlightSearchRequests.add(dedupeKey); - // finallyData still carries the terminal `state`, but not the `isLoading` clear: releaseRequest owns that so an - // early-finishing request cannot report the shared snapshot as settled while another one is still in flight. - const {optimisticData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals, false); + const {optimisticData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals); const {backendQueryJSON, limit, exactMatchFilterKeys} = getBackendQueryJSON(queryJSON); const query = { ...backendQueryJSON, @@ -1138,16 +1117,11 @@ function search({ await Onyx.update(failureData ?? []); await Onyx.update(finallyData ?? []); throw error; + }) + .finally(() => { + inFlightSearchRequests.delete(dedupeKey); }); - const releaseRequest = () => { - inFlightSearchRequests.delete(dedupeKey); - - if (!hasInFlightSearchRequestForHash(queryJSON.hash)) { - Onyx.merge(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}`, {search: {isLoading: false}}); - } - }; - // Catch here so every caller (the page-load fire in useSearchPageSetup and the re-search handlers // in SearchPage/SearchPageNarrow) is covered without a separate catch each. Failure and terminal state // are already applied, so this only prevents the rejection from floating into the browser's @@ -1159,10 +1133,10 @@ function search({ }; if (skipWaitForWrites) { - return startRequest().catch(handleSearchError).finally(releaseRequest); + return startRequest().catch(handleSearchError); } - return waitForWrites(READ_COMMANDS.SEARCH).then(startRequest).catch(handleSearchError).finally(releaseRequest); + return waitForWrites(READ_COMMANDS.SEARCH).then(startRequest).catch(handleSearchError); } /** diff --git a/tests/unit/Search/searchSnapshotStateTest.ts b/tests/unit/Search/searchSnapshotStateTest.ts index 3bfa15f33fc2..bf1b72dc394e 100644 --- a/tests/unit/Search/searchSnapshotStateTest.ts +++ b/tests/unit/Search/searchSnapshotStateTest.ts @@ -45,10 +45,6 @@ function getCapturedSearchOnyxData(): NonNullable>}) { const {optimisticData, successData, failureData, finallyData} = getCapturedSearchOnyxData(); diff --git a/tests/unit/Search/searchTotalsLoadingDataTest.ts b/tests/unit/Search/searchTotalsLoadingDataTest.ts index 641d7114f39f..7fe201f4f7fd 100644 --- a/tests/unit/Search/searchTotalsLoadingDataTest.ts +++ b/tests/unit/Search/searchTotalsLoadingDataTest.ts @@ -6,8 +6,6 @@ import {buildSearchQueryJSON} from '@libs/SearchQueryUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import Onyx from 'react-native-onyx'; - jest.mock('@libs/API', () => ({ makeRequestWithSideEffects: jest.fn(), waitForWrites: jest.fn(), @@ -32,16 +30,13 @@ type SearchLoadingState = { currency?: string | null; }; -type SearchOnyxUpdate = { - key: string; - value?: { - search?: SearchLoadingState; - }; -}; - type SearchRequestData = { - optimisticData?: SearchOnyxUpdate[]; - finallyData?: SearchOnyxUpdate[]; + optimisticData?: Array<{ + key: string; + value?: { + search?: SearchLoadingState; + }; + }>; }; type SearchRequestParams = { @@ -72,16 +67,6 @@ function getSearchLoadingUpdateForHash(hash: number) { return optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}` && !!update.value?.search?.isLoading)?.value?.search; } -/** Narrows an Onyx.merge payload to the `{search: {isLoading: false}}` write that settles a snapshot. */ -function isLoadingClear(value: unknown) { - if (typeof value !== 'object' || value === null || !('search' in value)) { - return false; - } - - const {search: searchValue} = value; - return typeof searchValue === 'object' && searchValue !== null && 'isLoading' in searchValue && searchValue.isLoading === false; -} - function getLastSearchRequestParams() { const makeRequestWithSideEffectsMock = getMakeRequestWithSideEffectsMock(); const [, requestParams] = makeRequestWithSideEffectsMock.mock.calls.at(-1) ?? []; @@ -199,29 +184,6 @@ describe('search loading totals handling', () => { expect(makeRequestWithSideEffects).toHaveBeenCalledTimes(1); }); - - it('does not hand the API layer a finallyData that would settle the snapshot mid-flight', async () => { - const queryJSON = getQueryJSON(); - - await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, shouldCalculateTotals: true, isLoading: false}); - - const [, , requestData] = getMakeRequestWithSideEffectsMock().mock.calls.at(-1) ?? []; - // finallyData still settles the terminal `state`, but only search() itself may clear `isLoading`. - expect(requestData?.finallyData?.some((update) => isLoadingClear(update.value))).toBe(false); - }); - - it('clears the snapshot loading state once the last overlapping request settles', async () => { - const queryJSON = getQueryJSON(); - const onyxMergeSpy = jest.spyOn(Onyx, 'merge'); - - await searchTwiceConcurrently(false, true); - - const loadingClears = onyxMergeSpy.mock.calls.filter(([key, value]) => key === `${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` && isLoadingClear(value)); - - // Two requests overlap on one snapshot, but only the one that finishes last may report it as settled. - expect(loadingClears).toHaveLength(1); - onyxMergeSpy.mockRestore(); - }); }); it('dedupes concurrent search requests by hash and offset', async () => { From 4c59dd72239f40b28c28449d1ea372871d30ac4d Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Wed, 5 Aug 2026 00:07:22 +0700 Subject: [PATCH 6/7] remove code comment --- src/components/Search/SearchBulkActionsButton.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/Search/SearchBulkActionsButton.tsx b/src/components/Search/SearchBulkActionsButton.tsx index 0fcb59fd7a07..2dc19f8678e1 100644 --- a/src/components/Search/SearchBulkActionsButton.tsx +++ b/src/components/Search/SearchBulkActionsButton.tsx @@ -136,8 +136,6 @@ function SearchBulkActionsButton({queryJSON}: SearchBulkActionsButtonProps) { const allMatchingItemsCount = currentSearchResults?.search?.count; const hasSearchErrors = Object.keys(currentSearchResults?.errors ?? {}).length > 0; - // The server count is the only source for how many items "select all" covers, so keep the button loading until it - // arrives. Offline or on error it never will, so fall back to the count of the items we do have selected. const isAllMatchingItemsCountLoading = areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !hasSearchErrors; const selectionButtonText = translate('workspace.common.selected', { count: areAllMatchingItemsSelected && typeof allMatchingItemsCount === 'number' ? allMatchingItemsCount : selectedItemsCount, From 977a743e1db7e3098b0e480dd4ddaef30fe05a15 Mon Sep 17 00:00:00 2001 From: lorretheboy Date: Thu, 6 Aug 2026 15:21:49 +0700 Subject: [PATCH 7/7] fix: copy --- src/languages/el.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/languages/el.ts b/src/languages/el.ts index e134e25db620..4248678b3663 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -9371,7 +9371,6 @@ ${reportName}`, exportedTo: 'Εξήχθη σε', exportAll: { selectAllMatchingItems: 'Επιλέξτε όλα τα στοιχεία που ταιριάζουν', - allMatchingItemsSelected: 'Έχουν επιλεγεί όλα τα στοιχεία που ταιριάζουν', selectAllOnThisPage: 'Επιλέξτε όλα σε αυτή τη σελίδα', }, errors: {