[HOLD for payment 2024-12-25] [$250] Distance-Distance field is stuck on pending... and no error shown when updating of waypoints fails

[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+kh271000023@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit a distance expense with two waypoints.
4. Go to transaction thread.
5. Go offline.
6. Click Distance.
7. Edit the distance by swapping the position and save it.

Expected Result:

The distance field will change to "Pending" when editing the distance by swapping in offline mode.

Actual Result:

The distance field does not show "Pending" when editing the distance by swapping in offline mode.
It shows the same amount but it is grayed out.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6658691_1731061670854.20241108_182248.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021854958931528313891
• Upwork Job ID: 1854958931528313891
• Last Price Increase: 2024-11-22
• Automatic offers:
• FitseTLT | Contributor | 105096400

Issue Owner

Current Issue Owner: @bfitzexpensify

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (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-11 16:54:54 UTC.

Proposal

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

Distance-Distance field does not show Pending when editing distance by swapping in offline mode

What is the root cause of that problem?

We call updateWaypoints on Drag of waypoints

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

Line 519 in 533aed6

onDragEnd={updateWaypoints}

But we are not resetting the obsolete comment.customUnit.quantity in updateWaypoints

App/src/libs/actions/Transaction.ts

Lines 323 to 324 in 8a83e2b

	comment: {
	waypoints,

so hasRoute will return true so we see obsolete distance instead of pending

App/src/libs/TransactionUtils/index.ts

Lines 694 to 696 in 533aed6

	function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType?: boolean): boolean {
	return !!transaction?.routes?.route0?.geometry?.coordinates || (!!isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);
	}

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

We should reset it same as we are doing in saveWaypoints here

App/src/libs/actions/Transaction.ts

Lines 90 to 93 in 8a83e2b

	},
	customUnit: {
	quantity: null,
	},

            customUnit: {
                quantity: null,
            },

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

The distance field does not show "Pending" when editing the distance by swapping in offline mode.

What is the root cause of that problem?

The getDistanceForDisplay function is responsible for returning either the actual distance or Pending... depending on the arguments passed, the key one in our case for showing Pending... being hasRoute.

This is where the function is called and the hasRoute is passed. This is what the hasRoute function checks:

App/src/libs/TransactionUtils/index.ts

Lines 694 to 696 in 533aed6

	function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType?: boolean): boolean {
	return !!transaction?.routes?.route0?.geometry?.coordinates || (!!isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);
	}

In our case the first condition always passes because in updateWaypoints we set geometry.coordinates to null, resulting in !!transaction?.routes?.route0?.geometry?.coordinates passing because coordinates object exists with null value.

The reason why the second condition passes as well is because the !!transaction?.comment?.customUnit?.quantity check returns true as well since we do have quantity calculated already from the current transaction route and when we update it while online we don't reset the parent object as we should.

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

In updateWaypoints set customUnit object to null since its always populated by BE upon returning online.

function updateWaypoints(transactionID: string, waypoints: WaypointCollection, isDraft = false): Promise<void | void[]> {
    return Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
        comment: {
            waypoints,
            customUnit: null, // <- Add this line
        },
...

This change will also account for the following case of distance expense confirmation while offline:

Distance Cofirmation Change - Offline

Screen.Recording.2024-11-08.at.14.28.09.mov

Solution result

MacOS: Chrome

Screen.Recording.2024-11-08.at.14.17.42.mov

[hoangzinh]

Thanks for your proposals @FitseTLT @ikevin127.

@ikevin127 provided a more thorough root cause than @FitseTLT, I couldn't understand the root cause without your proposal. However, I lean towards to @FitseTLT solution, I think we only need to optimistic set

           customUnit: {
                quantity: null,
            },

is enough to fix our issue.

[ikevin127]

@hoangzinh Thanks for the feedback, however, regarding your decision, quoting from the proposal template (code):

ATTN: Contributor+

You are the first line of defense in making sure every proposal has a clear and easily understood problem with a "root cause". Do not approve any proposals that lack a satisfying explanation to the first two prompts. It is CRITICALLY important that we understand the root cause at a minimum even if the solution doesn't directly address it. When we avoid this step, we can end up solving the wrong problems entirely or just writing hacks and workarounds.
...
3. Choose the first proposal that has a reasonable answer to all the required questions.

I thought we cannot hire a contributor who's proposal's RCA is not clear enough, in this case I'd expect the proposal that provided a clear RCA and working solution to be assigned.

My bad, I didn't know that there are exceptions to those rules. I'll make sure to avoid issues reviewed by you in the future since I usually post proposals with those rules in mind 👍

[FitseTLT]

I think this issue has pretty clear RCA that's why I put a concise proposal.
It's pretty clear updateWaypoints is called on drag

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

Line 519 in 533aed6

onDragEnd={updateWaypoints}

and the obsolete customUnit data should be cleared and that is what we do for saveWaypoints which I mentioned in my proposal (that's why the problem doesn't occur if we change or add a waypoint from IOURequestStepWaypoint)
I would have made the review's job easier if I put more detail, I'm sorry about that 🙏 , but I thought it was pretty straightforward.