&_embed only embedding first 10 categories

[manyourisms]

Normally our posts don't have more than 10 categories, but I did notice that when adding more than 10 to a post, a query using &_embed still only embedding the first 10. After searching through the class-wp-rest-server.php code, it doesn't seem like that should be the case, but alas! I'm too unfamiliar with how it is building the links for a post behind the scenes, but surely it must be happening there if I was to guess.

Hitting the category link href at wp-json/wp/v2/categories?post=<postId> also only returns 10 categories.

[manyourisms]

Response showing it only coming back with 10 categories

[manyourisms]

Edit post page showing much more than 10 categories for the post.

[kadamwhite]

Hitting the category link at wp-json/wp/v2/categories?post=<postId> and receiving 10 responses is expected; collections maintain standard pagination behavior when using query parameters like post=, so you can see the next page of responses with &page=2.

Only permitting up to 10 records to be embedded, however, is unexpected. Good catch! The origin of the issue appears to be that we use the REST API internally to embed links collections, and these requests obey the REST API's standard 10-post collection response limit. I'd definitely consider changing this to work in a more predictable way.

[manyourisms]

Wunderbar! Thanks for the clarification on the categories endpoint, sorry to have missed that.

Replying to kadamwhite:

Hitting the category link at wp-json/wp/v2/categories?post=<postId> and receiving 10 responses is expected; collections maintain standard pagination behavior when using query parameters like post=, so you can see the next page of responses with &page=2.

Only permitting up to 10 records to be embedded, however, is unexpected. Good catch! The origin of the issue appears to be that we use the REST API internally to embed links collections, and these requests obey the REST API's standard 10-post collection response limit. I'd definitely consider changing this to work in a more predictable way.

[rmccue]

@kadamwhite What sort of changes did you have in mind for making this more predictable?

[manyourisms]

Replying to rmccue:

@kadamwhite What sort of changes did you have in mind for making this more predictable?

I would think the page parameter would apply only to the main resource you are requesting, IE posts in this case. I would expect all embeds for those 10 resources to come back (unless there was an additional paging param for the embeds for some reason).

[kadamwhite]

I would also feel like we'd get back all embeds. If I have a resource that includes a link to that resource's categories, I would expect to be able to follow it and get a comprehensive list.

This obviously raises the question of how to handle situations (rare, but they happen) where one post may have 100+ tags or categories. I think to start we should hard-cap in that edge-case, but otherwise return a full list when embedded.

[TimothyBlynJacobs]

A naive solution to this would be to always append per_page=100 in WP_REST_Server::embed_links(). However that wouldn't be ideal for single routes, and we have no guarantee that collection routes actually implement their per_page cap at 100.

What we could do is inside of ::embed_links() use the new ::match_request_to_handler() and ::respond_to_request() methods to reimplement ::dispatch() but to first look if a per_page arg is registered and if so, find it's maximum value and apply it to the request.

@lpawlik are you interested in this one?

[lpawlik]

@TimothyBlynJacobs sure thing, I'll add it to my list 👍

Trac ticket: https://core.trac.wordpress.org/ticket/43439

Allow _embed to include all embedded object such as taxonomies, comments etc.

@TimothyBJacobs @spacedmonkey this is an initial commit for https://core.trac.wordpress.org/ticket/43439. It seems to be solving reported issue however, I am concerned about the fact that it's using \WP_REST_Server::match_request_to_handler that is modifying the state of passed request object, and the same method is called later via dispatch method. Is it our concern that it's called second time? Do you have any other ideas to address this issue?

[TimothyBlynJacobs]

Hey @lpawlik! Just following up on our ​conversation in Slack about how we could alleviate the duplicate matching issue.

[lpawlik]

Hey @TimothyBlynJacobs - thanks for the ping. I'll be working on this, later this week and I hope to have something ready for the review early next week :)

Follow on from ​https://github.com/WordPress/wordpress-develop/pull/672

Trac ticket: https://core.trac.wordpress.org/ticket/43439

[spacedmonkey]

In 57623:

REST API: Set maximum 'per_page' for embedded REST API requests.

This enhancement refines the REST API server to automatically establish the maximum 'per_page' value for embedded objects, adhering to the endpoint's schema when not explicitly defined in the request. This adjustment elevates the limit from the default of 10 items to 100 items, significantly improving the likelihood of receiving the complete dataset of embedded objects.

Props manyourisms, lpawlik, spacedmonkey, kadamwhite, TimothyBlynJacobs.
Fixes #43439.

Committed.

[Mamaduka]

I believe r57623 caused a regression for the navigation-fallback endpoint. The embedded object no longer returns the `raw` property values.

The regression was missed by the test_embedded_navigation_post_contains_required_fields test since it doesn't match actual usage. It makes separate requests with the embed context instead of using the _embed=true parameter.

More details in Gutenberg issue - ​https://github.com/WordPress/gutenberg/issues/58987.

[Mamaduka]

I am reopening the ticket since regression needs to be resolved before we proceed with a committed solution.

TBD

Trac ticket: https://core.trac.wordpress.org/ticket/43439

@TimothyBJacobs @spacedmonkey would you be able to help validate the fix here. This is a regression that happened during the 6.5 cycle, therefor, it would be good to land this fix on the same release.

This fix looks good to me as well.

I'd love if we could have unit tests that are covering this specifically as part of the WP_REST_Server tests, instead of the incidental coverage we are getting as part of the navigation controller. But I don't think that needs to block this from getting merged considering the timing.

[swissspidy]

In 57659:

REST API: Pass correct context to embedded items.

Fixes a regression introduced in [57623] where navigation embed items were missing raw property values.

Props mamaduka, swissspidy, youknowriad, timothyblynjacobs.
Fixes #43439.

Committed in https://core.trac.wordpress.org/changeset/57659

Committed in https://core.trac.wordpress.org/changeset/57659