Skip to content

Commit

Permalink
feat(settings): Rollout new account recovery flow at 15%
Browse files Browse the repository at this point in the history
Because:

* We want to rollout the new account recovery key creation flow to 15% of users before rolling out to 100% on prod.

This commit:

* Add NewRecoveryKeyUI experiment with 15% rollout
* Passes experiment group to Settings App, allows forceExperiment
* Conditionally render new UI if user is in treatment group AND feature flag is turned on
* Update related tests

Closes #FXA-8030

Co-authored-by: Lauren Zugai <[email protected]>
  • Loading branch information
2 people authored and julianpoy committed Jul 20, 2023
1 parent 0d6f1d0 commit 47309f6
Show file tree
Hide file tree
Showing 20 changed files with 170 additions and 42 deletions.
8 changes: 6 additions & 2 deletions packages/functional-tests/pages/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ export abstract class BaseLayout {
return `${this.baseUrl}/${this.path}`;
}

goto(waitUntil: 'networkidle' | 'domcontentloaded' | 'load' = 'load') {
return this.page.goto(this.url, { waitUntil });
goto(
waitUntil: 'networkidle' | 'domcontentloaded' | 'load' = 'load',
query?: string
) {
const url = query ? `${this.url}?${query}` : this.url;
return this.page.goto(url, { waitUntil });
}

screenshot() {
Expand Down
4 changes: 2 additions & 2 deletions packages/functional-tests/pages/settings/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ export abstract class SettingsLayout extends BaseLayout {
return this.page.locator('[data-testid=drop-down-avatar-menu]');
}

goto() {
return super.goto('load');
goto(query?: string) {
return super.goto('load', query);
}

async alertBarText() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async function addAccountRecoveryKeyFlow({
credentials,
pages: { settings, recoveryKey },
}) {
await settings.goto();
await settings.goto('isInRecoveryKeyExperiment=true');
await settings.recoveryKey.clickCreate();
await recoveryKey.clickStart();
await recoveryKey.setPassword(credentials.password);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ test.describe('recovery key react', () => {
await settings.goto();
let status = await settings.recoveryKey.statusText();
expect(status).toEqual('Not Set');
await settings.recoveryKey.clickCreate();

// Check which account recovery key generation flow to use (based on feature flag)
// TODO in FXA-7419 - remove the condition and else block that goes through the old key generation flow
if (config.featureFlags.showRecoveryKeyV2 === true) {
await settings.goto('isInRecoveryKeyExperiment=true');
await settings.recoveryKey.clickCreate();
// View 1/4 info
await recoveryKey.clickStart();
// View 2/4 confirm password and generate key
Expand All @@ -46,6 +47,7 @@ test.describe('recovery key react', () => {
await recoveryKey.setHint(hint);
await recoveryKey.clickFinish();
} else {
await settings.recoveryKey.clickCreate();
await recoveryKey.setPassword(credentials.password);
await recoveryKey.submit();

Expand Down Expand Up @@ -91,7 +93,7 @@ test.describe('recovery key react', () => {
await resetPasswordReact.fillEmailToResetPwd(credentials.email);
await resetPasswordReact.confirmResetPasswordHeadingVisible();

// We need to append `&showReactApp=true` to reset link inorder to enroll in reset password experiment
// We need to append `&showReactApp=true` to reset link in order to enroll in reset password experiment
let link = await target.email.waitForEmail(
credentials.email,
EmailType.recovery,
Expand Down
2 changes: 1 addition & 1 deletion packages/functional-tests/tests/settings/password.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ test.describe('severity-1 #smoke', () => {
// Create recovery key
// TODO in FXA-7419 - remove condition and only keep new recovery key flow (remove content of else block)
if (config.featureFlags.showRecoveryKeyV2 === true) {
await settings.goto();
await settings.goto('isInRecoveryKeyExperiment=true');

await settings.recoveryKey.clickCreate();
// View 1/4 info
Expand Down
4 changes: 3 additions & 1 deletion packages/functional-tests/tests/settings/recoveryKey.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ test.describe('new recovery key test', () => {
const config = await login.getConfig();
test.skip(config.featureFlags.showRecoveryKeyV2 !== true);

await settings.goto();
await settings.goto('isInRecoveryKeyExperiment=true');
let status = await settings.recoveryKey.statusText();
expect(status).toEqual('Not Set');
await settings.recoveryKey.clickCreate();
Expand Down Expand Up @@ -355,6 +355,7 @@ test.describe('new recovery key test', () => {
page,
pages: { settings, recoveryKey, login },
}) => {
await settings.goto('isInRecoveryKeyExperiment=true');
// Create new recovery key
await settings.recoveryKey.clickCreate();
// View 1/4 info
Expand Down Expand Up @@ -510,6 +511,7 @@ test.describe('new recovery key test', () => {
});

test('revoke recovery key', async ({ pages: { settings } }) => {
await settings.goto('isInRecoveryKeyExperiment=true');
await settings.recoveryKey.clickDelete();
await settings.clickModalConfirm();
await settings.waitForAlertBar();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const experimentGroupingRules = [
require('./push'),
require('./third-party-auth'),
require('./generalized-react-app'),
require('./new-recovery-key-UI'),
].map((ExperimentGroupingRule) => new ExperimentGroupingRule());

class ExperimentChoiceIndex {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

/**
* Feature flag for new account recovery key UI.
*
*/
'use strict';

const BaseGroupingRule = require('./base');

const GROUPS = [
'control',
// Treatment branch is the new account recovery key creation flow
'treatment',
];

// This experiment is disabled by default. If you would like to go through
// the flow, load email-first screen and append query params
// `?forceExperiment=newRecoveryKeyUI&forceExperimentGroup=treatment`
const ROLLOUT_RATE = 0.15;

module.exports = class NewRecoveryKeyUI extends BaseGroupingRule {
constructor() {
super();
this.name = 'newRecoveryKeyUI';
this.groups = GROUPS;
this.rolloutRate = ROLLOUT_RATE;
}

/**
* Enable new recovery key creation flow if user is in the treatment group.
*
* @param {Object} subject data used to decide
* @returns {Any}
*/
choose(subject = {}) {
let choice = false;

if (this.bernoulliTrial(this.rolloutRate, subject.uniqueUserId)) {
choice = this.uniformChoice(GROUPS, subject.uniqueUserId);
}

return choice;
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import ExperimentMixin from '../views/mixins/experiment-mixin';

export default {
dependsOn: [ExperimentMixin],

isInNewRecoveryKeyUIExperiment() {
const experimentGroup =
this.getAndReportExperimentGroup('newRecoveryKeyUI');
return experimentGroup === 'treatment';
},
};
6 changes: 5 additions & 1 deletion packages/fxa-content-server/app/scripts/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import VerificationReasons from './verification-reasons';
import WouldYouLikeToSync from '../views/would_you_like_to_sync';
import { isAllowed } from 'fxa-shared/configuration/convict-format-allow-list';
import ReactExperimentMixin from './generalized-react-app-experiment-mixin';
import RecoveryKeyExperimentMixin from './new-recovery-key-UI-experiment-mixin';
import { getClientReactRouteGroups } from '../../../server/lib/routes/react-app/route-groups-client';

const NAVIGATE_AWAY_IN_MOBILE_DELAY_MS = 75;
Expand Down Expand Up @@ -120,7 +121,7 @@ let Router = Backbone.Router.extend({
},
});

Cocktail.mixin(Router, ReactExperimentMixin);
Cocktail.mixin(Router, ReactExperimentMixin, RecoveryKeyExperimentMixin);

Router = Router.extend({
routes: {
Expand Down Expand Up @@ -389,6 +390,8 @@ Router = Router.extend({
return this.navigateAway(redirectUrl);
}

const isInRecoveryKeyExperiment = this.isInNewRecoveryKeyUIExperiment();

// All other flows should redirect to the settings page
const settingsEndpoint = '/settings';
const settingsLink = `${settingsEndpoint}${Url.objToSearchString({
Expand All @@ -400,6 +403,7 @@ Router = Router.extend({
isSampledUser,
service,
uniqueUserId,
...(isInRecoveryKeyExperiment && { isInRecoveryKeyExperiment }),
})}`;
this.navigateAway(settingsLink);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import sinon from 'sinon';

describe('lib/experiments/grouping-rules/index', () => {
it('EXPERIMENT_NAMES is exported', () => {
assert.lengthOf(ExperimentGroupingRules.EXPERIMENT_NAMES, 6);
assert.lengthOf(ExperimentGroupingRules.EXPERIMENT_NAMES, 7);
});

describe('choose', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { assert } from 'chai';
import Experiment from 'lib/experiments/grouping-rules/new-recovery-key-UI';

describe('lib/experiments/grouping-rules/new-recovery-key-UI', () => {
let experiment;

beforeEach(() => {
experiment = new Experiment();
});

describe('choose', () => {
it('returns treatment if valid clientId', () => {
const rules = experiment.groups;
assert.isTrue(
rules.include(
experiment.choose({
experimentGroupingRules: { choose: () => experiment.name },
uniqueUserId: 'user-id',
})
)
);
});

it('returns false if rollout 0%', () => {
experiment.rolloutRate = 0;
assert.isFalse(
experiment.choose({
experimentGroupingRules: { choose: () => experiment.name },
uniqueUserId: 'user-id',
})
);
});
});
});
9 changes: 7 additions & 2 deletions packages/fxa-settings/src/components/App/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const App = ({
}: { flowQueryParams: QueryParams } & RouteComponentProps) => {
const [isSignedIn, setIsSignedIn] = useState<boolean>();

const { showReactApp } = flowQueryParams;
const { showReactApp, isInRecoveryKeyExperiment } = flowQueryParams;
const { loading, error } = useInitialState();
const account = useAccount();
const [email, setEmail] = useState<string>();
Expand All @@ -63,6 +63,11 @@ export const App = ({

const { metricsEnabled } = account;

// TODO Remove feature flag and experiment logic in FXA-7419
const showRecoveryKeyV2 = !!(
config.showRecoveryKeyV2 && isInRecoveryKeyExperiment === 'true'
);

useEffect(() => {
Metrics.init(metricsEnabled || !isSignedIn, flowQueryParams);
if (metricsEnabled) {
Expand Down Expand Up @@ -227,7 +232,7 @@ export const App = ({
<ThirdPartyAuthCallback path="/post_verify/third_party_auth/callback/*" />
</>
)}
<Settings path="/settings/*" />
<Settings path="/settings/*" {...{ showRecoveryKeyV2 }} />
</ScrollToTop>
</Router>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import React, { useState } from 'react';
import { RouteComponentProps, useNavigate } from '@reach/router';
import { RouteComponentProps, useLocation, useNavigate } from '@reach/router';
import { HomePath } from '../../../constants';
import { usePageViewEvent } from '../../../lib/metrics';
import { useAccount, useFtlMsgResolver } from '../../../models';
Expand All @@ -23,6 +23,7 @@ export enum RecoveryKeyAction {

export const PageRecoveryKeyCreate = (props: RouteComponentProps) => {
usePageViewEvent(viewName);
const location = useLocation();

const { recoveryKey } = useAccount();
const ftlMsgResolver = useFtlMsgResolver();
Expand All @@ -48,7 +49,7 @@ export const PageRecoveryKeyCreate = (props: RouteComponentProps) => {

// TODO: Remove feature flag param in FXA-7419
const navigateBackward = () => {
navigate(HomePath);
navigate(`${HomePath}${location.search}`);
};

const navigateForward = (e?: React.MouseEvent<HTMLElement>) => {
Expand All @@ -57,7 +58,7 @@ export const PageRecoveryKeyCreate = (props: RouteComponentProps) => {
setCurrentStep(currentStep + 1);
} else {
// TODO: Remove feature flag param in FXA-7419
navigate(HomePath);
navigate(`${HomePath}${location.search}`);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import { DeleteAccountPath } from 'fxa-settings/src/constants';
import { Localized } from '@fluent/react';
import DataCollection from '../DataCollection';

export const PageSettings = (_: RouteComponentProps) => {
export const PageSettings = ({
// This should technically never be `undefined` but because this is temporary,
// allowing `undefined` makes updating tests and stories easier.
showRecoveryKeyV2,
}: { showRecoveryKeyV2?: boolean } & RouteComponentProps) => {
const { uid } = useAccount();

Metrics.setProperties({
Expand All @@ -32,7 +36,7 @@ export const PageSettings = (_: RouteComponentProps) => {
</div>
<div className="flex-7 max-w-full">
<Profile />
<Security />
<Security {...{ showRecoveryKeyV2 }} />
<ConnectedServices />
<LinkedAccounts />
<DataCollection />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ const PwdDate = ({ passwordCreated }: { passwordCreated: number }) => {
);
};

export const Security = () => {
export const Security = ({
showRecoveryKeyV2,
}: {
showRecoveryKeyV2?: boolean;
}) => {
const { passwordCreated, hasPassword } = useAccount();
const { l10n } = useLocalization();
const localizedNotSet = l10n.getString('security-not-set', null, 'Not Set');
Expand Down Expand Up @@ -81,7 +85,7 @@ export const Security = () => {
</Localized>
<hr className="unit-row-hr" />

<UnitRowRecoveryKey />
<UnitRowRecoveryKey {...{ showRecoveryKeyV2 }} />
<hr className="unit-row-hr" />
<UnitRowTwoStepAuth />

Expand Down
Loading

0 comments on commit 47309f6

Please sign in to comment.