Bug: Pagination class is bugful

[amirandev]

PHP Version

7.4

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL 5.6

What happened?

Codeginiter pagination returns wrong page numbers.
I opened C:\xampp\htdocs\zedge.ge\vendor\codeigniter4\framework\system\Pager\PagerRenderer.php in my editor and "OMG"
I can't believe that some one made that mistakes. Maybe the developer was high when he/she was writing the code.

Steps to Reproduce

Look at this

<ul class="pagination" aria-label="Page navigation">
   <li class="page-item">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=1" aria-label="First">First</a>
   </li>
   <li class="page-item">
      <!--it should return 2-->
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=4" aria-label="Previous">Previous</a>
   </li>
   <li class="page-item ">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=3">3</a>
   </li>
   <li class="page-item ">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=4">4</a>
   </li>
   <li class="page-item active">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=5">5</a>
   </li>
</ul>

Expected Output

<ul class="pagination" aria-label="Page navigation">
   <li class="page-item">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=1" aria-label="First">First</a>
   </li>
   <li class="page-item">
      <!--it should not return 4-->
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=2" aria-label="Previous">Previous</a>
   </li>
   <li class="page-item ">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=3">3</a>
   </li>
   <li class="page-item ">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=4">4</a>
   </li>
   <li class="page-item active">
      <a class="page-link" href="http://zedge.ge/admin/knowledge_base?page=5">5</a>
   </li>
</ul>

Anything else?

No response

[amirandev]

Copy and paste corrected code in new version

<?php

/**
 * This file is part of CodeIgniter 4 framework.
 *
 * (c) CodeIgniter Foundation <admin@codeigniter.com>
 *
 * For the full copyright and license information, please view
 * the LICENSE file that was distributed with this source code.
 */

namespace CodeIgniter\Pager;

use CodeIgniter\HTTP\URI;

/**
 * Class PagerRenderer
 *
 * This class is passed to the view that describes the pagination,
 * and is used to get the link information and provide utility
 * methods needed to work with pagination.
 */
class PagerRenderer
{
    /**
     * First page number.
     *
     * @var int
     */
    protected $first;

    /**
     * Last page number.
     *
     * @var int
     */
    protected $last;

    /**
     * Current page number.
     *
     * @var int
     */
    protected $current;

    /**
     * Total number of items.
     *
     * @var int
     */
    protected $total;

    /**
     * Total number of pages.
     *
     * @var int
     */
    protected $pageCount;

    /**
     * URI base for pagination links
     *
     * @var URI
     */
    protected $uri;

    /**
     * Segment number used for pagination.
     *
     * @var int
     */
    protected $segment;

    /**
     * Name of $_GET parameter
     *
     * @var string
     */
    protected $pageSelector;

    /**
     * Constructor.
     */
    public function __construct(array $details)
    {
        $this->first        = 1;
        $this->last         = $details['pageCount'];
        $this->current      = $details['currentPage'];
        $this->total        = $details['total'];
        $this->uri          = $details['uri'];
        $this->pageCount    = $details['pageCount'];
        $this->segment      = $details['segment'] ?? 0;
        $this->pageSelector = $details['pageSelector'] ?? 'page';
    }

    /**
     * Sets the total number of links that should appear on either
     * side of the current page. Adjusts the first and last counts
     * to reflect it.
     *
     * @return PagerRenderer
     */
    public function setSurroundCount(?int $count = null)
    {
        $this->updatePages($count);

        return $this;
    }

    /**
     * Checks to see if there is a "previous" page before our "first" page.
     */
    public function hasPrevious(): bool
    {
        return $this->first > 1;
    }

