WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#39959 assigned defect (bug)

Clarify parameters for callbacks for `register_rest_field()`

Reported by: flixos90 Owned by: TimothyBlynJacobs
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords:
Focuses: Cc:
PR Number:

Description

There is an issue going on with register_rest_field() in combination with the WP_REST_Meta_Fields classes: The method WP_REST_Meta_Fields::register_field() uses callbacks that accept an object ID only. However, the REST controller methods that handle the additional fields always pass an object to these callbacks.

Working with metadata via the REST API is still possible now, since the WP_REST_Meta_Fields::register_field() method is never used in Core, instead metadata is handled manually by each controller. So it is now a minor issue, but I think we should figure out what kind of parameters the register_rest_field() callbacks can accept (I could not find anything clear in the documentation) and then actually use WP_REST_Meta_Fields::register_field(), as I don't think there's a point in handling meta manually if we have this method already available in a more abstracted way.

Furthermore, this inconsistency gets more critical once developers start writing their own endpoints for custom object types that use their own metadata table. One might assume that the WP_REST_Meta_Fields base class can be leveraged to handle meta automatically, but because of the inconsistencies now, doing this would likely cause fatal errors and confusion.

Change History (10)

#1 @flixos90
3 years ago

  • Version set to 4.7

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


3 years ago

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


3 years ago

#4 follow-up: @flixos90
3 years ago

To give a precise example: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php#L432 passes an object to the update_callback passed to register_rest_field(), but https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L124 which is passed to register_rest_field() as an update_callback accepts an object ID only. So which of the two is right? Should register_rest_field() callbacks accept an object or an ID?

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


3 years ago

#6 @kadamwhite
3 years ago

My sense is that they should be allowed to accept objects, for consistency with what we're doing now; but other parts of WP generally pass IDs. @rmccue Can you speak to why this class method is not being leveraged by the native endpoints?

#7 in reply to: ↑ 4 @jnylen0
3 years ago

Replying to flixos90:

So which of the two is right? Should register_rest_field() callbacks accept an object or an ID?

They should accept an object. See the first code example at https://developer.wordpress.org/rest-api/extending-the-rest-api/modifying-responses/.

It sounds like the unused WP_REST_Meta_Fields::register_field() method was never tested, and it should be updated to accept an object. Also, we need to improve the register_rest_field documentation to indicate the types/signatures of the values accepted by its $args argument.

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


19 months ago

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


3 months ago

#10 @TimothyBlynJacobs
3 months ago

  • Owner set to TimothyBlynJacobs
  • Status changed from new to assigned

Going to take a look at this and how it relates to #44432.

Note: See TracTickets for help on using tickets.