Premium Analytics: remove the Videos report performance chart - #50801
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 3 files.
|
37620ed to
d0ba0f3
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Premium Analytics Videos report to remove the performance chart and rely solely on a comparison-aware records table backed by the complete-stats summary request, while preserving video-detail linking and enabling plays/impressions deltas when comparison rows match.
Changes:
- Remove the Videos report performance chart (and its chart-specific aggregation / interval handling) and render only the records table.
- Switch the Videos report data source to a single comparison-aware complete-stats summary request; enrich comparison rows to include both
previousPlaysandpreviousImpressions. - Update the table fields to render metric deltas via
MetricWithComparison, and adjust/add Jest tests accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/premium-analytics/routes/reports/videos/page.tsx | Removes the performance chart and renders a comparison-aware records table with stable row IDs. |
| projects/packages/premium-analytics/routes/reports/videos/page.test.tsx | Updates page tests for chart removal, comparison-enabled fields, and table loading behavior during fetches. |
| projects/packages/premium-analytics/routes/reports/videos/config/use-report-records.ts | Refactors the report hook to use a single complete-stats summary request via useStatsVideoPlays. |
| projects/packages/premium-analytics/routes/reports/videos/config/use-report-records.test.ts | Reworks hook tests to validate complete-stats request params, comparison metrics, loading/fetching, and refetch/error behavior. |
| projects/packages/premium-analytics/routes/reports/videos/config/index.ts | Removes now-obsolete exports tied to the deleted chart aggregation logic. |
| projects/packages/premium-analytics/routes/reports/videos/config/fields.tsx | Adds optional comparison rendering for Plays/Impressions using MetricWithComparison. |
| projects/packages/premium-analytics/routes/reports/videos/config/fields.test.tsx | Extends field tests to cover metric deltas and comparison-disabled behavior. |
| projects/packages/premium-analytics/routes/reports/videos/config/aggregate.ts | Removes chart aggregation helpers no longer needed after chart removal. |
| projects/packages/premium-analytics/routes/reports/videos/config/aggregate.test.ts | Removes tests that only validated the deleted chart aggregation behavior. |
| projects/packages/premium-analytics/packages/data/src/queries/stats-video-plays-summary-query.ts | Adjusts the summary query param shape to match legacy request expectations (notably excluding days). |
| projects/packages/premium-analytics/packages/data/src/queries/tests/stats-queries.test.ts | Updates query-factory expectations for the new legacy-compatible summary request params. |
| projects/packages/premium-analytics/packages/data/src/processing/stats/video-plays.ts | Extends merged comparison rows to include previousImpressions alongside previousPlays. |
| projects/packages/premium-analytics/packages/data/src/processing/stats/tests/video-plays.test.ts | Updates merge tests to validate impressions comparison propagation. |
| projects/packages/premium-analytics/packages/data/src/hooks/use-stats-video-plays.ts | Updates inline documentation to reflect the dedicated legacy-compatible summary request path. |
| projects/packages/premium-analytics/changelog/remove-report-performance-chart-videos | Adds a changelog entry describing the user-facing Videos report change. |
487062e to
1dba026
Compare
chihsuan
left a comment
There was a problem hiding this comment.
Thanks, nice cleanup. Left a few notes but nothing blocking.
Pass the records straight to the table instead of emptying it while fetching. The queries carry placeholderData, so blanking the rows dropped the user's search, sorting and page position on every date or comparison change. The loading state still reflects both isLoading and isFetching.
Report only a primary-query failure as an error. useReport combines both queries, so a comparison-only failure hid the whole table. The table now shows the current-period rows without deltas. Delete the unused useStatsVideoPlaysSummary hook. The report reads the summary through useStatsVideoPlays. The query and its params type stay, because useStatsVideoPlays still calls the query.
57b6c83 to
f12c063
Compare
Proposed changes
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
fnm exec --using=24.15.0 pnpm jetpack build plugins/premium-analytics --deps.