Repository navigation
[Search] Refetch results when a member's custom fields change - #103441
Conversation
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
…h-on-custom-field-edit # Conflicts: # src/hooks/useSearchAutoRefetch.ts
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@shubham1206agra 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] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![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: 9816a36d06
ℹ️ 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".
…ion objects in the custom field tests
| const shouldCalculateTotalsOnRetry = useSearchShouldCalculateTotals(currentSearchKey, true, areAllMatchingItemsSelected); | ||
|
|
||
| const previousReportActions = usePrevious(reportActions); | ||
| const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY); |
There was a problem hiding this comment.
We should use a selector here to fetch only the fields we care about and avoid expensive full collection reads.
There was a problem hiding this comment.
Done in cff5786, createMemberCustomFieldsSelector in selectors/Policy.ts returns only the two custom field values for the members of the policies in the snapshot, so Search no longer re-renders on other policy writes.
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Good from product. 👍
| useEffect(() => { | ||
| // Snapshot and focus updates re-run this effect with unchanged collections, which would diff a collection against itself. | ||
| if (transactions === previousTransactions && reportActions === previousReportActions && !hasPendingSearchRef.current) { | ||
| if (transactions === previousTransactions && reportActions === previousReportActions && policies === previousPolicies && !hasPendingSearchRef.current) { |
There was a problem hiding this comment.
@grgia @luacmartins This feels like an expensive operation (Also, we never did deepEqual here to know if members custom field changed or not).
There was a problem hiding this comment.
Yea, I agree. I commented above on using a selector for the policies
There was a problem hiding this comment.
Covered by the same selector in cff5786. Onyx deep-equals selector output, so this identity check now only passes when a custom field value actually changed, and the hook compares a small projection rather than the policy collection.
…ll policy collection
|
@grgia conflicts |
…h-on-custom-field-edit # Conflicts: # src/selectors/Policy.ts
|
@shubham1206agra I think this is ready for your review |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-10-26.at.8.01.59.PM.mov |
|
@grgia conflicts |
…h-on-custom-field-edit
|
@grgia we have failing checks |
| @@ -14692,91 +14692,48 @@ var require_lib = __commonJS({ | |||
| }; | |||
| } | |||
| ; | |||
| var ca = class { | |||
There was a problem hiding this comment.
Why do we have these changes?
|
@grgia bump |
…h-on-custom-field-edit # Conflicts: # src/components/Search/index.tsx
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.5.28-4 🚀
|
|
No help site update needed. This is a bug fix: the Custom field 1 and Custom field 2 columns in Spend > Expenses now show a member's updated values when you return to the tab, which is what users already expect, and no help article says otherwise. |
|
Hi @grgia. QA team failed this PR with the original issue 1793659931799.bandicam_2026-11-03_05-42-54-871.mp4 |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.5.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.5.30-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Spend > Expenses now refetches its results when a member's custom field 1 or 2 changes on a workspace the results come from, so the Custom field columns show the new values when you return to the tab.
Editing a member's custom field under Workspace > Members and switching back to Spend left the columns on the old values until something else happened to run a search (opening Inbox and coming back, or opening the expense). The columns read
submitterUserIDandsubmitterPayrollIDoff the Search snapshot, and the server recomputes both from the workspace's employee list on every Search call, so the stale part is the snapshot: nothing asked for a new one.useSearchAutoRefetchalready refetches when a transaction in the results is edited; this adds the same trigger for a custom field value change on a policy present in the results, deferred while Spend is not the active screen and fired when it regains focus.createMemberCustomFieldsSelector, which returns only the two custom field values for the members of the policies in the snapshot, so other policy writes never reach the hooktests/unit/useSearchAutoRefetchTest.tsFixed Issues
$ #94960
PROPOSAL:
Tests
Setup: a workspace with at least one submitted expense, and the member custom field names set under Workspace > Overview (or any workspace where Custom field 1 / Custom field 2 columns can be selected in Spend > Expenses).
Searchrequest is sent on returning to the tab and the two columns show the new values without opening Inbox or the expense.Searchrequest is sent on returning to Spend.Unit tests,
npx jest tests/unit/useSearchAutoRefetchTest.ts tests/unit/PolicySelectorTest.ts:Before:
After:
Offline tests
Searchrequest is attempted while offline and the columns keep the old values.Searchrequest goes out and the columns show the new values.QA Steps
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
MacOS: Desktop