Skip to content

Commit 02cc4c2

Browse files
authored
Merge pull request #20314 from mozilla/fxa-13393
fix(settings): fix OTP engage timing and add change email glean metric
2 parents f07da59 + 79823e0 commit 02cc4c2

12 files changed

Lines changed: 136 additions & 7 deletions

File tree

packages/functional-tests/pages/signinPasswordlessCode.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,9 @@ export class SigninPasswordlessCodePage extends BaseTokenCodePage {
4242
get resendSuccessBanner() {
4343
return this.page.getByText(/A new code was sent/);
4444
}
45+
46+
get useDifferentAccountLink() {
47+
this.checkPath();
48+
return this.page.getByRole('link', { name: 'Use a different account' });
49+
}
4550
}

packages/functional-tests/tests/passwordless/signinPasswordless.spec.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,15 @@ test.describe('severity-1 #smoke', () => {
3030
await page.waitForURL(/signin_passwordless_code/);
3131
await expect(signinPasswordlessCode.heading).toBeVisible();
3232

33-
// Get OTP code from email
33+
// Click "Use a different account" to verify change email glean event,
34+
// then re-enter same email to complete the flow
35+
await signinPasswordlessCode.useDifferentAccountLink.click();
36+
await expect(page).not.toHaveURL(/signin_passwordless_code/);
37+
await target.emailClient.clear(email);
38+
await signin.fillOutEmailFirstForm(email);
39+
await page.waitForURL(/signin_passwordless_code/);
40+
41+
// Get the fresh OTP code (previous codes were cleared)
3442
const code = await target.emailClient.getPasswordlessSignupCode(email);
3543
await signinPasswordlessCode.fillOutCodeForm(code);
3644

@@ -41,6 +49,7 @@ test.describe('severity-1 #smoke', () => {
4149
gleanEventsHelper.assertEventOrder([
4250
'email_first_view',
4351
'reg_otp_view',
52+
'reg_otp_change_email',
4453
'reg_otp_submit',
4554
'reg_otp_submit_success',
4655
]);

packages/fxa-auth-server/lib/routes/passwordless.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ class PasswordlessHandler {
453453
this.statsd.increment('passwordless.sendCode.success', {
454454
...getClientServiceTags(request),
455455
isResend: String(isResend),
456+
isNewAccount: String(isNewAccount),
456457
});
457458

458459
// Record security event

packages/fxa-settings/src/components/FormVerifyCode/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export type FormVerifyCodeProps = {
4848
submitFormOnPaste?: boolean;
4949
cmsButton?: CmsButtonType;
5050
onEngageCb?: () => void;
51+
onChangeCb?: () => void;
5152
};
5253

5354
type FormData = {
@@ -66,6 +67,7 @@ const FormVerifyCode = ({
6667
submitFormOnPaste,
6768
cmsButton,
6869
onEngageCb,
70+
onChangeCb,
6971
}: FormVerifyCodeProps) => {
7072
const [isFocused, setIsFocused] = useState<boolean>(false);
7173
const [isSubmitting, setIsSubmitting] = useState<boolean>(false);
@@ -160,6 +162,7 @@ const FormVerifyCode = ({
160162
} else {
161163
setCodeErrorMessage('');
162164
}
165+
onChangeCb?.();
163166
}}
164167
onFocusCb={viewName ? onFocus : undefined}
165168
onPaste={submitFormOnPaste ? onPaste : undefined}

packages/fxa-settings/src/lib/glean/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,9 @@ const recordEventMetric = (
643643
case 'reg_otp_email_confirmation_resend_code':
644644
reg.otpEmailConfirmationResendCode.record();
645645
break;
646+
case 'reg_otp_change_email':
647+
reg.otpChangeEmail.record();
648+
break;
646649
case 'login_otp_view':
647650
login.otpView.record();
648651
break;
@@ -663,6 +666,9 @@ const recordEventMetric = (
663666
case 'login_otp_email_confirmation_resend_code':
664667
login.otpEmailConfirmationResendCode.record();
665668
break;
669+
case 'login_otp_change_email':
670+
login.otpChangeEmail.record();
671+
break;
666672
case 'error_view':
667673
error.view.record({
668674
reason: gleanPingMetrics?.event?.['reason'] || '',

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.test.tsx

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ jest.mock('../../../lib/glean', () => ({
4747
submitSuccess: jest.fn(),
4848
error: jest.fn(),
4949
resendCode: jest.fn(),
50+
changeEmail: jest.fn(),
5051
},
5152
passwordlessReg: {
5253
view: jest.fn(),
@@ -55,6 +56,7 @@ jest.mock('../../../lib/glean', () => ({
5556
submitSuccess: jest.fn(),
5657
error: jest.fn(),
5758
resendCode: jest.fn(),
59+
changeEmail: jest.fn(),
5860
},
5961
isDone: jest.fn().mockResolvedValue(undefined),
6062
},
@@ -841,14 +843,20 @@ describe('SigninPasswordlessCode page', () => {
841843
expect(mockGleanPasswordlessLogin.view).not.toHaveBeenCalled();
842844
});
843845

844-
it('emits engage event on first focus of code input', async () => {
846+
it('does not emit engage on mount', () => {
845847
render({ isSignup: false });
848+
expect(mockGleanPasswordlessLogin.engage).not.toHaveBeenCalled();
849+
});
850+
851+
it('emits engage event on first keystroke', async () => {
852+
render({ isSignup: false });
853+
const user = userEvent.setup();
846854
const input = screen.getByLabelText('Enter 8-digit code');
847-
fireEvent.focus(input);
855+
await user.type(input, '1');
848856
expect(mockGleanPasswordlessLogin.engage).toHaveBeenCalledTimes(1);
849857

850-
// Subsequent focuses should not re-emit
851-
fireEvent.focus(input);
858+
// Subsequent keystrokes should not re-emit
859+
await user.type(input, '2');
852860
expect(mockGleanPasswordlessLogin.engage).toHaveBeenCalledTimes(1);
853861
});
854862

@@ -921,6 +929,22 @@ describe('SigninPasswordlessCode page', () => {
921929
});
922930
});
923931

932+
it('emits changeEmail event on "Use a different account" click for signin', () => {
933+
render({ isSignup: false });
934+
const link = screen.getByText('Use a different account');
935+
fireEvent.click(link);
936+
expect(mockGleanPasswordlessLogin.changeEmail).toHaveBeenCalledTimes(1);
937+
expect(mockGleanPasswordlessReg.changeEmail).not.toHaveBeenCalled();
938+
});
939+
940+
it('emits changeEmail event on "Use a different account" click for signup', () => {
941+
render({ isSignup: true });
942+
const link = screen.getByText('Use a different account');
943+
fireEvent.click(link);
944+
expect(mockGleanPasswordlessReg.changeEmail).toHaveBeenCalledTimes(1);
945+
expect(mockGleanPasswordlessLogin.changeEmail).not.toHaveBeenCalled();
946+
});
947+
924948
it('uses reg metrics for signup flow on submit', async () => {
925949
render({ isSignup: true });
926950
await submitCode();

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ const SigninPasswordlessCode = ({
7878
? GleanMetrics.passwordlessReg
7979
: GleanMetrics.passwordlessLogin;
8080

81+
const [hasEngaged, setHasEngaged] = useState(false);
8182
const gleanViewTracked = useRef(false);
8283
const sendErrorProcessed = useRef(false);
8384

@@ -437,6 +438,7 @@ const SigninPasswordlessCode = ({
437438
e: React.MouseEvent<HTMLAnchorElement>
438439
) => {
439440
e.preventDefault();
441+
gleanOtp.changeEmail();
440442

441443
// Remove email from query params if present
442444
const searchParams = new URLSearchParams(window.location.search);
@@ -522,7 +524,12 @@ const SigninPasswordlessCode = ({
522524
color: cmsInfo?.shared?.buttonColor,
523525
text: cmsButtonText,
524526
},
525-
onEngageCb: () => gleanOtp.engage(),
527+
onChangeCb: () => {
528+
if (hasEngaged === false) {
529+
setHasEngaged(true);
530+
gleanOtp.engage();
531+
}
532+
},
526533
}}
527534
/>
528535

packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,24 @@ login:
11261126
expires: never
11271127
data_sensitivity:
11281128
- interaction
1129+
otp_change_email:
1130+
type: event
1131+
description: |
1132+
Passwordless OTP Change Email (Login)
1133+
Event that indicates the user clicked "Use a different account" on the passwordless OTP code page during login.
1134+
send_in_pings:
1135+
- events
1136+
notification_emails:
1137+
1138+
1139+
bugs:
1140+
- https://mozilla-hub.atlassian.net/browse/FXA-13393
1141+
data_reviews:
1142+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1830504
1143+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1844121
1144+
expires: never
1145+
data_sensitivity:
1146+
- interaction
11291147
password_reset:
11301148
create_new_recovery_key_message_click:
11311149
type: event
@@ -1825,6 +1843,24 @@ reg:
18251843
expires: never
18261844
data_sensitivity:
18271845
- interaction
1846+
otp_change_email:
1847+
type: event
1848+
description: |
1849+
Passwordless OTP Change Email (Registration)
1850+
Event that indicates the user clicked "Use a different account" on the passwordless OTP code page during registration.
1851+
send_in_pings:
1852+
- events
1853+
notification_emails:
1854+
1855+
1856+
bugs:
1857+
- https://mozilla-hub.atlassian.net/browse/FXA-13393
1858+
data_reviews:
1859+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1830504
1860+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1844121
1861+
expires: never
1862+
data_sensitivity:
1863+
- interaction
18281864

18291865
cad_firefox:
18301866
view:

packages/fxa-shared/metrics/glean/web/event.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
// AUTOGENERATED BY glean_parser v14.5.2. DO NOT EDIT. DO NOT COMMIT.
66

7-
import StringMetricType from '@mozilla/glean/private/metrics/string';
87
import BooleanMetricType from '@mozilla/glean/private/metrics/boolean';
8+
import StringMetricType from '@mozilla/glean/private/metrics/string';
99

1010
/**
1111
* The name of the framework used by the app (ie React or Backbone).

packages/fxa-shared/metrics/glean/web/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ export const eventsMap = {
248248
submitSuccess: 'reg_otp_submit_success',
249249
error: 'reg_otp_submit_frontend_error',
250250
resendCode: 'reg_otp_email_confirmation_resend_code',
251+
changeEmail: 'reg_otp_change_email',
251252
},
252253

253254
passwordlessLogin: {
@@ -257,6 +258,7 @@ export const eventsMap = {
257258
submitSuccess: 'login_otp_submit_success',
258259
error: 'login_otp_submit_frontend_error',
259260
resendCode: 'login_otp_email_confirmation_resend_code',
261+
changeEmail: 'login_otp_change_email',
260262
},
261263

262264
error: {

0 commit comments

Comments
 (0)