always return the unreadcount when marking read to set update request as 0, dont create new feeds that only consist of unreadcount updates

9 files changed, 83 insertions, 30 deletions

diff --git a/controller/feedcontroller.php b/controller/feedcontroller.php
index 53257d73c..11cb60d5e 100644
--- a/controller/feedcontroller.php
+++ b/controller/feedcontroller.php
@@ -162,7 +162,14 @@ class FeedController extends Controller {
 			$feed = $this->feedBusinessLayer->update($feedId, $userId);
 
 			$params = array(
-				'feeds' => array($feed)
+				'feeds' => array(
+					// only pass unreadcount to not accidentally readd
+					// the feed again
+					array(
+						'id' => $feed->getId(),
+						'unreadCount' => $feed->getUnreadCount()
+					)
+				)
 			);
 
 			return $this->renderJSON($params);
diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php
index 5387f40a2..9081578ed 100644
--- a/controller/itemcontroller.php
+++ b/controller/itemcontroller.php
@@ -169,7 +169,16 @@ class ItemController extends Controller {
 		$highestItemId = (int) $this->params('highestItemId');
 
 		$this->itemBusinessLayer->readFeed($feedId, $highestItemId, $userId);
-		return $this->renderJSON();
+
+		$params = array(
+			'feeds' => array(
+				array(
+					'id' => $feedId,
+					'unreadCount' => 0
+				)
+			)
+		);
+		return $this->renderJSON($params);
 	}
 
 
diff --git a/js/app/services/models/feedmodel.coffee b/js/app/services/models/feedmodel.coffee
index fb0ba8e14..f0296957e 100644
--- a/js/app/services/models/feedmodel.coffee
+++ b/js/app/services/models/feedmodel.coffee
@@ -62,24 +62,27 @@ angular.module('News').factory 'FeedModel',
 			if updateById or updateByUrlHash
 				@update(data, clearCache)
 			else
-				# if the item is not yet in the name cache it must be added
-				@_urlHash[data.urlHash] = data
-				
-				# in case there is an id it can go through the normal add method
-				if angular.isDefined(data.id)
-					super(data, clearCache)
-
-				# if there is no id we just want it to appear in the list
-				else
-					@_data.push(data)
-					if clearCache
-						@_invalidateCache()
+				if angular.isDefined(data.urlHash)
+					# if the item is not yet in the name cache it must be added
+					@_urlHash[data.urlHash] = data
+					
+					# in case there is an id it can go through the normal add method
+					if angular.isDefined(data.id)
+						super(data, clearCache)
+
+					# if there is no id we just want it to appear in the list
+					else
+						@_data.push(data)
+						if clearCache
+							@_invalidateCache()
 
 
 		update: (data, clearCache=true) ->
 			# only when the id on the updated item does not exist we wish
 			# to update by name, otherwise we always update by id
-			item = @_urlHash[data.urlHash]
+			if angular.isDefined(data.urlHash)
+				item = @_urlHash[data.urlHash]
+
 			# update by name
 			if angular.isUndefined(data.id)	and angular.isDefined(item)
 				angular.extend(item, data)
diff --git a/js/public/app.js b/js/public/app.js
index 624c4575d..15504d914 100644
--- a/js/public/app.js
+++ b/js/public/app.js
@@ -1617,13 +1617,15 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
           if (updateById || updateByUrlHash) {
             return this.update(data, clearCache);
           } else {
-            this._urlHash[data.urlHash] = data;
-            if (angular.isDefined(data.id)) {
-              return FeedModel.__super__.add.call(this, data, clearCache);
-            } else {
-              this._data.push(data);
-              if (clearCache) {
-                return this._invalidateCache();
+            if (angular.isDefined(data.urlHash)) {
+              this._urlHash[data.urlHash] = data;
+              if (angular.isDefined(data.id)) {
+                return FeedModel.__super__.add.call(this, data, clearCache);
+              } else {
+                this._data.push(data);
+                if (clearCache) {
+                  return this._invalidateCache();
+                }
               }
             }
           }
@@ -1635,7 +1637,9 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
           if (clearCache == null) {
             clearCache = true;
           }
-          item = this._urlHash[data.urlHash];
+          if (angular.isDefined(data.urlHash)) {
+            item = this._urlHash[data.urlHash];
+          }
           if (angular.isUndefined(data.id) && angular.isDefined(item)) {
             return angular.extend(item, data);
           } else {
diff --git a/js/tests/services/businesslayer/subsriptionsbusinesslayerSpec.coffee b/js/tests/services/businesslayer/subsriptionsbusinesslayerSpec.coffee
index 8ab904c0a..48ae9e03c 100644
--- a/js/tests/services/businesslayer/subsriptionsbusinesslayerSpec.coffee
+++ b/js/tests/services/businesslayer/subsriptionsbusinesslayerSpec.coffee
@@ -40,7 +40,7 @@ describe 'SubscriptionsBusinessLayer', ->
 
 
 	it 'should be visible shows all items is set to true and there are feeds', =>
-		@FeedModel.add({id: 3, unreadCount: 5})
+		@FeedModel.add({id: 3, unreadCount: 5, urlHash: 'hi'})
 
 		expect(@SubscriptionsBusinessLayer.isVisible()).toBe(true)
 
@@ -66,7 +66,7 @@ describe 'SubscriptionsBusinessLayer', ->
 
 
 	it 'should mark all feeds as read', =>
-		item = {id: 3, unreadCount: 132}
+		item = {id: 3, unreadCount: 132, urlHash: 'hi'}
 		@FeedModel.add(item)
 
 		@SubscriptionsBusinessLayer.markAllRead()
diff --git a/js/tests/services/models/feedmodelSpec.coffee b/js/tests/services/models/feedmodelSpec.coffee
index 6df9d6e9f..6b6eb85f6 100644
--- a/js/tests/services/models/feedmodelSpec.coffee
+++ b/js/tests/services/models/feedmodelSpec.coffee
@@ -120,4 +120,18 @@ describe 'FeedModel', ->
 		item2 = {faviconLink: null, urlHash: 'his', test: 'heheh', folderId: 0}
 		@FeedModel.add(item2)
 
-		expect(@FeedModel.getAllOfFolder(0).length).toBe(2)
\ No newline at end of file
+		expect(@FeedModel.getAllOfFolder(0).length).toBe(2)
+
+
+	it 'should only update feeds that contain only an id but no url hash', =>
+		item = {id: 3, unreadCount: 232}
+		@FeedModel.add(item)
+		expect(@FeedModel.size()).toBe(0)
+
+		item2 = {id: 3, unreadCount: 2, faviconLink: null, urlHash: 'his'}
+		@FeedModel.add(item2)
+		@FeedModel.add(item)
+
+		expect(@FeedModel.size()).toBe(1)
+		expect(@FeedModel.getById(3).unreadCount).toBe(232)
+
diff --git a/tests/unit/controller/FeedControllerTest.php b/tests/unit/controller/FeedControllerTest.php
index c7c69fb03..428ecc7d4 100644
--- a/tests/unit/controller/FeedControllerTest.php
+++ b/tests/unit/controller/FeedControllerTest.php
@@ -298,14 +298,20 @@ class FeedControllerTest extends ControllerTestUtility {
 
 
 	public function testUpdate(){
+		$feed = new Feed();
+		$feed->setId(3);
+		$feed->setUnreadCount(44);
 		$result = array(
 			'feeds' => array(
-				new Feed()
+				array(
+					'id' => $feed->getId(),
+					'unreadCount' => $feed->getUnreadCount()
+				)
 			)
 		);
 
 		$url = array(
-				'feedId' => 4
+			'feedId' => 4
 		);
 		$this->controller = $this->getPostController(array(), $url);
 
@@ -315,7 +321,7 @@ class FeedControllerTest extends ControllerTestUtility {
 		$this->feedBusinessLayer->expects($this->once())
 			->method('update')
 			->with($this->equalTo($url['feedId']), $this->equalTo($this->user))
-			->will($this->returnValue($result['feeds'][0]));
+			->will($this->returnValue($feed));
 
 		$response = $this->controller->update();
 
diff --git a/tests/unit/controller/ItemControllerTest.php b/tests/unit/controller/ItemControllerTest.php
index 953330855..417997d30 100644
--- a/tests/unit/controller/ItemControllerTest.php
+++ b/tests/unit/controller/ItemControllerTest.php
@@ -197,6 +197,14 @@ class ItemControllerTest extends ControllerTestUtility {
 			'highestItemId' => 5
 		);
 		$this->controller = $this->getPostController($post, $url);
+		$expected = array(
+			'feeds' => array(
+				array(
+					'id' => 4,
+					'unreadCount' => 0
+				)
+			)
+		);
 
 		$this->api->expects($this->once())
 			->method('getUserId')
@@ -207,6 +215,7 @@ class ItemControllerTest extends ControllerTestUtility {
 
 		$response = $this->controller->readFeed();
 		$this->assertTrue($response instanceof JSONResponse);
+		$this->assertEquals($expected, $response->getParams());
 	}
 
 
diff --git a/utility/feedfetcher.php b/utility/feedfetcher.php
index f48839029..9dc13bde9 100644
--- a/utility/feedfetcher.php
+++ b/utility/feedfetcher.php
@@ -177,12 +177,13 @@ class FeedFetcher implements IFeedFetcher {
 
 						// remove //
 						$favicon = ltrim($favicon, '/');
+						$httpsFavicon = $favicon;
 
 						// if it does not start with http, add it
 						if (strpos($favicon, 'http') !== 0){
 							$favicon = 'http://' . $favicon;
 							$httpsFavicon = 'https://' . $favicon;
-						} 
+						}
 
 						// if its already valid, return it
 						if ($this->isValidFavIcon($favicon)){