From 60c3bec474fce32a0234588048611b7cb0265afa Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 11:52:47 -0400 Subject: [PATCH 01/12] load policy employee list into new onyx key --- src/libs/actions/Policy.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 9470ea25f4e0..beac6b585f48 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -33,6 +33,8 @@ Onyx.connect({ }, }); +const defaultEmployeeListEntry = () => ({pendingAction: null}); + /** * Simplifies the employeeList response into an object containing an array of emails * @@ -46,7 +48,9 @@ function getSimplifiedEmployeeList(employeeList) { .unique() .value(); - return employeeListEmails; + const result = {}; + _.each(employeeListEmails, email => result[email] = defaultEmployeeListEntry()); + return result; } /** @@ -92,7 +96,6 @@ function getSimplifiedPolicyObject(fullPolicyOrPolicySummary, isFromFullPolicy) // "GetFullPolicy" and "GetPolicySummaryList" returns different policy objects. If policy is retrieved by "GetFullPolicy", // avatarUrl will be nested within the key "value" avatarURL: fullPolicyOrPolicySummary.avatarURL || lodashGet(fullPolicyOrPolicySummary, 'value.avatarURL', ''), - employeeList: getSimplifiedEmployeeList(lodashGet(fullPolicyOrPolicySummary, 'value.employeeList')), customUnit: customUnitSimplified, }; } @@ -259,10 +262,8 @@ function loadFullPolicy(policyID) { return; } - Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, { - ...allPolicies[`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`], - ...getSimplifiedPolicyObject(policy, true), - }); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, getSimplifiedPolicyObject(policy, true)); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policy.id}`, getSimplifiedEmployeeList(lodashGet(policy, 'value.employeeList', {}))); }); } From 097f78e62db7489daea94ae271fb891a4dd37e12 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:08:48 -0400 Subject: [PATCH 02/12] connect policyMemberList in withFullPolicy --- src/pages/workspace/withFullPolicy.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/pages/workspace/withFullPolicy.js b/src/pages/workspace/withFullPolicy.js index 96f191245700..228254d97ab7 100644 --- a/src/pages/workspace/withFullPolicy.js +++ b/src/pages/workspace/withFullPolicy.js @@ -35,7 +35,7 @@ function isPreviousRouteInSameWorkspace(routeName, policyID) { } const fullPolicyPropTypes = { - /** The full policy object for the current route (as opposed to the policy summary object) */ + /** The policy object for the current route */ policy: PropTypes.shape({ /** The ID of the policy */ id: PropTypes.string, @@ -57,10 +57,13 @@ const fullPolicyPropTypes = { /** The URL for the policy avatar */ avatarURL: PropTypes.string, - - /** A list of emails for the employees on the policy */ - employeeList: PropTypes.arrayOf(PropTypes.string), }), + + /** The policy member list for the current route */ + policyMemberList: PropTypes.objectOf(PropTypes.shape({ + /** The pending user action for the policy member if there is one */ + pendingAction: PropTypes.string, + })), }; const fullPolicyDefaultProps = { @@ -98,13 +101,14 @@ export default function (WrappedComponent) { previousRouteName = currentRoute.name; previousRoutePolicyID = policyID; - const rest = _.omit(props, ['forwardedRef', 'policy']); + const rest = _.omit(props, ['forwardedRef', 'policy', 'policyMemberList']); return ( ); }; @@ -121,6 +125,9 @@ export default function (WrappedComponent) { policy: { key: props => `${ONYXKEYS.COLLECTION.POLICY}${getPolicyIDFromRoute(props.route)}`, }, + policyMemberList: { + key: props => `${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${getPolicyIDFromRoute(props.route)}`, + }, })(withFullPolicy); } From 0712c4a3d8e9b73404ca6e63aac8cc385e854ca7 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:10:27 -0400 Subject: [PATCH 03/12] use policyMemberList in WorkspaceMembersPage --- src/pages/workspace/WorkspaceMembersPage.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index 8f10c91b1992..449f7748b7d6 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -106,7 +106,8 @@ class WorkspaceMembersPage extends React.Component { */ toggleAllUsers() { this.setState({showTooltipForLogin: ''}); - const removableMembers = _.without(this.props.policy.employeeList, this.props.session.email, this.props.policy.owner); + const policyMemberList = _.keys(lodashGet(this.props, 'policyMemberList', {})); + const removableMembers = _.without(policyMemberList, this.props.session.email, this.props.policy.owner); this.setState(prevState => ({ selectedEmployees: removableMembers.length !== prevState.selectedEmployees.length ? removableMembers @@ -263,9 +264,9 @@ class WorkspaceMembersPage extends React.Component { } render() { - const policyEmployeeList = lodashGet(this.props, 'policy.employeeList', []); + const policyMemberList = _.keys(lodashGet(this.props, 'policyMemberList', {})); const removableMembers = _.without(this.props.policy.employeeList, this.props.session.email, this.props.policy.owner); - const data = _.chain(policyEmployeeList) + const data = _.chain(policyMemberList) .map(email => this.props.personalDetails[email]) .filter() .sortBy(person => person.displayName.toLowerCase()) From c134503a956b2d7a9dc627876afad6c0759a2220 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:12:49 -0400 Subject: [PATCH 04/12] use policyMemberList in WorkspaceInvitePage --- src/pages/workspace/WorkspaceInvitePage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/WorkspaceInvitePage.js b/src/pages/workspace/WorkspaceInvitePage.js index ae2c8627e1e9..bf44055c9c9b 100644 --- a/src/pages/workspace/WorkspaceInvitePage.js +++ b/src/pages/workspace/WorkspaceInvitePage.js @@ -87,8 +87,8 @@ class WorkspaceInvitePage extends React.Component { } getExcludedUsers() { - const policyEmployeeList = lodashGet(this.props, 'policy.employeeList', []); - return [...CONST.EXPENSIFY_EMAILS, ...policyEmployeeList]; + const policyMemberList = _.keys(lodashGet(this.props, 'policyMemberList', {})); + return [...CONST.EXPENSIFY_EMAILS, ...policyMemberList]; } /** From 2a7ef4847804d261d478c4b69fed5549129a14e6 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:47:01 -0400 Subject: [PATCH 05/12] use new member list key when creating policies --- src/libs/actions/Policy.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index beac6b585f48..42c412bc36b7 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -146,14 +146,16 @@ function create(name = '') { Report.fetchChatReportsByIDs([response.policy.chatReportIDAdmins, response.policy.chatReportIDAnnounce, response.ownerPolicyExpenseChatID]); // We are awaiting this merge so that we can guarantee our policy is available to any React components connected to the policies collection before we navigate to a new route. - return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${response.policyID}`, { - employeeList: getSimplifiedEmployeeList(response.policy.employeeList), - id: response.policyID, - type: response.policy.type, - name: response.policy.name, - role: CONST.POLICY.ROLE.ADMIN, - outputCurrency: response.policy.outputCurrency, - }); + return Promise.all( + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${response.policyID}`, { + id: response.policyID, + type: response.policy.type, + name: response.policy.name, + role: CONST.POLICY.ROLE.ADMIN, + outputCurrency: response.policy.outputCurrency, + }), + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${response.policyID}`, getSimplifiedEmployeeList(response.policy.employeeList)), + ); }) .then(() => Promise.resolve(lodashGet(res, 'policyID'))); } From 94f4aee9a543df5779110c393212aa2fc603266e Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:48:36 -0400 Subject: [PATCH 06/12] use new member list key when removing members --- src/libs/actions/Policy.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 42c412bc36b7..edd0f002f5d5 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -294,14 +294,9 @@ function removeMembers(members, policyID) { return; } - const key = `${ONYXKEYS.COLLECTION.POLICY}${policyID}`; - - // Make a shallow copy to preserve original data and remove the members - const policy = _.clone(allPolicies[key]); - policy.employeeList = _.without(policy.employeeList, ...members); - - // Optimistically remove the members from the policy - Onyx.set(key, policy); + const employeeListUpdate = {}; + _.each(members, login => employeeListUpdate[login] = null); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeListUpdate); // Make the API call to remove a login from the policy DeprecatedAPI.Policy_Employees_Remove({ @@ -312,9 +307,10 @@ function removeMembers(members, policyID) { if (data.jsonCode === 200) { return; } - const policyDataWithMembersRemoved = _.clone(allPolicies[key]); - policyDataWithMembersRemoved.employeeList = [...policyDataWithMembersRemoved.employeeList, ...members]; - Onyx.set(key, policyDataWithMembersRemoved); + + // Rollback removal on failure + _.each(members, login => employeeListUpdate[login] = defaultEmployeeListEntry()); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeListUpdate); // Show the user feedback that the removal failed const errorMessage = data.jsonCode === 666 ? data.message : Localize.translateLocal('workspace.people.genericFailureMessage'); From cd5e71f62ef4d283d084ec0e44d1243752fe5e61 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 12:52:20 -0400 Subject: [PATCH 07/12] use new member list key when inviting members --- src/libs/actions/Policy.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index edd0f002f5d5..e5675ae845d6 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -326,16 +326,15 @@ function removeMembers(members, policyID) { * @param {String} policyID */ function invite(logins, welcomeNote, policyID) { - const key = `${ONYXKEYS.COLLECTION.POLICY}${policyID}`; - const newEmployeeList = _.map(logins, login => OptionsListUtils.addSMSDomainIfPhoneNumber(login)); - - // Make a shallow copy to preserve original data, and concat the login - const policy = _.clone(allPolicies[key]); - policy.employeeList = [...policy.employeeList, ...newEmployeeList]; - policy.alertMessage = ''; + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { + alertMessage: '', + }); // Optimistically add the user to the policy - Onyx.merge(key, policy); + const newEmployeeLogins = _.map(logins, login => OptionsListUtils.addSMSDomainIfPhoneNumber(login)); + const employeeUpdate = {}; + _.each(newEmployeeLogins, login => employeeUpdate[login] = defaultEmployeeListEntry()); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeUpdate); // Make the API call to merge the login into the policy DeprecatedAPI.Policy_Employees_Merge({ @@ -355,16 +354,15 @@ function invite(logins, welcomeNote, policyID) { } // If the operation failed, undo the optimistic addition - const policyDataWithoutLogin = _.clone(allPolicies[key]); - policyDataWithoutLogin.employeeList = _.without(allPolicies[key].employeeList, ...newEmployeeList); + _.each(newEmployeeLogins, login => employeeUpdate[login] = null); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeUpdate); // Show the user feedback that the addition failed - policyDataWithoutLogin.alertMessage = Localize.translateLocal('workspace.invite.genericFailureMessage'); + let alertMessage = Localize.translateLocal('workspace.invite.genericFailureMessage'); if (data.jsonCode === 402) { - policyDataWithoutLogin.alertMessage += ` ${Localize.translateLocal('workspace.invite.pleaseEnterValidLogin')}`; + alertMessage += ` ${Localize.translateLocal('workspace.invite.pleaseEnterValidLogin')}`; } - - Onyx.set(key, policyDataWithoutLogin); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {alertMessage}); }); } From f775f697ec91a200161a815486c336081786323f Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 13:28:26 -0400 Subject: [PATCH 08/12] use existing policyMemberPropType --- src/pages/workspace/withFullPolicy.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/workspace/withFullPolicy.js b/src/pages/workspace/withFullPolicy.js index 228254d97ab7..6e935f4717b3 100644 --- a/src/pages/workspace/withFullPolicy.js +++ b/src/pages/workspace/withFullPolicy.js @@ -9,6 +9,7 @@ import CONST from '../../CONST'; import getComponentDisplayName from '../../libs/getComponentDisplayName'; import * as Policy from '../../libs/actions/Policy'; import ONYXKEYS from '../../ONYXKEYS'; +import policyMemberPropType from '../policyMemberPropType'; let previousRouteName = ''; let previousRoutePolicyID = ''; @@ -60,10 +61,7 @@ const fullPolicyPropTypes = { }), /** The policy member list for the current route */ - policyMemberList: PropTypes.objectOf(PropTypes.shape({ - /** The pending user action for the policy member if there is one */ - pendingAction: PropTypes.string, - })), + policyMemberList: PropTypes.objectOf(policyMemberPropType), }; const fullPolicyDefaultProps = { From 78730c6d2deb43d7c236def77b3363d2f87d2dd6 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 17:08:31 -0400 Subject: [PATCH 09/12] use policyMemberList for removableMembers --- src/pages/workspace/WorkspaceMembersPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index 449f7748b7d6..eca3a2bcbe5f 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -265,7 +265,7 @@ class WorkspaceMembersPage extends React.Component { render() { const policyMemberList = _.keys(lodashGet(this.props, 'policyMemberList', {})); - const removableMembers = _.without(this.props.policy.employeeList, this.props.session.email, this.props.policy.owner); + const removableMembers = _.without(policyMemberList, this.props.session.email, this.props.policy.owner); const data = _.chain(policyMemberList) .map(email => this.props.personalDetails[email]) .filter() From f95bd12e6dccbdec7a406f1a89d63ba840af926e Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 19 Aug 2022 17:09:30 -0400 Subject: [PATCH 10/12] updated getSimplifiedEmployeeList jsdoc --- src/libs/actions/Policy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index e5675ae845d6..3063606272e2 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -36,10 +36,10 @@ Onyx.connect({ const defaultEmployeeListEntry = () => ({pendingAction: null}); /** - * Simplifies the employeeList response into an object containing an array of emails + * Simplifies the employeeList response into an object mapping employee email to a default employee list entry * * @param {Object} employeeList - * @returns {Array} + * @returns {Object} */ function getSimplifiedEmployeeList(employeeList) { const employeeListEmails = _.chain(employeeList) From ca233bf553dcbb18c9f1988b8e2cacbf1b70fa9b Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Tue, 23 Aug 2022 12:05:26 -0400 Subject: [PATCH 11/12] use empty object for default employee list entry --- src/libs/actions/Policy.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 3063606272e2..2e84ed1cf5cc 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -33,8 +33,6 @@ Onyx.connect({ }, }); -const defaultEmployeeListEntry = () => ({pendingAction: null}); - /** * Simplifies the employeeList response into an object mapping employee email to a default employee list entry * @@ -49,7 +47,7 @@ function getSimplifiedEmployeeList(employeeList) { .value(); const result = {}; - _.each(employeeListEmails, email => result[email] = defaultEmployeeListEntry()); + _.each(employeeListEmails, email => result[email] = {}); return result; } @@ -309,7 +307,7 @@ function removeMembers(members, policyID) { } // Rollback removal on failure - _.each(members, login => employeeListUpdate[login] = defaultEmployeeListEntry()); + _.each(members, login => employeeListUpdate[login] = {}); Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeListUpdate); // Show the user feedback that the removal failed @@ -333,7 +331,7 @@ function invite(logins, welcomeNote, policyID) { // Optimistically add the user to the policy const newEmployeeLogins = _.map(logins, login => OptionsListUtils.addSMSDomainIfPhoneNumber(login)); const employeeUpdate = {}; - _.each(newEmployeeLogins, login => employeeUpdate[login] = defaultEmployeeListEntry()); + _.each(newEmployeeLogins, login => employeeUpdate[login] = {}); Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`, employeeUpdate); // Make the API call to merge the login into the policy From f05e06c1cd1a1213833e923f6fc5c202414de4da Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Tue, 23 Aug 2022 12:19:30 -0400 Subject: [PATCH 12/12] simplify getSimplifiedEmployeList --- src/libs/actions/Policy.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 2e84ed1cf5cc..1d0c5c596a4c 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -40,15 +40,12 @@ Onyx.connect({ * @returns {Object} */ function getSimplifiedEmployeeList(employeeList) { - const employeeListEmails = _.chain(employeeList) + return _.chain(employeeList) .pluck('email') .flatten() .unique() + .reduce((map, email) => ({...map, [email]: {}}), {}) .value(); - - const result = {}; - _.each(employeeListEmails, email => result[email] = {}); - return result; } /**