Premium Analytics: point Jetpack's analytics links at the new dashboard - #50926
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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! Jetpack 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. |
Code Coverage SummaryCoverage changed in 10 files. Only the first 5 are listed here.
2 files are newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
18af56b to
aa9d523
Compare
chihsuan
left a comment
There was a problem hiding this comment.
Nice work @louwie17 tested well as described. 👍
One thing worth surfacing in case it hasn't come up: since #50915 the legacy Stats menu deliberately keeps rendering alongside Stats v2 while the flag is on, so the docblocks here that describe it in the present tense ("the Stats page is no longer registered") don't match what's on screen yet.
I also left a question to confirm the plan-usage widget behavior but non-blocking. Thanks!
| function PlanUsageBar( { limit, usage, daysToReset, overLimitMonths }: PlanUsageBarProps ) { | ||
| const usageValue = usage ?? 0; | ||
| const isOverLimit = usageValue >= limit; | ||
| const upgradeHref = upgradeUrl(); |
There was a problem hiding this comment.
Once a site is over its limit, this leaves it with a warning but no action it can take, so the gap may become more visible as the dashboard rolls out.
Just wanted to confirm keep the CTA and point it there when that route is available or we want to ship without an upgrade entry point?
d6f6960 to
b775ef8
Compare
Seven links across Jetpack still pointed at the legacy Stats page. Rather than spell a URL at each call site, add getAnalyticsUrl() to script-data: callers describe where they want to go and the helper resolves which analytics UI the site runs, the page slug, the site and blog identifiers, the section vocabulary, and the date encoding. Premium Analytics announces itself by publishing an `analytics` key on jetpack_admin_js_script_data. Its presence is the branch signal, so no caller has to ask Jetpack about the feature flag; when absent, the helper builds the legacy Stats URLs byte-for-byte as before. Notable details: - The dashboard keeps its whole router path in a single `p` search param, and reads from/to as offset-bearing instants in the site timezone. A bare YYYY-MM-DD would be parsed as UTC midnight and land a day early west of UTC, so the helper takes calendar days and owns the encoding, reading the zone offset at each boundary so a DST-transition day gets the right offset on each end. - At-a-Glance chart bars now open the period the bar stands for. The old link ignored the Days/Weeks/Months tab and always deep-linked one day. - getAnalyticsUrl() returns null when the user lacks the capability to open the dashboard, and every call site hides its control rather than rendering a dead link. - The plan-usage upgrade CTA is removed. It pointed at a Stats tier purchase screen that was a Calypso route inside the CDN-served Odyssey bundle, so it left with that dashboard and has no counterpart here. - My Jetpack's Stats get_purchase_url() returns null, the base-class default, which falls back to the existing /add-stats interstitial. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
…nks to Three reductions, all from review feedback: - Drop the `report` and `video` views. Nothing constructed them; they were built from the design doc rather than from a caller, along with a 13-member report id union and their tests. - Stop hand-rolling timezone math. `toFixedOffset`, `offsetAt`, `offsetToMinutes` and the two-pass DST offset read are replaced by date-fns + @date-fns/tz, the same libraries the dashboard's own `dateToISOStringWithTZ` uses. The DST-transition test still passes, so the bespoke version was only reimplementing the library. - Stop rebuilding legacy Stats URLs. Those links already worked, so each call site keeps its original expression untouched and only routes to the dashboard when it has replaced the Stats page. That also restores `force_refresh=1` on My Jetpack's card and the prop threading this had removed, so the non-dashboard path is now unchanged from trunk rather than merely equivalent to it. `hasAnalyticsDashboard()` is the new gate. It answers a different question from a null URL: false means "keep your existing link", whereas null means the dashboard is the analytics UI but this user cannot open it. Those must not collapse — `stats_admin_menu` is not registered once the dashboard replaces Stats, so there is no page to fall back to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
The helper was in js-packages/script-data, which meant adding date-fns to a previously zero-dependency package that 23 projects depend on and that is autopublished to npm. It only ever had two consumers, and one of them did not need it at all. My Jetpack drops it entirely. Its Stats card already receives the destination as the product's `manage_url`, which `Stats::get_manage_url()` resolves server-side, so the JS was rebuilding a URL PHP had already sent. The one thing the URL cannot carry is whether to append `force_refresh=1` — that is now a `premiumAnalyticsEnabled` entry in `get_my_jetpack_flags()`, alongside the existing showFullJetpackStatsCard and isStatsModuleActive flags. That leaves the Jetpack plugin as the only consumer, so the helper moves to `_inc/shared/analytics-url.ts`. Its three call sites live in different trees built by two webpack configs, and the extensions build has no bare-specifier root, so each imports it by relative path. `_inc/shared/` is new: `_inc/client`, `modules/` and `extensions/` had no shared JS between them before this. script-data goes back to exactly trunk apart from the `analytics` key's type — the shape of the global is its job, building URLs from it is not. The jest bootstrap it needed goes away with the test, which now runs under the plugin's existing client config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
…fy the helper script-data is now byte-identical to trunk. The `analytics` script-data key is declared where it is read, via module augmentation, so the package that types the global does not carry a key only the Jetpack plugin uses. Test simplification (405 -> 227 lines), mostly by table-driving repetitive cases. One test was worse than redundant: chart-bar-range's "reads the calendar day in UTC" case monkey-patched Date.prototype.getTimezoneOffset, which the implementation never calls, so it passed regardless of correctness. Removed, and timezone independence is now verified by running both date suites under UTC, Pacific/Kiritimati (+14), Pacific/Niue (-11), America/New_York and Asia/Kathmandu (+05:45). Added the DST fall-back case, so both a 23-hour and a 25-hour day are covered rather than only spring-forward. Cleanups from a review pass: - Drop the `encoded.includes( 'Invalid' )` guard. date-fns `format()` throws a RangeError for an unusable zone; it never returns an "Invalid" string, so only the catch was ever reachable. - DASHBOARD_SECTIONS was an identity map keyed by its own union. It is a list of accepted slugs, which is all it was doing. - Inline analyticsPath, whose docblock restated its signature. - Drop `export` from three types nothing imports, and the WHY repeated three times in the same file. - barLink took siteAdminUrl and siteRawUrl as parameters while already reading this.props on its first line. - format_gmt_offset used floor with a 1e-6 epsilon to guard a rounding case that cannot arise for quarter-hour offsets; round() says the same thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
Roughly half of the new files were comment. Kept the reasoning a reader cannot infer — why a bare YYYY-MM-DD will not do, why the router's `p` param is encoded twice, why hasAnalyticsDashboard() differs from a null URL — and dropped the restatements around it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
Adding date-fns and @date-fns/tz to plugins/jetpack changes peer resolution for the @wordpress/* packages it shares them with, which pnpm records as new snapshot keys. Earlier commits added only the importer entries, so the resolution section was left stale and CI's lock check failed. Regenerated with `pnpm install --resolution-only`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
Since #50915 the Stats menu deliberately keeps rendering alongside the dashboard, so the docblocks describing it as deregistered were wrong. Reworded to the present tense: the dashboard is the analytics UI, and Stats remains available for now. The behaviour is unchanged, but its reason was wrong too. Hiding a link for a user who cannot open the dashboard is right not because there is no Stats page to fall back to, but because the dashboard capability maps to `manage_options` or `view_stats` — a user who fails it cannot open the Stats page either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
Reverts the plan-usage changes: render.tsx, its test, its story, and the changelog entry that announced the removal. The link was removed on the grounds that it pointed at a Stats screen that no longer existed. It does still exist — since #50915 the Stats menu keeps rendering alongside the dashboard, and following the link on a flag-on site lands on a working Stats tier picker. Whether the CTA needs a new home is a question for when the Stats page actually goes away, not for this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB
b775ef8 to
3823ce2
Compare
Fixes WOOA7S-1780
Proposed changes
Several Jetpack surfaces still linked to
admin.php?page=stats, which Premium Analytics replaces whenjetpack_premium_analytics_enabledis on. Some of those URLs are confined to one spot, so the logic was updated inline; the rest needed the same non-trivial URL construction, so they go through one shared helper.Every surface keeps its existing Stats link untouched and only routes to the dashboard where that has replaced the Stats page — the old links already worked, so nothing rebuilds them.
Updated inline
packages/my-jetpack— Stats card (src/products/class-stats.php).get_manage_url()returns the dashboard URL;get_purchase_url()returnsnull, the base-class default, which falls the action button back to the existing#/add-statsinterstitial. The card's JS needs no URL logic at all — it already receives the destination as the product'smanage_url. ApremiumAnalyticsEnabledentry inget_my_jetpack_flags()covers the one thing the URL can't carry: whether to appendforce_refresh=1, which only the cached Stats page wants.Updated via the shared helper
All in
plugins/jetpack, eachhasAnalyticsDashboard() ? getAnalyticsUrl( … ) : <existing Stats URL>:_inc/client/at-a-glance/stats/dash-stats-bottom.jsx_inc/client/at-a-glance/stats/index.jsxmodules/subscriptions/newsletter-widget/src/helpers.tsextensions/shared/memberships/subscribers-affirmation.jsxThe helper
plugins/jetpack/_inc/shared/analytics-url.tsexports two functions:hasAnalyticsDashboard()— whether the dashboard has replaced the Stats page. Premium Analytics publishes ananalyticskey onjetpack_admin_js_script_data(page slug,can_view, site timezone) only where it is the analytics UI, so its presence is the signal and no caller has to ask Jetpack about the feature flag.getAnalyticsUrl( view )— takes a discriminated union (dashboardorpost, each with an optional section and date range) and resolves the rest: page slug, section vocabulary, date encoding. Returnsnullwhen the user can't open the dashboard, so call sites hide the control.It lives in the plugin rather than a shared package because the plugin is its only consumer. Two details are worth a reviewer's eye:
from/toasyyyy-MM-dd'T'HH:mm:ss.SSSxxxin the site timezone and parses them back as instants, so a bareYYYY-MM-DDwould be read as UTC midnight and land a day early west of UTC.date-fns+@date-fns/tzdo the conversion, so a day that crosses a DST transition gets the correct — and different — offset at each boundary.@wordpress/bootkeeps the router's whole path-and-search in a singlepparam and decodes twice on the way back. A single layer would decode the+of the UTC offset to a space and the date picker would reject the range. A test pins this.js-packages/script-datais untouched.Note on capabilities
This branch originally hid analytics links from Authors and Editors, because the dashboard required
manage_options. WOOA7S-1779 has since landed on trunk, mappingjetpack_view_analyticstomanage_options OR view_stats, andcan_viewnow reports that meta capability — so a site that granted an editorview_statskeeps that access. The earlier caveat no longer applies.Related product discussion/links
page=statsredirect stub, and revisiting the plan-usage "Upgrade now" CTA when the Stats page is actually retired — it still resolves today, so it is left as-is here. The dashboard's/reports/$reportand/video/$videoIdroutes are not modelled in the helper yet — add them when something needs to link there.Does this pull request change what data or activity we track or use?
No. The new script-data payload is a bool, two strings and one
current_user_can()check; no new tracking events, and the existing Tracks calls on these links are unchanged.Testing instructions
The flag-off path is the main regression risk, so start there.
1. Flag off (default) — nothing should change
With
jetpack_premium_analytics_enabledunset, confirm each link still points atadmin.php?page=stats:admin.php?page=jetpack#/dashboard→ first card: "View detailed stats", and click a chart bar (try each of the Days / Weeks / Months tabs).admin.php?page=my-jetpack→ Stats card: primary button, and "View detailed stats" when the plan status offers an upgrade (the click should still carry&force_refresh=1).wp-admin/index.php→ "Jetpack Newsletter" widget: the two subscriber counts and the "View subscriber stats" quick link.2. Flag on, as an admin
Every link above should now land on
admin.php?page=jetpack-premium-analytics-wp-adminwith the right route:3. Flag on, as an Editor (the row most likely to be skipped)
Switch to a user with
edit_postsbut notmanage_options:4. Flag on, tiered Stats plan
admin.php?page=jetpack-premium-analytics-wp-admin→ Traffic, near the bottom: the Plan usage meter and its "Upgrade now" link should be unchanged from trunk. Needs a site withviews_limit > 0, otherwise you get the empty state.Automated:
jetpack test php packages/premium-analytics,jetpack test php packages/my-jetpack,jetpack test js packages/my-jetpack, and for the pluginpnpm run test-gui/test-client/test-extensions.Note that
_inc/client/at-a-glance/**/test/component.jsxis dormant in CI — the guitestMatchonly picks upcomponent.js. I wired in the new range test explicitly but did not enable those two files: they fail on an unrelated@wordpress/themeESM transform error in that config, which deserves its own issue.🤖 Generated with Claude Code
https://claude.ai/code/session_01DQezwoePExAo1sc1onk7fB