From 5bdc2df7f484f6618919a63247045027d43b78f7 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Mon, 9 Nov 2020 23:29:25 +0100 Subject: Fix unread count and related issues Issue GH-924 Signed-off-by: Sean Molenaar --- AUTHORS.md | 3 +-- CHANGELOG.md | 1 + appinfo/routes.php | 2 -- lib/Command/Updater/UpdateFeed.php | 7 ++++- lib/Db/FeedMapperV2.php | 17 +++++++++--- lib/Db/ItemMapperV2.php | 2 ++ lib/Service/FeedServiceV2.php | 3 ++- lib/Service/ItemServiceV2.php | 5 ++++ tests/Integration/Db/FeedMapperTest.php | 7 ----- tests/Unit/Command/UpdateFeedTest.php | 47 ++++++++++++++++++++++++++++++++- 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( '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,12 +80,51 @@ 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); } + /** + * 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 */ -- cgit v1.2.3