Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pager #2489

Merged
merged 6 commits into from
Jan 22, 2020
Merged

Fix pager #2489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
88e3d78
Fix that Pager::getCurrentPage() try to return string type
MashinaMashina Dec 28, 2019
df56424
Opportunity to change parameter whith we get current page
MashinaMashina Jan 14, 2020
1cb04cb
Priority fix
MashinaMashina Jan 14, 2020
7436e69
pageSelector is always set
MashinaMashina Jan 14, 2020
86ac8c8
Remove setPageSelector(), fix backward compatibility
MashinaMashina Jan 14, 2020
309afd1
Pager new tests
MashinaMashina Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions system/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,14 +1122,13 @@ public function chunk(int $size, Closure $userFunc)
*/
public function paginate(int $perPage = 20, string $group = 'default', int $page = 0)
{
// Get the necessary parts.
$page = $page >= 1 ? $page : (ctype_digit($_GET['page'] ?? '') && $_GET['page'] > 1 ? $_GET['page'] : 1);

$pager = \Config\Services::pager(null, null, false);
$page = $page >= 1 ? $page : $pager->getCurrentPage($group);
$total = $this->countAllResults(false);

// Store it in the Pager library so it can be
// paginated in the views.
$pager = \Config\Services::pager();
$this->pager = $pager->store($group, $page, $perPage, $total);

$offset = ($page - 1) * $perPage;
Expand Down
41 changes: 29 additions & 12 deletions system/Pager/Pager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ public function getPageURI(int $page = null, string $group = 'default', bool $re
}
else
{
$uri->addQuery('page', $page);
$uri->addQuery($this->groups[$group]['pageSelector'], $page);
}

if ($this->only)
Expand All @@ -357,7 +357,7 @@ public function getPageURI(int $page = null, string $group = 'default', bool $re

if (! $segment)
{
$query['page'] = $page;
$query[$this->groups[$group]['pageSelector']] = $page;
}

$uri->setQueryArray($query);
Expand Down Expand Up @@ -503,13 +503,31 @@ protected function ensureGroup(string $group)
}

$this->groups[$group] = [
'uri' => clone Services::request()->uri,
'hasMore' => false,
'total' => null,
'perPage' => $this->config->perPage,
'pageCount' => 1,
'uri' => clone Services::request()->uri,
'hasMore' => false,
'total' => null,
'perPage' => $this->config->perPage,
'pageCount' => 1,
'pageSelector' => $group === 'default' ? 'page' : 'page_' . $group,
];

$this->calculateCurrentPage($group);

if ($_GET)
{
$this->groups[$group]['uri'] = $this->groups[$group]['uri']->setQueryArray($_GET);
}
}

//--------------------------------------------------------------------

/**
* Calculating the current page
*
* @param string $group
*/
protected function calculateCurrentPage(string $group)
{
if (array_key_exists($group, $this->segment))
{
try
Expand All @@ -523,12 +541,11 @@ protected function ensureGroup(string $group)
}
else
{
$this->groups[$group]['currentPage'] = $_GET['page_' . $group] ?? $_GET['page'] ?? 1;
}
$pageSelector = $this->groups[$group]['pageSelector'];

if ($_GET)
{
$this->groups[$group]['uri'] = $this->groups[$group]['uri']->setQueryArray($_GET);
$page = (int) ($_GET[$pageSelector] ?? 1);

$this->groups[$group]['currentPage'] = $page < 1 ? 1 : $page;
}
}

Expand Down
35 changes: 21 additions & 14 deletions system/Pager/PagerRenderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ class PagerRenderer
* @var integer
*/
protected $segment;

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

//--------------------------------------------------------------------

/**
Expand All @@ -103,13 +109,14 @@ class PagerRenderer
*/
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->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';
}

//--------------------------------------------------------------------
Expand Down Expand Up @@ -164,7 +171,7 @@ public function getPrevious()

if ($this->segment === 0)
{
$uri->addQuery('page', $this->first - 1);
$uri->addQuery($this->pageSelector, $this->first - 1);
}
else
{
Expand Down Expand Up @@ -208,7 +215,7 @@ public function getNext()

if ($this->segment === 0)
{
$uri->addQuery('page', $this->last + 1);
$uri->addQuery($this->pageSelector, $this->last + 1);
}
else
{
Expand All @@ -231,7 +238,7 @@ public function getFirst(): string

if ($this->segment === 0)
{
$uri->addQuery('page', 1);
$uri->addQuery($this->pageSelector, 1);
}
else
{
Expand All @@ -254,7 +261,7 @@ public function getLast(): string

if ($this->segment === 0)
{
$uri->addQuery('page', $this->pageCount);
$uri->addQuery($this->pageSelector, $this->pageCount);
}
else
{
Expand All @@ -277,7 +284,7 @@ public function getCurrent(): string

if ($this->segment === 0)
{
$uri->addQuery('page', $this->current);
$uri->addQuery($this->pageSelector, $this->current);
}
else
{
Expand Down Expand Up @@ -306,7 +313,7 @@ public function links(): array
for ($i = $this->first; $i <= $this->last; $i ++)
{
$links[] = [
'uri' => (string) ($this->segment === 0 ? $uri->addQuery('page', $i) : $uri->setSegment($this->segment, $i)),
'uri' => (string) ($this->segment === 0 ? $uri->addQuery($this->pageSelector, $i) : $uri->setSegment($this->segment, $i)),
'title' => (int) $i,
'active' => ($i === $this->current),
];
Expand Down
26 changes: 13 additions & 13 deletions tests/system/Pager/PagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ public function testGetTotalPagesCalcsCorrectValue()

public function testGetNextURIUsesCurrentURI()
{
$_GET['page'] = 2;
$_GET['page_foo'] = 2;

$this->pager->store('foo', 2, 12, 70);

$expected = current_url(true);
$expected = (string)$expected->setQuery('page=3');
$expected = (string)$expected->setQuery('page_foo=3');

$this->assertEquals((string)$expected, $this->pager->getNextPageURI('foo'));
}
Expand All @@ -225,19 +225,19 @@ public function testGetNextURICorrectOnFirstPage()
$this->pager->store('foo', 1, 12, 70);

$expected = current_url(true);
$expected = (string)$expected->setQuery('page=2');
$expected = (string)$expected->setQuery('page_foo=2');

$this->assertEquals($expected, $this->pager->getNextPageURI('foo'));
}

public function testGetPreviousURIUsesCurrentURI()
{
$_GET['page'] = 2;
$_GET['page_foo'] = 2;

$this->pager->store('foo', 2, 12, 70);

$expected = current_url(true);
$expected = (string)$expected->setQuery('page=1');
$expected = (string)$expected->setQuery('page_foo=1');

$this->assertEquals((string)$expected, $this->pager->getPreviousPageURI('foo'));
}
Expand All @@ -252,28 +252,28 @@ public function testGetNextURIReturnsNullOnFirstPage()
public function testGetNextURIWithQueryStringUsesCurrentURI()
{
$_GET = [
'page' => 3,
'status' => 1,
'page_foo' => 3,
'status' => 1,
];

$expected = current_url(true);
$expected = (string)$expected->setQueryArray($_GET);

$this->pager->store('foo', $_GET['page'] - 1, 12, 70);
$this->pager->store('foo', $_GET['page_foo'] - 1, 12, 70);

$this->assertEquals((string)$expected, $this->pager->getNextPageURI('foo'));
}

public function testGetPreviousURIWithQueryStringUsesCurrentURI()
{
$_GET = [
'page' => 1,
'status' => 1,
'page_foo' => 1,
'status' => 1,
];
$expected = current_url(true);
$expected = (string)$expected->setQueryArray($_GET);

$this->pager->store('foo', $_GET['page'] + 1, 12, 70);
$this->pager->store('foo', $_GET['page_foo'] + 1, 12, 70);

$this->assertEquals((string)$expected, $this->pager->getPreviousPageURI('foo'));
}
Expand Down Expand Up @@ -383,12 +383,12 @@ public function testBasedURI()
$this->config = new Pager();
$this->pager = new \CodeIgniter\Pager\Pager($this->config, Services::renderer());

$_GET['page'] = 2;
$_GET['page_foo'] = 2;

$this->pager->store('foo', 2, 12, 70);

$expected = current_url(true);
$expected = (string)$expected->setQuery('page=1');
$expected = (string)$expected->setQuery('page_foo=1');

$this->assertEquals((string)$expected, $this->pager->getPreviousPageURI('foo'));
}
Expand Down