Skip to content

Commit 4df73ac

Browse files
vbudhramvbudhram
committed
feat(fxa-settings): add reason extra_key to OTP password metrics
Because: - OTP/passwordless users going through the set-password flow were indistinguishable from third-party-auth users in Glean telemetry, making OTP adoption impossible to measure. - change_password_submit did not distinguish between changing an existing password and creating one for the first time. - The third_party_auth_set_password view and engage events were declared in the schema but never fired from code. This commit: - Adds reason extra_key to all four third_party_auth_set_password events ('otp' vs 'third_party_auth'), wires view/engage/submit in SetPassword/index.tsx, and success in container.tsx. Engage fires on the first keystroke (not focus) to match the pattern used by SigninPasswordlessCode. - Adds reason extra_key to change_password_submit ('change' vs 'create') and emits it from Settings/Security CTA. - Regenerates the Glean bindings for accountPref.ts, thirdPartyAuthSetPassword.ts, and event.ts. - Bridges the new reason extras in fxa-settings/src/lib/glean/index.ts and adds unit-test coverage for both reason values on all five affected events in glean/index.test.ts. - Extends signinPasswordless.spec.ts to assert reason='otp' on all four set-password events during the Sync OTP flow (attaches a fresh GleanEventsHelper to the Sync browser page). - Extends settings/changePassword.spec.ts to assert reason='change' on account_pref_change_password_submit. Fixes FXA-13429
1 parent a80ff65 commit 4df73ac

11 files changed

Lines changed: 222 additions & 39 deletions

File tree

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import { expect, test } from '../../lib/fixtures/standard';
6+
import { GleanEventsHelper } from '../../lib/glean';
67
import {
78
relayDesktopOAuthQueryParams,
89
syncDesktopOAuthQueryParams,
@@ -1032,6 +1033,13 @@ test.describe('severity-2', () => {
10321033
syncOAuthBrowserPages: { page, signin, signinPasswordlessCode },
10331034
testAccountTracker,
10341035
}) => {
1036+
// syncOAuthBrowserPages runs in a separate Firefox instance, so the
1037+
// default gleanEventsHelper fixture (bound to the main page) does not
1038+
// capture pings from this flow. Attach a fresh helper to the sync page
1039+
// before any navigation so route interception is in place.
1040+
const gleanEventsHelper = new GleanEventsHelper(page);
1041+
await gleanEventsHelper.start();
1042+
10351043
// Create passwordless account via API first (no password)
10361044
const { email } = await testAccountTracker.signUpPasswordless();
10371045
const password = (testAccountTracker.accounts[0] as any).password;
@@ -1065,6 +1073,23 @@ test.describe('severity-2', () => {
10651073
await expect(
10661074
page.getByRole('heading', { name: 'Sync is turned on' })
10671075
).toBeVisible();
1076+
1077+
// Verify every third_party_auth_set_password event emitted during the
1078+
// OTP flow carries reason='otp' so telemetry can distinguish this
1079+
// flow from the third-party-auth set-password path.
1080+
await gleanEventsHelper.waitForEvent(
1081+
'third_party_auth_set_password_success'
1082+
);
1083+
for (const eventName of [
1084+
'third_party_auth_set_password_view',
1085+
'third_party_auth_set_password_engage',
1086+
'third_party_auth_set_password_submit',
1087+
'third_party_auth_set_password_success',
1088+
]) {
1089+
const pings = gleanEventsHelper.getEventsByName(eventName);
1090+
expect(pings.length).toBeGreaterThan(0);
1091+
expect(pings[0].extras.reason).toBe('otp');
1092+
}
10681093
});
10691094

10701095
test('passwordless signin - Sync with TOTP and set password', async ({

‎packages/functional-tests/tests/settings/changePassword.spec.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ test.describe('severity-1 #smoke', () => {
3535
target,
3636
pages: { page, changePassword, settings, signin },
3737
testAccountTracker,
38+
gleanEventsHelper,
3839
}) => {
3940
const credentials = await testAccountTracker.signUp();
4041
await signInAccount(target, page, settings, signin, credentials);
@@ -51,6 +52,14 @@ test.describe('severity-1 #smoke', () => {
5152
await expect(settings.settingsHeading).toBeVisible();
5253
await expect(settings.alertBar).toHaveText('Password updated');
5354

55+
// Account already has a password, so the change password CTA should
56+
// emit reason='change' (vs 'create' for passwordless accounts).
57+
const submitPings = gleanEventsHelper.getEventsByName(
58+
'account_pref_change_password_submit'
59+
);
60+
expect(submitPings.length).toBeGreaterThan(0);
61+
expect(submitPings[0].extras.reason).toBe('change');
62+
5463
// Sign out and login with new password
5564
await settings.signOut();
5665
await signin.fillOutEmailFirstForm(credentials.email);

‎packages/fxa-settings/src/components/Settings/Security/index.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ export const Security = forwardRef<HTMLDivElement>((_, ref) => {
6868
}
6969
prefixDataTestId="password"
7070
ctaOnClickAction={() => {
71-
GleanMetrics.accountPref.changePasswordSubmit();
71+
GleanMetrics.accountPref.changePasswordSubmit({
72+
event: { reason: hasPassword ? 'change' : 'create' },
73+
});
7274
}}
7375
>
7476
{hasPassword ? (

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

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import * as accountPref from 'fxa-shared/metrics/glean/web/accountPref';
1616
import * as accountBanner from 'fxa-shared/metrics/glean/web/accountBanner';
1717
import * as deleteAccount from 'fxa-shared/metrics/glean/web/deleteAccount';
1818
import * as thirdPartyAuth from 'fxa-shared/metrics/glean/web/thirdPartyAuth';
19+
import * as thirdPartyAuthSetPassword from 'fxa-shared/metrics/glean/web/thirdPartyAuthSetPassword';
1920
import { userIdSha256, userId } from 'fxa-shared/metrics/glean/web/account';
2021
import {
2122
oauthClientId,
@@ -687,6 +688,45 @@ describe('lib/glean', () => {
687688
});
688689
});
689690

691+
describe('thirdPartyAuthSetPassword', () => {
692+
// Each event is recorded with a `reason` extra key that distinguishes
693+
// OTP/passwordless flows ('otp') from third-party auth flows
694+
// ('third_party_auth'). Both values are asserted below so the bridge
695+
// between GleanMetrics and the generated event metric stays in sync.
696+
const cases: Array<{
697+
method: 'view' | 'engage' | 'submit' | 'success';
698+
eventName: string;
699+
}> = [
700+
{ method: 'view', eventName: 'third_party_auth_set_password_view' },
701+
{ method: 'engage', eventName: 'third_party_auth_set_password_engage' },
702+
{ method: 'submit', eventName: 'third_party_auth_set_password_submit' },
703+
{
704+
method: 'success',
705+
eventName: 'third_party_auth_set_password_success',
706+
},
707+
];
708+
709+
for (const { method, eventName } of cases) {
710+
for (const reason of ['otp', 'third_party_auth'] as const) {
711+
it(`submits ${eventName} with reason='${reason}'`, async () => {
712+
const spy = sandbox.spy(
713+
thirdPartyAuthSetPassword[method],
714+
'record'
715+
);
716+
GleanMetrics.thirdPartyAuthSetPassword[method]({
717+
event: { reason },
718+
});
719+
await GleanMetrics.isDone();
720+
sinon.assert.calledOnce(setEventNameStub);
721+
sinon.assert.calledWith(setEventNameStub, eventName);
722+
sinon.assert.calledOnce(setEventReasonStub);
723+
sinon.assert.calledWith(setEventReasonStub, reason);
724+
sinon.assert.calledWith(spy, { reason });
725+
});
726+
}
727+
}
728+
});
729+
690730
describe('accountPref', () => {
691731
it('submits a ping with the account_pref_view event name', async () => {
692732
GleanMetrics.accountPref.view();
@@ -793,16 +833,30 @@ describe('lib/glean', () => {
793833
sinon.assert.called(spy);
794834
});
795835

796-
it('submits a ping with the account_pref_change_password_submit event name', async () => {
797-
GleanMetrics.accountPref.changePasswordSubmit();
836+
it('submits a ping with the account_pref_change_password_submit event name and forwards reason', async () => {
798837
const spy = sandbox.spy(accountPref.changePasswordSubmit, 'record');
838+
GleanMetrics.accountPref.changePasswordSubmit({
839+
event: { reason: 'change' },
840+
});
799841
await GleanMetrics.isDone();
800842
sinon.assert.calledOnce(setEventNameStub);
801843
sinon.assert.calledWith(
802844
setEventNameStub,
803845
'account_pref_change_password_submit'
804846
);
805-
sinon.assert.called(spy);
847+
sinon.assert.calledOnce(setEventReasonStub);
848+
sinon.assert.calledWith(setEventReasonStub, 'change');
849+
sinon.assert.calledWith(spy, { reason: 'change' });
850+
});
851+
852+
it('forwards reason="create" to account_pref_change_password_submit', async () => {
853+
const spy = sandbox.spy(accountPref.changePasswordSubmit, 'record');
854+
GleanMetrics.accountPref.changePasswordSubmit({
855+
event: { reason: 'create' },
856+
});
857+
await GleanMetrics.isDone();
858+
sinon.assert.calledWith(setEventReasonStub, 'create');
859+
sinon.assert.calledWith(spy, { reason: 'create' });
806860
});
807861

808862
it('submits a ping with the account_pref_device_signout event name', async () => {

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,9 @@ const recordEventMetric = (
533533
});
534534
break;
535535
case 'account_pref_change_password_submit':
536-
accountPref.changePasswordSubmit.record();
536+
accountPref.changePasswordSubmit.record({
537+
reason: gleanPingMetrics?.event?.['reason'] || '',
538+
});
537539
break;
538540
case 'account_pref_google_unlink_submit':
539541
accountPref.googleUnlinkSubmit.record({
@@ -722,8 +724,25 @@ const recordEventMetric = (
722724
reason: gleanPingMetrics?.event?.['reason'] || '',
723725
});
724726
break;
727+
case 'third_party_auth_set_password_view':
728+
thirdPartyAuthSetPassword.view.record({
729+
reason: gleanPingMetrics?.event?.['reason'] || '',
730+
});
731+
break;
732+
case 'third_party_auth_set_password_engage':
733+
thirdPartyAuthSetPassword.engage.record({
734+
reason: gleanPingMetrics?.event?.['reason'] || '',
735+
});
736+
break;
737+
case 'third_party_auth_set_password_submit':
738+
thirdPartyAuthSetPassword.submit.record({
739+
reason: gleanPingMetrics?.event?.['reason'] || '',
740+
});
741+
break;
725742
case 'third_party_auth_set_password_success':
726-
thirdPartyAuthSetPassword.success.record();
743+
thirdPartyAuthSetPassword.success.record({
744+
reason: gleanPingMetrics?.event?.['reason'] || '',
745+
});
727746
break;
728747
case 'promo_qr_mobile_view':
729748
promoQrMobile.view.record();

‎packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ const SetPasswordContainer = ({
135135
);
136136
persistAccount({ uid, hasPassword: true });
137137

138-
GleanMetrics.thirdPartyAuthSetPassword.success();
138+
GleanMetrics.thirdPartyAuthSetPassword.success({
139+
event: { reason: isPasswordlessFlow ? 'otp' : 'third_party_auth' },
140+
});
139141

140142
const navigationOptions: NavigationOptions = {
141143
email,
@@ -180,6 +182,7 @@ const SetPasswordContainer = ({
180182
integration,
181183
finishOAuthFlowHandler,
182184
getKeyFetchToken,
185+
isPasswordlessFlow,
183186
offeredSyncEngines,
184187
location.search,
185188
]

‎packages/fxa-settings/src/pages/PostVerify/SetPassword/index.tsx‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@ import AppLayout from '../../../components/AppLayout';
88
import { FormSetupAccount } from '../../../components/FormSetupAccount';
99
import { SetPasswordFormData, SetPasswordProps } from './interfaces';
1010
import { useForm } from 'react-hook-form';
11-
import { useCallback, useState } from 'react';
11+
import { useCallback, useEffect, useState } from 'react';
1212
import { useFtlMsgResolver } from '../../../models';
1313
import { getLocalizedErrorMessage } from '../../../lib/error-utils';
1414
import Banner from '../../../components/Banner';
1515
import {
1616
getCmsHeadlineClassName,
1717
getCmsHeadlineStyle,
1818
} from '../../../components/CardHeader';
19+
import GleanMetrics from '../../../lib/glean';
1920

2021
export const SetPassword = ({
2122
email,
@@ -29,8 +30,19 @@ export const SetPassword = ({
2930
useState<boolean>(false);
3031
const [bannerErrorText, setBannerErrorText] = useState<string>('');
3132

33+
const gleanReason = isPasswordlessFlow ? 'otp' : 'third_party_auth';
34+
35+
useEffect(() => {
36+
GleanMetrics.thirdPartyAuthSetPassword.view({
37+
event: { reason: gleanReason },
38+
});
39+
}, [gleanReason]);
40+
3241
const onSubmit = useCallback(
3342
async ({ newPassword }: SetPasswordFormData) => {
43+
GleanMetrics.thirdPartyAuthSetPassword.submit({
44+
event: { reason: gleanReason },
45+
});
3446
setCreatePasswordLoading(true);
3547
setBannerErrorText('');
3648

@@ -47,19 +59,40 @@ export const SetPassword = ({
4759
return;
4860
}
4961
},
50-
[createPasswordHandler, ftlMsgResolver]
62+
[createPasswordHandler, ftlMsgResolver, gleanReason]
5163
);
5264

53-
const { handleSubmit, register, getValues, errors, formState, trigger } =
54-
useForm<SetPasswordFormData>({
55-
mode: 'onChange',
56-
criteriaMode: 'all',
57-
defaultValues: {
58-
email,
59-
newPassword: '',
60-
confirmPassword: '',
61-
},
62-
});
65+
const {
66+
handleSubmit,
67+
register,
68+
getValues,
69+
errors,
70+
formState,
71+
trigger,
72+
watch,
73+
} = useForm<SetPasswordFormData>({
74+
mode: 'onChange',
75+
criteriaMode: 'all',
76+
defaultValues: {
77+
email,
78+
newPassword: '',
79+
confirmPassword: '',
80+
},
81+
});
82+
83+
// Fire engage on the first keystroke into the password field (not on focus
84+
// alone) so the event reflects actual user intent to type a password.
85+
// Matches the engage pattern used by SigninPasswordlessCode.
86+
const [hasEngaged, setHasEngaged] = useState(false);
87+
const newPasswordValue = watch('newPassword');
88+
useEffect(() => {
89+
if (hasEngaged === false && newPasswordValue) {
90+
setHasEngaged(true);
91+
GleanMetrics.thirdPartyAuthSetPassword.engage({
92+
event: { reason: gleanReason },
93+
});
94+
}
95+
}, [hasEngaged, newPasswordValue, gleanReason]);
6396

6497
const cmsInfo = integration?.getCmsInfo?.();
6598
const cmsPage = cmsInfo?.PostVerifySetPasswordPage;
@@ -122,7 +155,6 @@ export const SetPassword = ({
122155
onSubmit={handleSubmit(onSubmit)}
123156
// This page is only shown during the Sync flow
124157
isSync={true}
125-
submitButtonGleanId="third-party-auth-set-password-submit"
126158
passwordFormType="post-verify-set-password"
127159
cmsButton={cmsButton}
128160
/>

0 commit comments

Comments
 (0)