Fix Reports split not reverting when split expense is deleted#81388
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.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6d1db7334
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Please fix conflict. And address bot review |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Testing steps align with intended product design and behavior.
|
@situchan, pr is ready for your review. Thanks for your patience. |
|
@situchan, kindly bump. thanks. |
|
Please merge main |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-03-03.at.9.12.48.PM.mov |
|
@dangrous, I've addressed your comments. Please take another look. thanks. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3c618534f
ℹ️ 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".
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03fa71ea3b
ℹ️ 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".
🤖 Code ReviewSummary: This PR adds split-revert logic to the search delete flow so that deleting a split expense from search properly reverts the split when only one sibling remains — matching the existing behavior in All CI checks pass. Findings1. Failure data: snapshot may lose original transaction (Medium)In
If the original transaction wasn't visible in search results, Suggestion: Consider using 2. Extra API params vs existing flow (Low)The hook sends 3. What's correct and well-done
4. Style / minorThe Overall: The logic is correct and well-structured. The only actionable concern is finding #1 (failure snapshot restoration). The rest are informational observations about minor divergences from the existing flow that don't affect correctness in practice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fe4a01a5e
ℹ️ 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".
🤖 Code ReviewSummary: This PR adds split-revert logic to the search delete flow so that deleting a split expense from search properly reverts the split when only one sibling remains — matching the existing behavior in All CI checks pass. Findings1. Failure snapshot may lose original transaction (Low-Medium)In
If the original transaction wasn't visible in search results, Suggestion: Consider 2. Extra API params vs existing flow (Informational)The hook sends 3. What's correct and well-structured
4.
|
I don't think finding #1 is applicable here. The failure path intentionally restores the search snapshot and the transaction collection to their own previous states, which can differ. const failureSnapshotData: PrefixedRecord<typeof ONYXKEYS.COLLECTION.TRANSACTION, Transaction | null> = {
...optimisticDeletedSplitTransactions,
[`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`]: previousSnapshotOriginalTransaction ?? null,
};
failureData = [
failureSnapshotUpdate,
...Object.entries(optimisticDeletedSplitTransactions).map(([transactionKey, transaction]) => ({
onyxMethod: Onyx.METHOD.SET,
key: transactionKey,
value: transaction,
})),
{
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`,
value: optimisticOriginalTransaction ?? null,
},
];If the original transaction was not present in the search snapshot before the optimistic update, falling back to |
|
retested. looks good |
|
Thank you all, @dangrous, this seems ready to merge. |
|
🚧 @dangrous 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Just an FYI - we're deprecating DeleteMoneyRequestOnSearch https://github.com/Expensify/Expensify/issues/610939. So a lot of the changes in this PR will be reverted. |
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.3.40-0 🚀
|
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.3.40-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|
Explanation of Change
Ensure report split state is properly reverted after deleting a split expense.
Fixed Issues
$ #77396
PROPOSAL: #77396 (comment)
Tests
Offline tests
Same as the tests.
QA Steps
Same as the tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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.mov
Android: mWeb Chrome
Screen.Recording.2026-02-07.at.6.27.42.PM.mov
iOS: Native
Screen.Recording.2026-02-07.at.6.41.26.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-02-07.at.6.46.34.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-02-07.at.6.14.54.PM.mov