Fix distance still shows Pending when preview and report header already show distance #46165

bernhardoj · 2024-07-25T06:31:41Z

Details

When creating a new distance request, the Distance field shows Pending even though the distance value exists. This PR fix it along with some cleanup.

Fixed Issues

$ #43569
PROPOSAL: #43569 (comment)

Tests

Same as QA Steps

Verify that no errors appear in the JS console

Offline tests

Same as QA Steps

QA Steps

Prerequisite:

Have a workspace with multiple distance rates
P2P distance enabled
Enable distance rate tax tracking
Set Tax Rate and Tax reclaimable on for each rate

Open a workspace expense chat
Press the + button > Submit expense > Switch to distance tab
Enter any waypoints and next
In the confirmation page, try changing the rate value
Verify the amount and tax is updated
Submit the request
Open the request transaction thread
Verify the Distance field shows the distance value instead of Pending...
Press the Distance field and update the waypoints
Verify the distance value is updated

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

android.mp4

Android: mWeb Chrome

android.mweb.mp4

iOS: Native

ios.mp4

iOS: mWeb Safari

ios.mweb.mp4

MacOS: Chrome / Safari

web.mp4

MacOS: Desktop

desktop.mp4

melvin-bot · 2024-07-25T06:31:45Z

@parasharrajat Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

bernhardoj · 2024-07-25T06:37:06Z

src/components/MoneyRequestConfirmationList.tsx

-            return DistanceRequestUtils.convertToDistanceInMeters(value, unit);
-        }
-        return value;
-    }, [isMovingTransactionFromTrackExpense, unit, transaction, canUseP2PDistanceRequests]);


The distance logic here was updated in #43819 to fix #42959. The issue they face is actually the same as ours, which is the transaction.comment.customUnit.quantity value is in meters from GetRouteForDraft, but in the workspace unit when the request is completely created.

I have retested the step from the linked PR and verify it still works as expected as in the linked PR.

web_2.mp4

parasharrajat · 2024-07-27T19:38:10Z

src/libs/TransactionUtils/getDistanceInMeters.ts

+    if (transaction?.routes?.route0?.distance) {
+        return transaction.routes.route0.distance;
+    }
+    if (transaction?.comment?.customUnit?.quantity && unit) {
+        return DistanceRequestUtils.convertToDistanceInMeters(transaction.comment.customUnit.quantity, unit);


let's add some comments to explain these conditions.

parasharrajat · 2024-07-30T12:59:46Z

Screenshots

🔲 iOS / native

🔲 iOS / Safari

🔲 MacOS / Desktop

31.07.2024_18.19.55_REC.mp4

🔲 MacOS / Chrome

31.07.2024_18.16.15_REC.mp4

🔲 Android / Chrome

🔲 Android / native

parasharrajat · 2024-07-30T16:39:29Z

@bernhardoj Looks like we have a bug when we share the tracked expense with accountant.

Here should the distance should be converted from miles to Km when unit of the workspace is km. Right?

30.07.2024_22.05.31_REC.mp4

paultsimura · 2024-07-30T17:21:51Z

FYI guys, to avoid conflicts and discrepancies, I've copied your getDistanceInMeters code straight into my #40021

bernhardoj · 2024-07-31T01:25:18Z

@parasharrajat yep, I noticed that too but that behavior is expected from #43819. (which means it's happening on main too)

parasharrajat · 2024-07-31T07:06:54Z

I see. I will continue testing then

parasharrajat · 2024-07-31T13:19:24Z

I am seeing some differences on Staging and Dev when we are transferring the tracked expenses to Workspace.

Staging

31.07.2024_18.46.23_REC.mp4

Dev

31.07.2024_18.48.18_REC.mp4

On Dev,
While transferring the transaction to workspace, we see KM(default Unit) on Confirm page. But when the transaction is created, it is shown in miles.

bernhardoj · 2024-07-31T13:40:22Z

@parasharrajat by dev you mean main right? I can repro it after merging main into this PR. Maybe someone is trying to fix #46165 (comment) but not complete. As long as the behavior matches between this PR and main, I think we are good.

parasharrajat · 2024-07-31T14:26:02Z

by dev you mean main right?

yes.

parasharrajat · 2024-07-31T21:22:39Z