WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 3 weeks ago

#49985 assigned defect (bug)

REST API: Using _embed and _fields query parameters in the same query

Reported by: anouarbenothman Owned by: TimothyBlynJacobs
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.4
Component: REST API Keywords: needs-unit-tests early has-patch
Focuses: rest-api Cc:

Description

When performing a REST API request and adding the _embed and _fields query parameters. the API attempts to skip unneeded fields but does not embed resources that are linked with embeddable set to true.

In some cases we may need _embed and _fields in the same query.
I am requesting the feature to be added to allow the filtering of resources and embedding them (the embeddable ones) at the same time when specifying the _embed and _fields.

Example:
http://<site>/wp-json/wp/v2/posts?_fields=title,author&_embed=author

If implemented, the above request should only return title and author, and embed the author.

Change History (22)

#1 @ocean90
13 months ago

Hello @anouarbenothman, welcome to Trac!

Thanks for the report. I noticed this a while ago too but seems like I never created a ticket for it. The workaround is to include _links in the fields too so you get at least all embed data.

No embed: https://wordpress.org/news/wp-json/wp/v2/posts?_embed=true&_fields=id,title
With embed: https://wordpress.org/news/wp-json/wp/v2/posts?_embed=true&_fields=id,title,_links

#49538 is about getting only a subset of the embed data.

#2 @TimothyBlynJacobs
13 months ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Yeah this is a bit of an annoying issue. The problem is that rest_filter_response_fields will remove the _links field before link embedding is processed. So the server doesn't know what resources to embed.

We could perhaps fix this by not excluding _links in rest_filter_response_fields if _embed is set.

Another step would be to reduce the _links to only contain entries specified by _embed if _links is not included in _fields at all.

#3 @anouarbenothman
13 months ago

  • Keywords needs-patch needs-unit-tests removed

Thank you very much for the reply.

#4 @anouarbenothman
13 months ago

  • Keywords needs-patch needs-unit-tests added

#5 @TimothyBlynJacobs
7 months ago

  • Keywords early added
  • Milestone changed from Future Release to 5.7
  • Owner set to TimothyBlynJacobs
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 months ago

This ticket was mentioned in PR #867 on WordPress/wordpress-develop by engahmeds3ed.


4 months ago

  • Keywords has-patch added; needs-patch removed

https://core.trac.wordpress.org/ticket/49985

  • [x] Adjust the code to check if request has embed and _links are not inside fields then add specific embeds fields to be fetched only.
  • [ ] Adjust tests

#8 @prbot
4 months ago

TimothyBJacobs commented on PR #867:

Thanks for working on this @engahmeds3ed!

The patch looks good to me! It'd be great to get test coverage on this as well.

The test failures are a bit odd, it looks like the rest_filter_response_fields tests are passing an array instead of a proper WP_REST_Request object. I think changing the tests to properly pass a request object would be the fix here. I don't think we need to adjust rest_filter_response_fields to accept an array.

#9 @prbot
4 months ago

engahmeds3ed commented on PR #867:

Thanks @TimothyBJacobs for your help here ( as usual ), this is my first time writing WP Core test here and interact with all of these coding standards, plz when u have a minute check this one and tell me if this test is enough or we need to add some other test cases?!

#10 @prbot
4 months ago

TimothyBJacobs commented on PR #867:

Looks great to me @engahmeds3ed, thanks for working on it! Just a minor feedback item.

#11 @prbot
4 months ago

TimothyBJacobs commented on PR #867:

Looks great to me @engahmeds3ed, thanks for working on it! Just a minor feedback item.

#12 @prbot
4 months ago

TimothyBJacobs commented on PR #867:

@engahmeds3ed Sorry for not noticing the other day, but this actually isn't quite right yet. We need to handle when _embed=1 or just ?_embed, in other words when all embeddable links should be embedded. See rest_parse_embed_param.

We could probably do this by looping over $response→get_links() and looking for links that are set as embeddable.

#13 @prbot
4 months ago

engahmeds3ed commented on PR #867:

@TimothyBJacobs good catch, I'm adjusting that now and add also new test case for it

#14 @prbot
3 months ago

TimothyBJacobs commented on PR #867:

A couple of things we need to handle. When requesting a collection with _embed=1, for instance http://localhost:8889/wp-json/wp/v2/posts?_embed=1&_fields=id,author, I get the following response.

[
  {
    "id": 1,
    "author": 1
  }
]

That's because in this case the links aren't a part of WP_REST_Response::get_links() but in each individual item's _links. So we need to do the same kind of wp_is_numeric_array check we see at the bottom of rest_filter_response_fields.

