-
Notifications
You must be signed in to change notification settings - Fork 851
VideoPress: fix video query to only return VideoPress videos #46689
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
Remove conflicting media_type parameter when querying for VideoPress videos. Using both media_type=video and mime_type=video/videopress caused WordPress REST API to return all video types instead of just VideoPress videos.
Add Data_Test to verify that Data::get_video_data() produces correct WP_Query parameters: - VideoPress queries use only video/videopress mime type (not all video types) - Local video queries use media_type=video with meta_query to exclude VideoPress videos Uses mock plugin pattern from my-jetpack package (assets/*.txt files) to make Status::is_standalone_plugin_active() return true in tests.
|
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! |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
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 fixes a bug where VideoPress video queries were incorrectly returning all video types instead of only VideoPress videos. The issue was caused by WordPress merging mime types when both media_type=video and mime_type=video/videopress parameters were used together in REST API queries.
Changes:
- Modified
Data::get_video_data()to use onlymime_type=video/videopressfor VideoPress queries, removing the redundantmedia_type=videoparameter - Updated JavaScript resolvers to remove
media_type: 'video'from VideoPress-specific queries while retaining it for local video queries - Added comprehensive PHP tests to verify correct query parameters for both VideoPress and local video queries
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
projects/packages/videopress/src/class-data.php |
Removes media_type parameter from VideoPress queries; only sets it for local video queries along with no_videopress flag |
projects/packages/videopress/src/client/state/resolvers.js |
Removes media_type: 'video' from getVideos and getUploadedVideoCount resolvers (VideoPress queries only) |
projects/packages/videopress/tests/php/Data_Test.php |
Adds unit tests verifying correct WP_Query parameters for VideoPress and local video queries |
projects/packages/videopress/tests/php/assets/videopress-mock-plugin.txt |
Adds mock plugin file for test environment setup |
projects/packages/videopress/changelog/fix-videopress-mime-type-filter |
Documents the bug fix in changelog format |
Code Coverage SummaryCoverage changed in 4 files.
|
When viewing the VideoPress library in Jetpack → VideoPress on a site without any VideoPress uploads, local videos (MP4, WebM, etc.) from the media library would incorrectly appear in the library. This happened because the video query included all video mime types instead of filtering to only
video/videopress.Proposed changes:
Data::get_video_data()to query onlyvideo/videopressmime type instead of all video typesmedia_type=videoANDmime_type=video/videopressin REST API queries - WordPress merges these parameters, resulting in all 16 video mime types being included in the querymedia_type: 'video'from JS resolvers for VideoPress-specific queriesOther information:
Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Automated test verification:
jp test php packages/videopress -- --filter=Data_TestThis runs the new
Data_Testwhich verifies:video/videopressmime typemedia_type=videowithmeta_queryto exclude VideoPress videos