try marking same items as read if they contain the same content

7 files changed, 145 insertions, 68 deletions

diff --git a/db/itemmapper.php b/db/itemmapper.php
index 895b0f7ae..a2ad70a67 100644
--- a/db/itemmapper.php
+++ b/db/itemmapper.php
@@ -379,5 +379,29 @@ class ItemMapper extends NewsMapper {
         }
     }
 
+    public function readItem($itemId, $isRead, $lastModified, $userId) {
+        $item = $this->find($itemId, $userId);
+
+        // reading an item should set all of the same items as read, whereas
+        // marking an item as unread should only mark the selected instance
+        // as unread
+        if ($isRead) {
+            $sql = 'UPDATE `*PREFIX*news_items`
+                SET `status` = `status` & ?
+                    AND `last_modified` = ?
+                WHERE `fingerprint` = ?
+                    AND feed_id IN (
+                        SELECT `f`.`id` FROM `*PREFIX*news_feeds` `f`
+                            WHERE `f`.`user_id` = ?
+                    )';
+            $params = [~StatusFlag::UNREAD, $lastModified,
+                       $item->getFingerprint(), $userId];
+            $this->execute($sql, $params);
+        } else {
+            $item->setLastModified($lastModified);
+            $item->setUnread();
+            $this->update($item);
+        }
+    }
 
 }
diff --git a/db/mysql/itemmapper.php b/db/mysql/itemmapper.php
index 5fb28880b..720f03744 100644
--- a/db/mysql/itemmapper.php
+++ b/db/mysql/itemmapper.php
@@ -63,5 +63,22 @@ class ItemMapper extends \OCA\News\Db\ItemMapper {
 
     }
 
+    public function readItem($itemId, $isRead, $lastModified, $userId) {
+        if ($isRead) {
+            $sql = 'UPDATE `*PREFIX*news_items` `items`
+                JOIN `*PREFIX*news_feeds` `feeds`
+                    ON `feeds`.`id` = `items`.`feed_id`
+                SET `items`.`status` = `items`.`status` & ?
+                    AND `items`.`last_modified` = ?
+                WHERE `items`.`fingerprint` = ?
+                    AND `feeds`.`user_id` = ?';
+            $params = [~StatusFlag::UNREAD, $lastModified,
+                       $item->getFingerprint(), $userId];
+            $this->execute($sql, $params);
+        } else {
+            // no other behavior for mysql if should be marked unread
+            parent::readItem($itemId, $isRead, $lastModified, $userId);
+        }
+    }
 
 }
diff --git a/service/itemservice.php b/service/itemservice.php
index 95a07d425..a571c15ac 100644
--- a/service/itemservice.php
+++ b/service/itemservice.php
@@ -148,14 +148,8 @@ class ItemService extends Service {
      * @throws ServiceNotFoundException if the item does not exist
      */
     public function read($itemId, $isRead, $userId){
-        $item = $this->find($itemId, $userId);
-        $item->setLastModified($this->timeFactory->getTime());
-        if($isRead){
-            $item->setRead();
-        } else {
-            $item->setUnread();
-        }
-        $this->itemMapper->update($item);
+        $lastModified = $this->timeFactory->getTime();
+        $this->itemMapper->readItem($itemId, $isRead, $lastModified, $userId);
     }
 
 
diff --git a/tests/integration/db/ItemMapperTest.php b/tests/integration/db/ItemMapperTest.php
index a36b1db9e..14c989cbf 100644
--- a/tests/integration/db/ItemMapperTest.php
+++ b/tests/integration/db/ItemMapperTest.php
@@ -225,30 +225,73 @@ class ItemMapperTest extends IntegrationTest {
     }
 
 
-    /* TBD
-    public function testFindAllFolder () {
-
+    public function testReadItem() {
+        $this->loadFixtures('readitem');
+        // assert that all items are unread
+        $feed = $this->feedMapper->where(['userId' => 'john'])[0];
+        $items = $this->itemMapper->where(['feedId' => $feed->getId()]);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isUnread());
+        }
+        $feed = $this->feedMapper->where(['userId' => 'test'])[0];
+        $items = $this->itemMapper->where(['feedId' => $feed->getId()]);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isUnread());
+        }
+
+        // read an item
+        $duplicateItem = $this->itemMapper->where(['feedId' => $feed->getId()])[0];
+        $this->itemMapper->readItem($duplicateItem->getId(), true, 1000, $this->user);
+
+        // assert that all test user's same items are read
+        $items = $this->itemMapper->where(['feedId' => $feed->getId(), 'title' => 'blubb']);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isRead());
+        }
+
+        // assert that a different item is not read
+        $items = $this->itemMapper->where(['feedId' => $feed->getId(), 'title' => 'blubbs']);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isUnread());
+        }
+
+        // assert that other user's same items stayed the same
+        $johnsFeed = $this->feedMapper->where(['userId' => 'john'])[0];
+        $items = $this->itemMapper->where(['feedId' => $johnsFeed->getId()]);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isUnread());
+        }
     }
 
-
-    public function testFindAllFeed () {
-
-    }
-
-
-    public function testFindAllNew () {
-
+    public function testUnreadItem() {
+        $this->loadFixtures('readitem');
+        // unread an item
+        $feed = $this->feedMapper->where(['userId' => 'test'])[0];
+        $duplicateItem = $this->itemMapper->where(['feedId' => $feed->getId()])[0];
+        $this->itemMapper->readItem($duplicateItem->getId(), true, 1000, $this->user);
+        $this->itemMapper->readItem($duplicateItem->getId(), false, 1000, $this->user);
+
+        // assert that only one item is now unread
+        $items = $this->itemMapper->where(['feedId' => $feed->getId(), 'title' => 'blubb']);
+        foreach ($items as $item) {
+            if ($item->getId() === $duplicateItem->getId()) {
+                $this->assertTrue($item->isUnread());
+            } else {
+                $this->assertTrue($item->isRead());
+            }
+        }
+
+        // assert that other user's same items stayed the same
+        $johnsFeed = $this->feedMapper->where(['userId' => 'john'])[0];
+        $items = $this->itemMapper->where(['feedId' => $johnsFeed->getId()]);
+        foreach ($items as $item) {
+            $this->assertTrue($item->isUnread());
+        }
     }
 
-
-    public function testFindAllNewFolder () {
-
-    }
-
-
-    public function testFindAllNewFeed () {
-
+    protected function tearDown() {
+        parent::tearDown();
+        $this->clearUserNewsDatabase('john');
     }
 
-    */
 }
