Merge pull request #17524 from mozilla/FXA-10386

fix(settings): Improve value of extra data included with automated Glean page load pings
2024-09-10 14:33:59 -07:00 · 2024-09-10 14:33:59 -07:00 · 31001f565e
--- a/packages/fxa-react/components/Head/index.tsx
+++ b/packages/fxa-react/components/Head/index.tsx
@ -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 from 'react';
+import React, { useEffect } from 'react';
 import { useLocalization } from '@fluent/react';
 import { Helmet } from 'react-helmet';
 import { determineLocale, determineDirection } from '@fxa/shared/l10n';
@ -14,19 +14,25 @@ const localeDirection = determineDirection(supportedUserLocale);

 const Head = ({ title }: { title?: string }) => {
  const { l10n } = useLocalization();
+
+  const customTitle = title
+    ? l10n.getString(
+        'app-page-title-2',
+        { title },
+        `${title} | Mozilla accounts`
+      )
+    : l10n.getString('app-default-title-2', null, 'Mozilla accounts');
+
+  // setting the document title here ensures it gets picked up by Glean automatic page load metrics
+  useEffect(() => {
+    document.title = customTitle;
+  });
+
  return (
    <Helmet
      htmlAttributes={{ lang: supportedUserLocale, dir: localeDirection }}
    >
-      <title>
-        {title
-          ? l10n.getString(
-              'app-page-title-2',
-              { title },
-              `${title} | Mozilla accounts`
-            )
-          : l10n.getString('app-default-title-2', null, 'Mozilla accounts')}
-      </title>
+      <title>{customTitle}</title>
    </Helmet>
  );
 };
