[$500] [MEDIUM] Money Request - Offline Paid request in User A isn't deleted after returning online

[kbecciv]

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.28-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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

1. Using secondary device(User B), send request to main device(User A)
2. Go offline with User A and pay request elsewhere
3. Delete money request on secondary device(B)
4. On main device(A), go back online and observe request

Expected Result:

Sent money request is deleted on main device after returning online.

Actual Result:

Money request is not deleted and loses its paid status after returning online. Paying request does nothing.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/721cb864-d4ea-4f4c-b970-153b791c1c6c

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018c93e10b09de847a
• Upwork Job ID: 1748727434202918912
• Last Price Increase: 2024-02-03

Issue Owner

Current Issue Owner: @greg-schroeder

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[kbecciv]

We think that this bug might be related to #wave6-collect-submitters CC @greg-schroeder

[rojiphil]

@greg-schroeder @slafortune @allroundexperts I am not sure if we should allow payment(i.e. settlement) when offline. Or is this intentional?

[slafortune]

Adding you to this GH @greg-schroeder so you can clarify the expected outcome.

[greg-schroeder]

Hi! I'll take a look at this shortly

[greg-schroeder]

I am not sure if we should allow payment(i.e. settlement) when offline. Or is this intentional?

Yes, it's intentional afaik, but it should definitely be clearer. I chatted with @mountiny and he said that the paid action should still show up and the user should be able to click X to clear it up, alongside an error that explains what happened.

[allroundexperts]

Still looking for proposals.

[slafortune]

We'll give it another day before reaching out else where

[rojiphil]

Proposal

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

Money Request - Offline Paid request in User A isn't deleted after returning online

What is the root cause of that problem?

There is no current error handling mechanism in FE for payMoneyRequest when the money request report is deleted from the remote end and before the payMoneyRequest reaches the BE. This is the root cause.

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

As mentioned here, BE can send error message for the report preview and money request action. As a result of this, FE will show error messages for the report preview and money request action. Now when the user clears the error message, we can handle this here by looking specifically for the error message (sent by BE) for report preview and money request actions. If the error message is for a deleted report, we can reset the report preview action. Additionally, if the user clears the error from money request action, we can navigate back to the Chat Report screen.

We can implement the above-mentioned as follows here

const isMoneyRequestAction = ReportActionUtils.isMoneyRequestAction(reportAction);
if(isMoneyRequestAction || ReportActionUtils.isReportPreviewAction(reportAction)) {
    const latestErrorMsg = ErrorUtils.getLatestErrorMessage(reportAction as OnyxDataWithErrors);
    if(latestErrorMsg && latestErrorMsg[0] === CONST.ERROR.REPORT_DELETED) {
        const report = ReportUtils.getReport(reportID);
        const reportPreviewActionID = isMoneyRequestAction ? report?.parentReportActionID : reportAction.reportActionID;
        const chatReportID = isMoneyRequestAction ? report?.parentReportID : report?.reportID;

        // Let us remove the Report Preview action from chatReportID
        if(reportPreviewActionID && chatReportID) {
            Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`, {
                [reportPreviewActionID]: null,
            });
        }

        // Also, if we are clearing the error for Money Request action, we also need to navigate back to chat report
        if(isMoneyRequestAction && chatReportID) {
            Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(chatReportID));
        }
        return;
    }
}

In the above implementation, we can define CONST.ERROR.REPORT_DELETED here like this:

    REPORT_DELETED: 'iou.error.reportDeleted’,

I think it would be better for the BE to send an error code like iou.error.reportDeleted than the error string. This will help us to abide by the localization principle. BE can send an array with isTranslated set to false as demonstrated below:

In this case, we also need to add the error translations here and here

What alternative solutions did you explore? (Optional)

[rojiphil]

Sorry for the delay in response here. This completely went out of my mind.

@allroundexperts Do you think the above proposal is fine to solve the problem from FE or would we need some BE support here?

[greg-schroeder]

Thanks for the proposal! Let's see what Sibtain thinks

[greg-schroeder]

Go away melvin

[slafortune]

Does Melvin need me to tell them what to do??

[melvin-bot[bot]]

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

[slafortune]

@allroundexperts will you have time to review the proposal shared in the next day or so?

[allroundexperts]

Yep, on it today.

[allroundexperts]

@rojiphil I think it should be fine to just clear the report action but why do we need to refetch the report? I don't understand that part. Also, it would be great if you can share a short demo of your solution.

[greg-schroeder]

@rojiphil is next up to respond to the above! Just commenting to get Melvin off our backs

[slafortune]

@rojiphil will you be able to update today?

[melvin-bot[bot]]

@slafortune, @greg-schroeder, @allroundexperts Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rojiphil]

I think it should be fine to just clear the report action but why do we need to refetch the report? I don't understand that part. Also, it would be great if you can share a short demo of your solution

@allroundexperts I am unsure if payMoneyRequest can fail for any other reason. That is why I was suggesting fetching the report so that the report action would come back if at all failure is due to a reason other than the remote User deleting the Money Request. But now I am wondering if this looks like a workaround. I think we may have to look at this differently here.

If we look at the PayMoneyRequest response, BE throws 666 as jsonCode and code. Can we confirm in BE if this means that the money request is deleted?

If so, we can use this information in FE to clear the report action. Alternatively, BE can also set the report action to null on the failure of payMoneyRequest. But, this may not be good UX as the user would notice that the money request has disappeared suddenly.

What do you think about this?

[melvin-bot[bot]]

@slafortune @greg-schroeder @allroundexperts this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[greg-schroeder]

I'm unassigning myself as we have two people from BZ assigned and Stevie was the original assignee. @allroundexperts do you mind responding to the question posed above when you're able? Thanks!

[allroundexperts]

I think it should be fine to just clear the report action but why do we need to refetch the report? I don't understand that part. Also, it would be great if you can share a short demo of your solution

@allroundexperts I am unsure if payMoneyRequest can fail for any other reason. That is why I was suggesting fetching the report so that the report action would come back if at all failure is due to a reason other than the remote User deleting the Money Request. But now I am wondering if this looks like a workaround. I think we may have to look at this differently here.

If we look at the PayMoneyRequest response, BE throws 666 as jsonCode and code. Can we confirm in BE if this means that the money request is deleted?

If so, we can use this information in FE to clear the report action. Alternatively, BE can also set the report action to null on the failure of payMoneyRequest. But, this may not be good UX as the user would notice that the money request has disappeared suddenly.

What do you think about this?

Yes, if the code corresponds to the request being deleted, then the solution would be simplified. I agree with you @rojiphil. @slafortune Can you please loop in an internal engineer to confirm the meaning of the error code 666?

[melvin-bot[bot]]

@slafortune @allroundexperts this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@slafortune, @allroundexperts Huh... This is 4 days overdue. Who can take care of this?