Skip to content

49954 approval workflow editing #54178

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

Merged
merged 27 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b79a1f7
update approver remove
huult Dec 13, 2024
8927c7c
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Dec 13, 2024
a554fb0
update utils func
huult Dec 13, 2024
b98fe65
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Dec 16, 2024
c9ec972
Update workflow for removing a member
huult Dec 16, 2024
22d979e
Remove member if not an approver
huult Dec 16, 2024
27862f1
update logic to remove workflow
huult Dec 16, 2024
44d7ea1
Update case remove approval workflow
huult Dec 16, 2024
1ab0c73
add useMemo into owner details
huult Dec 16, 2024
a795b6b
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Dec 16, 2024
806869b
add unit test for updateWorkflowDataOnApproverRemoval
huult Dec 16, 2024
c444d48
remove check defaultHasOwner
huult Dec 16, 2024
16b58f6
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Dec 27, 2024
08d6f46
fix eslint
huult Dec 27, 2024
676ce9d
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Jan 14, 2025
2d49a59
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Jan 16, 2025
62648c0
fix lint
huult Jan 16, 2025
269adfc
add descriptions and avoid repetition
huult Jan 16, 2025
9dc8dcb
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 3, 2025
e2affba
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 3, 2025
99f3ec4
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 4, 2025
c629035
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 4, 2025
23bd5c4
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 5, 2025
f830c03
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 7, 2025
7b316a3
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 11, 2025
b740dd6
fix eslint
huult Feb 11, 2025
9612808
Merge remote-tracking branch 'upstream/main' into 49954-approval-work…
huult Feb 17, 2025
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
140 changes: 138 additions & 2 deletions src/libs/WorkflowUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import CONST from '@src/CONST';
import type {ApprovalWorkflowOnyx, 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 PersonalDetails from '@src/types/onyx/PersonalDetails';
import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee';

const INITIAL_APPROVAL_WORKFLOW: ApprovalWorkflowOnyx = {
Expand Down Expand Up @@ -157,7 +158,7 @@ function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover,
return 1;
}

return (a.approvers.at(0)?.displayName ?? '-1').localeCompare(b.approvers.at(0)?.displayName ?? '-1');
return (a.approvers.at(0)?.displayName ?? CONST.DEFAULT_NUMBER_ID).toString().localeCompare((b.approvers.at(0)?.displayName ?? CONST.DEFAULT_NUMBER_ID).toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure this is written wrong. @getusha Can you get this corrected?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can spin up a quick PR but curious, is there any regression?

});

// Add a default workflow if one doesn't exist (no employees submit to the default approver)
Expand Down Expand Up @@ -200,6 +201,32 @@ type ConvertApprovalWorkflowToPolicyEmployeesParams = {
type: ValueOf<typeof CONST.APPROVAL_WORKFLOW.TYPE>;
};

type UpdateWorkflowDataOnApproverRemovalParams = {
/**
* An array of approval workflows that need to be updated.
*/
approvalWorkflows: ApprovalWorkflow[];
/**
* The email of the approver being removed
*/
removedApprover: PersonalDetails;
/**
* The email of the workspace owner
*/
ownerDetails: PersonalDetails;
};

type UpdateWorkflowDataOnApproverRemovalResult = Array<
ApprovalWorkflow & {
/**
* @property {boolean} [removeApprovalWorkflow] - A flag that determines if the approval workflow should be removed.
* - `true`: Indicates the approval workflow needs to be removed.
* - `false` or `undefined`: No removal is required; the workflow will be updated instead.
*/
removeApprovalWorkflow?: boolean;
}
>;

