Conversation
the default-layout policy asks the widget registry once it can answer
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Premium Analytics
[Tests] Includes Tests
Docs
labels
Contributor
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Videopress 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. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. 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. |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
phan cannot see past the finally
…ity-videopress-layout-validation # Conflicts: # projects/packages/premium-analytics/docs/dashboard-widgets.md
retrofox
changed the title
a renamed type keeps the layouts saved under its old name; contract 1.1.0
a Widget_Type object as type dropped the whole sections route on PHP 8
…ity-videopress-former-names # Conflicts: # projects/packages/premium-analytics/docs/dashboard-sections.md # projects/packages/premium-analytics/src/dashboard-layout.php # projects/packages/premium-analytics/tests/php/Dashboard_Layout_Test.php
…ity-videopress-former-names
build the broken instances by hand for phan; cover the non-instance branch and the sanitizer
the video plays hook too, provisional; widget contract 1.2.0; the facade must be fully declared
type videopress/top-videos, former jpa/videopress; the Traffic seed moves too
jetpack-mu-wpcom calls the VideoPress registrants on Simple and Atomic
github-actions
Bot
added
[mu wpcom Feature] Premium Analytics
[Package] Jetpack mu wpcom
…pdate/pa-extensibility-videopress-sdk-exports
…date/pa-extensibility-videopress-top-videos
the video plays hook too, provisional; widget contract 1.2.0; the facade must be fully declared
…ss-sdk-exports' into update/pa-extensibility-videopress-top-videos # Conflicts: # projects/plugins/jetpack/composer.lock # projects/plugins/videopress/composer.lock
the video plays hook too, provisional; widget contract 1.2.0; the facade must be fully declared
if-guard the generated accessor; drop suppressions the test-only dep made unused
wp-build writes it into build/, which phan does not parse
…ity-videopress-sdk-exports
with describeError and the row limit; the video plays hook stays provisional
…date/pa-extensibility-videopress-top-videos # Conflicts: # projects/js-packages/premium-analytics-sdk/changelog/update-pa-extensibility-videopress-sdk-exports # projects/js-packages/premium-analytics-sdk/src/index.d.ts # projects/packages/premium-analytics/changelog/update-pa-extensibility-videopress-sdk-exports # projects/packages/premium-analytics/docs/dashboard-widgets.md # projects/packages/premium-analytics/packages/sdk/src/index.ts # projects/packages/premium-analytics/packages/widgets-toolkit/src/components/leaderboard/leaderboard.module.scss # projects/packages/premium-analytics/widgets/videopress/__tests__/videopress.test.tsx # projects/packages/premium-analytics/widgets/videopress/style.module.css # projects/packages/videopress/widgets/top-videos/render.tsx # projects/packages/videopress/widgets/top-videos/style.module.css
they moved to the VideoPress package with the widget
covers the rows, the status, the error copy and the footer link
the widget suite that exercised it left the package
the Leaderboard caps its rows; a consumer sizes its own request
…date/pa-extensibility-videopress-top-videos
the SDK no longer exposes the dashboard's row limit
…ity-videopress-top-videos
…ity-videopress-top-videos
drops the package's own hook; the availability docblock stops naming the widget
…ity-videopress-top-videos
the button takes the linked report by id, contract 1.3.0; the parity test drops the moved widget
the registration examples point at its live code; SDK names grouped by stability
…-extensibility-videopress
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to WOOA7S-2221
Draft umbrella. The design record for the VideoPress consumer stack; the steps land through their own PRs and this branch accumulates them. Steps 1 to 4 are merged; step 5 is in review. This PR closes without merging once step 5 lands.
Proposed changes
The Top videos widget (
jpa/videopress) leaves the Premium Analytics package forprojects/packages/videopress, the owner of the knowledge of whether VideoPress runs on a site. It follows the shape the Ads package established in #52635, with two additions the Ads move did not need.Why VideoPress needs more than Ads did
Ads had its own section, outside the customer preview. Top videos sits inside the Traffic section's default layout, inside the preview. A rename there cannot leave "Widget is no longer available" tiles in persisted layouts, and a default layout cannot seed a type a site has not registered. Steps 1 and 2 are those two guarantees.
The steps
former_names; the client and the default-layout filter map old names to new ones.Leaderboard,describeError()anduseStatsVideoPlays;WIDGET_API_VERSION1.2.0.packages/videopressregistersvideopress/top-videosand its Traffic instance; PA drops the widget.What stays in Premium Analytics
The Videos report, the
/video/$videoIdroute and the twojpa/video-detail-*widgets that route composes, plusvideopress-availability.phpfor their gate. They move when reports and detail routes become registrable, in a separate plan.Decisions
packages/videopressdoes not requirepremium-analytics(my-jetpackandconfigdepend on VideoPress); premium-analytics is arequire-devfor tests and phan, and the runtime needs no dependency because the registration action fires only when the dashboard is loaded.The SDK exposes a widget kind, not the parts one is built from. Step 3 first carried the eight pieces a leaderboard widget wires by hand; #52965 (WOOA7S-2239) turned that wiring into the
Leaderboardcomponent, and the SDK exports the component.useStatsVideoPlaysstays in the Premium Analytics data layer and the SDK re-exports it, the WordAds precedent, since the Videos report reads it too. That is stage 1. Stage 2 (WOOA7S-2234) moves the data handling to VideoPress: its own proxy to WordPress.com, its own transforms, and the widget running them on the engine the SDK exposes (WOOA7S-2230).The widget's pure tests move with it into
packages/videopress. Its render suite is rewritten against a stand-in for the SDK module, since the old one drove the widget through the dashboard's internal packages, which a consumer cannot import. The story does not move for the same reason: a harness that renders the real SDK for consumers is the question #52635 left open for the Ads package, and it moves to the SDK audit (WOOA7S-2228).Related product discussion/links
Linear: WOOA7S-2221 and its sub-issues. Previous stacks: WOOA7S-2162 (sections), WOOA7S-2183 (widget types, Ads pilot).
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Each step PR carries its own. Once step 4 is in: on a site with VideoPress, the Traffic section shows Top videos as before,
GET /wp-json/wpcom/v2/widget-moduleslistsvideopress/top-videoswith thejetpack-videopress-pkgtext domain, and a Traffic layout persisted withjpa/videopressstill renders the widget. Without VideoPress the type is absent and the Traffic default has no tile for it.