Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Exclude unsupported params in all controllers (extends #2770)#2804

Merged
joehoyle merged 4 commits intoexclude-unsupported-paramsfrom
exclude-unsupported-params-more-broadly
Oct 14, 2016
Merged

Exclude unsupported params in all controllers (extends #2770)#2804
joehoyle merged 4 commits intoexclude-unsupported-paramsfrom
exclude-unsupported-params-more-broadly

Conversation

@kadamwhite
Copy link
Contributor

@kadamwhite kadamwhite commented Oct 11, 2016

This is a PR against #2770, not develop , and extends the approach taken there to apply to all other controllers in which get_items assigns query properties from requests.

  • Attachments (No get_items defined, extends WP_REST_Posts_Controller)
  • Comments
  • Post Statuses (No change, get_items assigns no values from $request)
  • Post Types (Needed? Only "context" is used)
  • Revisions (Needed? Only "parent" is used, and it's part of the URL string)
  • Taxonomies
  • Terms
  • Users

@kadamwhite kadamwhite changed the title (WIP) Exclude unsupported params more broadly Exclude unsupported params in all controllers (extends #2770) Oct 11, 2016
@kadamwhite
Copy link
Contributor Author

@rachelbaker This now includes almost all properties across all controllers. The duplication is evident as discussed in #2770 but I think I'm more in favor of duplication now and a mindful abstraction later, than prematurely introducing an insufficient abstraction now.

@kadamwhite
Copy link
Contributor Author

Open questions:

  1. Should context be checked in this way, or are we able to treat it as "global"?
  2. If a parameter is part of the route URL, do we need to check it in this manner?

These questions determine whether post types & revisions need similar treatment

// values are accepted as-passed, and their internal WP_Query parameter
// name equivalents (some are the same). Only values which are also
// present in $registered will be set.
$parameter_mappings = array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about including this parameter mapping in the collection param definition itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean in the schema? Certainly not opposed, but I guess I'm not sure how that would look, given what's there now.

@joehoyle
Copy link
Member

I think it's ok for this to stay where it is in get_items rather than moving to get_collection params, this is a procedural / controller thing, and get_collection_params is a bit more pure in what it does.

@joehoyle
Copy link
Member

Revisions (Needed? Only "parent" is used, and it's part of the URL string)

I don't think so no.

Post Types (Needed? Only "context" is used)

Like #2770 I don't think we need to solve context

@joehoyle joehoyle merged commit 3d08e90 into exclude-unsupported-params Oct 14, 2016
@joehoyle joehoyle deleted the exclude-unsupported-params-more-broadly branch October 14, 2016 14:50
@kadamwhite kadamwhite restored the exclude-unsupported-params-more-broadly branch October 15, 2016 17:15
@kadamwhite kadamwhite deleted the exclude-unsupported-params-more-broadly branch October 15, 2016 17:18
BE-Webdesign added a commit that referenced this pull request Oct 15, 2016
…broadly

Exclude unsupported params more broadly (was #2804)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants