[Payment due][$500] Distance Expense - Impossible to remove a waypoint when editing a distance request offline

[isagoico]

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.26-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @paultsimura
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705505264249909 & https://expensify.slack.com/archives/C01GTK53T8Q/p1705484273717249

Action Performed:

1. Create a distance request with 3 waypoints
2. Navigate to the distance request details
3. Disable the internet connection
4. Click on the distance field to edit
5. Click on a waypoint
6. Click on the 3 dot menu > Delete waypoint
7. Hit Save
8. Go back online

Expected Result:

The waypoint should be deleted.

Actual Result:

The waypoint is not deleted after hitting the option to delete. After going back online, the 3 waypoints are still displayed.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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

Screen.Recording.2024-01-17.at.10.28.52.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01979a5064cc9154e6
• Upwork Job ID: 1747739146196148224
• Last Price Increase: 2024-01-24
• Automatic offers:
• alitoshmatov | Reviewer | 28124284
• paultsimura | Contributor | 28124285

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @lschurr (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 - @rushatgabhane (External)

[isagoico]

Posting @paultsimura proposal here:

Proposal

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

When editing a Distance request offline, the waypoint cannot be removed

What is the root cause of that problem?

This is because we do not handle the Editing flow correctly when passing the isDraft parameter to Transaction.removeWaypoint: we pass true always:

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 145 in 1f72b5f

Transaction.removeWaypoint(transaction, pageIndex, true);

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 165 in 1f72b5f

Transaction.removeWaypoint(transaction, pageIndex, true);

When we are creating a new request, we correctly remove the waypoint from a draft transaction. But when we edit an existing transaction, we should modify the real, not draft transaction.

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

We should pass the isDraft parameter conditionally depending on the IOU action, similar to how we pass it to Transaction.saveWaypoint:
action === CONST.IOU.ACTION.CREATE

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 138 in 1f72b5f

const saveWaypoint = (waypoint) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE);

What alternative solutions did you explore? (Optional)

[isagoico]

@tgolen confirmed the bug and the proposed fix here: https://expensify.slack.com/archives/C049HHMV9SM/p1705505264249909

[Vlad-AIMaster]

I will take a look at the issue and come up with a solution.

[melvin-bot]

📣 @Vlad-AIMaster! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[DylanDylann]

Proposal

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

The waypoint is not deleted after hitting the option to delete. After going back online, the 3 waypoints are still displayed.

What is the root cause of that problem?

The RCA here

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 145 in 4da7fda

Transaction.removeWaypoint(transaction, pageIndex, true);

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 165 in 4da7fda

Transaction.removeWaypoint(transaction, pageIndex, true);

We always pass isDraft: true to removeWaypoint function, so that new updates will be saved to the draft transaction

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

After refactoring, we use action field from the route to indentify if users create or edit their request. So we also need to rely on action field from the route to determine whether we should save new data into a transaction or a draft transaction

So, in these places

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 145 in 4da7fda

Transaction.removeWaypoint(transaction, pageIndex, true);

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 165 in 4da7fda

Transaction.removeWaypoint(transaction, pageIndex, true);

We should update isDraft pram to action === CONST.IOU.ACTION.CREATE like we did here

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 138 in 1f72b5f

const saveWaypoint = (waypoint) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE);

One more thing that the above proposal is omitted, we also need to update isDraft pram to action === CONST.IOU.ACTION.CREATE for updateWaypoint function here

App/src/pages/iou/request/step/IOURequestStepDistance.js

Line 159 in 4da7fda

Transaction.updateWaypoints(transactionID, newWaypoints, true).then(() => {

The updateWaypoint function also be used here

App/src/components/DistanceRequest/index.js

Line 203 in 4da7fda

Transaction.updateWaypoints(transactionID, newWaypoints).then(() => {

But this component will be removed here and we only use IOURequestStepDistance so that we don't need to fix in here

What alternative solutions did you explore? (Optional)

NA

[paultsimura]

One more thing that the above proposal is omitted

In my defense, this is a minor addition (that doesn't affect the issue, just covers a different flow) that 100% would have been caught during the PR since I always check other similar use cases when making a change.

This looks very similar to the discussion here:
#29895 (comment)
#29895 (comment)

Moreover, the change in here makes no UX difference, since this component is used only in the Create flow (not in Edit flow), so action === CONST.IOU.ACTION.CREATE here always will be true:

App/src/pages/iou/request/step/IOURequestStepDistance.js

Line 159 in 4da7fda

Transaction.updateWaypoints(transactionID, newWaypoints, true).then(() => {

[lschurr]

@rushatgabhane could you take a look at this one?

[lschurr]

Bump @rushatgabhane

[melvin-bot]

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

[alitoshmatov]

I will try to take a look at this today.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[alitoshmatov]

@paultsimura Thank you for proposal. Your RCA is correct. Your solution also works and on point.

[alitoshmatov]

@DylanDylann Thank you for proposal. Your proposal is very similar to @paultsimura 's proposal. I think it would be fair to go with his proposal.

[alitoshmatov]

We can go with @paultsimura 's proposal.

C+ reviewed 🎀 👀 🎀

[paultsimura]

We should wait for the hold anyway - this issue is still not complete without testing the issue we're holding for

[lschurr]

On hold

[lschurr]

On hold

[lschurr]

On hold

[lschurr]

Hold

[lschurr]

Still on hold

[paultsimura]

@lschurr this can be taken off hold – #37560 is in Production already.
Also, it seems like the issue was fixed by 4641b08 while the PR was on hold for Onyx fix.

Here's the timeline:

1. The PR was created and ready for review
2. I've noticed an Onyx bug that had to be fixed for this issue to be fully unblocked – [HOLD for payment 2024-08-07] [$500] Onyx updates are not being queued in the correct order #37560 – we've been holding for it the whole time.
3. Fix/34610: Remove NewDistanceRequestPage and EditRequestDistancePage #35302 was merged, which solved this issue from the Client side.

Should we be eligible for compensation here if the issue was resolved by another PR while this one was in progress?

[alitoshmatov]

Recently had a similar case and got this comment by @mallenexpensify . Not completely sure if it also applies here

VictoriaExpensify both nyomanjyotisa and alitoshmatov are due compensation here. Per the internal SO

If a contributor has been hired for a job and we decide to close the job before it is successfully completed, full payment is due for C+ and the contributor. One caveat here might be if the hired contributor wasn't working on their PR with urgency. If you're unsure, post in #bug-zero.

[lschurr]

Thanks @paultsimura and @alitoshmatov - I checked with the team and we agreed that we should pay full comp for the contributor on the PR and half comp to the C+ since proposals were reviewed but the PR was not.

Payment summary:

• Contributor: @paultsimura $500 (Paid in Upwork)
• Reviewer: @alitoshmatov $250 (Paid in Upwork)

[paultsimura]

Thanks, I've accepted the offer.

[lschurr]

All set!