Fix To-do search results disappearing when sort is clicked#83074
Fix To-do search results disappearing when sort is clicked#83074
Conversation
Use similarSearchHash instead of primaryHash in isTodoSearch to determine whether to use live Onyx data for To-do searches. The primaryHash includes sortBy/sortOrder, so changing sort caused the hash to no longer match the suggested To-do search hashes, making shouldUseLiveData flip to false and falling back to an empty API snapshot. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing
Added |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
melvin-bot
bot
requested review from
heyjennahay and
rojiphil
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ffaf73754
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/libs/SearchUIUtils.ts
Outdated
| function isTodoSearch(similarSearchHash: number, suggestedSearches: Record<string, SearchTypeMenuItem>) { | ||
| const TODO_KEYS: SearchKey[] = [CONST.SEARCH.SEARCH_KEYS.SUBMIT, CONST.SEARCH.SEARCH_KEYS.APPROVE, CONST.SEARCH.SEARCH_KEYS.PAY, CONST.SEARCH.SEARCH_KEYS.EXPORT]; | ||
| const matchedSearchKey = Object.values(suggestedSearches).find((search) => search.hash === hash)?.key; | ||
| const matchedSearchKey = Object.values(suggestedSearches).find((search) => search.similarSearchHash === similarSearchHash)?.key; |
There was a problem hiding this comment.
Using similarSearchHash here broadens matching beyond the four canned To-do searches and can return live To-do Onyx data for unrelated custom/saved searches. In getQueryHashes, similarSearchHash intentionally ignores most filter values (it keeps filter keys and only a few value-based filters like action), so a query like action:approve to:<other user> can collide with the canned Approve search and set shouldUseLiveData to true, causing incorrect results to be shown. This regression is introduced by switching the match from search.hash to search.similarSearchHash.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex hmm... can you please share the test steps to reproduce the regression?
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
There was a problem hiding this comment.
@MelvinBot What do you think of the regression mentioned here? If this does not cause regression, can you please share test steps to validate the same?
There was a problem hiding this comment.
The Codex bot raises a technically valid concern, but the practical regression risk is very low for the reasons below.
What the Codex bot claims
similarSearchHash ignores most filter values (only including filter key names), so a custom query like action:approve to:<otherUser> could collide with the canned Approve To-do search action:approve to:<currentUser>, since the to filter's value is not part of the similarSearchHash.
Why this is correct in theory
Looking at getQueryHashes in SearchQueryUtils.ts:349-380, the similarSearchHash is built from a Set that includes:
- Filter key names (e.g.,
"to","from") for most filters - Full
"action:<value>"strings only for theactionfilter (viasimilarSearchValueBasedFilters)
So action:approve to:123 and action:approve to:456 do produce the same similarSearchHash, which is by design for searchKey resolution in Search/index.tsx:279.
Why this is unlikely to cause a real regression
-
The
actionfilter is not user-exposed in the search bar — users cannot typeaction:approve to:<otherUser>in the search UI. Theactionfilter is only used internally by the canned To-do search suggestions. There is no UI path (filter form, search bar, saved searches) that allows a user to construct a query with both anactionfilter and a differenttovalue. -
This same
similarSearchHashmatching was already used before this PR — thesearchKeyresolution inSearch/index.tsx:279was already usingsimilarSearchHashto match against suggested searches:const searchKey = useMemo(() => Object.values(suggestedSearches).find( (search) => search.similarSearchHash === similarSearchHash )?.key, [suggestedSearches, similarSearchHash]);
This PR simply extends the same pattern to
isTodoSearchfor consistency, so that bothsearchKeyandshouldUseLiveDatause the same hash comparison. The original bug occurred precisely because they were using different hashes (primaryHashvssimilarSearchHash), causingshouldUseLiveDatato gofalsewhilesearchKeystill matched. -
The
currentSearchKeyguard provides additional protection —shouldUseLiveDatarequires!!currentSearchKeyto be true. ThecurrentSearchKeyis only set whensearchKeyresolves (via the samesimilarSearchHashmatch), so these two checks are already coupled.
Test steps to validate no regression
- Navigate to Reports > To-do > Approve — verify reports display correctly
- Click any sort column header (e.g., "From") — verify reports remain visible and are sorted (this was the original bug)
- Click the sort column again to toggle sort order — verify reports remain visible with reversed sort
- Switch between To-do options (Submit, Approve, Pay, Export) while a non-default sort is active — verify reports display correctly for each option
- Navigate to a non-To-do search (e.g., Reports > Expenses or a saved search) — verify it still uses API snapshot data and not live To-do data
- Go back to a To-do search — verify it still shows live data correctly
TL;DR: The Codex concern is theoretically valid but not practically reachable through the UI. The similarSearchHash matching was already being used for searchKey resolution before this PR — this change just aligns isTodoSearch with the same pattern, fixing the sort-breaking bug consistently.
There was a problem hiding this comment.
Valid concern. similarSearchHash intentionally strips filter values (keeping only filter keys + the action value), so a custom search like action:approve to:<other_user> would produce the same hash as the canned Approve search and incorrectly trigger shouldUseLiveData.
I've pushed a fix that switches to recentSearchHash instead. recentSearchHash includes all filter values (preventing false matches) but excludes sortBy/sortOrder/columns/limit (still fixing the original sort-column bug from #83053).
The similarSearchHash property is kept on SearchTypeMenuItem since it's still used legitimately elsewhere (e.g., EmptySearchView, SearchTypeMenu, useSearchTypeMenu for menu item highlighting).
|
The only failing check is This check will resolve once a reviewer posts the reviewer checklist comment. |
…ching similarSearchHash strips filter values, so custom searches like `action:approve to:<other_user>` would collide with the canned Approve search and incorrectly trigger live data mode. recentSearchHash includes all filter values but still excludes sortBy/sortOrder/columns/limit, fixing the original sort-column bug without introducing false matches. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
@luacmartins 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] |
The SearchTypeMenuItem type now requires recentSearchHash, but the test mocks in useSuggestedSearchDefaultNavigationTest were not updated. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing The other failing check ( |
|
Fixed the failing All checks are now passing (typecheck, all 8 test jobs, ESLint, Prettier, Storybook, spellcheck, codecov, verifySignedCommits). Only |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp83074-android-hybrid-002.mp4Android: mWeb Chrome83074-mweb-chrome-002.mp4iOS: HybridApp83074-ios-hybrid-002.mp4iOS: mWeb Safari83074-mweb-safari-002.mp4MacOS: Chrome / Safari83074-web-chrome-003.mp4 |
rojiphil
left a comment
rojiphil
left a comment
There was a problem hiding this comment.
@luacmartins Changes LGTM.
Over to you. Thanks.
3 participants
Explanation of Change
The
isTodoSearchfunction inSearchUIUtils.tswas comparing the current search'sprimaryHashagainst suggested To-do search hashes. TheprimaryHashincludessortByandsortOrder, so when a user changes the sort column in a To-do view (e.g., Approve), the hash no longer matched any suggested To-do search. This causedshouldUseLiveDatato flip tofalse, making the app fall back to an empty API snapshot instead of the live Onyx data that To-do searches use.The fix changes
isTodoSearchto compare usingsimilarSearchHashinstead, which is sort-independent. This matches the existing pattern used bysearchKeyresolution inSearch/index.tsx. TheSearchContextnow trackscurrentSimilarSearchHashalongsidecurrentSearchHashso it can pass the correct hash toisTodoSearch.Fixed Issues
$ #83053
PROPOSAL: #83053 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Screen.Recording.2026-02-20.at.10.22.22.AM.mov
Android: Native
N/A - This is a logic-only change affecting web/desktop sort behavior
Android: mWeb Chrome
N/A - This is a logic-only change affecting web/desktop sort behavior
iOS: Native
N/A - This is a logic-only change affecting web/desktop sort behavior
iOS: mWeb Safari
N/A - This is a logic-only change affecting web/desktop sort behavior
MacOS: Chrome / Safari
N/A - Draft PR, screenshots to be added during review