From e0b519bb005801158e3e3607f593030e219fd116 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Mon, 3 Jun 2024 21:08:39 +0700 Subject: [PATCH] fix: bug of extra first & last page showing up in small data set. --- CHANGELOG.md | 1 + .../src/Pager/Internal/ProximityPager.php | 66 +++++---- tests/src/App/Form/PagerParametersType.php | 1 + .../Pager/KeysetPagerSmallDatasetTest.php | 137 ++++++++++++++++++ 4 files changed, 180 insertions(+), 25 deletions(-) create mode 100644 tests/src/IntegrationTests/Pager/KeysetPagerSmallDatasetTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e2f0ef8..a12d929 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ # 0.8.5 * build: Symfony 7.1 compatibility +* fix: bug of extra first & last page showing up in small data set. # 0.8.4 diff --git a/packages/rekapager-core/src/Pager/Internal/ProximityPager.php b/packages/rekapager-core/src/Pager/Internal/ProximityPager.php index 189df8d..1273f90 100644 --- a/packages/rekapager-core/src/Pager/Internal/ProximityPager.php +++ b/packages/rekapager-core/src/Pager/Internal/ProximityPager.php @@ -88,23 +88,43 @@ public function __construct( $this->lastPage = $this->decorate($lastPage->withPageNumber(null)); } - // check previous pages + // preps if ($currentIsFirstPage) { $previousPages = []; } else { - $previousPages = $currentPage->getPreviousPages($this->proximity * 2); + $previousPages = $currentPage->getPreviousPages($this->proximity * 2 + 2); + } + + $nextPages = $currentPage->getNextPages($this->proximity * 2 + 2); + + // calculate the number of neighboring pages to show + + $previousPagesCount = \count($previousPages); + $nextPagesCount = \count($nextPages); + + $neighboringPreviousPagesCount = min($previousPagesCount, $this->proximity); + $neighboringNextPagesCount = min($nextPagesCount, $this->proximity); + + if ($neighboringPreviousPagesCount < $this->proximity) { + $neighboringNextPagesCount += $this->proximity - $neighboringPreviousPagesCount; + } + + if ($neighboringNextPagesCount < $this->proximity) { + $neighboringPreviousPagesCount += $this->proximity - $neighboringNextPagesCount; } - if (\count($previousPages) >= $this->proximity + 2) { + // check previous pages + + if ($previousPagesCount >= $neighboringPreviousPagesCount + 2) { $currentIsFirstPage = false; $firstIsFirstPage = false; $hasGapToFirstPage = true; - } elseif (\count($previousPages) === $this->proximity + 1) { + } elseif ($previousPagesCount === $neighboringPreviousPagesCount + 1) { $currentIsFirstPage = false; $firstIsFirstPage = false; $hasGapToFirstPage = false; - } elseif (\count($previousPages) === 0) { + } elseif ($previousPagesCount === 0) { $currentIsFirstPage = true; $firstIsFirstPage = false; $hasGapToFirstPage = false; @@ -116,17 +136,15 @@ public function __construct( // check next pages - $nextPages = $currentPage->getNextPages($this->proximity * 2); - - if (\count($nextPages) >= $this->proximity + 2) { + if ($nextPagesCount >= $neighboringNextPagesCount + 2) { $currentIsLastPage = false; $lastIsLastPage = false; $hasGapToLastPage = true; - } elseif (\count($nextPages) === $this->proximity + 1) { + } elseif ($nextPagesCount === $neighboringNextPagesCount + 1) { $currentIsLastPage = false; $lastIsLastPage = false; $hasGapToLastPage = false; - } elseif (\count($nextPages) === 0) { + } elseif ($nextPagesCount === 0) { $currentIsLastPage = true; $lastIsLastPage = false; $hasGapToLastPage = false; @@ -136,22 +154,9 @@ public function __construct( $hasGapToLastPage = false; } - // calculate the number of neighboring pages to show - - $previousPagesCount = min(\count($previousPages), $this->proximity); - $nextPagesCount = min(\count($nextPages), $this->proximity); - - if ($previousPagesCount < $this->proximity) { - $nextPagesCount += $this->proximity - $previousPagesCount; - } - - if ($nextPagesCount < $this->proximity) { - $previousPagesCount += $this->proximity - $nextPagesCount; - } - // add previous pages - foreach (range(0, $previousPagesCount - 1) as $i) { + foreach (range(0, $neighboringPreviousPagesCount - 1) as $i) { $page = array_pop($previousPages); if ($page === null) { @@ -163,7 +168,7 @@ public function __construct( // add next pages - foreach (range(0, $nextPagesCount - 1) as $i) { + foreach (range(0, $neighboringNextPagesCount - 1) as $i) { $page = array_shift($nextPages); if ($page === null) { @@ -196,6 +201,14 @@ public function __construct( $this->hasHiddenPagesBefore = true; } + // if first page in the previous neighboring pages is the first page, + // then remove it + + $firstInPreviousPages = reset($this->previousNeighboringPages) ?: null; + if ($firstInPreviousPages?->getPageNumber() === 1) { + array_shift($this->previousNeighboringPages); + } + // append last page if ($currentIsLastPage) { @@ -205,6 +218,9 @@ public function __construct( // page from the next neighboring pages, and set it as the last page $lastPage = array_pop($this->nextNeighboringPages); $this->lastPage = $lastPage; + // } elseif (!$lastIsLastPage && !$hasGapToLastPage) { + // $lastPage = array_pop($this->nextNeighboringPages); + // $this->lastPage = $lastPage; } else { if ($hasGapToLastPage) { $this->hasHiddenPagesAfter = true; diff --git a/tests/src/App/Form/PagerParametersType.php b/tests/src/App/Form/PagerParametersType.php index bfe49a2..b733727 100644 --- a/tests/src/App/Form/PagerParametersType.php +++ b/tests/src/App/Form/PagerParametersType.php @@ -48,6 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void ]) ->add('itemsPerPage', ChoiceType::class, [ 'choices' => [ + '3' => 3, '5' => 5, '10' => 10, '20' => 20, diff --git a/tests/src/IntegrationTests/Pager/KeysetPagerSmallDatasetTest.php b/tests/src/IntegrationTests/Pager/KeysetPagerSmallDatasetTest.php new file mode 100644 index 0000000..746e435 --- /dev/null +++ b/tests/src/IntegrationTests/Pager/KeysetPagerSmallDatasetTest.php @@ -0,0 +1,137 @@ + + * + * For the full copyright and license information, please view the LICENSE file + * that was distributed with this source code. + */ + +namespace Rekalogika\Rekapager\Tests\IntegrationTests\Pager; + +use PHPUnit\Framework\Attributes\DataProviderExternal; +use Rekalogika\Rekapager\Tests\IntegrationTests\DataProvider\PageableGeneratorProvider; + +class KeysetPagerSmallDatasetTest extends PagerTestCase +{ + public function getItemsPerPage(): int + { + return 3; + } + + public function getSetName(): string + { + return 'small'; + } + + #[DataProviderExternal(PageableGeneratorProvider::class, 'keyset')] + public function testFirstPage(string $pageableGeneratorClass): void + { + $pageable = $this->createPageableFromGenerator($pageableGeneratorClass); + $pager = $this->createPagerFromPageable($pageable); + + $this->assertPager( + $pager, + proximity: 2, + hasPrevious: false, + hasNext: true, + hasFirst: false, + hasLast: true, + hasGapToFirstPage: false, + hasGapToLastPage: false, + numOfPreviousNeighboringPages: 0, + numOfNextNeighboringPages: 2, + firstPageNumber: null, + lastPageNumber: 4, + currentPageNumber: 1, + previousPageNumbers: [], + nextPageNumbers: [2, 3], + currentCount: 3, + ); + } + + #[DataProviderExternal(PageableGeneratorProvider::class, 'keyset')] + public function testSecondPage(string $pageableGeneratorClass): void + { + $pageable = $this->createPageableFromGenerator($pageableGeneratorClass); + $page = $this->getNthPageFromBeginning($pageable, 2); + $pager = $this->createPagerFromPage($page); + + $this->assertPager( + $pager, + proximity: 2, + hasPrevious: true, + hasNext: true, + hasFirst: true, + hasLast: true, + hasGapToFirstPage: false, + hasGapToLastPage: false, + numOfPreviousNeighboringPages: 0, + numOfNextNeighboringPages: 1, + firstPageNumber: 1, + lastPageNumber: 4, + currentPageNumber: 2, + previousPageNumbers: [], + nextPageNumbers: [3], + currentCount: 3, + ); + } + + #[DataProviderExternal(PageableGeneratorProvider::class, 'keyset')] + public function testThirdPage(string $pageableGeneratorClass): void + { + $pageable = $this->createPageableFromGenerator($pageableGeneratorClass); + $page = $this->getNthPageFromBeginning($pageable, 3); + $pager = $this->createPagerFromPage($page); + + $this->assertPager( + $pager, + proximity: 2, + hasPrevious: true, + hasNext: true, + hasFirst: true, + hasLast: true, + hasGapToFirstPage: false, + hasGapToLastPage: false, + numOfPreviousNeighboringPages: 1, + numOfNextNeighboringPages: 0, + firstPageNumber: 1, + lastPageNumber: 4, + currentPageNumber: 3, + previousPageNumbers: [2], + nextPageNumbers: [], + currentCount: 3, + ); + } + + #[DataProviderExternal(PageableGeneratorProvider::class, 'keyset')] + public function testFourthPage(string $pageableGeneratorClass): void + { + $pageable = $this->createPageableFromGenerator($pageableGeneratorClass); + $page = $this->getNthPageFromBeginning($pageable, 4); + $pager = $this->createPagerFromPage($page); + + $this->assertPager( + $pager, + proximity: 2, + hasPrevious: true, + hasNext: false, + hasFirst: true, + hasLast: false, + hasGapToFirstPage: false, + hasGapToLastPage: false, + numOfPreviousNeighboringPages: 2, + numOfNextNeighboringPages: 0, + firstPageNumber: 1, + lastPageNumber: null, + currentPageNumber: 4, + previousPageNumbers: [2, 3], + nextPageNumbers: [], + currentCount: 1, + ); + } +}