Skip to content

feat(pair): add reason to the /pair choice screen view event - #21010

Open
dschom wants to merge 1 commit into
mainfrom
FXA-14133
Open

feat(pair): add reason to the /pair choice screen view event#21010
dschom wants to merge 1 commit into
mainfrom
FXA-14133

Conversation

@dschom

@dschom dschom commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Because

  • Users reach the /pair choice screen from several flows (password sign-in, registration, email OTP sign-in, passkey sign-in), and cad_firefox.choice_view carries no way to tell them apart.
  • Splitting that funnel by originating flow needs the attribution on the view event itself, rather than inferred by joining across other events.

This pull request

  • Adds a reason extra key to cad_firefox.choice_view and regenerates the Glean bindings. The permitted values live in one place, fxa-shared/metrics/glean/pair-reasons.ts, which also exports the runtime guard both /pair implementations validate with.
  • Derives the reason in getSyncNavigate. passwordCreationReason is the primary signal: Sync always requires encryption keys, so every passwordless OTP or passkey sign-in is routed through /post_verify/set_password before it can reach /pair, making that page the only place that still knows how the session was established. The raw session flags (isPasskeySession, isPasswordlessOtpSignin) cover the flows that skip it.
  • Resolves the reason before the interstitial early returns so it rides along on /signup_confirmed_sync and /inline_recovery_key_setup, which reach /pair via hardNavigate('/pair', {}, true). Without this the reason was lost for every flow that stops at an interstitial, and password_reg was unreachable outside Send Tab.
  • Declares origin: 'signup' at the sign-up callers that set showSignupConfirmedSync, which otherwise fell through to password_login.
  • Reports no reason for third-party auth rather than folding it into password_login; it has no bucket of its own in FXA-14133.
  • Reads the value in React /pair from router state with a query-param fallback for the interstitial hand-off, validating both. The reason is deliberately kept out of the soft-nav URL so an account-level attribute is not written to history or server access logs on every pair visit.
  • Validates the Backbone query param against the shared allowlist before recording it, since that value arrives from the URL.
  • Adds handleNavigation-level tests asserting the reason that actually reaches /pair. The pass-through had no coverage, so the derivation could be wired to signals no Sync flow carries and the suite would still pass.
  • Parses app/scripts/lib/glean/*.ts with @typescript-eslint/parser in fxa-content-server (precedent: fxa-profile-server). That directory is TypeScript while the app is otherwise babel-eslint, so the dispatcher could not be linted at all; scoped to that directory to avoid surfacing pre-existing violations in unrelated .ts files.

Issue that this pull request solves

Closes: FXA-14133

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@dschom
dschom requested a review from a team as a code owner August 12, 2026 01:10
@dschom
dschom marked this pull request as draft August 12, 2026 01:33
Because:
 - Users reach the /pair choice screen from several flows (password sign-in,
   registration, email OTP sign-in, passkey sign-in), and cad_firefox.choice_view
   carries no way to tell them apart.
 - Splitting that funnel by originating flow needs the attribution on the view
   event itself, rather than inferred by joining across other events.

This commit:
 - Adds a `reason` extra key to cad_firefox.choice_view and regenerates the Glean
   bindings. The permitted values live in one place,
   fxa-shared/metrics/glean/pair-reasons.ts, which also exports the runtime guard
   both /pair implementations validate with.
 - Derives the reason in getSyncNavigate. `passwordCreationReason` is the primary
   signal: Sync always requires encryption keys, so every passwordless OTP or
   passkey sign-in is routed through /post_verify/set_password before it can
   reach /pair, making that page the only place that still knows how the session
   was established. The raw session flags (isPasskeySession,
   isPasswordlessOtpSignin) cover the flows that skip it.
 - Resolves the reason before the interstitial early returns so it rides along on
   /signup_confirmed_sync and /inline_recovery_key_setup, which reach /pair via
   hardNavigate('/pair', {}, true). Without this the reason was lost for every
   flow that stops at an interstitial, and password_reg was unreachable outside
   Send Tab.
 - Declares origin: 'signup' at the sign-up callers that set
   showSignupConfirmedSync, which otherwise fell through to password_login.
 - Reports no reason for third-party auth rather than folding it into
   password_login; it has no bucket of its own in FXA-14133.
 - Reads the value in React /pair from router state with a query-param fallback
   for the interstitial hand-off, validating both. The reason is deliberately
   kept out of the soft-nav URL so an account-level attribute is not written to
   history or server access logs on every pair visit.
 - Validates the Backbone query param against the shared allowlist before
   recording it, since that value arrives from the URL.
 - Adds handleNavigation-level tests asserting the reason that actually reaches
   /pair. The pass-through had no coverage, so the derivation could be wired to
   signals no Sync flow carries and the suite would still pass.
 - Parses app/scripts/lib/glean/*.ts with @typescript-eslint/parser in
   fxa-content-server (precedent: fxa-profile-server). That directory is
   TypeScript while the app is otherwise babel-eslint, so the dispatcher could
   not be linted at all; scoped to that directory to avoid surfacing pre-existing
   violations in unrelated .ts files.
@dschom
dschom marked this pull request as ready for review August 12, 2026 15:30
if (isSyncDesktopV3Integration(integration)) {
const { to } = getSyncNavigate(location.search, {
showSignupConfirmedSync: true,
origin: 'signup',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is acting like a sinup action... but let's double check...


expect(hardNavigateSpy).toHaveBeenCalledWith(
'/pair?showSuccessMessage=true'
'/pair?showSuccessMessage=true&pairReason=password_login'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason you would need this would be for Backbone pairing. I don't see any changes in fxa-content-server (and don't think we need them) so, remove?

@dschom dschom Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Initially Claude also included content server here, but I rolled those back and this fell through the cracks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants