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:[email protected], 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?

src/components/Search/SearchRouter/SearchRouter.tsx

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

I am following the slack thread for this and will ask there

TLDR this will be implemented as a holistic approach throughout the app, so we decided to do nothing for now.

src/components/Search/SearchRouter/SearchRouter.tsx

src/components/Search/SearchRouter/getQueryWithSubstitutions.ts

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

src/components/Search/SearchRouter/SearchRouterList.tsx

src/components/Search/SearchRouter/getQueryWithSubstitutions.ts

src/components/Search/SearchRouter/getUpdatedSubstitutionsMap.ts

rayane-djouah · 2024-11-03T22:13:16Z

src/components/Search/SearchRouter/SearchRouter.tsx

+
+                    filteredAutocompleteSuggestions = filteredCards.map((card) => ({
+                        filterKey: CONST.SEARCH.SYNTAX_FILTER_KEYS.CARD_ID,
+                        text: card.bank,


Related to #51633 (review):

When searching for cardID:, I get a list of different cards but it's difficult to differentiate between them:

Screen.Recording.2024-11-03.at.10.31.50.PM.mov

I suggest using CardUtils.getCardDescription(card.cardID.toString()) to format the output as <bank> - <lastFourPAN || Not Activated>:

Suggested change

text: card.bank,

text: CardUtils.getCardDescription(card.cardID.toString()),

Result:

Screen.Recording.2024-11-03.at.10.34.56.PM.mov

Alternatively, we could use card?.nameValuePairs?.cardTitle to display the user-defined nickname for the card:

Suggested change

text: card.bank,

text: card?.nameValuePairs?.cardTitle || card.bank,

Result:

Screen.Recording.2024-11-03.at.10.54.18.PM.mov

@Expensify/design, could we get your input on this?

Note: This issue is also present in the filters page menu item and in the results page header:

Screen.Recording.2024-11-03.at.11.09.52.PM.mov

Good point. I used the card generating function to not add new code and it looks nice

Personally I like the idea of doing the last 4 digits with the dot separator: Card name • 1234

I think we're leaning towards a similar solution for rooms that have the same name across workspaces (i.e. #social, #admins, etc.

Definitely want to hear from the rest of @Expensify/design though.

Also, does the query syntax for this need to be cardID: or could it just be card:?

Interesting, for this idea @dannymcclain:

Personally I like the idea of doing the last 4 digits with the dot separator:

The way I understand it is that if you don't give the card a nickname, the card's default name includes the last four. But if you give it a custom nickname, you can overwrite the card name and put whatever you want there.

I think I lean towards just doing something that uses the card name exactly as it appears in the cards table, so if there is a nickname, use that. But if not, use the default name (e.g. Expensify Card - 1234

This way we don't have to worry about any custom naming logic here, we just use whatever is in the table. Thoughts?

Ahh yeah ok that makes sense! I didn't realize it worked that way. Totally down for that approach in that case!

and @dannymcclain it is possible to changed cardID: into card: but it requires a bit of work and would best be coordinated with backend.
@luacmartins if making this change it would be best to switch it all across the grammar I think

There is no 4 last digits shown in this case, but I can add them.

What about the Expensify Cards table? Can you show us what some of those entries look like, especially ones without nicknames?

@luacmartins if making this change it would be best to switch it all across the grammar I think

yea, we can do this. I imagine we might wanna change some others, like taxRate to tax-rate and expenseType to expense-type. I'll spin up an issue to address that separately. Created issue here #51966

@shawnborton sorry for my ignorance, but having never worked with the card related code I'm not sure I know how to properly simulate expensify cards 😅

Here are screenshots of 2 different methods of generating name

1. same as in wallet page
### 2. using card `title` or `bank`
The 3rd one from the top is what I believe is an "expensify card" since I set the bank name to Expensify Bank.
If that doesn't answer the question, then I need @luacmartins help

Oh man I am torn here... the Wallet naming seems to make the most sense to me at first, but then we run into the issue where we use the generic name for Expensify Cards and thus all unnamed Expensify Cards would look the same right? Let's see if @luacmartins has any thoughts here.

src/components/Search/SearchRouter/SearchRouter.tsx

Kicu · 2024-11-04T13:18:51Z

@luacmartins @rayane-djouah I addressed I believe all the comments, with 2 special notes:

about report/room names, there is a conversation on slack where it was decided to leave them as they are for now, because they will get a better display in future across the app, @JmillsExpensify confirmed this
about something looking differently in SearchPage - my next (and hopefully last) step is to move all this functionality to SearchPage results as well and share the code, so that in there everything looks and behaves the same in regards to autocomplete. That will come in a separate PR

Please re-check.

luacmartins

LGTM and tests well! Nice job. @rayane-djouah all yours

src/components/Search/SearchRouter/SearchRouter.tsx

rayane-djouah · 2024-11-06T10:18:02Z

src/components/Search/SearchRouter/SearchRouter.tsx

            const alreadyAutocompletedKeys: string[] = [];
            ranges.forEach((range) => {
                if (!autocompleteType || range.key !== autocompleteType) {
                    return;
                }
                alreadyAutocompletedKeys.push(range.value.toLowerCase());
            });
+
+            let filteredAutocompleteSuggestions: AutocompleteItemData[] | undefined;
            switch (autocompleteType) {


NAB: the switch block could be refactored to reduce code duplication by using a helper function

yes, but I suggest I do this in a separate PR, similarly how i described it here: #51633 (comment)

rayane-djouah · 2024-11-06T10:38:34Z

Bug:

Autocomplete Option for "You" Missing

Action Performed:

Typed from:<your current account email>

Expected Result:

An autocomplete option should be available for the current account, similar to the "from" filter selector page.

Actual Result:

There is no autocomplete option for "You."

Reproduction Video

Screen.Recording.2024-11-06.at.11.34.20.AM.mov

rayane-djouah · 2024-11-06T11:05:35Z

Bug: I noticed that the autocomplete options for "from," "to," and "in" are not sorted by most recent, unlike the filter selector pages.

Filter selector	autocomplete

Kicu · 2024-11-06T17:12:06Z

@rayane-djouah

I noticed that the autocomplete options for "from," "to," and "in" are not sorted by most recent,

About this I'm not sure how they should be sorted. This sort order is already merged on main and I didn't really change the logic of how the options are generated.

rayane-djouah

I'm not sure how they should be sorted.

We could use a similar approach to what is implemented in the filter selector pages:

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines 46 to 76 in 0a8f897

    
           const {options, areOptionsInitialized} = useOptionsList({ 
        
               shouldInitialize: didScreenTransitionEnd, 
        
           }); 
        
           const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false}); 
        
           const [selectedOptions, setSelectedOptions] = useState<OptionData[]>([]); 
        
           const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState(''); 
        
           const cleanSearchTerm = useMemo(() => searchTerm.trim().toLowerCase(), [searchTerm]); 
        
           const defaultOptions = useMemo(() => { 
        
               if (!areOptionsInitialized) { 
        
                   return defaultListOptions; 
        
               } 
        
               return OptionsListUtils.getFilteredOptions({ 
        
                   reports: options.reports, 
        
                   personalDetails: options.personalDetails, 
        
                   selectedOptions, 
        
                   excludeLogins: CONST.EXPENSIFY_EMAILS, 
        
                   maxRecentReportsToShow: 0, 
        
               }); 
        
           }, [areOptionsInitialized, options.personalDetails, options.reports, selectedOptions]); 
        
           const chatOptions = useMemo(() => { 
        
               return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, { 
        
                   selectedOptions, 
        
                   excludeLogins: CONST.EXPENSIFY_EMAILS, 
        
                   maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, 
        
               }); 
        
           }, [defaultOptions, cleanSearchTerm, selectedOptions]);

App/src/components/Search/SearchFiltersChatsSelector.tsx

Lines 64 to 78 in 0a8f897

    
           const defaultOptions = useMemo(() => { 
        
               if (!areOptionsInitialized || !isScreenTransitionEnd) { 
        
                   return defaultListOptions; 
        
               } 
        
               return OptionsListUtils.getSearchOptions(options, '', undefined, false); 
        
           }, [areOptionsInitialized, isScreenTransitionEnd, options]); 
        
           const chatOptions = useMemo(() => { 
        
               return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, { 
        
                   selectedOptions, 
        
                   excludeLogins: CONST.EXPENSIFY_EMAILS, 
        
                   maxRecentReportsToShow: 0, 
        
               }); 
        
           }, [defaultOptions, cleanSearchTerm, selectedOptions]);

The sorting for reports and personalDetails is handled in the getOptions function here:

App/src/libs/OptionsListUtils.ts

Lines 1816 to 1833 in 0a8f897

    
           // Sorting the reports works like this: 
        
           // - Order everything by the last message timestamp (descending) 
        
           // - When searching, self DM is put at the top 
        
           // - All archived reports should remain at the bottom 
        
           const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => { 
        
               const report = option.item; 
        
               if (option.private_isArchived) { 
        
                   return CONST.DATE.UNIX_EPOCH; 
        
               } 
        
               if (searchValue) { 
        
                   return [option.isSelfDM, report?.lastVisibleActionCreated]; 
        
               } 
        
               return report?.lastVisibleActionCreated; 
        
           }); 
        
           orderedReportOptions.reverse();

