Commit 0a69ccd0 authored by Robert Knight's avatar Robert Knight

Fix 'Location' sort order for annotations

1398661 extracted the code for getting a location from an annotation
into a separate module. However that function expects an Annotation but
the sort predicate function was being passed a Thread.

Consequently the predicate function was returning
Number.POSITIVE_INFINITY for every thread and so sort ordering fell back
to the default 'tie-breaker' comparator - which is the original index of
the element in the list being sorted.

Fixes #3106
parent b1989cf8
...@@ -89,7 +89,9 @@ module.exports = function AppController( ...@@ -89,7 +89,9 @@ module.exports = function AppController(
predicateFn = ['-!!message', 'message.updated']; predicateFn = ['-!!message', 'message.updated'];
break; break;
case 'Location': case 'Location':
predicateFn = annotationMetadata.location; predicateFn = function (thread) {
return annotationMetadata.location(thread.message);
};
break; break;
} }
$scope.sort = { $scope.sort = {
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// ES2015 polyfills // ES2015 polyfills
require('core-js/es6/promise'); require('core-js/es6/promise');
require('core-js/fn/array/find'); require('core-js/fn/array/find');
require('core-js/fn/array/find-index');
require('core-js/fn/object/assign'); require('core-js/fn/object/assign');
// URL constructor, required by IE 10/11, // URL constructor, required by IE 10/11,
......
'use strict'; 'use strict';
var angular = require('angular'); var angular = require('angular');
var proxyquire = require('proxyquire');
var annotationFixtures = require('./annotation-fixtures');
var events = require('../events'); var events = require('../events');
var util = require('./util');
describe('AppController', function () { describe('AppController', function () {
var $controller = null; var $controller = null;
var $scope = null; var $scope = null;
var $rootScope = null; var $rootScope = null;
var fakeAnnotationMetadata = null;
var fakeAnnotationUI = null; var fakeAnnotationUI = null;
var fakeAuth = null; var fakeAuth = null;
var fakeDrafts = null; var fakeDrafts = null;
...@@ -28,9 +32,18 @@ describe('AppController', function () { ...@@ -28,9 +32,18 @@ describe('AppController', function () {
return $controller('AppController', locals); return $controller('AppController', locals);
}; };
before(function () { beforeEach(function () {
return angular.module('h', []) fakeAnnotationMetadata = {
.controller('AppController', require('../app-controller')) location: function () { return 0; },
};
var AppController = proxyquire('../app-controller', util.noCallThru({
'angular': angular,
'./annotation-metadata': fakeAnnotationMetadata,
}));
angular.module('h', [])
.controller('AppController', AppController)
.controller('AnnotationUIController', angular.noop); .controller('AnnotationUIController', angular.noop);
}); });
...@@ -263,4 +276,24 @@ describe('AppController', function () { ...@@ -263,4 +276,24 @@ describe('AppController', function () {
assert.equal(fakeWindow.confirm.callCount, 0); assert.equal(fakeWindow.confirm.callCount, 0);
}); });
}); });
describe('sorting', function () {
function annotationThread() {
return {message: annotationFixtures.defaultAnnotation()};
}
it('sorts threads by location when sort name is "Location"', function () {
var threads = [annotationThread(), annotationThread()];
fakeAnnotationMetadata.location = function (annotation) {
return threads.findIndex(function (thread) {
return thread.message === annotation;
});
};
createController();
$scope.sort.name = 'Location';
$scope.$digest();
assert.equal($scope.sort.predicate(threads[0]), 0);
assert.equal($scope.sort.predicate(threads[1]), 1);
});
});
}); });
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment