From 1502e40d5dbf049b5d1b33c3df0bf15827187976 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:00:52 +0300 Subject: [PATCH 01/12] Thread the view-only context through the PDF pipeline. Add a required viewOnly flag to the shared PDF getData params, and pass the value the orchestrator already reads from useViewOnly into every widget's getData call. The per-widget commits that follow read the flag to decide whether to resolve a link. --- .../pdf-export/PDFExportOrchestrator.test.tsx | 55 +++++++++++++++++-- .../pdf-export/PDFExportOrchestrator.tsx | 1 + assets/js/googlesitekit/widgets/types.ts | 5 ++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx b/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx index 90db505b5d8..4ddeb8bb260 100644 --- a/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx +++ b/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx @@ -29,7 +29,10 @@ import { WPDataRegistry } from '@wordpress/data/build-types/registry'; /** * Internal dependencies */ -import { VIEW_CONTEXT_MAIN_DASHBOARD } from '@/js/googlesitekit/constants'; +import { + VIEW_CONTEXT_MAIN_DASHBOARD, + VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, +} from '@/js/googlesitekit/constants'; import { CORE_PDF } from '@/js/googlesitekit/datastore/pdf/constants'; import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants'; import { CORE_USER } from '@/js/googlesitekit/datastore/user/constants'; @@ -46,6 +49,7 @@ import { createTestRegistry, provideModules, provideSiteInfo, + provideUserCapabilities, provideUserInfo, render, waitFor, @@ -191,10 +195,21 @@ describe( 'PDFExportOrchestrator', () => { dispatch.assignWidget( widgetSlug, areaSlug ); } - function renderOrchestrator() { + /** + * Renders the orchestrator under a dashboard view context. + * + * @since 1.181.0 + * @since n.e.x.t Added the `viewContext` parameter. + * + * @param viewContext The dashboard view context to render under. + * @return The render result for the orchestrator. + */ + function renderOrchestrator( + viewContext: string = VIEW_CONTEXT_MAIN_DASHBOARD + ) { return render( {} } />, { registry, - viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, + viewContext, } ); } @@ -290,15 +305,45 @@ describe( 'PDFExportOrchestrator', () => { expect( getData ).toHaveBeenCalledTimes( 1 ); - const { dates, signal } = getData.mock.calls[ 0 ][ 0 ]; - // End date is shifted back one day from the reference date. + const { dates, signal, viewOnly } = getData.mock.calls[ 0 ][ 0 ]; + // The end date shifts back one day from the reference date. expect( dates.endDate ).toBe( '2021-01-09' ); expect( dates.compareStartDate ).toBeDefined(); expect( signal ).toBeInstanceOf( AbortSignal ); + // The main dashboard isn't view-only, so `viewOnly` is false and each + // widget's `getData` loader resolves the links the PDF shows. + expect( viewOnly ).toBe( false ); expect( pdf ).toHaveBeenCalledTimes( 1 ); } ); + it( 'should pass viewOnly as true to each widget loader on a view-only dashboard', async () => { + // On a view-only dashboard, the orchestrator reads the viewable + // modules, and that read needs the user's capabilities in the store. + provideUserCapabilities( registry ); + + const getData: jest.Mock = jest.fn( () => + Promise.resolve( { data: { totalUsers: 100 } } ) + ); + registerPDFWidget( 'trafficArea', 'trafficWidget', getData ); + registry.dispatch( CORE_PDF ).setSelection( { + contextSlugs: [ CONTEXT_MAIN_DASHBOARD_TRAFFIC ], + widgetSlugs: [ 'trafficWidget' ], + } ); + + renderOrchestrator( VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY ); + + await waitFor( () => { + expect( registry.select( CORE_PDF ).getStatus() ).toBe( 'success' ); + } ); + + expect( getData ).toHaveBeenCalledTimes( 1 ); + + // Each widget's `getData` loader reads `viewOnly` and leaves out the + // links a view-only dashboard doesn't show. + expect( getData.mock.calls[ 0 ][ 0 ].viewOnly ).toBe( true ); + } ); + it( 'should transition to error and not build a PDF when the only widget fails', async () => { const getData = jest.fn( () => Promise.reject( new Error( 'report failed' ) ) diff --git a/assets/js/components/pdf-export/PDFExportOrchestrator.tsx b/assets/js/components/pdf-export/PDFExportOrchestrator.tsx index 54ec237a533..d4b705e5a07 100644 --- a/assets/js/components/pdf-export/PDFExportOrchestrator.tsx +++ b/assets/js/components/pdf-export/PDFExportOrchestrator.tsx @@ -336,6 +336,7 @@ const PDFExportOrchestrator: FC< PDFExportOrchestratorProps > = ( { registry, dates, signal, + viewOnly, } ); throwIfAborted( signal ); diff --git a/assets/js/googlesitekit/widgets/types.ts b/assets/js/googlesitekit/widgets/types.ts index ad94ff4b2fd..5611c6b6ac2 100644 --- a/assets/js/googlesitekit/widgets/types.ts +++ b/assets/js/googlesitekit/widgets/types.ts @@ -42,6 +42,7 @@ export interface PDFReportDates { * Parameters a PDF widget's `getData` loader receives. * * @since 1.183.0 + * @since n.e.x.t Added `viewOnly`. */ export interface GetPDFDataParams { /** WordPress data registry, with `resolveSelect` added. */ @@ -55,6 +56,8 @@ export interface GetPDFDataParams { dates: PDFReportDates; /** Signal that cancels the export. */ signal: AbortSignal; + /** Whether the export runs on a view-only dashboard, where a loader leaves out the links an administrator sees. */ + viewOnly: boolean; } /** @@ -96,6 +99,7 @@ export interface WidgetPDFData { * PDF export configuration for a widget. * * @since 1.181.0 + * @since n.e.x.t Added `viewOnly` to the `getData` params. */ export interface WidgetPDFConfig { Component: PDFWidgetComponent; @@ -103,6 +107,7 @@ export interface WidgetPDFConfig { registry: unknown; dates: PDFReportDates; signal: AbortSignal; + viewOnly: boolean; } ) => Promise< WidgetPDFData >; label?: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- The registry `select` is loosely typed, so `isActive` predicates can read store selectors without casting. From 1e800a9c495352b6d62f27672ea1db650183dfaf Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:00:59 +0300 Subject: [PATCH 02/12] Render a PDF link as plain text when it has no URL. With an empty href, PDFLink renders its text in the default color with no annotation. A widget then passes a row's link straight through, and a view-only export's empty link reads as plain text without the widget branching on its own. --- .../PDFLink.test.tsx | 26 +++++++++++++++++++ .../shared-react-pdf-components/PDFLink.tsx | 16 ++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx index f7178a990c1..f2b1a73421d 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx @@ -107,4 +107,30 @@ describe( 'PDFLink', () => { expect( linkJSON ).toContain( '#6c726e' ); } ); + + it( 'renders plain text in the default color when the href is empty', () => { + // An empty `href` is how a caller passes a row that has no link, so + // the text renders plain instead of as a link. + const tree = renderLink( { href: '', children: 'View dashboard' } ); + + expect( tree.type ).toBe( 'pdf-text' ); + + const treeJSON = JSON.stringify( tree ); + expect( treeJSON ).toContain( 'View dashboard' ); + expect( treeJSON ).not.toContain( PDF_COLORS.CONTENT_SECONDARY ); + } ); + + it( 'leaves out the trailing icon when the href is empty', () => { + const tree = renderLink( { + href: '', + trailingIcon: ( + + + + ), + children: 'View dashboard', + } ); + + expect( JSON.stringify( tree ) ).not.toContain( 'M1 2 3 4' ); + } ); } ); diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx index abb6c62730f..84436a9f620 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx @@ -1,5 +1,6 @@ /** - * A text link for the PDF report, with an optional trailing icon. + * A text link for the PDF report, with an optional trailing icon. With no URL, + * the text renders plain. * * Site Kit by Google, Copyright 2026 Google LLC * @@ -42,7 +43,7 @@ const styles = createPDFStyles( { } ); export interface PDFLinkProps { - /** Link target URL. */ + /** Link target URL. With no URL, the text renders plain, in the default text color and with no trailing icon. */ href?: string; /** Typography type for the link text. */ type?: PDFTypographyType; @@ -62,6 +63,17 @@ const PDFLink: FC< PDFLinkProps > = ( { style, children, } ) => { + // With no URL, the text renders plain, in the default text color. A caller + // that has no link for a row passes an empty `href`, so the row reads as + // plain text. + if ( ! href ) { + return ( + + { children } + + ); + } + const linkColor: Style = { color: PDF_COLORS.CONTENT_SECONDARY }; return ( From 7d3007f2691dba3df1cf81a10b1fe34da1b5ccd6 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:01:06 +0300 Subject: [PATCH 03/12] Link top search query rows only for an administrator. Resolve the Search Console report link for each query only when the export is not view-only, and render the query through PDFLink. A view-only export leaves the link empty, so the query reads as plain text, matching the dashboard widget. --- ...DashboardPopularKeywordsWidgetPDF.test.tsx | 21 ++++++++++--- .../DashboardPopularKeywordsWidgetPDF.tsx | 6 ++-- .../getPDFData.test.ts | 31 ++++++++++++++++++- .../getPDFData.ts | 20 +++++++++--- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.test.tsx b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.test.tsx index 6b6d8e4a182..d0b695643bb 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.test.tsx +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.test.tsx @@ -37,10 +37,8 @@ const DATA = { { keys: [ 'dog toys' ], clicks: 300, impressions: 900 }, ], links: { - 'cat food': - 'https://search.google.com/search-console/performance/search-analytics?cat-food', - 'dog toys': - 'https://search.google.com/search-console/performance/search-analytics?dog-toys', + 'cat food': 'https://example.com/search-console-report/cat-food', + 'dog toys': 'https://example.com/search-console-report/dog-toys', }, }; @@ -108,6 +106,21 @@ describe( 'DashboardPopularKeywordsWidgetPDF', () => { expect( json ).toContain( DATA.links[ 'cat food' ] ); expect( json ).toContain( DATA.links[ 'dog toys' ] ); + // The `Link` primitive from `@react-pdf` renders as `pdf-link` under + // the test mock. So a `pdf-link` in the tree means the query rendered + // as a link, not as text that happens to hold the URL. + expect( json ).toContain( 'pdf-link' ); + } ); + + it( 'renders each query as plain text when a query has no link', () => { + // An empty `links` map gives every query an empty link, so each one + // renders as plain text. + const json = renderJSON( { data: { ...DATA, links: {} } } ); + + expect( json ).toContain( 'cat food' ); + expect( json ).toContain( 'dog toys' ); + // No `pdf-link` in the tree means every query rendered as plain text. + expect( json ).not.toContain( 'pdf-link' ); } ); it( 'renders each query link in the link color', () => { diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.tsx b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.tsx index c21c00178b3..b46a9e320aa 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.tsx +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/DashboardPopularKeywordsWidgetPDF.tsx @@ -54,7 +54,7 @@ interface SearchQueryRow { rank: number; /** The search query text. */ query: string; - /** Search Console report link for the query. */ + /** Search Console report link for the query. Empty when the query has no link, so the query renders as plain text. */ queryURL: string; /** Number of clicks for the query. */ clicks: number; @@ -89,7 +89,9 @@ const DashboardPopularKeywordsWidgetPDF: FC< PDFWidgetComponentProps > = ( { // 66.7% of the 1084px row (723px). width: '66.7%', // Show the rank number before the query. The query links to its - // Search Console report, like the dashboard widget. + // Search Console report, like the dashboard widget. When the query + // has no link, `PDFLink` renders it as plain text instead of as a + // link. cell: ( row ) => ( diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts index 534b83db085..613363529f9 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts @@ -107,6 +107,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( fetchMock ).toHaveFetchedTimes( 1, reportEndpoint ); @@ -120,6 +121,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // Build the expected link from the same selector the loader uses, so the @@ -139,6 +141,22 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { } ); } ); + it( 'builds no query links on a view-only dashboard', async () => { + fetchMock.getOnce( reportEndpoint, { body: REPORT, status: 200 } ); + + const result = await getPDFData( { + registry, + dates: DATES, + signal: new AbortController().signal, + viewOnly: true, + } ); + + // The dashboard widget shows a view-only user each query as plain text, + // so the PDF gets the rows with no link to render. + expect( result.data?.rows ).toEqual( REPORT ); + expect( result.data?.links ).toEqual( {} ); + } ); + it( 'requests the report with the query dimension, a limit of 10 rows, and the date range', async () => { fetchMock.getOnce( reportEndpoint, { body: REPORT, status: 200 } ); @@ -146,6 +164,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); const [ [ requestURL ] ] = fetchMock.calls( reportEndpoint ); @@ -162,7 +181,12 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { const { signal } = new AbortController(); - await getPDFData( { registry, dates: DATES, signal } ); + await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); // The registry starts resolver runs from a timeout. Wait for those // timeouts to finish, so any extra request shows up in the call count @@ -185,6 +209,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -198,6 +223,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // Null data tells the report document to skip the widget, so the PDF @@ -225,6 +251,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for the report request to dispatch before aborting. @@ -267,6 +294,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: firstController.signal, + viewOnly: false, } ); while ( deferredResolvers.length < 1 ) { @@ -283,6 +311,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( secondRun.data?.rows ).toEqual( REPORT ); diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts index 5e0305ceea7..adcbcb40380 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts @@ -40,11 +40,11 @@ export interface PopularKeywordRow { * Data the Top search queries PDF widget renders. */ export interface PopularKeywordsPDFData { - /** Report rows and the per-query links, or `null` when the export is canceled or the report has no rows. */ + /** Report rows and the per-query links, or `null` when the user cancels the export or the report has no rows. */ data: { /** The Top search queries report rows. */ rows: PopularKeywordRow[]; - /** Map of search query to its Search Console report link. */ + /** Map of search query to its Search Console report link. Empty on a view-only dashboard, where each query shows as plain text. */ links: Record< string, string >; } | null; } @@ -91,21 +91,26 @@ function getQueryLinkMap( * * Loads the Top search queries report and passes the abort signal to the * request. Builds a Search Console report link for each query, the same link the - * dashboard widget shows. Returns `{ data: null }` when the signal aborts or the - * report has no rows, so the report document skips this widget. + * dashboard widget shows. For a view-only user, the loader builds no links, + * because the dashboard widget shows the query as plain text. Returns + * `{ data: null }` when the signal aborts or the report has no rows, so the + * report document skips this widget. * * @since 1.183.0 + * @since n.e.x.t Leaves out the query links on a view-only dashboard. * * @param params Loader parameters. * @param params.registry WordPress data registry. * @param params.dates Report date range. * @param params.signal Cancellation signal. + * @param params.viewOnly Whether the export runs on a view-only dashboard. * @return The report rows and the per-query links. */ export default async function getPDFData( { registry, dates, signal, + viewOnly, }: GetPDFDataParams ): Promise< PopularKeywordsPDFData > { if ( signal.aborted ) { return { data: null }; @@ -137,5 +142,10 @@ export default async function getPDFData( { return { data: null }; } - return { data: { rows, links: getQueryLinkMap( registry, dates, rows ) } }; + // A view-only user has no access to the Search Console report, so the + // dashboard widget shows them the query as plain text. Resolve no link, and + // the PDF renders the query as plain text too. + const links = viewOnly ? {} : getQueryLinkMap( registry, dates, rows ); + + return { data: { rows, links } }; } From f2270a816696ef78357621ef4bf02b418ccf0248 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:01:13 +0300 Subject: [PATCH 04/12] Point the top content PDF title at the Analytics report. Link each title to the same All pages and screens report the dashboard widget opens for an administrator. The loader built the entity dashboard URL before. Keep the public URL on the URL line, and resolve neither link on a view-only export. --- .../ModulePopularPagesWidgetGA4PDF.test.tsx | 29 +++-- .../ModulePopularPagesWidgetGA4PDF.tsx | 18 +-- .../getPDFData.test.ts | 113 +++++++++++++++--- .../ModulePopularPagesWidgetGA4/getPDFData.ts | 45 ++++--- 4 files changed, 153 insertions(+), 52 deletions(-) diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx index 268a5bf4b6b..04037482b25 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx @@ -55,13 +55,11 @@ const DATA = { titles: { '/home-page': 'Home Title', '/about-page': 'About Title' }, links: { '/home-page': { - detailsURL: - 'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&permaLink=http%3A%2F%2Fexample.com%2Fhome-page', + serviceURL: 'https://example.com/analytics-report/report-1', permaLink: 'http://example.com/home-page', }, '/about-page': { - detailsURL: - 'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&permaLink=http%3A%2F%2Fexample.com%2Fabout-page', + serviceURL: 'https://example.com/analytics-report/report-2', permaLink: 'http://example.com/about-page', }, }, @@ -126,16 +124,29 @@ describe( 'ModulePopularPagesWidgetGA4PDF', () => { ); } ); - it( 'links each title to its entity dashboard and each URL to its page', () => { + it( 'links each title to its Analytics report and each URL to its page', () => { const json = renderJSON( { data: DATA } ); - // The title links to the entity dashboard. - expect( json ).toContain( - 'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&permaLink=http%3A%2F%2Fexample.com%2Fhome-page' - ); + // The title links to the page's Analytics report. + expect( json ).toContain( DATA.links[ '/home-page' ].serviceURL ); // The URL line links to the page itself. expect( json ).toContain( 'http://example.com/home-page' ); expect( json ).toContain( 'http://example.com/about-page' ); + // The `Link` primitive from `@react-pdf` renders as `pdf-link` under + // the test mock. So a `pdf-link` in the tree means the lines rendered + // as links, not as text that happens to hold the URL. + expect( json ).toContain( 'pdf-link' ); + } ); + + it( 'renders the title and the URL as plain text when a row has no links', () => { + // An empty `links` map gives the row empty links, so neither line + // links out. + const json = renderJSON( { data: { ...DATA, links: {} } } ); + + expect( json ).toContain( 'Home Title' ); + expect( json ).toContain( '/home-page' ); + // No `pdf-link` in the tree means both lines rendered as plain text. + expect( json ).not.toContain( 'pdf-link' ); } ); it( 'renders the title and URL links without an underline', () => { diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx index 4eb88093fd8..3b8f025d299 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx @@ -62,9 +62,9 @@ interface PopularPageRow { title: string; /** Page path shown under the title, shortened with an ellipsis when longer than `MAX_URL_LENGTH`. */ displayURL: string; - /** URL of the page's entity dashboard, which the title links to. */ - detailsURL: string; - /** The page's own public URL, which the URL line links to. */ + /** Analytics report link for the page, which the title links to. Empty when the page has no link, so the title renders as plain text. */ + serviceURL: string; + /** The page's own public URL, which the URL line links to. Empty when the page has no link, so the URL line renders as plain text. */ permaLink: string; /** Pageviews count, as the report's raw string. */ pageviews: string; @@ -87,7 +87,7 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { const tableRows: PopularPageRow[] = rows.map( ( row, index ) => { const url = row.dimensionValues?.[ 0 ]?.value ?? ''; - const { detailsURL = '', permaLink = '' } = links[ url ] ?? {}; + const { serviceURL = '', permaLink = '' } = links[ url ] ?? {}; const displayURL = url.length > MAX_URL_LENGTH ? `${ url.slice( 0, MAX_URL_LENGTH ) }…` @@ -97,7 +97,7 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { rank: index + 1, title: titles[ url ] ?? '', displayURL, - detailsURL, + serviceURL, permaLink, pageviews: row.metricValues?.[ 0 ]?.value ?? '', sessions: row.metricValues?.[ 1 ]?.value ?? '', @@ -112,15 +112,17 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { // 42.8% of the 1084px row, about 464px. width: '42.8%', // Shows the page title above its URL, with the row rank to the - // left. The title links to the page's Site Kit entity dashboard, - // and the URL links to the page itself. + // left. The title links to the page's Analytics report, like the + // dashboard widget, and the URL links to the page itself. When a + // row has no links, `PDFLink` renders both lines as plain text + // instead of as links. cell: ( row ) => ( { `${ row.rank }.` } - { row.title } + { row.title } { row.displayURL } diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts index f3cadb5d028..3b8fe875747 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts @@ -23,6 +23,7 @@ import fetchMock from 'fetch-mock-jest'; import { createTestRegistry, provideSiteInfo, + provideUserInfo, waitForDefaultTimeouts, } from 'tests/js/utils'; @@ -35,6 +36,7 @@ import { WPDataRegistry } from '@wordpress/data/build-types/registry'; * Internal dependencies */ import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; +import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants'; import getPDFData from './getPDFData'; type Registry = WPDataRegistry & GetPDFDataParams[ 'registry' ]; @@ -93,22 +95,30 @@ const TITLES_REPORT = { }; /** - * Per-page links the loader builds for each page path, from the default test - * admin URL and reference site URL. The title links to the entity dashboard, - * and the permaLink to the page itself. + * Analytics property ID the registry settings hold, for the page links. */ -const LINKS = { - '/': { - detailsURL: - 'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&permaLink=http%3A%2F%2Fexample.com%2F', - permaLink: 'http://example.com/', - }, - '/about': { - detailsURL: - 'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&permaLink=http%3A%2F%2Fexample.com%2Fabout', - permaLink: 'http://example.com/about', - }, -}; +const PROPERTY_ID = '123456789'; + +/** + * Builds the Analytics report link the loader resolves for a page path. + * + * The link comes from the same selector the loader uses, so a test checks the + * page filter and date range, not the URL format. + * + * @since n.e.x.t + * + * @param registry Registry that holds the Analytics property. + * @param pagePath Page path from a report row. + * @return The All pages and screens report link for the page. + */ +function getExpectedPageLink( registry: Registry, pagePath: string ): string { + return registry + .select( MODULES_ANALYTICS_4 ) + .getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates: { startDate: DATES.startDate, endDate: DATES.endDate }, + } ); +} /** * Sets up `fetchMock` so each report request returns the matching fixture. Only @@ -139,6 +149,12 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { beforeEach( () => { registry = createTestRegistry() as Registry; provideSiteInfo( registry ); + provideUserInfo( registry ); + // Receive the Analytics property so each page link builds without + // fetching the module settings, which these report tests don't mock. + registry + .dispatch( MODULES_ANALYTICS_4 ) + .receiveGetSettings( { propertyID: PROPERTY_ID } ); } ); it( 'returns rows, page titles, and per-page links from the two reports', async () => { @@ -161,16 +177,57 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // The loader requests the main report first, then the page titles report. expect( requestedReports ).toEqual( [ 'main', 'page-titles' ] ); + // An unset Analytics property would make the expected link and the + // loader's link both empty, so check the link holds a URL first. + const homeLink = getExpectedPageLink( registry, '/' ); + expect( homeLink ).toBeTruthy(); + + expect( result ).toEqual( { + data: { + rows: MAIN_REPORT.rows, + titles: { '/': 'Home', '/about': 'About' }, + links: { + '/': { + serviceURL: homeLink, + permaLink: 'http://example.com/', + }, + '/about': { + serviceURL: getExpectedPageLink( registry, '/about' ), + permaLink: 'http://example.com/about', + }, + }, + }, + } ); + } ); + + it( 'builds no page links on a view-only dashboard', async () => { + fetchMock.get( reportEndpoint, ( requestURL ) => ( { + body: requestURL.includes( 'pageTitle' ) + ? TITLES_REPORT + : MAIN_REPORT, + status: 200, + } ) ); + + const result = await getPDFData( { + registry, + dates: DATES, + signal: new AbortController().signal, + viewOnly: true, + } ); + + // The rows and the titles still render. Only the links drop, so the PDF + // shows each title and URL as plain text. expect( result ).toEqual( { data: { rows: MAIN_REPORT.rows, titles: { '/': 'Home', '/about': 'About' }, - links: LINKS, + links: {}, }, } ); } ); @@ -214,6 +271,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.titles ).toEqual( { @@ -228,7 +286,12 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { const { signal } = new AbortController(); - await getPDFData( { registry, dates: DATES, signal } ); + await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); // The registry starts resolver runs from a timeout. Wait for those // timeouts, so an extra run would add its request to the calls this test @@ -254,6 +317,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); await waitForDefaultTimeouts(); @@ -272,6 +336,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -298,6 +363,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for the main report request to dispatch before aborting. @@ -347,6 +413,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: firstController.signal, + viewOnly: false, } ); while ( deferredResolvers.length < 1 ) { @@ -363,13 +430,23 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( secondRun ).toEqual( { data: { rows: MAIN_REPORT.rows, titles: { '/': 'Home', '/about': 'About' }, - links: LINKS, + links: { + '/': { + serviceURL: getExpectedPageLink( registry, '/' ), + permaLink: 'http://example.com/', + }, + '/about': { + serviceURL: getExpectedPageLink( registry, '/about' ), + permaLink: 'http://example.com/about', + }, + }, }, } ); expect( fetchMock.calls( reportEndpoint ) ).toHaveLength( 3 ); diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts index 3b87b88c54e..14e6680007c 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts @@ -32,12 +32,12 @@ import { getFullURL } from '@/js/util'; import { getPopularPagesReportArgs } from './reportOptions'; /** - * Links for one page row: the entity dashboard URL for the title, and the + * Links for one page row: the Analytics report link for the title, and the * page's own public URL for the URL line. */ export interface PopularPageLinks { - /** URL of the page's entity dashboard, which the page title links to. */ - detailsURL: string; + /** Analytics report link for the page, which the page title links to. */ + serviceURL: string; /** The page's own public URL, which the URL line links to. */ permaLink: string; } @@ -46,46 +46,50 @@ export interface PopularPageLinks { * Data the Top content over time PDF widget renders. */ export interface PopularPagesPDFData { - /** Report rows, the page titles, and the per-page links, or `null` when the export is canceled or the report has no rows. */ + /** Report rows, the page titles, and the per-page links, or `null` when the user cancels the export or the report has no rows. */ data: { /** Rows of the Most popular pages report. */ rows: ReportRow[]; /** Map of page path to page title. */ titles: Record< string, string >; - /** Map of page path to its entity dashboard URL and public URL. */ + /** Map of page path to its Analytics report link and public URL. Empty on a view-only dashboard, where each title and URL show as plain text. */ links: Record< string, PopularPageLinks >; } | null; } /** - * Maps each page path to its entity dashboard URL and public URL. + * Maps each page path to its Analytics report link and public URL. * - * The title links to the page's Site Kit detail view, the same entity dashboard - * link the dashboard builds for a page. The URL line links to the page itself. + * The title links to the same All pages and screens report the dashboard + * widget links to for an administrator. The URL line links to the page itself. * * @since 1.182.0 + * @since n.e.x.t Links the title to the page's Analytics report instead of its entity dashboard. * * @param registry WordPress data registry. + * @param dates Report date range. * @param pagePaths Page paths from the main report rows. - * @return Map of page path to its entity dashboard URL and public URL. + * @return Map of page path to its Analytics report link and public URL. */ function getPopularPageLinkMap( registry: GetPDFDataParams[ 'registry' ], + dates: GetPDFDataParams[ 'dates' ], pagePaths: string[] ): Record< string, PopularPageLinks > { - const coreSite = registry.select( CORE_SITE ); - const siteURL = coreSite.getReferenceSiteURL(); + const siteURL = registry.select( CORE_SITE ).getReferenceSiteURL(); + const analytics = registry.select( MODULES_ANALYTICS_4 ); + const { startDate, endDate } = dates; const links: Record< string, PopularPageLinks > = {}; pagePaths.forEach( ( pagePath ) => { - const permaLink = getFullURL( siteURL, pagePath ); links[ pagePath ] = { - detailsURL: - coreSite.getAdminURL( 'googlesitekit-dashboard', { - permaLink, + serviceURL: + analytics.getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates: { startDate, endDate }, } ) ?? '', - permaLink, + permaLink: getFullURL( siteURL, pagePath ), }; } ); @@ -102,17 +106,20 @@ function getPopularPageLinkMap( * * @since 1.182.0 * @since 1.183.0 Returns null data when the report has no rows. + * @since n.e.x.t Links each title to its Analytics report, and leaves out the page links on a view-only dashboard. * * @param params Loader parameters. * @param params.registry WordPress data registry. * @param params.dates Report date range. * @param params.signal Cancellation signal. + * @param params.viewOnly Whether the export runs on a view-only dashboard. * @return The report rows and the page-path-to-title map. */ export default async function getPDFData( { registry, dates, signal, + viewOnly, }: GetPDFDataParams ): Promise< PopularPagesPDFData > { if ( signal.aborted ) { return { data: null }; @@ -143,7 +150,11 @@ export default async function getPDFData( { } const pagePaths = getPagePaths( report ); - const links = getPopularPageLinkMap( registry, pagePaths ); + // For a view-only user, the loader builds no page links, so the title and + // the URL both render as plain text. + const links = viewOnly + ? {} + : getPopularPageLinkMap( registry, dates, pagePaths ); if ( pagePaths.length === 0 ) { return { data: { rows, titles: {}, links } }; From b9abd0a1df622874154b0f6d83a3faf02f91051d Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:01:22 +0300 Subject: [PATCH 05/12] Link top earning page titles only for an administrator. Build the Analytics report link for each page only when the export is not view-only, and render the title through PDFLink. A view-only export leaves the link empty, so the title reads as plain text. --- .../getPDFData.test.ts | 98 +++++++++++++++++-- .../getPDFData.ts | 72 ++++++++++++-- .../indexPDF.test.tsx | 28 +++++- .../indexPDF.tsx | 24 +++-- 4 files changed, 198 insertions(+), 24 deletions(-) diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts index 3119a1bd8cc..d054330f764 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts @@ -20,13 +20,18 @@ * External dependencies */ import fetchMock from 'fetch-mock-jest'; -import { createTestRegistry, waitForDefaultTimeouts } from 'tests/js/utils'; +import { + createTestRegistry, + provideUserInfo, + waitForDefaultTimeouts, +} from 'tests/js/utils'; /** * Internal dependencies */ import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; import { MODULES_ADSENSE } from '@/js/modules/adsense/datastore/constants'; +import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants'; import getPDFData from './getPDFData'; // The registry `getPDFData` receives: the WordPress data registry with @@ -41,10 +46,15 @@ const reportEndpoint = new RegExp( ); /** - * AdSense account ID seeded into the registry. + * AdSense account ID the registry settings hold. */ const ACCOUNT_ID = 'pub-1234567890'; +/** + * Analytics property ID the registry settings hold, for the page links. + */ +const PROPERTY_ID = '123456789'; + /** * Date range fixture passed to the loader. */ @@ -112,17 +122,44 @@ function provideReports( { } ) ); } +/** + * Builds the Analytics report link the loader resolves for a page path. + * + * The link comes from the same selector the loader uses, so a test checks the + * page filter and date range, not the URL format. + * + * @since n.e.x.t + * + * @param registry Registry that holds the Analytics property. + * @param pagePath Page path from a report row. + * @return The All pages and screens report link for the page. + */ +function getExpectedPageLink( registry: Registry, pagePath: string ): string { + return registry + .select( MODULES_ANALYTICS_4 ) + .getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates: { startDate: DATES.startDate, endDate: DATES.endDate }, + } ); +} + describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { let registry: Registry; beforeEach( () => { registry = createTestRegistry() as Registry; + provideUserInfo( registry ); registry .dispatch( MODULES_ADSENSE ) .receiveGetSettings( { accountID: ACCOUNT_ID } ); + // Receive the Analytics property so each page link builds without + // fetching the module settings, which these report tests don't mock. + registry + .dispatch( MODULES_ANALYTICS_4 ) + .receiveGetSettings( { propertyID: PROPERTY_ID } ); } ); - it( 'returns rows, currency code, and page titles from the two reports', async () => { + it( 'returns rows, currency code, page titles, and page links from the two reports', async () => { const requestedReports: string[] = []; fetchMock.get( reportEndpoint, ( requestURL ) => { @@ -142,20 +179,50 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // The loader requests the main report first, then the page titles report. expect( requestedReports ).toEqual( [ 'main', 'page-titles' ] ); + // An unset Analytics property would make the expected link and the + // loader's link both empty, so check the link holds a URL first. + const homeLink = getExpectedPageLink( registry, '/' ); + expect( homeLink ).toBeTruthy(); + expect( result ).toEqual( { data: { rows: MAIN_REPORT.rows, currencyCode: 'EUR', titles: { '/': 'Home', '/about': 'About' }, + links: { + '/': homeLink, + '/about': getExpectedPageLink( registry, '/about' ), + }, }, } ); } ); + it( 'builds no page links on a view-only dashboard', async () => { + provideReports(); + + const result = await getPDFData( { + registry, + dates: DATES, + signal: new AbortController().signal, + viewOnly: true, + } ); + + // The dashboard widget shows a view-only user each page title as plain + // text, so the PDF gets the rows and titles with no link to render. + expect( result.data?.rows ).toEqual( MAIN_REPORT.rows ); + expect( result.data?.titles ).toEqual( { + '/': 'Home', + '/about': 'About', + } ); + expect( result.data?.links ).toEqual( {} ); + } ); + it( 'requests the main report with the expected options', async () => { provideReports(); @@ -163,6 +230,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // The first report request is the main Top earning pages report. @@ -181,9 +249,15 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { } ); it( 'resolves the AdSense settings before building the report when they are not loaded yet', async () => { - // Fresh registry without the settings seeded in `beforeEach`, matching - // an export that starts before the AdSense store has loaded them. + // Fresh registry without the AdSense settings from `beforeEach`, + // matching an export that starts before the AdSense store has loaded + // them. The user info and the Analytics property go back in, so the + // page links build without their own settings fetch. registry = createTestRegistry() as Registry; + provideUserInfo( registry ); + registry + .dispatch( MODULES_ANALYTICS_4 ) + .receiveGetSettings( { propertyID: PROPERTY_ID } ); fetchMock.getOnce( new RegExp( '^/google-site-kit/v1/modules/adsense/data/settings' ), @@ -195,6 +269,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // The report filter targets the fetched account, not `(undefined)`. @@ -245,6 +320,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.titles ).toEqual( { @@ -259,7 +335,12 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { const { signal } = new AbortController(); - await getPDFData( { registry, dates: DATES, signal } ); + await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); // The registry starts resolver runs from a timeout. Wait for those // timeouts, so an extra run would add its request to the calls this test @@ -288,12 +369,13 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); await waitForDefaultTimeouts(); expect( result ).toEqual( { - data: { rows: [], currencyCode: 'EUR', titles: {} }, + data: { rows: [], currencyCode: 'EUR', titles: {}, links: {} }, } ); expect( fetchMock.calls( reportEndpoint ) ).toHaveLength( 1 ); } ); @@ -306,6 +388,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -332,6 +415,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for the main report request to dispatch before aborting. diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts index c19be565d13..f94a19de21d 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts @@ -30,10 +30,18 @@ import { } from '@/js/modules/analytics-4/utils/page-titles-report'; import { getTopEarningPagesReportOptions } from './getTopEarningPagesReportOptions'; +/** + * Data the Top earning pages PDF widget renders. + */ export interface TopEarningPagesPDFData { + /** Rows of the Top earning pages report. */ rows: ReportRow[]; + /** Currency code from the report metadata, for formatting the earnings. */ currencyCode: string; + /** Map of page path to page title. */ titles: Record< string, string >; + /** Map of page path to its Analytics report link. Empty on a view-only dashboard, where each page title shows as plain text. */ + links: Record< string, string >; } /** @@ -47,15 +55,51 @@ interface GetPDFDataResult { data: TopEarningPagesPDFData | null; } +/** + * Maps each page path to its Analytics report link. + * + * Builds the same All pages and screens report link the dashboard widget shows + * for each page. The PDF component renders each page title as this link. + * + * @since n.e.x.t + * + * @param registry WordPress data registry. + * @param dates Report date range. + * @param pagePaths Page paths from the main report rows. + * @return Map of page path to its Analytics report link. + */ +function getPageLinkMap( + registry: GetPDFDataParams[ 'registry' ], + dates: GetPDFDataParams[ 'dates' ], + pagePaths: string[] +): Record< string, string > { + const analytics = registry.select( MODULES_ANALYTICS_4 ); + const { startDate, endDate } = dates; + const links: Record< string, string > = {}; + + pagePaths.forEach( ( pagePath ) => { + links[ pagePath ] = + analytics.getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates: { startDate, endDate }, + } ) ?? ''; + } ); + + return links; +} + /** * Loads the report rows, currency code, and page titles for the Top earning * pages PDF widget. * * Resolves the linked AdSense account ID, loads the Top earning pages report, - * then a second report that matches each page path to its title. Passes the - * abort signal to both requests and returns `{ data: null }` as soon as the - * signal aborts, so cancelling the export stops the work and the widget renders - * its empty state. + * then a second report that matches each page path to its title. Builds an + * Analytics report link for each page, the same link the dashboard widget + * shows. For a view-only user, the loader builds no links, because the + * dashboard widget shows the page title as plain text. Passes the cancellation + * signal to both requests and returns `{ data: null }` when the signal fires. + * Canceling the export stops the work, and the widget renders its empty + * state. * * @since n.e.x.t * @@ -63,22 +107,25 @@ interface GetPDFDataResult { * @param params.registry WordPress data registry. * @param params.dates Report date range. * @param params.signal Cancellation signal. - * @return The report rows, currency code, and the page-path-to-title map. + * @param params.viewOnly Whether the export runs on a view-only dashboard. + * @return The report rows, currency code, page titles, and per-page links. */ export default async function getPDFData( { registry, dates, signal, + viewOnly, }: GetPDFDataParams ): Promise< GetPDFDataResult > { if ( signal.aborted ) { return { data: null }; } // The linked account ID lives in the AdSense settings, and the PDF export - // path does not otherwise resolve them (eligibility only checks Analytics' - // `adSenseLinked`). Resolve them before reading the ID, or it can be - // undefined and the report queries a malformed `Google AdSense account - // (undefined)` ad source, returning an empty or wrong report. + // path doesn't otherwise resolve them (eligibility only checks the + // Analytics `adSenseLinked` setting). Resolve them before reading the ID, + // or it stays `undefined` and the report queries a malformed `Google + // AdSense account (undefined)` ad source, returning an empty or wrong + // report. await registry.resolveSelect( MODULES_ADSENSE ).getSettings(); if ( signal.aborted ) { @@ -107,8 +154,12 @@ export default async function getPDFData( { const currencyCode = report?.metadata?.currencyCode ?? ''; const pagePaths = getPagePaths( report ); + // For a view-only user, the loader builds no page links, so each page title + // renders as plain text. + const links = viewOnly ? {} : getPageLinkMap( registry, dates, pagePaths ); + if ( pagePaths.length === 0 ) { - return { data: { rows, currencyCode, titles: {} } }; + return { data: { rows, currencyCode, titles: {}, links } }; } const titlesArgs = getPageTitlesReportOptions( dates, pagePaths ); @@ -127,6 +178,7 @@ export default async function getPDFData( { rows, currencyCode, titles: getPageTitleMap( pagePaths, titlesReport ), + links, }, }; } diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx index 4599ab2e78a..64cf6fd1dbc 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx @@ -49,6 +49,10 @@ const DATA = { ], currencyCode: 'EUR', titles: { '/home-page': 'Home Title', '/about-page': 'About Title' }, + links: { + '/home-page': 'https://example.com/analytics-report/home-page', + '/about-page': 'https://example.com/analytics-report/about-page', + }, }; /** @@ -99,6 +103,28 @@ describe( 'DashboardTopEarningPagesWidgetGA4PDF', () => { expect( json ).toContain( 'About Title' ); } ); + it( 'renders each page title as its Analytics report link', () => { + const json = renderJSON( { data: DATA } ); + + expect( json ).toContain( DATA.links[ '/home-page' ] ); + expect( json ).toContain( DATA.links[ '/about-page' ] ); + // The `Link` primitive from `@react-pdf` renders as `pdf-link` under + // the test mock. So a `pdf-link` in the tree means the title rendered + // as a link, not as text that happens to hold the URL. + expect( json ).toContain( 'pdf-link' ); + } ); + + it( 'renders each page title as plain text when a row has no link', () => { + // An empty `links` map gives every row an empty link, so each title + // renders as plain text. + const json = renderJSON( { data: { ...DATA, links: {} } } ); + + expect( json ).toContain( 'Home Title' ); + expect( json ).toContain( 'About Title' ); + // No `pdf-link` in the tree means every title rendered as plain text. + expect( json ).not.toContain( 'pdf-link' ); + } ); + it( 'numbers each row by its rank in the page title cell', () => { const json = renderJSON( { data: DATA } ); @@ -106,7 +132,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4PDF', () => { expect( json ).toContain( '2.' ); } ); - it( 'renders the page title in the Site Kit teal color', () => { + it( 'renders the page title links in the Site Kit teal color', () => { const json = renderJSON( { data: DATA } ); expect( json ).toContain( '#108080' ); diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.tsx b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.tsx index 0ba2424e515..0924373bce5 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.tsx +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.tsx @@ -32,6 +32,7 @@ import { __ } from '@wordpress/i18n'; */ import { createPDFStyles } from '@/js/components/pdf-export/pdf-scale'; import { PDF_COLORS } from '@/js/components/pdf-export/pdf-theme'; +import PDFLink from '@/js/components/pdf-export/shared-react-pdf-components/PDFLink'; import PDFTable, { PDFTableColumn, } from '@/js/components/pdf-export/shared-react-pdf-components/PDFTable'; @@ -55,9 +56,10 @@ const styles = createPDFStyles( { rank: { color: PDF_COLORS.SURFACES_ON_SURFACE, }, - pageTitle: { + // The group fills the row remainder after the rank, so a long page title + // wraps inside the cell. + titleGroup: { flex: 1, - color: PDF_COLORS.CONTENT_SECONDARY, }, noData: { paddingHorizontal: 24, @@ -66,8 +68,13 @@ const styles = createPDFStyles( { } ); interface TopEarningPageRow { + /** Position of the page in the table, starting at 1. */ rank: number; + /** Page title shown in the first cell. */ title: string; + /** Analytics report link for the page, which the title links to. Empty when the page has no link, so the title renders as plain text. */ + serviceURL: string; + /** Earnings amount, as the report's raw string. */ earnings: string; } @@ -78,6 +85,7 @@ const DashboardTopEarningPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { const topEarningPagesData = data as TopEarningPagesPDFData | null; const rows = topEarningPagesData?.rows ?? []; const titles = topEarningPagesData?.titles ?? {}; + const links = topEarningPagesData?.links ?? {}; const currencyCode = topEarningPagesData?.currencyCode ?? ''; // The report's first dimension is `pagePath`, so the first dimension value is @@ -88,6 +96,7 @@ const DashboardTopEarningPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { return { rank: index + 1, title: titles[ pagePath ] ?? '', + serviceURL: links[ pagePath ] ?? '', earnings: row.metricValues?.[ 0 ]?.value ?? '', }; } ); @@ -96,15 +105,18 @@ const DashboardTopEarningPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { { // The page title column has no header, matching the dashboard widget. header: '', - // Shows the page title with the row rank to its left. + // Shows the page title with the row rank to its left. The title + // links to the page's Analytics report, like the dashboard widget. + // When the page has no link, `PDFLink` renders the title as plain + // text instead of as a link. cell: ( row ) => ( { `${ row.rank }.` } - - { row.title } - + + { row.title } + ), }, From 67900da067ac7dc10f384e567f3d1dc5e2384d57 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:01:37 +0300 Subject: [PATCH 06/12] Link visitor groups top content only for an administrator. Pass the loader a link resolver, which the card builder calls for each top content page path, since only the builder reads those rows. The resolver returns an empty link on a view-only export, so the tile renders the title as plain text. --- .../PDFYourVisitorGroupsTile.test.tsx | 39 +++++++++- .../PDFYourVisitorGroupsTile.tsx | 33 ++++++--- .../buildPDFAudienceCard.test.ts | 50 +++++++++++-- .../buildPDFAudienceCard.ts | 20 +++-- .../AudienceTilesWidget/getPDFData.test.ts | 74 +++++++++++++++++-- .../AudienceTilesWidget/getPDFData.ts | 35 +++++++++ 6 files changed, 220 insertions(+), 31 deletions(-) diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.test.tsx b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.test.tsx index b2d9921e59c..6b3171ce907 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.test.tsx +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.test.tsx @@ -46,8 +46,16 @@ const TOP_CITIES = [ ]; const TOP_CONTENT = [ - { title: 'First post title', pageviews: 847 }, - { title: 'Second post title', pageviews: 596 }, + { + title: 'First post title', + pageviews: 847, + serviceURL: 'https://example.com/analytics-report/first-post', + }, + { + title: 'Second post title', + pageviews: 596, + serviceURL: 'https://example.com/analytics-report/second-post', + }, ]; /** @@ -129,6 +137,33 @@ describe( 'PDFYourVisitorGroupsTile', () => { expect( json ).toContain( '596' ); } ); + it( 'links each top content title to its Analytics report', () => { + const json = renderTile(); + + expect( json ).toContain( TOP_CONTENT[ 0 ].serviceURL ); + expect( json ).toContain( TOP_CONTENT[ 1 ].serviceURL ); + // The `Link` primitive from `@react-pdf` renders as `pdf-link` under + // the test mock. So a `pdf-link` in the tree means the title rendered + // as a link, not as text that happens to hold the URL. + expect( json ).toContain( 'pdf-link' ); + } ); + + it( 'renders each top content title as plain text when a row has no link', () => { + // An empty `serviceURL` gives every row an empty link, so each title + // renders as plain text. + const json = renderTile( { + topContent: TOP_CONTENT.map( ( content ) => ( { + ...content, + serviceURL: '', + } ) ), + } ); + + expect( json ).toContain( 'First post title' ); + expect( json ).toContain( 'Second post title' ); + // No `pdf-link` in the tree means every title rendered as plain text. + expect( json ).not.toContain( 'pdf-link' ); + } ); + it( "hides the delta chip when the change can't be calculated", () => { // A zero previous value leaves no calculable change for any metric. const json = renderTile( { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.tsx b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.tsx index 39085d61f86..7a4eb789a42 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.tsx +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/PDFYourVisitorGroupsTile.tsx @@ -43,6 +43,7 @@ import { import { createPDFStyles } from '@/js/components/pdf-export/pdf-scale'; import { PDF_COLORS } from '@/js/components/pdf-export/pdf-theme'; import PDFCard from '@/js/components/pdf-export/shared-react-pdf-components/PDFCard'; +import PDFLink from '@/js/components/pdf-export/shared-react-pdf-components/PDFLink'; import PDFTypography from '@/js/components/pdf-export/shared-react-pdf-components/PDFTypography'; import { numFmt } from '@/js/util'; import type { @@ -108,14 +109,16 @@ const styles = createPDFStyles( { contentRow: { flexDirection: 'row', justifyContent: 'space-between', - marginVertical: 6, + marginBottom: 6, }, - contentLink: { + // The title fills the row remainder before the pageviews count. + contentTitle: { flex: 1, - color: PDF_COLORS.CONTENT_SECONDARY, marginRight: 30, - // A long title truncates to one line with an ellipsis. `@react-pdf` - // wraps text by default, so `maxLines` caps it at one line. + }, + // A long title truncates to one line with an ellipsis. `@react-pdf` + // wraps text by default, so `maxLines` caps it at one line. + contentTitleText: { maxLines: 1, textOverflow: 'ellipsis', }, @@ -239,14 +242,22 @@ const PDFYourVisitorGroupsTile: FC< PDFYourVisitorGroupsTileProps > = ( { > { __( 'Top content by pageviews', 'google-site-kit' ) } + { /* + * The page title links to its Analytics report, like the + * dashboard tile. When the page has no link, `PDFLink` + * renders the title as plain text instead of as a link. + */ } { topContent.map( ( content ) => ( - - { content.title } - + + + { content.title } + + { numFmt( content.pageviews ) } diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts index 3b24df69286..fabea401410 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts @@ -137,7 +137,19 @@ describe( 'buildTopCities', () => { } ); describe( 'buildTopContent', () => { - it( 'resolves each page path to its title and falls back to the path', () => { + /** + * Maps a page path to a stable link fixture. + * + * @since n.e.x.t + * + * @param pagePath Page path of a top content row. + * @return The link fixture for the page. + */ + function getContentServiceURL( pagePath: string ): string { + return `https://example.com/analytics-report${ pagePath }`; + } + + it( 'resolves each page path to its title and its link, and falls back to the path', () => { const content = buildTopContent( buildReport( [ { @@ -151,12 +163,21 @@ describe( 'buildTopContent', () => { ] ), buildReport( [ { dimensionValues: [ { value: '/a' }, { value: 'Title A' } ] }, - ] ) + ] ), + getContentServiceURL ); expect( content ).toEqual( [ - { title: 'Title A', pageviews: 847 }, - { title: '/b', pageviews: 5 }, + { + title: 'Title A', + pageviews: 847, + serviceURL: 'https://example.com/analytics-report/a', + }, + { + title: '/b', + pageviews: 5, + serviceURL: 'https://example.com/analytics-report/b', + }, ] ); } ); @@ -167,7 +188,7 @@ describe( 'buildTopContent', () => { } ) ); expect( - buildTopContent( buildReport( rows ), undefined ) + buildTopContent( buildReport( rows ), undefined, () => '' ) ).toHaveLength( 3 ); } ); } ); @@ -214,12 +235,18 @@ describe( 'buildPDFAudienceCard', () => { }, topContentPageTitlesResult: { response: buildReport( [] ) }, totalPageviews: 1000, + getContentServiceURL: () => '', ...overrides, }; } - it( 'builds a card with the audience name, metrics, and pageviews share', () => { - const card = buildPDFAudienceCard( buildCardInput() ); + it( 'builds a card with the audience name, metrics, pageviews share, and top content links', () => { + const card = buildPDFAudienceCard( + buildCardInput( { + getContentServiceURL: ( pagePath: string ) => + `https://example.com/analytics-report${ pagePath }`, + } ) + ); expect( card?.audienceName ).toBe( 'Custom A' ); expect( card?.metrics.visitors.current ).toBe( 10 ); @@ -230,6 +257,15 @@ describe( 'buildPDFAudienceCard', () => { expect( card?.topCities ).toEqual( [ { name: 'Dublin', percentage: 5 }, ] ); + // The card hands each top content page path to the link resolver, so + // the row holds the link its title renders as. + expect( card?.topContent ).toEqual( [ + { + title: '/a', + pageviews: 80, + serviceURL: 'https://example.com/analytics-report/a', + }, + ] ); } ); it( 'drops the audience when the metrics report failed', () => { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts index 0a8d9e80d4f..1d5c4087392 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts @@ -58,6 +58,8 @@ export interface AudienceTileTopContent { title: string; /** The page's pageviews. */ pageviews: number; + /** Analytics report link for the page, which the title links to. Empty when the page has no link, so the title renders as plain text. */ + serviceURL: string; } /** One audience card's fully loaded data. */ @@ -191,17 +193,20 @@ export function buildTopCities( } /** - * Builds an audience's top content, resolving each page path to its page title. + * Builds an audience's top content, resolving each page path to its page title + * and to its Analytics report link. * * @since n.e.x.t * - * @param report The top content report, or `undefined`. - * @param titlesReport The page titles report used to resolve a path to its title, or `undefined`. + * @param report The top content report, or `undefined`. + * @param titlesReport The page titles report used to resolve a path to its title, or `undefined`. + * @param getContentServiceURL Maps a page path to its Analytics report link, or to an empty string when the page has no link. * @return Up to three top content pages. */ export function buildTopContent( report: Report | undefined, - titlesReport: Report | undefined + titlesReport: Report | undefined, + getContentServiceURL: ( pagePath: string ) => string ): AudienceTileTopContent[] { const titlesByPath = ( titlesReport?.rows || [] ).reduce( ( titles: Record< string, string >, row: ReportRow ) => { @@ -225,6 +230,7 @@ export function buildTopContent( return { title: titlesByPath[ pagePath ] || pagePath, pageviews: Number( row.metricValues?.[ 0 ]?.value || 0 ), + serviceURL: getContentServiceURL( pagePath ), }; } ); } @@ -261,6 +267,8 @@ export interface AudienceCardInput { topContentPageTitlesResult: FetchReportResult; /** The site's total pageviews, the percentage denominator. */ totalPageviews: number; + /** Maps a top content page path to its Analytics report link, or to an empty string when the page has no link. */ + getContentServiceURL: ( pagePath: string ) => string; } /** @@ -284,6 +292,7 @@ export function buildPDFAudienceCard( topContentResult, topContentPageTitlesResult, totalPageviews, + getContentServiceURL, } = input; // Don't render when a metrics or cities report has any error, or when @@ -324,7 +333,8 @@ export function buildPDFAudienceCard( ), topContent: buildTopContent( topContentResult.response, - topContentPageTitlesResult.response + topContentPageTitlesResult.response, + getContentServiceURL ), }; } diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts index feaafb37411..9ef8507314d 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts @@ -196,6 +196,20 @@ function buildRegistry( { } return isSiteKitPartialData ? found : null; }, + // Serializes the report type, the page filter, and the date range into + // the link. A test then proves what the loader asked the selector by + // reading the link a top content row holds. + getServiceReportURL: ( + type: string, + { + filters, + dates, + }: { + filters: { unifiedPagePathScreen: string }; + dates: { startDate: string; endDate: string }; + } + ) => + `https://example.com/analytics-report/${ type }?path=${ filters.unifiedPagePathScreen }&range=${ dates.startDate }:${ dates.endDate }`, }; // The loader reads only `resolveSelect`, `select`, and `dispatch`. The mock @@ -222,20 +236,29 @@ function buildRegistry( { * * @since n.e.x.t * - * @param registry The mock registry. - * @param options Run options. - * @param options.aborted Whether the signal is aborted before the run. + * @param registry The mock registry. + * @param options Run options. + * @param options.aborted Whether the signal is aborted before the run. + * @param options.viewOnly Whether the export runs on a view-only dashboard. * @return The loader result. */ function runPDFData( registry: PDFRegistry, - { aborted = false }: { aborted?: boolean } = {} + { + aborted = false, + viewOnly = false, + }: { aborted?: boolean; viewOnly?: boolean } = {} ) { const controller = new AbortController(); if ( aborted ) { controller.abort(); } - return getPDFData( { registry, dates: DATES, signal: controller.signal } ); + return getPDFData( { + registry, + dates: DATES, + signal: controller.signal, + viewOnly, + } ); } /** @@ -330,6 +353,7 @@ describe( 'AudienceTilesWidget getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( fetchGetReport ).toHaveBeenCalled(); @@ -398,10 +422,48 @@ describe( 'AudienceTilesWidget getPDFData', () => { { name: 'Dublin', percentage: 40 / card.metrics.visitors.current }, ] ); expect( card.topContent ).toEqual( [ - { title: 'Post One', pageviews: 80 }, + { + title: 'Post One', + pageviews: 80, + serviceURL: `https://example.com/analytics-report/all-pages-and-screens?path=/post-1&range=${ DATES.startDate }:${ DATES.endDate }`, + }, ] ); } ); + it( 'links each top content page to the same All pages and screens report the dashboard tile links to', async () => { + const { registry } = buildRegistry(); + + const audiences = getAudiences( await runPDFData( registry ) ); + + // The stub selector serializes its type, page filter, and date range + // into the link. So this equality proves the loader asks the same + // selector the dashboard tile asks, with the page path and the report + // date range. + audiences.forEach( ( audience ) => { + expect( audience.topContent[ 0 ].serviceURL ).toBe( + `https://example.com/analytics-report/all-pages-and-screens?path=/post-1&range=${ DATES.startDate }:${ DATES.endDate }` + ); + } ); + } ); + + it( 'builds no top content links on a view-only dashboard', async () => { + const { registry } = buildRegistry(); + + const audiences = getAudiences( + await runPDFData( registry, { viewOnly: true } ) + ); + + // The dashboard tile shows a view-only user each page title as plain + // text, so every card's top content rows hold no link. + const contentRows = audiences.flatMap( + ( audience ) => audience.topContent + ); + expect( contentRows ).not.toHaveLength( 0 ); + contentRows.forEach( ( content ) => { + expect( content.serviceURL ).toBe( '' ); + } ); + } ); + it( 'excludes a failed audience while the other two still load', async () => { const { registry } = buildRegistry( { failing: new Set( [ diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts index 8a25e203a6d..57f52819d2a 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts @@ -230,6 +230,8 @@ async function fetchAudienceReports( * fetches their reports in parallel, and builds a card from each. It drops any * audience whose reports failed, and returns `{ data: null }` when fewer than * two cards remain, so a single-card row never appears. It captures no charts. + * For a view-only user, the loader builds no top content links, because the + * dashboard tile shows each page title as plain text. * * @since n.e.x.t * @@ -237,12 +239,14 @@ async function fetchAudienceReports( * @param params.registry WordPress data registry. * @param params.dates Report date range, with the current day excluded. * @param params.signal Cancellation signal. + * @param params.viewOnly Whether the export runs on a view-only dashboard. * @return The loaded audience cards, or `{ data: null }` when the section is omitted or canceled. */ export default async function getPDFData( { registry, dates, signal, + viewOnly, }: GetPDFDataParams ): Promise< AudienceTilesPDFData > { if ( signal.aborted ) { return { data: null }; @@ -355,6 +359,36 @@ export default async function getPDFData( { ?.value ) || 0; + /** + * Maps a top content page path to its Analytics report link. + * + * Each page title links to the same All pages and screens report the + * dashboard tile links to. For a view-only user, this function resolves no + * link, so each page title renders as plain text, matching how the + * dashboard tile shows it. + * + * @since n.e.x.t + * + * @param pagePath Page path of a top content row. + * @return The page's Analytics report link, or an empty string for a view-only user. + */ + function getContentServiceURL( pagePath: string ): string { + if ( viewOnly ) { + return ''; + } + + const { startDate, endDate } = dates; + + return ( + registry + .select( MODULES_ANALYTICS_4 ) + .getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates: { startDate, endDate }, + } ) ?? '' + ); + } + const audiences = audienceResourceNames .map( ( audienceResourceName: string, index: number ) => { const audience = findAvailableAudience( audienceResourceName ); @@ -372,6 +406,7 @@ export default async function getPDFData( { topContentResult: cardResults[ offset + 1 ], topContentPageTitlesResult: cardResults[ offset + 2 ], totalPageviews, + getContentServiceURL, } ); } ) .filter( ( audience ): audience is AudienceTilePDFData => !! audience ); From f5c19ab10ffda38377a476caeac7a8ecce6083de Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 16 Jul 2026 20:01:44 +0300 Subject: [PATCH 07/12] Pass the view-only flag through the non-linked widgets. These widgets render no row links, so their loaders take the new flag and leave it unread. The three that declare their own params interface add the field there. Each holds a comment saying the loader ignores it, so the shared pipeline typechecks. --- .../ModuleOverviewWidget/getPDFData.test.ts | 15 ++++++++++++++- .../getPDFData.test.ts | 19 ++++++++++++++++++- .../getPDFData.ts | 2 ++ .../getPDFData.test.ts | 7 +++++++ .../DashboardPageSpeedWidget/getPDFData.ts | 2 ++ .../SearchFunnelWidgetGA4/getPDFData.test.ts | 12 +++++++++++- .../SearchFunnelWidgetGA4/getPDFData.ts | 2 ++ 7 files changed, 56 insertions(+), 3 deletions(-) diff --git a/assets/js/modules/adsense/components/module/ModuleOverviewWidget/getPDFData.test.ts b/assets/js/modules/adsense/components/module/ModuleOverviewWidget/getPDFData.test.ts index 2dcf0481f1a..b0a3224da70 100644 --- a/assets/js/modules/adsense/components/module/ModuleOverviewWidget/getPDFData.test.ts +++ b/assets/js/modules/adsense/components/module/ModuleOverviewWidget/getPDFData.test.ts @@ -314,6 +314,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).toEqual( { @@ -348,7 +349,12 @@ describe( 'ModuleOverviewWidget getPDFData', () => { provideReportsWithData(); const signal = new AbortController().signal; - await getPDFData( { registry, dates: DATES, signal } ); + await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); expect( mockEnsureGoogleChartsLoaded ).toHaveBeenCalledTimes( 1 ); expect( mockRenderGoogleChartToDataURI ).toHaveBeenCalledTimes( 4 ); @@ -410,6 +416,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.metrics.pageCTR ).toBeNull(); @@ -451,6 +458,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // Empty data is not a failure. The report skips the section. @@ -501,6 +509,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow( /Earning performance over time reports failed to load/ @@ -527,6 +536,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.metrics.estimatedEarnings ).toBeNull(); @@ -552,6 +562,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow( /all Earning performance over time charts failed to render/ @@ -568,6 +579,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -595,6 +607,7 @@ describe( 'ModuleOverviewWidget getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for all four report fetches to dispatch before aborting. diff --git a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts index f9cd1d0973f..33934c581e6 100644 --- a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts @@ -215,6 +215,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).toEqual( { @@ -294,7 +295,12 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { } ); const signal = new AbortController().signal; - const result = await getPDFData( { registry, dates: DATES, signal } ); + const result = await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); expect( mockEnsureGoogleChartsLoaded ).toHaveBeenCalledTimes( 1 ); @@ -382,6 +388,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); // Six countries collapse to the top four plus a single "Others" row, @@ -435,6 +442,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); const pieCalls = mockRenderGoogleChartToDataURI.mock.calls.filter( @@ -498,6 +506,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.locationBreakdown ).toBeNull(); @@ -558,6 +567,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.locationBreakdown ).toBeNull(); @@ -591,6 +601,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); const calls = fetchMock.calls( reportEndpoint ); @@ -610,6 +621,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal, + viewOnly: false, } ); // The registry starts resolver runs from a timeout. Wait the @@ -638,6 +650,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -668,6 +681,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for the report fetches to dispatch before aborting. @@ -732,6 +746,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -765,6 +780,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: firstController.signal, + viewOnly: false, } ); // Wait for all five report requests to start before aborting. @@ -782,6 +798,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( fetchMock.calls( reportEndpoint ) ).toHaveLength( 10 ); diff --git a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts index 468bbe287f4..63ac58a9d4f 100644 --- a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts @@ -113,6 +113,8 @@ export interface GetPDFDataParams { >; /** Signal that cancels the export. */ signal: AbortSignal; + /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ + viewOnly: boolean; } /** diff --git a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts index 07ded02dd45..e367387e60e 100644 --- a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts +++ b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts @@ -105,6 +105,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ); expect( resolveSelectSpy ).toHaveBeenCalledWith( CORE_SITE ); @@ -125,6 +126,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -160,6 +162,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -178,6 +181,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow(); @@ -194,6 +198,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow(); expect( console ).toHaveErrored(); @@ -209,6 +214,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -225,6 +231,7 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: controller.signal, + viewOnly: false, } ); // Null data tells the report document to skip the widget, so the PDF diff --git a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts index 860315da6c2..adbb5fb3475 100644 --- a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts +++ b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts @@ -67,6 +67,8 @@ export interface GetPDFDataParams { dates: unknown; /** The export's abort signal. When it aborts, the loader resolves with null data. */ signal: AbortSignal; + /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ + viewOnly: boolean; } export interface StrategyData { diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts index eae8a2e9c19..dd12678af02 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts @@ -222,6 +222,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data ).toEqual( { @@ -253,7 +254,12 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { provideReports( registry ); const signal = new AbortController().signal; - await getPDFData( { registry, dates: DATES, signal } ); + await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); expect( mockEnsureGoogleChartsLoaded ).toHaveBeenCalledTimes( 1 ); expect( mockRenderGoogleChartToDataURI ).toHaveBeenCalledTimes( 4 ); @@ -317,6 +323,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.metrics.impressions ).not.toBeNull(); @@ -359,6 +366,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow( /all Search traffic over time metrics failed/ ); @@ -379,6 +387,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -407,6 +416,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); // Wait for all four report fetches to dispatch before aborting. diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts index 84c9246c240..752e0583848 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts @@ -150,6 +150,8 @@ export interface GetPDFDataParams { }; /** Cancellation signal. */ signal: AbortSignal; + /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ + viewOnly: boolean; } export interface SearchFunnelMetric { From 4727bf104a2fda5e6c04e03f85a2189c3bf0a1ef Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Tue, 21 Jul 2026 21:21:48 +0300 Subject: [PATCH 08/12] Adress CR feedback. --- .../create-key-metric-tile-data-loader.ts | 4 +- .../components/KeyMetrics/getPDFData.test.ts | 14 +++- assets/js/components/KeyMetrics/getPDFData.ts | 6 +- .../shared-react-pdf-components/PDFHeader.tsx | 26 +++----- .../PDFLink.test.tsx | 15 +++++ .../shared-react-pdf-components/PDFLink.tsx | 3 - assets/js/googlesitekit/widgets/types.ts | 7 +- .../getPDFData.test.ts | 64 +++++++++++++++++++ .../getPDFData.ts | 13 ++++ .../indexPDF.test.tsx | 2 + .../buildPDFAudienceCard.ts | 4 +- .../getPDFData.test.ts | 19 +----- .../getPDFData.ts | 2 - .../ModulePopularPagesWidgetGA4PDF.test.tsx | 6 +- .../ModulePopularPagesWidgetGA4PDF.tsx | 21 +++--- .../getPDFData.test.ts | 56 +++++++++++++--- .../ModulePopularPagesWidgetGA4/getPDFData.ts | 61 +++++++++++------- .../getPDFData.test.ts | 7 -- .../DashboardPageSpeedWidget/getPDFData.ts | 2 - .../SearchFunnelWidgetGA4/getPDFData.test.ts | 12 +--- .../SearchFunnelWidgetGA4/getPDFData.ts | 2 - 21 files changed, 226 insertions(+), 120 deletions(-) diff --git a/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts b/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts index a5c6072c025..cfff8ad4c4c 100644 --- a/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts +++ b/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts @@ -64,12 +64,12 @@ interface FetchReportResult { export default function createKeyMetricTileDataLoader< TData >( buildReports: ( dates: PDFReportDates ) => TileReportRequest[], extract: ( reports: unknown[] ) => TData -): ( params: GetPDFDataParams ) => Promise< TData | null > { +): ( params: Omit< GetPDFDataParams, 'viewOnly' > ) => Promise< TData | null > { return async function getTileData( { registry, dates, signal, - }: GetPDFDataParams ): Promise< TData | null > { + }: Omit< GetPDFDataParams, 'viewOnly' > ): Promise< TData | null > { if ( signal.aborted ) { return null; } diff --git a/assets/js/components/KeyMetrics/getPDFData.test.ts b/assets/js/components/KeyMetrics/getPDFData.test.ts index cd1cc9cc7da..dfc6bad6631 100644 --- a/assets/js/components/KeyMetrics/getPDFData.test.ts +++ b/assets/js/components/KeyMetrics/getPDFData.test.ts @@ -84,8 +84,15 @@ describe( 'Key Metrics getPDFData', () => { ] ); const { signal } = new AbortController(); - const result = await getPDFData( { registry, dates: DATES, signal } ); + const result = await getPDFData( { + registry, + dates: DATES, + signal, + viewOnly: false, + } ); + // The aggregate loader has no view-only branch, so each tile loader + // receives only the registry, dates, and signal. expect( getTileDataA ).toHaveBeenCalledWith( { registry, dates: DATES, @@ -131,6 +138,7 @@ describe( 'Key Metrics getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result.data?.tiles ).toEqual( [ @@ -176,6 +184,7 @@ describe( 'Key Metrics getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ) ).rejects.toThrow( 'All Key Metrics PDF tiles failed to load.' ); } ); @@ -190,6 +199,7 @@ describe( 'Key Metrics getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -210,6 +220,7 @@ describe( 'Key Metrics getPDFData', () => { registry, dates: DATES, signal: controller.signal, + viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -239,6 +250,7 @@ describe( 'Key Metrics getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, + viewOnly: false, } ); expect( preload ).toHaveBeenCalledTimes( 1 ); diff --git a/assets/js/components/KeyMetrics/getPDFData.ts b/assets/js/components/KeyMetrics/getPDFData.ts index 662e6f0646e..474887e7bcf 100644 --- a/assets/js/components/KeyMetrics/getPDFData.ts +++ b/assets/js/components/KeyMetrics/getPDFData.ts @@ -44,8 +44,10 @@ interface KeyMetricPDFEntry { default: ComponentType< never >; } >; }; - /** Resolves the tile's data, or `null` when the export is canceled. */ - getTileData: ( params: GetPDFDataParams ) => Promise< unknown >; + /** Resolves the tile's data, or `null` when the caller cancels the export. Key Metrics tiles have no view-only links, so the loader takes no `viewOnly`. */ + getTileData: ( + params: Omit< GetPDFDataParams, 'viewOnly' > + ) => Promise< unknown >; }; } diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFHeader.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFHeader.tsx index 723a1a56db9..a099e77756f 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFHeader.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFHeader.tsx @@ -201,24 +201,14 @@ const PDFHeader: FC< PDFHeaderProps > = ( { - { dashboardURL ? ( - - { getSiteHost( siteURL ) } - - ) : ( - - { getSiteHost( siteURL ) } - - ) } + + { getSiteHost( siteURL ) } + diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx index f2b1a73421d..215f548fa36 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx @@ -120,6 +120,21 @@ describe( 'PDFLink', () => { expect( treeJSON ).not.toContain( PDF_COLORS.CONTENT_SECONDARY ); } ); + it( 'renders plain text in the default color when the href is undefined', () => { + // `href` is optional, so a caller can leave it out. An undefined `href` + // renders plain text, the same as an empty one. + const tree = renderLink( { + href: undefined, + children: 'View dashboard', + } ); + + expect( tree.type ).toBe( 'pdf-text' ); + + const treeJSON = JSON.stringify( tree ); + expect( treeJSON ).toContain( 'View dashboard' ); + expect( treeJSON ).not.toContain( PDF_COLORS.CONTENT_SECONDARY ); + } ); + it( 'leaves out the trailing icon when the href is empty', () => { const tree = renderLink( { href: '', diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx index 84436a9f620..367ac78c6f2 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx @@ -63,9 +63,6 @@ const PDFLink: FC< PDFLinkProps > = ( { style, children, } ) => { - // With no URL, the text renders plain, in the default text color. A caller - // that has no link for a row passes an empty `href`, so the row reads as - // plain text. if ( ! href ) { return ( diff --git a/assets/js/googlesitekit/widgets/types.ts b/assets/js/googlesitekit/widgets/types.ts index 5611c6b6ac2..27ebfc84f04 100644 --- a/assets/js/googlesitekit/widgets/types.ts +++ b/assets/js/googlesitekit/widgets/types.ts @@ -103,12 +103,7 @@ export interface WidgetPDFData { */ export interface WidgetPDFConfig { Component: PDFWidgetComponent; - getData: ( params: { - registry: unknown; - dates: PDFReportDates; - signal: AbortSignal; - viewOnly: boolean; - } ) => Promise< WidgetPDFData >; + getData: ( params: GetPDFDataParams ) => Promise< WidgetPDFData >; label?: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- The registry `select` is loosely typed, so `isActive` predicates can read store selectors without casting. isActive?: ( select: ( storeName: string ) => any ) => boolean; diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts index 31cb3f5de18..7980e028698 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts @@ -282,6 +282,70 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { expect( mainRequest ).not.toContain( '(undefined)' ); } ); + it( 'resolves the Analytics settings before building the links when they are not loaded yet', async () => { + // Fresh registry without the Analytics settings from `beforeEach`, so the + // export starts before the Analytics store has loaded the property. The + // AdSense settings go back in, so the main report builds on its own. + registry = createTestRegistry() as Registry; + provideUserInfo( registry ); + registry + .dispatch( MODULES_ADSENSE ) + .receiveGetSettings( { accountID: ACCOUNT_ID } ); + + const analyticsSettings = new RegExp( + '^/google-site-kit/v1/modules/analytics-4/data/settings' + ); + fetchMock.get( analyticsSettings, { + body: { propertyID: PROPERTY_ID }, + status: 200, + } ); + provideReports(); + + const result = await getPDFData( { + registry, + dates: DATES, + signal: new AbortController().signal, + viewOnly: false, + } ); + + // The loader fetched the Analytics settings, so each page link holds the + // property's report URL instead of the empty string an unset property gives. + expect( fetchMock ).toHaveFetched( analyticsSettings ); + expect( result.data?.links[ '/' ] ).toBeTruthy(); + expect( result.data?.links[ '/' ] ).toBe( + getExpectedPageLink( registry, '/' ) + ); + } ); + + it( 'does not resolve the Analytics settings on a view-only dashboard', async () => { + // Fresh registry without the Analytics settings, and a view-only export, + // which builds no page links and so never reads the property. + registry = createTestRegistry() as Registry; + provideUserInfo( registry ); + registry + .dispatch( MODULES_ADSENSE ) + .receiveGetSettings( { accountID: ACCOUNT_ID } ); + + const analyticsSettings = new RegExp( + '^/google-site-kit/v1/modules/analytics-4/data/settings' + ); + fetchMock.get( analyticsSettings, { + body: { propertyID: PROPERTY_ID }, + status: 200, + } ); + provideReports(); + + const result = await getPDFData( { + registry, + dates: DATES, + signal: new AbortController().signal, + viewOnly: true, + } ); + + expect( fetchMock ).not.toHaveFetched( analyticsSettings ); + expect( result.data?.links ).toEqual( {} ); + } ); + it( 'keeps the first title for a repeated path and falls back to "(unknown)" for a missing one', async () => { const mainReport = { metadata: { currencyCode: 'EUR' }, diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts index 3f961d28198..c57ab4228b8 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts @@ -155,6 +155,19 @@ export default async function getPDFData( { return { data: null }; } + // Each page title links to its Analytics report, which `getServiceReportURL` + // builds from the Analytics property ID in the Analytics settings. The PDF + // export path resolves the AdSense settings but not the Analytics ones, so + // resolve them here, or the property ID stays undefined and every link comes + // back empty. A view-only export builds no links, so it skips the resolution. + if ( ! viewOnly ) { + await registry.resolveSelect( MODULES_ANALYTICS_4 ).getSettings(); + + if ( signal.aborted ) { + return { data: null }; + } + } + const currencyCode = report?.metadata?.currencyCode ?? ''; const pagePaths = getPagePaths( report ); diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx index b52e5d6008a..6b7d09fb8ab 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/indexPDF.test.tsx @@ -123,6 +123,8 @@ describe( 'DashboardTopEarningPagesWidgetGA4PDF', () => { expect( json ).toContain( 'About Title' ); // No `pdf-link` in the tree means every title rendered as plain text. expect( json ).not.toContain( 'pdf-link' ); + // Plain text drops the teal link color for the default text color. + expect( json ).not.toContain( '#108080' ); } ); it( 'numbers each row by its rank in the page title cell', () => { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts index 1d5c4087392..059976ae693 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts @@ -58,8 +58,8 @@ export interface AudienceTileTopContent { title: string; /** The page's pageviews. */ pageviews: number; - /** Analytics report link for the page, which the title links to. Empty when the page has no link, so the title renders as plain text. */ - serviceURL: string; + /** Analytics report link for the page, which the title links to, when the page has one. */ + serviceURL?: string; } /** One audience card's fully loaded data. */ diff --git a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts index 33934c581e6..f9cd1d0973f 100644 --- a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.test.ts @@ -215,7 +215,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data ).toEqual( { @@ -295,12 +294,7 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { } ); const signal = new AbortController().signal; - const result = await getPDFData( { - registry, - dates: DATES, - signal, - viewOnly: false, - } ); + const result = await getPDFData( { registry, dates: DATES, signal } ); expect( mockEnsureGoogleChartsLoaded ).toHaveBeenCalledTimes( 1 ); @@ -388,7 +382,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); // Six countries collapse to the top four plus a single "Others" row, @@ -442,7 +435,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); const pieCalls = mockRenderGoogleChartToDataURI.mock.calls.filter( @@ -506,7 +498,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data?.locationBreakdown ).toBeNull(); @@ -567,7 +558,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data?.locationBreakdown ).toBeNull(); @@ -601,7 +591,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); const calls = fetchMock.calls( reportEndpoint ); @@ -621,7 +610,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal, - viewOnly: false, } ); // The registry starts resolver runs from a timeout. Wait the @@ -650,7 +638,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, - viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -681,7 +668,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, - viewOnly: false, } ); // Wait for the report fetches to dispatch before aborting. @@ -746,7 +732,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, - viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -780,7 +765,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: firstController.signal, - viewOnly: false, } ); // Wait for all five report requests to start before aborting. @@ -798,7 +782,6 @@ describe( 'DashboardAllTrafficWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( fetchMock.calls( reportEndpoint ) ).toHaveLength( 10 ); diff --git a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts index 63ac58a9d4f..468bbe287f4 100644 --- a/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/dashboard/DashboardAllTrafficWidgetGA4/getPDFData.ts @@ -113,8 +113,6 @@ export interface GetPDFDataParams { >; /** Signal that cancels the export. */ signal: AbortSignal; - /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ - viewOnly: boolean; } /** diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx index 04037482b25..7554b49db3e 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.test.tsx @@ -55,11 +55,11 @@ const DATA = { titles: { '/home-page': 'Home Title', '/about-page': 'About Title' }, links: { '/home-page': { - serviceURL: 'https://example.com/analytics-report/report-1', + titleURL: 'https://example.com/analytics-report/report-1', permaLink: 'http://example.com/home-page', }, '/about-page': { - serviceURL: 'https://example.com/analytics-report/report-2', + titleURL: 'https://example.com/analytics-report/report-2', permaLink: 'http://example.com/about-page', }, }, @@ -128,7 +128,7 @@ describe( 'ModulePopularPagesWidgetGA4PDF', () => { const json = renderJSON( { data: DATA } ); // The title links to the page's Analytics report. - expect( json ).toContain( DATA.links[ '/home-page' ].serviceURL ); + expect( json ).toContain( DATA.links[ '/home-page' ].titleURL ); // The URL line links to the page itself. expect( json ).toContain( 'http://example.com/home-page' ); expect( json ).toContain( 'http://example.com/about-page' ); diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx index 3b8f025d299..134c29cc3bc 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/ModulePopularPagesWidgetGA4PDF.tsx @@ -62,9 +62,9 @@ interface PopularPageRow { title: string; /** Page path shown under the title, shortened with an ellipsis when longer than `MAX_URL_LENGTH`. */ displayURL: string; - /** Analytics report link for the page, which the title links to. Empty when the page has no link, so the title renders as plain text. */ - serviceURL: string; - /** The page's own public URL, which the URL line links to. Empty when the page has no link, so the URL line renders as plain text. */ + /** The link the title points to: the Analytics report for an administrator, the entity dashboard for a view-only user. */ + titleURL: string; + /** The page's own public URL, which the URL line links to. */ permaLink: string; /** Pageviews count, as the report's raw string. */ pageviews: string; @@ -87,7 +87,7 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { const tableRows: PopularPageRow[] = rows.map( ( row, index ) => { const url = row.dimensionValues?.[ 0 ]?.value ?? ''; - const { serviceURL = '', permaLink = '' } = links[ url ] ?? {}; + const { titleURL = '', permaLink = '' } = links[ url ] ?? {}; const displayURL = url.length > MAX_URL_LENGTH ? `${ url.slice( 0, MAX_URL_LENGTH ) }…` @@ -97,7 +97,7 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { rank: index + 1, title: titles[ url ] ?? '', displayURL, - serviceURL, + titleURL, permaLink, pageviews: row.metricValues?.[ 0 ]?.value ?? '', sessions: row.metricValues?.[ 1 ]?.value ?? '', @@ -112,17 +112,18 @@ const ModulePopularPagesWidgetGA4PDF: FC< PDFWidgetComponentProps > = ( { // 42.8% of the 1084px row, about 464px. width: '42.8%', // Shows the page title above its URL, with the row rank to the - // left. The title links to the page's Analytics report, like the - // dashboard widget, and the URL links to the page itself. When a - // row has no links, `PDFLink` renders both lines as plain text - // instead of as links. + // left. The title links to the page's Analytics report for an + // administrator and to its entity dashboard for a view-only user, + // like the dashboard widget, and the URL links to the page itself. + // When a row has no link, `PDFLink` renders the line as plain text + // instead of as a link. cell: ( row ) => ( { `${ row.rank }.` } - { row.title } + { row.title } { row.displayURL } diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts index 3b8fe875747..4e057d36a35 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts @@ -35,8 +35,10 @@ import { WPDataRegistry } from '@wordpress/data/build-types/registry'; /** * Internal dependencies */ +import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants'; import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants'; +import { getFullURL } from '@/js/util'; import getPDFData from './getPDFData'; type Registry = WPDataRegistry & GetPDFDataParams[ 'registry' ]; @@ -120,6 +122,28 @@ function getExpectedPageLink( registry: Registry, pagePath: string ): string { } ); } +/** + * Builds the entity dashboard link the loader resolves for a view-only page path. + * + * The link comes from the same selector the loader uses, so a test checks the + * title falls back to the page's entity dashboard, not the URL format. + * + * @since n.e.x.t + * + * @param registry Registry that holds the reference site URL. + * @param pagePath Page path from a report row. + * @return The page's entity dashboard link. + */ +function getExpectedDetailsURL( registry: Registry, pagePath: string ): string { + const siteURL = registry.select( CORE_SITE ).getReferenceSiteURL(); + + return registry + .select( CORE_SITE ) + .getAdminURL( 'googlesitekit-dashboard', { + permaLink: getFullURL( siteURL, pagePath ), + } ); +} + /** * Sets up `fetchMock` so each report request returns the matching fixture. Only * the page titles report requests the `pageTitle` dimension, so a request whose @@ -194,11 +218,11 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { titles: { '/': 'Home', '/about': 'About' }, links: { '/': { - serviceURL: homeLink, + titleURL: homeLink, permaLink: 'http://example.com/', }, '/about': { - serviceURL: getExpectedPageLink( registry, '/about' ), + titleURL: getExpectedPageLink( registry, '/about' ), permaLink: 'http://example.com/about', }, }, @@ -206,7 +230,7 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { } ); } ); - it( 'builds no page links on a view-only dashboard', async () => { + it( 'links each title to its entity dashboard on a view-only dashboard', async () => { fetchMock.get( reportEndpoint, ( requestURL ) => ( { body: requestURL.includes( 'pageTitle' ) ? TITLES_REPORT @@ -221,13 +245,29 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { viewOnly: true, } ); - // The rows and the titles still render. Only the links drop, so the PDF - // shows each title and URL as plain text. + // A view-only user can't reach the Analytics report, so each title falls + // back to the page's entity dashboard, the same link the dashboard widget + // renders. The URL line still links to the page itself. + const homeDetailsURL = getExpectedDetailsURL( registry, '/' ); + expect( homeDetailsURL ).toBeTruthy(); + expect( homeDetailsURL ).not.toBe( + getExpectedPageLink( registry, '/' ) + ); + expect( result ).toEqual( { data: { rows: MAIN_REPORT.rows, titles: { '/': 'Home', '/about': 'About' }, - links: {}, + links: { + '/': { + titleURL: homeDetailsURL, + permaLink: 'http://example.com/', + }, + '/about': { + titleURL: getExpectedDetailsURL( registry, '/about' ), + permaLink: 'http://example.com/about', + }, + }, }, } ); } ); @@ -439,11 +479,11 @@ describe( 'ModulePopularPagesWidgetGA4 getPDFData', () => { titles: { '/': 'Home', '/about': 'About' }, links: { '/': { - serviceURL: getExpectedPageLink( registry, '/' ), + titleURL: getExpectedPageLink( registry, '/' ), permaLink: 'http://example.com/', }, '/about': { - serviceURL: getExpectedPageLink( registry, '/about' ), + titleURL: getExpectedPageLink( registry, '/about' ), permaLink: 'http://example.com/about', }, }, diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts index 14e6680007c..e546041a488 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts @@ -32,12 +32,12 @@ import { getFullURL } from '@/js/util'; import { getPopularPagesReportArgs } from './reportOptions'; /** - * Links for one page row: the Analytics report link for the title, and the - * page's own public URL for the URL line. + * Links for one page row: the link the page title points to, and the page's own + * public URL for the URL line. */ export interface PopularPageLinks { - /** Analytics report link for the page, which the page title links to. */ - serviceURL: string; + /** The link the page title points to: the Analytics report for an administrator, the page's entity dashboard for a view-only user. */ + titleURL: string; /** The page's own public URL, which the URL line links to. */ permaLink: string; } @@ -52,44 +52,63 @@ export interface PopularPagesPDFData { rows: ReportRow[]; /** Map of page path to page title. */ titles: Record< string, string >; - /** Map of page path to its Analytics report link and public URL. Empty on a view-only dashboard, where each title and URL show as plain text. */ + /** Map of page path to its title link and public URL. */ links: Record< string, PopularPageLinks >; } | null; } /** - * Maps each page path to its Analytics report link and public URL. + * Maps each page path to its title link and public URL. * - * The title links to the same All pages and screens report the dashboard - * widget links to for an administrator. The URL line links to the page itself. + * The title links to the same All pages and screens report the dashboard widget + * links to for an administrator, and to the page's entity dashboard for a + * view-only user, the same `serviceURL || detailsURL` fallback the dashboard + * widget's `DetailsPermaLinks` renders. The URL line links to the page itself + * for every user. * * @since 1.182.0 - * @since n.e.x.t Links the title to the page's Analytics report instead of its entity dashboard. + * @since n.e.x.t Links the title to the Analytics report for an administrator and to the entity dashboard for a view-only user. * * @param registry WordPress data registry. * @param dates Report date range. * @param pagePaths Page paths from the main report rows. - * @return Map of page path to its Analytics report link and public URL. + * @param viewOnly Whether the export runs on a view-only dashboard. + * @return Map of page path to its title link and public URL. */ function getPopularPageLinkMap( registry: GetPDFDataParams[ 'registry' ], dates: GetPDFDataParams[ 'dates' ], - pagePaths: string[] + pagePaths: string[], + viewOnly: boolean ): Record< string, PopularPageLinks > { - const siteURL = registry.select( CORE_SITE ).getReferenceSiteURL(); + const coreSite = registry.select( CORE_SITE ); + const siteURL = coreSite.getReferenceSiteURL(); const analytics = registry.select( MODULES_ANALYTICS_4 ); const { startDate, endDate } = dates; const links: Record< string, PopularPageLinks > = {}; pagePaths.forEach( ( pagePath ) => { - links[ pagePath ] = { - serviceURL: - analytics.getServiceReportURL( 'all-pages-and-screens', { + const permaLink = getFullURL( siteURL, pagePath ); + + // An administrator's title links to the Analytics report. A view-only + // user has no service link, so the title falls back to the page's entity + // dashboard, the same `serviceURL || detailsURL` fallback the dashboard + // widget renders for that user. + const serviceURL = viewOnly + ? null + : analytics.getServiceReportURL( 'all-pages-and-screens', { filters: { unifiedPagePathScreen: pagePath }, dates: { startDate, endDate }, - } ) ?? '', - permaLink: getFullURL( siteURL, pagePath ), + } ); + + const detailsURL = coreSite.getAdminURL( 'googlesitekit-dashboard', { + permaLink, + } ); + + links[ pagePath ] = { + titleURL: serviceURL || detailsURL || '', + permaLink, }; } ); @@ -106,7 +125,7 @@ function getPopularPageLinkMap( * * @since 1.182.0 * @since 1.183.0 Returns null data when the report has no rows. - * @since n.e.x.t Links each title to its Analytics report, and leaves out the page links on a view-only dashboard. + * @since n.e.x.t Links each title to the Analytics report for an administrator and to the entity dashboard for a view-only user. * * @param params Loader parameters. * @param params.registry WordPress data registry. @@ -150,11 +169,7 @@ export default async function getPDFData( { } const pagePaths = getPagePaths( report ); - // For a view-only user, the loader builds no page links, so the title and - // the URL both render as plain text. - const links = viewOnly - ? {} - : getPopularPageLinkMap( registry, dates, pagePaths ); + const links = getPopularPageLinkMap( registry, dates, pagePaths, viewOnly ); if ( pagePaths.length === 0 ) { return { data: { rows, titles: {}, links } }; diff --git a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts index e367387e60e..07ded02dd45 100644 --- a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts +++ b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.test.ts @@ -105,7 +105,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ); expect( resolveSelectSpy ).toHaveBeenCalledWith( CORE_SITE ); @@ -126,7 +125,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -162,7 +160,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -181,7 +178,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ) ).rejects.toThrow(); @@ -198,7 +194,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ) ).rejects.toThrow(); expect( console ).toHaveErrored(); @@ -214,7 +209,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data ).not.toBeNull(); @@ -231,7 +225,6 @@ describe( 'getPDFData', () => { registry, dates: undefined, signal: controller.signal, - viewOnly: false, } ); // Null data tells the report document to skip the widget, so the PDF diff --git a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts index adbb5fb3475..860315da6c2 100644 --- a/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts +++ b/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeedWidget/getPDFData.ts @@ -67,8 +67,6 @@ export interface GetPDFDataParams { dates: unknown; /** The export's abort signal. When it aborts, the loader resolves with null data. */ signal: AbortSignal; - /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ - viewOnly: boolean; } export interface StrategyData { diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts index dd12678af02..eae8a2e9c19 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.test.ts @@ -222,7 +222,6 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data ).toEqual( { @@ -254,12 +253,7 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { provideReports( registry ); const signal = new AbortController().signal; - await getPDFData( { - registry, - dates: DATES, - signal, - viewOnly: false, - } ); + await getPDFData( { registry, dates: DATES, signal } ); expect( mockEnsureGoogleChartsLoaded ).toHaveBeenCalledTimes( 1 ); expect( mockRenderGoogleChartToDataURI ).toHaveBeenCalledTimes( 4 ); @@ -323,7 +317,6 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ); expect( result.data?.metrics.impressions ).not.toBeNull(); @@ -366,7 +359,6 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: new AbortController().signal, - viewOnly: false, } ) ).rejects.toThrow( /all Search traffic over time metrics failed/ ); @@ -387,7 +379,6 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, - viewOnly: false, } ); expect( result ).toEqual( { data: null } ); @@ -416,7 +407,6 @@ describe( 'SearchFunnelWidgetGA4 getPDFData', () => { registry, dates: DATES, signal: controller.signal, - viewOnly: false, } ); // Wait for all four report fetches to dispatch before aborting. diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts index 752e0583848..84c9246c240 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/getPDFData.ts @@ -150,8 +150,6 @@ export interface GetPDFDataParams { }; /** Cancellation signal. */ signal: AbortSignal; - /** Whether the export runs on a view-only dashboard. This widget has no links to leave out, so the loader doesn't read it. */ - viewOnly: boolean; } export interface SearchFunnelMetric { From 9555d95efa14d20e042d4b7df6d0a2df6f4c2975 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Tue, 21 Jul 2026 22:03:09 +0300 Subject: [PATCH 09/12] Refactor PDF data loading parameters for consistency. Updated the PDF data loader interfaces to use a unified parameter structure, improving clarity and maintainability. Introduced `PDFDataLoaderParams` and adjusted related functions accordingly. New tests added for the `getAllPagesReportURL` utility to ensure correct functionality. --- .../create-key-metric-tile-data-loader.ts | 6 +- assets/js/components/KeyMetrics/getPDFData.ts | 11 ++-- assets/js/googlesitekit/widgets/types.ts | 16 ++++-- .../getPDFData.ts | 23 +++++--- .../AudienceTilesWidget/getPDFData.ts | 12 ++-- .../ModulePopularPagesWidgetGA4/getPDFData.ts | 7 ++- .../analytics-4/utils/page-report-url.test.ts | 57 +++++++++++++++++++ .../analytics-4/utils/page-report-url.ts | 57 +++++++++++++++++++ 8 files changed, 160 insertions(+), 29 deletions(-) create mode 100644 assets/js/modules/analytics-4/utils/page-report-url.test.ts create mode 100644 assets/js/modules/analytics-4/utils/page-report-url.ts diff --git a/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts b/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts index cfff8ad4c4c..c0300ea61ec 100644 --- a/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts +++ b/assets/js/components/KeyMetrics/create-key-metric-tile-data-loader.ts @@ -20,7 +20,7 @@ * Internal dependencies */ import { - GetPDFDataParams, + PDFDataLoaderParams, PDFReportDates, } from '@/js/googlesitekit/widgets/types'; @@ -64,12 +64,12 @@ interface FetchReportResult { export default function createKeyMetricTileDataLoader< TData >( buildReports: ( dates: PDFReportDates ) => TileReportRequest[], extract: ( reports: unknown[] ) => TData -): ( params: Omit< GetPDFDataParams, 'viewOnly' > ) => Promise< TData | null > { +): ( params: PDFDataLoaderParams ) => Promise< TData | null > { return async function getTileData( { registry, dates, signal, - }: Omit< GetPDFDataParams, 'viewOnly' > ): Promise< TData | null > { + }: PDFDataLoaderParams ): Promise< TData | null > { if ( signal.aborted ) { return null; } diff --git a/assets/js/components/KeyMetrics/getPDFData.ts b/assets/js/components/KeyMetrics/getPDFData.ts index 474887e7bcf..f6296600322 100644 --- a/assets/js/components/KeyMetrics/getPDFData.ts +++ b/assets/js/components/KeyMetrics/getPDFData.ts @@ -25,7 +25,10 @@ import { ComponentType } from 'react'; * Internal dependencies */ import { CORE_USER } from '@/js/googlesitekit/datastore/user/constants'; -import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; +import { + GetPDFDataParams, + PDFDataLoaderParams, +} from '@/js/googlesitekit/widgets/types'; import { KEY_METRICS_WIDGETS } from './key-metrics-widgets'; /** @@ -44,10 +47,8 @@ interface KeyMetricPDFEntry { default: ComponentType< never >; } >; }; - /** Resolves the tile's data, or `null` when the caller cancels the export. Key Metrics tiles have no view-only links, so the loader takes no `viewOnly`. */ - getTileData: ( - params: Omit< GetPDFDataParams, 'viewOnly' > - ) => Promise< unknown >; + /** Resolves the tile's data, or `null` when the caller cancels the export. */ + getTileData: ( params: PDFDataLoaderParams ) => Promise< unknown >; }; } diff --git a/assets/js/googlesitekit/widgets/types.ts b/assets/js/googlesitekit/widgets/types.ts index 27ebfc84f04..0e17e670bdc 100644 --- a/assets/js/googlesitekit/widgets/types.ts +++ b/assets/js/googlesitekit/widgets/types.ts @@ -39,12 +39,11 @@ export interface PDFReportDates { } /** - * Parameters a PDF widget's `getData` loader receives. + * Base parameters a PDF widget's `getData` loader receives. * - * @since 1.183.0 - * @since n.e.x.t Added `viewOnly`. + * @since n.e.x.t */ -export interface GetPDFDataParams { +export interface PDFDataLoaderParams { /** WordPress data registry, with `resolveSelect` added. */ registry: WPDataRegistry & { // `resolveSelect` is on the registry at runtime but missing from the @@ -56,6 +55,15 @@ export interface GetPDFDataParams { dates: PDFReportDates; /** Signal that cancels the export. */ signal: AbortSignal; +} + +/** + * Parameters a PDF widget's `getData` loader receives. + * + * @since 1.183.0 + * @since n.e.x.t Added `viewOnly`. + */ +export interface GetPDFDataParams extends PDFDataLoaderParams { /** Whether the export runs on a view-only dashboard, where a loader leaves out the links an administrator sees. */ viewOnly: boolean; } diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts index c57ab4228b8..e11213ba203 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts @@ -23,6 +23,7 @@ import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; import { MODULES_ADSENSE } from '@/js/modules/adsense/datastore/constants'; import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants'; import { Report, ReportRow } from '@/js/modules/analytics-4/datastore/types'; +import { getAllPagesReportURL } from '@/js/modules/analytics-4/utils/page-report-url'; import { getPagePaths, getPageTitleMap, @@ -66,22 +67,30 @@ interface GetPDFDataResult { * @param registry WordPress data registry. * @param dates Report date range. * @param pagePaths Page paths from the main report rows. - * @return Map of page path to its Analytics report link. + * @param viewOnly Whether the export runs on a view-only dashboard. + * @return Map of page path to its Analytics report link, empty for a view-only user. */ function getPageLinkMap( registry: GetPDFDataParams[ 'registry' ], dates: GetPDFDataParams[ 'dates' ], - pagePaths: string[] + pagePaths: string[], + viewOnly: boolean ): Record< string, string > { + // A view-only user sees each page title as plain text on the dashboard, so + // the PDF builds no links either. + if ( viewOnly ) { + return {}; + } + const analytics = registry.select( MODULES_ANALYTICS_4 ); const { startDate, endDate } = dates; const links: Record< string, string > = {}; pagePaths.forEach( ( pagePath ) => { links[ pagePath ] = - analytics.getServiceReportURL( 'all-pages-and-screens', { - filters: { unifiedPagePathScreen: pagePath }, - dates: { startDate, endDate }, + getAllPagesReportURL( analytics, pagePath, { + startDate, + endDate, } ) ?? ''; } ); @@ -171,9 +180,7 @@ export default async function getPDFData( { const currencyCode = report?.metadata?.currencyCode ?? ''; const pagePaths = getPagePaths( report ); - // For a view-only user, the loader builds no page links, so each page title - // renders as plain text. - const links = viewOnly ? {} : getPageLinkMap( registry, dates, pagePaths ); + const links = getPageLinkMap( registry, dates, pagePaths, viewOnly ); if ( pagePaths.length === 0 ) { return { data: { rows, currencyCode, titles: {}, links } }; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts index 57f52819d2a..c95c1f1d1c1 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts @@ -34,6 +34,7 @@ import { getAudienceTilesTopContentReportOptions, getAudienceTilesTotalPageviewsReportOptions, } from '@/js/modules/analytics-4/utils/audienceTilesReportOptions'; +import { getAllPagesReportURL } from '@/js/modules/analytics-4/utils/page-report-url'; import { AudienceTilePDFData, AvailableAudience, @@ -380,12 +381,11 @@ export default async function getPDFData( { const { startDate, endDate } = dates; return ( - registry - .select( MODULES_ANALYTICS_4 ) - .getServiceReportURL( 'all-pages-and-screens', { - filters: { unifiedPagePathScreen: pagePath }, - dates: { startDate, endDate }, - } ) ?? '' + getAllPagesReportURL( + registry.select( MODULES_ANALYTICS_4 ), + pagePath, + { startDate, endDate } + ) ?? '' ); } diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts index e546041a488..5865daf4220 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts @@ -23,6 +23,7 @@ import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants'; import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types'; import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants'; import { Report, ReportRow } from '@/js/modules/analytics-4/datastore/types'; +import { getAllPagesReportURL } from '@/js/modules/analytics-4/utils/page-report-url'; import { getPagePaths, getPageTitleMap, @@ -97,9 +98,9 @@ function getPopularPageLinkMap( // widget renders for that user. const serviceURL = viewOnly ? null - : analytics.getServiceReportURL( 'all-pages-and-screens', { - filters: { unifiedPagePathScreen: pagePath }, - dates: { startDate, endDate }, + : getAllPagesReportURL( analytics, pagePath, { + startDate, + endDate, } ); const detailsURL = coreSite.getAdminURL( 'googlesitekit-dashboard', { diff --git a/assets/js/modules/analytics-4/utils/page-report-url.test.ts b/assets/js/modules/analytics-4/utils/page-report-url.test.ts new file mode 100644 index 00000000000..9f5e44a53e1 --- /dev/null +++ b/assets/js/modules/analytics-4/utils/page-report-url.test.ts @@ -0,0 +1,57 @@ +/** + * Analytics 4 page report URL helper tests. + * + * Site Kit by Google, Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Internal dependencies + */ +import { getAllPagesReportURL } from './page-report-url'; + +describe( 'getAllPagesReportURL', () => { + const DATES = { startDate: '2025-01-01', endDate: '2025-01-28' }; + + it( 'reads the all-pages report URL filtered to the page path', () => { + const getServiceReportURL = jest.fn( + () => 'https://analytics.example.com/report' + ); + + const url = getAllPagesReportURL( + { getServiceReportURL }, + '/pricing', + DATES + ); + + expect( url ).toBe( 'https://analytics.example.com/report' ); + expect( getServiceReportURL ).toHaveBeenCalledWith( + 'all-pages-and-screens', + { + filters: { unifiedPagePathScreen: '/pricing' }, + dates: DATES, + } + ); + } ); + + it( 'returns undefined when the property has not loaded', () => { + const url = getAllPagesReportURL( + { getServiceReportURL: () => undefined }, + '/pricing', + DATES + ); + + expect( url ).toBeUndefined(); + } ); +} ); diff --git a/assets/js/modules/analytics-4/utils/page-report-url.ts b/assets/js/modules/analytics-4/utils/page-report-url.ts new file mode 100644 index 00000000000..159ef86be99 --- /dev/null +++ b/assets/js/modules/analytics-4/utils/page-report-url.ts @@ -0,0 +1,57 @@ +/** + * Analytics 4 page report URL helper. + * + * Shared by the PDF export loaders that link a page title to its All pages and + * screens report. + * + * Site Kit by Google, Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * The Analytics 4 selectors this helper reads. + */ +interface ReportURLSelect { + /** Returns the URL of an Analytics report for a report type and arguments. */ + getServiceReportURL: ( + reportType: string, + args: { + filters: Record< string, string >; + dates: { startDate: string; endDate: string }; + } + ) => string | undefined; +} + +/** + * Builds the All pages and screens report URL a page title links to. + * + * @since n.e.x.t + * + * @param analytics The Analytics 4 store's selectors. + * @param pagePath The page path to filter the report to. + * @param dates The report's date range. + * @param dates.startDate The first day of the range (YYYY-MM-DD). + * @param dates.endDate The last day of the range (YYYY-MM-DD). + * @return The report URL, or `undefined` before the Analytics property loads. + */ +export function getAllPagesReportURL( + analytics: ReportURLSelect, + pagePath: string, + dates: { startDate: string; endDate: string } +): string | undefined { + return analytics.getServiceReportURL( 'all-pages-and-screens', { + filters: { unifiedPagePathScreen: pagePath }, + dates, + } ); +} From 06bbf72e070306bf4d96131fe097da5bf0f06044 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 13:37:41 +0300 Subject: [PATCH 10/12] Enhance JSDoc comments for clarity and consistency across files. Updated parameter types and return types in various functions to improve documentation accuracy. This change ensures better understanding of the codebase for future developers and maintains consistency in JSDoc formatting. --- .../pdf-export/PDFExportOrchestrator.test.tsx | 4 +- .../getPDFData.test.ts | 6 +-- .../getPDFData.ts | 42 ++++++++++------- .../buildPDFAudienceCard.test.ts | 4 +- .../buildPDFAudienceCard.ts | 8 ++-- .../AudienceTilesWidget/getPDFData.test.ts | 10 ++-- .../AudienceTilesWidget/getPDFData.ts | 16 +++---- .../getPDFData.test.ts | 12 ++--- .../ModulePopularPagesWidgetGA4/getPDFData.ts | 47 ++++++++++++------- .../analytics-4/utils/page-report-url.ts | 12 ++--- .../getPDFData.ts | 12 ++--- 11 files changed, 95 insertions(+), 78 deletions(-) diff --git a/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx b/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx index 4ddeb8bb260..10e61950fc8 100644 --- a/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx +++ b/assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx @@ -201,8 +201,8 @@ describe( 'PDFExportOrchestrator', () => { * @since 1.181.0 * @since n.e.x.t Added the `viewContext` parameter. * - * @param viewContext The dashboard view context to render under. - * @return The render result for the orchestrator. + * @param {string} viewContext The dashboard view context to render under. + * @return {Object} The render result for the orchestrator. */ function renderOrchestrator( viewContext: string = VIEW_CONTEXT_MAIN_DASHBOARD diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts index 7980e028698..8d151a53f39 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts @@ -130,9 +130,9 @@ function provideReports( { * * @since n.e.x.t * - * @param registry Registry that holds the Analytics property. - * @param pagePath Page path from a report row. - * @return The All pages and screens report link for the page. + * @param {Registry} registry Registry that holds the Analytics property. + * @param {string} pagePath Page path from a report row. + * @return {string} The All pages and screens report link for the page. */ function getExpectedPageLink( registry: Registry, pagePath: string ): string { return registry diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts index e11213ba203..cb79bebaf3d 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts @@ -64,18 +64,24 @@ interface GetPDFDataResult { * * @since n.e.x.t * - * @param registry WordPress data registry. - * @param dates Report date range. - * @param pagePaths Page paths from the main report rows. - * @param viewOnly Whether the export runs on a view-only dashboard. - * @return Map of page path to its Analytics report link, empty for a view-only user. + * @param {Object} params Link map parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range. + * @param {string[]} params.pagePaths Page paths from the main report rows. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Object} Map of page path to its Analytics report link, empty for a view-only user. */ -function getPageLinkMap( - registry: GetPDFDataParams[ 'registry' ], - dates: GetPDFDataParams[ 'dates' ], - pagePaths: string[], - viewOnly: boolean -): Record< string, string > { +function getPageLinkMap( { + registry, + dates, + pagePaths, + viewOnly, +}: { + registry: GetPDFDataParams[ 'registry' ]; + dates: GetPDFDataParams[ 'dates' ]; + pagePaths: string[]; + viewOnly: boolean; +} ): Record< string, string > { // A view-only user sees each page title as plain text on the dashboard, so // the PDF builds no links either. if ( viewOnly ) { @@ -111,12 +117,12 @@ function getPageLinkMap( * * @since n.e.x.t * - * @param params Loader parameters. - * @param params.registry WordPress data registry. - * @param params.dates Report date range. - * @param params.signal Cancellation signal. - * @param params.viewOnly Whether the export runs on a view-only dashboard. - * @return The report rows, currency code, page titles, and per-page links. + * @param {Object} params Loader parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range. + * @param {AbortSignal} params.signal Cancellation signal. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Promise} The report rows, currency code, page titles, and per-page links. */ export default async function getPDFData( { registry, @@ -180,7 +186,7 @@ export default async function getPDFData( { const currencyCode = report?.metadata?.currencyCode ?? ''; const pagePaths = getPagePaths( report ); - const links = getPageLinkMap( registry, dates, pagePaths, viewOnly ); + const links = getPageLinkMap( { registry, dates, pagePaths, viewOnly } ); if ( pagePaths.length === 0 ) { return { data: { rows, currencyCode, titles: {}, links } }; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts index fabea401410..b8fde1f22f6 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.test.ts @@ -142,8 +142,8 @@ describe( 'buildTopContent', () => { * * @since n.e.x.t * - * @param pagePath Page path of a top content row. - * @return The link fixture for the page. + * @param {string} pagePath Page path of a top content row. + * @return {string} The link fixture for the page. */ function getContentServiceURL( pagePath: string ): string { return `https://example.com/analytics-report${ pagePath }`; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts index 059976ae693..598dd5418be 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/buildPDFAudienceCard.ts @@ -198,10 +198,10 @@ export function buildTopCities( * * @since n.e.x.t * - * @param report The top content report, or `undefined`. - * @param titlesReport The page titles report used to resolve a path to its title, or `undefined`. - * @param getContentServiceURL Maps a page path to its Analytics report link, or to an empty string when the page has no link. - * @return Up to three top content pages. + * @param {Object} report The top content report, or `undefined`. + * @param {Object} titlesReport The page titles report that resolves a path to its title, or `undefined`. + * @param {Function} getContentServiceURL Maps a page path to its Analytics report link, or to an empty string when the page has no link. + * @return {Object[]} Up to three top content pages. */ export function buildTopContent( report: Report | undefined, diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts index 9ef8507314d..7e9d417431a 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts @@ -236,11 +236,11 @@ function buildRegistry( { * * @since n.e.x.t * - * @param registry The mock registry. - * @param options Run options. - * @param options.aborted Whether the signal is aborted before the run. - * @param options.viewOnly Whether the export runs on a view-only dashboard. - * @return The loader result. + * @param {PDFRegistry} registry The mock registry. + * @param {Object} options Run options. + * @param {boolean} options.aborted Whether the signal aborts before the run. + * @param {boolean} options.viewOnly Whether the export runs on a view-only dashboard. + * @return {Promise} The loader result. */ function runPDFData( registry: PDFRegistry, diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts index c95c1f1d1c1..773df876291 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.ts @@ -236,12 +236,12 @@ async function fetchAudienceReports( * * @since n.e.x.t * - * @param params Loader parameters. - * @param params.registry WordPress data registry. - * @param params.dates Report date range, with the current day excluded. - * @param params.signal Cancellation signal. - * @param params.viewOnly Whether the export runs on a view-only dashboard. - * @return The loaded audience cards, or `{ data: null }` when the section is omitted or canceled. + * @param {Object} params Loader parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range, with the current day excluded. + * @param {AbortSignal} params.signal Cancellation signal. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Promise} The loaded audience cards, or `{ data: null }` when the loader omits the section or the user cancels the export. */ export default async function getPDFData( { registry, @@ -370,8 +370,8 @@ export default async function getPDFData( { * * @since n.e.x.t * - * @param pagePath Page path of a top content row. - * @return The page's Analytics report link, or an empty string for a view-only user. + * @param {string} pagePath Page path of a top content row. + * @return {string} The page's Analytics report link, or an empty string for a view-only user. */ function getContentServiceURL( pagePath: string ): string { if ( viewOnly ) { diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts index 4e057d36a35..f5f46514b85 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.test.ts @@ -109,9 +109,9 @@ const PROPERTY_ID = '123456789'; * * @since n.e.x.t * - * @param registry Registry that holds the Analytics property. - * @param pagePath Page path from a report row. - * @return The All pages and screens report link for the page. + * @param {Registry} registry Registry that holds the Analytics property. + * @param {string} pagePath Page path from a report row. + * @return {string} The All pages and screens report link for the page. */ function getExpectedPageLink( registry: Registry, pagePath: string ): string { return registry @@ -130,9 +130,9 @@ function getExpectedPageLink( registry: Registry, pagePath: string ): string { * * @since n.e.x.t * - * @param registry Registry that holds the reference site URL. - * @param pagePath Page path from a report row. - * @return The page's entity dashboard link. + * @param {Registry} registry Registry that holds the reference site URL. + * @param {string} pagePath Page path from a report row. + * @return {string} The page's entity dashboard link. */ function getExpectedDetailsURL( registry: Registry, pagePath: string ): string { const siteURL = registry.select( CORE_SITE ).getReferenceSiteURL(); diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts index 5865daf4220..b1f1ac824e8 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts @@ -70,18 +70,24 @@ export interface PopularPagesPDFData { * @since 1.182.0 * @since n.e.x.t Links the title to the Analytics report for an administrator and to the entity dashboard for a view-only user. * - * @param registry WordPress data registry. - * @param dates Report date range. - * @param pagePaths Page paths from the main report rows. - * @param viewOnly Whether the export runs on a view-only dashboard. - * @return Map of page path to its title link and public URL. + * @param {Object} params Link map parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range. + * @param {string[]} params.pagePaths Page paths from the main report rows. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Object} Map of page path to its title link and public URL. */ -function getPopularPageLinkMap( - registry: GetPDFDataParams[ 'registry' ], - dates: GetPDFDataParams[ 'dates' ], - pagePaths: string[], - viewOnly: boolean -): Record< string, PopularPageLinks > { +function getPopularPageLinkMap( { + registry, + dates, + pagePaths, + viewOnly, +}: { + registry: GetPDFDataParams[ 'registry' ]; + dates: GetPDFDataParams[ 'dates' ]; + pagePaths: string[]; + viewOnly: boolean; +} ): Record< string, PopularPageLinks > { const coreSite = registry.select( CORE_SITE ); const siteURL = coreSite.getReferenceSiteURL(); const analytics = registry.select( MODULES_ANALYTICS_4 ); @@ -128,12 +134,12 @@ function getPopularPageLinkMap( * @since 1.183.0 Returns null data when the report has no rows. * @since n.e.x.t Links each title to the Analytics report for an administrator and to the entity dashboard for a view-only user. * - * @param params Loader parameters. - * @param params.registry WordPress data registry. - * @param params.dates Report date range. - * @param params.signal Cancellation signal. - * @param params.viewOnly Whether the export runs on a view-only dashboard. - * @return The report rows and the page-path-to-title map. + * @param {Object} params Loader parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range. + * @param {AbortSignal} params.signal Cancellation signal. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Promise} The report rows and the page-path-to-title map. */ export default async function getPDFData( { registry, @@ -170,7 +176,12 @@ export default async function getPDFData( { } const pagePaths = getPagePaths( report ); - const links = getPopularPageLinkMap( registry, dates, pagePaths, viewOnly ); + const links = getPopularPageLinkMap( { + registry, + dates, + pagePaths, + viewOnly, + } ); if ( pagePaths.length === 0 ) { return { data: { rows, titles: {}, links } }; diff --git a/assets/js/modules/analytics-4/utils/page-report-url.ts b/assets/js/modules/analytics-4/utils/page-report-url.ts index 159ef86be99..104e07cb283 100644 --- a/assets/js/modules/analytics-4/utils/page-report-url.ts +++ b/assets/js/modules/analytics-4/utils/page-report-url.ts @@ -38,12 +38,12 @@ interface ReportURLSelect { * * @since n.e.x.t * - * @param analytics The Analytics 4 store's selectors. - * @param pagePath The page path to filter the report to. - * @param dates The report's date range. - * @param dates.startDate The first day of the range (YYYY-MM-DD). - * @param dates.endDate The last day of the range (YYYY-MM-DD). - * @return The report URL, or `undefined` before the Analytics property loads. + * @param {ReportURLSelect} analytics The Analytics 4 store's selectors. + * @param {string} pagePath The page path to filter the report to. + * @param {Object} dates The report's date range. + * @param {string} dates.startDate The first day of the range (YYYY-MM-DD). + * @param {string} dates.endDate The last day of the range (YYYY-MM-DD). + * @return {string|undefined} The report URL, or `undefined` before the Analytics property loads. */ export function getAllPagesReportURL( analytics: ReportURLSelect, diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts index adcbcb40380..36d3e9259bf 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.ts @@ -99,12 +99,12 @@ function getQueryLinkMap( * @since 1.183.0 * @since n.e.x.t Leaves out the query links on a view-only dashboard. * - * @param params Loader parameters. - * @param params.registry WordPress data registry. - * @param params.dates Report date range. - * @param params.signal Cancellation signal. - * @param params.viewOnly Whether the export runs on a view-only dashboard. - * @return The report rows and the per-query links. + * @param {Object} params Loader parameters. + * @param {Object} params.registry WordPress data registry. + * @param {Object} params.dates Report date range. + * @param {AbortSignal} params.signal Cancellation signal. + * @param {boolean} params.viewOnly Whether the export runs on a view-only dashboard. + * @return {Promise} The report rows and the per-query links. */ export default async function getPDFData( { registry, From a71e4c09b8ae8b8d918b003b04cc57d603903764 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 14:04:16 +0300 Subject: [PATCH 11/12] Refactor getPDFData parameter types for consistency. Updated the parameter types in the getPageLinkMap and getPopularPageLinkMap functions to use a more concise Pick type for better readability and maintainability. --- .../DashboardTopEarningPagesWidgetGA4/getPDFData.ts | 5 +---- .../module/ModulePopularPagesWidgetGA4/getPDFData.ts | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts index cb79bebaf3d..ce8a894ff50 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.ts @@ -76,11 +76,8 @@ function getPageLinkMap( { dates, pagePaths, viewOnly, -}: { - registry: GetPDFDataParams[ 'registry' ]; - dates: GetPDFDataParams[ 'dates' ]; +}: Pick< GetPDFDataParams, 'registry' | 'dates' | 'viewOnly' > & { pagePaths: string[]; - viewOnly: boolean; } ): Record< string, string > { // A view-only user sees each page title as plain text on the dashboard, so // the PDF builds no links either. diff --git a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts index b1f1ac824e8..eb4d39fd73f 100644 --- a/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts +++ b/assets/js/modules/analytics-4/components/module/ModulePopularPagesWidgetGA4/getPDFData.ts @@ -82,11 +82,8 @@ function getPopularPageLinkMap( { dates, pagePaths, viewOnly, -}: { - registry: GetPDFDataParams[ 'registry' ]; - dates: GetPDFDataParams[ 'dates' ]; +}: Pick< GetPDFDataParams, 'registry' | 'dates' | 'viewOnly' > & { pagePaths: string[]; - viewOnly: boolean; } ): Record< string, PopularPageLinks > { const coreSite = registry.select( CORE_SITE ); const siteURL = coreSite.getReferenceSiteURL(); From 5c5a425855326477ee26cb8121da72fcbd64732c Mon Sep 17 00:00:00 2001 From: Matthew Riley MacPherson Date: Fri, 24 Jul 2026 10:24:48 +0100 Subject: [PATCH 12/12] Apply suggestions from code review. Co-authored-by: Matthew Riley MacPherson --- .../shared-react-pdf-components/PDFLink.test.tsx | 12 ++++++------ .../shared-react-pdf-components/PDFLink.tsx | 2 +- .../getPDFData.test.ts | 2 +- .../dashboard/AudienceTilesWidget/getPDFData.test.ts | 4 ++-- .../getPDFData.test.ts | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx index 215f548fa36..7b352da0094 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.test.tsx @@ -108,9 +108,9 @@ describe( 'PDFLink', () => { expect( linkJSON ).toContain( '#6c726e' ); } ); - it( 'renders plain text in the default color when the href is empty', () => { - // An empty `href` is how a caller passes a row that has no link, so - // the text renders plain instead of as a link. + it( 'renders plain text in the default color when the href is an empty string', () => { + // An empty `href` is means a row with no link, so the text + // should render as plain text instead of as a link. const tree = renderLink( { href: '', children: 'View dashboard' } ); expect( tree.type ).toBe( 'pdf-text' ); @@ -121,8 +121,8 @@ describe( 'PDFLink', () => { } ); it( 'renders plain text in the default color when the href is undefined', () => { - // `href` is optional, so a caller can leave it out. An undefined `href` - // renders plain text, the same as an empty one. + // An undefined/missing `href` should renders plain text, + // the same as an empty string above. const tree = renderLink( { href: undefined, children: 'View dashboard', @@ -135,7 +135,7 @@ describe( 'PDFLink', () => { expect( treeJSON ).not.toContain( PDF_COLORS.CONTENT_SECONDARY ); } ); - it( 'leaves out the trailing icon when the href is empty', () => { + it( 'omits the trailing icon when the href is empty', () => { const tree = renderLink( { href: '', trailingIcon: ( diff --git a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx index 367ac78c6f2..768449e79fb 100644 --- a/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx +++ b/assets/js/components/pdf-export/shared-react-pdf-components/PDFLink.tsx @@ -43,7 +43,7 @@ const styles = createPDFStyles( { } ); export interface PDFLinkProps { - /** Link target URL. With no URL, the text renders plain, in the default text color and with no trailing icon. */ + /** Link target URL. When an empty string: plaintext is still rendered (in the default text color and with no trailing icon). */ href?: string; /** Typography type for the link text. */ type?: PDFTypographyType; diff --git a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts index 8d151a53f39..8d63e07718d 100644 --- a/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts +++ b/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4/getPDFData.test.ts @@ -203,7 +203,7 @@ describe( 'DashboardTopEarningPagesWidgetGA4 getPDFData', () => { } ); } ); - it( 'builds no page links on a view-only dashboard', async () => { + it( 'does not include page links on a view-only dashboard', async () => { provideReports(); const result = await getPDFData( { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts index 7e9d417431a..43557fedf9d 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/getPDFData.test.ts @@ -436,8 +436,8 @@ describe( 'AudienceTilesWidget getPDFData', () => { const audiences = getAudiences( await runPDFData( registry ) ); // The stub selector serializes its type, page filter, and date range - // into the link. So this equality proves the loader asks the same - // selector the dashboard tile asks, with the page path and the report + // into the link. Ensure the loader uses the same selector the + // dashboard tile uses, with the page path and the report // date range. audiences.forEach( ( audience ) => { expect( audience.topContent[ 0 ].serviceURL ).toBe( diff --git a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts index 613363529f9..ce947f48b69 100644 --- a/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts +++ b/assets/js/modules/search-console/components/dashboard/DashboardPopularKeywordsWidget/getPDFData.test.ts @@ -141,7 +141,7 @@ describe( 'DashboardPopularKeywordsWidget getPDFData', () => { } ); } ); - it( 'builds no query links on a view-only dashboard', async () => { + it( 'does not generate query links on a view-only dashboard', async () => { fetchMock.getOnce( reportEndpoint, { body: REPORT, status: 200 } ); const result = await getPDFData( {