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

fixes issue with terms controller request params 'parent' and 'per_page' being 0 #1581

Closed
Toddses wants to merge 1 commit into from
Closed

fixes issue with terms controller request params 'parent' and 'per_page' being 0 #1581

Toddses wants to merge 1 commit into from

Conversation

@Toddses
Copy link
Contributor

@Toddses Toddses commented Sep 22, 2015

For the /terms/ endpoints, if the parent param is 0 (eg. retrieve all top-level terms), it fails to take into account that no taxonomy with an id of 0 exists.

For the per_page, it tries to figure out the $max_pages by dividing the total items by the per_page parameter. This of course fails because of dividing by 0.

@Toddses Toddses changed the title fixes issue with request params 'parent' and 'per_page' being 0 fixes issue with terms controller request params 'parent' and 'per_page' being 0 Sep 22, 2015
@Toddses
Copy link
Contributor Author

Toddses commented Sep 24, 2015

this may be an issue with the other controllers as well, not just the terms controller.

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

rachelbaker commented Sep 27, 2015

@Toddses Hello, and thanks for submitting this PR. per_page should never be 0. I think the best approach to fixing that bug, is setting a default value (see #1603) so we don't divide by zero if the per_page argument isn't set.

rachelbaker added a commit that referenced this pull request Sep 27, 2015
@rachelbaker
The parent param can be 0 so we can't use the absint callback
bf90520 
See #1581
rachelbaker added a commit that referenced this pull request Sep 27, 2015
@rachelbaker
Merge pull request #1604 from WP-API/refresh-1581
47fd4f1 
Limit Scope and Fix PHPCS for #1581
@rachelbaker rachelbaker mentioned this pull request Sep 27, 2015
Merged
@rachelbaker
Copy link
Member

rachelbaker commented Sep 27, 2015

Merged in #1604

@Toddses
Copy link
Contributor Author

Toddses commented Sep 28, 2015

What about when we want to query for all terms? Passing 0 in the number arg to the get_terms() function is one way. The default for number is to return all terms. However, if the default value is 10 in the terms controller, and we can't pass 0, there's no way to query for all terms.

@Toddses Toddses deleted the terms-controller-fix branch September 28, 2015 14:30
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Toddses @rachelbaker