Skip to content

Commit

Permalink
Merge pull request #17216 from mozilla/glean-frontend-account-delete
Browse files Browse the repository at this point in the history
feat(glean): Add frontend account deletion metrics
  • Loading branch information
vpomerleau authored Jul 12, 2024
2 parents cfce479 + 7197149 commit ff47414
Show file tree
Hide file tree
Showing 13 changed files with 524 additions and 44 deletions.
4 changes: 3 additions & 1 deletion packages/fxa-settings/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@
"webpack": "^5.84.1"
},
"nx": {
"implicitDependencies": ["crypto-relier"],
"implicitDependencies": [
"crypto-relier"
],
"tags": [
"scope:frontend",
"type:core"
Expand Down
6 changes: 5 additions & 1 deletion packages/fxa-settings/src/components/App/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ jest.mock('../Settings/ScrollToTop', () => ({

jest.mock('../../lib/glean', () => ({
__esModule: true,
default: { initialize: jest.fn(), getEnabled: jest.fn() },
default: {
initialize: jest.fn(),
getEnabled: jest.fn(),
accountPref: { view: jest.fn() },
},
}));

const mockMetricsQueryAccountAmplitude = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import React from 'react';
import 'mutationobserver-shim';
import { screen, fireEvent, act, within } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import {
mockAppContext,
mockSession,
Expand All @@ -13,34 +14,57 @@ import {
import { PageDeleteAccount } from '.';
import { typeByTestIdFn } from '../../../lib/test-utils';
import { Account, AppContext } from '../../../models';
import { logViewEvent, usePageViewEvent } from '../../../lib/metrics';
import { logViewEvent } from '../../../lib/metrics';
import { MOCK_EMAIL } from '../../../pages/mocks';
import GleanMetrics from '../../../lib/glean';

jest.mock('../../../lib/metrics', () => ({
logViewEvent: jest.fn(),
usePageViewEvent: jest.fn(),
}));

jest.mock('../../../lib/glean', () => ({
__esModule: true,
default: {
deleteAccount: {
view: jest.fn(),
engage: jest.fn(),
submit: jest.fn(),
passwordView: jest.fn(),
passwordSubmit: jest.fn(),
},
},
}));

const account = {
primaryEmail: {
email: 'rfeeley@mozilla.com',
email: MOCK_EMAIL,
},
uid: '0123456789abcdef',
metricsEnabled: true,
hasPassword: true,
} as unknown as Account;

const pwdlessAccount = {
primaryEmail: {
email: MOCK_EMAIL,
},
uid: '0123456789abcdef',
metricsEnabled: true,
hasPassword: false,
destroy: jest.fn().mockResolvedValue({}),
} as unknown as Account;

const session = mockSession(true, false);

window.URL.createObjectURL = jest.fn();
console.error = jest.fn();

const advanceStep = async () => {
await act(async () => {
const inputs = screen.getAllByTestId('checkbox-input');
inputs.forEach((el) => fireEvent.click(el));
const continueBtn = await screen.findByTestId('continue-button');
fireEvent.click(continueBtn);
});
const advanceStep = () => {
const inputs = screen.getAllByTestId('checkbox-input');
inputs.forEach((el) => fireEvent.click(el));
const continueBtn = screen.getByTestId('continue-button');
fireEvent.click(continueBtn);
};

describe('PageDeleteAccount', () => {
Expand Down Expand Up @@ -129,7 +153,7 @@ describe('PageDeleteAccount', () => {
expect(screen.getByTestId('delete-account-button')).toBeEnabled();
});

it('Gets valid response on submit and emits metrics events', async () => {
it('gets valid response on submit', async () => {
renderWithRouter(
<AppContext.Provider value={mockAppContext({ account, session })}>
<PageDeleteAccount />
Expand All @@ -139,30 +163,13 @@ describe('PageDeleteAccount', () => {
await advanceStep();
await typeByTestIdFn('delete-account-confirm-input-field')('hunter67');

// TODO: more extensive metrics tests
expect(usePageViewEvent).toHaveBeenCalledWith('settings.delete-account');
expect(logViewEvent).toHaveBeenCalledWith(
'flow.settings.account-delete',
'terms-checked.success'
);

const deleteAccountButton = screen.getByTestId('delete-account-button');
expect(deleteAccountButton).toBeEnabled();

expect(window.location.pathname).toContainEqual('/');
});

it('deletes account if no password set', async () => {
const pwdlessAccount = {
primaryEmail: {
email: 'rfeeley@mozilla.com',
},
uid: '0123456789abcdef',
metricsEnabled: true,
hasPassword: false,
destroy: jest.fn().mockResolvedValue({}),
} as unknown as Account;

renderWithRouter(
<AppContext.Provider
value={mockAppContext({ account: pwdlessAccount, session })}
Expand All @@ -180,4 +187,68 @@ describe('PageDeleteAccount', () => {
'confirm-password.success'
);
});

describe('glean metrics', () => {
it('emits expect event on first step view', () => {
renderWithRouter(
<AppContext.Provider value={mockAppContext({ account, session })}>
<PageDeleteAccount />
</AppContext.Provider>
);

expect(GleanMetrics.deleteAccount.view).toHaveBeenCalled();
});

describe('account with password set', () => {
it('emits expected metrics during account deletion flow', async () => {
renderWithRouter(
<AppContext.Provider value={mockAppContext({ account, session })}>
<PageDeleteAccount />
</AppContext.Provider>
);

const inputs = screen.getAllByTestId('checkbox-input');
await Promise.all(
inputs.map(async (el) => {
await userEvent.click(el);
})
);
expect(GleanMetrics.deleteAccount.engage).toHaveBeenCalled();
await userEvent.click(screen.getByRole('button', { name: 'Continue' }));
expect(GleanMetrics.deleteAccount.submit).toHaveBeenCalled();

// password confirmation step
expect(GleanMetrics.deleteAccount.passwordView).toHaveBeenCalled();
await userEvent.type(
screen.getByLabelText('Enter password'),
'hunter67'
);
await userEvent.click(screen.getByRole('button', { name: 'Delete' }));
expect(GleanMetrics.deleteAccount.passwordSubmit).toHaveBeenCalled();
});
});

describe('account without password set', () => {
it('emits expected metrics during account deletion flow', async () => {
renderWithRouter(
<AppContext.Provider
value={mockAppContext({ account: pwdlessAccount, session })}
>
<PageDeleteAccount />
</AppContext.Provider>
);

const inputs = screen.getAllByTestId('checkbox-input');
await Promise.all(
inputs.map(async (el) => {
await userEvent.click(el);
})
);
expect(GleanMetrics.deleteAccount.engage).toHaveBeenCalled();
await userEvent.click(screen.getByRole('button', { name: 'Continue' }));
expect(GleanMetrics.deleteAccount.submit).toHaveBeenCalled();
expect(GleanMetrics.deleteAccount.passwordView).not.toHaveBeenCalled();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
import { hardNavigate } from 'fxa-react/lib/utils';
import LinkExternal from 'fxa-react/components/LinkExternal';
import { getErrorFtlId } from '../../../lib/error-utils';
import GleanMetrics from '../../../lib/glean';

type FormData = {
password: string;
Expand Down Expand Up @@ -107,6 +108,7 @@ export const PageDeleteAccount = (_: RouteComponentProps) => {
const [subtitleText, setSubtitleText] = useState<string>(
l10n.getString('delete-account-step-1-2', null, 'Step 1 of 2')
);
const [hasEngagedWithForm, setHasEngagedWithForm] = useState(false);
const [checkedBoxes, setCheckedBoxes] = useState<string[]>([]);
const allBoxesChecked = Object.keys(checkboxLabels).every((element) =>
checkedBoxes.includes(element)
Expand All @@ -117,13 +119,18 @@ export const PageDeleteAccount = (_: RouteComponentProps) => {

const account = useAccount();

useEffect(() => {
GleanMetrics.deleteAccount.view();
}, []);

useEffect(() => {
if (!account.hasPassword) {
setSubtitleText('');
}
}, [account.hasPassword]);

const advanceStep = () => {
GleanMetrics.deleteAccount.submit();
// Accounts that do not have a password set, will delete immediately
if (!account.hasPassword) {
deleteAccount('');
Expand All @@ -132,7 +139,7 @@ export const PageDeleteAccount = (_: RouteComponentProps) => {
l10n.getString('delete-account-step-2-2', null, 'Step 2 of 2')
);
setConfirmed(true);

GleanMetrics.deleteAccount.passwordView();
logViewEvent('flow.settings.account-delete', 'terms-checked.success');
}
};
Expand Down Expand Up @@ -169,6 +176,10 @@ export const PageDeleteAccount = (_: RouteComponentProps) => {
event.persist();
setCheckedBoxes((existing) => {
if (event.target.checked) {
if (!hasEngagedWithForm) {
GleanMetrics.deleteAccount.engage();
setHasEngagedWithForm(true);
}
return [...existing, labelText];
} else {
return [...existing.filter((text) => text !== labelText)];
Expand Down Expand Up @@ -319,6 +330,9 @@ export const PageDeleteAccount = (_: RouteComponentProps) => {
className="cta-caution py-2 mx-2 px-4 tablet:px-10"
data-testid="delete-account-button"
disabled={disabled}
onClick={() => {
GleanMetrics.deleteAccount.passwordSubmit();
}}
>
Delete
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,28 @@

import React from 'react';
import { screen } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import PageSettings from '.';
import { renderWithRouter } from '../../../models/mocks';
import * as Metrics from '../../../lib/metrics';
import GleanMetrics from '../../../lib/glean';

jest.spyOn(Metrics, 'setProperties');
jest.spyOn(Metrics, 'usePageViewEvent');
jest.mock('../../../lib/metrics', () => ({
setProperties: jest.fn(),
usePageViewEvent: jest.fn(),
}));

jest.mock('../../../lib/glean', () => ({
__esModule: true,
default: {
accountPref: {
view: jest.fn(),
},
deleteAccount: {
settingsSubmit: jest.fn(),
},
},
}));

beforeEach(() => {
const mockIntersectionObserver = jest.fn();
Expand All @@ -20,15 +36,36 @@ beforeEach(() => {
window.IntersectionObserver = mockIntersectionObserver;
});

it('renders without imploding', async () => {
renderWithRouter(<PageSettings />);
expect(screen.getByTestId('settings-profile')).toBeInTheDocument();
expect(screen.getByTestId('settings-security')).toBeInTheDocument();
expect(screen.getByTestId('settings-connected-services')).toBeInTheDocument();
expect(screen.getByTestId('settings-delete-account')).toBeInTheDocument();
expect(screen.queryByTestId('settings-data-collection')).toBeInTheDocument();
expect(Metrics.setProperties).toHaveBeenCalledWith({
uid: 'abc123',
describe('PageSettings', () => {
it('renders without imploding', async () => {
renderWithRouter(<PageSettings />);
expect(screen.getByTestId('settings-profile')).toBeInTheDocument();
expect(screen.getByTestId('settings-security')).toBeInTheDocument();
expect(
screen.getByTestId('settings-connected-services')
).toBeInTheDocument();
expect(screen.getByTestId('settings-delete-account')).toBeInTheDocument();
expect(
screen.queryByTestId('settings-data-collection')
).toBeInTheDocument();
expect(Metrics.setProperties).toHaveBeenCalledWith({
lang: null,
uid: 'abc123',
});
});

describe('glean metrics', () => {
it('emits the expected event on render', async () => {
renderWithRouter(<PageSettings />);
expect(GleanMetrics.accountPref.view).toHaveBeenCalled();
});

it('emits the expected event on click of Delete account button', async () => {
renderWithRouter(<PageSettings />);
await userEvent.click(
screen.getByRole('link', { name: 'Delete account' })
);
expect(GleanMetrics.deleteAccount.settingsSubmit).toHaveBeenCalled();
});
});
expect(Metrics.usePageViewEvent).toHaveBeenCalledWith('settings');
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* 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 React, { useRef } from 'react';
import React, { useEffect, useRef } from 'react';
import { Link, RouteComponentProps } from '@reach/router';
import Nav from '../Nav';
import Security from '../Security';
Expand All @@ -15,6 +15,7 @@ import { useAccount } from '../../../models';
import { DeleteAccountPath } from 'fxa-settings/src/constants';
import { Localized } from '@fluent/react';
import DataCollection from '../DataCollection';
import GleanMetrics from '../../../lib/glean';

export const PageSettings = (_: RouteComponentProps) => {
const { uid } = useAccount();
Expand All @@ -25,6 +26,10 @@ export const PageSettings = (_: RouteComponentProps) => {
});
Metrics.usePageViewEvent(Metrics.settingsViewName);

useEffect(() => {
GleanMetrics.accountPref.view();
}, []);

// Scroll to effect
const profileRef = useRef<HTMLDivElement>(null);
const securityRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -57,6 +62,7 @@ export const PageSettings = (_: RouteComponentProps) => {
data-testid="settings-delete-account"
className="cta-caution text-sm transition-standard mt-12 py-2 px-5 mobileLandscape:py-1"
to={DeleteAccountPath}
onClick={() => GleanMetrics.deleteAccount.settingsSubmit()}
>
Delete account
</Link>
Expand Down
Loading

0 comments on commit ff47414

Please sign in to comment.