Render the PDF report's row links for an administrator alone by shervElmi · Pull Request #13130 · google/site-kit-wp

shervElmi · 2026-07-16T17:06:01Z

Summary

Addresses issue:

Render PDF links based on context (admin/view-only) #12958

Relevant technical choices

shared-react-pdf-components/PDFLink.tsx
- Renders plain text when href is empty, so a widget passes its URL straight through and never branches. The IB gave each widget that conditional, which repeated the same markup across the report.
AudienceTilesWidget/getPDFData.ts and buildPDFAudienceCard.ts
- The loader passes a link resolver, not a resolved map. Only the card builder reads the top content rows, so only it knows each row's page path.
DashboardAllTrafficWidgetGA4/getPDFData.ts, DashboardPageSpeedWidget/getPDFData.ts, and SearchFunnelWidgetGA4/getPDFData.ts
- Each declares its own params interface instead of importing the shared GetPDFDataParams, so each needed viewOnly to typecheck. A comment marks that the loader doesn't read it.
AudienceTilesWidget/PDFYourVisitorGroupsTile.tsx
- The top content rows use a bottom margin, not a vertical one, to match the design's 6px row gap.

PR Author Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 5.2 and PHP 7.4.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have added a QA Brief on the issue linked above.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

Run the code.
Ensure the acceptance criteria are satisfied.
Reassess the implementation with the IB.
Ensure no unrelated changes are included.
Ensure CI checks pass.
Check Storybook where applicable.
Ensure there is a QA Brief.
Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

Ensure the PR has the correct target branch.
Double-check that the PR is okay to be merged.
Ensure the corresponding issue has a ZenHub release assigned.
Add a changelog message to the issue.

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.

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.

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.

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.

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.

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.

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.

github-actions · 2026-07-16T17:06:20Z

🤖 This comment is automatically updated by CI workflows. Each section is managed independently.

🎭 Playwright reports for a71e4c0:

📚 Storybook for a71e4c0:

View Storybook

📦 Build files for a71e4c0:

Download development build
Download production build

zutigrm

Thanks @shervElmi Nice work.

There are just few things we should polish and fix, before it is ready.

We also need to align with AC point "The links present in a generated PDF match those the same user sees on their dashboard." which current implementation misses in one case for view only users:

You can see above, view only users can click top content pages, and see them in entity dashboard, but in PDF they can't which is not matching the dashboard behaviour hence mismatch with the pointed AC. This is the PDF:

There is also a conflict with develop we should resolve

zutigrm · 2026-07-20T12:06:43Z

+	 * @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 and return should include {type}, you can check this for example:

site-kit-wp/assets/js/analytics-advanced-tracking.ts

Lines 26 to 34 in 87b004e

/**

* Sends a tracking event to Analytics via gtag.

*

* @since 1.18.0

*

* @param {string} action Event action / event name.

* @param {Record< string, unknown >|null} metadata Additional event metadata to send, or `null`.

* @return {void}

*/

Thanks @zutigrm! I don't think we add parameter types to JSDoc in TypeScript files.

CC @eugene-manuilov @tofumatt

We still include them in TypeScript files as well, as stated in our doc:

site-kit-wp/docs/context/js/jsdoc.md

Line 124 in 7c18370

In TypeScript files, define an `interface` (or `type`) for the shape instead of a `\@typedef`, and annotate the function with it — see `docs/context/js/component-conventions.md`. The JSDoc block stays (description, `\@since`, `\@param`, `\@return`), and `\@param`/`\@return` still carry their `{type}` annotations as in JS — only `\@typedef`/`\@property` are dropped in favour of the TypeScript type.

The JSDoc block stays (description, \@since, \@param, \@return), and \@param/\@return still carry their {type} annotations as in JS — only \@typedef/\@property are dropped in favour of the TypeScript type.

For clarification: the TS Types should be the “Source of truth” and the ones we’ll use going forward.

Eventually we’ll remove this ESLint rule from TypeScript files, but I just haven’t got around to it yet. 😅

For now: yes, we tend to "redundantly" document them twice, but treat the TS types as the main place for documentation, etc. 🙂

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.

zutigrm

Thanks @shervElmi , looks good now. Just one more merge conflict to be resolved, and left you one reply

zutigrm · 2026-07-22T07:04:45Z

+	 * @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.


We still include them in TypeScript files as well, as stated in our doc:

site-kit-wp/docs/context/js/jsdoc.md

Line 124 in 7c18370

In TypeScript files, define an `interface` (or `type`) for the shape instead of a `\@typedef`, and annotate the function with it — see `docs/context/js/component-conventions.md`. The JSDoc block stays (description, `\@since`, `\@param`, `\@return`), and `\@param`/`\@return` still carry their `{type}` annotations as in JS — only `\@typedef`/`\@property` are dropped in favour of the TypeScript type.

The JSDoc block stays (description, \@since, \@param, \@return), and \@param/\@return still carry their {type} annotations as in JS — only \@typedef/\@property are dropped in favour of the TypeScript type.

…pdf-view-only-links

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.

Updated the parameter types in the getPageLinkMap and getPopularPageLinkMap functions to use a more concise Pick type for better readability and maintainability.

zutigrm

LGTM

shervElmi added 7 commits July 16, 2026 20:00

Render a PDF link as plain text when it has no URL.

1e800a9

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.

Link top earning page titles only for an administrator.

b9abd0a

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.

shervElmi marked this pull request as ready for review July 16, 2026 17:21

zutigrm requested changes Jul 20, 2026

View reviewed changes

shervElmi added 3 commits July 21, 2026 13:23

Merge branch 'develop' into enhancement/12958-pdf-view-only-links

ad4467f

Adress CR feedback.

4727bf1

zutigrm reviewed Jul 22, 2026

View reviewed changes

shervElmi added 3 commits July 22, 2026 11:01

Merge remote-tracking branch 'origin/develop' into enhancement/12958-…

5bca045

…pdf-view-only-links

Refactor getPDFData parameter types for consistency.

a71e4c0

Updated the parameter types in the getPageLinkMap and getPopularPageLinkMap functions to use a more concise Pick type for better readability and maintainability.

zutigrm approved these changes Jul 22, 2026

View reviewed changes

	/**
	* Sends a tracking event to Analytics via gtag.
	*
	* @since 1.18.0
	*
	* @param {string} action Event action / event name.
	* @param {Record< string, unknown >\|null} metadata Additional event metadata to send, or `null`.
	* @return {void}
	*/

Uh oh!

Conversation

shervElmi commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Relevant technical choices

PR Author Checklist

Code Reviewer Checklist

Merge Reviewer Checklist

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zutigrm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zutigrm Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

shervElmi Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zutigrm Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tofumatt Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zutigrm left a comment

Choose a reason for hiding this comment

Uh oh!

zutigrm Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zutigrm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shervElmi commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

zutigrm left a comment •

edited

Loading

shervElmi Jul 21, 2026 •

edited

Loading

zutigrm Jul 22, 2026 •

edited

Loading

zutigrm Jul 22, 2026 •

edited

Loading