diff options
author | Bernhard Posselt <nukeawhale@gmail.com> | 2013-04-29 13:25:04 +0200 |
---|---|---|
committer | Bernhard Posselt <nukeawhale@gmail.com> | 2013-04-29 13:30:01 +0200 |
commit | 3fc18156ae0b586e8de0c82949acfa6291317536 (patch) | |
tree | 201e97511bea44c58b5e9d78d8cf36ae2e2b54f9 | |
parent | a03b54c6a59837d0045c140ea7aef3fae95daa95 (diff) |
go back to order by id, fix #138, use a newest item id to prevent marking items as read that the user didnt see yet fix #141, also update the starred count periodically
25 files changed, 266 insertions, 257 deletions
@@ -6,4 +6,5 @@ Bernhard Posselt <nukeawhale@gmail.com> Contributors: Raghu Nayyar <me@iraghu.com> -Jan-Christoph Borchardt (http://jancborchardt.net)
\ No newline at end of file +Jan-Christoph Borchardt (http://jancborchardt.net) +Robin Appelman <icewind@owncloud.com>
\ No newline at end of file diff --git a/appinfo/routes.php b/appinfo/routes.php index f6b05ed01..fd7b6ad20 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -141,12 +141,6 @@ $this->create('news_items', '/items')->get()->action( } ); -$this->create('news_items_starred', '/items/starred')->get()->action( - function($params){ - App::main('ItemController', 'starred', $params, new DIContainer()); - } -); - $this->create('news_items_read', '/items/{itemId}/read')->post()->action( function($params){ App::main('ItemController', 'read', $params, new DIContainer()); diff --git a/businesslayer/itembusinesslayer.php b/businesslayer/itembusinesslayer.php index 794da73b7..72323f413 100644 --- a/businesslayer/itembusinesslayer.php +++ b/businesslayer/itembusinesslayer.php @@ -49,8 +49,8 @@ class ItemBusinessLayer extends BusinessLayer { } - public function findAllNew($id, $type, $updatedSince, - $showAll, $userId){ + public function findAllNew($id, $type, $updatedSince, $showAll, $userId){ + $status = $this->statusFlag->typeToStatus($type, $showAll); switch($type){ @@ -71,36 +71,28 @@ class ItemBusinessLayer extends BusinessLayer { } - public function findAll($id, $type, $limit, $offset, $newestItemId, $showAll, - $userId){ - + public function findAll($id, $type, $limit, $offset, + $showAll, $userId){ $status = $this->statusFlag->typeToStatus($type, $showAll); switch($type){ case FeedType::FEED: $items = $this->mapper->findAllFeed($id, $limit, $offset, - $newestItemId, $status, - $userId); + $status, $userId); break; case FeedType::FOLDER: $items = $this->mapper->findAllFolder($id, $limit, $offset, - $newestItemId, $status, - $userId); + $status, $userId); break; default: - $items = $this->mapper->findAll($limit, $offset, $newestItemId, - $status, $userId); + $items = $this->mapper->findAll($limit, $offset, $status, + $userId); } return $items; } - public function starredCount($userId){ - return $this->mapper->starredCount($userId); - } - - public function star($feedId, $guidHash, $isStarred, $userId){ // FIXME: this can throw two possible exceptions $item = $this->mapper->findByGuidHash($guidHash, $feedId, $userId); @@ -151,4 +143,8 @@ class ItemBusinessLayer extends BusinessLayer { } + public function starredCount($userId){ + return $this->mapper->starredCount($userId); + } + } diff --git a/controller/feedcontroller.php b/controller/feedcontroller.php index 09f638170..d19008b5a 100644 --- a/controller/feedcontroller.php +++ b/controller/feedcontroller.php @@ -29,6 +29,7 @@ use \OCA\AppFramework\Controller\Controller; use \OCA\AppFramework\Core\API; use \OCA\AppFramework\Http\Request; +use \OCA\News\BusinessLayer\ItemBusinessLayer; use \OCA\News\BusinessLayer\FeedBusinessLayer; use \OCA\News\BusinessLayer\FolderBusinessLayer; use \OCA\News\BusinessLayer\BusinessLayerException; @@ -39,13 +40,16 @@ class FeedController extends Controller { private $feedBusinessLayer; private $folderBusinessLayer; + private $itemBusinessLayer; public function __construct(API $api, Request $request, FeedBusinessLayer $feedBusinessLayer, - FolderBusinessLayer $folderBusinessLayer){ + FolderBusinessLayer $folderBusinessLayer, + ItemBusinessLayer $itemBusinessLayer){ parent::__construct($api, $request); $this->feedBusinessLayer = $feedBusinessLayer; $this->folderBusinessLayer = $folderBusinessLayer; + $this->itemBusinessLayer = $itemBusinessLayer; } @@ -56,12 +60,20 @@ class FeedController extends Controller { */ public function feeds(){ $userId = $this->api->getUserId(); - $result = $this->feedBusinessLayer->findAll($userId); + // this method is also used to update the interface + // because of this we also pass the starred count and the newest + // item id which will be used for marking feeds read $params = array( - 'feeds' => $result + 'feeds' => $this->feedBusinessLayer->findAll($userId), + 'starred' => $this->itemBusinessLayer->starredCount($userId) ); + try { + $params['newestItemId'] = + $this->itemBusinessLayer->getNewestItemId($userId); + } catch (BusinessLayerException $ex) {} + return $this->renderJSON($params); } diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php index 459435a7e..a4d9abf3f 100644 --- a/controller/itemcontroller.php +++ b/controller/itemcontroller.php @@ -61,7 +61,6 @@ class ItemController extends Controller { $type = (int) $this->params('type'); $id = (int) $this->params('id'); $offset = (int) $this->params('offset', 0); - $newestItemId = (int) $this->params('newestItemId'); $this->api->setUserValue('lastViewedFeedId', $id); $this->api->setUserValue('lastViewedFeedType', $type); @@ -76,14 +75,12 @@ class ItemController extends Controller { if($offset === 0) { $params['newestItemId'] = $this->itemBusinessLayer->getNewestItemId($userId); - $newestItemId = $params['newestItemId']; $params['feeds'] = $this->feedBusinessLayer->findAll($userId); + $params['starred'] = $this->itemBusinessLayer->starredCount($userId); } - $params['items'] = $this->itemBusinessLayer->findAll( - $id, $type, $limit, - $offset, $newestItemId, - $showAll, $userId); + $params['items'] = $this->itemBusinessLayer->findAll($id, $type, $limit, + $offset, $showAll, $userId); // this gets thrown if there are no items // in that case just return an empty array } catch(BusinessLayerException $ex) {} @@ -92,23 +89,6 @@ class ItemController extends Controller { } - /** - * @IsAdminExemption - * @IsSubAdminExemption - * @Ajax - */ - public function starred(){ - $userId = $this->api->getUserId(); - $starredCount = $this->itemBusinessLayer->starredCount($userId); - - $params = array( - 'starred' => (int) $starredCount - ); - - return $this->renderJSON($params); - } - - private function setStarred($isStarred){ $userId = $this->api->getUserId(); $feedId = (int) $this->params('feedId'); diff --git a/db/itemmapper.php b/db/itemmapper.php index ec6d2389e..097479ecc 100644 --- a/db/itemmapper.php +++ b/db/itemmapper.php @@ -110,7 +110,7 @@ class ItemMapper extends Mapper implements IMapper { $result = $this->execute($sql, $params)->fetchRow(); - return $result['size']; + return (int) $result['size']; } @@ -164,40 +164,42 @@ class ItemMapper extends Mapper implements IMapper { } - public function findAll($limit, $offset, $newestItemId, $status, $userId){ - $sql = 'AND `items`.`id` < ? '; - $sql .= 'ORDER BY `items`.`pub_date` DESC, `items`.`id` DESC '; - $params = array($userId, $newestItemId); - + public function findAllFeed($id, $limit, $offset, $status, $userId){ + $params = array($userId, $id); + $sql = 'AND `items`.`feed_id` = ? '; + if($offset !== 0){ + $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, $offset); + return $this->findAllRows($sql, $params, $limit); } - public function findAllFolder($id, $limit, $offset, $newestItemId, $status, - $userId){ - - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`id` < ? ' . - 'ORDER BY `items`.`pub_date` DESC, `items`.`id` DESC '; - $params = array($userId, $id, $newestItemId); - + public function findAllFolder($id, $limit, $offset, $status, $userId){ + $params = array($userId, $id); + $sql = 'AND `feeds`.`folder_id` = ? '; + if($offset !== 0){ + $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, $offset); + return $this->findAllRows($sql, $params, $limit); } - public function findAllFeed($id, $limit, $offset, $newestItemId, $status, - $userId){ - - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`id` < ? ' . - 'ORDER BY `items`.`pub_date` DESC, `items`.`id` DESC '; + public function findAll($limit, $offset, $status, $userId){ + $params = array($userId); + $sql = ''; + if($offset !== 0){ + $sql .= 'AND `items`.`id` < ? '; + array_push($params, $offset); + } + $sql .= 'ORDER BY `items`.`id` DESC '; $sql = $this->makeSelectQueryStatus($sql, $status); - $params = array($userId, $id, $newestItemId); - - return $this->findAllRows($sql, $params, $limit, $offset); + return $this->findAllRows($sql, $params, $limit); } diff --git a/dependencyinjection/dicontainer.php b/dependencyinjection/dicontainer.php index 1c3b5f378..db647e852 100644 --- a/dependencyinjection/dicontainer.php +++ b/dependencyinjection/dicontainer.php @@ -92,7 +92,8 @@ class DIContainer extends BaseContainer { $this['FeedController'] = $this->share(function($c){ return new FeedController($c['API'], $c['Request'], - $c['FeedBusinessLayer'], $c['FolderBusinessLayer']); + $c['FeedBusinessLayer'], $c['FolderBusinessLayer'], + $c['ItemBusinessLayer']); }); $this['ItemController'] = $this->share(function($c){ diff --git a/js/app/services/businesslayer/businesslayer.coffee b/js/app/services/businesslayer/businesslayer.coffee index 622351333..3096eb71e 100644 --- a/js/app/services/businesslayer/businesslayer.coffee +++ b/js/app/services/businesslayer/businesslayer.coffee @@ -25,7 +25,7 @@ angular.module('News').factory '_BusinessLayer', -> class BusinessLayer - constructor: (@_activeFeed, @_persistence, @_itemModel, @_type, @_newest) -> + constructor: (@_activeFeed, @_persistence, @_itemModel, @_type) -> load: (id) -> diff --git a/js/app/services/businesslayer/feedbusinesslayer.coffee b/js/app/services/businesslayer/feedbusinesslayer.coffee index 796a6a6eb..d7b78a75b 100644 --- a/js/app/services/businesslayer/feedbusinesslayer.coffee +++ b/js/app/services/businesslayer/feedbusinesslayer.coffee @@ -66,14 +66,11 @@ FeedModel, NewLoading, _ExistsError, Utils, $rootScope, UndoQueue, NewestItem)-> markFeedRead: (feedId) -> feed = @_feedModel.getById(feedId) - if angular.isDefined(feed) + newestItemId = @_newestItem.getId() + + if angular.isDefined(feed) and angular.isDefined(newestItemId) feed.unreadCount = 0 - if @_activeFeed.getId() == feedId and - @_activeFeed.getType() == @_feedType.Feed - highestItemId = @_newestItem.getId() - else - highestItemId = 0 - @_persistence.setFeedRead(feedId, highestItemId) + @_persistence.setFeedRead(feedId, newestItemId) for item in @_itemModel.getAll() item.setRead() diff --git a/js/app/services/businesslayer/itembusinesslayer.coffee b/js/app/services/businesslayer/itembusinesslayer.coffee index 7a2063a97..d9b5951ac 100644 --- a/js/app/services/businesslayer/itembusinesslayer.coffee +++ b/js/app/services/businesslayer/itembusinesslayer.coffee @@ -106,12 +106,11 @@ StarredBusinessLayer, NewestItem) -> loadNext: (callback) -> - size = @_itemModel.size() - if size != 0 + lowestItemId = @_itemModel.getLowestId() + if lowestItemId != 0 @_persistence.getItems @_activeFeed.getType(), @_activeFeed.getId(), - size, - @_newestItem.getId(), + lowestItemId, callback else callback() diff --git a/js/app/services/models/itemmodel.coffee b/js/app/services/models/itemmodel.coffee index 12f557421..1104db4b3 100644 --- a/js/app/services/models/itemmodel.coffee +++ b/js/app/services/models/itemmodel.coffee @@ -21,8 +21,8 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. ### angular.module('News').factory 'ItemModel', -['_Model', '_MaximumQuery', '_MinimumQuery', 'StatusFlag', -(_Model, _MaximumQuery, _MinimumQuery, StatusFlag) -> +['_Model', '_MinimumQuery', 'StatusFlag', +(_Model, _MinimumQuery, StatusFlag) -> class ItemModel extends _Model @@ -99,5 +99,15 @@ angular.module('News').factory 'ItemModel', super(id) + getLowestId: -> + query = new _MinimumQuery('id') + lowestId = @get(query) + + if angular.isDefined(lowestId) + return lowestId.id + else + return 0 + + return new ItemModel() ]
\ No newline at end of file diff --git a/js/app/services/persistence.coffee b/js/app/services/persistence.coffee index 92ced046b..341cf66f6 100644 --- a/js/app/services/persistence.coffee +++ b/js/app/services/persistence.coffee @@ -44,14 +44,13 @@ $rootScope) -> @getAllFolders() @getAllFeeds() @userSettingsRead() - @getStarredItems() @userSettingsLanguage() ### ITEM CONTROLLER ### - getItems: (type, id, offset, newestItemId=0, onSuccess=null) -> + getItems: (type, id, offset, onSuccess=null) -> onSuccess or= -> # show different loading signs @@ -74,31 +73,12 @@ $rootScope) -> offset: offset id: id type: type - newestItemId: newestItemId onSuccess: successCallbackWrapper onFailure: failureCallbackWrapper @_request.get 'news_items', params - getStarredItems: (onSuccess) -> - onSuccess or= -> - - # loading sign handling - @_feedLoading.increase() - successCallbackWrapper = (data) => - onSuccess() - @_feedLoading.decrease() - failureCallbackWrapper = (data) => - @_feedLoading.decrease() - - params = - onSuccess: successCallbackWrapper - onFailure: failureCallbackWrapper - - @_request.get 'news_items_starred', params - - starItem: (feedId, guidHash) -> ### Stars an item diff --git a/js/public/app.js b/js/public/app.js index ab12599c9..bc9a9b7b0 100644 --- a/js/public/app.js +++ b/js/public/app.js @@ -751,12 +751,11 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. var BusinessLayer; BusinessLayer = (function() { - function BusinessLayer(_activeFeed, _persistence, _itemModel, _type, _newest) { + function BusinessLayer(_activeFeed, _persistence, _itemModel, _type) { this._activeFeed = _activeFeed; this._persistence = _persistence; this._itemModel = _itemModel; this._type = _type; - this._newest = _newest; } BusinessLayer.prototype.load = function(id) { @@ -857,17 +856,13 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. }; FeedBusinessLayer.prototype.markFeedRead = function(feedId) { - var feed, highestItemId, item, _i, _len, _ref, _results; + var feed, item, newestItemId, _i, _len, _ref, _results; feed = this._feedModel.getById(feedId); - if (angular.isDefined(feed)) { + newestItemId = this._newestItem.getId(); + if (angular.isDefined(feed) && angular.isDefined(newestItemId)) { feed.unreadCount = 0; - if (this._activeFeed.getId() === feedId && this._activeFeed.getType() === this._feedType.Feed) { - highestItemId = this._newestItem.getId(); - } else { - highestItemId = 0; - } - this._persistence.setFeedRead(feedId, highestItemId); + this._persistence.setFeedRead(feedId, newestItemId); _ref = this._itemModel.getAll(); _results = []; for (_i = 0, _len = _ref.length; _i < _len; _i++) { @@ -1400,11 +1395,11 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. }; ItemBusinessLayer.prototype.loadNext = function(callback) { - var size; + var lowestItemId; - size = this._itemModel.size(); - if (size !== 0) { - return this._persistence.getItems(this._activeFeed.getType(), this._activeFeed.getId(), size, this._newestItem.getId(), callback); + lowestItemId = this._itemModel.getLowestId(); + if (lowestItemId !== 0) { + return this._persistence.getItems(this._activeFeed.getType(), this._activeFeed.getId(), lowestItemId, callback); } else { return callback(); } @@ -2101,7 +2096,7 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. __extends = function(child, parent) { for (var key in parent) { if (__hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; }; angular.module('News').factory('ItemModel', [ - '_Model', '_MaximumQuery', '_MinimumQuery', 'StatusFlag', function(_Model, _MaximumQuery, _MinimumQuery, StatusFlag) { + '_Model', '_MinimumQuery', 'StatusFlag', function(_Model, _MinimumQuery, StatusFlag) { var ItemModel; ItemModel = (function(_super) { @@ -2192,6 +2187,18 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. return ItemModel.__super__.removeById.call(this, id); }; + ItemModel.prototype.getLowestId = function() { + var lowestId, query; + + query = new _MinimumQuery('id'); + lowestId = this.get(query); + if (angular.isDefined(lowestId)) { + return lowestId.id; + } else { + return 0; + } + }; + return ItemModel; })(_Model); @@ -2415,7 +2422,6 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. this.getAllFolders(); this.getAllFeeds(); this.userSettingsRead(); - this.getStarredItems(); return this.userSettingsLanguage(); }; @@ -2424,13 +2430,10 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. */ - Persistence.prototype.getItems = function(type, id, offset, newestItemId, onSuccess) { + Persistence.prototype.getItems = function(type, id, offset, onSuccess) { var failureCallbackWrapper, loading, params, successCallbackWrapper, _this = this; - if (newestItemId == null) { - newestItemId = 0; - } if (onSuccess == null) { onSuccess = null; } @@ -2453,8 +2456,7 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. limit: this._config.itemBatchSize, offset: offset, id: id, - type: type, - newestItemId: newestItemId + type: type }, onSuccess: successCallbackWrapper, onFailure: failureCallbackWrapper @@ -2462,26 +2464,6 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>. return this._request.get('news_items', params); }; - Persistence.prototype.getStarredItems = function(onSuccess) { - var failureCallbackWrapper, params, successCallbackWrapper, - _this = this; - - onSuccess || (onSuccess = function() {}); - this._feedLoading.increase(); - successCallbackWrapper = function(data) { - onSuccess(); - return _this._feedLoading.decrease(); - }; - failureCallbackWrapper = function(data) { - return _this._feedLoading.decrease(); - }; - params = { - onSuccess: successCallbackWrapper, - onFailure: failureCallbackWrapper - }; - return this._request.get('news_items_starred', params); - }; - Persistence.prototype.starItem = function(feedId, guidHash) { /* Stars an item diff --git a/js/tests/controllers/itemcontrollerSpec.coffee b/js/tests/controllers/itemcontrollerSpec.coffee index 3a7e6712a..fe1e85c0b 100644 --- a/js/tests/controllers/itemcontrollerSpec.coffee +++ b/js/tests/controllers/itemcontrollerSpec.coffee @@ -101,7 +101,7 @@ describe 'ItemController', -> expect(@persistence.getItems).not.toHaveBeenCalled() - it 'should autoPage with the newest Item Id', => + it 'should autoPage with the lowest Item Id', => @NewestItem.handle(25) @persistence.getItems = jasmine.createSpy('getItems') @@ -116,12 +116,11 @@ describe 'ItemController', -> @scope.$broadcast 'autoPage' expect(@persistence.getItems).toHaveBeenCalledWith( - @FeedType.Folder, 3, 3, 25, jasmine.any(Function) + @FeedType.Folder, 3, 3, jasmine.any(Function) ) it 'should not prevent autopaging if there are no items', => - @NewestItem.handle(25) @scope.$broadcast 'autoPage' @persistence.getItems = jasmine.createSpy('getItems') @@ -130,12 +129,11 @@ describe 'ItemController', -> @scope.$broadcast 'autoPage' expect(@persistence.getItems).toHaveBeenCalledWith( - @FeedType.Folder, 3, 1, 25, jasmine.any(Function) + @FeedType.Folder, 3, 3, jasmine.any(Function) ) it 'should not send multiple autopage requests at once', => - @NewestItem.handle(25) @persistence.getItems = jasmine.createSpy('getItems') item1 = {id: 3, guidHash: 'abcd', feedId: 3} @ItemModel.add(item1) @@ -148,14 +146,14 @@ describe 'ItemController', -> @scope.$broadcast 'autoPage' expect(@persistence.getItems).not.toHaveBeenCalledWith( - @FeedType.Folder, 2, 1, 25, jasmine.any(Function) + @FeedType.Folder, 2, 3, jasmine.any(Function) ) it 'should allow another autopaging request if the last one finished', => @NewestItem.handle(25) @persistence.getItems = jasmine.createSpy('getItems') - @persistence.getItems.andCallFake (type, id, offset, newestItemId, + @persistence.getItems.andCallFake (type, id, offset, onSuccess) -> onSuccess() diff --git a/js/tests/services/businesslayer/feedbusinesslayerSpec.coffee b/js/tests/services/businesslayer/feedbusinesslayerSpec.coffee index 1c663d279..feb687b2c 100644 --- a/js/tests/services/businesslayer/feedbusinesslayerSpec.coffee +++ b/js/tests/services/businesslayer/feedbusinesslayerSpec.coffee @@ -93,6 +93,12 @@ describe 'FeedBusinessLayer', -> expect(count).toBe(169) + it 'should not mark feed read when no highest item id', => + @persistence.setFeedRead = jasmine.createSpy('setFeedRead') + @FeedBusinessLayer.markFeedRead(5) + expect(@persistence.setFeedRead).not.toHaveBeenCalled() + + it 'should mark feed as read', => @NewestItem.handle(25) @ActiveFeed.handle({type: @FeedType.Feed, id: 5}) |