From 1725ba0a8230baef6c66ea14e5495ce15d8f64ee Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Wed, 6 Oct 2021 14:14:14 -0700 Subject: [PATCH 01/10] Add shouldCreateNewPolicy param to getPolicySummaries action --- src/libs/actions/Policy.js | 83 +++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 33 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index de08b01caa7..c83c1e213c2 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -77,11 +77,54 @@ function updateAllPolicies(policyCollection) { }); } +/** + * Merges the passed in login into the specified policy + * + * @param {String} [name] + * @param {Boolean} [shouldAutomaticallyReroute] + * @returns {Promise} + */ +function create(name = '', shouldAutomaticallyReroute = true) { + let res = null; + return API.Policy_Create({type: CONST.POLICY.TYPE.FREE, policyName: name}) + .then((response) => { + if (response.jsonCode !== 200) { + // Show the user feedback + const errorMessage = translateLocal('workspace.new.genericFailureMessage'); + Growl.error(errorMessage, 5000); + return; + } + res = response; + + // We are awaiting this merge so that we can guarantee our policy is available to any React components connected to the policies collection before we navigate to a new route. + return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${response.policyID}`, { + employeeList: getSimplifiedEmployeeList(response.policy.employeeList), + id: response.policyID, + type: response.policy.type, + name: response.policy.name, + role: CONST.POLICY.ROLE.ADMIN, + }); + }).then(() => { + if (shouldAutomaticallyReroute) { + Navigation.dismissModal(); + Navigation.navigate(ROUTES.getWorkspaceCardRoute(res.policyID)); + } + return Promise.resolve(res.policyID); + }); +} + /** * Fetches the policySummaryList from the API and saves a simplified version in Onyx + * + * @param {Boolean} [shouldCreateNewPolicy] */ -function getPolicySummaries() { - API.GetPolicySummaryList() +function getPolicySummaries(shouldCreateNewPolicy = false) { + let newPolicyID; + (shouldCreateNewPolicy ? create('', false) : Promise.resolve()) + .then(({policyID}) => { + newPolicyID = policyID; + return API.GetPolicySummaryList(); + }) .then((data) => { if (data.jsonCode === 200) { const policyDataToStore = _.reduce(data.policySummaryList, (memo, policy) => ({ @@ -90,6 +133,11 @@ function getPolicySummaries() { }), {}); updateAllPolicies(policyDataToStore); } + + if (shouldCreateNewPolicy) { + Navigation.dismissModal(); + Navigation.navigate(ROUTES.getWorkspaceCardRoute(newPolicyID)); + } }); } @@ -212,37 +260,6 @@ function invite(logins, welcomeNote, policyID) { }); } -/** - * Merges the passed in login into the specified policy - * - * @param {String} [name] - */ -function create(name = '') { - let res = null; - API.Policy_Create({type: CONST.POLICY.TYPE.FREE, policyName: name}) - .then((response) => { - if (response.jsonCode !== 200) { - // Show the user feedback - const errorMessage = translateLocal('workspace.new.genericFailureMessage'); - Growl.error(errorMessage, 5000); - return; - } - res = response; - - // We are awaiting this merge so that we can guarantee our policy is available to any React components connected to the policies collection before we navigate to a new route. - return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${response.policyID}`, { - employeeList: getSimplifiedEmployeeList(response.policy.employeeList), - id: response.policyID, - type: response.policy.type, - name: response.policy.name, - role: CONST.POLICY.ROLE.ADMIN, - }); - }).then(() => { - Navigation.dismissModal(); - Navigation.navigate(ROUTES.getWorkspaceCardRoute(res.policyID)); - }); -} - /** * @param {Object} file * @returns {Promise} From 6ea4750ace721abf9a5bdf6836823f3fbb5bbac3 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Wed, 6 Oct 2021 15:53:18 -0700 Subject: [PATCH 02/10] Get rid of policySummaryList API call, fix race condition --- src/libs/API.js | 12 ---- .../Navigation/AppNavigator/AuthScreens.js | 62 +++++++---------- src/libs/Navigation/Navigation.js | 13 ++++ src/libs/actions/Policy.js | 66 +++++++------------ 4 files changed, 63 insertions(+), 90 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index b87711ef3ef..2933b6fef98 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -477,17 +477,6 @@ function GetPolicyList() { return Network.post(commandName, parameters); } -/** - * @returns {Promise} - */ -function GetPolicySummaryList() { - const commandName = 'Get'; - const parameters = { - returnValueList: 'policySummaryList', - }; - return Network.post(commandName, parameters); -} - /** * @returns {Promise} */ @@ -1104,7 +1093,6 @@ export { GetShortLivedAuthToken, GetIOUReport, GetPolicyList, - GetPolicySummaryList, GetReportSummaryList, GetRequestCountryCode, Graphite_Timer, diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index ce580391a5f..feeb4c8b87c 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Onyx, {withOnyx} from 'react-native-onyx'; +import Str from 'expensify-common/lib/str'; import moment from 'moment'; import _ from 'underscore'; import lodashGet from 'lodash/get'; @@ -27,10 +28,9 @@ import Navigation from '../Navigation'; import * as User from '../../actions/User'; import {setModalVisibility} from '../../actions/Modal'; import NameValuePair from '../../actions/NameValuePair'; -import {getPolicySummaries, getPolicyList} from '../../actions/Policy'; +import {getPolicyList} from '../../actions/Policy'; import modalCardStyleInterpolator from './modalCardStyleInterpolator'; import createCustomModalStackNavigator from './createCustomModalStackNavigator'; -import Permissions from '../../Permissions'; import getOperatingSystem from '../../getOperatingSystem'; import {fetchFreePlanVerifiedBankAccount} from '../../actions/BankAccounts'; @@ -112,22 +112,6 @@ const modalScreenListeners = { }, }; -let hasLoadedPolicies = false; - -/** - * We want to only load policy info if you are in the freePlan beta. - * @param {Array} betas - */ -function loadPoliciesBehindBeta(betas) { - // When removing the freePlan beta, simply load the policyList and the policySummaries in componentDidMount(). - // Policy info loading should not be blocked behind the defaultRooms beta alone. - if (!hasLoadedPolicies && (Permissions.canUseFreePlan(betas) || Permissions.canUseDefaultRooms(betas))) { - getPolicyList(); - getPolicySummaries(); - hasLoadedPolicies = true; - } -} - const propTypes = { /** Information about the network */ network: PropTypes.shape({ @@ -135,21 +119,19 @@ const propTypes = { isOffline: PropTypes.bool, }), - /** List of betas available to current user */ - betas: PropTypes.arrayOf(PropTypes.string), - ...windowDimensionsPropTypes, }; const defaultProps = { network: {isOffline: true}, - betas: [], }; class AuthScreens extends React.Component { constructor(props) { super(props); + this.hasLoadedPolicies = false; + Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); Timing.start(CONST.TIMING.HOMEPAGE_REPORTS_LOADED); } @@ -192,7 +174,7 @@ class AuthScreens extends React.Component { UnreadIndicatorUpdater.listenForReportChanges(); fetchFreePlanVerifiedBankAccount(); - loadPoliciesBehindBeta(this.props.betas); + this.loadPolicies(); // Refresh the personal details, timezone and betas every 30 minutes // There is no pusher event that sends updated personal details data yet @@ -228,19 +210,11 @@ class AuthScreens extends React.Component { } shouldComponentUpdate(nextProps) { - if (nextProps.isSmallScreenWidth !== this.props.isSmallScreenWidth) { - return true; - } - - if (nextProps.betas !== this.props.betas) { - return true; - } - - return false; + return nextProps.isSmallScreenWidth !== this.props.isSmallScreenWidth; } componentDidUpdate() { - loadPoliciesBehindBeta(this.props.betas); + this.loadPolicies(); } componentWillUnmount() { @@ -253,7 +227,24 @@ class AuthScreens extends React.Component { cleanupSession(); clearInterval(this.interval); this.interval = null; - hasLoadedPolicies = false; + } + + /** + * Load policies, maybe creating a new policy first. + */ + loadPolicies() { + const {path, params} = Navigation.getCurrentRoute() || {}; + + // Don't try to load policies until the path is loaded and we know whether or not we need to create a new policy + if (!path) { + return; + } + + if (!this.hasLoadedPolicies) { + this.hasLoadedPolicies = true; + const shouldCreateFreePolicy = Str.startsWith(path, ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN) && params.exitTo === ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN; + getPolicyList(shouldCreateFreePolicy); + } } render() { @@ -457,8 +448,5 @@ export default compose( network: { key: ONYXKEYS.NETWORK, }, - betas: { - key: ONYXKEYS.BETAS, - }, }), )(AuthScreens); diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index 12279df62d2..df9cd4cee52 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -122,6 +122,18 @@ function navigate(route = ROUTES.HOME) { linkTo(navigationRef.current, route); } +/** + * @returns {Object|undefined} + */ +function getCurrentRoute() { + if (!navigationRef.isReady()) { + console.debug('[Navigation] navigate failed because navigation ref was not yet ready'); + return; + } + + return navigationRef.getCurrentRoute(); +} + /** * Dismisses a screen presented modally and returns us back to the previous view. * @@ -224,6 +236,7 @@ DismissModal.defaultProps = { export default { navigate, + getCurrentRoute, dismissModal, isActiveRoute, goBack, diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index c83c1e213c2..1c234a8b7fa 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -22,7 +22,23 @@ Onyx.connect({ }); /** - * Takes a full policy summary that is returned from the policySummaryList and simplifies it so we are only storing + * Simplifies the employeeList response into an object containing an array of emails + * + * @param {Object} employeeList + * @returns {Array} + */ +function getSimplifiedEmployeeList(employeeList) { + const employeeListEmails = _.chain(employeeList) + .pluck('email') + .flatten() + .unique() + .value(); + + return employeeListEmails; +} + +/** + * Takes a full policy that is returned from the policyList and simplifies it so we are only storing * the pieces of data that we need to in Onyx * * @param {Object} fullPolicy @@ -30,6 +46,8 @@ Onyx.connect({ * @param {String} fullPolicy.name * @param {String} fullPolicy.role * @param {String} fullPolicy.type + * @param {Object} fullPolicy.value.employeeList + * @param {String} [fullPolicy.value.avatarURL] * @returns {Object} */ function getSimplifiedPolicyObject(fullPolicy) { @@ -38,25 +56,11 @@ function getSimplifiedPolicyObject(fullPolicy) { name: fullPolicy.name, role: fullPolicy.role, type: fullPolicy.type, + employeeList: getSimplifiedEmployeeList(fullPolicy.value.employeeList), + avatarURL: lodashGet(fullPolicy, 'value.avatarURL', ''), }; } -/** - * Simplifies the employeeList response into an object containing an array of emails - * - * @param {Object} employeeList - * @returns {Array} - */ -function getSimplifiedEmployeeList(employeeList) { - const employeeListEmails = _.chain(employeeList) - .pluck('email') - .flatten() - .unique() - .value(); - - return employeeListEmails; -} - /** * Used to update ALL of the policies at once. If a policy is present locally, but not in the policies object passed here it will be removed. * @param {Object} policyCollection - object of policy key and partial policy object @@ -114,20 +118,20 @@ function create(name = '', shouldAutomaticallyReroute = true) { } /** - * Fetches the policySummaryList from the API and saves a simplified version in Onyx + * Fetches policy list from the API and saves a simplified version in Onyx, optionally creating a new policy first. * * @param {Boolean} [shouldCreateNewPolicy] */ -function getPolicySummaries(shouldCreateNewPolicy = false) { +function getPolicyList(shouldCreateNewPolicy = false) { let newPolicyID; (shouldCreateNewPolicy ? create('', false) : Promise.resolve()) .then(({policyID}) => { newPolicyID = policyID; - return API.GetPolicySummaryList(); + return API.GetPolicyList(); }) .then((data) => { if (data.jsonCode === 200) { - const policyDataToStore = _.reduce(data.policySummaryList, (memo, policy) => ({ + const policyDataToStore = _.reduce(data.policyList, (memo, policy) => ({ ...memo, [`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedPolicyObject(policy), }), {}); @@ -141,25 +145,6 @@ function getPolicySummaries(shouldCreateNewPolicy = false) { }); } -/** - * Fetches the policyList from the API and saves a simplified version in Onyx - */ -function getPolicyList() { - API.GetPolicyList() - .then((data) => { - if (data.jsonCode === 200) { - const policyDataToStore = _.reduce(data.policyList, (memo, policy) => ({ - ...memo, - [`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: { - employeeList: getSimplifiedEmployeeList(policy.value.employeeList), - avatarURL: lodashGet(policy, 'value.avatarURL', ''), - }, - }), {}); - updateAllPolicies(policyDataToStore); - } - }); -} - /** * Is the user an admin of a free policy (aka workspace)? * @@ -329,7 +314,6 @@ function hideWorkspaceAlertMessage(policyID) { } export { - getPolicySummaries, getPolicyList, removeMembers, invite, From 27a0d69876086536119823661193179e7f448e2f Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Wed, 6 Oct 2021 15:57:03 -0700 Subject: [PATCH 03/10] Fix authentication race condition in LogInWithShortLivedTokenPage --- src/pages/LogInWithShortLivedTokenPage.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/pages/LogInWithShortLivedTokenPage.js b/src/pages/LogInWithShortLivedTokenPage.js index 88265401f24..f00765ac0f6 100644 --- a/src/pages/LogInWithShortLivedTokenPage.js +++ b/src/pages/LogInWithShortLivedTokenPage.js @@ -52,11 +52,6 @@ const defaultProps = { }; class LogInWithShortLivedTokenPage extends Component { - constructor(props) { - super(props); - this.state = {hasRun: false}; - } - componentDidMount() { const accountID = parseInt(lodashGet(this.props.route.params, 'accountID', ''), 10); const email = lodashGet(this.props.route.params, 'email', ''); @@ -69,11 +64,11 @@ class LogInWithShortLivedTokenPage extends Component { } signInWithShortLivedToken(accountID, email, shortLivedToken, encryptedAuthToken); - this.setState({hasRun: true}); } componentDidUpdate() { - if (this.state.hasRun || !this.props.betas) { + const email = lodashGet(this.props.route.params, 'email', ''); + if (!this.props.betas || !this.props.session.authToken || email !== this.props.session.email) { return; } From 12a2930d6c681fc0333480a8e4508d06524be94a Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Wed, 6 Oct 2021 16:20:50 -0700 Subject: [PATCH 04/10] Deprecate WorkspaceNew page --- src/libs/Navigation/AppNavigator/AuthScreens.js | 8 +------- src/libs/Navigation/linkingConfig.js | 1 - src/pages/LogInWithShortLivedTokenPage.js | 7 ++++++- src/pages/workspace/WorkspaceNew.js | 16 ---------------- 4 files changed, 7 insertions(+), 25 deletions(-) delete mode 100644 src/pages/workspace/WorkspaceNew.js diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index feeb4c8b87c..8829f5b17cb 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -70,7 +70,6 @@ import defaultScreenOptions from './defaultScreenOptions'; import * as API from '../../API'; import {setLocale} from '../../actions/App'; import {cleanupSession} from '../../actions/Session'; -import WorkspaceNew from '../../../pages/workspace/WorkspaceNew'; Onyx.connect({ key: ONYXKEYS.MY_PERSONAL_DETAILS, @@ -242,7 +241,7 @@ class AuthScreens extends React.Component { if (!this.hasLoadedPolicies) { this.hasLoadedPolicies = true; - const shouldCreateFreePolicy = Str.startsWith(path, ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN) && params.exitTo === ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN; + const shouldCreateFreePolicy = Str.startsWith(path, ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN) && params.exitTo === ROUTES.WORKSPACE_NEW; getPolicyList(shouldCreateFreePolicy); } } @@ -317,11 +316,6 @@ class AuthScreens extends React.Component { options={defaultScreenOptions} component={LogInWithShortLivedTokenPage} /> - {/* These are the various modal routes */} {/* Note: Each modal must have it's own stack navigator since we want to be able to navigate to any diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index 5311901ab6d..7f246559de0 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -33,7 +33,6 @@ export default { [SCREENS.LOGIN_WITH_VALIDATE_CODE_WORKSPACE_CARD]: ROUTES.LOGIN_WITH_VALIDATE_CODE_WORKSPACE_CARD, [SCREENS.LOGIN_WITH_VALIDATE_CODE_2FA_WORKSPACE_CARD]: ROUTES.LOGIN_WITH_VALIDATE_CODE_2FA_WORKSPACE_CARD, [SCREENS.LOG_IN_WITH_SHORT_LIVED_TOKEN]: ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN, - WorkspaceNew: ROUTES.WORKSPACE_NEW, // Modal Screens Settings: { diff --git a/src/pages/LogInWithShortLivedTokenPage.js b/src/pages/LogInWithShortLivedTokenPage.js index f00765ac0f6..b44f9f628fa 100644 --- a/src/pages/LogInWithShortLivedTokenPage.js +++ b/src/pages/LogInWithShortLivedTokenPage.js @@ -2,6 +2,7 @@ import React, {Component} from 'react'; import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; +import ROUTES from '../ROUTES'; import compose from '../libs/compose'; import ONYXKEYS from '../ONYXKEYS'; import {signInWithShortLivedToken} from '../libs/actions/Session'; @@ -73,7 +74,11 @@ class LogInWithShortLivedTokenPage extends Component { } // exitTo is URI encoded because it could contain a variable number of slashes (i.e. "workspace/new" vs "workspace//card") - const exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', '')); + let exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', '')); + if (exitTo === ROUTES.WORKSPACE_NEW) { + // New workspace creation is handled in AuthScreens, not in its own screen + exitTo = ''; + } // In order to navigate to a modal, we first have to dismiss the current modal. But there is no current // modal you say? I know, it confuses me too. Without dismissing the current modal, if the user cancels out diff --git a/src/pages/workspace/WorkspaceNew.js b/src/pages/workspace/WorkspaceNew.js deleted file mode 100644 index cbd987d4999..00000000000 --- a/src/pages/workspace/WorkspaceNew.js +++ /dev/null @@ -1,16 +0,0 @@ -import React, {Component} from 'react'; -import * as Policy from '../../libs/actions/Policy'; -import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; - -class WorkspaceNew extends Component { - componentDidMount() { - // After the workspace is created, the user will automatically be directed to its settings page - Policy.create(); - } - - render() { - return ; - } -} - -export default WorkspaceNew; From da3ea688a9936d918cd6b020f0ecbe952fd6d059 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 7 Oct 2021 17:08:35 -0700 Subject: [PATCH 05/10] Use Linking.getInitialURL instead of Navigation.getCurrentURL --- .../Navigation/AppNavigator/AuthScreens.js | 35 ++++++------------- src/libs/Navigation/Navigation.js | 13 ------- src/libs/Navigation/getPathName/index.js | 4 --- .../Navigation/getPathName/index.native.js | 1 - src/libs/actions/Policy.js | 2 +- 5 files changed, 11 insertions(+), 44 deletions(-) delete mode 100644 src/libs/Navigation/getPathName/index.js delete mode 100644 src/libs/Navigation/getPathName/index.native.js diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 8829f5b17cb..6541bd899a8 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import {Linking} from 'react-native'; import Onyx, {withOnyx} from 'react-native-onyx'; import Str from 'expensify-common/lib/str'; import moment from 'moment'; @@ -129,8 +130,6 @@ class AuthScreens extends React.Component { constructor(props) { super(props); - this.hasLoadedPolicies = false; - Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); Timing.start(CONST.TIMING.HOMEPAGE_REPORTS_LOADED); } @@ -173,7 +172,15 @@ class AuthScreens extends React.Component { UnreadIndicatorUpdater.listenForReportChanges(); fetchFreePlanVerifiedBankAccount(); - this.loadPolicies(); + // Load policies, maybe creating a new policy first. + Linking.getInitialURL() + .then((url) => { + const path = new URL(url).pathname; + const exitTo = new URLSearchParams(url).get('exitTo'); + const shouldCreateFreePolicy = Str.startsWith(path, Str.normalizeUrl(ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN)) && exitTo === ROUTES.WORKSPACE_NEW; + console.log('RORY_DEBUG calling getPolicyList w/ shouldCreateFreePolicy', shouldCreateFreePolicy); + getPolicyList(shouldCreateFreePolicy); + }); // Refresh the personal details, timezone and betas every 30 minutes // There is no pusher event that sends updated personal details data yet @@ -212,10 +219,6 @@ class AuthScreens extends React.Component { return nextProps.isSmallScreenWidth !== this.props.isSmallScreenWidth; } - componentDidUpdate() { - this.loadPolicies(); - } - componentWillUnmount() { if (this.unsubscribeSearchShortcut) { this.unsubscribeSearchShortcut(); @@ -228,24 +231,6 @@ class AuthScreens extends React.Component { this.interval = null; } - /** - * Load policies, maybe creating a new policy first. - */ - loadPolicies() { - const {path, params} = Navigation.getCurrentRoute() || {}; - - // Don't try to load policies until the path is loaded and we know whether or not we need to create a new policy - if (!path) { - return; - } - - if (!this.hasLoadedPolicies) { - this.hasLoadedPolicies = true; - const shouldCreateFreePolicy = Str.startsWith(path, ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN) && params.exitTo === ROUTES.WORKSPACE_NEW; - getPolicyList(shouldCreateFreePolicy); - } - } - render() { const commonModalScreenOptions = { headerShown: false, diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index df9cd4cee52..12279df62d2 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -122,18 +122,6 @@ function navigate(route = ROUTES.HOME) { linkTo(navigationRef.current, route); } -/** - * @returns {Object|undefined} - */ -function getCurrentRoute() { - if (!navigationRef.isReady()) { - console.debug('[Navigation] navigate failed because navigation ref was not yet ready'); - return; - } - - return navigationRef.getCurrentRoute(); -} - /** * Dismisses a screen presented modally and returns us back to the previous view. * @@ -236,7 +224,6 @@ DismissModal.defaultProps = { export default { navigate, - getCurrentRoute, dismissModal, isActiveRoute, goBack, diff --git a/src/libs/Navigation/getPathName/index.js b/src/libs/Navigation/getPathName/index.js deleted file mode 100644 index 161cece2a2e..00000000000 --- a/src/libs/Navigation/getPathName/index.js +++ /dev/null @@ -1,4 +0,0 @@ -export default (initialUrl) => { - const initialURLObject = new URL(initialUrl); - return initialURLObject.pathname; -}; diff --git a/src/libs/Navigation/getPathName/index.native.js b/src/libs/Navigation/getPathName/index.native.js deleted file mode 100644 index 94bc2b70e39..00000000000 --- a/src/libs/Navigation/getPathName/index.native.js +++ /dev/null @@ -1 +0,0 @@ -export default () => ''; diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 1c234a8b7fa..0bbd10247b1 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -125,7 +125,7 @@ function create(name = '', shouldAutomaticallyReroute = true) { function getPolicyList(shouldCreateNewPolicy = false) { let newPolicyID; (shouldCreateNewPolicy ? create('', false) : Promise.resolve()) - .then(({policyID}) => { + .then((policyID) => { newPolicyID = policyID; return API.GetPolicyList(); }) From b4c0f71093d288a2e2f4239869cf335bc29cfb4a Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 7 Oct 2021 17:14:56 -0700 Subject: [PATCH 06/10] Remove console log --- src/libs/Navigation/AppNavigator/AuthScreens.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 6541bd899a8..72f26868fa7 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -178,7 +178,6 @@ class AuthScreens extends React.Component { const path = new URL(url).pathname; const exitTo = new URLSearchParams(url).get('exitTo'); const shouldCreateFreePolicy = Str.startsWith(path, Str.normalizeUrl(ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN)) && exitTo === ROUTES.WORKSPACE_NEW; - console.log('RORY_DEBUG calling getPolicyList w/ shouldCreateFreePolicy', shouldCreateFreePolicy); getPolicyList(shouldCreateFreePolicy); }); From 8327635f123d0bdbe0dc9970e19d1bdf7fa26638 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 7 Oct 2021 17:49:54 -0700 Subject: [PATCH 07/10] Cleanup per review comments --- src/libs/actions/Policy.js | 7 +++++-- src/pages/LogInWithShortLivedTokenPage.js | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 0bbd10247b1..b1ee84da8d2 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -113,7 +113,7 @@ function create(name = '', shouldAutomaticallyReroute = true) { Navigation.dismissModal(); Navigation.navigate(ROUTES.getWorkspaceCardRoute(res.policyID)); } - return Promise.resolve(res.policyID); + return Promise.resolve(lodashGet(res, 'policyID')); }); } @@ -124,7 +124,10 @@ function create(name = '', shouldAutomaticallyReroute = true) { */ function getPolicyList(shouldCreateNewPolicy = false) { let newPolicyID; - (shouldCreateNewPolicy ? create('', false) : Promise.resolve()) + const createPolicyPromise = shouldCreateNewPolicy + ? create('', false) + : Promise.resolve(); + createPolicyPromise .then((policyID) => { newPolicyID = policyID; return API.GetPolicyList(); diff --git a/src/pages/LogInWithShortLivedTokenPage.js b/src/pages/LogInWithShortLivedTokenPage.js index b44f9f628fa..da341feb780 100644 --- a/src/pages/LogInWithShortLivedTokenPage.js +++ b/src/pages/LogInWithShortLivedTokenPage.js @@ -74,10 +74,10 @@ class LogInWithShortLivedTokenPage extends Component { } // exitTo is URI encoded because it could contain a variable number of slashes (i.e. "workspace/new" vs "workspace//card") - let exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', '')); + const exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', '')); if (exitTo === ROUTES.WORKSPACE_NEW) { // New workspace creation is handled in AuthScreens, not in its own screen - exitTo = ''; + return; } // In order to navigate to a modal, we first have to dismiss the current modal. But there is no current From cf625d17fede0fa264380da28ab6864a7371053b Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Fri, 8 Oct 2021 09:49:59 -0700 Subject: [PATCH 08/10] Safe-access policyID --- src/libs/actions/Policy.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index b1ee84da8d2..c6698bb7b1f 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -109,11 +109,12 @@ function create(name = '', shouldAutomaticallyReroute = true) { role: CONST.POLICY.ROLE.ADMIN, }); }).then(() => { + const policyID = lodashGet(res, 'policyID'); if (shouldAutomaticallyReroute) { Navigation.dismissModal(); - Navigation.navigate(ROUTES.getWorkspaceCardRoute(res.policyID)); + Navigation.navigate(ROUTES.getWorkspaceCardRoute(policyID)); } - return Promise.resolve(lodashGet(res, 'policyID')); + return Promise.resolve(policyID); }); } From 49878f195b3b39f4c806f9870165e7b67500af2c Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Fri, 8 Oct 2021 10:16:38 -0700 Subject: [PATCH 09/10] Load policy summaries first, then full policies --- src/libs/API.js | 12 ++++++++++++ src/libs/actions/Policy.js | 39 +++++++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 2933b6fef98..b87711ef3ef 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -477,6 +477,17 @@ function GetPolicyList() { return Network.post(commandName, parameters); } +/** + * @returns {Promise} + */ +function GetPolicySummaryList() { + const commandName = 'Get'; + const parameters = { + returnValueList: 'policySummaryList', + }; + return Network.post(commandName, parameters); +} + /** * @returns {Promise} */ @@ -1093,6 +1104,7 @@ export { GetShortLivedAuthToken, GetIOUReport, GetPolicyList, + GetPolicySummaryList, GetReportSummaryList, GetRequestCountryCode, Graphite_Timer, diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 7e406843214..3b5284a95b6 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -56,11 +56,22 @@ function getSimplifiedPolicyObject(fullPolicy) { name: fullPolicy.name, role: fullPolicy.role, type: fullPolicy.type, - employeeList: getSimplifiedEmployeeList(fullPolicy.value.employeeList), + employeeList: getSimplifiedEmployeeList(lodashGet(fullPolicy, 'value.employeeList')); avatarURL: lodashGet(fullPolicy, 'value.avatarURL', ''), }; } +/** + * @param {Array} policyList + * @returns {Object} + */ +function transformPolicyListToOnyxCollection(policyList) { + return _.reduce(policyList, (memo, policy) => ({ + ...memo, + [`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedPolicyObject(policy), + }), {}); +} + /** * Used to update ALL of the policies at once. If a policy is present locally, but not in the policies object passed here it will be removed. * @param {Object} policyCollection - object of policy key and partial policy object @@ -121,6 +132,15 @@ function create(name = '', shouldAutomaticallyReroute = true) { /** * Fetches policy list from the API and saves a simplified version in Onyx, optionally creating a new policy first. * + * More specifically, this action will: + * 1. Optionally create a new policy. + * 2. Fetch policy summaries. + * 3. Optionally navigate to the new policy. + * 4. Then fetch full policies. + * + * This way, we ensure that there's no race condition between creating the new policy and fetching existing ones, + * and we also don't have to wait for full policies to load before navigating to the new policy. + * * @param {Boolean} [shouldCreateNewPolicy] */ function getPolicyList(shouldCreateNewPolicy = false) { @@ -131,20 +151,25 @@ function getPolicyList(shouldCreateNewPolicy = false) { createPolicyPromise .then((policyID) => { newPolicyID = policyID; - return API.GetPolicyList(); + return API.GetPolicySummaryList(); }) .then((data) => { if (data.jsonCode === 200) { - const policyDataToStore = _.reduce(data.policyList, (memo, policy) => ({ - ...memo, - [`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedPolicyObject(policy), - }), {}); + const policyDataToStore = transformPolicyListToOnyxCollection(data.policySummaryList || []); updateAllPolicies(policyDataToStore); } if (shouldCreateNewPolicy) { Navigation.dismissModal(); - Navigation.navigate(ROUTES.getWorkspaceCardRoute(newPolicyID)); + Navigation.navigate(newPolicyID ? ROUTES.getWorkspaceCardRoute(newPolicyID) : ROUTES.HOME); + } + + return API.GetPolicyList(); + }) + .then((data) => { + if (data.jsonCode === 200) { + const policyDataToStore = transformPolicyListToOnyxCollection(data.policyList || []); + updateAllPolicies(policyDataToStore); } }); } From 3fec33917d62c23c54e1a08f408c121231b3abce Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Fri, 8 Oct 2021 10:22:52 -0700 Subject: [PATCH 10/10] Fix semicolon typo --- src/libs/actions/Policy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 3b5284a95b6..289436cfee3 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -56,7 +56,7 @@ function getSimplifiedPolicyObject(fullPolicy) { name: fullPolicy.name, role: fullPolicy.role, type: fullPolicy.type, - employeeList: getSimplifiedEmployeeList(lodashGet(fullPolicy, 'value.employeeList')); + employeeList: getSimplifiedEmployeeList(lodashGet(fullPolicy, 'value.employeeList')), avatarURL: lodashGet(fullPolicy, 'value.avatarURL', ''), }; }