From 65d28653ae1a1d2a7680ccfa694dbb7e2d4d750f Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Wed, 6 Apr 2016 23:43:27 +0200 Subject: [PATCH] try marking same items as read if they contain the same content --- db/itemmapper.php | 24 ++++++ db/mysql/itemmapper.php | 17 +++++ service/itemservice.php | 10 +-- tests/integration/db/ItemMapperTest.php | 77 +++++++++++++++----- tests/integration/fixtures/data/readitem.php | 33 +++++++++ tests/integration/fixtures/itemfixture.php | 3 +- tests/unit/service/ItemServiceTest.php | 43 ++--------- 7 files changed, 142 insertions(+), 65 deletions(-) create mode 100644 tests/integration/fixtures/data/readitem.php 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 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); - public function testFindAllFeed () { + // 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 testFindAllNew () { - + protected function tearDown() { + parent::tearDown(); + $this->clearUserNewsDatabase('john'); } - - public function testFindAllNewFolder () { - - } - - - public function testFindAllNewFeed () { - - } - - */ } 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 @@ + + * @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 { } - -