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

[5.x] Prevent incompatible pagination parameter combinations #10415

Merged
merged 7 commits into from
Jul 10, 2024
Merged

[5.x] Prevent incompatible pagination parameter combinations #10415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1df414e
Prevent incompatible pagination parameter combinations.
jesseleite Jul 8, 2024
12e5cfb
Actually, we do technically allow `offset` with `paginate`.
jesseleite Jul 8, 2024
1bf9124
We don’t need `limit` once making it `paginate`, and this is necessar…
jesseleite Jul 8, 2024
9eab139
Rename method, since this logic is for v2 style behaviour (ie. `pagin…
jesseleite Jul 8, 2024
eb0305e
Add test coverage for new exception handling.
jesseleite Jul 8, 2024
efc753b
.
jesseleite Jul 8, 2024
397b50d
dont need to check for integers for all of them
jasonvarga Jul 10, 2024
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
22 changes: 20 additions & 2 deletions src/Tags/Concerns/GetsQueryResults.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ trait GetsQueryResults
{
protected function results($query)
{
$this->setPaginationParameterPrecedence();
$this
->allowLegacyStylePaginationLimiting()
->preventIncompatiblePaginationParameters();

if ($paginate = $this->params->int('paginate')) {
return $this->paginatedResults($query, $paginate);
Expand All @@ -30,11 +32,27 @@ protected function results($query)
return $query->get();
}

protected function setPaginationParameterPrecedence()
protected function allowLegacyStylePaginationLimiting()
{
if ($this->params->get('paginate') === true) {
$this->params->put('paginate', $this->params->get('limit'));
$this->params->forget('limit');
}

return $this;
}

protected function preventIncompatiblePaginationParameters()
{
if ($this->params->int('paginate') && $this->params->has('limit')) {
throw new \Exception('Cannot use [paginate] integer in combination with [limit] param.');
}

if ($this->params->has('paginate') && $this->params->has('chunk')) {
throw new \Exception('Cannot use [paginate] in combination with [chunk] param.');
}

return $this;
}

protected function paginatedResults($query, $perPage)
Expand Down
48 changes: 46 additions & 2 deletions tests/Tags/Collection/EntriesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ public function it_gets_paginated_entries_in_a_collection()

$this->assertCount(5, $this->getEntries());
$this->assertCount(3, $this->getEntries(['paginate' => 3])); // recommended v3 style
$this->assertCount(4, $this->getEntries(['paginate' => true, 'limit' => 4])); // v2 style
$this->assertCount(3, $this->getEntries(['paginate' => 3, 'limit' => 4])); // precedence test
$this->assertCount(4, $this->getEntries(['paginate' => true, 'limit' => 4])); // v2 style pagination limiting
$this->assertCount(5, $this->getEntries(['paginate' => true])); // ignore if no perPage set

$this->assertEquals(['a', 'b', 'c'], $this->getEntryIds(['paginate' => 3]));
Expand Down Expand Up @@ -128,6 +127,51 @@ public function it_gets_offset_paginated_entries_in_a_collection()
$this->assertEquals(['f', 'g'], $this->getEntryIds(['paginate' => 3, 'offset' => 2]));
}

#[Test]
public function it_should_throw_exception_if_trying_to_paginate_and_limit_at_same_time()
{
$this->makeEntry('a')->save();
$this->makeEntry('b')->save();
$this->makeEntry('c')->save();
$this->makeEntry('d')->save();
$this->makeEntry('e')->save();

$this->expectException(\Exception::class);
$this->expectExceptionMessage('Cannot use [paginate] integer in combination with [limit] param.');

$this->assertCount(3, $this->getEntries(['paginate' => 3, 'limit' => 4]));
}

#[Test]
public function it_should_throw_exception_if_trying_to_paginate_and_chunk_at_same_time()
{
$this->makeEntry('a')->save();
$this->makeEntry('b')->save();
$this->makeEntry('c')->save();
$this->makeEntry('d')->save();
$this->makeEntry('e')->save();

$this->expectException(\Exception::class);
$this->expectExceptionMessage('Cannot use [paginate] in combination with [chunk] param.');

$this->assertCount(3, $this->getEntries(['paginate' => true, 'chunk' => 2]));
}

#[Test]
public function it_should_throw_exception_if_trying_to_paginate_with_integer_and_chunk_at_same_time()
{
$this->makeEntry('a')->save();
$this->makeEntry('b')->save();
$this->makeEntry('c')->save();
$this->makeEntry('d')->save();
$this->makeEntry('e')->save();

$this->expectException(\Exception::class);
$this->expectExceptionMessage('Cannot use [paginate] in combination with [chunk] param.');

$this->assertCount(3, $this->getEntries(['paginate' => 3, 'chunk' => 2]));
}

#[Test]
public function it_gets_localized_site_entries_in_a_collection()
{
Expand Down
Loading