From 65df175ebe6490fb6506cd0c2a375a4856ebb660 Mon Sep 17 00:00:00 2001
From: Kevin Ditscheid <kevinditscheid@gmail.com>
Date: Thu, 12 Apr 2018 12:55:26 +0200
Subject: [PATCH] [BUGFIX] Fix wrong offset calculation

Integrate a general function into the AbstractController to
handle offset calculation for the pagination.
---
 Classes/Controller/AbstractController.php     | 16 +++++++++
 Classes/Controller/LatestController.php       |  2 +-
 .../Controller/ListByCategoryController.php   |  7 ++--
 Classes/Controller/OverviewController.php     | 33 ++++++-------------
 .../Domain/Repository/AbstractRepository.php  | 13 ++++++++
 Classes/Domain/Repository/NewsRepository.php  |  8 ++---
 6 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/Classes/Controller/AbstractController.php b/Classes/Controller/AbstractController.php
index 251c5b9..af31b71 100644
--- a/Classes/Controller/AbstractController.php
+++ b/Classes/Controller/AbstractController.php
@@ -189,4 +189,20 @@ abstract class AbstractController extends ActionController {
 			'teaserImageObject' => $teaserImageObject,
 		];
 	}
+
+	/**
+	 * Calculate the pagination offset
+	 *
+	 * @param int $currentPageBrowserPage
+	 * @param int $newsLimitPerPage
+	 * @return int
+	 */
+	protected function calculatePaginationOffset(int $currentPageBrowserPage, int $newsLimitPerPage = NULL){
+		$offset = 0;
+		$newsPerPage = $newsLimitPerPage ?? (int) $this->settings['newsLimitPerPage'];
+		if ($currentPageBrowserPage > 0 && $newsPerPage > 0) {
+			$offset = $currentPageBrowserPage * $newsPerPage;
+		}
+		return $offset;
+	}
 }
diff --git a/Classes/Controller/LatestController.php b/Classes/Controller/LatestController.php
index d08977f..03a95c5 100644
--- a/Classes/Controller/LatestController.php
+++ b/Classes/Controller/LatestController.php
@@ -100,7 +100,7 @@ class LatestController extends AbstractController {
 
 		// Redo this function, until $newsMetaData gets the $limit. Needed because of languagevisibility.
 		if (count($newsMetaData) < $limit) {
-			$offset += $limit;
+			$offset += $limit - 1;
 			$maxCount = $this->newsRepository->getCountOfLastUpdatedOrHighlightedNewsByCategories(
 				FALSE, $categoryUids, TRUE, $this->settings['sortBy'], $tagUids, $startTime, $endTime
 			);
diff --git a/Classes/Controller/ListByCategoryController.php b/Classes/Controller/ListByCategoryController.php
index d17c917..5d63356 100644
--- a/Classes/Controller/ListByCategoryController.php
+++ b/Classes/Controller/ListByCategoryController.php
@@ -96,15 +96,14 @@ class ListByCategoryController extends AbstractController {
 		$startTime = (int) $this->settings['starttime'];
 		$endTime = (int) $this->settings['endtime'];
 
-		$offset = 0;
 		$newsPerPage = (int) $this->settings['newsLimitPerPage'];
-		if ($currentPageBrowserPage && $newsPerPage) {
-			$offset = $currentPageBrowserPage * $newsPerPage;
-		}
+
 		$newsCount = $this->newsRepository->newsCountByCategories($categoryUids, $tagUids, $startTime, $endTime);
 		$numberOfPages = ($newsPerPage <= 0 ? 0 : ceil($newsCount / $newsPerPage));
 
 		$headerSet = FALSE;
+		$offset = $this->calculatePaginationOffset($currentPageBrowserPage);
+
 		$news = $this->newsRepository->findAllSortedNewsByCategories(
 			$categoryUids, $newsPerPage, $offset, $this->settings['sortBy'], $tagUids, $startTime, $endTime
 		)->toArray();
diff --git a/Classes/Controller/OverviewController.php b/Classes/Controller/OverviewController.php
index a0aa8e8..415fe29 100644
--- a/Classes/Controller/OverviewController.php
+++ b/Classes/Controller/OverviewController.php
@@ -133,23 +133,18 @@ class OverviewController extends AbstractController {
 	 * @param array $newsByCategory
 	 * @param array $allNews
 	 * @param array $newsFilter
-	 * @param boolean $isInitialCall
 	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 */
 	protected function overviewWithCategories(
-		array $newsByCategory = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE, $currentPageBrowserPage = 0
+		array $newsByCategory = [], array $allNews = [], array $newsFilter = [], $currentPageBrowserPage = 0
 	) {
 		$newsLimitPerCategory = (int) $this->settings['newsLimit'];
 		$this->categoryRepository->setDefaultOrderings(['sorting' => Query::ORDER_ASCENDING]);
 
-		$offset = 0;
-
-		if ($currentPageBrowserPage && $newsLimitPerCategory) {
-			$offset = ($currentPageBrowserPage * $newsLimitPerCategory) - ($isInitialCall ? 0 : 1);
-		}
+		$offset = $this->calculatePaginationOffset($currentPageBrowserPage, $newsLimitPerCategory);
 
 		if ($this->settings['onlyNewsWithinThisPageSection']) {
 			/** @noinspection PhpUndefinedMethodInspection */
@@ -246,7 +241,7 @@ class OverviewController extends AbstractController {
 		if ($maxNewsPerCategory < $newsLimitPerCategory && count($allNews) < $newsLimitPerCategory) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				$this->overviewWithCategories($newsByCategory, $allNews, $newsFilter, FALSE, $nextPage);
+				$this->overviewWithCategories($newsByCategory, $allNews, $newsFilter, $nextPage);
 				return;
 			}
 		}
@@ -278,22 +273,18 @@ class OverviewController extends AbstractController {
 	 * @param array $newsByTag
 	 * @param array $allNews
 	 * @param array $newsFilter
-	 * @param boolean $isInitialCall
 	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 */
 	protected function overviewWithTags(
-		array $newsByTag = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE, $currentPageBrowserPage = 0
+		array $newsByTag = [], array $allNews = [], array $newsFilter = [], $currentPageBrowserPage = 0
 	) {
 		$newsLimitPerTag = (int) $this->settings['newsLimit'];
 		$this->tagRepository->setDefaultOrderings(['sorting' => Query::ORDER_ASCENDING]);
 
-		$offset = 0;
-		if ($currentPageBrowserPage && $newsLimitPerTag) {
-			$offset = ($currentPageBrowserPage * $newsLimitPerTag) - ($isInitialCall ? 0 : 1);
-		}
+		$offset = $this->calculatePaginationOffset($currentPageBrowserPage, $newsLimitPerTag);
 
 		$tagPid = (int) $this->settings['tagPid'];
 		if (!$tagPid) {
@@ -399,7 +390,7 @@ class OverviewController extends AbstractController {
 		if ($maxNewsPerTag < $newsLimitPerTag && count($allNews) < $newsLimitPerTag) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				$this->overviewWithTags($newsByTag, $allNews, $newsFilter, FALSE, $nextPage);
+				$this->overviewWithTags($newsByTag, $allNews, $newsFilter, $nextPage);
 				return;
 			}
 		}
@@ -440,7 +431,6 @@ class OverviewController extends AbstractController {
 	 *
 	 * @param array $newsMetaData
 	 * @param array $newsFilter
-	 * @param boolean $isInitialCall
 	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
@@ -448,7 +438,7 @@ class OverviewController extends AbstractController {
 	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\NoSuchArgumentException
 	 */
 	protected function overviewWithoutCategoriesAction(
-		array $newsMetaData = [], array $newsFilter = NULL, $isInitialCall = TRUE, $currentPageBrowserPage = 0
+		array $newsMetaData = [], array $newsFilter = NULL, $currentPageBrowserPage = 0
 	) {
 		// remember selection of the filter values, if any
 		$selectedTag = $this->tagRepository->findByUid((int) $newsFilter['tag']);
@@ -456,12 +446,9 @@ class OverviewController extends AbstractController {
 		$this->view->assign('selectedTag', $selectedTag);
 		$this->view->assign('selectedCategory', $selectedCategory);
 
-		$offset = 0;
+		$offset = $this->calculatePaginationOffset($currentPageBrowserPage);
+
 		$newsPerPage = (int) $this->settings['newsLimit'];
-		if ($currentPageBrowserPage && $newsPerPage) {
-			// might be necessary for the recursive calling performance wise
-			$offset = ($currentPageBrowserPage * $newsPerPage) - ($isInitialCall ? 0 : 1);
-		}
 
 		$startTime = (int) $this->settings['starttime'];
 		$endTime = (int) $this->settings['endtime'];
@@ -528,7 +515,7 @@ class OverviewController extends AbstractController {
 		if (count($newsMetaData) < $newsPerPage) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				$this->overviewWithoutCategoriesAction($newsMetaData, $newsFilter, FALSE, $nextPage);
+				$this->overviewWithoutCategoriesAction($newsMetaData, $newsFilter, $nextPage);
 				return;
 			}
 		}
diff --git a/Classes/Domain/Repository/AbstractRepository.php b/Classes/Domain/Repository/AbstractRepository.php
index edea6ae..49ad3c2 100644
--- a/Classes/Domain/Repository/AbstractRepository.php
+++ b/Classes/Domain/Repository/AbstractRepository.php
@@ -28,6 +28,7 @@ namespace SGalinski\SgNews\Domain\Repository;
 
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Extbase\Persistence\Generic\Typo3QuerySettings;
+use TYPO3\CMS\Extbase\Persistence\QueryInterface;
 use TYPO3\CMS\Extbase\Persistence\Repository;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3\CMS\Frontend\Page\PageRepository;
@@ -79,4 +80,16 @@ abstract class AbstractRepository extends Repository {
 
 		return $statement;
 	}
+
+	/**
+	 * This count returns the right amount of results, in contrast to $query->execute()->count(), which does not respect
+	 * the language- nor the workspace overlay.
+	 *
+	 * @param QueryInterface $query The query object to use the count on
+	 *
+	 * @return int
+	 */
+	public function getCount(QueryInterface $query) {
+		return \count($query->execute(true));
+	}
 }
diff --git a/Classes/Domain/Repository/NewsRepository.php b/Classes/Domain/Repository/NewsRepository.php
index a3f840b..079b48b 100644
--- a/Classes/Domain/Repository/NewsRepository.php
+++ b/Classes/Domain/Repository/NewsRepository.php
@@ -169,7 +169,7 @@ class NewsRepository extends AbstractRepository {
 		} elseif (count($constraints) === 1) {
 			$query->matching($constraints[0]);
 		}
-		return $query->count();
+		return $this->getCount($query);
 	}
 
 	/**
@@ -215,9 +215,9 @@ class NewsRepository extends AbstractRepository {
 		$onlyHighlighted = FALSE, array $categoryIds = NULL, $hideNeverHighlightedNews = FALSE, $sortBy = 'date',
 		array $tagIds = NULL, $startTime = 0, $endTime = 0
 	): int {
-		return $this->getQueryForLastUpdatedOrHighlightedNewsByCategories(
+		return $this->getCount($this->getQueryForLastUpdatedOrHighlightedNewsByCategories(
 			0, $onlyHighlighted, $categoryIds, 0, $hideNeverHighlightedNews, $sortBy, $tagIds, $startTime, $endTime
-		)->count();
+		));
 	}
 
 	/**
@@ -450,6 +450,6 @@ class NewsRepository extends AbstractRepository {
 			$query->matching($constraints[0]);
 		}
 
-		return $query->count();
+		return $this->getCount($query);
 	}
 }
-- 
GitLab