-
Notifications
You must be signed in to change notification settings - Fork 2k
STATS-60 - Fix: match the Video stats CSV download to the production export #103287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new normalizer (statsVideoPlaysCompleteStats) to process video plays data when the query includes complete stats, matching the CSV format of production.
- Introduces a new normalizer to enrich video plays data with impressions, watch time, and retention rate.
- Sets a complete_stats flag in the stats summary module to trigger the new behavior.
- Updates CSV export logic to use normalized data for video plays stats.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
client/state/stats/lists/utils.js | Adds the statsVideoPlaysCompleteStats normalizer function. |
client/my-sites/stats/summary/index.jsx | Adds moduleQuery.complete_stats to trigger the complete stats flow. |
client/my-sites/stats/stats-download-csv/index.jsx | Uses getSiteStatsNormalizedData for statsVideoPlays to fetch CSV data. |
Comments suppressed due to low confidence (1)
client/state/stats/lists/utils.js:483
- Consider renaming the 'label' property to 'title' to match the header row and ensure consistency between the object property and CSV header.
label: item.title,
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
const data = | ||
statType === 'statsVideoPlays' | ||
? getSiteStatsNormalizedData( state, siteId, statType, query ) | ||
: getSiteStatsCSVData( state, siteId, statType, query ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am adding a condition here because getSiteStatsCSVData
calles another function that only keeps three columns of data in the export. This is being used in all the other exports hence I did not refactor this to handle this data for now to avoid a larger PR
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~259 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~5 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
Part of STATS-60
Proposed Changes
statsVideoPlaysCompleteStats
to normalize the stats from video stats query wherecomplete_stats=1
is passedWhy are these changes being made?
Testing Instructions
Pre-merge Checklist