[$250] Split distance-Distance amount reverts to original amount when saving distance without editing

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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): applausetester+kh2710002@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Open DM.
3. Split a distance expense with the user.
4. Go to transaction thread.
5. Note that distance is split in half.
6. Click Distance field.
7. Click Save without modifying anything.
8. Note that Distance field reverts to the original distance, but the amount does not change.
9. Click on the receipt.
10. Go back to main chat.
11. 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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021856609239459283873
• Upwork Job ID: 1856609239459283873
• Last Price Increase: 2024-11-13

Issue Owner

Current Issue Owner: @allroundexperts

[melvin-bot[bot]]

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.

[FitseTLT]

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

[melvin-bot[bot]]

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

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.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~021856609239459283873

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

[nkdengineer]

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

[Christinadobrzyn]

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.

[melvin-bot[bot]]

@allroundexperts, @Christinadobrzyn Huh... This is 4 days overdue. Who can take care of this?

[allroundexperts]

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

[melvin-bot[bot]]

Triggered auto assignment to @blimpich, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[FitseTLT]

@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

[nkdengineer]

Just another note: I'm the first one to explain why this only happens for distance requests for split distances.

[FitseTLT]

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

[allroundexperts]

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.

[melvin-bot[bot]]

@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!

[allroundexperts]

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