Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/fxa-settings/src/lib/glean/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as login from 'fxa-shared/metrics/glean/web/login';
import * as accountPref from 'fxa-shared/metrics/glean/web/accountPref';
import * as passkey from 'fxa-shared/metrics/glean/web/passkey';
import * as promoQrMobile from 'fxa-shared/metrics/glean/web/promoQrMobile';
import * as dtmDesktop from 'fxa-shared/metrics/glean/web/dtmDesktop';
import * as accountBanner from 'fxa-shared/metrics/glean/web/accountBanner';
import * as deleteAccount from 'fxa-shared/metrics/glean/web/deleteAccount';
import * as thirdPartyAuth from 'fxa-shared/metrics/glean/web/thirdPartyAuth';
Expand Down Expand Up @@ -910,6 +911,17 @@ describe('lib/glean', () => {
});
});

describe('dtmDesktop', () => {
it('submits a ping with the dtm_desktop_qr_skip name', async () => {
const spy = sandbox.spy(dtmDesktop.qrSkip, 'record');
GleanMetrics.dtmDesktop.qrSkip();
await GleanMetrics.isDone();
sinon.assert.calledOnce(setEventNameStub);
sinon.assert.calledWith(setEventNameStub, 'dtm_desktop_qr_skip');
sinon.assert.calledOnce(spy);
});
});

