Enable active client manager, multiple tabs, and previous PR cleanup

README.md

@@ -15,7 +15,7 @@ This application is built with the following principles.
 1. **UI Binds to data on disk** 
     - Ion is a Pub/Sub library to connect the application to the data stored on disk.
     - UI components subscribe to Ion (using `withIon()`) and any change to the Ion data is published to the component by calling `setState()` with the changed data.
-    - Libraries subscribe to Ion (with `Ion.connect()`) and any change to the Ion data is published to the callback callback with the changed data.
+    - Libraries subscribe to Ion (with `Ion.connect()`) and any change to the Ion data is published to the callback with the changed data.
     - The UI should never call any Ion methods except for `Ion.connect()`. That is the job of Actions (see next section).
     - The UI always triggers an Action when something needs to happen (eg. a person inputs data, the UI triggers an Action with this data).
     - The UI should be as flexible as possible when it comes to:
@@ -27,7 +27,7 @@ This application is built with the following principles.
     - When data needs to be written to or read from the server, this is done through Actions only.
     - Public action methods should never return anything (not data or a promise). This is done to ensure that action methods can be called in parallel with no dependency on other methods (see discussion above).
     - Actions should favor using `Ion.merge()` over `Ion.set()` so that other values in an object aren't completely overwritten.
-    - In general, the operations that happen inside an action should be done in parallel and not in sequence ((eg. don't use the promise of one Ion method to trigger a second Ion method). Ion is built so that every operation is done in parallel and it doesn't matter what order they finish in. XHRs on the other hand need to be handled in sequence with promise chains in order to access and act upon the response.
+    - In general, the operations that happen inside an action should be done in parallel and not in sequence (eg. don't use the promise of one Ion method to trigger a second Ion method). Ion is built so that every operation is done in parallel and it doesn't matter what order they finish in. XHRs on the other hand need to be handled in sequence with promise chains in order to access and act upon the response.
     - If an Action needs to access data stored on disk, use a local variable and `Ion.connect()`
     - Data should be optimistically stored on disk whenever possible without waiting for a server response. Example of creating a new optimistic comment:
         1. user adds a comment 

package.json

@@ -43,7 +43,6 @@
     "prop-types": "^15.7.2",
     "pusher-js": "^7.0.0",
     "react": "^16.13.1",
-    "react-beforeunload": "^2.2.2",
     "react-dom": "^16.13.1",
     "react-native": "0.63.2",
     "react-native-config": "^1.3.3",

src/Expensify.js

@@ -2,8 +2,6 @@ import React, {Component} from 'react';
 import {View} from 'react-native';
 import PropTypes from 'prop-types';
 import {recordCurrentRoute} from './lib/actions/App';
-
-// import {Beforeunload} from 'react-beforeunload';
 import SignInPage from './page/SignInPage';
 import HomePage from './page/home/HomePage';
 import Ion from './lib/Ion';
@@ -78,9 +76,6 @@ class Expensify extends Component {
         }
         const redirectTo = !this.state.authToken ? ROUTES.SIGNIN : this.props.redirectTo;
         return (
-
-            // TODO: Mobile does not support Beforeunload
-            // <Beforeunload onBeforeunload={ActiveClientManager.removeClient}>
             <Router>
                 {/* If there is ever a property for redirecting, we do the redirect here */}
                 {/* Leave this as a ternary or else iOS throws an error about text not being wrapped in <Text> */}
@@ -101,8 +96,6 @@ class Expensify extends Component {
                     <Route path={['/home', '/']} component={HomePage} />
                 </Switch>
             </Router>
-
-        // </Beforeunload>
         );
     }
 }

src/IONKEYS.js

@@ -2,7 +2,7 @@
  * This is a file containing constants for all the top level keys in our store
  */
 export default {
-    ACTIVE_CLIENTS: 'activeClients',
+    ACTIVE_CLIENTS: 'activeClients2',
     APP_REDIRECT_TO: 'appRedirectTo',
     CURRENT_URL: 'currentURL',
     CREDENTIALS: 'credentials',

src/components/withIon.js

@@ -98,7 +98,6 @@ export default function (mapIonToState) {
              * @param {object} mapping
              * @param {string|function} mapping.key key to connect to. can be a string or a
              * function that takes this.props as an argument and returns a string
-             *
              * @param {string} statePropertyName the name of the state property that Ion will add the data to
              * @param {boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the
              *  component

src/lib/ActiveClientManager/index.js

@@ -0,0 +1,41 @@
+import _ from 'underscore';
+import Guid from '../Guid';
+import Ion from '../Ion';
+import IONKEYS from '../../IONKEYS';
+
+const clientID = Guid();
+const maxClients = 20;
+
+let activeClients;
+Ion.connect({
+    key: IONKEYS.ACTIVE_CLIENTS,
+
+    callback: (val) => {
+        activeClients = val;
+        if (activeClients.length >= maxClients) {
+            activeClients.shift();
+            Ion.set(IONKEYS.ACTIVE_CLIENTS, activeClients);
+        }
+    },
+});
+
+/**
+ * Add our client ID to the list of active IDs
+ */
+function init() {
+    Ion.merge(IONKEYS.ACTIVE_CLIENTS, [clientID]);
+}
+
+/**
+ * The last GUID is the most recent GUID, so that should be the leader
+ *
+ * @returns {boolean}
+ */
+function isClientTheLeader() {
+    return _.last(activeClients) === clientID;
+}
+
+export {
+    init,
+    isClientTheLeader
+};

src/lib/ActiveClientManager/index.native.js

@@ -0,0 +1,14 @@
+/**
+ * For native devices, there will never be more than one
+ * client running at a time, so this lib is a big no-op
+ */
+
+function init() {}
+function isClientTheLeader() {
+    return true;
+}
+
+export {
+    init,
+    isClientTheLeader
+};

src/lib/Ion.js

@@ -1,30 +1,17 @@
 import _ from 'underscore';
 import AsyncStorage from '@react-native-community/async-storage';
+import addStorageEventHandler from './addStorageEventHandler';
 import Str from './Str';
 import IONKEYS from '../IONKEYS';
 
 // Keeps track of the last connectionID that was used so we can keep incrementing it
 let lastConnectionID = 0;
 
-/**
- * Initialize the store with actions and listening for storage events
- */
-function init() {
-    // Subscribe to the storage event so changes to local storage can be captured
-    // TODO: Refactor window events
-    // window.addEventListener('storage', (e) => {
-    //   try {
-    //     keyChanged(e.key, JSON.parse(e.newValue));
-    //   } catch (e) {
-    //     console.error(`Could not parse value from local storage. Key: ${e.key}`);
-    //   }
-    // });
-}
-
 // Holds a mapping of all the react components that want their state subscribed to a store key
 const callbackToStateMapping = {};
 
 /**
+ * When a key change happens, search for any callbacks matching the regex pattern and trigger those callbacks
  * Get some data from the store
  *
  * @param {string} key
@@ -97,6 +84,13 @@ function keyChanged(key, data) {
     });
 }
 
+/**
+ * Initialize the store with actions and listening for storage events
+ */
+function init() {
+    addStorageEventHandler((key, newValue) => keyChanged(key, newValue));
+}
+
 /**
  * Sends the data obtained from the keys to the connection. It either:
  *     - sets state on the withIonInstances
@@ -238,7 +232,24 @@ function clear() {
  * @param {*} val
  */
 function merge(key, val) {
-    // Values that are objects can be merged into storage
+    // Arrays need to be manually merged because the AsyncStorage behavior
+    // is not desired when merging arrays. `AsyncStorage.mergeItem('test', [1]);
+    // will result in `{0: 1}` being set in storage, when `[1]` is what is expected
+    if (_.isArray(val)) {
+        let newArray;
+        get(key)
+            .then((prevVal) => {
+                const previousValue = prevVal || [];
+                newArray = [...previousValue, ...val];
+                return AsyncStorage.setItem(key, JSON.stringify(newArray));
+            })
+            .then(() => {
+                keyChanged(key, newArray);
+            });
+        return;
+    }
+
+    // Values that are objects are merged normally into storage
     if (_.isObject(val)) {
         AsyncStorage.mergeItem(key, JSON.stringify(val))
             .then(() => get(key))

src/lib/Notification/BrowserNotifications.js

@@ -1,8 +1,6 @@
 // Web and desktop implementation only. Do not import for direct use. Use Notification.
-
 import Str from '../Str';
 import CONST from '../../CONST';
-import * as ActiveClientManager from '../ActiveClientManager';
 
 const EXPENSIFY_ICON_URL = `${CONST.CLOUDFRONT_URL}/images/favicon-2019.png`;
 const DEFAULT_DELAY = 4000;
@@ -105,11 +103,6 @@ export default {
      * @param {Function} params.onClick
      */
     pushReportCommentNotification({reportAction, onClick}) {
-        if (!ActiveClientManager.isClientTheLeader()) {
-            console.debug('[BrowserNotifications] Skipping notification because this client is not the leader');
-            return;
-        }
-
         const {person, message} = reportAction;
         const plainTextPerson = Str.htmlDecode(person.map(f => f.text).join());
 

src/lib/actions/PersonalDetails.js

@@ -88,15 +88,17 @@ function fetch() {
             const allPersonalDetails = formatPersonalDetails(data.personalDetailsList);
             Ion.merge(IONKEYS.PERSONAL_DETAILS, allPersonalDetails);
 
-            // Get my personal details so they can be easily accessed and subscribed to on their own key
+            // Set my personal details so they can be easily accessed and subscribed to on their own key
             Ion.merge(IONKEYS.MY_PERSONAL_DETAILS, allPersonalDetails[currentUserEmail] || {});
 
             // Get the timezone and put it in Ion
             fetchTimezone();
         })
         .catch(error => console.error('Error fetching personal details', error));
 
-    // Refresh the personal details every 30 minutes
+    // Refresh the personal details every 30 minutes because there is no
+    // pusher event that sends updated personal details data yet
+    // See https://github.com/Expensify/ReactNativeChat/issues/468
     setTimeout(fetch, 1000 * 60 * 30);
 }
 
@@ -108,9 +110,8 @@ function fetch() {
 function getForEmails(emailList) {
     queueRequest('PersonalDetails_GetForEmails', {emailList})
         .then((data) => {
-            const details = _.omit(data, ['jsonCode', 'requestID']);
-            const formattedDetails = formatPersonalDetails(details);
-            Ion.merge(IONKEYS.PERSONAL_DETAILS, formattedDetails);
+            const details = _.pick(data, emailList.split(','));
+            Ion.merge(IONKEYS.PERSONAL_DETAILS, formatPersonalDetails(details));
         });
 }
 

src/lib/actions/Report.js

@@ -11,6 +11,7 @@ import ExpensiMark from '../ExpensiMark';
 import Notification from '../Notification';
 import * as PersonalDetails from './PersonalDetails';
 import {redirect} from './App';
+import * as ActiveClientManager from '../ActiveClientManager';
 import Visibility from '../Visibility';
 
 let currentUserEmail;
@@ -192,6 +193,11 @@ function updateReportWithNewAction(reportID, reportAction) {
         [reportAction.sequenceNumber]: reportAction,
     });
 
+    if (!ActiveClientManager.isClientTheLeader()) {
+        console.debug('[NOTIFICATION] Skipping notification because this client is not the leader');
+        return;
+    }
+
     // If this comment is from the current user we don't want to parrot whatever they wrote back to them.
     if (reportAction.actorEmail === currentUserEmail) {
         console.debug('[NOTIFICATION] No notification because comment is from the currently logged in user');

src/lib/actions/Session.js

@@ -32,11 +32,16 @@ function signIn(partnerUserID, partnerUserSecret, twoFactorAuthCode = '', exitTo
  */
 function signOut() {
     redirectToSignIn();
-    API.deleteLogin({
-        partnerUserID: credentials && credentials.login
-    });
     Ion.clear();
     Pusher.disconnect();
+
+    if (!credentials || !credentials.login) {
+        return;
+    }
+
+    API.deleteLogin({
+        partnerUserID: credentials.login
+    });
 }
 
 export {

src/lib/addStorageEventHandler/index.js

@@ -0,0 +1,19 @@
+/**
+ * Listens for storage events so that multiple tabs can keep track of what
+ * other tabs are doing
+ *
+ * @param {function} callback
+ */
+function addStorageEventHandler(callback) {
+    window.addEventListener('storage', (e) => {
+        let newValue;
+        try {
+            newValue = JSON.parse(e.newValue);
+        } catch (err) {
+            console.error('Could not parse the newValue of the storage event', err, e);
+        }
+        callback(e.key, JSON.parse(newValue));
+    });
+}
+
+export default addStorageEventHandler;

src/lib/addStorageEventHandler/index.native.js

@@ -0,0 +1,8 @@
+/**
+ * Native clients don't have storage events and that's OK because
+ * you can't have multiple native clients open at the same time on the same
+ * device
+ */
+function addStorageEventHandler() {}
+
+export default addStorageEventHandler;

src/page/home/report/ReportActionCompose.js

@@ -93,7 +93,6 @@ class ReportActionCompose extends React.Component {
         const trimmedComment = this.comment.trim();
 
         // Don't submit empty comments
-        // @TODO show an error in the UI
         if (!trimmedComment) {
             return;
         }