From 37e8cc5ab1b27bac53137e4d49ca2317d1f24e61 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 23 Jul 2024 19:35:28 +0200 Subject: [PATCH 01/20] Add ApprovalWorkflow Onyx types --- src/types/onyx/ApprovalWorkflow.ts | 84 ++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 src/types/onyx/ApprovalWorkflow.ts diff --git a/src/types/onyx/ApprovalWorkflow.ts b/src/types/onyx/ApprovalWorkflow.ts new file mode 100644 index 000000000000..929876cd0b75 --- /dev/null +++ b/src/types/onyx/ApprovalWorkflow.ts @@ -0,0 +1,84 @@ +import type {AvatarSource} from '@libs/UserUtils'; + +/** + * Approver in the approval workflow + */ +type Approver = { + /** + * Email of the approver + */ + email: string; + + /** + * Email of the user this user forwards all approved reports to + */ + forwardsTo?: string; + + /** + * Avatar URL of the current user from their personal details + */ + avatar?: AvatarSource; + + /** + * Display name of the current user from their personal details + */ + displayName?: string; + + /** + * Is this approver in more than one workflow + */ + isInMultipleWorkflows: boolean; + + /** + * Is this approver in a circular reference + */ + isCircularReference?: boolean; +}; + +/** + * + */ +type Member = { + /** + * Email of the member + */ + email: string; + + /** + * Avatar URL of the current user from their personal details + */ + avatar?: AvatarSource; + + /** + * Display name of the current user from their personal details + */ + displayName?: string; +}; + +/** + * Approval workflow for a group of employees + */ +type ApprovalWorkflow = { + /** + * List of member emails in the workflow + */ + members: Member[]; + + /** + * List of approvers in the workflow + */ + approvers: Approver[]; + + /** + * Is this the default workflow + */ + isDefault: boolean; + + /** + * Is this workflow being edited vs created + */ + isBeingEdited: boolean; +}; + +export default ApprovalWorkflow; +export type {Approver, Member}; From 8a316a5828b446532d83bc41594359eb7057712b Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 23 Jul 2024 19:36:11 +0200 Subject: [PATCH 02/20] Implement functions that transform the data back and forth --- src/libs/WorkflowUtils.ts | 159 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 src/libs/WorkflowUtils.ts diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts new file mode 100644 index 000000000000..5d5bb5c10ee9 --- /dev/null +++ b/src/libs/WorkflowUtils.ts @@ -0,0 +1,159 @@ +import lodashMapKeys from 'lodash/mapKeys'; +import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; +import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; +import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; +import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; + +type GetApproversParams = { + /** + * List of employees in the policy + */ + employees: PolicyEmployeeList; + + /** + * Personal details of the employees where the key is the email + */ + personalDetailsByEmail: PersonalDetailsList; + + /** + * Email of the first approver + */ + firstEmail: string; +}; + +/** Get the list of approvers for a given workflow */ +function getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}: GetApproversParams): Approver[] { + const approvers: Approver[] = []; + // Keep track of approver emails to detect circular references + const currentApproverEmails = new Set(); + + let nextEmail: string | undefined = firstEmail; + while (nextEmail) { + if (!employees[nextEmail]) { + break; + } + + const isCircularReference = currentApproverEmails.has(nextEmail); + approvers.push({ + email: nextEmail, + forwardsTo: employees[nextEmail].forwardsTo, + avatar: personalDetailsByEmail[nextEmail]?.avatar, + displayName: personalDetailsByEmail[nextEmail]?.displayName, + isInMultipleWorkflows: false, + isCircularReference, + }); + + // If we've already seen this approver, break to prevent infinite loop + if (isCircularReference) { + break; + } + currentApproverEmails.add(nextEmail); + + // If there is a forwardsTo, set the next approver to the forwardsTo + nextEmail = employees[nextEmail].forwardsTo; + } + + return approvers; +} + +type ConvertPolicyEmployeesToApprovalWorkflowsParams = { + /** + * List of employees in the policy + */ + employees: PolicyEmployeeList; + + /** + * Personal details of the employees + */ + personalDetails: PersonalDetailsList; + + /** + * Email of the default approver for the policy + */ + defaultApprover: string; +}; + +/** Convert a list of policy employees to a list of approval workflows */ +function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}: ConvertPolicyEmployeesToApprovalWorkflowsParams): ApprovalWorkflow[] { + const approvalWorkflows: Record = {}; + + // Keep track of how many times each approver is used to detect approvers in multiple workflows + const approverCountsByEmail: Record = {}; + const personalDetailsByEmail = lodashMapKeys(personalDetails, (value, key) => value?.login ?? key); + + Object.values(employees).forEach((employee) => { + const {email, submitsTo} = employee; + if (!email || !submitsTo) { + return; + } + + const member: Member = {email, avatar: personalDetailsByEmail[email]?.avatar, displayName: personalDetailsByEmail[email]?.displayName ?? email}; + if (!approvalWorkflows[submitsTo]) { + const approvers = getApprovalWorkflowApprovers({employees, firstEmail: submitsTo, personalDetailsByEmail}); + approvers.forEach((approver) => (approverCountsByEmail[approver.email] = (approverCountsByEmail[approver.email] ?? 0) + 1)); + + approvalWorkflows[submitsTo] = { + members: [], + approvers, + isDefault: defaultApprover === submitsTo, + isBeingEdited: false, + }; + } + approvalWorkflows[submitsTo].members.push(member); + }); + + // Add isInMultipleWorkflows to each approver + Object.values(approvalWorkflows).forEach((workflow) => + workflow.approvers.map((approver) => ({ + ...approver, + isInMultipleWorkflows: approverCountsByEmail[approver.email] > 1, + })), + ); + + const sortedApprovalWorkflows = Object.values(approvalWorkflows).sort((a, b) => { + if (a.isDefault) { + return -1; + } + + if (b.isDefault) { + return 1; + } + + const aDisplayName = b.approvers.at(0)?.displayName ?? '-1'; + const bDisplayName = a.approvers.at(0)?.displayName ?? '-1'; + return aDisplayName < bDisplayName ? 1 : -1; + }); + + return sortedApprovalWorkflows; +} + +/** Convert an approval workflow to a list of policy employees */ +function convertApprovalWorkflowToPolicyEmployees(approvalWorkflow: ApprovalWorkflow, employeeList: PolicyEmployeeList): PolicyEmployeeList { + const employees: PolicyEmployeeList = {}; + const firstApprover = approvalWorkflow.approvers.at(0); + + if (!firstApprover) { + throw new Error('Approval workflow must have at least one approver'); + } + + approvalWorkflow.members.forEach(({email}) => { + employees[email] = { + ...employeeList[email], + submitsTo: firstApprover.email, + }; + }); + + approvalWorkflow.approvers.forEach((approver, index) => { + const nextApprover = approvalWorkflow.approvers[index + 1]; + if (nextApprover) { + employees[approver.email] = { + ...employeeList[approver.email], + forwardsTo: nextApprover.email, + }; + } + }); + + return employees; +} + +export {getApprovalWorkflowApprovers, convertPolicyEmployeesToApprovalWorkflows, convertApprovalWorkflowToPolicyEmployees}; From ed067dea0d5c46b7da3310f9bc6f0c6053cece58 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 23 Jul 2024 19:36:26 +0200 Subject: [PATCH 03/20] Start working on tests for WorkflowUtils --- tests/unit/WorkflowUtilsTest.ts | 73 +++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 tests/unit/WorkflowUtilsTest.ts diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts new file mode 100644 index 000000000000..902bd20d9202 --- /dev/null +++ b/tests/unit/WorkflowUtilsTest.ts @@ -0,0 +1,73 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import * as WorkflowUtils from '@src/libs/WorkflowUtils'; +import type {Approver} from '@src/types/onyx/ApprovalWorkflow'; +import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; +import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; +import * as TestHelper from '../utils/TestHelper'; + +const personalDetails: PersonalDetailsList = {}; +const personalDetailsByEmail: PersonalDetailsList = {}; + +function buildApprover(accountID: number, approver: Partial = {}): Approver { + return { + email: `${accountID}@example.com`, + forwardsTo: undefined, + avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_7.png', + displayName: `${accountID}@example.com User`, + isInMultipleWorkflows: false, + isCircularReference: false, + ...approver, + }; +} + +describe('WorkflowUtils', () => { + beforeAll(() => { + for (let accountID = 0; accountID < 10; accountID++) { + const email = `${accountID}@example.com`; + personalDetails[accountID] = TestHelper.buildPersonalDetails(email, accountID, email); + personalDetailsByEmail[email] = personalDetails[email]; + } + }); + + it('Should return no approvers for empty employees object', () => { + const employees: PolicyEmployeeList = {}; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + + expect(approvers).toEqual([]); + }); + + it('Should return just one approver if there is no forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + }, + }; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + + expect(approvers).toEqual([buildApprover(1)]); + }); + + it('Should return just one approver if there is no forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + }, + }; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + + expect(approvers).toEqual([buildApprover(1)]); + }); +}); From b47115f90036b0ffbf8c932a29672c36cca70e87 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 09:13:58 +0200 Subject: [PATCH 04/20] Add getApprovalWorkflowApprovers tests --- tests/unit/WorkflowUtilsTest.ts | 198 ++++++++++++++++++++++++++------ 1 file changed, 162 insertions(+), 36 deletions(-) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 902bd20d9202..2ceb2fdf45fb 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -25,49 +25,175 @@ describe('WorkflowUtils', () => { for (let accountID = 0; accountID < 10; accountID++) { const email = `${accountID}@example.com`; personalDetails[accountID] = TestHelper.buildPersonalDetails(email, accountID, email); - personalDetailsByEmail[email] = personalDetails[email]; + personalDetailsByEmail[email] = personalDetails[accountID]; } }); - it('Should return no approvers for empty employees object', () => { - const employees: PolicyEmployeeList = {}; - const firstEmail = '1@example.com'; - const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + describe('getApprovalWorkflowApprovers', () => { + it('Should return no approvers for empty employees object', () => { + const employees: PolicyEmployeeList = {}; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); - expect(approvers).toEqual([]); - }); + expect(approvers).toEqual([]); + }); - it('Should return just one approver if there is no forwardsTo', () => { - const employees: PolicyEmployeeList = { - '1@example.com': { - email: '1@example.com', - forwardsTo: undefined, - }, - '2@example.com': { - email: '2@example.com', - forwardsTo: undefined, - }, - }; - const firstEmail = '1@example.com'; - const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + it('Should return just one approver if there is no forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + }, + }; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); - expect(approvers).toEqual([buildApprover(1)]); - }); + expect(approvers).toEqual([buildApprover(1)]); + }); + + it('Should return just one approver if there is no forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + }, + }; + const firstEmail = '1@example.com'; + const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + + expect(approvers).toEqual([buildApprover(1)]); + }); + + it('Should return a list of approver when there are forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: '2@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: '3@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: '4@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: undefined, + }, + '5@example.com': { + email: '5@example.com', + forwardsTo: undefined, + }, + }; + + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '1@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(1, {forwardsTo: '2@example.com'}), + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '2@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '3@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + }); + + it('Should return a list of approver when there are forwardsTo', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: '2@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: '3@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: '4@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: undefined, + }, + '5@example.com': { + email: '5@example.com', + forwardsTo: undefined, + }, + }; + + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '1@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(1, {forwardsTo: '2@example.com'}), + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '2@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '3@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4), + ]); + }); - it('Should return just one approver if there is no forwardsTo', () => { - const employees: PolicyEmployeeList = { - '1@example.com': { - email: '1@example.com', - forwardsTo: undefined, - }, - '2@example.com': { - email: '2@example.com', - forwardsTo: undefined, - }, - }; - const firstEmail = '1@example.com'; - const approvers = WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail, personalDetailsByEmail}); + it('Should return a list of approver with circular references', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: '2@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: '3@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: '4@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: '5@example.com', + }, + '5@example.com': { + email: '5@example.com', + forwardsTo: '1@example.com', + }, + }; - expect(approvers).toEqual([buildApprover(1)]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '1@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(1, {forwardsTo: '2@example.com'}), + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4, {forwardsTo: '5@example.com'}), + buildApprover(5, {forwardsTo: '1@example.com'}), + buildApprover(1, {forwardsTo: '2@example.com', isCircularReference: true}), + ]); + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '2@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(2, {forwardsTo: '3@example.com'}), + buildApprover(3, {forwardsTo: '4@example.com'}), + buildApprover(4, {forwardsTo: '5@example.com'}), + buildApprover(5, {forwardsTo: '1@example.com'}), + buildApprover(1, {forwardsTo: '2@example.com'}), + buildApprover(2, {forwardsTo: '3@example.com', isCircularReference: true}), + ]); + }); }); }); From a98914d03a93313cc8d91014d53436e044b95ce7 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 09:27:03 +0200 Subject: [PATCH 05/20] Add a cases where user forwards to themselfs --- tests/unit/WorkflowUtilsTest.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 2ceb2fdf45fb..e8912a890e40 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -195,5 +195,19 @@ describe('WorkflowUtils', () => { buildApprover(2, {forwardsTo: '3@example.com', isCircularReference: true}), ]); }); + + it('Should return a list of approver with circular references', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: '1@example.com', + }, + }; + + expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '1@example.com', personalDetailsByEmail})).toEqual([ + buildApprover(1, {forwardsTo: '1@example.com'}), + buildApprover(1, {forwardsTo: '1@example.com', isCircularReference: true}), + ]); + }); }); }); From 8d63e994295aeb1a5ad8925a126ed6a8b7608c70 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 16:25:16 +0200 Subject: [PATCH 06/20] Add tests for convertPolicyEmployeesToApprovalWorkflows --- tests/unit/WorkflowUtilsTest.ts | 147 +++++++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 1 deletion(-) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index e8912a890e40..0379fa967604 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/naming-convention */ import * as WorkflowUtils from '@src/libs/WorkflowUtils'; -import type {Approver} from '@src/types/onyx/ApprovalWorkflow'; +import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; +import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import * as TestHelper from '../utils/TestHelper'; @@ -8,6 +9,14 @@ import * as TestHelper from '../utils/TestHelper'; const personalDetails: PersonalDetailsList = {}; const personalDetailsByEmail: PersonalDetailsList = {}; +function buildMember(accountID: number): Member { + return { + email: `${accountID}@example.com`, + avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_7.png', + displayName: `${accountID}@example.com User`, + }; +} + function buildApprover(accountID: number, approver: Partial = {}): Approver { return { email: `${accountID}@example.com`, @@ -20,6 +29,16 @@ function buildApprover(accountID: number, approver: Partial = {}): App }; } +function buildWorkflow(memberIDs: number[], approverIDs: number[], workflow: Partial = {}): ApprovalWorkflow { + return { + members: memberIDs.map(buildMember), + approvers: approverIDs.map((id) => buildApprover(id)), + isDefault: false, + isBeingEdited: false, + ...workflow, + }; +} + describe('WorkflowUtils', () => { beforeAll(() => { for (let accountID = 0; accountID < 10; accountID++) { @@ -210,4 +229,130 @@ describe('WorkflowUtils', () => { ]); }); }); + + describe('convertPolicyEmployeesToApprovalWorkflows', () => { + it('Should return an empty list if there are no employees', () => { + const employees: PolicyEmployeeList = {}; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + expect(workflows).toEqual([]); + }); + + it('Should transform all users into one default workflow', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + }; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + expect(workflows).toEqual([buildWorkflow([1, 2], [1], {isDefault: true})]); + }); + + it('Should transform all users into two workflows', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + }; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + expect(workflows).toEqual([buildWorkflow([2, 3], [1], {isDefault: true}), buildWorkflow([1, 4], [4])]); + }); + + it('Should sort the workflows (first the default and then based on the first approver display name)', () => { + const employees: PolicyEmployeeList = { + '5@example.com': { + email: '5@example.com', + forwardsTo: undefined, + submitsTo: '3@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + }; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + expect(workflows).toEqual([buildWorkflow([3, 2], [1], {isDefault: true}), buildWorkflow([5], [3]), buildWorkflow([4, 1], [4])]); + }); + + it('Should mark users that are used in multiple workflows', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '3@example.com': { + email: '3@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + }; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + expect(workflows).toEqual([buildWorkflow([2, 3], [1], {isDefault: true}), buildWorkflow([1, 4], [4])]); + }); + }); }); From 52ad6c23230a977e6dddb654ec941ec4a36a99f6 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 20:59:13 +0200 Subject: [PATCH 07/20] Finish tests for convertPolicyEmployeesToApprovalWorkflows --- src/libs/WorkflowUtils.ts | 42 ++++++++++----------- tests/unit/WorkflowUtilsTest.ts | 67 ++++++++++++++++++++++++++++++--- 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 5d5bb5c10ee9..dbe5afc78a0b 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -102,27 +102,27 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, approvalWorkflows[submitsTo].members.push(member); }); - // Add isInMultipleWorkflows to each approver - Object.values(approvalWorkflows).forEach((workflow) => - workflow.approvers.map((approver) => ({ - ...approver, - isInMultipleWorkflows: approverCountsByEmail[approver.email] > 1, - })), - ); - - const sortedApprovalWorkflows = Object.values(approvalWorkflows).sort((a, b) => { - if (a.isDefault) { - return -1; - } - - if (b.isDefault) { - return 1; - } - - const aDisplayName = b.approvers.at(0)?.displayName ?? '-1'; - const bDisplayName = a.approvers.at(0)?.displayName ?? '-1'; - return aDisplayName < bDisplayName ? 1 : -1; - }); + const sortedApprovalWorkflows = Object.values(approvalWorkflows) + .map((workflow) => ({ + ...workflow, + approvers: workflow.approvers.map((approver) => ({ + ...approver, + isInMultipleWorkflows: approverCountsByEmail[approver.email] > 1, + })), + })) + .sort((a, b) => { + if (a.isDefault) { + return -1; + } + + if (b.isDefault) { + return 1; + } + + const aDisplayName = b.approvers.at(0)?.displayName ?? '-1'; + const bDisplayName = a.approvers.at(0)?.displayName ?? '-1'; + return aDisplayName < bDisplayName ? 1 : -1; + }); return sortedApprovalWorkflows; } diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 0379fa967604..222a5019f330 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -325,34 +325,89 @@ describe('WorkflowUtils', () => { expect(workflows).toEqual([buildWorkflow([3, 2], [1], {isDefault: true}), buildWorkflow([5], [3]), buildWorkflow([4, 1], [4])]); }); - it('Should mark users that are used in multiple workflows', () => { + it('Should mark approvers that are used in multiple workflows', () => { const employees: PolicyEmployeeList = { '1@example.com': { email: '1@example.com', - forwardsTo: undefined, - submitsTo: '4@example.com', + forwardsTo: '3@example.com', + submitsTo: '2@example.com', }, '2@example.com': { email: '2@example.com', - forwardsTo: undefined, + forwardsTo: '3@example.com', submitsTo: '1@example.com', }, '3@example.com': { email: '3@example.com', - forwardsTo: undefined, + forwardsTo: '4@example.com', submitsTo: '1@example.com', }, '4@example.com': { email: '4@example.com', forwardsTo: undefined, + submitsTo: '1@example.com', + }, + }; + const defaultApprover = '1@example.com'; + + const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); + + const defaultWorkflow = buildWorkflow([2, 3, 4], [1, 3, 4], {isDefault: true}); + defaultWorkflow.approvers[0].forwardsTo = '3@example.com'; + defaultWorkflow.approvers[1].forwardsTo = '4@example.com'; + defaultWorkflow.approvers[1].isInMultipleWorkflows = true; + defaultWorkflow.approvers[2].isInMultipleWorkflows = true; + const secondWorkflow = buildWorkflow([1], [2, 3, 4]); + secondWorkflow.approvers[0].forwardsTo = '3@example.com'; + secondWorkflow.approvers[1].forwardsTo = '4@example.com'; + secondWorkflow.approvers[1].isInMultipleWorkflows = true; + secondWorkflow.approvers[2].isInMultipleWorkflows = true; + + expect(workflows).toEqual([defaultWorkflow, secondWorkflow]); + }); + + it('Should build multiple workflows with many approvers', () => { + const employees: PolicyEmployeeList = { + '1@example.com': { + email: '1@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, + '2@example.com': { + email: '2@example.com', + forwardsTo: undefined, submitsTo: '4@example.com', }, + '3@example.com': { + email: '3@example.com', + forwardsTo: undefined, + submitsTo: '4@example.com', + }, + '4@example.com': { + email: '4@example.com', + forwardsTo: '5@example.com', + submitsTo: '1@example.com', + }, + '5@example.com': { + email: '5@example.com', + forwardsTo: '6@example.com', + submitsTo: '1@example.com', + }, + '6@example.com': { + email: '6@example.com', + forwardsTo: undefined, + submitsTo: '1@example.com', + }, }; const defaultApprover = '1@example.com'; const workflows = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails}); - expect(workflows).toEqual([buildWorkflow([2, 3], [1], {isDefault: true}), buildWorkflow([1, 4], [4])]); + const defaultWorkflow = buildWorkflow([1, 4, 5, 6], [1], {isDefault: true}); + const secondWorkflow = buildWorkflow([2, 3], [4, 5, 6]); + secondWorkflow.approvers[0].forwardsTo = '5@example.com'; + secondWorkflow.approvers[1].forwardsTo = '6@example.com'; + expect(workflows).toEqual([defaultWorkflow, secondWorkflow]); }); }); }); From 0584034ea6556415eeb024200e881ff4f1a7995e Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 21:43:13 +0200 Subject: [PATCH 08/20] Improve and write tests for convertApprovalWorkflowToPolicyEmployees --- src/libs/WorkflowUtils.ts | 37 ++++++++------ tests/unit/WorkflowUtilsTest.ts | 88 +++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index dbe5afc78a0b..3d5903ba948a 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -127,33 +127,40 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, return sortedApprovalWorkflows; } +type ConvertApprovalWorkflowToPolicyEmployeesParams = { + approvalWorkflow: ApprovalWorkflow; + employeeList: PolicyEmployeeList; +}; + /** Convert an approval workflow to a list of policy employees */ -function convertApprovalWorkflowToPolicyEmployees(approvalWorkflow: ApprovalWorkflow, employeeList: PolicyEmployeeList): PolicyEmployeeList { - const employees: PolicyEmployeeList = {}; +function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList}: ConvertApprovalWorkflowToPolicyEmployeesParams): PolicyEmployeeList { + const updatedEmployeeList: PolicyEmployeeList = {}; const firstApprover = approvalWorkflow.approvers.at(0); if (!firstApprover) { throw new Error('Approval workflow must have at least one approver'); } - approvalWorkflow.members.forEach(({email}) => { - employees[email] = { - ...employeeList[email], - submitsTo: firstApprover.email, + approvalWorkflow.approvers.forEach((approver, index) => { + if (updatedEmployeeList[approver.email]) { + return; + } + + const nextApprover = approvalWorkflow.approvers.at(index + 1); + updatedEmployeeList[approver.email] = { + ...employeeList[approver.email], + forwardsTo: nextApprover?.email, }; }); - approvalWorkflow.approvers.forEach((approver, index) => { - const nextApprover = approvalWorkflow.approvers[index + 1]; - if (nextApprover) { - employees[approver.email] = { - ...employeeList[approver.email], - forwardsTo: nextApprover.email, - }; - } + approvalWorkflow.members.forEach(({email}) => { + updatedEmployeeList[email] = { + ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : employeeList[email]), + submitsTo: firstApprover.email, + }; }); - return employees; + return updatedEmployeeList; } export {getApprovalWorkflowApprovers, convertPolicyEmployeesToApprovalWorkflows, convertApprovalWorkflowToPolicyEmployees}; diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 222a5019f330..14ded5101850 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -4,11 +4,20 @@ import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; +import type PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import * as TestHelper from '../utils/TestHelper'; const personalDetails: PersonalDetailsList = {}; const personalDetailsByEmail: PersonalDetailsList = {}; +function buildPolicyEmployee(accountID: number, policyEmployee: Partial = {}): PolicyEmployee { + return { + email: `${accountID}@example.com`, + role: 'user', + ...policyEmployee, + }; +} + function buildMember(accountID: number): Member { return { email: `${accountID}@example.com`, @@ -410,4 +419,83 @@ describe('WorkflowUtils', () => { expect(workflows).toEqual([defaultWorkflow, secondWorkflow]); }); }); + + describe('convertApprovalWorkflowToPolicyEmployees', () => { + it('Should return an updated employee list for a simple default workflow', () => { + const approvalWorkflow: ApprovalWorkflow = { + members: [buildMember(1), buildMember(2)], + approvers: [buildApprover(1)], + isDefault: true, + isBeingEdited: false, + }; + const employeeList: PolicyEmployeeList = { + '1@example.com': buildPolicyEmployee(1, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + }; + + const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList}); + + expect(convertedEmployees).toEqual({ + '1@example.com': buildPolicyEmployee(1, {forwardsTo: undefined, submitsTo: '1@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com', role: 'admin'}), + }); + }); + + it('Should return an updated employee list for a complex workflow', () => { + const approvalWorkflow: ApprovalWorkflow = { + members: [buildMember(4), buildMember(5), buildMember(6)], + approvers: [buildApprover(1, {forwardsTo: '2@example.com'}), buildApprover(2, {forwardsTo: '2@example.com'}), buildApprover(3)], + isDefault: false, + isBeingEdited: false, + }; + const employeeList: PolicyEmployeeList = { + '1@example.com': buildPolicyEmployee(1, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '5@example.com': buildPolicyEmployee(5, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '6@example.com': buildPolicyEmployee(6, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + }; + + const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList}); + + expect(convertedEmployees).toEqual({ + '1@example.com': buildPolicyEmployee(1, {forwardsTo: '2@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: '3@example.com', submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: undefined, submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com'}), + '5@example.com': buildPolicyEmployee(5, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com'}), + '6@example.com': buildPolicyEmployee(6, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com'}), + }); + }); + + it('Should return an updated employee list for a workflow with a circular reference', () => { + const approvalWorkflow: ApprovalWorkflow = { + members: [buildMember(4)], + approvers: [ + buildApprover(1, {forwardsTo: '2@example.com'}), + buildApprover(2, {forwardsTo: '2@example.com'}), + buildApprover(3, {forwardsTo: '1@example.com'}), + buildApprover(1, {forwardsTo: '2@example.com'}), + ], + isDefault: false, + isBeingEdited: false, + }; + const employeeList: PolicyEmployeeList = { + '1@example.com': buildPolicyEmployee(1, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + }; + + const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList}); + + expect(convertedEmployees).toEqual({ + '1@example.com': buildPolicyEmployee(1, {forwardsTo: '2@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: '3@example.com', submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: '1@example.com', submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com'}), + }); + }); + }); }); From 142a12d4d5db57dcafa17cbf978f7970287b998f Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 24 Jul 2024 22:04:57 +0200 Subject: [PATCH 09/20] Add useful comments and simplify the logic --- src/libs/WorkflowUtils.ts | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 3d5903ba948a..2028652f8e52 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -81,6 +81,7 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, const approverCountsByEmail: Record = {}; const personalDetailsByEmail = lodashMapKeys(personalDetails, (value, key) => value?.login ?? key); + // Add each employee to the appropriate workflow Object.values(employees).forEach((employee) => { const {email, submitsTo} = employee; if (!email || !submitsTo) { @@ -102,29 +103,23 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, approvalWorkflows[submitsTo].members.push(member); }); - const sortedApprovalWorkflows = Object.values(approvalWorkflows) - .map((workflow) => ({ - ...workflow, - approvers: workflow.approvers.map((approver) => ({ - ...approver, - isInMultipleWorkflows: approverCountsByEmail[approver.email] > 1, - })), - })) - .sort((a, b) => { - if (a.isDefault) { - return -1; - } - - if (b.isDefault) { - return 1; - } - - const aDisplayName = b.approvers.at(0)?.displayName ?? '-1'; - const bDisplayName = a.approvers.at(0)?.displayName ?? '-1'; - return aDisplayName < bDisplayName ? 1 : -1; - }); + // Sort the workflows by the first approver's name (default workflow has priority) + const sortedApprovalWorkflows = Object.values(approvalWorkflows).sort((a, b) => { + if (a.isDefault) { + return -1; + } + + return (a.approvers[0]?.displayName ?? '-1').localeCompare(b.approvers[0]?.displayName ?? '-1'); + }); - return sortedApprovalWorkflows; + // Add a flag to each approver to indicate if they are in multiple workflows + return sortedApprovalWorkflows.map((workflow) => ({ + ...workflow, + approvers: workflow.approvers.map((approver) => ({ + ...approver, + isInMultipleWorkflows: approverCountsByEmail[approver.email] > 1, + })), + })); } type ConvertApprovalWorkflowToPolicyEmployeesParams = { From 8eea2b37372c170548a1d316faa132bd4fee438f Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 15:10:02 +0200 Subject: [PATCH 10/20] Add new onyx key for approval workflows --- src/ONYXKEYS.ts | 4 ++++ src/types/onyx/index.ts | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index 00f37508612d..4bf7987f9ceb 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -387,6 +387,9 @@ const ONYXKEYS = { NVP_PRIVATE_CANCELLATION_DETAILS: 'nvp_private_cancellationDetails', + /** Stores the information about currently edited advanced approval workflow */ + APPROVAL_WORKFLOW: 'approvalWorkflow', + /** Collection Keys */ COLLECTION: { DOWNLOAD: 'download_', @@ -856,6 +859,7 @@ type OnyxValuesMapping = { [ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED]: number; [ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END]: number; [ONYXKEYS.NVP_PRIVATE_CANCELLATION_DETAILS]: OnyxTypes.CancellationDetails[]; + [ONYXKEYS.APPROVAL_WORKFLOW]: OnyxTypes.ApprovalWorkflow; }; type OnyxValues = OnyxValuesMapping & OnyxCollectionValuesMapping & OnyxFormValuesMapping & OnyxFormDraftValuesMapping; diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts index 8f1a6f43dd4a..7c68211f0621 100644 --- a/src/types/onyx/index.ts +++ b/src/types/onyx/index.ts @@ -1,5 +1,6 @@ import type Account from './Account'; import type AccountData from './AccountData'; +import type ApprovalWorkflow from './ApprovalWorkflow'; import type {BankAccountList} from './BankAccount'; import type BankAccount from './BankAccount'; import type Beta from './Beta'; @@ -214,4 +215,5 @@ export type { StripeCustomerID, BillingStatus, CancellationDetails, + ApprovalWorkflow, }; From e4d750ca4f6107323fcb32484220361407e06f18 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 15:10:25 +0200 Subject: [PATCH 11/20] Add API commands for approval workflows --- .../API/parameters/WorkspaceApprovalParams.ts | 21 +++++++++++++++++++ src/libs/API/parameters/index.ts | 1 + src/libs/API/types.ts | 6 ++++++ 3 files changed, 28 insertions(+) create mode 100644 src/libs/API/parameters/WorkspaceApprovalParams.ts diff --git a/src/libs/API/parameters/WorkspaceApprovalParams.ts b/src/libs/API/parameters/WorkspaceApprovalParams.ts new file mode 100644 index 000000000000..97cca1060a63 --- /dev/null +++ b/src/libs/API/parameters/WorkspaceApprovalParams.ts @@ -0,0 +1,21 @@ +import type {PolicyEmployee} from '@src/types/onyx'; + +type CreateWorkspaceApprovalParams = { + authToken: string; + policyID: string; + employees: PolicyEmployee[]; +}; + +type UpdateWorkspaceApprovalParams = { + authToken: string; + policyID: string; + employees: PolicyEmployee[]; +}; + +type RemoveWorkspaceApprovalParams = { + authToken: string; + policyID: string; + employees: PolicyEmployee[]; +}; + +export type {CreateWorkspaceApprovalParams, UpdateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams}; diff --git a/src/libs/API/parameters/index.ts b/src/libs/API/parameters/index.ts index 997eb415a848..e16691c992f2 100644 --- a/src/libs/API/parameters/index.ts +++ b/src/libs/API/parameters/index.ts @@ -267,3 +267,4 @@ export type {default as UpdateNetSuiteCustomersJobsParams} from './UpdateNetSuit export type {default as CopyExistingPolicyConnectionParams} from './CopyExistingPolicyConnectionParams'; export type {default as ExportSearchItemsToCSVParams} from './ExportSearchItemsToCSVParams'; export type {default as UpdateExpensifyCardLimitParams} from './UpdateExpensifyCardLimitParams'; +export type {CreateWorkspaceApprovalParams, UpdateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams} from './WorkspaceApprovalParams'; diff --git a/src/libs/API/types.ts b/src/libs/API/types.ts index 70a0e91aba10..c4218b44f165 100644 --- a/src/libs/API/types.ts +++ b/src/libs/API/types.ts @@ -319,6 +319,9 @@ const WRITE_COMMANDS = { UPDATE_SAGE_INTACCT_NON_REIMBURSABLE_EXPENSES_EXPORT_ACCOUNT: 'UpdateSageIntacctNonreimbursableExpensesExportAccount', UPDATE_SAGE_INTACCT_NON_REIMBURSABLE_EXPENSES_EXPORT_VENDOR: 'UpdateSageIntacctNonreimbursableExpensesExportVendor', EXPORT_SEARCH_ITEMS_TO_CSV: 'ExportSearchToCSV', + CREATE_WORKSPACE_APPROVAL: 'CreateWorkspaceApproval', + UPDATE_WORKSPACE_APPROVAL: 'UpdateWorkspaceApproval', + REMOVE_WORKSPACE_APPROVAL: 'RemoveWorkspaceApproval', } as const; type WriteCommand = ValueOf; @@ -644,6 +647,9 @@ type WriteCommandParameters = { [WRITE_COMMANDS.UPDATE_SAGE_INTACCT_SYNC_TAX_CONFIGURATION]: Parameters.UpdateSageIntacctGenericTypeParams<'enabled', boolean>; [WRITE_COMMANDS.UPDATE_SAGE_INTACCT_USER_DIMENSION]: Parameters.UpdateSageIntacctGenericTypeParams<'dimensions', string>; [WRITE_COMMANDS.EXPORT_SEARCH_ITEMS_TO_CSV]: Parameters.ExportSearchItemsToCSVParams; + [WRITE_COMMANDS.CREATE_WORKSPACE_APPROVAL]: Parameters.CreateWorkspaceApprovalParams; + [WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL]: Parameters.UpdateWorkspaceApprovalParams; + [WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL]: Parameters.RemoveWorkspaceApprovalParams; }; const READ_COMMANDS = { From b971bc8b1e67b16255d80484dfbe397fd7f3209a Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 15:10:47 +0200 Subject: [PATCH 12/20] Add Onyx actions for approval workflows --- src/libs/actions/Workflow.ts | 196 +++++++++++++++++++++++++++++ src/types/onyx/ApprovalWorkflow.ts | 3 + 2 files changed, 199 insertions(+) create mode 100644 src/libs/actions/Workflow.ts diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts new file mode 100644 index 000000000000..9f4809696913 --- /dev/null +++ b/src/libs/actions/Workflow.ts @@ -0,0 +1,196 @@ +import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; +import * as API from '@libs/API'; +import type {CreateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams, UpdateWorkspaceApprovalParams} from '@libs/API/parameters'; +import {WRITE_COMMANDS} from '@libs/API/types'; +import * as NetworkStore from '@libs/Network/NetworkStore'; +import {convertApprovalWorkflowToPolicyEmployees} from '@libs/WorkflowUtils'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {ApprovalWorkflow, Policy} from '@src/types/onyx'; +import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; + +let currentApprovalWorkflow: ApprovalWorkflow | undefined; +Onyx.connect({ + key: ONYXKEYS.APPROVAL_WORKFLOW, + callback: (approvalWorkflow) => { + currentApprovalWorkflow = approvalWorkflow; + }, +}); + +let allPolicies: OnyxCollection; +Onyx.connect({ + key: ONYXKEYS.COLLECTION.POLICY, + waitForCollectionCallback: true, + callback: (value) => (allPolicies = value), +}); + +function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { + const authToken = NetworkStore.getAuthToken(); + if (!authToken) { + return; + } + + const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; + const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); + + const optimisticData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: { + isLoading: true, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList}, + }, + ]; + + const failureData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: {...approvalWorkflow, isLoading: false}, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList: previousEmployeeList}, + }, + ]; + + const successData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.SET, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: null, + }, + ]; + + const parameters: CreateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + API.write(WRITE_COMMANDS.CREATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); +} + +function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { + const authToken = NetworkStore.getAuthToken(); + if (!authToken) { + return; + } + + const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; + const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); + + const optimisticData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: { + isLoading: true, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList}, + }, + ]; + + const failureData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: {...approvalWorkflow, isLoading: false}, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList: previousEmployeeList}, + }, + ]; + + const successData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.SET, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: null, + }, + ]; + + const parameters: UpdateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); +} + +function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { + const authToken = NetworkStore.getAuthToken(); + if (!authToken) { + return; + } + + const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; + const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList, removeWorkflow: true}); + + const optimisticData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: { + isLoading: true, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList}, + }, + ]; + + const failureData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: {...approvalWorkflow, isLoading: false}, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, + value: {employeeList: previousEmployeeList}, + }, + ]; + + const successData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.SET, + key: ONYXKEYS.APPROVAL_WORKFLOW, + value: null, + }, + ]; + + const parameters: RemoveWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + API.write(WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); +} + +function setApprovalWorkflowMembers(members: Member[]) { + Onyx.merge(ONYXKEYS.APPROVAL_WORKFLOW, {members}); +} + +function setApprovalWorkflowApprover(approver: Approver, index: number) { + if (!currentApprovalWorkflow) { + return; + } + + const updatedApprovers = [...currentApprovalWorkflow.approvers]; + updatedApprovers[index] = approver; + Onyx.merge(ONYXKEYS.APPROVAL_WORKFLOW, {approvers: updatedApprovers}); +} + +function setApprovalWorkflow(approvalWorkflow: ApprovalWorkflow) { + Onyx.merge(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); +} + +function clearApprovalWorkflow() { + Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, null); +} + +export {createApprovalWorkflow, updateApprovalWorkflow, removeApprovalWorkflow, setApprovalWorkflowMembers, setApprovalWorkflowApprover, setApprovalWorkflow, clearApprovalWorkflow}; diff --git a/src/types/onyx/ApprovalWorkflow.ts b/src/types/onyx/ApprovalWorkflow.ts index 929876cd0b75..1bc3d2f44306 100644 --- a/src/types/onyx/ApprovalWorkflow.ts +++ b/src/types/onyx/ApprovalWorkflow.ts @@ -78,6 +78,9 @@ type ApprovalWorkflow = { * Is this workflow being edited vs created */ isBeingEdited: boolean; + + /** Whether we are waiting for the API action to complete */ + isLoading?: boolean; }; export default ApprovalWorkflow; From f90748c6387e880bf1764bf2e8658f6c74aa26a9 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 15:11:23 +0200 Subject: [PATCH 13/20] Add removeWorkflow mode to convertApprovalWorkflowToPolicyEmployees --- src/libs/WorkflowUtils.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 2028652f8e52..a513e006f8fe 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -125,10 +125,11 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, type ConvertApprovalWorkflowToPolicyEmployeesParams = { approvalWorkflow: ApprovalWorkflow; employeeList: PolicyEmployeeList; + removeWorkflow?: boolean; }; /** Convert an approval workflow to a list of policy employees */ -function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList}: ConvertApprovalWorkflowToPolicyEmployeesParams): PolicyEmployeeList { +function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList, removeWorkflow = false}: ConvertApprovalWorkflowToPolicyEmployeesParams): PolicyEmployeeList { const updatedEmployeeList: PolicyEmployeeList = {}; const firstApprover = approvalWorkflow.approvers.at(0); @@ -144,14 +145,14 @@ function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeLis const nextApprover = approvalWorkflow.approvers.at(index + 1); updatedEmployeeList[approver.email] = { ...employeeList[approver.email], - forwardsTo: nextApprover?.email, + forwardsTo: removeWorkflow ? undefined : nextApprover?.email, }; }); approvalWorkflow.members.forEach(({email}) => { updatedEmployeeList[email] = { ...(updatedEmployeeList[email] ? updatedEmployeeList[email] : employeeList[email]), - submitsTo: firstApprover.email, + submitsTo: removeWorkflow ? undefined : firstApprover.email, }; }); From e0341b44a6e92b514e6fbe36c46fc51ab402364f Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 15:13:09 +0200 Subject: [PATCH 14/20] Add test for removeWorkflow option --- tests/unit/WorkflowUtilsTest.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 14ded5101850..bfe8d8da2c02 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -497,5 +497,33 @@ describe('WorkflowUtils', () => { '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: '1@example.com'}), }); }); + + it('Should return an updated employee list for a complex workflow when removing', () => { + const approvalWorkflow: ApprovalWorkflow = { + members: [buildMember(4), buildMember(5), buildMember(6)], + approvers: [buildApprover(1, {forwardsTo: '2@example.com'}), buildApprover(2, {forwardsTo: '2@example.com'}), buildApprover(3)], + isDefault: false, + isBeingEdited: false, + }; + const employeeList: PolicyEmployeeList = { + '1@example.com': buildPolicyEmployee(1, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '5@example.com': buildPolicyEmployee(5, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + '6@example.com': buildPolicyEmployee(6, {forwardsTo: 'previous@example.com', submitsTo: 'previous@example.com'}), + }; + + const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList, removeWorkflow: true}); + + expect(convertedEmployees).toEqual({ + '1@example.com': buildPolicyEmployee(1, {forwardsTo: undefined, submitsTo: 'previous@example.com', role: 'admin'}), + '2@example.com': buildPolicyEmployee(2, {forwardsTo: undefined, submitsTo: 'previous@example.com'}), + '3@example.com': buildPolicyEmployee(3, {forwardsTo: undefined, submitsTo: 'previous@example.com'}), + '4@example.com': buildPolicyEmployee(4, {forwardsTo: 'previous@example.com', submitsTo: undefined}), + '5@example.com': buildPolicyEmployee(5, {forwardsTo: 'previous@example.com', submitsTo: undefined}), + '6@example.com': buildPolicyEmployee(6, {forwardsTo: 'previous@example.com', submitsTo: undefined}), + }); + }); }); }); From de18619bbf1516cd7f727e116e1cb4d69e5ca308 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Thu, 25 Jul 2024 17:09:41 +0200 Subject: [PATCH 15/20] Remove deuplicated test --- tests/unit/WorkflowUtilsTest.ts | 43 +-------------------------------- 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index bfe8d8da2c02..1ee6642e27b8 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -100,48 +100,7 @@ describe('WorkflowUtils', () => { expect(approvers).toEqual([buildApprover(1)]); }); - it('Should return a list of approver when there are forwardsTo', () => { - const employees: PolicyEmployeeList = { - '1@example.com': { - email: '1@example.com', - forwardsTo: '2@example.com', - }, - '2@example.com': { - email: '2@example.com', - forwardsTo: '3@example.com', - }, - '3@example.com': { - email: '3@example.com', - forwardsTo: '4@example.com', - }, - '4@example.com': { - email: '4@example.com', - forwardsTo: undefined, - }, - '5@example.com': { - email: '5@example.com', - forwardsTo: undefined, - }, - }; - - expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '1@example.com', personalDetailsByEmail})).toEqual([ - buildApprover(1, {forwardsTo: '2@example.com'}), - buildApprover(2, {forwardsTo: '3@example.com'}), - buildApprover(3, {forwardsTo: '4@example.com'}), - buildApprover(4), - ]); - expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '2@example.com', personalDetailsByEmail})).toEqual([ - buildApprover(2, {forwardsTo: '3@example.com'}), - buildApprover(3, {forwardsTo: '4@example.com'}), - buildApprover(4), - ]); - expect(WorkflowUtils.getApprovalWorkflowApprovers({employees, firstEmail: '3@example.com', personalDetailsByEmail})).toEqual([ - buildApprover(3, {forwardsTo: '4@example.com'}), - buildApprover(4), - ]); - }); - - it('Should return a list of approver when there are forwardsTo', () => { + it('Should return a list of approvers when forwardsTo is defined', () => { const employees: PolicyEmployeeList = { '1@example.com': { email: '1@example.com', From a23e4fe0a5b622a9ab1fdc61811d1865d0f541fb Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Fri, 26 Jul 2024 08:39:19 +0200 Subject: [PATCH 16/20] Update the comments according to feedback --- src/libs/WorkflowUtils.ts | 11 +++++++++++ src/types/onyx/ApprovalWorkflow.ts | 15 ++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index a513e006f8fe..6e8fc3322da0 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -123,8 +123,19 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, } type ConvertApprovalWorkflowToPolicyEmployeesParams = { + /** + * Approval workflow to convert + */ approvalWorkflow: ApprovalWorkflow; + + /** + * Current list of employees in the policy + */ employeeList: PolicyEmployeeList; + + /** + * Should the workflow be removed from the employees + */ removeWorkflow?: boolean; }; diff --git a/src/types/onyx/ApprovalWorkflow.ts b/src/types/onyx/ApprovalWorkflow.ts index 929876cd0b75..018996000540 100644 --- a/src/types/onyx/ApprovalWorkflow.ts +++ b/src/types/onyx/ApprovalWorkflow.ts @@ -25,18 +25,21 @@ type Approver = { displayName?: string; /** - * Is this approver in more than one workflow + * Is this user used as an approver in more than one workflow (used to show a warning) */ isInMultipleWorkflows: boolean; /** - * Is this approver in a circular reference + * Is this approver in a circular reference (approver forwards to themselves, or a cycle of forwards) + * + * example: A -> A (self forwards) + * example: A -> B -> C -> A (cycle) */ isCircularReference?: boolean; }; /** - * + * Member in the approval workflow */ type Member = { /** @@ -65,12 +68,14 @@ type ApprovalWorkflow = { members: Member[]; /** - * List of approvers in the workflow + * List of approvers in the workflow (the order of approvers in this array is important) + * + * The first approver in the array is the first approver in the workflow, next approver is the one they forward to, etc. */ approvers: Approver[]; /** - * Is this the default workflow + * Is this the default workflow for the policy (first approver of this workflow is the same as the policy's default approver) */ isDefault: boolean; From 7e9f4cec743d08ff4556919d293a91f5a4fb5db4 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Fri, 26 Jul 2024 08:47:01 +0200 Subject: [PATCH 17/20] DRY WorkspaceApprovalParams --- src/libs/API/parameters/WorkspaceApprovalParams.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/libs/API/parameters/WorkspaceApprovalParams.ts b/src/libs/API/parameters/WorkspaceApprovalParams.ts index 97cca1060a63..67f96b7852e7 100644 --- a/src/libs/API/parameters/WorkspaceApprovalParams.ts +++ b/src/libs/API/parameters/WorkspaceApprovalParams.ts @@ -6,16 +6,8 @@ type CreateWorkspaceApprovalParams = { employees: PolicyEmployee[]; }; -type UpdateWorkspaceApprovalParams = { - authToken: string; - policyID: string; - employees: PolicyEmployee[]; -}; +type UpdateWorkspaceApprovalParams = CreateWorkspaceApprovalParams; -type RemoveWorkspaceApprovalParams = { - authToken: string; - policyID: string; - employees: PolicyEmployee[]; -}; +type RemoveWorkspaceApprovalParams = CreateWorkspaceApprovalParams; export type {CreateWorkspaceApprovalParams, UpdateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams}; From 7617d0c2f2750748adec36e1d31ed7c8af5d4689 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Fri, 26 Jul 2024 08:47:31 +0200 Subject: [PATCH 18/20] Connect to authToken with Onyx.connect --- src/libs/actions/Workflow.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 9f4809696913..3b8f08570494 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -3,7 +3,6 @@ import Onyx from 'react-native-onyx'; import * as API from '@libs/API'; import type {CreateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams, UpdateWorkspaceApprovalParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; -import * as NetworkStore from '@libs/Network/NetworkStore'; import {convertApprovalWorkflowToPolicyEmployees} from '@libs/WorkflowUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ApprovalWorkflow, Policy} from '@src/types/onyx'; @@ -24,8 +23,15 @@ Onyx.connect({ callback: (value) => (allPolicies = value), }); +let authToken: string | undefined; +Onyx.connect({ + key: ONYXKEYS.SESSION, + callback: (value) => { + authToken = value?.authToken; + }, +}); + function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - const authToken = NetworkStore.getAuthToken(); if (!authToken) { return; } @@ -74,7 +80,6 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork } function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - const authToken = NetworkStore.getAuthToken(); if (!authToken) { return; } @@ -123,7 +128,6 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork } function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - const authToken = NetworkStore.getAuthToken(); if (!authToken) { return; } From 9fdedc734aa9d9861315ca441e1bdc2932f48a61 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Fri, 26 Jul 2024 09:30:43 +0200 Subject: [PATCH 19/20] Update the logic to update approvalMode --- src/libs/actions/Workflow.ts | 58 +++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 3b8f08570494..fe336c3d51bd 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -4,6 +4,7 @@ import * as API from '@libs/API'; import type {CreateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams, UpdateWorkspaceApprovalParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; import {convertApprovalWorkflowToPolicyEmployees} from '@libs/WorkflowUtils'; +import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ApprovalWorkflow, Policy} from '@src/types/onyx'; import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; @@ -32,12 +33,15 @@ Onyx.connect({ }); function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - if (!authToken) { + const policy = allPolicies?.[policyID]; + + if (!authToken || !policy) { return; } - const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; - const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); + const previousEmployeeList = {...policy.employeeList}; + const previousApprovalMode = policy.approvalMode; + const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); const optimisticData: OnyxUpdate[] = [ { @@ -50,7 +54,10 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: {employeeList}, + value: { + employeeList: updatedEmployees, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }, }, ]; @@ -63,7 +70,10 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: {employeeList: previousEmployeeList}, + value: { + employeeList: previousEmployeeList, + approvalMode: previousApprovalMode, + }, }, ]; @@ -75,17 +85,19 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork }, ]; - const parameters: CreateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + const parameters: CreateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(updatedEmployees)}; API.write(WRITE_COMMANDS.CREATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - if (!authToken) { + const policy = allPolicies?.[policyID]; + + if (!authToken || !policy) { return; } - const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; - const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); + const previousEmployeeList = {...policy.employeeList}; + const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList}); const optimisticData: OnyxUpdate[] = [ { @@ -98,7 +110,7 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: {employeeList}, + value: {employeeList: updatedEmployees}, }, ]; @@ -123,17 +135,23 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork }, ]; - const parameters: UpdateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + const parameters: UpdateWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(updatedEmployees)}; API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWorkflow) { - if (!authToken) { + const policy = allPolicies?.[policyID]; + + if (!authToken || !policy) { return; } - const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList}; - const employeeList = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList, removeWorkflow: true}); + const previousEmployeeList = {...policy.employeeList}; + const updatedEmployees = convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList: previousEmployeeList, removeWorkflow: true}); + const updatedEmployeeList = {...previousEmployeeList, ...updatedEmployees}; + + // If there is more than one workflow, we need to keep the advanced approval mode (first workflow is the default) + const hasMoreThanOneWorkflow = Object.values(updatedEmployeeList).some((employee) => !!employee.submitsTo && employee.submitsTo !== policy.approver); const optimisticData: OnyxUpdate[] = [ { @@ -146,7 +164,10 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: {employeeList}, + value: { + employeeList: updatedEmployees, + approvalMode: hasMoreThanOneWorkflow ? CONST.POLICY.APPROVAL_MODE.ADVANCED : CONST.POLICY.APPROVAL_MODE.BASIC, + }, }, ]; @@ -159,7 +180,10 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - value: {employeeList: previousEmployeeList}, + value: { + employeeList: previousEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }, }, ]; @@ -171,7 +195,7 @@ function removeApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork }, ]; - const parameters: RemoveWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(employeeList)}; + const parameters: RemoveWorkspaceApprovalParams = {policyID, authToken, employees: Object.values(updatedEmployees)}; API.write(WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL, parameters, {optimisticData, failureData, successData}); } From 7b43485abb0bb8db08d4912b6bc4e03eef583155 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Fri, 26 Jul 2024 11:17:32 +0200 Subject: [PATCH 20/20] Use the correct plural form --- tests/unit/WorkflowUtilsTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 1ee6642e27b8..6359ba01ed23 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -141,7 +141,7 @@ describe('WorkflowUtils', () => { ]); }); - it('Should return a list of approver with circular references', () => { + it('Should return a list of approvers with circular references', () => { const employees: PolicyEmployeeList = { '1@example.com': { email: '1@example.com', @@ -183,7 +183,7 @@ describe('WorkflowUtils', () => { ]); }); - it('Should return a list of approver with circular references', () => { + it('Should return a list of approvers with circular references', () => { const employees: PolicyEmployeeList = { '1@example.com': { email: '1@example.com',