Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Add Terms Controller Collection Args Details - #1603

#1603
Merged
rmccue merged 9 commits into
developWP-API/WP-API:developfrom
add-terms-collection-argsWP-API/WP-API:add-terms-collection-argsCopy head branch name to clipboard
Sep 28, 2015
Merged

Add Terms Controller Collection Args Details#1603
rmccue merged 9 commits into
developWP-API/WP-API:developfrom
add-terms-collection-argsWP-API/WP-API:add-terms-collection-argsCopy head branch name to clipboard

Conversation

@rachelbaker

@rachelbaker rachelbaker commented Sep 27, 2015

Copy link
Copy Markdown
Member
  • 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

rachelbaker commented Sep 27, 2015

Copy link
Copy Markdown
Member Author

@WP-API/amigos #reviewmerge

@rachelbaker

rachelbaker commented Sep 27, 2015

Copy link
Copy Markdown
Member Author

Related #1581

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

danielbachhuber commented Sep 27, 2015

Copy link
Copy Markdown
Member

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

rachelbaker commented Sep 28, 2015

Copy link
Copy Markdown
Member Author

@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

danielbachhuber commented Sep 28, 2015

Copy link
Copy Markdown
Member

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

rmccue commented Sep 28, 2015

Copy link
Copy Markdown
Member

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.

3 participants

@rachelbaker @danielbachhuber @rmccue
Morty Proxy This is a proxified and sanitized view of the page, visit original site.