Commit 347dcefa authored by Robert Knight's avatar Robert Knight

Preserve selection when switching accounts

When visiting a direct-linked annotation (ie. #annotations:<id> is
present in the URL) we want to preserve the selection when the user
signs in, whether the annotation was private or in a group, in which
case they will need to sign in to see it, or public, in which case they
might need to sign in to reply.

Previously public direct-linked annotations became deselected when
signing in because they were unloaded when switching accounts, and this
fired the same ANNOTATION_DELETED event as when an annotation is
removed, which results in the annotation being removed from the
selection.

This commit enables the selection to be preserved by introducing a
different event within the app when annotations are unloaded vs.
deleted. When an annotation is unloaded, it is not removed from the
selection.
parent 54dea463
...@@ -42,14 +42,14 @@ function annotationMapper($rootScope, threading, store) { ...@@ -42,14 +42,14 @@ function annotationMapper($rootScope, threading, store) {
} }
function unloadAnnotations(annotations) { function unloadAnnotations(annotations) {
annotations.forEach(function (annotation) { var unloaded = annotations.map(function (annotation) {
var container = getContainer(threading, annotation); var container = getContainer(threading, annotation);
if (container !== null && annotation !== container.message) { if (container !== null && annotation !== container.message) {
annotation = angular.copy(annotation, container.message); annotation = angular.copy(annotation, container.message);
} }
return annotation;
$rootScope.$emit(events.ANNOTATION_DELETED, annotation);
}); });
$rootScope.$emit(events.ANNOTATIONS_UNLOADED, unloaded);
} }
function createAnnotation(annotation) { function createAnnotation(annotation) {
......
...@@ -130,6 +130,15 @@ module.exports = class AnnotationSync ...@@ -130,6 +130,15 @@ module.exports = class AnnotationSync
return unless bodies.length return unless bodies.length
@bridge.call('loadAnnotations', bodies) @bridge.call('loadAnnotations', bodies)
'annotationsUnloaded': (annotations) ->
self = this
annotations.forEach (annotation) ->
# In the client, unloading an annotation is handled the same way as
# deleting an annotation. Within the app however, we handle the events
# differently in some cases
delete self.cache[annotation.$$tag]
self._mkCallRemotelyAndParseResults('deleteAnnotation')(annotation)
_syncCache: (channel) -> _syncCache: (channel) ->
# Synchronise (here to there) the items in our cache # Synchronise (here to there) the items in our cache
annotations = (this._format a for t, a of @cache) annotations = (this._format a for t, a of @cache)
......
...@@ -40,4 +40,7 @@ module.exports = { ...@@ -40,4 +40,7 @@ module.exports = {
/** A set of annotations were loaded from the server. */ /** A set of annotations were loaded from the server. */
ANNOTATIONS_LOADED: 'annotationsLoaded', ANNOTATIONS_LOADED: 'annotationsLoaded',
/** An annotation is unloaded. */
ANNOTATIONS_UNLOADED: 'annotationsUnloaded',
}; };
...@@ -38,7 +38,7 @@ describe('annotationMapper', function() { ...@@ -38,7 +38,7 @@ describe('annotationMapper', function() {
sandbox.restore(); sandbox.restore();
}); });
describe('.loadAnnotations()', function () { describe('#loadAnnotations()', function () {
it('triggers the annotationLoaded event', function () { it('triggers the annotationLoaded event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$emit');
var annotations = [{id: 1}, {id: 2}, {id: 3}]; var annotations = [{id: 1}, {id: 2}, {id: 3}];
...@@ -108,14 +108,13 @@ describe('annotationMapper', function() { ...@@ -108,14 +108,13 @@ describe('annotationMapper', function() {
}); });
}); });
describe('.unloadAnnotations()', function () { describe('#unloadAnnotations()', function () {
it('triggers the annotationDeleted event', function () { it('triggers the annotationsUnloaded event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$emit');
var annotations = [{id: 1}, {id: 2}, {id: 3}]; var annotations = [{id: 1}, {id: 2}, {id: 3}];
annotationMapper.unloadAnnotations(annotations); annotationMapper.unloadAnnotations(annotations);
annotations.forEach(function (annot) { assert.calledWith($rootScope.$emit,
assert.calledWith($rootScope.$emit, events.ANNOTATION_DELETED, annot); events.ANNOTATIONS_UNLOADED, annotations);
});
}); });
it('replaces the properties on the cached annotation with those from the deleted one', function () { it('replaces the properties on the cached annotation with those from the deleted one', function () {
...@@ -125,14 +124,14 @@ describe('annotationMapper', function() { ...@@ -125,14 +124,14 @@ describe('annotationMapper', function() {
fakeThreading.idTable[1] = cached; fakeThreading.idTable[1] = cached;
annotationMapper.unloadAnnotations(annotations); annotationMapper.unloadAnnotations(annotations);
assert.calledWith($rootScope.$emit, events.ANNOTATION_DELETED, { assert.calledWith($rootScope.$emit, events.ANNOTATIONS_UNLOADED, [{
id: 1, id: 1,
url: 'http://example.com' url: 'http://example.com'
}); }]);
}); });
}); });
describe('.createAnnotation()', function () { describe('#createAnnotation()', function () {
it('creates a new annotaton resource', function () { it('creates a new annotaton resource', function () {
var ann = {}; var ann = {};
fakeStore.AnnotationResource.returns(ann); fakeStore.AnnotationResource.returns(ann);
...@@ -157,7 +156,7 @@ describe('annotationMapper', function() { ...@@ -157,7 +156,7 @@ describe('annotationMapper', function() {
}); });
}); });
describe('.deleteAnnotation()', function () { describe('#deleteAnnotation()', function () {
it('deletes the annotation on the server', function () { it('deletes the annotation on the server', function () {
var p = Promise.resolve(); var p = Promise.resolve();
var ann = {$delete: sandbox.stub().returns(p)}; var ann = {$delete: sandbox.stub().returns(p)};
......
...@@ -300,6 +300,23 @@ describe 'AnnotationSync', -> ...@@ -300,6 +300,23 @@ describe 'AnnotationSync', ->
assert.notCalled(fakeBridge.call) assert.notCalled(fakeBridge.call)
describe 'the "annotationsUnloaded" event', ->
it 'sends calls to delete the annotations over the bridge', ->
ann = {id: 1, $$tag: 'tag1'}
annSync = createAnnotationSync()
annSync.cache.tag1 = ann
options.emit('annotationsUnloaded', [ann])
assert.calledWith(fakeBridge.call, 'deleteAnnotation',
{msg: ann, tag: ann.$$tag}, sinon.match.func)
it 'removes the annotations from the cache', ->
ann = {id: 1, $$tag: 'tag1'}
annSync = createAnnotationSync()
annSync.cache.tag1 = ann
options.emit('annotationsUnloaded', [ann])
assert.isUndefined(annSync.cache.tag1)
describe 'the "annotationsLoaded" event', -> describe 'the "annotationsLoaded" event', ->
it 'formats the provided annotations', -> it 'formats the provided annotations', ->
annotations = [{id: 1}, {id: 2}, {id: 3}] annotations = [{id: 1}, {id: 2}, {id: 3}]
......
{module, inject} = angular.mock {module, inject} = angular.mock
events = require('../events')
threading = require('../threading')
describe 'Threading', -> describe 'Threading', ->
instance = null instance = null
$rootScope = null
before -> before ->
angular.module('h', []) angular.module('h', [])
...@@ -9,8 +13,16 @@ describe 'Threading', -> ...@@ -9,8 +13,16 @@ describe 'Threading', ->
beforeEach module('h') beforeEach module('h')
beforeEach inject (_threading_) -> beforeEach inject (_threading_, _$rootScope_) ->
instance = _threading_ instance = _threading_
$rootScope = _$rootScope_
describe 'when an annotation is unloaded', ->
it 'removes the annotation from the thread', ->
$rootScope.$broadcast(events.ANNOTATIONS_LOADED, [{id: 'a'}])
assert.equal(instance.root.children.length, 1)
$rootScope.$broadcast(events.ANNOTATIONS_UNLOADED, [{id: 'a'}])
assert.equal(instance.root.children.length, 0)
describe 'pruneEmpties', -> describe 'pruneEmpties', ->
it 'keeps public messages with no children', -> it 'keeps public messages with no children', ->
......
...@@ -36,6 +36,7 @@ module.exports = class Threading ...@@ -36,6 +36,7 @@ module.exports = class Threading
$rootScope.$on(events.ANNOTATION_CREATED, this.annotationCreated) $rootScope.$on(events.ANNOTATION_CREATED, this.annotationCreated)
$rootScope.$on(events.ANNOTATION_DELETED, this.annotationDeleted) $rootScope.$on(events.ANNOTATION_DELETED, this.annotationDeleted)
$rootScope.$on(events.ANNOTATIONS_LOADED, this.annotationsLoaded) $rootScope.$on(events.ANNOTATIONS_LOADED, this.annotationsLoaded)
$rootScope.$on(events.ANNOTATIONS_UNLOADED, this.annotationsUnloaded)
# TODO: Refactor the jwz API for progressive updates. # TODO: Refactor the jwz API for progressive updates.
# Right now the idTable is wiped when `messageThread.thread()` is called and # Right now the idTable is wiped when `messageThread.thread()` is called and
...@@ -118,6 +119,10 @@ module.exports = class Threading ...@@ -118,6 +119,10 @@ module.exports = class Threading
this.pruneEmpties(@root) this.pruneEmpties(@root)
break break
annotationsUnloaded: (event, annotations) =>
for annot in annotations
this.annotationDeleted(event, annot)
annotationsLoaded: (event, annotations) => annotationsLoaded: (event, annotations) =>
messages = (@root.flattenChildren() or []).concat(annotations) messages = (@root.flattenChildren() or []).concat(annotations)
this.thread(messages) this.thread(messages)
...@@ -179,7 +179,9 @@ module.exports = function WidgetController( ...@@ -179,7 +179,9 @@ module.exports = function WidgetController(
// case, searchClients.length > 0), as a result of switching to the group // case, searchClients.length > 0), as a result of switching to the group
// containing the selected annotation. // containing the selected annotation.
// //
// In that case, we don't want to trigger reloading annotations again. // In that case, we don't want to trigger reloading annotations again and we
// also want to preserve the selection if the user visits a direct link to a
// group annotation whilst signed out, then signs in.
if (searchClients.length) { if (searchClients.length) {
return; return;
} }
......
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