diff --git a/tests/integration/fixtures/data/readitem.php b/tests/integration/fixtures/data/readitem.php
new file mode 100644
index 000000000..7e3d68fad
--- /dev/null
+++ b/tests/integration/fixtures/data/readitem.php
@@ -0,0 +1,33 @@
+<?php
+/**
+ * ownCloud - News
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later. See the COPYING file.
+ *
+ * @author Bernhard Posselt <dev@bernhard-posselt.com>
+ * @copyright Bernhard Posselt 2015
+ */
+
+return [
+    'feeds' => [
+        [
+            'title' => 'john feed',
+            'userId' => 'john',
+            'items' => [
+                ['title' => 'blubb', 'status' => 2],
+                ['title' => 'blubb', 'status' => 2]
+            ]
+        ],
+        [
+            'title' => 'test feed',
+            'userId' => 'test',
+            'items' => [
+                ['title' => 'blubb', 'status' => 2],
+                ['title' => 'blubbs', 'status' => 2],
+                ['title' => 'blubb', 'status' => 2],
+                ['title' => 'blubb', 'status' => 2]
+            ]
+        ]
+    ]
+];
diff --git a/tests/integration/fixtures/itemfixture.php b/tests/integration/fixtures/itemfixture.php
index af074718f..9e6e07fe2 100644
--- a/tests/integration/fixtures/itemfixture.php
+++ b/tests/integration/fixtures/itemfixture.php
@@ -42,9 +42,8 @@ class ItemFixture extends Item {
             $defaults['guidHash'] = $defaults['guid'];
         }
 
-        $this->generateSearchIndex();
-
         $this->fillDefaults($defaults);
+        $this->generateSearchIndex();
     }
 
 }
diff --git a/tests/unit/service/ItemServiceTest.php b/tests/unit/service/ItemServiceTest.php
index 0c062ff4c..e9bf47490 100644
--- a/tests/unit/service/ItemServiceTest.php
+++ b/tests/unit/service/ItemServiceTest.php
@@ -272,34 +272,6 @@ class ItemServiceTest extends \PHPUnit_Framework_TestCase {
         $this->assertTrue($item->isUnstarred());
     }
 
-    public function testUnread(){
-        $itemId = 3;
-        $item = new Item();
-        $item->setStatus(128);
-        $item->setId($itemId);
-        $item->setRead();
-
-        $expectedItem = new Item();
-        $expectedItem->setStatus(128);
-        $expectedItem->setUnread();
-        $expectedItem->setId($itemId);
-        $expectedItem->setLastModified($this->time);
-
-        $this->mapper->expects($this->once())
-            ->method('find')
-            ->with($this->equalTo($itemId), $this->equalTo($this->user))
-            ->will($this->returnValue($item));
-
-        $this->mapper->expects($this->once())
-            ->method('update')
-            ->with($this->equalTo($expectedItem));
-
-        $this->itemService->read($itemId, false, $this->user);
-
-        $this->assertTrue($item->isUnread());
-    }
-
-
     public function testRead(){
         $itemId = 3;
         $item = new Item();
@@ -314,17 +286,14 @@ class ItemServiceTest extends \PHPUnit_Framework_TestCase {
         $expectedItem->setLastModified($this->time);
 
         $this->mapper->expects($this->once())
-            ->method('find')
-            ->with($this->equalTo($itemId), $this->equalTo($this->user))
+            ->method('readItem')
+            ->with($this->equalTo($itemId),
+            $this->equalTo(true),
+            $this->equalTo($this->time),
+            $this->equalTo($this->user))
             ->will($this->returnValue($item));
 
-        $this->mapper->expects($this->once())
-            ->method('update')
-            ->with($this->equalTo($expectedItem));
-
         $this->itemService->read($itemId, true, $this->user);
-
-        $this->assertTrue($item->isRead());
     }
 
 
@@ -471,5 +440,3 @@ class ItemServiceTest extends \PHPUnit_Framework_TestCase {
 
 
 }
-
-