ZF-5376: Allow Zend_Paginator to Accept An Infinite Item Count Per Page

Description

It would be nice if Zend_Paginator supported an infinite item count per page by passing in a zero (0) as the argument to the setItemCountPerPage method. This would be useful when one wants allow the user to select the item count per page, e.g. "show 25, 50, 100, All items per page". Currently passing a negative one (-1) provides this functionality but is not an intended feature according to this email exchange with Matthew Ratzloff:

On Mon, Dec 29, 2008 at 5:18 PM, Matthew Ratzloff matt@builtfromsource.com wrote:

Hi Bradley, Sorry about the confusion, I misread your requirement. The -1 trick is in fact not an intended feature (if it were, it would be 0, to be in line with typical PHP API), but if it works, use it for now. Whatever fix will just test for < 1, so your code should be forward-compatible. I highly encourage you to file an enhancement ticket so that this can be implemented properly. If you do, please copy and paste this e-mail exchange as a comment into the ticket. Thanks, -Matt

On Mon, Dec 29, 2008 at 11:56 AM, Bradley Holt bradley.holt@foundline.com wrote: >

Matt,

Taking a closer look at the documentation, the "All" scrolling style "returns every page." I'm not trying to show every page in the paginator control, I'm trying to have an infinite item count per page (which, of course, will make it so I have only one page of items). Like I said, passing a negative one (-1) to the setItemCountPerPage method does the job - I just don't know if this is an intended feature or something that just happens to work now but may not be supported in the future.

Thanks, Bradley

On Mon, Dec 29, 2008 at 1:00 PM, Matthew Ratzloff matt@builtfromsource.com wrote: >

Hi Bradley, Use the scrolling style "All" instead. Please see http://framework.zend.com/manual/en/… for more information. -Matt

On Sun, Dec 28, 2008 at 6:40 PM, Bradley Holt bradley.holt@foundline.com wrote: >

I was looking for a way to have Zend_Paginator show all of the items. For example, I have a selector that allows a user to show 25, 50, 100, or all records. I couldn't find any documentation but by experimenting I found that I could pass a negative one (-1) to setItemCountPerPage and it would show all items (actually, I think any negative number will show all items). Since I couldn't find any documentation on this I'm wondering if this is a supported feature of Zend_Paginator that just happens to work now but might break in the future.

Thanks, Bradley

-- Bradley Holt bradley.holt@foundline.com

-- Bradley Holt bradley.holt@foundline.com

Comments

This allows a user to use paginator objects throughout an application.

+1

Looks like a simple patch?

Paginator.php line 607


    public function setCurrentPageNumber($pageNumber)
    {
        $this->_currentPageNumber = (integer) $pageNumber;
        $this->_currentItems      = null;
        $this->_currentItemCount  = null;

        return $this;
    }

becomes


    public function setCurrentPageNumber($pageNumber)
    {
        $this->_currentPageNumber = (integer) $pageNumber;
        $this->_currentItems      = null;
        $this->_currentItemCount  = null;
        if ($pageNumber <= 0) $this->setItemCountPerPage( $this->getTotalItemCount() );

        return $this;
    }

I've attached a patch to allow for a negative one to be passed to Zend_Paginator::setItemCountPerPage() which will explicitly allow for an infinite (technically the total item count, but effectively infinite) number of items per page. The patch also includes a new assertion in the unit tests for this component. With some adapters, this may technically be a BC break (but I'm not sure) since all values below one were treated the same before, but now zero will be treated as before, but values below zero will get an item count per page equal to the total item count. If this patch looks good, I will go ahead and commit it.

PHP is inconsistent in its API, so this method should accept -1, 0, or null as triggers for this behavior.

0: http://php.net/manual/en/… (seconds) http://php.net/manual/en/function.ldap-read.php (sizeLimit) http://php.net/manual/en/function.ldap-search.php (sizeLimit)

-1: http://www.php.net/manual/en/ini.list.php

-1, 0, null: http://php.net/manual/en/function.preg-split.php (limit) http://php.net/manual/en/function.preg-replace.php (limit)

Values less than one was what the original issue description said the value should be to trigger an infinite item count per page would. However, this caused an existing unit test to break so I didn't feel it was an appropriate change. That's why I went with negative one (well, anything less than zero) instead.

This is the existing assertion that fails if we change the behavior to accept zero as the infinite item count per page:


$this->_paginator->setItemCountPerPage(0);
$this->assertEquals(10, $this->_paginator->getItemCountPerPage());

I didn't feel it was my place to change an existing assertion as I don't know the history behind that expected behavior. If you are confident that assertion can safely be modified, then I can change the behavior to accept -1, 0, or null as triggers for this behavior. The assertion would have to modified as follows for the tests to pass with this new behavior:


$this->_paginator->setItemCountPerPage(0);
$this->assertEquals(101, $this->_paginator->getItemCountPerPage());

That test doesn't make any logical sense to me, anyway; I think it's safe to change. Passing 0 as an argument has never been a documented use of the API, so I have no BC concerns about this change.

Thanks for taking this on, Bradley!