From 917eb35f5d3c367c42508048debb0b62b2fb7582 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Thu, 18 Aug 2022 20:28:09 +0200 Subject: [PATCH 01/18] Remove unnneded code for DeleteMembersFromWorkspace --- src/libs/actions/Policy.js | 59 ++++++++++++++------------------------ 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 9470ea25f4e0..dbe6c94156b7 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -2,6 +2,7 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; import lodashGet from 'lodash/get'; import * as DeprecatedAPI from '../deprecatedAPI'; +import * as API from '../API'; import ONYXKEYS from '../../ONYXKEYS'; import * as PersonalDetails from './PersonalDetails'; import Growl from '../Growl'; @@ -13,6 +14,7 @@ import ROUTES from '../../ROUTES'; import * as OptionsListUtils from '../OptionsListUtils'; import * as Report from './Report'; import * as Pusher from '../Pusher/pusher'; +import DateUtils from '../DateUtils'; const allPolicies = {}; Onyx.connect({ @@ -290,33 +292,21 @@ function removeMembers(members, policyID) { if (members.length === 0) { 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); - - // Make the API call to remove a login from the policy - DeprecatedAPI.Policy_Employees_Remove({ + const membersListKey = `${ONYXKEYS.COLLECTION.POLICY_MEMBER_LIST}${policyID}`; + const optimisticData = [{ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: membersListKey, + value: _.object(members, Array(members.length).fill({pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE})), + }]; + const failureData = [{ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: membersListKey, + value: _.object(members, Array(members.length).fill({errors: {[DateUtils.getMicroseconds()]: 'There was a priblem removing that workspace member'}})), + }]; + API.write('DeleteMembersFromWorkspace', { emailList: members.join(','), policyID, - }) - .then((data) => { - if (data.jsonCode === 200) { - return; - } - const policyDataWithMembersRemoved = _.clone(allPolicies[key]); - policyDataWithMembersRemoved.employeeList = [...policyDataWithMembersRemoved.employeeList, ...members]; - Onyx.set(key, policyDataWithMembersRemoved); - - // Show the user feedback that the removal failed - const errorMessage = data.jsonCode === 666 ? data.message : Localize.translateLocal('workspace.people.genericFailureMessage'); - Growl.show(errorMessage, CONST.GROWL.ERROR, 5000); - }); + }, {optimisticData, failureData}); } /** @@ -519,21 +509,14 @@ function updateLastAccessedWorkspace(policyID) { function subscribeToPolicyEvents() { _.each(allPolicies, (policy) => { const pusherChannelName = `public-policyEditor-${policy.id}${CONFIG.PUSHER.SUFFIX}`; - Pusher.subscribe(pusherChannelName, 'policyEmployeeRemoved', ({removedEmails, policyExpenseChatIDs, defaultRoomChatIDs}) => { - // Refetch the policy expense chats to update their state and their actions to get the archive reason - if (!_.isEmpty(policyExpenseChatIDs)) { - Report.fetchChatReportsByIDs(policyExpenseChatIDs); - _.each(policyExpenseChatIDs, (reportID) => { - Report.fetchInitialActions(reportID); - }); - } - + Pusher.subscribe(pusherChannelName, 'policyEmployeeRemoved', ({removedEmails, defaultRoomChatIDs}) => { // Remove the default chats if we are one of the users getting removed - if (removedEmails.includes(sessionEmail) && !_.isEmpty(defaultRoomChatIDs)) { - _.each(defaultRoomChatIDs, (chatID) => { - Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${chatID}`, null); - }); + if (!removedEmails.includes(sessionEmail) || _.isEmpty(defaultRoomChatIDs)) { + return; } + _.each(defaultRoomChatIDs, (chatID) => { + Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${chatID}`, null); + }); }); }); } From 8a680a764a53d54011a6ae808d1112221d18b273 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Fri, 2 Sep 2022 16:32:34 +0200 Subject: [PATCH 02/18] Use employeeList in members page --- src/CONST.js | 1 + src/libs/actions/Policy.js | 2 +- src/pages/workspace/WorkspaceMembersPage.js | 21 +++++++++++++-------- src/pages/workspace/withFullPolicy.js | 4 ---- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index 0a4f6cfe0475..9b1ce730eab0 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -662,6 +662,7 @@ const CONST = { FREE: 'free', PERSONAL: 'personal', CORPORATE: 'corporate', + TEAM: 'team', }, ROLE: { ADMIN: 'admin', diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index c869f14767c8..59542a7ff05f 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -291,7 +291,7 @@ function removeMembers(members, policyID) { const failureData = [{ onyxMethod: CONST.ONYX.METHOD.MERGE, key: membersListKey, - value: _.object(members, Array(members.length).fill({errors: {[DateUtils.getMicroseconds()]: 'There was a priblem removing that workspace member'}})), + value: _.object(members, Array(members.length).fill({errors: {[DateUtils.getMicroseconds()]: 'There was a problem removing that workspace member'}})), }]; API.write('DeleteMembersFromWorkspace', { emailList: members.join(','), diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index f687d964ab36..8d94e5416e11 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -264,14 +264,19 @@ class WorkspaceMembersPage extends React.Component { } render() { - const policyMemberList = _.keys(lodashGet(this.props, 'policyMemberList', {})); - const removableMembers = _.without(policyMemberList, this.props.session.email, this.props.policy.owner); - const data = _.chain(policyMemberList) - .map(email => this.props.personalDetails[email]) - .filter() - .sortBy(person => person.displayName.toLowerCase()) - .map(person => ({...person})) // TODO: here we will add the pendingAction and errors prop - .value(); + const policyMemberList = lodashGet(this.props, 'policyMemberList', {}); + const removableMembers = []; + let data = _.map(policyMemberList, (value, email) => { + if (email !== this.props.session.email && email !== this.props.policy.owner) { + removableMembers.push(email); + } + const details = this.props.personalDetails[email]; + return { + ...value, + ...details, + }; + }); + data = _.sortBy(data, value => value.displayName.toLowerCase()); const policyID = lodashGet(this.props.route, 'params.policyID'); const policyName = lodashGet(this.props.policy, 'name'); diff --git a/src/pages/workspace/withFullPolicy.js b/src/pages/workspace/withFullPolicy.js index 224fdaf77364..ef7fb93efb55 100644 --- a/src/pages/workspace/withFullPolicy.js +++ b/src/pages/workspace/withFullPolicy.js @@ -9,7 +9,6 @@ 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 = ''; @@ -76,9 +75,6 @@ const fullPolicyPropTypes = { */ errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)), }), - - /** The policy member list for the current route */ - policyMemberList: PropTypes.objectOf(policyMemberPropType), }; const fullPolicyDefaultProps = { From 1d21907c8e4bb493e5bab5a9c0c7239734490288 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Wed, 7 Sep 2022 16:13:10 +0100 Subject: [PATCH 03/18] Transalate error --- src/languages/en.js | 1 + src/languages/es.js | 1 + src/libs/actions/Policy.js | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/languages/en.js b/src/languages/en.js index 00d6f9e6f9d3..0da6da2acdba 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -809,6 +809,7 @@ export default { selectAll: 'Select all', error: { cannotRemove: 'You cannot remove yourself or the workspace owner.', + genericRemove: 'There was a problem removing that workspace member.', }, }, card: { diff --git a/src/languages/es.js b/src/languages/es.js index 75554f7c91e8..910987b40a11 100644 --- a/src/languages/es.js +++ b/src/languages/es.js @@ -811,6 +811,7 @@ export default { selectAll: 'Seleccionar todo', error: { cannotRemove: 'No puedes eliminarte ni a ti mismo ni al dueño del espacio de trabajo.', + genericRemove: 'Ha ocurrido un problema al eliminar al miembro del espacio de trabajo.', }, }, card: { diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 3d07f2ae0bdc..79a93bbe8cf7 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -226,7 +226,7 @@ function removeMembers(members, policyID) { const failureData = [{ onyxMethod: CONST.ONYX.METHOD.MERGE, key: membersListKey, - value: _.object(members, Array(members.length).fill({errors: {[DateUtils.getMicroseconds()]: 'There was a problem removing that workspace member'}})), + value: _.object(members, Array(members.length).fill({errors: {[DateUtils.getMicroseconds()]: Localize.translateLocal('workspace.people.error.genericRemove')}})), }]; API.write('DeleteMembersFromWorkspace', { emailList: members.join(','), From b9f7705dec21db3f83dbfc44081cc4067cd6341a Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 13 Sep 2022 15:38:11 +0200 Subject: [PATCH 04/18] Update to new Onyx version and fix editing comments --- package-lock.json | 16 ++++++++-------- package.json | 2 +- src/libs/actions/Report.js | 1 + 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index 50eaad994c61..205a27309850 100644 --- a/package-lock.json +++ b/package-lock.json @@ -68,7 +68,7 @@ "react-native-image-picker": "^4.8.5", "react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972", "react-native-modal": "^13.0.0", - "react-native-onyx": "1.0.15", + "react-native-onyx": "1.0.16", "react-native-pdf": "^6.6.2", "react-native-performance": "^2.0.0", "react-native-permissions": "^3.0.1", @@ -33881,9 +33881,9 @@ } }, "node_modules/react-native-onyx": { - "version": "1.0.15", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.15.tgz", - "integrity": "sha512-uIJped+agmOppnCoDcs/w3qFertkLhLHyhmEEBXp0OhzNKuCs01wg7ccYFZxOVv+CtzFbtkLVB1VW3Ty/zYogA==", + "version": "1.0.16", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.16.tgz", + "integrity": "sha512-JAe5mae6PTLtBfgLGUZUawifwkoJAzzv8KoIoL0jiErUfZA1E5rc+vlRFaSsr/jHVjeDJZvabQtNkkHwu3USeA==", "dependencies": { "ascii-table": "0.0.9", "lodash": "^4.17.21", @@ -44475,7 +44475,7 @@ "@oguzhnatly/react-native-image-manipulator": { "version": "git+ssh://git@github.com/Expensify/react-native-image-manipulator.git#c5f654fc9d0ad7cc5b89d50b34ecf8b0e3f4d050", "integrity": "sha512-PvrSoCq5PS1MA5ZWUpB0khfzH6sM8SI6YiVl4i2SItPr7IeRxiWfI4n45VhBCCElc1z5GhAwTZOBaIzXTX7/og==", - "from": "@oguzhnatly/react-native-image-manipulator@https://github.com/Expensify/react-native-image-manipulator#c5f654fc9d0ad7cc5b89d50b34ecf8b0e3f4d050" + "from": "@oguzhnatly/react-native-image-manipulator@github:Expensify/react-native-image-manipulator#c5f654fc9d0ad7cc5b89d50b34ecf8b0e3f4d050" }, "@onfido/active-video-capture": { "version": "0.0.1", @@ -67592,9 +67592,9 @@ } }, "react-native-onyx": { - "version": "1.0.15", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.15.tgz", - "integrity": "sha512-uIJped+agmOppnCoDcs/w3qFertkLhLHyhmEEBXp0OhzNKuCs01wg7ccYFZxOVv+CtzFbtkLVB1VW3Ty/zYogA==", + "version": "1.0.16", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.16.tgz", + "integrity": "sha512-JAe5mae6PTLtBfgLGUZUawifwkoJAzzv8KoIoL0jiErUfZA1E5rc+vlRFaSsr/jHVjeDJZvabQtNkkHwu3USeA==", "requires": { "ascii-table": "0.0.9", "lodash": "^4.17.21", diff --git a/package.json b/package.json index 85672217086e..f198ca3f0de6 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,7 @@ "react-native-image-picker": "^4.8.5", "react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972", "react-native-modal": "^13.0.0", - "react-native-onyx": "1.0.15", + "react-native-onyx": "1.0.16", "react-native-pdf": "^6.6.2", "react-native-performance": "^2.0.0", "react-native-permissions": "^3.0.1", diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 105ca3964226..2b089ef8f65d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1263,6 +1263,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) { isEdited: true, html: htmlForNewComment, text: textForNewComment, + type: originalReportAction.message[0].type, }], }, }; From 29ddff3515db4ba105b37dbe84838b267c409457 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 13 Sep 2022 15:57:47 +0200 Subject: [PATCH 05/18] Fix active clients --- src/libs/ActiveClientManager/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index 7f0d4bf0cd91..4cfb845d8352 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -7,7 +7,7 @@ import * as ActiveClients from '../actions/ActiveClients'; const clientID = Str.guid(); const maxClients = 20; -let activeClients; +let activeClients = []; let resolveIsReadyPromise; const isReadyPromise = new Promise((resolve) => { @@ -24,7 +24,9 @@ function isReady() { Onyx.connect({ key: ONYXKEYS.ACTIVE_CLIENTS, callback: (val) => { - activeClients = !val ? [] : val; + if (val) { + activeClients = _.unique(activeClients.concat(val)); + } if (activeClients.length >= maxClients) { activeClients.shift(); ActiveClients.setActiveClients(activeClients); From f92c03aad959e1354c1afa04930db3db7df196da Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 13 Sep 2022 18:32:54 +0200 Subject: [PATCH 06/18] Address review comments --- src/libs/ActiveClientManager/index.js | 6 +++--- src/pages/workspace/WorkspaceMembersPage.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index 4cfb845d8352..66ed0fb22b37 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -7,7 +7,7 @@ import * as ActiveClients from '../actions/ActiveClients'; const clientID = Str.guid(); const maxClients = 20; -let activeClients = []; +const activeClients = []; let resolveIsReadyPromise; const isReadyPromise = new Promise((resolve) => { @@ -24,8 +24,8 @@ function isReady() { Onyx.connect({ key: ONYXKEYS.ACTIVE_CLIENTS, callback: (val) => { - if (val) { - activeClients = _.unique(activeClients.concat(val)); + if (val && activeClients.indexOf(val) < 0) { + activeClients.push(val); } if (activeClients.length >= maxClients) { activeClients.shift(); diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index 0f1e11a44df4..d1bf775cf8a0 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -285,13 +285,13 @@ class WorkspaceMembersPage extends React.Component { render() { const policyMemberList = lodashGet(this.props, 'policyMemberList', {}); const removableMembers = []; - let data = _.map(policyMemberList, (value, email) => { + let data = _.map(policyMemberList, (policyMember, email) => { if (email !== this.props.session.email && email !== this.props.policy.owner) { removableMembers.push(email); } const details = this.props.personalDetails[email]; return { - ...value, + ...policyMember, ...details, }; }); From 57df98a38660c31b79e892b8bc6d6ee6b3d4873b Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Thu, 15 Sep 2022 15:15:35 +0200 Subject: [PATCH 07/18] Use each instead of map --- 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 d1bf775cf8a0..cb5111d42243 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -285,15 +285,16 @@ class WorkspaceMembersPage extends React.Component { render() { const policyMemberList = lodashGet(this.props, 'policyMemberList', {}); const removableMembers = []; - let data = _.map(policyMemberList, (policyMember, email) => { + let data = []; + _.each(policyMemberList, (policyMember, email) => { if (email !== this.props.session.email && email !== this.props.policy.owner) { removableMembers.push(email); } const details = this.props.personalDetails[email]; - return { + data.push({ ...policyMember, ...details, - }; + }); }); data = _.sortBy(data, value => value.displayName.toLowerCase()); const policyID = lodashGet(this.props.route, 'params.policyID'); From 207d66f15f60172c30ecd08edf639dd471ed7410 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Fri, 16 Sep 2022 21:28:17 +0200 Subject: [PATCH 08/18] Fix activeClients bug and inviting new users --- package-lock.json | 14 +++--- package.json | 2 +- src/libs/ActiveClientManager/index.js | 53 ++++++++++++++++----- src/libs/actions/ActiveClients.js | 12 +---- src/pages/workspace/WorkspaceMembersPage.js | 2 +- 5 files changed, 51 insertions(+), 32 deletions(-) diff --git a/package-lock.json b/package-lock.json index 08ecc6578f77..75abce6d59ea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -68,7 +68,7 @@ "react-native-image-picker": "^4.8.5", "react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972", "react-native-modal": "^13.0.0", - "react-native-onyx": "1.0.16", + "react-native-onyx": "1.0.17", "react-native-pdf": "^6.6.2", "react-native-performance": "^2.0.0", "react-native-permissions": "^3.0.1", @@ -34354,9 +34354,9 @@ } }, "node_modules/react-native-onyx": { - "version": "1.0.16", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.16.tgz", - "integrity": "sha512-JAe5mae6PTLtBfgLGUZUawifwkoJAzzv8KoIoL0jiErUfZA1E5rc+vlRFaSsr/jHVjeDJZvabQtNkkHwu3USeA==", + "version": "1.0.17", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.17.tgz", + "integrity": "sha512-ls2GjURfpBcGnIkwVrg2uuLnTBwd0vrEiUvbMo+GF3k81GAp2flCkVTM7ciAbo155Izk50dm0uXHYq1PIjwTxw==", "dependencies": { "ascii-table": "0.0.9", "lodash": "^4.17.21", @@ -68399,9 +68399,9 @@ } }, "react-native-onyx": { - "version": "1.0.16", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.16.tgz", - "integrity": "sha512-JAe5mae6PTLtBfgLGUZUawifwkoJAzzv8KoIoL0jiErUfZA1E5rc+vlRFaSsr/jHVjeDJZvabQtNkkHwu3USeA==", + "version": "1.0.17", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-1.0.17.tgz", + "integrity": "sha512-ls2GjURfpBcGnIkwVrg2uuLnTBwd0vrEiUvbMo+GF3k81GAp2flCkVTM7ciAbo155Izk50dm0uXHYq1PIjwTxw==", "requires": { "ascii-table": "0.0.9", "lodash": "^4.17.21", diff --git a/package.json b/package.json index 44a6c522d857..8db5c3cd0038 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,7 @@ "react-native-image-picker": "^4.8.5", "react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972", "react-native-modal": "^13.0.0", - "react-native-onyx": "1.0.16", + "react-native-onyx": "1.0.17", "react-native-pdf": "^6.6.2", "react-native-performance": "^2.0.0", "react-native-permissions": "^3.0.1", diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index 66ed0fb22b37..18b5a42c5d87 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -1,3 +1,9 @@ +/** + * When you have many tabs in one browser, the data pf Onyx is shared between all of them. Since we persist write requests in Onyx, we need to ensure that + * only one tab is processing those saved requests or we would be duplicating data (or creating errors). + * This file ensures exactly that by tracking all the clientIDs connected, storing the most recent one last and it considers that last clientID the "leader". + */ + import _ from 'underscore'; import Onyx from 'react-native-onyx'; import Str from 'expensify-common/lib/str'; @@ -6,31 +12,53 @@ import * as ActiveClients from '../actions/ActiveClients'; const clientID = Str.guid(); const maxClients = 20; - -const activeClients = []; - -let resolveIsReadyPromise; -const isReadyPromise = new Promise((resolve) => { - resolveIsReadyPromise = resolve; +let addedSelf = false; +let activeClients = []; +let resolveInitedPromise; +const initedPromise = new Promise((resolve) => { + resolveInitedPromise = resolve; +}); +let resolveSavedSelfPromise; +const savedSelfPromise = new Promise((resolve) => { + resolveSavedSelfPromise = resolve; }); /** + * Determines when the client is ready. We need to wait both till we saved our ID in onyx AND the init method was called * @returns {Promise} */ function isReady() { - return isReadyPromise; + return Promise.all([initedPromise, savedSelfPromise]); } Onyx.connect({ key: ONYXKEYS.ACTIVE_CLIENTS, callback: (val) => { - if (val && activeClients.indexOf(val) < 0) { - activeClients.push(val); + // We only add this client once, ensuring it is the last in the array (that determines who is leader) + if (!addedSelf) { + activeClients = _.without(val, clientID); + activeClients.push(clientID); + ActiveClients.setActiveClients(activeClients); } - if (activeClients.length >= maxClients) { + + // Remove from the beginning of the list any clients that are past the limit, to avoid having thousands of them + let removed = false; + while (activeClients.length >= maxClients) { activeClients.shift(); - ActiveClients.setActiveClients(activeClients); + removed = true; + } + + // Save the clients back to onyx, if they changed + if (removed || !addedSelf) { + ActiveClients.setActiveClients(activeClients).then(() => { + // If we just added ourselves, we need to resolve the promise so that isReady fires + if (addedSelf) { + return; + } + resolveSavedSelfPromise(); + }); } + addedSelf = true; }, }); @@ -38,8 +66,7 @@ Onyx.connect({ * Add our client ID to the list of active IDs */ function init() { - ActiveClients.addClient(clientID) - .then(resolveIsReadyPromise); + resolveInitedPromise(); } /** diff --git a/src/libs/actions/ActiveClients.js b/src/libs/actions/ActiveClients.js index 2a3689b2c099..80b82ee38653 100644 --- a/src/libs/actions/ActiveClients.js +++ b/src/libs/actions/ActiveClients.js @@ -5,18 +5,10 @@ import ONYXKEYS from '../../ONYXKEYS'; * @param {Array} activeClients */ function setActiveClients(activeClients) { - Onyx.set(ONYXKEYS.ACTIVE_CLIENTS, activeClients); -} - -/** - * @param {Number} clientID - * @returns {Promise} - */ -function addClient(clientID) { - return Onyx.merge(ONYXKEYS.ACTIVE_CLIENTS, [clientID]); + return Onyx.set(ONYXKEYS.ACTIVE_CLIENTS, activeClients); } export { + // eslint-disable-next-line import/prefer-default-export setActiveClients, - addClient, }; diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index 54d7be95fc31..d324ab6a3a0e 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -290,7 +290,7 @@ class WorkspaceMembersPage extends React.Component { if (email !== this.props.session.email && email !== this.props.policy.owner) { removableMembers.push(email); } - const details = this.props.personalDetails[email]; + const details = this.props.personalDetails[email] || {displayName: email, login: email}; data.push({ ...policyMember, ...details, From 74654d5210e0b1098fd9ade35e08e1e28e2f1be7 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Mon, 19 Sep 2022 17:31:21 +0200 Subject: [PATCH 09/18] Lint --- src/libs/actions/ActiveClients.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/ActiveClients.js b/src/libs/actions/ActiveClients.js index 80b82ee38653..744944cfef1c 100644 --- a/src/libs/actions/ActiveClients.js +++ b/src/libs/actions/ActiveClients.js @@ -3,6 +3,7 @@ import ONYXKEYS from '../../ONYXKEYS'; /** * @param {Array} activeClients + * @return {Promise} */ function setActiveClients(activeClients) { return Onyx.set(ONYXKEYS.ACTIVE_CLIENTS, activeClients); From c682412051346dee71c586757301170c2676b472 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Mon, 19 Sep 2022 18:38:54 +0200 Subject: [PATCH 10/18] Remove unnecessary code --- .../Navigation/AppNavigator/AuthScreens.js | 1 - src/libs/actions/Policy.js | 19 ------------------- 2 files changed, 20 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index a2f9d082e3c3..4c537241995f 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -110,7 +110,6 @@ class AuthScreens extends React.Component { authEndpoint: `${CONFIG.EXPENSIFY.URL_API_ROOT}api?command=AuthenticatePusher`, }).then(() => { User.subscribeToUserEvents(); - Policy.subscribeToPolicyEvents(); }); // Listen for report changes and fetch some data we need on initialization diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 4904ea5d15d4..f3d6a4f49a50 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -685,24 +685,6 @@ function updateLastAccessedWorkspace(policyID) { Onyx.set(ONYXKEYS.LAST_ACCESSED_WORKSPACE_POLICY_ID, policyID); } -/** - * Subscribe to public-policyEditor-[policyID] events. - */ -function subscribeToPolicyEvents() { - _.each(allPolicies, (policy) => { - const pusherChannelName = `public-policyEditor-${policy.id}${CONFIG.PUSHER.SUFFIX}`; - Pusher.subscribe(pusherChannelName, 'policyEmployeeRemoved', ({removedEmails, defaultRoomChatIDs}) => { - // Remove the default chats if we are one of the users getting removed - if (!removedEmails.includes(sessionEmail) || _.isEmpty(defaultRoomChatIDs)) { - return; - } - _.each(defaultRoomChatIDs, (chatID) => { - Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${chatID}`, null); - }); - }); - }); -} - /** * Removes an error after trying to delete a member * @@ -987,7 +969,6 @@ export { updateWorkspaceCustomUnit, updateCustomUnitRate, updateLastAccessedWorkspace, - subscribeToPolicyEvents, clearDeleteMemberError, clearAddMemberError, openWorkspaceReimburseView, From 083405a9d125a7fad9b8a9e58a8c4c16dca2faec Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 20 Sep 2022 13:26:19 +0200 Subject: [PATCH 11/18] Update src/libs/ActiveClientManager/index.js Co-authored-by: Marc Glasser --- src/libs/ActiveClientManager/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index 18b5a42c5d87..627d40a4f19b 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -1,5 +1,5 @@ /** - * When you have many tabs in one browser, the data pf Onyx is shared between all of them. Since we persist write requests in Onyx, we need to ensure that + * When you have many tabs in one browser, the data of Onyx is shared between all of them. Since we persist write requests in Onyx, we need to ensure that * only one tab is processing those saved requests or we would be duplicating data (or creating errors). * This file ensures exactly that by tracking all the clientIDs connected, storing the most recent one last and it considers that last clientID the "leader". */ From 687266baca22bb9089fb62e2f4eefdbbbd79c07b Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 20 Sep 2022 14:03:15 +0200 Subject: [PATCH 12/18] Simplify ActiveClientManager --- src/libs/ActiveClientManager/index.js | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index 18b5a42c5d87..939274e70aa7 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -12,12 +12,7 @@ import * as ActiveClients from '../actions/ActiveClients'; const clientID = Str.guid(); const maxClients = 20; -let addedSelf = false; let activeClients = []; -let resolveInitedPromise; -const initedPromise = new Promise((resolve) => { - resolveInitedPromise = resolve; -}); let resolveSavedSelfPromise; const savedSelfPromise = new Promise((resolve) => { resolveSavedSelfPromise = resolve; @@ -28,18 +23,13 @@ const savedSelfPromise = new Promise((resolve) => { * @returns {Promise} */ function isReady() { - return Promise.all([initedPromise, savedSelfPromise]); + return savedSelfPromise; } Onyx.connect({ key: ONYXKEYS.ACTIVE_CLIENTS, callback: (val) => { - // We only add this client once, ensuring it is the last in the array (that determines who is leader) - if (!addedSelf) { - activeClients = _.without(val, clientID); - activeClients.push(clientID); - ActiveClients.setActiveClients(activeClients); - } + activeClients = val; // Remove from the beginning of the list any clients that are past the limit, to avoid having thousands of them let removed = false; @@ -49,16 +39,11 @@ Onyx.connect({ } // Save the clients back to onyx, if they changed - if (removed || !addedSelf) { + if (removed) { ActiveClients.setActiveClients(activeClients).then(() => { - // If we just added ourselves, we need to resolve the promise so that isReady fires - if (addedSelf) { - return; - } resolveSavedSelfPromise(); }); } - addedSelf = true; }, }); @@ -66,7 +51,9 @@ Onyx.connect({ * Add our client ID to the list of active IDs */ function init() { - resolveInitedPromise(); + activeClients = _.without(activeClients, clientID); + activeClients.push(clientID); + ActiveClients.setActiveClients(activeClients); } /** From 019b365b977ea9272a5d242235baa6b16b32f10e Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 20 Sep 2022 14:19:46 +0200 Subject: [PATCH 13/18] Move promise to correct place --- src/libs/ActiveClientManager/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index ae567f7fd3f3..c94f038b0097 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -40,9 +40,7 @@ Onyx.connect({ // Save the clients back to onyx, if they changed if (removed) { - ActiveClients.setActiveClients(activeClients).then(() => { - resolveSavedSelfPromise(); - }); + ActiveClients.setActiveClients(activeClients); } }, }); @@ -53,7 +51,7 @@ Onyx.connect({ function init() { activeClients = _.without(activeClients, clientID); activeClients.push(clientID); - ActiveClients.setActiveClients(activeClients); + ActiveClients.setActiveClients(activeClients).then(resolveSavedSelfPromise); } /** From 2c56131a32105bf1b80a0d1214ae54b214e4c121 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 20 Sep 2022 14:28:27 +0200 Subject: [PATCH 14/18] Linting --- src/libs/Navigation/AppNavigator/AuthScreens.js | 1 - src/libs/actions/Policy.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 4c537241995f..09bc898409f4 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -21,7 +21,6 @@ import KeyboardShortcut from '../../KeyboardShortcut'; import Navigation from '../Navigation'; import * as User from '../../actions/User'; import * as Modal from '../../actions/Modal'; -import * as Policy from '../../actions/Policy'; import modalCardStyleInterpolator from './modalCardStyleInterpolator'; import createCustomModalStackNavigator from './createCustomModalStackNavigator'; diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index f3d6a4f49a50..a03b3b19df80 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -8,14 +8,12 @@ import * as API from '../API'; import ONYXKEYS from '../../ONYXKEYS'; import * as PersonalDetails from './PersonalDetails'; import Growl from '../Growl'; -import CONFIG from '../../CONFIG'; import CONST from '../../CONST'; import * as Localize from '../Localize'; import Navigation from '../Navigation/Navigation'; import ROUTES from '../../ROUTES'; import * as OptionsListUtils from '../OptionsListUtils'; import * as Report from './Report'; -import * as Pusher from '../Pusher/pusher'; import DateUtils from '../DateUtils'; const allPolicies = {}; From a6505128ce3364ca1e5b213dd5e4f038fb97eaf6 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Tue, 20 Sep 2022 16:21:39 +0200 Subject: [PATCH 15/18] Add comment about init method --- src/libs/ActiveClientManager/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/ActiveClientManager/index.js b/src/libs/ActiveClientManager/index.js index c94f038b0097..908a500d6b72 100644 --- a/src/libs/ActiveClientManager/index.js +++ b/src/libs/ActiveClientManager/index.js @@ -46,7 +46,8 @@ Onyx.connect({ }); /** - * Add our client ID to the list of active IDs + * Add our client ID to the list of active IDs. + * We want to ensure we have no duplicates and that the activeClient gets added at the end of the array (see isClientTheLeader) */ function init() { activeClients = _.without(activeClients, clientID); From a5beaf401730a6652a6a7cb859ee53611d61d9c9 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Wed, 28 Sep 2022 19:51:45 +0200 Subject: [PATCH 16/18] Show no access page when you lose access to a chat --- .../BlockingViews/FullPageNotFoundView.js | 12 +- src/languages/en.js | 1 + src/languages/es.js | 1 + src/libs/actions/Report.js | 17 --- src/pages/home/ReportScreen.js | 128 +++++++++--------- 5 files changed, 77 insertions(+), 82 deletions(-) diff --git a/src/components/BlockingViews/FullPageNotFoundView.js b/src/components/BlockingViews/FullPageNotFoundView.js index 8a4447bb0cdc..d212bf9e9134 100644 --- a/src/components/BlockingViews/FullPageNotFoundView.js +++ b/src/components/BlockingViews/FullPageNotFoundView.js @@ -18,10 +18,18 @@ const propTypes = { /** If true, child components are replaced with a blocking "not found" view */ shouldShow: PropTypes.bool, + + /** The key in the translations file to use for the title */ + titleKey: PropTypes.string, + + /** The key in the translations file to use for the subtitle */ + subtitleKey: PropTypes.string, }; const defaultProps = { shouldShow: false, + titleKey: 'notFound.notHere', + subtitleKey: 'notFound.pageNotFound', }; // eslint-disable-next-line rulesdir/no-negated-variables @@ -37,8 +45,8 @@ const FullPageNotFoundView = (props) => { diff --git a/src/languages/en.js b/src/languages/en.js index 952998e2fbb2..61726e6f5a93 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -514,6 +514,7 @@ export default { iouReportNotFound: 'The payment details you are looking for cannot be found.', notHere: "Hmm... it's not here", pageNotFound: 'That page is nowhere to be found.', + noAccess: 'You don\'t have access to this chat', }, setPasswordPage: { enterPassword: 'Enter a password', diff --git a/src/languages/es.js b/src/languages/es.js index fc2d2308a989..8a797e287ffc 100644 --- a/src/languages/es.js +++ b/src/languages/es.js @@ -514,6 +514,7 @@ export default { iouReportNotFound: 'Los detalles del pago que estás buscando no se pudieron encontrar.', notHere: 'Hmm… no está aquí', pageNotFound: 'La página que buscas no existe.', + noAccess: 'No tienes acceso a este chat', }, setPasswordPage: { enterPassword: 'Escribe una contraseña', diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index a42b655aca2f..24048c61c3c2 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -337,14 +337,6 @@ function fetchChatReportsByIDs(chatList, shouldRedirectIfInaccessible = false) { // Fetch the personal details if there are any PersonalDetails.getFromReportParticipants(_.values(simplifiedReports)); return simplifiedReports; - }) - .catch((err) => { - if (err.message !== CONST.REPORT.ERROR.INACCESSIBLE_REPORT) { - return; - } - - // eslint-disable-next-line no-use-before-define - handleInaccessibleReport(); }); } @@ -1409,14 +1401,6 @@ function navigateToConciergeChat() { Navigation.navigate(ROUTES.getReportRoute(conciergeChatReportID)); } -/** - * Handle the navigation when report is inaccessible - */ -function handleInaccessibleReport() { - Growl.error(Localize.translateLocal('notFound.chatYouLookingForCannotBeFound')); - navigateToConciergeChat(); -} - /** * Creates a policy room, fetches it, and navigates to it. * @param {String} policyID @@ -1743,7 +1727,6 @@ export { getSimplifiedIOUReport, syncChatAndIOUReports, navigateToConciergeChat, - handleInaccessibleReport, setReportWithDraft, createPolicyRoom, addPolicyReport, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 09085ae3f7a0..aea0e63c96bb 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -30,6 +30,7 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/ import OfflineIndicator from '../../components/OfflineIndicator'; import OfflineWithFeedback from '../../components/OfflineWithFeedback'; import withDrawerState, {withDrawerPropTypes} from '../../components/withDrawerState'; +import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView'; const propTypes = { /** Navigation route context info provided by react navigation */ @@ -175,10 +176,6 @@ class ReportScreen extends React.Component { */ storeCurrentlyViewedReport() { const reportIDFromPath = getReportID(this.props.route); - if (_.isNaN(reportIDFromPath)) { - Report.handleInaccessibleReport(); - return; - } // Always reset the state of the composer view when the current reportID changes toggleReportActionComposeView(true); @@ -237,71 +234,76 @@ class ReportScreen extends React.Component { style={[styles.appContent, styles.flex1, {marginTop: this.state.viewportOffsetTop}]} keyboardAvoidingViewBehavior={Platform.OS === 'android' ? '' : 'padding'} > - - Navigation.navigate(ROUTES.HOME)} - /> - - this.setState({skeletonViewContainerHeight: event.nativeEvent.layout.height})} - > - {this.shouldShowLoader() - ? ( - - ) - : ( - - )} - {(isArchivedRoom || hideComposer) && ( - - {isArchivedRoom && ( - + Navigation.navigate(ROUTES.HOME)} + /> + + this.setState({skeletonViewContainerHeight: event.nativeEvent.layout.height})} + > + {this.shouldShowLoader() + ? ( + + ) + : ( + )} - {!this.props.isSmallScreenWidth && ( - - {hideComposer && ( - - )} - - )} - - )} - {(!hideComposer && this.props.session.shouldShowComposeInput) && ( - - - - + {isArchivedRoom && ( + - - - - )} - + )} + {!this.props.isSmallScreenWidth && ( + + {hideComposer && ( + + )} + + )} + + )} + {(!hideComposer && this.props.session.shouldShowComposeInput) && ( + + + + + + + + )} + + ); } From d3def61e235bf644cec463039e0f1036b0bdb06d Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Wed, 28 Sep 2022 20:44:37 +0200 Subject: [PATCH 17/18] Lint --- src/libs/actions/Policy.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index ac3ec9b500d8..ffcbfdc7b443 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -6,13 +6,11 @@ import Str from 'expensify-common/lib/str'; import * as DeprecatedAPI from '../deprecatedAPI'; import * as API from '../API'; import ONYXKEYS from '../../ONYXKEYS'; -import Growl from '../Growl'; import CONST from '../../CONST'; import * as Localize from '../Localize'; import Navigation from '../Navigation/Navigation'; import ROUTES from '../../ROUTES'; import * as OptionsListUtils from '../OptionsListUtils'; -import * as Report from './Report'; import DateUtils from '../DateUtils'; import * as ReportUtils from '../ReportUtils'; From 713e5a4fa55554fca34885e85fec35622f2ce7a5 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Fri, 30 Sep 2022 18:47:28 +0200 Subject: [PATCH 18/18] Fix not found page showing close --- .../BlockingViews/FullPageNotFoundView.js | 17 +++++++++++++++-- src/pages/home/ReportScreen.js | 5 +++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/components/BlockingViews/FullPageNotFoundView.js b/src/components/BlockingViews/FullPageNotFoundView.js index d212bf9e9134..6502852afd2a 100644 --- a/src/components/BlockingViews/FullPageNotFoundView.js +++ b/src/components/BlockingViews/FullPageNotFoundView.js @@ -24,12 +24,24 @@ const propTypes = { /** The key in the translations file to use for the subtitle */ subtitleKey: PropTypes.string, + + /** Whether we should show a back icon */ + shouldShowBackButton: PropTypes.bool, + + /** Whether we should show a close button */ + shouldShowCloseButton: PropTypes.bool, + + /** Method to trigger when pressing the back button of the header */ + onBackButtonPress: PropTypes.func, }; const defaultProps = { shouldShow: false, titleKey: 'notFound.notHere', subtitleKey: 'notFound.pageNotFound', + shouldShowBackButton: true, + shouldShowCloseButton: true, + onBackButtonPress: () => Navigation.dismissModal(), }; // eslint-disable-next-line rulesdir/no-negated-variables @@ -38,8 +50,9 @@ const FullPageNotFoundView = (props) => { return ( <> Navigation.dismissModal()} + shouldShowBackButton={props.shouldShowBackButton} + shouldShowCloseButton={props.shouldShowCloseButton} + onBackButtonPress={props.onBackButtonPress} onCloseButtonPress={() => Navigation.dismissModal()} /> diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 3e0a158a2598..df9620502537 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -239,6 +239,11 @@ class ReportScreen extends React.Component { { + Navigation.navigate(ROUTES.HOME); + }} >