Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/components/Search/SearchAutocompleteList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ const defaultListOptions = {

const EMPTY_RANK_MAP: ReadonlyMap<string, number> = 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: [],
Expand Down Expand Up @@ -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<ReadonlyMap<string, number>>(EMPTY_RANK_MAP);
Expand All @@ -407,7 +420,7 @@ function SearchAutocompleteList({
const buildRankMap = (options: OptionData[]): Map<string, number> => {
const rank = new Map<string, number>();
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);
}
Expand Down Expand Up @@ -519,15 +532,16 @@ 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);
}
}
// 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++});
Expand Down
97 changes: 97 additions & 0 deletions tests/ui/components/SearchAutocompleteListTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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(
<OnyxListItemProvider>
<LocaleContextProvider>
<SearchAutocompleteList
autocompleteQueryValue="te"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: Why is this the user's query? Shouldn't it be something more like Al for Alice?

handleSearch={jest.fn()}
onListItemPress={jest.fn()}
/>
</LocaleContextProvider>
</OnyxListItemProvider>,
);

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(
<OnyxListItemProvider>
<LocaleContextProvider>
<SearchAutocompleteList
autocompleteQueryValue="te"
handleSearch={jest.fn()}
onListItemPress={jest.fn()}
/>
</LocaleContextProvider>
</OnyxListItemProvider>,
);

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);
});
});
Loading