From 3fc18156ae0b586e8de0c82949acfa6291317536 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Mon, 29 Apr 2013 13:25:04 +0200 Subject: 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 --- js/app/services/businesslayer/businesslayer.coffee | 2 +- .../businesslayer/feedbusinesslayer.coffee | 11 ++-- .../businesslayer/itembusinesslayer.coffee | 7 +-- js/app/services/models/itemmodel.coffee | 14 ++++- js/app/services/persistence.coffee | 22 +------- js/public/app.js | 66 ++++++++-------------- js/tests/controllers/itemcontrollerSpec.coffee | 12 ++-- .../businesslayer/feedbusinesslayerSpec.coffee | 6 ++ .../businesslayer/itembusinesslayerSpec.coffee | 2 +- js/tests/services/models/itemmodelSpec.coffee | 9 +++ js/tests/services/persistenceSpec.coffee | 18 +----- 11 files changed, 67 insertions(+), 102 deletions(-) (limited to 'js') 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 . ### 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 . 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 . }; 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 . }; 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 . __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 . 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 . this.getAllFolders(); this.getAllFeeds(); this.userSettingsRead(); - this.getStarredItems(); return this.userSettingsLanguage(); }; @@ -2424,13 +2430,10 @@ License along with this library. If not, see . */ - 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 . 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 . 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}) diff --git a/js/tests/services/businesslayer/itembusinesslayerSpec.coffee b/js/tests/services/businesslayer/itembusinesslayerSpec.coffee index 4b8d520ee..8a30c6808 100644 --- a/js/tests/services/businesslayer/itembusinesslayerSpec.coffee +++ b/js/tests/services/businesslayer/itembusinesslayerSpec.coffee @@ -247,4 +247,4 @@ describe 'ItemBusinessLayer', -> @ItemBusinessLayer.loadNext(callback) expect(@persistence.getItems).toHaveBeenCalledWith( - @FeedType.Feed, 3, 4, 13, jasmine.any(Function)) + @FeedType.Feed, 3, 1, jasmine.any(Function)) diff --git a/js/tests/services/models/itemmodelSpec.coffee b/js/tests/services/models/itemmodelSpec.coffee index b4a74d122..3999467c9 100644 --- a/js/tests/services/models/itemmodelSpec.coffee +++ b/js/tests/services/models/itemmodelSpec.coffee @@ -102,3 +102,12 @@ describe 'ItemModel', -> item.setUnstarred() expect(@ItemModel.getById(3).isStarred()).toBe(false) + + + it 'should return the lowest id', => + @ItemModel.add({id: 2, guidHash: 'abc', feedId: 2, status: 16}) + @ItemModel.add({id: 3, guidHash: 'abcd', feedId: 2, status: 16}) + @ItemModel.add({id: 1, guidHash: 'abce', feedId: 2, status: 16}) + @ItemModel.add({id: 6, guidHash: 'abcf', feedId: 2, status: 16}) + + expect(@ItemModel.getLowestId()).toBe(1) \ No newline at end of file diff --git a/js/tests/services/persistenceSpec.coffee b/js/tests/services/persistenceSpec.coffee index b0a057d16..e42d3bb17 100644 --- a/js/tests/services/persistenceSpec.coffee +++ b/js/tests/services/persistenceSpec.coffee @@ -56,12 +56,10 @@ describe 'Persistence', -> id: 5 limit: @config.itemBatchSize offset: 3 - newestItemId: 4 onSuccess: -> @Persistence.getItems(params.data.type, params.data.id, - params.data.offset, params.data.newestItemId, - params.onSuccess) + params.data.offset, params.onSuccess) expected = onSuccess: jasmine.any(Function) @@ -71,24 +69,10 @@ describe 'Persistence', -> id: 5 limit: @config.itemBatchSize offset: 3 - newestItemId: 4 expect(@req.get).toHaveBeenCalledWith('news_items', expected) - it 'send a correct get starred items request', => - params = - onSuccess: -> - - @Persistence.getStarredItems(params.onSuccess) - - expected = - onSuccess: jasmine.any(Function) - onFailure: jasmine.any(Function) - - expect(@req.get).toHaveBeenCalledWith('news_items_starred', expected) - - it 'send a correct star item request', => params = routeParams: -- cgit v1.2.3