Skip to content

Premium Analytics: add API proxy REST controller#49506

Open
kangzj wants to merge 4 commits into
trunkfrom
echo/wooa7s-1438-api-proxy-controller
Open

Premium Analytics: add API proxy REST controller#49506
kangzj wants to merge 4 commits into
trunkfrom
echo/wooa7s-1438-api-proxy-controller

Conversation

@kangzj

@kangzj kangzj commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes https://linear.app/a8c/issue/WOOA7S-1438/pr-2-migrate-api-proxy-rest-controller-to-premium-analytics

Why

The extracted analytics dashboard's data layer needs an authenticated, same-origin way to read WPCOM analytics for the connected site. This adds the server-side proxy that forwards those reads to WPCOM and briefly caches the result, so the dashboard can render real data without each request leaving the WordPress origin or re-authenticating.

Proposed changes

  • Add Api_Proxy_Controller (src/REST/class-api-proxy-controller.php) — a WP_REST_Controller that registers GET /jetpack-premium-analytics/v1/proxy/(?P<endpoint>.*) on rest_api_init.
  • Forward the captured endpoint plus its query params to /sites/<blog_id>/analytics/<endpoint> via Jetpack\Connection\Client::wpcom_json_api_request_as_blog(), using the connected site's blog ID.
  • Cache successful (200) responses in a 5-minute transient keyed on the request signature (endpoint + sorted query params), prefixed with the package slug. Cache hits are served without contacting WPCOM. Pagination headers (x-wp-total, x-wp-totalpages) are forwarded back.
  • Gate the route on manage_options; return a 403 no_connection error when Jetpack isn't connected and 500 api_error on a transport/upstream failure.
  • Wire Api_Proxy_Controller::register() into Analytics::init(); add the automattic/jetpack-connection runtime dependency and the automattic/jetpack-test-environment dev dependency.

This migrates WooCommerce Analytics' ApiProxy, stripping the WC-specific surface: WC_REST_ControllerWP_REST_Controller, view_woocommerce_reportsmanage_options, and the LoggerTrait / Utilities / RegistrableInterface dependencies (the controller registers its own route). Per the issue, caching is implemented as a server-side transient (the source only set Cache-Control headers).

API changes

Adds GET /jetpack-premium-analytics/v1/proxy/<endpoint> (admin-only), which proxies to the WPCOM analytics API for the connected blog and caches successful responses for 5 minutes.

Request
GET /wp-json/jetpack-premium-analytics/v1/proxy/reports/totals?period=week
X-WP-Nonce: <nonce for a manage_options user>
Verification output (live, Jetpack-connected dev site — blog 254382938)
## 1. Route registered
   /jetpack-premium-analytics/v1/proxy/(?P<endpoint>.*)

## 2. Cache HIT via rest_do_request (no WPCOM call)
   status: 200
   x-wp-total: 42
   body: {"orders":42,"revenue":1234.56}

## 3. Cache MISS on connected site -> forwarded to WPCOM
   blog id: 254382938 (connected: yes)
   proxied status from WPCOM: 404
   proxied body from WPCOM: {"code":"rest_no_route","message":"No route was found matching the URL and request method.","data":{"status":404}}
   # 404 is WPCOM's own response for this test blog (no analytics data); the proxy
   # faithfully forwards the upstream status + body. The connection check passed.

## 4. Permission gate (unauthenticated HTTP)
   HTTP 401 (manage_options required)

Note on jp phan: Phan could not run locally — the host only has PHP 8.5.3, which Phan 5.5.2's bundled var_representation_polyfill fatals on while parsing the WordPress stubs (a case X; parse error). This affects every phan run in the repo on this host, not this change; php -l is clean on all three files and types/APIs were verified against the Connection package. CI runs phan on a pinned PHP and is the authoritative gate.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No. This relays existing analytics reads from the connected site to WPCOM over the existing Jetpack blog token; it introduces no new tracking or data collection. The 5-minute transient stores only the proxied response payload.

Testing instructions

Acceptance criteria from the issue:

  • jp build packages/premium-analytics succeeds
  • jp test php packages/premium-analytics passes (new test included — 8 tests, 14 assertions)
  • jp phan packages/premium-analytics passes — deferred to CI (see note above; not runnable on host PHP 8.5)
  • jp changelog add packages/premium-analytics entry included
  • PR is <500 LoC of production code, excluding tests (~293 LoC controller)
  • Proxy route responds with a cached payload on a Jetpack-connected dev site

To verify locally:

  • Activate the premium-analytics plugin on a Jetpack-connected site (requires Gutenberg active).
  • Confirm the route is registered: wp eval 'do_action("rest_api_init"); print_r(array_filter(array_keys(rest_get_server()->get_routes()), fn($r)=>str_contains($r,"premium-analytics/v1/proxy")));'
  • As a non-admin / unauthenticated, GET /wp-json/jetpack-premium-analytics/v1/proxy/reports/totals401/403.
  • As an admin on a connected site, the request is forwarded to /sites/<blog_id>/analytics/reports/totals and the upstream status + body are returned.
  • Seed a transient for a request signature, repeat the request, and confirm the cached payload is returned without an outbound WPCOM call.

@kangzj

This comment has been minimized.

@claude

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

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:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

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 github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jun 10, 2026
@kangzj kangzj self-assigned this Jun 10, 2026
@kangzj

This comment has been minimized.

@claude

This comment has been minimized.

@kangzj

This comment has been minimized.

@claude

This comment has been minimized.

kangzj added 2 commits June 10, 2026 16:34
Migrate WooCommerce Analytics' ApiProxy into the premium-analytics
package as Api_Proxy_Controller. It forwards authenticated dashboard
requests to the connected site's WPCOM analytics endpoint via
Jetpack\Connection\Client, caches successful responses in a 5-minute
transient keyed on the request signature, and returns them.

Strips the WC-specific surface from the source: WC_REST_Controller
parent becomes WP_REST_Controller, the view_woocommerce_reports
permission becomes manage_options, and the LoggerTrait/Utilities/
RegistrableInterface dependencies are dropped (the controller registers
its own route on rest_api_init from Analytics::init()).
- Declare the endpoint route arg with a sanitize_callback.
- Return 502 (and skip caching) when a 200 response body isn't decodable JSON.
- Strip the _locale routing param alongside rest_route from forwarded params.
- Cast the site ID to int when building the WPCOM path.
- Cover the new json-decode-failure and routing-param-stripping paths in tests.
@kangzj kangzj force-pushed the echo/wooa7s-1438-api-proxy-controller branch from 657b0be to be78b1c Compare June 10, 2026 07:36
@jp-launch-control

jp-launch-control Bot commented Jun 10, 2026

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/packages/premium-analytics/src/class-analytics.php 0/28 (0.00%) 0.00% 1 ❤️‍🩹

1 file is newly checked for coverage.

File Coverage
projects/packages/premium-analytics/src/REST/class-api-proxy-controller.php 52/112 (46.43%) ❤️‍🩹

Full summary · PHP report

kangzj added 2 commits June 10, 2026 16:48
…ate plugin lock

- Suppress PhanPluginMixedKeyNoKey on the register_rest_route args (the
  mixed key/no-key shape is required by the WP API; phan #4852).
- Suppress PhanUndeclaredMethod on the Closure::call() rebinding in the
  cache-key test helper.
- Refresh projects/plugins/premium-analytics/composer.lock for the new
  jetpack-connection dependency.
@kangzj

kangzj commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Review-cycle summary — bd2aeee8ae8284

5 rounds; CI green (39 required checks pass); claude[bot] reviewed and LGTM'd; 1 conflict rebase against trunk; 1 flaky infra job reran clean.

What changed during the cycle

Commits added:

  • bd2aeee — Premium Analytics: add API proxy REST controller
  • be78b1c — Address review: sanitize endpoint arg, guard non-JSON 200, strip _locale
  • 07b5e13 — Fix CI: phan suppressions for register_rest_route + test closure, update plugin lock
  • 8ae8284 — Add changelog entry for plugins/premium-analytics dependency update

Diff summary: 7 files changed, 520 insertions(+), 1 deletion(-) (518 lines excluding lockfiles).

Review items addressed:

Source Item Resolution
claude[bot] Endpoint param lacked sanitize_callback Declared endpoint route arg with sanitize_text_field (be78b1c)
claude[bot] Silent json_decode failure on a 200 body Return 502 api_error + skip caching when a 200 body isn't decodable (be78b1c)
claude[bot] _locale query param leaked to WPCOM Stripped alongside rest_route (be78b1c)
claude[bot] Empty site-ID edge case Cast blog ID to int, path uses %d (be78b1c)
claude[bot] Multisite cache key / CACHE_TTL constant Explained, no change — transients are per-site-scoped; *_IN_SECONDS in class consts is an established phan-safe pattern
CI (phan) PhanPluginMixedKeyNoKey on register_rest_route Suppressed with documented reason (inherent to the WP API; phan #4852) (07b5e13)
CI (phan) PhanUndeclaredMethod on test Closure::call() Suppressed inline (07b5e13)
CI (lock files) Plugin lock stale after new jetpack-connection dep Refreshed plugins/premium-analytics/composer.lock + changelog (07b5e13, 8ae8284)

Rebase: PR 1 (Sync_Status_Tracker) landed on trunk and touched composer.json + class-analytics.php; rebased keeping both sides (both require entries, both use imports, both init calls).

Flaky CI: PHP tests: PHP 8.4 WP latest failed once on a Docker-registry timeout pulling mariadb:12.0 (container init, tests never ran); reran and passed. All other matrix cells were green throughout.

Unaddressed (flagged for owner): None.

CI: all required checks passing.

@kangzj kangzj added [Status] Needs Team Review Obsolete. Use Needs Review instead. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant