FSE: Add API endpoint to side-load images #34823

obenland · 2019-07-22T18:34:39Z

Changes proposed in this Pull Request

Adds the ability to side-load images.

Testing instructions

Send a POST request to the /wp-json/wp/v2/media/image endpoint on your test site with an image url in the body.

See #34603

matticbot · 2019-07-22T18:34:42Z

Test live: https://calypso.live/?branch=add/fse-image-endpoint

obenland · 2019-07-22T18:36:09Z

Currently the endpoint will only side-load one image at a time, and won't assign it to a post.

Before implementing that I wanted to discuss if that's really needed, and if so how that request format should look like.

matticbot · 2019-07-22T18:41:14Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

getdave

A few minor comments.

.../full-site-editing-plugin/starter-page-templates/class-wp-rest-image-sideload-controller.php

marekhrabe · 2019-07-23T12:21:57Z

Any chance to make also a batch version of the endpoint as mentioned here? Not having to take care of multiple concurrent requests on the frontend would be nice. https://developer.wordpress.org/rest-api/requests/#internal-requests

If the endpoint is called several times with the same URL would it make multiple attachments or only one per URL? If multiple, can we prevent this by for example adding post meta to the attachment with the source URL and checking for it first before side loading the image?

What is the response format here? I'm not sure I can deduct it from the code. Can we make it an object with id and url and originalUrl keys?

See #34823 (comment)

obenland · 2019-07-23T22:21:23Z

Valid requests:

Batch sideload with post_id 1

POST /wp-json/fse/v1/batch?requests%5B0%5D%5Bmethod%5D=POST&requests%5B0%5D%5Broute%5D=/fse/v1/sideload/image&requests%5B0%5D%5Bparams%5D%5Burl%5D=https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg&requests%5B0%5D%5Bparams%5D%5Bpost_id%5D=1

Structure:

{
  requests: [
    {
      method: 'POST',
      route: 'fse/v1/sideload/image',
      params: {
        url: 'https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg',
        post_id: 1 // optional
      }
    }
  ]
}

Single sideload, no post_id

POST /wp-json/fse/v1/sideload/image?url=https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg

Return object single:

{
    "id": 139,
    "date": "2019-07-23T22:51:50",
    "date_gmt": "2019-07-23T22:51:50",
    "guid": {
        "rendered": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg",
        "raw": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg"
    },
    "modified": "2019-07-23T22:51:50",
    "modified_gmt": "2019-07-23T22:51:50",
    "slug": "phone-table-13",
    "status": "inherit",
    "type": "attachment",
    "link": "http://localhost/trunk/phone-table-13/",
    "template": "",
    "description": {
        "raw": "",
        "rendered": "<p class=\"attachment\"><a href='http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg'><img width=\"300\" height=\"200\" src=\"http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-300x200.jpg\" class=\"attachment-medium size-medium\" alt=\"\" srcset=\"http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-300x200.jpg 300w, http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-768x512.jpg 768w, http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-1024x683.jpg 1024w, http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg 1600w\" sizes=\"100vw\" /></a></p>\n"
    },
    "caption": {
        "raw": "",
        "rendered": ""
    },
    "alt_text": "",
    "media_type": "image",
    "mime_type": "image/jpeg",
    "media_details": {
        "width": 1600,
        "height": 1067,
        "file": "2019/07/phone-table-13.jpg",
        "sizes": {
            "thumbnail": {
                "file": "phone-table-13-150x150.jpg",
                "width": 150,
                "height": 150,
                "mime_type": "image/jpeg",
                "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-150x150.jpg"
            },
            "medium": {
                "file": "phone-table-13-300x200.jpg",
                "width": 300,
                "height": 200,
                "mime_type": "image/jpeg",
                "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-300x200.jpg"
            },
            "medium_large": {
                "file": "phone-table-13-768x512.jpg",
                "width": 768,
                "height": 512,
                "mime_type": "image/jpeg",
                "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-768x512.jpg"
            },
            "large": {
                "file": "phone-table-13-1024x683.jpg",
                "width": 1024,
                "height": 683,
                "mime_type": "image/jpeg",
                "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13-1024x683.jpg"
            },
            "full": {
                "file": "phone-table-13.jpg",
                "width": 1600,
                "height": 1067,
                "mime_type": "image/jpeg",
                "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg"
            }
        },
        "image_meta": {
            "aperture": "1.8",
            "credit": "",
            "camera": "Canon EOS 1000D",
            "caption": "",
            "created_timestamp": "0",
            "copyright": "",
            "focal_length": "50",
            "iso": "200",
            "shutter_speed": "0.004",
            "title": "",
            "orientation": "0",
            "keywords": []
        }
    },
    "post": 1,
    "source_url": "http://localhost/trunk/wp-content/uploads/2019/07/phone-table-13.jpg",
    "_links": {
        "replies": [
            {
                "embeddable": true,
                "href": "http://localhost/trunk/wp-json/wp/v2/comments?post=139"
            }
        ],
        "self": [
            {
                "href": "http://localhost/trunk/wp-json/wp/v2/media/139"
            }
        ],
        "collection": [
            {
                "href": "http://localhost/trunk/wp-json/wp/v2/media"
            }
        ],
        "about": [
            {
                "href": "http://localhost/trunk/wp-json/wp/v2/types/attachment"
            }
        ]
    }
}

See #34823 (comment)

retrofox · 2019-07-25T12:13:23Z

.../full-site-editing-plugin/starter-page-templates/class-wp-rest-sideload-image-controller.php

+		$id = media_sideload_image(
+			$request->get_param( 'url' ),
+			$request->get_param( 'post_id' ),
+			null,


Maybe it worths adding the possibility to set up the image title, through of a title parameter for instanance:x

$id = media_sideload_image( $request->get_param( 'url' ), $request->get_param( 'post_id' ), $request->get_param( 'title' ), 'id' );

I don't think we currently have that information in our templates. Not sure how generic we want to make this endpoint?

Yeah, in fact, if we want to make it more generic we should consider changing the namespace.
Anyway, there's anything wrong to allow the title attr? In the worse of cases, we just ignore it.

...e-editing/full-site-editing-plugin/starter-page-templates/class-wp-rest-batch-controller.php

retrofox · 2019-07-25T13:46:09Z

Testing /batch endpoint passing the sideload/image route I get a lot of PHP Notices:

[25-Jul-2019 13:38:04 UTC] PHP Notice:  Undefined index: type in /var/www/html/wp-includes/rest-api.php on line 1141
[25-Jul-2019 13:38:04 UTC] PHP Stack trace:
[25-Jul-2019 13:38:04 UTC] PHP   1. {main}() /var/www/html/index.php:0
[25-Jul-2019 13:38:04 UTC] PHP   2. require() /var/www/html/index.php:17
[25-Jul-2019 13:38:04 UTC] PHP   3. wp() /var/www/html/wp-blog-header.php:16
[25-Jul-2019 13:38:04 UTC] PHP   4. WP->main() /var/www/html/wp-includes/functions.php:1105
[25-Jul-2019 13:38:04 UTC] PHP   5. WP->parse_request() /var/www/html/wp-includes/class-wp.php:737
[25-Jul-2019 13:38:04 UTC] PHP   6. do_action_ref_array() /var/www/html/wp-includes/class-wp.php:387
[25-Jul-2019 13:38:04 UTC] PHP   7. WP_Hook->do_action() /var/www/html/wp-includes/plugin.php:531
[25-Jul-2019 13:38:04 UTC] PHP   8. WP_Hook->apply_filters() /var/www/html/wp-includes/class-wp-hook.php:310
[25-Jul-2019 13:38:04 UTC] PHP   9. rest_api_loaded() /var/www/html/wp-includes/class-wp-hook.php:286
[25-Jul-2019 13:38:04 UTC] PHP  10. WP_REST_Server->serve_request() /var/www/html/wp-includes/rest-api.php:309
[25-Jul-2019 13:38:04 UTC] PHP  11. WP_REST_Server->dispatch() /var/www/html/wp-includes/rest-api/class-wp-rest-server.php:329
[25-Jul-2019 13:38:04 UTC] PHP  12. WP_REST_Request->sanitize_params() /var/www/html/wp-includes/rest-api/class-wp-rest-server.php:888
[25-Jul-2019 13:38:04 UTC] PHP  13. rest_parse_request_arg() /var/www/html/wp-includes/rest-api/class-wp-rest-request.php:793
[25-Jul-2019 13:38:04 UTC] PHP  14. rest_validate_request_arg() /var/www/html/wp-includes/rest-api.php:1002
[25-Jul-2019 13:38:04 UTC] PHP  15. rest_validate_value_from_schema() /var/www/html/wp-includes/rest-api.php:965
[25-Jul-2019 13:38:04 UTC] PHP  16. rest_validate_value_from_schema() /var/www/html/wp-includes/rest-api.php:1150

Despite this, it works and uploads an image only if we add only one route.
When I define two routes, I get this one:

{
    "POST \/fse\/v1\/sideload\/image": {
        "code": "rest_invalid_param",
        "message": "Invalid parent type.",
        "data": {
            "status": 400
        }
    }
}

This is the request body

requests
Array
(
    [0] => Array
        (
            [method] => POST
            [route] => /fse/v1/sideload/image
            [params] => Array
                (
                    [url] => https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg
                )

        )

    [1] => Array
        (
            [method] => POST
            [route] => /fse/v1/sideload/image
            [params] => Array
                (
                    [url] => https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg
                )

        )

)

@retrofox

Props @retrofox.

obenland · 2019-07-25T18:06:13Z

@retrofox Thanks so much for your review! All errors should be fixed now, would you mind testing them again?

retrofox · 2019-07-25T19:29:27Z

Tested again and it works as expected. Created two media entries and a post entry hitting the /batch endpoint. 👍
I'm still getting the PHP Notice: Undefined index: type... notice but not sure if it's a blocker.

obenland · 2019-07-25T19:32:18Z

I'm still getting the PHP Notice: Undefined index: type... notice but not sure if it's a blocker.
What's your query string for that?

@marekhrabe

/cc @marekhrabe

obenland · 2019-07-25T19:43:53Z

@marekhrabe The API now sanitizes image URLs and reuses previously sideloaded images.

obenland · 2019-08-12T21:30:21Z

Valid requests:

Batch sideload with post_id 1

POST /wp-json/fse/v1/sideload/image/batch?resources[0][url]=https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg&resources[0][post_id]=1&resources[1][url]=https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg

Structure:

{
  resources: [
    {
      url: 'https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg',
      post_id: 1 // optional
    }
  ]
}

Single sideload, no post_id

POST /wp-json/fse/v1/sideload/image?url=https://a8ctm1.files.wordpress.com/2019/06/phone-table.jpg

marekhrabe

🚢

vindl · 2019-08-13T10:26:32Z

I'm seeing the following fatal error when trying to create a new page or edit an existing one:

Fatal error: Uncaught Error: Class 'A8C\FSE\WP_REST_Sideload_Image_Controller' not found in /var/www/html/wp-content/plugins/full-site-editing-plugin/starter-page-templates/class-starter-page-templates.php:82 Stack trace: #0 /var/www/html/wp-includes/class-wp-hook.php(286): A8C\FSE\Starter_Page_Templates->register_rest_api(Object(WP_REST_Server)) #1 /var/www/html/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(NULL, Array) #2 /var/www/html/wp-includes/plugin.php(465): WP_Hook->do_action(Array) #3 /var/www/html/wp-includes/rest-api.php(475): do_action('rest_api_init', Object(WP_REST_Server)) #4 /var/www/html/wp-includes/rest-api.php(433): rest_get_server() #5 /var/www/html/wp-includes/rest-api.php(1394): rest_do_request(Object(WP_REST_Request)) #6 [internal function]: rest_preload_api_request(Array, '/') #7 /var/www/html/wp-admin/edit-form-blocks.php(76): array_reduce(Array, 'rest_preload_ap...', Array) #8 /var/www/html/wp-admin/post.php(178): include('/var/www/html/w...') #9 {main} thrown in /var/www/html/wp-content/plugins/full-site-editing-plugin/starter-page-templates/class-starter-page-templates.php on line 82

Maybe smoke testing the plugin should be added to testing instructions, since editing a page is quite a common flow to test both for SPT and FSE. I'm going to revert this in order to unblock our FSE plugin sync on Dotcom.

This reverts commit fb996ff.

obenland · 2019-08-13T14:57:16Z

I'm going to revert this in order to unblock our FSE plugin sync on Dotcom.

@vindl Why was this blocking the plugin sync? I thought there was already a diff for that?
This was not meant to go into the current release

@retrofox

* FSE: Add API endpoint to side-load images * Address Dave's feedback * Rename class to match rest base * Add batch endpoint See #34823 (comment) * Add post_id parameter * Make sure we link to the appropriate URLs * Remove empty line * FSE: Add API endpoint to side-load images * Address Dave's feedback * Rename class to match rest base * Add batch endpoint See #34823 (comment) * Add post_id parameter * Make sure we link to the appropriate URLs * Remove empty line * Fix errors from review Props @retrofox. * Look for previously sideloaded images before saving it * Sanitize image url /cc @marekhrabe * Merge `/batch` endpoint into sideload endpoint

This reverts commit fb996ff.

…35328)" This reverts commit 8aff6a8.

) * Revert "Revert "FSE: Add API endpoint to side-load images (#34823)" (#35328)" This reverts commit 8aff6a8. * Add proper namespacing

FSE: Add API endpoint to side-load images

06112aa

obenland added [Type] Enhancement API [Goal] Full Site Editing labels Jul 22, 2019

obenland requested review from marekhrabe and a team July 22, 2019 18:34

obenland requested review from a team as code owners July 22, 2019 18:34

obenland self-assigned this Jul 22, 2019

obenland added the [Status] In Progress label Jul 22, 2019

getdave reviewed Jul 23, 2019

View reviewed changes

.../full-site-editing-plugin/starter-page-templates/class-wp-rest-image-sideload-controller.php Outdated Show resolved Hide resolved

getdave reviewed Jul 23, 2019

View reviewed changes

.../full-site-editing-plugin/starter-page-templates/class-wp-rest-image-sideload-controller.php Outdated Show resolved Hide resolved

obenland added 4 commits July 23, 2019 14:50

Address Dave's feedback

0769169

Rename class to match rest base

95b656b

Add batch endpoint

fa8232f

See #34823 (comment)

Add post_id parameter

08f8dfd

obenland added 2 commits July 23, 2019 15:52

Make sure we link to the appropriate URLs

63128bc

Remove empty line

3adb4e9

obenland added 5 commits July 24, 2019 12:26

FSE: Add API endpoint to side-load images

1fb25c1

Address Dave's feedback

5b70fb1

Rename class to match rest base

0480011

Add batch endpoint

249aa03

See #34823 (comment)

Add post_id parameter

b4323f9

retrofox reviewed Jul 25, 2019

View reviewed changes

...e-editing/full-site-editing-plugin/starter-page-templates/class-wp-rest-batch-controller.php Outdated Show resolved Hide resolved

Fix errors from review

e90f4bb

Props @retrofox.

Look for previously sideloaded images before saving it

49e542c

Sanitize image url

f1f69dd

/cc @marekhrabe

obenland added this to the Ajax: Iteration 7 milestone Aug 8, 2019

Merge /batch endpoint into sideload endpoint

28cb1bf

obenland added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Aug 12, 2019

marekhrabe approved these changes Aug 12, 2019

View reviewed changes

obenland merged commit fb996ff into master Aug 12, 2019

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 12, 2019

vindl added a commit that referenced this pull request Aug 13, 2019

Revert "FSE: Add API endpoint to side-load images (#34823)"

8146b15

This reverts commit fb996ff.

vindl mentioned this pull request Aug 13, 2019

Revert "FSE: Add API endpoint to side-load images" #35328

Merged

vindl added a commit that referenced this pull request Aug 13, 2019

Revert "FSE: Add API endpoint to side-load images (#34823)" (#35328)

8aff6a8

This reverts commit fb996ff.

getdave pushed a commit that referenced this pull request Aug 13, 2019

Revert "FSE: Add API endpoint to side-load images (#34823)" (#35328)

97cb2c6

This reverts commit fb996ff.

obenland added a commit that referenced this pull request Aug 13, 2019

Revert "Revert "FSE: Add API endpoint to side-load images (#34823)" (#…

b566abe

…35328)" This reverts commit 8aff6a8.

obenland added a commit that referenced this pull request Aug 16, 2019

(2P) Revert "Revert "FSE: Add API endpoint to side-load images"" (#35344

b7e8ff7

) * Revert "Revert "FSE: Add API endpoint to side-load images (#34823)" (#35328)" This reverts commit 8aff6a8. * Add proper namespacing

sirreal deleted the add/fse-image-endpoint branch November 27, 2020 11:16

jeherve mentioned this pull request Jan 18, 2021

Carousel: images not displayed when a Tiled Gallery uses external images Automattic/jetpack#18362

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FSE: Add API endpoint to side-load images #34823

FSE: Add API endpoint to side-load images #34823

obenland commented Jul 22, 2019

matticbot commented Jul 22, 2019

obenland commented Jul 22, 2019

matticbot commented Jul 22, 2019

getdave left a comment

marekhrabe commented Jul 23, 2019

obenland commented Jul 23, 2019 •

edited

Loading

retrofox Jul 25, 2019

obenland Jul 25, 2019

retrofox Jul 25, 2019

retrofox commented Jul 25, 2019

obenland commented Jul 25, 2019

retrofox commented Jul 25, 2019 •

edited

Loading

obenland commented Jul 25, 2019

obenland commented Jul 25, 2019

obenland commented Aug 12, 2019

marekhrabe left a comment

vindl commented Aug 13, 2019

obenland commented Aug 13, 2019 •

edited

Loading

FSE: Add API endpoint to side-load images #34823

FSE: Add API endpoint to side-load images #34823

Conversation

obenland commented Jul 22, 2019

Changes proposed in this Pull Request

Testing instructions

matticbot commented Jul 22, 2019

obenland commented Jul 22, 2019

matticbot commented Jul 22, 2019

getdave left a comment

Choose a reason for hiding this comment

marekhrabe commented Jul 23, 2019

obenland commented Jul 23, 2019 • edited Loading

Valid requests:

Batch sideload with post_id 1

Single sideload, no post_id

Return object single:

retrofox Jul 25, 2019

Choose a reason for hiding this comment

obenland Jul 25, 2019

Choose a reason for hiding this comment

retrofox Jul 25, 2019

Choose a reason for hiding this comment

retrofox commented Jul 25, 2019

obenland commented Jul 25, 2019

retrofox commented Jul 25, 2019 • edited Loading

obenland commented Jul 25, 2019

obenland commented Jul 25, 2019

obenland commented Aug 12, 2019

Valid requests:

Batch sideload with post_id 1

Single sideload, no post_id

marekhrabe left a comment

Choose a reason for hiding this comment

vindl commented Aug 13, 2019

obenland commented Aug 13, 2019 • edited Loading

obenland commented Jul 23, 2019 •

edited

Loading

retrofox commented Jul 25, 2019 •

edited

Loading

obenland commented Aug 13, 2019 •

edited

Loading