diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 497218766a2d..949bb58138c9 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -5758,7 +5758,7 @@ function shareTrackedExpense(trackedExpenseParams: TrackedExpenseParams) { optimisticData: addAccountantToWorkspaceOptimisticData, successData: addAccountantToWorkspaceSuccessData, failureData: addAccountantToWorkspaceFailureData, - } = buildUpdateWorkspaceMembersRoleOnyxData(policyID, [accountantAccountID], CONST.POLICY.ROLE.ADMIN); + } = buildUpdateWorkspaceMembersRoleOnyxData(policyID, [accountantEmail], [accountantAccountID], CONST.POLICY.ROLE.ADMIN); optimisticData?.push(...addAccountantToWorkspaceOptimisticData); successData?.push(...addAccountantToWorkspaceSuccessData); failureData?.push(...addAccountantToWorkspaceFailureData); diff --git a/src/libs/actions/Policy/Member.ts b/src/libs/actions/Policy/Member.ts index 776289434a76..7e02801c1fdd 100644 --- a/src/libs/actions/Policy/Member.ts +++ b/src/libs/actions/Policy/Member.ts @@ -28,17 +28,7 @@ import * as ReportUtils from '@libs/ReportUtils'; import * as FormActions from '@userActions/FormActions'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type { - ImportedSpreadsheetMemberData, - InvitedEmailsToAccountIDs, - PersonalDetailsList, - Policy, - PolicyEmployee, - PolicyOwnershipChangeChecks, - Report, - ReportAction, - ReportActions, -} from '@src/types/onyx'; +import type {ImportedSpreadsheetMemberData, InvitedEmailsToAccountIDs, Policy, PolicyEmployee, PolicyOwnershipChangeChecks, Report, ReportAction, ReportActions} from '@src/types/onyx'; import type {PendingAction} from '@src/types/onyx/OnyxCommon'; import type {JoinWorkspaceResolution} from '@src/types/onyx/OriginalMessage'; import type {ApprovalRule} from '@src/types/onyx/Policy'; @@ -54,7 +44,6 @@ type OnyxDataReturnType = { }; type WorkspaceMembersRoleData = { - accountID: number; email: string; role: ValueOf; }; @@ -109,12 +98,6 @@ Onyx.connect({ }, }); -let allPersonalDetails: OnyxEntry; -Onyx.connect({ - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - callback: (val) => (allPersonalDetails = val), -}); - let policyOwnershipChecks: Record; Onyx.connect({ key: ONYXKEYS.POLICY_OWNERSHIP_CHANGE_CHECKS, @@ -133,12 +116,6 @@ function isApprover(policy: OnyxEntry, employeeLogin: string) { ); } -/** Temporary function alias for isApprover with employeeAccountID */ -function isApproverTemp(policy: OnyxEntry, employeeAccountID: number) { - const employeeLogin = allPersonalDetails?.[employeeAccountID]?.login; - return isApprover(policy, employeeLogin ?? ''); -} - /** * Returns the policy of the report * @deprecated Get the data straight from Onyx - This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 @@ -430,20 +407,19 @@ function resetAccountingPreferredExporter(policyID: string, loginList: string[]) * Remove the passed members from the policy employeeList * Please see https://github.com/Expensify/App/blob/main/README.md#Security for more details */ -function removeMembers(accountIDs: number[], policyID: string) { - // In case user selects only themselves (admin), their email will be filtered out and the members - // array passed will be empty, prevent the function from proceeding in that case as there is no one to remove - if (accountIDs.length === 0) { +function removeMembers(policyID: string, selectedMemberEmails: string[], policyMemberEmailsToAccountIDs: Record) { + if (selectedMemberEmails.length === 0) { return; } + const accountIDs = selectedMemberEmails.map((email) => policyMemberEmailsToAccountIDs[email]).filter((id) => id !== undefined); + const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${policyID}` as const; // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 // eslint-disable-next-line @typescript-eslint/no-deprecated const policy = getPolicy(policyID); const workspaceChats = ReportUtils.getWorkspaceChats(policyID, accountIDs); - const emailList = accountIDs.map((accountID) => allPersonalDetails?.[accountID]?.login).filter((login) => !!login) as string[]; const optimisticClosedReportActions = workspaceChats.map(() => ReportUtils.buildOptimisticClosedReportAction(sessionEmail, policy?.name ?? '', CONST.REPORT.ARCHIVE_REASON.REMOVED_FROM_POLICY), ); @@ -453,18 +429,19 @@ function removeMembers(accountIDs: number[], policyID: string) { CONST.REPORT.CHAT_TYPE.POLICY_ADMINS, policy?.id, policy?.name ?? '', - accountIDs.filter((accountID) => { - const login = allPersonalDetails?.[accountID]?.login; - const role = login ? policy?.employeeList?.[login]?.role : ''; - return role === CONST.POLICY.ROLE.ADMIN || role === CONST.POLICY.ROLE.AUDITOR; - }), + selectedMemberEmails + .filter((login) => { + const role = login ? policy?.employeeList?.[login]?.role : ''; + return role === CONST.POLICY.ROLE.ADMIN || role === CONST.POLICY.ROLE.AUDITOR; + }) + .map((login) => policyMemberEmailsToAccountIDs[login]), ); - const preferredExporterOnyxData = resetAccountingPreferredExporter(policyID, emailList); + const preferredExporterOnyxData = resetAccountingPreferredExporter(policyID, selectedMemberEmails); const optimisticMembersState: OnyxCollectionInputValue = {}; const successMembersState: OnyxCollectionInputValue = {}; const failureMembersState: OnyxCollectionInputValue = {}; - emailList.forEach((email) => { + selectedMemberEmails.forEach((email) => { optimisticMembersState[email] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}; successMembersState[email] = null; failureMembersState[email] = {errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.people.error.genericRemove')}; @@ -474,7 +451,7 @@ function removeMembers(accountIDs: number[], policyID: string) { const employee = policy?.employeeList?.[employeeEmail]; optimisticMembersState[employeeEmail] = optimisticMembersState[employeeEmail] ?? {}; failureMembersState[employeeEmail] = failureMembersState[employeeEmail] ?? {}; - if (employee?.submitsTo && emailList.includes(employee?.submitsTo)) { + if (employee?.submitsTo && selectedMemberEmails.includes(employee?.submitsTo)) { optimisticMembersState[employeeEmail] = { ...optimisticMembersState[employeeEmail], submitsTo: policy?.owner, @@ -484,7 +461,7 @@ function removeMembers(accountIDs: number[], policyID: string) { submitsTo: employee?.submitsTo, }; } - if (employee?.forwardsTo && emailList.includes(employee?.forwardsTo)) { + if (employee?.forwardsTo && selectedMemberEmails.includes(employee?.forwardsTo)) { optimisticMembersState[employeeEmail] = { ...optimisticMembersState[employeeEmail], forwardsTo: policy?.owner, @@ -494,7 +471,7 @@ function removeMembers(accountIDs: number[], policyID: string) { forwardsTo: employee?.forwardsTo, }; } - if (employee?.overLimitForwardsTo && emailList.includes(employee?.overLimitForwardsTo)) { + if (employee?.overLimitForwardsTo && selectedMemberEmails.includes(employee?.overLimitForwardsTo)) { optimisticMembersState[employeeEmail] = { ...optimisticMembersState[employeeEmail], overLimitForwardsTo: policy?.owner, @@ -507,7 +484,7 @@ function removeMembers(accountIDs: number[], policyID: string) { }); const approvalRules: ApprovalRule[] = policy?.rules?.approvalRules ?? []; - const optimisticApprovalRules = approvalRules.filter((rule) => !emailList.includes(rule?.approver ?? '')); + const optimisticApprovalRules = approvalRules.filter((rule) => !selectedMemberEmails.includes(rule?.approver ?? '')); const optimisticData: OnyxUpdate[] = [ { @@ -515,7 +492,7 @@ function removeMembers(accountIDs: number[], policyID: string) { key: policyKey, value: { employeeList: optimisticMembersState, - approver: emailList.includes(policy?.approver ?? '') ? policy?.owner : policy?.approver, + approver: selectedMemberEmails.includes(policy?.approver ?? '') ? policy?.owner : policy?.approver, rules: { ...(policy?.rules ?? {}), approvalRules: optimisticApprovalRules, @@ -630,7 +607,7 @@ function removeMembers(accountIDs: number[], policyID: string) { if (!isEmptyObject(policy?.primaryLoginsInvited ?? {})) { // Take the current policy members and remove them optimistically const employeeListEmails = Object.keys(allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]?.employeeList ?? {}); - const remainingLogins = employeeListEmails.filter((email) => !emailList.includes(email)); + const remainingLogins = employeeListEmails.filter((email) => !selectedMemberEmails.includes(email)); const invitedPrimaryToSecondaryLogins: Record = {}; if (policy?.primaryLoginsInvited) { @@ -671,28 +648,21 @@ function removeMembers(accountIDs: number[], policyID: string) { }); const params: DeleteMembersFromWorkspaceParams = { - emailList: emailList.join(','), + emailList: selectedMemberEmails.join(','), policyID, }; API.write(WRITE_COMMANDS.DELETE_MEMBERS_FROM_WORKSPACE, params, {optimisticData, successData, failureData}); } -function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: number[], newRole: ValueOf) { +function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, selectedMemberEmails: string[], selectedMemberAccountIDs: number[], newRole: ValueOf) { const previousEmployeeList = {...allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]?.employeeList}; - const memberRoles: WorkspaceMembersRoleData[] = accountIDs.reduce((result: WorkspaceMembersRoleData[], accountID: number) => { - if (!allPersonalDetails?.[accountID]?.login) { - return result; - } - - result.push({ - accountID, - email: allPersonalDetails?.[accountID]?.login ?? '', + const memberRoles: WorkspaceMembersRoleData[] = selectedMemberEmails.map((login: string) => { + return { + email: login, role: newRole, - }); - - return result; - }, []); + }; + }); const optimisticData: OnyxUpdate[] = [ { @@ -754,7 +724,7 @@ function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: n const failureDataParticipants: Record = {...adminRoom.participants}; const optimisticParticipants: Record = {}; if (newRole === CONST.POLICY.ROLE.ADMIN || newRole === CONST.POLICY.ROLE.AUDITOR) { - accountIDs.forEach((accountID) => { + selectedMemberAccountIDs.forEach((accountID) => { if (adminRoom?.participants?.[accountID]) { return; } @@ -762,7 +732,7 @@ function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: n failureDataParticipants[accountID] = null; }); } else { - accountIDs.forEach((accountID) => { + selectedMemberAccountIDs.forEach((accountID) => { if (!adminRoom?.participants?.[accountID]) { return; } @@ -790,8 +760,8 @@ function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: n return {optimisticData, successData, failureData, memberRoles}; } -function updateWorkspaceMembersRole(policyID: string, accountIDs: number[], newRole: ValueOf) { - const {optimisticData, successData, failureData, memberRoles} = buildUpdateWorkspaceMembersRoleOnyxData(policyID, accountIDs, newRole); +function updateWorkspaceMembersRole(policyID: string, selectedMemberEmails: string[], selectedMemberAccountIDs: number[], newRole: ValueOf) { + const {optimisticData, successData, failureData, memberRoles} = buildUpdateWorkspaceMembersRoleOnyxData(policyID, selectedMemberEmails, selectedMemberAccountIDs, newRole); const params: UpdateWorkspaceMembersRoleParams = { policyID, @@ -1148,11 +1118,10 @@ function askToJoinPolicy(policyID: string) { /** * Removes an error after trying to delete a member */ -function clearDeleteMemberError(policyID: string, accountID: number) { - const email = allPersonalDetails?.[accountID]?.login ?? ''; +function clearDeleteMemberError(policyID: string, login: string) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { employeeList: { - [email]: { + [login]: { pendingAction: null, errors: null, }, @@ -1163,11 +1132,10 @@ function clearDeleteMemberError(policyID: string, accountID: number) { /** * Removes an error after trying to add a member */ -function clearAddMemberError(policyID: string, accountID: number) { - const email = allPersonalDetails?.[accountID]?.login ?? ''; +function clearAddMemberError(policyID: string, login: string, accountID: number) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { employeeList: { - [email]: null, + [login]: null, }, }); Onyx.merge(`${ONYXKEYS.PERSONAL_DETAILS_LIST}`, { @@ -1390,5 +1358,4 @@ export { clearWorkspaceInviteRoleDraft, setImportedSpreadsheetMemberData, clearImportedSpreadsheetMemberData, - isApproverTemp, }; diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index be16bb7b39db..2df272d09f27 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -3,7 +3,6 @@ import {deepEqual} from 'fast-equals'; import React, {useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; import type {TextInput} from 'react-native'; import {InteractionManager, View} from 'react-native'; -import type {OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import Button from '@components/Button'; import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu'; @@ -38,7 +37,7 @@ import { clearInviteDraft, clearWorkspaceOwnerChangeFlow, downloadMembersCSV, - isApproverTemp, + isApprover, openWorkspaceMembersPage, removeMembers, updateWorkspaceMembersRole, @@ -50,7 +49,7 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {WorkspaceSplitNavigatorParamList} from '@libs/Navigation/types'; import {isPersonalDetailsReady, sortAlphabetically} from '@libs/OptionsListUtils'; -import {getAccountIDsByLogins, getDisplayNameOrDefault, getPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils'; +import {getDisplayNameOrDefault, getPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils'; import {getMemberAccountIDsForWorkspace, isDeletedPolicyEmployee, isExpensifyTeam, isPaidGroupPolicy, isPolicyAdmin as isPolicyAdminUtils} from '@libs/PolicyUtils'; import {getDisplayNameForParticipant} from '@libs/ReportUtils'; import StringUtils from '@libs/StringUtils'; @@ -61,8 +60,8 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; -import type {PersonalDetails, PersonalDetailsList, PolicyEmployee, PolicyEmployeeList} from '@src/types/onyx'; -import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon'; +import type {PersonalDetails, PolicyEmployee, PolicyEmployeeList} from '@src/types/onyx'; +import type {PendingAction} from '@src/types/onyx/OnyxCommon'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import MemberRightIcon from './MemberRightIcon'; import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscreenLoading'; @@ -79,26 +78,14 @@ function invertObject(object: Record): Record { return Object.fromEntries(invertedEntries); } -type MemberOption = Omit & {accountID: number}; +type MemberOption = Omit & {accountID: number; login: string}; function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembersPageProps) { - const {policyMemberEmailsToAccountIDs, employeeListDetails} = useMemo(() => { - const emailsToAccountIDs = getMemberAccountIDsForWorkspace(policy?.employeeList, true); - const details = Object.keys(policy?.employeeList ?? {}).reduce>((acc, email) => { - const employee = policy?.employeeList?.[email]; - const accountID = emailsToAccountIDs[email]; - if (!employee) { - return acc; - } - acc[accountID] = employee; - return acc; - }, {}); - return {policyMemberEmailsToAccountIDs: emailsToAccountIDs, employeeListDetails: details}; - }, [policy?.employeeList]); + const policyMemberEmailsToAccountIDs = useMemo(() => getMemberAccountIDsForWorkspace(policy?.employeeList, true), [policy?.employeeList]); + const employeeListDetails = useMemo(() => policy?.employeeList ?? ({} as PolicyEmployeeList), [policy?.employeeList]); const currentUserPersonalDetails = useCurrentUserPersonalDetails(); const styles = useThemeStyles(); const [removeMembersConfirmModalVisible, setRemoveMembersConfirmModalVisible] = useState(false); - const [errors, setErrors] = useState({}); const {isOffline} = useNetwork(); const prevIsOffline = usePrevious(isOffline); const accountIDs = useMemo(() => Object.values(policyMemberEmailsToAccountIDs ?? {}).map((accountID) => Number(accountID)), [policyMemberEmailsToAccountIDs]); @@ -107,22 +94,20 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const [isOfflineModalVisible, setIsOfflineModalVisible] = useState(false); const [isDownloadFailureModalVisible, setIsDownloadFailureModalVisible] = useState(false); const isOfflineAndNoMemberDataAvailable = isEmptyObject(policy?.employeeList) && isOffline; - const prevPersonalDetails = usePrevious(personalDetails); const {translate, formatPhoneNumber, localeCompare} = useLocalize(); const {isAccountLocked, showLockedAccountModal} = useContext(LockedAccountContext); const filterEmployees = useCallback( - (employee?: PolicyEmployee) => { + (employee: PolicyEmployee | undefined) => { if (!employee?.email) { return false; } - const employeeAccountID = getAccountIDsByLogins([employee.email]).at(0); - if (!employeeAccountID) { + if (employee.email === policy?.owner || employee.email === currentUserPersonalDetails.login) { return false; } - const isPendingDelete = employeeListDetails?.[employeeAccountID]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; - return accountIDs.includes(employeeAccountID) && !isPendingDelete; + const isPendingDelete = employee.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; + return !isPendingDelete; }, - [accountIDs, employeeListDetails], + [currentUserPersonalDetails.login, policy?.owner], ); const [selectedEmployees, setSelectedEmployees] = useFilteredSelection(employeeListDetails, filterEmployees); @@ -159,29 +144,20 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const canSelectMultiple = isPolicyAdmin && (shouldUseNarrowLayout ? isMobileSelectionModeEnabled : true); const confirmModalPrompt = useMemo(() => { - const approverAccountID = selectedEmployees.find((selectedEmployee) => isApproverTemp(policy, selectedEmployee)); - if (!approverAccountID) { + const approverEmail = selectedEmployees.find((selectedEmployee) => isApprover(policy, selectedEmployee)); + if (!approverEmail) { + const firstSelectedEmployeeAccountID = policyMemberEmailsToAccountIDs[selectedEmployees[0]]; return translate('workspace.people.removeMembersPrompt', { count: selectedEmployees.length, - memberName: formatPhoneNumber(getPersonalDetailsByIDs({accountIDs: selectedEmployees, currentUserAccountID}).at(0)?.displayName ?? ''), + memberName: formatPhoneNumber(getPersonalDetailsByIDs({accountIDs: [firstSelectedEmployeeAccountID], currentUserAccountID}).at(0)?.displayName ?? ''), }); } + const approverAccountID = policyMemberEmailsToAccountIDs[approverEmail]; return translate('workspace.people.removeMembersWarningPrompt', { memberName: getDisplayNameForParticipant({accountID: approverAccountID}), ownerName: getDisplayNameForParticipant({accountID: policy?.ownerAccountID}), }); - }, [selectedEmployees, translate, policy, currentUserAccountID, formatPhoneNumber]); - /** - * Get filtered personalDetails list with current employeeList - */ - const filterPersonalDetails = (members: OnyxEntry, details: OnyxEntry): PersonalDetailsList => - Object.keys(members ?? {}).reduce((acc, key) => { - const memberAccountIdKey = policyMemberEmailsToAccountIDs[key] ?? ''; - if (details?.[memberAccountIdKey]) { - acc[memberAccountIdKey] = details[memberAccountIdKey]; - } - return acc; - }, {} as PersonalDetailsList); + }, [selectedEmployees, policyMemberEmailsToAccountIDs, translate, policy, formatPhoneNumber, currentUserAccountID]); /** * Get members for the current workspace */ @@ -189,51 +165,17 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers openWorkspaceMembersPage(route.params.policyID, Object.keys(getMemberAccountIDsForWorkspace(policy?.employeeList))); }, [route.params.policyID, policy?.employeeList]); - /** - * Check if the current selection includes members that cannot be removed - */ - const validateSelection = useCallback(() => { - const newErrors: Errors = {}; - selectedEmployees.forEach((member) => { - if (member !== policy?.ownerAccountID && member !== session?.accountID) { - return; - } - newErrors[member] = translate('workspace.people.error.cannotRemove'); - }); - setErrors(newErrors); - }, [selectedEmployees, policy?.ownerAccountID, session?.accountID, translate]); - useEffect(() => { getWorkspaceMembers(); }, [getWorkspaceMembers]); useEffect(() => { - validateSelection(); - }, [validateSelection]); - - useEffect(() => { - if (removeMembersConfirmModalVisible && !deepEqual(accountIDs, prevAccountIDs)) { - setRemoveMembersConfirmModalVisible(false); + if (!removeMembersConfirmModalVisible || deepEqual(accountIDs, prevAccountIDs)) { + return; } - setSelectedEmployees((prevSelectedEmployees) => { - // Filter all personal details in order to use the elements needed for the current workspace - const currentPersonalDetails = filterPersonalDetails(policy?.employeeList ?? {}, personalDetails); - // We need to filter the previous selected employees by the new personal details, since unknown/new user id's change when transitioning from offline to online - const prevSelectedElements = prevSelectedEmployees.map((id) => { - const prevItem = prevPersonalDetails?.[id]; - const res = Object.values(currentPersonalDetails).find((item) => prevItem?.login === item?.login); - return res?.accountID ?? id; - }); - - const currentSelectedElements = Object.entries(getMemberAccountIDsForWorkspace(policy?.employeeList)) - .filter((employee) => policy?.employeeList?.[employee[0]]?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) - .map((employee) => employee[1]); - - // This is an equivalent of the lodash intersection function. The reduce method below is used to filter the items that exist in both arrays. - return [prevSelectedElements, currentSelectedElements].reduce((prev, members) => prev.filter((item) => members.includes(item))); - }); + setRemoveMembersConfirmModalVisible(false); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [policy?.employeeList, policyMemberEmailsToAccountIDs]); + }, [accountIDs]); useEffect(() => { const isReconnecting = prevIsOffline && !isOffline; @@ -260,19 +202,13 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers * Please see https://github.com/Expensify/App/blob/main/README.md#Security for more details */ const removeUsers = () => { - if (!isEmptyObject(errors)) { - return; - } - - // Remove the admin from the list - const accountIDsToRemove = session?.accountID ? selectedEmployees.filter((id) => id !== session.accountID) : selectedEmployees; - - // Check if any of the account IDs are approvers - const hasApprovers = accountIDsToRemove.some((accountID) => isApproverTemp(policy, accountID)); + // Check if any of the members are approvers + const hasApprovers = selectedEmployees.some((email) => isApprover(policy, email)); if (hasApprovers) { const ownerEmail = ownerDetails.login; - accountIDsToRemove.forEach((accountID) => { + selectedEmployees.forEach((login) => { + const accountID = policyMemberEmailsToAccountIDs[login]; const removedApprover = personalDetails?.[accountID]; if (!removedApprover?.login || !ownerEmail) { return; @@ -297,7 +233,7 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => { setSelectedEmployees([]); - removeMembers(accountIDsToRemove, route.params.policyID); + removeMembers(policyID, selectedEmployees, policyMemberEmailsToAccountIDs); }); }; @@ -305,9 +241,6 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers * Show the modal to confirm removal of the selected members */ const askForConfirmationToRemove = () => { - if (!isEmptyObject(errors)) { - return; - } setRemoveMembersConfirmModalVisible(true); }; @@ -316,46 +249,42 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers */ const toggleAllUsers = (memberList: MemberOption[]) => { const enabledAccounts = memberList.filter((member) => !member.isDisabled && !member.isDisabledCheckbox); - const someSelected = enabledAccounts.some((member) => selectedEmployees.includes(member.accountID)); + const someSelected = selectedEmployees.length > 0; if (someSelected) { setSelectedEmployees([]); } else { - const everyAccountId = enabledAccounts.map((member) => member.accountID); - setSelectedEmployees(everyAccountId); + const everyLogin = enabledAccounts.map((member) => member.login); + setSelectedEmployees(everyLogin); } - - validateSelection(); }; /** * Add user from the selectedEmployees list */ const addUser = useCallback( - (accountID: number) => { - setSelectedEmployees((prevSelected) => [...prevSelected, accountID]); - validateSelection(); + (login: string) => { + setSelectedEmployees((prevSelected) => [...prevSelected, login]); }, - [validateSelection, setSelectedEmployees], + [setSelectedEmployees], ); /** * Remove user from the selectedEmployees list */ const removeUser = useCallback( - (accountID: number) => { - setSelectedEmployees((prevSelected) => prevSelected.filter((id) => id !== accountID)); - validateSelection(); + (login: string) => { + setSelectedEmployees((prevSelected) => prevSelected.filter((email) => email !== login)); }, - [validateSelection, setSelectedEmployees], + [setSelectedEmployees], ); /** * Toggle user from the selectedEmployees list */ const toggleUser = useCallback( - (accountID: number, pendingAction?: PendingAction) => { - if (accountID === policy?.ownerAccountID && accountID !== session?.accountID) { + (login: string, pendingAction?: PendingAction) => { + if (login === policy?.owner && login !== currentUserPersonalDetails.login) { return; } @@ -364,13 +293,13 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers } // Add or remove the user if the checkbox is enabled - if (selectedEmployees.includes(accountID)) { - removeUser(accountID); + if (selectedEmployees.includes(login)) { + removeUser(login); } else { - addUser(accountID); + addUser(login); } }, - [selectedEmployees, addUser, removeUser, policy?.ownerAccountID, session?.accountID], + [policy?.owner, currentUserPersonalDetails.login, selectedEmployees, removeUser, addUser], ); /** Opens the member details page */ @@ -394,9 +323,9 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const dismissError = useCallback( (item: MemberOption) => { if (item.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { - clearDeleteMemberError(route.params.policyID, item.accountID); + clearDeleteMemberError(route.params.policyID, item.login); } else { - clearAddMemberError(route.params.policyID, item.accountID); + clearAddMemberError(route.params.policyID, item.login, item.accountID); } }, [route.params.policyID], @@ -433,8 +362,9 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const isPendingDeleteOrError = isPolicyAdmin && (policyEmployee.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || !isEmptyObject(policyEmployee.errors)); result.push({ - keyForList: String(accountID), + keyForList: details.login ?? '', accountID, + login: details.login ?? '', isDisabledCheckbox: !(isPolicyAdmin && accountID !== policy?.ownerAccountID && accountID !== session?.accountID), isDisabled: isPendingDeleteOrError, isInteractive: !details.isOptimisticPersonalDetail, @@ -495,7 +425,7 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers if (isEmptyObject(invitedEmailsToAccountIDsDraft) || accountIDs === prevAccountIDs) { return; } - const invitedEmails = Object.values(invitedEmailsToAccountIDsDraft).map(String); + const invitedEmails = Object.keys(invitedEmailsToAccountIDsDraft); selectionListRef.current?.scrollAndHighlightItem?.(invitedEmails); clearInviteDraft(route.params.policyID); }, [invitedEmailsToAccountIDsDraft, isFocused, accountIDs, prevAccountIDs, route.params.policyID]); @@ -557,17 +487,14 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers }; const changeUserRole = (role: ValueOf) => { - if (!isEmptyObject(errors)) { - return; - } - - const accountIDsToUpdate = selectedEmployees.filter((accountID) => { - const email = personalDetails?.[accountID]?.login ?? ''; - return policy?.employeeList?.[email]?.role !== role; + const loginsToUpdate = selectedEmployees.filter((login) => { + return policy?.employeeList?.[login]?.role !== role; }); + const accountIDsToUpdate = loginsToUpdate.map((login) => policyMemberEmailsToAccountIDs[login]).filter((id) => id !== undefined); + setSelectedEmployees([]); - updateWorkspaceMembersRole(route.params.policyID, accountIDsToUpdate, role); + updateWorkspaceMembersRole(route.params.policyID, loginsToUpdate, accountIDsToUpdate, role); }; const getBulkActionsButtonOptions = () => { @@ -802,17 +729,17 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers ref={selectionListRef} canSelectMultiple={canSelectMultiple} sections={[{data: filteredData, isDisabled: false}]} - selectedItems={selectedEmployees.map(String)} + selectedItems={selectedEmployees} ListItem={TableListItem} shouldUseDefaultRightHandSideCheckmark={false} turnOnSelectionModeOnLongPress={isPolicyAdmin} - onTurnOnSelectionMode={(item) => item && toggleUser(item?.accountID)} + onTurnOnSelectionMode={(item) => item && toggleUser(item.login)} shouldUseUserSkeletonView disableKeyboardShortcuts={removeMembersConfirmModalVisible} headerMessage={shouldUseNarrowLayout ? headerMessage : undefined} onSelectRow={openMemberDetails} shouldSingleExecuteRowSelect={!isPolicyAdmin} - onCheckboxPress={(item) => toggleUser(item.accountID)} + onCheckboxPress={(item) => toggleUser(item.login)} onSelectAll={filteredData.length > 0 ? () => toggleAllUsers(filteredData) : undefined} onDismissError={dismissError} showLoadingPlaceholder={isLoading} diff --git a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx index e577f9ce5988..8d29c54c9625 100644 --- a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx +++ b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx @@ -177,15 +177,15 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM // Function to remove a member and close the modal const removeMemberAndCloseModal = useCallback(() => { - removeMembers([accountID], policyID); + removeMembers(policyID, [memberLogin], {[memberLogin]: accountID}); const previousEmployeesCount = Object.keys(policy?.employeeList ?? {}).length; const remainingEmployeeCount = previousEmployeesCount - 1; if (remainingEmployeeCount === 1 && policy?.preventSelfApproval) { // We can't let the "Prevent Self Approvals" enabled if there's only one workspace user - setPolicyPreventSelfApproval(route.params.policyID, false); + setPolicyPreventSelfApproval(policyID, false); } setIsRemoveMemberConfirmModalVisible(false); - }, [accountID, policy?.employeeList, policy?.preventSelfApproval, policyID, route.params.policyID]); + }, [accountID, memberLogin, policy?.employeeList, policy?.preventSelfApproval, policyID]); const removeUser = useCallback(() => { const ownerEmail = ownerDetails?.login; @@ -264,9 +264,9 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM const changeRole = useCallback( ({value}: ListItemType) => { setIsRoleSelectionModalVisible(false); - updateWorkspaceMembersRole(policyID, [accountID], value); + updateWorkspaceMembersRole(policyID, [memberLogin], [accountID], value); }, - [accountID, policyID], + [accountID, memberLogin, policyID], ); const startChangeOwnershipFlow = useCallback(() => { diff --git a/tests/actions/PolicyMemberTest.ts b/tests/actions/PolicyMemberTest.ts index 92f4fe6e2fec..1691b855d671 100644 --- a/tests/actions/PolicyMemberTest.ts +++ b/tests/actions/PolicyMemberTest.ts @@ -110,7 +110,7 @@ describe('actions/PolicyMember', () => { Onyx.set(`${ONYXKEYS.PERSONAL_DETAILS_LIST}`, {[fakeUser2.accountID]: fakeUser2}); await waitForBatchedUpdates(); // When a user's role is set as admin on a policy - Member.updateWorkspaceMembersRole(fakePolicy.id, [fakeUser2.accountID], CONST.POLICY.ROLE.ADMIN); + Member.updateWorkspaceMembersRole(fakePolicy.id, [fakeUser2.login ?? ''], [fakeUser2.accountID], CONST.POLICY.ROLE.ADMIN); await waitForBatchedUpdates(); await new Promise((resolve) => { const connection = Onyx.connect({ @@ -153,7 +153,7 @@ describe('actions/PolicyMember', () => { }); await waitForBatchedUpdates(); // When an admin is demoted from their admin role to a user role - Member.updateWorkspaceMembersRole(fakePolicy.id, [fakeUser2.accountID], CONST.POLICY.ROLE.USER); + Member.updateWorkspaceMembersRole(fakePolicy.id, [fakeUser2.login ?? ''], [fakeUser2.accountID], CONST.POLICY.ROLE.USER); await waitForBatchedUpdates(); await new Promise((resolve) => { const connection = Onyx.connect({ @@ -459,7 +459,12 @@ describe('actions/PolicyMember', () => { // When removing am admin, auditor, and user members mockFetch?.pause?.(); - Member.removeMembers([adminAccountID, auditorAccountID, userAccountID], policyID); + const memberEmailsToAccountIDs = { + [adminEmail]: adminAccountID, + [auditorEmail]: auditorAccountID, + [userEmail]: userAccountID, + }; + Member.removeMembers(policyID, [adminEmail, auditorEmail, userEmail], memberEmailsToAccountIDs); await waitForBatchedUpdates(); @@ -500,6 +505,7 @@ describe('actions/PolicyMember', () => { const workspaceReportID = '1'; const expenseReportID = '2'; const userAccountID = 1236; + const userEmail = 'user@example.com'; await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${workspaceReportID}`, { ...createRandomReport(Number(workspaceReportID)), @@ -518,7 +524,7 @@ describe('actions/PolicyMember', () => { // When removing a member from the workspace mockFetch?.pause?.(); - Member.removeMembers([userAccountID], policyID); + Member.removeMembers(policyID, [userEmail], {[userEmail]: userAccountID}); await waitForBatchedUpdates();