diff options
-rw-r--r-- | controller/feedcontroller.php | 9 | ||||
-rw-r--r-- | controller/itemcontroller.php | 11 | ||||
-rw-r--r-- | js/app/services/models/feedmodel.coffee | 29 | ||||
-rw-r--r-- | js/public/app.js | 20 | ||||
-rw-r--r-- | js/tests/services/businesslayer/subsriptionsbusinesslayerSpec.coffee | 4 | ||||
-rw-r--r-- | js/tests/services/models/feedmodelSpec.coffee | 16 | ||||
-rw-r--r-- | tests/unit/controller/FeedControllerTest.php | 12 | ||||
-rw-r--r-- | tests/unit/controller/ItemControllerTest.php | 9 | ||||
-rw-r--r-- | utility/feedfetcher.php | 3 |
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)){ |