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>
This commit is contained in:
parent
5c18b971bf
commit
5ba6f04bae
|
@ -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
|
||||
|
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
@ -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
|
||||
}
|
||||
}
|
||||
};
|
||||
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);
|
||||
}));
|
||||
|
||||
var ctrl = $controller('ContentController', {
|
||||
data: {},
|
||||
$route: route
|
||||
});
|
||||
|
||||
expect(ctrl.orderBy()).toBe('-id');
|
||||
|
||||
SettingsResource.set('oldestFirst', true);
|
||||
|
||||
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);
|
||||
}));
|
||||
|
||||
|
||||
|
|
|
@ -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)"
|
||||
|
|
Loading…
Reference in New Issue