Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#56546 closed enhancement (fixed)

Permit queries by ID in /search REST endpoint

Reported by: kadamwhite's profile kadamwhite Owned by: kadamwhite's profile kadamwhite
Milestone: 6.1 Priority: normal
Severity: normal Version: 6.1
Component: REST API Keywords: has-patch commit has-unit-tests needs-dev-note
Focuses: rest-api Cc:

Description

At present there is no way to retrieve a resource if you only know its object type, but not its subtype. Because terms and posts are segregated by term type and post type, if I know I have a custom post type object of ID 12, I cannot get any further information about it from the REST API without somehow either determining its object subtype, or trying all custom post type endpoints in turn.

The /search controller permits searching across subtypes, but does not support the "include" and "exclude" parameters common to individual post type endpoints. We should support these parameters on the search/ endpoint, so that (when relevant) there is a way to query for results across subtypes with or without a specific list of IDs.

Change History (8)

This ticket was mentioned in PR #3228 on WordPress/wordpress-develop by kadamwhite.


3 years ago
#1

  • Keywords has-patch added

Permits retrieving a resource of unknown subtype, for example retrieving a custom post type object when you only know its ID with the "include" parameter. Alternatively, allows you to search without including results from a known list of target IDs with the "exclude" parameter.

Currently introduces include/exclude for post and term search controllers only.

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

#2 @kadamwhite
3 years ago

Note on participants: this approach was developed with input from @goldenapples.

TimothyBJacobs commented on PR #3228:


3 years ago
#3

One thing about supporting include and exclude means that it prevents us from ever having a search controller that would allow searching across multiple object types ( terms and posts for instance ) in the same search request. I think it us unlikely we'll ever be able to support this, however. So I don't think that unlikely possibility should block landing this feature.

We could mark include and exclude as constants on WP_REST_Search_Controller like the WP_REST_Search_Controller::PROP_SUBTYPE constant works. Then, we'd retrieve the value from the request object like $request[https://github.com/WordPress/wordpress-develop/blob/da03cf1c6b3ab71ea7e56f4a050d87c41150726e/src/wp-includes/rest-api/search/class-wp-rest-post-search-handler.php#L55 WP_REST_Search_Controller::PROP_INCLUDE ], similarly to how we do [here].

We aren't really consistent about this throughout the search handlers already, for instance the search, per_page and page query parameters are not used as props. So I think it's fine to omit introducing a constant for this. The REST API already relies on "magic strings" for all request parameters.

Tests look great to me.

#4 @kadamwhite
3 years ago

  • Owner set to kadamwhite
  • Status changed from new to accepted

One thing about supporting include and exclude means that it prevents us from ever having a search controller that would allow searching across multiple object types ( terms and posts for instance ) in the same search request. I think it us unlikely we'll ever be able to support this, however. So I don't think that unlikely possibility should block landing this feature.

While I concur that building such a controller—at least in core—is unlikely, I'm not sure I entirely agree with your conclusion. If we did end up with such a controller, as a consumer of it I would expect that if I pass include=12, and somehow have both a post with ID 12 and a term with ID 12 in my available results set, I would get both of those back in the list and would have to differentiate them by subtype. The reason we don't have any controller that behaves this way in core is because that would be confusing and deviate from how we expect a REST API to behave, but to me it would be "correct" in terms of what the user was asking for.

All that said, you're right, we have no such controller and it seems highly unlikely we'd add one. :)

We aren't really consistent about [how we reference request properties] throughout the search handlers already

Good point. I missed that inconsistently when this controller went in, and definitely agree it's confusing. It's not clear why the authors of this controller decided to abstract the param string names into constants when it's not done elsewhere in the API.

To maintain as much cross-controller consistency as possible, I agree we should stick to the way we do this in other controllers, which uses the inline string literals.

#5 @TimothyBlynJacobs
3 years ago

  • Keywords commit has-unit-tests added

Awesome, marking as ready for commit.

#6 @TimothyBlynJacobs
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54123:

REST API: Add support for searching resources by id.

This brings support for the include and exclude collection parameters to the Search Controller. This can be used to find an item by id when it's subtype is unknown.

Props kadamwhite.
Fixes #56546.

TimothyBJacobs commented on PR #3228:


3 years ago
#7

Committed in a400e99225abe191fd02f42ed28704441d6c91ea.

#8 @TimothyBlynJacobs
3 years ago

  • Keywords needs-dev-note added
  • Milestone changed from Awaiting Review to 6.1
Note: See TracTickets for help on using tickets.