From fb82b65c568e3d8ad0c39d301771b42cb2f8b058 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski <60450261+mateuuszzzzz@users.noreply.github.com> Date: Thu, 28 Nov 2024 08:31:28 +0100 Subject: [PATCH] [HybridApp] Refactor `SignInPage` PR v2 (#144) * Simplify HybridApp.ts * Use early returns * Rearrange HybridApp module * Remove redundant keys * Remove unused import * Refactor remaining parts of code * Do not init with useEffect * Remove console.debug * Reduce repeating code * Remove unused import * reset sign-in logic on sign out --------- Co-authored-by: war-in --- src/libs/HybridApp.ts | 9 +-- .../Navigation/AppNavigator/AuthScreens.tsx | 2 +- src/libs/actions/HybridApp/index.ts | 73 ++++++++++++++----- src/libs/actions/Session/index.ts | 5 -- src/libs/actions/SignInRedirect.ts | 8 +- src/pages/settings/InitialSettingsPage.tsx | 4 +- .../ValidateCodeForm/BaseValidateCodeForm.tsx | 34 +++------ 7 files changed, 80 insertions(+), 55 deletions(-) diff --git a/src/libs/HybridApp.ts b/src/libs/HybridApp.ts index 4a5dea65371..720698c2d71 100644 --- a/src/libs/HybridApp.ts +++ b/src/libs/HybridApp.ts @@ -14,16 +14,14 @@ let currentTryNewDot: OnyxEntry; Onyx.connect({ key: ONYXKEYS.HYBRID_APP, callback: (hybridApp) => { - console.debug('Last hybridApp value', {hybridApp}); - handleSignInFlow(hybridApp, currentTryNewDot); + handleChangeInHybridAppSignInFlow(hybridApp, currentTryNewDot); }, }); Onyx.connect({ key: ONYXKEYS.NVP_TRYNEWDOT, callback: (tryNewDot) => { - console.debug('Last tryNewDot value', {tryNewDot}); - handleSignInFlow(currentHybridApp, tryNewDot); + handleChangeInHybridAppSignInFlow(currentHybridApp, tryNewDot); }, }); @@ -50,7 +48,7 @@ function shouldUseOldApp(tryNewDot?: TryNewDot) { return tryNewDot?.classicRedirect.dismissed === true; } -function handleSignInFlow(hybridApp: OnyxEntry, tryNewDot: OnyxEntry) { +function handleChangeInHybridAppSignInFlow(hybridApp: OnyxEntry, tryNewDot: OnyxEntry) { if (!NativeModules.HybridAppModule) { return; } @@ -112,7 +110,6 @@ const init: Init = () => { return; } - // Setup event listeners DeviceEventEmitter.addListener(CONST.EVENTS.HYBRID_APP.ON_SIGN_IN_FINISHED, onOldDotSignInFinished); }; diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.tsx b/src/libs/Navigation/AppNavigator/AuthScreens.tsx index 819e8ac29ff..0e909b5ca61 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.tsx +++ b/src/libs/Navigation/AppNavigator/AuthScreens.tsx @@ -1,6 +1,6 @@ import {findFocusedRoute} from '@react-navigation/native'; import React, {memo, useEffect, useMemo, useRef, useState} from 'react'; -import {NativeModules, View} from 'react-native'; +import {View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import Onyx, {withOnyx} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; diff --git a/src/libs/actions/HybridApp/index.ts b/src/libs/actions/HybridApp/index.ts index 8ef4dfeca49..66e3ef1f5a5 100644 --- a/src/libs/actions/HybridApp/index.ts +++ b/src/libs/actions/HybridApp/index.ts @@ -1,49 +1,80 @@ import {NativeModules} from 'react-native'; import Onyx from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; +import * as Session from '@userActions/Session'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {HybridApp} from '@src/types/onyx'; import type HybridAppSettings from './types'; +/* + * Parses initial settings passed from OldDot app + */ function parseHybridAppSettings(hybridAppSettings: string): HybridAppSettings { return JSON.parse(hybridAppSettings) as HybridAppSettings; } -function setOldDotSignInError(oldDotSignInError: string | null) { - Onyx.merge(ONYXKEYS.HYBRID_APP, {oldDotSignInError}); -} - +/* + * Changes value of `readyToShowAuthScreens` + */ function setReadyToShowAuthScreens(readyToShowAuthScreens: boolean) { Onyx.merge(ONYXKEYS.HYBRID_APP, {readyToShowAuthScreens}); } -function setUseNewDotSignInPage(useNewDotSignInPage: boolean) { - Onyx.merge(ONYXKEYS.HYBRID_APP, {useNewDotSignInPage}); -} - -function setLoggedOutFromOldDot(loggedOutFromOldDot: boolean) { - Onyx.merge(ONYXKEYS.HYBRID_APP, {loggedOutFromOldDot}); -} - +/* + * Changes NewDot sign-in state + */ function setNewDotSignInState(newDotSignInState: ValueOf) { Onyx.merge(ONYXKEYS.HYBRID_APP, {newDotSignInState}); } +/* + * Changes OldDot sign-in state + */ function setOldDotSignInState(oldDotSignInState: ValueOf) { Onyx.merge(ONYXKEYS.HYBRID_APP, {oldDotSignInState}); } -function resetHybridAppSignInState() { +/* + * Starts HybridApp sign-in flow from the beginning. + * In certain cases, it can perform sign-out if necessary + */ +function resetSignInFlow(signOut = false) { + if (signOut) { + Onyx.merge(ONYXKEYS.HYBRID_APP, { + oldDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.WAITING_FOR_SIGN_OUT, + newDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.WAITING_FOR_SIGN_OUT, + }); + + Session.signOut(); + + Onyx.merge(ONYXKEYS.SESSION, null); + } + Onyx.merge(ONYXKEYS.HYBRID_APP, { readyToShowAuthScreens: false, + oldDotSignInError: null, + oldDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED, + newDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED, + }); +} + +/* + * Sets proper sign-in state after sign-out on NewDot side + */ +function resetStateAfterSignOut() { + Onyx.merge(ONYXKEYS.HYBRID_APP, { useNewDotSignInPage: true, + readyToShowAuthScreens: false, oldDotSignInError: null, oldDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED, newDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED, }); } +/* + * Starts OldDot sign-in flow + */ function startOldDotSignIn(autoGeneratedLogin: string, autoGeneratedPassword: string) { Onyx.merge(ONYXKEYS.HYBRID_APP, { oldDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.STARTED, @@ -52,6 +83,9 @@ function startOldDotSignIn(autoGeneratedLogin: string, autoGeneratedPassword: st NativeModules.HybridAppModule.signInToOldDot(autoGeneratedLogin, autoGeneratedPassword); } +/* + * Retries sign-in on OldDot side + */ function retryOldDotSignInAfterFailure(autoGeneratedLogin: string, autoGeneratedPassword: string) { Onyx.merge(ONYXKEYS.HYBRID_APP, { oldDotSignInError: null, @@ -61,6 +95,9 @@ function retryOldDotSignInAfterFailure(autoGeneratedLogin: string, autoGenerated NativeModules.HybridAppModule.signInToOldDot(autoGeneratedLogin, autoGeneratedPassword); } +/* + * Updates Onyx state after successful OldDot sign-in + */ function finishOldDotSignIn(errorMessage: string | null) { Onyx.merge(ONYXKEYS.HYBRID_APP, { oldDotSignInError: errorMessage, @@ -68,6 +105,9 @@ function finishOldDotSignIn(errorMessage: string | null) { }); } +/* + * Updates Onyx state after start of React Native runtime based on initial `useNewDotSignInPage` value + */ function prepareHybridAppAfterTransitionToNewDot(hybridApp: HybridApp) { if (hybridApp?.useNewDotSignInPage) { return Onyx.merge(ONYXKEYS.HYBRID_APP, { @@ -79,6 +119,7 @@ function prepareHybridAppAfterTransitionToNewDot(hybridApp: HybridApp) { }); } + // When we transition with useNewDotSignInPage === false, it means that we're already authenticated on NewDot side. return Onyx.merge(ONYXKEYS.HYBRID_APP, { ...hybridApp, readyToShowAuthScreens: true, @@ -87,15 +128,13 @@ function prepareHybridAppAfterTransitionToNewDot(hybridApp: HybridApp) { export { parseHybridAppSettings, - setOldDotSignInError, setReadyToShowAuthScreens, - setUseNewDotSignInPage, - setLoggedOutFromOldDot, setNewDotSignInState, setOldDotSignInState, - resetHybridAppSignInState, + resetSignInFlow, retryOldDotSignInAfterFailure, finishOldDotSignIn, startOldDotSignIn, prepareHybridAppAfterTransitionToNewDot, + resetStateAfterSignOut, }; diff --git a/src/libs/actions/Session/index.ts b/src/libs/actions/Session/index.ts index 1f0e0f5a8ee..c89d9fb76ff 100644 --- a/src/libs/actions/Session/index.ts +++ b/src/libs/actions/Session/index.ts @@ -39,7 +39,6 @@ import * as SessionUtils from '@libs/SessionUtils'; import {clearSoundAssetsCache} from '@libs/Sound'; import Timers from '@libs/Timers'; import {hideContextMenu} from '@pages/home/report/ContextMenu/ReportActionContextMenu'; -import * as App from '@userActions/App'; import {KEYS_TO_PRESERVE, openApp} from '@userActions/App'; import {KEYS_TO_PRESERVE_DELEGATE_ACCESS} from '@userActions/Delegate'; import * as Device from '@userActions/Device'; @@ -504,10 +503,6 @@ function signInAfterTransitionFromOldDot(route: Route, hybridAppSettings: string } return Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS); }) - .then(() => { - HybridAppActions.setUseNewDotSignInPage(!!hybridApp.useNewDotSignInPage); - HybridAppActions.setLoggedOutFromOldDot(!!hybridApp.loggedOutFromOldDot); - }) .catch((error) => { Log.hmmm('[HybridApp] Initialization of HybridApp has failed. Forcing transition', {error}); }) diff --git a/src/libs/actions/SignInRedirect.ts b/src/libs/actions/SignInRedirect.ts index 3042ddda684..48734e79456 100644 --- a/src/libs/actions/SignInRedirect.ts +++ b/src/libs/actions/SignInRedirect.ts @@ -1,8 +1,9 @@ +import {NativeModules} from 'react-native'; import Onyx from 'react-native-onyx'; import * as ErrorUtils from '@libs/ErrorUtils'; import type {OnyxKey} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; -import {setUseNewDotSignInPage} from './HybridApp'; +import * as HybridAppActions from './HybridApp'; import * as Policy from './Policy/Policy'; let currentIsOffline: boolean | undefined; @@ -39,7 +40,10 @@ function clearStorageAndRedirect(errorMessage?: string): Promise { // `Onyx.clear` reinitializes the Onyx instance with initial values so use `Onyx.merge` instead of `Onyx.set` Onyx.merge(ONYXKEYS.SESSION, {errors: ErrorUtils.getMicroSecondOnyxErrorWithMessage(errorMessage)}); - setUseNewDotSignInPage(true); + + if (NativeModules.HybridAppModule) { + HybridAppActions.resetStateAfterSignOut(); + } }); } diff --git a/src/pages/settings/InitialSettingsPage.tsx b/src/pages/settings/InitialSettingsPage.tsx index 05d9e240e97..61206e84695 100755 --- a/src/pages/settings/InitialSettingsPage.tsx +++ b/src/pages/settings/InitialSettingsPage.tsx @@ -266,7 +266,7 @@ function InitialSettingsPage({currentUserPersonalDetails}: InitialSettingsPagePr HybridAppActions.retryOldDotSignInAfterFailure(credentials.autoGeneratedLogin, credentials.autoGeneratedPassword); } else { signOutAndRedirectToSignIn(undefined, undefined, false); - HybridAppActions.resetHybridAppSignInState(); + HybridAppActions.resetStateAfterSignOut(); } }, } @@ -302,7 +302,7 @@ function InitialSettingsPage({currentUserPersonalDetails}: InitialSettingsPagePr icon: Expensicons.Exit, action: () => { signOut(false); - HybridAppActions.resetHybridAppSignInState(); + HybridAppActions.resetStateAfterSignOut(); }, }, ], diff --git a/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.tsx b/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.tsx index 4157c51ce12..9dd5082c08c 100755 --- a/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.tsx +++ b/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.tsx @@ -1,8 +1,8 @@ import {useIsFocused} from '@react-navigation/native'; import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useState} from 'react'; import type {ForwardedRef} from 'react'; -import {View} from 'react-native'; -import Onyx, {useOnyx} from 'react-native-onyx'; +import {NativeModules, View} from 'react-native'; +import {useOnyx} from 'react-native-onyx'; import Button from '@components/Button'; import SafariFormWrapper from '@components/Form/SafariFormWrapper'; import FormHelpMessage from '@components/FormHelpMessage'; @@ -27,7 +27,6 @@ import ChangeExpensifyLoginLink from '@pages/signin/ChangeExpensifyLoginLink'; import Terms from '@pages/signin/Terms'; import * as HybridAppActions from '@userActions/HybridApp'; import * as SessionActions from '@userActions/Session'; -import {signOut} from '@userActions/Session'; import * as User from '@userActions/User'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; @@ -160,9 +159,10 @@ function BaseValidateCodeForm({autoComplete, isUsingRecoveryCode, setIsUsingReco * Trigger the reset validate code flow and ensure the 2FA input field is reset to avoid it being permanently hidden */ const resendValidateCode = () => { - HybridAppActions.setOldDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED); - HybridAppActions.setNewDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED); - HybridAppActions.setOldDotSignInError(null); + if (NativeModules.HybridAppModule) { + HybridAppActions.resetSignInFlow(); + } + User.resendValidateCode(credentials?.login ?? ''); inputValidateCodeRef.current?.clear(); // Give feedback to the user to let them know the email was sent so that they don't spam the button. @@ -184,13 +184,12 @@ function BaseValidateCodeForm({autoComplete, isUsingRecoveryCode, setIsUsingReco * Clears local and Onyx sign in states */ const clearSignInData = useCallback(() => { - clearLocalSignInData(); - if (session?.authToken) { - signOut(); - Onyx.set(ONYXKEYS.SESSION, null); + if (NativeModules.HybridAppModule && session?.authToken) { + HybridAppActions.resetSignInFlow(true); } + + clearLocalSignInData(); SessionActions.clearSignInData(); - HybridAppActions.resetHybridAppSignInState(); }, [clearLocalSignInData, session?.authToken]); useImperativeHandle(forwardedRef, () => ({ @@ -247,17 +246,8 @@ function BaseValidateCodeForm({autoComplete, isUsingRecoveryCode, setIsUsingReco * Check that all the form fields are valid, then trigger the submit callback */ const validateAndSubmitForm = useCallback(() => { - if (session?.authToken) { - HybridAppActions.setOldDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.WAITING_FOR_SIGN_OUT); - HybridAppActions.setNewDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.WAITING_FOR_SIGN_OUT); - signOut(); - Onyx.merge(ONYXKEYS.SESSION, { - authToken: null, - }).then(() => { - HybridAppActions.setOldDotSignInError(null); - HybridAppActions.setOldDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.NOT_STARTED); - HybridAppActions.setNewDotSignInState(CONST.HYBRID_APP_SIGN_IN_STATE.STARTED); - }); + if (NativeModules.HybridAppModule && session?.authToken) { + HybridAppActions.resetSignInFlow(true); } if (account?.isLoading) {