[WIP]fix: reland Search re-fire fix with the deploy blocker regressions closed - #98558
[WIP]fix: reland Search re-fire fix with the deploy blocker regressions closed#98558BartekObudzinski wants to merge 3 commits into
Conversation
|
Verified ✅ Screen.Recording.2026-08-13.at.12.24.12.mov |
|
Verified ✅ Screen.Recording.2026-08-13.at.13.19.34.mov |
|
Verified ✅ Screen.Recording.2026-08-13.at.12.53.02.mov |
|
Verified ✅ Screen.Recording.2026-08-13.at.12.56.13.mov |
|
@QichenZhu 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] |
|
@mananjadhav @mountiny One of you needs to 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] |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65a7566c02
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Explanation of Change
Relands #98305, which #98548 reverted over four deploy blockers. The blockers are fixed here and I verified all four flows manually.
98305 stopped a non chat Search from re-firing on every new report action anywhere in the account, which accounted for roughly 26 percent of all Search API calls. It left a non chat search refetching only when the set of transaction IDs changes.
Search rows and the footer total are served from the
snapshot_<hash>snapshot. Any flow that changes a field without adding or removing a transaction therefore left the snapshot stale, which is all four blockers:submitMoneyRequestOnSearchclears the snapshot report only on thesubmitsearch key, so on Spend > Expenses the row keepsaction: SUBMITand Submit stays on the menu. The submit itself does reach the server.Two fixes:
useSearchHighlightAndScrollnow also refetches when a transaction the results already display changes value. Onyx keeps one value object per collection member andOnyxCache.mergereturns the original object when nothing changed, so an identity check tells an edit from an untouched row exactly. This covers 98494, 98543, 98544 and any other field edit. It cannot restart the loop, because a Search response only writes snapshot keys.submitMoneyRequestOnSearchcall sites inuseSearchBulkActionsnow callrefreshSearchAfterReportAction, the same escape hatch approve and pay already use. This covers 98502.#98498 by @Valforte diagnosed 98494 correctly and I reused its analysis. Its explicit refresh is left out because
changeTransactionsReportwritestransactions_<id>.reportIDoptimistically, so the check above already covers those paths.Worth QA attention: deletion from search results becomes reachable on non chat searches for the first time.
Unit tests: 15/15 in
tests/unit/useSearchHighlightAndScrollTest.ts, 1357/1357 across all Search suites. Two tests added, and the new trigger's test was confirmed failing without the fix.Fixed Issues
$ #98048
PROPOSAL:
Tests
Preconditions
Search.The fix
Searchrequest appears in DevTools.Regression checks
Offline tests
Searchrequest is attempted while offline and the app does not error.QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari