WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 2 months ago

#38786 closed defect (bug) (fixed)

register_meta()'s show_in_rest not mapping to specific fields

Reported by: garubi Owned by: rachelbaker
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:

Description

I'm following the directions found on the (future) documentation for the REST API https://github.com/WP-API/wp-api-meta-endpoints/blob/new-meta-new-life/README.md#apispecific-options.

Reading it, looks like we could register any custom field to be exposed via REST API and map it to an API-specific field name.
i.e the field could be "telephone_number" but exposed to the API as "phone".

function register_new_meta() {   
    $args = array(
        //'sanitize_callback' => 'sanitize_my_meta_key',
        //'auth_callback' => 'authorize_my_meta_key',
        'type' => 'string',
        'description' => 'test register_field',
        'single' => true,
    'show_in_rest' => array(
        'name' => 'phone'
    )
    register_meta( 'post', 'telephone_number', $args );   
}

I tested it and in the response to my GET I get the 'phone' field empty, while if I omit the 'name' => 'phone' argument, setting only show_in_rest => true like this:

add_action( 'init', 'register_new_meta');

function register_new_meta() {
     $args = array(
        'type' => 'string',
        'description' => 'Extra information',
        'single' => true,
        'show_in_rest' => true,
    );
    register_meta( 'post', 'telephone_number', $args );   
}

I get the 'wpcf-informazioni' field populated with its content.

So is the "mapping" to the "name" not working (maybe the document I referenced isn't updated), or am I doing something wrong... ?

Attachments (2)

38786.diff (10.9 KB) - added by tharsheblows 5 months ago.
support for custom names in get / update / delete meta
38786.2.diff (11.3 KB) - added by joehoyle 5 months ago.

Download all attachments as: .zip

Change History (20)

#1 @joehoyle
5 months ago

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

#2 @tharsheblows
5 months ago

First pass at a fix. nb: I didn't want the stored meta key names to leak out in error messages if someone used a custom name. (Ie if there's an error message, it should use $name from 'show_in_rest' => array( 'name' => $name ))

@tharsheblows
5 months ago

support for custom names in get / update / delete meta

#3 @garubi
5 months ago

@tharsheblows, @joehoyle just tested the patch on a test WP 4.7-beta3-39232 installation and it works as expected.
Would be great if this fix could go in the 4.7 release.

#4 @garubi
5 months ago

  • Keywords has-patch added

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


5 months ago

#6 @danielbachhuber
5 months ago

  • Keywords has-unit-tests added
  • Milestone changed from Awaiting Review to 4.7

#7 @danielbachhuber
5 months ago

While 38786.diff looks a bit hairy because of the variable renaming, it looks good to me.

@joehoyle
5 months ago

#8 @joehoyle
5 months ago

I added a fresh patch with the implementation to be made a lot more like the Settings endpoint, whereby the arrays are keyed by meta_key, and $args['name'] is the "visual" name in the REST API. I took the tests from @tharsheblows's patch.

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


5 months ago

#10 @rmccue
5 months ago

  • Owner changed from joehoyle to rmccue
  • Status changed from assigned to reviewing

#11 @rmccue
5 months ago

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

In 39328:

REST API: Correctly map meta keys to field names.

This accidentally assumed $name was the same as $meta_key, which ruined the whole point of $name.

Props tharsheblows, joehoyle.
Fixes #38786.

#12 @jnylen0
4 months ago

@rmccue this seems like a good one to get into either 4.7.1 or 4.7.2, do you agree?

#13 @rmccue
4 months ago

  • Keywords fixed-major added
  • Milestone changed from 4.7 to 4.7.2

@jnylen0 Too late for 4.7.1 I think, but I agree, given the new release cycle.

#14 @jnylen0
4 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

👍🏻 Reopening so that someone can port this to the 4.7 branch.

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


3 months ago

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


3 months ago

#17 @peterwilsoncc
3 months ago

  • Owner changed from rmccue to rachelbaker
  • Status changed from reopened to assigned

Reassigning this to you @rachelbaker to review for backporting.

#18 @rachelbaker
2 months ago

  • Keywords fixed-major removed
  • Milestone changed from 4.7.3 to 4.7
  • Resolution set to fixed
  • Status changed from assigned to closed

@jnylen0 @rmccue
Looks like r39328 is already in the 4.7 branch (see: https://core.trac.wordpress.org/browser/branches/4.7/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php?annotate=blame#L68) because it came along when 4.7 was branched with r39356.

Closing this with the correct milestone of 4.7.

Note: See TracTickets for help on using tickets.