diff --git a/src/components/Search/SearchAutocompleteList.tsx b/src/components/Search/SearchAutocompleteList.tsx index 530d57790267..87a95a3d50be 100644 --- a/src/components/Search/SearchAutocompleteList.tsx +++ b/src/components/Search/SearchAutocompleteList.tsx @@ -99,6 +99,19 @@ const defaultListOptions = { const EMPTY_RANK_MAP: ReadonlyMap = new Map(); +// A DM's keyForList changes from the accountID to the reportID once its report loads from search, which would move the +// row between sections. To keep it stable, key DMs and personal details by accountID instead. We can't do this for every +// account-backed option though: task/expense reports also carry an accountID, and keying them by it would let them +// masquerade as the DM row for the same person. So only DMs and personal details use the accountID; everything else +// keeps its reportID/keyForList. The `account-` prefix keeps accountIDs from clashing with reportIDs. +function getStableRankKey(option: {accountID?: number | null; keyForList?: string; reportID?: string; isDM?: boolean}): string | undefined { + const isDMOrPersonalDetail = !!option.isDM || !option.reportID; + if (isDMOrPersonalDetail && option.accountID && option.accountID !== CONST.DEFAULT_NUMBER_ID) { + return `account-${option.accountID}`; + } + return option.keyForList ?? option.reportID ?? undefined; +} + const emptyOptionList = { reports: [], personalDetails: [], @@ -398,7 +411,7 @@ function SearchAutocompleteList({ return reportOptions.slice(0, 20); }, [autocompleteQueryValue, searchOptions]); - // Locked rank map (keyForList -> originalIndex) capturing the order of locally-known + // Locked rank map (stable key -> originalIndex) capturing the order of locally-known // results at the moment the query changes. Recomputed only when the query changes, so server // reports merged into Onyx later do not shift the rows already visible in the top section. const [frozenLocalRank, setFrozenLocalRank] = useState>(EMPTY_RANK_MAP); @@ -407,7 +420,7 @@ function SearchAutocompleteList({ const buildRankMap = (options: OptionData[]): Map => { const rank = new Map(); for (const [index, option] of options.entries()) { - const key = option.keyForList ?? option.reportID ?? (option.accountID ? String(option.accountID) : undefined); + const key = getStableRankKey(option); if (key) { rank.set(key, index); } @@ -519,7 +532,8 @@ function SearchAutocompleteList({ const localRows: AutocompleteListItem[] = []; const serverRows: AutocompleteListItem[] = []; for (const item of nextStyledRecentReports) { - if (item.keyForList && frozenLocalRank.has(item.keyForList)) { + const stableKey = getStableRankKey(item); + if (stableKey && frozenLocalRank.has(stableKey)) { localRows.push(item); } else { serverRows.push(item); @@ -527,7 +541,7 @@ function SearchAutocompleteList({ } // Sort the local section by the rank captured at query-change time so it cannot // reorder when the API returns. - localRows.sort((a, b) => (frozenLocalRank.get(a.keyForList ?? '') ?? 0) - (frozenLocalRank.get(b.keyForList ?? '') ?? 0)); + localRows.sort((a, b) => (frozenLocalRank.get(getStableRankKey(a) ?? '') ?? 0) - (frozenLocalRank.get(getStableRankKey(b) ?? '') ?? 0)); if (localRows.length > 0 || !isLoadingOptions) { pushSection({title: translate('search.recentChats'), data: localRows, sectionIndex: sectionIndex++}); diff --git a/tests/ui/components/SearchAutocompleteListTest.tsx b/tests/ui/components/SearchAutocompleteListTest.tsx index f3cd8ffd835c..f49a322cb3ff 100644 --- a/tests/ui/components/SearchAutocompleteListTest.tsx +++ b/tests/ui/components/SearchAutocompleteListTest.tsx @@ -4,7 +4,11 @@ import {LocaleContextProvider} from '@components/LocaleContextProvider'; import OnyxListItemProvider from '@components/OnyxListItemProvider'; import SearchAutocompleteList from '@components/Search/SearchAutocompleteList'; +import useFilteredOptions from '@hooks/useFilteredOptions'; + +import {combineOrderingOfReportsAndPersonalDetails} from '@libs/OptionsListUtils'; import Parser from '@libs/Parser'; +import type {OptionData} from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -88,6 +92,15 @@ describe('SearchAutocompleteList', () => { beforeEach(() => { mockHtmlToText.mockClear(); + // Reset to defaults so tests don't leak into each other; the DM test overrides these per phase. + jest.mocked(useFilteredOptions).mockReturnValue({ + options: {reports: [], personalDetails: []}, + isLoading: false, + loadMore: jest.fn(), + hasMore: false, + isLoadingMore: false, + }); + jest.mocked(combineOrderingOfReportsAndPersonalDetails).mockReturnValue({recentReports: [], personalDetails: []}); }); afterEach(async () => { @@ -173,4 +186,88 @@ describe('SearchAutocompleteList', () => { expect(mockHtmlToText).toHaveBeenCalled(); }); + + it('keeps a known DM in "Recent chats" after its report loads from search', async () => { + const mockUseFilteredOptions = jest.mocked(useFilteredOptions); + const mockCombineOrdering = jest.mocked(combineOrderingOfReportsAndPersonalDetails); + + // Before the report loads, the DM is just a personal detail keyed by accountID. + const dmAsPersonalDetail: OptionData = {reportID: '', keyForList: '123', accountID: 123, text: 'Alice', alternateText: '', lastMessageText: ''}; + mockCombineOrdering.mockReturnValue({recentReports: [dmAsPersonalDetail], personalDetails: []}); + + const {rerender, toJSON} = render( + + + + + , + ); + + await waitForBatchedUpdatesWithAct(); + + // The DM shows up under "Recent chats" and its position is now frozen. + const treeAfterFreeze = JSON.stringify(toJSON()); + expect(treeAfterFreeze).toContain('Recent chats'); + expect(treeAfterFreeze).toContain('Alice'); + expect(treeAfterFreeze.indexOf('Recent chats')).toBeLessThan(treeAfterFreeze.indexOf('Alice')); + + // Now the search results come back. The DM's report loads, so its keyForList flips to the reportID + // (accountID stays the same). We hand back a new options reference so the list recomputes without + // touching the query, otherwise the frozen ranks would be rebuilt. + const dmAsReport: OptionData = {reportID: '456', keyForList: '456', accountID: 123, isDM: true, text: 'Alice', alternateText: '', lastMessageText: ''}; + // Alice also has a task report. It carries her accountID too, but it isn't the DM, so it should end up + // in the server section rather than pinned under "Recent chats". + const aliceTaskReport: OptionData = {reportID: '999', keyForList: '999', accountID: 123, isDM: false, isTaskReport: true, text: 'Alice Task', alternateText: '', lastMessageText: ''}; + const brandNewServerReport: OptionData = {reportID: '789', keyForList: '789', accountID: 0, text: 'Bob', alternateText: '', lastMessageText: ''}; + mockUseFilteredOptions.mockReturnValue({ + options: {reports: [], personalDetails: []}, + isLoading: false, + loadMore: jest.fn(), + hasMore: false, + isLoadingMore: false, + }); + mockCombineOrdering.mockReturnValue({recentReports: [dmAsReport, aliceTaskReport, brandNewServerReport], personalDetails: []}); + + rerender( + + + + + , + ); + + await waitForBatchedUpdatesWithAct(); + + const treeAfterServer = JSON.stringify(toJSON()); + const recentChatsIndex = treeAfterServer.indexOf('Recent chats'); + // The first "Alice" is the DM row under "Recent chats". + const aliceDMIndex = treeAfterServer.indexOf('Alice'); + const serverResultsIndex = treeAfterServer.indexOf('Search results'); + const aliceTaskIndex = treeAfterServer.indexOf('Alice Task'); + const bobIndex = treeAfterServer.indexOf('Bob'); + + // Both section headers and all three rows rendered. + expect(recentChatsIndex).toBeGreaterThanOrEqual(0); + expect(serverResultsIndex).toBeGreaterThan(recentChatsIndex); + expect(aliceDMIndex).toBeGreaterThanOrEqual(0); + expect(aliceTaskIndex).toBeGreaterThanOrEqual(0); + expect(bobIndex).toBeGreaterThanOrEqual(0); + + // The DM stayed under "Recent chats" (before the "Search results" header) despite the keyForList flip. + expect(aliceDMIndex).toBeGreaterThan(recentChatsIndex); + expect(aliceDMIndex).toBeLessThan(serverResultsIndex); + + // Alice's task report and Bob's report are in the server section - the task wasn't pinned just + // because it shares Alice's accountID. + expect(aliceTaskIndex).toBeGreaterThan(serverResultsIndex); + expect(bobIndex).toBeGreaterThan(serverResultsIndex); + }); });