[Hold pending BE] [$250] Distance - Distance edit system message only appears after refreshing the page

[izarutskaya]

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.32-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: https://expensify.testrail.io/index.php?/tests/view/4945031
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
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.
4. Go to transaction thread.
5. Click Distance.
6. Edit the distance and save it.

Expected Result:

Distance edit system message will appear after the new distance is saved.

Actual Result:

Distance edit system message does not appear after the new distance is saved. It only appears after refreshing the page.

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

Bug6599691_1726046020964.20240911_170930.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021834276537749459128
• Upwork Job ID: 1834276537749459128
• Last Price Increase: 2024-11-21

Issue Owner

Current Issue Owner: @Gonals

[melvin-bot]

Triggered auto assignment to @miljakljajic (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]

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

[melvin-bot]

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

[melvin-bot]

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

[tryevertthhub]

I got it issue, and I have faced this problem before while I developing notification app. I resolved with Socket.
Socket.IO can be an excellent solution to handle real-time updates in this case.
Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

[ZhenjaHorbach]

I got it issue, and I have faced this problem before while I developing notification app. I resolved with Socket. Socket.IO can be an excellent solution to handle real-time updates in this case. Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

@tryevertthhub
Thanks for your proposal !
But can you provide the proposal using PROPOSAL_TEMPLATE with code examples please?

[klajdipaja]

Edited by proposal-police: This proposal was edited at 2024-11-06 13:03:37 UTC.

Proposal

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

When the distance is changed in a distance based report the message that "distance was changed" is not shown in the chat.

What is the root cause of that problem?

The changes of a report are derived from the reportActions_ entry in Onyx which when the distance is change is not updated.
The screenshot below is the response of the BE when distance is changed, as you can see we do not have actions for the current report.

In contrast you can see below another screenshot when the category is updated on the same report on which case the message is shown:

In the screenshots you can also see how the Changed category messages are shown but not the distance ones.

When we reopen the report the Open Report API brings all the reportActions_ that include the ones indicating the distance update. Another screenshot of that below for illustration:

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

On the front end we can show a message optimistically for the changes to the distance that will show while the user is waiting for a response from the backend.
There are a few things to do to support that:

Step 1. Create optimistic data for waypoint changes

We build the optimistic data in this lines:

App/src/libs/actions/IOU.ts