    /**
     * Returns a URL to the "previous" page. The previous page is NOT the
     * page before the current page, but is the page just before the
     * "first" page.
     *
     * You MUST call hasPrevious() first, or this value may be invalid.
     *
     * @return string|null
     */
    public function getPrevious()
    {
        if (! $this->hasPrevious()) {
            return null;
        }

        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->current - 1);
        } else {
            $uri->setSegment($this->segment, $this->current - 1);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Checks to see if there is a "next" page after our "last" page.
     */
    public function hasNext(): bool
    {
        return $this->pageCount > $this->last;
    }

    /**
     * Returns a URL to the "next" page. The next page is NOT, the
     * page after the current page, but is the page that follows the
     * "last" page.
     *
     * You MUST call hasNext() first, or this value may be invalid.
     *
     * @return string|null
     */
    public function getNext()
    {
        if (! $this->hasNext()) {
            return null;
        }

        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->current + 1);
        } else {
            $uri->setSegment($this->segment, $this->current + 1);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Returns the URI of the first page.
     */
    public function getFirst(): string
    {
        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, 1);
        } else {
            $uri->setSegment($this->segment, 1);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Returns the URI of the last page.
     */
    public function getLast(): string
    {
        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->pageCount);
        } else {
            $uri->setSegment($this->segment, $this->pageCount);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Returns the URI of the current page.
     */
    public function getCurrent(): string
    {
        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->current);
        } else {
            $uri->setSegment($this->segment, $this->current);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Returns an array of links that should be displayed. Each link
     * is represented by another array containing of the URI the link
     * should go to, the title (number) of the link, and a boolean
     * value representing whether this link is active or not.
     */
    public function links(): array
    {
        $links = [];

        $uri = clone $this->uri;

        for ($i = $this->first; $i <= $this->last; $i++) {
            $uri     = $this->segment === 0 ? $uri->addQuery($this->pageSelector, $i) : $uri->setSegment($this->segment, $i);
            $links[] = [
                'uri'    => URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment()),
                'title'  => $i,
                'active' => ($i === $this->current),
            ];
        }

        return $links;
    }

    /**
     * Updates the first and last pages based on $surroundCount,
     * which is the number of links surrounding the active page
     * to show.
     *
     * @param int|null $count The new "surroundCount"
     */
    protected function updatePages(?int $count = null)
    {
        if ($count === null) {
            return;
        }

        $this->first = $this->current - $count > 0 ? $this->current - $count : 1;
        $this->last  = $this->current + $count <= $this->pageCount ? $this->current + $count : (int) $this->pageCount;
    }

    /**
     * Checks to see if there is a "previous" page before our "first" page.
    */
    public function hasPreviousPage(): bool
    {
        return $this->current > 1;
    }

    /**
     * Returns a URL to the "previous" page.
     *
     * You MUST call hasPreviousPage() first, or this value may be invalid.
     *
     * @return string|null
     */
    public function getPreviousPage()
    {
        if (! $this->hasPreviousPage()) {
            return null;
        }

        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->current - 1);
        } else {
            $uri->setSegment($this->segment, $this->current - 1);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Checks to see if there is a "next" page after our "last" page.
     */
    public function hasNextPage(): bool
    {
        return $this->current < $this->last;
    }

    /**
     * Returns a URL to the "next" page.
     *
     * You MUST call hasNextPage() first, or this value may be invalid.
     *
     * @return string|null
     */
    public function getNextPage()
    {
        if (! $this->hasNextPage()) {
            return null;
        }

        $uri = clone $this->uri;

        if ($this->segment === 0) {
            $uri->addQuery($this->pageSelector, $this->current + 1);
        } else {
            $uri->setSegment($this->segment, $this->current + 1);
        }

        return URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(), $uri->getFragment());
    }

    /**
     * Returns the page number of the first page.
     */
    public function getFirstPageNumber(): int
    {
        return $this->first;
    }

    /**
     * Returns the page number of the current page.
     */
    public function getCurrentPageNumber(): int
    {
        return $this->current;
    }

    /**
     * Returns the page number of the last page.
     */
    public function getLastPageNumber(): int
    {
        return $this->last;
    }

    /**
     * Returns total number of pages.
     */
    public function getPageCount(): int
    {
        return $this->pageCount;
    }

    /**
     * Returns the previous page number.
     */
    public function getPreviousPageNumber(): ?int
    {
        return ($this->current === 1) ? null : $this->current - 1;
    }

    /**
     * Returns the next page number.
     */
    public function getNextPageNumber(): ?int
    {
        return ($this->current === $this->pageCount) ? null : $this->current + 1;
    }
}

[kenjis]

@amirandev Thank you for reporting.

But I don't understand why the pages should go 1, 4, 3, 4, 5, not 1, 2, 3, 4, 5.

[amirandev]

@amirandev Thank you for reporting.

But I don't understand why the pages hould go 1, 4, 3, 4, 5, not 1, 2, 3, 4, 5.

Sorry, I faultly pasted the codes in opposite textareas

[paulbalandan]

I think you need to use getPreviousPage() to get to page 2.
Please see this issue #3575 and #3575 (comment)
See also this #3575 (comment)

[kenjis]

It seems this is not a bug. It is the specification:
#2881 (comment)

See the example: #2881 (comment)

[kenjis]

I sent a PR to be clear: #5935

[paulbalandan]

Documentation fixed in #5935

[septesian]

I dont want to prolong this discussion but this problem appears to have been fixed in 4.0.3. The changes include two (one to hasPrevious and one to hasNext) that dont appear in the code listed above. Im running 4.1.9 and this fix seems to have disappeared and I assume the reason this issue has arisen.

[kenjis]

What do you mean?

If the latest documentation is different from the implementaion, please create a new issue.
https://codeigniter4.github.io/CodeIgniter4/libraries/pagination.html#creating-the-view

[septesian]

The documention does seem to tally with the current functionality. However in issue #2881 there was some discussion about the functionality not working they way that pagination rendering was normally expected to function. It certainly dosnt function in the same way as V3. As a result changes were made in 4.0.3 to reflect these concerns these changes seems to have been removed rather than perhaps change the documentation.