Merge pull request #20397 from Expensify/neil-collection-policy-members

Policy members keyed by accountID

src/ONYXKEYS.js

@@ -111,6 +111,7 @@ export default {
         DOWNLOAD: 'download_',
         POLICY: 'policy_',
         POLICY_MEMBER_LIST: 'policyMemberList_',
+        POLICY_MEMBERS: 'policyMembers_',
         WORKSPACE_INVITE_MEMBERS_DRAFT: 'workspaceInviteMembersDraft_',
         REPORT: 'report_',
         REPORT_ACTIONS: 'reportActions_',

src/components/AvatarWithIndicator.js

@@ -29,7 +29,7 @@ const propTypes = {
     /* Onyx Props */
 
     /** The employee list of all policies (coming from Onyx) */
-    policiesMemberList: PropTypes.objectOf(policyMemberPropType),
+    allPolicyMembers: PropTypes.objectOf(PropTypes.objectOf(policyMemberPropType)),
 
     /** All the user's policies (from Onyx via withFullPolicy) */
     policies: PropTypes.objectOf(policyPropTypes.policy),
@@ -62,7 +62,7 @@ const propTypes = {
 const defaultProps = {
     tooltipText: '',
     reimbursementAccount: {},
-    policiesMemberList: {},
+    allPolicyMembers: {},
     policies: {},
     bankAccountList: {},
     cardList: {},
@@ -75,7 +75,7 @@ const AvatarWithIndicator = (props) => {
     // If a policy was just deleted from Onyx, then Onyx will pass a null value to the props, and
     // those should be cleaned out before doing any error checking
     const cleanPolicies = _.pick(props.policies, (policy) => policy);
-    const cleanPolicyMembers = _.pick(props.policiesMemberList, (member) => member);
+    const cleanAllPolicyMembers = _.pick(props.allPolicyMembers, (policyMembers) => policyMembers);
 
     // All of the error & info-checking methods are put into an array. This is so that using _.some() will return
     // early as soon as the first error / info condition is returned. This makes the checks very efficient since
@@ -85,7 +85,7 @@ const AvatarWithIndicator = (props) => {
         () => PaymentMethods.hasPaymentMethodError(props.bankAccountList, props.cardList),
         () => _.some(cleanPolicies, PolicyUtils.hasPolicyError),
         () => _.some(cleanPolicies, PolicyUtils.hasCustomUnitsError),
-        () => _.some(cleanPolicyMembers, PolicyUtils.hasPolicyMemberError),
+        () => _.some(cleanAllPolicyMembers, PolicyUtils.hasPolicyMemberError),
         () => !_.isEmpty(props.reimbursementAccount.errors),
         () => UserUtils.hasLoginListError(props.loginList),
 
@@ -114,8 +114,8 @@ AvatarWithIndicator.propTypes = propTypes;
 AvatarWithIndicator.displayName = 'AvatarWithIndicator';
 
 export default withOnyx({
-    policiesMemberList: {
-        key: ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST,
+    allPolicyMembers: {
+        key: ONYXKEYS.COLLECTION.POLICY_MEMBERS,
     },
     policies: {
         key: ONYXKEYS.COLLECTION.POLICY,

src/libs/PolicyUtils.js

@@ -5,14 +5,14 @@ import CONST from '../CONST';
 import ONYXKEYS from '../ONYXKEYS';
 
 /**
- * Checks if we have any errors stored within the POLICY_MEMBER_LIST. Determines whether we should show a red brick road error or not.
- * Data structure: {email: {role:'user', errors: []}, email2: {role:'admin', errors: [{1231312313: 'Unable to do X'}]}, ...}
+ * Checks if we have any errors stored within the POLICY_MEMBERS. Determines whether we should show a red brick road error or not.
+ * Data structure: {accountID: {role:'user', errors: []}, accountID2: {role:'admin', errors: [{1231312313: 'Unable to do X'}]}, ...}
  *
- * @param {Object} policyMemberList
+ * @param {Object} policyMembers
  * @returns {Boolean}
  */
-function hasPolicyMemberError(policyMemberList) {
-    return _.some(policyMemberList, (member) => !_.isEmpty(member.errors));
+function hasPolicyMemberError(policyMembers) {
+    return _.some(policyMembers, (member) => !_.isEmpty(member.errors));
 }
 
 /**
@@ -53,12 +53,12 @@ function hasCustomUnitsError(policy) {
  *
  * @param {Object} policy
  * @param {String} policy.id
- * @param {Object} policyMembers
+ * @param {Object} policyMembersCollection
  * @returns {String}
  */
-function getPolicyBrickRoadIndicatorStatus(policy, policyMembers) {
-    const policyMemberList = lodashGet(policyMembers, `${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policy.id}`, {});
-    if (hasPolicyMemberError(policyMemberList) || hasCustomUnitsError(policy) || hasPolicyErrorFields(policy)) {
+function getPolicyBrickRoadIndicatorStatus(policy, policyMembersCollection) {
+    const policyMembers = lodashGet(policyMembersCollection, `${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policy.id}`, {});
+    if (hasPolicyMemberError(policyMembers) || hasCustomUnitsError(policy) || hasPolicyErrorFields(policy)) {
         return CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
     }
     return '';
@@ -100,4 +100,31 @@ function isExpensifyTeam(email) {
  */
 const isPolicyAdmin = (policy) => lodashGet(policy, 'role') === CONST.POLICY.ROLE.ADMIN;
 
-export {hasPolicyMemberError, hasPolicyError, hasPolicyErrorFields, hasCustomUnitsError, getPolicyBrickRoadIndicatorStatus, shouldShowPolicy, isExpensifyTeam, isPolicyAdmin};
+/**
+ * @param {Object} policyMembers
+ * @param {Object} personalDetails
+ * @returns {Object}
+ *
+ * Create an object mapping member emails to their accountIDs. Filter for members without errors, and get the login email from the personalDetail object using the accountID.
+ * TODO: Clean up uses of this function to work with accountIDs instead of emails.
+ *
+ * We only return members without errors. Otherwise, the members with errors would immediately be removed before the user has a chance to read the error.
+ */
+function getClientPolicyMemberEmailsToAccountIDs(policyMembers, personalDetails) {
+    return _.chain(policyMembers)
+        .filter((member) => _.isEmpty(member.errors))
+        .keys()
+        .reduce((result, accountID) => {
+            const personalDetail = personalDetails[accountID];
+            if (personalDetail && personalDetail.login) {
+                return {
+                    ...result,
+                    [personalDetail.login]: accountID,
+                }
+            }
+            return result;
+        }, {})
+        .value();
+}
+
+export {hasPolicyMemberError, hasPolicyError, hasPolicyErrorFields, hasCustomUnitsError, getPolicyBrickRoadIndicatorStatus, shouldShowPolicy, isExpensifyTeam, isPolicyAdmin, getClientPolicyMemberEmailsToAccountIDs};

src/libs/actions/Policy.js

@@ -186,34 +186,35 @@ function hasActiveFreePolicy(policies) {
  * Remove the passed members from the policy employeeList
  *
  * @param {Array} members
+ * @param {Array} accountIDs
  * @param {String} policyID
  */
-function removeMembers(members, policyID) {
+function removeMembers(members, accountIDs, policyID) {
     // 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 noone to remove
     if (members.length === 0) {
         return;
     }
-    const membersListKey = `${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`;
+    const membersListKey = `${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID}`;
     const optimisticData = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: membersListKey,
-            value: _.object(members, Array(members.length).fill({pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE})),
+            value: _.object(accountIDs, Array(members.length).fill({pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE})),
         },
     ];
     const successData = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: membersListKey,
-            value: _.object(members, Array(members.length).fill(null)),
+            value: _.object(accountIDs, Array(members.length).fill(null)),
         },
     ];
     const failureData = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: membersListKey,
-            value: _.object(members, Array(members.length).fill({errors: ErrorUtils.getMicroSecondOnyxError('workspace.people.error.genericRemove')})),
+            value: _.object(accountIDs, Array(members.length).fill({errors: ErrorUtils.getMicroSecondOnyxError('workspace.people.error.genericRemove')})),
         },
     ];
     API.write(
@@ -230,11 +231,11 @@ function removeMembers(members, policyID) {
  * Optimistically create a chat for each member of the workspace, creates both optimistic and success data for onyx.
  *
  * @param {String} policyID
- * @param {Array} members
+ * @param {Object} invitedEmailsToAccountIDs
  * @param {Array} betas
  * @returns {Object} - object with onyxSuccessData, onyxOptimisticData, and optimisticReportIDs (map login to reportID)
  */
-function createPolicyExpenseChats(policyID, members, betas) {
+function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs, betas) {
     const workspaceMembersChats = {
         onyxSuccessData: [],
         onyxOptimisticData: [],
@@ -323,25 +324,25 @@ function createPolicyExpenseChats(policyID, members, betas) {
 /**
  * Adds members to the specified workspace/policyID
  *
- * @param {Array<String>} memberLogins
+ * @param {Object} invitedEmailsToAccountIDs
  * @param {String} welcomeNote
  * @param {String} policyID
  * @param {Array<String>} betas
  */
-function addMembersToWorkspace(memberLogins, welcomeNote, policyID, betas) {
+function addMembersToWorkspace(invitedEmailsToAccountIDs, welcomeNote, policyID, betas) {
     const membersListKey = `${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`;
-    const logins = _.map(memberLogins, (memberLogin) => OptionsListUtils.addSMSDomainIfPhoneNumber(memberLogin));
+    const accountIDs = _.values(invitedEmailsToAccountIDs);
 
     // create onyx data for policy expense chats for each new member
-    const membersChats = createPolicyExpenseChats(policyID, logins, betas);
+    const membersChats = createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs, betas);
 
     const optimisticData = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: membersListKey,
 
             // Convert to object with each key containing {pendingAction: ‘add’}
-            value: _.object(logins, Array(logins.length).fill({pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD})),
+            value: _.object(accountIDs, Array(accountIDs.length).fill({pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD})),
         },
         ...membersChats.onyxOptimisticData,
     ];
@@ -353,7 +354,7 @@ function addMembersToWorkspace(memberLogins, welcomeNote, policyID, betas) {
 
             // Convert to object with each key clearing pendingAction. We don’t
             // need to remove the members since that will be handled by onClose of OfflineWithFeedback.
-            value: _.object(logins, Array(logins.length).fill({pendingAction: null, errors: null})),
+            value: _.object(accountIDs, Array(accountIDs.length).fill({pendingAction: null, errors: null})),
         },
         ...membersChats.onyxSuccessData,
     ];
@@ -366,15 +367,16 @@ function addMembersToWorkspace(memberLogins, welcomeNote, policyID, betas) {
             // Convert to object with each key containing the error. We don’t
             // need to remove the members since that is handled by onClose of OfflineWithFeedback.
             value: _.object(
-                logins,
-                Array(logins.length).fill({
+                accountIDs,
+                Array(accountIDs.length).fill({
                     errors: ErrorUtils.getMicroSecondOnyxError('workspace.people.error.genericAdd'),
                 }),
             ),
         },
         ...membersChats.onyxFailureData,
     ];
 
+    const logins = _.map(_.keys(invitedEmailsToAccountIDs), (memberLogin) => OptionsListUtils.addSMSDomainIfPhoneNumber(memberLogin));
     API.write(
         'AddMembersToWorkspace',
         {
@@ -1079,8 +1081,13 @@ function openWorkspaceInvitePage(policyID, clientMemberEmails) {
     });
 }
 
-function setWorkspaceInviteMembersDraft(policyID, memberEmails) {
-    Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MEMBERS_DRAFT}${policyID}`, memberEmails);
+
+/**
+ * @param {String} policyID
+ * @param {Object} invitedEmailsToAccountIDs
+ */
+function setWorkspaceInviteMembersDraft(policyID, invitedEmailsToAccountIDs) {
+    Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MEMBERS_DRAFT}${policyID}`, invitedEmailsToAccountIDs);
 }
 
 export {

src/pages/settings/InitialSettingsPage.js

@@ -99,8 +99,8 @@ const propTypes = {
         errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)),
     }),
 
-    /** List of policy members */
-    policyMembers: PropTypes.objectOf(policyMemberPropType),
+    /** Members keyed by accountID for all policies */
+    allPolicyMembers: PropTypes.objectOf(PropTypes.objectOf(policyMemberPropType)),
 
     ...withLocalizePropTypes,
     ...withCurrentUserPersonalDetailsPropTypes,
@@ -118,7 +118,7 @@ const defaultProps = {
     bankAccountList: {},
     cardList: {},
     loginList: {},
-    policyMembers: {},
+    allPolicyMembers: {},
     ...withCurrentUserPersonalDetailsDefaultProps,
 };
 
@@ -170,7 +170,7 @@ class InitialSettingsPage extends React.Component {
             !_.isEmpty(this.props.reimbursementAccount.errors) ||
             _.chain(this.props.policies)
                 .filter((policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN)
-                .some((policy) => PolicyUtils.hasPolicyError(policy) || PolicyUtils.getPolicyBrickRoadIndicatorStatus(policy, this.props.policyMembers))
+                .some((policy) => PolicyUtils.hasPolicyError(policy) || PolicyUtils.getPolicyBrickRoadIndicatorStatus(policy, this.props.allPolicyMembers))
                 .value()
                 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR
                 : null;
@@ -393,8 +393,8 @@ export default compose(
         policies: {
             key: ONYXKEYS.COLLECTION.POLICY,
         },
-        policyMembers: {
-            key: ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST,
+        allPolicyMembers: {
+            key: ONYXKEYS.COLLECTION.POLICY_MEMBERS,
         },
         userWallet: {
             key: ONYXKEYS.USER_WALLET,

src/pages/workspace/WorkspaceInitialPage.js

@@ -91,7 +91,7 @@ const WorkspaceInitialPage = (props) => {
     );
 
     const policyName = lodashGet(policy, 'name', '');
-    const hasMembersError = PolicyUtils.hasPolicyMemberError(props.policyMemberList);
+    const hasMembersError = PolicyUtils.hasPolicyMemberError(props.policyMembers);
     const hasGeneralSettingsError = !_.isEmpty(lodashGet(policy, 'errorFields.generalSettings', {})) || !_.isEmpty(lodashGet(policy, 'errorFields.avatar', {}));
     const hasCustomUnitsError = PolicyUtils.hasCustomUnitsError(policy);
     const menuItems = [

src/pages/workspace/WorkspaceInviteMessagePage.js

@@ -44,7 +44,7 @@ const propTypes = {
     /** Beta features list */
     betas: PropTypes.arrayOf(PropTypes.string),
 
-    invitedMembersDraft: PropTypes.arrayOf(PropTypes.string),
+    invitedEmailsToAccountIDsDraft: PropTypes.objectOf(PropTypes.number),
 
     /** URL Route params */
     route: PropTypes.shape({
@@ -63,7 +63,7 @@ const defaultProps = {
     ...policyDefaultProps,
     personalDetails: {},
     betas: [],
-    invitedMembersDraft: [],
+    invitedEmailsToAccountIDsDraft: {},
 };
 
 class WorkspaceInviteMessagePage extends React.Component {
@@ -115,7 +115,7 @@ class WorkspaceInviteMessagePage extends React.Component {
     }
 
     sendInvitation() {
-        Policy.addMembersToWorkspace(this.props.invitedMembersDraft, this.state.welcomeNote, this.props.route.params.policyID, this.props.betas);
+        Policy.addMembersToWorkspace(this.props.invitedEmailsToAccountIDsDraft, this.state.welcomeNote, this.props.route.params.policyID, this.props.betas);
         Policy.setWorkspaceInviteMembersDraft(this.props.route.params.policyID, []);
         Navigation.navigate(ROUTES.getWorkspaceMembersRoute(this.props.route.params.policyID));
     }
@@ -131,7 +131,7 @@ class WorkspaceInviteMessagePage extends React.Component {
 
     validate() {
         const errorFields = {};
-        if (_.isEmpty(this.props.invitedMembersDraft)) {
+        if (_.isEmpty(this.props.invitedEmailsToAccountIDsDraft)) {
             errorFields.welcomeMessage = this.props.translate('workspace.inviteMessage.inviteNoMembersError');
         }
         return errorFields;
@@ -178,7 +178,7 @@ class WorkspaceInviteMessagePage extends React.Component {
                         <View style={[styles.mv4, styles.justifyContentCenter, styles.alignItemsCenter]}>
                             <MultipleAvatars
                                 size={CONST.AVATAR_SIZE.LARGE}
-                                icons={OptionsListUtils.getAvatarsForLogins(this.props.invitedMembersDraft, this.props.personalDetails)}
+                                icons={OptionsListUtils.getAvatarsForLogins(this.props.invitedEmailsToAccountIDsDraft, this.props.personalDetails)}
                                 shouldStackHorizontally
                                 secondAvatarStyle={[styles.secondAvatarInline]}
                             />
@@ -221,7 +221,7 @@ export default compose(
         betas: {
             key: ONYXKEYS.BETAS,
         },
-        invitedMembersDraft: {
+        invitedEmailsToAccountIDsDraft: {
             key: ({route}) => `${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MEMBERS_DRAFT}${route.params.policyID.toString()}`,
         },
     }),