feat: update amounts/percentages split logic to match OD (rework) - #81626
Conversation
…ve expense appears
|
@ShridharGoel 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] |
|
@ShridharGoel You can simply copy & paste the checklist from initial PR and only thing to do as reviewer is verify that the 3 regression tests listed in PR description are passing ✅ If it's a 🟢 pass you can approve and we can merge this again and hopefully no revert this time 🤞 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ef3ae2d9
ℹ️ 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".
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.
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-24.at.12.30.24.PM.movAndroid: mWeb ChromeScreen.Recording.2026-01-24.at.12.19.31.PM.moviOS: HybridAppScreen.Recording.2026-01-24.at.12.33.53.PM.moviOS: mWeb SafariScreen.Recording.2026-01-24.at.12.37.48.PM.movMacOS: Chrome / SafariScreen.Recording.2026-01-22.at.1.25.02.AM.mov |
|
It doesn't redistribute when editing splits, is that expected? Screen.Recording.2026-02-06.at.4.36.52.PM.mov |
|
Bug: Warning doesn't show when the total becomes greater than expected. Screen.Recording.2026-02-06.at.4.40.14.PM.mov |
Yes, if the splits were manually edited by the user before, they won't be auto-redistributed whenever adding new ones / editing / removing - this is part of the original PRs tests where this is expected.
✅ Addressed
I was comparing absolute values, but that deviated from staging behaviour so I reverted those specific changes to have preivous behaviour (staging) maintained ✅ ( @ShridharGoel Please give it a final pass and let me know if everything works as expected 🙏 |
This comment was marked as resolved.
This comment was marked as resolved.
|
@ShridharGoel Ok I think I got it (the AI not me), the ✅ I went back and made sure every previous case is corrected as well:
|
|
Bug: In edit mode, existing splits are now initialized with @blimpich Do you think this should be NAB (and handled in a follow-up if needed)? Screen.Recording.2026-02-12.at.11.13.48.AM.mov |
|
This was reported before by Codex, thought I fixed it already but there were conflicts due to refactoring and I probably reverted my changes without intention. Hold on it, I'll check again! |
|
@ShridharGoel I resolved this issue once again - mind taking a final pass at this ? 🙌 |
|
Seems to work well now |
|
🚧 @blimpich 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/blimpich in version: 9.3.19-0 🚀
|
|
@ikevin127 this PR is failing due to #82483 |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.19-5 🚀
|


Explanation of Change
Note
Rework of PR #79273 which had 3 regressions listed below in tests.
In Classic, when a user splits an expense, we try to update the amounts for all splits to equal the amount of the original transaction automatically. We opted to go a simpler route with NewDot splits to start, avoiding any automatic updates, and punted adding this logic to a follow-up. This is that follow-up:
Problem: When users are editing the separate portions of a split they are typically trying to ensure all the portions add up to the original transaction amount.
Solution: Automatically change amounts and percentages when creating a split to add up to the original transaction amount to remove clicks/steps for the submitter. To do so, we'll implement our Classic logic:
Core logic:
Behavior for Adding Splits
Simple adding splits Example:
Further editing example:
Error handling:
Fixed Issues
$ #70799
PROPOSAL:
Tests
Test 1: Adding Splits Auto-Redistributes
Test 2: Edited Splits Stay Locked
Test 3: Editing Existing Saved Splits (All Locked)
🧪 (2) Regressions tests
1. Split - Split amounts do not auto redistribute after removing split #81251
2. Split - Split amount field changes to NaN after clearing other split amount field #81241
NaNis displayed anywhere-, type a complete number like-50and verify the redistribution works correctlyOffline tests
Offline Test 1: Create Splits While Offline
Offline Test 2: Add Split While Offline
Offline Test 3: No Data Loss on Reconnect
QA Steps
Same as Tests.
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: HybridApp
android-hybrid.mp4
Android: mWeb Chrome
android-mweb.mp4
iOS: HybridApp
ios-hybrid.mov
iOS: mWeb Safari
ios-mweb.mov
MacOS: Chrome / Safari
web.mov