Fix unread count and related issues

Issue GH-924

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>

10 files changed, 76 insertions, 18 deletions

diff --git a/AUTHORS.md b/AUTHORS.md
index 7cb56a2d8..f20045b78 100644
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -11,11 +11,11 @@
 * [Daniel Schaal](mailto:daniel@schaal.email)
 * [Davide Saurino](mailto:davide.saurino@alcacoop.it)
 * [raghunayyar](mailto:me@iraghu.com)
+* [anoy](mailto:anoymouserver+github@mailbox.org)
 * [bastei](mailto:bastei@users.noreply.github.com)
 * [Bernhard Posselt](mailto:bep@foryouandyourcustomers.com)
 * [Thomas Müller](mailto:thomas.mueller@tmit.eu)
 * [Hoàng Đức Hiếu](mailto:hdhoang@zahe.me)
-* [anoy](mailto:anoymouserver+github@mailbox.org)
 * [Daniel Opitz](mailto:git@copynpaste.de)
 * [Sean Molenaar](mailto:sean@m2mobi.com)
 * [rakekniven](mailto:mark.ziegler@rakekniven.de)
@@ -128,7 +128,6 @@
 * [YMHuang](mailto:ymhuang@fmbase.tw)
 * [Zach DeCook](mailto:zachdecook@gmail.com)
 * [amittel](mailto:arnulf.mittelstaedt@gmail.com)
-* [anoy](mailto:anoymouserver@users.noreply.github.com)
 * [b_b](mailto:brunobergot@gmail.com)
 * [bjoerns1983](mailto:bjoern@sengotta.net)
 * [blackcrack](mailto:blackcrack@blackysgate.de)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8718d1127..06467b26e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.
 ## Unreleased
 - Remove deprecated YouTube playlist API
 - Locale-aware sorting for folders and feeds
+- Fix empty unread item count
 
 ## 15.1.0-rc1
 
