Repository navigation
Premium Analytics: add a per-section all-time and yearly date selection - #50930
Conversation
louwie17
added
the
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?
|
|
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 4 files.
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
|
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for the thoughtful work here! @louwie17
One integration issue seems important to surface before this ships. I tested selecting 2025 on Insights: the header, selected pill, and URL all correctly say 2025, but only Posting activity widget responds to that range.
Screen.Recording.2026-08-03.at.11.54.38.AM.mov
I think this creates a misleading section-level contract. The page says “Insights · 2025”, while most widgets still show all-time or current-year data.
Raising this for awareness, I think we should decide before shipping whether:
- the section-level year must filter every widget;
- widgets that intentionally remain all-time need explicit labels or grouping; or
- the global year control should wait until the remaining widget endpoints support it.
Could we capture this in a follow-up issue and clarify the intended widget-level behavior in this PR? I also left a few inline comments.
| * when they lose focus, so only the active section's header is ever rendered | ||
| * and one set of controls is enough. | ||
| */ | ||
| const dateControls = |
There was a problem hiding this comment.
I checked the comparison behavior locally. The comparison params survive and are re-derived for Insights, but today’s Insights widgets only request the primary range and render no deltas, so there is no visible impact.
But just wanted to say, this looks like URL hygiene for now, with a future risk if an Insights widget starts consuming comparison implicitly.
There was a problem hiding this comment.
Hmm yeah, at the moment it doesn't quite feel like insights would support comparison, but would be a good thing to be aware off for the time being.
Lets do this as a follow up, given this would also impact the other tabs potentially.
| ): DateFilterSurface { | ||
| // Null until the sections resolve. Reconciling before then would judge the | ||
| // preset against the wrong surface and undo a `?section=` deep link's preset. | ||
| const surface = section ? resolveDateFilterSurface( section.date_filter ) : null; |
There was a problem hiding this comment.
minor: Would it make sense if we compare date_filter directly with DATE_FILTER_YEAR and drop the resolver?
year is the only opt-in surface, so missing or unknown values already fall through to the range UI. This removes a helper whose unknown input conflicts with the closed TS union.
| * | ||
| * @return void | ||
| * | ||
| * @phan-suppress PhanPluginMixedKeyNoKey |
There was a problem hiding this comment.
nit: Could the Phan suppression be scoped to the reported mixed-key line with @phan-suppress-next-line?
d9af076 to
2319458
Compare
| * when they lose focus, so only the active section's header is ever rendered | ||
| * and one set of controls is enough. | ||
| */ | ||
| const dateControls = |
There was a problem hiding this comment.
Hmm yeah, at the moment it doesn't quite feel like insights would support comparison, but would be a good thing to be aware off for the time being.
Lets do this as a follow up, given this would also impact the other tabs potentially.
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for addressing feedback! @louwie17
One thing the rebase moved, worth a look before merge: the subtitle now appends vs. Previous period whenever a comparison is applied. Could we drop comparisonPresetId from the subtitle on the year surface?
Other than that, everything looks good.
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
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
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
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
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
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
1e2b1bf to
5b7dd06
Compare
github-actions
Bot
added
[Status] UI Changes
2 participants
Fixes WOOA7S-1787 (partially — the date-selection slice)
Proposed changes
date_filtersection arg toDashboard_Section—range(the default) oryear— parsed alongsidelabel/order/default_layout, exposed in the sections REST response, and documented in a new schema for that route. An unrecognized value falls back torange.analytics/insightswithdate_filter => year. Every other section is unchanged.DashboardSection.date_filter) and rendersDateYearFilterforyearsections,DateFiltersPanelfor everything else, through the section header's controls slot.SectionHeadersubtitle — the slot Premium Analytics: add a per-section header with title and date controls #50894 left with a TODO — with the resolved selection ("All time", "2024") on the year surface.This builds on #50754, which shipped the
all-time/year-<YYYY>presets and theDateYearFiltercomponent but deliberately did not mount them. This PR is that mounting, made configurable per section.Cross-surface presets
?preset=is one search param shared by every section, so a section switch can land a preset the visible control cannot represent. Rather than leave the filter looking unset, the surface the user is now looking at takes over:yearsectionall-timeall-timeoryear-2024→ arangesectionlast-30-days)range)The correction is a replace, not a push, so Back does not return to the state that triggered it and get corrected straight back out of.
?preset=year-2024still survives a reload untouched while you stay on Insights — that round-trip is what #50754 widenedPrimaryPresetIdfor, andneedsReportDateParamsSeedalready treats year-surface presets as range-based.startYearLeft unset, so
getYearSurfacePresetsfalls back toDEFAULT_YEAR_SURFACE_COUNT(6 years — the window the design shows). Nothing in this package knows how far back a site's data goes:getStoreInfo()is still a stub and there is no cheap first-post/site-created source that works on both connected and Simple sites. Rather than build new data plumbing here, this is left as a follow-up — a younger site currently offers years it has nothing to show for.Custom periods and comparison — open question
Not implemented, per the issue's "should we?" framing. Current behaviour on a
yearsection, for the team to decide on:DateYearFilteroffers only all time and single years, and acustompreset arriving from another section is replaced withall-time.DateFiltersPanel), but thecomp/compare_*params pass through and stay live. They are not stale:buildRangePatchre-derives the comparison window against each new primary range, soprevious-periodon year 2024 resolves to 2023. The effect is a coherent "2024 vs 2023" that the user cannot turn off without going back to another section. If that is not wanted, the fix is to clear comparison when moving onto the year surface — deliberately left out of this PR.REST response shape
Yes, the sections response changed: each section now carries a
date_filterkey, and the route gained aschemacallback. It is a purely additive field with arangedefault, so existing consumers keep working — but flagging it because WPCOM's public-api callsDashboard_Support_Routes::register()by name from thewpcomrepo (wp-content/rest-api-plugins/jetpack-endpoints/premium-analytics-dashboard.php), per AGENTS.md and #50266. No signature change, so no WPCOM-side update should be required; a WPCOM-side change would only be needed if something there asserts on the exact response shape.Related product discussion/links
all-timeand yearly date filter #50754 (year presets +DateYearFilter) and Premium Analytics: add a per-section header with title and date controls #50894 (per-section header)Does this pull request change what data or activity we track or use?
No.
Testing instructions
jetpack build packages/premium-analytics --depsAll time | 2026 | 2025 | 2024 | 2023 | 2022 | 2021pills instead of the date-range picker, with the selection named as a subtitle under the title. There is no comparison dropdown.?preset=year-2024&from=…&to=…. Reload: the same year stays selected.2024, switch to Traffic — the picker reads "Last 30 days" rather than a custom range. From Traffic on "Last 7 days", switch to Insights — "All time" is selected rather than every pill unpressed. In both directions, Back goes to the previous section, not into a correction loop.GET /wpcom/v2/dashboards/<name>/sectionsreturnsdate_filteron every section (yearfor insights,rangefor the rest);OPTIONSon the same route returns the new schema.