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

11 files changed, 67 insertions, 102 deletions

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})
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: