Merge pull request Expensify#10452 from Expensify/ionatan_DeleteMembe…

…rsFromWorkspace

Migrate policy API to remove policy members

package.json

@@ -94,7 +94,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.17",
     "react-native-pdf": "^6.6.2",
     "react-native-performance": "^2.0.0",
     "react-native-permissions": "^3.0.1",

src/components/BlockingViews/FullPageNotFoundView.js

@@ -18,10 +18,30 @@ 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,
+
+    /** 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
@@ -30,15 +50,16 @@ const FullPageNotFoundView = (props) => {
         return (
             <>
                 <HeaderWithCloseButton
-                    shouldShowBackButton
-                    onBackButtonPress={() => Navigation.dismissModal()}
+                    shouldShowBackButton={props.shouldShowBackButton}
+                    shouldShowCloseButton={props.shouldShowCloseButton}
+                    onBackButtonPress={props.onBackButtonPress}
                     onCloseButtonPress={() => Navigation.dismissModal()}
                 />
                 <View style={styles.flex1}>
                     <BlockingView
                         icon={Expensicons.QuestionMark}
-                        title={props.translate('notFound.notHere')}
-                        subtitle={props.translate('notFound.pageNotFound')}
+                        title={props.translate(props.titleKey)}
+                        subtitle={props.translate(props.subtitleKey)}
                     />
                 </View>
             </>

src/languages/en.js

@@ -525,6 +525,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',
@@ -822,6 +823,7 @@ export default {
             error: {
                 genericAdd: 'There was a problem adding this workspace member.',
                 cannotRemove: 'You cannot remove yourself or the workspace owner.',
+                genericRemove: 'There was a problem removing that workspace member.',
             },
         },
         card: {

src/languages/es.js

@@ -525,6 +525,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',
@@ -824,6 +825,7 @@ export default {
             error: {
                 genericAdd: 'Ha ocurrido un problema al agregar el miembro al espacio de trabajo',
                 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: {

src/libs/ActiveClientManager/index.js

@@ -1,3 +1,9 @@
+/**
+ * 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".
+ */
+
 import _ from 'underscore';
 import Onyx from 'react-native-onyx';
 import Str from 'expensify-common/lib/str';
@@ -6,38 +12,47 @@ import * as ActiveClients from '../actions/ActiveClients';
 
 const clientID = Str.guid();
 const maxClients = 20;
-
-let activeClients;
-
-let resolveIsReadyPromise;
-const isReadyPromise = new Promise((resolve) => {
-    resolveIsReadyPromise = resolve;
+let activeClients = [];
+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 savedSelfPromise;
 }
 
 Onyx.connect({
     key: ONYXKEYS.ACTIVE_CLIENTS,
     callback: (val) => {
-        activeClients = !val ? [] : val;
-        if (activeClients.length >= maxClients) {
+        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;
+        while (activeClients.length >= maxClients) {
             activeClients.shift();
+            removed = true;
+        }
+
+        // Save the clients back to onyx, if they changed
+        if (removed) {
             ActiveClients.setActiveClients(activeClients);
         }
     },
 });
 
 /**
- * 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.addClient(clientID)
-        .then(resolveIsReadyPromise);
+    activeClients = _.without(activeClients, clientID);
+    activeClients.push(clientID);
+    ActiveClients.setActiveClients(activeClients).then(resolveSavedSelfPromise);
 }
 
 /**

src/libs/Navigation/AppNavigator/AuthScreens.js

@@ -20,7 +20,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';
 
@@ -100,7 +99,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

src/libs/actions/ActiveClients.js

@@ -3,20 +3,13 @@ import ONYXKEYS from '../../ONYXKEYS';
 
 /**
  * @param {Array} activeClients
+ * @return {Promise}
  */
 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,
 };

src/libs/actions/Policy.js

@@ -6,15 +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 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';
 import * as ReportUtils from '../ReportUtils';
 
@@ -209,29 +205,21 @@ function removeMembers(members, policyID) {
     if (members.length === 0) {
         return;
     }
-
-    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({
+    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()]: Localize.translateLocal('workspace.people.error.genericRemove')}})),
+    }];
+    API.write('DeleteMembersFromWorkspace', {
         emailList: members.join(','),
         policyID,
-    })
-        .then((data) => {
-            if (data.jsonCode === 200) {
-                return;
-            }
-
-            // Rollback removal on failure
-            _.each(members, login => employeeListUpdate[login] = {});
-            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');
-            Growl.show(errorMessage, CONST.GROWL.ERROR, 5000);
-        });
+    }, {optimisticData, failureData});
 }
 
 /**
@@ -653,31 +641,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, 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.reconnect(reportID);
-                });
-            }
-
-            // 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);
-                });
-            }
-        });
-    });
-}
-
 /**
  * Removes an error after trying to delete a member
  *
@@ -974,7 +937,6 @@ export {
     updateWorkspaceCustomUnit,
     updateCustomUnitRate,
     updateLastAccessedWorkspace,
-    subscribeToPolicyEvents,
     clearDeleteMemberError,
     clearAddMemberError,
     clearDeleteWorkspaceError,

src/libs/actions/Report.js

@@ -331,14 +331,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();
         });
 }
 
@@ -1086,6 +1078,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
                 isEdited: true,
                 html: htmlForNewComment,
                 text: textForNewComment,
+                type: originalReportAction.message[0].type,
             }],
         },
     };
@@ -1212,14 +1205,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
@@ -1544,7 +1529,6 @@ export {
     getSimplifiedIOUReport,
     syncChatAndIOUReports,
     navigateToConciergeChat,
-    handleInaccessibleReport,
     setReportWithDraft,
     createPolicyRoom,
     addPolicyReport,