[HOLD for payment 2024-08-19] [$250] Send invoice - Hold option is present for invoice and the invoice cannot be held

[m-natarajan]

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: 9.0.7-4
Reproducible in staging?: y
Reproducible in production?: no
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4728016
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: applause internal team
Slack conversation:

Action Performed:

Precondition:

• User has received invoice.
• The following steps will be performed by the invoice receiver.

1. Go to staging.new.expensify.com
2. Go to invoice chat.
3. Click on the invoice preview.
4. Click on the report header.
5. Click Hold.
6. Enter a reason and save it.

Expected Result:

There should be no Hold option for invoice.

Actual Result:

Hold option is present for invoice and the invoice cannot be held.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Bug6544135_1721145772156.20240717_000026.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0178cd3e6d26424c8a
• Upwork Job ID: 1813589312856914205
• Last Price Increase: 2024-08-08
• Automatic offers:
• ishpaul777 | Reviewer | 103157119

Issue Owner

Current Issue Owner: @ishpaul777

[melvin-bot]

Triggered auto assignment to @Beamanator (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[neonbhai]

Proposal

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

Hold option is shown for invoice requests

What is the root cause of that problem?

From PR #44880

shouldShowHoldAction here does not account for invoice reports

App/src/pages/ReportDetailsPage.tsx

Lines 521 to 524 in c7d7bf4

	const shouldShowHoldAction =
	caseID !== CASES.DEFAULT &&
	(canHoldUnholdReportAction.canHoldRequest || canHoldUnholdReportAction.canUnholdRequest) &&
	!ReportUtils.isArchivedRoom(transactionThreadReportID ? report : parentReport, parentReportNameValuePairs);

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

We should not show Hold option for invoice reports. We will add a check for !ReportUtils.isInvoiceReport(report) in shouldShowHoldAction

[etCoderDysto]

Proposal

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

Hold option is present for invoice and the invoice cannot be held

What is the root cause of that problem?

We are referencing isPolicyAdmin function that is declared here, rather making isPolicyAdmin() function call. Therefore canModifyStatus is true for invoice recipient.

App/src/libs/ReportUtils.ts

Line 2826 in 12acd13

const canModifyStatus = !isTrackExpenseMoneyReport && (isPolicyAdmin || isActionOwner || isApprover);

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

• We should check if the user is a policy admin by calling isPolicyAdmin function.

const isAdmin = isPolicyAdmin(moneyRequestReport?.policyID ?? '-1', allPolicies);

• use isAdmin in place of isPolicyAdmin

What alternative solutions did you explore? (Optional)

Result:
For invoice scenario

Invoice.mp4

For admin for a workspace scenario

Admin.mp4

[rafecolton]

Seems like we are already enforcing non-holding of invoices on the back-end due to the error shown in the video. So I think that means this is only a front-end issue where we need to not show the button. @neonbhai's solution sounds simplest to me, but it's possible I'm missing some nuance about how invoices work.

[etCoderDysto]

Updated admin workspace scenario video

[cead22]

I agree this is a front end issue, so removing the deploy blocker label

[chiragsalian]

nice, i was just looking here and agree. it looks like a frontend issue and not a blocker for backend.

[thienlnam]

Going to remove the blocker - this is a new feature and I don't think invoices on NewDot are really used yet

[neonbhai]

Seems like we are already enforcing non-holding of invoices on the back-end due to the error shown in the video. So I think that means this is only a front-end issue where we need to not show the button. @neonbhai's solution sounds simplest to me, but it's possible I'm missing some nuance about how invoices work.

@rafecolton hi, production does not show Hold option to either the workspace admin, send or receiver of an invoice. We should be fine to hide it from the Report Details Page

[cdOut]

I’m already working on a PR to clean up hold/unhold logic, I could handle this case there unless there are any objections against it.

[melvin-bot]

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

[cdOut]

Will be sent for a re-review today and should be merged in a few days max.

[stephanieelliott]

Just linking the PR for easy ref: #45151

[melvin-bot]

@Beamanator, @stephanieelliott, @cdOut, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[cdOut]

PR is currently being re-reviewed.

[ishpaul777]

Thanks for update 🙏

[ishpaul777]

PR is in works #45151

[melvin-bot]

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

[ishpaul777]

#45151 WIP

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[ishpaul777]

PR is merged and on staging

[stephanieelliott]

Ok seems this was deployed to prod on 8/12 per this comment: #45151 (comment)

Updating for 7 day hold with payment pending labels.

[ishpaul777]

No payment required here i was not involved in PR

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @cdOut - no payment due (agenty)
• Contributor+: @ishpaul777 - no payment/no review required

Upwork job is here: https://www.upwork.com/jobs/~0178cd3e6d26424c8a