[HOLD for payment 2025-02-05] [$250] Rules - Disabling rules doesn't disable the prevent self approvals feature

[vincdargento]

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.76-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): biruknew45+22kdj@gmail.com
Issue reported by: Applause Internal Team
Device used: Windows Windows 11 - Chrome, Samsung Galaxy A13 - Android 14 - Chrome
App Component: Workspace Settings

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Create workspace and enable workflows and rules
3. Go to workflows and enable add approvals
4. Go to rules and enable prevent self approvals
5. Go to more features and disable rules
6. Go to workspace chat and submit an expense

Expected Result:

After disabling rules the prevent self approvals feature should be disabled and the user should be able to approve their expense.

Actual Result:

Disabling rules doesn't disable the prevent self approvals feature

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021878944581059149612
• Upwork Job ID: 1878944581059149612
• Last Price Increase: 2025-01-13
• Automatic offers:
• mkzie2 | Contributor | 105735782

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

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

[Tony-MK]

Edited by proposal-police: This proposal was edited at 2024-12-16 15:40:35 UTC.

Proposal

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

Disabling rules doesn't disable the prevent self approvals feature

What is the root cause of that problem?

When disabling the rules feature, the policy?.preventSelfApproval is still present and set to true since the enablePolicyRules function doesn't clear `policy?.preventSelfApproval.

App/src/components/MoneyReportHeader.tsx

Line 141 in b8579b1

const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(moneyRequestReport);

The policy?.preventSelfApproval is used in the
ReportUtils.isAllowedToApproveExpenseReport function to check if the approval should be disabled in MoneyReportHeader and ReportPreview.

App/src/libs/ReportUtils.ts

Lines 8030 to 8033 in b8579b1

	function isAllowedToApproveExpenseReport(report: OnyxEntry<Report>, approverAccountID?: number, reportPolicy?: OnyxEntry<Policy> | SearchPolicy): boolean {
	const policy = reportPolicy ?? getPolicy(report?.policyID);
	const isOwner = (approverAccountID ?? currentUserAccountID) === report?.ownerAccountID;
	return !(policy?.preventSelfApproval && isOwner);

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

In the enablePolicyRules function, clear policy?.preventSelfApproval for the FE fix.

preventSelfApproval: null, // For optimisticData
....
preventSelfApproval: policy?.preventSelfApproval, // For FailureData

A similar change on the backend is also required.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

Add tests to for ReportUtils.isAllowedToApproveExpenseReport function with different policies and reports which have randomized values for areRulesEnabled, preventSelfApproval and report?.ownerAccountID.

What alternative solutions did you explore? (Optional)

Improve the ReportUtils.isAllowedToApproveExpenseReport function by returning false if the policy rules are disabled.

App/src/libs/ReportUtils.ts

Line 8033 in b8579b1

return !(policy?.preventSelfApproval && isOwner);

return !(policy?.areRulesEnabled && policy?.preventSelfApproval && isOwner);

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-12-16 15:32:07 UTC.

Proposal

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

Disabling rules doesn't disable the prevent self approvals feature

What is the root cause of that problem?

We always return false if policy?.preventSelfApproval is true and the approver is the owner of the policy expense chat

App/src/libs/ReportUtils.ts

Lines 8030 to 8035 in b8579b1

	function isAllowedToApproveExpenseReport(report: OnyxEntry<Report>, approverAccountID?: number, reportPolicy?: OnyxEntry<Policy> | SearchPolicy): boolean {
	const policy = reportPolicy ?? getPolicy(report?.policyID);
	const isOwner = (approverAccountID ?? currentUserAccountID) === report?.ownerAccountID;
	return !(policy?.preventSelfApproval && isOwner);
	}

Then the approve button is disabled here

App/src/components/ReportActionItem/ReportPreview.tsx

Line 144 in b8579b1

const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(iouReport);

and here

App/src/components/MoneyReportHeader.tsx

Line 141 in b8579b1

const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(moneyRequestReport);

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

We should return early true if the rules feature is disabled.

if (!policy?.areRulesEnabled) {
    return true;
}

App/src/libs/ReportUtils.ts

Lines 8030 to 8035 in b8579b1

	function isAllowedToApproveExpenseReport(report: OnyxEntry<Report>, approverAccountID?: number, reportPolicy?: OnyxEntry<Policy> | SearchPolicy): boolean {
	const policy = reportPolicy ?? getPolicy(report?.policyID);
	const isOwner = (approverAccountID ?? currentUserAccountID) === report?.ownerAccountID;
	return !(policy?.preventSelfApproval && isOwner);
	}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We should add a test for isAllowedToApproveExpenseReport function to ensure that it returns true when the rules is disabled even policy?.preventSelfApproval is true and the approver is the owner of the report

What alternative solutions did you explore? (Optional)

When we disable the rules feature, we should disable all expense's rules in optimistic data and backend also need to handle that

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

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

[melvin-bot]

@JmillsExpensify Still overdue 6 days?! Let's take care of this!

[melvin-bot]

@JmillsExpensify Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot]

@JmillsExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@JmillsExpensify 12 days overdue. Walking. Toward. The. Light...

[melvin-bot]

This issue has not been updated in over 14 days. @JmillsExpensify eroding to Weekly issue.