Skip to content

Commit

Permalink
Merge pull request #5706 from Expensify/Rory-FixLoadingSpinnerRaceCon…
Browse files Browse the repository at this point in the history
…dition

Add shouldCreateNewPolicy param to getPolicySummaries action

(cherry picked from commit 30ce47c)
  • Loading branch information
marcaaron authored and AndrewGable committed Oct 14, 2021
1 parent 9538ced commit 7adf523
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 109 deletions.
58 changes: 12 additions & 46 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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';
import _ from 'underscore';
import lodashGet from 'lodash/get';
Expand All @@ -27,10 +29,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';

Expand Down Expand Up @@ -65,7 +66,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,
Expand Down Expand Up @@ -107,38 +107,18 @@ 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({
/** Is the network currently offline or not */
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 {
Expand Down Expand Up @@ -187,7 +167,14 @@ class AuthScreens extends React.Component {
UnreadIndicatorUpdater.listenForReportChanges();
fetchFreePlanVerifiedBankAccount();

loadPoliciesBehindBeta(this.props.betas);
// 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;
getPolicyList(shouldCreateFreePolicy);
});

// Refresh the personal details, timezone and betas every 30 minutes
// There is no pusher event that sends updated personal details data yet
Expand Down Expand Up @@ -223,19 +210,7 @@ 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;
}

componentDidUpdate() {
loadPoliciesBehindBeta(this.props.betas);
return nextProps.isSmallScreenWidth !== this.props.isSmallScreenWidth;
}

componentWillUnmount() {
Expand All @@ -248,7 +223,6 @@ class AuthScreens extends React.Component {
cleanupSession();
clearInterval(this.interval);
this.interval = null;
hasLoadedPolicies = false;
}

render() {
Expand Down Expand Up @@ -310,11 +284,6 @@ class AuthScreens extends React.Component {
options={defaultScreenOptions}
component={LogInWithShortLivedTokenPage}
/>
<RootStack.Screen
name="WorkspaceNew"
options={defaultScreenOptions}
component={WorkspaceNew}
/>

{/* 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
Expand Down Expand Up @@ -422,8 +391,5 @@ export default compose(
network: {
key: ONYXKEYS.NETWORK,
},
betas: {
key: ONYXKEYS.BETAS,
},
}),
)(AuthScreens);
4 changes: 0 additions & 4 deletions src/libs/Navigation/getPathName/index.js

This file was deleted.

1 change: 0 additions & 1 deletion src/libs/Navigation/getPathName/index.native.js

This file was deleted.

1 change: 0 additions & 1 deletion src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
134 changes: 100 additions & 34 deletions src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,34 @@ 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
* @param {String} fullPolicy.id
* @param {String} fullPolicy.name
* @param {String} fullPolicy.role
* @param {String} fullPolicy.type
<<<<<<< HEAD
* @param {String} fullPolicy.outputCurrency
=======
>>>>>>> 30ce47c12 (Merge pull request #5706 from Expensify/Rory-FixLoadingSpinnerRaceCondition)
* @param {Object} fullPolicy.value.employeeList
* @param {String} [fullPolicy.value.avatarURL]
* @returns {Object}
Expand All @@ -41,26 +60,24 @@ function getSimplifiedPolicyObject(fullPolicy) {
name: fullPolicy.name,
role: fullPolicy.role,
type: fullPolicy.type,
<<<<<<< HEAD
outputCurrency: fullPolicy.outputCurrency,
=======
>>>>>>> 30ce47c12 (Merge pull request #5706 from Expensify/Rory-FixLoadingSpinnerRaceCondition)
employeeList: getSimplifiedEmployeeList(lodashGet(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}
* @param {Array<Object>} policyList
* @returns {Object}
*/
function getSimplifiedEmployeeList(employeeList) {
const employeeListEmails = _.chain(employeeList)
.pluck('email')
.flatten()
.unique()
.value();

return employeeListEmails;
function transformPolicyListToOnyxCollection(policyList) {
return _.reduce(policyList, (memo, policy) => ({
...memo,
[`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedPolicyObject(policy),
}), {});
}

/**
Expand All @@ -84,35 +101,82 @@ function updateAllPolicies(policyCollection) {
}

/**
* Fetches the policySummaryList from the API and saves a simplified version in Onyx
* Merges the passed in login into the specified policy
*
* @param {String} [name]
* @param {Boolean} [shouldAutomaticallyReroute]
* @returns {Promise}
*/
function getPolicySummaries() {
API.GetPolicySummaryList()
.then((data) => {
if (data.jsonCode === 200) {
const policyDataToStore = _.reduce(data.policySummaryList, (memo, policy) => ({
...memo,
[`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedPolicyObject(policy),
}), {});
updateAllPolicies(policyDataToStore);
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(() => {
const policyID = lodashGet(res, 'policyID');
if (shouldAutomaticallyReroute) {
Navigation.dismissModal();
Navigation.navigate(policyID ? ROUTES.getWorkspaceCardRoute(policyID) : ROUTES.HOME);
}
return Promise.resolve(policyID);
});
}

/**
* Fetches the policyList 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.
*
* 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() {
API.GetPolicyList()
function getPolicyList(shouldCreateNewPolicy = false) {
let newPolicyID;
const createPolicyPromise = shouldCreateNewPolicy
? create('', false)
: Promise.resolve();
createPolicyPromise
.then((policyID) => {
newPolicyID = policyID;
return API.GetPolicySummaryList();
})
.then((data) => {
if (data.jsonCode === 200) {
const policyDataToStore = transformPolicyListToOnyxCollection(data.policySummaryList || []);
updateAllPolicies(policyDataToStore);
}

if (shouldCreateNewPolicy) {
Navigation.dismissModal();
Navigation.navigate(newPolicyID ? ROUTES.getWorkspaceCardRoute(newPolicyID) : ROUTES.HOME);
}

return 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', ''),
},
}), {});
const policyDataToStore = transformPolicyListToOnyxCollection(data.policyList || []);
updateAllPolicies(policyDataToStore);
}
});
Expand Down Expand Up @@ -219,6 +283,7 @@ function invite(logins, welcomeNote, policyID) {
}

/**
<<<<<<< HEAD
* Merges the passed in login into the specified policy
*
* @param {String} [name]
Expand Down Expand Up @@ -251,6 +316,8 @@ function create(name = '') {
}

/**
=======
>>>>>>> 30ce47c12 (Merge pull request #5706 from Expensify/Rory-FixLoadingSpinnerRaceCondition)
* @param {Object} file
* @returns {Promise}
*/
Expand Down Expand Up @@ -320,7 +387,6 @@ function hideWorkspaceAlertMessage(policyID) {
}

export {
getPolicySummaries,
getPolicyList,
removeMembers,
invite,
Expand Down
14 changes: 7 additions & 7 deletions src/pages/LogInWithShortLivedTokenPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -52,11 +53,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', '');
Expand All @@ -69,16 +65,20 @@ 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;
}

// exitTo is URI encoded because it could contain a variable number of slashes (i.e. "workspace/new" vs "workspace/<ID>/card")
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
return;
}

// 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
Expand Down
Loading

0 comments on commit 7adf523

Please sign in to comment.