--- a/packages/fxa-settings/src/components/App/index.tsx
+++ b/packages/fxa-settings/src/components/App/index.tsx
@ -260,10 +260,6 @@ const SettingsRoutes = ({
  const location = useLocation();
  const isSync = integration != null ? integration.isSync() : false;

-  useEffect(() => {
-    GleanMetrics.pageLoad();
-  }, [location.pathname]);
-
  // If the user is not signed in, they cannot access settings! Direct them accordingly
  if (!isSignedIn) {
    const params = new URLSearchParams(window.location.search);
@ -308,10 +304,10 @@ const AuthAndAccountSetupRoutes = ({
  const localAccount = currentAccount();
  // TODO: MozServices / string discrepancy, FXA-6802
  const serviceName = integration.getServiceName() as MozServices;
-
  const location = useLocation();
+
  useEffect(() => {
-    GleanMetrics.pageLoad();
+    GleanMetrics.pageLoad(location.pathname);
  }, [location.pathname]);

  return (
--- a/packages/fxa-settings/src/components/AppLayout/index.tsx
+++ b/packages/fxa-settings/src/components/AppLayout/index.tsx
@ -19,6 +19,7 @@ type AppLayoutProps = {

 export const AppLayout = ({ title, children, widthClass }: AppLayoutProps) => {
  const { l10n } = useLocalization();
+
  return (
    <>
      <Head {...{ title }} />
--- a/packages/fxa-settings/src/components/Settings/PageSettings/index.tsx
+++ b/packages/fxa-settings/src/components/Settings/PageSettings/index.tsx
@ -17,6 +17,7 @@ import DataCollection from '../DataCollection';
 import GleanMetrics from '../../../lib/glean';
 import ProductPromo, { ProductPromoType } from '../ProductPromo';
 import SideBar from '../Sidebar';
+import Head from 'fxa-react/components/Head';

 export const PageSettings = (_: RouteComponentProps) => {
  const { uid } = useAccount();
@ -40,6 +41,7 @@ export const PageSettings = (_: RouteComponentProps) => {

  return (
    <div id="fxa-settings" className="flex">
+      <Head />
      <div className="hidden desktop:block desktop:flex-2">
        <SideBar
          {...{
--- a/packages/fxa-settings/src/components/Settings/index.test.tsx
+++ b/packages/fxa-settings/src/components/Settings/index.test.tsx
@ -3,9 +3,7 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 import React, { ReactNode } from 'react';
-import { render } from '@testing-library/react';
 import { History } from '@reach/router';
-import App from '.';
 import { Account, AppContext, useInitialSettingsState } from '../../models';
 import {
  mockAppContext,
@ -17,7 +15,7 @@ import { Config } from '../../lib/config';
 import * as NavTiming from 'fxa-shared/metrics/navigation-timing';
 import { SETTINGS_PATH } from '../../constants';
 import AppLocalizationProvider from 'fxa-react/lib/AppLocalizationProvider';
-import { Subject, createMockSettingsIntegration } from './mocks';
+import { Subject } from './mocks';

 jest.mock('../../models', () => ({
  ...jest.requireActual('../../models'),
@ -31,8 +29,6 @@ jest.mock('./ScrollToTop', () => ({
  ),
 }));

-const integration = createMockSettingsIntegration();
-
 describe('performance metrics', () => {
  beforeEach(() => {
    jest.clearAllMocks();
@ -56,7 +52,7 @@ describe('performance metrics', () => {
      metricsEnabled: true,
      hasPassword: true,
    } as unknown as Account;
-    render(
+    renderWithRouter(
      <AppContext.Provider value={mockAppContext({ account, config })}>
        <Subject />
      </AppContext.Provider>
@ -72,11 +68,9 @@ describe('performance metrics', () => {
      metricsEnabled: false,
      hasPassword: true,
    } as unknown as Account;
-    render(
+    renderWithRouter(
      <AppContext.Provider value={mockAppContext({ account, config })}>
-        <AppContext.Provider value={mockAppContext({ account, config })}>
-          <Subject />
-        </AppContext.Provider>
+        <Subject />
      </AppContext.Provider>
    );
    expect(NavTiming.observeNavigationTiming).not.toHaveBeenCalled();
--- a/packages/fxa-settings/src/components/Settings/index.tsx
+++ b/packages/fxa-settings/src/components/Settings/index.tsx
@ -12,8 +12,12 @@ import {
  useInitialSettingsState,
  useSession,
 } from '../../models';
-import { Redirect, Router, RouteComponentProps } from '@reach/router';
-import Head from 'fxa-react/components/Head';
+import {
+  Redirect,
+  Router,
+  RouteComponentProps,
+  useLocation,
+} from '@reach/router';
 import PageSettings from './PageSettings';
 import PageChangePassword from './PageChangePassword';
 import PageCreatePassword from './PageCreatePassword';
@ -33,6 +37,7 @@ import { hardNavigate } from 'fxa-react/lib/utils';
 import { SettingsIntegration } from './interfaces';
 import { currentAccount } from '../../lib/cache';
 import { setCurrentAccount } from '../../lib/storage-utils';
+import GleanMetrics from '../../lib/glean';

 export const Settings = ({
  integration,
@ -41,6 +46,7 @@ export const Settings = ({
  const { metricsEnabled, hasPassword } = useAccount();
  const session = useSession();
  const account = useAccount();
+  const location = useLocation();

  useEffect(() => {
    if (config.metrics.navTiming.enabled && metricsEnabled) {
@ -67,6 +73,10 @@ export const Settings = ({

  const { loading, error } = useInitialSettingsState();

+  useEffect(() => {
+    !loading && GleanMetrics.pageLoad(location.pathname);
+  }, [loading, location.pathname]);
+
  if (loading) {
    return <LoadingSpinner fullScreen />;
  }
@ -78,7 +88,6 @@ export const Settings = ({

  return (
    <AppLayout {...{ integration }}>
-      <Head />
      <Router basepath={SETTINGS_PATH}>
        <ScrollToTop default>
          <PageSettings path="/" />
--- a/packages/fxa-settings/src/lib/glean/index.ts
+++ b/packages/fxa-settings/src/lib/glean/index.ts
@ -63,7 +63,7 @@ type GleanMetricsT = {
  setEnabled: (enabled: boolean) => void;
  getEnabled: () => boolean;
  isDone: () => Promise<void>;
-  pageLoad: () => void;
+  pageLoad: (url?: string) => void;
  handleClickEvent(event: Event): void;
 } & {
  [k in EventMapKeys]: { [eventKey in keyof EventsMap[k]]: PingFn };
@ -568,8 +568,12 @@ export const GleanMetrics: Pick<
    return gleanEnabled;
  },

-  pageLoad: () => {
-    GleanMetricsAPI.pageLoad();
+  pageLoad: (url?: string) => {
+    if (url) {
+      GleanMetricsAPI.pageLoad({ url });
+    } else {
+      GleanMetricsAPI.pageLoad();
+    }
  },

  handleClickEvent(event: Event) {