Fetch all Home card totals in one grouped Search - #100022
Conversation
Opening Home fired one Search API call per displayable card. The queries
differed only by cardID, and each row consumed only {count, total, currency}
off the resulting snapshot.
Replace the per-card fan-out with a single `group-by:card` query scoped to the
displayed card set. The response carries a group per card with exactly the
fields a row reads. On the observed 14-request Home visit, 11 of those requests
were per-card; grouping leaves 4.
This also collapses a subscription over the entire SNAPSHOT collection - whose
selector re-ran on every Search-page write - into a single-key read.
The per-card query string stays: it is a card row's tap-through target. It is
just no longer sent to the API.
Drops the per-card wiped-count cache. It existed because tapping a card row
navigated the Search screen onto that card's own snapshot key, where
`shouldCalculateTotals: false` nulled count/total/currency out from under the
Home row. Neither half of that can happen now: the grouped hash is a different
query the Search screen never lands on, and that write only touches `search`,
never the `group_` entries the row now reads.
Fixes callstack-internal/expensify-issues#2913
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
adhorodyski
changed the title
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.
|
- Remove three stray `mockedBuildCardGroupQuery.mockImplementation` copies that duplicated the `beforeEach` and left the following line mis-indented. - Drop `displayableCardIDsKey`. The grouped query hash already encodes the card set, so key the search effect on it directly. - Stop mocking `buildCardGroupQuery` in the hook test. The test derives the grouped snapshot key from it, so a stand-in could drift from what ships. - Delete `covers the whole third-party card set with one grouped request`. It re-asserted what `costs one search() for a multi-card account` already pins; `fireSearches` does not branch on card kind. - Drop a redundant `toBeDefined` in the hash test. - Add `refires the grouped search when the displayable card set changes`. The effect-key swap above had no coverage; this fails if the card set is dropped from the dep list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
![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: 92cde9960b
ℹ️ 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".
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-09-02.09.49.07.movAndroid: mWeb Chrome2026-09-02.09.49.07.moviOS: HybridApp2026-09-02.09.44.37.moviOS: mWeb Safari2026-09-02.09.42.57.movMacOS: Chrome / Safari2026-09-02.09.41.53.mov |
Putting the displayed cardIDs in the query put them in the snapshot hash. deletePersonalCard merges the card out of CARD_LIST optimistically, moving Home to a hash nothing has written, so every remaining card row loses its total, offline until reconnect since fireSearches skips while offline. Scope the query to from:accountID + groupBy:card and filter groups by card in the hook. The hash holds still across a card add or delete, so the rows read the snapshot already loaded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Great find if this all works well
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
Tested the adhoc and saw the same as staging/production.
|
🚧 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.70-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a draft docs PR. This is a data-fetching refactor with no change to documented behavior. It replaces one The only article that covers this surface is Expensify Home Overview, and every statement it makes still holds:
What I checked
One pre-existing gap, unrelated to this PR and out of scope here: the FAQ Why don't I see the Your spend section? lists only "an active Expensify Card assigned to you," while the code also builds rows for displayable third-party and personal cards via Since no docs PR was created, there is nothing to label |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.72-0 🚀
|
|
No help site changes are required, so I did not open a docs PR. This PR swaps N per-card What I checkedThe only article covering this surface is
No UI strings, labels, section names, or visibility rules were added or removed, so there is nothing new to name or capitalize in the docs. One thing worth your call, not a docs change I'd make unprompted: the search effect now keys on the grouped query hash rather than the card set ( You asked me to close with the line below. There is no linked help site PR to review, since none was needed — if you disagree with that call, reply and I will open one. @adhorodyski, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Hi @adhorodyski. QA team tried using some internal accounts. Most of them have card transactions from the last 30 days and have more than 5 requests. Is there a specific account that should be used? Or can you check it internally? |
7 participants
Explanation of Change
Home reads every card total from one
group-by:cardSearch.useYourSpendDatasendstype:expense from:<accountID> date>=<30 days ago> group-by:card. Each row's{total, currency}comes from that response'sgroup_<cardID>entry, filtered to the cards Home displays. NocardIDfilter, so the hash holds still as cards come and go.deletePersonalCarddrops a card fromCARD_LISToptimistically, even offline, and the surviving rows keep the snapshot already loaded.Home request telemetry, 2026-08-31 10:00–11:00 UTC: 4,333 visits, median 2
Searchcalls, heaviest 14, eleven of them per-card.fireSearchessends at most three per fire: grouped card, awaiting-approval, repaid-last-30-days.Not covered: groups come back for cards Home hides, and a newly displayable card gets its row on the next focused fire.
Fixed Issues
$ #99914
PROPOSAL: N/A
Tests
Automated —
npx jest tests/unit/HomePage/YourSpendSection/ tests/unit/Search/yourSpendQueryBuildersTest.ts(130 passing). Nine tests cover this section, each driven red first by mutating the implementation:costs one search() for a multi-card account, not one per cardfires no card search when the account has no displayable cardslength > 0guardkeeps the surviving rows when a card is deleted, without refetchingresolves each card row total from its own group in the one grouped snapshotreads per-card totals from data, so a wipe of the snapshot-level totals cannot drop a rowsearch.countgroups by card…/carries no cardID filter…groupBy/ scopes bycardIDresolves from to the numeric accountID/windows the query to the same 30 daysfrom/ widens the windowManual:
Search.queryfield. Exactly one carriesgroup-by:card, and no request carries acardID:filter. All thequerystrings are distinct — two identical ones is a bug.Searchfires for the card section.Offline tests
Searchrequest is attempted and the rows are unchanged.Searchfires and the totals refresh.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
🤖 Generated with Claude Code