diff --git a/appinfo/routes.php b/appinfo/routes.php
index 7bf843cb2..36ed97b10 100644
--- a/appinfo/routes.php
+++ b/appinfo/routes.php
@@ -59,8 +59,6 @@ return ['routes' => [
 ['name' => 'api#index', 'url' => '/api', 'verb' => 'GET'],
 
 // API 1.2
-['name' => 'user_api#index', 'url' => '/api/v1-2/user', 'verb' => 'GET'],
-['name' => 'user_api#avatar', 'url' => '/api/v1-2/user/avatar', 'verb' => 'GET'],
 ['name' => 'utility_api#version', 'url' => '/api/v1-2/version', 'verb' => 'GET'],
 ['name' => 'utility_api#status', 'url' => '/api/v1-2/status', 'verb' => 'GET'],
 ['name' => 'utility_api#before_update', 'url' => '/api/v1-2/cleanup/before-update', 'verb' => 'GET'],
diff --git a/lib/Command/Updater/UpdateFeed.php b/lib/Command/Updater/UpdateFeed.php
index 71e88a15f..93fa2a8d3 100644
--- a/lib/Command/Updater/UpdateFeed.php
+++ b/lib/Command/Updater/UpdateFeed.php
@@ -52,7 +52,7 @@ class UpdateFeed extends Command
         $userId = $input->getArgument('user-id');
         try {
             $feed = $this->feedService->findForUser($userId, $feedId);
-            $this->feedService->fetch($feed);
+            $updated_feed = $this->feedService->fetch($feed);
         } catch (\Exception $e) {
             $output->writeln(
                 '<error>Could not update feed with id ' . $feedId .
@@ -62,6 +62,11 @@ class UpdateFeed extends Command
             return 1;
         }
 
+        if ($updated_feed->getUpdateErrorCount() !== 0) {
+            $output->writeln($updated_feed->getLastUpdateError());
+            return 255;
+        }
+
         return 0;
     }
 }
diff --git a/lib/Db/FeedMapperV2.php b/lib/Db/FeedMapperV2.php
index 05b272112..366ece141 100644
--- a/lib/Db/FeedMapperV2.php
+++ b/lib/Db/FeedMapperV2.php
@@ -16,6 +16,7 @@ namespace OCA\News\Db;
 use OCA\News\Utility\Time;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\AppFramework\Db\Entity;
 
@@ -50,10 +51,18 @@ class FeedMapperV2 extends NewsMapperV2
     public function findAllFromUser(string $userId, array $params = []): array
     {
         $builder = $this->db->getQueryBuilder();
-        $builder->addSelect('*')
-                ->from($this->tableName)
-                ->where('user_id = :user_id')
-                ->andWhere('deleted_at = 0')
+        $builder->select('feeds.*', $builder->func()->count('items.id', 'unreadCount'))
+                ->from($this->tableName, 'feeds')
+                ->leftJoin(
+                    'feeds',
+                    ItemMapperV2::TABLE_NAME,
+                    'items',
+                    'items.feed_id = feeds.id AND items.unread = :unread'
+                )
+                ->where('feeds.user_id = :user_id')
+                ->andWhere('feeds.deleted_at = 0')
+                ->groupBy('feeds.id')
+                ->setParameter(':unread', true, IQueryBuilder::PARAM_BOOL)
                 ->setParameter(':user_id', $userId);
 
         return $this->findEntities($builder);
diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
index 8bd939e71..4f6f82413 100644
--- a/lib/Db/ItemMapperV2.php
+++ b/lib/Db/ItemMapperV2.php
@@ -134,6 +134,8 @@ class ItemMapperV2 extends NewsMapperV2
     /**
      * Delete items from feed that are over the max item threshold
      *
+     * TODO: Implement
+     *
      * @param int $threshold Deletion threshold
      */
     public function deleteOverThreshold(int $threshold)
diff --git a/lib/Service/FeedServiceV2.php b/lib/Service/FeedServiceV2.php
index 0d1b45600..94878d5fe 100644
--- a/lib/Service/FeedServiceV2.php
+++ b/lib/Service/FeedServiceV2.php
@@ -228,6 +228,7 @@ class FeedServiceV2 extends Service
 
         $feed->setFolderId($folderId)
             ->setUserId($userId)
+            ->setHttpLastModified(null)
             ->setArticlesPerUpdate(count($items));
 
         if (!is_null($title)) {
@@ -236,7 +237,7 @@ class FeedServiceV2 extends Service
 
         if (!is_null($user)) {
             $feed->setBasicAuthUser($user)
-                ->setBasicAuthUser($password);
+                 ->setBasicAuthPassword($password);
         }
 
         return $this->mapper->insert($feed);
diff --git a/lib/Service/ItemServiceV2.php b/lib/Service/ItemServiceV2.php
index b4cb16e22..d0093a07e 100644
--- a/lib/Service/ItemServiceV2.php
+++ b/lib/Service/ItemServiceV2.php
@@ -72,6 +72,11 @@ class ItemServiceV2 extends Service
         return $this->mapper->findAll();
     }
 
+    /**
+     * Insert an item or update.
+     *
+     * @param Item $item
+     */
     public function insertOrUpdate(Item $item)
     {
         try {
diff --git a/tests/Integration/Db/FeedMapperTest.php b/tests/Integration/Db/FeedMapperTest.php
index f07103d10..803e9260b 100644
--- a/tests/Integration/Db/FeedMapperTest.php
+++ b/tests/Integration/Db/FeedMapperTest.php
@@ -119,9 +119,6 @@ class FeedMapperTest extends IntegrationTest
         $this->assertEquals($feed->getTitle(), $fetched->getTitle());
     }
 
-    /**
-     * @expectedException OCP\AppFramework\Db\MultipleObjectsReturnedException
-     */
     public function testFindByUrlHashMoreThanOneResult()
     {
         $this->expectException('OCP\AppFramework\Db\MultipleObjectsReturnedException');
@@ -143,10 +140,6 @@ class FeedMapperTest extends IntegrationTest
         $this->feedMapper->findByUrlHash($feed1->getUrlHash(), $this->user);
     }
 
-
-    /**
-     * @expectedException OCP\AppFramework\Db\DoesNotExistException
-     */
     public function testFindByUrlHashNotExisting()
     {
         $this->expectException('OCP\AppFramework\Db\DoesNotExistException');
diff --git a/tests/Unit/Command/UpdateFeedTest.php b/tests/Unit/Command/UpdateFeedTest.php
index 05d12fa66..bd0cd34e4 100644
--- a/tests/Unit/Command/UpdateFeedTest.php
+++ b/tests/Unit/Command/UpdateFeedTest.php
@@ -67,6 +67,12 @@ class UpdateFeedTest extends TestCase
 
         $feed = $this->createMock(Feed::class);
 
+        $feed->expects($this->exactly(1))
+            ->method('getUpdateErrorCount')
+            ->willReturn(0);
+        $feed->expects($this->exactly(0))
+            ->method('getLastUpdateError');
+
         $this->service->expects($this->exactly(1))
                            ->method('findForUser')
                            ->with('admin', '1')
@@ -74,7 +80,8 @@ class UpdateFeedTest extends TestCase
 
         $this->service->expects($this->exactly(1))
                            ->method('fetch')
-                           ->with($feed);
+                           ->with($feed)
+                           ->willReturn($feed);
 
         $result = $this->command->run($this->consoleInput, $this->consoleOutput);
         $this->assertSame(0, $result);
@@ -83,6 +90,44 @@ class UpdateFeedTest extends TestCase
     /**
      * Test a valid call will work
      */
+    public function testValidFeedError()
+    {
+        $this->consoleInput->expects($this->exactly(2))
+                           ->method('getArgument')
+                           ->will($this->returnValueMap([
+                               ['feed-id', '1'],
+                               ['user-id', 'admin'],
+                           ]));
+
+        $feed = $this->createMock(Feed::class);
+        $feed->expects($this->exactly(1))
+             ->method('getUpdateErrorCount')
+             ->willReturn(10);
+        $feed->expects($this->exactly(1))
+             ->method('getLastUpdateError')
+             ->willReturn('Problem');
+
+        $this->service->expects($this->exactly(1))
+                           ->method('findForUser')
+                           ->with('admin', '1')
+                           ->willReturn($feed);
+
+        $this->service->expects($this->exactly(1))
+                           ->method('fetch')
+                           ->with($feed)
+                           ->willReturn($feed);
+
+        $this->consoleOutput->expects($this->exactly(1))
+                           ->method('writeln')
+                           ->with('Problem');
+
+        $result = $this->command->run($this->consoleInput, $this->consoleOutput);
+        $this->assertSame(255, $result);
+    }
+
+    /**
+     * Test a valid call will work
+     */
     public function testInValid()
     {
         $this->consoleInput->expects($this->exactly(2))