feat(cad): Fast follows to CAD/pair redirects

This commit: * Tweaks the 'Signed in successfully' success banner image/spacing/conditional display * Redirects signed-in desktop CAD users to /pair * Redirects end flow of inline recovery key to /pair * Updates some tests to waitForURL due to some local flakiness fixes FXA-10466 Co-authored-by: Lauren Zugai <[email protected]>
mozilla · Sep 26, 2024 · fcb5370 · fcb5370
1 parent a8156e5
commit fcb5370
Show file tree

Hide file tree

Showing 16 changed files with 49 additions and 46 deletions.
diff --git a/packages/functional-tests/tests/misc/recoveryKeyPromo.spec.ts b/packages/functional-tests/tests/misc/recoveryKeyPromo.spec.ts
@@ -150,7 +150,7 @@ test.describe('recovery key promo', () => {
       await inlineRecoveryKey.fillOutHint('hint');
       await inlineRecoveryKey.clickFinish();
 
-      await expect(connectAnotherDevice.header).toBeEnabled();
+      await expect(connectAnotherDevice.fxaConnected).toBeEnabled();
     });
 
     test('can setup recovery key inline after email code', async ({
@@ -187,8 +187,8 @@ test.describe('recovery key promo', () => {
       await inlineRecoveryKey.fillOutHint('hint');
       await inlineRecoveryKey.clickFinish();
 
-      await page.waitForURL(/connect_another_device/);
-      await expect(connectAnotherDevice.header).toBeAttached();
+      await page.waitForURL(/pair/);
+      await expect(connectAnotherDevice.fxaConnected).toBeEnabled();
     });
 
     test('can setup recovery key inline after 2FA', async ({
@@ -235,8 +235,8 @@ test.describe('recovery key promo', () => {
       await inlineRecoveryKey.fillOutHint('hint');
       await inlineRecoveryKey.clickFinish();
 
-      await page.waitForURL(/connect_another_device/);
-      await expect(connectAnotherDevice.header).toBeEnabled();
+      await page.waitForURL(/pair/);
+      await expect(connectAnotherDevice.fxaConnected).toBeEnabled();
 
       await settings.goto();
       await settings.disconnectTotp(); // Required before teardown
@@ -266,10 +266,10 @@ test.describe('recovery key promo', () => {
       await inlineRecoveryKey.clickDoItLater();
 
       // User taken to connect another device page
-      await page.waitForURL(/connect_another_device/);
-      await expect(connectAnotherDevice.header).toBeEnabled();
+      await page.waitForURL(/pair/);
+      await expect(connectAnotherDevice.fxaConnected).toBeEnabled();
 
-      await connectAnotherDevice.startBrowsingButton.click();
+      await connectAnotherDevice.clickNotNowPair();
 
       await expect(settings.settingsHeading).toBeVisible();
       await expect(settings.recoveryKey.status).toHaveText('Not Set');

diff --git a/packages/functional-tests/tests/oauth/oauthPromptNone.spec.ts b/packages/functional-tests/tests/oauth/oauthPromptNone.spec.ts
@@ -33,6 +33,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.signInPromptNone();
 
       //Verify error message
+      await page.waitForURL(/authorization/);
       await expect(page.getByText('User is not signed in')).toBeVisible();
     });
 
@@ -118,7 +119,8 @@ test.describe('severity-1 #smoke', () => {
       await page.goto(`${target.relierUrl}/?${query.toString()}`);
       await relier.signInPromptNone();
 
-      //Verify error message
+      // Verify error message
+      await page.waitForURL(/authorization/);
       await expect(page.getByText('User is not signed in')).toBeVisible();
     });
 

diff --git a/packages/functional-tests/tests/oauth/signin.spec.ts b/packages/functional-tests/tests/oauth/signin.spec.ts
@@ -118,6 +118,7 @@ test.describe('severity-1 #smoke', () => {
       await signup.fillOutEmailForm(email);
       await signup.fillOutSignupForm(password, '21');
       // Dont verify account and attempt to login via relier
+      await page.waitForURL(/confirm_signup_code/);
       await expect(page).toHaveURL(/confirm_signup_code/);
       await relier.goto();
       await relier.clickEmailFirst();

diff --git a/packages/functional-tests/tests/postVerify/syncForcePasswordChange.spec.ts b/packages/functional-tests/tests/postVerify/syncForcePasswordChange.spec.ts
@@ -40,7 +40,7 @@ test.describe('severity-2 #smoke', () => {
       credentials.password = newPassword;
 
       // Verify logged in on connect another device page
-      await expect(connectAnotherDevice.header).toBeVisible();
+      await expect(connectAnotherDevice.fxaConnected).toBeEnabled();
     });
   });
 });
diff --git a/packages/functional-tests/tests/react-conversion/oauthSignin.spec.ts b/packages/functional-tests/tests/react-conversion/oauthSignin.spec.ts
@@ -25,7 +25,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickEmailFirst();
 
-      await expect(page).toHaveURL(/oauth\//);
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -47,7 +47,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickEmailFirst();
 
-      await expect(page).toHaveURL(/oauth\//);
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -64,6 +64,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.clickEmailFirst();
 
       // wait for navigation
+      await page.waitForURL(/oauth\/signin/);
       await expect(page).toHaveURL(/oauth\/signin/);
 
       // reload page with React experiment params
@@ -89,7 +90,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickEmailFirst();
 
-      await expect(page).toHaveURL(/oauth\//);
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -106,6 +107,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.clickEmailFirst();
 
       // wait for navigation
+      await page.waitForURL(/oauth\/signin/);
       await expect(page).toHaveURL(/oauth\/signin/);
 
       // reload page with React experiment params
@@ -124,7 +126,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickEmailFirst();
 
-      await expect(page).toHaveURL(/oauth\//);
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -155,7 +157,8 @@ test.describe('severity-1 #smoke', () => {
 
       await relier.goto();
       await relier.clickEmailFirst();
-      await expect(page).toHaveURL(/oauth\//);
+
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -199,6 +202,8 @@ test.describe('severity-1 #smoke', () => {
 
       await relier.goto();
       await relier.clickEmailFirst();
+
+      await page.waitForURL(/oauth\/signin/);
       await expect(page).toHaveURL(/oauth\/signin/);
 
       // reload page with React experiment params
@@ -238,7 +243,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickChooseFlow();
 
-      await expect(page).toHaveURL(/oauth\//);
+      await page.waitForURL(/oauth\//);
 
       // reload page with React experiment params
       await page.goto(
@@ -260,6 +265,7 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickChooseFlow();
 
+      await page.waitForURL(/oauth\/signin/);
       await expect(page).toHaveURL(/oauth\/signin/);
 
       // reload page with React experiment params

diff --git a/packages/functional-tests/tests/react-conversion/oauthSignup.spec.ts b/packages/functional-tests/tests/react-conversion/oauthSignup.spec.ts
@@ -32,9 +32,9 @@ test.describe('severity-1 #smoke', () => {
       const { email, password } =
         testAccountTracker.generateSignupAccountDetails();
 
-      relier.goto();
+      await relier.goto();
 
-      relier.clickEmailFirst();
+      await relier.clickEmailFirst();
 
       // wait for navigation
       await expect(page).toHaveURL(/oauth\//);
@@ -66,9 +66,9 @@ test.describe('severity-1 #smoke', () => {
       const { email, password } =
         testAccountTracker.generateSignupAccountDetails();
 
-      relier.goto();
+      await relier.goto();
 
-      relier.clickEmailFirst();
+      await relier.clickEmailFirst();
 
       // wait for navigation, and get search params
       await page.waitForURL(/oauth\//);

diff --git a/packages/functional-tests/tests/react-conversion/signup.spec.ts b/packages/functional-tests/tests/react-conversion/signup.spec.ts
@@ -140,7 +140,7 @@ test.describe('severity-2 #smoke', () => {
       await expect(signup.cannotCreateAccountHeading).toBeVisible();
     });
 
-    test('signup via product page and redirect after confirm', async ({
+    test('signup via product subscription page and redirect after confirm', async ({
       page,
       target,
       pages: { confirmSignupCode, relier, settings, signup, subscribe },

diff --git a/packages/fxa-content-server/app/images/icon-circle-check-outline.svg b/packages/fxa-content-server/app/images/icon-circle-check-outline.svg
diff --git a/packages/fxa-content-server/app/scripts/templates/pair/index.mustache b/packages/fxa-content-server/app/scripts/templates/pair/index.mustache
@@ -2,7 +2,10 @@
 <div class="card-base">
   <header class="relative">
     {{#showSuccessMessage}}
-      <h2 id="fxa-connected-heading" class="w-full text-md font-bold p-3 my-3 rounded bg-green-500 text-gray-900 text-center">{{#t}}Signed in successfully!{{/t}}</h2>
+      <div id="fxa-connected-heading"
+          class="w-full text-md font-bold p-3 mb-5 rounded bg-green-500 text-gray-900 flex items-center justify-center before:content-icon-circle-check-outline before:flex">
+        <span class="ps-3 align-middle">{{#t}}Signed in successfully!{{/t}}</span>
+      </div>
     {{/showSuccessMessage}}
 
     {{#needsMobileConfirmed}}

diff --git a/packages/fxa-content-server/app/scripts/views/connect_another_device.js b/packages/fxa-content-server/app/scripts/views/connect_another_device.js
@@ -59,13 +59,8 @@ const ConnectAnotherDeviceView = FormView.extend({
       this.navigate('/settings');
     }
 
-    // If users are signed in, directly access this page (no query params)
-    // on desktop, redirect them
-    if (
-      this._isSignedIn() &&
-      window.location.search === '' &&
-      !this.getUserAgent().isMobile()
-    ) {
+    // If users are signed in on desktop and access this page, redirect them
+    if (this._isSignedIn() && !this.getUserAgent().isMobile()) {
       this.navigate('/pair');
     }
   },

diff --git a/packages/fxa-content-server/app/scripts/views/pair/index.js b/packages/fxa-content-server/app/scripts/views/pair/index.js
@@ -138,8 +138,9 @@ class PairIndexView extends FormView {
 
   showSuccessMessage() {
     return (
-      !!this.model.get('showSuccessMessage') ||
-      !!this.getSearchParam('showSuccessMessage')
+      !this.model.get('needsMobileConfirmed') &&
+      (!!this.model.get('showSuccessMessage') ||
+        !!this.getSearchParam('showSuccessMessage'))
     );
   }
 }

diff --git a/packages/fxa-content-server/tailwind.config.js b/packages/fxa-content-server/tailwind.config.js
@@ -36,6 +36,9 @@ config.theme.extend = {
   content: {
     ...config.theme.extend.content,
     'circle-check': "inline('../images/circle-check.svg')",
+    // Note, this is also used in fxa-settings but as an inlined SVG
+    'icon-circle-check-outline':
+      "inline('../images/icon-circle-check-outline.svg')",
     lock: "inline('../images/icon-lock.svg')",
     alert: "inline('../images/icon-warning-red-50.svg')",
     'check-blue': "inline('../images/icon-check-blue-50.svg')",

diff --git a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/index.test.tsx b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/index.test.tsx
@@ -61,16 +61,12 @@ describe('InlineRecoveryKeySetup', () => {
     });
   });
 
-  it('clicks `do it later` navigates to `connect_another_device`', async () => {
+  it('clicks `do it later` navigates to `pair`', async () => {
     renderWithLocalizationProvider(<Subject />);
     await act(async () => {
       fireEvent.click(await screen.findByText('Do it later'));
     });
-    expect(ReactUtils.hardNavigate).toHaveBeenCalledWith(
-      '/connect_another_device',
-      {},
-      true
-    );
+    expect(ReactUtils.hardNavigate).toHaveBeenCalledWith('/pair', {}, true);
     expect(
       localStorage.getItem(
         Constants.DISABLE_PROMO_ACCOUNT_RECOVERY_KEY_DO_IT_LATER

diff --git a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/index.tsx b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/index.tsx
@@ -35,7 +35,7 @@ export const InlineRecoveryKeySetup = ({
     // We do a hard navigate because this page is still in the content server, this
     // also keeps all query params so that correct metrics are emitted
     // but does not show the signed into FF success message
-    hardNavigate('/connect_another_device', {}, true);
+    hardNavigate('/pair', {}, true);
     return <></>;
   };
 
@@ -52,8 +52,7 @@ export const InlineRecoveryKeySetup = ({
             <RecoveryKeySetupHint
               {...{ viewName }}
               navigateForward={() => {
-                // Navigate to CAD without success messaging
-                hardNavigate('/connect_another_device', {}, true);
+                hardNavigate('/pair', {}, true);
               }}
               updateRecoveryKeyHint={updateRecoveryHintHandler}
             />

diff --git a/packages/fxa-settings/src/pages/Signin/CompleteSignin/container.test.tsx b/packages/fxa-settings/src/pages/Signin/CompleteSignin/container.test.tsx
@@ -83,11 +83,7 @@ describe('CompleteSignin container', () => {
 
       expect(screen.getByText('Validating sign-in…')).toBeInTheDocument();
       await waitFor(() => {
-        expect(ReactUtils.hardNavigate).toHaveBeenCalledWith(
-          '/connect_another_device',
-          {},
-          true
-        );
+        expect(ReactUtils.hardNavigate).toHaveBeenCalledWith('/pair', {}, true);
       });
     });
 

diff --git a/packages/fxa-settings/src/pages/Signin/CompleteSignin/container.tsx b/packages/fxa-settings/src/pages/Signin/CompleteSignin/container.tsx
@@ -74,7 +74,7 @@ const CompleteSigninContainer = (_: RouteComponentProps) => {
     // TODO in FXA-9132 - Add metrics event
     // Backbone had 'verification.success' and 'signin.success';
 
-    hardNavigate('/connect_another_device', {}, true);
+    hardNavigate('/pair', {}, true);
   };
 
   if (validationError) {