Split binary to booleans (#203)

* replaced old status with 2 flags for unread and starred

* add fields to db, replace int(1,0) with booleans in sql queries, removed StatusFlags class + refactor code relying to it

* add repair step for migration

* again use integer(1,0) instead of bool in sql queries, because of sqlite doesn't support true/false

* add/fix unit tests for new boolean status

* set unread/starred flags as statements in sql

* fixed mysql unknown column items.unread, fixed marking of read items on repair step

* remove unnecessary bool casts

* add empty checks to Items::is* methods

* update migration to use native sql instead of the querybuilder

* don't cast the flags manually, let the api do the work

2 files changed, 11 insertions, 20 deletions

diff --git a/lib/Service/FeedService.php b/lib/Service/FeedService.php
index f17b759e3..d24fd259e 100644
--- a/lib/Service/FeedService.php
+++ b/lib/Service/FeedService.php
@@ -264,7 +264,7 @@ class FeedService extends Service {
 
                         // update modes: 0 nothing, 1 set unread
                         if ($existingFeed->getUpdateMode() === 1) {
-                            $dbItem->setUnread();
+                            $dbItem->setUnread(true);
                         }
 
                         $this->itemMapper->update($dbItem);
diff --git a/lib/Service/ItemService.php b/lib/Service/ItemService.php
index f657b3eff..fee00ffc2 100644
--- a/lib/Service/ItemService.php
+++ b/lib/Service/ItemService.php
@@ -13,11 +13,11 @@
 
 namespace OCA\News\Service;
 
+use OCA\News\Db\Item;
 use OCP\IConfig;
 use OCP\AppFramework\Db\DoesNotExistException;
 
 use OCA\News\Db\ItemMapper;
-use OCA\News\Db\StatusFlag;
 use OCA\News\Db\FeedType;
 use OCA\News\Config\Config;
 use OCA\News\Utility\Time;
@@ -25,19 +25,16 @@ use OCA\News\Utility\Time;
 
 class ItemService extends Service {
 
-    private $statusFlag;
     private $config;
     private $timeFactory;
     private $itemMapper;
     private $systemConfig;
 
     public function __construct(ItemMapper $itemMapper,
-                                StatusFlag $statusFlag,
                                 Time $timeFactory,
                                 Config $config,
                                 IConfig $systemConfig){
         parent::__construct($itemMapper);
-        $this->statusFlag = $statusFlag;
         $this->config = $config;
         $this->timeFactory = $timeFactory;
         $this->itemMapper = $itemMapper;
@@ -56,20 +53,18 @@ class ItemService extends Service {
      * @return array of items
      */
     public function findAllNew($id, $type, $updatedSince, $showAll, $userId){
-        $status = $this->statusFlag->typeToStatus($type, $showAll);
-
         switch($type){
             case FeedType::FEED:
                 return $this->itemMapper->findAllNewFeed(
-                    $id, $updatedSince, $status, $userId
+                    $id, $updatedSince, $showAll, $userId
                 );
             case FeedType::FOLDER:
                 return $this->itemMapper->findAllNewFolder(
-                    $id, $updatedSince, $status, $userId
+                    $id, $updatedSince, $showAll, $userId
                 );
             default:
                 return $this->itemMapper->findAllNew(
-                    $updatedSince, $status, $userId
+                    $updatedSince, $type, $showAll, $userId
                 );
         }
     }
@@ -90,22 +85,20 @@ class ItemService extends Service {
      */
     public function findAll($id, $type, $limit, $offset, $showAll, $oldestFirst,
                             $userId, $search=[]){
-        $status = $this->statusFlag->typeToStatus($type, $showAll);
-
         switch($type){
             case FeedType::FEED:
                 return $this->itemMapper->findAllFeed(
-                    $id, $limit, $offset, $status, $oldestFirst, $userId,
+                    $id, $limit, $offset, $showAll, $oldestFirst, $userId,
                     $search
                 );
             case FeedType::FOLDER:
                 return $this->itemMapper->findAllFolder(
-                    $id, $limit, $offset, $status, $oldestFirst, $userId,
+                    $id, $limit, $offset, $showAll, $oldestFirst, $userId,
                     $search
                 );
             default:
                 return $this->itemMapper->findAll(
-                    $limit, $offset, $status, $oldestFirst, $userId, $search
+                    $limit, $offset, $type, $showAll, $oldestFirst, $userId, $search
                 );
         }
     }
@@ -122,15 +115,13 @@ class ItemService extends Service {
      */
     public function star($feedId, $guidHash, $isStarred, $userId){
         try {
+            /** @var Item $item */
             $item = $this->itemMapper->findByGuidHash(
                 $guidHash, $feedId, $userId
             );
 
-            if($isStarred){
-                $item->setStarred();
-            } else {
-                $item->setUnstarred();
-            }
+            $item->setStarred($isStarred);
+
             $this->itemMapper->update($item);
         } catch(DoesNotExistException $ex) {
             throw new ServiceNotFoundException($ex->getMessage());