From db996cb4cf672184115ffac85a13c5a47073f644 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Fri, 14 Apr 2023 16:07:29 +0800 Subject: [PATCH 01/28] clean up the deep link code --- src/ONYXKEYS.js | 3 - .../DeeplinkWrapper/index.website.js | 66 ++++++++----------- src/libs/Authentication.js | 10 --- src/libs/actions/Session/index.js | 42 ++++++------ src/pages/LogInWithShortLivedAuthTokenPage.js | 28 ++++++-- 5 files changed, 72 insertions(+), 77 deletions(-) diff --git a/src/ONYXKEYS.js b/src/ONYXKEYS.js index 6fe8a0625cd8..2a453daadce8 100755 --- a/src/ONYXKEYS.js +++ b/src/ONYXKEYS.js @@ -194,7 +194,4 @@ export default { // Is app in beta version IS_BETA: 'isBeta', - - // Whether the auth token is valid - IS_TOKEN_VALID: 'isTokenValid', }; diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index c0f77ae4ead2..86470499e9de 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -7,7 +7,7 @@ import CONST from '../../CONST'; import CONFIG from '../../CONFIG'; import * as Browser from '../../libs/Browser'; import ONYXKEYS from '../../ONYXKEYS'; -import * as Authentication from '../../libs/Authentication'; +import * as Session from '../../libs/actions/Session'; const propTypes = { /** Children to render. */ @@ -21,6 +21,9 @@ const propTypes = { /** Currently logged-in user authToken */ authToken: PropTypes.string, + + /** The short-lived auth token for navigating to desktop app */ + shortLivedAuthToken: PropTypes.string, }), }; @@ -28,6 +31,7 @@ const defaultProps = { session: { email: '', authToken: '', + shortLivedAuthToken: '', }, }; @@ -36,53 +40,43 @@ class DeeplinkWrapper extends PureComponent { super(props); this.state = { - appInstallationCheckStatus: (this.isMacOSWeb() && CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV) - ? CONST.DESKTOP_DEEPLINK_APP_STATE.CHECKING : CONST.DESKTOP_DEEPLINK_APP_STATE.NOT_INSTALLED, + isShortLivedAuthTokenLoading: this.isMacOSWeb() && CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV, }; - this.focused = true; } componentDidMount() { if (!this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV) { return; } - - window.addEventListener('blur', () => { - this.focused = false; - }); - - const expensifyUrl = new URL(CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL); - const params = new URLSearchParams(); - params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); if (!this.props.session.authToken) { - const expensifyDeeplinkUrl = `${CONST.DEEPLINK_BASE_URL}${expensifyUrl.host}/transition?${params.toString()}`; - this.openRouteInDesktopApp(expensifyDeeplinkUrl); + this.openRouteInDesktopApp(); return; } - Authentication.getShortLivedAuthToken().then((shortLivedAuthToken) => { - params.set('email', this.props.session.email); - params.set('shortLivedAuthToken', `${shortLivedAuthToken}`); - const expensifyDeeplinkUrl = `${CONST.DEEPLINK_BASE_URL}${expensifyUrl.host}/transition?${params.toString()}`; - this.openRouteInDesktopApp(expensifyDeeplinkUrl); - }).catch(() => { - // If the request is successful, we call the updateAppInstallationCheckStatus before the prompt pops up. - // If not, we only need to make sure that the state will be updated. - this.updateAppInstallationCheckStatus(); - }); + Session.getShortLivedAuthToken(); } - updateAppInstallationCheckStatus() { - setTimeout(() => { - if (!this.focused) { - this.setState({appInstallationCheckStatus: CONST.DESKTOP_DEEPLINK_APP_STATE.INSTALLED}); - } else { - this.setState({appInstallationCheckStatus: CONST.DESKTOP_DEEPLINK_APP_STATE.NOT_INSTALLED}); - } - }, 500); + componentDidUpdate(prevProps) { + if (prevProps.session.shortLivedAuthToken || !this.props.session.shortLivedAuthToken) { + return; + } + this.openRouteInDesktopApp(); + Session.removeShortLivedAuthToken(); } - openRouteInDesktopApp(expensifyDeeplinkUrl) { - this.updateAppInstallationCheckStatus(); + openRouteInDesktopApp() { + this.setState({ + isShortLivedAuthTokenLoading: false, + }); + + const params = new URLSearchParams(); + params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); + const session = this.props.session; + if (session.email && session.shortLivedAuthToken) { + params.set('email', session.email); + params.set('shortLivedAuthToken', session.shortLivedAuthToken); + } + const expensifyUrl = new URL(CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL); + const expensifyDeeplinkUrl = `${CONST.DEEPLINK_BASE_URL}${expensifyUrl.host}/transition?${params.toString()}`; // This check is necessary for Safari, otherwise, if the user // does NOT have the Expensify desktop app installed, it's gonna @@ -101,7 +95,6 @@ class DeeplinkWrapper extends PureComponent { if (!iframe.parentNode) { return; } - iframe.parentNode.removeChild(iframe); }, 100); } else { @@ -119,10 +112,9 @@ class DeeplinkWrapper extends PureComponent { } render() { - if (this.state.appInstallationCheckStatus === CONST.DESKTOP_DEEPLINK_APP_STATE.CHECKING) { + if (this.state.isShortLivedAuthTokenLoading) { return ; } - return this.props.children; } } diff --git a/src/libs/Authentication.js b/src/libs/Authentication.js index 52575cf2f8b8..cd32c958eb25 100644 --- a/src/libs/Authentication.js +++ b/src/libs/Authentication.js @@ -105,17 +105,7 @@ function reauthenticate(command = '') { }); } -function getShortLivedAuthToken() { - return Network.post('OpenOldDotLink', {shouldRetry: false}).then((response) => { - if (response && response.shortLivedAuthToken) { - return Promise.resolve(response.shortLivedAuthToken); - } - return Promise.reject(); - }); -} - export { reauthenticate, Authenticate, - getShortLivedAuthToken, }; diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index b0de89d816db..a94aade60fb2 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -241,11 +241,6 @@ function signInWithShortLivedAuthToken(email, authToken) { isLoading: true, }, }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.IS_TOKEN_VALID, - value: true, - }, ]; const successData = [ @@ -266,29 +261,13 @@ function signInWithShortLivedAuthToken(email, authToken) { isLoading: false, }, }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.IS_TOKEN_VALID, - value: false, - }, ]; // If the user is signing in with a different account from the current app, should not pass the auto-generated login as it may be tied to the old account. // scene 1: the user is transitioning to newDot from a different account on oldDot. // scene 2: the user is transitioning to desktop app from a different account on web app. const oldPartnerUserID = credentials.login === email ? credentials.autoGeneratedLogin : ''; - // eslint-disable-next-line rulesdir/no-api-side-effects-method - API.makeRequestWithSideEffects( - 'SignInWithShortLivedAuthToken', - {authToken, oldPartnerUserID}, - {optimisticData, successData, failureData}, - null, - ).then((response) => { - if (response) { - return; - } - Navigation.navigate(); - }); + API.read('SignInWithShortLivedAuthToken', {authToken, oldPartnerUserID, shouldRetry: false}, {optimisticData, successData, failureData}); } /** @@ -642,6 +621,23 @@ function authenticatePusher(socketID, channelName, callback) { }); } +function getShortLivedAuthToken() { + // TODO: If we have any way to read the shortLivedAuthToken and save it, we can also no longer call makeRequestWithSideEffects. + // eslint-disable-next-line rulesdir/no-api-side-effects-method + API.makeRequestWithSideEffects( + 'OpenOldDotLink', {shouldRetry: false}, {}, + ).then((response) => { + const {shortLivedAuthToken} = response; + Onyx.merge(ONYXKEYS.SESSION, {shortLivedAuthToken}); + }); +} + +function removeShortLivedAuthToken() { + Onyx.merge(ONYXKEYS.SESSION, { + shortLivedAuthToken: '', + }); +} + export { beginSignIn, updatePasswordAndSignin, @@ -664,4 +660,6 @@ export { reauthenticatePusher, invalidateCredentials, invalidateAuthToken, + getShortLivedAuthToken, + removeShortLivedAuthToken, }; diff --git a/src/pages/LogInWithShortLivedAuthTokenPage.js b/src/pages/LogInWithShortLivedAuthTokenPage.js index 7a8b10d2c6c1..cb49c4e5dd36 100644 --- a/src/pages/LogInWithShortLivedAuthTokenPage.js +++ b/src/pages/LogInWithShortLivedAuthTokenPage.js @@ -16,6 +16,7 @@ import withLocalize, {withLocalizePropTypes} from '../components/withLocalize'; import compose from '../libs/compose'; import TextLink from '../components/TextLink'; import ONYXKEYS from '../ONYXKEYS'; +import networkPropTypes from '../components/networkPropTypes'; const propTypes = { /** The parameters needed to authenticate with a short lived token are in the URL */ @@ -35,12 +36,23 @@ const propTypes = { ...withLocalizePropTypes, - /** Whether the short lived auth token is valid */ - isTokenValid: PropTypes.bool, + /** The details about the account that the user is signing in with */ + account: PropTypes.shape({ + /** Whether a sign is loading */ + isLoading: PropTypes.bool, + }), + + /** Props to detect online status */ + network: networkPropTypes, }; const defaultProps = { - isTokenValid: true, + account: { + isLoading: false, + }, + network: { + isOffline: false, + }, }; class LogInWithShortLivedAuthTokenPage extends Component { @@ -62,7 +74,12 @@ class LogInWithShortLivedAuthTokenPage extends Component { } render() { - if (this.props.isTokenValid) { + // TODO: Not sure if we should redirect the user to the login page or just show the expiration screen or offline message + if (this.props.network.isOffline) { + Navigation.navigate(); + return; + } + if (this.props.account.isLoading) { return ; } return ( @@ -107,6 +124,7 @@ LogInWithShortLivedAuthTokenPage.defaultProps = defaultProps; export default compose( withLocalize, withOnyx({ - isTokenValid: {key: ONYXKEYS.IS_TOKEN_VALID}, + account: {key: ONYXKEYS.ACCOUNT}, + network: {key: ONYXKEYS.NETWORK}, }), )(LogInWithShortLivedAuthTokenPage); From cc48aafc235e8d1ee4190be27700184ef6c35110 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Sat, 15 Apr 2023 01:11:15 +0800 Subject: [PATCH 02/28] Add some comments to explain the deep link code --- src/components/DeeplinkWrapper/index.website.js | 9 +++++++-- src/pages/LogInWithShortLivedAuthTokenPage.js | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 86470499e9de..8a7ff5bbc8d3 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -48,6 +48,10 @@ class DeeplinkWrapper extends PureComponent { if (!this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV) { return; } + + // We need to clear the old short-lived auth token if it exists. + Session.removeShortLivedAuthToken(); + if (!this.props.session.authToken) { this.openRouteInDesktopApp(); return; @@ -59,8 +63,9 @@ class DeeplinkWrapper extends PureComponent { if (prevProps.session.shortLivedAuthToken || !this.props.session.shortLivedAuthToken) { return; } + + // Now that there is a shortLivedAuthToken, the route to the desktop app can be opened. this.openRouteInDesktopApp(); - Session.removeShortLivedAuthToken(); } openRouteInDesktopApp() { @@ -96,7 +101,7 @@ class DeeplinkWrapper extends PureComponent { return; } iframe.parentNode.removeChild(iframe); - }, 100); + }, 0); } else { window.location.href = expensifyDeeplinkUrl; } diff --git a/src/pages/LogInWithShortLivedAuthTokenPage.js b/src/pages/LogInWithShortLivedAuthTokenPage.js index cb49c4e5dd36..43339d6956b6 100644 --- a/src/pages/LogInWithShortLivedAuthTokenPage.js +++ b/src/pages/LogInWithShortLivedAuthTokenPage.js @@ -74,7 +74,7 @@ class LogInWithShortLivedAuthTokenPage extends Component { } render() { - // TODO: Not sure if we should redirect the user to the login page or just show the expiration screen or offline message + // redirect the user to the login page if there is a sudden disconnection if (this.props.network.isOffline) { Navigation.navigate(); return; From 728cc1870a3e50b32478a947d3f6931ed01ce410 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Mon, 17 Apr 2023 21:43:45 +0800 Subject: [PATCH 03/28] Update some comments to explain the deep link code --- src/components/DeeplinkWrapper/index.website.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 8a7ff5bbc8d3..fee29286e478 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -49,7 +49,9 @@ class DeeplinkWrapper extends PureComponent { return; } - // We need to clear the old short-lived auth token if it exists. + // We need to clear the old short-lived auth token if it exists, + // so that after getting a new short-lived auth token, + // we can open the popup that navigates the user to the desktop app. Session.removeShortLivedAuthToken(); if (!this.props.session.authToken) { @@ -92,10 +94,9 @@ class DeeplinkWrapper extends PureComponent { document.body.appendChild(iframe); iframe.contentWindow.location.href = expensifyDeeplinkUrl; - // Since we're creating an iframe for Safari to handle - // deeplink we need to give this iframe some time for - // it to do what it needs to do. After that we can just - // remove the iframe. + // Since we're creating an iframe for Safari to handle deeplink, + // we need to give Safari some time to open the pop-up window. + // After that we can just remove the iframe. setTimeout(() => { if (!iframe.parentNode) { return; From f1f19a737fdc916b615e305ec79223a26a54fc84 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Mon, 17 Apr 2023 23:50:08 +0800 Subject: [PATCH 04/28] Update comment for removing the short-lived auth token --- src/components/DeeplinkWrapper/index.website.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index fee29286e478..7b3a1283dbcf 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -49,9 +49,7 @@ class DeeplinkWrapper extends PureComponent { return; } - // We need to clear the old short-lived auth token if it exists, - // so that after getting a new short-lived auth token, - // we can open the popup that navigates the user to the desktop app. + // Since we open the popup based on a valid short-lived auth token, make sure its value is empty before getting a new token. Session.removeShortLivedAuthToken(); if (!this.props.session.authToken) { From 4945eb1bd60d39cee4debc0d9a98b20b6ff3a340 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Mon, 17 Apr 2023 23:51:14 +0800 Subject: [PATCH 05/28] specify the route to redirect --- src/pages/LogInWithShortLivedAuthTokenPage.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/LogInWithShortLivedAuthTokenPage.js b/src/pages/LogInWithShortLivedAuthTokenPage.js index 43339d6956b6..742e5c49de33 100644 --- a/src/pages/LogInWithShortLivedAuthTokenPage.js +++ b/src/pages/LogInWithShortLivedAuthTokenPage.js @@ -17,6 +17,7 @@ import compose from '../libs/compose'; import TextLink from '../components/TextLink'; import ONYXKEYS from '../ONYXKEYS'; import networkPropTypes from '../components/networkPropTypes'; +import ROUTES from '../ROUTES'; const propTypes = { /** The parameters needed to authenticate with a short lived token are in the URL */ @@ -76,7 +77,7 @@ class LogInWithShortLivedAuthTokenPage extends Component { render() { // redirect the user to the login page if there is a sudden disconnection if (this.props.network.isOffline) { - Navigation.navigate(); + Navigation.navigate(ROUTES.HOME); return; } if (this.props.account.isLoading) { From 6dc06df4c050f9320c43e45ec70c5c8e8d34d2ad Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Mon, 17 Apr 2023 23:56:51 +0800 Subject: [PATCH 06/28] Update comment for removing the short-lived auth token --- src/components/DeeplinkWrapper/index.website.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 7b3a1283dbcf..32d2d28f1cd4 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -49,7 +49,9 @@ class DeeplinkWrapper extends PureComponent { return; } - // Since we open the popup based on a valid short-lived auth token, make sure its value is empty before getting a new token. + // Since there is no way to know if a previous short-lived authToken is still valid, + // any previous short-lived authToken must be cleared out and a new one must be fetched + // so that the popup window will only open when we know the short-lived authToken is valid. Session.removeShortLivedAuthToken(); if (!this.props.session.authToken) { From cff96430f6d20baf0595c7e76c6fcea8c649042d Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 19 Apr 2023 21:43:19 +0800 Subject: [PATCH 07/28] makeRequestWithSideEffects can be removed in the PR 17364 --- src/libs/actions/Session/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index a94aade60fb2..b1dab275ce8c 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -622,7 +622,6 @@ function authenticatePusher(socketID, channelName, callback) { } function getShortLivedAuthToken() { - // TODO: If we have any way to read the shortLivedAuthToken and save it, we can also no longer call makeRequestWithSideEffects. // eslint-disable-next-line rulesdir/no-api-side-effects-method API.makeRequestWithSideEffects( 'OpenOldDotLink', {shouldRetry: false}, {}, From 3c73416da42374ecd33e7b82ba6ae9f99c993dec Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Tue, 25 Apr 2023 23:07:43 +0800 Subject: [PATCH 08/28] rename TRANSITION_FROM_OLD_DOT to TRANSITION --- src/ROUTES.js | 2 +- src/SCREENS.js | 2 +- .../DeeplinkWrapper/index.website.js | 27 +++++++++---------- .../Navigation/AppNavigator/AuthScreens.js | 2 +- .../Navigation/AppNavigator/PublicScreens.js | 2 +- src/libs/Navigation/linkingConfig.js | 2 +- src/libs/actions/App.js | 9 ++++--- src/libs/actions/Welcome.js | 2 +- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/ROUTES.js b/src/ROUTES.js index 470768a594ac..20c477a82731 100644 --- a/src/ROUTES.js +++ b/src/ROUTES.js @@ -103,7 +103,7 @@ export default { getReportDetailsRoute: reportID => `r/${reportID}/details`, REPORT_SETTINGS: 'r/:reportID/settings', getReportSettingsRoute: reportID => `r/${reportID}/settings`, - TRANSITION_FROM_OLD_DOT: 'transition', + TRANSITION: 'transition', VALIDATE_LOGIN: 'v/:accountID/:validateCode', GET_ASSISTANCE: 'get-assistance/:taskID', getGetAssistanceRoute: taskID => `get-assistance/${taskID}`, diff --git a/src/SCREENS.js b/src/SCREENS.js index 24ea27fe9689..159d126d7888 100644 --- a/src/SCREENS.js +++ b/src/SCREENS.js @@ -7,5 +7,5 @@ export default { LOADING: 'Loading', REPORT: 'Report', NOT_FOUND: 'not-found', - TRANSITION_FROM_OLD_DOT: 'TransitionFromOldDot', + TRANSITION: 'Transition', }; diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 32d2d28f1cd4..5a9086ef3540 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -1,13 +1,13 @@ import PropTypes from 'prop-types'; -import React, {PureComponent} from 'react'; +import {PureComponent} from 'react'; import {withOnyx} from 'react-native-onyx'; -import FullScreenLoadingIndicator from '../FullscreenLoadingIndicator'; -import styles from '../../styles/styles'; +import Str from 'expensify-common/lib/str'; import CONST from '../../CONST'; import CONFIG from '../../CONFIG'; import * as Browser from '../../libs/Browser'; import ONYXKEYS from '../../ONYXKEYS'; import * as Session from '../../libs/actions/Session'; +import ROUTES from '../../ROUTES'; const propTypes = { /** Children to render. */ @@ -38,14 +38,11 @@ const defaultProps = { class DeeplinkWrapper extends PureComponent { constructor(props) { super(props); - - this.state = { - isShortLivedAuthTokenLoading: this.isMacOSWeb() && CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV, - }; + this.shouldNotShowPopup = !this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV; } componentDidMount() { - if (!this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV) { + if (this.shouldNotShowPopup) { return; } @@ -54,6 +51,11 @@ class DeeplinkWrapper extends PureComponent { // so that the popup window will only open when we know the short-lived authToken is valid. Session.removeShortLivedAuthToken(); + // If the current link is transition from oldDot, we should wait for the web navigation to complete + const isTransitioning = Str.startsWith(window.location.path, Str.normalizeUrl(ROUTES.TRANSITION)); + if (isTransitioning) { + return; + } if (!this.props.session.authToken) { this.openRouteInDesktopApp(); return; @@ -71,9 +73,9 @@ class DeeplinkWrapper extends PureComponent { } openRouteInDesktopApp() { - this.setState({ - isShortLivedAuthTokenLoading: false, - }); + if (this.shouldNotShowPopup) { + return; + } const params = new URLSearchParams(); params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); @@ -118,9 +120,6 @@ class DeeplinkWrapper extends PureComponent { } render() { - if (this.state.isShortLivedAuthTokenLoading) { - return ; - } return this.props.children; } } diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 776508f27864..a325eed5256f 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -213,7 +213,7 @@ class AuthScreens extends React.Component { }} /> { const LogOutPreviousUserPage = require('../../../pages/LogOutPreviousUserPage').default; diff --git a/src/libs/Navigation/AppNavigator/PublicScreens.js b/src/libs/Navigation/AppNavigator/PublicScreens.js index 855efccad709..531c38b37708 100644 --- a/src/libs/Navigation/AppNavigator/PublicScreens.js +++ b/src/libs/Navigation/AppNavigator/PublicScreens.js @@ -17,7 +17,7 @@ const PublicScreens = () => ( component={SignInPage} /> diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index 76e00c675852..464d6ea603ce 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -27,7 +27,7 @@ export default { // Main Routes SetPassword: ROUTES.SET_PASSWORD_WITH_VALIDATE_CODE, ValidateLogin: ROUTES.VALIDATE_LOGIN, - [SCREENS.TRANSITION_FROM_OLD_DOT]: ROUTES.TRANSITION_FROM_OLD_DOT, + [SCREENS.TRANSITION]: ROUTES.TRANSITION, Concierge: ROUTES.CONCIERGE, // Modal Screens diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 30c825f40f90..8f608736ed29 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -220,14 +220,14 @@ function setUpPoliciesAndNavigate(session) { const makeMeAdmin = url.searchParams.get('makeMeAdmin'); const policyName = url.searchParams.get('policyName'); - // Sign out the current user if we're transitioning from oldDot with a different user - const isTransitioningFromOldDot = Str.startsWith(url.pathname, Str.normalizeUrl(ROUTES.TRANSITION_FROM_OLD_DOT)); - if (isLoggingInAsNewUser && isTransitioningFromOldDot) { + // Sign out the current user if we're transitioning with a different user + const isTransitioning = Str.startsWith(url.pathname, Str.normalizeUrl(ROUTES.TRANSITION)); + if (isLoggingInAsNewUser && isTransitioning) { Session.signOut(); } const shouldCreateFreePolicy = !isLoggingInAsNewUser - && isTransitioningFromOldDot + && isTransitioning && exitTo === ROUTES.WORKSPACE_NEW; if (shouldCreateFreePolicy) { Policy.createWorkspace(ownerEmail, makeMeAdmin, policyName, true); @@ -244,6 +244,7 @@ function setUpPoliciesAndNavigate(session) { // We must call dismissModal() to remove the /transition route from history Navigation.dismissModal(); Navigation.navigate(exitTo); + Session.getShortLivedAuthToken(); }); }); } diff --git a/src/libs/actions/Welcome.js b/src/libs/actions/Welcome.js index 63bdc9411572..ecdb95f572f6 100644 --- a/src/libs/actions/Welcome.js +++ b/src/libs/actions/Welcome.js @@ -108,7 +108,7 @@ function show({routes, showCreateMenu}) { // create menu right now. We should also stay on the workspace page if that is our destination. const topRoute = _.last(routes); const isWorkspaceRoute = topRoute.name === 'Settings' && topRoute.params.path.includes('workspace'); - const transitionRoute = _.find(routes, route => route.name === SCREENS.TRANSITION_FROM_OLD_DOT); + const transitionRoute = _.find(routes, route => route.name === SCREENS.TRANSITION); const exitingToWorkspaceRoute = lodashGet(transitionRoute, 'params.exitTo', '') === 'workspace/new'; const isDisplayingWorkspaceRoute = isWorkspaceRoute || exitingToWorkspaceRoute; From a12900931c8694ceaf153dc3f7014fa256a43218 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 26 Apr 2023 06:54:11 +0800 Subject: [PATCH 09/28] If the current link is transition from oldDot, we should wait for it to finish --- src/components/DeeplinkWrapper/index.website.js | 5 +++-- src/libs/Navigation/Navigation.js | 15 +++++++++++++++ src/libs/actions/App.js | 2 +- src/libs/actions/Policy.js | 1 + src/libs/actions/Session/index.js | 7 +++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 5a9086ef3540..b33d3566f0ab 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -51,9 +51,10 @@ class DeeplinkWrapper extends PureComponent { // so that the popup window will only open when we know the short-lived authToken is valid. Session.removeShortLivedAuthToken(); - // If the current link is transition from oldDot, we should wait for the web navigation to complete - const isTransitioning = Str.startsWith(window.location.path, Str.normalizeUrl(ROUTES.TRANSITION)); + // If the current link is transition from oldDot, we should wait for it to finish, then we will know the final route. + const isTransitioning = Str.startsWith(window.location.pathname, Str.normalizeUrl(ROUTES.TRANSITION)); if (isTransitioning) { + Session.getShortLivedAuthTokenAfterTransition(); return; } if (!this.props.session.authToken) { diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index 08577b3f5d57..eef1fa2d41e9 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -29,6 +29,11 @@ let reportScreenIsReadyPromise = new Promise((resolve) => { resolveReportScreenIsReadyPromise = resolve; }); +let resolveTransitionIsEndPromise; +const transitionIsEndPromise = new Promise((resolve) => { + resolveTransitionIsEndPromise = resolve; +}); + let isLoggedIn = false; let pendingRoute = null; @@ -313,6 +318,14 @@ function drawerGoBack(backRoute) { navigate(backRoute); } +function isTransitionEnd() { + return transitionIsEndPromise; +} + +function setIsTransitionEnd() { + return resolveTransitionIsEndPromise(); +} + export default { canNavigate, navigate, @@ -335,6 +348,8 @@ export default { isReportScreenReady, setIsReportScreenIsReady, drawerGoBack, + isTransitionEnd, + setIsTransitionEnd, }; export { diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 8f608736ed29..f1178ce0dbb5 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -244,7 +244,7 @@ function setUpPoliciesAndNavigate(session) { // We must call dismissModal() to remove the /transition route from history Navigation.dismissModal(); Navigation.navigate(exitTo); - Session.getShortLivedAuthToken(); + Navigation.setIsTransitionEnd(); }); }); } diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index f090af06be1f..a99bb0a5b4d9 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -958,6 +958,7 @@ function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '', Navigation.dismissModal(); // Dismiss /transition route for OldDot to NewDot transitions } Navigation.navigate(ROUTES.getWorkspaceInitialRoute(policyID)); + Navigation.setIsTransitionEnd(); }); } diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index b1dab275ce8c..c5ba803019ba 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -637,6 +637,12 @@ function removeShortLivedAuthToken() { }); } +function getShortLivedAuthTokenAfterTransition() { + Navigation.isTransitionEnd().then(() => { + getShortLivedAuthToken(); + }); +} + export { beginSignIn, updatePasswordAndSignin, @@ -661,4 +667,5 @@ export { invalidateAuthToken, getShortLivedAuthToken, removeShortLivedAuthToken, + getShortLivedAuthTokenAfterTransition, }; From cb5f434962838dd71bdab61a0b4fe1f2136a2fc3 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 26 Apr 2023 16:34:22 +0800 Subject: [PATCH 10/28] prevent the popup from appearing repeatedly --- src/components/DeeplinkWrapper/index.website.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index b33d3566f0ab..6769c50feb58 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -78,6 +78,10 @@ class DeeplinkWrapper extends PureComponent { return; } + // Once the popup has appeared after the page is loaded, it does not need to be displayed again. + // e.g. When a user refresh one tab, the popup should not appear again in other tabs. + this.shouldNotShowPopup = true; + const params = new URLSearchParams(); params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); const session = this.props.session; From 1f252d67efe78b9b3257db2bd4eabc367070a9af Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 26 Apr 2023 23:23:41 +0800 Subject: [PATCH 11/28] rename TRANSITION to TRANSITION_BETWEEN_APPS --- src/ROUTES.js | 2 +- src/SCREENS.js | 2 +- src/libs/Navigation/AppNavigator/AuthScreens.js | 2 +- src/libs/Navigation/AppNavigator/PublicScreens.js | 2 +- src/libs/Navigation/linkingConfig.js | 2 +- src/libs/actions/App.js | 2 +- src/libs/actions/Welcome.js | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ROUTES.js b/src/ROUTES.js index 20c477a82731..67ba78d1aa51 100644 --- a/src/ROUTES.js +++ b/src/ROUTES.js @@ -103,7 +103,7 @@ export default { getReportDetailsRoute: reportID => `r/${reportID}/details`, REPORT_SETTINGS: 'r/:reportID/settings', getReportSettingsRoute: reportID => `r/${reportID}/settings`, - TRANSITION: 'transition', + TRANSITION_BETWEEN_APPS: 'transition', VALIDATE_LOGIN: 'v/:accountID/:validateCode', GET_ASSISTANCE: 'get-assistance/:taskID', getGetAssistanceRoute: taskID => `get-assistance/${taskID}`, diff --git a/src/SCREENS.js b/src/SCREENS.js index 159d126d7888..550368a7fb0a 100644 --- a/src/SCREENS.js +++ b/src/SCREENS.js @@ -7,5 +7,5 @@ export default { LOADING: 'Loading', REPORT: 'Report', NOT_FOUND: 'not-found', - TRANSITION: 'Transition', + TRANSITION_BETWEEN_APPS: 'TransitionBetweenApps', }; diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index a325eed5256f..f57d911fd927 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -213,7 +213,7 @@ class AuthScreens extends React.Component { }} /> { const LogOutPreviousUserPage = require('../../../pages/LogOutPreviousUserPage').default; diff --git a/src/libs/Navigation/AppNavigator/PublicScreens.js b/src/libs/Navigation/AppNavigator/PublicScreens.js index 531c38b37708..7db1e8f7b180 100644 --- a/src/libs/Navigation/AppNavigator/PublicScreens.js +++ b/src/libs/Navigation/AppNavigator/PublicScreens.js @@ -17,7 +17,7 @@ const PublicScreens = () => ( component={SignInPage} /> diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index 464d6ea603ce..a2cd68b08f0e 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -27,7 +27,7 @@ export default { // Main Routes SetPassword: ROUTES.SET_PASSWORD_WITH_VALIDATE_CODE, ValidateLogin: ROUTES.VALIDATE_LOGIN, - [SCREENS.TRANSITION]: ROUTES.TRANSITION, + [SCREENS.TRANSITION_BETWEEN_APPS]: ROUTES.TRANSITION_BETWEEN_APPS, Concierge: ROUTES.CONCIERGE, // Modal Screens diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index f1178ce0dbb5..cb99fe0f828d 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -221,7 +221,7 @@ function setUpPoliciesAndNavigate(session) { const policyName = url.searchParams.get('policyName'); // Sign out the current user if we're transitioning with a different user - const isTransitioning = Str.startsWith(url.pathname, Str.normalizeUrl(ROUTES.TRANSITION)); + const isTransitioning = Str.startsWith(url.pathname, Str.normalizeUrl(ROUTES.TRANSITION_BETWEEN_APPS)); if (isLoggingInAsNewUser && isTransitioning) { Session.signOut(); } diff --git a/src/libs/actions/Welcome.js b/src/libs/actions/Welcome.js index ecdb95f572f6..33124eff511c 100644 --- a/src/libs/actions/Welcome.js +++ b/src/libs/actions/Welcome.js @@ -108,7 +108,7 @@ function show({routes, showCreateMenu}) { // create menu right now. We should also stay on the workspace page if that is our destination. const topRoute = _.last(routes); const isWorkspaceRoute = topRoute.name === 'Settings' && topRoute.params.path.includes('workspace'); - const transitionRoute = _.find(routes, route => route.name === SCREENS.TRANSITION); + const transitionRoute = _.find(routes, route => route.name === SCREENS.TRANSITION_BETWEEN_APPS); const exitingToWorkspaceRoute = lodashGet(transitionRoute, 'params.exitTo', '') === 'workspace/new'; const isDisplayingWorkspaceRoute = isWorkspaceRoute || exitingToWorkspaceRoute; From 5f7193707b3f7212d2525f02d412ffe6b7449866 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 26 Apr 2023 23:23:59 +0800 Subject: [PATCH 12/28] rename variable and explain the reason for the code. --- src/components/DeeplinkWrapper/index.website.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 6769c50feb58..d7aa336e0d39 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -38,11 +38,11 @@ const defaultProps = { class DeeplinkWrapper extends PureComponent { constructor(props) { super(props); - this.shouldNotShowPopup = !this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV; + this.hasPopupBeenOpenedBefore = !this.isMacOSWeb() || CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV; } componentDidMount() { - if (this.shouldNotShowPopup) { + if (this.hasPopupBeenOpenedBefore) { return; } @@ -51,9 +51,9 @@ class DeeplinkWrapper extends PureComponent { // so that the popup window will only open when we know the short-lived authToken is valid. Session.removeShortLivedAuthToken(); - // If the current link is transition from oldDot, we should wait for it to finish, then we will know the final route. - const isTransitioning = Str.startsWith(window.location.pathname, Str.normalizeUrl(ROUTES.TRANSITION)); - if (isTransitioning) { + // If the current link is transition from oldDot, we should wait for it to finish, + // because we should pass the final path to the desktop app instead of the transition path. + if (Str.startsWith(window.location.pathname, Str.normalizeUrl(ROUTES.TRANSITION_BETWEEN_APPS))) { Session.getShortLivedAuthTokenAfterTransition(); return; } @@ -74,13 +74,13 @@ class DeeplinkWrapper extends PureComponent { } openRouteInDesktopApp() { - if (this.shouldNotShowPopup) { + if (this.hasPopupBeenOpenedBefore) { return; } // Once the popup has appeared after the page is loaded, it does not need to be displayed again. // e.g. When a user refresh one tab, the popup should not appear again in other tabs. - this.shouldNotShowPopup = true; + this.hasPopupBeenOpenedBefore = true; const params = new URLSearchParams(); params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); From a73760026ada268563f485864b1d24fa18aa64ad Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 26 Apr 2023 23:29:05 +0800 Subject: [PATCH 13/28] correct the condition of variable hasPopupBeenOpenedBefore --- src/components/DeeplinkWrapper/index.website.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index d7aa336e0d39..d752886e8b15 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -38,7 +38,7 @@ const defaultProps = { class DeeplinkWrapper extends PureComponent { constructor(props) { super(props); - this.hasPopupBeenOpenedBefore = !this.isMacOSWeb() || CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV; + this.hasPopupBeenOpenedBefore = !this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV; } componentDidMount() { From c7b323dec7858b8eeb56d9fac29c48b811409681 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Fri, 28 Apr 2023 11:54:42 +0800 Subject: [PATCH 14/28] Refactor the deeplink logic: clean up the DeepLinkWrapper/index.website.js move openRouteInDesktopApp from DeepLinkWrapper to Browser lib. rename getShortLivedAuthToken/getShortLivedAuthTokenAfterTransition to beginDeepLinkRedirect/beginDeepLinkRedirectAfterTransition and move them to App.js --- .../DeeplinkWrapper/index.website.js | 110 ++---------------- src/libs/Browser/index.js | 5 + src/libs/Browser/index.web.js | 35 ++++++ src/libs/Navigation/Navigation.js | 8 ++ src/libs/actions/App.js | 23 ++++ src/libs/actions/Session/index.js | 27 +---- 6 files changed, 82 insertions(+), 126 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index d752886e8b15..9e7ca2c491a6 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -1,118 +1,31 @@ import PropTypes from 'prop-types'; import {PureComponent} from 'react'; -import {withOnyx} from 'react-native-onyx'; import Str from 'expensify-common/lib/str'; -import CONST from '../../CONST'; -import CONFIG from '../../CONFIG'; import * as Browser from '../../libs/Browser'; -import ONYXKEYS from '../../ONYXKEYS'; -import * as Session from '../../libs/actions/Session'; import ROUTES from '../../ROUTES'; +import * as App from '../../libs/actions/App'; +import CONST from '../../CONST'; +import CONFIG from '../../CONFIG'; const propTypes = { /** Children to render. */ children: PropTypes.node.isRequired, - - /** Session info for the currently logged-in user. */ - session: PropTypes.shape({ - - /** Currently logged-in user email */ - email: PropTypes.string, - - /** Currently logged-in user authToken */ - authToken: PropTypes.string, - - /** The short-lived auth token for navigating to desktop app */ - shortLivedAuthToken: PropTypes.string, - }), -}; - -const defaultProps = { - session: { - email: '', - authToken: '', - shortLivedAuthToken: '', - }, }; class DeeplinkWrapper extends PureComponent { - constructor(props) { - super(props); - this.hasPopupBeenOpenedBefore = !this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV; - } - componentDidMount() { - if (this.hasPopupBeenOpenedBefore) { + if (!this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV) { return; } - // Since there is no way to know if a previous short-lived authToken is still valid, - // any previous short-lived authToken must be cleared out and a new one must be fetched - // so that the popup window will only open when we know the short-lived authToken is valid. - Session.removeShortLivedAuthToken(); - - // If the current link is transition from oldDot, we should wait for it to finish, - // because we should pass the final path to the desktop app instead of the transition path. + // If the current page is opened from oldDot, there maybe non-idempotent operations (e.g. create a new workspace) during the transition period, + // which obviously should not be executed again in the desktop app. + // We only need to begin the deeplink redirect after sign-in and navigation are completed. if (Str.startsWith(window.location.pathname, Str.normalizeUrl(ROUTES.TRANSITION_BETWEEN_APPS))) { - Session.getShortLivedAuthTokenAfterTransition(); - return; - } - if (!this.props.session.authToken) { - this.openRouteInDesktopApp(); + App.beginDeepLinkRedirectAfterTransition(); return; } - Session.getShortLivedAuthToken(); - } - - componentDidUpdate(prevProps) { - if (prevProps.session.shortLivedAuthToken || !this.props.session.shortLivedAuthToken) { - return; - } - - // Now that there is a shortLivedAuthToken, the route to the desktop app can be opened. - this.openRouteInDesktopApp(); - } - - openRouteInDesktopApp() { - if (this.hasPopupBeenOpenedBefore) { - return; - } - - // Once the popup has appeared after the page is loaded, it does not need to be displayed again. - // e.g. When a user refresh one tab, the popup should not appear again in other tabs. - this.hasPopupBeenOpenedBefore = true; - - const params = new URLSearchParams(); - params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); - const session = this.props.session; - if (session.email && session.shortLivedAuthToken) { - params.set('email', session.email); - params.set('shortLivedAuthToken', session.shortLivedAuthToken); - } - const expensifyUrl = new URL(CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL); - const expensifyDeeplinkUrl = `${CONST.DEEPLINK_BASE_URL}${expensifyUrl.host}/transition?${params.toString()}`; - - // This check is necessary for Safari, otherwise, if the user - // does NOT have the Expensify desktop app installed, it's gonna - // show an error in the page saying that the address is invalid - if (CONST.BROWSER.SAFARI === Browser.getBrowser()) { - const iframe = document.createElement('iframe'); - iframe.style.display = 'none'; - document.body.appendChild(iframe); - iframe.contentWindow.location.href = expensifyDeeplinkUrl; - - // Since we're creating an iframe for Safari to handle deeplink, - // we need to give Safari some time to open the pop-up window. - // After that we can just remove the iframe. - setTimeout(() => { - if (!iframe.parentNode) { - return; - } - iframe.parentNode.removeChild(iframe); - }, 0); - } else { - window.location.href = expensifyDeeplinkUrl; - } + App.beginDeepLinkRedirect(); } isMacOSWeb() { @@ -130,7 +43,4 @@ class DeeplinkWrapper extends PureComponent { } DeeplinkWrapper.propTypes = propTypes; -DeeplinkWrapper.defaultProps = defaultProps; -export default withOnyx({ - session: {key: ONYXKEYS.SESSION}, -})(DeeplinkWrapper); +export default DeeplinkWrapper; diff --git a/src/libs/Browser/index.js b/src/libs/Browser/index.js index dbc21c9a1f3f..2cbf71e6c45b 100644 --- a/src/libs/Browser/index.js +++ b/src/libs/Browser/index.js @@ -10,8 +10,13 @@ function isMobileSafari() { return false; } +function openRouteInDesktopApp() { + +} + export { getBrowser, isMobile, isMobileSafari, + openRouteInDesktopApp, }; diff --git a/src/libs/Browser/index.web.js b/src/libs/Browser/index.web.js index e7eb1db26d26..62a9ecef83af 100644 --- a/src/libs/Browser/index.web.js +++ b/src/libs/Browser/index.web.js @@ -1,4 +1,5 @@ import CONST from '../../CONST'; +import CONFIG from '../../CONFIG'; /** * Fetch browser name from UA string @@ -50,8 +51,42 @@ function isMobileSafari() { return /iP(ad|od|hone)/i.test(userAgent) && /WebKit/i.test(userAgent) && !(/(CriOS|FxiOS|OPiOS|mercury)/i.test(userAgent)); } +function openRouteInDesktopApp(shortLivedAuthToken = '', email = '') { + const params = new URLSearchParams(); + params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); + if (email && shortLivedAuthToken) { + params.set('email', email); + params.set('shortLivedAuthToken', shortLivedAuthToken); + } + const expensifyUrl = new URL(CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL); + const expensifyDeeplinkUrl = `${CONST.DEEPLINK_BASE_URL}${expensifyUrl.host}/transition?${params.toString()}`; + + // This check is necessary for Safari, otherwise, if the user + // does NOT have the Expensify desktop app installed, it's gonna + // show an error in the page saying that the address is invalid + if (CONST.BROWSER.SAFARI === getBrowser()) { + const iframe = document.createElement('iframe'); + iframe.style.display = 'none'; + document.body.appendChild(iframe); + iframe.contentWindow.location.href = expensifyDeeplinkUrl; + + // Since we're creating an iframe for Safari to handle deeplink, + // we need to give Safari some time to open the pop-up window. + // After that we can just remove the iframe. + setTimeout(() => { + if (!iframe.parentNode) { + return; + } + iframe.parentNode.removeChild(iframe); + }, 0); + } else { + window.location.href = expensifyDeeplinkUrl; + } +} + export { getBrowser, isMobile, isMobileSafari, + openRouteInDesktopApp, }; diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index eef1fa2d41e9..50aab775196e 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -318,10 +318,18 @@ function drawerGoBack(backRoute) { navigate(backRoute); } +/** + * If the current page is opened from oldDot, we can use this to delay the task until sign-in and navigation end. + * @return {Promise} + */ function isTransitionEnd() { return transitionIsEndPromise; } +/** + * Perform a task after sign-in and navigation, e.g. begin the deeplink redirection + * @return {*} + */ function setIsTransitionEnd() { return resolveTransitionIsEndPromise(); } diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index cb99fe0f828d..d669e8a5704c 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -15,6 +15,7 @@ import ROUTES from '../../ROUTES'; import * as SessionUtils from '../SessionUtils'; import getCurrentUrl from '../Navigation/currentUrl'; import * as Session from './Session'; +import * as Browser from '../Browser'; let currentUserAccountID; let currentUserEmail = ''; @@ -287,6 +288,26 @@ function openProfile() { Navigation.navigate(ROUTES.SETTINGS_PROFILE); } +function beginDeepLinkRedirect() { + if (!currentUserAccountID) { + Browser.openRouteInDesktopApp(); + return; + } + + // eslint-disable-next-line rulesdir/no-api-side-effects-method + API.makeRequestWithSideEffects( + 'OpenOldDotLink', {shouldRetry: false}, {}, + ).then((response) => { + Browser.openRouteInDesktopApp(response.shortLivedAuthToken, currentUserEmail); + }); +} + +function beginDeepLinkRedirectAfterTransition() { + Navigation.isTransitionEnd().then(() => { + beginDeepLinkRedirect(); + }); +} + export { setLocale, setLocaleAndNavigate, @@ -295,4 +316,6 @@ export { openProfile, openApp, reconnectApp, + beginDeepLinkRedirect, + beginDeepLinkRedirectAfterTransition, }; diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index c5ba803019ba..0297b5b3a4fd 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -267,7 +267,7 @@ function signInWithShortLivedAuthToken(email, authToken) { // scene 1: the user is transitioning to newDot from a different account on oldDot. // scene 2: the user is transitioning to desktop app from a different account on web app. const oldPartnerUserID = credentials.login === email ? credentials.autoGeneratedLogin : ''; - API.read('SignInWithShortLivedAuthToken', {authToken, oldPartnerUserID, shouldRetry: false}, {optimisticData, successData, failureData}); + API.read('SignInWithShortLivedAuthToken', {authToken, oldPartnerUserID}, {optimisticData, successData, failureData}); } /** @@ -621,28 +621,6 @@ function authenticatePusher(socketID, channelName, callback) { }); } -function getShortLivedAuthToken() { - // eslint-disable-next-line rulesdir/no-api-side-effects-method - API.makeRequestWithSideEffects( - 'OpenOldDotLink', {shouldRetry: false}, {}, - ).then((response) => { - const {shortLivedAuthToken} = response; - Onyx.merge(ONYXKEYS.SESSION, {shortLivedAuthToken}); - }); -} - -function removeShortLivedAuthToken() { - Onyx.merge(ONYXKEYS.SESSION, { - shortLivedAuthToken: '', - }); -} - -function getShortLivedAuthTokenAfterTransition() { - Navigation.isTransitionEnd().then(() => { - getShortLivedAuthToken(); - }); -} - export { beginSignIn, updatePasswordAndSignin, @@ -665,7 +643,4 @@ export { reauthenticatePusher, invalidateCredentials, invalidateAuthToken, - getShortLivedAuthToken, - removeShortLivedAuthToken, - getShortLivedAuthTokenAfterTransition, }; From c5ee14089e7c711fb3f65de30617b158cfc7da67 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 3 May 2023 00:26:24 +0800 Subject: [PATCH 15/28] try to improve the transition logic. --- .../DeeplinkWrapper/index.website.js | 7 ++--- src/libs/Browser/index.web.js | 5 +--- src/libs/Navigation/Navigation.js | 23 ---------------- src/libs/actions/App.js | 26 ++++++++++++++----- src/libs/actions/Policy.js | 3 +-- src/pages/LogInWithShortLivedAuthTokenPage.js | 5 +--- 6 files changed, 27 insertions(+), 42 deletions(-) diff --git a/src/components/DeeplinkWrapper/index.website.js b/src/components/DeeplinkWrapper/index.website.js index 9e7ca2c491a6..b60348435411 100644 --- a/src/components/DeeplinkWrapper/index.website.js +++ b/src/components/DeeplinkWrapper/index.website.js @@ -18,9 +18,10 @@ class DeeplinkWrapper extends PureComponent { return; } - // If the current page is opened from oldDot, there maybe non-idempotent operations (e.g. create a new workspace) during the transition period, - // which obviously should not be executed again in the desktop app. - // We only need to begin the deeplink redirect after sign-in and navigation are completed. + // If the current url path is /transition..., meaning it was opened from oldDot, during this transition period: + // 1. The user session may not exist, because sign-in has not been completed yet. + // 2. There may be non-idempotent operations (e.g. create a new workspace), which obviously should not be executed again in the desktop app. + // So we need to wait until after sign-in and navigation are complete before starting the deeplink redirect. if (Str.startsWith(window.location.pathname, Str.normalizeUrl(ROUTES.TRANSITION_BETWEEN_APPS))) { App.beginDeepLinkRedirectAfterTransition(); return; diff --git a/src/libs/Browser/index.web.js b/src/libs/Browser/index.web.js index 62a9ecef83af..7729e71c5004 100644 --- a/src/libs/Browser/index.web.js +++ b/src/libs/Browser/index.web.js @@ -74,10 +74,7 @@ function openRouteInDesktopApp(shortLivedAuthToken = '', email = '') { // we need to give Safari some time to open the pop-up window. // After that we can just remove the iframe. setTimeout(() => { - if (!iframe.parentNode) { - return; - } - iframe.parentNode.removeChild(iframe); + document.body.removeChild(iframe); }, 0); } else { window.location.href = expensifyDeeplinkUrl; diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index 50aab775196e..08577b3f5d57 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -29,11 +29,6 @@ let reportScreenIsReadyPromise = new Promise((resolve) => { resolveReportScreenIsReadyPromise = resolve; }); -let resolveTransitionIsEndPromise; -const transitionIsEndPromise = new Promise((resolve) => { - resolveTransitionIsEndPromise = resolve; -}); - let isLoggedIn = false; let pendingRoute = null; @@ -318,22 +313,6 @@ function drawerGoBack(backRoute) { navigate(backRoute); } -/** - * If the current page is opened from oldDot, we can use this to delay the task until sign-in and navigation end. - * @return {Promise} - */ -function isTransitionEnd() { - return transitionIsEndPromise; -} - -/** - * Perform a task after sign-in and navigation, e.g. begin the deeplink redirection - * @return {*} - */ -function setIsTransitionEnd() { - return resolveTransitionIsEndPromise(); -} - export default { canNavigate, navigate, @@ -356,8 +335,6 @@ export default { isReportScreenReady, setIsReportScreenIsReady, drawerGoBack, - isTransitionEnd, - setIsTransitionEnd, }; export { diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index d669e8a5704c..643705faf92b 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -185,6 +185,23 @@ function reconnectApp() { }); } +/** + * This promise is used so that deeplink component know when a transition is end. + * This is necessary because we want to begin deeplink redirection after the transition is end. + */ +let resolveTransitionIsEndPromise; +const transitionIsEndPromise = new Promise((resolve) => { + resolveTransitionIsEndPromise = resolve; +}); + +function waitForTransitionEnd() { + return transitionIsEndPromise; +} + +function endTransition() { + return resolveTransitionIsEndPromise(); +} + /** * This action runs when the Navigator is ready and the current route changes * @@ -231,7 +248,7 @@ function setUpPoliciesAndNavigate(session) { && isTransitioning && exitTo === ROUTES.WORKSPACE_NEW; if (shouldCreateFreePolicy) { - Policy.createWorkspace(ownerEmail, makeMeAdmin, policyName, true); + Policy.createWorkspace(ownerEmail, makeMeAdmin, policyName, true).then(endTransition); return; } if (!isLoggingInAsNewUser && exitTo) { @@ -245,8 +262,7 @@ function setUpPoliciesAndNavigate(session) { // We must call dismissModal() to remove the /transition route from history Navigation.dismissModal(); Navigation.navigate(exitTo); - Navigation.setIsTransitionEnd(); - }); + }).then(endTransition); }); } } @@ -303,9 +319,7 @@ function beginDeepLinkRedirect() { } function beginDeepLinkRedirectAfterTransition() { - Navigation.isTransitionEnd().then(() => { - beginDeepLinkRedirect(); - }); + waitForTransitionEnd().then(beginDeepLinkRedirect); } export { diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index a99bb0a5b4d9..136bc1a12817 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -952,13 +952,12 @@ function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '', ], }); - Navigation.isNavigationReady() + return Navigation.isNavigationReady() .then(() => { if (transitionFromOldDot) { Navigation.dismissModal(); // Dismiss /transition route for OldDot to NewDot transitions } Navigation.navigate(ROUTES.getWorkspaceInitialRoute(policyID)); - Navigation.setIsTransitionEnd(); }); } diff --git a/src/pages/LogInWithShortLivedAuthTokenPage.js b/src/pages/LogInWithShortLivedAuthTokenPage.js index 742e5c49de33..540497bbeae2 100644 --- a/src/pages/LogInWithShortLivedAuthTokenPage.js +++ b/src/pages/LogInWithShortLivedAuthTokenPage.js @@ -44,16 +44,13 @@ const propTypes = { }), /** Props to detect online status */ - network: networkPropTypes, + network: networkPropTypes.isRequired, }; const defaultProps = { account: { isLoading: false, }, - network: { - isOffline: false, - }, }; class LogInWithShortLivedAuthTokenPage extends Component { From d9a3cbc67f61d65823bd6384976e090fa97a0404 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 3 May 2023 06:55:30 +0800 Subject: [PATCH 16/28] add a comment for openRouteInDesktopApp --- src/libs/Browser/index.web.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libs/Browser/index.web.js b/src/libs/Browser/index.web.js index 7729e71c5004..344e56db5e7e 100644 --- a/src/libs/Browser/index.web.js +++ b/src/libs/Browser/index.web.js @@ -51,6 +51,11 @@ function isMobileSafari() { return /iP(ad|od|hone)/i.test(userAgent) && /WebKit/i.test(userAgent) && !(/(CriOS|FxiOS|OPiOS|mercury)/i.test(userAgent)); } +/** + * The session information needs to be passed to the Desktop app, and the only way to do that is by using query params. There is no other way to transfer the data. + * @param {String} shortLivedAuthToken + * @param {String} email + */ function openRouteInDesktopApp(shortLivedAuthToken = '', email = '') { const params = new URLSearchParams(); params.set('exitTo', `${window.location.pathname}${window.location.search}${window.location.hash}`); From 0349229f031a3f93c32aa0b5bdf4e70fc8fde036 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Thu, 4 May 2023 08:11:10 +0800 Subject: [PATCH 17/28] move navigation logic from createWorkspace to createWorkspaceAndNavigateToIt --- src/libs/actions/App.js | 41 ++++++++++++++----- src/libs/actions/Policy.js | 13 +----- .../FloatingActionButtonAndPopover.js | 3 +- src/pages/workspace/WorkspacesListPage.js | 3 +- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index deefec7632f6..2ec255a3e057 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -200,17 +200,37 @@ function reconnectApp() { * This promise is used so that deeplink component know when a transition is end. * This is necessary because we want to begin deeplink redirection after the transition is end. */ -let resolveTransitionIsEndPromise; -const transitionIsEndPromise = new Promise((resolve) => { - resolveTransitionIsEndPromise = resolve; +let resolveSignOnTransitionToFinishPromise; +const signOnTransitionToFinishPromise = new Promise((resolve) => { + resolveSignOnTransitionToFinishPromise = resolve; }); -function waitForTransitionEnd() { - return transitionIsEndPromise; +function waitForSignOnTransitionToFinish() { + return signOnTransitionToFinishPromise; } -function endTransition() { - return resolveTransitionIsEndPromise(); +function endSignOnTransition() { + return resolveSignOnTransitionToFinishPromise(); +} + +/** + * create a new workspace and navigate to it + * + * @param {String} [ownerEmail] Optional, the email of the account to make the owner of the policy + * @param {Boolean} [makeMeAdmin] Optional, leave the calling account as an admin on the policy + * @param {String} [policyName] Optional, custom policy name we will use for created workspace + * @param {Boolean} [transitionFromOldDot] Optional, if the user is transitioning from old dot + */ +function createWorkspaceAndNavigateToIt(ownerEmail = '', makeMeAdmin = false, policyName = '', transitionFromOldDot = false) { + const policyID = Policy.generatePolicyID(); + Policy.createWorkspace(ownerEmail, makeMeAdmin, policyName, policyID); + Navigation.isNavigationReady() + .then(() => { + if (transitionFromOldDot) { + Navigation.dismissModal(); // Dismiss /transition route for OldDot to NewDot transitions + } + Navigation.navigate(ROUTES.getWorkspaceInitialRoute(policyID)); + }).then(endSignOnTransition); } /** @@ -259,7 +279,7 @@ function setUpPoliciesAndNavigate(session) { && isTransitioning && exitTo === ROUTES.WORKSPACE_NEW; if (shouldCreateFreePolicy) { - Policy.createWorkspace(ownerEmail, makeMeAdmin, policyName, true).then(endTransition); + createWorkspaceAndNavigateToIt(ownerEmail, makeMeAdmin, policyName, true); return; } if (!isLoggingInAsNewUser && exitTo) { @@ -273,7 +293,7 @@ function setUpPoliciesAndNavigate(session) { // We must call dismissModal() to remove the /transition route from history Navigation.dismissModal(); Navigation.navigate(exitTo); - }).then(endTransition); + }).then(endSignOnTransition); }); } } @@ -330,7 +350,7 @@ function beginDeepLinkRedirect() { } function beginDeepLinkRedirectAfterTransition() { - waitForTransitionEnd().then(beginDeepLinkRedirect); + waitForSignOnTransitionToFinish().then(beginDeepLinkRedirect); } export { @@ -342,6 +362,7 @@ export { openApp, reconnectApp, confirmReadyToOpenApp, + createWorkspaceAndNavigateToIt, beginDeepLinkRedirect, beginDeepLinkRedirectAfterTransition, }; diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 83ba1c2218dd..85f9fa53ba9b 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -850,10 +850,9 @@ function generatePolicyID() { * @param {String} [ownerEmail] Optional, the email of the account to make the owner of the policy * @param {Boolean} [makeMeAdmin] Optional, leave the calling account as an admin on the policy * @param {String} [policyName] Optional, custom policy name we will use for created workspace - * @param {Boolean} [transitionFromOldDot] Optional, if the user is transitioning from old dot + * @param {String} [policyID] Optional, custom policy id we will use for created workspace */ -function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '', transitionFromOldDot = false) { - const policyID = generatePolicyID(); +function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '', policyID = generatePolicyID()) { const workspaceName = policyName || generateDefaultWorkspaceName(ownerEmail); const { @@ -1057,14 +1056,6 @@ function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '', }, ], }); - - return Navigation.isNavigationReady() - .then(() => { - if (transitionFromOldDot) { - Navigation.dismissModal(); // Dismiss /transition route for OldDot to NewDot transitions - } - Navigation.navigate(ROUTES.getWorkspaceInitialRoute(policyID)); - }); } /** diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index ebff5f4e6a51..3bd758d48a68 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -21,6 +21,7 @@ import withNavigation from '../../../../components/withNavigation'; import * as Welcome from '../../../../libs/actions/Welcome'; import withNavigationFocus from '../../../../components/withNavigationFocus'; import withDrawerState from '../../../../components/withDrawerState'; +import * as App from '../../../../libs/actions/App'; /** * @param {Object} [policy] @@ -225,7 +226,7 @@ class FloatingActionButtonAndPopover extends React.Component { iconHeight: 40, text: this.props.translate('workspace.new.newWorkspace'), description: this.props.translate('workspace.new.getTheExpensifyCardAndMore'), - onSelected: () => Policy.createWorkspace(), + onSelected: () => App.createWorkspaceAndNavigateToIt(), }, ] : []), ]} diff --git a/src/pages/workspace/WorkspacesListPage.js b/src/pages/workspace/WorkspacesListPage.js index f1dbbbb7f1d3..33b1daa49e7e 100755 --- a/src/pages/workspace/WorkspacesListPage.js +++ b/src/pages/workspace/WorkspacesListPage.js @@ -26,6 +26,7 @@ import BlockingView from '../../components/BlockingViews/BlockingView'; import {withNetwork} from '../../components/OnyxProvider'; import * as ReimbursementAccountProps from '../ReimbursementAccount/reimbursementAccountPropTypes'; import * as ReportUtils from '../../libs/ReportUtils'; +import * as App from '../../libs/actions/App'; const propTypes = { /* Onyx Props */ @@ -202,7 +203,7 @@ class WorkspacesListPage extends Component {