From 0f4ce775a5aa0fbcf7351fbb9d4eb9c2d571be03 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Wed, 29 Oct 2025 00:01:01 +0700 Subject: [PATCH 1/7] Self approval stays enabled if all members are removed by multi selection --- src/libs/actions/Policy/Member.ts | 102 +++++++++++++----- src/libs/actions/Policy/Policy.ts | 29 +++-- src/libs/actions/Workflow.ts | 53 ++++++++- src/pages/workspace/WorkspaceMembersPage.tsx | 34 +----- .../members/WorkspaceMemberDetailsPage.tsx | 11 +- .../rules/ExpenseReportRulesSection.tsx | 10 +- 6 files changed, 155 insertions(+), 84 deletions(-) diff --git a/src/libs/actions/Policy/Member.ts b/src/libs/actions/Policy/Member.ts index ab9a40eccf46..23e86760eb08 100644 --- a/src/libs/actions/Policy/Member.ts +++ b/src/libs/actions/Policy/Member.ts @@ -26,12 +26,14 @@ import * as PhoneNumber from '@libs/PhoneNumber'; import {getDefaultApprover, isUserPolicyAdmin} from '@libs/PolicyUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; +import {updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; import * as FormActions from '@userActions/FormActions'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type { ImportedSpreadsheetMemberData, InvitedEmailsToAccountIDs, + PersonalDetails, PersonalDetailsList, Policy, PolicyEmployee, @@ -40,13 +42,15 @@ import type { ReportAction, ReportActions, } from '@src/types/onyx'; +import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; import type {PendingAction} from '@src/types/onyx/OnyxCommon'; import type {JoinWorkspaceResolution} from '@src/types/onyx/OriginalMessage'; import type {ApprovalRule} from '@src/types/onyx/Policy'; import type {NotificationPreference, Participant} from '@src/types/onyx/Report'; import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import {createPolicyExpenseChats} from './Policy'; +import {getRemoveApprocalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '../Workflow'; +import {createPolicyExpenseChats, getSetPolicyPreventSelfApprovalOnyxData} from './Policy'; type OnyxDataReturnType = { optimisticData: OnyxUpdate[]; @@ -434,7 +438,7 @@ function resetAccountingPreferredExporter(policyID: string, loginList: string[]) * Remove the passed members from the policy employeeList * Please see https://github.com/Expensify/App/blob/main/README.md#Security for more details */ -function removeMembers(accountIDs: number[], policyID: string) { +function removeMembers(accountIDs: number[], policyID: string, approvalWorkflows: ApprovalWorkflow[]) { // In case user selects only themselves (admin), their email will be filtered out and the members // array passed will be empty, prevent the function from proceeding in that case as there is no one to remove if (accountIDs.length === 0) { @@ -446,6 +450,44 @@ function removeMembers(accountIDs: number[], policyID: string) { // eslint-disable-next-line @typescript-eslint/no-deprecated const policy = getPolicy(policyID); + const optimisticData: OnyxUpdate[] = []; + const successData: OnyxUpdate[] = []; + const failureData: OnyxUpdate[] = []; + + // Update approval workflows after member removal + // Check if any of the account IDs are approvers + const hasApprovers = accountIDs.some((accountID) => isApproverTemp(policy, accountID)); + const ownerDetails = allPersonalDetails?.[policy?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID] ?? ({} as PersonalDetails); + + if (hasApprovers) { + const ownerEmail = ownerDetails.login; + accountIDs.forEach((accountID) => { + const removedApprover = allPersonalDetails?.[accountID]; + if (!removedApprover?.login || !ownerEmail) { + return; + } + const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ + approvalWorkflows, + removedApprover, + ownerDetails, + }); + updatedWorkflows.forEach((workflow) => { + if (workflow?.removeApprovalWorkflow) { + const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; + const onyxDataForRemoveApprovalWorkflow = getRemoveApprocalWorkflowOnyxData(updatedWorkflow, policy); + optimisticData.push(...(onyxDataForRemoveApprovalWorkflow.optimisticData ?? [])); + successData.push(...(onyxDataForRemoveApprovalWorkflow.successData ?? [])); + failureData.push(...(onyxDataForRemoveApprovalWorkflow.failureData ?? [])); + } else { + const onyxDataForUpdateApprovalWorkflow = getUpdateApprovalWorkflowOnyxData(workflow, [], [], policy); + optimisticData.push(...(onyxDataForUpdateApprovalWorkflow.optimisticData ?? [])); + successData.push(...(onyxDataForUpdateApprovalWorkflow.successData ?? [])); + failureData.push(...(onyxDataForUpdateApprovalWorkflow.failureData ?? [])); + } + }); + }); + } + const workspaceChats = ReportUtils.getWorkspaceChats(policyID, accountIDs); const emailList = accountIDs.map((accountID) => allPersonalDetails?.[accountID]?.login).filter((login) => !!login) as string[]; const optimisticClosedReportActions = workspaceChats.map(() => @@ -513,38 +555,32 @@ function removeMembers(accountIDs: number[], policyID: string) { const approvalRules: ApprovalRule[] = policy?.rules?.approvalRules ?? []; const optimisticApprovalRules = approvalRules.filter((rule) => !emailList.includes(rule?.approver ?? '')); - const optimisticData: OnyxUpdate[] = [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - employeeList: optimisticMembersState, - approver: emailList.includes(policy?.approver ?? '') ? policy?.owner : policy?.approver, - rules: { - ...(policy?.rules ?? {}), - approvalRules: optimisticApprovalRules, - }, + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: policyKey, + value: { + employeeList: optimisticMembersState, + approver: emailList.includes(policy?.approver ?? '') ? policy?.owner : policy?.approver, + rules: { + ...(policy?.rules ?? {}), + approvalRules: optimisticApprovalRules, }, }, - ]; + }); optimisticData.push(...announceRoomMembers.optimisticData, ...adminRoomMembers.optimisticData, ...preferredExporterOnyxData.optimisticData); - const successData: OnyxUpdate[] = [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: {employeeList: successMembersState}, - }, - ]; + successData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: policyKey, + value: {employeeList: successMembersState}, + }); successData.push(...announceRoomMembers.successData, ...adminRoomMembers.successData, ...preferredExporterOnyxData.successData); - const failureData: OnyxUpdate[] = [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: {employeeList: failureMembersState, approver: policy?.approver, rules: policy?.rules}, - }, - ]; + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: policyKey, + value: {employeeList: failureMembersState, approver: policy?.approver, rules: policy?.rules}, + }); failureData.push(...announceRoomMembers.failureData, ...adminRoomMembers.failureData, ...preferredExporterOnyxData.failureData); const pendingChatMembers = ReportUtils.getPendingChatMembers(accountIDs, [], CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); @@ -679,6 +715,16 @@ function removeMembers(accountIDs: number[], policyID: string) { policyID, }; + // Update "Prevent Self Approvals" after member removal + const previousEmployeesCount = Object.values(policy?.employeeList ?? {}).filter((employee) => employee.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; + const remainingEmployeeCount = previousEmployeesCount - accountIDs.length; + if (remainingEmployeeCount === 1 && policy?.preventSelfApproval) { + const onyxDataForSetPolicyPreventSelfApproval = getSetPolicyPreventSelfApprovalOnyxData(policyID, false); + optimisticData.push(...(onyxDataForSetPolicyPreventSelfApproval.optimisticData ?? [])); + successData.push(...(onyxDataForSetPolicyPreventSelfApproval.successData ?? [])); + failureData.push(...(onyxDataForSetPolicyPreventSelfApproval.failureData ?? [])); + } + API.write(WRITE_COMMANDS.DELETE_MEMBERS_FROM_WORKSPACE, params, {optimisticData, successData, failureData}); } diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 85d74289991f..85b142e54acd 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -5650,18 +5650,13 @@ function setPolicyPreventMemberCreatedTitle(policyID: string, enforced: boolean) }); } -/** - * Call the API to enable or disable self approvals for the reports - * @param policyID - id of the policy to apply the naming pattern to - * @param preventSelfApproval - flag whether to prevent workspace members from approving their own expense reports - */ -function setPolicyPreventSelfApproval(policyID: string, preventSelfApproval: boolean) { +function getSetPolicyPreventSelfApprovalOnyxData(policyID: string, preventSelfApproval: boolean): OnyxData { // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 // eslint-disable-next-line @typescript-eslint/no-deprecated const policy = getPolicy(policyID); if (preventSelfApproval === policy?.preventSelfApproval) { - return; + return {}; } const optimisticData: OnyxUpdate[] = [ @@ -5706,6 +5701,25 @@ function setPolicyPreventSelfApproval(policyID: string, preventSelfApproval: boo }, ]; + return {optimisticData, failureData, successData}; +} + +/** + * Call the API to enable or disable self approvals for the reports + * @param policyID - id of the policy to apply the naming pattern to + * @param preventSelfApproval - flag whether to prevent workspace members from approving their own expense reports + */ +function setPolicyPreventSelfApproval(policyID: string, preventSelfApproval: boolean) { + // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 + // eslint-disable-next-line @typescript-eslint/no-deprecated + const policy = getPolicy(policyID); + + if (preventSelfApproval === policy?.preventSelfApproval) { + return; + } + + const {optimisticData, failureData, successData} = getSetPolicyPreventSelfApprovalOnyxData(policyID, preventSelfApproval); + const parameters: SetPolicyPreventSelfApprovalParams = { preventSelfApproval, policyID, @@ -6505,4 +6519,5 @@ export { clearPolicyTitleFieldError, inviteWorkspaceEmployeesToUber, setWorkspaceConfirmationCurrency, + getSetPolicyPreventSelfApprovalOnyxData, }; diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 2dc8c4d562e2..b2b5d6536caa 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -13,6 +13,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {ApprovalWorkflowOnyx, PersonalDetailsList, Policy} from '@src/types/onyx'; import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; +import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; let personalDetailsByEmail: PersonalDetailsList = {}; @@ -90,9 +91,9 @@ function createApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: Onyx API.write(WRITE_COMMANDS.CREATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } -function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRemove: Member[], approversToRemove: Approver[], policy: OnyxEntry) { +function getUpdateApprovalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, membersToRemove: Member[], approversToRemove: Approver[], policy: OnyxEntry): OnyxData { if (!policy) { - return; + return {}; } const previousDefaultApprover = getDefaultApprover(policy); @@ -109,7 +110,7 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem // If there are no changes to the employees list, we can exit early if (isEmptyObject(updatedEmployees) && !newDefaultApprover) { - return; + return {}; } const optimisticData: OnyxUpdate[] = [ @@ -150,6 +151,33 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem }, ]; + return {optimisticData, failureData, successData}; +} + +function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRemove: Member[], approversToRemove: Approver[], policy: OnyxEntry) { + if (!policy) { + return; + } + + const previousDefaultApprover = getDefaultApprover(policy); + const newDefaultApprover = approvalWorkflow.isDefault ? approvalWorkflow.approvers.at(0)?.email : undefined; + const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); + const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({ + previousEmployeeList, + approvalWorkflow, + type: CONST.APPROVAL_WORKFLOW.TYPE.UPDATE, + membersToRemove, + approversToRemove, + defaultApprover: newDefaultApprover ?? previousDefaultApprover ?? '', + }); + + // If there are no changes to the employees list, we can exit early + if (isEmptyObject(updatedEmployees) && !newDefaultApprover) { + return; + } + + const {optimisticData, failureData, successData} = getUpdateApprovalWorkflowOnyxData(approvalWorkflow, membersToRemove, approversToRemove, policy); + const parameters: UpdateWorkspaceApprovalParams = { policyID: policy.id, employees: JSON.stringify(Object.values(updatedEmployees)), @@ -158,9 +186,9 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } -function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry) { +function getRemoveApprocalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry): OnyxData { if (!policy) { - return; + return {}; } const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); @@ -208,6 +236,19 @@ function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: Onyx }, ]; + return {optimisticData, failureData, successData}; +} + +function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry) { + if (!policy) { + return; + } + + const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); + const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({previousEmployeeList, approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.REMOVE}); + + const {optimisticData, failureData, successData} = getRemoveApprocalWorkflowOnyxData(approvalWorkflow, policy); + const parameters: RemoveWorkspaceApprovalParams = {policyID: policy.id, employees: JSON.stringify(Object.values(updatedEmployees))}; API.write(WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } @@ -330,4 +371,6 @@ export { clearApprovalWorkflowApprovers, clearApprovalWorkflow, validateApprovalWorkflow, + getRemoveApprocalWorkflowOnyxData, + getUpdateApprovalWorkflowOnyxData, }; diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index 6a05e9cb66c6..5da3d6c1b1fd 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -43,7 +43,6 @@ import { removeMembers, updateWorkspaceMembersRole, } from '@libs/actions/Policy/Member'; -import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import {getLatestErrorMessageField} from '@libs/ErrorUtils'; import Log from '@libs/Log'; @@ -55,14 +54,14 @@ import {getAccountIDsByLogins, getDisplayNameOrDefault, getPersonalDetailsByIDs} import {getMemberAccountIDsForWorkspace, isDeletedPolicyEmployee, isExpensifyTeam, isPaidGroupPolicy, isPolicyAdmin as isPolicyAdminUtils} from '@libs/PolicyUtils'; import {getDisplayNameForParticipant} from '@libs/ReportUtils'; import tokenizedSearch from '@libs/tokenizedSearch'; -import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; +import {convertPolicyEmployeesToApprovalWorkflows} from '@libs/WorkflowUtils'; import {close} from '@userActions/Modal'; import {dismissAddedWithPrimaryLoginMessages} from '@userActions/Policy/Policy'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; -import type {PersonalDetails, PersonalDetailsList, PolicyEmployee, PolicyEmployeeList} from '@src/types/onyx'; +import type {PersonalDetailsList, PolicyEmployee, PolicyEmployeeList} from '@src/types/onyx'; import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import MemberRightIcon from './MemberRightIcon'; @@ -146,7 +145,6 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const policyID = route.params.policyID; const illustrations = useMemoizedLazyIllustrations(['ReceiptWrangler'] as const); - const ownerDetails = personalDetails?.[policy?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID] ?? ({} as PersonalDetails); const {approvalWorkflows} = useMemo( () => convertPolicyEmployeesToApprovalWorkflows({ @@ -268,37 +266,11 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers // Remove the admin from the list const accountIDsToRemove = session?.accountID ? selectedEmployees.filter((id) => id !== session.accountID) : selectedEmployees; - // Check if any of the account IDs are approvers - const hasApprovers = accountIDsToRemove.some((accountID) => isApproverTemp(policy, accountID)); - - if (hasApprovers) { - const ownerEmail = ownerDetails.login; - accountIDsToRemove.forEach((accountID) => { - const removedApprover = personalDetails?.[accountID]; - if (!removedApprover?.login || !ownerEmail) { - return; - } - const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ - approvalWorkflows, - removedApprover, - ownerDetails, - }); - updatedWorkflows.forEach((workflow) => { - if (workflow?.removeApprovalWorkflow) { - const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; - removeApprovalWorkflowAction(updatedWorkflow, policy); - } else { - updateApprovalWorkflow(workflow, [], [], policy); - } - }); - }); - } - setRemoveMembersConfirmModalVisible(false); // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => { setSelectedEmployees([]); - removeMembers(accountIDsToRemove, route.params.policyID); + removeMembers(accountIDsToRemove, route.params.policyID, approvalWorkflows); }); }; diff --git a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx index 9b99ef51149b..3ce73bba4847 100644 --- a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx +++ b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx @@ -25,7 +25,6 @@ import usePrevious from '@hooks/usePrevious'; import useStyleUtils from '@hooks/useStyleUtils'; import useThemeIllustrations from '@hooks/useThemeIllustrations'; import useThemeStyles from '@hooks/useThemeStyles'; -import {setPolicyPreventSelfApproval} from '@libs/actions/Policy/Policy'; import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow'; import {getAllCardsForWorkspace, getCardFeedIcon, getCompanyFeeds, getPlaidInstitutionIconUrl, isExpensifyCardFullySetUp, lastFourNumbersFromCardName, maskCardNumber} from '@libs/CardUtils'; import {convertToDisplayString} from '@libs/CurrencyUtils'; @@ -177,15 +176,9 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM // Function to remove a member and close the modal const removeMemberAndCloseModal = useCallback(() => { - removeMembers([accountID], policyID); - const previousEmployeesCount = Object.keys(policy?.employeeList ?? {}).length; - const remainingEmployeeCount = previousEmployeesCount - 1; - if (remainingEmployeeCount === 1 && policy?.preventSelfApproval) { - // We can't let the "Prevent Self Approvals" enabled if there's only one workspace user - setPolicyPreventSelfApproval(route.params.policyID, false); - } + removeMembers([accountID], policyID, approvalWorkflows); setIsRemoveMemberConfirmModalVisible(false); - }, [accountID, policy?.employeeList, policy?.preventSelfApproval, policyID, route.params.policyID]); + }, [accountID, approvalWorkflows, policyID]); const removeUser = useCallback(() => { const ownerEmail = ownerDetails?.login; diff --git a/src/pages/workspace/rules/ExpenseReportRulesSection.tsx b/src/pages/workspace/rules/ExpenseReportRulesSection.tsx index bbd6949ad662..2a32e95ad4ea 100644 --- a/src/pages/workspace/rules/ExpenseReportRulesSection.tsx +++ b/src/pages/workspace/rules/ExpenseReportRulesSection.tsx @@ -25,6 +25,8 @@ function ExpenseReportRulesSection({policyID}: ExpenseReportRulesSectionProps) { const {environmentURL} = useEnvironment(); const workflowApprovalsUnavailable = getWorkflowApprovalsUnavailable(policy); const autoPayApprovedReportsUnavailable = !policy?.areWorkflowsEnabled || policy?.reimbursementChoice !== CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES || !hasVBBA(policyID); + const membersCount = Object.values(policy?.employeeList ?? {}).filter((employee) => employee.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; + const shouldLockPreventSelfApprovals = workflowApprovalsUnavailable || membersCount === 1; const renderFallbackSubtitle = ({featureName, variant = 'unlock'}: {featureName: string; variant?: 'unlock' | 'enable'}) => { const moreFeaturesLink = `${environmentURL}/${ROUTES.WORKSPACE_MORE_FEATURES.getRoute(policyID)}`; @@ -37,14 +39,14 @@ function ExpenseReportRulesSection({policyID}: ExpenseReportRulesSectionProps) { const optionItems = [ { title: translate('workspace.rules.expenseReportRules.preventSelfApprovalsTitle'), - subtitle: workflowApprovalsUnavailable + subtitle: shouldLockPreventSelfApprovals ? renderFallbackSubtitle({featureName: translate('common.approvals').toLowerCase()}) : translate('workspace.rules.expenseReportRules.preventSelfApprovalsSubtitle'), - shouldParseSubtitle: workflowApprovalsUnavailable, + shouldParseSubtitle: shouldLockPreventSelfApprovals, switchAccessibilityLabel: translate('workspace.rules.expenseReportRules.preventSelfApprovalsTitle'), isActive: policy?.preventSelfApproval && !workflowApprovalsUnavailable, - disabled: workflowApprovalsUnavailable, - showLockIcon: workflowApprovalsUnavailable, + disabled: shouldLockPreventSelfApprovals, + showLockIcon: shouldLockPreventSelfApprovals, pendingAction: policy?.pendingFields?.preventSelfApproval, onToggle: (isEnabled: boolean) => setPolicyPreventSelfApproval(policyID, isEnabled), }, From fdabc3e93068ae33c7c26c691b382b5a74cf0e48 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Wed, 29 Oct 2025 00:05:50 +0700 Subject: [PATCH 2/7] update member detail page --- .../members/WorkspaceMemberDetailsPage.tsx | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx index 3ce73bba4847..f89942e7c794 100644 --- a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx +++ b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx @@ -25,7 +25,6 @@ import usePrevious from '@hooks/usePrevious'; import useStyleUtils from '@hooks/useStyleUtils'; import useThemeIllustrations from '@hooks/useThemeIllustrations'; import useThemeStyles from '@hooks/useThemeStyles'; -import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow'; import {getAllCardsForWorkspace, getCardFeedIcon, getCompanyFeeds, getPlaidInstitutionIconUrl, isExpensifyCardFullySetUp, lastFourNumbersFromCardName, maskCardNumber} from '@libs/CardUtils'; import {convertToDisplayString} from '@libs/CurrencyUtils'; import navigateAfterInteraction from '@libs/Navigation/navigateAfterInteraction'; @@ -33,7 +32,7 @@ import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavig import {getDisplayNameOrDefault, getPhoneNumber} from '@libs/PersonalDetailsUtils'; import {isControlPolicy} from '@libs/PolicyUtils'; import shouldRenderTransferOwnerButton from '@libs/shouldRenderTransferOwnerButton'; -import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; +import {convertPolicyEmployeesToApprovalWorkflows} from '@libs/WorkflowUtils'; import Navigation from '@navigation/Navigation'; import type {SettingsNavigatorParamList} from '@navigation/types'; import NotFoundPage from '@pages/ErrorPage/NotFoundPage'; @@ -180,37 +179,6 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM setIsRemoveMemberConfirmModalVisible(false); }, [accountID, approvalWorkflows, policyID]); - const removeUser = useCallback(() => { - const ownerEmail = ownerDetails?.login; - const removedApprover = personalDetails?.[accountID]; - - // If the user is not an approver, proceed with member removal - if (!isApproverUserAction(policy, memberLogin) || !removedApprover?.login || !ownerEmail) { - removeMemberAndCloseModal(); - return; - } - - // Update approval workflows after approver removal - const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ - approvalWorkflows, - removedApprover, - ownerDetails, - }); - - updatedWorkflows.forEach((workflow) => { - if (workflow?.removeApprovalWorkflow) { - const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; - - removeApprovalWorkflowAction(updatedWorkflow, policy); - } else { - updateApprovalWorkflow(workflow, [], [], policy); - } - }); - - // Remove the member and close the modal - removeMemberAndCloseModal(); - }, [accountID, approvalWorkflows, ownerDetails, personalDetails, policy, removeMemberAndCloseModal, memberLogin]); - const navigateToProfile = useCallback(() => { Navigation.navigate(ROUTES.PROFILE.getRoute(accountID, Navigation.getActiveRoute())); }, [accountID]); @@ -335,7 +303,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM danger title={translate('workspace.people.removeMemberTitle')} isVisible={isRemoveMemberConfirmModalVisible} - onConfirm={removeUser} + onConfirm={removeMemberAndCloseModal} onCancel={() => setIsRemoveMemberConfirmModalVisible(false)} prompt={confirmModalPrompt} confirmText={translate('common.remove')} From 7ece60cde133208cc23c928daa46eed1756c96a5 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Wed, 29 Oct 2025 00:14:09 +0700 Subject: [PATCH 3/7] update test --- tests/actions/PolicyMemberTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/actions/PolicyMemberTest.ts b/tests/actions/PolicyMemberTest.ts index 6b1183ab9723..67bba19c7c9e 100644 --- a/tests/actions/PolicyMemberTest.ts +++ b/tests/actions/PolicyMemberTest.ts @@ -458,7 +458,7 @@ describe('actions/PolicyMember', () => { // When removing am admin, auditor, and user members mockFetch?.pause?.(); - Member.removeMembers([adminAccountID, auditorAccountID, userAccountID], policyID); + Member.removeMembers([adminAccountID, auditorAccountID, userAccountID], policyID, []); await waitForBatchedUpdates(); @@ -517,7 +517,7 @@ describe('actions/PolicyMember', () => { // When removing a member from the workspace mockFetch?.pause?.(); - Member.removeMembers([userAccountID], policyID); + Member.removeMembers([userAccountID], policyID, []); await waitForBatchedUpdates(); From 0180d006993e58b84b67192a702535bc2148571a Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Wed, 29 Oct 2025 00:33:43 +0700 Subject: [PATCH 4/7] resolve spell check --- src/libs/actions/Policy/Member.ts | 4 ++-- src/libs/actions/Workflow.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy/Member.ts b/src/libs/actions/Policy/Member.ts index 23e86760eb08..ffb00ca5a474 100644 --- a/src/libs/actions/Policy/Member.ts +++ b/src/libs/actions/Policy/Member.ts @@ -49,7 +49,7 @@ import type {ApprovalRule} from '@src/types/onyx/Policy'; import type {NotificationPreference, Participant} from '@src/types/onyx/Report'; import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import {getRemoveApprocalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '../Workflow'; +import {getRemoveApprovalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '../Workflow'; import {createPolicyExpenseChats, getSetPolicyPreventSelfApprovalOnyxData} from './Policy'; type OnyxDataReturnType = { @@ -474,7 +474,7 @@ function removeMembers(accountIDs: number[], policyID: string, approvalWorkflows updatedWorkflows.forEach((workflow) => { if (workflow?.removeApprovalWorkflow) { const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; - const onyxDataForRemoveApprovalWorkflow = getRemoveApprocalWorkflowOnyxData(updatedWorkflow, policy); + const onyxDataForRemoveApprovalWorkflow = getRemoveApprovalWorkflowOnyxData(updatedWorkflow, policy); optimisticData.push(...(onyxDataForRemoveApprovalWorkflow.optimisticData ?? [])); successData.push(...(onyxDataForRemoveApprovalWorkflow.successData ?? [])); failureData.push(...(onyxDataForRemoveApprovalWorkflow.failureData ?? [])); diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index b2b5d6536caa..f6e3823ad524 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -186,7 +186,7 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } -function getRemoveApprocalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry): OnyxData { +function getRemoveApprovalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry): OnyxData { if (!policy) { return {}; } @@ -247,7 +247,7 @@ function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: Onyx const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({previousEmployeeList, approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.REMOVE}); - const {optimisticData, failureData, successData} = getRemoveApprocalWorkflowOnyxData(approvalWorkflow, policy); + const {optimisticData, failureData, successData} = getRemoveApprovalWorkflowOnyxData(approvalWorkflow, policy); const parameters: RemoveWorkspaceApprovalParams = {policyID: policy.id, employees: JSON.stringify(Object.values(updatedEmployees))}; API.write(WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); @@ -371,6 +371,6 @@ export { clearApprovalWorkflowApprovers, clearApprovalWorkflow, validateApprovalWorkflow, - getRemoveApprocalWorkflowOnyxData, + getRemoveApprovalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData, }; From 7b5932d63edb7bc7f02e5f9d08c5d8a330a328e0 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Thu, 30 Oct 2025 19:49:58 +0700 Subject: [PATCH 5/7] fix lint --- src/libs/actions/Policy/Member.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Policy/Member.ts b/src/libs/actions/Policy/Member.ts index 923e3251e653..abcc8f97d554 100644 --- a/src/libs/actions/Policy/Member.ts +++ b/src/libs/actions/Policy/Member.ts @@ -26,6 +26,7 @@ import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import {updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; import * as FormActions from '@userActions/FormActions'; +import {getRemoveApprovalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '@userActions/Workflow'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type { @@ -47,7 +48,6 @@ import type {ApprovalRule} from '@src/types/onyx/Policy'; import type {NotificationPreference, Participant} from '@src/types/onyx/Report'; import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import {getRemoveApprovalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '../Workflow'; import {createPolicyExpenseChats, getSetPolicyPreventSelfApprovalOnyxData} from './Policy'; type OnyxDataReturnType = { From 701afd096c341f92ec387500fbf31ab73a6e7d06 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Fri, 7 Nov 2025 16:43:43 +0700 Subject: [PATCH 6/7] fix: incorrect pending action --- src/libs/WorkflowUtils.ts | 12 ++++++++---- src/libs/actions/Workflow.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 5a248c9003ef..02cdfa7f8fb3 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -260,10 +260,11 @@ function convertApprovalWorkflowToPolicyEmployees({ return; } + const previousPendingAction = previousEmployeeList[approver.email]?.pendingAction; updatedEmployeeList[approver.email] = { email: approver.email, forwardsTo, - pendingAction, + pendingAction: previousPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? previousPendingAction : pendingAction, pendingFields: { forwardsTo: pendingAction, }, @@ -279,10 +280,11 @@ function convertApprovalWorkflowToPolicyEmployees({ return; } + const previousPendingAction = previousEmployeeList[email]?.pendingAction; updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo, - pendingAction, + pendingAction: previousPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? previousPendingAction : pendingAction, pendingFields: { submitsTo: pendingAction, }, @@ -292,20 +294,22 @@ function convertApprovalWorkflowToPolicyEmployees({ // For each member to remove, we update the employee list with submitsTo set to '' // which will set the submitsTo field to the default approver email on backend. membersToRemove?.forEach(({email}) => { + const previousPendingAction = previousEmployeeList[email]?.pendingAction; updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo: defaultApprover, - pendingAction, + pendingAction: previousPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? previousPendingAction : pendingAction, }; }); // For each approver to remove, we update the employee list with forwardsTo set to '' // which will reset the forwardsTo on the backend. approversToRemove?.forEach(({email}) => { + const previousPendingAction = previousEmployeeList[email]?.pendingAction; updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), forwardsTo: '', - pendingAction, + pendingAction: previousPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? previousPendingAction : pendingAction, }; }); diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 50e8b2e2101c..4e4c598ed16a 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -183,7 +183,7 @@ function getRemoveApprovalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, p return {}; } - const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); + const previousEmployeeList = policy.employeeList ?? {}; const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({previousEmployeeList, approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.REMOVE}); const updatedEmployeeList = {...previousEmployeeList, ...updatedEmployees}; From b6a13fe7e82d22fffc53f167a6724f294ff87cd0 Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Sat, 22 Nov 2025 21:58:55 +0700 Subject: [PATCH 7/7] fix test --- tests/actions/PolicyMemberTest.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/actions/PolicyMemberTest.ts b/tests/actions/PolicyMemberTest.ts index d253777988c6..c745287d97ac 100644 --- a/tests/actions/PolicyMemberTest.ts +++ b/tests/actions/PolicyMemberTest.ts @@ -572,7 +572,10 @@ describe('actions/PolicyMember', () => { // When removing a member and the request fails mockFetch?.fail?.(); - Member.removeMembers(policyID, [userEmail], {[userEmail]: userAccountID}); + const allPersonalDetails = { + [userAccountID]: {login: userEmail}, + } as unknown as PersonalDetailsList; + Member.removeMembers(policyID, [userEmail], {[userEmail]: userAccountID}, [], allPersonalDetails); await waitForBatchedUpdates();