Fix commuter distance flicker after rate changes - #98371
Conversation
|
@marufsharifi 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-08-12.at.2.35.13.AM.mov |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b65e71d19f
ℹ️ 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".
|
@fedirjh, could you please check the failed actions. thanks. |
|
@marufsharifi This is ready. |
|
@MelvinBot Review |
Co-authored-by: Maruf Sharifi <maruf.sharifi.work@gmail.com>
ReviewThe logic change is sound — deriving the converted quantity from the exact 🔴 Dangling duplicate expression breaks
|
Explanation of Change
When a workspace distance unit changed, the optimistic rate update converted the transaction's already rounded quantity into the new unit. For example, an exact 10 km route stored as 6.21 miles became 9.99 km optimistically, then changed to 10 km when the server response arrived. This caused the Original and Removed commuter distances to flicker between different values.
This change derives the converted quantity from the transaction's exact
routeDistanceMetersvalue when available. Older transactions without that field retain the existing quantity-conversion fallback. The commuter exclusion, amount, and merchant are therefore calculated from the same exact route distance as the server.Fixed Issues
$ #98231
PROPOSAL:
Tests
Automated validation:
bun run test -- --runInBand --watchman=false tests/unit/TransactionUtilsTest.tsbun x eslint src/libs/TransactionUtils/index.ts tests/unit/TransactionUtilsTest.tsbun run typecheck-tsgoreports only the two existing unrelated baseline errors.Verify that no errors appear in the JS console
Offline tests
QA Steps
Same as Tests.
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
CleanShot.2026-08-11.at.22.29.32.mp4
Android: Native
Not tested yet.
Android: mWeb Chrome
Not tested yet.
iOS: Native
Not tested yet.
iOS: mWeb Safari
Not tested yet.
MacOS: Chrome / Safari
Not tested yet.