Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA][CRITICAL] [Advanced Approval Workflows] Implement utils that transform data between backend and frontend #46168

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 173 additions & 0 deletions src/libs/WorkflowUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
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<string>();

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<string, ApprovalWorkflow> = {};

// Keep track of how many times each approver is used to detect approvers in multiple workflows
const approverCountsByEmail: Record<string, number> = {};
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) {
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);
});

// 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');
});

// 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 = {
/**
* Approval workflow to convert
*/
approvalWorkflow: ApprovalWorkflow;
blazejkustra marked this conversation as resolved.
Show resolved Hide resolved

/**
* Current list of employees in the policy
*/
employeeList: PolicyEmployeeList;

/**
* Should the workflow be removed from the employees
*/
removeWorkflow?: boolean;
};

/** Convert an approval workflow to a list of policy employees */
function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList, removeWorkflow = false}: ConvertApprovalWorkflowToPolicyEmployeesParams): PolicyEmployeeList {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the result of this method is in the wrong format, but it's close! There are two formats for policy employees:

  • What is stored in Onyx only used on the frontend (an object keyed by email):
{
    '1@example.com': {email: '1@example.com'},
}
  • What is stored in the database and the format that the backend users (an array of objects):
[
    {email: '1@example.com'},
]

I believe this method needs to produce the second format because that is the format that the backend API will be expecting. You can see this being done in the API parameters for updating a workspace members role (this endpoint is also a direct alias for Policy_Employees_Merge command for OldDot.

Shouldn't be a big deal to change this and then update the tests to match for format. I don't think we need to support converting workflows into the first employeeList format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see in your second PR that you will need to use the first format for the optimistic data here: https://github.com/Expensify/App/pull/46189/files#diff-0322d176e073c6db8f9e2a5203bc59580b8e4cda517da2ccddf6d53318789ff3R45-R47

So it needs to support both formats.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or... I could also change the backend to accept the employeeList in the same format that the frontend is using 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think converting to an array it's not a big deal (one call to Object.values()). Therefore, I don't think it's necessary to change the format on the backend.

const updatedEmployeeList: PolicyEmployeeList = {};
const firstApprover = approvalWorkflow.approvers.at(0);

if (!firstApprover) {
throw new Error('Approval workflow must have at least one approver');
}

approvalWorkflow.approvers.forEach((approver, index) => {
if (updatedEmployeeList[approver.email]) {
return;
}

const nextApprover = approvalWorkflow.approvers.at(index + 1);
updatedEmployeeList[approver.email] = {
...employeeList[approver.email],
forwardsTo: removeWorkflow ? undefined : nextApprover?.email,
};
});

approvalWorkflow.members.forEach(({email}) => {
updatedEmployeeList[email] = {
...(updatedEmployeeList[email] ? updatedEmployeeList[email] : employeeList[email]),
submitsTo: removeWorkflow ? undefined : firstApprover.email,
};
});

return updatedEmployeeList;
}

export {getApprovalWorkflowApprovers, convertPolicyEmployeesToApprovalWorkflows, convertApprovalWorkflowToPolicyEmployees};
89 changes: 89 additions & 0 deletions src/types/onyx/ApprovalWorkflow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
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 user used as an approver in more than one workflow (used to show a warning)
*/
isInMultipleWorkflows: boolean;

/**
* 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 = {
/**
* 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 (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 for the policy (first approver of this workflow is the same as the policy's default approver)
*/
isDefault: boolean;

/**
* Is this workflow being edited vs created
*/
isBeingEdited: boolean;
};

export default ApprovalWorkflow;
export type {Approver, Member};
Loading
Loading