Refactor to avoid renders and add tests

Rather than watching a bunch of subproperties, the most complex of
which are the targets, deep watch the whole annotation. The targets
were being deep watched already. This just seems much easier and
clearer than debouncing the render calls, and scales to whatever
properties much change in the future. We'll re-evaluate if this
becomes a bottleneck.

h/static/scripts/directives/annotation.coffee

@@ -223,31 +223,24 @@ AnnotationController = [
     $scope.$on '$destroy', ->
       drafts.remove model
 
-    # Render on updates.
-    $scope.$watch (-> model.updated), (updated, old) =>
-      return if updated is old
-      if updated then drafts.remove model
-      this.render()  # XXX: TODO: don't clobber the view when collaborating
-
-    # Update once logged in.
-    $scope.$watch (-> model.user), (user, old) =>
-      return if user is old
-      if highlight and vm.isHighlight()
-        if user
+    # Watch the model.
+    # XXX: TODO: don't clobber the view when collaborating
+    $scope.$watch (-> model), (model, old) =>
+      # Discard saved drafts
+      if model.updated != old.updated
+        drafts.remove model
+
+      # Save highlights once logged in.
+      if highlight and this.isHighlight()
+        if model.user
           annotator.publish 'annotationCreated', model
+          highlight = false  # skip this on future updates
         else
           drafts.add model, -> this.revert()
-      else
-        this.render()
 
-    # Calculate things neded for the visual diff support
-    $scope.$watch (-> model.target), (targets, old) =>
-      return if targets is old
       this.render()
     , true
 
-    $scope.$watch (-> model), => this.render()
-
     # Start editing brand new annotations immediately
     unless model.id? or (highlight and this.isHighlight()) then this.edit()
 

tests/js/directives/annotation-test.coffee

@@ -31,46 +31,6 @@ describe 'h.directives.annotation', ->
   afterEach ->
     sandbox.restore()
 
-  it 'provides a document title', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'A special document')
-
-  it 'uses the first title when there are more than one', ->
-    annotation.document.title = ['first title', 'second title']
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'first title')
-
-  it 'truncates long titles', ->
-    annotation.document.title = '''A very very very long title that really
-    shouldn't be found on a page on the internet.'''
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'A very very very long title th…')
-
-  it 'provides a document uri', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.uri, 'http://example.com')
-
-  it 'provides an extracted domain from the uri', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.domain, 'example.com')
-
-  it 'uses the domain for the title if the title is not present', ->
-    delete annotation.document.title
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'example.com')
-
-  it 'skips the document object if no document is present on the annotation', ->
-    delete annotation.document
-    controller = createController()
-    $scope.$digest()
-    assert.isNull(controller.document)
-
   describe '#reply', ->
     controller = null
     container = null
@@ -107,3 +67,60 @@ describe 'h.directives.annotation', ->
       controller.reply()
       newAnnotation = annotator.publish.lastCall.args[1]
       assert.notInclude(newAnnotation.permissions.read, 'group:__world__')
+
+  describe '#render', ->
+    controller = null
+
+    beforeEach ->
+      controller = createController()
+      sandbox.spy(controller, 'render')
+
+    afterEach ->
+      sandbox.restore()
+
+    it 'is called exactly once during the first digest', ->
+      $scope.$digest()
+      assert.calledOnce(controller.render)
+
+    it 'is called exactly once on model changes', ->
+      $scope.$digest()
+      assert.calledOnce(controller.render)
+      $scope.$digest()
+      assert.calledOnce(controller.render)  # still
+
+      annotation.booz = 'baz'
+      $scope.$digest()
+      assert.calledTwice(controller.render)
+
+    it 'provides a document title', ->
+      controller.render()
+      assert.equal(controller.document.title, 'A special document')
+
+    it 'uses the first title when there are more than one', ->
+      annotation.document.title = ['first title', 'second title']
+      controller.render()
+      assert.equal(controller.document.title, 'first title')
+
+    it 'truncates long titles', ->
+      annotation.document.title = '''A very very very long title that really
+      shouldn't be found on a page on the internet.'''
+      controller.render()
+      assert.equal(controller.document.title, 'A very very very long title th…')
+
+    it 'provides a document uri', ->
+      controller.render()
+      assert.equal(controller.document.uri, 'http://example.com')
+
+    it 'provides an extracted domain from the uri', ->
+      controller.render()
+      assert.equal(controller.document.domain, 'example.com')
+
+    it 'uses the domain for the title if the title is not present', ->
+      delete annotation.document.title
+      controller.render()
+      assert.equal(controller.document.title, 'example.com')
+
+    it 'skips the document object if no document is present on the annotation', ->
+      delete annotation.document
+      controller.render()
+      assert.isNull(controller.document)