Repository navigation
Premium Analytics: Wire Simple bootstrap behind gate - #50266
Conversation
kangzj
added
Enhancement
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Jetpack mu wpcom
|
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! Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. 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. |
Code Coverage SummaryCoverage changed in 8 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
|
github-actions
Bot
added
[Status] In Progress
[Plugin] mu wpcom
cbc71db to
e211109
Compare
|
Sandbox setup for Simple/public-api testing: 1. Prepare a WordPress.com Simple test site. Purchase a premium plan for the site, but do not install any plugin on it — this must exercise the WPCOM Simple path, not a Jetpack/Atomic/plugin-installed path. 2. Gate the test blog in. In the sandbox shell, allow production writes, then add the rollout sticker in allow-sandbox-production-writesadd_blog_sticker( 'jetpack-premium-analytics', null, null, <blog_id> );3. Install both sides on the sandbox. Check out the WPCOM PR branch — 228534-ghe-Automattic/wpcom — which adds the public-api loader serving the dashboard support routes (widget modules, default layout, sections): cd ~/public_html
git fetch origin add/premium-analytics-dashboard-endpoints
git checkout add/premium-analytics-dashboard-endpointsThen pull this Jetpack PR's changeset into the mu-wpcom plugin slot: bin/jetpack-downloader test jetpack-mu-wpcom-plugin jasper/wooa7s-1636-simple-premium-analytics-pocEither order works — the loader no-ops until the package files are present — but both are required before the routes appear on public-api as 4. Sandbox the hostnames. Point both Verify: open |
3204097 to
81290b0
Compare
dognose24
left a comment
dognose24
left a comment
There was a problem hiding this comment.
Read the whole change — the architecture calls all look right to me: the named boot steps make each platform's surface explicit, the environment branch lives in exactly one resolver (with the middleware-is-too-late rationale documented from sandbox evidence), Simple registering no local REST is the correct call against WPCOM centralization, and the test coverage is thorough (gate sticker/filter/default-off with the alias-mock + separate-process handling done properly; resolver paths for both platforms; the upgrades global/site-override cases; nested filter serialization). I also checked the Atomic angle: the gate registration sits inside the IS_WPCOM Simple-only block, so wpcom_has_blog_sticker() is never reachable where it doesn't exist.
Main question is deploy ordering (inline on constants.ts), plus a rollout-window note and two smaller items inline. Two summary-level nits:
StatsProxyParamswidened toRecord< string, unknown >— nested report filters are covered by tests, butunknownalso admits valuesaddQueryArgscan't serialize (Dates, functions). A recursive JSON-ish param type would keep the compile-time guard without blocking the filters use case. Non-blocking.- The composer
versionspins are well-justified in the PR body (the branchless WPCOMSH CI solve), but that knowledge currently lives only here — when someone adds a sixth path dependency the same CI failure will reappear with no breadcrumb. Worth a line in the monorepo build docs or neartools/cli/commands/build.js'soptions.versionsso the next person finds it.
| /** REST namespace that exposes the dashboard's default layout. */ | ||
| export const DASHBOARD_REST_NAMESPACE = 'jetpack/v4'; | ||
| /** REST namespace that exposes Premium Analytics dashboard endpoints. */ | ||
| export const DASHBOARD_REST_NAMESPACE = 'wpcom/v2'; |
There was a problem hiding this comment.
On Simple this namespace is served by the WPCOM-side public-api registrations, which aren't in this PR — so there's a deploy-order dependency: if this ships to a stickered site before the WPCOM endpoints exist, the dashboard boots with no widget registry. Since the gate limits the blast radius to test sites this isn't a blocker, but could you link the WPCOM-side diff and confirm (a) it deploys first, and (b) the route names are locked in sync with these? widget-modules in particular is a fairly generic name in a shared namespace — renaming after WPCOM centralization enumerates it gets much harder.
There was a problem hiding this comment.
The WPCOM-side diff is up: 228534-ghe-Automattic/wpcom — adds the wpcom/v2 widget-modules/default-layout/sections registration on public-api for Simple. It's deploy-order tolerant by design: every file and function it touches is existence-guarded (rest-namespace.php acts as the version sentinel), so it no-ops until this Jetpack package ships and can merge before or after this PR with no hard ordering requirement either way. Route names are locked to the shared DASHBOARD_REST_NAMESPACE / route-registration function names in this PR, and the WPCOM loader references those exact fully-qualified PHP function names, so a rename here would need a matching WPCOM-side update — flagging that coupling for whoever touches route names next.
| * | ||
| * @return void | ||
| */ | ||
| function register_widget_modules_rest_route() { | ||
| register_rest_route( | ||
| 'jetpack/v4', | ||
| DASHBOARD_REST_NAMESPACE, |
There was a problem hiding this comment.
The jetpack/v4 → wpcom/v2 move is a breaking rename for connected sites. Client and server ship in the same package so steady-state is consistent, but during a rollout window a cached page holding the old bundle can call jetpack/v4/widget-modules against a site whose PHP already moved (or vice versa) and 404. If that transient window matters, dual-registering the old namespace for one release is a cheap bridge; if it's acceptable, fine to say so and move on.
There was a problem hiding this comment.
Leaving this as a straight rename rather than dual-registering a bridge namespace. Client and server ship in the same package build, so within a single deploy there's no version skew between the two — the window you're describing is specifically a long-lived open tab holding an old in-memory JS bundle across a deploy. That self-heals on the tab's next full page load, since this dashboard is a fresh server-rendered admin_init interceptor per page load, not a persistent SPA that survives across deploys. Given today's user base is one test-gated Simple site plus the existing connected-site users (who get PHP+JS from the same commit), I think the risk is real but narrow enough to accept without the permanent extra surface of dual-registering jetpack/v4. Happy to add the bridge if you'd rather not rely on that argument.
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for addressing feedback @kangzj Great work!
Tested again on my simple site and everything works as expected 🚀
Note on the Calypso sidebar (out of scope here)
The menu entry only shows in wp-admin (/wp-admin/admin.php?page=jetpack-premium-analytics-wp-admin). It does not appear in the Calypso sidebar at wordpress.com/home/<site>. That makes sense given it need wp-calypso changes.
Just wanted to confirm is adding the Calypso sidebar entry something you're planning to follow up on separately, or is it intentionally out of scope for the Simple rollout for now?
The path-repository versions pins are not needed for the monorepo solve: composer resolves feature branches to dev-trunk on its own, and the locks regenerate cleanly without them. Mirror/sandbox builds should be re-verified before merge.
The test replaces the whole jetpack-script-data module, so the data layer's new isSimpleSite import resolved to undefined and the stats query rejected before rendering rows.
Report calls were always stats-proxy v2 requests; the fetchers just built the path by hand. A fetchReport() helper on the shared resolver now owns the path, so no fetcher touches transport or environment concerns, and the reports path constants disappear.
Simple's dashboard requests go to WPCOM public-api, where the support endpoints are registered separately. The rest_api_init hooks move from file scope into boot(), so the Simple bootstrap registers no local REST surface while connected Jetpack sites keep serving the routes locally.
Replace the shared boot() and its boolean flag with named private steps. init() and init_wpcom_simple() each read as an explicit list of what their platform registers, so the Simple/Jetpack difference is visible at the entry point instead of buried in flag branches.
This reverts commit 81290b04a1cfe394d2091f6d1754efe4f9dde24f.
One-line wrapper methods hide nothing; call the collaborators directly from the entry points and keep named steps only for multi-line pieces.
The composer pins revert restored a lock entry pointing at a pre-rebase blaze tree; regenerate against current trunk.
The dynamic report route still read connection/sync state directly from script data instead of the shared site-readiness helpers, so on Simple it always redirected to /connect - which then bounced back to / since /connect already treats Simple as connected. Route every guard through the same isPremiumAnalyticsSiteConnected() / isPremiumAnalyticsInitialSyncFinished() helpers the other four route guards already use.
…tests to UTC getDaysBetweenInclusive() and getPeriodsBetweenInclusive() built plain UTC-tagged Date objects but fed them to date-fns's local-calendar diff functions, which read local getters. On a negative-offset machine, a UTC midnight instant landing on a week/month/year boundary reads as the previous local day on only one side of the range, skewing bucket counts (e.g. a 4-week range computing as 5 weeks). Anchor both dates with the existing localTZDate() helper in UTC so the diff is deterministic regardless of the host timezone. Also mock a fixed site timezone in stats-queries.test.ts, matching the convention already used elsewhere in the package, so the WordAds 'yesterday' clamp (which is legitimately site-timezone-dependent) no longer inherits the test machine's real system timezone. Both fixes verified across UTC, negative, and extreme positive offsets; the jest TZ=UTC pin is no longer needed and is removed.
- Require callers to opt into a global (non-site-scoped) WPCOM request via an explicit global flag, instead of inferring it from an endpoint-name match against 'upgrades'. A future endpoint under the same prefix (e.g. upgrades/foo) would otherwise silently miss the global-request handling. - Fail loudly when a global Simple request has no site to scope to, instead of silently sending an unscoped request that could return data across every site the caller can access. - Replace StatsProxyParams' Record<string, unknown> with a recursive serializable-query type, restoring compile-time rejection of non-serializable values (functions, Dates, class instances) while still admitting nested filter arrays. - Move getNoticesPath() out of constants.ts and next to resolveStatsProxyRequest(), since it is a runtime resolver, not a static value, and belongs with the rest of the transport logic.
Leaves a breadcrumb for the next contributor who adds a path dependency and hits the same WPCOMSH-style composer solve failure on a detached-HEAD CI checkout with no COMPOSER_ROOT_VERSION override.
…d routes WPCOM's public-api loader previously required several internal PA files directly, in a specific order, and referenced the three route registrar functions by name - a future internal refactor could break that coupling silently. Dashboard_Support_Routes::register_for_wpcom() is now the one public, stable call a host needs: it owns the file list, load order, and function names internally. Analytics::init() calls the same method for connected sites, so there is one implementation of the registration, not two.
- Extract boot_shared_services() so the two init entry points show only their real platform difference (sync bootstrap, local API, dashboard support routes), instead of repeating the four identical steps both share. - Rename Dashboard_Support_Routes::register_for_wpcom() to register(): the method also serves connected Jetpack sites, so the WPCOM-specific name was misleading at its own call site. Matches this package's existing ::register() convention (Sync\Configuration, REST\Notices_Controller, REST\Api_Proxy_Controller).
Trim the boot-step and Dashboard_Support_Routes comments to one line each; move the ::register() vs ::register_for_wpcom() naming rationale out of the class docblock. Add an AGENTS.md section naming the exact method WPCOM's public-api process calls and the exact file to update on the WPCOM side if it ever changes.
b18ba00 to
af2a3f5
Compare
The rebase merged trunk's new jetpack-status dependency and updated package reference hashes into premium-analytics' composer.json, which the three downstream plugin locks hadn't picked up yet.
…n test test_register_is_safe_to_call_twice() intentionally calls register() twice in a row to verify the second call is a no-op; Phan's PhanPluginDuplicateAdjacentStatement plugin otherwise flags this as a likely copy-paste bug.
…tpack/v4 Explains the WPCOM REST centralization constraint that forced the rename and gives explicit guidance for future endpoints: anything that needs to reach both connected Jetpack sites and WPCOM Simple must register under wpcom/v2 via DASHBOARD_REST_NAMESPACE; connected-only endpoints can stay under jetpack-premium-analytics/v1.
Explains the redirect-loop bug chihsuan found on the /reports route (one route reading connection state directly while the others used the shared isPremiumAnalyticsSiteConnected() helpers) and gives future route authors a concrete check: grep routes/ for the helper and match its shape rather than re-deriving the check from script data.
Both Analytics::init() (every request on a connected site) and Dashboard_Support_Routes::register() (every public-api request across all of WPCOM, on Simple) used to eagerly parse the widget manifest, hydrate the widget type registry, and require dashboard-layout.php / dashboard-sections.php - regardless of whether the request would ever read any of it. - widget-modules.php now hydrates the registry itself, lazily, inside its own REST callback and boot-deps filter - the only two readers. Analytics::load_widget_registry() is removed; nothing else needed it. - Dashboard_Support_Routes::register() now only hooks rest_api_init; the actual file loads and route registration moved to a new boot_routes() method that runs there instead, so a request that never dispatches a REST route never pays for any of this.
github-actions
Bot
removed
the
[Status] Needs Review
Fixes https://linear.app/a8c/issue/WOOA7S-1636/wpcom-simple-wire-up-premium-analytics-behind-the-feature-gate
Why this matters
WordPress.com Simple customers can't use Premium Analytics today: the dashboard only boots through the self-hosted Jetpack plugin path, and its data layer assumes a local WPCOM proxy that doesn't exist on Simple. This PR wires the dashboard into the Simple platform behind a rollout sticker, so gated Simple sites get the same Premium Analytics experience with data served straight from public-api — no Jetpack connection or sync wait involved.
Proposed changes
Analytics::init_wpcom_simple()so WordPress.com Simple can boot Premium Analytics without registering the local proxy, notices, or sync bootstrap services used by connected Jetpack sites.jetpack-mu-wpcombehind thejetpack-premium-analyticsblog sticker, with a filter override for staged testing.wpcom/v2on connected Jetpack sites. On Simple the package registers no local REST surface at all — the dashboard support endpoints are registered separately on WPCOM public-api, so therest_api_inithooks moved from file scope into an explicitregister_dashboard_support_routes()boot step that only the Jetpack-site entry point calls.init()andinit_wpcom_simple()each compose named atomic boot steps, making the per-platform surface explicit.v1.xproxy calls become/rest/v1.x/...,v2report calls become/wpcom/v2/..., notices become/wpcom/v2/jetpack-stats-dashboard/notices, and globalupgradescalls keepglobal: trueplussite=<blog_id>.resolveStatsProxyRequest()in the data package owns the environment branch, built on the shared@automattic/jetpack-script-datahelpers. Woo report fetchers route through afetchReport()helper on the same resolver, so no fetcher builds paths or knows about environments. There is no separate apiFetch rewrite middleware — an earlier fallback middleware was removed because every Premium Analytics request already flows through the data helpers, and sandbox testing showed middleware rewriting can run after WPCOM's own path scoping.Findings / implementation notes
jetpack-mu-wpcomthrough the WPCOM-only Simple feature block. Premium Analytics follows that platform wiring instead of the self-hosted plugin path.wp.apiFetch: WPCOM admin middleware recognizes/wpcom/v2and/rest/v1.xpaths, sends them topublic-api.wordpress.com, and scopes non-globalrequests to the current blog./jetpack-premium-analytics/v1/proxy/...before the PA rewrite sees it, producing public-api URLs under the wrong namespace. The fix is to emit WPCOM paths at the source for Simple — which is also why no fallback rewrite middleware ships.v1.1requests, the client now passes paths such as/rest/v1.1/stats/location-views/countryto apiFetch. WPCOM then dispatches them to public-api as/rest/sites/<blog_id>/v1.1/stats/location-views/country.wpcom/v2routes from the site.upgradesendpoint is the exception. It is a global public-api endpoint, so the Simple path builder marks itglobal: trueand appendssite=<blog_id>instead of relying on WPCOM site scoping./wpcom/v2/jetpack-stats-dashboard/notices.admin_menuis too late becauseAnalytics::init_wpcom_simple()registers its ownadmin_menucallback. The PoC boots onplugins_loaded, then Premium Analytics can register the menu for the current request.versionspins are required. The WPCOMSH CI job buildspackages/premium-analyticswithout--for-mirrorson a branchless detached-HEAD checkout, where composer's version guesser labels path packagesdev-<sha>(no branch alias), so packagist'sautomattic/woocommerce-analyticscaret constraints cannot resolve. The pins declare the five affected packages asdev-trunk, mirroring whattools/cli/commands/build.jsinjects (options.versions) for mirror builds. Removing them was attempted and reverted after theWP Cloud Unit Testing for WPCOMSHjob failed on exactly that solve.Relevant code paths
projects/packages/jetpack-mu-wpcom/src/class-jetpack-mu-wpcom.phpwires the gated WPCOM Simple bootstrap and callsAnalytics::init_wpcom_simple()onplugins_loaded.projects/packages/premium-analytics/src/class-analytics.phpadds the dedicated Simple boot method and skips local connected-site services for Simple.projects/packages/premium-analytics/src/widget-modules.phpregisters/wpcom/v2/widget-moduleswith a Premium Analytics function callback.projects/packages/premium-analytics/src/dashboard-layout.phpregisters/wpcom/v2/dashboards/(?P<name>...)/default-layoutwith a Premium Analytics function callback.projects/packages/premium-analytics/routes/dashboard/hooks/constants.tsmirrors thewpcom/v2namespace used by the client-side data layer.projects/packages/premium-analytics/packages/data/src/api/stats-proxy-fetch.tsresolves every stats and report request through one environment-aware resolver: local proxy paths on Jetpack sites, direct/rest/v1.x//wpcom/v2public-api paths and the globalupgradeshandling on Simple.fetchReport()wraps it for the Wooanalytics/reportsnamespace, so report fetchers pass endpoint suffixes only.projects/packages/premium-analytics/packages/data/src/api/constants.tscentralizes the Simple-aware notices path viaisSimpleSite()from@automattic/jetpack-script-data.projects/packages/premium-analytics/routes/site-readiness.tscentralizes the connected/synced decision so route guards do not repeat Simple checks.projects/packages/jetpack-mu-wpcom/tests/php/features/premium-analytics/Wpcom_Simple_Premium_Analytics_Test.php,projects/packages/premium-analytics/tests/php/Analytics_Test.php,projects/packages/premium-analytics/tests/php/Widget_Modules_Test.php,projects/packages/premium-analytics/tests/php/Dashboard_Layout_Test.php,projects/packages/premium-analytics/packages/data/src/api/__tests__/stats-proxy-fetch.test.ts,projects/packages/premium-analytics/packages/data/src/queries/__tests__/stats-queries.test.ts, andprojects/packages/premium-analytics/tests/js/site-readiness.test.ts.projects/packages/jetpack-mu-wpcom/src/features/wpcom-simple-odyssey-stats/wpcom-simple-odyssey-stats.phpandprojects/packages/jetpack-mu-wpcom/src/class-jetpack-mu-wpcom.php(load_wpcom_simple_odyssey_stats()) boot Odyssey Stats on Simple from the MU package.wp-content/rest-api-plugins/centralized/centralize.phpadds/sites/{site}to registered site-specific routes and treats function callbacks as site-specific by default.wp-content/mu-plugins/rest-api.phpimplementswpcom_rest_api_v2_load_plugin()as an idempotent class instantiation helper.wp-calypso/apps/odyssey-stats/src/lib/get-api.tsswitches from Jetpack REST namespaces to public-api namespaces//sites/<siteId>paths when Odyssey is not running in a Jetpack site.wp-calypso/packages/wpcom-proxy-request/src/index.jsdefines thepublic-api.wordpress.comproxied REST origin used by Calypso packages.wp-calypso/packages/help-center/src/data/use-support-status.tsandwp-calypso/packages/help-center/src/data/use-submit-support-ticket.tsuseapiFetch( { ..., global: true } )for non-site-scoped WPCOM requests when not usingwpcom-proxy-requestdirectly.Related product discussion/links
Does this pull request change what data or activity we track or use?
No. This changes Premium Analytics bootstrap and API transport wiring for gated WordPress.com Simple sites; it does not add new tracking or change analytics event collection.
Testing instructions
Local verification already run on this branch:
composer phpunitfromprojects/packages/premium-analyticspassed: 328 tests, 905 assertions.composer phpunitfromprojects/packages/jetpack-mu-wpcompassed: 671 tests, 1949 assertions.CI=true pnpm testinprojects/packages/premium-analyticspassed: 116 suites, 824 tests.CI=true pnpm typecheckinprojects/packages/premium-analyticspassed.CI=true jp build packages/premium-analyticspassed.CI=true jp build plugins/premium-analyticspassed.CI=true jp build packages/jetpack-mu-wpcompassed.Step-by-step PoC smoke test on WordPress.com Simple:
jetpack-mu-wpcomandjetpack-premium-analyticspackage changes.jetpack-premium-analyticsblog sticker and no local override is returning true forjetpack_premium_analytics_wpcom_simple_enabled.wp-admin/for that site and confirm thePremium Analyticsmenu is absent.wp-admin/admin.php?page=jetpack-premium-analytics-wp-admindirectly and confirm the Premium Analytics app does not render.jetpack-premium-analyticsblog sticker to the same Simple test site, or temporarily force thejetpack_premium_analytics_wpcom_simple_enabledfilter to return true in the sandbox.wp-admin/and confirm thePremium Analyticsmenu appears.wp-admin/admin.php?page=jetpack-premium-analytics-wp-admin./connector/syncing; Simple should be treated as connected and synced./wpcom/v2/sites/<blog_id>/widget-modules?_envelope=1(served by the separate public-api registrations, not by routes booted on the site) and/wpcom/v2/sites/<blog_id>/dashboards/jetpack-premium-analytics_dashboard/default-layout?_envelope=1.v1.1data requests no longer use the Premium Analytics proxy namespace. For example,stats/location-views/countryshould appear on public-api as/rest/sites/<blog_id>/v1.1/stats/location-views/country?..., not/jetpack-premium-analytics/v1/sites/<blog_id>/proxy/v1.1/stats/location-views/country?....v2analytics report requests are sent through/wpcom/v2/sites/<blog_id>/analytics/reports/...on public-api, not through/jetpack-premium-analytics/v1/proxy/v2/....upgradesrequests go to the global public-api endpoint withsite=<blog_id>in the query string and are not rewritten with a/sites/<blog_id>path prefix./wpcom/v2/sites/<blog_id>/jetpack-stats-dashboard/noticesafter WPCOM scoping.jetpack-premium-analyticssticker or the temporary filter override and reloadwp-admin/; confirm thePremium Analyticsmenu disappears again.