Lines 2574 to 2575 in 7b9a0cd

	const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, isFromExpenseReport, policy, updatedTransaction);
	if (!hasPendingWaypoints && !(hasModifiedDistanceRate && TransactionUtils.isFetchingWaypointsFromServer(transaction))) {

But as you can see we use them only if change in the IOU request is not related to the distance. We have to modify the condition to remove the waypoint checks and all for the optimistic data to be pushed even when there's a waypoint change.
To allow for the optimistic data to be used we will replace the condition with

if (!hasModifiedDistanceRate) {

Step 2. Set the merchant and oldMerchant to the optimistic data

Distance messages are returned by the BE in the merchant and oldMerchant fields so it makes sense that we put the offline message in the merchant field.

We have to add a new block in ReportUtils.getModifiedExpenseOriginalMessage like this one:
https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts#L3642-L3645

Here's what the block would be:

if ('waypoints' in transactionChanges) {
        originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction);
        originalMessage.merchant = Localize.translateLocal('iou.updatedTheDistanceOptimistically');
    }

Step 3. Show the message for change distance.
The messages are resolved in ModifiedExpenseMessage.getForReportAction. Here we need to add a block that checks the change type and based on that return the message.
Here I recommend adding a block that returns the optimistic message if there all this conditions are fullfilled:

1. merchant has changed (hasModifiedMerchant)
2. the oldMerchant value matches the DISTANCE_MERCHANT regex which means that the user changed the distance
3. the merchant field which we set to the offline value matches that value which indicates that the action has an optimistic message and not the real one

   const optimisticDistanceUpdateMessage = Localize.translateLocal('iou.updatedTheDistanceOptimistically');

    const hasOptimisticDistanceUpdate =
        hasModifiedMerchant &&
        CONST.REGEX.DISTANCE_MERCHANT.test(reportActionOriginalMessage?.oldMerchant ?? '') &&
        (reportActionOriginalMessage?.merchant ?? '') === optimisticDistanceUpdateMessage;

    if (hasOptimisticDistanceUpdate) {
        return optimisticDistanceUpdateMessage;
    }

Step 4. Add translations to en.ts and es.ts

NOTE. Because of the issue in the BE not returning the report actions when the distance is updated the message would not dissappear unless we clear out the storage manually or logut and login again.

What alternative solutions did you explore? (Optional)

We could fix this in the front end if that was necessary by calling the Open Report API again when the distance has changed. If that's a solution you would like to asses I can than update my proposal but I do not think that should be the way to go.

[ZhenjaHorbach]

@klajdipaja
Thanks for your proposal !
I will review it today or tomorrow !

[tryevertthhub]

Edited by proposal-police: This proposal was edited at 2024-09-16 13:17:33 UTC.

Proposal

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

Distance edit system message does not appear after the new distance is saved. It only appears after refreshing the page.

What is the root cause of that problem?

The cause is asynchrony between the backend and the frontend.

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

I have faced this problem before while I developing notification app. I resolved with Socket.
Socket.IO can be an excellent solution to handle real-time updates in this case.
Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

What alternative solutions did you explore? (Optional)

You can solve this problem simply by using a socket.
An example of this can be found in the notification example in the Upwork.

Using automatic page refresh is also a good idea.
This is example code, in Next.js, I used router.fresh()

  const onSubmit: SubmitHandler<FieldValues> = (data) => {
      setIsLoading(true);
      signIn('credentials', {
        ...data,
        redirect: false
      })
      .then((callback) => {
        setIsLoading(false);

        if(callback?.ok) {
            toast.success('Logged in');
            router.refresh();
            loginModal.onClose();
        }

        if(callback?.error) {
            toast.error(callback.error);
        }
      })

    }
    ```
As klajdipaja already mentioned, using the Open Report API when the distance changes is one way.
However, page refresh is a more flexible way.

[ZhenjaHorbach]

@klajdipaja
Thanks for your proposal !
But I'm not sure that this is a problem with BE
I think that the problem is related to optimistic data

For example you can try to turn off the Internet
And you will notice that after changing the receipt details
Messages are added(only except changing Distance)

@tryevertthhub
And thank you for your updates !
But I'm not sure we need to implement new libraries
We just need to fix one bug using available code

[tryevertthhub]

@ZhenjaHorbach
we don't need add new library, just add refresh function
and I hope discuss more detail.
this is my email: tryevertth@gmail.com
this is my discord id: crazyolaf

[klajdipaja]

@ZhenjaHorbach You are right, it does seem to be related to the Optimistic data too.
According to this PR #47145 we cannot get the distance change information in the FE and because of that the optimistic data were disabled for the distance changes. It seems like that was the intended behavior for offline mode. If we want to change that we can use Turf.js to calculate the distance and have an offline message that might and probably will not reflect the same info as when the user goes online

We would need to disable this check in IOU#getUpdateMoneyRequestParams when the user is offline to build the optimistic data based on Turf.js distance calculations.

if (!hasPendingWaypoints && !(hasModifiedDistanceRate && TransactionUtils.isFetchingWaypointsFromServer(transaction))) 

And then add another case to ReportUtils#getModifiedExpenseOriginalMessage to build the message for the changes to the distance as if we would get them from the backend.

If we want to go with this approach we need to sync with the BE their logic on the distance and total value calculations.

And after that we still need from BE to actually return the MODIFIEDEXPENSE action as I mentioned in my proposal so that the user gets the "real" values from the BE.

If that's an approach you would consider I can update my proposal accordingly.

[ZhenjaHorbach]

@ZhenjaHorbach You are right, it does seem to be related to the Optimistic data too. According to this PR #47145 we cannot get the distance change information in the FE and because of that the optimistic data were disabled for the distance changes. It seems like that was the intended behavior for offline mode. If we want to change that we can use Turf.js to calculate the distance and have an offline message that might and probably will not reflect the same info as when the user goes online

We would need to disable this check in IOU#getUpdateMoneyRequestParams when the user is offline to build the optimistic data based on Turf.js distance calculations.

if (!hasPendingWaypoints && !(hasModifiedDistanceRate && TransactionUtils.isFetchingWaypointsFromServer(transaction))) 

And then add another case to ReportUtils#getModifiedExpenseOriginalMessage to build the message for the changes to the distance as if we would get them from the backend.

If we want to go with this approach we need to sync with the BE their logic on the distance and total value calculations.

And after that we still need from BE to actually return the MODIFIEDEXPENSE action as I mentioned in my proposal so that the user gets the "real" values from the BE.

If that's an approach you would consider I can update my proposal accordingly.

Oh, interesting
Thanks for the detailed explanation !
I didn't notice that we have a mention that we disabled optimistic data for the distance changes
I will check tomorrow more deeply

[melvin-bot]

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

[ZhenjaHorbach]

Looks like this issue is really related to BE
So this proposal makes sense for me !

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[klajdipaja]

Is there a reward for the proposal on this case when the proposal finds the root cause of the issue but it needs a BE fix ?

[Gonals]

Currently focusing on Travel and won't be able to get to this in a timely manner. I'll look for someone to take over

[ZhenjaHorbach]

Not overdue

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach 10 days overdue. Is anyone even seeing these? Hello?

[Gonals]

Getting to this soon!

[ZhenjaHorbach]

Not overdue

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach Still overdue 6 days?! Let's take care of this!

[Gonals]

Still working on travel. Hopefully back to this soon

[Gonals]

Same

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach Whoops! This issue is 2 days overdue. Let's get this updated quick!

[ZhenjaHorbach]

Not overdue

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot]

@Gonals, @RachCHopkins, @ZhenjaHorbach 12 days overdue now... This issue's end is nigh!

[Gonals]

Getting to this soon

[Gonals]

Same