Skip to content
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

Make sure the media API returns the VideoPress GUID #6492

Merged
merged 2 commits into from
Feb 28, 2017

Conversation

dbtlr
Copy link
Contributor

@dbtlr dbtlr commented Feb 24, 2017

In creating parity of features between Jetpack and Calypso, we've run into an issue where Jetpack sites are not returning the VideoPress GUID as expected. This change adds in a stub of the similar function that WordPress.com uses in its API, so that the API endpoint will return the GUID as expected.

Note

This is currently a pretty large blocker for the VideoPress thumbnail picker in Calypso. The sooner we can get this tested and merged the better.

To test

  1. Upload a Video w/ VideoPress enabled.
  2. Go https://developer.wordpress.com/docs/api/console/
  3. Enter the URL: /sites/$site_id/media/ w/ the jetpack site domain name as the $site_id
  4. Check the response and verify that it contains the videopress_guid for any VideoPress posts that were uploaded.

@dbtlr dbtlr added [Pri] BLOCKER [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Feb 24, 2017
@dbtlr dbtlr requested a review from jeherve February 24, 2017 14:50
@dbtlr
Copy link
Contributor Author

dbtlr commented Feb 24, 2017

Just another note, it appears that WordPress.com developer console doesn't authenticate Jetpack connections as well as I thought. The best way to test this is actually to:

  1. Go to a Jetpack site in Calypso
  2. Open a post
  3. Open the inspector
  4. Add a new media item to open the media window
  5. Check the response for the /site/$site_id/media call in the network inspector for the videopress_guid

donnapep added a commit to Automattic/wp-calypso that referenced this pull request Feb 27, 2017
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

I was uncertain of this because we're adding a function but not calling it anywhere, but then I realized that existing code was just wrapped in a function_exists condition. Works well, looks good!

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 28, 2017
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well in my tests. It should be good to merge after a minor change.

return false;
}

if ( $post->post_mime_type !== 'video/videopress' ) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use Yoda notation here, as per the WP coding standards?

Copy link
Member

Choose a reason for hiding this comment

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

Took care of it in af420e5

@see https://github.com/Automattic/jetpack/pull/6492/files#r103463656

Also added an empty line at the end of the file as per coding standards, and description for the 2 function parameters to make phpcs happy.
jeherve added a commit that referenced this pull request Feb 28, 2017
@dereksmart dereksmart merged commit 67b8ac3 into master Feb 28, 2017
@dereksmart dereksmart deleted the add/videopress-media-endpoint branch February 28, 2017 15:22
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 28, 2017
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Pri] BLOCKER [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants