Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create separate transition pages to sign the user in / out #8855

Merged
merged 70 commits into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
0275158
Once logged in, wait for betas before navigating
neil-marcellini Apr 4, 2022
ae4f499
Remove extra update to session and accountID
neil-marcellini Apr 5, 2022
e2daffb
Merge pull request #8498 from Expensify/neil-continue-setup
neil-marcellini Apr 26, 2022
ff3dbaf
Merge branch 'main' of github.com:Expensify/App into neil-transition-…
neil-marcellini Apr 26, 2022
37dfb3b
Remove beta check on reimbursement account page
neil-marcellini Apr 28, 2022
f5ac630
Don't wait for betas before navigating
neil-marcellini Apr 28, 2022
76035f9
Merge pull request #8819 from Expensify/neil-remove-beta-check
marcaaron Apr 28, 2022
1ae8690
Separate transition page to log out the old user
neil-marcellini Apr 29, 2022
732517d
Only log in the transitioning user
neil-marcellini Apr 29, 2022
c4a6a67
Separate transition pages for the transition route
neil-marcellini Apr 29, 2022
908a938
Navigate to transition exit from the auth screens
neil-marcellini Apr 30, 2022
4c65e1e
Remove extra param from signInWithShortLivedToken
neil-marcellini May 2, 2022
d1800a5
Navigate in transition page so navigation is ready
neil-marcellini May 2, 2022
9ac316a
Simplify the comment about dismissing the modal
neil-marcellini May 2, 2022
e647868
Revert "Navigate in transition page so navigation is ready"
neil-marcellini May 2, 2022
019dba9
Navigate in AuthScreens when navigation is ready
neil-marcellini May 2, 2022
56a79d2
Fix create login requests before Network is ready
neil-marcellini Mar 22, 2022
50082e7
Merge branch 'main' of github.com:Expensify/App into neil-transition-…
neil-marcellini May 6, 2022
5f621ea
Merge branch 'main' of github.com:Expensify/App into neil-separate-tr…
neil-marcellini May 6, 2022
7a243c2
Merge branch 'neil-transition-flows' into neil-separate-transition-pages
neil-marcellini May 6, 2022
328f385
Use the new createOnReadyTask for navigation
neil-marcellini May 6, 2022
f4b40d4
Revert "Fix create login requests before Network is ready"
neil-marcellini May 6, 2022
8ccf60f
Call the proper navigationReadyTask methods
neil-marcellini May 6, 2022
60635af
Fix don't show create menu on workspace/new
neil-marcellini May 9, 2022
96e7fd4
Simplify comment and remove log lines
neil-marcellini May 9, 2022
d47dce8
Rename LogOutOldUserPage to LogOutPreviousUserPage
neil-marcellini May 9, 2022
6dd4573
Only set navigation ready when we are logged in.
neil-marcellini May 9, 2022
b490551
Clean up default props and their use
neil-marcellini May 9, 2022
174e012
Fix shouldCreateFreePolicy indentation
neil-marcellini May 9, 2022
607e48d
Remove unused Log import
neil-marcellini May 9, 2022
dd54853
Clean up propTypes and props more
neil-marcellini May 9, 2022
468c5a0
Use a NavigationReadyDetector in the navigator
neil-marcellini May 17, 2022
f901904
Initial wait for onyx to clear
neil-marcellini May 18, 2022
853740e
Merge branch 'main' of github.com:Expensify/App into neil-separate-tr…
neil-marcellini May 18, 2022
d34921a
Reset required data ready task on log out
neil-marcellini May 19, 2022
f976228
Revert "Reset required data ready task on log out"
neil-marcellini May 19, 2022
56f9ba8
Merge in Network fixes from main
neil-marcellini May 20, 2022
511cc68
Don't navigate to the workspace chat on transition
neil-marcellini May 20, 2022
3a158f3
Navigation is ready once the user has transitioned
neil-marcellini May 20, 2022
35a18a0
Get the policy list even if there's no initial URL
neil-marcellini May 23, 2022
6607d59
Remove leftover fragment tags
neil-marcellini May 23, 2022
ecdbe41
Add JSDoc to waitForOnyxToClear()
neil-marcellini May 23, 2022
bb8bf06
Make transition page props required
neil-marcellini May 23, 2022
7e3030c
Get params.email directly from the required route
neil-marcellini May 23, 2022
f291838
Navigate to exit route without a separate function
neil-marcellini May 23, 2022
326b3b1
Use a promise directly to wait for Onyx to clear
neil-marcellini May 23, 2022
ed16b92
Update the navigation readiness based on its state
neil-marcellini May 23, 2022
a7cb78e
Remove unused import
neil-marcellini May 23, 2022
f0bc0f8
Track navigation readiness with a promise directly
neil-marcellini May 23, 2022
3cd291d
Fix waitForOnyxToClear promises
neil-marcellini May 23, 2022
f3b265d
Rename transition to transitionFromOldDot
neil-marcellini May 23, 2022
a34c511
Fix ROUTES.TRANSITION_FROM_OLD_DOT to match link
neil-marcellini May 23, 2022
4729aca
Create a setupPoliciesAndNavigate action
neil-marcellini May 23, 2022
35b5777
Ensure the user is logged in before policy setup
neil-marcellini May 23, 2022
320ec95
Remove unnecessary check for session in action
neil-marcellini May 24, 2022
fe274ed
Pass session to setUpPoliciesAndNavigate
neil-marcellini May 24, 2022
0921b6c
Change navigationRef import in NavigationRoot
neil-marcellini May 24, 2022
09e3bf1
Move waitForOnyxToClear into the action
neil-marcellini May 24, 2022
b5da43a
Add a detailed comment to setUpPoliciesAndNavigate
neil-marcellini May 24, 2022
1c211f1
Rename waitForOnyxToClear methods
neil-marcellini May 24, 2022
5d928e6
Fix merge conflicts in the welcome action
neil-marcellini May 24, 2022
2097b8e
Fix Onyx.clear by updating to the latest version
neil-marcellini May 31, 2022
e85a80c
Remove waitForOnyxToClear
neil-marcellini May 31, 2022
dc6fa72
Install Onyx using the npm package
neil-marcellini May 31, 2022
eee230f
Merge main to fix Onyx version conflicts
neil-marcellini May 31, 2022
dcc3e79
Wait for navigation just before navigating
neil-marcellini May 31, 2022
4baed9e
Clear the session error on a successful sign in
neil-marcellini May 31, 2022
552e13c
Again, setIsNavigationReady() in a transition page
neil-marcellini May 31, 2022
0e20ad0
Add a detailed comment about setIsNavigationReady
neil-marcellini Jun 1, 2022
0222143
Merge branch 'main' of github.com:Expensify/App into neil-separate-tr…
neil-marcellini Jun 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"react-native-image-size": "^1.1.3",
"react-native-keyboard-spacer": "^0.4.1",
"react-native-modal": "^13.0.0",
"react-native-onyx": "git+https://github.com/Expensify/react-native-onyx.git#4cc46d1ad70312d2e10adf7cdd7065ab9b472113",
"react-native-onyx": "1.0.5",
"react-native-pdf": "^6.2.2",
"react-native-performance": "^2.0.0",
"react-native-permissions": "^3.0.1",
Expand Down
2 changes: 1 addition & 1 deletion src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default {
getReportDetailsRoute: reportID => `r/${reportID}/details`,
REPORT_SETTINGS: 'r/:reportID/settings',
getReportSettingsRoute: reportID => `r/${reportID}/settings`,
LOGIN_WITH_SHORT_LIVED_TOKEN: 'transition',
TRANSITION_FROM_OLD_DOT: 'transition',
VALIDATE_LOGIN: 'v/:accountID/:validateCode',
GET_ASSISTANCE: 'get-assistance/:taskID',
getGetAssistanceRoute: taskID => `get-assistance/${taskID}`,
Expand Down
2 changes: 1 addition & 1 deletion src/SCREENS.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ export default {
HOME: 'Home',
LOADING: 'Loading',
REPORT: 'Report',
LOG_IN_WITH_SHORT_LIVED_TOKEN: 'LogInWithShortLivedToken',
TRANSITION_FROM_OLD_DOT: 'TransitionFromOldDot',
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
};
39 changes: 4 additions & 35 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import React from 'react';
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 Down Expand Up @@ -34,11 +32,11 @@ import MainDrawerNavigator from './MainDrawerNavigator';
import * as ModalStackNavigators from './ModalStackNavigators';
import SCREENS from '../../../SCREENS';
import Timers from '../../Timers';
import LogInWithShortLivedTokenPage from '../../../pages/LogInWithShortLivedTokenPage';
import ValidateLoginPage from '../../../pages/ValidateLoginPage';
import defaultScreenOptions from './defaultScreenOptions';
import * as App from '../../actions/App';
import * as Session from '../../actions/Session';
import LogOutPreviousUserPage from '../../../pages/LogOutPreviousUserPage';
import networkPropTypes from '../../../components/networkPropTypes';
import {withNetwork} from '../../../components/OnyxProvider';

Expand Down Expand Up @@ -109,17 +107,7 @@ class AuthScreens extends React.Component {
App.getAppData(false);

App.fixAccountAndReloadData();

// Load policies, maybe creating a new policy first.
Linking.getInitialURL()
.then((url) => {
if (this.shouldCreateFreePolicy(url)) {
Policy.createAndGetPolicyList();
return;
}

Policy.getPolicyList();
});
App.setUpPoliciesAndNavigate(this.props.session);

// 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 @@ -165,25 +153,6 @@ class AuthScreens extends React.Component {
this.interval = null;
}

/**
* @param {String} [url]
* @returns {Boolean}
*/
shouldCreateFreePolicy(url = '') {
if (!url) {
return false;
}

const path = new URL(url).pathname;
const params = new URLSearchParams(url);
const exitTo = params.get('exitTo');
const email = params.get('email');
const isLoggingInAsNewUser = !_.isNull(this.props.session.email) && (email !== this.props.session.email);
return !isLoggingInAsNewUser
&& Str.startsWith(path, Str.normalizeUrl(ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN))
&& exitTo === ROUTES.WORKSPACE_NEW;
}

render() {
const commonModalScreenOptions = {
headerShown: false,
Expand Down Expand Up @@ -239,9 +208,9 @@ class AuthScreens extends React.Component {
component={ValidateLoginPage}
/>
<RootStack.Screen
name={SCREENS.LOG_IN_WITH_SHORT_LIVED_TOKEN}
name={SCREENS.TRANSITION_FROM_OLD_DOT}
options={defaultScreenOptions}
component={LogInWithShortLivedTokenPage}
component={LogOutPreviousUserPage}
/>

{/* These are the various modal routes */}
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/AppNavigator/PublicScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const PublicScreens = () => (
component={SignInPage}
/>
<RootStack.Screen
name={SCREENS.LOG_IN_WITH_SHORT_LIVED_TOKEN}
name={SCREENS.TRANSITION_FROM_OLD_DOT}
options={defaultScreenOptions}
component={LogInWithShortLivedTokenPage}
/>
Expand Down
25 changes: 25 additions & 0 deletions src/libs/Navigation/Navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import ONYXKEYS from '../../ONYXKEYS';
import linkingConfig from './linkingConfig';
import navigationRef from './navigationRef';

let resolveNavigationIsReadyPromise;
let navigationIsReadyPromise = new Promise((resolve) => {
resolveNavigationIsReadyPromise = resolve;
});

let isLoggedIn = false;
Onyx.connect({
key: ONYXKEYS.SESSION,
Expand Down Expand Up @@ -186,6 +191,23 @@ function isActiveRoute(routePath) {
return getActiveRoute().substring(1) === routePath;
}

/**
* @returns {Promise}
*/
function isNavigationReady() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment that doesn't have to be a blocker .... we only have this code because of a known bug in react-navigation. If possible, we should create a minimal reproduction and a fresh issue in the react-navigation repo, and see if we can either submit a PR to fix it or hire the maintainer to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it would help us though as the navigation becomes available and unavailable when conditionally rendering the navigators e.g. in Neil's case we are opening the app while logged in then logging out then logging in and trying to navigate. Unless the onReady() callback can be called multiple times while changing the root navigator. Not really sure what the expected behavior is there, but seems like the maintainer implied it was not a bug and suggested we stop conditionally rendering the navigators (easier said than done of course 😄 ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we conditionally render the Public and Auth screens rather than the navigators? They both use a stack navigator.

Copy link
Contributor

@roryabraham roryabraham Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we conditionally render the Public and Auth screens rather than the navigators

I looked into this a while back, while working on this exact same flow. It was ... problematic. I don't remember everything about it, but here's some context including an expo snack. Not sayin' it's impossible, only that I gave up. 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it will be a big change, but if it solves all of our "navigator is not ready" issues then it could be worth looking into... eventually!

return navigationIsReadyPromise;
}

function setIsNavigationReady() {
resolveNavigationIsReadyPromise();
}

function resetIsNavigationReady() {
navigationIsReadyPromise = new Promise((resolve) => {
resolveNavigationIsReadyPromise = resolve;
});
}

export default {
canNavigate,
navigate,
Expand All @@ -196,6 +218,9 @@ export default {
closeDrawer,
getDefaultDrawerState,
setDidTapNotification,
isNavigationReady,
setIsNavigationReady,
resetIsNavigationReady,
};

export {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default {
// Main Routes
SetPassword: ROUTES.SET_PASSWORD_WITH_VALIDATE_CODE,
ValidateLogin: ROUTES.VALIDATE_LOGIN,
[SCREENS.LOG_IN_WITH_SHORT_LIVED_TOKEN]: ROUTES.LOGIN_WITH_SHORT_LIVED_TOKEN,
[SCREENS.TRANSITION_FROM_OLD_DOT]: ROUTES.TRANSITION_FROM_OLD_DOT,

// Modal Screens
Settings: {
Expand Down
60 changes: 59 additions & 1 deletion src/libs/actions/App.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {AppState} from 'react-native';
import {AppState, Linking} from 'react-native';
import Onyx from 'react-native-onyx';
import lodashGet from 'lodash/get';
import Str from 'expensify-common/lib/str';
import ONYXKEYS from '../../ONYXKEYS';
import * as DeprecatedAPI from '../deprecatedAPI';
import CONST from '../../CONST';
Expand All @@ -15,6 +16,8 @@ import * as GeoLocation from './GeoLocation';
import * as BankAccounts from './BankAccounts';
import * as Policy from './Policy';
import NetworkConnection from '../NetworkConnection';
import Navigation from '../Navigation/Navigation';
import ROUTES from '../../ROUTES';

let currentUserAccountID;
Onyx.connect({
Expand Down Expand Up @@ -135,6 +138,60 @@ function fixAccountAndReloadData() {
});
}

/**
* This action runs every time the AuthScreens are mounted. The navigator may
* not be ready yet, and therefore we need to wait before navigating within this
* action and any actions this method calls.
*
* getInitialURL allows us to access params from the transition link more easily
* than trying to extract them from the navigation state.

* The transition link contains an exitTo param that contains the route to
* navigate to after the user is signed in. A user can transition from OldDot
* with a different account than the one they are currently signed in with, so
* we only navigate if they are not signing in as a new user. Once they are
* signed in as that new user, this action will run again and the navigation
* will occur.

* When the exitTo route is 'workspace/new', we create a new
* workspace and navigate to it via Policy.createAndGetPolicyList.
*
* We subscribe to the session using withOnyx in the AuthScreens and
* pass it in as a parameter. withOnyx guarantees that the value has been read
* from Onyx because it will not render the AuthScreens until that point.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent comments here.

* @param {Object} session
*/
function setUpPoliciesAndNavigate(session) {
Linking.getInitialURL()
.then((url) => {
if (!url) {
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
Policy.getPolicyList();
return;
}
const path = new URL(url).pathname;
const params = new URLSearchParams(url);
const exitTo = params.get('exitTo');
const email = params.get('email');
const isLoggingInAsNewUser = session.email !== email;
const shouldCreateFreePolicy = !isLoggingInAsNewUser
&& Str.startsWith(path, Str.normalizeUrl(ROUTES.TRANSITION_FROM_OLD_DOT))
&& exitTo === ROUTES.WORKSPACE_NEW;
if (shouldCreateFreePolicy) {
Policy.createAndGetPolicyList();
return;
}
Policy.getPolicyList();
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
if (!isLoggingInAsNewUser && exitTo) {
Navigation.isNavigationReady()
.then(() => {
// We must call dismissModal() to remove the /transition route from history
Navigation.dismissModal();
Navigation.navigate(exitTo);
});
}
});
}

// When the app reconnects from being offline, fetch all initialization data
NetworkConnection.onReconnect(() => getAppData(true, false));

Expand All @@ -145,4 +202,5 @@ export {
getLocale,
getAppData,
fixAccountAndReloadData,
setUpPoliciesAndNavigate,
};
1 change: 1 addition & 0 deletions src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ function createAndGetPolicyList() {
newPolicyID = policyID;
return getPolicyList();
})
.then(Navigation.isNavigationReady)
.then(() => {
Navigation.dismissModal();
navigateToPolicy(newPolicyID);
Expand Down
27 changes: 12 additions & 15 deletions src/libs/actions/Session/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function setSuccessfulSignInData(data) {
PushNotification.register(data.accountID);
Onyx.merge(ONYXKEYS.SESSION, {
shouldShowComposeInput: true,
error: null,
..._.pick(data, 'authToken', 'accountID', 'email', 'encryptedAuthToken'),
});
}
Expand Down Expand Up @@ -270,28 +271,24 @@ function signIn(password, twoFactorAuthCode) {
/**
* Uses a short lived authToken to continue a user's session from OldDot
*
* @param {String} accountID
* @param {String} email
* @param {String} shortLivedToken
* @param {String} exitTo
*/
function signInWithShortLivedToken(accountID, email, shortLivedToken) {
function signInWithShortLivedToken(email, shortLivedToken) {
Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true});

createTemporaryLogin(shortLivedToken, email).then((response) => {
Onyx.merge(ONYXKEYS.SESSION, {
accountID,
email,
});
if (response.jsonCode !== CONST.JSON_CODE.SUCCESS) {
return;
}
createTemporaryLogin(shortLivedToken, email)
.then((response) => {
if (response.jsonCode !== CONST.JSON_CODE.SUCCESS) {
return;
}

User.getUserDetails();
Onyx.merge(ONYXKEYS.ACCOUNT, {success: true});
}).finally(() => {
Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false});
});
User.getUserDetails();
Onyx.merge(ONYXKEYS.ACCOUNT, {success: true});
}).finally(() => {
Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false});
});
}

/**
Expand Down
17 changes: 9 additions & 8 deletions src/libs/actions/Welcome.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as Policy from './Policy';
import ONYXKEYS from '../../ONYXKEYS';
import NameValuePair from './NameValuePair';
import CONST from '../../CONST';
import SCREENS from '../../SCREENS';

let resolveIsReadyPromise;
let isReadyPromise = new Promise((resolve) => {
Expand Down Expand Up @@ -113,20 +114,20 @@ function show({routes, showCreateMenu}) {
// Set the NVP back to false so we don't automatically run welcome actions again
NameValuePair.set(CONST.NVP.IS_FIRST_TIME_NEW_EXPENSIFY_USER, false, ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER);

// If we are rendering the SidebarScreen at the same time as a workspace route that means we've already created a workspace via workspace/new and should not open the global
// create menu right now. We should also stay on the workspace page if that is our destination.
const topRouteName = lodashGet(_.last(routes), 'name', '');
const transitionRoute = _.find(routes, route => route.name === SCREENS.TRANSITION_FROM_OLD_DOT);
const exitingToWorkspaceRoute = lodashGet(transitionRoute, 'params.exitTo', '') === 'workspace/new';
const isDisplayingWorkspaceRoute = topRouteName.toLowerCase().includes('workspace') || exitingToWorkspaceRoute;

// We want to display the Workspace chat first since that means a user is already in a Workspace and doesn't need to create another one
const workspaceChatReport = _.find(allReports, report => ReportUtils.isPolicyExpenseChat(report) && report.ownerEmail === email);
if (workspaceChatReport) {
if (workspaceChatReport && !isDisplayingWorkspaceRoute) {
Navigation.navigate(ROUTES.getReportRoute(workspaceChatReport.reportID));
return;
}

// If we are rendering the SidebarScreen at the same time as a workspace route that means we've already created a workspace via workspace/new and should not open the global
// create menu right now.
const topRouteName = lodashGet(_.last(routes), 'name', '');
const loginWithShortLivedTokenRoute = _.find(routes, route => route.name === 'LogInWithShortLivedToken');
const exitingToWorkspaceRoute = lodashGet(loginWithShortLivedTokenRoute, 'params.exitTo', '') === 'workspace/new';
const isDisplayingWorkspaceRoute = topRouteName.toLowerCase().includes('workspace') || exitingToWorkspaceRoute;

// If user is not already an admin of a free policy and we are not navigating them to their workspace or creating a new workspace via workspace/new then
// we will show the create menu.
if (!Policy.isAdminOfFreePolicy(allPolicies) && !isDisplayingWorkspaceRoute) {
Expand Down
Loading