Commit d2418ea3 authored by Alice Wyan's avatar Alice Wyan

Fix incorrect usages of the $rootScope

As described on the trello card:

https://trello.com/c/2NHJkRLa/407-audit-client-for-incorrect-rootscope-usage

there are components or controllers subscribing to events directly on
the root scope via `$rootScope.$on()`. This patch moves those hooks to
the correct `$scope` and causes those events to be `$broadcast` instead
of `$emit` to the root scope.
parent dbd16d1d
...@@ -21,13 +21,13 @@ function annotationMapper($rootScope, annotationUI, store) { ...@@ -21,13 +21,13 @@ function annotationMapper($rootScope, annotationUI, store) {
var existing = getExistingAnnotation(annotationUI, annotation.id); var existing = getExistingAnnotation(annotationUI, annotation.id);
if (existing) { if (existing) {
angular.copy(annotation, existing); angular.copy(annotation, existing);
$rootScope.$emit(events.ANNOTATION_UPDATED, existing); $rootScope.$broadcast(events.ANNOTATION_UPDATED, existing);
return; return;
} }
loaded.push(annotation); loaded.push(annotation);
}); });
$rootScope.$emit(events.ANNOTATIONS_LOADED, loaded); $rootScope.$broadcast(events.ANNOTATIONS_LOADED, loaded);
} }
function unloadAnnotations(annotations) { function unloadAnnotations(annotations) {
...@@ -38,11 +38,11 @@ function annotationMapper($rootScope, annotationUI, store) { ...@@ -38,11 +38,11 @@ function annotationMapper($rootScope, annotationUI, store) {
} }
return annotation; return annotation;
}); });
$rootScope.$emit(events.ANNOTATIONS_UNLOADED, unloaded); $rootScope.$broadcast(events.ANNOTATIONS_UNLOADED, unloaded);
} }
function createAnnotation(annotation) { function createAnnotation(annotation) {
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, annotation); $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED, annotation);
return annotation; return annotation;
} }
...@@ -50,7 +50,7 @@ function annotationMapper($rootScope, annotationUI, store) { ...@@ -50,7 +50,7 @@ function annotationMapper($rootScope, annotationUI, store) {
return store.annotation.delete({ return store.annotation.delete({
id: annotation.id, id: annotation.id,
}).then(function () { }).then(function () {
$rootScope.$emit(events.ANNOTATION_DELETED, annotation); $rootScope.$broadcast(events.ANNOTATION_DELETED, annotation);
return annotation; return annotation;
}); });
} }
......
...@@ -20,7 +20,7 @@ function AnnotationUIController($rootScope, $scope, annotationUI) { ...@@ -20,7 +20,7 @@ function AnnotationUIController($rootScope, $scope, annotationUI) {
$scope.focusedAnnotations = state.focusedAnnotationMap; $scope.focusedAnnotations = state.focusedAnnotationMap;
}); });
$rootScope.$on(events.ANNOTATION_DELETED, function (event, annotation) { $scope.$on(events.ANNOTATION_DELETED, function (event, annotation) {
annotationUI.removeSelectedAnnotation(annotation.id); annotationUI.removeSelectedAnnotation(annotation.id);
}); });
} }
......
...@@ -125,16 +125,7 @@ function AnnotationController( ...@@ -125,16 +125,7 @@ function AnnotationController(
// When a new annotation is created, remove any existing annotations that // When a new annotation is created, remove any existing annotations that
// are empty. // are empty.
// $scope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty);
// This event is currently emitted with $emit rather than $broadcast so
// we have to listen for it on the $rootScope and manually de-register
// on destruction.
var removeNewAnnotListener =
$rootScope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty);
vm.$onDestroy = function () {
removeNewAnnotListener();
};
// Call `onGroupFocused()` whenever the currently-focused group changes. // Call `onGroupFocused()` whenever the currently-focused group changes.
$scope.$on(events.GROUP_FOCUSED, onGroupFocused); $scope.$on(events.GROUP_FOCUSED, onGroupFocused);
...@@ -207,7 +198,7 @@ function AnnotationController( ...@@ -207,7 +198,7 @@ function AnnotationController(
vm.annotation.permissions = permissions.private(); vm.annotation.permissions = permissions.private();
save(vm.annotation).then(function(model) { save(vm.annotation).then(function(model) {
model.$$tag = vm.annotation.$$tag; model.$$tag = vm.annotation.$$tag;
$rootScope.$emit(events.ANNOTATION_CREATED, model); $rootScope.$broadcast(events.ANNOTATION_CREATED, model);
}); });
} else { } else {
// User isn't logged in, save to drafts. // User isn't logged in, save to drafts.
...@@ -382,7 +373,7 @@ function AnnotationController( ...@@ -382,7 +373,7 @@ function AnnotationController(
vm.revert = function() { vm.revert = function() {
drafts.remove(vm.annotation); drafts.remove(vm.annotation);
if (isNew(vm.annotation)) { if (isNew(vm.annotation)) {
$rootScope.$emit(events.ANNOTATION_DELETED, vm.annotation); $rootScope.$broadcast(events.ANNOTATION_DELETED, vm.annotation);
} }
}; };
...@@ -411,7 +402,7 @@ function AnnotationController( ...@@ -411,7 +402,7 @@ function AnnotationController(
events.ANNOTATION_CREATED : events.ANNOTATION_UPDATED; events.ANNOTATION_CREATED : events.ANNOTATION_UPDATED;
drafts.remove(vm.annotation); drafts.remove(vm.annotation);
$rootScope.$emit(event, updatedModel); $rootScope.$broadcast(event, updatedModel);
}).catch(function (reason) { }).catch(function (reason) {
vm.isSaving = false; vm.isSaving = false;
vm.edit(); vm.edit();
......
...@@ -699,9 +699,9 @@ describe('annotation', function() { ...@@ -699,9 +699,9 @@ describe('annotation', function() {
it('emits annotationCreated when saving an annotation succeeds', function () { it('emits annotationCreated when saving an annotation succeeds', function () {
var controller = createController(); var controller = createController();
sandbox.spy($rootScope, '$emit'); sandbox.spy($rootScope, '$broadcast');
return controller.save().then(function() { return controller.save().then(function() {
assert.calledWith($rootScope.$emit, events.ANNOTATION_CREATED); assert.calledWith($rootScope.$broadcast, events.ANNOTATION_CREATED);
}); });
}); });
...@@ -852,7 +852,7 @@ describe('annotation', function() { ...@@ -852,7 +852,7 @@ describe('annotation', function() {
it('removes the current annotation if empty', function () { it('removes the current annotation if empty', function () {
var annotation = fixtures.newEmptyAnnotation(); var annotation = fixtures.newEmptyAnnotation();
createDirective(annotation); createDirective(annotation);
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation()); fixtures.newAnnotation());
assert.calledWith(fakeDrafts.remove, annotation); assert.calledWith(fakeDrafts.remove, annotation);
}); });
...@@ -860,7 +860,7 @@ describe('annotation', function() { ...@@ -860,7 +860,7 @@ describe('annotation', function() {
it('does not remove the current annotation if is is not new', function () { it('does not remove the current annotation if is is not new', function () {
createDirective(fixtures.defaultAnnotation()); createDirective(fixtures.defaultAnnotation());
fakeDrafts.get.returns({text: '', tags: []}); fakeDrafts.get.returns({text: '', tags: []});
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation()); fixtures.newAnnotation());
assert.notCalled(fakeDrafts.remove); assert.notCalled(fakeDrafts.remove);
}); });
...@@ -869,7 +869,7 @@ describe('annotation', function() { ...@@ -869,7 +869,7 @@ describe('annotation', function() {
var annotation = fixtures.newEmptyAnnotation(); var annotation = fixtures.newEmptyAnnotation();
var parts = createDirective(annotation); var parts = createDirective(annotation);
parts.scope.$destroy(); parts.scope.$destroy();
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation()); fixtures.newAnnotation());
assert.notCalled(fakeDrafts.remove); assert.notCalled(fakeDrafts.remove);
}); });
...@@ -878,7 +878,7 @@ describe('annotation', function() { ...@@ -878,7 +878,7 @@ describe('annotation', function() {
var annotation = fixtures.newAnnotation(); var annotation = fixtures.newAnnotation();
createDirective(annotation); createDirective(annotation);
fakeDrafts.get.returns({text: 'An incomplete thought'}); fakeDrafts.get.returns({text: 'An incomplete thought'});
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation()); fixtures.newAnnotation());
assert.notCalled(fakeDrafts.remove); assert.notCalled(fakeDrafts.remove);
}); });
...@@ -887,7 +887,7 @@ describe('annotation', function() { ...@@ -887,7 +887,7 @@ describe('annotation', function() {
var annotation = fixtures.newAnnotation(); var annotation = fixtures.newAnnotation();
createDirective(annotation); createDirective(annotation);
fakeDrafts.get.returns({tags: ['a-tag']}); fakeDrafts.get.returns({tags: ['a-tag']});
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED, $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation()); fixtures.newAnnotation());
assert.notCalled(fakeDrafts.remove); assert.notCalled(fakeDrafts.remove);
}); });
...@@ -937,9 +937,9 @@ describe('annotation', function() { ...@@ -937,9 +937,9 @@ describe('annotation', function() {
it('deletes the annotation if it was new', function () { it('deletes the annotation if it was new', function () {
var controller = createDirective(fixtures.newAnnotation()).controller; var controller = createDirective(fixtures.newAnnotation()).controller;
sandbox.spy($rootScope, '$emit'); sandbox.spy($rootScope, '$broadcast');
controller.revert(); controller.revert();
assert.calledWith($rootScope.$emit, events.ANNOTATION_DELETED); assert.calledWith($rootScope.$broadcast, events.ANNOTATION_DELETED);
}); });
}); });
......
...@@ -37,86 +37,86 @@ describe('annotationMapper', function() { ...@@ -37,86 +37,86 @@ describe('annotationMapper', function() {
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, '$broadcast');
var annotations = [{id: 1}, {id: 2}, {id: 3}]; var annotations = [{id: 1}, {id: 2}, {id: 3}];
annotationMapper.loadAnnotations(annotations); annotationMapper.loadAnnotations(annotations);
assert.called($rootScope.$emit); assert.called($rootScope.$broadcast);
assert.calledWith($rootScope.$emit, events.ANNOTATIONS_LOADED, assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED,
[{id: 1}, {id: 2}, {id: 3}]); [{id: 1}, {id: 2}, {id: 3}]);
}); });
it('also includes replies in the annotationLoaded event', function () { it('also includes replies in the annotationLoaded event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1}]; var annotations = [{id: 1}];
var replies = [{id: 2}, {id: 3}]; var replies = [{id: 2}, {id: 3}];
annotationMapper.loadAnnotations(annotations, replies); annotationMapper.loadAnnotations(annotations, replies);
assert.called($rootScope.$emit); assert.called($rootScope.$broadcast);
assert.calledWith($rootScope.$emit, events.ANNOTATIONS_LOADED, assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED,
[{id: 1}, {id: 2}, {id: 3}]); [{id: 1}, {id: 2}, {id: 3}]);
}); });
it('triggers the annotationUpdated event for each loaded annotation', function () { it('triggers the annotationUpdated event for each loaded annotation', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1}, {id: 2}, {id: 3}]; var annotations = [{id: 1}, {id: 2}, {id: 3}];
annotationUI.addAnnotations(angular.copy(annotations)); annotationUI.addAnnotations(angular.copy(annotations));
annotationMapper.loadAnnotations(annotations); annotationMapper.loadAnnotations(annotations);
assert.called($rootScope.$emit); assert.called($rootScope.$broadcast);
assert.calledWith($rootScope.$emit, events.ANNOTATION_UPDATED, assert.calledWith($rootScope.$broadcast, events.ANNOTATION_UPDATED,
annotations[0]); annotations[0]);
}); });
it('also triggers annotationUpdated for cached replies', function () { it('also triggers annotationUpdated for cached replies', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1}]; var annotations = [{id: 1}];
var replies = [{id: 2}, {id: 3}, {id: 4}]; var replies = [{id: 2}, {id: 3}, {id: 4}];
annotationUI.addAnnotations([{id:3}]); annotationUI.addAnnotations([{id:3}]);
annotationMapper.loadAnnotations(annotations, replies); annotationMapper.loadAnnotations(annotations, replies);
assert($rootScope.$emit.calledWith(events.ANNOTATION_UPDATED, assert($rootScope.$broadcast.calledWith(events.ANNOTATION_UPDATED,
{id: 3})); {id: 3}));
}); });
it('replaces the properties on the cached annotation with those from the loaded one', function () { it('replaces the properties on the cached annotation with those from the loaded one', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1, url: 'http://example.com'}]; var annotations = [{id: 1, url: 'http://example.com'}];
annotationUI.addAnnotations([{id:1, $$tag: 'tag1'}]); annotationUI.addAnnotations([{id:1, $$tag: 'tag1'}]);
annotationMapper.loadAnnotations(annotations); annotationMapper.loadAnnotations(annotations);
assert.called($rootScope.$emit); assert.called($rootScope.$broadcast);
assert.calledWith($rootScope.$emit, events.ANNOTATION_UPDATED, { assert.calledWith($rootScope.$broadcast, events.ANNOTATION_UPDATED, {
id: 1, id: 1,
url: 'http://example.com', url: 'http://example.com',
}); });
}); });
it('excludes cached annotations from the annotationLoaded event', function () { it('excludes cached annotations from the annotationLoaded event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1, url: 'http://example.com'}]; var annotations = [{id: 1, url: 'http://example.com'}];
annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]); annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]);
annotationMapper.loadAnnotations(annotations); annotationMapper.loadAnnotations(annotations);
assert.called($rootScope.$emit); assert.called($rootScope.$broadcast);
assert.calledWith($rootScope.$emit, events.ANNOTATIONS_LOADED, []); assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED, []);
}); });
}); });
describe('#unloadAnnotations()', function () { describe('#unloadAnnotations()', function () {
it('triggers the annotationsUnloaded event', function () { it('triggers the annotationsUnloaded event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1}, {id: 2}, {id: 3}]; var annotations = [{id: 1}, {id: 2}, {id: 3}];
annotationMapper.unloadAnnotations(annotations); annotationMapper.unloadAnnotations(annotations);
assert.calledWith($rootScope.$emit, assert.calledWith($rootScope.$broadcast,
events.ANNOTATIONS_UNLOADED, annotations); 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 () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var annotations = [{id: 1, url: 'http://example.com'}]; var annotations = [{id: 1, url: 'http://example.com'}];
annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]); annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]);
annotationMapper.unloadAnnotations(annotations); annotationMapper.unloadAnnotations(annotations);
assert.calledWith($rootScope.$emit, events.ANNOTATIONS_UNLOADED, [{ assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_UNLOADED, [{
id: 1, id: 1,
url: 'http://example.com', url: 'http://example.com',
}]); }]);
...@@ -131,10 +131,10 @@ describe('annotationMapper', function() { ...@@ -131,10 +131,10 @@ describe('annotationMapper', function() {
}); });
it('emits the "beforeAnnotationCreated" event', function () { it('emits the "beforeAnnotationCreated" event', function () {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var ann = {}; var ann = {};
annotationMapper.createAnnotation(ann); annotationMapper.createAnnotation(ann);
assert.calledWith($rootScope.$emit, assert.calledWith($rootScope.$broadcast,
events.BEFORE_ANNOTATION_CREATED, ann); events.BEFORE_ANNOTATION_CREATED, ann);
}); });
}); });
...@@ -147,21 +147,21 @@ describe('annotationMapper', function() { ...@@ -147,21 +147,21 @@ describe('annotationMapper', function() {
}); });
it('triggers the "annotationDeleted" event on success', function (done) { it('triggers the "annotationDeleted" event on success', function (done) {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
var ann = {}; var ann = {};
annotationMapper.deleteAnnotation(ann).then(function () { annotationMapper.deleteAnnotation(ann).then(function () {
assert.calledWith($rootScope.$emit, assert.calledWith($rootScope.$broadcast,
events.ANNOTATION_DELETED, ann); events.ANNOTATION_DELETED, ann);
}).then(done, done); }).then(done, done);
$rootScope.$apply(); $rootScope.$apply();
}); });
it('does not emit an event on error', function (done) { it('does not emit an event on error', function (done) {
sandbox.stub($rootScope, '$emit'); sandbox.stub($rootScope, '$broadcast');
fakeStore.annotation.delete.returns(Promise.reject()); fakeStore.annotation.delete.returns(Promise.reject());
var ann = {id: 'test-id'}; var ann = {id: 'test-id'};
annotationMapper.deleteAnnotation(ann).catch(function () { annotationMapper.deleteAnnotation(ann).catch(function () {
assert.notCalled($rootScope.$emit); assert.notCalled($rootScope.$broadcast);
}).then(done, done); }).then(done, done);
$rootScope.$apply(); $rootScope.$apply();
}); });
......
...@@ -60,7 +60,7 @@ describe('AnnotationUIController', function () { ...@@ -60,7 +60,7 @@ describe('AnnotationUIController', function () {
describe('on annotationDeleted', function () { describe('on annotationDeleted', function () {
it('removes the deleted annotation from the selection', function () { it('removes the deleted annotation from the selection', function () {
annotationUI.selectAnnotations(['1']); annotationUI.selectAnnotations(['1']);
$rootScope.$emit('annotationDeleted', { id: 1 }); $rootScope.$broadcast('annotationDeleted', { id: 1 });
assert.equal(annotationUI.getState().selectedAnnotationMap, null); assert.equal(annotationUI.getState().selectedAnnotationMap, null);
}); });
}); });
......
...@@ -326,24 +326,24 @@ describe('WidgetController', function () { ...@@ -326,24 +326,24 @@ describe('WidgetController', function () {
* not part of the selection. * not part of the selection.
*/ */
it('clears the selection', function () { it('clears the selection', function () {
$rootScope.$emit('beforeAnnotationCreated', {}); $rootScope.$broadcast('beforeAnnotationCreated', {});
assert.called($scope.clearSelection); assert.called($scope.clearSelection);
}); });
it('does not clear the selection if the new annotation is a highlight', function () { it('does not clear the selection if the new annotation is a highlight', function () {
$rootScope.$emit('beforeAnnotationCreated', {$highlight: true}); $rootScope.$broadcast('beforeAnnotationCreated', {$highlight: true});
assert.notCalled($scope.clearSelection); assert.notCalled($scope.clearSelection);
}); });
it('does not clear the selection if the new annotation is a reply', function () { it('does not clear the selection if the new annotation is a reply', function () {
$rootScope.$emit('beforeAnnotationCreated', { $rootScope.$broadcast('beforeAnnotationCreated', {
references: ['parent-id'], references: ['parent-id'],
}); });
assert.notCalled($scope.clearSelection); assert.notCalled($scope.clearSelection);
}); });
it('scrolls the viewport to the new annotation', function () { it('scrolls the viewport to the new annotation', function () {
$rootScope.$emit('beforeAnnotationCreated', {$$tag: '123'}); $rootScope.$broadcast('beforeAnnotationCreated', {$$tag: '123'});
assert.called(windowScroll); assert.called(windowScroll);
}); });
}); });
......
...@@ -314,7 +314,7 @@ module.exports = function WidgetController( ...@@ -314,7 +314,7 @@ module.exports = function WidgetController(
// When a direct-linked annotation is successfully anchored in the page, // When a direct-linked annotation is successfully anchored in the page,
// focus and scroll to it // focus and scroll to it
$rootScope.$on(events.ANNOTATIONS_SYNCED, function (event, tags) { $scope.$on(events.ANNOTATIONS_SYNCED, function (event, tags) {
var selectedAnnot = firstSelectedAnnotation(); var selectedAnnot = firstSelectedAnnotation();
if (!selectedAnnot) { if (!selectedAnnot) {
return; return;
...@@ -455,7 +455,7 @@ module.exports = function WidgetController( ...@@ -455,7 +455,7 @@ module.exports = function WidgetController(
}, 200); }, 200);
} }
$rootScope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, data) { $scope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, data) {
if (data.$highlight || (data.references && data.references.length > 0)) { if (data.$highlight || (data.references && data.references.length > 0)) {
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