DB: Fix offset quotes

[SMillerDev]

Fixes GH-1200

[codecov-io]

Codecov Report

Merging #1201 (a3b1ed3) into master (43deed2) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1201      +/-   ##
============================================
+ Coverage     87.07%   87.15%   +0.07%     
- Complexity      691      693       +2     
============================================
  Files            60       60              
  Lines          2492     2484       -8     
============================================
- Hits           2170     2165       -5     
+ Misses          322      319       -3     

Impacted Files	Coverage Δ	Complexity Δ
lib/Db/ItemMapperV2.php	100.00% <100.00%> (ø)	53.00 <2.00> (+2.00)
lib/Fetcher/FeedFetcher.php	88.23% <0.00%> (-0.18%)	36.00% <0.00%> (ø%)
lib/Service/FeedServiceV2.php	100.00% <0.00%> (ø)	28.00% <0.00%> (ø%)
lib/Command/Debug/ItemList.php	0.00% <0.00%> (ø)	6.00% <0.00%> (ø%)
lib/Command/Debug/FeedItemList.php	0.00% <0.00%> (ø)	6.00% <0.00%> (ø%)
lib/Command/Debug/FolderItemList.php	0.00% <0.00%> (ø)	8.00% <0.00%> (ø%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43deed2...a3b1ed3. Read the comment docs.

[Grotax]

Looks good 👍

[anoymouserver]

It fetches the exact same articles again, resulting in not showing any new ones

// inital: /apps/news/items?limit=40&oldestFirst=false&search=&showAll=true&type=6
{"newestItemId": 129155, "feeds": [ /* feed data */ ], "starred": 543, "items": [ /* 40 items */ ]}

// after scrolling: /apps/news/items?limit=40&offset=129115&oldestFirst=false&showAll=true&type=6
{"items": [ /* 39 items */ ]}

I had to make a few changes to make it work:

diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
index bfa1d111c..24c70ea89 100644
--- a/lib/Db/ItemMapperV2.php
+++ b/lib/Db/ItemMapperV2.php
@@ -412,10 +412,10 @@ class ItemMapperV2 extends NewsMapperV2
     private function offsetWhere(bool $oldestFirst): string
     {
         if ($oldestFirst === true) {
-            return 'items.id < :offset';
+            return 'items.id > :offset';
         }
 
-        return 'items.id > :offset';
+        return 'items.id < :offset';
     }
 
     /**
@@ -445,14 +445,17 @@ class ItemMapperV2 extends NewsMapperV2
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
             ->andWhere('items.feed_id = :feedId')
-            ->andWhere($this->offsetWhere($oldestFirst))
             ->setParameter('userId', $userId)
             ->setParameter('feedId', $feedId)
-            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
+        if ($offset !== 0) {
+            $builder->andWhere($this->offsetWhere($oldestFirst))
+                    ->setParameter('offset', $offset);
+        }
+
         if ($search !== []) {
             foreach ($search as $key => $term) {
                 $term = $this->db->escapeLikeParameter($term);
@@ -501,13 +504,16 @@ class ItemMapperV2 extends NewsMapperV2
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
             ->andWhere($folderWhere)
-            ->andWhere($this->offsetWhere($oldestFirst))
             ->setParameter('userId', $userId)
-            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
+        if ($offset !== 0) {
+            $builder->andWhere($this->offsetWhere($oldestFirst))
+                    ->setParameter('offset', $offset);
+        }
+
         if ($search !== []) {
             foreach ($search as $key => $term) {
                 $term = $this->db->escapeLikeParameter($term);
@@ -548,13 +554,16 @@ class ItemMapperV2 extends NewsMapperV2
             ->from($this->tableName, 'items')
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
-            ->andWhere($this->offsetWhere($oldestFirst))
             ->setParameter('userId', $userId)
-            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
+        if ($offset !== 0) {
+            $builder->andWhere($this->offsetWhere($oldestFirst))
+                    ->setParameter('offset', $offset);
+        }
+
         if ($search !== []) {
             foreach ($search as $key => $term) {
                 $term = $this->db->escapeLikeParameter($term);

I have adopted most of them from the old ItemMapper, after seaching for the differences..

The operator has to be other way around

news/lib/Db/ItemMapper.php

Lines 202 to 209 in ceba810

	private function getOperator(bool $oldestFirst): string
	{
	if ($oldestFirst) {
	return '>';
	} else {
	return '<';
	}
	}

The offset is apparently 0 for the initial request, so no offset filter should be applied

news/lib/Db/ItemMapper.php

Lines 333 to 337 in ceba810

	if ($offset !== 0) {
	$sql .= 'AND `items`.`id` ' .
	$this->getOperator($oldestFirst) . ' ? ';
	$params[] = $offset;
	}

[SMillerDev]

Applied the patch, thanks @anoymouserver

[SMillerDev]

Really github? Request review means "dismiss existing reviews"?

[Grotax]

Good work guys :)