-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Self approval stays enabled if all members are removed by multi selection #73676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0f4ce77
fdabc3e
7ece60c
0180d00
a373e7b
7b5932d
7c192e1
10ae33b
701afd0
b04cb24
b6a13fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -24,17 +24,31 @@ | |||
| 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 {getRemoveApprovalWorkflowOnyxData, getUpdateApprovalWorkflowOnyxData} from '@userActions/Workflow'; | ||||
| import CONST from '@src/CONST'; | ||||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||||
| import type {ImportedSpreadsheetMemberData, InvitedEmailsToAccountIDs, Policy, PolicyEmployee, PolicyOwnershipChangeChecks, Report, ReportAction, ReportActions} from '@src/types/onyx'; | ||||
| import type { | ||||
| ImportedSpreadsheetMemberData, | ||||
| InvitedEmailsToAccountIDs, | ||||
| PersonalDetails, | ||||
| PersonalDetailsList, | ||||
| Policy, | ||||
| PolicyEmployee, | ||||
| PolicyOwnershipChangeChecks, | ||||
| Report, | ||||
| 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 {createPolicyExpenseChats, getSetPolicyPreventSelfApprovalOnyxData} from './Policy'; | ||||
|
|
||||
| type OnyxDataReturnType = { | ||||
| optimisticData: OnyxUpdate[]; | ||||
|
|
@@ -48,7 +62,7 @@ | |||
| }; | ||||
|
|
||||
| const allPolicies: OnyxCollection<Policy> = {}; | ||||
| Onyx.connect({ | ||||
| key: ONYXKEYS.COLLECTION.POLICY, | ||||
| callback: (val, key) => { | ||||
| if (!key) { | ||||
|
|
@@ -64,7 +78,7 @@ | |||
| }); | ||||
|
|
||||
| let allReportActions: OnyxCollection<ReportActions>; | ||||
| Onyx.connect({ | ||||
| key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, | ||||
| waitForCollectionCallback: true, | ||||
| callback: (actions) => (allReportActions = actions), | ||||
|
|
@@ -72,7 +86,7 @@ | |||
|
|
||||
| let sessionEmail = ''; | ||||
| let sessionAccountID = 0; | ||||
| Onyx.connect({ | ||||
| key: ONYXKEYS.SESSION, | ||||
| callback: (val) => { | ||||
| sessionEmail = val?.email ?? ''; | ||||
|
|
@@ -81,7 +95,7 @@ | |||
| }); | ||||
|
|
||||
| let policyOwnershipChecks: Record<string, PolicyOwnershipChangeChecks>; | ||||
| Onyx.connect({ | ||||
| key: ONYXKEYS.POLICY_OWNERSHIP_CHANGE_CHECKS, | ||||
| callback: (value) => { | ||||
| policyOwnershipChecks = value ?? {}; | ||||
|
|
@@ -395,7 +409,13 @@ | |||
| * 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(policyID: string, selectedMemberEmails: string[], policyMemberEmailsToAccountIDs: Record<string, number>) { | ||||
| function removeMembers( | ||||
| policyID: string, | ||||
| selectedMemberEmails: string[], | ||||
| policyMemberEmailsToAccountIDs: Record<string, number>, | ||||
| approvalWorkflows: ApprovalWorkflow[], | ||||
| allPersonalDetails: OnyxEntry<PersonalDetailsList>, | ||||
| ) { | ||||
| if (selectedMemberEmails.length === 0) { | ||||
| return; | ||||
| } | ||||
|
|
@@ -407,6 +427,44 @@ | |||
| // 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 = selectedMemberEmails.some((selectedMemberEmail) => isApprover(policy, selectedMemberEmail)); | ||||
| 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 = getRemoveApprovalWorkflowOnyxData(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 optimisticClosedReportActions = workspaceChats.map(() => | ||||
| ReportUtils.buildOptimisticClosedReportAction(sessionEmail, policy?.name ?? '', CONST.REPORT.ARCHIVE_REASON.REMOVED_FROM_POLICY), | ||||
|
|
@@ -477,38 +535,32 @@ | |||
| const approvalRules: ApprovalRule[] = policy?.rules?.approvalRules ?? []; | ||||
| const optimisticApprovalRules = approvalRules.filter((rule) => !selectedMemberEmails.includes(rule?.approver ?? '')); | ||||
|
|
||||
| const optimisticData: OnyxUpdate[] = [ | ||||
| { | ||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||
| key: policyKey, | ||||
| value: { | ||||
| employeeList: optimisticMembersState, | ||||
| approver: selectedMemberEmails.includes(policy?.approver ?? '') ? policy?.owner : policy?.approver, | ||||
| rules: { | ||||
| ...(policy?.rules ?? {}), | ||||
| approvalRules: optimisticApprovalRules, | ||||
| }, | ||||
| optimisticData.push({ | ||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||
| key: policyKey, | ||||
| value: { | ||||
| employeeList: optimisticMembersState, | ||||
| approver: selectedMemberEmails.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); | ||||
|
|
@@ -647,6 +699,16 @@ | |||
| policyID, | ||||
| }; | ||||
|
|
||||
| // Update "Prevent Self Approvals" after member removal | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| 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 ?? [])); | ||||
| } | ||||
|
|
||||
|
Comment on lines
+703
to
+711
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this above |
||||
| API.write(WRITE_COMMANDS.DELETE_MEMBERS_FROM_WORKSPACE, params, {optimisticData, successData, failureData}); | ||||
| } | ||||
|
|
||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,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'; | ||
|
|
||
| type SetApprovalWorkflowApproverParams = { | ||
|
|
@@ -82,9 +83,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<Policy>) { | ||
| function getUpdateApprovalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, membersToRemove: Member[], approversToRemove: Approver[], policy: OnyxEntry<Policy>): OnyxData { | ||
| if (!policy) { | ||
| return; | ||
| return {}; | ||
| } | ||
|
|
||
| const previousDefaultApprover = getDefaultApprover(policy); | ||
|
|
@@ -101,7 +102,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[] = [ | ||
|
|
@@ -142,6 +143,33 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem | |
| }, | ||
| ]; | ||
|
|
||
| return {optimisticData, failureData, successData}; | ||
| } | ||
|
|
||
| function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRemove: Member[], approversToRemove: Approver[], policy: OnyxEntry<Policy>) { | ||
| 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; | ||
| } | ||
|
Comment on lines
+154
to
+169
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we return this from |
||
|
|
||
| const {optimisticData, failureData, successData} = getUpdateApprovalWorkflowOnyxData(approvalWorkflow, membersToRemove, approversToRemove, policy); | ||
|
|
||
| const parameters: UpdateWorkspaceApprovalParams = { | ||
| policyID: policy.id, | ||
| employees: JSON.stringify(Object.values(updatedEmployees)), | ||
|
|
@@ -150,12 +178,12 @@ function updateApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, membersToRem | |
| API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); | ||
| } | ||
|
|
||
| function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry<Policy>) { | ||
| function getRemoveApprovalWorkflowOnyxData(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry<Policy>): OnyxData { | ||
| if (!policy) { | ||
| return; | ||
| 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}; | ||
|
|
||
|
|
@@ -200,6 +228,19 @@ function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: Onyx | |
| }, | ||
| ]; | ||
|
|
||
| return {optimisticData, failureData, successData}; | ||
| } | ||
|
|
||
| function removeApprovalWorkflow(approvalWorkflow: ApprovalWorkflow, policy: OnyxEntry<Policy>) { | ||
| 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}); | ||
|
Comment on lines
+239
to
+240
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we return this from |
||
|
|
||
| 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}); | ||
| } | ||
|
|
@@ -322,4 +363,6 @@ export { | |
| clearApprovalWorkflowApprovers, | ||
| clearApprovalWorkflow, | ||
| validateApprovalWorkflow, | ||
| getRemoveApprovalWorkflowOnyxData, | ||
| getUpdateApprovalWorkflowOnyxData, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it
letand move the onyx update assignment out ofif... else...statement