WP-API / widgets-endpoints

Experimental WordPress REST API Widgets endpoints.
7 stars 1 forks source link

/widgets/:id or /widgets/:id_base? #4

Open NateWr opened 8 years ago

NateWr commented 8 years ago

The get_widget_endpoint branch implements (mostly) the base /widgets endpoint.

I was surprised to see the next route was /widgets/:id_base. I would expect that the endpoint would work like posts, where you retrieved a specific widget with /widgets/:id. Wouldn't that make more sense? We could implement id_base as a filter on the /widgets base just like sidebar.

(I appreciate that I wrote the initial endpoint registration. Just want to double-check that there wasn't some decisions about /widgets/:id_base that I can't remember).

westonruter commented 8 years ago

The /widgets/:id_base request is another way to say /widgets/:type, and it would return all widgets of a given type. Widget IDs are different than post IDs in that the numeric part is actually just an array index for an array of instances of a given type. In other words, two different widgets can have the same numeric part, e.g. search-2 and text-2. So we can't identify a widget simply by the 2. In some ways, this situation is similar to how terms IDs were ambiguous prior to the most recent releases. So this is why I suggest /widgets/:id_base/:number to address a single widget, and /widgets/:id_base to reference the collection of widgets of a given type (which could also be referenced via /widgets?filter[type]=:id_base).

Ideally widgets would be eliminated from being stored in options altogether and they would have a full-on widget_instance post type (as implemented in feature plugin). When this happens, widget instances could indeed be referenced by a single numeric ID since this would be a globally auto-incremented post ID value, such as /widgets/123.

NateWr commented 8 years ago

I thought that we would be "short-circuiting" this issue by using [id_base]-[array_index_key] as the id. I guess I should have used /widgets/:slug to describe what I intended, because each instance has a unique slug (eg - '/widgets/recent-posts-2`).

I am struggling to find the use-case for the /widgets/:typeendpoint, as I don't know when or why I would want a collection of a specific type. I understand that's how the instances are stored in the database. But it's not how the data is used anywhere in WordPress, as far as I am aware.

It seems more logical to me that :type would be a filter I could pass to the /widgets endpoint rather than an endpoint on it's own. And being able to query a single item with /widgets/<slug> seems closer to the pattern followed by other endpoints.

With the current patter, /widgets/:id_base/:number, I'd have to split the widget id string in the client before making the request, effectively making the id not usable in the client without some extra work.

Edit - sorry hit enter before finishing. Anyway, I will go ahead and issue a PR for the get_widget_endpoint branch for now. It implements the base /widgets and you'll see using the [id_base]-[array_index_key] as a slug seems like a sensible approach. But whatever is decided is fine. I just thought it seemed a bit at odds to me to have a /widgets/type/number endpoint.

westonruter commented 8 years ago

@NateWr yeah, I think you're right. We don't need /widgets/:type after all, especially if #35669 gets implemented and widgets get stored in custom post type instead of options. Once that happens, a widget instances would be addressable by a single integer ID just like any other post, and /widgets/:id would work. In the mean time, we could still implement this but the format of :id would allow for a widget ID as it exists currently in Core, as id_base followed by number (/^(?P<id_base>.+)-(?P<number>\d+)/$). If the widget_instance post is added to Core, then this pattern can be changed to include the integer post ID: (/^(?:(?P<post_id>\d+)|(?P<id_base>.+)-(?P<number>\d+))/$). When a non-post_id request is made, the old widget ID can be looked up by post_name, assuming that this is where the old widget IDs get stored.

In both cases, listing all widgets of a given type can be done via /widgets?filter[type]=:id_base instead of via the /widgets/:type which I initially proposed.

:+1:

NateWr commented 8 years ago

I think I've set this up as you suggest now. See WP-API/menus-endpoints#17