[Awaiting checklist completion] Approver - The expense approver can be deleted, a warning doesn't appear.

[mitarachim]

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.3.13-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from BrowserStack: #79891
Email or phone of affected tester (no customers): n/a
Issue reported by: Applause Internal Team
Bug source: Pull Request QA execution
Device used: Mac 15.4.1 Safari
App Component: Workspace Settings

Action Performed:

1. Go to https://staging.new.expensify.com
2. Create a workspace
   3.Add another member to a workspace.
   4.Create a new report with an expense and submit it.
   5.In that report select 'change the approver' and add the new member as an approver.
   6.Go to workspace members and try to remove that member.

Expected Result:

Approver can't be deleted, a warning message appears under the approvers field,
The red dot appears near the Workspace and Members tabs

Actual Result:

Approver is deleted, the warning doesn't appear, but there is a red dot near the Workspace, and the Members tab

Workaround:

Unknown

Platforms:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari

Screenshots/Videos

Bug7068766_1770326562230.Recording__1505.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @dukenv0307

[melvin-bot]

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

[melvin-bot]

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

[daledah]

🚨 Edited by proposal-police: This proposal was edited at 2026-02-06 11:26:40 UTC.

Proposal

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

Approver is deleted, the warning doesn't appear, but there is a red dot near the Workspace, and the Members tab

What is the root cause of that problem?

The isApprover function in src/libs/actions/Policy/Member.ts only checks policy-level approvers (policy.approver, employee.submitsTo, employee.forwardsTo, employee.overLimitForwardsTo), but it does NOT check report-level approvers. When a user uses "Change the approver" on a submitted report, the addReportApprover function sets report.managerID to the new approver's account ID. This is a report-level assignment that is not reflected in the policy's employeeList fields. As a result, when trying to remove that member, isApprover returns false, no warning is shown, and the member is optimistically removed. The backend then detects the conflict and returns an error (red dot), but the frontend has already processed the removal.

App/src/libs/actions/Policy/Member.ts

Lines 78 to 85 in dedd11d

	function isApprover(policy: OnyxEntry<Policy>, employeeLogin: string) {
	if (policy?.approver === employeeLogin) {
	return true;
	}
	return Object.values(policy?.employeeList ?? {}).some(
	(employee) => employee?.submitsTo === employeeLogin || employee?.forwardsTo === employeeLogin || employee?.overLimitForwardsTo === employeeLogin,
	);
	}

App/src/libs/actions/IOU/index.ts

Lines 13401 to 13460 in dedd11d

	function addReportApprover(
	report: OnyxTypes.Report,
	newApproverEmail: string,
	newApproverAccountID: number,
	accountID: number,
	email: string,
	policy: OnyxEntry<OnyxTypes.Policy>,
	hasViolations: boolean,
	isASAPSubmitBetaEnabled: boolean,
	reportCurrentNextStepDeprecated: OnyxEntry<OnyxTypes.ReportNextStepDeprecated>,
	) {
	const takeControlReportAction = buildOptimisticChangeApproverReportAction(newApproverAccountID, accountID);
	// buildOptimisticNextStep is used in parallel
	// eslint-disable-next-line @typescript-eslint/no-deprecated
	const optimisticNextStepDeprecated = buildNextStepNew({
	report: {...report, managerID: newApproverAccountID},
	predictedNextStatus: report.statusNum ?? CONST.REPORT.STATUS_NUM.SUBMITTED,
	shouldFixViolations: false,
	isUnapprove: true,
	policy,
	currentUserAccountIDParam: accountID,
	currentUserEmailParam: email,
	hasViolations,
	isASAPSubmitBetaEnabled,
	bypassNextApproverID: newApproverAccountID,
	});
	const optimisticNextStep = buildOptimisticNextStep({
	report: {...report, managerID: newApproverAccountID},
	predictedNextStatus: report.statusNum ?? CONST.REPORT.STATUS_NUM.SUBMITTED,
	shouldFixViolations: false,
	isUnapprove: true,
	policy,
	currentUserAccountIDParam: accountID,
	currentUserEmailParam: email,
	hasViolations,
	isASAPSubmitBetaEnabled,
	bypassNextApproverID: newApproverAccountID,
	});
	const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.REPORT | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.NEXT_STEP> = {
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`,
	value: {
	managerID: newApproverAccountID,
	nextStep: optimisticNextStep,
	pendingFields: {
	nextStep: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	},
	},
	},
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
	value: {
	[takeControlReportAction.reportActionID]: takeControlReportAction,
	},
	},
	{

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

1. For the the warning doesn't appear bug we should extend the isApprover function to also check if the member is set as managerID on any submitted expense report in the workspace.

 function isApprover(policy: OnyxEntry<Policy>, employeeLogin: string) {
     if (policy?.approver === employeeLogin) {
         return true;
     }
-    return Object.values(policy?.employeeList ?? {}).some(
+    if (Object.values(policy?.employeeList ?? {}).some(
         (employee) => employee?.submitsTo === employeeLogin || employee?.forwardsTo === employeeLogin || employee?.overLimitForwardsTo === employeeLogin,
-    );
+    )) {
+        return true;
+    }
+
+    // Check if the member is a report-level approver (set via "Change the approver")
+    const memberAccountID = PersonalDetailsUtils.getAccountIDsByLogins([employeeLogin]).at(0);
+    if (memberAccountID) {
+        const workspaceReports = ReportUtils.getAllWorkspaceReports(policy?.id);
+        if (workspaceReports.some(
+            (report) => report?.managerID === memberAccountID && report?.statusNum === CONST.REPORT.STATUS_NUM.SUBMITTED,
+        )) {
+            return true;
+        }
+    }
+
+    return false;
 }

What alternative solutions did you explore? (Optional)

N/A

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.

[twinkle-react]

🚨 Edited by proposal-police: This proposal was edited at 2026-02-06 10:14:37 UTC.

Proposal

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

Approver users, either workspace-level approvers or report-level managers, can be removed even when the UI does not reliably show the approver warning, so the DELETE API is called and approval workflows become inconsistent.

What is the root cause of that problem?

In this implementation, the earliest incorrect assumption is that caller components will always detect approvers and block deletion before calling the shared deletion action. In src/libs/actions/Policy/Member.ts, the removeMembers function performs deletions unconditionally and does not check for report-level manager assignments, so it can issue the DELETE even when a selected member is an approver.

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

State the single invariant you’re enforcing
"The shared policy member deletion action will never call the delete API when any selected member is an approver (workspace-level or report-level); instead it will set a per-login Onyx error on those approvers."

Point to the earliest incorrect assumption
Earliest incorrect assumption: callers will detect approvers and block deletion prior to calling removeMembers.
Evidence: removeMembers constructs optimistic, success, and failure Onyx payloads and calls API.write(WRITE_COMMANDS.DELETE_MEMBERS_FROM_WORKSPACE, ...) unconditionally as the first client-side mutation.

Apply the smallest shared-layer fix
Where: src/libs/actions/Policy/Member.ts, function removeMembers(...).
What: Insert a short guard at the top of removeMembers that uses the function policyID if present, builds a deduplicated set of approvers, detects report-level managers on submitted reports, and blocks deletion by merging per-login errors into Onyx instead of calling the API.

Why this layer
removeMembers is the single shared owner of deletion. Centralizing the check removes duplication and race windows across callers.

The exact code and scope
Files changed: src/libs/actions/Policy/Member.ts
Function modified: removeMembers(policy: OnyxEntry, selectedMemberEmails: string[], policyMemberEmailsToAccountIDs: Record<string, number>)
Scope size: One file, single guard insertion of about 25 to 40 lines, no new modules and no signature changes.

Exact code to insert, immediately after the function early-return and before any optimistic or API.write logic:

// File: src/libs/actions/Policy/Member.ts
// --- START: Block approver deletions at shared action layer ---
// Mixed selections (approver + non-approver) intentionally block the entire deletion;
// callers must update workflows/approvers and retry.
const resolvedPolicyID =
    typeof policyID !== 'undefined' ? policyID : (policy?.id ?? policy?.policyID);

// Build deduped set of workspace approvers
const approverLoginsSet = new Set(selectedMemberEmails.filter((login) => isApprover(policy, login)));

// Short-circuit: if not all selected are already workspace approvers and we have a policyID, check report-level managers
if (resolvedPolicyID && approverLoginsSet.size < selectedMemberEmails.length) {
    const workspaceReports = ReportUtils.getAllWorkspaceReports(resolvedPolicyID) || [];
    for (const login of selectedMemberEmails) {
        if (approverLoginsSet.has(login)) {
            continue;
        }
        const accountID = policyMemberEmailsToAccountIDs[login];
        if (!accountID) {
            // Gracefully skip unmapped logins
            continue;
        }
        const isReportApprover = workspaceReports.some(
            (report) =>
                Number(report?.managerID) === Number(accountID) &&
                Number(report?.statusNum) === Number(CONST.REPORT.STATUS_NUM.SUBMITTED),
        );
        if (isReportApprover) {
            approverLoginsSet.add(login);
        }
    }
}

if (approverLoginsSet.size > 0) {
    // Ensure we have a valid policyID before merging to Onyx (avoid POLICYundefined)
    if (!resolvedPolicyID) {
        return;
    }
    const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${resolvedPolicyID}` as const;
    const employeeErrors: Record<string, {errors: Record<string, string> | null}> = {};
    for (const login of Array.from(approverLoginsSet)) {
        employeeErrors[login] = {
            errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.people.error.genericRemove'),
        };
    }
    // Merge only per-login errors to avoid clobbering other employeeList entries
    Onyx.merge(policyKey, {
        employeeList: employeeErrors,
    });
    // Block deletion — callers must update approval workflows / approvers and retry.
    return;
}
// --- END: Block approver deletions at shared action layer ---

What alternative solutions did you explore? (Optional)

Caller-side blocking and backend enforcement were considered, but both are weaker or higher scope compared to a single shared-layer guard.

[QichenZhu]

This wasn't an issue earlier this week. See the screenshots I took then vs. yesterday. The offending commit is likely within the regression period.

[twilight2294]

Proposal

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

When a workspace member is assigned as an approver on a specific expense report via the "Change Approver" feature, attempting to remove them from the workspace does not show the expected warning dialog. The removal proceeds and then fails with a backend error (red dot), even though the member should be protected from removal.

What is the root cause of that problem?

The root cause is that the isApprover() function in src/libs/actions/Policy/Member.ts only checks for policy-level approver assignments and does not account for report-level approvers created through the "Change Approver" feature.

The isApprover() function currently checks three policy-level fields:
https://github.com/Expensify/App/blob/main/src/libs/actions/Policy/Member.ts#L78-L85

It checks policy.approver (the default workspace approver) and the submitsTo, forwardsTo, and overLimitForwardsTo fields in the policy's employeeList. These fields represent workspace-level approval workflows.

However, when a user uses the "Change Approver" feature on a submitted report (via ReportChangeApproverPage → ReportAddApproverPage), it calls the addReportApprover() function which only updates the managerID field on that specific report:
https://github.com/Expensify/App/blob/main/src/libs/actions/IOU/index.ts#L13401

This creates a report-level approver relationship that exists only in report.managerID and is NOT reflected in the policy's employeeList fields. Because isApprover() doesn't check for report-level approvers, it returns false for these members.

The isApprover() function is called before attempting member removal in both the bulk removal flow and individual member removal flow:

• Bulk removal: https://github.com/Expensify/App/blob/main/src/pages/workspace/WorkspaceMembersPage.tsx#L165 and https://github.com/Expensify/App/blob/main/src/pages/workspace/WorkspaceMembersPage.tsx#L231
• Individual removal: https://github.com/Expensify/App/blob/main/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx#L126 and https://github.com/Expensify/App/blob/main/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx#L185

Since isApprover() returns false, the warning dialog is not shown, the removal proceeds to the API, and the backend correctly rejects it (because the backend knows this member is an active approver), resulting in a red dot error.

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

We should extend the isApprover() function in src/libs/actions/Policy/Member.ts to also check if the employee is a managerID on any active expense report in the workspace:

https://github.com/Expensify/App/blob/main/src/libs/actions/Policy/Member.ts#L78-L85

import * as ReportUtils from '@libs/ReportUtils';
import {getAccountIDsByLogins} from '@libs/PersonalDetailsUtils';

function isApprover(policy: OnyxEntry<Policy>, employeeLogin: string) {
    // Check 1: Policy-level default approver
    if (policy?.approver === employeeLogin) {
        return true;
    }
    
    // Check 2: Policy-level workflow approver (submitsTo, forwardsTo, overLimitForwardsTo)
    const isPolicyWorkflowApprover = Object.values(policy?.employeeList ?? {}).some(
        (employee) => employee?.submitsTo === employeeLogin || employee?.forwardsTo === employeeLogin || employee?.overLimitForwardsTo === employeeLogin,
    );
    if (isPolicyWorkflowApprover) {
        return true;
    }
    
    // Check 3: Report-level approver (managerID on active expense reports)
    const employeeAccountID = getAccountIDsByLogins([employeeLogin]).at(0);
    if (employeeAccountID && policy?.id) {
        const workspaceReports = ReportUtils.getAllWorkspaceReports(policy.id);
        return workspaceReports.some(
            (report) =>
                ReportUtils.isExpenseReport(report) &&
                !ReportUtils.isSettled(report) &&
                !ReportUtils.isClosedReport(report) &&
                report?.managerID === employeeAccountID,
        );
    }
    
    return false;
}

This ensures that members who are report-level approvers (via report.managerID) are properly detected and protected from removal, triggering the appropriate warning dialog.

What alternative solutions did you explore?

Instead of modifying isApprover(), we could add a separate isReportLevelApprover() check at each call site in WorkspaceMembersPage.tsx and WorkspaceMemberDetailsPage.tsx. However, this approach would require changes in multiple locations and could miss future call sites. Fixing isApprover() centrally ensures all existing and future uses of this function benefit from the complete approver check.