From 0abd282444d3de3f53cd876b97a567f55ee47ae9 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 15 Nov 2024 12:25:59 +0000 Subject: [PATCH 1/2] FIX: Make IsFirst and IsLast work as expected for PaginatedList (fixes #11465) --- src/View/SSViewer_Scope.php | 19 ++++++++++++++----- tests/php/View/SSViewerTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/View/SSViewer_Scope.php b/src/View/SSViewer_Scope.php index 15a6744773f..0c1694beb19 100644 --- a/src/View/SSViewer_Scope.php +++ b/src/View/SSViewer_Scope.php @@ -10,6 +10,7 @@ use SilverStripe\ORM\FieldType\DBFloat; use SilverStripe\ORM\FieldType\DBInt; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\PaginatedList; /** * This tracks the current scope for an SSViewer instance. It has three goals: @@ -305,8 +306,8 @@ public function next() } if (!$this->itemIterator) { - // Note: it is important that getIterator() is called before count() as implemenations may rely on - // this to efficiency get both the number of records and an iterator (e.g. DataList does this) + // Note: it is important that getIterator() is called before count() as implementations may rely on + // this to efficiently get both the number of records and an iterator (e.g. DataList does this) // Item may be an array or a regular IteratorAggregate if (is_array($this->item)) { @@ -319,11 +320,19 @@ public function next() $this->itemIterator->rewind(); } - // If the item implements Countable, use that to fetch the count, otherwise we have to inspect the - // iterator and then rewind it. - if ($this->item instanceof Countable) { + // Special case: we *don't* want to use count() on PaginatedList. This is because it'll call + // PaginatedList::count(), which currently returns the full list count rather than the count of items + // on the current page (which is what we need for the iterator count) + if ($this->item instanceof PaginatedList) { + // We have to re-fetch the iterator before calling getInnerIterator(): we need to count a copy of the + // inner iterator because it's a generator so can't be rewound or cloned + $innerIterator = $this->item->getIterator()->getInnerIterator(); + $this->itemIteratorTotal = iterator_count($innerIterator); + } elseif ($this->item instanceof Countable) { + // If the item implements Countable, use that to fetch the count $this->itemIteratorTotal = count($this->item); } else { + // Otherwise we have to inspect the iterator and then rewind it $this->itemIteratorTotal = iterator_count($this->itemIterator); $this->itemIterator->rewind(); } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index 3b55fa9d9ea..c7092f370dc 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -1480,6 +1480,37 @@ public function testSSViewerBasicIteratorSupport() $this->assertEquals("", $result, "Only numbers that are multiples of 11 are returned. I.e. nothing returned"); } + public function testSSViewerBasicIteratorSupportWithPaginatedList() + { + $list = new ArrayList([ + ['Val' => 1], + ['Val' => 2], + ['Val' => 3], + ['Val' => 4], + ['Val' => 5], + ['Val' => 6], + ]); + $paginatedList = new PaginatedList($list); + $paginatedList->setPageLength(2); + $data = new ArrayData([ + 'PaginatedList' => $paginatedList + ]); + + $result = $this->render('<% loop PaginatedList %><% if $IsFirst %>$Val<% end_if %><% end_loop %>', $data); + $this->assertEquals("1", $result, "Only the first item on the first page is rendered"); + + $result = $this->render('<% loop PaginatedList %><% if $IsLast %>$Val<% end_if %><% end_loop %>', $data); + $this->assertEquals("2", $result, "Only the last item on the first page is rendered"); + + $paginatedList->setCurrentPage(2); + + $result = $this->render('<% loop PaginatedList %><% if $IsFirst %>$Val<% end_if %><% end_loop %>', $data); + $this->assertEquals("3", $result, "Only the first item on the second page is rendered"); + + $result = $this->render('<% loop PaginatedList %><% if $IsLast %>$Val<% end_if %><% end_loop %>', $data); + $this->assertEquals("4", $result, "Only the last item on the second page is rendered"); + } + /** * Test $Up works when the scope $Up refers to was entered with a "with" block */ From f1eab1b58cc96551da9fdf4f8517060e16bbac9b Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 25 Nov 2024 10:20:24 +1300 Subject: [PATCH 2/2] MNT Fix unit test --- tests/php/View/Embed/MockResponse.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/php/View/Embed/MockResponse.php b/tests/php/View/Embed/MockResponse.php index 8744e49c75c..65785047afd 100644 --- a/tests/php/View/Embed/MockResponse.php +++ b/tests/php/View/Embed/MockResponse.php @@ -36,26 +36,32 @@ public function getBody() public function getReasonPhrase() { + return ''; } public function getProtocolVersion() { + return ''; } public function getHeaders() { + return []; } public function getHeader($name) { + return ''; } public function getHeaderLine($name) { + return ''; } public function hasHeader($name) { + return false; } public function withHeader($name, $value)