WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#51131 closed enhancement (fixed)

Remove constraint that search handlers must return integer ids

Reported by: TimothyBlynJacobs Owned by: TimothyBlynJacobs
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.0
Component: REST API Keywords: needs-unit-tests good-first-bug has-patch
Focuses: Cc:

Description

The WP_REST_Search_Controller currently enforces that the id property returned by WP_REST_Search_Handler::search_items is an integer. AFAICT this constraint isn't required. But it does mean that items that don't have integer ids can't be searched.

A string id isn't uncommon, for example uuids or hashids. But this also causes a particular issue in Gutenberg where we want to expose post formats via the API. Ideally the formats would be referred to by their slug, but currently Gutenberg has to create fake ids to satisfy the search controller.

Attachments (2)

51131.diff (514 bytes) - added by stoyangeorgiev 2 months ago.
Remove absint from the result ids
51131.1.diff (1.1 KB) - added by stoyangeorgiev 2 months ago.
Changed param type and schema type

Download all attachments as: .zip

Change History (11)

@stoyangeorgiev
2 months ago

Remove absint from the result ids

#1 @stoyangeorgiev
2 months ago

  • Keywords has-patch added; needs-patch removed

I have uploaded a patch where I have removed the absint for the result ids.

#2 @TimothyBlynJacobs
2 months ago

Thanks for working on a patch @stoyangeorgiev!

There are a couple of other changes we need to make too. The type hints for the $id parameter should be updated to int|string. We'll also need to update the id schema in WP_REST_Search_Controller to be an array of array( 'integer', 'string' ) to reflect that it can be either of those types.

@stoyangeorgiev
2 months ago

Changed param type and schema type

#3 @stoyangeorgiev
2 months ago

Hello there @TimothyBlynJacobs,

I have done the changes, you mentioned above. Changed the parameter hint and also added the array of types to the schema.

#4 @TimothyBlynJacobs
2 months ago

Thanks @stoyangeorgiev!

We also need to do similar doc block updates for WP_REST_Search_Handler.

Could you also open this as a Pull Request? That way we can automatically get tests and linting to run.

https://github.com/WordPress/wordpress-develop

This ticket was mentioned in PR #564 on WordPress/wordpress-develop by stoyan-g.


2 months ago

Removed the constraints in WP_REST_Search_Controller, updated id schema, and updated the docs. Edited the docs for WP_REST_Search_Handler.

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

#6 @stoyangeorgiev
2 months ago

Hello there @TimothyBlynJacobs,

I have opened a Pull Request and all checks have passed successfully.

#7 @TimothyBlynJacobs
8 weeks ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 49088:

REST API: Allow for string ids in the search controller.

Previously, the search controller infrastructure required that the id property was an integer. This prevents data models that use a string id from utilizing the search infrastructure.

This commit lifts the restraint that search handlers return integer ids. This will allow for the Post Formats search handler coming in 5.6 to use slugs instead of creating fake ids.

Props stoyangeorgiev.
Fixes #51131.

#8 @prbot
8 weeks ago

TimothyBJacobs commented on PR #564:

Fixed in a3eb0d8. Thanks again or the patch @stoyan-g!

#9 @SergeyBiryukov
8 weeks ago

In 49089:

Docs: Add @since notes for the $id parameter of REST API search controller accepting a string.

Follow-up to [49088].

See #51131.

Note: See TracTickets for help on using tickets.