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

Add Terms Controller Collection Args Details #1603

Merged
rmccue merged 9 commits into from
Sep 28, 2015
Merged

Add Terms Controller Collection Args Details #1603

rmccue merged 9 commits into from
Sep 28, 2015

Conversation

@rachelbaker
Copy link
Member

@rachelbaker rachelbaker commented Sep 27, 2015

  • Add missing collection args details to the Terms Controller
    • fill-in details for the per_page, page, hide_empty, and search args that are already handled in the get_items method
    • add a sanitize_callback to the order arg
    • add a sanitize_callback and add missing valid values to the enum array for the orderby arg
  • Don't hardcode the hide_empty collection arg to being false in get_items method

@rachelbaker
Copy link
Member Author

rachelbaker commented Sep 27, 2015

@WP-API/amigos #reviewmerge

@rachelbaker
Copy link
Member Author

rachelbaker commented Sep 27, 2015

Related #1581

@rachelbaker rachelbaker mentioned this pull request Sep 27, 2015
Closed
@danielbachhuber
Copy link
Member

danielbachhuber commented Sep 27, 2015

If we're exposing hide_empty as an argument now, can we add a test for it?

Also, I think I didn't expose it originally because I wanted to have a conversation about its naming. Should we rename it? Querying posts has with_comments, so it could become with_posts

@rachelbaker
Copy link
Member Author

rachelbaker commented Sep 28, 2015

@danielbachhuber I added a test for hide_empty. I am just trying to fill out our collection args for the documentation, and don't feel strongly about renaming it.

@danielbachhuber
Copy link
Member

danielbachhuber commented Sep 28, 2015

I added a test for hide_empty. I am just trying to fill out our collection args for the documentation

Thanks, makes sense.

don't feel strongly about renaming it.

@rmccue @joehoyle opinions?

@rmccue
Copy link
Member

rmccue commented Sep 28, 2015

Hmm, hide_empty is the more generic term there, so it'd make sense to make it consistent on that instead, but I'm wary of having to rename stuff particularly when it's in WP Query (which everyone knows).

Merging in the meantime, we can continue discussing the change though.

@rmccue rmccue added this to the 2.0 Beta 5 milestone Sep 28, 2015
rmccue added a commit that referenced this pull request Sep 28, 2015
@rmccue
Merge pull request #1603 from WP-API/add-terms-collection-args
3ba6893 
Add Terms Controller Collection Args Details
@rmccue rmccue merged commit 3ba6893 into develop Sep 28, 2015
@rmccue rmccue deleted the add-terms-collection-args branch September 28, 2015 23:16
@dave-green-uk dave-green-uk mentioned this pull request Jun 26, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Documentation Review

Projects

None yet

Milestone

2.0 Beta 6

Development

Successfully merging this pull request may close these issues.

4 participants

@rachelbaker @danielbachhuber @rmccue