Repository navigation
Premium Analytics: support view_stats-level dashboard access - #50889
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! |
Code Coverage SummaryCoverage changed in 6 files. Only the first 5 are listed here.
1 file is newly checked for coverage.
|
chihsuan
added
[Status] Needs Review
kangzj
left a comment
kangzj
left a comment
There was a problem hiding this comment.
Nice, thorough capability layer — well-tested (393 passing PHP tests, dedicated Capabilities_Test.php pinning the store-report helper to the proxy's own check). One low-severity inline gap worth a look, but not a blocker.
| 'permission_callback' => static function () { | ||
| return current_user_can( 'manage_options' ); | ||
| }, | ||
| 'permission_callback' => array( Capabilities::class, 'current_user_can_view_analytics' ), |
There was a problem hiding this comment.
This route's permission check was widened from manage_options to Capabilities::current_user_can_view_analytics() (manage_options OR view_stats), but the handler it guards (get_dashboard_default_layout_for()) doesn't re-check the store-report capability when the requested dashboard resolves to woocommerce/store.
A view_stats-only reader — exactly the population this PR admits to the dashboard but explicitly not to store data (see test_omits_woocommerce_dashboard_section_from_a_view_stats_reader) — can call GET /wpcom/v2/dashboards/traffic/default-layout?name=woocommerce/store. WP's get_param() resolves query-string values ahead of the URL route capture (the same shadowing risk Api_Proxy_Controller::validate_data_endpoint()'s ?endpoint= comment already defends against), so $request['name'] resolves to 'woocommerce/store' regardless of the URL path, and the Store tab's default widget layout (jpa/store-performance, jpa/orders-over-time, etc.) gets seeded with no current_user_can_view_store_reports() check.
Impact is limited — only default widget-type identifiers leak (strings already public in the plugin source/JS bundles), not real store data, since the proxy call for actual data stays correctly gated on view_woocommerce_reports. But it's inconsistent with the stated design goal in current_user_can_view_store_reports()'s own docblock ("surfaces backed by the prefix are hidden from readers who fail it"), and no test currently covers this route with a $name-shadowed request.
Suggest gating on Capabilities::current_user_can_view_store_reports() when the resolved section is woocommerce/store, mirroring what is_woocommerce_dashboard_section_available_to_current_user() and the /sections/{section}/default-layout route already do via Dashboard_Section::is_available().
| * | ||
| * @since $$next-version$$ | ||
| */ | ||
| class Capabilities { |
There was a problem hiding this comment.
Love it how we are centralizing the capabilities check 👍
|
Forms E2E test is failing but it doesn't seem related:
|
…ithout store access
github-actions
Bot
added
[Status] UI Changes
The Store section is now gated on the current user's capabilities (#50889), so the availability filter alone no longer surfaces it in the built-in sections test. Drop the removed `containerElement` prop from `DateFiltersPanel`, and take trunk's `getSectionSubtitle` for the header subtitle instead of the year-only label this branch added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2
The Store section is now gated on the current user's capabilities (#50889), so the availability filter alone no longer surfaces it in the built-in sections test. Drop the removed `containerElement` prop from `DateFiltersPanel`, and take trunk's `getSectionSubtitle` for the header subtitle instead of the year-only label this branch added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2
…on (#50930) * Premium Analytics: add a per-section all-time and yearly date selection Make the date-filter surface a per-section configuration and turn it on for Insights, which reports on whole history rather than a rolling window. Sections gain a `date_filter` arg (`range`, the default, or `year`), exposed in the sections REST response and its new schema. The dashboard renders `DateYearFilter` for `year` sections and keeps `DateFiltersPanel` everywhere else, and reconciles the shared `?preset=` param with whichever surface the active section shows so a section switch never leaves the control unable to represent the selection. Builds on #50754, which shipped the year presets and the filter component without mounting them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 * Premium Analytics: make the frontend section date_filter optional WPCOM's public-api registers the sections route from its own checkout, so a Simple site can be served a payload built before the field existed. A required field modelled that as impossible. `resolveDateFilterSurface` already treats a missing value as the date-range surface, so nothing changes at runtime. Drops the fixture lines the required field had forced into two files that have nothing to do with date filters, and covers the now-reachable case: a section served without the field resolves to the range surface. The PHP side is unchanged — `Dashboard_Section::$date_filter` already defaults to `range` and the schema already declares that default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 * Premium Analytics: keep the section date filter working on current trunk The Store section is now gated on the current user's capabilities (#50889), so the availability filter alone no longer surfaces it in the built-in sections test. Drop the removed `containerElement` prop from `DateFiltersPanel`, and take trunk's `getSectionSubtitle` for the header subtitle instead of the year-only label this branch added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 * Premium Analytics: apply the date selection review feedback Drop `resolveDateFilterSurface` and compare `date_filter` with the year constant directly: `year` is the only opt-in surface, so a missing or unknown value already falls through to the range UI, and the helper's `unknown` parameter fought the closed union. Its coverage moves to the hook, where the fallback is a behaviour rather than a fixture shape. Scope the Phan suppression to the array entry it is reported on instead of the whole function. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 * Premium Analytics: explain both Store section gates in the test The filter stands in for WooCommerce being active and the admin user satisfies the capability check; the section needs both, so neither line is redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 * Premium Analytics: drop the comparison from the year surface subtitle The year surface has no comparison control, so the header should not announce "vs. Previous period" for a comparison the reader cannot switch off from there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAX3YkQxDLLXhKXjft6qU2 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Fixes WOOA7S-1779
Proposed changes
Premium Analytics replaces the Stats UI, but its menu and supporting routes were limited to administrators. That prevented users granted access through Allow Jetpack Stats to be viewed by from opening the replacement dashboard.
This PR aligns dashboard and store-report access with the existing Jetpack Stats and WooCommerce permissions:
manage_optionsorview_statsmanage_optionsorview_woocommerce_reportsstore,orders,coupons,bookings, andvisitorswidgetsmanage_optionsoractivate_wordads)A dedicated
jetpack_view_analyticsmeta capability represents the dashboard rule because WordPress menu registration accepts one capability andview_statsis not registered in every supported environment.This also brings store reports in line with WooCommerce Analytics (WOOA7S-551).
Follow-ups and known gaps:
view_stats, whose data the proxy then denies. Tracked in WOOA7S-1727.?page=jetpack-premium-analyticsentry point not gated WOOA7S-1821view_statsmay reach the syncing screen while its status endpoint remains administrator-only.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Use a site with Premium Analytics available and WooCommerce active, and create two test users: one Editor and one Shop manager.
Stats access lives in Jetpack → Settings → Traffic → Jetpack Stats (expand the card). It renders two role checkbox groups — Count logged in page views from and Allow Jetpack Stats to be viewed by. Only the second one grants
view_stats.wp-admin/admin.php?page=jetpack-premium-analytics-wp-adminloads./jetpack-premium-analytics/v1/reports/csv-exportand produce a file.Optional, as the Stats-enabled editor:
/wpcom/v2/dashboards/jetpack-premium-analytics_dashboard/sectionsand/wpcom/v2/widget-modulesreturn 200, the latter with the store-category widgets filtered out, while/jetpack-premium-analytics/v1/proxy/v2/analytics/...returns 403.