[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 <[email protected]>
Expensify · Nov 28, 2024 · fb82b65 · fb82b65
1 parent 50c3fed
commit fb82b65
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 55 deletions.
diff --git a/src/libs/HybridApp.ts b/src/libs/HybridApp.ts
@@ -14,16 +14,14 @@ let currentTryNewDot: OnyxEntry<TryNewDot>;
 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<HybridApp>, tryNewDot: OnyxEntry<TryNewDot>) {
+function handleChangeInHybridAppSignInFlow(hybridApp: OnyxEntry<HybridApp>, tryNewDot: OnyxEntry<TryNewDot>) {
     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
@@ -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
@@ -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<typeof CONST.HYBRID_APP_SIGN_IN_STATE>) {
     Onyx.merge(ONYXKEYS.HYBRID_APP, {newDotSignInState});
 }
 
+/*
+ * Changes OldDot sign-in state
+ */
 function setOldDotSignInState(oldDotSignInState: ValueOf<typeof CONST.HYBRID_APP_SIGN_IN_STATE>) {
     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,13 +95,19 @@ 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,
         oldDotSignInState: CONST.HYBRID_APP_SIGN_IN_STATE.FINISHED,
     });
 }
 
+/*
+ * 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
@@ -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
@@ -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<void> {
 
         // `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
@@ -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
@@ -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) {