Fix: Prevent changing currency on unreported distance expenses#76577
Fix: Prevent changing currency on unreported distance expenses#76577trjExpensify merged 10 commits intoExpensify:mainfrom
Conversation
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.
|
|
@eVoloshchak 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] |
trjExpensify
left a comment
There was a problem hiding this comment.
I'm not convinced this is how we should be solving the bug report.
On Classic, you can't change the currency of a distance expense created, whether that's reported on a workspace or unreported. You can change the amount, but not the currency.
So I'm thinking we should do the same with unreported expenses on NewDot too, and not allow changing the currency of an unreported distance expense. CC: @Expensify/product-pr @cristipaval
|
Waiting for the product review decision |
|
@trjExpensify, should I add this as a requirement in the issue related to distance requests from the |
|
Yeah, if you think that's best... or have this PR updated to focus on disabling currency on unreported distance expenses. |
|
let's focus this one on disabling the currency, since our PR is already in review. I'll inform the contributors on our PR about this one. cc @dangrous |
|
PR updated! |
|
@nyomanjyotisa, could you merge the latest |
|
@eVoloshchak I've merged with the latest |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-12-08.at.22.30.30.movAndroid: mWeb ChromeScreen.Recording.2025-12-08.at.22.31.27.moviOS: HybridAppScreen.Recording.2025-12-08.at.22.34.17.moviOS: mWeb SafariScreen.Recording.2025-12-08.at.22.32.26.movMacOS: Chrome / SafariScreen.Recording.2025-12-08.at.22.28.33.mov |
src/libs/TransactionUtils/index.ts
Outdated
| ); | ||
| } | ||
| // If modifiedMerchant is empty but modifiedCurrency exists, recalculate the merchant | ||
| if (!transaction?.modifiedMerchant && transaction?.modifiedCurrency) { |
There was a problem hiding this comment.
Can we just combine these two conditionals, so something like
if (
(policy?.customUnits && !isUnreportedAndHasInvalidDistanceRateTransaction(transaction, policy))
|| (!transaction?.modifiedMerchant && transaction?.modifiedCurrency)
) {
They come out the same, right?
There was a problem hiding this comment.
Yeah, we can do that. PR updated!
|
@trjExpensify or @heyjennahay do you want to take a look before we merge? |
|
Going to let @trjExpensify take this one |
trjExpensify
left a comment
There was a problem hiding this comment.
Thanks for making the update!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/trjExpensify in version: 9.2.78-0 🚀
|
|
🚧 @grgia 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.78-8 🚀
|
Explanation of Change
Prevent the user from changing the currency on unreported distance expenses
Fixed Issues
$ #74894
PROPOSAL: #74894 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
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-Native.mp4
Android: mWeb Chrome
Android-mWeb.Chrome.mp4
iOS: Native
iOS-Native.mp4
iOS: mWeb Safari
iOS-mWeb.Safari.mp4
MacOS: Chrome / Safari
MacOS-Chrome.mp4