Skip to content

Refactored usage of ONYXKEYS.PERSONAL_DETAILS_LIST from Member Actions #70581

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.

Sign up for GitHub

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
tgolen merged 5 commits into from
Oct 23, 2025
Merged

Refactored usage of ONYXKEYS.PERSONAL_DETAILS_LIST from Member Actions #70581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc62364
Refactored usage of ONYXKEYS.PERSONAL_DETAILS_LIST from Member Actions
shubham1206agra Sep 15, 2025
74ca44f
Merge branch 'Expensify:main' into refactor-onyx-51
shubham1206agra Oct 7, 2025
26967ae
Fixed some minor bugs
shubham1206agra Oct 7, 2025
49e1551
Rename variables
shubham1206agra Oct 7, 2025
eaaf33f
Merge branch 'Expensify:main' into refactor-onyx-51
shubham1206agra Oct 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
2 changes: 1 addition & 1 deletion src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
101 changes: 34 additions & 67 deletions src/libs/actions/Policy/Member.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -54,7 +44,6 @@ type OnyxDataReturnType = {
};

type WorkspaceMembersRoleData = {
accountID: number;
email: string;
role: ValueOf<typeof CONST.POLICY.ROLE>;
};
Expand Down Expand Up @@ -109,12 +98,6 @@ Onyx.connect({
},
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (val) => (allPersonalDetails = val),
});

let policyOwnershipChecks: Record<string, PolicyOwnershipChangeChecks>;
Onyx.connect({
key: ONYXKEYS.POLICY_OWNERSHIP_CHANGE_CHECKS,
Expand All @@ -133,12 +116,6 @@ function isApprover(policy: OnyxEntry<Policy>, employeeLogin: string) {
);
}

/** Temporary function alias for isApprover with employeeAccountID */
function isApproverTemp(policy: OnyxEntry<Policy>, 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
Expand Down Expand Up @@ -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<string, number>) {
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),
);
Expand All @@ -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<PolicyEmployee> = {};
const successMembersState: OnyxCollectionInputValue<PolicyEmployee> = {};
const failureMembersState: OnyxCollectionInputValue<PolicyEmployee> = {};
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')};
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -507,15 +484,15 @@ 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[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
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,
Expand Down Expand Up @@ -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<string, string> = {};

if (policy?.primaryLoginsInvited) {
Expand Down Expand Up @@ -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<typeof CONST.POLICY.ROLE>) {
function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, selectedMemberEmails: string[], selectedMemberAccountIDs: number[], newRole: ValueOf<typeof CONST.POLICY.ROLE>) {
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[] = [
{
Expand Down Expand Up @@ -754,15 +724,15 @@ function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: n
const failureDataParticipants: Record<number, Participant | null> = {...adminRoom.participants};
const optimisticParticipants: Record<number, Participant | null> = {};
if (newRole === CONST.POLICY.ROLE.ADMIN || newRole === CONST.POLICY.ROLE.AUDITOR) {
accountIDs.forEach((accountID) => {
selectedMemberAccountIDs.forEach((accountID) => {
if (adminRoom?.participants?.[accountID]) {
return;
}
optimisticParticipants[accountID] = {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS};
failureDataParticipants[accountID] = null;
});
} else {
accountIDs.forEach((accountID) => {
selectedMemberAccountIDs.forEach((accountID) => {
if (!adminRoom?.participants?.[accountID]) {
return;
}
Expand Down Expand Up @@ -790,8 +760,8 @@ function buildUpdateWorkspaceMembersRoleOnyxData(policyID: string, accountIDs: n
return {optimisticData, successData, failureData, memberRoles};
}

function updateWorkspaceMembersRole(policyID: string, accountIDs: number[], newRole: ValueOf<typeof CONST.POLICY.ROLE>) {
const {optimisticData, successData, failureData, memberRoles} = buildUpdateWorkspaceMembersRoleOnyxData(policyID, accountIDs, newRole);
function updateWorkspaceMembersRole(policyID: string, selectedMemberEmails: string[], selectedMemberAccountIDs: number[], newRole: ValueOf<typeof CONST.POLICY.ROLE>) {
const {optimisticData, successData, failureData, memberRoles} = buildUpdateWorkspaceMembersRoleOnyxData(policyID, selectedMemberEmails, selectedMemberAccountIDs, newRole);

const params: UpdateWorkspaceMembersRoleParams = {
policyID,
Expand Down Expand Up @@ -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,
},
Expand All @@ -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}`, {
Expand Down Expand Up @@ -1390,5 +1358,4 @@ export {
clearWorkspaceInviteRoleDraft,
setImportedSpreadsheetMemberData,
clearImportedSpreadsheetMemberData,
isApproverTemp,
};
Loading
Loading