App/src/libs/OptionsListUtils.ts

Lines 1892 to 1894 in 0a8f897

    
               // PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435 
        
               allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text?.toLowerCase()], 'asc'); 
        
           }

rayane-djouah · 2024-11-06T18:01:11Z

src/components/Search/SearchRouter/SearchRouter.tsx

    const [cardList = {}] = useOnyx(ONYXKEYS.CARD_LIST);
-    const cardAutocompleteList = Object.values(cardList ?? {}).map((card) => card.bank);
+    const cardAutocompleteList = Object.values(cardList);
    const personalDetailsForParticipants = usePersonalDetails();


I believe the issue arises from using usePersonalDetails instead of OptionsListUtils.getFilteredOptions for "from" and "to"

rayane-djouah · 2024-11-06T18:01:30Z

src/components/Search/SearchRouter/SearchRouter.tsx

+                        text: participant.name,
+                        autocompleteID: participant.accountID,
+                    }));
+                    break;
                }
                case CONST.SEARCH.SYNTAX_FILTER_KEYS.IN: {
                    const filteredChats = searchOptions.recentReports
                        .filter((chat) => chat.text?.toLowerCase()?.includes(autocompleteValue.toLowerCase()))
                        .sort((chatA, chatB) => (chatA > chatB ? 1 : -1))


for "in", we're correctly using OptionsListUtils.getSearchOptions, but here we override the sort logic.

rayane-djouah · 2024-11-06T18:04:15Z

This sort order is already merged on main and I didn't really change the logic of how the options are generated.

Yes, I agree that this is not a blocker for this PR, as it stems from #51237. I just wanted to mention it in case you'd like to address it now. Otherwise, it seems fine to handle it in a follow-up PR.

luacmartins · 2024-11-06T18:19:05Z

I think those are NAB and we can address them in a follow up. Up to you @Kicu

Kicu · 2024-11-06T18:21:27Z

@rayane-djouah will be looking into your comments tomorrow, for now I added a missing toLowerCase and now the matching is slightly better. It should now match your "You" user better if you type in, butI think it won't match by email. Because we use only 1 field: name so it might be either an email or displayName but not both.

Will try to figure out tomorrow if we can do it better, but I would also like to try to push forward this PR

Kicu · 2024-11-06T18:22:03Z

@luacmartins if you're happy with how this works then I'd prefer to merge and then start a new PR that will fix everything together and improve the code.

luacmartins · 2024-11-06T18:23:00Z

Sounds good. Let's push this PR forward then and address those in a follow up. @rayane-djouah

luacmartins · 2024-11-06T18:23:16Z

Reviewing it again now

rayane-djouah · 2024-11-06T18:23:59Z

Reviewing again...

luacmartins

LGTM and tests well

rayane-djouah · 2024-11-06T18:57:34Z

Bug (Reproducible on Staging, Not a Blocker for This PR):

Type cardID: or in: and select the first option
Type ,
Observe that the selected option remains visible in the autocomplete list.

Screen.Recording.2024-11-06.at.7.37.32.PM.mov

Note: This issue does not occur with from, to, or taxRate:

Screen.Recording.2024-11-06.at.7.38.12.PM.mov

Note: The issue also occurs with "tag":

Screen.Recording.2024-11-06.at.7.39.35.PM.mov

rayane-djouah

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Screen.Recording.2024-11-06.at.7.55.27.PM.mov

Android: mWeb Chrome

Screen.Recording.2024-11-06.at.7.53.26.PM.mov

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-11-06.at.19.51.30.mp4

iOS: mWeb Safari

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-11-06.at.19.53.04.mp4

MacOS: Chrome / Safari

Screen.Recording.2024-11-06.at.7.32.37.PM.mov

MacOS: Desktop

Untitled.mov

luacmartins · 2024-11-06T19:18:59Z

Gonna merge this one. @Kicu let's address the comments above in a follow up!

OSBotify · 2024-11-06T19:29:38Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

Kicu · 2024-11-07T08:27:16Z

will link when I have a PR fixing all these things 👍

github-actions · 2024-11-07T13:08:44Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.0.59-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	success ✅

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

rayane-djouah suggested changes Nov 3, 2024

View reviewed changes

Kicu added 2 commits November 4, 2024 14:14

Improve types around Search autocomplete and add minor tweaks

7f2b3e7

Merge branch 'main' into kicu/autocomplete-query-ids

1523d88

Kicu requested a review from rayane-djouah November 4, 2024 15:42

luacmartins mentioned this pull request Nov 4, 2024

[Search v2.5] Update grammar filters #51966

Open

luacmartins self-requested a review November 4, 2024 17:35

luacmartins self-requested a review November 5, 2024 19:58

luacmartins previously approved these changes Nov 5, 2024

View reviewed changes

rayane-djouah suggested changes Nov 6, 2024

View reviewed changes

289Adam289 mentioned this pull request Nov 6, 2024

[HOLD #51633] Improve Search in suggestion in Search Router #52100

Open

49 tasks

Merge branch 'main' into kicu/autocomplete-query-ids

735b44e

rayane-djouah reviewed Nov 6, 2024

View reviewed changes

Improve autocomplete case matching

82cb2ec

Kicu dismissed luacmartins’s stale review via 82cb2ec November 6, 2024 18:14

luacmartins approved these changes Nov 6, 2024

View reviewed changes

rayane-djouah approved these changes Nov 6, 2024

View reviewed changes

melvin-bot bot requested a review from luacmartins November 6, 2024 19:00

luacmartins approved these changes Nov 6, 2024

View reviewed changes

luacmartins merged commit c755314 into Expensify:main Nov 6, 2024
17 of 19 checks passed

github-actions bot mentioned this pull request Nov 7, 2024

Deploy Checklist: New Expensify 2024-11-07 #52178

Open

47 tasks

lanitochka17 mentioned this pull request Nov 7, 2024

Search - Search history does not show the last 4 card digits #52192

Open

8 tasks

	text: card.bank,
	text: CardUtils.getCardDescription(card.cardID.toString()),

	text: card.bank,
	text: card?.nameValuePairs?.cardTitle \|\| card.bank,

	const {options, areOptionsInitialized} = useOptionsList({
	shouldInitialize: didScreenTransitionEnd,
	});

	const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
	const [selectedOptions, setSelectedOptions] = useState<OptionData[]>([]);
	const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState('');
	const cleanSearchTerm = useMemo(() => searchTerm.trim().toLowerCase(), [searchTerm]);

	const defaultOptions = useMemo(() => {
	if (!areOptionsInitialized) {
	return defaultListOptions;
	}

	return OptionsListUtils.getFilteredOptions({
	reports: options.reports,
	personalDetails: options.personalDetails,
	selectedOptions,
	excludeLogins: CONST.EXPENSIFY_EMAILS,
	maxRecentReportsToShow: 0,
	});
	}, [areOptionsInitialized, options.personalDetails, options.reports, selectedOptions]);

	const chatOptions = useMemo(() => {
	return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, {
	selectedOptions,
	excludeLogins: CONST.EXPENSIFY_EMAILS,
	maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
	});
	}, [defaultOptions, cleanSearchTerm, selectedOptions]);

	const defaultOptions = useMemo(() => {
	if (!areOptionsInitialized \|\| !isScreenTransitionEnd) {
	return defaultListOptions;
	}
	return OptionsListUtils.getSearchOptions(options, '', undefined, false);
	}, [areOptionsInitialized, isScreenTransitionEnd, options]);

	const chatOptions = useMemo(() => {
	return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, {
	selectedOptions,
	excludeLogins: CONST.EXPENSIFY_EMAILS,
	maxRecentReportsToShow: 0,
	});
	}, [defaultOptions, cleanSearchTerm, selectedOptions]);

	// Sorting the reports works like this:
	// - Order everything by the last message timestamp (descending)
	// - When searching, self DM is put at the top
	// - All archived reports should remain at the bottom
	const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => {
	const report = option.item;
	if (option.private_isArchived) {
	return CONST.DATE.UNIX_EPOCH;
	}

	if (searchValue) {
	return [option.isSelfDM, report?.lastVisibleActionCreated];
	}

	return report?.lastVisibleActionCreated;
	});
	orderedReportOptions.reverse();

	// PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435
	allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text?.toLowerCase()], 'asc');
	}

Make autocomplete work with entity ids #51633

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannymcclain commented Nov 1, 2024

luacmartins commented Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luacmartins Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1. same as in wallet page

Choose a reason for hiding this comment

Kicu commented Nov 4, 2024

luacmartins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayane-djouah commented Nov 6, 2024

Bug:

Action Performed:

Expected Result:

Actual Result:

rayane-djouah commented Nov 6, 2024

Kicu commented Nov 6, 2024

rayane-djouah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayane-djouah commented Nov 6, 2024

luacmartins commented Nov 6, 2024

Kicu commented Nov 6, 2024

Kicu commented Nov 6, 2024

luacmartins commented Nov 6, 2024

luacmartins commented Nov 6, 2024

rayane-djouah commented Nov 6, 2024

luacmartins left a comment

Choose a reason for hiding this comment

rayane-djouah commented Nov 6, 2024

rayane-djouah left a comment

Choose a reason for hiding this comment

Reviewer Checklist

Screenshots/Videos

luacmartins commented Nov 6, 2024

OSBotify commented Nov 6, 2024

Kicu commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

Kicu commented Oct 29, 2024 •

edited

Loading

luacmartins commented Nov 1, 2024 •

edited

Loading

luacmartins Nov 4, 2024 •

edited

Loading