Flag the group's own snapshot entry when a whole-group delete removes it - #100675
Open
nkdengineer wants to merge 6 commits into
Open
Flag the group's own snapshot entry when a whole-group delete removes it#100675nkdengineer wants to merge 6 commits into
nkdengineer wants to merge 6 commits into
Conversation
nkdengineer
marked this pull request as ready for review
September 9, 2026 09:05
melvin-bot
Bot
requested review from
cretadn22 and
flaviadefaria
and removed request for
a team
September 9, 2026 09:05
|
@cretadn22 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19582b148a
ℹ️ 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".
26 tasks
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Deleting every expense in a group made the group row flash: it disappeared, came back, then disappeared again.
A grouped search reads from two Onyx keys. The main snapshot holds one
group_<id>entry per group and nothing else —getMemberSectionsbuilds one row per group entry withtransactions: []. Each group's children live under a separate hash, pulled in byuseSearchSnapshotviauseMultipleSnapshots.The group row had no
pendingActionof its own — it derived one from its children, and that derivation requiredtransactions.length > 0. Since the two keys are cleared at different times, that produced three states:DELETE→OfflineWithFeedbackhides the row's content. Row gone.transactions.length === 0, nothing left to derive from →pendingActionisundefined. Row back — the flash.Searchresponse drops the group entry. Row gone for good.The fix moves the delete state onto the group's own snapshot entry, which is the only thing that outlives its children:
pendingActionlives on a sharedSearchGroupBaseinSearchResults.ts, which also folds up thecount/total/currencythat all ten group models repeated.getMemberSectionsalready spreads the group entry into the row, soitem.pendingActionlights up and the existingisRowDeletedchecks just work. The narrow layout no longer derives pending-delete from children.Which groups get that flag is coverage, not "clicked the group checkbox":
isSelectedViaGroupstill means the user selected through the group header. Export uses it so a group-row click can export as a group even whenlimit:left children unloaded.getSelectedGroupKeys, which includes a group only when the selected children cover the group's snapshotcount(or, when count is missing, whenisEntireGroupSelectedis set). Selecting every loaded child one by one is enough when that loaded set is the whole group. Clicking the group checkbox withlimit:Y < X is not.stampGroupCoverageFlagswritesisEntireGroupSelectedwithout touchingisSelectedViaGroup.A
limit:smaller than the group count only deletes the loaded rows, so the group stays in the list. In that case:The flag rides in the delete request's own
optimisticData/failureData, so a failed delete puts the row back. Delete runs once per transaction, so all of a group's requests carry the same idempotent merge and any one failing restores the row.No
successDatais needed: theSearchresponse replaces the whole snapshot, so a surviving group loses the flag and a deleted group loses its entry.The view drops flagged rows from the list when online so the row collapses away instead of leaving a zero-height wrapper; offline it stays put with the usual pending-delete styling.
One deliberate trade-off: a fully deleted group row now pops out rather than playing
FadeOutUp, because it leaves the list on the same render the flag arrives. The surviving rows still slide up viaLinearTransition, and the fade was mostly invisible anyway —hideChildrenhad already collapsed the row's content a step earlier.Fixed Issues
$ #100122
PROPOSAL:
Tests
Offline tests
QA Steps
Same as test
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
Screen.Recording.2026-09-09.at.15.25.42.mov
Android: mWeb Chrome
Screen.Recording.2026-09-09.at.15.22.49.mov
iOS: Native
Screen.Recording.2026-09-09.at.15.27.46.mov
iOS: mWeb Safari
Screen.Recording.2026-09-09.at.15.23.28.mov
MacOS: Chrome / Safari
Screen.Recording.2026-09-09.at.15.19.21.mov
Screen.Recording.2026-09-09.at.15.20.56.mov
Screen.Recording.2026-09-09.at.15.56.01.mov