summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTucker McKnight <tucker.mcknight@gmail.com>2021-02-08 23:11:33 -0700
committerBenjamin Brahmer <info@b-brahmer.de>2021-02-22 19:44:33 +0100
commit5ba6f04bae27136430ab2a5c35c05e221538ec0c (patch)
tree31a76b675061b117ed8af863ec97cf39ed61904d
parent5c18b971bf8a37a549e41e21683fa00d460b2bd0 (diff)
Fix incorrect article sorting
Currently, an article with an ID of 1000 will show up earlier than one with ID = 900, because the IDs are being treated as strings and compared alphabetically. (1 comes before 9, and the comparison stops there.) We need to parse them as integers to ensure that 900 is less than 1000. Signed-off-by: Tucker McKnight <tucker.mcknight@gmail.com>
-rw-r--r--CHANGELOG.md1
-rw-r--r--js/controller/ContentController.js69
-rw-r--r--js/tests/unit/controller/ContentControllerSpec.js38
-rw-r--r--templates/part.content.php2
4 files changed, 54 insertions, 56 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c7b5cd099..c791d6534 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1
### Fixed
- Item list throwing error for folder and "all items"
+- Articles with high IDs can be placed lower than articles with low IDs (#1147)
## [15.3.2] - 2021-02-10
No changes compared to RC2
diff --git a/js/controller/ContentController.js b/js/controller/ContentController.js
index 74c475f4c..c87362557 100644
--- a/js/controller/ContentController.js
+++ b/js/controller/ContentController.js
@@ -8,8 +8,7 @@
* @copyright Bernhard Posselt 2014
*/
app.controller('ContentController', function (Publisher, FeedResource, ItemResource, SettingsResource, data, $route,
- $routeParams, $location, FEED_TYPE, ITEM_AUTO_PAGE_SIZE, Loading,
- $filter) {
+ $routeParams, $location, FEED_TYPE, ITEM_AUTO_PAGE_SIZE, Loading) {
'use strict';
var self = this;
@@ -18,14 +17,36 @@ app.controller('ContentController', function (Publisher, FeedResource, ItemResou
// distribute data to models based on key
Publisher.publishAll(data);
+ var getOrdering = function () {
+ var ordering = SettingsResource.get('oldestFirst');
+
+ if (self.isFeed()) {
+ var feed = FeedResource.getById($routeParams.id);
+ if (feed && feed.ordering === 1) {
+ ordering = true;
+ } else if (feed && feed.ordering === 2) {
+ ordering = false;
+ }
+ }
+
+ return ordering;
+ };
+
this.getFirstItem = function () {
- var orderFilter = $filter('orderBy');
- var orderedItems = orderFilter(this.getItems(), this.orderBy());
+ var orderedItems = this.getItems();
+ var item = orderedItems[orderedItems.length - 1];
var firstItem = orderedItems[0];
- if (firstItem === undefined) {
+ // If getOrdering == 1, then the sorting is set to
+ // newest first. So, item should be the first item
+ //
+ if (getOrdering()) {
+ item = firstItem;
+ }
+ if (item === undefined) {
return undefined;
- } else {
- return firstItem.id;
+ }
+ else {
+ return item.id;
}
};
@@ -85,28 +106,11 @@ app.controller('ContentController', function (Publisher, FeedResource, ItemResou
item.keepUnread = !item.keepUnread;
};
-
- var getOrdering = function () {
- var ordering = SettingsResource.get('oldestFirst');
-
- if (self.isFeed()) {
- var feed = FeedResource.getById($routeParams.id);
- if (feed && feed.ordering === 1) {
- ordering = true;
- } else if (feed && feed.ordering === 2) {
- ordering = false;
- }
- }
-
- return ordering;
- };
-
- this.orderBy = function () {
- if (getOrdering()) {
- return 'id';
- } else {
- return '-id';
- }
+
+ this.sortIds = function(first, second) {
+ var firstInt = parseInt(first.value);
+ var secondInt = parseInt(second.value);
+ return (firstInt < secondInt) ? 1 : -1;
};
this.isCompactView = function () {
@@ -147,6 +151,8 @@ app.controller('ContentController', function (Publisher, FeedResource, ItemResou
return $route.current.$$route.type === FEED_TYPE.FEED;
};
+ this.oldestFirst = getOrdering();
+
this.autoPage = function () {
if (this.isNothingMoreToAutoPage) {
return;
@@ -165,14 +171,13 @@ app.controller('ContentController', function (Publisher, FeedResource, ItemResou
var type = $route.current.$$route.type;
var id = $routeParams.id;
- var oldestFirst = getOrdering();
var showAll = SettingsResource.get('showAll');
var self = this;
var search = $location.search().search;
Loading.setLoading('autopaging', true);
- ItemResource.autoPage(type, id, oldestFirst, showAll, search).then(function (response) {
+ ItemResource.autoPage(type, id, this.oldestFirst, showAll, search).then(function (response) {
Publisher.publishAll(response.data);
if (response.data.items.length >= ITEM_AUTO_PAGE_SIZE) {
@@ -216,4 +221,4 @@ app.controller('ContentController', function (Publisher, FeedResource, ItemResou
};
this.activeItem = this.getFirstItem();
-}); \ No newline at end of file
+});
diff --git a/js/tests/unit/controller/ContentControllerSpec.js b/js/tests/unit/controller/ContentControllerSpec.js
index 2e49e74db..e8992ca8c 100644
--- a/js/tests/unit/controller/ContentControllerSpec.js
+++ b/js/tests/unit/controller/ContentControllerSpec.js
@@ -64,27 +64,19 @@ describe('ContentController', function () {
}));
- it('should return order by',
- inject(function ($controller, SettingsResource, FEED_TYPE) {
- var route = {
- current: {
- $$route: {
- type: FEED_TYPE.FOLDER
- }
- }
- };
-
- var ctrl = $controller('ContentController', {
- data: {},
- $route: route
- });
-
- expect(ctrl.orderBy()).toBe('-id');
-
- SettingsResource.set('oldestFirst', true);
+ it('should sort feed items', inject(function ($controller) {
+ var ctrl = $controller('ContentController', {
+ data: {}
+ });
+ var first = {value: 11, type: 'number'};
+ var second = {value: 12, type: 'number'};
+ var third = {value: 101, type: 'number'};
+ expect(ctrl.sortIds(first, second)).toBe(1);
+ expect(ctrl.sortIds(second, first)).toBe(-1);
+ expect(ctrl.sortIds(second, second)).toBe(-1);
+ expect(ctrl.sortIds(first, third)).toBe(1);
+ }));
- expect(ctrl.orderBy()).toBe('id');
- }));
it('should return order if custom ordering',
inject(function ($controller, SettingsResource, FeedResource,
@@ -107,11 +99,11 @@ describe('ContentController', function () {
}
});
- expect(ctrl.orderBy()).toBe('id');
+ expect(ctrl.oldestFirst).toBe(true);
- SettingsResource.set('oldestFirst', true);
+ SettingsResource.set('oldestFirst', false);
- expect(ctrl.orderBy()).toBe('id');
+ expect(ctrl.oldestFirst).toBe(true);
}));
diff --git a/templates/part.content.php b/templates/part.content.php
index 120e9173a..9ce6ae10e 100644
--- a/templates/part.content.php
+++ b/templates/part.content.php
@@ -19,7 +19,7 @@
<ul>
<li class="item {{ ::Content.getFeed(item.feedId).cssClass }}"
ng-repeat="item in Content.getItems() |
- orderBy:[Content.orderBy()] track by item.id"
+ orderBy:'id':Content.oldestFirst:Content.sortIds track by item.id"
ng-mouseup="Content.markRead(item.id)"
ng-click="Content.markRead(item.id); Content.setItemActive(item.id)"
news-on-active="Content.setItemActive(item.id)"