/**
* This function converts an approval workflow into a list of policy employees.
* An optimized list is created that contains only the updated employees to maintain minimal data changes.
Expand Down Expand Up @@ -281,5 +308,114 @@ function convertApprovalWorkflowToPolicyEmployees({

return updatedEmployeeList;
}
function updateWorkflowDataOnApproverRemoval({approvalWorkflows, removedApprover, ownerDetails}: UpdateWorkflowDataOnApproverRemovalParams): UpdateWorkflowDataOnApproverRemovalResult {
const defaultWorkflow = approvalWorkflows.find((workflow) => workflow.isDefault);
const removedApproverEmail = removedApprover.login;
const ownerEmail = ownerDetails.login;
const ownerAvatar = ownerDetails.avatar ?? '';
const ownerDisplayName = ownerDetails.displayName ?? '';

return approvalWorkflows.flatMap((workflow) => {
const [currentApprover] = workflow.approvers;
const isSingleApprover = workflow.approvers.length === 1;
const isMultipleApprovers = workflow.approvers.length > 1;
const isApproverToRemove = currentApprover?.email === removedApproverEmail;
const defaultHasOwner = defaultWorkflow?.approvers.some((approver) => approver.email === ownerEmail);

if (workflow.isDefault) {
// Handle default workflow
if (isSingleApprover && isApproverToRemove && currentApprover?.email !== ownerEmail) {
return {
...workflow,
approvers: [
{
...currentApprover,
avatar: ownerAvatar,
displayName: ownerDisplayName,
email: ownerEmail ?? '',
},
],
};
}
return workflow;
}

if (isSingleApprover) {
// Remove workflows with a single approver when owner is the approver
if (currentApprover?.email === ownerEmail) {
return {
...workflow,
removeApprovalWorkflow: true,
};
}

// Handle case where the approver is to be removed
if (isApproverToRemove) {
// Remove workflow if the default workflow includes the owner or approver is to be replaced
if (defaultHasOwner) {
return {
...workflow,
removeApprovalWorkflow: true,
};
}

// Replace the approver with owner details
return {
...workflow,
approvers: [
{
...currentApprover,
avatar: ownerAvatar,
displayName: ownerDisplayName,
email: ownerEmail ?? '',
},
],
};
}
}

if (isMultipleApprovers && workflow.approvers.some((item) => item.email === removedApproverEmail)) {
const removedApproverIndex = workflow.approvers.findIndex((item) => item.email === removedApproverEmail);

// If the removed approver is the first in the list, return an empty array
if (removedApproverIndex === 0) {
return {
...workflow,
removeApprovalWorkflow: true,
};
}

const updateApprovers = workflow.approvers.slice(0, removedApproverIndex);
const updateApproversHasOwner = updateApprovers.some((approver) => approver.email === ownerEmail);

// If the owner is already in the approvers list, return the workflow with the updated approvers
if (updateApproversHasOwner) {
return {
...workflow,
approvers: updateApprovers,
};
}

// Update forwardsTo if necessary and prepare the new approver object
const updatedApprovers = updateApprovers.flatMap((item) => (item.forwardsTo === removedApproverEmail ? {...item, forwardsTo: ownerEmail} : item));

const newApprover = {
email: ownerEmail ?? '',
forwardsTo: undefined,
avatar: ownerDetails?.avatar ?? '',
displayName: ownerDetails?.displayName ?? '',
isCircularReference: workflow.approvers.at(removedApproverIndex)?.isCircularReference,
};

return {
...workflow,
approvers: [...updatedApprovers, newApprover],
};
}

// Return the unchanged workflow in other cases
return workflow;
});
}

export {calculateApprovers, convertPolicyEmployeesToApprovalWorkflows, convertApprovalWorkflowToPolicyEmployees, INITIAL_APPROVAL_WORKFLOW};
export {calculateApprovers, convertPolicyEmployeesToApprovalWorkflows, convertApprovalWorkflowToPolicyEmployees, INITIAL_APPROVAL_WORKFLOW, updateWorkflowDataOnApproverRemoval};
43 changes: 42 additions & 1 deletion src/pages/workspace/WorkspaceMembersPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
removeMembers,
updateWorkspaceMembersRole,
} from '@libs/actions/Policy/Member';
import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow';
import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import {formatPhoneNumber as formatPhoneNumberUtil} from '@libs/LocalePhoneNumber';
import Log from '@libs/Log';
Expand All @@ -52,13 +53,14 @@ import {isPersonalDetailsReady, sortAlphabetically} from '@libs/OptionsListUtils
import {getDisplayNameOrDefault, getPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils';
import {getMemberAccountIDsForWorkspace, isDeletedPolicyEmployee, isExpensifyTeam, isPaidGroupPolicy, isPolicyAdmin as isPolicyAdminUtils} from '@libs/PolicyUtils';
import {getDisplayNameForParticipant} from '@libs/ReportUtils';
import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils';
import {close} from '@userActions/Modal';
import {dismissAddedWithPrimaryLoginMessages, setPolicyPreventSelfApproval} from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {PersonalDetailsList, PolicyEmployeeList} from '@src/types/onyx';
import type {PersonalDetails, PersonalDetailsList, PolicyEmployeeList} from '@src/types/onyx';
import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscreenLoading';
Expand Down Expand Up @@ -114,6 +116,18 @@ function WorkspaceMembersPage({personalDetails, route, policy, currentUserPerson
const isFocused = useIsFocused();
const policyID = route.params.policyID;

const ownerDetails = personalDetails?.[policy?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID] ?? ({} as PersonalDetails);
const policyApproverEmail = policy?.approver;
const {approvalWorkflows} = useMemo(
() =>
convertPolicyEmployeesToApprovalWorkflows({
employees: policy?.employeeList ?? {},
defaultApprover: policyApproverEmail ?? policy?.owner ?? '',
personalDetails: personalDetails ?? {},
}),
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail],
);

const canSelectMultiple = isPolicyAdmin && (shouldUseNarrowLayout ? selectionMode?.isEnabled : true);

const confirmModalPrompt = useMemo(() => {
Expand Down Expand Up @@ -229,6 +243,33 @@ function WorkspaceMembersPage({personalDetails, route, policy, currentUserPerson
// Remove the admin from the list
const accountIDsToRemove = session?.accountID ? selectedEmployees.filter((id) => id !== session.accountID) : selectedEmployees;
const newEmployeesCount = previousEmployeesCount - accountIDsToRemove.length;

// Check if any of the account IDs are approvers
const hasApprovers = accountIDsToRemove.some((accountID) => isApprover(policy, accountID));

if (hasApprovers) {
const ownerEmail = ownerDetails.login;
accountIDsToRemove.forEach((accountID) => {
const removedApprover = personalDetails?.[accountID];
if (!removedApprover?.login || !ownerEmail) {
return;
}
const updatedWorkflows = updateWorkflowDataOnApproverRemoval({
approvalWorkflows,
removedApprover,
ownerDetails,
});
updatedWorkflows.forEach((workflow) => {
if (workflow?.removeApprovalWorkflow) {
const {removeApprovalWorkflow, ...updatedWorkflow} = workflow;
removeApprovalWorkflowAction(policyID, updatedWorkflow);
} else {
updateApprovalWorkflow(policyID, workflow, [], []);
}
});
});
}

setSelectedEmployees([]);
setRemoveMembersConfirmModalVisible(false);
InteractionManager.runAfterInteractions(() => {
Expand Down
49 changes: 47 additions & 2 deletions src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow';
import {getAllCardsForWorkspace, getCardFeedIcon, getCompanyFeeds, maskCardNumber} from '@libs/CardUtils';
import {convertToDisplayString} from '@libs/CurrencyUtils';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import {getDisplayNameOrDefault} from '@libs/PersonalDetailsUtils';
import {getWorkspaceAccountID} from '@libs/PolicyUtils';
import shouldRenderTransferOwnerButton from '@libs/shouldRenderTransferOwnerButton';
import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils';
import Navigation from '@navigation/Navigation';
import type {SettingsNavigatorParamList} from '@navigation/types';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
Expand Down Expand Up @@ -78,13 +80,24 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
const isSelectedMemberCurrentUser = accountID === currentUserPersonalDetails?.accountID;
const isCurrentUserAdmin = policy?.employeeList?.[personalDetails?.[currentUserPersonalDetails?.accountID]?.login ?? '']?.role === CONST.POLICY.ROLE.ADMIN;
const isCurrentUserOwner = policy?.owner === currentUserPersonalDetails?.login;
const ownerDetails = personalDetails?.[policy?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID] ?? ({} as PersonalDetails);
const ownerDetails = useMemo(() => personalDetails?.[policy?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID] ?? ({} as PersonalDetails), [personalDetails, policy?.ownerAccountID]);
const policyOwnerDisplayName = formatPhoneNumber(getDisplayNameOrDefault(ownerDetails)) ?? policy?.owner ?? '';
const hasMultipleFeeds = Object.values(getCompanyFeeds(cardFeeds)).filter((feed) => !feed.pending).length > 0;

const workspaceCards = getAllCardsForWorkspace(workspaceAccountID, cardList);
const hasWorkspaceCardsAssigned = !!workspaceCards && !!Object.values(workspaceCards).length;

const policyApproverEmail = policy?.approver;
const {approvalWorkflows} = useMemo(
() =>
convertPolicyEmployeesToApprovalWorkflows({
employees: policy?.employeeList ?? {},
defaultApprover: policyApproverEmail ?? policy?.owner ?? '',
personalDetails: personalDetails ?? {},
}),
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail],
);

useEffect(() => {
openPolicyCompanyCardsPage(policyID, workspaceAccountID);
}, [policyID, workspaceAccountID]);
Expand Down Expand Up @@ -145,11 +158,43 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
setIsRemoveMemberConfirmModalVisible(true);
};

const removeUser = useCallback(() => {
// Function to remove a member and close the modal
const removeMemberAndCloseModal = useCallback(() => {
removeMembers([accountID], policyID);
setIsRemoveMemberConfirmModalVisible(false);
}, [accountID, policyID]);

const removeUser = useCallback(() => {
const ownerEmail = ownerDetails?.login;
const removedApprover = personalDetails?.[accountID];

// If the user is not an approver, proceed with member removal
if (!isApproverUserAction(policy, accountID) || !removedApprover?.login || !ownerEmail) {
removeMemberAndCloseModal();
return;
}

// Update approval workflows after approver removal
const updatedWorkflows = updateWorkflowDataOnApproverRemoval({
approvalWorkflows,
removedApprover,
ownerDetails,
});

updatedWorkflows.forEach((workflow) => {
if (workflow?.removeApprovalWorkflow) {
const {removeApprovalWorkflow, ...updatedWorkflow} = workflow;

removeApprovalWorkflowAction(policyID, updatedWorkflow);
} else {
updateApprovalWorkflow(policyID, workflow, [], []);
}
});

// Remove the member and close the modal
removeMemberAndCloseModal();
}, [accountID, approvalWorkflows, ownerDetails, personalDetails, policy, policyID, removeMemberAndCloseModal]);

const navigateToProfile = useCallback(() => {
Navigation.navigate(ROUTES.PROFILE.getRoute(accountID, Navigation.getActiveRoute()));
}, [accountID]);
Expand Down
Loading