Fix regressions from SelfDM splits flow - #97475
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
About this issue — it's a fairly specific bug, we discussed it here 2026-07-30.15.25.10.mov |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5865e216f7
ℹ️ 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".
|
@dmkt9 can you please start reviewing it? |
Yes, I will review this tomorrow. Currently, it’s 11 p.m. in my local time zone |
| // A reverse split brings the original transaction back in place of every split, so all of them are removed | ||
| // outright rather than marked for deletion. A split left marked keeps its report action visible (the offline | ||
| // surfaces render a pending deletion on purpose), and its pending state has nothing left to resolve it. |
There was a problem hiding this comment.
I don't think not adding a pending deletion is the right approach. Without it, when a transaction is deleted while offline, it disappears immediately, which isn't the expected behavior.
There was a problem hiding this comment.
We should resolve this issue completely (perhaps in a follow-up PR) rather than adding a temporary fix.
There was a problem hiding this comment.
I already answered here
#97475 (comment)
And I think this case is more minnor then main issue
There was a problem hiding this comment.
No. If we're going with this approach, you should get confirmation from production first, since it changes the behavior.
There was a problem hiding this comment.
This change only affects offline behaviour until online is restored
In the case of the old behavior, we had dead expenses in offline mode all the time or until the cache was cleared
But asked here
https://expensify.slack.com/archives/C07NMDKEFMH/p1785489150929189?thread_ts=1785236402.359539&cid=C07NMDKEFMH
|
This doesn't fully resolve #97403. 2026-07-31.15-39-37.mp4 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dfb8d87b0
ℹ️ 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".
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari97403.mp497413.mp497424.mp4 |
By the way 2026-07-31.11.29.18.mov |
It's still different from our case. In my video, you can see that the distance rate, distance amount, and merchant are displayed incorrectly after splitting. |
@ZhenjaHorbach Did you check this comment? Should we address this one as well? |
|
I will ping when this PR is ready for rechecking |
|
@dmkt9 |
|
@ZhenjaHorbach Could you recheck? I still see that the bug hasn't been resolved with these changes. 2026-08-03.21-44-21.mp4 |
|
It's a BE issue |
Explanation of Change
Fixed Issues
$ #97403
#97413
#97424
PROPOSAL:
Tests
#97403 — Merchant and Distance field must use the same unit
Precondition: a workspace with Distance rates enabled, unit set to Miles.
the merchant (mi).
each merchant still shows mi.
#97413 — Deleting splits from Spend must not leave a stuck preview in Self DM
Precondition: account has a Self DM.
#97424 — Moving the remaining split to a workspace must not leave a message in Self DM
Precondition: account has a Self DM and at least one workspace.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
2026-07-30.11.46.07.mov
2026-07-30.12.49.51.mov
2026-07-30.14.26.28.mov
2026-07-30.15.25.10.mov