Skip to content

Commit 96efced

Browse files
committed
task(settings): Full production rollout of react email-first
Because: * We want to route all traffic to the react app version of the email-first pages This commit: * remove these pages from the experiment * Set fullProdRollout to true for email first routes * Update navigation to stay in react app (no hard navigation) * Update tests Closes #FXA-11221
1 parent 91bda52 commit 96efced

84 files changed

Lines changed: 693 additions & 824 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/functional-tests/pages/signin.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,14 @@ export class SigninPage extends BaseLayout {
3838
return this.page.getByRole('button', { name: 'Confirm' });
3939
}
4040

41+
// Use /Continue with.*Apple/ because of hidden bidi Unicode characters around "Apple" in its accessible name.
4142
get continueWithAppleButton() {
42-
return this.page.getByRole('button', { name: 'Continue with Apple' });
43+
return this.page.getByRole('button', { name: /Continue with .*Apple/ });
4344
}
4445

46+
// Use /Continue with.*Google/ because of hidden bidi Unicode characters around "Google" in its accessible name.
4547
get continueWithGoogleButton() {
46-
return this.page.getByRole('button', { name: 'Continue with Google' });
48+
return this.page.getByRole('button', { name: /Continue with .*Google/ });
4749
}
4850

4951
get emailFirstHeading() {

packages/functional-tests/tests/signin/redirect.spec.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,21 @@ test.describe('severity-2 #smoke', () => {
2727
target,
2828
page,
2929
pages: { signin },
30-
testAccountTracker,
3130
}) => {
32-
const credentials = await testAccountTracker.signUp();
33-
3431
await page.goto(
3532
`${target.contentServerUrl}/?redirect_to=javascript:alert(1)`
3633
);
37-
await signin.fillOutEmailFirstForm(credentials.email);
3834

39-
await expect(page).toHaveURL(/signin/);
40-
await expect(signin.badRequestHeading).toBeVisible();
35+
// only error message shown on screen in case of xss redirect_to
36+
await expect(
37+
page.getByRole('heading', { name: /Bad Request/ })
38+
).toBeVisible();
39+
await expect(signin.emailFirstHeading).toBeHidden();
4140
});
4241

4342
test('does not allow bogus redirect_to parameter', async ({
4443
target,
45-
pages: { page, signin },
44+
pages: { page, settings, signin },
4645
testAccountTracker,
4746
}) => {
4847
const credentials = await testAccountTracker.signUp();
@@ -54,11 +53,12 @@ test.describe('severity-2 #smoke', () => {
5453
await signin.fillOutPasswordForm(credentials.password);
5554

5655
await expect(page).not.toHaveURL(redirectTo);
56+
await expect(settings.settingsHeading).toBeVisible();
5757
});
5858

5959
test('allows valid redirect_to parameter', async ({
6060
target,
61-
pages: { page, signin },
61+
pages: { page, changePassword, signin },
6262
testAccountTracker,
6363
}) => {
6464
const credentials = await testAccountTracker.signUp();
@@ -70,6 +70,7 @@ test.describe('severity-2 #smoke', () => {
7070
await signin.fillOutPasswordForm(credentials.password);
7171

7272
await expect(page).toHaveURL(redirectTo);
73+
await expect(changePassword.changePasswordHeading).toBeVisible();
7374
});
7475
});
7576
});

packages/functional-tests/tests/syncV3/signinCached.spec.ts

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { SigninPage } from '../../pages/signin';
99

1010
test.describe('severity-2 #smoke', () => {
1111
test.describe('sync signin cached', () => {
12-
test('sign in on desktop then specify a different email on query parameter continues to cache desktop signin', async ({
12+
test('sign in on desktop then specify a different email on query parameter', async ({
1313
target,
1414
syncBrowserPages: { page, settings, signin, connectAnotherDevice },
1515
testAccountTracker,
@@ -23,30 +23,34 @@ test.describe('severity-2 #smoke', () => {
2323
);
2424
const query = { email: credentials.email };
2525
const queryParam = new URLSearchParams(query);
26-
await page.goto(
27-
`${
28-
target.contentServerUrl
29-
}?context=fx_desktop_v3&service=sync&action=email&${queryParam.toString()}`
30-
);
26+
await page.goto(`${target.contentServerUrl}?${queryParam.toString()}`);
3127

3228
//Check prefilled email
3329
await expect(signin.passwordFormHeading).toBeVisible();
3430
await expect(page.getByText(credentials.email)).toBeVisible();
3531
await signin.fillOutPasswordForm(credentials.password);
36-
await expect(page).toHaveURL(/pair/);
37-
await connectAnotherDevice.clickNotNowPair();
38-
await page.waitForURL(/settings/);
3932

4033
//Verify logged in on Settings page
4134
await expect(settings.settingsHeading).toBeVisible();
4235

43-
//Reset prefill and context
44-
await signin.clearSessionStorage();
36+
// verify that if we directly go to the index page,
37+
// the most recently signed in account is suggested
4538

46-
//Testing to make sure cached signin comes back after a refresh
47-
await page.goto(target.contentServerUrl, {
48-
waitUntil: 'load',
49-
});
39+
// however (not tested here because Playwright tests can't access browser menu)
40+
// if we instead clicked on "manage settings" from the Sync browser menu
41+
// the account settings would be loaded for the sync credential account
42+
43+
await page.goto(target.contentServerUrl);
44+
await expect(signin.cachedSigninHeading).toBeVisible();
45+
// suggests most recently signed in account
46+
await expect(page.getByText(credentials.email)).toBeVisible();
47+
await signin.signInButton.click();
48+
await expect(settings.settingsHeading).toBeVisible();
49+
50+
await settings.signOut();
51+
52+
// falls back to suggesting the account signed in to the browser
53+
await expect(signin.cachedSigninHeading).toBeVisible();
5054
await expect(page.getByText(syncCredentials.email)).toBeVisible();
5155
await signin.useDifferentAccountLink.click();
5256
await expect(signin.emailFirstHeading).toBeVisible();
@@ -63,30 +67,12 @@ test.describe('severity-2 #smoke', () => {
6367
signin,
6468
testAccountTracker
6569
);
66-
const credentials = await testAccountTracker.signUp();
67-
68-
await page.goto(target.contentServerUrl, {
69-
waitUntil: 'load',
70-
});
71-
await expect(page.getByText(syncCredentials.email)).toBeVisible();
72-
await signin.useDifferentAccountLink.click();
73-
await expect(signin.emailFirstHeading).toBeVisible();
74-
await signin.fillOutEmailFirstForm(credentials.email);
75-
await signin.fillOutPasswordForm(credentials.password);
7670

77-
//Verify logged in on Settings page
78-
await expect(settings.settingsHeading).toBeVisible();
79-
80-
//Reset prefill and context
81-
await signin.clearSessionStorage();
82-
83-
//Testing to make sure cached signin comes back after a refresh
8471
await page.goto(target.contentServerUrl, {
8572
waitUntil: 'load',
8673
});
74+
// signed in sync account suggested
8775
await expect(page.getByText(syncCredentials.email)).toBeVisible();
88-
await signin.useDifferentAccountLink.click();
89-
await expect(signin.emailFirstHeading).toBeVisible();
9076
});
9177
});
9278
});

packages/fxa-content-server/server/config/local.json-dist

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,6 @@
7171
},
7272
"rolloutRates": {
7373
"keyStretchV2": 1,
74-
"generalizedReactApp": 1
74+
"generalizedReactApp": 0
7575
}
7676
}

packages/fxa-content-server/server/lib/routes/react-app/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ const getReactRouteGroups = (showReactApp, reactRoute) => {
2525
return {
2626
emailFirstRoutes: {
2727
featureFlagOn: showReactApp.emailFirstRoutes,
28-
// the order of the routes in teh array is important. do not put '/'
28+
// the order of the routes in the array is important. do not put '/'
2929
// first.
3030
routes: reactRoute.getRoutes(['authorization', 'oauth', '/']),
31-
fullProdRollout: false,
31+
fullProdRollout: true,
3232
},
3333
simpleRoutes: {
3434
featureFlagOn: showReactApp.simpleRoutes,

packages/fxa-react/lib/test-utils/localizationProvider.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
15
import React from 'react';
26
import { render } from '@testing-library/react';
37
import AppLocalizationProvider from 'fxa-react/lib/AppLocalizationProvider';
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
export function mockWindowLocation({
6+
pathname = '/',
7+
search = '',
8+
hash = '',
9+
}: {
10+
pathname?: string;
11+
search?: string;
12+
hash?: string;
13+
}) {
14+
delete (window as any).location;
15+
16+
(window as any).location = {
17+
...window.location,
18+
assign: jest.fn(),
19+
reload: jest.fn(),
20+
replace: jest.fn(),
21+
pathname,
22+
search,
23+
hash,
24+
href: `http://localhost${pathname}${search}${hash}`,
25+
};
26+
}

packages/fxa-settings/src/components/App/index.test.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ describe('SettingsRoutes', () => {
356356
(useSession as jest.Mock).mockRestore();
357357
});
358358

359-
it('redirects to /signin if isSignedIn is false', async () => {
359+
it('redirects to email-first if isSignedIn is false', async () => {
360360
(firefox.requestSignedInUser as jest.Mock).mockResolvedValue(null);
361361
(currentAccount as jest.Mock).mockReturnValue(null);
362362
(useIntegration as jest.Mock).mockReturnValue({
@@ -436,6 +436,14 @@ describe('SettingsRoutes', () => {
436436
});
437437

438438
it('restores sync user when session is valid', async () => {
439+
// we only send a webChannel message to check for a sync user
440+
// if the userAgent is a version of Firefox
441+
Object.defineProperty(window.navigator, 'userAgent', {
442+
value:
443+
'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:118.0) Gecko/20100101 Firefox/118.0',
444+
configurable: true,
445+
});
446+
439447
(useIntegration as jest.Mock).mockReturnValue({
440448
isSync: () => true,
441449
isDesktopRelay: () => false,

0 commit comments

Comments
 (0)