Skip to content
This repository has been archived by the owner on Apr 4, 2020. It is now read-only.

Use meta_key for meta endpoint rather than id #2

Open
tharsheblows opened this issue Feb 3, 2016 · 10 comments
Open

Use meta_key for meta endpoint rather than id #2

tharsheblows opened this issue Feb 3, 2016 · 10 comments

Comments

@tharsheblows
Copy link

I've been working through adding in public contexts and using the additional bits in the discussion here: [https://core.trac.wordpress.org/ticket/35658](Provide additional data for registered meta through register_meta) and realised that the way I've done it, it would make more sense to have the individual meta endpoint be the meta_key rather than the meta ID as everything goes as meta_key or meta_key, meta_value. Ie the ID isn't used in update_{$object}_meta or delete_{$object}_meta.

Thought I'd ask about it now before continuing down this path. :)

edit: here's a sample response -- it's not complete and I'm not sure about some things but I hope shows where it's headed

[
  {
    "key": "test_for_hack_day",
    "value": [
      "This is a test for the hack day, hello hello!",
      "The second entry for test_for_hack_day meta key",
      "This is the third"
    ],
    "type": "string",
    "description": "This is the test for hack day description.",
    "_links": {
      "self": [
        {
          "href": "http://src.wordpress-develop.dev/wp-json/wp/v2/posts/10770/test_for_hack_day"
        }
      ],
      "collection": [
        {
          "href": "http://src.wordpress-develop.dev/wp-json/wp/v2/posts/10770/meta"
        }
      ],
      "about": [
        {
          "embeddable": true,
          "href": "http://src.wordpress-develop.dev/wp-json/wp/v2/posts/10770"
        }
      ]
    }
  }
]
@kjbenk
Copy link
Contributor

kjbenk commented Feb 22, 2016

I agree that we should add functionality to enable this endpoint .../wp-json/wp/v2/posts/1/meta?key=test_key.

Although I there could be some security risk that I am just not seeing but I think this would make it a lot easier to use since you will not have to call .../wp-json/wp/v2/posts/1/meta just to get all of the custom fields registered to a post.

@starfishmod
Copy link

I agree with this as to get the right meta data for a user would require 2 calls.
One to get the list, which then I have to go through and get the correct id, and then another to get the actual data?

In fact, is id kind of useless?
Again id is not used in the wp calls update_user_meta, get_user_meta etc

@rmccue
Copy link
Member

rmccue commented May 5, 2016

The reason we picked ID in the first place is because the combination of parent_id, key is not unique. WP allows multiple values with the same key on a single item (post/user/term).

@starfishmod
Copy link

@rmccue thanks for the info! However if the path is /users/{userid}/meta/{id/key}
then you know what type of key it's for because the paths are different. In fact this is used in get_item method using $this->parent_type.

Are you saying that in WP I can have 2 entries under user for the key "abc"? If so then that make sense why you are using id...

@rmccue
Copy link
Member

rmccue commented May 5, 2016

Are you saying that in WP I can have 2 entries under user for the key "abc"? If so then that make sense why you are using id...

That's correct, yeah. get_post_meta() takes a third parameter called $single that gives you back one result, but you can also get back the array of items in it. This is why core also has get_metadata_by_mid(), update_metadata_by_mid() and delete_metadata_by_mid().

@tharsheblows
Copy link
Author

tharsheblows commented May 5, 2016

My thinking was that it would be good to be able to use eg update_post_meta which uses post_id, meta_key, meta_value at its most granular as those are useful functions. But later I realised to replicate those custom fields in the edit screen in wp-admin, you need to be able to use delete_metadata_by_mid( 'post' , $mid ) as well. Those two aren't mutually exclusive.

For the update / delete meta by ID endpoint, you could have eg /wp-json/wp/v2/posts/meta/id route or /wp-json/wp/v2/meta/posts/i] (I am not sure which makes more sense), similar-ish to categories. Then update_post_meta(10770, 'meta_key', 'update value') would use /wp-json/wp/v2/posts/10770/meta/meta_key

@rmccue
Copy link
Member

rmccue commented May 5, 2016

The issue is with updating/deleting meta. Does DELETE /posts/42/meta/some_key mean delete all meta values with that key, or just one? What does GET return if there's multiple values?

@tharsheblows
Copy link
Author

tharsheblows commented May 5, 2016

It works just like delete_post_meta so yes, it deletes all of the keys unless you've targeted one value like in delete_post_meta( 10770, 'some_key', 'some_value' ) -- then it just deletes all keys with that value.

GET /posts/10770/meta/some_key returns an array like in get_post_meta( 10770, 'some_key' ) if there is more than one value.

So I guess I think of the /posts/10770/meta/some_key route as doing all the update_post_meta, delete_post_meta things and the /wp-json/wp/v2/posts/meta/id route as doing update_metadata_by_mid() and delete_metadata_by_mid()

@starfishmod
Copy link

I agree with @tharsheblows - when using keys it should work like the internal functions do.

@tharsheblows
Copy link
Author

Related is this: sc0ttkclark/wordpress-fields-api#39 Currently the Fields API is using register_rest_field for meta() (I think?) which makes eg update_metadata() easy but update_metadata_by_mid() not so much.

I don't know if eg get_metadata_by_mid() is ever contextless in real life, ie I don't know the object id of the metadata I want to delete. If I always know the object id, then the ID endpoints on the individual objects make sense. Otherwise, to use delete_metadata_by_mid( 'post', 42 ), you'd need a /wp-json/wp/v2/meta/posts/42 route.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants