Skip to content

Implement VideoPress metadata fetching#20237

Merged
mokagio merged 18 commits into
trunkfrom
add/videopress-metadata
Mar 20, 2023
Merged

Implement VideoPress metadata fetching#20237
mokagio merged 18 commits into
trunkfrom
add/videopress-metadata

Conversation

@fluiddot

@fluiddot fluiddot commented Mar 2, 2023

Copy link
Copy Markdown
Contributor

Related PRs:

Fixes #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.

  1. Go to a simple site in the browser.
  2. Go to the Media page.
  3. Upload a video.

NOTE: By default, the privacy of the video will be Site Default.

1 - Public site

To set the site public:

  1. Go to the Site Settings screen.
  2. Tap on the Privacy option.
  3. Change privacy to Public or Hidden.

1.1 - Public video

  1. Go to a simple site in the browser.
  2. Go to the Media page.
  3. Edit the uploaded video.
  4. Change Privacy to Public
  5. Open the app.
  6. Go to the Media screen.
  7. Tap on the uploaded video.
  8. Play the video.
  9. Observe that the video can be played.

1.2 - Private video

  1. Go to a simple site in the browser.
  2. Go to the Media page.
  3. Edit the uploaded video.
  4. Change Privacy to Private
  5. Open the app.
  6. Go to the Media screen.
  7. Tap on the uploaded video.
  8. Play the video.
  9. Observe that the video can be played.

1.3 - Site default video

  1. Go to a simple site in the browser.
  2. Go to the Media page.
  3. Edit the uploaded video.
  4. Change Privacy to Site Default
    NOTE: The privacy will be the same as the site, i.e. Public.
  5. Open the app.
  6. Go to the Media screen.
  7. Tap on the uploaded video.
  8. Play the video.
  9. Observe that the video can be played.

2 - Private site

To set the site public:

  1. Go to the Site Settings screen.
  2. Tap on the Privacy option.
  3. Change privacy to Private.

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

  1. Create a Story post.
  2. Open the media selection sheet by tapping on the square icon on the left side.
  3. Tap on the Media tab.
  4. Select an uploaded video that is private.
  5. Observe that the video is added to the story.

Classic editor (Aztec)

  1. Create a post.
  2. Switch to HTML mode by tapping on the three dots icon located at the top bar.
  3. Type some text without HTML tags (e.g. This is the classic editor.).
  4. Save the post and open it again.
  5. Observe that the post is opened with the classic editor.
  6. Tap on the ➕ button located in the bottom bar.
  7. Tap on the WordPress icon to display the items of the Media library.
  8. Select an uploaded video that is private.
  9. Observe that the video is added to the post.
  10. Tap on the video and play it.
  11. Observe that the video is played.

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

  1. Create a post.
  2. Tap on the ➕ button and add a Video block.
  3. Tap on the "Choose from device" option.
  4. Select a video file.
  5. Wait until the upload finish.
  6. Observe that the video is added to the block and that can be played.
  7. Switch to HTML mode.
  8. Observe that the video source points to VideoPress (e.g. https://videos.files.wordpress.com/<VIDEO_ID>/<FILENAME>)

Video upload outside the editor

  1. Create a post.
  2. Tap on the ➕ button and add a Video block.
  3. Tap on the "Choose from device" option.
  4. Select a video file.
  5. Before the upload finish, tap on the ❌ button to close the editor and save the post.
  6. In the Post list, wait until the upload finish.
  7. Open the same post.
  8. Observe that the video is added to the block and that can be played.
  9. Switch to HTML mode.
  10. Observe that the video source points to VideoPress (e.g. 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

  1. Repeat the same steps as in Simple site - Video upload within the editor.

Video upload outside the editor

  1. Repeat the same steps as in Simple site - Video upload outside the editor.

Self-hosted site without Jetpack

Video upload within the editor

  1. Repeat the same steps as in Simple site - Video upload within the editor.
  2. When switching to HTML mode, observe that the video URL points to the site instead of Video Press (i.e. https://<SITE>/wp-content/uploads/2023/03/<FILENAME>)

Video upload outside the editor

  1. Repeat the same steps as in Simple site - Video upload outside the editor.
  2. When switching to HTML mode, observe that the video URL points to the site instead of Video Press (i.e. https://<SITE>/wp-content/uploads/2023/03/<FILENAME>)

Regression Notes

  1. Potential unintended areas of impact
    I haven't identified unintended areas of impact apart from the ones outlined in the above test cases.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. 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:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot force-pushed the add/videopress-metadata branch from 3e6be22 to 14224a4 Compare March 7, 2023 18:22
@wpmobilebot

wpmobilebot commented Mar 7, 2023

Copy link
Copy Markdown
Contributor
You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20237-cb45330 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot

wpmobilebot commented Mar 7, 2023

Copy link
Copy Markdown
Contributor
You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20237-cb45330 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@fluiddot fluiddot marked this pull request as ready for review March 9, 2023 12:15
@fluiddot fluiddot changed the title [Gutenberg] Implement VideoPress metadata fetching Implement VideoPress metadata fetching Mar 9, 2023
@fluiddot fluiddot added this to the 22.0 milestone Mar 9, 2023
@twstokes twstokes self-requested a review March 12, 2023 19:43
@fluiddot fluiddot force-pushed the add/videopress-metadata branch from 55791ec to e8204d6 Compare March 14, 2023 11:42

@twstokes twstokes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good @fluiddot - great work! 👏 Apologies for the delay.

All tests passed on my side and I only had one question about a completion handler. Thanks!

Comment thread WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift
Comment thread WordPress/Classes/Services/Stories/StoryMediaLoader.swift
Comment thread WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift
@peril-wordpress-mobile

Copy link
Copy Markdown
Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@fluiddot fluiddot requested a review from twstokes March 16, 2023 10:16

@twstokes twstokes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @fluiddot! 🚀

Comment thread Podfile
pod 'Gridicons', '~> 1.1.0'

pod 'WordPressAuthenticator', '~> 5.1-beta'
pod 'WordPressAuthenticator', '~> 5.6-beta'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging the PR, probably we should check if there are any important changes between 5.1 to 5.6 in WordPressAuthenticator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔.

@wpmobilebot

Copy link
Copy Markdown
Contributor
Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr20237-12b62ab
Version21.9
Bundle IDcom.jetpack.alpha
Commit12b62ab
App Center Buildjetpack-installable-builds #4283
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot

Copy link
Copy Markdown
Contributor
WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr20237-12b62ab
Version21.9
Bundle IDorg.wordpress.alpha
Commit12b62ab
App Center BuildWPiOS - One-Offs #5259
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio

mokagio commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

Taking over merging to move forward with the 22.0 code freeze, of which this PR should be part of given its milestone.

@mokagio mokagio merged commit 8ae9999 into trunk Mar 20, 2023
@mokagio mokagio deleted the add/videopress-metadata branch March 20, 2023 02:15
@fluiddot

Copy link
Copy Markdown
Contributor Author

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 🙇 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Videos can't be played in private sites

4 participants