Skip to content

Commit

Permalink
Merge pull request #48086 from software-mansion-labs/approval-workflo…
Browse files Browse the repository at this point in the history 
…ws/fix-offline-v2
  • Loading branch information
@youssef-lr
youssef-lr authored Sep 4, 2024
2 parents 0f26b8a + 361d837 commit 14b8277
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 47 deletions.
41 changes: 21 additions & 20 deletions src/libs/WorkflowUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -205,6 +215,8 @@ function convertApprovalWorkflowToPolicyEmployees({
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);
const forwardsTo = type === CONST.APPROVAL_WORKFLOW.TYPE.REMOVE ? '' : nextApprover?.email ?? '';
Expand All @@ -216,6 +228,7 @@ function convertApprovalWorkflowToPolicyEmployees({
updatedEmployeeList[approver.email] = {
email: approver.email,
forwardsTo,
pendingAction,
};
});

Expand All @@ -226,38 +239,26 @@ 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,
};
});

Expand Down
27 changes: 10 additions & 17 deletions src/libs/actions/Workflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({previousEmployeeList, approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.CREATE});

Expand All @@ -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},
},
},
];
Expand All @@ -87,7 +86,6 @@ function createApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork
value: {
employeeList: previousEmployeeList,
approvalMode: previousApprovalMode,
pendingFields: {employeeList: null},
},
},
];
Expand All @@ -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}])),
},
},
];
Expand All @@ -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({
previousEmployeeList,
approvalWorkflow,
Expand All @@ -129,6 +127,10 @@ function updateApprovalWorkflow(policyID: string, approvalWorkflow: ApprovalWork
approversToRemove,
});

if (isEmptyObject(updatedEmployees) && !newDefaultApprover) {
return;
}

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -142,7 +144,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} : {}),
},
},
Expand Down Expand Up @@ -175,7 +176,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}])),
},
},
];
Expand All @@ -196,7 +197,7 @@ function removeApprovalWorkflow(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 updatedEmployees = convertApprovalWorkflowToPolicyEmployees({previousEmployeeList, approvalWorkflow, type: CONST.APPROVAL_WORKFLOW.TYPE.REMOVE});
const updatedEmployeeList = {...previousEmployeeList, ...updatedEmployees};

Expand All @@ -218,7 +219,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},
},
},
];
Expand All @@ -237,13 +237,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[] = [
Expand All @@ -256,7 +249,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}])),
},
},
];
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function WorkspaceWorkflowsPage({policy, betas, route}: WorkspaceWorkflowsPagePr
<OfflineWithFeedback
// eslint-disable-next-line react/no-array-index-key
key={`workflow-${index}`}
pendingAction={policy?.pendingFields?.employeeList}
pendingAction={workflow.pendingAction}
>
<ApprovalWorkflowSection
approvalWorkflow={workflow}
Expand Down
5 changes: 3 additions & 2 deletions src/types/onyx/ApprovalWorkflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {ValueOf} from 'type-fest';
import type {AvatarSource} from '@libs/UserUtils';
import type CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import type {OnyxValueWithOfflineFeedback} from './OnyxCommon';

/**
* Approver in the approval workflow
Expand Down Expand Up @@ -59,7 +60,7 @@ type Member = {
/**
* Approval workflow for a group of employees
*/
type ApprovalWorkflow = {
type ApprovalWorkflow = OnyxValueWithOfflineFeedback<{
/**
* List of member emails in the workflow
*/
Expand All @@ -76,7 +77,7 @@ type ApprovalWorkflow = {
* Is this the default workflow for the policy (first approver of this workflow is the same as the policy's default approver)
*/
isDefault: boolean;
};
}>;

/**
* Approval workflow for a group of employees with additional properties for the Onyx store
Expand Down
2 changes: 1 addition & 1 deletion src/types/onyx/Policy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,7 @@ type Policy = OnyxCommon.OnyxValueWithOfflineFeedback<
/** Workspace account ID configured for Expensify Card */
workspaceAccountID?: number;
} & Partial<PendingJoinRequestPolicy>,
'addWorkspaceRoom' | 'employeeList' | keyof ACHAccount | keyof Attributes
'addWorkspaceRoom' | keyof ACHAccount | keyof Attributes
>;

/** Stages of policy connection sync */
Expand Down
13 changes: 7 additions & 6 deletions tests/unit/WorkflowUtilsTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const personalDetailsByEmail: PersonalDetailsList = {};
function buildPolicyEmployee(accountID: number, policyEmployee: Partial<PolicyEmployee> = {}): PolicyEmployee {
return {
email: `${accountID}@example.com`,
pendingAction: 'add',
...policyEmployee,
};
}
Expand Down Expand Up @@ -417,12 +418,12 @@ describe('WorkflowUtils', () => {
const convertedEmployees = WorkflowUtils.convertApprovalWorkflowToPolicyEmployees({previousEmployeeList: {}, approvalWorkflow, type: 'remove'});

expect(convertedEmployees).toEqual({
'[email protected]': buildPolicyEmployee(1, {forwardsTo: ''}),
'[email protected]': buildPolicyEmployee(2, {forwardsTo: ''}),
'[email protected]': buildPolicyEmployee(3, {forwardsTo: ''}),
'[email protected]': buildPolicyEmployee(4, {submitsTo: ''}),
'[email protected]': buildPolicyEmployee(5, {submitsTo: ''}),
'[email protected]': buildPolicyEmployee(6, {submitsTo: ''}),
'[email protected]': buildPolicyEmployee(1, {forwardsTo: '', pendingAction: 'update'}),
'[email protected]': buildPolicyEmployee(2, {forwardsTo: '', pendingAction: 'update'}),
'[email protected]': buildPolicyEmployee(3, {forwardsTo: '', pendingAction: 'update'}),
'[email protected]': buildPolicyEmployee(4, {submitsTo: '', pendingAction: 'update'}),
'[email protected]': buildPolicyEmployee(5, {submitsTo: '', pendingAction: 'update'}),
'[email protected]': buildPolicyEmployee(6, {submitsTo: '', pendingAction: 'update'}),
});
});
});
Expand Down

0 comments on commit 14b8277

Please sign in to comment.