feat: Edit Distance Rate flow#40021
Conversation
…rate-edit # Conflicts: # src/pages/iou/request/step/IOURequestStepRate.tsx
# Conflicts: # src/components/MoneyTemporaryForRefactorRequestConfirmationList.tsx
…rate-edit # Conflicts: # src/components/MoneyTemporaryForRefactorRequestConfirmationList.tsx
# Conflicts: # src/libs/Navigation/types.ts # src/libs/ReportUtils.ts # src/libs/actions/IOU.ts # src/pages/iou/request/step/IOURequestStepDistanceRate.tsx
# Conflicts: # src/libs/ReportUtils.ts # src/libs/TransactionUtils.ts # src/libs/actions/IOU.ts # src/pages/iou/request/step/IOURequestStepDistanceRate.tsx # src/types/onyx/Transaction.ts
This comment was marked as outdated.
This comment was marked as outdated.
# Conflicts: # src/libs/actions/IOU.ts
b99e213 to
14a078f
Compare
14a078f to
b7fa71e
Compare
|
@mananjadhav 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] |
|
Catching up on the latest of the issue and the PR. I'll review this by tomorrow. |
|
Thanks @mananjadhav, please note that the optimistic message differs from the online one a little (e.g. using |
|
I'll finish this today. |
|
|
||
| if (Object.hasOwn(transactionChanges, 'customUnitRateID')) { | ||
| updatedTransaction.modifiedCustomUnitRateID = transactionChanges.customUnitRateID; | ||
| shouldStopSmartscan = true; |
There was a problem hiding this comment.
I don't have the context for this, and hence asking, this is needed to be true right?
There was a problem hiding this comment.
Yes, just like updating waypoints or amount manually
neil-marcellini
left a comment
There was a problem hiding this comment.
Overall looking really good! I'm very impressed. I have a couple important questions so requesting changes for that.
# Conflicts: # src/components/ReportActionItem/MoneyRequestView.tsx
neil-marcellini
left a comment
There was a problem hiding this comment.
Good to go. There are conflicts so hopefully you can also fix the NAB comment related to calculateAmountForUpdatedWaypointOrRate and transactionChanges. Please let me know via DM when it's ready for review again.
|
@neil-marcellini I've found a potential bug: If I've tracked a distance expense to myself, and then shared it with an accountant, it starts showing the Do you know if this is the correct behavior? Shouldn't it use the workspace rate I'm selecting while sharing it? 2024-08-03.-.14.13.-.Screen.Recording.2024-08-03.at.14.12.49.mp4 |
# Conflicts: # src/libs/TransactionUtils/getDistanceInMeters.ts # src/libs/TransactionUtils/index.ts # src/libs/actions/IOU.ts
| type RateAndUnit = { | ||
| unit: Unit; | ||
| rate: number; | ||
| currency: string; |
There was a problem hiding this comment.
Added this so the RateAndUnit is more consistent with the MileageRate and we could use the currency from their union type: MileageRate | RateAndUnit.
There was a problem hiding this comment.
NAB: IMO let's delete this type and just use MileageRate, since RateAndUnit is a subset of it, and the name is not great and now inaccurate. It can be cleaned up in a follow up.
Yep that seems to be a bug with our existing backend. I think we never implemented the rate updates for this track and share with accountant flow. Please report the bug. |
neil-marcellini
left a comment
There was a problem hiding this comment.
I think it's good to go. Thank you!
| type RateAndUnit = { | ||
| unit: Unit; | ||
| rate: number; | ||
| currency: string; |
There was a problem hiding this comment.
NAB: IMO let's delete this type and just use MileageRate, since RateAndUnit is a subset of it, and the name is not great and now inaccurate. It can be cleaned up in a follow up.
There was a problem hiding this comment.
NAB: Maybe call this modifiedDistanceFields or something similarly descriptive?
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 9.0.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
2 similar comments
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
|
Coming from #46880, we should have added the rate as a |
|
Coming from #57730, the distance is not updating when the distance rate is changed offline. |
Details
This PR implements the "Edit Rate" functionality for Distance requests.
Fixed Issues
$ #36987
PROPOSAL: #36987 (comment)
Tests
Same as QA
Offline tests
Same as QA
QA Steps
Test 1:
Pre-requisite: have a workspace with multiple Distance Rates;
Test 2:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so 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
2024-07-22.-.13.07.-.android.mp4
Android: mWeb Chrome
2024-07-22.-.12.47.-.chrome.mp4
iOS: Native
2024-07-31.-.11.08.-.Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-07-31.at.11.08.31.mp4
iOS: mWeb Safari
2024-07-22.-.12.47.-.Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-07-22.at.12.30.56.mp4
MacOS: Chrome / Safari
2024-07-22.-.12.47.-.Screen.Recording.2024-07-22.at.12.11.41.mp4
MacOS: Desktop
2024-07-22.-.12.47.-.Screen.Recording.2024-07-22.at.12.22.34.mp4