describe('loginTotpBackup', () => {
it('submits a ping with the login_backup_code_view name', async () => {
GleanMetrics.loginBackupCode.view();
Expand Down
4 changes: 4 additions & 0 deletions packages/fxa-settings/src/lib/glean/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import * as event from 'fxa-shared/metrics/glean/web/event';
import * as email from 'fxa-shared/metrics/glean/web/email';
import * as error from 'fxa-shared/metrics/glean/web/error';
import * as promoQrMobile from 'fxa-shared/metrics/glean/web/promoQrMobile';
import * as dtmDesktop from 'fxa-shared/metrics/glean/web/dtmDesktop';
import * as reg from 'fxa-shared/metrics/glean/web/reg';
import * as login from 'fxa-shared/metrics/glean/web/login';
import * as cachedLogin from 'fxa-shared/metrics/glean/web/cachedLogin';
Expand Down Expand Up @@ -962,6 +963,9 @@ const recordEventMetric = (
nimbus_user_id: gleanPingMetrics?.event?.['nimbusUserId'] || '',
});
break;
case 'dtm_desktop_qr_skip':
dtmDesktop.qrSkip.record();
break;
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider';
import { MemoryRouter } from 'react-router';
import * as Sentry from '@sentry/react';
Expand All @@ -22,6 +23,16 @@ jest.mock('react-router', () => ({
useNavigate: () => mockNavigate,
}));

const mockQrSkip = jest.fn();
jest.mock('../../../../lib/glean', () => ({
__esModule: true,
default: {
dtmDesktop: {
qrSkip: (...args: unknown[]) => mockQrSkip(...args),
},
},
}));

// Stub QRCode so the test can read the encoded value without decoding an SVG.
// The container's contract is the value it hands down; how that value is drawn
// is the page's concern, covered in index.test.tsx.
Expand Down Expand Up @@ -228,6 +239,58 @@ describe('Pair2/Authority/ScanQR container', () => {
expect(mockNavigate).not.toHaveBeenCalled();
});

describe('skip', () => {
// Waits for the QR to render so the channel is fully set up before the
// user leaves, as it would be in the browser.
const skip = async () => {
const user = userEvent.setup();
renderContainer();
await waitFor(() =>
expect(screen.getByTestId('scan-qr-code')).toHaveAttribute(
'data-value',
MOCK_PAIR_URL
)
);

await user.click(screen.getByRole('button', { name: 'Skip for now' }));
};

it('records the qr_skip Glean event', async () => {
await skip();

expect(mockQrSkip).toHaveBeenCalledTimes(1);
});

// Skipping ends the flow, so unlike the other exits from this page the
// channel does not outlive it.
it('closes the channel', async () => {
await skip();

expect(integration.destroy).toHaveBeenCalledTimes(1);
});

// Every pairing promo exits to settings. The pairing query parameters stay
// behind: nothing in settings reads them.
it('navigates to settings without the pairing query', async () => {
await skip();

expect(mockNavigate).toHaveBeenCalledTimes(1);
expect(mockNavigate).toHaveBeenCalledWith('/settings');
});

// Leaving must not wait on the channel: one that will not close is the
// integration's problem to report, not a reason to hold the user here.
it('still navigates and reports to Sentry when the channel will not close', async () => {
const err = new Error('socket already gone');
integration.destroy.mockRejectedValue(err);

await skip();

expect(mockNavigate).toHaveBeenCalledWith('/settings');
await waitFor(() => expect(captureException).toHaveBeenCalledWith(err));
});
});

it('throws when handed an integration that is not the pairing authority', () => {
expect(() => renderContainer(MOCK_NON_PAIRING_INTEGRATION)).toThrow(
'Invalid integration type. Expected PairingAuthorityIntegration.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import React, { useEffect, useState } from 'react';
import { useNavigate } from 'react-router';
import * as Sentry from '@sentry/react';
import ScanQR from '.';
import {
Expand All @@ -11,15 +12,18 @@ import {
PairingAuthorityIntegration,
} from '../../../../models';
import { useNavigateWithQuery } from '../../../../lib/hooks';
import GleanMetrics from '../../../../lib/glean';

/**
* Owns the pairing channel for the authority. Mints a channel on mount so the
* QR always scans to one that exists on the channel server. The channel then
* outlives this page — the authority moves on while the supplicant is still
* joining — so it is only torn down when creation itself fails.
* joining — so it is only torn down when creation itself fails or the user
* skips pairing.
*/
const ScanQRContainer = ({ integration }: { integration: Integration }) => {
const navigateWithQuery = useNavigateWithQuery();
const navigate = useNavigate();
const [qrCodeValue, setQrCodeValue] = useState('');

if (!(integration instanceof PairingAuthorityIntegration)) {
Expand Down Expand Up @@ -79,7 +83,18 @@ const ScanQRContainer = ({ integration }: { integration: Integration }) => {
};
}, [integration, navigateWithQuery]);

return <ScanQR {...{ qrCodeValue }} />;
const onSkip = () => {
GleanMetrics.dtmDesktop.qrSkip();
// Skipping ends the flow, so the channel goes with it. `destroy()` drops

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 state handler first, so the close cannot route to the timeout page.
// Not awaited: a channel that will not close must not hold the user here.
integration.destroy().catch((err) => Sentry.captureException(err));
// Settings is the exit from every pairing promo, so the pairing query
// parameters stop here rather than following the user there.
navigate('/settings');
};

return <ScanQR {...{ qrCodeValue, onSkip }} />;
};

export default ScanQRContainer;
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ pair2-authority-scan-qr-instruction = Scan the QR code with your phone or tablet
pair2-authority-scan-qr-code-aria-label = QR code to connect your mobile device
# Link to a support article for users having trouble scanning the QR code
pair2-authority-scan-qr-help-link = Get help scanning
# Button shown below the QR code card. Leaves the pairing flow and takes the user to their account settings.
pair2-authority-scan-qr-skip-button = Skip for now
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { FluentBundle } from '@fluent/bundle';
import { getFtlBundle, testL10n } from 'fxa-react/lib/test-utils';
import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider';
Expand Down Expand Up @@ -121,10 +122,17 @@ describe('Pair2/Authority/ScanQR page', () => {
renderWithLocalizationProvider(<Subject />);

const link = screen.getByRole('link', { name: /Get help scanning/ });
expect(link).toHaveAttribute(
'href',
Constants.SYNC_SUMO_URL
);
expect(link).toHaveAttribute('href', Constants.SYNC_SUMO_URL);
expect(link).toHaveAttribute('target', '_blank');
});

it('calls onSkip from the skip button', async () => {
const user = userEvent.setup();
const onSkip = jest.fn();
renderWithLocalizationProvider(<Subject {...{ onSkip }} />);

await user.click(screen.getByRole('button', { name: 'Skip for now' }));

expect(onSkip).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ export type ScanQRProps = {
* pairing value lands with the flow wiring, and this card only renders it.
*/
qrCodeValue?: string;
/** Leaves the pairing flow without scanning. */
onSkip?: () => void;
};

/**
* The desktop screen that shows the pairing QR code. The user scans it with
* their phone or tablet to start syncing; there is no button to press, so the
* only action is the link out to scanning help.
* their phone or tablet to start syncing. Pairing is one of several promos a
* sync sign-in can land on, so the card is followed by a way out of the flow
* alongside the link to scanning help.
*/
const ScanQR = ({ qrCodeValue }: ScanQRProps) => {
const ScanQR = ({ qrCodeValue, onSkip }: ScanQRProps) => {
const ftlMsgResolver = useFtlMsgResolver();
// `QRCode` takes a plain string, so this is the one label on the card that
// cannot be resolved with `FtlMsg`.
Expand All @@ -35,8 +38,11 @@ const ScanQR = ({ qrCodeValue }: ScanQRProps) => {
);

return (
<AppLayout>
<div className="text-center">
// The card is rendered here rather than by `AppLayout` so the skip button
// can sit below it: inside, the artwork is flush with the card's bottom
// edge and leaves no room.
<AppLayout wrapInCard={false}>
<div className="card text-center mobileLandscape:my-4">
<FtlMsg id="pair2-authority-scan-qr-heading">
<h1 className="card-header">Scan to connect your mobile device</h1>
</FtlMsg>
Expand Down Expand Up @@ -96,6 +102,20 @@ const ScanQR = ({ qrCodeValue }: ScanQRProps) => {
</div>
</div>
</div>

{/* Below `mobileLandscape` the card is transparent and has no bottom
margin, so the gap to the artwork is set here. */}
<div className="mb-6 mt-6 flex justify-center mobileLandscape:mt-0">
<FtlMsg id="pair2-authority-scan-qr-skip-button">
<button
type="button"
onClick={onSkip}
className="cta-neutral cta-base-p w-auto"
>
Skip for now
</button>
</FtlMsg>
</div>
</AppLayout>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ export const MOCK_LONG_QR_CODE_VALUE = MOCK_QR_CODE_VALUE.repeat(12);

export const Subject = ({
qrCodeValue = MOCK_QR_CODE_VALUE,
}: Partial<ScanQRProps> = {}) => <ScanQR {...{ qrCodeValue }} />;
onSkip = () => {},
}: Partial<ScanQRProps> = {}) => <ScanQR {...{ qrCodeValue, onSkip }} />;

/** The channel URL a successfully created channel resolves to. */
export const MOCK_PAIR_URL = `${window.location.origin}/pair#channel_id=chan-1&channel_key=key-1&v=2`;
Expand Down
20 changes: 20 additions & 0 deletions packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4439,3 +4439,23 @@ promo_qr_mobile:
branch:
description: The value proposition experiment branch shown to the user
type: string

dtm_desktop:

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.

What does dtm stand for?

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.

desktop to mobile

qr_skip:
type: event
description: |
User clicked "Skip for now" on the desktop (authority) QR code screen,
leaving the pairing flow for account settings.
send_in_pings:
- events
notification_emails:
- vzare@mozilla.com
- fxa-staff@mozilla.com
bugs:
- https://mozilla-hub.atlassian.net/browse/FXA-14523
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1830504
- https://bugzilla.mozilla.org/show_bug.cgi?id=1844121
expires: never
data_sensitivity:
- interaction
24 changes: 24 additions & 0 deletions packages/fxa-shared/metrics/glean/web/dtmDesktop.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* 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/. */

// AUTOGENERATED BY glean_parser v19.2.0. DO NOT EDIT. DO NOT COMMIT.

import EventMetricType from '@mozilla/glean/private/metrics/event';

/**
* User clicked "Skip for now" on the desktop (authority) QR code screen,
* leaving the pairing flow for account settings.
*
* Generated from `dtm_desktop.qr_skip`.
*/
export const qrSkip = new EventMetricType(
{
category: 'dtm_desktop',
name: 'qr_skip',
sendInPings: ['events'],
lifetime: 'ping',
disabled: false,
},
[]
);
4 changes: 4 additions & 0 deletions packages/fxa-shared/metrics/glean/web/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,4 +361,8 @@ export const eventsMap = {
promoQrMobile: {
view: 'promo_qr_mobile_view',
},

dtmDesktop: {
qrSkip: 'dtm_desktop_qr_skip',
},
} as const;
Loading