Expensify
[$250] Split distance-Distance amount reverts to original amount when saving distance without editing
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.59-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team
Action Performed:
- Go to staging.new.expensify.com
- Open DM.
- Split a distance expense with the user.
- Go to transaction thread.
- Note that distance is split in half.
- Click Distance field.
- Click Save without modifying anything.
- Note that Distance field reverts to the original distance, but the amount does not change.
- Click on the receipt.
- Go back to main chat.
- Note that the distance in the receipt and expense preview remains split in half and does not revert to the original amount.
Expected Result:
In Step 7, if saving distance without changing the waypoints is meant to revert the split distance to the original distance, it should also reflect the changes in expense amount, receipt and expense preview.
Actual Result:
In Step 8, after saving distance without changing the waypoints, Distance field reverts to the original distance, but the amount does not change. The receipt and expense preview still show the old amount and old distance.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [x] Android: mWeb Chrome
- [x] iOS: Standalone
- [x] iOS: HybridApp
- [x] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/c968b024-bcd0-472b-946a-3e593c78c373
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021856609239459283873
- Upwork Job ID: 1856609239459283873
- Last Price Increase: 2024-11-13
- Automatic offers:
- FitseTLT | Contributor | 105164804
Issue Owner
Current Issue Owner: @FitseTLT
Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Edited by proposal-police: This proposal was edited at 2024-11-08 13:31:14 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Split distance-Distance amount reverts to original amount when saving distance without editing
What is the root cause of that problem?
We are only submitting waypoints to the backend when the old address and the new one are different
https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L457
but in this case it is the same and we should revert the transaction back to it's back up value
https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L226-L230
but that only happens transactionWasSaved is false but we already set it to true here
https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L448
so now in money request view we will see the routes.route0.distance that was populated via useFetchRoute when we opened the distance page
https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L88-L93
What changes do you think we should make in order to solve the problem?
For editing case, we only don't want to restore the transaction if the old and new address are different. So we should set transactionWasSaved to true above here https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L461 change https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L447-L448
if (!isCreatingNewRequest && !isEditing) {
transactionWasSaved.current = true;
so the transaction will be restored from the backup on clicking submit without changing the waypoints
Side Note: Though not directly linked to this issue I have observed that the distance we are setting optimstically is just the total distance and it only gets it's correct split value from the BE so we can fix that by setting the split distance in customUnit of the transaction same as we are already doing for the splitShare.amount https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/libs/actions/IOU.ts#L4228
What alternative solutions did you explore? (Optional)
We can also clear routes of the transaction here
https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/pages/iou/request/step/IOURequestStepDistance.tsx#L227
@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Job added to Upwork: https://www.upwork.com/jobs/~021856609239459283873
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
In Step 8, after saving distance without changing the waypoints, Distance field reverts to the original distance, but the amount does not change. The receipt and expense preview still show the old amount and old distance.
What is the root cause of that problem?
When we create a split distance, the sub-transaction has quantity split from the total quantity
If we open the distance edit page, the quantity is updated to the current distance of waypoints. Then when we click on the save button without modifying anything, we do nothing here, but we still update transactionWasSaved.current to true.
Then the transaction is not reverted to the previous
https://github.com/Expensify/App/blob/c24d217f511e35d85b2b7828966e0a7ca5c0190e/src/pages/iou/request/step/IOURequestStepDistance.tsx#L448-L450
What changes do you think we should make in order to solve the problem?
When we edit the waypoint we should only update transactionWasSaved.current to true if we call updateMoneyRequestDistance. We can move these lines to here
And add transactionWasSaved.current = true here
We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint
https://github.com/Expensify/App/blob/c24d217f511e35d85b2b7828966e0a7ca5c0190e/src/pages/iou/request/step/IOURequestStepDistance.tsx#L230
What alternative solutions did you explore? (Optional)
NA
hey @allroundexperts this one is kinda weird (at least I think so) let me know if you need any questions answered from the team about expectations here. I think it's pretty clear but I had a hard time narrowing down the issue.
@allroundexperts, @Christinadobrzyn Huh... This is 4 days overdue. Who can take care of this?
~~Thanks for the proposals everyone.~~
~~I think @nkdengineer is correct in pointing out that the transaction will be reverted after we change the waypoint. @nkdengineer's proposal looks better.~~ ~~Let's go with them.~~
~~🎀 👀 🎀 C+ reviewed~~
Triggered auto assignment to @blimpich, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@allroundexperts @nkdengineer 's proposal is exactly as mine. I think both of you misunderstood my proposal. This part of his suggestion
When we edit the waypoint we should only update transactionWasSaved.current to true if we call updateMoneyRequestDistance. We can move these lines to here
is the same as what I said to exclude the case for isEditing
if (!isCreatingNewRequest && !isEditing) {
transactionWasSaved.current = true;
And this part of his solution
And add transactionWasSaved.current = true here
I have already stated: For editing case, we only don't want to restore the transaction if the old and new address are different. So we should set transactionWasSaved to true above here
App/src/pages/iou/request/step/IOURequestStepDistance.tsx
Line 461 in 8a83e2b
IOU.updateMoneyRequestDistance({
@nkdengineer has mislead you with his comments @allroundexperts Please re-take a look at my proposal 👍
cc @blimpich
Just another note: I'm the first one to explain why this only happens for distance requests for split distances.
Just another note: I'm the first one to explain why this only happens for distance requests for split distances.
The root cause of the issue has no relation with whether it is a split distance request or not. The result of the problem only gets visible for split distance request because we populate the distance with the splitted amount for split request and the fact that we are not reverting the transaction will be visible in this case because the distance will be set to the unsplitted/full amount. And the problem should be solved for all cases and that's why it is not important and I didn't mention. @allroundexperts bump on https://github.com/Expensify/App/issues/52241#issuecomment-2484426386 You have selected the second proposal which is exactly the same as mine which is the first one please re-consider your review and let's proceed into creating the PR. Thx
Just another note: I'm the first one to explain why this only happens for distance requests for split distances.
The root cause of the issue has no relation with whether it is a split distance request or not. The result of the problem only gets visible for split distance request because we populate the distance with the splitted amount for split request and the fact that we are not reverting the transaction will be visible in this case because the distance will be set to the unsplitted/full amount. And the problem should be solved for all cases and that's why it is not important and I didn't mention. @allroundexperts bump on #52241 (comment) You have selected the second proposal which is exactly the same as mine which is the first one please re-consider your review and let's proceed into creating the PR. Thx
@FitseTLT Please give me some time to re-review it. I'll get back on this today.
@blimpich @allroundexperts @Christinadobrzyn this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
I reviewed this again. Specifically, I tried to test if the transaction would be reverted after we change the waypoint. Although it seemed like that would be the case initially, but upon inserting the proposal code in the app, I wasn't able to verify that.
@nkdengineer can you elaborate the following more?
We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint
We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint
@allroundexperts this is my mistake, I missed the first step when reading the proposal.
The root cause of the issue has no relation with whether it is a split distance request or not
In general theory, if we do not change the address, nothing will happen even if we don't restore the transaction from the backup. So if we don't explain what happened to the split distance, it won't be clear why this bug happens.
@blimpich, @allroundexperts, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!
@blimpich, @allroundexperts, @Christinadobrzyn 6 days overdue. This is scarier than being forced to listen to Vogon poetry!
Hi @allroundexperts can you provide an update on this? I'm catching up after some ooo and not sure where we are. TY!
@nkdengineer Thanks for being honest here. Given that the transaction is not reverted, I'd agree with @FitseTLT that @nkdengineer's proposal is almost the same as @FitseTLT. As such, Let's go with @FitseTLT.
🎀 👀 🎀 C+ reviewed
Current assignee @blimpich is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Keep in mind: Code of Conduct | Contributing 📖
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.74-8 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/53628
If no regressions arise, payment will be issued on 2024-12-19. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @allroundexperts requires payment through NewDot Manual Requests
- @FitseTLT requires payment automatic offer (Contributor)
@allroundexperts @Christinadobrzyn @allroundexperts The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]