[HOLD][$250] Send invoice - Unable to dismiss error after editing the amount of paid invoice

[lanitochka17]

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.82-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. [User A] Send invoice to User B
3. [User A and B] Go to invoice thread
4. [User B] Pay the invoice
5. [User A] Quickly change the amount
6. [User A] Dismiss the error under the amount edit system message

Expected Result:

User A will be able to dismiss the error under the amount edit system message

Actual Result:

User A is unable to dismiss the error under the amount edit system message

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

Add any screenshot/video evidence

Bug6510708_1718191227558.20240612_191648.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016ab97d06171fcfe0
• Upwork Job ID: 1801113320162208595
• Last Price Increase: 2024-06-27
• Automatic offers:
• ZhenjaHorbach | Contributor | 102939661

Issue Owner

Current Issue Owner: @cristipaval

[melvin-bot]

Triggered auto assignment to @kadiealexander (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.

[lanitochka17]

@kadiealexander FYI 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

[lanitochka17]

We think that this bug might be related to #vip-bills

[melvin-bot]

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

[melvin-bot]

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

[dominictb]

Proposal

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

User A is unable to dismiss the error under the amount edit system message

What is the root cause of that problem?

When we clear the error of a report action, we get the originalReportID of the action here but it's wrong after we send invoice.

The problem is the iou action is duplicated after SendInvoice API is complete because BE doesn't have the reportActionID of optimistic iou action in front-end to generate iou action.

It leads transactionThreadReportID here being undefined and this function return the reportID as expense reportID.

The invoice room still displays as the combine report because when we get reportActions here that doesn't include the optimistic iou action.

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

We should pass iouReportActionID as a parameter of SendInvoice API here and back-end need to use this ID to generate the iou action.

What alternative solutions did you explore? (Optional)

NA

[dominictb]

Proposal edited to add potential enhancements

[dominictb]

Proposal edited to add another alternative solution

[kadiealexander]

@getusha bump, please review.

[getusha]

Reviewing, testing @dominictb's proposal.

[getusha]

Thanks for the proposal @dominictb,
I am not sure i understand your RCA in full. i feel like this might be a regression from https://github.com/Expensify/App/pull/37875/files could you expand on that please? and lets make sure we don't introduce the linked issue.

[dominictb]

I am not sure i understand your RCA in full. i feel like this might be a regression from https://github.com/Expensify/App/pull/37875/files could you expand on that please?

@getusha Thanks for your feedback. I don't think it's a regression from #37875. The problem is for the combine report we have both iou report actions and transaction thread report actions but we always pass the report.reportID when we clear the report action error. As such, the error cannot be deleted when we dismiss the system message that is a report action of transaction thread report.

App/src/pages/home/report/ReportActionItem.tsx

Line 997 in a451de4

onClose={() => ReportActions.clearAllRelatedReportActionErrors(report.reportID, action)}

lets make sure we don't introduce the linked issue.

Sure, my proposal will not introduce the linked issue because it will not have any effect on clearing child or parent report action errors.

[getusha]

@getusha Thanks for your feedback. I don't think it's a regression from #37875. The problem is for the combine report we have both iou report actions and transaction thread report actions but we always pass the report.reportID when we clear the report action error. As such, the error cannot be deleted when we dismiss the system message that is a report action of transaction thread report.

@dominictb we have to understand where this issue came from, i tried to see commit history and it looked like it always has been like that. can we investigate about that?

[dominictb]

I think it happens after we add the one transaction view here #36934.

[getusha]

@dominictb thanks, i'll check it! could you share a branch so that i can test your solution?

[dominictb]

@getusha I will prepare my branch and share tomorrow morning.

[dominictb]

@getusha After testing, I found the correct RCA. I updated my proposal with the new RCA and new solution. To test my solution without BE change you can test here.

[melvin-bot]

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

[cristipaval]

Held on #43797

[cristipaval]

@cristipaval As per my proposal, we still need FE change to add another param to SendInvoice API. Since the C+ also approved my proposal here, can you please assign me here, and then I can handle the FE change once the BE change is complete?

This issue has the same root cause as this one, and we already have a contributor who proposed a more complete solution.

[ZhenjaHorbach]

Not overdue
Another issue is in progress

[cristipaval]

Still held

[cristipaval]

still held

[cristipaval]

We're actively working on the issue that this one is held on

[rayane-djouah]

PR is merged

[cristipaval]

The PR fixing the issue that this one is held on has been merged. Waiting for it to get deployed.

[cristipaval]

This issue can be retested in staging

[ZhenjaHorbach]

@cristipaval @kadiealexander
I can't reproduce
Let's close this issue !

[ZhenjaHorbach]

@cristipaval @kadiealexander
Friendly bump !

Maybe we should ping the testers for final confirmation to close this issue?