Fix: New expense not visible in grouped-by from search when created offline#92132
Fix: New expense not visible in grouped-by from search when created offline#92132aswin-s wants to merge 19 commits into
Conversation
…ting expense offline When the search view has groupBy:from active, creating an expense offline did not write a group_<fromAccountID> entry into optimisticSnapshotData. getMemberSections renders exclusively from group_-prefixed Onyx keys, so the new expense was invisible until the next online sync.
The initial fix always computed count:1 and total:transaction.amount because optimisticSnapshotData is a fresh object per call. Subscribe to the SNAPSHOT collection at module level so the existing group_ entry can be read and its count/total incremented correctly. Also preserve the existing group's currency rather than overwriting it.
aswin-s
changed the title
aswin-s
changed the title
- Narrow group key template literal with 'as const' to satisfy PrefixedRecord index type - Optional-chain allSnapshots access since OnyxCollection may be undefined - Bump eslint-seatbelt baseline for new module-level Onyx.connect
Co-locate the snapshot Onyx subscription with the existing IOU module-level getters (allTransactions, allReports, etc.) and expose it via getAllSnapshots(). Avoids introducing a new seatbelt-baselined file; the existing IOU/index.ts no-onyx-connect count ticks from 10 to 11.
…User> shouldOptimisticallyUpdateSearch previously gated the optimistic write on hasNoFlatFilters or one of the canned Submit/Approve/UnapprovedCash queries. A flat search with from:me as the only filter matched none of those, so an offline-created expense never reached the from-filtered snapshot. Add matchesFromQuery so the user's own from-filter passes the gate.
…e new transaction getSearchOnyxUpdate only wrote to the currently active search's snapshot, so an offline expense created from a chat never reached the from:<me> filter or the groupBy:from view loaded in another tab/route. Persist each loaded search's query string under a new SEARCH_QUERY_BY_HASH Onyx key (the SEARCH API response wipes anything stored on snapshot.search), then iterate that map and apply the optimistic write to every snapshot whose query still matches via shouldOptimisticallyUpdateSearch.
Mirror the snapshot collection's lifecycle in the IOU module-level subscription: diff the previous vs current hash set on each callback and merge nulls for hashes that were removed. This bounds SEARCH_QUERY_BY_HASH to the size of the loaded snapshot collection.
- Search.ts: clarify the SEARCH_QUERY_BY_HASH side effect with a more explicit comment - IOU/index.ts: use String.slice instead of String.replace for hash extraction - IOU/index.ts: build the eviction payload with an explicit Record<string, string | null> to make the deletion-via-null intent type-visible - SearchUpdate.ts: rename groupTransactionsQuery -> groupTransactionsQueryJSON to match the file's existing JSON-suffix naming convention
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.
|
…x.ts The fanout in SearchUpdate.ts depends on subscribing to SEARCH_QUERY_BY_HASH at module level (same pattern used by the existing snapshot subscription), which is the 12th Onyx.connect violation in this file. Bumping the baseline from 11 to 12 via SEATBELT_INCREASE keeps the existing grandfathered count visible while letting this PR pass CI.
Adds two minimal tests for the refactored guard at the top of getSearchOnyxUpdate: returns undefined when participant has no accountID, and when there is no current user account. Closes part of the patch coverage gap from Expensify#92132 without dragging in fan-out path mocking.
melvin-bot
Bot
requested review from
eVoloshchak and
flaviadefaria
and removed request for
a team
|
@eVoloshchak 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] |
melvin-bot
Bot
requested review from
a team and
mjasikowski
and removed request for
a team
|
@mjasikowski 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: 6c8b543049
ℹ️ 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".
|
@aswin-s, this isn't the approach described in the accepted proposal, was there a problem with the original approach? |
@eVoloshchak — yes, the original approach was correct in principle: I added the group_ entry into optimisticSnapshotData inside the groupBy === FROM branch exactly as proposed. That first commit fixed the grouped view. However, while implementing it I noticed the issue title and reproduction video were actually showing two distinct but related cases: Both were broken for the same underlying reason (no optimistic write reaching those snapshots), so I had to addressed both. |
All group types in the SearchResultDataType union (SearchMemberGroup, SearchCardGroup, SearchCategoryGroup, etc.) share count, total, and currency properties, so narrowing to SearchMemberGroup was unsafe and unnecessary.
|
@eVoloshchak Could you take another look at this PR? |
|
Apologies, I'm a bit late 
|
|
@eVoloshchak Merged latest main. |
2 participants
Explanation of Change
When a user created an expense offline from anywhere other than the currently-active search view (e.g. from an Aswin S chat), the new expense was missing from already-loaded
from:<me>filter andGroup By: Fromsnapshots.Root cause:
getSearchOnyxUpdateonly wrote optimistic data into the snapshot of the currently active search query (getCurrentSearchQueryJSON().hash). Any other loaded snapshot whose query the new transaction would satisfy never received the optimistic write.Fix:
from:<currentUser>as the sole flat filter to passshouldOptimisticallyUpdateSearch(newmatchesFromQuerybranch).SEARCH_QUERY_BY_HASHOnyx key fromgetOnyxLoadingData(a dedicated key, because the SEARCH API response wipes anything stored onsnapshot.search).getSearchOnyxUpdatearound awriteForQueryhelper that iterates the hash→query map and fans the optimistic write to every snapshot whose stored query still matches the new transaction. ForgroupBy:frommatches, also write thegroup_<fromAccountID>entry and pre-populate the per-member transactions snapshot.SEARCH_QUERY_BY_HASHentries are evicted when their snapshot disappears, keeping the map bounded.Fixed Issues
$ #91453
PROPOSAL: #91453 (comment)
Tests
+1and total is incremented by the new amount.Offline tests
from:<me>filter and the group-by:from snapshots even though the expense was created from a chat.QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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-native.mp4
Android: mWeb Chrome
android-web.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
Web.mp4