Also, for a single item, the _links aren't automatically reduced. For instance, http://localhost:8889/wp-json/wp/v2/posts/1?_embed=replies,author&_fields=id,author.

{
  "id": 1,
  "author": 1,
  "_links": {
    "self": [
      {
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/posts\/1"
      }
    ],
    "collection": [
      {
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/posts"
      }
    ],
    "about": [
      {
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/types\/post"
      }
    ],
    "author": [
      {
        "embeddable": true,
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/users\/1"
      }
    ],
    "replies": [
      {
        "embeddable": true,
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/comments?post=1"
      }
    ],
    "version-history": [
      {
        "count": 0,
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/posts\/1\/revisions"
      }
    ],
    "wp:attachment": [
      {
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/media?parent=1"
      }
    ],
    "wp:term": [
      {
        "taxonomy": "category",
        "embeddable": true,
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/categories?post=1"
      },
      {
        "taxonomy": "post_tag",
        "embeddable": true,
        "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/tags?post=1"
      }
    ],
    "curies": [
      {
        "name": "wp",
        "href": "https:\/\/api.w.org\/{rel}",
        "templated": true
      }
    ]
  },
  "_embedded": {
    "author": [
      {
        "id": 1,
        "name": "admin",
        "url": "http:\/\/localhost:8889",
        "description": "",
        "link": "http:\/\/localhost:8889\/author\/admin\/",
        "slug": "admin",
        "avatar_urls": {
          "24": "http:\/\/2.gravatar.com\/avatar\/b642b4217b34b1e8d3bd915fc65c4452?s=24&d=mm&r=g",
          "48": "http:\/\/2.gravatar.com\/avatar\/b642b4217b34b1e8d3bd915fc65c4452?s=48&d=mm&r=g",
          "96": "http:\/\/2.gravatar.com\/avatar\/b642b4217b34b1e8d3bd915fc65c4452?s=96&d=mm&r=g"
        },
        "_links": {
          "self": [
            {
              "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/users\/1"
            }
          ],
          "collection": [
            {
              "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/users"
            }
          ]
        }
      }
    ],
    "replies": [
      [
        {
          "id": 1,
          "parent": 0,
          "author": 0,
          "author_name": "A WordPress Commenter",
          "author_url": "https:\/\/wordpress.org\/",
          "date": "2021-01-17T02:03:45",
          "content": {
            "rendered": "<p>Hi, this is a comment.<br \/>\nTo get started with moderating, editing, and deleting comments, please visit the Comments screen in the dashboard.<br \/>\nCommenter avatars come from <a href=\"https:\/\/gravatar.com\">Gravatar<\/a>.<\/p>\n"
          },
          "link": "http:\/\/localhost:8889\/hello-world\/#comment-1",
          "type": "comment",
          "author_avatar_urls": {
            "24": "http:\/\/1.gravatar.com\/avatar\/d7a973c7dab26985da5f961be7b74480?s=24&d=mm&r=g",
            "48": "http:\/\/1.gravatar.com\/avatar\/d7a973c7dab26985da5f961be7b74480?s=48&d=mm&r=g",
            "96": "http:\/\/1.gravatar.com\/avatar\/d7a973c7dab26985da5f961be7b74480?s=96&d=mm&r=g"
          },
          "_links": {
            "self": [
              {
                "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/comments\/1"
              }
            ],
            "collection": [
              {
                "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/comments"
              }
            ],
            "up": [
              {
                "embeddable": true,
                "post_type": "post",
                "href": "http:\/\/localhost:8889\/wp-json\/wp\/v2\/posts\/1"
              }
            ]
          }
        }
      ]
    ]
  }
}

To fix this, we'll probably need to manually call WP_REST_Response::remove_link for the links that aren't necessary.

#15 @TimothyBlynJacobs
3 months ago

  • Type changed from enhancement to defect (bug)

Reclassifying this as a bug with the aim of landing the fix this week.

#16 follow-up: @TimothyBlynJacobs
3 months ago

  • Milestone changed from 5.7 to 5.8

Going to go ahead and punt this to 5.8. The changes needed are a bit complex and we're coming up on RC1.

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 weeks ago

#22 in reply to: ↑ 16 @chaion07
3 weeks ago

Replying to TimothyBlynJacobs:
Hi @TimothyBlynJacobs do you have this planned and ready for execution anytime soon for 5.8?

Going to go ahead and punt this to 5.8. The changes needed are a bit complex and we're coming up on RC1.

Thank you in advance!

Note: See TracTickets for help on using tickets.