From ef4067a01d86b160364fc3dab1936e4f90836120 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:07:09 -0600 Subject: [PATCH 01/47] Remove Ion.get() from active client manager --- src/lib/ActiveClientManager.js | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/lib/ActiveClientManager.js b/src/lib/ActiveClientManager.js index 8d68c313237ea..80ac16541f07a 100644 --- a/src/lib/ActiveClientManager.js +++ b/src/lib/ActiveClientManager.js @@ -1,26 +1,32 @@ -import _ from 'underscore'; import Guid from './Guid'; import Ion from './Ion'; import IONKEYS from '../IONKEYS'; const clientID = Guid(); +// @TODO make all this work by uncommenting code. This will work once +// there is a cross-platform method for onBeforeUnload +// See https://github.com/Expensify/ReactNativeChat/issues/413 +// let activeClients; +Ion.connect({ + key: IONKEYS.ACTIVE_CLIENTS, + + // callback: val => activeClients = val, +}); + /** * Add our client ID to the list of active IDs * * @returns {Promise} */ -const init = () => Ion.merge(IONKEYS.ACTIVE_CLIENTS, {clientID}); +// @TODO need to change this to Ion.merge() once we support multiple tabs +const init = () => Ion.set(IONKEYS.ACTIVE_CLIENTS, {[clientID]: clientID}); /** * Remove this client ID from the array of active client IDs when this client is exited - * - * @returns {Promise} */ function removeClient() { - return Ion.get(IONKEYS.ACTIVE_CLIENTS) - .then(activeClientIDs => _.omit(activeClientIDs, clientID)) - .then(newActiveClientIDs => Ion.set(IONKEYS.ACTIVE_CLIENTS, newActiveClientIDs)); + // Ion.set(IONKEYS.ACTIVE_CLIENTS, _.omit(activeClients, clientID)); } /** @@ -29,8 +35,9 @@ function removeClient() { * @returns {Promise} */ function isClientTheLeader() { - return Ion.get(IONKEYS.ACTIVE_CLIENTS) - .then(activeClientIDs => _.first(activeClientIDs) === clientID); + return true; + + // return _.first(activeClients) === clientID; } export { From f147cfdda628bc19eac490f0f386392395f23f38 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:10:53 -0600 Subject: [PATCH 02/47] Remove Ion.get() from singinredirect action --- src/lib/actions/Session.js | 7 +++---- src/lib/actions/SignInRedirect.js | 35 ++++++++++++++++--------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index a49106cd565e1..c16aa8f8a8e96 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -52,15 +52,14 @@ function deleteLogin(authToken, login) { /** * Sign out of our application - * - * @returns {Promise} */ function signOut() { - return redirectToSignIn() - .then(() => Ion.multiGet([IONKEYS.SESSION, IONKEYS.CREDENTIALS])) + Ion.multiGet([IONKEYS.SESSION, IONKEYS.CREDENTIALS]) .then(data => deleteLogin(data.session.authToken, data.credentials.login)) .then(Ion.clear) .catch(err => Ion.merge(IONKEYS.SESSION, {error: err.message})); + + redirectToSignIn(); } export { diff --git a/src/lib/actions/SignInRedirect.js b/src/lib/actions/SignInRedirect.js index ff7c1a7473948..e4bd853eca3a9 100644 --- a/src/lib/actions/SignInRedirect.js +++ b/src/lib/actions/SignInRedirect.js @@ -2,30 +2,31 @@ import Ion from '../Ion'; import IONKEYS from '../../IONKEYS'; import ROUTES from '../../ROUTES'; +let currentURL; +Ion.connect({ + key: IONKEYS.CURRENT_URL, + callback: val => currentURL = val, +}); + /** * Redirects to the sign in page and handles adding any exitTo params to the URL. * Normally this method would live in Session.js, but that would cause a circular dependency with Network.js. - * - * @returns {Promise} */ function redirectToSignIn() { - return Ion.get(IONKEYS.CURRENT_URL) - .then((url) => { - if (!url) { - return; - } + if (!currentURL) { + return; + } - // If there is already an exitTo, or has the URL of signin, don't redirect - if (url.indexOf('exitTo') !== -1 || url.indexOf('signin') !== -1) { - return; - } + // If there is already an exitTo, or has the URL of signin, don't redirect + if (currentURL.indexOf('exitTo') !== -1 || currentURL.indexOf('signin') !== -1) { + return; + } - // When the URL is at the root of the site, go to sign-in, otherwise add the exitTo - const urlWithExitTo = url === '/' - ? ROUTES.SIGNIN - : `${ROUTES.SIGNIN}/exitTo${url}`; - return Ion.set(IONKEYS.APP_REDIRECT_TO, urlWithExitTo); - }); + // When the URL is at the root of the site, go to sign-in, otherwise add the exitTo + const urlWithExitTo = currentURL === '/' + ? ROUTES.SIGNIN + : `${ROUTES.SIGNIN}/exitTo${currentURL}`; + Ion.set(IONKEYS.APP_REDIRECT_TO, urlWithExitTo); } export default redirectToSignIn; From 4b08616a3e1aac3ab2c5f08768bc55f1808548ee Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:14:41 -0600 Subject: [PATCH 03/47] Remove Ion.get() from chat switcher --- src/page/home/sidebar/ChatSwitcherView.js | 58 +++++++++++++---------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index f2208a3bee86a..f6f224f389501 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -10,6 +10,31 @@ import ChatSwitcherList from './ChatSwitcherList'; import ChatSwitcherSearchForm from './ChatSwitcherSearchForm'; import {fetchOrCreateChatReport} from '../../../lib/actions/Report'; +const personalDetailsPropTypes = PropTypes.objectOf(PropTypes.shape({ + // The login of the person (either email or phone number) + login: PropTypes.string.isRequired, + + // The URL of the person's avatar (there should already be a default avatarURL if + // the person doesn't have their own avatar uploaded yet) + avatarURL: PropTypes.string.isRequired, + + // The first name of the person + firstName: PropTypes.string, + + // The last name of the person + lastName: PropTypes.string, + + // The combination of `${firstName} ${lastName}` (could be an empty string) + fullName: PropTypes.string, + + // This is either the user's full name, or their login if full name is an empty string + displayName: PropTypes.string.isRequired, + + // Either the user's full name and their login, or just the login if the full name is empty + // `${fullName} (${login})` + displayNameWithEmail: PropTypes.string.isRequired, +})); + const propTypes = { // A method that is triggered when the TextInput gets focus onFocus: PropTypes.func.isRequired, @@ -22,33 +47,14 @@ const propTypes = { // All of the personal details for everyone // The keys of this object are the logins of the users, and the values are an object // with their details - personalDetails: PropTypes.objectOf(PropTypes.shape({ - // The login of the person (either email or phone number) - login: PropTypes.string.isRequired, + personalDetails: personalDetailsPropTypes, - // The URL of the person's avatar (there should already be a default avatarURL if - // the person doesn't have their own avatar uploaded yet) - avatarURL: PropTypes.string.isRequired, - - // The first name of the person - firstName: PropTypes.string, - - // The last name of the person - lastName: PropTypes.string, - - // The combination of `${firstName} ${lastName}` (could be an empty string) - fullName: PropTypes.string, - - // This is either the user's full name, or their login if full name is an empty string - displayName: PropTypes.string.isRequired, - - // Either the user's full name and their login, or just the login if the full name is empty - // `${fullName} (${login})` - displayNameWithEmail: PropTypes.string.isRequired, - })), + // The personal details of the person who is currently logged in + myPersonalDetails: personalDetailsPropTypes, }; const defaultProps = { personalDetails: {}, + myPersonalDetails: {}, }; class ChatSwitcherView extends React.Component { @@ -124,8 +130,7 @@ class ChatSwitcherView extends React.Component { * @param {string} option.value */ fetchChatReportAndRedirect(option) { - Ion.get(IONKEYS.MY_PERSONAL_DETAILS, 'login') - .then(currentLogin => fetchOrCreateChatReport([currentLogin, option.login])) + fetchOrCreateChatReport([this.props.myPersonalDetails.login, option.login]) .then(reportID => Ion.set(IONKEYS.APP_REDIRECT_TO, `/${reportID}`)); this.reset(); } @@ -273,4 +278,7 @@ export default withIon({ // my_personal_details to overwrite this value key: `^${IONKEYS.PERSONAL_DETAILS}$`, }, + myPersonalDetails: { + key: IONKEYS.MY_PERSONAL_DETAILS, + }, })(ChatSwitcherView); From 0cb7111d47a5973b3b54b27663b829f04d0d583f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:18:26 -0600 Subject: [PATCH 04/47] Remove Ion.get() from personalDetails --- src/lib/actions/PersonalDetails.js | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index 1cf46f389188a..aeb8bf31e10cf 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -5,6 +5,12 @@ import IONKEYS from '../../IONKEYS'; import md5 from '../md5'; import CONST from '../../CONST'; +let currentUserEmail; +Ion.connect({ + key: IONKEYS.SESSION, + callback: val => currentUserEmail = val.email; +}); + /** * Returns the URL for a user's avatar and handles someone not having any avatar at all * @@ -59,28 +65,16 @@ function formatPersonalDetails(personalDetailsList) { * @returns {Promise} */ function fetch() { - let currentLogin; - let myPersonalDetails; - const requestPromise = Ion.get(IONKEYS.SESSION, 'email') - .then((login) => { - if (!login) { - throw Error('No login'); - } - - currentLogin = login; - return queueRequest('Get', { - returnValueList: 'personalDetailsList', - }); - }) + const requestPromise = queueRequest('Get', { + returnValueList: 'personalDetailsList', + }) .then((data) => { 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 - myPersonalDetails = allPersonalDetails[currentLogin] || {}; - - return Ion.merge(IONKEYS.PERSONAL_DETAILS, allPersonalDetails); + Ion.merge(IONKEYS.MY_PERSONAL_DETAILS, allPersonalDetails[currentUserEmail] || {}); }) - .then(() => Ion.merge(IONKEYS.MY_PERSONAL_DETAILS, myPersonalDetails)) .catch((error) => { if (error.message === 'No login') { // eslint-disable-next-line no-console From 1259771ad12ad6ce52328d25155f9efae278242f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:29:38 -0600 Subject: [PATCH 05/47] Remove Ion.get() from report actions view --- src/lib/actions/PersonalDetails.js | 2 +- src/lib/actions/Report.js | 18 +++++++++++------- src/page/home/report/ReportActionsView.js | 16 ++-------------- src/page/home/sidebar/ChatSwitcherView.js | 8 ++++---- 4 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index aeb8bf31e10cf..4e52555fcd90e 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -8,7 +8,7 @@ import CONST from '../../CONST'; let currentUserEmail; Ion.connect({ key: IONKEYS.SESSION, - callback: val => currentUserEmail = val.email; + callback: val => currentUserEmail = val.email, }); /** diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index fcdede6a91673..dae5b02c5c8e4 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -417,23 +417,27 @@ function addAction(reportID, text) { * Updates the last read action ID on the report. It optimistically makes the change to the store, and then let's the * network layer handle the delayed write. * - * @param {string} accountID * @param {number} reportID * @param {number} sequenceNumber */ -function updateLastReadActionID(accountID, reportID, sequenceNumber) { - // Mark the report as not having any unread items +function updateLastReadActionID(reportID, sequenceNumber) { + const currentMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; + + if (sequenceNumber <= currentMaxSequenceNumber) { + return; + } + + // Update the lastReadActionID on the report optimistically Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { hasUnread: false, reportNameValuePairs: { - [`lastReadActionID_${accountID}`]: sequenceNumber, + [`lastReadActionID_${currentUserAccountID}`]: sequenceNumber, } }); - - // Update the lastReadActionID on the report optimistically + // Mark the report as not having any unread items queueRequest('Report_SetLastReadActionID', { - accountID, + currentUserAccountID, reportID, sequenceNumber, }); diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 0c241ed98fa8d..4b56bb7d8fb9d 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -121,18 +121,7 @@ class ReportActionsView extends React.Component { * @param {number} maxSequenceNumber */ recordlastReadActionID(maxSequenceNumber) { - let myAccountID; - Ion.get(IONKEYS.SESSION, 'accountID') - .then((accountID) => { - myAccountID = accountID; - const path = `reportNameValuePairs.lastReadActionID_${accountID}`; - return Ion.get(`${IONKEYS.REPORT}_${this.props.reportID}`, path, 0); - }) - .then((lastReadActionID) => { - if (maxSequenceNumber > lastReadActionID) { - updateLastReadActionID(myAccountID, this.props.reportID, maxSequenceNumber); - } - }); + updateLastReadActionID(this.props.reportID, maxSequenceNumber); } /** @@ -180,12 +169,11 @@ class ReportActionsView extends React.Component { ReportActionsView.propTypes = propTypes; ReportActionsView.defaultProps = defaultProps; -const key = `${IONKEYS.REPORT_ACTIONS}_%DATAFROMPROPS%`; export default compose( withRouter, withIon({ reportActions: { - key, + key: `${IONKEYS.REPORT_ACTIONS}_%DATAFROMPROPS%`, loader: fetchActions, loaderParams: ['%DATAFROMPROPS%'], pathForProps: 'reportID', diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index f6f224f389501..25c7854a3042a 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -10,7 +10,7 @@ import ChatSwitcherList from './ChatSwitcherList'; import ChatSwitcherSearchForm from './ChatSwitcherSearchForm'; import {fetchOrCreateChatReport} from '../../../lib/actions/Report'; -const personalDetailsPropTypes = PropTypes.objectOf(PropTypes.shape({ +const personalDetailsPropTypes = PropTypes.shape({ // The login of the person (either email or phone number) login: PropTypes.string.isRequired, @@ -33,7 +33,7 @@ const personalDetailsPropTypes = PropTypes.objectOf(PropTypes.shape({ // Either the user's full name and their login, or just the login if the full name is empty // `${fullName} (${login})` displayNameWithEmail: PropTypes.string.isRequired, -})); +}); const propTypes = { // A method that is triggered when the TextInput gets focus @@ -47,14 +47,14 @@ const propTypes = { // All of the personal details for everyone // The keys of this object are the logins of the users, and the values are an object // with their details - personalDetails: personalDetailsPropTypes, + personalDetails: PropTypes.objectOf(personalDetailsPropTypes), // The personal details of the person who is currently logged in myPersonalDetails: personalDetailsPropTypes, }; const defaultProps = { personalDetails: {}, - myPersonalDetails: {}, + myPersonalDetails: null, }; class ChatSwitcherView extends React.Component { From f2179d9c5271c8deef9c635a7184636749295199 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:30:48 -0600 Subject: [PATCH 06/47] Remove the exported method \o/ --- src/lib/Ion.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/Ion.js b/src/lib/Ion.js index b079b20677ed3..94ee1379bffec 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -243,7 +243,6 @@ const Ion = { disconnect, set, multiSet, - get, multiGet, merge, clear, From 08c231f448ade827c168b557e760bee7f67ff53c Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:42:06 -0600 Subject: [PATCH 07/47] Remove Ion.multiGet() from session action --- src/lib/actions/Session.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index c16aa8f8a8e96..ca073903cbccc 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -4,6 +4,18 @@ import IONKEYS from '../../IONKEYS'; import CONFIG from '../../CONFIG'; import redirectToSignIn from './SignInRedirect'; +let session; +Ion.connect({ + key: IONKEYS.SESSION, + callback: val => session = val, +}); + +let credentials; +Ion.connect({ + key: IONKEYS.CREDENTIALS, + callback: val => credentials = val, +}); + /** * Sign in with the API * @@ -54,8 +66,7 @@ function deleteLogin(authToken, login) { * Sign out of our application */ function signOut() { - Ion.multiGet([IONKEYS.SESSION, IONKEYS.CREDENTIALS]) - .then(data => deleteLogin(data.session.authToken, data.credentials.login)) + deleteLogin(session.authToken, credentials.login) .then(Ion.clear) .catch(err => Ion.merge(IONKEYS.SESSION, {error: err.message})); From ce44cacecf3ef749f718ade3241ffbbf2729c663 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 16:52:15 -0600 Subject: [PATCH 08/47] Remove Ion.multiGet() --- src/lib/Ion.js | 20 -------------------- src/lib/actions/Report.js | 19 +++++++++++++++---- src/lib/actions/Session.js | 5 +++-- src/page/home/sidebar/SidebarLinks.js | 13 +------------ 4 files changed, 19 insertions(+), 38 deletions(-) diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 94ee1379bffec..83eaf2de837f3 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -173,25 +173,6 @@ function getInitialStateFromConnectionID(connectionID) { return get(config.key, config.path, config.defaultValue); } -/** - * Get multiple keys of data - * - * @param {string[]} keys - * @returns {Promise} - */ -function multiGet(keys) { - // AsyncStorage returns the data in an array format like: - // [ ['@MyApp_user', 'myUserValue'], ['@MyApp_key', 'myKeyValue'] ] - // This method will transform the data into a better JSON format like: - // {'@MyApp_user': 'myUserValue', '@MyApp_key': 'myKeyValue'} - return AsyncStorage.multiGet(keys) - .then(arrayOfData => _.reduce(arrayOfData, (finalData, keyValuePair) => ({ - ...finalData, - [keyValuePair[0]]: JSON.parse(keyValuePair[1]), - }), {})) - .catch(err => console.error(`Unable to get item from persistent storage. Error: ${err}`, keys)); -} - /** * Sets multiple keys and values. Example * Ion.multiSet({'key1': 'a', 'key2': 'b'}); @@ -243,7 +224,6 @@ const Ion = { disconnect, set, multiSet, - multiGet, merge, clear, init, diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index dae5b02c5c8e4..eb07984e3270b 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -251,9 +251,12 @@ function fetchChatReports() { /** * Get all of our reports * + * @param {boolean} redirectToFirstReport this is set to false when the network reconnect + * code runes + * * @returns {Promise} */ -function fetchAll() { +function fetchAll(redirectToFirstReport = true) { let fetchedReports; // Request each report one at a time to allow individual reports to fail if access to it is prevented by Auth @@ -280,8 +283,16 @@ function fetchAll() { return _.isEmpty(report) ? null : _.values(report)[0]; })); - // Store the first report ID in Ion - Ion.set(IONKEYS.FIRST_REPORT_ID, _.first(_.pluck(fetchedReports, 'reportID')) || 0); + // Set the first report ID so that the logged in person can be redirected there + // if they are on the home page + if (redirectToFirstReport && currentURL === '/') { + const firstReportID = _.first(_.pluck(fetchedReports, 'reportID')) || 0; + + // If we're on the home page, then redirect to the first report ID + if (firstReportID) { + Ion.set(IONKEYS.APP_REDIRECT_TO, `/${firstReportID}`); + } + } _.each(fetchedReports, (report) => { // Merge the data into Ion. Don't use set() here or multiSet() because then that would @@ -463,7 +474,7 @@ Ion.connect({ // When the app reconnects from being offline, fetch all of the reports and their actions onReconnect(() => { - fetchAll().then(reports => _.each(reports, report => fetchActions(report.reportID))); + fetchAll(false).then(reports => _.each(reports, report => fetchActions(report.reportID))); }); export { fetchAll, diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index ca073903cbccc..c2f5f38540a76 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -49,8 +49,10 @@ function signIn(login, password, twoFactorAuthCode = '', exitTo, useExpensifyLog /** * Delete login + * * @param {string} authToken * @param {string} login + * * @returns {Promise} */ function deleteLogin(authToken, login) { @@ -67,10 +69,9 @@ function deleteLogin(authToken, login) { */ function signOut() { deleteLogin(session.authToken, credentials.login) + .then(redirectToSignIn) .then(Ion.clear) .catch(err => Ion.merge(IONKEYS.SESSION, {error: err.message})); - - redirectToSignIn(); } export { diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index 1af2ef40dd678..ee0f42d2f7b28 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -117,18 +117,7 @@ export default compose( key: `${IONKEYS.REPORT}_[0-9]+$`, addAsCollection: true, collectionID: 'reportID', - loader: () => fetchAll().then(() => { - // After the reports are loaded for the first time, redirect to the first reportID in the list - Ion.multiGet([IONKEYS.CURRENT_URL, IONKEYS.FIRST_REPORT_ID]).then((values) => { - const currentURL = values[IONKEYS.CURRENT_URL] || ''; - const firstReportID = values[IONKEYS.FIRST_REPORT_ID] || 0; - - // If we're on the home page, then redirect to the first report ID - if (currentURL === '/' && firstReportID) { - Ion.set(IONKEYS.APP_REDIRECT_TO, `/${firstReportID}`); - } - }); - }), + loader: fetchAll, } }), )(SidebarLinks); From 035ec3022e28138ee6f3b40fc23a9f11cc82f76b Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 4 Sep 2020 17:01:34 -0600 Subject: [PATCH 09/47] Fix timezone Fixes https://github.com/Expensify/ReactNativeChat/issues/402 --- src/lib/DateUtils.js | 22 ++++-------- src/lib/actions/PersonalDetails.js | 55 ++++++++++++++---------------- 2 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/lib/DateUtils.js b/src/lib/DateUtils.js index cac52395e2e93..6ca538b4e1882 100644 --- a/src/lib/DateUtils.js +++ b/src/lib/DateUtils.js @@ -4,19 +4,14 @@ import moment from 'moment'; // eslint-disable-next-line no-unused-vars import momentTimzone from 'moment-timezone'; import Str from './Str'; +import Ion from './Ion'; +import IONKEYS from '../IONKEYS'; -// Non-Deprecated Methods - -/** - * Gets the user's stored time-zone NVP - * - * @returns {string} - * - * @private - */ -function getTimezone() { - return 'America/Los_Angeles'; -} +let timezone; +Ion.connect({ + key: IONKEYS.MY_PERSONAL_DETAILS, + callback: val => timezone = val.timezone || 'America/Los_Angeles', +}); /** * Gets the user's stored time-zone NVP and returns a localized @@ -29,9 +24,6 @@ function getTimezone() { * @private */ function getLocalMomentFromTimestamp(timestamp) { - // We need a default here for flows where we may not have initialized the TIME_ZONE NVP like generatng PDFs in - // printablereport.php - const timezone = getTimezone(); return moment.unix(timestamp).tz(timezone); } diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index 4e52555fcd90e..d44f461548f29 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -1,4 +1,5 @@ import _ from 'underscore'; +import lodashGet from 'lodash.get'; import Ion from '../Ion'; import {onReconnect, queueRequest} from '../Network'; import IONKEYS from '../../IONKEYS'; @@ -59,6 +60,26 @@ function formatPersonalDetails(personalDetailsList) { }, {}); } +/** + * Get the timezone of the logged in user + * + * @returns {Promise} + */ +function fetchTimezone() { + const requestPromise = queueRequest('Get', { + returnValueList: 'nameValuePairs', + name: 'timeZone', + }) + .then((data) => { + const timezone = lodashGet(data, 'nameValuePairs.timeZone.selected', 'America/Los_Angeles'); + Ion.merge(IONKEYS.MY_PERSONAL_DETAILS, {timezone}); + }); + + // Refresh the timezone every 30 minutes + setTimeout(fetchTimezone, 1000 * 60 * 30); + return requestPromise; +} + /** * Get the personal details for our organization * @@ -74,6 +95,9 @@ function fetch() { // Get 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) => { if (error.message === 'No login') { @@ -90,26 +114,6 @@ function fetch() { return requestPromise; } -/** - * Get the timezone of the logged in user - * - * @returns {Promise} - */ -function fetchTimezone() { - const requestPromise = queueRequest('Get', { - returnValueList: 'nameValuePairs', - name: 'timeZone', - }) - .then((data) => { - const timezone = data.nameValuePairs.timeZone.selected || 'America/Los_Angeles'; - Ion.merge(IONKEYS.MY_PERSONAL_DETAILS, {timezone}); - }); - - // Refresh the timezone every 30 minutes - setTimeout(fetchTimezone, 1000 * 60 * 30); - return requestPromise; -} - /** * Get personal details for a list of emails. * @@ -117,15 +121,8 @@ function fetchTimezone() { * @returns {Promise} */ function getForEmails(emailList) { - let detailsFormatted; - return queueRequest('PersonalDetails_GetForEmails', { - emailList, - }) - .then((details) => { - detailsFormatted = formatPersonalDetails(details); - return Ion.merge(IONKEYS.PERSONAL_DETAILS, detailsFormatted); - }) - .then(() => detailsFormatted); + return queueRequest('PersonalDetails_GetForEmails', {emailList}) + .then(details => Ion.merge(IONKEYS.PERSONAL_DETAILS, formatPersonalDetails(details))); } // When the app reconnects from being offline, fetch all of the personal details From d800ef04f712e24e550e3f2a194c48332d5534e1 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 14:39:26 -0600 Subject: [PATCH 10/47] Remove path and default value --- src/Expensify.js | 10 ++++++--- src/lib/Ion.js | 26 +++++------------------ src/lib/Network.js | 10 +++++++-- src/lib/actions/Session.js | 7 +----- src/page/SignInPage.js | 18 ++++++++++------ src/page/home/HeaderView.js | 25 +++++++++++----------- src/page/home/report/ReportActionsView.js | 1 - src/page/home/sidebar/SidebarBottom.js | 23 ++++++++++---------- src/page/home/sidebar/SidebarLink.js | 17 +++++++++------ src/page/home/sidebar/SidebarLinks.js | 1 - 10 files changed, 65 insertions(+), 73 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index 3b4a8797425e7..751c1a052bf12 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -50,15 +50,19 @@ class Expensify extends Component { } componentDidMount() { - Ion.connect({key: IONKEYS.SESSION, path: 'authToken', callback: this.removeLoadingState}); + Ion.connect({ + key: IONKEYS.SESSION, + callback: this.removeLoadingState, + }); } /** * When the authToken is updated, the app should remove the loading state and handle the authToken * - * @param {string} authToken + * @param {object} session + * @param {string} session.authToken */ - removeLoadingState(authToken) { + removeLoadingState({authToken}) { this.setState({ authToken, isLoading: false, diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 83eaf2de837f3..a3a6389a0cbff 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -1,4 +1,3 @@ -import lodashGet from 'lodash.get'; import _ from 'underscore'; import AsyncStorage from '@react-native-community/async-storage'; @@ -27,20 +26,11 @@ const callbackToStateMapping = {}; * Get some data from the store * * @param {string} key - * @param {string} [extraPath] passed to _.get() in order to return just a piece of the localStorage object - * @param {mixed} [defaultValue] passed to the second param of _.get() in order to specify a default value if the value - * we are looking for doesn't exist in the object yet * @returns {*} */ -function get(key, extraPath, defaultValue) { +function get(key) { return AsyncStorage.getItem(key) .then(val => JSON.parse(val)) - .then((val) => { - if (extraPath) { - return lodashGet(val, extraPath, defaultValue); - } - return val; - }) .catch(err => console.error(`Unable to get item from persistent storage. Key: ${key} Error: ${err}`)); } @@ -54,12 +44,8 @@ function keyChanged(key, data) { // Find components that were added with connect() and trigger their setState() method with the new data _.each(callbackToStateMapping, (mappedComponent) => { if (mappedComponent && mappedComponent.regex.test(key)) { - const newValue = mappedComponent.path - ? lodashGet(data, mappedComponent.path, mappedComponent.defaultValue) - : data || mappedComponent.defaultValue || null; - if (_.isFunction(mappedComponent.callback)) { - mappedComponent.callback(newValue, key); + mappedComponent.callback(data, key); } if (!mappedComponent.withIonInstance) { @@ -71,14 +57,14 @@ function keyChanged(key, data) { // Add the data to an array of existing items mappedComponent.withIonInstance.setState((prevState) => { const collection = prevState[mappedComponent.statePropertyName] || {}; - collection[newValue[mappedComponent.collectionID]] = newValue; + collection[data[mappedComponent.collectionID]] = data; return { [mappedComponent.statePropertyName]: collection, }; }); } else { mappedComponent.withIonInstance.setState({ - [mappedComponent.statePropertyName]: newValue, + [mappedComponent.statePropertyName]: data, }); } } @@ -90,8 +76,6 @@ function keyChanged(key, data) { * * @param {object} mapping the mapping information to connect Ion to the components state * @param {string} mapping.keyPattern - * @param {string} [mapping.path] a specific path of the store object to map to the state - * @param {mixed} [mapping.defaultValue] to return if the there is nothing from the store * @param {string} mapping.statePropertyName the name of the property in the state to connect the data to * @param {boolean} [mapping.addAsCollection] rather than setting a single state value, this will add things to an array * @param {string} [mapping.collectionID] the name of the ID property to use for the collection @@ -170,7 +154,7 @@ function getInitialStateFromConnectionID(connectionID) { [value[config.collectionID]]: value, }), {})); } - return get(config.key, config.path, config.defaultValue); + return get(config.key); } /** diff --git a/src/lib/Network.js b/src/lib/Network.js index dbad2332c86eb..9e985f17f0aed 100644 --- a/src/lib/Network.js +++ b/src/lib/Network.js @@ -10,10 +10,16 @@ import Guid from './Guid'; import redirectToSignIn from './actions/SignInRedirect'; let authToken; -Ion.connect({key: IONKEYS.SESSION, path: 'authToken', callback: val => authToken = val}); +Ion.connect({ + key: IONKEYS.SESSION, + callback: val => authToken = val.authToken, +}); let isOffline; -Ion.connect({key: IONKEYS.NETWORK, path: 'isOffline', callback: val => isOffline = val}); +Ion.connect({ + key: IONKEYS.NETWORK, + callback: val => isOffline = val.isOffline, +}); let credentials; Ion.connect({key: IONKEYS.CREDENTIALS, callback: val => credentials = val}); diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index c2f5f38540a76..a198ceaa8ed7a 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -39,12 +39,7 @@ function signIn(login, password, twoFactorAuthCode = '', exitTo, useExpensifyLog partnerUserSecret: password, twoFactorAuthCode, exitTo - }) - .catch((err) => { - console.error(err); - console.debug('[SIGNIN] Request error'); - return Ion.merge(IONKEYS.SESSION, {error: err.message}); - }); + }).catch(err => Ion.merge(IONKEYS.SESSION, {error: err.message})); } /** diff --git a/src/page/SignInPage.js b/src/page/SignInPage.js index a51c99b193883..2eb08829f64f9 100644 --- a/src/page/SignInPage.js +++ b/src/page/SignInPage.js @@ -25,12 +25,17 @@ const propTypes = { /* Ion Props */ - // Error to display when there is a session error returned - error: PropTypes.string, + // The session of the logged in person + session: PropTypes.shape({ + // Error to display when there is a session error returned + error: PropTypes.string, + }), }; const defaultProps = { - error: null, + session: { + error: null, + }, }; class App extends Component { @@ -114,9 +119,9 @@ class App extends Component { > Log In - {this.props.error && ( + {this.props.session.error && ( - {this.props.error} + {this.props.session.error} )} @@ -133,7 +138,6 @@ App.defaultProps = defaultProps; export default compose( withRouter, withIon({ - // Bind this.props.error to the error in the session object - error: {key: IONKEYS.SESSION, path: 'error', defaultValue: null}, + session: {key: IONKEYS.SESSION}, }) )(App); diff --git a/src/page/home/HeaderView.js b/src/page/home/HeaderView.js index 3a74ddf5a6642..29b0f10206004 100644 --- a/src/page/home/HeaderView.js +++ b/src/page/home/HeaderView.js @@ -17,13 +17,17 @@ const propTypes = { shouldShowHamburgerButton: PropTypes.bool.isRequired, /* Ion Props */ - - // Name of the report (if we have one) - reportName: PropTypes.string, + // The report currently being looked at + report: PropTypes.shape({ + // Name of the report + reportName: PropTypes.string, + }), }; const defaultProps = { - reportName: null, + report: { + reportName: '', + }, }; const HeaderView = props => ( @@ -41,11 +45,11 @@ const HeaderView = props => ( /> )} - {props.reportName && ( + {props.report && props.report.reportName ? ( - {props.reportName} + {props.report.reportName} - )} + ) : null} ); @@ -57,13 +61,8 @@ HeaderView.defaultProps = defaultProps; export default compose( withRouter, withIon({ - // Map this.props.reportName to the data for a specific report in the store, - // and bind it to the reportName property. - // It uses the data returned from the props path (ie. the reportID) to replace %DATAFROMPROPS% in the key it - // binds to - reportName: { + report: { key: `${IONKEYS.REPORT}_%DATAFROMPROPS%`, - path: 'reportName', pathForProps: 'match.params.reportID', }, }), diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 4b56bb7d8fb9d..e3e9e8819feac 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -4,7 +4,6 @@ import PropTypes from 'prop-types'; import _ from 'underscore'; import lodashGet from 'lodash.get'; import Text from '../../../components/Text'; -import Ion from '../../../lib/Ion'; import withIon from '../../../components/withIon'; import {fetchActions, updateLastReadActionID} from '../../../lib/actions/Report'; import IONKEYS from '../../../IONKEYS'; diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index bc6901ffca9f3..5aec81125617a 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -25,19 +25,24 @@ const propTypes = { avatarURL: PropTypes.string, }), - // Is this person offline? - isOffline: PropTypes.bool, + // Information about the network + network: PropTypes.shape({ + // Is the network currently offline or not + isOffline: PropTypes.bool, + }) }; const defaultProps = { myPersonalDetails: {}, - isOffline: false, + network: { + isOffline: false, + } }; -const SidebarBottom = ({myPersonalDetails, isOffline, insets}) => { +const SidebarBottom = ({myPersonalDetails, network, insets}) => { const indicatorStyles = [ styles.statusIndicator, - isOffline ? styles.statusIndicatorOffline : styles.statusIndicatorOnline + network.isOffline ? styles.statusIndicatorOffline : styles.statusIndicatorOnline ]; // On the very first sign in or after clearing storage these @@ -76,15 +81,9 @@ SidebarBottom.defaultProps = defaultProps; SidebarBottom.displayName = 'SidebarBottom'; export default withIon({ - // Map this.props.userDisplayName to the personal details key in the store and bind it to the displayName property - // and load it with data from getPersonalDetails() myPersonalDetails: { key: IONKEYS.MY_PERSONAL_DETAILS, loader: getPersonalDetails, }, - isOffline: { - key: IONKEYS.NETWORK, - path: 'isOffline', - defaultValue: false, - }, + network: {key: IONKEYS.NETWORK}, })(SidebarBottom); diff --git a/src/page/home/sidebar/SidebarLink.js b/src/page/home/sidebar/SidebarLink.js index 46b44dcf35ed5..df2572341e4ad 100644 --- a/src/page/home/sidebar/SidebarLink.js +++ b/src/page/home/sidebar/SidebarLink.js @@ -25,12 +25,17 @@ const propTypes = { /* Ion Props */ - // Does the report for this link have unread comments? - isUnread: PropTypes.bool, + // The report object for this link + report: PropTypes.shape({ + // Does the report for this link have unread comments? + isUnread: PropTypes.bool, + }), }; const defaultProps = { - isUnread: false, + report: { + isUnread: false, + }, reportName: '', }; @@ -40,7 +45,7 @@ const SidebarLink = (props) => { const linkWrapperActiveStyle = isReportActive && styles.sidebarLinkWrapperActive; const linkActiveStyle = isReportActive ? styles.sidebarLinkActive : styles.sidebarLink; const textActiveStyle = isReportActive ? styles.sidebarLinkActiveText : styles.sidebarLinkText; - const textActiveUnreadStyle = props.isUnread + const textActiveUnreadStyle = props.report.isUnread ? [textActiveStyle, styles.sidebarLinkTextUnread] : [textActiveStyle]; return ( @@ -63,10 +68,8 @@ SidebarLink.defaultProps = defaultProps; export default compose( withRouter, withIon({ - isUnread: { + report: { key: `${IONKEYS.REPORT}_%DATAFROMPROPS%`, - path: 'hasUnread', - defaultValue: false, pathForProps: 'reportID', } }), diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index ee0f42d2f7b28..6bcec0ccdaf60 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -9,7 +9,6 @@ import SidebarLink from './SidebarLink'; import withIon from '../../../components/withIon'; import IONKEYS from '../../../IONKEYS'; import {fetchAll} from '../../../lib/actions/Report'; -import Ion from '../../../lib/Ion'; import PageTitleUpdater from '../../../lib/PageTitleUpdater'; import ChatSwitcherView from './ChatSwitcherView'; import SafeAreaInsetPropTypes from '../../SafeAreaInsetPropTypes'; From f50712aee46fcf4bafddb44fdf40958b03c0f5ec Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 15:42:17 -0600 Subject: [PATCH 11/47] Move initialization of Ion values into connect() --- src/components/withIon.js | 8 +-- src/lib/Ion.js | 89 +++++++++++++++------------ src/lib/actions/Report.js | 2 +- src/page/home/MainView.js | 2 +- src/page/home/sidebar/SidebarLinks.js | 2 +- 5 files changed, 55 insertions(+), 48 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index 2761536f626fc..ae8a183965cfe 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -72,7 +72,7 @@ export default function (mapIonToState) { * nothing at mapping.path * @param {boolean} [mapping.addAsCollection] rather than setting a single state value, this will add things * to an array - * @param {string} [mapping.collectionID] the name of the ID property to use for the collection + * @param {string} [mapping.indexBy] the name of the ID property to use for the collection * @param {string} [mapping.pathForProps] the statePropertyName can contain the string %DATAFROMPROPS% wich * will be replaced with data from the props matching this path. That way, the component can connect to an * Ion key that uses data from this.props. @@ -113,12 +113,6 @@ export default function (mapIonToState) { this.actionConnectionIDs[connectionID] = connectionID; } - // Pre-fill the state with any data already in the store - if (mapping.initWithStoredValues !== false) { - Ion.getInitialStateFromConnectionID(connectionID) - .then(data => this.setState({[statePropertyName]: data})); - } - // Load the data from an API request if necessary if (mapping.loader) { const paramsForLoaderFunction = _.map(mapping.loaderParams, (loaderParam) => { diff --git a/src/lib/Ion.js b/src/lib/Ion.js index a3a6389a0cbff..bc88460ec342a 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -57,7 +57,7 @@ function keyChanged(key, data) { // Add the data to an array of existing items mappedComponent.withIonInstance.setState((prevState) => { const collection = prevState[mappedComponent.statePropertyName] || {}; - collection[data[mappedComponent.collectionID]] = data; + collection[data[mappedComponent.indexBy]] = data; return { [mappedComponent.statePropertyName]: collection, }; @@ -78,31 +78,69 @@ function keyChanged(key, data) { * @param {string} mapping.keyPattern * @param {string} mapping.statePropertyName the name of the property in the state to connect the data to * @param {boolean} [mapping.addAsCollection] rather than setting a single state value, this will add things to an array - * @param {string} [mapping.collectionID] the name of the ID property to use for the collection + * @param {string} [mapping.indexBy] the name of a property to index the collection by * @param {object} [mapping.withIonInstance] whose setState() method will be called with any changed data * This is used by React components to connect to Ion * @param {object} [mapping.callback] a method that will be called with changed data * This is used by any non-React code to connect to Ion + * @param {boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the + * component * @returns {number} an ID to use when calling disconnect */ function connect(mapping) { const connectionID = lastConnectionID++; - const connectionMapping = { + const config = { ...mapping, regex: RegExp(mapping.key), }; - callbackToStateMapping[connectionID] = connectionMapping; - - // If the mapping has a callback, trigger it with the existing data - // in Ion so it initializes properly - // @TODO remove the if statement when this is supported by react components - // @TODO need to support full regex key connections for callbacks. - // This would look something like getInitialStateFromConnectionID - if (mapping.callback) { - get(mapping.key) - .then(val => keyChanged(mapping.key, val)); + callbackToStateMapping[connectionID] = config; + + if (mapping.initWithStoredValues === false) { + return connectionID; } + /** + * Sends the data obtained from the keys to the connection. It either: + * - sets state on the withIonInstances + * - triggers the callback function + * + * @param {*} val + * @param {string} [key] + */ + function sendDataToConnection(val, key) { + if (config.withIonInstance) { + config.withIonInstance.setState({ + [config.statePropertyName]: val, + }); + } else if (_.isFunction(config.callback)) { + config.callback(val, key); + } + } + + // Get all the data from Ion to initialize the connection with + AsyncStorage.getAllKeys() + .then((keys) => { + // Find all the keys matched by the config regex + const matchingKeys = _.filter(keys, key => config.regex.test(key)); + + if (matchingKeys.length === 0) { + return; + } + + if (config.indexBy) { + Promise.all(_.map(matchingKeys, key => get(key))) + .then(values => _.reduce(values, (finalObject, value) => ({ + ...finalObject, + [value[config.indexBy]]: value, + }), {})) + .then(sendDataToConnection); + } else { + _.each(matchingKeys, (key) => { + get(key).then(val => sendDataToConnection(val, key)); + }); + } + }); + return connectionID; } @@ -133,30 +171,6 @@ function set(key, val) { }); } -/** - * Returns initial state for a connection config so that stored data - * is available shortly after the first render. - * - * @param {Number} connectionID - * @return {Promise} - */ -function getInitialStateFromConnectionID(connectionID) { - const config = callbackToStateMapping[connectionID]; - if (config.addAsCollection) { - return AsyncStorage.getAllKeys() - .then((keys) => { - const regex = RegExp(config.key); - const matchingKeys = _.filter(keys, key => regex.test(key)); - return Promise.all(_.map(matchingKeys, key => get(key))); - }) - .then(values => _.reduce(values, (finalObject, value) => ({ - ...finalObject, - [value[config.collectionID]]: value, - }), {})); - } - return get(config.key); -} - /** * Sets multiple keys and values. Example * Ion.multiSet({'key1': 'a', 'key2': 'b'}); @@ -211,7 +225,6 @@ const Ion = { merge, clear, init, - getInitialStateFromConnectionID, }; export default Ion; diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index eb07984e3270b..9feba000aefaa 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -461,7 +461,7 @@ function updateLastReadActionID(reportID, sequenceNumber) { * @param {object} report */ function handleReportChanged(report) { - if (report.reportName === undefined) { + if (report && report.reportName === undefined) { fetchChatReportsByIDs([report.reportID]); } diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index 80b9eb97bfc29..79a79a9dab8a8 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -80,7 +80,7 @@ export default compose( reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, addAsCollection: true, - collectionID: 'reportID', + indexBy: 'reportID', }, }), )(MainView); diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index 6bcec0ccdaf60..0886238f9877d 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -115,7 +115,7 @@ export default compose( reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, addAsCollection: true, - collectionID: 'reportID', + indexBy: 'reportID', loader: fetchAll, } }), From b1e7dde36850e4679bf0bfdd6da662c79dd43468 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 17:07:53 -0600 Subject: [PATCH 12/47] Remove addAsCollection --- src/components/withIon.js | 5 ----- src/lib/Ion.js | 5 ++--- src/page/home/MainView.js | 1 - src/page/home/sidebar/SidebarLinks.js | 1 - 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index ae8a183965cfe..180dcce4650fa 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -67,11 +67,6 @@ export default function (mapIonToState) { * Takes a single mapping and binds the state of the component to the store * * @param {object} mapping - * @param {string} [mapping.path] a specific path of the store object to map to the state - * @param {mixed} [mapping.defaultValue] Used in conjunction with mapping.path to return if the there is - * nothing at mapping.path - * @param {boolean} [mapping.addAsCollection] rather than setting a single state value, this will add things - * to an array * @param {string} [mapping.indexBy] the name of the ID property to use for the collection * @param {string} [mapping.pathForProps] the statePropertyName can contain the string %DATAFROMPROPS% wich * will be replaced with data from the props matching this path. That way, the component can connect to an diff --git a/src/lib/Ion.js b/src/lib/Ion.js index bc88460ec342a..a83be0461399f 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -52,8 +52,8 @@ function keyChanged(key, data) { return; } - // Set the state of the react component with either the pathed data, or the data - if (mappedComponent.addAsCollection) { + // Set the state of the react component with the data + if (mappedComponent.indexBy) { // Add the data to an array of existing items mappedComponent.withIonInstance.setState((prevState) => { const collection = prevState[mappedComponent.statePropertyName] || {}; @@ -77,7 +77,6 @@ function keyChanged(key, data) { * @param {object} mapping the mapping information to connect Ion to the components state * @param {string} mapping.keyPattern * @param {string} mapping.statePropertyName the name of the property in the state to connect the data to - * @param {boolean} [mapping.addAsCollection] rather than setting a single state value, this will add things to an array * @param {string} [mapping.indexBy] the name of a property to index the collection by * @param {object} [mapping.withIonInstance] whose setState() method will be called with any changed data * This is used by React components to connect to Ion diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index 79a79a9dab8a8..f8bfc2acf4f05 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -79,7 +79,6 @@ export default compose( withIon({ reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, - addAsCollection: true, indexBy: 'reportID', }, }), diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index 0886238f9877d..7fdeed9da2cdc 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -114,7 +114,6 @@ export default compose( withIon({ reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, - addAsCollection: true, indexBy: 'reportID', loader: fetchAll, } From 873c032097a9d7885f7934e4aae8bf6d31538a79 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 17:13:50 -0600 Subject: [PATCH 13/47] Move loaders into the home page --- src/page/home/HomePage.js | 10 ++++++++-- src/page/home/sidebar/SidebarBottom.js | 2 -- src/page/home/sidebar/SidebarLinks.js | 2 -- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/page/home/HomePage.js b/src/page/home/HomePage.js index 6f7e1e40c2e38..394f60aaa8818 100644 --- a/src/page/home/HomePage.js +++ b/src/page/home/HomePage.js @@ -12,7 +12,8 @@ import styles, {getSafeAreaPadding} from '../../style/StyleSheet'; import Header from './HeaderView'; import Sidebar from './sidebar/SidebarView'; import Main from './MainView'; -import {subscribeToReportCommentEvents} from '../../lib/actions/Report'; +import {subscribeToReportCommentEvents, fetchAll as fetchAllReports} from '../../lib/actions/Report'; +import {fetch as fetchPersonalDetails} from '../../lib/actions/PersonalDetails'; const windowSize = Dimensions.get('window'); const widthBreakPoint = 1000; @@ -35,6 +36,12 @@ export default class App extends React.Component { // Listen for report comment events subscribeToReportCommentEvents(); + // Fetch all the personal details + fetchPersonalDetails(); + + // Fetch all the reports + fetchAllReports(); + Dimensions.addEventListener('change', this.toggleHamburgerBasedOnDimensions); StatusBar.setBarStyle('dark-content', true); @@ -134,4 +141,3 @@ export default class App extends React.Component { ); } } -App.displayName = 'App'; diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index 5aec81125617a..10bb400c503f7 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -6,7 +6,6 @@ import Text from '../../../components/Text'; import AppLinks from './AppLinks'; import {signOut} from '../../../lib/actions/Session'; import IONKEYS from '../../../IONKEYS'; -import {fetch as getPersonalDetails} from '../../../lib/actions/PersonalDetails'; import withIon from '../../../components/withIon'; import SafeAreaInsetPropTypes from '../../SafeAreaInsetPropTypes'; @@ -83,7 +82,6 @@ SidebarBottom.displayName = 'SidebarBottom'; export default withIon({ myPersonalDetails: { key: IONKEYS.MY_PERSONAL_DETAILS, - loader: getPersonalDetails, }, network: {key: IONKEYS.NETWORK}, })(SidebarBottom); diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index 7fdeed9da2cdc..60f698bf8593d 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -8,7 +8,6 @@ import Text from '../../../components/Text'; import SidebarLink from './SidebarLink'; import withIon from '../../../components/withIon'; import IONKEYS from '../../../IONKEYS'; -import {fetchAll} from '../../../lib/actions/Report'; import PageTitleUpdater from '../../../lib/PageTitleUpdater'; import ChatSwitcherView from './ChatSwitcherView'; import SafeAreaInsetPropTypes from '../../SafeAreaInsetPropTypes'; @@ -115,7 +114,6 @@ export default compose( reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, indexBy: 'reportID', - loader: fetchAll, } }), )(SidebarLinks); From 89220f1b6ac1156d8a3c716e0df74ae4c7504346 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 17:21:49 -0600 Subject: [PATCH 14/47] Removed last loader --- src/page/home/report/ReportActionsView.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index e3e9e8819feac..fac26556750fc 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -42,10 +42,11 @@ class ReportActionsView extends React.Component { componentDidMount() { this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); + fetchActions(this.props.reportID); } componentDidUpdate(prevProps) { - const isReportVisible = this.props.reportID === this.props.match.params.reportID; + const isReportVisible = this.props.reportID === parseInt(this.props.match.params.reportID, 10); // When the number of actions change, wait three seconds, then record the max action // This will make the unread indicator go away if you receive comments in the same chat you're looking at @@ -173,8 +174,6 @@ export default compose( withIon({ reportActions: { key: `${IONKEYS.REPORT_ACTIONS}_%DATAFROMPROPS%`, - loader: fetchActions, - loaderParams: ['%DATAFROMPROPS%'], pathForProps: 'reportID', }, }), From 8dd64ed05ece17b3b3b59e15e4e00e2c1d123a7a Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 5 Sep 2020 17:22:41 -0600 Subject: [PATCH 15/47] Remove code for the loader --- src/components/withIon.js | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index 180dcce4650fa..c8528dae7c78e 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -67,6 +67,7 @@ export default function (mapIonToState) { * Takes a single mapping and binds the state of the component to the store * * @param {object} mapping + * @param {string} statePropertyName the name of the state property that Ion will add the data to * @param {string} [mapping.indexBy] the name of the ID property to use for the collection * @param {string} [mapping.pathForProps] the statePropertyName can contain the string %DATAFROMPROPS% wich * will be replaced with data from the props matching this path. That way, the component can connect to an @@ -75,12 +76,8 @@ export default function (mapIonToState) { * For example, if a component wants to connect to the Ion key "report_22" and * "22" comes from this.props.match.params.reportID. The statePropertyName would be set to * "report_%DATAFROMPROPS%" and pathForProps would be set to "match.params.reportID" - * @param {function} [mapping.loader] a method that will be called after connection to Ion in order to load - * it with data. Typically this will be a method that makes an XHR to load data from the API. - * @param {mixed[]} [mapping.loaderParams] An array of params to be passed to the loader method * @param {boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the * component - * @param {string} statePropertyName the name of the state property that Ion will add the data to */ connectMappingToIon(mapping, statePropertyName) { const ionConnectionConfig = { @@ -107,21 +104,6 @@ export default function (mapIonToState) { connectionID = Ion.connect(ionConnectionConfig); this.actionConnectionIDs[connectionID] = connectionID; } - - // Load the data from an API request if necessary - if (mapping.loader) { - const paramsForLoaderFunction = _.map(mapping.loaderParams, (loaderParam) => { - // Some params might com from the props data - if (loaderParam === '%DATAFROMPROPS%') { - return lodashGet(this.props, mapping.pathForProps); - } - return loaderParam; - }); - - // Call the loader function and pass it any params. The loader function will take care of putting - // data into Ion - mapping.loader(...paramsForLoaderFunction || []); - } } render() { From 6ef55358b4373449ad92b4c5e00076c24fb23562 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 7 Sep 2020 14:00:12 -0600 Subject: [PATCH 16/47] Add more stuff to the philosophy --- README.md | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 10e537ac2031f..0cf20475fc9d3 100644 --- a/README.md +++ b/README.md @@ -2,15 +2,34 @@ # Philosophy This application is built with the following principles. -1. **Offline first** - All data that is brought into the app should be stored immediately in Ion which puts the data into persistent storage (eg. localStorage on browser platforms). -1. **UI Binds to Ion** - UI components bind to Ion so that any change to the Ion data is automatically reflected in the component by calling setState() with the changed data. -1. **Actions manage Ion Data** - When the UI needs to request or write data from the server, this is done through Actions exclusively. - 1. Actions should never return data, see the first point. Example: if the action is `fetchReports()`, it does not return the reports, `fetchReports()` returns nothing. The action makes an XHR, then puts the data into Ion (using `Ion.set()` or `Ion.merge()`). Any UI that is subscribed to that piece of data in Ion is automatically updated. +1. **Offline first** + - All data that is brought into the app should be stored immediately in Ion which puts the data into persistent storage (eg. localStorage on browser platforms) + - All data that is displayed, comes from persistent storage (Ion) +1. **UI Binds to Ion** + - UI components bind to Ion (with `Ion.connect()`) so that any change to the Ion data is automatically reflected in the component by calling setState() with the changed data. + - Libraries bind to Ion (with `Ion.connect()`) and use a callback instead of having `setState()` be called + - The UI should be as flexible as possible when it comes to: + - Incomplete data (always assume data is incomplete or not there) + - Order of events (all operations can and should happen in parallel rather than in sequence) +1. **Actions manage Ion Data** + - When data needs to be written to or read from the server, this is done through Actions exclusively + - Actions should never return data, see the first point. Example: if the action is `fetchReports()`, it does not return the reports, `fetchReports()` returns nothing. The action makes an XHR, then puts the data into Ion (using `Ion.set()` or `Ion.merge()`). + - Actions should favor using `Ion.merge()` over `Ion.set()` so that other values in an object aren't completely overwritten + - All Ion operations should happen in parallel and never in sequence + - If an Action needs access to Ion data, use a local variable and `Ion.connect()` + - Data should be optimistically added to Ion whenever possible without waiting for a server response 1. **Cross Platform 99.9999%** 1. A feature isn't done until it works on all platforms. Accordingly, don't even bother writing a platform-specific code block because you're just going to need to undo it. 1. If the reason you can't write cross platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths. 1. If there is a feature that simply doesn't exist on all platforms and thus doesn't exist in RN, rather than doing if (platform=iOS) { }, instead write a "shim" library that is implemented with NOOPs on the other platforms. For example, rather than injecting platform-specific multi-tab code (which can only work on browsers, because it's the only platform with multiple tabs), write a TabManager class that just is NOOP for non-browser platforms. This encapsulates the platform-specific code into a platform library, rather than sprinkling through the business logic. - 1. Put all platform specific code in a dedicated branch, like /platform, and reject any PR that attempts to put platform-specific code anywhere else. This maintains a strict separation between business logic and platform code. + 1. Put all platform specific code in a dedicated files and folders, like /platform, and reject any PR that attempts to put platform-specific code anywhere else. This maintains a strict separation between business logic and platform code. +1. **Data Flow** - Ideally, this is how data flows through the app: + 1. Server pushes data to the disk of any client + 1. Disk pushes data to the UI + 1. UI pushes data to people's brains + 1. Brain pushes data into UI inputs + 1. UI inputs pushes data to the server + 1. Go to 1 ## Getting Started 1. Install `node` & `npm`: `brew install node` From 48dab3503f00b71bca65b177a979f05c8692eabe Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 7 Sep 2020 14:49:31 -0600 Subject: [PATCH 17/47] Improve explanation of callback connect --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0cf20475fc9d3..30c43862be1ee 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ This application is built with the following principles. - All data that is displayed, comes from persistent storage (Ion) 1. **UI Binds to Ion** - UI components bind to Ion (with `Ion.connect()`) so that any change to the Ion data is automatically reflected in the component by calling setState() with the changed data. - - Libraries bind to Ion (with `Ion.connect()`) and use a callback instead of having `setState()` be called + - Libraries bind to Ion (with `Ion.connect()`) and use a callback which is triggered with the changed data - The UI should be as flexible as possible when it comes to: - Incomplete data (always assume data is incomplete or not there) - Order of events (all operations can and should happen in parallel rather than in sequence) From fe07f44cac0cc6867974669425544a3fe4aa6e99 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 7 Sep 2020 14:50:59 -0600 Subject: [PATCH 18/47] Move data flow to the beginning --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 30c43862be1ee..d014a7c88e232 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,13 @@ # Philosophy This application is built with the following principles. +1. **Data Flow** - Ideally, this is how data flows through the app: + 1. Server pushes data to the disk of any client + 1. Disk pushes data to the UI + 1. UI pushes data to people's brains + 1. Brain pushes data into UI inputs + 1. UI inputs pushes data to the server + 1. Go to 1 1. **Offline first** - All data that is brought into the app should be stored immediately in Ion which puts the data into persistent storage (eg. localStorage on browser platforms) - All data that is displayed, comes from persistent storage (Ion) @@ -23,13 +30,6 @@ This application is built with the following principles. 1. If the reason you can't write cross platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths. 1. If there is a feature that simply doesn't exist on all platforms and thus doesn't exist in RN, rather than doing if (platform=iOS) { }, instead write a "shim" library that is implemented with NOOPs on the other platforms. For example, rather than injecting platform-specific multi-tab code (which can only work on browsers, because it's the only platform with multiple tabs), write a TabManager class that just is NOOP for non-browser platforms. This encapsulates the platform-specific code into a platform library, rather than sprinkling through the business logic. 1. Put all platform specific code in a dedicated files and folders, like /platform, and reject any PR that attempts to put platform-specific code anywhere else. This maintains a strict separation between business logic and platform code. -1. **Data Flow** - Ideally, this is how data flows through the app: - 1. Server pushes data to the disk of any client - 1. Disk pushes data to the UI - 1. UI pushes data to people's brains - 1. Brain pushes data into UI inputs - 1. UI inputs pushes data to the server - 1. Go to 1 ## Getting Started 1. Install `node` & `npm`: `brew install node` From b472695187e95cc450f812c27396e029eaf3226e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 7 Sep 2020 14:52:51 -0600 Subject: [PATCH 19/47] Wrap code in code block --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d014a7c88e232..912e082b90b85 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ This application is built with the following principles. - All data that is brought into the app should be stored immediately in Ion which puts the data into persistent storage (eg. localStorage on browser platforms) - All data that is displayed, comes from persistent storage (Ion) 1. **UI Binds to Ion** - - UI components bind to Ion (with `Ion.connect()`) so that any change to the Ion data is automatically reflected in the component by calling setState() with the changed data. + - UI components bind to Ion (with `Ion.connect()`) so that any change to the Ion data is automatically reflected in the component by calling `setState()` with the changed data. - Libraries bind to Ion (with `Ion.connect()`) and use a callback which is triggered with the changed data - The UI should be as flexible as possible when it comes to: - Incomplete data (always assume data is incomplete or not there) From 5d79d9f888d799bd679b312707f2fce88470d05a Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 8 Sep 2020 07:54:37 -0600 Subject: [PATCH 20/47] Add more clarity --- README.md | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 912e082b90b85..9413acf60f2dd 100644 --- a/README.md +++ b/README.md @@ -3,28 +3,32 @@ # Philosophy This application is built with the following principles. 1. **Data Flow** - Ideally, this is how data flows through the app: - 1. Server pushes data to the disk of any client - 1. Disk pushes data to the UI - 1. UI pushes data to people's brains - 1. Brain pushes data into UI inputs - 1. UI inputs pushes data to the server + 1. Server pushes data to the disk of any client (Server -> Pusher event -> Action listening to pusher event -> Ion). + 1. Disk pushes data to the UI (Ion -> withIon()/connect() -> React component). + 1. UI pushes data to people's brains (React component -> device screen). + 1. Brain pushes data into UI inputs (Device input -> React component). + 1. UI inputs pushes data to the server (React component -> Action -> XHR to server). 1. Go to 1 1. **Offline first** - - All data that is brought into the app should be stored immediately in Ion which puts the data into persistent storage (eg. localStorage on browser platforms) - - All data that is displayed, comes from persistent storage (Ion) -1. **UI Binds to Ion** - - UI components bind to Ion (with `Ion.connect()`) so that any change to the Ion data is automatically reflected in the component by calling `setState()` with the changed data. - - Libraries bind to Ion (with `Ion.connect()`) and use a callback which is triggered with the changed data + - All data that is brought into the app should be stored immediately on disk in persistent storage (eg. localStorage on browser platforms). + - All data that is displayed, comes from persistent storage. +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 bind to Ion (using `withIon()`) and any change to the Ion data is automatically reflected in the component by calling `setState()` with the changed data. + - Libraries bind to Ion (with `Ion.connect()`) and use a callback which is triggered 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 interacts with an Action when something needs to happen (eg. a person inputs data, the UI passes that data to an Action to be processed). - The UI should be as flexible as possible when it comes to: - - Incomplete data (always assume data is incomplete or not there) + - Incomplete or missing data (always assume data is incomplete or not there) - Order of events (all operations can and should happen in parallel rather than in sequence) 1. **Actions manage Ion Data** - - When data needs to be written to or read from the server, this is done through Actions exclusively - - Actions should never return data, see the first point. Example: if the action is `fetchReports()`, it does not return the reports, `fetchReports()` returns nothing. The action makes an XHR, then puts the data into Ion (using `Ion.set()` or `Ion.merge()`). - - Actions should favor using `Ion.merge()` over `Ion.set()` so that other values in an object aren't completely overwritten - - All Ion operations should happen in parallel and never in sequence - - If an Action needs access to Ion data, use a local variable and `Ion.connect()` - - Data should be optimistically added to Ion whenever possible without waiting for a server response + - When data needs to be written to or read from the server, this is done through Actions only. + - Action methods should never return anything (not data or a promise). There are very few exceptions to this. The only time an action is allowed to return a promise or data is for internal use by that action only. Any libraries, actions, or React components that need access to the data from the action should be subscribing to that data with Ion. + - Actions should favor using `Ion.merge()` over `Ion.set()` so that other values in an object aren't completely overwritten. + - All Ion operations should happen in parallel and never in sequence (eg. don't use the promise of one Ion method to trigger a second Ion method). + - The only time Actions should use promises to do operations in sequence is when working with XHRs. + - 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 (eg. creating a new comment) 1. **Cross Platform 99.9999%** 1. A feature isn't done until it works on all platforms. Accordingly, don't even bother writing a platform-specific code block because you're just going to need to undo it. 1. If the reason you can't write cross platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths. From eec5bda3f187e09d91711ffc9e84636a4563b909 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 8 Sep 2020 13:24:30 -0600 Subject: [PATCH 21/47] Add more info about how the app is structured --- README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/README.md b/README.md index 9413acf60f2dd..2d62dc204f80b 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ This application is built with the following principles. 3. Install dependencies: `npm install` 4. Run `cp .env.example .env` and edit `.env` to have your local config options +You can use any IDE or code editing tool for developing on any platform. Use your favorite! ## Running the web app 🕸 * To run a **Development Server**: `npm run web` @@ -90,3 +91,37 @@ This application is built with the following principles. 1. If running on the iOS simulator `⌘D`, or `⌘M` on Android emulator will open the debugging menu. 2. This will allow you to attach a debugger in your IDE, React Developer Tools, or your browser. 3. For more information on how to attach a debugger, see [React Native Debugging Documentation](https://reactnative.dev/docs/debugging#chrome-developer-tools) + +## Things to know or brush up on before jumping into the code +1. The major difference between React-Native and React are the [components](https://reactnative.dev/docs/components-and-apis) that are used in the `render()` method. Everything else is exactly the same. If you learn React, you've already learned 98% of React-Native. +1. The application uses [React-Router](https://reactrouter.com/native/guides/quick-start) for navigating between parts of the app. +1. [Higher Order Components](https://reactjs.org/docs/higher-order-components.html) are used to connect React components to persistent storage via Ion. + +## Structure of the app +These are the main pieces of the application. + +### Ion +This is a persistent storage solution wrapped in a Pub/Sub library. In general that means: + +- Ion stores and retrieves data from persistent storage +- Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object +- Collections of data are usually not stored as a single key (eg. an array with multiple objects), but as individual keys+ID (eg. `report_1234`, `report_4567`, etc.) +- Ion allows other code to subscribe to changes in data, and then publishes change events whenever data is changed +- Anything needing to read Ion data needs to: + 1. Know what key the data is stored in (for web, you can find this by looking in the JS console > Application > local storage) + 2. Subscribe to changes of the data for a particular key or set of keys. React components use `withIon()` and non-React libs use `Ion.connect()`. + 3. Get initialized with the current value of that key from persistent storage (Ion does this automatically as part of the connection process) +- Subscribing to Ion keys is done using a regex pattern. For example, since all reports are stored as individual keys like `report_1234`, then if code needs to know about all the reports (eg. display a list of them in the nav menu), then it would subscribe to the key pattern `report_[0-9]+$`. + +### Actions +Actions are responsible for managing what is on disk. This is usually: + +- Subscribing to Pusher events to receive data from the server that will get put immediately into Ion +- Making XHRs to request necessary data from the server and then immediately putting that data into Ion +- Handling any business logic with input coming from the UI layer + +### The UI layer +This layer is soley responsible for: + +- Reflecting exactly the data that is in persistent storage by using `withIon()` to bind to Ion data. +- Taking user input and passing it to an action From 6b431c878faeb3f41a7fb2319bd61e6c9a1abb7d Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 8 Sep 2020 13:25:49 -0600 Subject: [PATCH 22/47] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2d62dc204f80b..c843ea94884da 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Actions are responsible for managing what is on disk. This is usually: - Handling any business logic with input coming from the UI layer ### The UI layer -This layer is soley responsible for: +This layer is solely responsible for: - Reflecting exactly the data that is in persistent storage by using `withIon()` to bind to Ion data. - Taking user input and passing it to an action From ae35b0d7836ef35bf65db3900f6999ff9d725d77 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 8 Sep 2020 17:18:57 -0600 Subject: [PATCH 23/47] Combine into a single merge --- src/lib/actions/Report.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index 9feba000aefaa..9b371d74dabc0 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -183,17 +183,14 @@ function updateReportWithNewAction(reportID, reportAction) { const previousMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; const newMaxSequenceNumber = reportAction.sequenceNumber; + const hasNewSequenceNumber = newMaxSequenceNumber > previousMaxSequenceNumber; // Mark the report as unread if there is a new max sequence number - if (newMaxSequenceNumber > previousMaxSequenceNumber) { - Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { - hasUnread: true, - maxSequenceNumber: newMaxSequenceNumber, - }); - } - + // Record the new sequence number if there is one // Add the action into Ion Ion.merge(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, { + hasUnread: hasNewSequenceNumber, + maxSequenceNumber: hasNewSequenceNumber ? newMaxSequenceNumber : previousMaxSequenceNumber, [reportAction.sequenceNumber]: reportAction, }); From fea2c7bcf87bd45fcce229b71683f7b942a81c26 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 8 Sep 2020 17:19:35 -0600 Subject: [PATCH 24/47] Update code comments --- src/lib/actions/Report.js | 2 +- src/page/home/sidebar/ChatSwitcherView.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index 9b371d74dabc0..4e0aa51d0dfea 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -30,7 +30,7 @@ Ion.connect({ callback: val => currentURL = val, }); -// Use a regex pattern here for an exact match so it doesn't also match "my_personal_details" +// Use a regex pattern here for an exact match so it doesn't also match "myPersonalDetails" let personalDetails; Ion.connect({ key: `^${IONKEYS.PERSONAL_DETAILS}$`, diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index 25c7854a3042a..619350514592b 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -275,7 +275,7 @@ ChatSwitcherView.defaultProps = defaultProps; export default withIon({ personalDetails: { // Exact match for the personal_details key as we don't want - // my_personal_details to overwrite this value + // myPersonalDetails to overwrite this value key: `^${IONKEYS.PERSONAL_DETAILS}$`, }, myPersonalDetails: { From e3981935195173fe5656e15e71ac371bb45af93f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 07:03:55 -0600 Subject: [PATCH 25/47] Fix value initialization when the key doesn't exist --- src/Expensify.js | 4 ++-- src/lib/API.js | 23 ++++++++++++++--------- src/lib/DateUtils.js | 2 +- src/lib/Ion.js | 2 ++ src/lib/actions/PersonalDetails.js | 2 +- src/lib/actions/Report.js | 6 +++++- src/lib/actions/Session.js | 7 ------- src/page/SignInPage.js | 2 +- 8 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index 0bcf9d0ab4906..b041c2d5ddb15 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -62,9 +62,9 @@ class Expensify extends Component { * @param {object} session * @param {string} session.authToken */ - removeLoadingState({authToken}) { + removeLoadingState(session) { this.setState({ - authToken, + authToken: session ? session.authToken : null, isLoading: false, }); } diff --git a/src/lib/API.js b/src/lib/API.js index 9d6a67346dd68..af7e5f2187e66 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -21,20 +21,22 @@ let networkRequestQueue = []; let reauthenticating = false; let authToken; -Ion.connect({key: IONKEYS.SESSION, path: 'authToken', callback: val => authToken = val}); +Ion.connect({ + key: IONKEYS.SESSION, + callback: val => authToken = val ? val.authToken : null, +}); -// We susbcribe to changes to the online/offline status of the network to determine when we should fire off API calls -// vs queueing them for later. When going reconnecting, ie, going from offline to online, we fire off all the API calls -// that we have in the queue +// We subscribe to changes to the online/offline status of the network to determine when we should fire off API calls +// vs queueing them for later. When reconnecting, ie, going from offline to online, all the reconnection callbacks +// are triggered (this is usually Actions that need to re-download data from the server) let isOffline; Ion.connect({ key: IONKEYS.NETWORK, - path: 'isOffline', - callback: (isCurrentlyOffline) => { - if (isOffline && !isCurrentlyOffline) { + callback: (val) => { + if (isOffline && !val.isOffline) { _.each(reconnectionCallbacks, callback => callback()); } - isOffline = isCurrentlyOffline; + isOffline = val && val.isOffline; } }); @@ -42,7 +44,10 @@ Ion.connect({ // When the user's authToken expires we use this login to re-authenticate and get a new authToken // and use that new authToken in subsequent API calls let credentials; -Ion.connect({key: IONKEYS.CREDENTIALS, callback: ionCredentials => credentials = ionCredentials}); +Ion.connect({ + key: IONKEYS.CREDENTIALS, + callback: ionCredentials => credentials = ionCredentials, +}); /** * @param {string} login diff --git a/src/lib/DateUtils.js b/src/lib/DateUtils.js index 6ca538b4e1882..34ca3a3f1fe6b 100644 --- a/src/lib/DateUtils.js +++ b/src/lib/DateUtils.js @@ -10,7 +10,7 @@ import IONKEYS from '../IONKEYS'; let timezone; Ion.connect({ key: IONKEYS.MY_PERSONAL_DETAILS, - callback: val => timezone = val.timezone || 'America/Los_Angeles', + callback: val => timezone = val ? val.timezone : 'America/Los_Angeles', }); /** diff --git a/src/lib/Ion.js b/src/lib/Ion.js index a83be0461399f..3e4e523f0f2de 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -122,7 +122,9 @@ function connect(mapping) { // Find all the keys matched by the config regex const matchingKeys = _.filter(keys, key => config.regex.test(key)); + // If the key being connected to does not exist, initialize the value with null if (matchingKeys.length === 0) { + sendDataToConnection(null, config.key); return; } diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index e37ad37432ea0..00c9d7dfb996a 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -9,7 +9,7 @@ import CONST from '../../CONST'; let currentUserEmail; Ion.connect({ key: IONKEYS.SESSION, - callback: val => currentUserEmail = val.email, + callback: val => currentUserEmail = val ? val.email : null, }); /** diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index 5b24072e49d36..ff09c21af6cef 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -460,7 +460,11 @@ function updateLastReadActionID(reportID, sequenceNumber) { * @param {object} report */ function handleReportChanged(report) { - if (report && report.reportName === undefined) { + if (!report) { + return; + } + + if (report.reportName === undefined) { fetchChatReportsByIDs([report.reportID]); } diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index f8a08bfe1b408..2d0a67426c099 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -3,12 +3,6 @@ import * as API from '../API'; import IONKEYS from '../../IONKEYS'; import redirectToSignIn from './SignInRedirect'; -let session; -Ion.connect({ - key: IONKEYS.SESSION, - callback: val => session = val, -}); - let credentials; Ion.connect({ key: IONKEYS.CREDENTIALS, @@ -41,7 +35,6 @@ function signIn(partnerUserID, partnerUserSecret, twoFactorAuthCode = '', exitTo function signOut() { redirectToSignIn(); API.deleteLogin({ - authToken: session.authToken, partnerUserID: credentials.login }); Ion.clear(); diff --git a/src/page/SignInPage.js b/src/page/SignInPage.js index 2eb08829f64f9..34cbe12c6c4ac 100644 --- a/src/page/SignInPage.js +++ b/src/page/SignInPage.js @@ -119,7 +119,7 @@ class App extends Component { > Log In - {this.props.session.error && ( + {this.props.session && this.props.session.error && ( {this.props.session.error} From 9f51a44bce634e393bb58d9a4be8ad85b64a6af5 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 07:19:51 -0600 Subject: [PATCH 26/47] Updates from Carlos' review --- src/lib/Ion.js | 4 ++-- src/lib/actions/Report.js | 12 +++++++----- src/page/home/report/ReportActionsView.js | 12 +----------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 3e4e523f0f2de..c3b14603fd33d 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -75,7 +75,7 @@ function keyChanged(key, data) { * Subscribes a react component's state directly to a store key * * @param {object} mapping the mapping information to connect Ion to the components state - * @param {string} mapping.keyPattern + * @param {string} mapping.key * @param {string} mapping.statePropertyName the name of the property in the state to connect the data to * @param {string} [mapping.indexBy] the name of a property to index the collection by * @param {object} [mapping.withIonInstance] whose setState() method will be called with any changed data @@ -120,7 +120,7 @@ function connect(mapping) { AsyncStorage.getAllKeys() .then((keys) => { // Find all the keys matched by the config regex - const matchingKeys = _.filter(keys, key => config.regex.test(key)); + const matchingKeys = _.filter(keys, config.regex.test); // If the key being connected to does not exist, initialize the value with null if (matchingKeys.length === 0) { diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index ff09c21af6cef..edd87195cda08 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -250,12 +250,12 @@ function fetchChatReports() { /** * Get all of our reports * - * @param {boolean} redirectToFirstReport this is set to false when the network reconnect - * code runes + * @param {boolean} shouldRedirectToFirstReport this is set to false when the network reconnect + * code runs * * @returns {Promise} */ -function fetchAll(redirectToFirstReport = true) { +function fetchAll(shouldRedirectToFirstReport = true) { let fetchedReports; // Request each report one at a time to allow individual reports to fail if access to it is prevented by Auth @@ -284,8 +284,8 @@ function fetchAll(redirectToFirstReport = true) { // Set the first report ID so that the logged in person can be redirected there // if they are on the home page - if (redirectToFirstReport && currentURL === '/') { - const firstReportID = _.first(_.pluck(fetchedReports, 'reportID')) || 0; + if (shouldRedirectToFirstReport && currentURL === '/') { + const firstReportID = _.first(_.pluck(fetchedReports, 'reportID')); // If we're on the home page, then redirect to the first report ID if (firstReportID) { @@ -464,6 +464,8 @@ function handleReportChanged(report) { return; } + // A report can be missing a name if a comment is received via pusher event + // and the report does not yet exist in Ion (eg. a new DM created with the logged in person) if (report.reportName === undefined) { fetchChatReportsByIDs([report.reportID]); } diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index fac26556750fc..9271e3e64731e 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -35,7 +35,6 @@ class ReportActionsView extends React.Component { constructor(props) { super(props); - this.recordlastReadActionID = _.debounce(this.recordlastReadActionID.bind(this), 1000, true); this.scrollToListBottom = this.scrollToListBottom.bind(this); this.recordMaxAction = this.recordMaxAction.bind(this); } @@ -111,17 +110,8 @@ class ReportActionsView extends React.Component { .pluck('sequenceNumber') .max() .value(); - this.recordlastReadActionID(maxVisibleSequenceNumber); - } - /** - * Takes a max seqNum and if it's greater than the last read action, then make a request to the API to - * update the report - * - * @param {number} maxSequenceNumber - */ - recordlastReadActionID(maxSequenceNumber) { - updateLastReadActionID(this.props.reportID, maxSequenceNumber); + updateLastReadActionID(this.props.reportID, maxVisibleSequenceNumber); } /** From eae6b7b6895b50b4237ff11b060850b47573f714 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 11:08:57 -0600 Subject: [PATCH 27/47] Updates to the readme --- README.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 9f73acdfe056c..378ab1766210b 100644 --- a/README.md +++ b/README.md @@ -7,26 +7,27 @@ This application is built with the following principles. 1. Disk pushes data to the UI (Ion -> withIon()/connect() -> React component). 1. UI pushes data to people's brains (React component -> device screen). 1. Brain pushes data into UI inputs (Device input -> React component). - 1. UI inputs pushes data to the server (React component -> Action -> XHR to server). + 1. UI inputs push data to the server (React component -> Action -> XHR to server). 1. Go to 1 1. **Offline first** - - All data that is brought into the app should be stored immediately on disk in persistent storage (eg. localStorage on browser platforms). + - All data that is brought into the app and is necessary to display the app when offline should be stored on disk in persistent storage (eg. localStorage on browser platforms). [AsyncStorage](https://react-native-community.github.io/async-storage/) is a cross-platform abstraction layer that is used to access persistent storage. - All data that is displayed, comes from persistent storage. 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 bind to Ion (using `withIon()`) and any change to the Ion data is automatically reflected in the component by calling `setState()` with the changed data. - - Libraries bind to Ion (with `Ion.connect()`) and use a callback which is triggered with the changed data. + - 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. - The UI should never call any Ion methods except for `Ion.connect()`. That is the job of Actions (see next section). - - The UI always interacts with an Action when something needs to happen (eg. a person inputs data, the UI passes that data to an Action to be processed). + - 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: - - Incomplete or missing data (always assume data is incomplete or not there) - - Order of events (all operations can and should happen in parallel rather than in sequence) + - Incomplete or missing data (always assume data is incomplete or not there). For example, when a comment is pushed to the client from a pusher event, it's possible that Ion does not have data for that report yet. That's OK. A partial report object is added to Ion for the report key `report_1234 = {reportID: 1234, hasUnread: true}`. Then there is code that monitors Ion for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that doesn't have a report name. + - The order that actions are done in. All actions should be done in parallel instead of sequence. + - Parallel actions are asynchronous methods that don't return promises. Any number of these actions can be called at one time and it doesn't matter when they complete. + - In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`. 1. **Actions manage Ion Data** - When data needs to be written to or read from the server, this is done through Actions only. - - Action methods should never return anything (not data or a promise). There are very few exceptions to this. The only time an action is allowed to return a promise or data is for internal use by that action only. Any libraries, actions, or React components that need access to the data from the action should be subscribing to that data with Ion. + - Action methods should never return anything (not data or a promise) - Actions should favor using `Ion.merge()` over `Ion.set()` so that other values in an object aren't completely overwritten. - - All Ion operations should happen in parallel and never in sequence (eg. don't use the promise of one Ion method to trigger a second Ion method). - - The only time Actions should use promises to do operations in sequence is when working with XHRs. + - 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 (eg. creating a new comment) 1. **Cross Platform 99.9999%** @@ -93,12 +94,12 @@ This is a persistent storage solution wrapped in a Pub/Sub library. In general t - Ion stores and retrieves data from persistent storage - Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object -- Collections of data are usually not stored as a single key (eg. an array with multiple objects), but as individual keys+ID (eg. `report_1234`, `report_4567`, etc.) +- Collections of data are usually not stored as a single key (eg. an array with multiple objects), but as individual keys+ID (eg. `report_1234`, `report_4567`, etc.). Store collections as individual keys when a component will bind directly to one of those keys. For example: reports are stored as individual keys because `SidebarLink.js` binds to the individual report keys for each link. However, report actions are stored as an array of objects because nothing binds directly to a single report action. - Ion allows other code to subscribe to changes in data, and then publishes change events whenever data is changed - Anything needing to read Ion data needs to: 1. Know what key the data is stored in (for web, you can find this by looking in the JS console > Application > local storage) 2. Subscribe to changes of the data for a particular key or set of keys. React components use `withIon()` and non-React libs use `Ion.connect()`. - 3. Get initialized with the current value of that key from persistent storage (Ion does this automatically as part of the connection process) + 3. Get initialized with the current value of that key from persistent storage (Ion does this by calling `setState()` or triggering the `callback` with the values currently on disk as part of the connection process) - Subscribing to Ion keys is done using a regex pattern. For example, since all reports are stored as individual keys like `report_1234`, then if code needs to know about all the reports (eg. display a list of them in the nav menu), then it would subscribe to the key pattern `report_[0-9]+$`. ### Actions From dcae0e871efea2c33c4267314a90ad69a77fa50f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 11:37:48 -0600 Subject: [PATCH 28/47] Remove promises from the code --- README.md | 6 +-- src/lib/Ion.js | 2 +- src/lib/actions/PersonalDetails.js | 13 ++--- src/lib/actions/Report.js | 65 ++++++++++++----------- src/lib/actions/Session.js | 5 +- src/page/home/sidebar/ChatSwitcherView.js | 4 +- 6 files changed, 42 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 378ab1766210b..40e4058e856b6 100644 --- a/README.md +++ b/README.md @@ -19,13 +19,13 @@ This application is built with the following principles. - 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: - - Incomplete or missing data (always assume data is incomplete or not there). For example, when a comment is pushed to the client from a pusher event, it's possible that Ion does not have data for that report yet. That's OK. A partial report object is added to Ion for the report key `report_1234 = {reportID: 1234, hasUnread: true}`. Then there is code that monitors Ion for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that doesn't have a report name. + - Incomplete or missing data. Always assume data is incomplete or not there. For example, when a comment is pushed to the client from a pusher event, it's possible that Ion does not have data for that report yet. That's OK. A partial report object is added to Ion for the report key `report_1234 = {reportID: 1234, hasUnread: true}`. Then there is code that monitors Ion for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that doesn't have a report name. - The order that actions are done in. All actions should be done in parallel instead of sequence. - - Parallel actions are asynchronous methods that don't return promises. Any number of these actions can be called at one time and it doesn't matter when they complete. + - Parallel actions are asynchronous methods that don't return promises. Any number of these actions can be called at one time and it doesn't matter what order they happen in or when they complete. - In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`. 1. **Actions manage Ion Data** - When data needs to be written to or read from the server, this is done through Actions only. - - Action methods should never return anything (not data or a promise) + - 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. - If an Action needs to access data stored on disk, use a local variable and `Ion.connect()` diff --git a/src/lib/Ion.js b/src/lib/Ion.js index c3b14603fd33d..2c684075a732c 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -120,7 +120,7 @@ function connect(mapping) { AsyncStorage.getAllKeys() .then((keys) => { // Find all the keys matched by the config regex - const matchingKeys = _.filter(keys, config.regex.test); + const matchingKeys = _.filter(keys, config.regex.test.bind(config.regex)); // If the key being connected to does not exist, initialize the value with null if (matchingKeys.length === 0) { diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index 00c9d7dfb996a..1ccb202925107 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -62,11 +62,9 @@ function formatPersonalDetails(personalDetailsList) { /** * Get the timezone of the logged in user - * - * @returns {Promise} */ function fetchTimezone() { - const requestPromise = queueRequest('Get', { + queueRequest('Get', { returnValueList: 'nameValuePairs', name: 'timeZone', }) @@ -77,16 +75,13 @@ function fetchTimezone() { // Refresh the timezone every 30 minutes setTimeout(fetchTimezone, 1000 * 60 * 30); - return requestPromise; } /** * Get the personal details for our organization - * - * @returns {Promise} */ function fetch() { - const requestPromise = queueRequest('Get', { + queueRequest('Get', { returnValueList: 'personalDetailsList', }) .then((data) => { @@ -111,17 +106,15 @@ function fetch() { // Refresh the personal details every 30 minutes setTimeout(fetch, 1000 * 60 * 30); - return requestPromise; } /** * Get personal details for a list of emails. * * @param {String} emailList - * @returns {Promise} */ function getForEmails(emailList) { - return queueRequest('PersonalDetails_GetForEmails', {emailList}) + queueRequest('PersonalDetails_GetForEmails', {emailList}) .then(details => Ion.merge(IONKEYS.PERSONAL_DETAILS, formatPersonalDetails(details))); } diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index edd87195cda08..ddf0745d2fa22 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -43,6 +43,7 @@ Ion.connect({ callback: val => myPersonalDetails = val, }); +// Keeps track of the max sequence number for each report const reportMaxSequenceNumbers = {}; // List of reportIDs that we define in .env @@ -119,7 +120,7 @@ function getChatReportName(sharedReportList) { * chat report IDs * * @param {Array} chatList - * @return {Promise} + * @return {Promise} only used internally when fetchAll() is called */ function fetchChatReportsByIDs(chatList) { let fetchedReports; @@ -236,7 +237,7 @@ function subscribeToReportCommentEvents() { * Get all chat reports and provide the proper report name * by fetching sharedReportList and personalDetails * - * @returns {Promise} + * @returns {Promise} only used internally when fetchAll() is called */ function fetchChatReports() { return queueRequest('Get', { @@ -247,15 +248,32 @@ function fetchChatReports() { .then(({chatList}) => fetchChatReportsByIDs(String(chatList).split(','))); } +/** + * Get the actions of a report + * + * @param {number} reportID + */ +function fetchActions(reportID) { + queueRequest('Report_GetHistory', {reportID}) + .then((data) => { + const indexedData = _.indexBy(data.history, 'sequenceNumber'); + const maxSequenceNumber = _.chain(data.history) + .pluck('sequenceNumber') + .max() + .value(); + Ion.set(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, indexedData); + Ion.merge(`${IONKEYS.REPORT}_${reportID}`, {maxSequenceNumber}); + }); +} + /** * Get all of our reports * * @param {boolean} shouldRedirectToFirstReport this is set to false when the network reconnect * code runs - * - * @returns {Promise} + * @param {boolean} shouldFetchActions whether or not the actions of the reports should also be fetched */ -function fetchAll(shouldRedirectToFirstReport = true) { +function fetchAll(shouldRedirectToFirstReport = true, shouldFetchActions = false) { let fetchedReports; // Request each report one at a time to allow individual reports to fail if access to it is prevented by Auth @@ -271,7 +289,7 @@ function fetchAll(shouldRedirectToFirstReport = true) { // parallel reportFetchPromises.push(fetchChatReports()); - return promiseAllSettled(reportFetchPromises) + promiseAllSettled(reportFetchPromises) .then((data) => { fetchedReports = _.compact(_.map(data, (promiseResult) => { // Grab the report from the promise result which stores it in the `value` key @@ -297,28 +315,11 @@ function fetchAll(shouldRedirectToFirstReport = true) { // Merge the data into Ion. Don't use set() here or multiSet() because then that would // overwrite any existing data (like if they have unread messages) Ion.merge(`${IONKEYS.REPORT}_${report.reportID}`, getSimplifiedReportObject(report)); - }); - return fetchedReports; - }); -} - -/** - * Get the actions of a report - * - * @param {number} reportID - * @returns {Promise} - */ -function fetchActions(reportID) { - return queueRequest('Report_GetHistory', {reportID}) - .then((data) => { - const indexedData = _.indexBy(data.history, 'sequenceNumber'); - const maxSequenceNumber = _.chain(data.history) - .pluck('sequenceNumber') - .max() - .value(); - Ion.set(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, indexedData); - Ion.merge(`${IONKEYS.REPORT}_${reportID}`, {maxSequenceNumber}); + if (shouldFetchActions) { + fetchActions(report.reportID); + } + }); }); } @@ -327,12 +328,11 @@ function fetchActions(reportID) { * set of participants * * @param {string[]} participants - * @returns {Promise} resolves with reportID */ function fetchOrCreateChatReport(participants) { let reportID; - return queueRequest('CreateChatReport', { + queueRequest('CreateChatReport', { emailList: participants.join(','), }) @@ -360,8 +360,8 @@ function fetchOrCreateChatReport(participants) { // overwrite any existing data (like if they have unread messages) Ion.merge(`${IONKEYS.REPORT}_${reportID}`, newReport); - // Return the reportID as the final return value - return reportID; + // Redirec the logged in person to the new report + Ion.set(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); }); } @@ -470,6 +470,7 @@ function handleReportChanged(report) { fetchChatReportsByIDs([report.reportID]); } + // Store the max sequence number for each report reportMaxSequenceNumbers[report.reportID] = report.maxSequenceNumber; } Ion.connect({ @@ -479,7 +480,7 @@ Ion.connect({ // When the app reconnects from being offline, fetch all of the reports and their actions onReconnect(() => { - fetchAll(false).then(reports => _.each(reports, report => fetchActions(report.reportID))); + fetchAll(false, true); }); export { fetchAll, diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index 2d0a67426c099..3740e5fd43947 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -16,12 +16,9 @@ Ion.connect({ * @param {string} partnerUserSecret * @param {string} twoFactorAuthCode * @param {string} exitTo - * @param {boolean} useExpensifyLogin - * - * @returns {Promise} */ function signIn(partnerUserID, partnerUserSecret, twoFactorAuthCode = '', exitTo) { - return API.authenticate({ + API.authenticate({ partnerUserID, partnerUserSecret, twoFactorAuthCode, diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index 619350514592b..49ac0071f6fff 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -2,7 +2,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import _ from 'underscore'; import withIon from '../../../components/withIon'; -import Ion from '../../../lib/Ion'; import IONKEYS from '../../../IONKEYS'; import Str from '../../../lib/Str'; import KeyboardShortcut from '../../../lib/KeyboardShortcut'; @@ -130,8 +129,7 @@ class ChatSwitcherView extends React.Component { * @param {string} option.value */ fetchChatReportAndRedirect(option) { - fetchOrCreateChatReport([this.props.myPersonalDetails.login, option.login]) - .then(reportID => Ion.set(IONKEYS.APP_REDIRECT_TO, `/${reportID}`)); + fetchOrCreateChatReport([this.props.myPersonalDetails.login, option.login]); this.reset(); } From e24e1e53a1d251dc95d97e0e81f75ef816cfb6f7 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 12:03:27 -0600 Subject: [PATCH 29/47] Remove most of Ion.set() in favor of merge() --- src/Expensify.js | 2 +- src/lib/API.js | 4 ++-- src/lib/ActiveClientManager.js | 3 ++- src/lib/Ion.js | 21 +++++++++++++++------ src/lib/actions/PersonalDetails.js | 8 +++++++- src/lib/actions/Report.js | 20 +++++++++++--------- src/lib/actions/SignInRedirect.js | 2 +- 7 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index b041c2d5ddb15..83680107b8f05 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -75,7 +75,7 @@ class Expensify extends Component { * @param {object} params.match */ recordCurrentRoute({match}) { - Ion.set(IONKEYS.CURRENT_URL, match.url); + Ion.merge(IONKEYS.CURRENT_URL, match.url); } render() { diff --git a/src/lib/API.js b/src/lib/API.js index af7e5f2187e66..f3efece5181fa 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -65,7 +65,7 @@ function createLogin(login, password) { partnerUserID: login, partnerUserSecret: password, }) - .then(() => Ion.set(IONKEYS.CREDENTIALS, {login, password})) + .then(() => Ion.merge(IONKEYS.CREDENTIALS, {login, password})) .catch(err => Ion.merge(IONKEYS.SESSION, {error: err})); } @@ -362,7 +362,7 @@ function authenticate(parameters) { .catch((err) => { console.error(err); console.debug('[SIGNIN] Request error'); - return Ion.merge(IONKEYS.SESSION, {error: err.message}); + Ion.merge(IONKEYS.SESSION, {error: err.message}); }); } diff --git a/src/lib/ActiveClientManager.js b/src/lib/ActiveClientManager.js index 80ac16541f07a..5a7b403072afd 100644 --- a/src/lib/ActiveClientManager.js +++ b/src/lib/ActiveClientManager.js @@ -19,7 +19,8 @@ Ion.connect({ * * @returns {Promise} */ -// @TODO need to change this to Ion.merge() once we support multiple tabs +// @TODO need to change this to Ion.merge() once we support multiple tabs since there is now way to remove +// clientIDs from this yet const init = () => Ion.set(IONKEYS.ACTIVE_CLIENTS, {[clientID]: clientID}); /** diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 2c684075a732c..d0b8e7b5199ec 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -207,14 +207,23 @@ function clear() { * Merge a new value into an existing value at a key * * @param {string} key - * @param {string} val - * @returns {Promise} + * @param {*} val */ function merge(key, val) { - return AsyncStorage.mergeItem(key, JSON.stringify(val)) - .then(() => get(key)) - .then((newObject) => { - keyChanged(key, newObject); + // Values that are objects can be merged into storage + if (_.isObject(val)) { + AsyncStorage.mergeItem(key, JSON.stringify(val)) + .then(() => get(key)) + .then((newObject) => { + keyChanged(key, newObject); + }); + return; + } + + // Anything else (strings and numbers) need to be set into storage + AsyncStorage.setItem(key, JSON.stringify(val)) + .then(() => { + keyChanged(key, val); }); } diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index 1ccb202925107..cddd38fdc8473 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -86,6 +86,7 @@ function fetch() { }) .then((data) => { const allPersonalDetails = formatPersonalDetails(data.personalDetailsList); + console.log(2, allPersonalDetails); Ion.merge(IONKEYS.PERSONAL_DETAILS, allPersonalDetails); // Get my personal details so they can be easily accessed and subscribed to on their own key @@ -115,7 +116,12 @@ function fetch() { */ function getForEmails(emailList) { queueRequest('PersonalDetails_GetForEmails', {emailList}) - .then(details => Ion.merge(IONKEYS.PERSONAL_DETAILS, formatPersonalDetails(details))); + .then((data) => { + const details = _.omit(data, ['jsonCode', 'requestID']); + const formattedDetails = formatPersonalDetails(details); + console.log(1, formattedDetails); + Ion.merge(IONKEYS.PERSONAL_DETAILS, formattedDetails); + }); } // When the app reconnects from being offline, fetch all of the personal details diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index ddf0745d2fa22..70e670f7905bd 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -158,9 +158,8 @@ function fetchChatReportsByIDs(chatList) { newReport.reportName = getChatReportName(report.sharedReportList); } - // Merge the data into Ion. Don't use set() here or multiSet() because then that would - // overwrite any existing data (like if they have unread messages) - return Ion.merge(`${IONKEYS.REPORT}_${report.reportID}`, newReport); + // Merge the data into Ion + Ion.merge(`${IONKEYS.REPORT}_${report.reportID}`, newReport); }); return Promise.all(ionPromises); @@ -188,10 +187,13 @@ function updateReportWithNewAction(reportID, reportAction) { // Mark the report as unread if there is a new max sequence number // Record the new sequence number if there is one - // Add the action into Ion - Ion.merge(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, { + Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { hasUnread: hasNewSequenceNumber, maxSequenceNumber: hasNewSequenceNumber ? newMaxSequenceNumber : previousMaxSequenceNumber, + }); + + // Add the action into Ion + Ion.merge(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, { [reportAction.sequenceNumber]: reportAction, }); @@ -214,7 +216,7 @@ function updateReportWithNewAction(reportID, reportAction) { reportAction, onClick: () => { // Navigate to this report onClick - Ion.set(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); + Ion.merge(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); } }); } @@ -261,7 +263,7 @@ function fetchActions(reportID) { .pluck('sequenceNumber') .max() .value(); - Ion.set(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, indexedData); + Ion.merge(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, indexedData); Ion.merge(`${IONKEYS.REPORT}_${reportID}`, {maxSequenceNumber}); }); } @@ -307,7 +309,7 @@ function fetchAll(shouldRedirectToFirstReport = true, shouldFetchActions = false // If we're on the home page, then redirect to the first report ID if (firstReportID) { - Ion.set(IONKEYS.APP_REDIRECT_TO, `/${firstReportID}`); + Ion.merge(IONKEYS.APP_REDIRECT_TO, `/${firstReportID}`); } } @@ -361,7 +363,7 @@ function fetchOrCreateChatReport(participants) { Ion.merge(`${IONKEYS.REPORT}_${reportID}`, newReport); // Redirec the logged in person to the new report - Ion.set(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); + Ion.merge(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); }); } diff --git a/src/lib/actions/SignInRedirect.js b/src/lib/actions/SignInRedirect.js index e4bd853eca3a9..cc891101066c9 100644 --- a/src/lib/actions/SignInRedirect.js +++ b/src/lib/actions/SignInRedirect.js @@ -26,7 +26,7 @@ function redirectToSignIn() { const urlWithExitTo = currentURL === '/' ? ROUTES.SIGNIN : `${ROUTES.SIGNIN}/exitTo${currentURL}`; - Ion.set(IONKEYS.APP_REDIRECT_TO, urlWithExitTo); + Ion.merge(IONKEYS.APP_REDIRECT_TO, urlWithExitTo); } export default redirectToSignIn; From baf26584db8e4eaea58eb930f12c655ea4e6e71f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 12:13:06 -0600 Subject: [PATCH 30/47] Remove console logs --- src/lib/actions/PersonalDetails.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index cddd38fdc8473..ccce0f3f36889 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -86,7 +86,6 @@ function fetch() { }) .then((data) => { const allPersonalDetails = formatPersonalDetails(data.personalDetailsList); - console.log(2, allPersonalDetails); Ion.merge(IONKEYS.PERSONAL_DETAILS, allPersonalDetails); // Get my personal details so they can be easily accessed and subscribed to on their own key @@ -119,7 +118,6 @@ function getForEmails(emailList) { .then((data) => { const details = _.omit(data, ['jsonCode', 'requestID']); const formattedDetails = formatPersonalDetails(details); - console.log(1, formattedDetails); Ion.merge(IONKEYS.PERSONAL_DETAILS, formattedDetails); }); } From a4acbd830c4c48b1b584c2dc3b96188ddb58cf02 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 14:11:05 -0600 Subject: [PATCH 31/47] Switch to merge --- src/lib/actions/App.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/actions/App.js b/src/lib/actions/App.js index f9d591d4dc747..1189731221b90 100644 --- a/src/lib/actions/App.js +++ b/src/lib/actions/App.js @@ -14,9 +14,9 @@ Ion.connect({ * @param {object} match */ function recordCurrentRoute({match}) { - Ion.set(IONKEYS.CURRENT_URL, match.url); + Ion.merge(IONKEYS.CURRENT_URL, match.url); if (match.url === currentRedirectTo) { - Ion.set(IONKEYS.APP_REDIRECT_TO, ''); + Ion.merge(IONKEYS.APP_REDIRECT_TO, ''); } } From 37bc9c5a9713adeefa250156e4113a09dcdf2c66 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 14:15:08 -0600 Subject: [PATCH 32/47] Switch to redirect() in all places --- src/lib/API.js | 10 +++------- src/lib/actions/App.js | 24 ++++++++++++------------ src/lib/actions/Report.js | 6 +++--- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index f3efece5181fa..fb6149807f4e1 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -8,6 +8,7 @@ import ROUTES from '../ROUTES'; import Str from './Str'; import Guid from './Guid'; import redirectToSignIn from './actions/SignInRedirect'; +import {redirect} from './actions/App'; // Holds all of the callbacks that need to be triggered when the network reconnects const reconnectionCallbacks = []; @@ -96,7 +97,6 @@ function queueRequest(command, data) { * * @param {object} data * @param {string} exitTo - * @returns {Promise} */ function setSuccessfulSignInData(data, exitTo) { let redirectTo; @@ -108,12 +108,8 @@ function setSuccessfulSignInData(data, exitTo) { } else { redirectTo = ROUTES.HOME; } - return Ion.multiSet({ - // The response from Authenticate includes requestID, jsonCode, etc - // but we only care about setting these three values in Ion - [IONKEYS.SESSION]: _.pick(data, 'authToken', 'accountID', 'email'), - [IONKEYS.APP_REDIRECT_TO]: redirectTo, - }); + redirect(redirectTo); + Ion.merge(IONKEYS.SESSION, _.pick(data, 'authToken', 'accountID', 'email')); } /** diff --git a/src/lib/actions/App.js b/src/lib/actions/App.js index 1189731221b90..fc12c0a79ab25 100644 --- a/src/lib/actions/App.js +++ b/src/lib/actions/App.js @@ -7,6 +7,17 @@ Ion.connect({ callback: val => currentRedirectTo = val, }); + +/** + * Redirect the app to a new page by updating the state in Ion + * + * @param {mixed} url + */ +function redirect(url) { + const formattedURL = (typeof url === 'string' && url.startsWith('/')) ? url : `/${url}`; + Ion.set(IONKEYS.APP_REDIRECT_TO, formattedURL); +} + /** * Keep the current route match stored in Ion so other libs can access it * Also reset the app_redirect_to in Ion so that if we go back to the current url the state will update @@ -16,21 +27,10 @@ Ion.connect({ function recordCurrentRoute({match}) { Ion.merge(IONKEYS.CURRENT_URL, match.url); if (match.url === currentRedirectTo) { - Ion.merge(IONKEYS.APP_REDIRECT_TO, ''); + redirect(''); } } - -/** - * Redirect the app to a new page by updating the state in Ion - * - * @param {mixed} url - */ -function redirect(url) { - const formattedURL = (typeof url === 'string' && url.startsWith('/')) ? url : `/${url}`; - Ion.set(IONKEYS.APP_REDIRECT_TO, formattedURL); -} - export { recordCurrentRoute, redirect, diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index 8ebfc3ed764a6..080ba5926c096 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -311,7 +311,7 @@ function fetchAll(shouldRedirectToFirstReport = true, shouldFetchActions = false // If we're on the home page, then redirect to the first report ID if (firstReportID) { - Ion.merge(IONKEYS.APP_REDIRECT_TO, `/${firstReportID}`); + redirect(`/${firstReportID}`); } } @@ -364,8 +364,8 @@ function fetchOrCreateChatReport(participants) { // overwrite any existing data (like if they have unread messages) Ion.merge(`${IONKEYS.REPORT}_${reportID}`, newReport); - // Redirec the logged in person to the new report - Ion.merge(IONKEYS.APP_REDIRECT_TO, `/${reportID}`); + // Redirect the logged in person to the new report + redirect(`/${reportID}`); }); } From a31a96614241b908de763d3ea7f14c1378dae79d Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 9 Sep 2020 14:17:50 -0600 Subject: [PATCH 33/47] Fix lint errors --- src/page/home/sidebar/ChatSwitcherView.js | 1 - src/page/home/sidebar/SidebarLinks.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index 568744ff9d823..49ac0071f6fff 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -8,7 +8,6 @@ import KeyboardShortcut from '../../../lib/KeyboardShortcut'; import ChatSwitcherList from './ChatSwitcherList'; import ChatSwitcherSearchForm from './ChatSwitcherSearchForm'; import {fetchOrCreateChatReport} from '../../../lib/actions/Report'; -import {redirect} from '../../../lib/actions/App'; const personalDetailsPropTypes = PropTypes.shape({ // The login of the person (either email or phone number) diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index c617489eb152a..b3f1151e4d585 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -13,7 +13,6 @@ import ChatSwitcherView from './ChatSwitcherView'; import SafeAreaInsetPropTypes from '../../SafeAreaInsetPropTypes'; import compose from '../../../lib/compose'; import {withRouter} from '../../../lib/Router'; -import {redirect} from '../../../lib/actions/App'; const propTypes = { // These are from withRouter From eab14a08bdc9eb82d9fc3f73993d7bb9f9ee9d14 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 08:33:54 -0600 Subject: [PATCH 34/47] Fixes from PR review --- README.md | 2 +- src/lib/Ion.js | 46 ++++++++++++----------- src/lib/actions/PersonalDetails.js | 10 +---- src/lib/actions/Report.js | 6 ++- src/page/SignInPage.js | 2 +- src/page/home/HeaderView.js | 2 +- src/page/home/sidebar/ChatSwitcherView.js | 13 ++++--- src/page/home/sidebar/SidebarBottom.js | 2 +- 8 files changed, 43 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 40e4058e856b6..f17b364cee205 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ This application is built with the following principles. 1. A feature isn't done until it works on all platforms. Accordingly, don't even bother writing a platform-specific code block because you're just going to need to undo it. 1. If the reason you can't write cross platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths. 1. If there is a feature that simply doesn't exist on all platforms and thus doesn't exist in RN, rather than doing if (platform=iOS) { }, instead write a "shim" library that is implemented with NOOPs on the other platforms. For example, rather than injecting platform-specific multi-tab code (which can only work on browsers, because it's the only platform with multiple tabs), write a TabManager class that just is NOOP for non-browser platforms. This encapsulates the platform-specific code into a platform library, rather than sprinkling through the business logic. - 1. Put all platform specific code in a dedicated files and folders, like /platform, and reject any PR that attempts to put platform-specific code anywhere else. This maintains a strict separation between business logic and platform code. + 1. Put all platform specific code in dedicated files and folders, like /platform, and reject any PR that attempts to put platform-specific code anywhere else. This maintains a strict separation between business logic and platform code. # Local development ## Getting started diff --git a/src/lib/Ion.js b/src/lib/Ion.js index d0b8e7b5199ec..921a8e65d9128 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -71,6 +71,28 @@ function keyChanged(key, data) { }); } +/** + * Sends the data obtained from the keys to the connection. It either: + * - sets state on the withIonInstances + * - triggers the callback function + * + * @param {object} config + * @param {object} [config.withIonInstance] + * @param {string} [config.statePropertyName] + * @param {function} [config.callback] + * @param {*} val + * @param {string} [key] + */ +function sendDataToConnection(config, val, key) { + if (config.withIonInstance) { + config.withIonInstance.setState({ + [config.statePropertyName]: val, + }); + } else if (_.isFunction(config.callback)) { + config.callback(val, key); + } +} + /** * Subscribes a react component's state directly to a store key * @@ -98,24 +120,6 @@ function connect(mapping) { return connectionID; } - /** - * Sends the data obtained from the keys to the connection. It either: - * - sets state on the withIonInstances - * - triggers the callback function - * - * @param {*} val - * @param {string} [key] - */ - function sendDataToConnection(val, key) { - if (config.withIonInstance) { - config.withIonInstance.setState({ - [config.statePropertyName]: val, - }); - } else if (_.isFunction(config.callback)) { - config.callback(val, key); - } - } - // Get all the data from Ion to initialize the connection with AsyncStorage.getAllKeys() .then((keys) => { @@ -124,7 +128,7 @@ function connect(mapping) { // If the key being connected to does not exist, initialize the value with null if (matchingKeys.length === 0) { - sendDataToConnection(null, config.key); + sendDataToConnection(config, null, config.key); return; } @@ -134,10 +138,10 @@ function connect(mapping) { ...finalObject, [value[config.indexBy]]: value, }), {})) - .then(sendDataToConnection); + .then(val => sendDataToConnection(config, val)); } else { _.each(matchingKeys, (key) => { - get(key).then(val => sendDataToConnection(val, key)); + get(key).then(val => sendDataToConnection(config, val, key)); }); } }); diff --git a/src/lib/actions/PersonalDetails.js b/src/lib/actions/PersonalDetails.js index ccce0f3f36889..47dbdfeabc163 100644 --- a/src/lib/actions/PersonalDetails.js +++ b/src/lib/actions/PersonalDetails.js @@ -94,15 +94,7 @@ function fetch() { // Get the timezone and put it in Ion fetchTimezone(); }) - .catch((error) => { - if (error.message === 'No login') { - // eslint-disable-next-line no-console - console.info('No email in store, not fetching personal details.'); - return; - } - - console.error('Error fetching personal details', error); - }); + .catch(error => console.error('Error fetching personal details', error)); // Refresh the personal details every 30 minutes setTimeout(fetch, 1000 * 60 * 30); diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index 080ba5926c096..f996e503b1ff3 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -336,6 +336,10 @@ function fetchAll(shouldRedirectToFirstReport = true, shouldFetchActions = false function fetchOrCreateChatReport(participants) { let reportID; + if (participants.length < 2) { + throw new Error('fetchOrCreateChatReport() must have at least two participants'); + } + queueRequest('CreateChatReport', { emailList: participants.join(','), }) @@ -451,7 +455,7 @@ function updateLastReadActionID(reportID, sequenceNumber) { // Mark the report as not having any unread items queueRequest('Report_SetLastReadActionID', { - currentUserAccountID, + accountID: currentUserAccountID, reportID, sequenceNumber, }); diff --git a/src/page/SignInPage.js b/src/page/SignInPage.js index 34cbe12c6c4ac..2eb08829f64f9 100644 --- a/src/page/SignInPage.js +++ b/src/page/SignInPage.js @@ -119,7 +119,7 @@ class App extends Component { > Log In - {this.props.session && this.props.session.error && ( + {this.props.session.error && ( {this.props.session.error} diff --git a/src/page/home/HeaderView.js b/src/page/home/HeaderView.js index 29b0f10206004..40ef957d07a07 100644 --- a/src/page/home/HeaderView.js +++ b/src/page/home/HeaderView.js @@ -45,7 +45,7 @@ const HeaderView = props => ( /> )} - {props.report && props.report.reportName ? ( + {props.report.reportName ? ( {props.report.reportName} diff --git a/src/page/home/sidebar/ChatSwitcherView.js b/src/page/home/sidebar/ChatSwitcherView.js index 49ac0071f6fff..898811a336411 100644 --- a/src/page/home/sidebar/ChatSwitcherView.js +++ b/src/page/home/sidebar/ChatSwitcherView.js @@ -49,11 +49,14 @@ const propTypes = { personalDetails: PropTypes.objectOf(personalDetailsPropTypes), // The personal details of the person who is currently logged in - myPersonalDetails: personalDetailsPropTypes, + session: PropTypes.shape({ + // The email of the person who is currently logged in + email: PropTypes.string.isRequired, + }), }; const defaultProps = { personalDetails: {}, - myPersonalDetails: null, + session: null, }; class ChatSwitcherView extends React.Component { @@ -129,7 +132,7 @@ class ChatSwitcherView extends React.Component { * @param {string} option.value */ fetchChatReportAndRedirect(option) { - fetchOrCreateChatReport([this.props.myPersonalDetails.login, option.login]); + fetchOrCreateChatReport([this.props.session.email, option.login]); this.reset(); } @@ -276,7 +279,7 @@ export default withIon({ // myPersonalDetails to overwrite this value key: `^${IONKEYS.PERSONAL_DETAILS}$`, }, - myPersonalDetails: { - key: IONKEYS.MY_PERSONAL_DETAILS, + session: { + key: IONKEYS.SESSION, }, })(ChatSwitcherView); diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index 10bb400c503f7..26c7adb638c2e 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -47,7 +47,7 @@ const SidebarBottom = ({myPersonalDetails, network, insets}) => { // On the very first sign in or after clearing storage these // details will not be present on the first render so we'll just // return nothing for now. - if (!myPersonalDetails) { + if (!myPersonalDetails || _.isEmpty(myPersonalDetails)) { return null; } From 20388173f506790ef04a581e2cc83a090493841e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 08:59:34 -0600 Subject: [PATCH 35/47] Fix browser notifications and lint error --- src/lib/Notification/BrowserNotifications.js | 31 +++++++++----------- src/page/home/sidebar/SidebarBottom.js | 1 + 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/lib/Notification/BrowserNotifications.js b/src/lib/Notification/BrowserNotifications.js index 3763ba56de150..6e98ddb6aee1c 100644 --- a/src/lib/Notification/BrowserNotifications.js +++ b/src/lib/Notification/BrowserNotifications.js @@ -105,25 +105,22 @@ export default { * @param {Function} params.onClick */ pushReportCommentNotification({reportAction, onClick}) { - ActiveClientManager.isClientTheLeader() - .then((isClientTheLeader) => { - if (!isClientTheLeader) { - return; - } - - const {person, message} = reportAction; + if (!ActiveClientManager.isClientTheLeader()) { + console.debug('[BrowserNotifications] Skipping notification because this client is not the leader'); + return; + } - const plainTextPerson = Str.htmlDecode(person.map(f => f.text).join()); + const {person, message} = reportAction; + const plainTextPerson = Str.htmlDecode(person.map(f => f.text).join()); - // Specifically target the comment part of the message - const plainTextMessage = Str.htmlDecode((message.find(f => f.type === 'COMMENT') || {}).text); + // Specifically target the comment part of the message + const plainTextMessage = Str.htmlDecode((message.find(f => f.type === 'COMMENT') || {}).text); - push({ - title: `New message from ${plainTextPerson}`, - body: plainTextMessage, - delay: 0, - onClick, - }); - }); + push({ + title: `New message from ${plainTextPerson}`, + body: plainTextMessage, + delay: 0, + onClick, + }); }, }; diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index 26c7adb638c2e..2621652b33ccd 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -1,6 +1,7 @@ import React from 'react'; import {Image, View} from 'react-native'; import PropTypes from 'prop-types'; +import _ from 'underscore'; import styles, {getSafeAreaMargins} from '../../../style/StyleSheet'; import Text from '../../../components/Text'; import AppLinks from './AppLinks'; From 4f9dadf4eb11077d036dfaeaddf57a1c0b892168 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 10:30:39 -0600 Subject: [PATCH 36/47] Work with null values in the props better --- src/page/SignInPage.js | 6 ++---- src/page/home/HeaderView.js | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/page/SignInPage.js b/src/page/SignInPage.js index 2eb08829f64f9..c989da8948df5 100644 --- a/src/page/SignInPage.js +++ b/src/page/SignInPage.js @@ -33,9 +33,7 @@ const propTypes = { }; const defaultProps = { - session: { - error: null, - }, + session: null, }; class App extends Component { @@ -119,7 +117,7 @@ class App extends Component { > Log In - {this.props.session.error && ( + {this.props.session && this.props.session.error && ( {this.props.session.error} diff --git a/src/page/home/HeaderView.js b/src/page/home/HeaderView.js index 40ef957d07a07..d6196c49c31c6 100644 --- a/src/page/home/HeaderView.js +++ b/src/page/home/HeaderView.js @@ -25,9 +25,7 @@ const propTypes = { }; const defaultProps = { - report: { - reportName: '', - }, + report: null, }; const HeaderView = props => ( @@ -45,7 +43,7 @@ const HeaderView = props => ( /> )} - {props.report.reportName ? ( + {props.report && props.report.reportName ? ( {props.report.reportName} From 9e2921abb3b97578c531caa59749df215342035b Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 12:15:25 -0600 Subject: [PATCH 37/47] Fix reports --- src/page/home/sidebar/SidebarLinks.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index b3f1151e4d585..497e73089d4d9 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -113,6 +113,7 @@ export default compose( withIon({ reports: { key: `${IONKEYS.REPORT}_[0-9]+$`, + indexBy: 'reportID', } }), )(SidebarLinks); From 3a0770bf43781d57c657720e3ceb0c99feaa2510 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 12:23:03 -0600 Subject: [PATCH 38/47] Fix null network --- src/page/home/sidebar/SidebarBottom.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index 2621652b33ccd..07a6dc4905e34 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -34,15 +34,13 @@ const propTypes = { const defaultProps = { myPersonalDetails: {}, - network: { - isOffline: false, - } + network: null, }; const SidebarBottom = ({myPersonalDetails, network, insets}) => { const indicatorStyles = [ styles.statusIndicator, - network.isOffline ? styles.statusIndicatorOffline : styles.statusIndicatorOnline + network && network.isOffline ? styles.statusIndicatorOffline : styles.statusIndicatorOnline ]; // On the very first sign in or after clearing storage these From cd77969f369e9dd0d6af65e7a4c52aa173380e3c Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 12:48:29 -0600 Subject: [PATCH 39/47] Fix an infinite redirect loop --- src/lib/API.js | 7 ++++++- src/lib/actions/App.js | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index daac283fb7f7c..64da1751cad54 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -56,6 +56,10 @@ Ion.connect({ * @returns {Promise} */ function createLogin(login, password) { + if (!authToken) { + throw new Error('createLogin() can\'t be called when there is no authToken'); + } + // Using xhr instead of request becasue request has logic to re-try API commands when we get a 407 authToken expired // in the response, and we call CreateLogin after getting a successful resposne to Authenticate so it's unlikely // that we'll get a 407. @@ -149,7 +153,8 @@ function request(command, parameters, type = 'post') { // We need to return the promise from setSuccessfulSignInData to ensure the authToken is updated before // we try to create a login below - return setSuccessfulSignInData(response, parameters.exitTo); + setSuccessfulSignInData(response, parameters.exitTo); + authToken = response.authToken; }) .then(() => { // If Expensify login, it's the users first time signing in and we need to diff --git a/src/lib/actions/App.js b/src/lib/actions/App.js index fc12c0a79ab25..b0ec4b95c2205 100644 --- a/src/lib/actions/App.js +++ b/src/lib/actions/App.js @@ -27,7 +27,7 @@ function redirect(url) { function recordCurrentRoute({match}) { Ion.merge(IONKEYS.CURRENT_URL, match.url); if (match.url === currentRedirectTo) { - redirect(''); + Ion.set(IONKEYS.APP_REDIRECT_TO, null); } } From b477dfda0dc60596661b542c5e09ca026b9bb46b Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 12:53:48 -0600 Subject: [PATCH 40/47] Try to correct styles in the footer --- src/page/home/sidebar/SidebarBottom.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/page/home/sidebar/SidebarBottom.js b/src/page/home/sidebar/SidebarBottom.js index 07a6dc4905e34..e53f8beb790ba 100644 --- a/src/page/home/sidebar/SidebarBottom.js +++ b/src/page/home/sidebar/SidebarBottom.js @@ -1,5 +1,5 @@ import React from 'react'; -import {Image, View} from 'react-native'; +import {Image, View, StyleSheet} from 'react-native'; import PropTypes from 'prop-types'; import _ from 'underscore'; import styles, {getSafeAreaMargins} from '../../../style/StyleSheet'; @@ -57,7 +57,7 @@ const SidebarBottom = ({myPersonalDetails, network, insets}) => { source={{uri: myPersonalDetails.avatarURL}} style={[styles.actionAvatar]} /> - + {myPersonalDetails.displayName && ( From affbdf298b67ed2343dbc36f02f8c9d3728711c5 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 13:02:00 -0600 Subject: [PATCH 41/47] Address PR comments --- README.md | 10 ++++++++-- src/lib/Ion.js | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f17b364cee205..70182f3ebbbdf 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ # Philosophy This application is built with the following principles. 1. **Data Flow** - Ideally, this is how data flows through the app: - 1. Server pushes data to the disk of any client (Server -> Pusher event -> Action listening to pusher event -> Ion). + 1. Server pushes data to the disk of any client (Server -> Pusher event -> Action listening to pusher event -> Ion). Currently the code only does this with report comments. Until we make more server changes, this steps is actually done by the client requesting data from the server via XHR and then storing the response in Ion. 1. Disk pushes data to the UI (Ion -> withIon()/connect() -> React component). 1. UI pushes data to people's brains (React component -> device screen). 1. Brain pushes data into UI inputs (Device input -> React component). @@ -29,7 +29,13 @@ This application is built with the following principles. - 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. - 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 (eg. creating a new comment) + - 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 + 2. comment is shown in the UI (by mocking the expected response from the server) + 3. comment is created in the server + 4. server responds + 5. UI updates with data from the server + 1. **Cross Platform 99.9999%** 1. A feature isn't done until it works on all platforms. Accordingly, don't even bother writing a platform-specific code block because you're just going to need to undo it. 1. If the reason you can't write cross platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths. diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 921a8e65d9128..898c6047e42c1 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -132,6 +132,12 @@ function connect(mapping) { return; } + // Currently, if a callback or react component is subscribing to a regex key + // and multiple keys match that regex, + // a data change will be published to the callback or react component for EACH + // matching key. In the future, this should be refactored so that identical + // React components or callbacks should only have a single data change published + // to them. if (config.indexBy) { Promise.all(_.map(matchingKeys, key => get(key))) .then(values => _.reduce(values, (finalObject, value) => ({ From ed43ca1d1e6544703abc74574bad73f089acb312 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 13:40:55 -0600 Subject: [PATCH 42/47] Fix unread indicators --- README.md | 2 +- src/lib/actions/Report.js | 24 ++++++++---------------- src/lib/actions/Session.js | 2 +- src/page/home/sidebar/SidebarLinks.js | 8 ++++---- 4 files changed, 14 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 70182f3ebbbdf..6462d5ae68d66 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ This application is built with the following principles. - 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: - - Incomplete or missing data. Always assume data is incomplete or not there. For example, when a comment is pushed to the client from a pusher event, it's possible that Ion does not have data for that report yet. That's OK. A partial report object is added to Ion for the report key `report_1234 = {reportID: 1234, hasUnread: true}`. Then there is code that monitors Ion for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that doesn't have a report name. + - Incomplete or missing data. Always assume data is incomplete or not there. For example, when a comment is pushed to the client from a pusher event, it's possible that Ion does not have data for that report yet. That's OK. A partial report object is added to Ion for the report key `report_1234 = {reportID: 1234, isUnread: true}`. Then there is code that monitors Ion for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that doesn't have a report name. - The order that actions are done in. All actions should be done in parallel instead of sequence. - Parallel actions are asynchronous methods that don't return promises. Any number of these actions can be called at one time and it doesn't matter what order they happen in or when they complete. - In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`. diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index f996e503b1ff3..fb3cd0b14eac0 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -96,7 +96,7 @@ function getSimplifiedReportObject(report) { reportID: report.reportID, reportName: report.reportName, reportNameValuePairs: report.reportNameValuePairs, - hasUnread: hasUnreadActions(report), + isUnread: hasUnreadActions(report), pinnedReport: configReportIDs.includes(report.reportID), }; } @@ -174,25 +174,19 @@ function fetchChatReportsByIDs(chatList) { * @param {object} reportAction */ function updateReportWithNewAction(reportID, reportAction) { + const previousMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; + const newMaxSequenceNumber = reportAction.sequenceNumber; + const hasNewSequenceNumber = newMaxSequenceNumber > previousMaxSequenceNumber; + // Always merge the reportID into Ion // If the report doesn't exist in Ion yet, then all the rest of the data will be filled out // by handleReportChanged Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { reportID, + isUnread: hasNewSequenceNumber, maxSequenceNumber: reportAction.sequenceNumber, }); - const previousMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; - const newMaxSequenceNumber = reportAction.sequenceNumber; - const hasNewSequenceNumber = newMaxSequenceNumber > previousMaxSequenceNumber; - - // Mark the report as unread if there is a new max sequence number - // Record the new sequence number if there is one - Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { - hasUnread: hasNewSequenceNumber, - maxSequenceNumber: hasNewSequenceNumber ? newMaxSequenceNumber : previousMaxSequenceNumber, - }); - // Add the action into Ion Ion.merge(`${IONKEYS.REPORT_ACTIONS}_${reportID}`, { [reportAction.sequenceNumber]: reportAction, @@ -212,7 +206,6 @@ function updateReportWithNewAction(reportID, reportAction) { return; } - console.debug('[NOTIFICATION] Creating notification'); Notification.showCommentNotification({ reportAction, @@ -440,14 +433,13 @@ function addAction(reportID, text) { */ function updateLastReadActionID(reportID, sequenceNumber) { const currentMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; - - if (sequenceNumber <= currentMaxSequenceNumber) { + if (sequenceNumber < currentMaxSequenceNumber) { return; } // Update the lastReadActionID on the report optimistically Ion.merge(`${IONKEYS.REPORT}_${reportID}`, { - hasUnread: false, + isUnread: false, reportNameValuePairs: { [`lastReadActionID_${currentUserAccountID}`]: sequenceNumber, } diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index 3740e5fd43947..71d5133680f9c 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -32,7 +32,7 @@ function signIn(partnerUserID, partnerUserSecret, twoFactorAuthCode = '', exitTo function signOut() { redirectToSignIn(); API.deleteLogin({ - partnerUserID: credentials.login + partnerUserID: credentials && credentials.login }); Ion.clear(); } diff --git a/src/page/home/sidebar/SidebarLinks.js b/src/page/home/sidebar/SidebarLinks.js index 497e73089d4d9..71de2c9f0c2fe 100644 --- a/src/page/home/sidebar/SidebarLinks.js +++ b/src/page/home/sidebar/SidebarLinks.js @@ -37,7 +37,7 @@ const propTypes = { reports: PropTypes.objectOf(PropTypes.shape({ reportID: PropTypes.number, reportName: PropTypes.string, - hasUnread: PropTypes.bool, + isUnread: PropTypes.bool, })), }; const defaultProps = { @@ -60,10 +60,10 @@ class SidebarLinks extends React.Component { // Filter the reports so that the only reports shown are pinned, unread, and the one matching the URL // eslint-disable-next-line max-len - const reportsToDisplay = _.filter(sortedReports, report => (report.pinnedReport || report.hasUnread || report.reportID === reportIDInUrl)); + const reportsToDisplay = _.filter(sortedReports, report => (report.pinnedReport || report.isUnread || report.reportID === reportIDInUrl)); // Updates the page title to indicate there are unread reports - PageTitleUpdater(_.any(reports, report => report.hasUnread)); + PageTitleUpdater(_.any(reports, report => report.isUnread)); return ( @@ -94,7 +94,7 @@ class SidebarLinks extends React.Component { key={report.reportID} reportID={report.reportID} reportName={report.reportName} - hasUnread={report.hasUnread} + isUnread={report.isUnread} onLinkClick={onLinkClick} /> ))} From 887df4056480cc6c81382c8908f9aeb3f12a9c34 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 13:46:47 -0600 Subject: [PATCH 43/47] Fix a few more Ion.set() --- src/lib/actions/App.js | 4 ++-- src/lib/actions/Report.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/actions/App.js b/src/lib/actions/App.js index b0ec4b95c2205..b17f7575b13db 100644 --- a/src/lib/actions/App.js +++ b/src/lib/actions/App.js @@ -15,7 +15,7 @@ Ion.connect({ */ function redirect(url) { const formattedURL = (typeof url === 'string' && url.startsWith('/')) ? url : `/${url}`; - Ion.set(IONKEYS.APP_REDIRECT_TO, formattedURL); + Ion.merge(IONKEYS.APP_REDIRECT_TO, formattedURL); } /** @@ -27,7 +27,7 @@ function redirect(url) { function recordCurrentRoute({match}) { Ion.merge(IONKEYS.CURRENT_URL, match.url); if (match.url === currentRedirectTo) { - Ion.set(IONKEYS.APP_REDIRECT_TO, null); + Ion.merge(IONKEYS.APP_REDIRECT_TO, null); } } diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index c1728e2e5fbbe..6a9d0f8890239 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -461,7 +461,7 @@ function updateLastReadActionID(reportID, sequenceNumber) { * @param {string} comment */ function saveReportComment(reportID, comment) { - Ion.set(`${IONKEYS.REPORT_DRAFT_COMMENT}_${reportID}`, comment); + Ion.merge(`${IONKEYS.REPORT_DRAFT_COMMENT}_${reportID}`, comment); } /** From 8c4f56459aa508d274628753e7595e4f1ef7ae25 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 15:02:57 -0600 Subject: [PATCH 44/47] Disconnect from pusher when signing out --- src/lib/API.js | 3 --- src/lib/Pusher/pusher.js | 21 ++++++++++++++++++--- src/lib/actions/Session.js | 3 +++ src/page/home/HomePage.js | 4 ++-- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index 64da1751cad54..e9bb87454281b 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -290,9 +290,6 @@ Pusher.registerCustomAuthorizer((channel, {authEndpoint}) => ({ }, })); -// Initialize the pusher connection -Pusher.init(); - /** * Events that happen on the pusher socket are used to determine if the app is online or offline. The offline setting * is stored in Ion so the rest of the app has access to it. diff --git a/src/lib/Pusher/pusher.js b/src/lib/Pusher/pusher.js index a8a210ec0ae3f..97278d0f0c983 100644 --- a/src/lib/Pusher/pusher.js +++ b/src/lib/Pusher/pusher.js @@ -22,9 +22,14 @@ function callSocketEventCallbacks(eventName, data) { * @param {String} appKey * @param {Object} [params] * @public + * @returns {Promise} resolves when Pusher has connected */ function init(appKey, params) { - if (!socket) { + return new Promise((resolve) => { + if (socket) { + return resolve(); + } + // Use this for debugging // Pusher.log = (message) => { // if (window.console && window.console.log) { @@ -62,6 +67,7 @@ function init(appKey, params) { socket.connection.bind('connected', () => { console.debug('[Pusher] connected'); callSocketEventCallbacks('connected'); + resolve(); }); socket.connection.bind('disconnected', () => { @@ -73,7 +79,7 @@ function init(appKey, params) { console.debug('[Pusher] state changed', states); callSocketEventCallbacks('state_change', states); }); - } + }); } /** @@ -171,7 +177,7 @@ function bindEventToChannel(channel, eventName, eventCallback = () => {}, isChun */ function subscribe(channelName, eventName, eventCallback = () => {}, isChunked = false) { return new Promise((resolve, reject) => { - // We cannot call subscribe() before init(). Prevent any attempt to do this on dev. + // We cannot call subscribe() before init(). Prevent any attempt to do this on dev. if (!socket) { throw new Error(`[Pusher] instance not found. Pusher.subscribe() most likely has been called before Pusher.init()`); @@ -340,6 +346,14 @@ function registerCustomAuthorizer(authorizer) { customAuthorizer = authorizer; } +/** + * Disconnect from Pusher + */ +function disconnect() { + socket.disconnect(); + socket = null; +} + if (window) { /** * Pusher socket for debugging purposes @@ -361,4 +375,5 @@ export { sendChunkedEvent, registerSocketEventCallback, registerCustomAuthorizer, + disconnect, }; diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index 71d5133680f9c..72531c2c1503b 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -2,6 +2,8 @@ import Ion from '../Ion'; import * as API from '../API'; import IONKEYS from '../../IONKEYS'; import redirectToSignIn from './SignInRedirect'; +import * as Pusher from '../Pusher/pusher'; +import {subscribeToReportCommentEvents} from './Report'; let credentials; Ion.connect({ @@ -35,6 +37,7 @@ function signOut() { partnerUserID: credentials && credentials.login }); Ion.clear(); + Pusher.disconnect(); } export { diff --git a/src/page/home/HomePage.js b/src/page/home/HomePage.js index 98d260504a091..7e36a78633566 100644 --- a/src/page/home/HomePage.js +++ b/src/page/home/HomePage.js @@ -14,6 +14,7 @@ import Sidebar from './sidebar/SidebarView'; import Main from './MainView'; import {subscribeToReportCommentEvents, fetchAll as fetchAllReports} from '../../lib/actions/Report'; import {fetch as fetchPersonalDetails} from '../../lib/actions/PersonalDetails'; +import * as Pusher from '../../lib/Pusher/pusher'; const windowSize = Dimensions.get('window'); const widthBreakPoint = 1000; @@ -34,8 +35,7 @@ export default class App extends React.Component { } componentDidMount() { - // Listen for report comment events - subscribeToReportCommentEvents(); + Pusher.init().then(subscribeToReportCommentEvents); // Fetch all the personal details fetchPersonalDetails(); From 679351ffb2517294d098126ff2caa4621584410b Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 15:08:33 -0600 Subject: [PATCH 45/47] Fix notifications --- src/lib/ActiveClientManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/ActiveClientManager.js b/src/lib/ActiveClientManager.js index 8a711c557c2ab..c6155c91b03df 100644 --- a/src/lib/ActiveClientManager.js +++ b/src/lib/ActiveClientManager.js @@ -38,7 +38,7 @@ function removeClient() { function isClientTheLeader() { // At the moment activeClients only has 1 value i.e., the latest clientID so let's compare if // the latest matches the current browsers clientID. - return activeClients.clientID === clientID; + return activeClients[clientID] === clientID; } export { From b1e127b6060afa6247ce2a750e1d0d1712cb94b1 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 10 Sep 2020 15:16:18 -0600 Subject: [PATCH 46/47] Remove unused lib --- src/lib/actions/Session.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/actions/Session.js b/src/lib/actions/Session.js index 72531c2c1503b..6a12f0f8c9e0e 100644 --- a/src/lib/actions/Session.js +++ b/src/lib/actions/Session.js @@ -3,7 +3,6 @@ import * as API from '../API'; import IONKEYS from '../../IONKEYS'; import redirectToSignIn from './SignInRedirect'; import * as Pusher from '../Pusher/pusher'; -import {subscribeToReportCommentEvents} from './Report'; let credentials; Ion.connect({ From aa194ecb6525a59748b57eccdaa16d0469ee4d26 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Sep 2020 15:47:01 -0700 Subject: [PATCH 47/47] Add exact path for root and redirect to HomePage or SignIn --- src/Expensify.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Expensify.js b/src/Expensify.js index 7961aed0e16f4..e83e06622a758 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -88,8 +88,17 @@ class Expensify extends Component { + ( + this.state.authToken + ? + : + )} + /> - +