Search live-merge: remove policies/violations from getSections#93271
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
…(snapshot card name)" This reverts commit a4436b5.
…rge-mopup # Conflicts: # src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx
BartekObudzinski
changed the title
BartekObudzinski
changed the title
melvin-bot
Bot
requested review from
ZhenjaHorbach and
joekaufmanexpensify
and removed request for
a team
|
@ZhenjaHorbach 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] |
![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: 47fbc41d35
ℹ️ 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".
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| const hasAnyVisibleViolations = reportItem?.hasVisibleViolations || hasSyncedMissingAttendeesViolation; | ||
| const hasLiveTransactions = liveReportTransactions.length > 0; | ||
| const hasAnyVisibleViolations = (hasLiveTransactions ? liveHasVisibleViolations : !!reportItem.hasVisibleViolations) || hasSyncedMissingAttendeesViolation; |
There was a problem hiding this comment.
Optional comment
But the current line is a little hard to read 😅
I think it's better :
const hasVisibleViolations = hasLiveTransactions
? liveHasVisibleViolations
: !!reportItem.hasVisibleViolations;
const hasAnyVisibleViolations = hasVisibleViolations || hasSyncedMissingAttendeesViolation;
There was a problem hiding this comment.
Done! Went with hasVisibleReportViolations for the intermediate, since hasVisibleViolations would shadow the SearchUIUtils import we call above.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-06-12.15.54.52.movAndroid: mWeb Chrome2026-06-12.15.58.51.moviOS: HybridApp2026-06-12.15.54.52.moviOS: mWeb Safari2026-06-12.15.58.51.movMacOS: Chrome / Safari2026-06-12.15.47.34.mov |
|
LGTM! |
|
No product review needed |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.9-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no docs changes required. I reviewed the changes in this PR against the help site articles under This is an internal performance refactor of the Search list's data flow. It removes the screen-level Help site articles document end-user behavior and product features, none of which are affected here. As a result, no draft PR was created and no Files reviewed
All changes are internal logic/plumbing; the visible Search UI (rows, totals, status, avatars, violation indicators) renders identically. Since no help site PR was created, there is nothing to mark |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.9-0 🚀
|
6 participants
Explanation of Change
getSectionsperforms a parallel double-merge of live Onyx data that the row layer already self-hydrates; that merge is the cascade source for Search jank. S3 removes the live-Onyx inputs that the rows genuinely shadow. Investigation showed only some inputs are cleanly removable — the rest are load-bearing and are kept (documented below), so this PR does NOT claim to reach the original "deps → ~7" goal.Removed:
policies—getReportSectionsresolves the report policy from the snapshot (getPolicyFromKey).ExpenseReportListItemrecomputes the violations badge at the row instead: newly-exportedhasVisibleViolationsover the live policy (parentPolicy ?? snapshot) + the report's live violations/transactions. Before the report's transactions hydrate into the live collection, the badge recomputes from the snapshot transactions joined with a scoped per-row live violations selector (transactionViolationsByIDsSelector), so rule changes update never-opened reports too; the snapshot-computed value only covers the pre-load window.allTransactionViolations— the report badge is computed live at the row (with the snapshot fallback above), so the screen-level violations input is dropped fromgetReportSections.No user-facing behavior change for the removed inputs (rows already self-hydrate the same data), with one accepted-staleness exception: report-row avatars in
getReportSectionsnow derive from the snapshot policy instead of live-merged policies. Every othergetSectionscaller (static list, grouped expansion, Home spend chart) already passes no live policies, so this aligns the main list with them. The correspondinguseMemodeps in<Search>are trimmed.Fixed Issues
$ #93671
PROPOSAL:
Tests
Note: automated — Prettier clean, ESLint 0 errors, typecheck clean on changed files (the only tsgo errors are pre-existing duplicate identifiers in
src/libs/API/types.tson latestmain, untouched here), React Compilercheck-changedpasses, unit suitesSearchUIUtilsTest+useSearchSections+TransactionGroupListItem= 363 passing. Latestmainis merged in. Runtime QA still pending — this is why the PR is a draft.Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari