Move the logic that updates the focused group for new annotations (#108)

Move the logic that updates the focused group for new annotations
out of the <annotation> component and into a service.

This fixes the issue where the event handler does not get invoked if no
<annotation> component instance exists for an annotation because it is
currently scrolled off-screen.

Fixes #105

h/static/scripts/directive/annotation.js

@@ -122,9 +122,6 @@ function AnnotationController(
       */
     newlyCreatedByHighlightButton = vm.annotation.$highlight || false;
 
-    // Call `onGroupFocused()` whenever the currently-focused group changes.
-    $scope.$on(events.GROUP_FOCUSED, onGroupFocused);
-
     // New annotations (just created locally by the client, rather then
     // received from the server) have some fields missing. Add them.
     vm.annotation.user = vm.annotation.user || session.state.userid;
@@ -154,13 +151,6 @@ function AnnotationController(
     }
   }
 
-  function onGroupFocused() {
-    // New annotations move to the new group, when a new group is focused.
-    if (isNew(vm.annotation) && !isReply(vm.annotation)) {
-      vm.annotation.group = groups.focused().id;
-    }
-  }
-
   /** Save this annotation if it's a new highlight.
    *
    * The highlight will be saved to the server if the user is logged in,

h/static/scripts/directive/test/annotation-test.js

@@ -867,40 +867,6 @@ describe('annotation', function() {
       });
     });
 
-    describe('when the focused group changes', function() {
-      it('moves new annotations to the focused group', function () {
-        var annotation = fixtures.newAnnotation();
-        annotation.group = 'old-group-id';
-        createDirective(annotation);
-        fakeGroups.focused = sandbox.stub().returns({id: 'new-group-id'});
-
-        $rootScope.$broadcast(events.GROUP_FOCUSED);
-
-        assert.equal(annotation.group, 'new-group-id');
-      });
-
-      it('does not move replies to the focused group', function () {
-        var annotation = fixtures.newReply();
-        fakeGroups.focused = sandbox.stub().returns({id: 'new-group-id'});
-        createDirective(annotation);
-        fakeGroups.focused = sandbox.stub().returns({id: 'new-group-id-2'});
-        $rootScope.$broadcast(events.GROUP_FOCUSED);
-
-        assert.equal(annotation.group, 'new-group-id');
-      });
-
-      it('does not modify the group of saved annotations', function () {
-        var annotation = fixtures.oldAnnotation();
-        annotation.group = 'old-group-id';
-        createDirective(annotation);
-        fakeGroups.focused = sandbox.stub().returns({id: 'new-group-id'});
-
-        $rootScope.$broadcast(events.GROUP_FOCUSED);
-
-        assert.equal(annotation.group, 'old-group-id');
-      });
-    });
-
     describe('reverting edits', function () {
       it('removes the current draft', function() {
         var controller = createDirective(fixtures.defaultAnnotation()).controller;

h/static/scripts/root-thread.js

@@ -159,6 +159,24 @@ function RootThread($rootScope, annotationUI, drafts, features, searchFilter, vi
     annotationUI.removeAnnotations(annotations);
   });
 
+  // Once the focused group state is moved to the app state store, then the
+  // logic in this event handler can be moved to the annotations reducer.
+  $rootScope.$on(events.GROUP_FOCUSED, function (event, focusedGroupId) {
+    var updatedAnnots = annotationUI.getState().annotations.filter(function (ann) {
+      return metadata.isNew(ann) && !metadata.isReply(ann);
+    }).map(function (ann) {
+      return Object.assign(ann, {
+        // FIXME - $$tag is currently a non-enumerable property so it has to be
+        // copied explicitly
+        $$tag: ann.$$tag,
+        group: focusedGroupId,
+      });
+    });
+    if (updatedAnnots.length > 0) {
+      annotationUI.addAnnotations(updatedAnnots);
+    }
+  });
+
   /**
    * Build the root conversation thread from the given UI state.
    * @return {Thread}

h/static/scripts/test/root-thread-test.js

@@ -400,4 +400,33 @@ describe('rootThread', function () {
       });
     });
   });
+
+  describe('when the focused group changes', function () {
+    it('moves new annotations to the focused group', function () {
+      fakeAnnotationUI.state.annotations = [{$$tag: 'a-tag'}];
+
+      $rootScope.$broadcast(events.GROUP_FOCUSED, 'private-group');
+
+      assert.calledWith(fakeAnnotationUI.addAnnotations, sinon.match([{
+        $$tag: 'a-tag',
+        group: 'private-group',
+      }]));
+    });
+
+    it('does not move replies to the new group', function () {
+      fakeAnnotationUI.state.annotations = [annotationFixtures.newReply()];
+
+      $rootScope.$broadcast(events.GROUP_FOCUSED, 'private-group');
+
+      assert.notCalled(fakeAnnotationUI.addAnnotations);
+    });
+
+    it('does not move saved annotations to the new group', function () {
+      fakeAnnotationUI.state.annotations = [annotationFixtures.defaultAnnotation()];
+
+      $rootScope.$broadcast(events.GROUP_FOCUSED, 'private-group');
+
+      assert.notCalled(fakeAnnotationUI.addAnnotations);
+    });
+  });
 });