Implement VideoPress metadata fetching#20237
Conversation
The new function retrieves all VideoPress metadata, including the token.
3e6be22 to
14224a4
Compare
You can test the changes in WordPress from this Pull Request by:
|
You can test the changes in Jetpack from this Pull Request by:
|
55791ec to
e8204d6
Compare
Generated by 🚫 dangerJS |
| pod 'Gridicons', '~> 1.1.0' | ||
|
|
||
| pod 'WordPressAuthenticator', '~> 5.1-beta' | ||
| pod 'WordPressAuthenticator', '~> 5.6-beta' |
There was a problem hiding this comment.
Before merging the PR, probably we should check if there are any important changes between 5.1 to 5.6 in WordPressAuthenticator.
There was a problem hiding this comment.
Good point.
The more interesting file to look at is Podfile.lock where WordPressAuthenticator results resolved as version 5.5.0.
The only differences between 5.5.0 and 5.6.0-beta.1 are of course all and only the changes bundled with 5.6.0-beta.1, and those are nothing more that the WordPressKit major version bump, which this PR verified.
I think this version requirement change is safe to merge. Even if an issue was introduced in going from ~> 5.1-beta to ~> 5.6-beta the culprit would not be in 5.6.0-beta.1
There was a problem hiding this comment.
By the way, was there a requirement in the code for something that came in WordPressAuthenticator 5.6.0?
Otherwise, we could have left the requirement as it was and trust in CocoaPods to resolve the correct WordPressAuthenticator version compatible with WordPressKit 7.0.0-beta.1.
Out of curiosity, I reverted this change on my machine and everything worked fine.
No big deal, though. We'll soon need to actually updated WordPressAuthenticator anyway: #20128
There was a problem hiding this comment.
The only differences between 5.5.0 and 5.6.0-beta.1 are of course all and only the changes bundled with 5.6.0-beta.1, and those are nothing more that the WordPressKit major version bump, which this PR verified.
Ah, good point. I thought might be more changes but based on Podfile.lock the only one was introduced in 5.6.0-beta.1 🤦 .
By the way, was there a requirement in the code for something that came in WordPressAuthenticator 5.6.0?
Actually, there's no change required in this PR from WordPressAuthenticator. However, when I upgraded WordPressKit to version 7.0 (reference), I was getting an error when installing the WordPressAuthenticator pod related to the minimum version. Due to this, I had to create wordpress-mobile/WordPressAuthenticator-iOS#754 and bump the minimum version of WordPressKit dependency to 7.0.
However, I've reverted the changes as you mentioned and I'm not getting now the error 🤔.
|
| App Name | ||
| Configuration | Release-Alpha | |
| Build Number | pr20237-12b62ab | |
| Version | 21.9 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 12b62ab | |
| App Center Build | jetpack-installable-builds #4283 |
|
| App Name | ||
| Configuration | Release-Alpha | |
| Build Number | pr20237-12b62ab | |
| Version | 21.9 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 12b62ab | |
| App Center Build | WPiOS - One-Offs #5259 |
|
Taking over merging to move forward with the 22.0 code freeze, of which this PR should be part of given its milestone. |
Great, thanks @mokagio for taking over and merging it 🙇 . |
Related PRs:
WordPressKitdependency to7.0WordPressAuthenticator-iOS#754Fixes #20195
This PR updates the logic to fetch VideoPress metadata and the usage of VideoPress videos.
To test
Media screen
Preparation:
As we need to test with a video, first we need to upload it. This is recommended to be done in the browser to ensure that the video points to a remote URL instead of a local cache version.
NOTE: By default, the privacy of the video will be
Site Default.1 - Public site
To set the site public:
1.1 - Public video
Public1.2 - Private video
Private1.3 - Site default video
Site DefaultNOTE: The privacy will be the same as the site, i.e.
Public.2 - Private site
To set the site public:
2.1 - Public video
Repeat the same steps as on the public site (1.1).
2.2 - Private video
Repeat the same steps as on the public site (1.2).
2.3 - Site default video
Repeat the same steps as on the public site (1.3).
Story post
Classic editor (Aztec)
This is the classic editor.).NOTE: The video's poster seems to disappear randomly. This doesn't look like a regression as I managed to reproduce it in previous versions. As far as I investigated, this is caused by how the video element is coded in the Classic editor when using a VideoPress video.
Gutenberg editor
Simple site
NOTE: Private videos won't be displayed in the editor (wordpress-mobile/gutenberg-mobile#5497), so it's recommended to use a public/hidden Simple site.
Video upload within the editor
https://videos.files.wordpress.com/<VIDEO_ID>/<FILENAME>)Video upload outside the editor
https://videos.files.wordpress.com/<VIDEO_ID>/<FILENAME>)Self-hosted site with Jetpack
NOTE: Free users can only have one video in VideoPress, so keep this in mind when testing. In case you already uploaded a video, you can delete it and upload a new one (navigate to
https://<SITE_HOST>/wp-admin/admin.php?page=jetpack-videopress#/). Alternatively, you can purchase the VideoPress plan to have unlimited uploads.Video upload within the editor
Video upload outside the editor
Self-hosted site without Jetpack
Video upload within the editor
https://<SITE>/wp-content/uploads/2023/03/<FILENAME>)Video upload outside the editor
https://<SITE>/wp-content/uploads/2023/03/<FILENAME>)Regression Notes
Potential unintended areas of impact
I haven't identified unintended areas of impact apart from the ones outlined in the above test cases.
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
I'm planning to create a follow-up PR to include automated tests.
PR submission checklist:
RELEASE-NOTES.txtif necessary.