Add new media endpoints #5281

retrofox · 2016-10-07T13:41:15Z

This PR adds new versions of get-media endpoint and add the new edit-media endpoint to the WP COM API.

Changes proposed in this Pull Request:

Add v1.2 `[POST]` `/sites/$site/media/$mediaID/edit`

The edit-media endpoint was added into the WP COM API a while ago. This patch adds the needed code to support the edit-media endpoint now for Jetpack sites.
The hardest part of this process is to detect when a media item should be added (new-media endpoint) or when it should be updated (edit-media endpoint).

The right action to take depends on the action parameter into the request body. it could be jetpack_upload_action (the only one action used at the moment) and jetpack_update_action. When the action is the last one, the attached metadata of the media item is updated according to the new uploaded media file instead of creating a new media item.
Also, other tasks are done in this editing process, such as store the original media file, keep the revision history, clean the remaining files when the media item is deleted, etc.

Add v1.2 `[GET]` `/sites/$site/media/$mediaID`

There aren't significant changes in this endpoint. It just adds some fields to the response body.

gwwar · 2016-10-07T20:40:09Z

cc @mcsf @gravityrail if we're missing anything major for a Jetpack enpoint

retrofox · 2016-10-07T21:00:14Z

Something that we have to achieve is to be able to upload a new image without creating a new record into of dB.

As far as I see we should split the request into two requests. First one to upload the new image and the second one to update the image-post record.

For this reason, I think we should create a new Jetpack action, for instance, jetpack_update_file. The behavior behind this action should be similar to jetpack_upload_action which is tied the upload-media endpoint.

So the jetpack_update_file action is added we should just upload the file and do not create a new record. Finally, in the second step of this request, we could update the attached file of the media post with the new file uploaded in the previous step.

retrofox · 2016-10-07T21:04:04Z

I wonder if we can modify upload_handler() to avoid creating a new record into dB: https://github.com/Automattic/jetpack/blob/master/class.jetpack.php#L3010

mcsf · 2016-10-08T16:17:33Z

@gwwar: for Jetpack stuff, you probably meant the other Miguel, @lezama. Signed: the other other Miguel

gwwar · 2016-10-09T16:47:11Z

😅 Sorry about that Miguel

samhotchkiss · 2016-11-09T19:48:52Z

Removing the milestone until this is completed-- please feel free to move it back into the appropriate milestone when it's out of "In Progress"

samhotchkiss · 2017-02-07T02:18:23Z

Closing this for now, feel free to reopen if it ever gets fresh attention

obenland · 2017-02-14T19:30:37Z

See Automattic/wp-calypso#11241

retrofox · 2017-02-15T21:13:27Z

Starting to work on this again.

dereksmart · 2017-02-28T14:15:56Z

_inc/lib/class.media.php

+
+		if ( ! empty( $original_media ) ) {
+			$original_file_parts = pathinfo( $original_media['file'] );
+			$original_file_ext = $original_file_parts['filename'];


This var is not used anywhere?

No. removing ...

dereksmart · 2017-02-28T14:21:49Z

_inc/lib/class.media.php

+}
+
+// hook: clean revision history when the media item is deleted
+$clean_revision_history = function( $media_id ) {


Unfortunately we need to support PHP 5.2.4 :(

Anonymous functions were introduced in 5.3

dereksmart · 2017-02-28T14:24:17Z

json-endpoints/class.wpcom-json-api-edit-media-v1-2-endpoint.php

+			}
+		}
+
+		return $post_update_action;


Looks like this may not be defined if there's no $attrs['alt'] set.

Yes, that's right. I've decided to implement the same behavior that we already do with other endpoints which is silence these errors when these metadata aren't updated, like here: https://github.com/Automattic/jetpack/blob/master/json-endpoints/class.wpcom-json-api-update-media-v1-1-endpoint.php#L41

We still returning an error when the media item updating (post) fails.

dereksmart · 2017-02-28T14:26:55Z

json-endpoints/class.wpcom-json-api-edit-media-v1-2-endpoint.php

+	 * @param  {String} $url - image URL to save locally
+	 * @return {Array|WP_Error} An array with information about the new file saved or a WP_Error is something went wrong.
+	 */
+	private function build_file_array_from_url( $media_id, $url ) {


do we need $media_id param?

yup, If I'm not wrong it's needed to update the media item (post object).

$insert['ID'] = $media_id; $update_action = wp_update_post( (object) $insert );

dereksmart · 2017-02-28T14:28:18Z

json-endpoints/class.wpcom-json-api-edit-media-v1-2-endpoint.php

+ * WARNING: This file is distributed verbatim in Jetpack.
+ * There should be nothing WordPress.com specific in this file.
+ *
+ * @hide-in-jetpack


You can remove this block -- it should only be added to wpcom as it tells our syncing script to omit it when building out the commits.

Yeah, it's confusing. tl;dr this @hide-in-jetpack tells a script that we use to hide this block from Jetpack and it's safe to remove from this PR.

Anyway, it's fine to leave it in too. Not a blocker.

dereksmart · 2017-02-28T14:43:32Z

_inc/lib/class.media.php

+/**
+ * Class to handle different actions related to media.
+ */
+class Media {


Let's namespace this Jetpack_

kraftbj · 2017-02-28T14:56:34Z

_inc/lib/class.media.php

+
+// hook: clean revision history when the media item is deleted
+function clean_revision_history( $media_id ) {
+	error_log( 'mas se perdio en la guerra ' );


I think this may have been left in...

🤦‍♂️ already removed

rralian · 2017-02-28T19:11:18Z

This is working well for me. And although github says changes requested, it looks like the changes @dereksmart asked for are handled. So if it looks good to him, I think this is good to 🚢

dereksmart

I'm happy :) merging!

retrofox · 2017-02-28T19:41:13Z

I'm happy too. :-D

@see

* 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

lezama · 2017-03-01T18:28:33Z

yay!

retrofox added the [Feature] WPCOM API label Oct 7, 2016

matticbot added the Touches WP.com Files label Oct 7, 2016

retrofox added the [Status] In Progress label Oct 7, 2016

jeherve added this to the 4.3.2 milestone Oct 7, 2016

jeherve added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Oct 7, 2016

retrofox changed the title ~~Update/add get and update 1 2 media endpoints~~ [WIP] Add new versions of get and update media endpoints Oct 7, 2016

samhotchkiss modified the milestones: 4.3.4, 4.3.2 Oct 11, 2016

zinigor modified the milestones: 4.4, 4.3.3 Oct 25, 2016

samhotchkiss modified the milestones: Not Currently Planned, 4.4 Nov 9, 2016

samhotchkiss closed this Feb 7, 2017

samhotchkiss removed the [Status] In Progress label Feb 7, 2017

jeherve added [Status] In Progress and removed [Status] In Progress labels Feb 8, 2017

jeherve deleted the update/add-get-and-update-1-2-media-endpoints branch February 8, 2017 14:57

retrofox restored the update/add-get-and-update-1-2-media-endpoints branch February 15, 2017 21:13

retrofox reopened this Feb 15, 2017

jeherve added the [Status] In Progress label Feb 15, 2017

retrofox added this to the Update media endpoints milestone Feb 16, 2017

retrofox added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Feb 28, 2017

retrofox force-pushed the update/add-get-and-update-1-2-media-endpoints branch from d3ffed6 to f89ed2f Compare February 28, 2017 14:13

dereksmart reviewed Feb 28, 2017

View reviewed changes

dereksmart requested changes Feb 28, 2017

View reviewed changes

kraftbj reviewed Feb 28, 2017

View reviewed changes

retrofox force-pushed the update/add-get-and-update-1-2-media-endpoints branch 3 times, most recently from a3e2408 to 8b882c6 Compare February 28, 2017 15:35

retrofox added 5 commits February 28, 2017 13:05

Add class.media php class

41b2115

Add get-media-v1-2 endpoint

55ae923

Add edit-media-v1-2 endpoint

b29b0b8

json-endpoint: add media endpoint definitions

01c4bb6

class.jetpack: edit media item

9941c20

retrofox force-pushed the update/add-get-and-update-1-2-media-endpoints branch from 8b882c6 to 9941c20 Compare February 28, 2017 16:05

dereksmart approved these changes Feb 28, 2017

View reviewed changes

dereksmart merged commit fee5812 into master Feb 28, 2017

dereksmart deleted the update/add-get-and-update-1-2-media-endpoints branch February 28, 2017 19:28

dereksmart removed the [Status] Needs Review This PR is ready for review. label Feb 28, 2017

This was referenced Feb 28, 2017

Add get-media-v1-2 endpoint #6426

Closed

Add edit-media-v1-2 endpoint #6428

Closed

jeherve added [Status] Needs Changelog and removed [Status] Needs Changelog labels Feb 28, 2017

jeherve added a commit that referenced this pull request Feb 28, 2017

Changelog: add #5281

f385d2d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new media endpoints #5281

Add new media endpoints #5281

retrofox commented Oct 7, 2016 •

edited

Loading

gwwar commented Oct 7, 2016

retrofox commented Oct 7, 2016 •

edited

Loading

retrofox commented Oct 7, 2016

mcsf commented Oct 8, 2016

gwwar commented Oct 9, 2016

samhotchkiss commented Nov 9, 2016

samhotchkiss commented Feb 7, 2017

obenland commented Feb 14, 2017 •

edited

Loading

retrofox commented Feb 15, 2017

dereksmart Feb 28, 2017

retrofox Feb 28, 2017

dereksmart Feb 28, 2017

dereksmart Feb 28, 2017

retrofox Feb 28, 2017

dereksmart Feb 28, 2017

retrofox Feb 28, 2017

dereksmart Feb 28, 2017

dereksmart Feb 28, 2017

dereksmart Feb 28, 2017

retrofox Feb 28, 2017

kraftbj Feb 28, 2017

retrofox Feb 28, 2017

rralian commented Feb 28, 2017

dereksmart left a comment

retrofox commented Feb 28, 2017

lezama commented Mar 1, 2017

Add new media endpoints #5281

Add new media endpoints #5281

Conversation

retrofox commented Oct 7, 2016 • edited Loading

Changes proposed in this Pull Request:

Add v1.2 [POST] /sites/$site/media/$mediaID/edit

Add v1.2 [GET] /sites/$site/media/$mediaID

gwwar commented Oct 7, 2016

retrofox commented Oct 7, 2016 • edited Loading

retrofox commented Oct 7, 2016

mcsf commented Oct 8, 2016

gwwar commented Oct 9, 2016

samhotchkiss commented Nov 9, 2016

samhotchkiss commented Feb 7, 2017

obenland commented Feb 14, 2017 • edited Loading

retrofox commented Feb 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rralian commented Feb 28, 2017

dereksmart left a comment

Choose a reason for hiding this comment

retrofox commented Feb 28, 2017

lezama commented Mar 1, 2017

retrofox commented Oct 7, 2016 •

edited

Loading

Add v1.2 `[POST]` `/sites/$site/media/$mediaID/edit`

Add v1.2 `[GET]` `/sites/$site/media/$mediaID`

retrofox commented Oct 7, 2016 •

edited

Loading

obenland commented Feb 14, 2017 •

edited

Loading