[CP Staging] preserve distance when submitting from IOU to WS#90679
Conversation
|
@aimane-chnaif 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: 605e21d958
ℹ️ 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.
|
|
Tests failing |
Code ReviewOverall: The approach is sound. The A few items to consider:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b88a391b4
ℹ️ 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".
| const quantityInMeters = DistanceRequestUtils.convertToDistanceInMeters(quantity, distanceUnit); | ||
| // 1m tolerance swallows float-precision noise between the two representations. | ||
| return Math.abs(quantityInMeters - routeDistanceMeters) > 1; |
There was a problem hiding this comment.
Detect overrides using unrounded distance source
hasManualDistanceOverride() compares customUnit.quantity (converted to meters) against routes.route0.distance, but customUnit.quantity is often stored rounded to 2 decimals when routes are updated (see src/libs/TransactionUtils/index.ts:771). That rounding alone can exceed the 1m tolerance (e.g., 0.01 mi granularity is ~16m), so non-overridden GPS/map expenses can be misclassified as “manual override”. When that happens, convertTrackedExpenseToRequest() drops waypoints, which removes route context needed to preserve/regenerate the map receipt on submit.
Useful? React with 👍 / 👎.
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
Failure is unrelated, I'll re-run it |
This comment has been minimized.
This comment has been minimized.
|
@aimane-chnaif checks passing! |
|
I see PR is cloned. So is this PR is still valid and I should continue fixing?! @trjExpensify @youssef-lr @aimane-chnaif |
|
@youssef-lr is on it. We're testing BE as well. |
|
We can continue here as it looks like PR is not yet in mergeable state. Please check Tom's feedback above |
|
Let's know if @youssef-lr is working on it right now. Asked in Slack |
|
I'm working on it, the condition here App/src/libs/actions/IOU/TrackExpense.ts Line 1428 in 5658532 transaction?.comment?.customUnit?.routeDistanceMeters;. I'm relying on this to send a param to the backend which predates this feature and is only used from Classic: hasDistanceBeenManuallyUpdated which fixes the issue Tom found. @mukhrr if you'd like to continue pushing this forward feel free to merge my branch into your PR. I'm gonna push this fix in a bit.
|
|
Though we also need a backend fix for that. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1ca924a39
ℹ️ 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".
|
I see bugs are fixed. Answering to comments now |
|
The bug is fixed without sending |
|
@aimane-chnaif @Beamanator comments are adressed |
Regression AnalysisI reviewed the diff for three potential regression areas. Overall this PR is low-risk, but there are two items worth verifying. 1. False-positive risk in
|
|
@youssef-lr looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
🚧 @youssef-lr 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! 🧪🧪
|
[CP Staging] preserve distance when submitting from IOU to WS (cherry picked from commit 0eea15d) (cherry-picked to staging by Beamanator)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/Beamanator in version: 9.3.74-6 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes required. This PR is a bug fix that preserves manually-edited distance values when submitting expenses from self DM to a workspace. The changes are internal code logic only ( The existing Distance Expenses help article already covers creating, editing, and submitting distance expenses accurately. |
|
Staging ✅ 2026-05-15_17-16-26.mp4 |
|
🚀 Cherry-picked to staging by https://github.com/Beamanator in version: 9.3.74-7 🚀
Bundle Size Analysis (Sentry): |
|
👋 Thanks for the review request, No help site changes are required for this PR. This PR fixes an internal bug where manually edited distance values revert when submitting a tracked expense from self DM to a workspace. The changes are limited to:
No user-facing features, workflows, terminology, or UI were added or changed — this is a bug fix to preserve existing behavior (manually edited distances). The help site already documents distance expenses at a feature level, and nothing in that documentation needs updating. |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.74-7 🚀
|
|
Working well in iOS 9.3.74-6 still 👍 |


Explanation of Change
Fixed Issues
$ #90561
PROPOSAL:
Tests
Verify distance will not revert to the original distance after editing it and submitting the expense from self DM.
Verify that no errors appear in the JS console
Offline tests
QA Steps
The 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: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari