From 3701866bd609c696cad306ace9a0e81ede988f82 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 27 Aug 2024 15:39:34 +0200 Subject: [PATCH 1/5] Update pendingAction/pendingFields when making an action offline --- src/libs/WorkflowUtils.ts | 19 +++++++++++++-- src/libs/actions/Workflow.ts | 23 +++++-------------- .../workflows/WorkspaceWorkflowsPage.tsx | 2 +- src/types/onyx/ApprovalWorkflow.ts | 5 ++-- src/types/onyx/Policy.ts | 2 +- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 88e420dfaf7e..19593615bf96 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -116,12 +116,17 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, // Add each employee to the appropriate workflow Object.values(employees).forEach((employee) => { - const {email, submitsTo} = employee; + const {email, submitsTo, pendingAction} = employee; if (!email || !submitsTo) { return; } - const member: Member = {email, avatar: personalDetailsByEmail[email]?.avatar, displayName: personalDetailsByEmail[email]?.displayName ?? email}; + const member: Member = { + email, + avatar: personalDetailsByEmail[email]?.avatar, + displayName: personalDetailsByEmail[email]?.displayName ?? email, + }; + if (!approvalWorkflows[submitsTo]) { const approvers = calculateApprovers({employees, firstEmail: submitsTo, personalDetailsByEmail}); if (submitsTo !== firstApprover) { @@ -132,9 +137,14 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, members: [], approvers, isDefault: defaultApprover === submitsTo, + pendingAction, }; } + approvalWorkflows[submitsTo].members.push(member); + if (pendingAction) { + approvalWorkflows[submitsTo].pendingAction = pendingAction; + } }); // Sort the workflows by the first approver's name (default workflow has priority) @@ -189,11 +199,14 @@ function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, membersToRe throw new Error('Approval workflow must have at least one approver'); } + const pendingAction = type === CONST.APPROVAL_WORKFLOW.TYPE.CREATE ? CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD : CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE; + approvalWorkflow.approvers.forEach((approver, index) => { const nextApprover = approvalWorkflow.approvers.at(index + 1); updatedEmployeeList[approver.email] = { email: approver.email, forwardsTo: type === CONST.APPROVAL_WORKFLOW.TYPE.REMOVE ? '' : nextApprover?.email ?? '', + pendingAction, }; }); @@ -201,6 +214,7 @@ function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, membersToRe updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo: type === CONST.APPROVAL_WORKFLOW.TYPE.REMOVE ? '' : firstApprover.email ?? '', + pendingAction, }; }); @@ -208,6 +222,7 @@ function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, membersToRe updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo: '', + pendingAction, }; }); diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 946d7682675d..3e9f6eb12060 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -52,7 +52,7 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork return; } - const previousEmployeeList = {...policy.employeeList}; + const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); const previousApprovalMode = policy.approvalMode; const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.CREATE}); @@ -70,7 +70,6 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork value: { employeeList: updatedEmployees, approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - pendingFields: {employeeList: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}, }, }, ]; @@ -87,7 +86,6 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork value: { employeeList: previousEmployeeList, approvalMode: previousApprovalMode, - pendingFields: {employeeList: null}, }, }, ]; @@ -102,7 +100,7 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, value: { - pendingFields: {employeeList: null}, + employeeList: Object.fromEntries(Object.keys(updatedEmployees).map((key) => [key, {pendingAction: null}])), }, }, ]; @@ -120,7 +118,7 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork const previousDefaultApprover = policy.approver ?? policy.owner; const newDefaultApprover = approvalWorkflow.isDefault ? approvalWorkflow.approvers[0].email : undefined; - const previousEmployeeList = {...policy.employeeList}; + const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.UPDATE, membersToRemove}); const optimisticData: OnyxUpdate[] = [ @@ -136,7 +134,6 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, value: { employeeList: updatedEmployees, - pendingFields: {employeeList: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}, ...(newDefaultApprover ? {approver: newDefaultApprover} : {}), }, }, @@ -169,7 +166,7 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, value: { - pendingFields: {employeeList: null}, + employeeList: Object.fromEntries(Object.keys(updatedEmployees).map((key) => [key, {pendingAction: null}])), }, }, ]; @@ -190,8 +187,8 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork return; } - const previousEmployeeList = {...policy.employeeList}; const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.REMOVE}); + const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); const updatedEmployeeList = {...previousEmployeeList, ...updatedEmployees}; const defaultApprover = policy.approver ?? policy.owner; @@ -212,7 +209,6 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork value: { employeeList: updatedEmployees, approvalMode: hasMoreThanOneWorkflow ? CONST.POLICY.APPROVAL_MODE.ADVANCED : CONST.POLICY.APPROVAL_MODE.BASIC, - pendingFields: {employeeList: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}, }, }, ]; @@ -231,13 +227,6 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, }, }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: { - pendingFields: {employeeList: null}, - }, - }, ]; const successData: OnyxUpdate[] = [ @@ -250,7 +239,7 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, value: { - pendingFields: {employeeList: null}, + employeeList: Object.fromEntries(Object.keys(updatedEmployees).map((key) => [key, {pendingAction: null}])), }, }, ]; diff --git a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx index bd0f2775a884..9c03e5001a28 100644 --- a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx +++ b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx @@ -176,7 +176,7 @@ function WorkspaceWorkflowsPage({policy, betas, route}: WorkspaceWorkflowsPagePr ; /** * Approval workflow for a group of employees with additional properties for the Onyx store diff --git a/src/types/onyx/Policy.ts b/src/types/onyx/Policy.ts index 0260b3354f79..44874e5d4248 100644 --- a/src/types/onyx/Policy.ts +++ b/src/types/onyx/Policy.ts @@ -1564,7 +1564,7 @@ type Policy = OnyxCommon.OnyxValueWithOfflineFeedback< /** Workspace account ID configured for Expensify Card */ workspaceAccountID?: number; } & Partial, - 'addWorkspaceRoom' | 'employeeList' | keyof ACHAccount | keyof Attributes + 'addWorkspaceRoom' | keyof ACHAccount | keyof Attributes >; /** Stages of policy connection sync */ From 54a76188a069ee8656588b3c3d13b8424b8e2cc5 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 28 Aug 2024 17:55:51 +0200 Subject: [PATCH 2/5] Fix tests --- tests/unit/WorkflowUtilsTest.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 8ef2b873bd8e..ed5c09374008 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -13,6 +13,7 @@ const personalDetailsByEmail: PersonalDetailsList = {}; function buildPolicyEmployee(accountID: number, policyEmployee: Partial = {}): PolicyEmployee { return { email: `${accountID}@example.com`, + pendingAction: 'add', ...policyEmployee, }; } @@ -417,12 +418,12 @@ describe('WorkflowUtils', () => { const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, type: 'remove'}); expect(convertedEmployees).toEqual({ - '1@example.com': buildPolicyEmployee(1, {forwardsTo: ''}), - '2@example.com': buildPolicyEmployee(2, {forwardsTo: ''}), - '3@example.com': buildPolicyEmployee(3, {forwardsTo: ''}), - '4@example.com': buildPolicyEmployee(4, {submitsTo: ''}), - '5@example.com': buildPolicyEmployee(5, {submitsTo: ''}), - '6@example.com': buildPolicyEmployee(6, {submitsTo: ''}), + '1@example.com': buildPolicyEmployee(1, {forwardsTo: '', pendingAction: 'update'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: '', pendingAction: 'update'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: '', pendingAction: 'update'}), + '4@example.com': buildPolicyEmployee(4, {submitsTo: '', pendingAction: 'update'}), + '5@example.com': buildPolicyEmployee(5, {submitsTo: '', pendingAction: 'update'}), + '6@example.com': buildPolicyEmployee(6, {submitsTo: '', pendingAction: 'update'}), }); }); }); From f0e1d9e9c946b96d263b82f67386f1ca3ba20184 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 4 Sep 2024 21:16:03 +0200 Subject: [PATCH 3/5] Fix a mistake when merging --- src/libs/WorkflowUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 9e3b91fc346e..17b1d5d22240 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -245,7 +245,6 @@ function convertApprovalWorkflowToPolicyEmployees({ } updatedEmployeeList[email] = { - ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), email, submitsTo, pendingAction, From ef4bcd73b3eb031969c2dc033089a63b6592c302 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 4 Sep 2024 21:44:04 +0200 Subject: [PATCH 4/5] Fix 402 error when updating with no changes in the payload --- src/libs/actions/Workflow.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index f61697ffcf74..4adb692919ee 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -127,6 +127,10 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork approversToRemove, }); + if (isEmptyObject(updatedEmployees) && !newDefaultApprover) { + return; + } + const optimisticData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, From 361d837e5127e72c1b54895106ceee55eba0ac44 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 4 Sep 2024 21:44:27 +0200 Subject: [PATCH 5/5] Improve pendingAction logic --- src/libs/WorkflowUtils.ts | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 17b1d5d22240..4682654d3583 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -239,39 +239,24 @@ function convertApprovalWorkflowToPolicyEmployees({ return; } - if (updatedEmployeeList[email]) { - updatedEmployeeList[email].submitsTo = submitsTo; - return; - } - updatedEmployeeList[email] = { - email, + ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo, pendingAction, }; }); membersToRemove?.forEach(({email}) => { - if (updatedEmployeeList[email]) { - updatedEmployeeList[email].submitsTo = ''; - return; - } - updatedEmployeeList[email] = { - email, + ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), submitsTo: '', pendingAction, }; }); approversToRemove?.forEach(({email}) => { - if (updatedEmployeeList[email]) { - updatedEmployeeList[email].forwardsTo = ''; - return; - } - updatedEmployeeList[email] = { - email, + ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : {email}), forwardsTo: '', pendingAction, };