From f39d62fc7f58fafabb442fa07b2daa330da91181 Mon Sep 17 00:00:00 2001 From: apeyada Date: Sun, 5 Jul 2026 17:22:10 +0100 Subject: [PATCH 1/3] fix known DMs missing in recent chats on fresh sign-in --- .../Search/SearchAutocompleteList.tsx | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/components/Search/SearchAutocompleteList.tsx b/src/components/Search/SearchAutocompleteList.tsx index 530d57790267..d12349ac75ff 100644 --- a/src/components/Search/SearchAutocompleteList.tsx +++ b/src/components/Search/SearchAutocompleteList.tsx @@ -99,6 +99,18 @@ const defaultListOptions = { const EMPTY_RANK_MAP: ReadonlyMap = new Map(); +// Stable identity for a recent-report row, used to key the frozen rank map and to test local vs. server membership. +// A 1:1 DM's `keyForList` flips from the participant's accountID (personal-detail option, before the DM report is in +// Onyx) to the reportID (once the report loads from the server search). Both option shapes keep `accountID` set, so we +// key on accountID for single-participant options and fall back to reportID/keyForList for group chats and rooms +// (where accountID is 0). The `account-` prefix avoids collisions between numeric accountIDs and reportIDs. +function getStableRankKey(option: {accountID?: number | null; keyForList?: string; reportID?: string}): string | undefined { + if (option.accountID && option.accountID !== CONST.DEFAULT_NUMBER_ID) { + return `account-${option.accountID}`; + } + return option.keyForList ?? option.reportID ?? undefined; +} + const emptyOptionList = { reports: [], personalDetails: [], @@ -398,7 +410,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 +419,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 +531,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 +540,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++}); From 432e6c24b539e00296629b0a9027c3498565f272 Mon Sep 17 00:00:00 2001 From: apeyada Date: Tue, 7 Jul 2026 08:15:33 +0100 Subject: [PATCH 2/3] add unit test --- .../components/SearchAutocompleteListTest.tsx | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/ui/components/SearchAutocompleteListTest.tsx b/tests/ui/components/SearchAutocompleteListTest.tsx index f3cd8ffd835c..892c42500538 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,16 @@ describe('SearchAutocompleteList', () => { beforeEach(() => { mockHtmlToText.mockClear(); + // Restore the default mock behavior so tests are order-independent (the frozen-rank + // test below reconfigures these mocks 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 +187,81 @@ describe('SearchAutocompleteList', () => { expect(mockHtmlToText).toHaveBeenCalled(); }); + + it('keeps a known DM pinned in the local "Recent chats" section after its keyForList flips from accountID to reportID', async () => { + const mockUseFilteredOptions = jest.mocked(useFilteredOptions); + const mockCombineOrdering = jest.mocked(combineOrderingOfReportsAndPersonalDetails); + + // Phase 1: the DM is only known locally as a personal-detail option, so its `keyForList` + // is the participant's accountID and the DM report is not yet in Onyx. + const dmAsPersonalDetail: OptionData = {reportID: '', keyForList: '123', accountID: 123, text: 'Alice', alternateText: '', lastMessageText: ''}; + mockCombineOrdering.mockReturnValue({recentReports: [dmAsPersonalDetail], personalDetails: []}); + + const {rerender, toJSON} = render( + + + + + , + ); + + await waitForBatchedUpdatesWithAct(); + + // The rank of the DM is frozen and it renders under "Recent chats". + const treeAfterFreeze = JSON.stringify(toJSON()); + expect(treeAfterFreeze).toContain('Recent chats'); + expect(treeAfterFreeze).toContain('Alice'); + expect(treeAfterFreeze.indexOf('Recent chats')).toBeLessThan(treeAfterFreeze.indexOf('Alice')); + + // Phase 2: the server search returns. The DM report lands in Onyx (so its `keyForList` + // flips to the reportID while `accountID` stays the same), and a brand-new report shows up. + // A fresh (distinct) listOptions reference is returned to force the memoized options to recompute + // without changing the query (which would otherwise rebuild the frozen rank map). + const dmAsReport: OptionData = {reportID: '456', keyForList: '456', accountID: 123, text: 'Alice', 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, brandNewServerReport], personalDetails: []}); + + rerender( + + + + + , + ); + + await waitForBatchedUpdatesWithAct(); + + const treeAfterServer = JSON.stringify(toJSON()); + const recentChatsIndex = treeAfterServer.indexOf('Recent chats'); + const aliceIndex = treeAfterServer.indexOf('Alice'); + const serverResultsIndex = treeAfterServer.indexOf('Search results'); + const bobIndex = treeAfterServer.indexOf('Bob'); + + // Both sections and both rows are present. + expect(recentChatsIndex).toBeGreaterThanOrEqual(0); + expect(serverResultsIndex).toBeGreaterThan(recentChatsIndex); + expect(aliceIndex).toBeGreaterThanOrEqual(0); + expect(bobIndex).toBeGreaterThanOrEqual(0); + + // The DM stayed in the frozen local section (before the "Search results" header) instead of + // being knocked into the server section by the keyForList flip, and the new report is server-side. + expect(aliceIndex).toBeGreaterThan(recentChatsIndex); + expect(aliceIndex).toBeLessThan(serverResultsIndex); + expect(bobIndex).toBeGreaterThan(serverResultsIndex); + }); }); From 3e0ecb513bfbba1792174e362b8a2c0364ee04e8 Mon Sep 17 00:00:00 2001 From: apeyada Date: Tue, 7 Jul 2026 16:20:50 +0100 Subject: [PATCH 3/3] address codex review --- .../Search/SearchAutocompleteList.tsx | 15 ++++--- .../components/SearchAutocompleteListTest.tsx | 44 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/components/Search/SearchAutocompleteList.tsx b/src/components/Search/SearchAutocompleteList.tsx index d12349ac75ff..87a95a3d50be 100644 --- a/src/components/Search/SearchAutocompleteList.tsx +++ b/src/components/Search/SearchAutocompleteList.tsx @@ -99,13 +99,14 @@ const defaultListOptions = { const EMPTY_RANK_MAP: ReadonlyMap = new Map(); -// Stable identity for a recent-report row, used to key the frozen rank map and to test local vs. server membership. -// A 1:1 DM's `keyForList` flips from the participant's accountID (personal-detail option, before the DM report is in -// Onyx) to the reportID (once the report loads from the server search). Both option shapes keep `accountID` set, so we -// key on accountID for single-participant options and fall back to reportID/keyForList for group chats and rooms -// (where accountID is 0). The `account-` prefix avoids collisions between numeric accountIDs and reportIDs. -function getStableRankKey(option: {accountID?: number | null; keyForList?: string; reportID?: string}): string | undefined { - if (option.accountID && option.accountID !== CONST.DEFAULT_NUMBER_ID) { +// 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; diff --git a/tests/ui/components/SearchAutocompleteListTest.tsx b/tests/ui/components/SearchAutocompleteListTest.tsx index 892c42500538..f49a322cb3ff 100644 --- a/tests/ui/components/SearchAutocompleteListTest.tsx +++ b/tests/ui/components/SearchAutocompleteListTest.tsx @@ -92,8 +92,7 @@ describe('SearchAutocompleteList', () => { beforeEach(() => { mockHtmlToText.mockClear(); - // Restore the default mock behavior so tests are order-independent (the frozen-rank - // test below reconfigures these mocks per phase). + // 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, @@ -188,12 +187,11 @@ describe('SearchAutocompleteList', () => { expect(mockHtmlToText).toHaveBeenCalled(); }); - it('keeps a known DM pinned in the local "Recent chats" section after its keyForList flips from accountID to reportID', async () => { + 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); - // Phase 1: the DM is only known locally as a personal-detail option, so its `keyForList` - // is the participant's accountID and the DM report is not yet in Onyx. + // 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: []}); @@ -211,17 +209,19 @@ describe('SearchAutocompleteList', () => { await waitForBatchedUpdatesWithAct(); - // The rank of the DM is frozen and it renders under "Recent chats". + // 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')); - // Phase 2: the server search returns. The DM report lands in Onyx (so its `keyForList` - // flips to the reportID while `accountID` stays the same), and a brand-new report shows up. - // A fresh (distinct) listOptions reference is returned to force the memoized options to recompute - // without changing the query (which would otherwise rebuild the frozen rank map). - const dmAsReport: OptionData = {reportID: '456', keyForList: '456', accountID: 123, text: 'Alice', alternateText: '', lastMessageText: ''}; + // 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: []}, @@ -230,7 +230,7 @@ describe('SearchAutocompleteList', () => { hasMore: false, isLoadingMore: false, }); - mockCombineOrdering.mockReturnValue({recentReports: [dmAsReport, brandNewServerReport], personalDetails: []}); + mockCombineOrdering.mockReturnValue({recentReports: [dmAsReport, aliceTaskReport, brandNewServerReport], personalDetails: []}); rerender( @@ -248,20 +248,26 @@ describe('SearchAutocompleteList', () => { const treeAfterServer = JSON.stringify(toJSON()); const recentChatsIndex = treeAfterServer.indexOf('Recent chats'); - const aliceIndex = treeAfterServer.indexOf('Alice'); + // 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 sections and both rows are present. + // Both section headers and all three rows rendered. expect(recentChatsIndex).toBeGreaterThanOrEqual(0); expect(serverResultsIndex).toBeGreaterThan(recentChatsIndex); - expect(aliceIndex).toBeGreaterThanOrEqual(0); + expect(aliceDMIndex).toBeGreaterThanOrEqual(0); + expect(aliceTaskIndex).toBeGreaterThanOrEqual(0); expect(bobIndex).toBeGreaterThanOrEqual(0); - // The DM stayed in the frozen local section (before the "Search results" header) instead of - // being knocked into the server section by the keyForList flip, and the new report is server-side. - expect(aliceIndex).toBeGreaterThan(recentChatsIndex); - expect(aliceIndex).toBeLessThan(serverResultsIndex); + // 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); }); });