Stop converting distance when rate unit changes on odometer expenses#84253
Stop converting distance when rate unit changes on odometer expenses#84253
Conversation
When a user changes the distance rate to one with a different unit, the distance quantity is no longer automatically converted. The raw number is retained and only the unit label changes, aligning with OldDot behavior where odometer readings are physical readings that should not be retroactively converted. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Add eslint-disable-next-line comments for 30 pre-existing @typescript-eslint/no-deprecated violations that were caught by the newly-introduced eslint.changed.config.mjs configuration. None of these deprecation usages were introduced by this PR. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
Fixed the failing The Added |
|
Ready for review |
|
@MelvinBot Ready for review. I will review this PR |
|
@DylanDylann 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] |
|
PR has been marked as Ready for Review ✅ |
| const transaction = isDraft ? allTransactionDrafts[`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`] : allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; | ||
|
|
||
| let newDistance; | ||
| if (newDistanceUnit && newDistanceUnit !== transaction?.comment?.customUnit?.distanceUnit) { | ||
| newDistance = DistanceRequestUtils.convertDistanceUnit(getDistanceInMeters(transaction, transaction?.comment?.customUnit?.distanceUnit), newDistanceUnit); | ||
| } |
There was a problem hiding this comment.
@MelvinBot I don't think we should remove it. If this is an odormeter transaction then skip this convert
There was a problem hiding this comment.
Restored the distance conversion logic in setMoneyRequestDistanceRate with an added check to skip the conversion for odometer transactions. Now:
- Odometer transactions: distance quantity is retained as-is (raw odometer reading)
- Manual/map distance transactions: distance quantity is still converted when the rate unit changes
…odometer Instead of completely removing the distance conversion logic, keep it for non-odometer transactions (manual/map distance) and only skip the conversion for odometer transactions where the raw reading should be retained as-is. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
@DylanDylann @Julesssss One of you needs to 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] |
Keep the distance unit conversion for manual/map distance transactions when the rate unit changes, but skip it for odometer transactions since odometer readings are physical car readings that should be retained as-is. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
1 similar comment
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @Julesssss 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.32-0 🚀
|
|
QA team failed this PR with an original issue 1772780102061.mi_to_km.mp4 |
|
We need to update from BE to make it work totally |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.32-3 🚀
|
Explanation of Change
When a user changes the distance rate to one with a different unit (e.g., from a miles rate to a kilometers rate), the distance quantity was being automatically converted (400 mi → 643.74 km). Per the updated product decision aligned with OldDot behavior, the distance number should NOT be converted — it is retained as-is and only the unit label changes. Odometer readings are physical car readings and should not be retroactively converted when the workspace unit changes.
This PR removes the distance conversion logic in two places:
TransactionUtils/index.ts(getUpdatedTransaction): Removed the block that convertedquantityby a conversion factor when the rate unit changed. Also updated the amount recalculation to use the new unit (viagetDistanceInMeters(updatedTransaction, unit)) instead of the old unit, so the unchanged raw quantity is correctly interpreted under the new rate for amount/merchant calculation.IOU/index.ts(setMoneyRequestDistanceRate): Removed the distance conversion for draft transactions. The Onyx merge now only updatescustomUnitRateIDanddistanceUnitwithout touchingquantity.Fixed Issues
$ #82650
PROPOSAL: #82650 (comment)
Tests
Offline tests
This change only affects how distance quantities are handled during rate changes. Offline behavior is unchanged — the rate change updates
distanceUnitand recalculates amount/merchant optimistically regardless of network state.QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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