Fix bug that would still show items after its feed or folder has been marked as deleted, fix #196

3 files changed, 42 insertions, 37 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ecb7d3c76..5a2f2debf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,6 @@
 ownCloud-news (0.102)
 * Fix marking read of all articles and folders on mysql and postgres
+* Fix bug that would still show items after its feed or folder has been marked as deleted
 
 ownCloud-news (0.101)
 
diff --git a/db/itemmapper.php b/db/itemmapper.php
index 99b315111..cd46678b7 100644
--- a/db/itemmapper.php
+++ b/db/itemmapper.php
@@ -56,7 +56,13 @@ class ItemMapper extends Mapper implements IMapper {
 		return 'SELECT `items`.* FROM `*PREFIX*news_items` `items` '.
 			'JOIN `*PREFIX*news_feeds` `feeds` ' .
 				'ON `feeds`.`id` = `items`.`feed_id` '.
-				'AND `feeds`.`user_id` = ? ' . $prependTo;
+				'AND `feeds`.`deleted_at` = 0 ' .
+				'AND `feeds`.`user_id` = ? ' . 
+				$prependTo .
+			'JOIN `*PREFIX*news_folders` `folders` ' .
+				'ON `folders`.`id` = `feeds`.`folder_id` ' .
+				'AND `folders`.`deleted_at` = 0 ' . 
+			'ORDER BY `items`.`id` DESC';
 	}
 
 	private function makeSelectQueryStatus($prependTo, $status) {
@@ -117,12 +123,12 @@ class ItemMapper extends Mapper implements IMapper {
 	public function readAll($highestItemId, $userId) {
 		$sql = 'UPDATE `*PREFIX*news_items` ' . 
 			'SET `status` = `status` & ? ' .
-                        'WHERE `feed_id` IN (' .
-                                'SELECT `id` FROM `*PREFIX*news_feeds` ' .
-                                        'WHERE `user_id` = ? ' .
-                                ') '.
-                        'AND `id` <= ?';
-                $params = array(~StatusFlag::UNREAD, $userId, $highestItemId);
+			'WHERE `feed_id` IN (' .
+				'SELECT `id` FROM `*PREFIX*news_feeds` ' .
+					'WHERE `user_id` = ? ' .
+				') '.
+			'AND `id` <= ?';
+		$params = array(~StatusFlag::UNREAD, $userId, $highestItemId);
 		$this->execute($sql, $params);
 	}
 
@@ -130,13 +136,13 @@ class ItemMapper extends Mapper implements IMapper {
 	public function readFolder($folderId, $highestItemId, $userId) {
 		$sql = 'UPDATE `*PREFIX*news_items` ' . 
 			'SET `status` = `status` & ? ' .
-                        'WHERE `feed_id` IN (' .
-                                'SELECT `id` FROM `*PREFIX*news_feeds` ' .
-                                        'WHERE `folder_id` = ? ' .
-                                        'AND `user_id` = ? ' .
-                                ') '.
-                        'AND `id` <= ?';
-                $params = array(~StatusFlag::UNREAD, $folderId, $userId, $highestItemId);
+			'WHERE `feed_id` IN (' .
+				'SELECT `id` FROM `*PREFIX*news_feeds` ' .
+					'WHERE `folder_id` = ? ' .
+					'AND `user_id` = ? ' .
+				') '.
+			'AND `id` <= ?';
+		$params = array(~StatusFlag::UNREAD, $folderId, $userId, $highestItemId);
 		$this->execute($sql, $params);
 	}
 
@@ -151,14 +157,15 @@ class ItemMapper extends Mapper implements IMapper {
 					'WHERE `user_id` = ? ' .
 					'AND `id` = ? ) ';
 		$params = array(~StatusFlag::UNREAD, $feedId, $highestItemId, $userId, 
-		                $feedId);
+				$feedId);
 		
 		$this->execute($sql, $params);
 	}
 
 
 	public function findAllNew($updatedSince, $status, $userId){
-		$sql = $this->makeSelectQueryStatus('AND `items`.`last_modified` >= ?', $status);
+		$sql = $this->makeSelectQueryStatus(
+			'AND `items`.`last_modified` >= ? ', $status);
 		$params = array($userId, $updatedSince);
 		return $this->findAllRows($sql, $params);
 	}
@@ -166,7 +173,7 @@ class ItemMapper extends Mapper implements IMapper {
 
 	public function findAllNewFolder($id, $updatedSince, $status, $userId){
 		$sql = 'AND `feeds`.`folder_id` = ? ' .
-				'AND `items`.`last_modified` >= ?';
+				'AND `items`.`last_modified` >= ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $status);
 		$params = array($userId, $id, $updatedSince);
 		return $this->findAllRows($sql, $params);
@@ -175,7 +182,7 @@ class ItemMapper extends Mapper implements IMapper {
 
 	public function findAllNewFeed($id, $updatedSince, $status, $userId){
 		$sql = 'AND `items`.`feed_id` = ? ' .
-				'AND `items`.`last_modified` >= ?';
+				'AND `items`.`last_modified` >= ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $status);
 		$params = array($userId, $id, $updatedSince);
 		return $this->findAllRows($sql, $params);
@@ -189,7 +196,6 @@ class ItemMapper extends Mapper implements IMapper {
 			$sql .= 'AND `items`.`id` < ? ';
 			array_push($params, $offset);
 		}
-		$sql .= 'ORDER BY `items`.`id` DESC ';
 		$sql = $this->makeSelectQueryStatus($sql, $status);
 		return $this->findAllRows($sql, $params, $limit);
 	}
@@ -202,7 +208,6 @@ class ItemMapper extends Mapper implements IMapper {
 			$sql .= 'AND `items`.`id` < ? ';
 			array_push($params, $offset);
 		}
-		$sql .= 'ORDER BY `items`.`id` DESC ';
 		$sql = $this->makeSelectQueryStatus($sql, $status);
 		return $this->findAllRows($sql, $params, $limit);
 	}
@@ -215,7 +220,6 @@ class ItemMapper extends Mapper implements IMapper {
 			$sql .= 'AND `items`.`id` < ? ';
 			array_push($params, $offset);
 		}
-		$sql .= 'ORDER BY `items`.`id` DESC ';
 		$sql = $this->makeSelectQueryStatus($sql, $status);
 		return $this->findAllRows($sql, $params, $limit);
 	}
diff --git a/tests/unit/db/ItemMapperTest.php b/tests/unit/db/ItemMapperTest.php
index 5f2639979..06565fc1f 100644
--- a/tests/unit/db/ItemMapperTest.php
+++ b/tests/unit/db/ItemMapperTest.php
@@ -83,7 +83,13 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 		return 'SELECT `items`.* FROM `*PREFIX*news_items` `items` '.
 			'JOIN `*PREFIX*news_feeds` `feeds` ' .
 				'ON `feeds`.`id` = `items`.`feed_id` '.
-				'AND `feeds`.`user_id` = ? ' . $prependTo;
+				'AND `feeds`.`deleted_at` = 0 ' .
+				'AND `feeds`.`user_id` = ? ' . 
+				$prependTo .
+			'JOIN `*PREFIX*news_folders` `folders` ' .
+				'ON `folders`.`id` = `feeds`.`folder_id` ' .
+				'AND `folders`.`deleted_at` = 0 ' . 
+			'ORDER BY `items`.`id` DESC';
 	}
 
 	private function makeSelectQueryStatus($prependTo, $status) {
@@ -170,7 +176,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testFindAllNew(){
-		$sql = 'AND `items`.`last_modified` >= ?';
+		$sql = 'AND `items`.`last_modified` >= ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->updatedSince);
 
@@ -184,7 +190,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 	public function testFindAllNewFolder(){
 		$sql = 'AND `feeds`.`folder_id` = ? ' .
-				'AND `items`.`last_modified` >= ?';
+				'AND `items`.`last_modified` >= ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 
 		$params = array($this->user, $this->id, $this->updatedSince);
@@ -198,7 +204,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 	public function testFindAllNewFeed(){
 		$sql = 'AND `items`.`feed_id` = ? ' .
-				'AND `items`.`last_modified` >= ?';
+				'AND `items`.`last_modified` >= ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->id, $this->updatedSince);
 
@@ -212,8 +218,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 	public function testFindAllFeed(){
 		$sql = 'AND `items`.`feed_id` = ? ' .
-			'AND `items`.`id` < ? ' .
-			'ORDER BY `items`.`id` DESC ';
+			'AND `items`.`id` < ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->id, $this->offset);
 		$this->setMapperResult($sql, $params, $this->rows);
@@ -225,8 +230,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testFindAllFeedOffsetZero(){
-		$sql = 'AND `items`.`feed_id` = ? ' .
-			'ORDER BY `items`.`id` DESC ';
+		$sql = 'AND `items`.`feed_id` = ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->id);
 		$this->setMapperResult($sql, $params, $this->rows);
@@ -239,8 +243,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 	public function testFindAllFolder(){
 		$sql = 'AND `feeds`.`folder_id` = ? ' .
-			'AND `items`.`id` < ? ' .
-			'ORDER BY `items`.`id` DESC ';
+			'AND `items`.`id` < ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->id, 
 			$this->offset);
@@ -253,8 +256,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testFindAllFolderOffsetZero(){
-		$sql = 'AND `feeds`.`folder_id` = ? ' .
-			'ORDER BY `items`.`id` DESC ';
+		$sql = 'AND `feeds`.`folder_id` = ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->id);
 		$this->setMapperResult($sql, $params, $this->rows);
@@ -266,8 +268,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testFindAll(){
-		$sql = 'AND `items`.`id` < ? ' .
-			'ORDER BY `items`.`id` DESC ';
+		$sql = 'AND `items`.`id` < ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
 		$params = array($this->user, $this->offset);
 		$this->setMapperResult($sql, $params, $this->rows);
@@ -279,8 +280,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testFindAllOffsetZero(){
-		$sql = 'ORDER BY `items`.`id` DESC ';
-		$sql = $this->makeSelectQueryStatus($sql, $this->status);
+		$sql = $this->makeSelectQueryStatus('', $this->status);
 		$params = array($this->user);
 		$this->setMapperResult($sql, $params, $this->rows);
 		$result = $this->mapper->findAll($this->limit,