Commit 5a04f66c authored by Robert Knight's avatar Robert Knight

Switch to the group containing the selected annotation

When the sidebar loads as a result of an '#annotations'
fragment, we do not know which group that annotation is in.

Handle this by first fetching annotations in all groups,
then finding the group containing the selected annotation(s)
and finally filtering the results by that group.

An alternative approach would be to first fetch the
selected annotations in order to determine the group and
then fetching annotations in that group.

This approach however avoids an extra round-trip to the server
in the common case where the total number of annotations across
all groups is less than the default chunk size (200).
parent c81fea3c
......@@ -2,6 +2,7 @@
// ES2015 polyfills
require('core-js/es6/promise');
require('core-js/fn/array/find');
require('core-js/fn/object/assign');
// URL constructor, required by IE 10/11,
......
......@@ -12,17 +12,19 @@ function noCallThru(stub) {
}
var searchClients;
function FakeSearchClient(resource) {
function FakeSearchClient(resource, opts) {
assert.ok(resource);
searchClients.push(this);
this.cancel = sinon.stub();
this.incremental = !!opts.incremental;
this.get = function (query) {
this.get = sinon.spy(function (query) {
assert.ok(query.uri);
this.emit('results', [{id: query.uri + '123', group: '__world__'}]);
this.emit('results', [{id: query.uri + '456', group: 'private-group'}]);
this.emit('end');
};
});
}
inherits(FakeSearchClient, EventEmitter);
......@@ -142,6 +144,69 @@ describe('WidgetController', function () {
assert.calledWith(loadSpy, [sinon.match({id: uris[1] + '123'})]);
assert.calledWith(loadSpy, [sinon.match({id: uris[1] + '456'})]);
});
context('when there is a selection', function () {
var uri = 'http://example.com';
var id = uri + '123';
beforeEach(function () {
fakeCrossFrame.frames = [{uri: uri}];
fakeAnnotationUI.selectedAnnotationMap[id] = true;
$scope.$digest();
});
it('switches to the selected annotation\'s group', function () {
assert.calledWith(fakeGroups.focus, '__world__');
assert.calledOnce(fakeAnnotationMapper.loadAnnotations);
assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
{id: uri + '123', group: '__world__'},
]);
});
it('fetches annotations for all groups', function () {
assert.calledWith(searchClients[0].get, {uri: uri, group: null});
});
it('loads annotations in one batch', function () {
assert.notOk(searchClients[0].incremental);
});
});
context('when there is no selection', function () {
var uri = 'http://example.com';
beforeEach(function () {
fakeCrossFrame.frames = [{uri: uri}];
fakeGroups.focused = function () { return { id: 'a-group' }; };
$scope.$digest();
});
it('fetches annotations for the current group', function () {
assert.calledWith(searchClients[0].get, {uri: uri, group: 'a-group'});
});
it('loads annotations in batches', function () {
assert.ok(searchClients[0].incremental);
});
});
context('when the selected annotation is not available', function () {
var uri = 'http://example.com';
var id = uri + 'does-not-exist';
beforeEach(function () {
fakeCrossFrame.frames = [{uri: uri}];
fakeAnnotationUI.selectedAnnotationMap[id] = true;
fakeGroups.focused = function () { return { id: 'private-group' }; };
$scope.$digest();
});
it('loads annotations from the focused group instead', function () {
assert.calledWith(fakeGroups.focus, 'private-group');
assert.calledWith(fakeAnnotationMapper.loadAnnotations,
[{group: "private-group", id: "http://example.com456"}]);
});
});
});
describe('when the focused group changes', function () {
......
......@@ -5,6 +5,21 @@ var angular = require('angular');
var events = require('./events');
var SearchClient = require('./search-client');
/**
* Returns the group ID of the first annotation in @p results whose
* ID is a key in @p selection.
*/
function groupIDFromSelection(selection, results) {
var id = Object.keys(selection)[0];
var annot = results.find(function (annot) {
return annot.id === id;
});
if (!annot) {
return;
}
return annot.group;
}
// @ngInject
module.exports = function WidgetController(
$scope, $rootScope, annotationUI, crossframe, annotationMapper,
......@@ -13,28 +28,46 @@ module.exports = function WidgetController(
$scope.threadRoot = threading.root;
$scope.sortOptions = ['Newest', 'Oldest', 'Location'];
var _resetAnnotations = function () {
function _resetAnnotations() {
// Unload all the annotations
annotationMapper.unloadAnnotations(threading.annotationList());
// Reload all the drafts
threading.thread(drafts.unsaved());
};
}
var searchClients = [];
function _loadAnnotationsFor(uri, group) {
var searchClient = new SearchClient(store.SearchResource);
var searchClient = new SearchClient(store.SearchResource, {
// If no group is specified, we are fetching annotations from
// all groups in order to find out which group contains the selected
// annotation, therefore we need to load all chunks before processing
// the results
incremental: !!group,
});
searchClients.push(searchClient);
searchClient.on('results', function (results) {
annotationMapper.loadAnnotations(results);
if (annotationUI.hasSelectedAnnotations()) {
var groupID = groupIDFromSelection(annotationUI.selectedAnnotationMap,
results);
if (!groupID) {
// If the selected annotation is not available, fall back to
// loading annotations for the currently focused group
groupID = groups.focused().id;
}
groups.focus(groupID);
results = results.filter(function (result) {
return result.group === groupID;
});
}
if (results.length) {
annotationMapper.loadAnnotations(results);
}
});
searchClient.on('end', function () {
searchClients.splice(searchClients.indexOf(searchClient), 1);
});
searchClient.get({
uri: uri,
group: group,
});
searchClient.get({uri: uri, group: group});
}
/**
......@@ -56,8 +89,22 @@ module.exports = function WidgetController(
}
}, []);
// If there is no selection, load annotations only for the focused group.
//
// If there is a selection, we load annotations for all groups, find out
// which group the first selected annotation is in and then filter the
// results on the client by that group.
//
// In the common case where the total number of annotations on
// a page that are visible to the user is not greater than
// the batch size, this saves an extra roundtrip to the server
// to fetch the selected annotation in order to determine which group
// it is in before fetching the remaining annotations.
var group = annotationUI.hasSelectedAnnotations() ?
null : groups.focused().id;
for (var i=0; i < urls.length; i++) {
_loadAnnotationsFor(urls[i], groups.focused().id);
_loadAnnotationsFor(urls[i], group);
}
if (urls.length > 0) {
......@@ -67,8 +114,17 @@ module.exports = function WidgetController(
};
$scope.$on(events.GROUP_FOCUSED, function () {
if (searchClients.length) {
// If the current group changes as a _result_ of loading annotations,
// avoid trying to load annotations again.
//
// If however the group changes because of a user action during
// loading of annotations, we should cancel the current load.
return;
}
annotationUI.clearSelectedAnnotations();
_resetAnnotations(annotationMapper, drafts, threading);
_resetAnnotations();
return loadAnnotations(crossframe.frames);
});
......
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