WOOA7S-1495: add Authors widget backed by Jetpack Stats - #49570
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
github-actions
Bot
added
the
[Status] Needs Author Reply
| * TODO: Remove the default range once we have a way to pass the launched date to the widget. | ||
| */ | ||
| const getDefaultReportParams = () => ( { | ||
| from: '2000-01-01T00:00:00', |
There was a problem hiding this comment.
Temporary hardcoded long date range to view the data in the widget, until we have date range picker in the dashboard
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
909996e to
d060b2d
Compare
5bb6dd9 to
7ff7f71
Compare
7ff7f71 to
22e10a7
Compare
e1f31ef to
49f10a6
Compare
|
@claude Review the PR please |
|
Claude finished @Nikschavan's task in 3m 43s —— View job PR Review — Authors widgetReviewed the 11 files in this commit. Note: the data package ( 🐛 Correctness / i18n1. "Untracked authors" never shows the translated string — label: item.name || 'Untracked Authors',So by the time data reaches 2. Authors are merged by display label — 🔧 Type safety3. 📝 Minor / consistency4. 5. 💬 Answers to your inline questions
✅ Nice touches
Note: the widget-dashboard CSS / data-package / devtools changes from the PR description aren't in this commit, so they weren't reviewed here. |
There was a problem hiding this comment.
Pull request overview
Adds a new Authors widget to the Jetpack Premium Analytics dashboard, using the Jetpack Stats “top-authors” endpoint and a new widget/toolkit implementation to render a leaderboard with comparison support.
Changes:
- Register new
jpa/authorswidget metadata (type/module JSON) with a configurablemaxattribute and a render entry usingWidgetRoot. - Add
AuthorsWidgetto the widgets-toolkit plus abuildTopAuthorsDatahelper to transform Stats responses into leaderboard chart data. - Export the new widget/helper from the widgets-toolkit public entrypoints and add a changelog entry.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/premium-analytics/widgets/authors/widget.ts | Defines the widget type (name/title/icon) and its max attribute. |
| projects/packages/premium-analytics/widgets/authors/widget.json | Widget picker metadata (title/description/category). |
| projects/packages/premium-analytics/widgets/authors/render.tsx | Render entry wiring WidgetRoot + AuthorsWidget, including a wide default report range. |
| projects/packages/premium-analytics/widgets/authors/package.json | Declares per-widget package dependencies. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/widgets/index.ts | Exports AuthorsWidget from the widgets toolkit widget barrel. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/widgets/authors/index.ts | Authors widget module re-export. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/widgets/authors/authors-widget.tsx | Main widget component: fetches top authors and renders a leaderboard with comparison/empty/loading/error handling. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/index.ts | Exposes AuthorsWidget from the toolkit public API. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/index.ts | Exports buildTopAuthorsData helper. |
| projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/build-top-authors-data.ts | Converts Stats “top-authors” report(s) into LeaderboardChart-ready data with deltas/shares. |
| projects/packages/premium-analytics/changelog/update-pa-authors-widget | Changelog entry for introducing the Authors widget. |
4eb1e79 to
36d5586
Compare
fe70374 to
017a5ac
Compare
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Nice work on the port! @Nikschavan
A couple of things before merge:
- This PR doesn't include a Storybook story —
AGENTS.md asksevery widget to ship one. Could you add it? - The description mentions a
withDevtoolsprop and a picker-preview change, but I don't see those in the diff. Did they land separately on rebase? Mind updating the description to match?
Also left a couple of inline comments.
afac0cf to
3f8c8f0
Compare
| @@ -0,0 +1,109 @@ | |||
| /** | |||
There was a problem hiding this comment.
Maybe we could coordinate those changes into https://github.com/Automattic/jetpack/blob/trunk/projects/packages/premium-analytics/packages/data/src/processing/stats/top-authors.ts, so that there's no need for more mappings?
There was a problem hiding this comment.
I was digging into whether buildTopAuthorsData could fold into the top-authors sanitizer like you suggested, and I'm not sure it can — wanted to run my thinking by you:
useStatsReportlooks like it runs the primary and comparison as two separate queries, each sanitized on its own (use-report.ts#L64-L67), so the sanitizer only ever sees one period — which seems to mean the period-over-period shares/deltas have to be computed in the widget, where bothprimary.dataandcomparison.dataare in hand. Does that match how you're reading it?- The shaping also pulls
LeaderboardChartData/calculateDeltafromwidgets-toolkit, and sincedatais the leaf the toolkit depends on, moving it intoprocessing/statswould invert that dependency. Is there a cleaner spot for it I might be missing?
For what it's worth, the current split seems to match the recently-merged widgets — sales-by-utm-channel composes useReportOrderAttribution + buildSalesByUtmData( primary.data ) in its render, and top-posts does the same — so I leaned toward following that pattern here.
That said, I think you're right that there's real duplication: authors, top-posts, locations, and sales-by-utm all hand-roll the rows→LeaderboardChartData step. Would a shared buildLeaderboardData in widgets-toolkit make sense as a follow-up across all of them? I'd lean toward a separate PR rather than scope-creeping this one — but let me know your thoughts, happy to go a different way if you'd prefer.
4bcf910 to
e512898
Compare
- widgets-toolkit: add AuthorsWidget (consumes the useStatsTopAuthors hook from the data package) and the buildTopAuthorsData leaderboard helper - widgets/authors: register the jpa/authors dashboard widget; render.tsx fetches via the existing /jetpack-premium-analytics/v1/proxy Stats proxy (v1.1 stats/top-authors) and wraps the widget in WidgetRoot - query client provider: add withDevtools prop so per-widget providers don't each render React Query devtools - widget-dashboard: inset picker previews with padding instead of scaling so content clears the selection checkbox
- Forward `max` to the Stats top-authors query so the API limit matches the rendered leaderboard instead of relying on the endpoint default. - Dedup and align authors by stable `author_id` (falling back to the display label) so distinct authors sharing a name aren't merged. - Translate the "Untracked authors" label in a single place: the processing layer leaves an empty label and the widget supplies the translated fallback. - Drop `StatsProxyParams`' index signature from `StatsReportParams` so `reportParams` no longer needs casting; make `useStatsReport` generic so `useStatsTopAuthors` returns a typed report (removes data casts). - Declare the widget's runtime deps (widgets-toolkit, @wordpress/i18n, @wordpress/icons) in its package.json. - Forward `setError` from the render entry to WidgetRoot.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The empty-state icon was dropped earlier but the import was left behind, failing ESLint and the exclude-list check in CI.
- Declare presentation (framed) in widget.json.
- Type render props via WidgetRenderProps<AuthorsAttributes> from
@wordpress/widget-primitives and default attributes to {}.
- Add @wordpress/widget-primitives as a devDependency (type-only import,
erased at build, keeps runtime deps host-agnostic).
Relocate buildTopAuthorsData and the authors render logic out of the shared widgets-toolkit package into widgets/authors, colocating the widget's tests. Wire the widget to its own dependencies, change the max authors field to integer, and stub CSS imports in jest so widgets-toolkit CSS doesn't get parsed as JS.
Extract AuthorsLeaderboard as an exported presentational component taking pre-built rows, and rename the data-connected wrapper to AuthorsReport so Storybook can exercise loading/empty/populated states. Trust the Stats API's server-side ranking and limiting: replace summarizeAuthors' cross-interval aggregation and sorting with a flat toAuthorItems, dropping the maxEntries param. Update tests accordingly.
…rebase Trunk now exports formatLegendLabels from the widgets-toolkit barrel, so the copy this branch added is redundant; keep trunk's and remove the duplicate to avoid a TS2300 duplicate-identifier error.
Removes the hardcoded all-time default report params and the date-fns dependency, so the widget reads report params from the dashboard date range through WidgetRoot context like the other Stats widgets.
cbf6bb2 to
9be7729
Compare
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks @Nikschavan LGTM.
I left additional comments but non-blocking.
The rest is all fine as follow-ups:
- The shared
buildLeaderboardDatainwidgets-toolkityou've floated. Agree a separate PR is the right scope, not this one. - Storybook: the story only exercises the presentational
AuthorsLeaderboardwith fixtures right now. Aligning it withsearch-terms(aWidgetDashboardWithWidgetstory + atop-authorsmock wired intorouteStatsReport()) would cover the real data path — can land separately.
| return typeof author.label === 'string' && author.label | ||
| ? author.label | ||
| : __( 'Untracked authors', 'jetpack-premium-analytics' ); | ||
| } |
There was a problem hiding this comment.
Claude code found this is a dead code since sanitizeStatsTopAuthorsResponse already substitutes item.name || 'Untracked Authors':
'Untracked Authors'` is untranslated there though.
|
|
||
| type AuthorsAttributes = NonNullable< ComponentProps< typeof WidgetRoot >[ 'attributes' ] > & { | ||
| max?: string | number; | ||
| }; |
There was a problem hiding this comment.
minor: Could AuthorsAttributes live in widget.ts instead?
AGENTS.md asks for the widget's own attribute shape to be declared and exported once from widget.ts and imported here
|
Thanks for the feedback, @chihsuan! I will merge this PR as-is and address your suggestions in a follow-up to keep the scope focused and expedite the release. |
github-actions
Bot
added
[Status] UI Changes
Proposed changes
Ports the Authors widget from the Jetpack Stats module into the Premium Analytics dashboard, backed by the Jetpack Stats API. This is a self-contained widget — the Stats data layer and the widgets-toolkit primitives it builds on already live in
trunk.widgets/authors/: adds thejpa/authorsdashboard widget —render.tsx(a presentationalAuthorsLeaderboardplus a data-connectedAuthorsReport, wrapped inWidgetRoot),widget.json, andwidget.tswith a configurable integermaxattribute. It reads its report params from the dashboard date range viaWidgetRootcontext, like the other Stats widgets.useStatsTopAuthorshook andtop-authorsStats sanitizer (already intrunk). A localbuildTopAuthorsDatahelper flattens the report and shapes it into leaderboard chart data (share-of-max plus period-over-period deltas), mirroring the Top posts widget.buildTopAuthorsData.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
/jetpack/v4/stats-app/sites/{blog_id}/stats/top-authors).Maximum authorsattribute and verify the list respects the limit.