[C+ Checklist Needs Completion] [$500] IOU - Approve button appears while approvals is disabled

[izarutskaya]

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.57-2
Reproducible in staging?: Y
Reproducible in production?: Y
Found when executing PR : #38253
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Pre-requisites:
Have a collect WS and make chat enabled on ND
Have 2 accounts (one admin, one employee)
Having `Make or track payments enabled
Toggle on delay submissions on ND (Logged in as the admin)
Toggle off Add approvals
On OD Make reimbursement direct with a VBBA

1. [Employee] Request money from the workspace chat and submit it
2. [Admin] Click on the IOU preview and observe the header

Expected Result:

"Pay with expensify" button should show up since this is the case where Delay submissions is enabled and approvals is disabled

Actual Result:

"Approve" button appears on the header

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

Bug6429854_1711614962501.Screen_Recording_20240328_112744_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014fc34bb4b2bfbb10
• Upwork Job ID: 1773511332097155072
• Last Price Increase: 2024-03-29
• Automatic offers:
• DylanDylann | Reviewer | 103218707

Issue Owner

Current Issue Owner: @greg-schroeder

[melvin-bot]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[izarutskaya]

We think this issue might be related to the #collect project.

[allgandalf]

Proposal

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

Approve button appears even when approvals are not required

What is the root cause of that problem?

In canApproveIOU we do not check if the approvalMode and autoReportingFrequency are optional:

App/src/libs/actions/IOU.ts

Line 4500 in 4fbd218

function canApproveIOU(iouReport: OnyxEntry<OnyxTypes.Report> | EmptyObject, chatReport: OnyxEntry<OnyxTypes.Report> | EmptyObject, policy: OnyxEntry<OnyxTypes.Policy> | EmptyObject) {

App/src/libs/actions/IOU.ts

Lines 4523 to 4524 in 4fbd218

	return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled;
	}

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

Add a check in canApproveIOU to check if the approval mode and delay submittion are Optional and if it is then return the value:

diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts
index b67e44fcf5..c361ed3e5e 100644
--- a/src/libs/actions/IOU.ts
+++ b/src/libs/actions/IOU.ts
@@ -4519,8 +4519,10 @@ function canApproveIOU(iouReport: OnyxEntry<OnyxTypes.Report> | EmptyObject, cha
     const isOpenExpenseReport = isPolicyExpenseChat && ReportUtils.isOpenExpenseReport(iouReport);
     const isApproved = ReportUtils.isReportApproved(iouReport);
     const iouSettled = ReportUtils.isSettled(iouReport?.reportID);
+    const isApprovalOptional = policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.OPTIONAL;
+    const canDelaySubmissions = policy?.autoReportingFrequency !== CONST.POLICY.AUTO_REPORTING_FREQUENCY.IMMEDIATE &&  policy?.autoReportingFrequency !== CONST.POLICY.AUTO_REPORTING_FREQUENCY.INSTANT;
 
-    return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled;
+    return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !(isApprovalOptional && canDelaySubmissions);
 }

What alternative solutions did you explore? (Optional)

N/A

[bernhardoj]

Proposal

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

The approve button is still displayed even though the add approvals option is disabled.

What is the root cause of that problem?

We already have a condition here to not show the approve button if the approval is disabled (isOnSubmitAndClosePolicy).

App/src/libs/actions/IOU.ts

Lines 4509 to 4513 in 4fbd218

	const isOnInstantSubmitPolicy = PolicyUtils.isInstantSubmitEnabled(policy);
	const isOnSubmitAndClosePolicy = PolicyUtils.isSubmitAndClose(policy);
	if (isOnInstantSubmitPolicy && isOnSubmitAndClosePolicy) {
	return false;
	}

However, the auto-reporting frequency needs to be instant (isOnInstantSubmitPolicy) to not show the approval button. In our case, the frequency is set to WEEKLY, so the condition is never met.

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

Change the condition to accept either isOnInstantSubmitPolicy or isOnSubmitAndClosePolicy.

if (isOnInstantSubmitPolicy || isOnSubmitAndClosePolicy) {

Or we can remove isOnInstantSubmitPolicy

[melvin-bot]

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

[melvin-bot]

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

[DylanDylann]

@bernhardoj's proposal looks good to me

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[stitesExpensify]

@bernhardoj do you know why we initially implemented it to only be disabled when isOnInstantSubmitPolicy? That seems like it was done intentionally, and I just want to make sure we aren't going to break something

[bernhardoj]

It was first implemented in #35470, but looks like in this PR #38253 (where the QA found the issue), the test step expect the approve button to not show.

cc: @luacmartins @lakchote

[luacmartins]

We should not show the approve button if approvals are disabled

[DylanDylann]

@luacmartins As mentioned here

Approver shouldn't be able to "Approve" an expense report IFF the report's policy is a Collect Policy, on "Instant Submit" reporting frequency, with "Submit & Close" approval mode.

Do we want to display the Approve button if users are in approval mode and not in "Instant Submit" reporting frequency

cc @Beamanator

[Beamanator]

Approver shouldn't be able to "Approve" an expense report IFF the report's policy is a Collect Policy, on "Instant Submit" reporting frequency, with "Submit & Close" approval mode.

Agreed (I wrote that lol)

Do we want to display the Approve button if users are in approval mode and not in "Instant Submit" reporting frequency

To be clear, you're asking "if users are in approval mode Submit & Close", right? I believe in that case we wouldn't want to show the "Approve" button either

Because "Submit & Close" basically means: After a submitter submits a report, what next? We close the report, we don't wait for the admin to Approve it

[bernhardoj]

Because "Submit & Close" basically means: After a submitter submits a report, what next? We close the report, we don't wait for the admin to Approve it

That makes sense, but currently, the condition expects the instant submit is on too to hide the approve button.

App/src/libs/actions/IOU.ts

Lines 4599 to 4601 in ebc8de8

	if (isOnInstantSubmitPolicy && isOnSubmitAndClosePolicy) {
	return false;
	}

I think it makes sense if we remove the instant submit condition because whether it's an instant or manual submit, the report will be closed immediately, right?

[Beamanator]

I tentatively think that definitely makes sense 😅 👍

[greg-schroeder]

Discussion continues here - @stitesExpensify @Beamanator we cool with @bernhardoj's approach to solve this one?

[melvin-bot]

📣 @DylanDylann 🎉 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

[bernhardoj]

PR is ready

cc: @DylanDylann

[greg-schroeder]

PR is being reviewed

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

• Don't show approve button when approval is disabled #46044

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

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

• @bernhardoj requires payment through NewDot Manual Requests
• @DylanDylann requires payment automatic offer (Reviewer)

[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:

• [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
• [@DylanDylann] 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:
• [@DylanDylann] 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:
• [@DylanDylann] Determine if we should create a regression test for this bug.
• [@DylanDylann] 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.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[greg-schroeder]

Payment summary:

Contributor: @bernhardoj - $500 - You can make a manual request
C+: @DylanDylann - $500 - Paying through Upwork

[greg-schroeder]

Please complete the checklist @DylanDylann when you're able!

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$500 approved for @bernhardoj

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: #35470
[@DylanDylann] 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: The author explained here
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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.

Regression Test Proposal

Prerequisite: Admin has a collect workspace and turns off the approval and sets the Delayed submission to MANUAL (or other than INSTANT)

1. As an employee, add a money request and submit it to the workspace expense chat
2. As the admin, open the workspace chat (refresh or reopen the app if needed to get the latest data)
3. Verify the pay button shows instead of the approve button on the report preview

Do we agree 👍 or 👎

[greg-schroeder]

Filed regression test, closing