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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* Internal dependencies
*/
import {
GetPDFDataParams,
PDFDataLoaderParams,
PDFReportDates,
} from '@/js/googlesitekit/widgets/types';

Expand Down Expand Up @@ -71,15 +71,15 @@ interface FetchReportResult {
export default function createKeyMetricTileDataLoader< TData >(
buildReports: (
dates: PDFReportDates,
registry: GetPDFDataParams[ 'registry' ]
registry: PDFDataLoaderParams[ 'registry' ]
) => TileReportRequest[] | Promise< TileReportRequest[] >,
extract: ( reports: unknown[] ) => TData | null
): ( params: GetPDFDataParams ) => Promise< TData | null > {
): ( params: PDFDataLoaderParams ) => Promise< TData | null > {
return async function getTileData( {
registry,
dates,
signal,
}: GetPDFDataParams ): Promise< TData | null > {
}: PDFDataLoaderParams ): Promise< TData | null > {
if ( signal.aborted ) {
return null;
}
Expand Down
23 changes: 21 additions & 2 deletions assets/js/components/KeyMetrics/getPDFData.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,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,
Expand Down Expand Up @@ -160,6 +167,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( result.data?.tiles.map( ( tile ) => tile.slug ) ).toEqual( [
Expand All @@ -186,6 +194,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( result.data?.tiles ).toEqual( [
Expand Down Expand Up @@ -217,6 +226,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( result.data?.tiles ).toEqual( [
Expand Down Expand Up @@ -250,6 +260,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( result ).toEqual( { data: null } );
Expand All @@ -274,6 +285,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} )
).rejects.toThrow( 'All Key Metrics PDF tiles failed to load.' );
} );
Expand Down Expand Up @@ -309,7 +321,9 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
// AdSense is not shared, so its tile is neither loaded nor exported.
// Only a view-only dashboard populates `viewableModules`.
viewOnly: true,
// AdSense isn't shared, so its tile is neither loaded nor exported.
viewableModules: [ 'analytics-4' ],
} );

Expand All @@ -329,6 +343,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( result ).toEqual( { data: null } );
Expand All @@ -347,6 +362,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: controller.signal,
viewOnly: false,
} );

expect( result ).toEqual( { data: null } );
Expand Down Expand Up @@ -377,6 +393,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: controller.signal,
viewOnly: false,
} );

expect( result ).toEqual( { data: null } );
Expand All @@ -401,6 +418,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: controller.signal,
viewOnly: false,
} );

expect( result ).toEqual( { data: null } );
Expand All @@ -427,6 +445,7 @@ describe( 'Key Metrics getPDFData', () => {
registry,
dates: DATES,
signal: new AbortController().signal,
viewOnly: false,
} );

expect( preload ).toHaveBeenCalledTimes( 1 );
Expand Down
7 changes: 5 additions & 2 deletions assets/js/components/KeyMetrics/getPDFData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import { ComponentType } from 'react';
import { CORE_USER } from '@/js/googlesitekit/datastore/user/constants';
import { CORE_WIDGETS } from '@/js/googlesitekit/widgets/datastore/constants';
import { AREA_MAIN_DASHBOARD_KEY_METRICS_PRIMARY } from '@/js/googlesitekit/widgets/default-areas';
import { GetPDFDataParams } from '@/js/googlesitekit/widgets/types';
import {
GetPDFDataParams,
PDFDataLoaderParams,
} from '@/js/googlesitekit/widgets/types';
import { KEY_METRICS_PDF_TILES } from './key-metrics-pdf-tiles';
import { KEY_METRICS_WIDGETS } from './key-metrics-widgets';

Expand All @@ -43,7 +46,7 @@ interface KeyMetricPDFTileConfig {
} >;
};
/** Resolves the tile's data, or `null` when the report has no data. */
getTileData: ( params: GetPDFDataParams ) => Promise< unknown >;
getTileData: ( params: PDFDataLoaderParams ) => Promise< unknown >;
}

const keyMetricsWidgets = KEY_METRICS_WIDGETS as Record<
Expand Down
55 changes: 50 additions & 5 deletions assets/js/components/pdf-export/PDFExportOrchestrator.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -46,6 +49,7 @@ import {
createTestRegistry,
provideModules,
provideSiteInfo,
provideUserCapabilities,
provideUserInfo,
render,
waitFor,
Expand Down Expand Up @@ -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 {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
) {
return render( <PDFExportOrchestrator onComplete={ () => {} } />, {
registry,
viewContext: VIEW_CONTEXT_MAIN_DASHBOARD,
viewContext,
} );
}

Expand Down Expand Up @@ -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' ) )
Expand Down
1 change: 1 addition & 0 deletions assets/js/components/pdf-export/PDFExportOrchestrator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ const PDFExportOrchestrator: FC< PDFExportOrchestratorProps > = ( {
registry,
dates,
signal,
viewOnly,
// The same module visibility the area discovery used, so a
// loader that composes several modules' tiles (Key Metrics) can
// keep only the tiles the user can view, as the dashboard does.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,24 +201,14 @@ const PDFHeader: FC< PDFHeaderProps > = ( {
</PDFTypography>
</View>
<View style={ styles.siteBlock }>
{ dashboardURL ? (
<PDFLink
href={ dashboardURL }
style={ {
color: PDF_COLORS.SURFACES_ON_SURFACE_VARIANT,
} }
>
{ getSiteHost( siteURL ) }
</PDFLink>
) : (
<PDFTypography
style={ {
color: PDF_COLORS.SURFACES_ON_SURFACE_VARIANT,
} }
>
{ getSiteHost( siteURL ) }
</PDFTypography>
) }
<PDFLink
href={ dashboardURL }
style={ {
color: PDF_COLORS.SURFACES_ON_SURFACE_VARIANT,
} }
>
{ getSiteHost( siteURL ) }
</PDFLink>
</View>
</View>
<View style={ styles.chipRow }>
Expand Down
Comment thread
zutigrm marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,45 @@ 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( '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: '',
trailingIcon: (
<Svg>
<Path d="M1 2 3 4" />
</Svg>
),
children: 'View dashboard',
} );

expect( JSON.stringify( tree ) ).not.toContain( 'M1 2 3 4' );
} );
} );
Original file line number Diff line number Diff line change
@@ -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
*
Expand Down Expand Up @@ -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;
Expand All @@ -62,6 +63,14 @@ const PDFLink: FC< PDFLinkProps > = ( {
style,
children,
} ) => {
if ( ! href ) {
Comment thread
zutigrm marked this conversation as resolved.
return (
<PDFTypography type={ type } size={ size } style={ style }>
{ children }
</PDFTypography>
);
}

Comment thread
zutigrm marked this conversation as resolved.
const linkColor: Style = { color: PDF_COLORS.CONTENT_SECONDARY };

return (
Expand Down
Loading
Loading