This repository has been archived by the owner. It is now read-only.

Declare post collection params so they can be exposed #1302

Closed
wants to merge 5 commits into
base: develop
from

Conversation

Projects
None yet
4 participants
@danielbachhuber
Copy link
Member

danielbachhuber commented May 26, 2015

See #924, #555

@@ -35,11 +35,12 @@ public function register_routes() {
}

This comment has been minimized.

@danielbachhuber

danielbachhuber May 26, 2015

Author Member

@WP-API/amigos What should I do with legacy get_allowed_query_vars() ?

This comment has been minimized.

@rachelbaker

rachelbaker May 27, 2015

Member

The API has always supported the public query_vars of WP_Query, included a few others, and provided a filter to add support for additional vars. There are some public query vars that are pretty pointless (example withcomments) but trying to normalize all these params sounds like a headache to me.

@danielbachhuber

This comment has been minimized.

Copy link
Member Author

danielbachhuber commented May 27, 2015

@WP-API/amigos I'm not 100% sure what we decided to do with WP_Query-style arguments (e.g. also supporting posts_per_page in addition to page).

Given we don't support WP_User_Query-style arguments et al, and trying to magically support them would be a pretty massive hack, I think we should drop them for now.

@rachelbaker

This comment has been minimized.

Copy link
Member

rachelbaker commented May 27, 2015

I'm not 100% sure what we decided to do with WP_Query-style arguments

I don't know if we discussed this, but it sounds like we should.

@rachelbaker

This comment has been minimized.

Copy link
Member

rachelbaker commented May 27, 2015

Looks like this needs some more discussion. @rmccue @joehoyle thoughts?

@joehoyle

This comment has been minimized.

Copy link
Contributor

joehoyle commented May 27, 2015

When we discussed this at LoopConf, I was under the impression we settled on the following:

  1. Create new query params, for example page, per_page, slug etc.
  2. Put all current WP_Query style WordPress query names under a filter or wp_query argument. So these would be equivalent:

GET /wp/v2/posts?per_page=10

GET /wp/v2/posts?wp_query[posts_per_page]=10

That way, we can support WordPress devs thinking the a WP_Query mindset, and give them the ability to construct more complex queries (though ofcourse of the public query vars that we currently use). Then have a clean top level args that will be consistent across all collections.

@danielbachhuber

This comment has been minimized.

Copy link
Member Author

danielbachhuber commented May 27, 2015

GET /wp/v2/posts?wp_query[posts_per_page]=10

I dig this — seems like a reasonable compromise.

@rmccue

This comment has been minimized.

Copy link
Member

rmccue commented May 27, 2015

If we're going to do it, keep the filter name so it's consistent with v1. That way, v2 = v1 + additional better named querying, rather than completely different.

@danielbachhuber

This comment has been minimized.

Copy link
Member Author

danielbachhuber commented May 27, 2015

I dig filter

@rmccue

This comment has been minimized.

Copy link
Member

rmccue commented Jun 17, 2015

Marking as Needs Refresh, since filter needs to be added in.

@danielbachhuber

This comment has been minimized.

Copy link
Member Author

danielbachhuber commented Dec 7, 2015

Closing in favor of #1793

@danielbachhuber danielbachhuber deleted the 924-post-collection-query branch Dec 7, 2015

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.