Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 12, 2024 · 65 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 12, 2024

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
  • 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 OwnerCurrent Issue Owner: @cristipaval
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 12, 2024
Copy link

melvin-bot bot commented Jun 12, 2024

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
Copy link
Author

@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
Copy link
Author

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

@kadiealexander kadiealexander added the External Added to denote the issue can be worked on by a contributor label Jun 13, 2024
@melvin-bot melvin-bot bot changed the title Send invoice - Unable to dismiss error after editing the amount of paid invoice [$250] Send invoice - Unable to dismiss error after editing the amount of paid invoice Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@dominictb
Copy link
Contributor

dominictb commented Jun 13, 2024

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
Copy link
Contributor

Proposal edited to add potential enhancements

@dominictb
Copy link
Contributor

Proposal edited to add another alternative solution

@kadiealexander
Copy link
Contributor

@getusha bump, please review.

@melvin-bot melvin-bot bot added the Overdue label Jun 17, 2024
@getusha
Copy link
Contributor

getusha commented Jun 17, 2024

Reviewing, testing @dominictb's proposal.

@melvin-bot melvin-bot bot removed the Overdue label Jun 17, 2024
@getusha
Copy link
Contributor

getusha commented Jun 17, 2024

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
Copy link
Contributor

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.

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
Copy link
Contributor

getusha commented Jun 17, 2024

@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
Copy link
Contributor

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

@getusha
Copy link
Contributor

getusha commented Jun 18, 2024

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

@dominictb
Copy link
Contributor

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

@dominictb
Copy link
Contributor

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

Copy link

melvin-bot bot commented Jun 20, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jun 20, 2024
Copy link

melvin-bot bot commented Jun 21, 2024

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

@ZhenjaHorbach
Copy link
Contributor

I can help with review

@dominictb
Copy link
Contributor

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

@melvin-bot melvin-bot bot added the Overdue label Jul 19, 2024
@cristipaval cristipaval changed the title [$250] Send invoice - Unable to dismiss error after editing the amount of paid invoice [HOLD][$250] Send invoice - Unable to dismiss error after editing the amount of paid invoice Jul 19, 2024
@cristipaval
Copy link
Contributor

Held on #43797

@melvin-bot melvin-bot bot removed the Overdue label Jul 19, 2024
@cristipaval
Copy link
Contributor

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

@cristipaval cristipaval added Weekly KSv2 and removed Daily KSv2 labels Jul 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
@ZhenjaHorbach
Copy link
Contributor

Not overdue
Another issue is in progress

@cristipaval
Copy link
Contributor

Still held

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 6, 2024
@cristipaval
Copy link
Contributor

still held

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 15, 2024
@cristipaval
Copy link
Contributor

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

@rayane-djouah
Copy link
Contributor

PR is merged

@melvin-bot melvin-bot bot added the Overdue label Aug 26, 2024
@cristipaval
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 26, 2024
@cristipaval
Copy link
Contributor

This issue can be retested in staging

@ZhenjaHorbach
Copy link
Contributor

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

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Sep 17, 2024

@cristipaval @kadiealexander
Friendly bump !

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Weekly KSv2
Projects
Development

No branches or pull requests

8 participants