Make autocomplete work with entity ids #51633

Kicu · 2024-10-29T09:05:37Z

Details

this PR implements autocomplete for every filter that is not "plain text" but requires saving some ids of the entity the user actually chosen (from, to, in, tax, cardID)
we save the user autocomplete choice and later substitute query with the entity id before sending request to backend (details below)
contains very small change to autocomplete parser, and 2 functions which I put into separate files to write tests for them
important: this is only expected to work in SearchRouter, SearchPageHeader will still work using old code; I will make this autocomplete work also on Results page in SearchPageHeader but in a separate PR. Doing it in here would make this PR grow way over 1k loc

Detailed autocomplete description

Previously we were storing the full query that user typed verbatim
For example user would type test from:john@expensify.com, then when submitting we would run a function standardizeQueryJSON to traverse the parsed tree and substitue all plain text values with id values for specific filters. (this helper fn did the job: https://github.com/Expensify/App/blob/main/src/libs/SearchQueryUtils.ts#L169)
However this breaks for any case where the plain text value is non-unique. That is the case for chat/report names. For example when I type in:adm into my autocomplete I get more than 5 different rooms all named #admins. Previous solution couldn't handle a string like this in:#admins hello because we don't know which room #admins room it might be. This is the source of bug [HOLD Search autocomplete][Search v2.5] - Input shows report ID instead of report name when selecting "Search in" #50976.

new implementation

the main source of truth is still the "plain text" text input value that the users sees
in addition any time a user picks (clicks) a choice from autocomplete list, we save data in a substitutionsMap object
when we need to process the query we can always correctly substitute a string that user sees in his query with an autocompleteID of the entity that can be stored in URL and sent to backend; these ids can be accountID for a user or reportID for chat/room/report, and some other ids for tags and cards
old standardizeQueryJSON function can soon be removed, but this mechanism described above now only works for SearchRouter, whereas input inside SearchHeaderPage still uses the old substitution mechanism, and thus needs the function
tests for getQueryWithSubstitutions and getUpdatedSubstitutionsMap should help understand this

Fixed Issues

$ #50976
$ #50947
$ #50944
$ #50943
PROPOSAL:

Tests

open SearchRouter via clicking on 🔎 icon or cmd +k shortcut
verify that when using contextual search (inside a room) room name is correctly displayed
test autocomplete for from:..., to:..., cardID:... and tax:...
in every case user should see plain text display name, but backend should receive id instead

Offline tests

QA Steps

open SearchRouter via clicking on 🔎 icon or cmd +k shortcut
verify that when using contextual search (inside a room) room name is correctly displayed
test autocomplete for from:..., to:..., cardID:... and tax:...
in every case user should see plain text display name and the search should work

PR Author Checklist

Screenshots/Videos

Android: Native

rec-autocomplete-andr.mp4

Android: mWeb Chrome

iOS: Native

rec-autocomplete-ios.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

rec-autocomplete-web.mp4

MacOS: Desktop

melvin-bot · 2024-10-31T12:02:23Z

@rayane-djouah Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

SzymczakJ

LGTM

luacmartins

Looking great! Left a few small comments.

NAB: @Expensify/design we might wanna find a way to differentiate between some of these reports. When I search for in:#admin I get a list of different reports, but I have no way of knowing which one is which. Maybe we can append the policy name to it?

luacmartins · 2024-10-31T19:04:56Z

src/components/Search/SearchRouter/SearchRouter.tsx

-
+        .filter((details): details is NonNullable<PersonalDetails> => !!(details && details?.login))
+        .map((details) => ({
+            name: details.login ?? '',


We might wanna use displayName ?? login here since I imagine most people would search based on names, not emails

luacmartins · 2024-10-31T19:08:06Z

src/components/Search/SearchRouter/SearchRouter.tsx

-                    return;
+                    filteredAutocompleteSuggestions = filteredChats.map((chat) => ({
+                        filterKey: CONST.SEARCH.SYNTAX_FILTER_KEYS.IN,
+                        text: chat.text ?? '',


I think this is ok for now, but we might have to add some more details to the name to help users differentiate multiple reports with the same name, e.g. admin rooms

luacmartins · 2024-10-31T19:11:37Z

src/components/Search/SearchRouter/SearchRouter.tsx

-            const queryString = SearchQueryUtils.buildSearchQueryString(standardizedQuery);
-            Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: queryString}));
+
+            const computeNodeValueFn = SearchQueryUtils.getUpdatedAmountValue;


NAB this variable seems unnecessary, we can just pass the param inline

Suggested change

const computeNodeValueFn = SearchQueryUtils.getUpdatedAmountValue;

const computeNodeValueFn = SearchQueryUtils.getUpdatedAmountValue;

luacmartins · 2024-10-31T19:12:12Z

src/components/Search/SearchRouter/SearchRouter.tsx

    );

+    const onAutocompleteSuggestionClick = (autocompleteKey: string, autocompleteId: string) => {


Suggested change

const onAutocompleteSuggestionClick = (autocompleteKey: string, autocompleteId: string) => {

const onAutocompleteSuggestionClick = (autocompleteKey: string, autocompleteID: string) => {

luacmartins · 2024-10-31T19:31:23Z

src/components/Search/SearchRouter/getQueryWithSubstitutions.ts

+import type {SearchAutocompleteQueryRange} from '@components/Search/types';
+import * as parser from '@libs/SearchParser/autocompleteParser';
+
+type SubstitutionEntry = {value: string};


Do we need the value key here? AFAIK we always have the map as {[filterKey:typedValue]: {value: [id]}}. Could we simplify it to {[filterKey:typedValue]: [id]}?

Suggested change

type SubstitutionEntry = {value: string};

type SubstitutionEntry = {value: string};

dannymcclain · 2024-11-01T13:28:33Z

@luacmartins I definitely think we should distinguish those somehow. Right now that list is basically unusable. There's a million ways we could do that, here are a few ideas:

I think I lean towards the simple dot separator pattern: in:#admins • Danny's Donuts. But let's see what @Expensify/design thinks or if they have any other/better ideas.

luacmartins · 2024-11-01T17:26:19Z

^ is being discussed here

SzymczakJ mentioned this pull request Oct 29, 2024

Provide autocomplete suggestions in SearchRouterList #51237

Merged

49 tasks

Kicu force-pushed the kicu/autocomplete-query-ids branch 3 times, most recently from e63486c to 533131a Compare October 30, 2024 13:30

rayane-djouah mentioned this pull request Oct 30, 2024

[HOLD Search autocomplete][Search v2.5] - Input shows report ID instead of report name when selecting "Search in" #50976

Open

8 tasks

Kicu force-pushed the kicu/autocomplete-query-ids branch from 533131a to 1e9929d Compare October 31, 2024 07:58

Kicu added 5 commits October 31, 2024 12:54

Add function for generating new query with autocomplete values

b93c8c8

Add autocomplete working for from/to and chat rooms

12987cd

Add autocomplete working for taxRates and taxes from policy

db340d7

Add autocomplete for cardIDs and correctly compute backend query

217d562

Fix autocomplete for contextual search item

e3e559b

Kicu force-pushed the kicu/autocomplete-query-ids branch from 2cc15d1 to e3e559b Compare October 31, 2024 11:54

Kicu marked this pull request as ready for review October 31, 2024 12:02

Kicu requested a review from a team as a code owner October 31, 2024 12:02

melvin-bot bot requested a review from rayane-djouah October 31, 2024 12:02

melvin-bot bot removed the request for review from a team October 31, 2024 12:02

Kicu changed the title ~~Autocomplete query ids WIP~~ Make autocomplete work with entity ids Oct 31, 2024

SzymczakJ approved these changes Oct 31, 2024

View reviewed changes

luacmartins self-requested a review October 31, 2024 16:13

luacmartins reviewed Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make autocomplete work with entity ids #51633

Make autocomplete work with entity ids #51633

Kicu commented Oct 29, 2024 •

edited

Loading

melvin-bot bot commented Oct 31, 2024

SzymczakJ left a comment

luacmartins left a comment

luacmartins Oct 31, 2024

luacmartins Oct 31, 2024

luacmartins Oct 31, 2024

luacmartins Oct 31, 2024

luacmartins Oct 31, 2024

dannymcclain commented Nov 1, 2024

luacmartins commented Nov 1, 2024 •

edited

Loading

	const computeNodeValueFn = SearchQueryUtils.getUpdatedAmountValue;
	const computeNodeValueFn = SearchQueryUtils.getUpdatedAmountValue;

		);

		const onAutocompleteSuggestionClick = (autocompleteKey: string, autocompleteId: string) => {

	type SubstitutionEntry = {value: string};
	type SubstitutionEntry = {value: string};

Make autocomplete work with entity ids #51633

Are you sure you want to change the base?

Make autocomplete work with entity ids #51633

Conversation

Kicu commented Oct 29, 2024 • edited Loading

Details

Detailed autocomplete description

new implementation

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Oct 31, 2024

SzymczakJ left a comment

Choose a reason for hiding this comment

luacmartins left a comment

Choose a reason for hiding this comment

luacmartins Oct 31, 2024

Choose a reason for hiding this comment

luacmartins Oct 31, 2024

Choose a reason for hiding this comment

luacmartins Oct 31, 2024

Choose a reason for hiding this comment

luacmartins Oct 31, 2024

Choose a reason for hiding this comment

luacmartins Oct 31, 2024

Choose a reason for hiding this comment

dannymcclain commented Nov 1, 2024

luacmartins commented Nov 1, 2024 • edited Loading

Kicu commented Oct 29, 2024 •

edited

Loading

luacmartins commented Nov 1, 2024 •

edited

Loading