[HOLD for payment 2024-11-11] [$250] Invoices - Invoice preview sender appears and disappears when sending invoices consecutively

[IuliiaHerets]

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: 9.0.51-1
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/5102388
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Send invoices.
3. Send an invoice to anyone.
4. In the invoice chat, click + > Send invoice.
5. Send another invoice.

Expected Result:

The invoice preview sender will not appear and disappear when sending invoices consecutively.

Actual Result:

The invoice preview sender appears then disappears when sending invoices consecutively.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6639618_1729340608381.20241019_201144.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021848369344890325291
• Upwork Job ID: 1848369344890325291
• Last Price Increase: 2024-10-21
• Automatic offers:
• hoangzinh | Reviewer | 104579791
• NJ-2020 | Contributor | 104579793

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

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

[IuliiaHerets]

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

[NJ-2020]

Proposal

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

Invoices - Invoice preview sender appears and disappears when sending invoices consecutively

What is the root cause of that problem?

When we want to display the report, we check if the displayAsGroup is false we will display the ReportActionItemSingle component which will show the avatar and the user information
Else we will show other component

The displayAsGroup only return true if the previous report.actorAccountID is equal to current report.actorAccountID

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

Lines 522 to 525 in 66cf824

	displayAsGroup={
	!ReportActionsUtils.isConsecutiveChronosAutomaticTimerAction(sortedVisibleReportActions, index, ReportUtils.chatIncludesChronosWithID(reportAction?.reportID)) &&
	ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(sortedVisibleReportActions, index)
	}

App/src/libs/ReportActionsUtils.ts

Line 581 in 66cf824

return currentAction.actorAccountID === previousAction.actorAccountID;

When we send the invoice we pass the wrong actorAccountID and after waiting a while after BE response, the BE return correct actorAccountID, because that we can see the report preview header is briefly showed and then disappear

App/src/libs/ReportUtils.ts

Line 5046 in 66cf824

actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.managerID ?? -1,

App/src/libs/ReportUtils.ts

Line 4495 in 66cf824

managerID: receiverAccountID,

As we can see the above code for managerID we're passing the receiverAccountID which is not correct

App/src/libs/actions/IOU.ts

Line 1946 in 66cf824

const reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, optimisticInvoiceReport, trimmedComment, optimisticTransaction);

Right here we pass the above data(reportPreviewAction) to onyx

App/src/libs/actions/IOU.ts

Line 1974 in 66cf824

reportPreviewAction,

So when we test on offline mode we can see the report preview header unless you turn on the Internet

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

Instead of passing the receiverAccountID here, we should pass the currentAccountID

App/src/libs/ReportUtils.ts

Line 4495 in 66cf824

managerID: receiverAccountID,

ownerAccountID: currentUserAccountID

Result

Screen.Recording.2024-10-19.at.21.15.25.mov

What alternative solutions did you explore? (Optional)

In here we can use ownerAccountID instead of managerID

App/src/libs/ReportUtils.ts

Line 5046 in 66cf824

actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.managerID ?? -1,

[melvin-bot]

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

[melvin-bot]

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

[hoangzinh]

@NJ-2020 RCA and his alternative solution look good to me. Just a minor suggestion: we can add an additional check that if the report is an invoice type, then we pass ownerAccountID instead of managerID

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[chiragsalian]

LGTM, feel free to create the PR @NJ-2020

[melvin-bot]

📣 @hoangzinh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @NJ-2020 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[NJ-2020]

PR ready
cc: @hoangzinh

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix report preview optimistic actorAccountID #51534

If no regressions arise, payment will be issued on 2024-11-11. 🎊

For reference, here are some details about the assignees on this issue:

• @hoangzinh requires payment automatic offer (Reviewer)
• @NJ-2020 requires payment automatic offer (Contributor)

[melvin-bot]

@hoangzinh @slafortune The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[abekkala]

PAYMENT SUMMARY FOR OCT 11

• Fix: @NJ-2020 [$250, if no regressions] offer
• PR Review: @hoangzinh [$250, if no regressions] offer
  please complete the checklist in above comment

[hoangzinh]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/40015/files#r1835616917

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  I think it's a small UI bug, we don't need a regression test for it.