[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 contributors@expensify.com 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
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/2fee6c73-3c74-48b4-9a5f-00a8d0b11f48

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[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[bot]]

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

[melvin-bot[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 https://github.com/Expensify/App/pull/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.

https://github.com/Expensify/App/blob/a451de460c45611f64bcd06961afb78b4e2f3c9b/src/pages/home/report/ReportActionItem.tsx#L997

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 https://github.com/Expensify/App/pull/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 https://github.com/Expensify/App/pull/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[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]]

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

[ZhenjaHorbach]

I can help with review

[getusha]

@kadiealexander, @ZhenjaHorbach will handle this a C+

[ZhenjaHorbach]

@dominictb Thank you for your proposal Today I will consider your proposal in more detail

[melvin-bot[bot]]

@kadiealexander 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? 💸

[ZhenjaHorbach]

Not overdue sorry for delay I'll check the proposal today or tomorrow

[melvin-bot[bot]]

@kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[kadiealexander]

@ZhenjaHorbach any update on reviewing the proposals?

[ZhenjaHorbach]

@ZhenjaHorbach any update on reviewing the proposals?

Sorry I will review and give an answer today !

[ZhenjaHorbach]

@dominictb

I'm a little confused As far as I see, you created a new branch with changes that are not in your proposal

https://github.com/Expensify/App/compare/main...dominictb:epsf-app:fix/43577

And I'm wondering Which solution should be considered first?

From the branch or the proposal ? And could you add this solution to your proposal?

[dominictb]

@ZhenjaHorbach After finding the RCA my proposal has only one solution in the main solution. I updated my proposal https://github.com/Expensify/App/issues/43577#issuecomment-2164405961 to remove the outdated alternative solution.

As far as I see, you created a new branch with changes that are not in your proposal

@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.

The change in this branch is used to test my proposal without BE change

[ZhenjaHorbach]

@dominictb

I'm not sure that back-end changes are necessary to solve this issue As far as I can see we create specific error with specific id (key) using getLatestErrorMessageField

https://github.com/Expensify/App/blob/dfbdce771b1c28b7d14218cdcacf5dd3b6bce2a1/src/pages/home/report/ReportActionItem.tsx#L915

https://github.com/Expensify/App/blob/dfbdce771b1c28b7d14218cdcacf5dd3b6bce2a1/src/libs/ErrorUtils.ts#L93

I think in this case we need to find places where we use getLatestErrorMessageField and find out how we remove such errors

Could you please look into this question and how reasonable is this ?

Otherwise, I'm waiting for other proposals

[melvin-bot[bot]]

@kadiealexander, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ZhenjaHorbach]

Not overdue Still waiting for proposals

[dominictb]

@ZhenjaHorbach The problem here is the iou action is duplicated after SendInvoice API is complete. It leads the getOriginalReportID returns the wrong reportID for the system message action when we clear the error here. So the error cannot be dismissed.

[dominictb]

The problem here is the iou action is duplicated after SendInvoice API is complete

Base on that we need to prevent the duplicate actions to solve this issue. And to do that we should send the iouReportActionID to BE and use this to generate the iou action as we do for other flow like request money, track expense,...

[ZhenjaHorbach]

@dominictb Thanks for the clarifications I'll recheck your proposal tomorrow

[ZhenjaHorbach]

@dominictb I looked into this issue in more detail I think your proposal might help

But what worries me That the errors will still be unclickable since we will make changes related to the creation of the invoice But not with existing invoices

[dominictb]

That the errors will still be unclickable since we will make changes related to the creation of the invoice

@ZhenjaHorbach I don't understand the case you mentioned. Can you clarify this?

[ZhenjaHorbach]

That the errors will still be unclickable since we will make changes related to the creation of the invoice

@ZhenjaHorbach I don't understand the case you mentioned. Can you clarify this?

Your solution covers new invoices Since we are making changes to SendInvoice request

But what if we already have an error in an already created invoice ?

[dominictb]

But what if we already have an error in an already created invoice ?

@ZhenjaHorbach There're no problem if the invoice room is already created because the problem here is the duplicated of iou action and it's created every time we create a send invoice.

[ZhenjaHorbach]

But what if we already have an error in an already created invoice ?

@ZhenjaHorbach There're no problem if the invoice room is already created because the problem here is the duplicated of iou action and it's created every time we create a send invoice.

Sorry for the misunderstanding But I'm not talking about the invoice room 😅 I'm talking about existing invoices that have already been created

[dominictb]

I'm talking about existing invoices that have already been created

@ZhenjaHorbach What is the existing invoices, can you please add an example video?

[ZhenjaHorbach]

I'm talking about existing invoices that have already been created

@ZhenjaHorbach What is the existing invoices, can you please add an example video?

We are going to make changes to SendInvoice request Right ? But what will happen to invoices that were created before these changes (SendInvoice request without iouReportActionID)? In my understanding nothing will change And this bug will still be relevant for old invoices

[ZhenjaHorbach]

Plus As far as I can see In the last main there were many changes related to invoices And perhaps this issue is no longer relevant

Update: In the last main we have strange visual bugs But we can dismiss error after editing the amount of paid invoice

https://github.com/Expensify/App/assets/68128028/a74852e4-3fb6-4ac3-b1a8-45040cc8e919

[dominictb]

@ZhenjaHorbach This error can be dismissed because you access to the transaction thread report. If you go to the combine report without reset cache or logout and login again, you can't dismiss this error.

[dominictb]

But what will happen to invoices that were created before these changes (SendInvoice request without iouReportActionID)?

@ZhenjaHorbach Nothing happen, because the optimistic data will be cleared after we logout and login again and every work well.

[melvin-bot[bot]]

@kadiealexander @ZhenjaHorbach this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[ZhenjaHorbach]

But what will happen to invoices that were created before these changes (SendInvoice request without iouReportActionID)?

@ZhenjaHorbach Nothing happen, because the optimistic data will be cleared after we logout and login again and every work well.

This is what I wanted to know, thank you But I'm not entirely sure whether to ignore such a problem Since users will not often log out of their accounts

But on the other side, we can think about this problem when we update BE using this proposal Or after the opinion of an internal engineer

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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