[WAITING ON FEDI - CHECKLIST] Web - Distance - Amount does not change if waypoint is changed via distance option

[kbecciv]

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: v1.4.23-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Click on FAB > Request money > Distance
2. Set start point and finish point > Click next > Select a workspace
3. Observe the amount
4. Click back two times
5. Change the finish way point > Click next > Select a workspace
6. Observe the amount has changed
7. Click back two times
8. Change the finish point to the original finish point selected on step 2
9. Click next > Select a workspace
10. Observe the amount
11. While on confirmation page click on distance
12. Change the finish point > Click on next
13. Observe the amount

Expected Result:

The amount should change since the finish point has been changed

Actual Result:

The amount does not change when user changes the finish point via the distance option from the confirmation page (without going back two times)

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6337433_1704875155064.PR-Fail-33959.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01eb4b0c3cc7b8e197
• Upwork Job ID: 1745052607647092736
• Last Price Increase: 2024-01-10
• Automatic offers:
• fedirjh | Reviewer | 28103764
• dukenv0307 | Contributor | 28103765

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[DanutGavrus]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Amount does not change if waypoint is changed via distance option.

What is the root cause of that problem?

In theMoneyTemporaryForRefactorRequestConfirmationList component we have a hook around shouldCalculateDistanceAmount which is not set to true when we change the distance from this component.

What changes do you think we should make in order to solve the problem?

We should set shouldCalculateDistanceAmount to true when changing any waypoint from here.

What alternative solutions did you explore?

[FIRST EDIT - Alternative solution added]

From what I see, getDistanceRequestAmount method inside DistanceRequestUtils makes use of simple math operations. I think we could remove our custom shouldCalculateDistanceAmount from MoneyTemporaryForRefactorRequestConfirmationList, and:

A. let the component calculate it each time.
B. make use of 'useMemo' so it re-calculates only when one or more values from distance, unit, or rate changes.

[FitseTLT]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Distance - Amount does not change if waypoint is changed via distance option

What is the root cause of that problem?

The root cause is that shouldCalculateDistanceAmount is not set to true on changing transaction route distance here

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 257 in e9f27ab

const shouldCalculateDistanceAmount = isDistanceRequest && iouAmount === 0;

What changes do you think we should make in order to solve the problem?

We should Change it to

const prevDistance = usePrevious(distance);
    const shouldCalculateDistanceAmount = isDistanceRequest && (iouAmount === 0 || distance !== prevDistance);

What alternative solutions did you explore? (Optional)

We can also use the same technique for rate and unit as we are using them to calculate amount

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 290 in e9f27ab

shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : iouAmount,

or
We can also use transaction.amount here

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 290 in e9f27ab

shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : iouAmount,

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The amount does not change when user changes the finish point via the distance option from the confirmation page (without going back two times)

What is the root cause of that problem?

We're not clearing the distance in Onyx when saving the waypoint.

When we save the new waypoint, we're already setting the amount to 0 here. It should have triggered this line to calculate and update the new correct amount to Onyx. But it didn't work because the distance is still stale (not updated via back-end yet), so the amount to be saved is still the old amount, we can verify this by adding logs, it will show that the transition of the amount is old amount > 0 > new amount.

So the problem here is that the distance is stale when saving new waypoint.

What changes do you think we should make in order to solve the problem?

Clear the distance in Onyx when saving the waypoint here

distance: null,

It doesn't make sense to keep the distance because the waypoint is already changed and the distance will be recalculated. We're already doing the same when removing the waypoint here

We also already clearing the old route here when saving waypoint due to the same reason.

// Clear the existing route so that we don't show an old route

What alternative solutions did you explore? (Optional)

We can check other operations related to amount as well, and make sure we also remove the distance in that case.

[jliexpensify]

Bump @fedirjh for reviews.

I've also let Dylan know here: https://expensify.slack.com/archives/C05DWUDHVK7/p1705021251503859

[fedirjh]

Thanks everyone for the proposal. I believe @dukenv0307 has correctly identified the root cause of the bug. shouldCalculateDistanceAmount evaluates to true when we select a new waypoint (which is expected), but the distance is not updated (stale). This distance is crucial in calculating the new amount. Resetting the distance seems to fix the issue.

Let's proceed with this proposal from @dukenv0307

🎀 👀 🎀 C+ reviewed