Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Paginator item counts for empty result sets are inaccurate #6808

Closed
davidomelettes opened this issue Oct 27, 2014 · 6 comments
Closed

Paginator item counts for empty result sets are inaccurate #6808

davidomelettes opened this issue Oct 27, 2014 · 6 comments
Assignees
Milestone

Comments

@davidomelettes
Copy link

The firstItemNumber property of the pages collection returned by Paginator/Paginator::_createPages() is 1, even if there are no items to be paginated. I feel it would be more accurate if this property had a value of 0 in this case.

@gianarb
Copy link
Contributor

gianarb commented Nov 12, 2014

mm I don't know..:bomb: But page 0 not exist, exist an empty page 1..

@Ocramius
Copy link
Member

@davidomelettes can you clarify on the requirements? Why is that so? Did you check existing test cases?

Consider that the paginator is very frontend-oriented, and from a user perspective, "0" is not a page number.

@davidomelettes
Copy link
Author

The use case which made me consider it a bug was when I used the paginator to display item numbers, rather than page numbers, on my pagination controls. e.g. "31 - 60 of 61 Items" instead of "Page 2 of 3". In the View, if your paginator has no items, the same display will read "1 - 0 of 0 items".

It's a trivial issue to work around, but it did make me wonder what the most "correct" values for the paginator page properties should be, with an empty result set. I did check the tests and there is only a single test with an empty result set (testThrowsExceptionWhenCollectionIsEmpty). I created #6809 to address this.

@davidomelettes
Copy link
Author

Actually, if 0 is no more accurate than 1 for firstItemNumber when the result set is empty, perhaps a better solution is simply to not set the properties at all? That would be more in line with how ->previous and ->next work.

It depends on how you might answer the question "what is the current page number of a paginator with no pages?"

@Ocramius
Copy link
Member

You'd still start from the first empty page IMO.

@Ocramius Ocramius added this to the 2.4.0 milestone Dec 5, 2014
Ocramius added a commit that referenced this issue Dec 5, 2014
Ocramius added a commit that referenced this issue Dec 5, 2014
Ocramius added a commit that referenced this issue Dec 5, 2014
@Ocramius
Copy link
Member

Ocramius commented Dec 5, 2014

Handled in #6809

@Ocramius Ocramius closed this as completed Dec 5, 2014
gianarb pushed a commit to zendframework/zend-paginator that referenced this issue May 15, 2015
…dding missing `@group` annotations for newly introduced test cases
gianarb pushed a commit to zendframework/zend-paginator that referenced this issue May 15, 2015
…void multiple internal calls to methods when values are already cached in `$pages`
gianarb pushed a commit to zendframework/zend-paginator that referenced this issue May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants