From 2b4da592f1c8a2210f9ba49a9e24eb2056e5d4a8 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Thu, 22 May 2014 02:15:56 +0200 Subject: simplify star and read --- appinfo/routes.php | 2 - controller/itemcontroller.php | 60 +++++---------- db/item.php | 33 ++++---- js/Gruntfile.js | 4 +- js/README.md | 2 +- js/build/app.js | 49 +++++++++--- js/service/FeedResource.js | 10 ++- js/service/FolderResource.js | 10 ++- js/service/ItemResource.js | 19 ++++- js/service/Resource.js | 4 +- js/tests/unit/controller/AppControllerSpec.js | 4 +- js/tests/unit/controller/ContentControllerSpec.js | 4 +- js/tests/unit/service/ItemResourceSpec.js | 24 +++++- js/tests/unit/service/ResourceSpec.js | 2 +- tests/unit/controller/ItemControllerTest.php | 92 +++++------------------ 15 files changed, 153 insertions(+), 166 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index fd9e513d3..36a3936f6 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -51,9 +51,7 @@ $application->registerRoutes($this, ['routes' => [ ['name' => 'item#new_items', 'url' => '/items/new', 'verb' => 'GET'], ['name' => 'item#readAll', 'url' => '/items/read', 'verb' => 'POST'], ['name' => 'item#read', 'url' => '/items/{itemId}/read', 'verb' => 'POST'], - ['name' => 'item#unread', 'url' => '/items/{itemId}/unread', 'verb' => 'POST'], ['name' => 'item#star', 'url' => '/items/{feedId}/{guidHash}/star', 'verb' => 'POST'], - ['name' => 'item#unstar', 'url' => '/items/{feedId}/{guidHash}/unstar', 'verb' => 'POST'], // export ['name' => 'export#opml', 'url' => '/export/opml', 'verb' => 'GET'], diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php index c27add3cb..486d0b922 100644 --- a/controller/itemcontroller.php +++ b/controller/itemcontroller.php @@ -32,8 +32,8 @@ class ItemController extends Controller { private $userId; private $settings; - public function __construct($appName, - IRequest $request, + public function __construct($appName, + IRequest $request, FeedService $feedService, ItemService $itemService, IConfig $settings, @@ -70,19 +70,19 @@ class ItemController extends Controller { try { // the offset is 0 if the user clicks on a new feed - // we need to pass the newest feeds to not let the unread count get + // we need to pass the newest feeds to not let the unread count get // out of sync if($offset === 0) { - $params['newestItemId'] = + $params['newestItemId'] = $this->itemService->getNewestItemId($this->userId); $params['feeds'] = $this->feedService->findAll($this->userId); $params['starred'] = $this->itemService->starredCount($this->userId); } - + $params['items'] = $this->itemService->findAll( $id, $type, $limit, $offset, $showAll, $this->userId, $oldestFirst ); - + // this gets thrown if there are no items // in that case just return an empty array } catch(ServiceException $ex) {} @@ -93,13 +93,13 @@ class ItemController extends Controller { /** * @NoAdminRequired - * + * * @param int $type * @param int $id * @param int $lastModified */ public function newItems($type, $id, $lastModified=0) { - $showAll = $this->settings->getUserValue($this->userId, $this->appName, + $showAll = $this->settings->getUserValue($this->userId, $this->appName, 'showAll') === '1'; $params = []; @@ -107,8 +107,8 @@ class ItemController extends Controller { try { $params['newestItemId'] = $this->itemService->getNewestItemId($this->userId); $params['feeds'] = $this->feedService->findAll($this->userId); - $params['starred'] = $this->itemService->starredCount($this->userId); - $params['items'] = $this->itemService->findAllNew($id, $type, + $params['starred'] = $this->itemService->starredCount($this->userId); + $params['items'] = $this->itemService->findAllNew($id, $type, $lastModified, $showAll, $this->userId); // this gets thrown if there are no items @@ -124,25 +124,12 @@ class ItemController extends Controller { * * @param int $feedId * @param string $guidHash + * @param bool $isStarred */ - public function star($feedId, $guidHash){ - try { - $this->itemService->star($feedId, $guidHash, true, $this->userId); - } catch(ServiceException $ex) { - return $this->error($ex, Http::STATUS_NOT_FOUND); - } - } - - - /** - * @NoAdminRequired - * - * @param int $feedId - * @param string $guidHash - */ - public function unstar($feedId, $guidHash){ + public function star($feedId, $guidHash, $isStarred){ try { - $this->itemService->star($feedId, $guidHash, false, $this->userId); + $this->itemService->star($feedId, $guidHash, $isStarred, + $this->userId); } catch(ServiceException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -153,10 +140,11 @@ class ItemController extends Controller { * @NoAdminRequired * * @param int $itemId + * @param bool $isRead */ - public function read($itemId){ + public function read($itemId, $isRead=true){ try { - $this->itemService->read($itemId, true, $this->userId); + $this->itemService->read($itemId, $isRead, $this->userId); } catch(ServiceException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -166,20 +154,6 @@ class ItemController extends Controller { /** * @NoAdminRequired * - * @param int $itemId - */ - public function unread($itemId){ - try { - $this->itemService->read($itemId, false, $this->userId); - } catch(ServiceException $ex) { - return $this->error($ex, Http::STATUS_NOT_FOUND); - } - } - - - /** - * @NoAdminRequired - * * @param int $highestItemId */ public function readAll($highestItemId){ diff --git a/db/item.php b/db/item.php index e7cbb7793..08c01932f 100644 --- a/db/item.php +++ b/db/item.php @@ -109,21 +109,22 @@ class Item extends Entity implements IAPI, \JsonSerializable { * Turns entitie attributes into an array */ public function jsonSerialize() { - return $this->serializeFields([ - 'id', - 'guidHash', - 'guid', - 'url', - 'title', - 'author', - 'pubDate', - 'body', - 'enclosureMime', - 'enclosureLink', - 'feedId', - 'status', - 'lastModified', - ]); + return [ + 'id' => $this->getId(), + 'guid' => $this->getGuid(), + 'guidHash' => $this->getGuidHash(), + 'url' => $this->getUrl(), + 'title' => $this->getTitle(), + 'author' => $this->getAuthor(), + 'pubDate' => $this->getPubDate(), + 'body' => $this->getBody(), + 'enclosureMime' => $this->getEnclosureMime(), + 'enclosureLink' => $this->getEnclosureLink(), + 'feedId' => $this->getFeedId(), + 'unread' => $this->isUnread(), + 'starred' => $this->isStarred(), + 'lastModified' => $this->getLastModified() + ]; } public function toAPI() { @@ -183,7 +184,7 @@ class Item extends Entity implements IAPI, \JsonSerializable { } else { $item->setUnstarred(); } - + return $item; } diff --git a/js/Gruntfile.js b/js/Gruntfile.js index b45385dc5..518627c9c 100644 --- a/js/Gruntfile.js +++ b/js/Gruntfile.js @@ -139,7 +139,7 @@ module.exports = function (grunt) { }, phpunit: { classes: { - dir: '../tests' + dir: '../tests/unit' }, options: { colors: true @@ -175,7 +175,7 @@ module.exports = function (grunt) { grunt.registerTask('default', ['jshint', 'concat', 'wrap', 'traceur', 'ngmin']); grunt.registerTask('dev', ['watch:concat']); grunt.registerTask('test', ['karma:unit']); - grunt.registerTask('phpunit', ['watch:phpunit']); + grunt.registerTask('php', ['watch:phpunit']); grunt.registerTask('e2e', ['protractor_webdriver', 'connect', 'protractor:chrome']); grunt.registerTask('ci-unit', ['default', 'karma:continuous']); grunt.registerTask('ci-e2e', ['protractor_webdriver', 'connect', 'protractor:phantomjs']); diff --git a/js/README.md b/js/README.md index 628e2c012..b529fbf6a 100644 --- a/js/README.md +++ b/js/README.md @@ -41,7 +41,7 @@ Single run mode: ## Testing Watch mode: - grunt phpunit + grunt php grunt test Single run mode: diff --git a/js/build/app.js b/js/build/app.js index 31ad58c70..eb88e72dc 100644 --- a/js/build/app.js +++ b/js/build/app.js @@ -202,42 +202,50 @@ var $__build_47_app__ = function () { app.factory('FeedResource', [ 'Resource', '$http', - function (Resource, $http) { + 'BASE_URL', + function (Resource, $http, BASE_URL) { 'use strict'; - var FeedResource = function FeedResource($http) { + var FeedResource = function FeedResource($http, BASE_URL) { $traceurRuntime.superCall(this, $FeedResource.prototype, 'constructor', [ $http, + BASE_URL, 'url' ]); }; var $FeedResource = FeedResource; $traceurRuntime.createClass(FeedResource, {}, {}, Resource); - return new FeedResource($http); + return new FeedResource($http, BASE_URL); } ]); app.factory('FolderResource', [ 'Resource', '$http', - function (Resource, $http) { + 'BASE_URL', + function (Resource, $http, BASE_URL) { 'use strict'; - var FolderResource = function FolderResource($http) { + var FolderResource = function FolderResource($http, BASE_URL) { $traceurRuntime.superCall(this, $FolderResource.prototype, 'constructor', [ $http, + BASE_URL, 'name' ]); }; var $FolderResource = FolderResource; $traceurRuntime.createClass(FolderResource, {}, {}, Resource); - return new FolderResource($http); + return new FolderResource($http, BASE_URL); } ]); app.factory('ItemResource', [ 'Resource', '$http', - function (Resource, $http) { + 'BASE_URL', + function (Resource, $http, BASE_URL) { 'use strict'; - var ItemResource = function ItemResource($http) { - $traceurRuntime.superCall(this, $ItemResource.prototype, 'constructor', [$http]); + var ItemResource = function ItemResource($http, BASE_URL) { + $traceurRuntime.superCall(this, $ItemResource.prototype, 'constructor', [ + $http, + BASE_URL + ]); }; var $ItemResource = ItemResource; $traceurRuntime.createClass(ItemResource, { @@ -261,9 +269,25 @@ var $__build_47_app__ = function () { }, getStarredCount: function () { return this.starredCount; + }, + markRead: function (itemId) { + var read = arguments[1] !== void 0 ? arguments[1] : true; + this.get(itemId).unread = !read; + }, + markFeedRead: function (feedId) { + for (var $item in this.values.filter(function (i) { + return i.feedId === feedId; + })) { + try { + throw undefined; + } catch (item) { + item = $item; + this.markRead(item); + } + } } }, {}, Resource); - return new ItemResource($http); + return new ItemResource($http, BASE_URL); } ]); app.service('Loading', function () { @@ -342,12 +366,13 @@ var $__build_47_app__ = function () { }); app.factory('Resource', function () { 'use strict'; - var Resource = function Resource(http) { - var id = arguments[1] !== void 0 ? arguments[1] : 'id'; + var Resource = function Resource(http, BASE_URL) { + var id = arguments[2] !== void 0 ? arguments[2] : 'id'; this.id = id; this.values = []; this.hashMap = {}; this.http = http; + this.BASE_URL = BASE_URL; }; $traceurRuntime.createClass(Resource, { receive: function (objs) { diff --git a/js/service/FeedResource.js b/js/service/FeedResource.js index 579396b9d..2eb9e4818 100644 --- a/js/service/FeedResource.js +++ b/js/service/FeedResource.js @@ -7,14 +7,16 @@ * @author Bernhard Posselt * @copyright Bernhard Posselt 2014 */ -app.factory('FeedResource', (Resource, $http) => { +app.factory('FeedResource', (Resource, $http, BASE_URL) => { 'use strict'; class FeedResource extends Resource { - constructor ($http) { - super($http, 'url'); + + constructor ($http, BASE_URL) { + super($http, BASE_URL, 'url'); } + } - return new FeedResource($http); + return new FeedResource($http, BASE_URL); }); \ No newline at end of file diff --git a/js/service/FolderResource.js b/js/service/FolderResource.js index 314900e37..1ea48e0bb 100644 --- a/js/service/FolderResource.js +++ b/js/service/FolderResource.js @@ -7,14 +7,16 @@ * @author Bernhard Posselt * @copyright Bernhard Posselt 2014 */ -app.factory('FolderResource', (Resource, $http) => { +app.factory('FolderResource', (Resource, $http, BASE_URL) => { 'use strict'; class FolderResource extends Resource { - constructor ($http) { - super($http, 'name'); + + constructor ($http, BASE_URL) { + super($http, BASE_URL, 'name'); } + } - return new FolderResource($http); + return new FolderResource($http, BASE_URL); }); \ No newline at end of file diff --git a/js/service/ItemResource.js b/js/service/ItemResource.js index 5750f2c6b..e6a8e1d2d 100644 --- a/js/service/ItemResource.js +++ b/js/service/ItemResource.js @@ -7,13 +7,13 @@ * @author Bernhard Posselt * @copyright Bernhard Posselt 2014 */ -app.factory('ItemResource', (Resource, $http) => { +app.factory('ItemResource', (Resource, $http, BASE_URL) => { 'use strict'; class ItemResource extends Resource { - constructor ($http) { - super($http); + constructor ($http, BASE_URL) { + super($http, BASE_URL); } receive (value, channel) { @@ -39,7 +39,18 @@ app.factory('ItemResource', (Resource, $http) => { return this.starredCount; } + markRead (itemId, read=true) { + this.get(itemId).unread = !read; + //http.get(); + } + + markFeedRead (feedId) { + for (let item in this.values.filter(i => i.feedId === feedId)) { + this.markRead(item); + } + } + } - return new ItemResource($http); + return new ItemResource($http, BASE_URL); }); \ No newline at end of file diff --git a/js/service/Resource.js b/js/service/Resource.js index f71126f02..7683099da 100644 --- a/js/service/Resource.js +++ b/js/service/Resource.js @@ -12,11 +12,12 @@ app.factory('Resource', () => { class Resource { - constructor (http, id='id') { + constructor (http, BASE_URL, id='id') { this.id = id; this.values = []; this.hashMap = {}; this.http = http; + this.BASE_URL = BASE_URL; } receive (objs) { @@ -74,6 +75,7 @@ app.factory('Resource', () => { getAll () { return this.values; } + } return Resource; diff --git a/js/tests/unit/controller/AppControllerSpec.js b/js/tests/unit/controller/AppControllerSpec.js index fc5780a40..772fdc565 100644 --- a/js/tests/unit/controller/AppControllerSpec.js +++ b/js/tests/unit/controller/AppControllerSpec.js @@ -12,7 +12,9 @@ describe('AppController', () => { let controller; - beforeEach(module('News')); + beforeEach(module('News', ($provide) => { + $provide.value('BASE_URL', 'base'); + })); beforeEach(inject(($controller) => { controller = $controller('AppController'); diff --git a/js/tests/unit/controller/ContentControllerSpec.js b/js/tests/unit/controller/ContentControllerSpec.js index 1dbd3d936..422bd21f4 100644 --- a/js/tests/unit/controller/ContentControllerSpec.js +++ b/js/tests/unit/controller/ContentControllerSpec.js @@ -10,7 +10,9 @@ describe('ContentController', () => { 'use strict'; - beforeEach(module('News')); + beforeEach(module('News', ($provide) => { + $provide.value('BASE_URL', 'base'); + })); it('should publish data to models', inject(($controller, Publisher, diff --git a/js/tests/unit/service/ItemResourceSpec.js b/js/tests/unit/service/ItemResourceSpec.js index acbf0850c..8d0f1864e 100644 --- a/js/tests/unit/service/ItemResourceSpec.js +++ b/js/tests/unit/service/ItemResourceSpec.js @@ -10,7 +10,9 @@ describe('ItemResource', () => { 'use strict'; - beforeEach(module('News')); + beforeEach(module('News', ($provide) => { + $provide.value('BASE_URL', 'base'); + })); it('should receive the newestItemId', inject((ItemResource) => { @@ -40,4 +42,24 @@ describe('ItemResource', () => { expect(ItemResource.size()).toBe(2); })); + + it ('should mark item as read', inject((ItemResource) => { + ItemResource.receive([ + { + id: 3, + feedId: 4, + unread: true + }, + { + id: 4, + feedId: 3, + unread: true + } + ], 'items'); + + ItemResource.markRead(3); + + expect(ItemResource.get(3).unread).toBe(false); + })); + }); \ No newline at end of file diff --git a/js/tests/unit/service/ResourceSpec.js b/js/tests/unit/service/ResourceSpec.js index 2fead3479..b1b8ed054 100644 --- a/js/tests/unit/service/ResourceSpec.js +++ b/js/tests/unit/service/ResourceSpec.js @@ -17,7 +17,7 @@ describe('Resource', () => { beforeEach(inject((Resource, $http) => { class ChildResource extends Resource { constructor ($http) { - super($http); + super($http, 'base'); } } diff --git a/tests/unit/controller/ItemControllerTest.php b/tests/unit/controller/ItemControllerTest.php index a80b77990..a09b7d297 100644 --- a/tests/unit/controller/ItemControllerTest.php +++ b/tests/unit/controller/ItemControllerTest.php @@ -44,11 +44,11 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { '\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); - $this->itemService = + $this->itemService = $this->getMockBuilder('\OCA\News\Service\ItemService') ->disableOriginalConstructor() ->getMock(); - $this->feedService = + $this->feedService = $this->getMockBuilder('\OCA\News\Service\FeedService') ->disableOriginalConstructor() ->getMock(); @@ -68,7 +68,7 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { ->method('read') ->with(4, true, $this->user); - $this->controller->read(4); + $this->controller->read(4, true); } @@ -87,42 +87,16 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { } - public function testUnread(){ - $this->itemService->expects($this->once()) - ->method('read') - ->with(4, false, $this->user); - - $this->controller->unread(4); - } - - - - public function testUnreadDoesNotExist(){ - $msg = 'hi'; - - $this->itemService->expects($this->once()) - ->method('read') - ->will($this->throwException(new ServiceNotFoundException($msg))); - - - $response = $this->controller->unread(4); - $params = json_decode($response->render(), true); - - $this->assertEquals($response->getStatus(), Http::STATUS_NOT_FOUND); - $this->assertEquals($msg, $params['message']); - } - - public function testStar(){ $this->itemService->expects($this->once()) ->method('star') ->with( - $this->equalTo(4), + $this->equalTo(4), $this->equalTo('test'), - $this->equalTo(true), + $this->equalTo(true), $this->equalTo($this->user)); - $this->controller->star(4, 'test'); + $this->controller->star(4, 'test', true); } @@ -133,35 +107,7 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { ->method('star') ->will($this->throwException(new ServiceNotFoundException($msg)));; - $response = $this->controller->star(4, 'test'); - $params = json_decode($response->render(), true); - - $this->assertEquals($response->getStatus(), Http::STATUS_NOT_FOUND); - $this->assertEquals($msg, $params['message']); - } - - - public function testUnstar(){ - $this->itemService->expects($this->once()) - ->method('star') - ->with( - $this->equalTo(4), - $this->equalTo('test'), - $this->equalTo(false), - $this->equalTo($this->user)); - - $this->controller->unstar(4, 'test'); - } - - - public function testUnstarDoesNotExist(){ - $msg = 'ho'; - - $this->itemService->expects($this->once()) - ->method('star') - ->will($this->throwException(new ServiceNotFoundException($msg)));; - - $response = $this->controller->unstar(4, 'test'); + $response = $this->controller->star(4, 'test', false); $params = json_decode($response->render(), true); $this->assertEquals($response->getStatus(), Http::STATUS_NOT_FOUND); @@ -176,7 +122,7 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { $this->itemService->expects($this->once()) ->method('readAll') - ->with($this->equalTo(5), + ->with($this->equalTo(5), $this->equalTo($this->user)); $this->feedService->expects($this->once()) ->method('findAll') @@ -245,11 +191,11 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { $this->itemService->expects($this->once()) ->method('findAll') ->with( - $this->equalTo(2), - $this->equalTo(FeedType::FEED), - $this->equalTo(3), + $this->equalTo(2), + $this->equalTo(FeedType::FEED), + $this->equalTo(3), $this->equalTo(0), - $this->equalTo(true), + $this->equalTo(true), $this->equalTo($this->user), $this->equalTo(false)) ->will($this->returnValue($result['items'])); @@ -266,11 +212,11 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { $this->itemService->expects($this->once()) ->method('findAll') - ->with($this->equalTo(2), - $this->equalTo(FeedType::FEED), - $this->equalTo(3), + ->with($this->equalTo(2), + $this->equalTo(FeedType::FEED), + $this->equalTo(3), $this->equalTo(10), - $this->equalTo(true), + $this->equalTo(true), $this->equalTo($this->user), $this->equalTo(true)) ->will($this->returnValue($result['items'])); @@ -330,10 +276,10 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase { $this->itemService->expects($this->once()) ->method('findAllNew') ->with( - $this->equalTo(2), - $this->equalTo(FeedType::FEED), + $this->equalTo(2), + $this->equalTo(FeedType::FEED), $this->equalTo(3), - $this->equalTo(true), + $this->equalTo(true), $this->equalTo($this->user)) ->will($this->returnValue($result['items'])); -- cgit v1.2.3