[HOLD for payment 2024-10-24] [$250] Invoices - Hold option is present in context menu before invoice report is opened

[IuliiaHerets]

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: v9.0.21-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/4864172
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Send invoice.
3. Enter amount, select any user and send the invoice.
4. Without clicking on the invoice preview (important), right click on the invoice preview in invoice chat.
5. Note that Hold option is present.
6. Click on the invoice preview.
7. Return to invoice chat.
8. Right click on the invoice preview.
9. Note that Hold option disappears.

Expected Result:

In Step 5, Hold option should not appear for invoice because invoices cannot be held.

Actual Result:

In Step 5, Hold option appears for invoice.
In Step 9, Hold option disappears after user opens the invoice report.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6573869_1723821203429.20240816_230930.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021839686152805650965
• Upwork Job ID: 1839686152805650965
• Last Price Increase: 2024-09-27

Issue Owner

Current Issue Owner: @VictoriaExpensify

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (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]

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

[IuliiaHerets]

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

[codewaseem]

Edited by proposal-police: This proposal was edited at 2024-09-17 16:16:35 UTC.

Proposal

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

The hold option appears in the context menu which should not happen for invoices.

What is the root cause of that problem?

The areHoldRequirementsMet condition doesn't exclude invoice type here:

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx

Lines 178 to 180 in 6aa873d

	const areHoldRequirementsMet = isMoneyRequestOrReport && !ReportUtils.isArchivedRoom(transactionThreadReportID ? childReport : parentReport, parentReportNameValuePairs);

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

Making sure areHoldRequirementsMet returns false for invoice type solves this problem. Here are the changes to be made

   const isMoneyRequestOrReport = isMoneyRequestReport || isSingleTransactionView;

    const areHoldRequirementsMet =
        !isInvoiceReport && isMoneyRequestOrReport && !ReportUtils.isArchivedRoom(transactionThreadReportID ? childReport : parentReport, parentReportNameValuePairs);

Added !isInvoiceReport to areHoldRequirementsMet and removed isInvoiceReport from isMoneyRequestOrReport which is not need.

To hide the hold option from ReportDetails view, need to update the ReportUtils.canHoldUnholdReportAction function with the following

function canHoldUnholdReportAction(reportAction: OnyxInputOrEntry<ReportAction>): {canHoldRequest: boolean; canUnholdRequest: boolean} {
    if (!ReportActionsUtils.isMoneyRequestAction(reportAction)) {
        return {canHoldRequest: false, canUnholdRequest: false};
    }

    const moneyRequestReportID = ReportActionsUtils.getOriginalMessage(reportAction)?.IOUReportID ?? 0;
    const moneyRequestReport = getReportOrDraftReport(String(moneyRequestReportID));

    if (!moneyRequestReportID || !moneyRequestReport) {
        return {canHoldRequest: false, canUnholdRequest: false};
    }

    if (isInvoiceReport(moneyRequestReport)) {
        return {
            canHoldRequest: false,
            canUnholdRequest: false,
        };
    }
   ...

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[VictoriaExpensify]

I agree this is an issue and we should fix it!

[melvin-bot]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[melvin-bot]

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

[melvin-bot]

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

[abdulrahuman5196]

Checking now

[codewaseem]

Friendly bump @abdulrahuman5196.

[abdulrahuman5196]

Sorry checking again

[abdulrahuman5196]

@VictoriaExpensify / @IuliiaHerets I don't understand the issue? Are we not supposed to show the Hold in the context menu, because if I go to the invoice view I can see the Hold option.
If we shouldn't show the Hold option in context, under what conditions we shouldn't show?

Screen.Recording.2024-08-29.at.8.29.58.PM.mov

[melvin-bot]

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

[melvin-bot]

@abdulrahuman5196, @VictoriaExpensify Still overdue 6 days?! Let's take care of this!

[VictoriaExpensify]

Posting to get some clarification on this one

[codewaseem]

@mallenexpensify I have applied for the Upwork job, please have a look. Thanks.

[mallenexpensify]

Offer sent @codewaseem !

[codewaseem]

@mallenexpensify thanks for the offer!

@abdulrahuman5196 @mjasikowski PR is ready for review. This is my first PR here, please let me know if I have missed anything.

Thanks!

[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.49-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• hide hold option for invoices #49875

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

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

• @codewaseem requires payment (Needs manual offer from BZ)
• @abdulrahuman5196 requires payment through NewDot Manual Requests

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] A discussion in #expensify-bugs 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:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@VictoriaExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[VictoriaExpensify]

Payment Summary:
Contributor: @codewaseem paid $250 via Upwork

@abdulrahuman5196 - can you please complete the checklist and I'll organise payment. TY!

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @codewaseem paid $(AMOUNT) via Upwork (LINK)
• Reviewer: @abdulrahuman5196 owed $250 via NewDot

BugZero Checklist (@VictoriaExpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1839686152805650965/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[codewaseem]

Thanks @VictoriaExpensify!

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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/45151/files#r1819687331

A discussion in #expensify-bugs 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:

Not required.

Determine if we should create a regression test for this bug.

Yes.

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Create an invoice with some amount and send it to another user.
2. Right-click on the invoice preview you just created to open the floating context menu.
3. Make sure there is no "Hold" option shown in the list.
4. Open the invoice by clicking the invoice preview.
5. Click on the invoice header to open the invoice details side-drawer.
6. Make sure there is no "Hold" option shown.

@VictoriaExpensify

[VictoriaExpensify]

Thanks @abdulrahuman5196 ! TestRail issue created - https://github.com/Expensify/Expensify/issues/439649

[VictoriaExpensify]

Payment Summary:

Contributor+: @abdulrahuman5196 owed $250 via NewDot

[JmillsExpensify]

$250 approved for @abdulrahuman5196