From 60a50c994ee60f3b3de275e663eb1a1d59e9e05b Mon Sep 17 00:00:00 2001 From: Shane Osbourne Date: Tue, 26 Nov 2024 17:44:52 +0000 Subject: [PATCH 1/2] ntp: add telemetry events for show/hide --- .../new-tab/telemetryEvent.notify.json | 36 +++++++++++++++++++ .../app/favorites/favorites.service.js | 4 +++ special-pages/pages/new-tab/app/index.js | 2 +- special-pages/pages/new-tab/app/new-tab.md | 20 +++++++++-- .../app/next-steps/next-steps.service.js | 4 +++ .../privacy-stats/components/PrivacyStats.js | 8 ++++- .../integration-tests/privacy-stats.spec.js | 27 ++++++++++++++ .../privacy-stats/privacy-stats.service.js | 4 +++ .../remote-messaging-framework/rmf.service.js | 4 +++ .../pages/new-tab/app/service.hooks.js | 21 +++++++---- special-pages/pages/new-tab/src/js/index.js | 9 ++++- special-pages/types/new-tab.ts | 18 ++++++++++ 12 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 special-pages/messages/new-tab/telemetryEvent.notify.json diff --git a/special-pages/messages/new-tab/telemetryEvent.notify.json b/special-pages/messages/new-tab/telemetryEvent.notify.json new file mode 100644 index 000000000..e222c0d70 --- /dev/null +++ b/special-pages/messages/new-tab/telemetryEvent.notify.json @@ -0,0 +1,36 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "NTP TelemetryEvent", + "type": "object", + "required": ["attributes"], + "properties": { + "attributes": { + "oneOf": [ + { + "type": "object", + "title": "Stats Show More", + "required": ["name", "value"], + "properties": { + "name": { + "const": "stats_toggle" + }, + "value": { + "type": "string", + "enum": ["show_more", "show_less"] + } + } + }, + { + "type": "object", + "title": "Example Telemetry Event", + "required": ["name"], + "properties": { + "name": { + "const": "ntp_example" + } + } + } + ] + } + } +} diff --git a/special-pages/pages/new-tab/app/favorites/favorites.service.js b/special-pages/pages/new-tab/app/favorites/favorites.service.js index 7955050f7..604be9888 100644 --- a/special-pages/pages/new-tab/app/favorites/favorites.service.js +++ b/special-pages/pages/new-tab/app/favorites/favorites.service.js @@ -29,6 +29,10 @@ export class FavoritesService { }); } + name() { + return 'FavoritesService'; + } + /** * @returns {Promise<{data: FavoritesData; config: FavoritesConfig}>} * @internal diff --git a/special-pages/pages/new-tab/app/index.js b/special-pages/pages/new-tab/app/index.js index 049a93425..d4808d75e 100644 --- a/special-pages/pages/new-tab/app/index.js +++ b/special-pages/pages/new-tab/app/index.js @@ -24,7 +24,7 @@ import { callWithRetry } from '../../../shared/call-with-retry.js'; * @throws Error */ export async function init(root, messaging, telemetry, baseEnvironment) { - const result = await callWithRetry(() => messaging.init()); + const result = await callWithRetry(() => messaging.initialSetup()); // handle fatal exceptions, the following things prevent anything from starting. if ('error' in result) { diff --git a/special-pages/pages/new-tab/app/new-tab.md b/special-pages/pages/new-tab/app/new-tab.md index 0630e5623..c564e2f0b 100644 --- a/special-pages/pages/new-tab/app/new-tab.md +++ b/special-pages/pages/new-tab/app/new-tab.md @@ -20,7 +20,7 @@ children: ## Notifications -- {@link "NewTab Messages".ContextMenuNotification `contextMenu`} +### {@link "NewTab Messages".ContextMenuNotification `contextMenu`} - Sent when the user right-clicks in the page - Note: Other widgets might prevent this (and send their own, eg: favorites) - Sends: {@link "NewTab Messages".ContextMenuNotify} @@ -41,10 +41,24 @@ children: } ``` -- {@link "NewTab Messages".ReportInitExceptionNotification `reportInitException`} +### {@link "NewTab Messages".TelemetryEventNotification `telemetryEvent`} + - These are generic events that might be useful to observe. For example, you can use these to decide when to send pixels. + - Sends a standard format `{ attributes: { name: string', value?: any } }` - see {@link "NewTab Messages".TelemetryEventNotification `telemetryEvent`} + - Example: + +```json +{ + "attributes": { + "name": "stats_toggle", + "value": "show_more" + } +} +``` + +### {@link "NewTab Messages".ReportInitExceptionNotification `reportInitException`} - Sent when the application fails to initialize (for example, a JavaScript exception prevented it) - Sends: `{ message: string }` - see {@link "NewTab Messages".ReportInitExceptionNotify} -- {@link "NewTab Messages".ReportPageExceptionNotification `reportPageException`} +### {@link "NewTab Messages".ReportPageExceptionNotification `reportPageException`} - Sent when the application failed after initialization (for example, a JavaScript exception prevented it) - Sends: `{ message: string }` - see {@link "NewTab Messages".ReportPageExceptionNotify} diff --git a/special-pages/pages/new-tab/app/next-steps/next-steps.service.js b/special-pages/pages/new-tab/app/next-steps/next-steps.service.js index 57e692f34..d8bef3a89 100644 --- a/special-pages/pages/new-tab/app/next-steps/next-steps.service.js +++ b/special-pages/pages/new-tab/app/next-steps/next-steps.service.js @@ -25,6 +25,10 @@ export class NextStepsService { }); } + name() { + return 'NextStepsService'; + } + /** * @returns {Promise<{data: NextStepsData; config: NextStepsConfig}>} * @internal diff --git a/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js b/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js index 900717392..cc6598420 100644 --- a/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js +++ b/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js @@ -1,7 +1,7 @@ import { Fragment, h } from 'preact'; import cn from 'classnames'; import styles from './PrivacyStats.module.css'; -import { useTypedTranslationWith } from '../../types.js'; +import { useMessaging, useTypedTranslationWith } from '../../types.js'; import { useContext, useState, useId, useCallback } from 'preact/hooks'; import { PrivacyStatsContext, PrivacyStatsProvider } from '../PrivacyStatsProvider.js'; import { useVisibility } from '../../widget-list/widget-config.provider.js'; @@ -133,6 +133,7 @@ export function Heading({ expansion, trackerCompanies, onToggle, buttonAttrs = { export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) { const { t } = useTypedTranslationWith(/** @type {enStrings} */ ({})); + const messaging = useMessaging(); const [formatter] = useState(() => new Intl.NumberFormat()); const defaultRowMax = 5; const sorted = sortStatsForDisplay(trackerCompanies); @@ -141,6 +142,11 @@ export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) { const hasmore = sorted.length > visible; const toggleListExpansion = () => { + if (hasmore) { + messaging.telemetryEvent({ attributes: { name: 'stats_toggle', value: 'show_more' } }); + } else { + messaging.telemetryEvent({ attributes: { name: 'stats_toggle', value: 'show_less' } }); + } if (visible === defaultRowMax) { setVisible(sorted.length); } diff --git a/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js b/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js index 268bda1e1..d4cd0f35f 100644 --- a/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js +++ b/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js @@ -27,6 +27,33 @@ test.describe('newtab privacy stats', () => { await page.getByLabel('Hide recent activity').click(); await page.getByLabel('Show recent activity').click(); }); + test('sending a pixel when show more is clicked', async ({ page }, workerInfo) => { + const ntp = NewtabPage.create(page, workerInfo); + await ntp.reducedMotion(); + await ntp.openPage({ additional: { stats: 'many' } }); + await page.getByLabel('Show More', { exact: true }).click(); + await page.getByLabel('Show Less').click(); + const calls1 = await ntp.mocks.waitForCallCount({ method: 'telemetryEvent', count: 2 }); + expect(calls1.length).toBe(2); + expect(calls1).toStrictEqual([ + { + payload: { + context: 'specialPages', + featureName: 'newTabPage', + method: 'telemetryEvent', + params: { attributes: { name: 'stats_toggle', value: 'show_more' } }, + }, + }, + { + payload: { + context: 'specialPages', + featureName: 'newTabPage', + method: 'telemetryEvent', + params: { attributes: { name: 'stats_toggle', value: 'show_less' } }, + }, + }, + ]); + }); test( 'hiding the expander when empty', { diff --git a/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.service.js b/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.service.js index 9c930fac5..8dcac54a1 100644 --- a/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.service.js +++ b/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.service.js @@ -24,6 +24,10 @@ export class PrivacyStatsService { }); } + name() { + return 'PrivacyStatsService'; + } + /** * @returns {Promise<{data: PrivacyStatsData; config: StatsConfig}>} * @internal diff --git a/special-pages/pages/new-tab/app/remote-messaging-framework/rmf.service.js b/special-pages/pages/new-tab/app/remote-messaging-framework/rmf.service.js index 256e240c7..0284a294e 100644 --- a/special-pages/pages/new-tab/app/remote-messaging-framework/rmf.service.js +++ b/special-pages/pages/new-tab/app/remote-messaging-framework/rmf.service.js @@ -17,6 +17,10 @@ export class RMFService { }); } + name() { + return 'RMFService'; + } + /** * @returns {Promise} * @internal diff --git a/special-pages/pages/new-tab/app/service.hooks.js b/special-pages/pages/new-tab/app/service.hooks.js index 7653a6151..044c0d222 100644 --- a/special-pages/pages/new-tab/app/service.hooks.js +++ b/special-pages/pages/new-tab/app/service.hooks.js @@ -23,6 +23,7 @@ */ import { useCallback, useEffect } from 'preact/hooks'; +import { useMessaging } from './types.js'; /** * @template D @@ -86,14 +87,16 @@ export function reducer(state, event) { * @param {import("preact").RefObject<{ * getInitial: () => Promise<{data: D, config: C}>; * destroy: () => void; + * name: () => string; * }>} params.service */ export function useInitialDataAndConfig({ dispatch, service }) { + const messaging = useMessaging(); useEffect(() => { if (!service.current) return console.warn('missing service'); - const stats = service.current; + const srv = service.current; async function init() { - const { config, data } = await stats.getInitial(); + const { config, data } = await srv.getInitial(); if (data) { dispatch({ kind: 'initial-data', data, config }); } else { @@ -107,12 +110,13 @@ export function useInitialDataAndConfig({ dispatch, service }) { init().catch((e) => { console.error('uncaught error', e); dispatch({ kind: 'error', error: e }); + messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data+config: ` + e.message }); }); return () => { - stats.destroy(); + srv.destroy(); }; - }, []); + }, [messaging]); } /** @@ -122,14 +126,16 @@ export function useInitialDataAndConfig({ dispatch, service }) { * @param {import("preact").RefObject<{ * getInitial: () => Promise; * destroy: () => void; + * name: () => string; * }>} params.service */ export function useInitialData({ dispatch, service }) { + const messaging = useMessaging(); useEffect(() => { if (!service.current) return console.warn('missing service'); - const stats = service.current; + const srv = service.current; async function init() { - const data = await stats.getInitial(); + const data = await srv.getInitial(); if (data) { dispatch({ kind: 'initial-data', data }); } else { @@ -143,10 +149,11 @@ export function useInitialData({ dispatch, service }) { init().catch((e) => { console.error('uncaught error', e); dispatch({ kind: 'error', error: e }); + messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data: ` + e.message }); }); return () => { - stats.destroy(); + srv.destroy(); }; }, []); } diff --git a/special-pages/pages/new-tab/src/js/index.js b/special-pages/pages/new-tab/src/js/index.js index 12d911e70..9dbef171a 100644 --- a/special-pages/pages/new-tab/src/js/index.js +++ b/special-pages/pages/new-tab/src/js/index.js @@ -28,7 +28,7 @@ export class NewTabPage { /** * @return {Promise} */ - init() { + initialSetup() { return this.messaging.request('initialSetup'); } @@ -55,6 +55,13 @@ export class NewTabPage { contextMenu(params) { this.messaging.notify('contextMenu', params); } + + /** + * @param {import("../../../../types/new-tab.js").NTPTelemetryEvent} event + */ + telemetryEvent(event) { + this.messaging.notify('telemetryEvent', event); + } } const baseEnvironment = new Environment().withInjectName(import.meta.injectName).withEnv(import.meta.env); diff --git a/special-pages/types/new-tab.ts b/special-pages/types/new-tab.ts index a0da34c32..069a46169 100644 --- a/special-pages/types/new-tab.ts +++ b/special-pages/types/new-tab.ts @@ -59,6 +59,7 @@ export interface NewTabMessages { | RmfPrimaryActionNotification | RmfSecondaryActionNotification | StatsSetConfigNotification + | TelemetryEventNotification | UpdateNotificationDismissNotification | WidgetsSetConfigNotification; requests: @@ -274,6 +275,23 @@ export interface StatsConfig { expansion: Expansion; animation?: Animation; } +/** + * Generated from @see "../messages/new-tab/telemetryEvent.notify.json" + */ +export interface TelemetryEventNotification { + method: "telemetryEvent"; + params: NTPTelemetryEvent; +} +export interface NTPTelemetryEvent { + attributes: StatsShowMore | ExampleTelemetryEvent; +} +export interface StatsShowMore { + name: "stats_toggle"; + value: "show_more" | "show_less"; +} +export interface ExampleTelemetryEvent { + name: "ntp_example"; +} /** * Generated from @see "../messages/new-tab/updateNotification_dismiss.notify.json" */ From d250d2efbe86c0f21e4b1aaf1d29d1a6fe56ac46 Mon Sep 17 00:00:00 2001 From: Shane Osbourne Date: Tue, 26 Nov 2024 19:54:35 +0000 Subject: [PATCH 2/2] some renaming --- special-pages/pages/new-tab/app/service.hooks.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/special-pages/pages/new-tab/app/service.hooks.js b/special-pages/pages/new-tab/app/service.hooks.js index 044c0d222..592deb7a8 100644 --- a/special-pages/pages/new-tab/app/service.hooks.js +++ b/special-pages/pages/new-tab/app/service.hooks.js @@ -94,9 +94,9 @@ export function useInitialDataAndConfig({ dispatch, service }) { const messaging = useMessaging(); useEffect(() => { if (!service.current) return console.warn('missing service'); - const srv = service.current; + const currentService = service.current; async function init() { - const { config, data } = await srv.getInitial(); + const { config, data } = await currentService.getInitial(); if (data) { dispatch({ kind: 'initial-data', data, config }); } else { @@ -110,11 +110,11 @@ export function useInitialDataAndConfig({ dispatch, service }) { init().catch((e) => { console.error('uncaught error', e); dispatch({ kind: 'error', error: e }); - messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data+config: ` + e.message }); + messaging.reportPageException({ message: `${currentService.name()}: failed to fetch initial data+config: ` + e.message }); }); return () => { - srv.destroy(); + currentService.destroy(); }; }, [messaging]); } @@ -133,9 +133,9 @@ export function useInitialData({ dispatch, service }) { const messaging = useMessaging(); useEffect(() => { if (!service.current) return console.warn('missing service'); - const srv = service.current; + const currentService = service.current; async function init() { - const data = await srv.getInitial(); + const data = await currentService.getInitial(); if (data) { dispatch({ kind: 'initial-data', data }); } else { @@ -149,11 +149,11 @@ export function useInitialData({ dispatch, service }) { init().catch((e) => { console.error('uncaught error', e); dispatch({ kind: 'error', error: e }); - messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data: ` + e.message }); + messaging.reportPageException({ message: `${currentService.name()}: failed to fetch initial data: ` + e.message }); }); return () => { - srv.destroy(); + currentService.destroy(); }; }, []); }