From 979fbf57dfdea53178ba1dd6ef7b9f631bef7c18 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Fri, 5 Aug 2022 11:11:04 -0700 Subject: [PATCH 1/2] Revert "Revert "Start Using BeginSignIn command and remove obsolete API command calls"" --- README.md | 10 +- src/ONYXKEYS.js | 3 - src/languages/en.js | 6 +- src/languages/es.js | 6 +- src/libs/Network/MainQueue.js | 2 +- src/libs/Network/enhanceParameters.js | 3 +- src/libs/actions/Session/index.js | 148 ++++++++-------------- src/libs/actions/User.js | 14 +- src/libs/deprecatedAPI.js | 25 ---- src/pages/SetPasswordPage.js | 26 +--- src/pages/signin/LoginForm.js | 6 +- src/pages/signin/PasswordForm.js | 7 +- src/pages/signin/ResendValidationForm.js | 31 +---- src/pages/signin/SignInPage.js | 29 ++--- tests/actions/ReimbursementAccountTest.js | 43 ++++--- tests/actions/ReportTest.js | 1 - tests/unit/NetworkTest.js | 2 +- tests/utils/TestHelper.js | 33 +++-- 18 files changed, 135 insertions(+), 260 deletions(-) diff --git a/README.md b/README.md index 55d9b8f1478e..8881a1ab3a71 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ For an M1 Mac, read this [SO](https://stackoverflow.com/c/expensify/questions/11 1. If you are having issues with **_Getting Started_**, please reference [React Native's Documentation](https://reactnative.dev/docs/environment-setup) 2. If you are running into CORS errors like (in the browser dev console) ```sh - Access to fetch at 'https://www.expensify.com/api?command=GetAccountStatus' from origin 'http://localhost:8080' has been blocked by CORS policy + Access to fetch at 'https://www.expensify.com/api?command=BeginSignIn' from origin 'http://localhost:8080' has been blocked by CORS policy ``` You probably have a misconfigured `.env` file - remove it (`rm .env`) and try again @@ -164,7 +164,7 @@ That action will then call `Onyx.merge()` to [set default data and a loading sta ```js function signIn(password, twoFactorAuthCode) { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: true}); Authentication.Authenticate({ ...defaultParams, password, @@ -177,7 +177,7 @@ function signIn(password, twoFactorAuthCode) { Onyx.merge(ONYXKEYS.ACCOUNT, {error: error.message}); }) .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } ``` @@ -188,7 +188,7 @@ Keeping our `Onyx.merge()` out of the view layer and in actions helps organize t // Bad validateAndSubmitForm() { // validate... - this.setState({loading: true}); + this.setState({isLoading: true}); signIn() .then((response) => { if (result.jsonCode === 200) { @@ -198,7 +198,7 @@ validateAndSubmitForm() { this.setState({error: response.message}); }) .finally(() => { - this.setState({loading: false}); + this.setState({isLoading: false}); }); } diff --git a/src/ONYXKEYS.js b/src/ONYXKEYS.js index 17302ba46467..a6fe6476435f 100755 --- a/src/ONYXKEYS.js +++ b/src/ONYXKEYS.js @@ -186,9 +186,6 @@ export default { // The policyID of the last workspace whose settings were accessed by the user LAST_ACCESSED_WORKSPACE_POLICY_ID: 'lastAccessedWorkspacePolicyID', - // Validating Email? - USER_SIGN_UP: 'userSignUp', - // List of Form ids FORMS: { ADD_DEBIT_CARD_FORM: 'addDebitCardForm', diff --git a/src/languages/en.js b/src/languages/en.js index b9360c8fdb3e..cbafe0744149 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -497,11 +497,9 @@ export default { }, resendValidationForm: { linkHasBeenResent: 'Link has been re-sent', - weSentYouMagicSignInLink: ({login}) => `We've sent a magic sign in link to ${login}. Check your Inbox and your Spam folder and wait 5-10 minutes before trying again.`, + weSentYouMagicSignInLink: ({login, loginType}) => `I've sent a magic sign-in link to ${login}. Please check your ${loginType} to sign in.`, resendLink: 'Resend link', validationCodeFailedMessage: 'It looks like there was an error with your validation link or it has expired.', - unvalidatedAccount: 'This account exists but isn\'t validated, please check your inbox for your magic link.', - newAccount: ({login, loginType}) => `Welcome ${login}, it's always great to see a new face around here! Please check your ${loginType} for a magic link to validate your account.`, }, detailsPage: { localTime: 'Local time', @@ -523,7 +521,7 @@ export default { passwordFormTitle: 'Welcome back to the New Expensify! Please set your password.', passwordNotSet: 'We were unable to set your new password. We have sent you a new password link to try again.', setPasswordLinkInvalid: 'This set password link is invalid or has expired. A new one is waiting for you in your email inbox!', - verifyingAccount: 'Verifying account', + validatingAccount: 'Verifying account', }, stepCounter: ({step, total}) => `Step ${step} of ${total}`, bankAccount: { diff --git a/src/languages/es.js b/src/languages/es.js index 86d9cd026e42..fdcb6778c643 100644 --- a/src/languages/es.js +++ b/src/languages/es.js @@ -497,11 +497,9 @@ export default { }, resendValidationForm: { linkHasBeenResent: 'El enlace se ha reenviado', - weSentYouMagicSignInLink: ({login}) => `Hemos enviado un enlace mágico de inicio de sesión a ${login}. Verifica tu bandeja de entrada y tu carpeta de correo no deseado y espera de 5 a 10 minutos antes de intentarlo de nuevo.`, + weSentYouMagicSignInLink: ({login, loginType}) => `Te he enviado un hiperenlace mágico para iniciar sesión a ${login}. Por favor revisa tu ${loginType}`, resendLink: 'Reenviar enlace', validationCodeFailedMessage: 'Parece que hubo un error con el enlace de validación o ha caducado.', - unvalidatedAccount: 'Esta cuenta existe pero no está validada, por favor busca el enlace mágico en tu bandeja de entrada', - newAccount: ({login, loginType}) => `¡Bienvenido ${login}, es genial ver una cara nueva por aquí! En tu ${loginType} encontrarás un enlace para validar tu cuenta, por favor, revísalo`, }, detailsPage: { localTime: 'Hora local', @@ -523,7 +521,7 @@ export default { passwordFormTitle: '¡Bienvenido de vuelta al Nuevo Expensify! Por favor, elige una contraseña.', passwordNotSet: 'No pudimos cambiar tu clave. Te hemos enviado un nuevo enlace para que intentes cambiar la clave nuevamente.', setPasswordLinkInvalid: 'El enlace para configurar tu contraseña ha expirado. Te hemos enviado un nuevo enlace a tu correo.', - verifyingAccount: 'Verificando cuenta', + validatingAccount: 'Verificando cuenta', }, stepCounter: ({step, total}) => `Paso ${step} de ${total}`, bankAccount: { diff --git a/src/libs/Network/MainQueue.js b/src/libs/Network/MainQueue.js index 2143cee9d851..bb0804565893 100644 --- a/src/libs/Network/MainQueue.js +++ b/src/libs/Network/MainQueue.js @@ -19,7 +19,7 @@ let networkRequestQueue = []; * @return {Boolean} */ function canMakeRequest(request) { - // Some requests are always made even when we are in the process of authenticating (typically because they require no authToken e.g. Log, GetAccountStatus) + // Some requests are always made even when we are in the process of authenticating (typically because they require no authToken e.g. Log, BeginSignIn) // However, if we are in the process of authenticating we always want to queue requests until we are no longer authenticating. return request.data.forceNetworkRequest === true || (!NetworkStore.isAuthenticating() && !SequentialQueue.isRunning()); } diff --git a/src/libs/Network/enhanceParameters.js b/src/libs/Network/enhanceParameters.js index 093c0d96c087..b7f82f47dda5 100644 --- a/src/libs/Network/enhanceParameters.js +++ b/src/libs/Network/enhanceParameters.js @@ -15,11 +15,10 @@ function isAuthTokenRequired(command) { 'Log', 'Graphite_Timer', 'Authenticate', - 'GetAccountStatus', + 'BeginSignIn', 'SetPassword', 'User_SignUp', 'ResendValidateCode', - 'User_ReopenAccount', 'ValidateEmail', ], command); } diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index 5059a434ad43..d88d35bfe304 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -18,12 +18,12 @@ import Timers from '../../Timers'; import * as Pusher from '../../Pusher/pusher'; import NetworkConnection from '../../NetworkConnection'; import * as User from '../User'; -import * as ValidationUtils from '../../ValidationUtils'; import * as Authentication from '../../Authentication'; import * as ErrorUtils from '../../ErrorUtils'; import * as Welcome from '../Welcome'; import * as API from '../../API'; import * as NetworkStore from '../../Network/NetworkStore'; +import DateUtils from '../../DateUtils'; let credentials = {}; Onyx.connect({ @@ -48,28 +48,6 @@ function setSuccessfulSignInData(data) { }); } -/** - * Create an account for the user logging in. - * This will send them a notification with a link to click on to validate the account and set a password - * - * @param {String} login - */ -function createAccount(login) { - Onyx.merge(ONYXKEYS.ACCOUNT, {error: ''}); - - DeprecatedAPI.User_SignUp({ - email: login, - }).then((response) => { - // A 405 means that the account needs to be validated. We should let the user proceed to the ResendValidationForm view. - if (response.jsonCode === 200 || response.jsonCode === 405) { - return; - } - - Onyx.merge(ONYXKEYS.CREDENTIALS, {login: null}); - Onyx.merge(ONYXKEYS.ACCOUNT, {error: response.message || `Unknown API Error: ${response.jsonCode}`}); - }); -} - /** * Clears the Onyx store and redirects user to the sign in page */ @@ -106,29 +84,16 @@ function signOutAndRedirectToSignIn() { Log.info('Redirecting to Sign In because signOut() was called'); } -/** - * Reopen the account and send the user a link to set password - * - * @param {String} [login] - */ -function reopenAccount(login = credentials.login) { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); - DeprecatedAPI.User_ReopenAccount({email: login}) - .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); - }); -} - /** * Resend the validation link to the user that is validating their account * * @param {String} [login] */ function resendValidationLink(login = credentials.login) { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: true}); DeprecatedAPI.ResendValidateCode({email: login}) .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } @@ -137,45 +102,42 @@ function resendValidationLink(login = credentials.login) { * * @param {String} login */ -function fetchAccountDetails(login) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); +function beginSignIn(login) { + const optimisticData = [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.ACCOUNT, + value: { + ...CONST.DEFAULT_ACCOUNT_DATA, + isLoading: true, + }, + }, + ]; - DeprecatedAPI.GetAccountStatus({email: login, forceNetworkRequest: true}) - .then((response) => { - if (response.jsonCode === 200) { - Onyx.merge(ONYXKEYS.CREDENTIALS, { - login: response.normalizedLogin, - }); - Onyx.merge(ONYXKEYS.ACCOUNT, { - accountExists: response.accountExists, - validated: response.validated, - closed: response.isClosed, - forgotPassword: false, - validateCodeExpired: false, - }); - - if (!response.accountExists) { - createAccount(login); - } else if (response.isClosed) { - reopenAccount(login); - } else if (!response.validated) { - resendValidationLink(login); - } - } else if (response.jsonCode === 402) { - Onyx.merge(ONYXKEYS.ACCOUNT, { - error: ValidationUtils.isNumericWithSpecialChars(login) - ? Localize.translateLocal('common.error.phoneNumber') - : Localize.translateLocal('loginForm.error.invalidFormatEmailLogin'), - }); - } else if (response.jsonCode === CONST.JSON_CODE.UNABLE_TO_RETRY) { - Onyx.merge(ONYXKEYS.ACCOUNT, {error: Localize.translateLocal('session.offlineMessageRetry')}); - } else { - Onyx.merge(ONYXKEYS.ACCOUNT, {error: response.message}); - } - }) - .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); - }); + const successData = [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.ACCOUNT, + value: { + isLoading: false, + }, + }, + ]; + + const failureData = [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.ACCOUNT, + value: { + isLoading: false, + errors: { + [DateUtils.getMicroseconds()]: 'Cannot get account details, please try again', + }, + }, + }, + ]; + + API.read('BeginSignIn', {email: login}, {optimisticData, successData, failureData}); } /** @@ -235,7 +197,7 @@ function createTemporaryLogin(authToken, email) { return createLoginResponse; }) .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } @@ -248,7 +210,7 @@ function createTemporaryLogin(authToken, email) { * @param {String} [twoFactorAuthCode] */ function signIn(password, twoFactorAuthCode) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}); Authentication.Authenticate({ useExpensifyLogin: true, @@ -263,10 +225,10 @@ function signIn(password, twoFactorAuthCode) { if (response.jsonCode !== 200) { const errorMessage = ErrorUtils.getAuthenticateErrorMessage(response); if (errorMessage === 'passwordForm.error.twoFactorAuthenticationEnabled') { - Onyx.merge(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true, loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true, isLoading: false}); return; } - Onyx.merge(ONYXKEYS.ACCOUNT, {error: Localize.translateLocal(errorMessage), loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {error: Localize.translateLocal(errorMessage), isLoading: false}); return; } @@ -283,7 +245,7 @@ function signIn(password, twoFactorAuthCode) { * @param {String} exitTo */ function signInWithShortLivedToken(email, shortLivedToken) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}); createTemporaryLogin(shortLivedToken, email) .then((response) => { @@ -294,7 +256,7 @@ function signInWithShortLivedToken(email, shortLivedToken) { User.getUserDetails(); Onyx.merge(ONYXKEYS.ACCOUNT, {success: true}); }).finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } @@ -322,7 +284,6 @@ function resetPassword() { key: ONYXKEYS.ACCOUNT, value: { isLoading: false, - validateCodeExpired: false, }, }, ], @@ -332,7 +293,6 @@ function resetPassword() { key: ONYXKEYS.ACCOUNT, value: { isLoading: false, - validateCodeExpired: false, }, }, ], @@ -349,7 +309,7 @@ function resetPassword() { * @param {Number} accountID */ function setPassword(password, validateCode, accountID) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true, validateCodeExpired: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}); DeprecatedAPI.SetPassword({ password, validateCode, @@ -365,7 +325,7 @@ function setPassword(password, validateCode, accountID) { Onyx.merge(ONYXKEYS.ACCOUNT, {error: response.message}); }) .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } @@ -419,7 +379,6 @@ function changePasswordAndSignIn(authToken, password) { password, }) .then((responsePassword) => { - Onyx.merge(ONYXKEYS.USER_SIGN_UP, {authToken: null}); if (responsePassword.jsonCode === 200) { signIn(password); return; @@ -432,7 +391,7 @@ function changePasswordAndSignIn(authToken, password) { } if (responsePassword.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) { // authToken has expired, and we have the account email, so we request a new magic link. - Onyx.merge(ONYXKEYS.ACCOUNT, {accountExists: true, validateCodeExpired: true, error: null}); + Onyx.merge(ONYXKEYS.ACCOUNT, {error: null}); resetPassword(); Navigation.navigate(ROUTES.HOME); return; @@ -448,7 +407,6 @@ function changePasswordAndSignIn(authToken, password) { * @param {String} authToken */ function validateEmail(accountID, validateCode) { - Onyx.merge(ONYXKEYS.USER_SIGN_UP, {isValidating: true}); Onyx.merge(ONYXKEYS.SESSION, {error: ''}); DeprecatedAPI.ValidateEmail({ accountID, @@ -456,19 +414,16 @@ function validateEmail(accountID, validateCode) { }) .then((responseValidate) => { if (responseValidate.jsonCode === 200) { - Onyx.merge(ONYXKEYS.USER_SIGN_UP, {authToken: responseValidate.authToken}); - Onyx.merge(ONYXKEYS.ACCOUNT, {accountExists: true, validated: true}); - Onyx.merge(ONYXKEYS.CREDENTIALS, {login: responseValidate.email}); + Onyx.merge(ONYXKEYS.CREDENTIALS, {login: responseValidate.email, authToken: responseValidate.authToken}); return; } if (responseValidate.jsonCode === 666) { - Onyx.merge(ONYXKEYS.ACCOUNT, {accountExists: true, validated: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {validated: true}); } if (responseValidate.jsonCode === 401) { Onyx.merge(ONYXKEYS.SESSION, {error: 'setPasswordPage.setPasswordLinkInvalid'}); } - }) - .finally(Onyx.merge(ONYXKEYS.USER_SIGN_UP, {isValidating: false})); + }); } // It's necessary to throttle requests to reauthenticate since calling this multiple times will cause Pusher to @@ -537,13 +492,12 @@ function setShouldShowComposeInput(shouldShowComposeInput) { } export { - fetchAccountDetails, + beginSignIn, setPassword, signIn, signInWithShortLivedToken, signOut, signOutAndRedirectToSignIn, - reopenAccount, resendValidationLink, resetPassword, clearSignInData, diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index d41fcf3ef974..a7b71d9fd3b5 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -50,21 +50,21 @@ function updatePassword(oldPassword, password) { { onyxMethod: CONST.ONYX.METHOD.MERGE, key: ONYXKEYS.ACCOUNT, - value: {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}, + value: {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}, }, ], successData: [ { onyxMethod: CONST.ONYX.METHOD.MERGE, key: ONYXKEYS.ACCOUNT, - value: {loading: false}, + value: {isLoading: false}, }, ], failureData: [ { onyxMethod: CONST.ONYX.METHOD.MERGE, key: ONYXKEYS.ACCOUNT, - value: {loading: false}, + value: {isLoading: false}, }, ], }); @@ -177,7 +177,7 @@ function updateNewsletterSubscription(isSubscribed) { * @returns {Promise} */ function setSecondaryLoginAndNavigate(login, password) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}); return DeprecatedAPI.User_SecondaryLogin_Send({ email: login, @@ -202,7 +202,7 @@ function setSecondaryLoginAndNavigate(login, password) { Onyx.merge(ONYXKEYS.USER, {error}); }).finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); }); } @@ -215,7 +215,7 @@ function setSecondaryLoginAndNavigate(login, password) { function validateLogin(accountID, validateCode) { const isLoggedIn = !_.isEmpty(sessionAuthToken); const redirectRoute = isLoggedIn ? ROUTES.getReportRoute(currentlyViewedReportID) : ROUTES.HOME; - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true}); DeprecatedAPI.ValidateEmail({ accountID, @@ -236,7 +236,7 @@ function validateLogin(accountID, validateCode) { Onyx.merge(ONYXKEYS.ACCOUNT, {error}); } }).finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false}); Navigation.navigate(redirectRoute); }); } diff --git a/src/libs/deprecatedAPI.js b/src/libs/deprecatedAPI.js index ebbff2141e26..4ec69b3c60a6 100644 --- a/src/libs/deprecatedAPI.js +++ b/src/libs/deprecatedAPI.js @@ -134,18 +134,6 @@ function Get(parameters, shouldUseSecure = false) { return Network.post(commandName, parameters, CONST.NETWORK.METHOD.POST, shouldUseSecure); } -/** - * @param {Object} parameters - * @param {String} parameters.email - * @param {Boolean} parameters.forceNetworkRequest - * @returns {Promise} - */ -function GetAccountStatus(parameters) { - const commandName = 'GetAccountStatus'; - requireParameters(['email'], parameters, commandName); - return Network.post(commandName, parameters); -} - /** * @param {Object} parameters * @param {String} parameters.debtorEmail @@ -341,17 +329,6 @@ function SetPassword(parameters) { return Network.post(commandName, parameters); } -/** - * @param {Object} parameters - * @param {String} parameters.email - * @returns {Promise} - */ -function User_ReopenAccount(parameters) { - const commandName = 'User_ReopenAccount'; - requireParameters(['email'], parameters, commandName); - return Network.post(commandName, parameters); -} - /** * @param {Object} parameters * @param {String} parameters.email @@ -717,7 +694,6 @@ export { DeleteLogin, DeleteBankAccount, Get, - GetAccountStatus, GetStatementPDF, GetIOUReport, GetFullPolicy, @@ -740,7 +716,6 @@ export { UpdatePolicy, User_SignUp, User_IsUsingExpensifyCard, - User_ReopenAccount, User_SecondaryLogin_Send, User_UploadAvatar, User_FixAccount, diff --git a/src/pages/SetPasswordPage.js b/src/pages/SetPasswordPage.js index ec5c3fb3599d..d3b14a6d3a7c 100755 --- a/src/pages/SetPasswordPage.js +++ b/src/pages/SetPasswordPage.js @@ -29,7 +29,7 @@ const propTypes = { error: PropTypes.string, /** Whether a sign on form is loading (being submitted) */ - loading: PropTypes.bool, + isLoading: PropTypes.bool, }), /** The credentials of the logged in person */ @@ -47,15 +47,6 @@ const propTypes = { error: PropTypes.string, }), - /** User signup object */ - userSignUp: PropTypes.shape({ - /** Is Validating Email */ - isValidating: PropTypes.bool, - - /** Auth token used to change password */ - authToken: PropTypes.string, - }), - /** The accountID and validateCode are passed via the URL */ route: validateLinkPropTypes, @@ -70,10 +61,6 @@ const defaultProps = { error: '', authToken: '', }, - userSignUp: { - isValidating: false, - authToken: '', - }, }; class SetPasswordPage extends Component { @@ -91,7 +78,7 @@ class SetPasswordPage extends Component { componentDidMount() { const accountID = lodashGet(this.props.route.params, 'accountID', ''); const validateCode = lodashGet(this.props.route.params, 'validateCode', ''); - if (this.props.userSignUp.authToken) { + if (this.props.credentials.authToken) { return; } Session.validateEmail(accountID, validateCode); @@ -104,15 +91,15 @@ class SetPasswordPage extends Component { const accountID = lodashGet(this.props.route.params, 'accountID', ''); const validateCode = lodashGet(this.props.route.params, 'validateCode', ''); - if (this.props.userSignUp.authToken) { - Session.changePasswordAndSignIn(this.props.userSignUp.authToken, this.state.password); + if (this.props.credentials.authToken) { + Session.changePasswordAndSignIn(this.props.credentials.authToken, this.state.password); } else { Session.setPassword(this.state.password, validateCode, accountID); } } render() { - const buttonText = this.props.userSignUp.isValidating ? this.props.translate('setPasswordPage.verifyingAccount') : this.props.translate('setPasswordPage.setPassword'); + const buttonText = !this.props.account.validated ? this.props.translate('setPasswordPage.validatingAccount') : this.props.translate('setPasswordPage.setPassword'); const sessionError = this.props.session.error && this.props.translate(this.props.session.error); const error = sessionError || this.props.account.error; return ( @@ -133,7 +120,7 @@ class SetPasswordPage extends Component { diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 0801487c8803..7936e59f1c74 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -26,14 +26,11 @@ const propTypes = { /** The details about the account that the user is signing in with */ account: PropTypes.shape({ - /** Whether or not the account already exists */ - accountExists: PropTypes.bool, - /** Whether or not two factor authentication is required */ requiresTwoFactorAuth: PropTypes.bool, /** Whether or not a sign on form is loading (being submitted) */ - loading: PropTypes.bool, + isLoading: PropTypes.bool, }), ...withLocalizePropTypes, @@ -183,7 +180,7 @@ class PasswordForm extends React.Component { success style={[styles.mv3]} text={this.props.translate('common.signIn')} - isLoading={this.props.account.loading} + isLoading={this.props.account.isLoading} onPress={this.validateAndSubmitForm} /> diff --git a/src/pages/signin/ResendValidationForm.js b/src/pages/signin/ResendValidationForm.js index 95956cb7b933..e643e94eed73 100755 --- a/src/pages/signin/ResendValidationForm.js +++ b/src/pages/signin/ResendValidationForm.js @@ -34,12 +34,6 @@ const propTypes = { /** Whether or not the account is validated */ validated: PropTypes.bool, - - /** Whether or not the account is closed */ - closed: PropTypes.bool, - - /** Whether or not the account already exists */ - accountExists: PropTypes.bool, }), /** Information about the network */ @@ -79,9 +73,7 @@ class ResendValidationForm extends React.Component { formSuccess: this.props.translate('resendValidationForm.linkHasBeenResent'), }); - if (this.props.account.closed) { - Session.reopenAccount(); - } else if (!this.props.account.validated) { + if (!this.props.account.validated) { Session.resendValidationLink(); } else { Session.resetPassword(); @@ -93,27 +85,10 @@ class ResendValidationForm extends React.Component { } render() { - const isNewAccount = !this.props.account.accountExists; - const isOldUnvalidatedAccount = this.props.account.accountExists && !this.props.account.validated; const isSMSLogin = Str.isSMSLogin(this.props.credentials.login); const login = isSMSLogin ? this.props.toLocalPhone(Str.removeSMSDomain(this.props.credentials.login)) : this.props.credentials.login; const loginType = (isSMSLogin ? this.props.translate('common.phone') : this.props.translate('common.email')).toLowerCase(); - let message = ''; - - if (isNewAccount) { - message = this.props.translate('resendValidationForm.newAccount', { - login, - loginType, - }); - } else if (this.props.account.validateCodeExpired) { - message = this.props.translate('resendValidationForm.validationCodeFailedMessage'); - } else if (isOldUnvalidatedAccount) { - message = this.props.translate('resendValidationForm.unvalidatedAccount'); - } else { - message = this.props.translate('resendValidationForm.weSentYouMagicSignInLink', { - login, - }); - } + return ( <> @@ -129,7 +104,7 @@ class ResendValidationForm extends React.Component { - {message} + {this.props.translate('resendValidationForm.weSentYouMagicSignInLink', {login, loginType})} {!_.isEmpty(this.state.formSuccess) && ( diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index c0d7945e9945..c19d8ba09fe9 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -4,7 +4,6 @@ import { } from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; -import lodashGet from 'lodash/get'; import ONYXKEYS from '../../ONYXKEYS'; import styles from '../../styles/styles'; import updateUnread from '../../libs/UnreadIndicatorUpdater/updateUnread/index'; @@ -20,17 +19,11 @@ const propTypes = { /** The details about the account that the user is signing in with */ account: PropTypes.shape({ - /** Whether or not the account already exists */ - accountExists: PropTypes.bool, - /** Error to display when there is an account error returned */ error: PropTypes.string, - /** Whether or not the account is validated */ + /** Whether the account is validated */ validated: PropTypes.bool, - - /** Whether or not the account is validated */ - forgotPassword: PropTypes.bool, }), /** The credentials of the person signing in */ @@ -60,27 +53,21 @@ class SignInPage extends Component { // - A login has not been entered yet const showLoginForm = !this.props.credentials.login; - const validateCodeExpired = lodashGet(this.props.account, 'validateCodeExpired', false); - - const validAccount = this.props.account.accountExists - && this.props.account.validated - && !this.props.account.forgotPassword - && !validateCodeExpired; - // Show the password form if // - A login has been entered - // - AND a GitHub username has been entered OR they already have access to New Expensify // - AND an account exists and is validated for this login // - AND a password hasn't been entered yet + // - AND haven't forgotten password const showPasswordForm = this.props.credentials.login - && validAccount - && !this.props.credentials.password; + && this.props.account.validated + && !this.props.credentials.password + && !this.props.account.forgotPassword; // Show the resend validation link form if // - A login has been entered - // - AND a GitHub username has been entered OR they already have access to this app - // - AND an account did not exist or is not validated for that login - const shouldShowResendValidationLinkForm = this.props.credentials.login && !validAccount; + // - AND is not validated or password is forgotten + const shouldShowResendValidationLinkForm = this.props.credentials.login + && (!this.props.account.validated || this.props.account.forgotPassword); const welcomeText = shouldShowResendValidationLinkForm ? '' diff --git a/tests/actions/ReimbursementAccountTest.js b/tests/actions/ReimbursementAccountTest.js index 0c0b53c25f90..cd0ae97aeda6 100644 --- a/tests/actions/ReimbursementAccountTest.js +++ b/tests/actions/ReimbursementAccountTest.js @@ -259,25 +259,30 @@ describe('actions/BankAccounts', () => { expect(reimbursementAccount.error).toBe(''); expect(reimbursementAccount.achData.currentStep).toBe(CONST.BANK_ACCOUNT.STEP.ACH_CONTRACT); - // WHEN we mock a sucessful call to SetupWithdrawalAccount while on the ACHContractStep - HttpUtils.xhr.mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, - achData: { - bankAccountID: TEST_BANK_ACCOUNT_ID, - }, - })); - - // And mock SetNameValuePair response - HttpUtils.xhr.mockImplementationOnce(() => Promise.resolve({jsonCode: 200})); - - // And mock the response of Get&returnValueList=bankAccountList - HttpUtils.xhr.mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, - bankAccountList: [{ - bankAccountID: TEST_BANK_ACCOUNT_ID, - state: BankAccount.STATE.PENDING, - }], - })); + HttpUtils.xhr.mockImplementation((command) => { + // WHEN we mock a sucessful call to SetupWithdrawalAccount while on the ACHContractStep + switch (command) { + case 'BankAccount_SetupWithdrawal': + return Promise.resolve({ + jsonCode: 200, + achData: { + bankAccountID: TEST_BANK_ACCOUNT_ID, + }, + }); + + // And mock the response of Get&returnValueList=bankAccountList + case 'Get': + return Promise.resolve({ + jsonCode: 200, + bankAccountList: [{ + bankAccountID: TEST_BANK_ACCOUNT_ID, + state: BankAccount.STATE.PENDING, + }], + }); + default: + return Promise.resolve({jsonCode: 200}); + } + }); // WHEN we call setupWithdrawalAccount via the ACHContractStep BankAccounts.setupWithdrawalAccount({ diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index da8c4de756fb..3955959b2d1f 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -204,7 +204,6 @@ describe('actions/Report', () => { it('should be updated correctly when new comments are added, deleted or marked as unread', () => { const REPORT_ID = 1; - let report; Onyx.connect({ key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, diff --git a/tests/unit/NetworkTest.js b/tests/unit/NetworkTest.js index b09b23d49645..0b5ce51ba248 100644 --- a/tests/unit/NetworkTest.js +++ b/tests/unit/NetworkTest.js @@ -260,7 +260,7 @@ test('Request will not run until credentials are read from Onyx', () => { const spyHttpUtilsXhr = jest.spyOn(HttpUtils, 'xhr').mockImplementation(() => Promise.resolve({})); // When we make a request - Session.fetchAccountDetails(TEST_USER_LOGIN); + Session.beginSignIn(TEST_USER_LOGIN); // Then we should expect that no requests have been made yet expect(spyHttpUtilsXhr).not.toHaveBeenCalled(); diff --git a/tests/utils/TestHelper.js b/tests/utils/TestHelper.js index d619e9d72ab2..e6a7dc5b3e97 100644 --- a/tests/utils/TestHelper.js +++ b/tests/utils/TestHelper.js @@ -20,27 +20,32 @@ function signInWithTestUser(accountID = 1, login = 'test@user.com', password = ' const originalXhr = HttpUtils.xhr; HttpUtils.xhr = jest.fn(); HttpUtils.xhr.mockImplementation(() => Promise.resolve({ + onyxData: [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.CREDENTIALS, + value: { + login, + }, + }, + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.ACCOUNT, + value: { + validated: true, + }, + }, + ], jsonCode: 200, - accountExists: true, - requiresTwoFactorAuth: false, - normalizedLogin: login, })); // Simulate user entering their login and populating the credentials.login - Session.fetchAccountDetails(login); + Session.beginSignIn(login); return waitForPromisesToResolve() .then(() => { - // First call to Authenticate + // Response is the same for calls to Authenticate and CreateLogin HttpUtils.xhr - .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, - accountID, - authToken, - email: login, - })) - - // Next call to CreateLogin - .mockImplementationOnce(() => Promise.resolve({ + .mockImplementation(() => Promise.resolve({ jsonCode: 200, accountID, authToken, From c857cf4a375aff1aab3a1a3c436add11bfde86f4 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Mon, 8 Aug 2022 13:34:55 -0700 Subject: [PATCH 2/2] Add more robust error handling for BeginSignIn command --- src/libs/actions/Session/index.js | 9 ++++++-- src/pages/signin/LoginForm.js | 37 +++++++++++++++++-------------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index d88d35bfe304..23373dcf93fb 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -363,11 +363,16 @@ function cleanupSession() { } function clearAccountMessages() { - Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''}); + Onyx.merge(ONYXKEYS.ACCOUNT, { + error: '', + success: '', + errors: [], + isLoading: false, + }); } /** - * Calls change password and signs if if successful. Otherwise, we request a new magic link + * Calls change password and signs if successful. Otherwise, we request a new magic link * if we know the account email. Otherwise or finally we redirect to the root of the nav. * @param {String} authToken * @param {String} password diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index f11b836c3327..dec680d100bb 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -5,7 +5,6 @@ import PropTypes from 'prop-types'; import _ from 'underscore'; import Str from 'expensify-common/lib/str'; import styles from '../../styles/styles'; -import Button from '../../components/Button'; import Text from '../../components/Text'; import * as Session from '../../libs/actions/Session'; import ONYXKEYS from '../../ONYXKEYS'; @@ -18,6 +17,7 @@ import TextInput from '../../components/TextInput'; import * as ValidationUtils from '../../libs/ValidationUtils'; import * as LoginUtils from '../../libs/LoginUtils'; import withToggleVisibilityView, {toggleVisibilityViewPropTypes} from '../../components/withToggleVisibilityView'; +import FormAlertWithSubmitButton from '../../components/FormAlertWithSubmitButton'; const propTypes = { /** Should we dismiss the keyboard when transitioning away from the page? */ @@ -59,6 +59,10 @@ class LoginForm extends React.Component { formError: false, login: '', }; + + if (this.props.account.errors || this.props.account.isLoading) { + Session.clearAccountMessages(); + } } componentDidMount() { @@ -93,7 +97,7 @@ class LoginForm extends React.Component { formError: null, }); - if (this.props.account.error) { + if (this.props.account.errors) { Session.clearAccountMessages(); } } @@ -136,6 +140,14 @@ class LoginForm extends React.Component { } render() { + const formErrorTranslated = this.state.formError && this.props.translate(this.state.formError); + const error = formErrorTranslated || _.chain(this.props.account.errors || []) + .keys() + .sortBy() + .reverse() + .map(key => this.props.account.errors[key]) + .first() + .value(); return ( <> @@ -154,28 +166,19 @@ class LoginForm extends React.Component { keyboardType={getEmailKeyboardType()} /> - {this.state.formError && ( - - {this.props.translate(this.state.formError)} - - )} - - {!this.state.formError && !_.isEmpty(this.props.account.error) && ( - - {this.props.account.error} - - )} {!_.isEmpty(this.props.account.success) && ( {this.props.account.success} )} -