Remove ONYXKEYS.CREDENTIALS - batch 1

src/HybridAppHandler.tsx

@@ -17,12 +17,14 @@ import isLoadingOnyxValue from './types/utils/isLoadingOnyxValue';
 function HybridAppHandler() {
     const {setSplashScreenState} = useSplashScreenActions();
     const [tryNewDot, tryNewDotMetadata] = useOnyx(ONYXKEYS.NVP_TRY_NEW_DOT);
+    const [credentials, credentialsMetadata] = useOnyx(ONYXKEYS.CREDENTIALS);
     const isLoadingTryNewDot = isLoadingOnyxValue(tryNewDotMetadata);
+    const isLoadingCredentials = isLoadingOnyxValue(credentialsMetadata);
 
     const finalizeTransitionFromOldDot = (hybridAppSettings: HybridAppSettings) => {
         const loggedOutFromOldDot = !!hybridAppSettings.hybridApp.loggedOutFromOldDot;
 
-        setupNewDotAfterTransitionFromOldDot(hybridAppSettings, tryNewDot).then(() => {
+        setupNewDotAfterTransitionFromOldDot(hybridAppSettings, tryNewDot, credentials).then(() => {
             if (loggedOutFromOldDot) {
                 endSpan(CONST.TELEMETRY.SPAN_APP_STARTUP);
                 endSpan(CONST.TELEMETRY.SPAN_BOOTSPLASH.ROOT);
@@ -34,7 +36,7 @@ function HybridAppHandler() {
     };
 
     useEffect(() => {
-        if (!CONFIG.IS_HYBRID_APP || isLoadingTryNewDot) {
+        if (!CONFIG.IS_HYBRID_APP || isLoadingTryNewDot || isLoadingCredentials) {
             return;
         }
 
@@ -73,7 +75,7 @@ function HybridAppHandler() {
 
             finalizeTransitionFromOldDot(hybridAppSettings);
         });
-    }, [finalizeTransitionFromOldDot, isLoadingTryNewDot, setSplashScreenState]);
+    }, [finalizeTransitionFromOldDot, isLoadingTryNewDot, isLoadingCredentials, setSplashScreenState]);
 
     return null;
 }

src/libs/actions/Session/index.ts

@@ -531,7 +531,7 @@ function callFunctionIfActionIsAllowed<TCallback extends ((...args: any[]) => an
 /**
  * Request a new validate / magic code for user to sign in via passwordless flow
  */
-function resendValidateCode(reasonParams: ResendValidateCodeParams, login = credentials.login) {
+function resendValidateCode(reasonParams: ResendValidateCodeParams, login: string | undefined) {
     const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.ACCOUNT>> = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
@@ -637,7 +637,7 @@ function buildOnyxDataToCleanUpAnonymousUser(): OnyxUpdate<typeof ONYXKEYS.PERSO
  * Creates an account for the new user and signs them into the application with the newly created account.
  *
  */
-function signUpUser(preferredLocale: Locale | undefined, hasSMSMarketingConsent?: boolean) {
+function signUpUser(login: string | undefined, preferredLocale: Locale | undefined, hasSMSMarketingConsent?: boolean) {
     const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.ACCOUNT>> = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
@@ -673,15 +673,15 @@ function signUpUser(preferredLocale: Locale | undefined, hasSMSMarketingConsent?
     ];
 
     Device.getDeviceInfoWithID().then((deviceInfo) => {
-        const params: SignUpUserParams = {email: credentials.login, preferredLocale: preferredLocale ?? null, deviceInfo};
+        const params: SignUpUserParams = {email: login, preferredLocale: preferredLocale ?? null, deviceInfo};
         if (hasSMSMarketingConsent !== undefined) {
             params.hasSMSMarketingConsent = hasSMSMarketingConsent;
         }
         API.write(WRITE_COMMANDS.SIGN_UP_USER, params, {optimisticData, successData, failureData});
     });
 }
 
-function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettings, tryNewDot?: TryNewDot) {
+function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettings, tryNewDot: TryNewDot | undefined, credentialsParam: Credentials | undefined) {
     const {hybridApp, ...newDotOnyxValues} = hybridAppSettings;
 
     const clearOnyxIfSigningIn = () => {
@@ -721,7 +721,7 @@ function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettin
 
             const stashedData = hybridApp?.delegateAccessData?.isDelegateAccess
                 ? {
-                      [ONYXKEYS.STASHED_CREDENTIALS]: credentials,
+                      [ONYXKEYS.STASHED_CREDENTIALS]: credentialsParam,
                       [ONYXKEYS.STASHED_SESSION]: session,
                   }
                 : {
@@ -742,8 +742,8 @@ function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettin
                         accountID: hybridApp?.delegateAccessData?.oldDotCurrentAccountID,
                     },
                     [ONYXKEYS.CREDENTIALS]: {
-                        autoGeneratedLogin: credentials?.autoGeneratedLogin ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedLogin,
-                        autoGeneratedPassword: credentials?.autoGeneratedPassword ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedPassword,
+                        autoGeneratedLogin: credentialsParam?.autoGeneratedLogin ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedLogin,
+                        autoGeneratedPassword: credentialsParam?.autoGeneratedPassword ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedPassword,
                     },
                 })
                     .then(() => Onyx.merge(ONYXKEYS.ACCOUNT, {primaryLogin: hybridApp?.delegateAccessData?.oldDotCurrentUserEmail}))
@@ -766,8 +766,8 @@ function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettin
                         // To avoid blocking the transition, we fall back to credentials from OldDot in this scenario.
                         // The `delegateAccessData` key is misleading in this context because, in the past, this code only handled Copilot. We are reusing the same logic here for this scenario.
                         [ONYXKEYS.CREDENTIALS]: {
-                            autoGeneratedLogin: credentials?.autoGeneratedLogin ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedLogin,
-                            autoGeneratedPassword: credentials?.autoGeneratedPassword ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedPassword,
+                            autoGeneratedLogin: credentialsParam?.autoGeneratedLogin ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedLogin,
+                            autoGeneratedPassword: credentialsParam?.autoGeneratedPassword ?? hybridApp.delegateAccessData?.oldDotAutoGeneratedPassword,
                         },
                     }),
                 )

src/pages/signin/SignUpWelcomeForm.tsx

@@ -54,7 +54,7 @@ function SignUpWelcomeForm() {
                     text={translate('welcomeSignUpForm.join')}
                     isLoading={account?.isLoading}
                     onPress={() => {
-                        signUpUser(preferredLocale, isPhoneSignup ? hasSMSMarketingConsent : undefined);
+                        signUpUser(login, preferredLocale, isPhoneSignup ? hasSMSMarketingConsent : undefined);
                         setReadyToShowAuthScreens(true);
                     }}
                     pressOnEnter

tests/actions/SessionTest.ts

@@ -28,6 +28,7 @@ import type {Credentials, Session} from '@src/types/onyx';
 
 import type {OnyxEntry} from 'react-native-onyx';
 
+import {CONST as COMMON_CONST} from 'expensify-common';
 import {openAuthSessionAsync} from 'expo-web-browser';
 import Onyx from 'react-native-onyx';
 
@@ -794,4 +795,192 @@ describe('Session', () => {
             expect(session?.signedInWithSAML).toBe(false);
         });
     });
+
+    describe('resendValidateCode', () => {
+        test('sends the login argument as the email param, independent of the CREDENTIALS Onyx cache', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            // CREDENTIALS is empty (cleared in beforeEach), so a correct email here proves the value comes from the param, not the module cache.
+            SessionUtil.resendValidateCode({reasonCode: null}, 'passed-in@expensify.com');
+            await waitForBatchedUpdates();
+
+            const call = writeSpy.mock.calls.at(0);
+            expect(call?.at(0)).toBe(WRITE_COMMANDS.REQUEST_NEW_VALIDATE_CODE);
+            expect(call?.at(1)).toEqual(expect.objectContaining({email: 'passed-in@expensify.com'}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('forwards the reasonCode from reasonParams to the API call', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.resendValidateCode({reasonCode: COMMON_CONST.VALIDATE_CODE_REASONS.SIGN_IN}, 'passed-in@expensify.com');
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(1)).toEqual(expect.objectContaining({reasonCode: COMMON_CONST.VALIDATE_CODE_REASONS.SIGN_IN}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('sends an undefined email when the login argument is undefined', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.resendValidateCode({reasonCode: null}, undefined);
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(1)).toEqual(expect.objectContaining({email: undefined}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('optimistically sets loadingForm to RESEND_VALIDATE_CODE_FORM', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.resendValidateCode({reasonCode: null}, 'passed-in@expensify.com');
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(2)).toEqual(
+                expect.objectContaining({
+                    optimisticData: expect.arrayContaining([
+                        expect.objectContaining({key: ONYXKEYS.ACCOUNT, value: expect.objectContaining({loadingForm: CONST.FORMS.RESEND_VALIDATE_CODE_FORM})}),
+                    ]),
+                }),
+            );
+
+            writeSpy.mockRestore();
+        });
+    });
+
+    describe('signUpUser', () => {
+        test('sends the login argument as the email param, independent of the CREDENTIALS Onyx cache', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            // CREDENTIALS is empty (cleared in beforeEach), so a correct email here proves the value comes from the param, not the module cache.
+            SessionUtil.signUpUser('new-user@expensify.com', undefined);
+            await waitForBatchedUpdates();
+
+            const call = writeSpy.mock.calls.at(0);
+            expect(call?.at(0)).toBe(WRITE_COMMANDS.SIGN_UP_USER);
+            expect(call?.at(1)).toEqual(expect.objectContaining({email: 'new-user@expensify.com'}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('forwards the preferredLocale to the API call', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.signUpUser('new-user@expensify.com', CONST.LOCALES.EN);
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(1)).toEqual(expect.objectContaining({preferredLocale: CONST.LOCALES.EN}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('includes hasSMSMarketingConsent when it is provided', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.signUpUser('new-user@expensify.com', undefined, true);
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(1)).toEqual(expect.objectContaining({hasSMSMarketingConsent: true}));
+
+            writeSpy.mockRestore();
+        });
+
+        test('omits hasSMSMarketingConsent when it is undefined', async () => {
+            // eslint-disable-next-line rulesdir/no-multiple-api-calls
+            const writeSpy = jest.spyOn(API, 'write').mockResolvedValue(undefined);
+
+            SessionUtil.signUpUser('new-user@expensify.com', undefined);
+            await waitForBatchedUpdates();
+
+            expect(writeSpy.mock.calls.at(0)?.at(1)).not.toHaveProperty('hasSMSMarketingConsent');
+
+            writeSpy.mockRestore();
+        });
+    });
+
+    describe('setupNewDotAfterTransitionFromOldDot', () => {
+        const buildHybridAppSettings = (isDelegateAccess: boolean) => ({
+            [ONYXKEYS.HYBRID_APP]: {
+                // false so `clearOnyxIfSigningIn` resolves without hitting redirectToSignIn
+                useNewDotSignInPage: false,
+                delegateAccessData: {
+                    isDelegateAccess,
+                    oldDotCurrentUserEmail: 'delegate@od.com',
+                    oldDotCurrentAuthToken: 'odAuthToken',
+                    oldDotCurrentEncryptedAuthToken: 'odEncryptedAuthToken',
+                    oldDotCurrentAccountID: 999,
+                    oldDotAutoGeneratedLogin: 'odAutoLogin',
+                    oldDotAutoGeneratedPassword: 'odAutoPassword',
+                },
+            },
+        });
+
+        test('writes the passed credentials into CREDENTIALS and STASHED_CREDENTIALS instead of reading the module cache', async () => {
+            // Take the imported-state branch to avoid clearing Onyx / redirecting.
+            await Onyx.set(ONYXKEYS.IS_USING_IMPORTED_STATE, true);
+            await waitForBatchedUpdates();
+
+            const onyxMultiSetSpy = jest.spyOn(Onyx, 'multiSet').mockResolvedValue(undefined);
+
+            const credentialsParam = {login: 'nd@user.com', autoGeneratedLogin: 'ndAutoLogin', autoGeneratedPassword: 'ndAutoPassword'};
+            await SessionUtil.setupNewDotAfterTransitionFromOldDot(buildHybridAppSettings(true), undefined, credentialsParam);
+            await waitForBatchedUpdates();
+
+            expect(onyxMultiSetSpy).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    [ONYXKEYS.STASHED_CREDENTIALS]: credentialsParam,
+                    [ONYXKEYS.CREDENTIALS]: {autoGeneratedLogin: 'ndAutoLogin', autoGeneratedPassword: 'ndAutoPassword'},
+                }),
+            );
+
+            onyxMultiSetSpy.mockRestore();
+        });
+
+        test('falls back to the OldDot delegate credentials when the passed credentials are undefined', async () => {
+            await Onyx.set(ONYXKEYS.IS_USING_IMPORTED_STATE, true);
+            await waitForBatchedUpdates();
+
+            const onyxMultiSetSpy = jest.spyOn(Onyx, 'multiSet').mockResolvedValue(undefined);
+
+            await SessionUtil.setupNewDotAfterTransitionFromOldDot(buildHybridAppSettings(true), undefined, undefined);
+            await waitForBatchedUpdates();
+
+            expect(onyxMultiSetSpy).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    [ONYXKEYS.CREDENTIALS]: {autoGeneratedLogin: 'odAutoLogin', autoGeneratedPassword: 'odAutoPassword'},
+                }),
+            );
+
+            onyxMultiSetSpy.mockRestore();
+        });
+
+        test('does not stash the passed credentials when the transition is not a delegate access', async () => {
+            await Onyx.set(ONYXKEYS.IS_USING_IMPORTED_STATE, true);
+            await waitForBatchedUpdates();
+
+            const onyxMultiSetSpy = jest.spyOn(Onyx, 'multiSet').mockResolvedValue(undefined);
+
+            const credentialsParam = {login: 'nd@user.com', autoGeneratedLogin: 'ndAutoLogin', autoGeneratedPassword: 'ndAutoPassword'};
+            await SessionUtil.setupNewDotAfterTransitionFromOldDot(buildHybridAppSettings(false), undefined, credentialsParam);
+            await waitForBatchedUpdates();
+
+            expect(onyxMultiSetSpy).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    [ONYXKEYS.STASHED_CREDENTIALS]: {},
+                }),
+            );
+
+            onyxMultiSetSpy.mockRestore();
+        });
+    });
 });

tests/unit/SignUpWelcomeFormTest.tsx

@@ -107,7 +107,7 @@ describe('SignUpWelcomeForm', () => {
         fireEvent.press(screen.getByLabelText('I agree to receive marketing texts from Expensify'));
         fireEvent.press(screen.getByText('Join'));
 
-        expect(signUpUserSpy).toHaveBeenCalledWith(undefined, true);
+        expect(signUpUserSpy).toHaveBeenCalledWith(PHONE_LOGIN, undefined, true);
     });
 
     it('passes hasSMSMarketingConsent=false to signUpUser when checkbox is unchecked and Join is pressed', async () => {
@@ -117,7 +117,7 @@ describe('SignUpWelcomeForm', () => {
 
         fireEvent.press(screen.getByText('Join'));
 
-        expect(signUpUserSpy).toHaveBeenCalledWith(undefined, false);
+        expect(signUpUserSpy).toHaveBeenCalledWith(PHONE_LOGIN, undefined, false);
     });
 
     it('passes hasSMSMarketingConsent=false when checkbox is checked then unchecked', async () => {
@@ -129,7 +129,7 @@ describe('SignUpWelcomeForm', () => {
         fireEvent.press(screen.getByLabelText('I agree to receive marketing texts from Expensify'));
         fireEvent.press(screen.getByText('Join'));
 
-        expect(signUpUserSpy).toHaveBeenCalledWith(undefined, false);
+        expect(signUpUserSpy).toHaveBeenCalledWith(PHONE_LOGIN, undefined, false);
     });
 
     it('omits the consent param when signing up with an email', async () => {
@@ -139,6 +139,6 @@ describe('SignUpWelcomeForm', () => {
 
         fireEvent.press(screen.getByText('Join'));
 
-        expect(signUpUserSpy).toHaveBeenCalledWith(undefined, undefined);
+        expect(signUpUserSpy).toHaveBeenCalledWith(EMAIL_LOGIN, undefined, undefined);
     });
 });