[Search v2.4] Search Router Polish Issues

[luacmartins]

• CMD + K should close the modal when it's already open
• Searching with filters that match a saved search should open the saved search in the LHN
• No need to show default searches in the recents.
• Clicking on a contextual filter in the search results takes you back to the default filters, rather than adding an extra search query to the existing search results. We need to preserve the current context and just add/update the type/status
• Using the router to Search in and putting a link removes the link for the search results
• If Search in suggests a workspace chat, we should add the reportID filter instead of in. Same goes for invoice and trip rooms
• BUG: A parsing error occurs when special characters, such as ä, are used.
• Filter order changes when selected via UI vs typed in the input
• Typing amount>200 returns expenses with amounts below $200 (we are still processing the value as cents)
• Improve performance of filter validation
• Some hidden chats show up without a name

[SzymczakJ]

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

[luacmartins]

@SzymczakJ just an FYI we're still adding more items to this issue as we test the feature

[SzymczakJ]

we're still adding more items

Then I'll focus on other issues and come back to this PR later.

[luacmartins]

Added a few more items to the list

[289Adam289]

I see the list is getting longer. I can also help with polish issues and follow ups from #49838

[289Adam289]

Regarding

Clicking on a contextual filter in the search results takes you back to the default filters, rather than adding an extra search query to the existing search results. We need to preserve the current context and just add/update the type/status

#49838 is solving this issue for status contextual filters. The reason type contextual filters reset query to default values is that lists of filters for type:expense and type:chat are very different. Similar behavior can be observed on github when changing between issues and prs.
If we want to preserve query when changing a type should we remove irrelevant filters (e.g. merchant in type:chat) or leave it as is.

[luacmartins]

I think that's fine for status for now. For type, we might wanna update the API/App to just ignore the irrelevant filters.

[melvin-bot]

@luacmartins, @SzymczakJ Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Kicu]

Hey I'd be happy to start working on some of these, as for now v2.5 is still being in the planning mode, so I think it's great time to do some polishing.

In addition Search seems like an important part of the App so I would like to also spend a bit of time on pure code refactors (if thats fine with you). Cleaner code should allow for faster iteration of features, and it would be nice to avoid cases similar to ReportUtils having 8000+ loc that everyone is afraid of touching 😅

---

Also this one:

CMD + K should close the modal when it's already open

will be fixed as soon as #49984 is merged, as Jakub fixed it.

[Kicu]

Another thing I wanted to confirm before actual implementation starts.
About this:

Clicking on a contextual filter in the search results takes you back to the default filters, rather than adding an extra search query to the existing search results.

see also this comment by Adam: #50250 (comment)

So originally (a long time ago in a galaxy far far away 😅) we had this design:

So we used to have logic where everything on the left side is just a "link" of sorts, so the Chats, Expenses etc and also Saved Searches kinda behave the same way.
And right now we're landing somewhere in the middle:

• status tabs are next to results and should change the current query (makes sense ✅)
• Expenses, Chats, etc.. are not next to results but they should also change the current query? 🤔
• Saved searches will move the user to a fresh query (also makes sense ✅)

Is this what we want to move forward with? Or should we bring back the dedicated type tabs?

CC @luacmartins @JmillsExpensify @shawnborton

[luacmartins]

Is this what we want to move forward with? Or should we bring back the dedicated type tabs?

We're descoping the decidated type tabs from v2.5 and I think for now the plan is to go with the current behavior, i.e. the LHN items navigate you to a brand new search and the status buttons update the status on the current custom search you're viewing. I think we can do nothing on this one for now. @JmillsExpensify for confirmation.

[289Adam289]

Remove unnecessary accountID lookup

Regarding this issue I believe accountID lookup is necessary for advanced filters from and to to work, because they are based on IDs. Current implementation replaces all emails that are present in onyx with IDs so that they appear as selected values in advanced filters and remaining emails are sent to backend unchanged.

We can try to refactor filters from and to to use emails not IDs that could make removing accountID lookup possible.

[luacmartins]

That one is more of a cleanup sine the API can now take emails directly too. I'd prioritize the other issues first (and autocomplete) and maybe we can even descope it

[luacmartins]

I couldn't reproduce BUG: I’m searching in a chat that has messages and getting no results on the search results page when I land there. in my tests today. I created a separate issue for it, so I'll remove it from the OP.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.51-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Search router polish  #50921

If no regressions arise, payment will be issued on 2024-10-29. 🎊

For reference, here are some details about the assignees on this issue:

• @SzymczakJ does not require payment (Contractor)

[luacmartins]

PR addressing most of these issues was merged.

[melvin-bot]

@luacmartins, @SzymczakJ Whoops! This issue is 2 days overdue. Let's get this updated quick!

[289Adam289]

Hi! I've noticed two checkboxes that haven't been addressed yet. @luacmartins Could you describe those a bit more and maybe create separate issues with clear reproduction steps? Especially more information is needed for:

If Search in suggests a workspace chat, we should add the reportID filter instead of in. Same goes for invoice and trip rooms

If we add the reportID instead of in user will not be able to quickly search for messages in the workspace chat. I am not entirely sure how to approach that.

[luacmartins]

I'll create a separate issue for that and close this one.

[luacmartins]

Created issue here - #51964