Distance offline - Can't delete red dot Error on the request preview in the expense report

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.52-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/4424763&group_by=cases:section_id&group_order=asc&group_id=295814
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Turn off the network
2. Crete distance request with way points kind of “hdhdfjfnfb”
3. Turn on the network
4. Wait for Reddot error
5. Open request details (now you have two reddot errors in the LHN)
6. Try to dismiss reports with reddot errors

Expected Result:

The user should be able to dismiss red dot errors because of invalid Distance expense

Actual Result:

There is no button to dismiss red dot error on the request preview in the expense report.

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

Screen.Recording.2024-03-15.at.14.43.12.mp4

View all open jobs on GitHub

[melvin-bot]

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

[izarutskaya]

@alexpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think that this bug might be related to #wave-collect - Release 1
CC @trjExpensify

[trjExpensify]

It would be great if the steps were more specific about which chats etc have the red brick road and don't clear properly. Here's my understanding of the issue being reported:

Action steps

1. Go offline
2. Crete a distance request with invalid waypoints (e.g "hdhdfjfnfb”)
3. Go back online
4. Observe the "unexpected error" message is added as the distance requests fails to create
5. Observe the red brick road (RBR) red dots added to both the workspace chat row and the expense report row in the LHN
6. Dismiss the error message ("X") beneath the report preview component in the workspace chat
7. Observe the expense report preview component disappears from the workspace chat and the RBR red dot is cleared from the workspace chat row in the LHN
8. Observe the expense report row in the LHN remains and with the RBR red dot
9. Click into the expense report
10. Click on the distance request preview
11. Observe the 404 "Hmm.. it's not there" full page blocking error

For the expected results. I think this is less about the RBR being dismissable on the expense report, but rather, why does it remain accessible at all? Recall you dismissed the error message and the report preview component disappeared, so it should have been deleted and thus removed from the LHN completely.

Expected Results
When the error message is dismissed, delete the distance request and expense report. (Of course, don't do that if there are other expenses on the report already!)

Let me know if I'm missing something! CC: @paultsimura @neil-marcellini @hayata-suenaga

9e4inAQbsN.mp4

[paultsimura]

For Expense requests, when the CreateDistanceRequest response comes, the Money Request and the Expense Report are removed, and only the Expense Report preview stays with the RBR. Am I missing something?

Screen.Recording.2024-03-15.at.15.03.4215.04.mp4

[trjExpensify]

Ah, interesting.. why isn't the expense report showing up in the LHN when you give it a go? See mine from above:

1. It fails after coming back online
2. The expense report row appears in the LHN with a red dot (in addition to the workspace chat row).
3. Dismissing the error on the expense report preview component inside the workspace chat doesn't remove the expense report row from the LHN

This is reproducible only for 1:1 Distance Requests (which is still under Beta).

My repro was distance requests to a workspace, so not 100% sure on that.

[paultsimura]

My repro was distance requests to a workspace, so not 100% sure on that.

That's weird indeed. Removed it from my comment to avoid misleading 🤔

[trjExpensify]

I thought it might be a violations beta or something, but I don't have that beta enabled on my @trj.chat domain.

[paultsimura]

why isn't the expense report showing up in the LHN when you give it a go?

It shows up, it just didn't fit the screen on the recording. I even tried pinning the optimistically created Expense report – it disappears after coming back online.

[trjExpensify]

Just tried it again, same result:

nBmuOa1o8z.mp4

[neil-marcellini]

I haven't investigated this much, but it should be noted that we never implemented the planned design for handling errors creating distance requests since it was deemed low priority.

Here is the issue for that [LOW] Distance: Handle errors with geocoding for offline distance requests. We keep having issues created about this. I think we should fix it and use a violations based approach.

[trjExpensify]

Ah yeah, totally. If we're committed to doing that, then let's go for it. Minimally, it shouldn't get stuck like it is now though in the current imp.

[alexpensify]

@neil-marcellini and @trjExpensify - should we assign this one as External?

[hayata-suenaga]

I didn't know that dismissing the error results in the deletion of the distance request. That's what I didn't expect...

But if the intended behavior is to delete the money request (expense report), then I agree it shouldn't be there after dismissing the error.

[trjExpensify]

Yeah, I think ideally we would handle this in a way that still creates a partial transaction that the member can edit the waypoints to fix it - @neil-marcellini references an approach in the linked issue.

I didn't think that was happening though. So if the expense fails to get created (not even "partially"), then dismissing the error on the "failed" expense would delete the expense report its on if:

• it's the only expense on the expense report
• there aren't any comments to preserve

Anyhow.. @hayata-suenaga do you think we can take the other issue external to implement a better process for this, or does it need to be internal? If it can go external, perhaps @paultsimura you might be interested in helping with that given you have good context around the distance feature.

[paultsimura]

Yes, I'd love to be a C+ if it goes external

[hayata-suenaga]

do you think we can take the other issue external to implement a better process for this, or does it need to be internal?

I read the requirements in the linked issue. I feel like this needs backend changes as well as front end changes. I'll wait for @neil-marcellini's response though 😄

[alexpensify]

Alright, waiting for @neil-marcellini's feedback.

[alexpensify]

Neil has been traveling, so I sent a chat so the last bump didn't get lost in the mix.

[neil-marcellini]

Hi thanks for the DM Al!

I really think that we should take this internal and design a complete solution with a small design doc. As Tom mentioned, we need to have partial transactions created instead of the request completely failing. Currently the request is not created at all in the backend, so that's why dismissing the error deletes everything. It would also go away if you sign out and back in. To do this the right way, we need backend changes.

Most of the desired behavior in the linked issue still makes sense, but it's written from the perspective of failing to create the initial distance request, rather than creating a partial transaction and using violations.

Let's also confirm broadly in Slack that we want to invest in fixing this now. I don't have the capacity to take this on, but maybe others are interested?

[alexpensify]

Ok, thanks! I'll start a discussion on Monday.

[alexpensify]

Thanks for the update! Just a heads up, @mallenexpensify will help move this GH forward when he returns from Holiday.

[mallenexpensify]

Checking on in #wave-collect

[mallenexpensify]

@paultsimura , I'm in the process of getting you added to  #wave-collect since there are offline distance conversations happening there.

This PR just hit production

• Reapply - Add offline search functionality for addresses #43011 (comment)

The above should (somewhat) help with minimizing the red dot error showing, if the user is traveling to/from the same places frequently.

For next steps specific to this issue, let's try to figure out why this distance expense that we couldn’t create isn’t being deleted when the RBR is dismissed. Do you have any idea why that might be happening @paultsimura ?

[neil-marcellini]

As I said in Slack, but repeating it here for @paultsimura, I think we don't clean up all the optimistic data for the distance request when it's deleted, so that's what we'll need to do.

[paultsimura]

Hey @neil-marcellini @mallenexpensify – from the discussion here and in Slack, I understand the proper solution, for now, will be to remove the failed distance request when dismissing the error. Is that correct?

If so – we can make this External as the solution is quite straightforward from what I see.

[melvin-bot]

@paultsimura, @mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

I'm uncertain, it sounds right but I defer to @neil-marcellini

[melvin-bot]

@paultsimura, @mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@paultsimura, @mallenexpensify Still overdue 6 days?! Let's take care of this!

[mallenexpensify]

@neil-marcellini 👀 above plz

[mallenexpensify]

Pinging Neil in NewDot for 👀

[neil-marcellini]

Sorry for the delay, thanks for the ping Matt.

Hey @neil-marcellini @mallenexpensify – from the discussion here and in Slack, I understand the proper solution, for now, will be to remove the failed distance request when dismissing the error. Is that correct?

If so – we can make this External as the solution is quite straightforward from what I see.

Yeah that's the plan. I think it was already implemented in this issue though [HOLD for payment 2024-07-26] [$250] BUG: Failed distance request isn't cleaned up properly. Matt can you please re-test and see if it's fixed now?

[mallenexpensify]

added Weekly and retest-weekly

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[mallenexpensify]

Not able to reproduce either, so closing.