Change vm.document to vm.document()

vm.document was a variable containing data duplicated from domainModel (not an
exact copy, but data extracted from domainModel anyway).

It's just easier to have a method for getting this - saves having to manage
another variable and update it at the correct times.

In making this change I noticed that there were several tests for
updateViewModel() that were actually tests for extractDocumentMetadata()
(which updateViewModel() was calling). These tests are no longer valud since
updateViewModel() no longer calls extractDocumentMetadata(), and there are
already duplicate unit tests of extractDocumentMetadata() itself.

This leaves the describe('updateViewModel()') empty of tests, and since
updateViewModel() is now pretty trivial, I decided just to delete the
describe() rather then leave it empty or think up some tests to add to it.

h/static/scripts/directive/annotation.js

@@ -39,8 +39,8 @@ function errorMessage(reason) {
 
 /** Extract a URI, domain and title from the given domain model object.
  *
- * @param {object} domainModel An annotation domain model object as received from the
- *   server-side API.
+ * @param {object} domainModel An annotation domain model object as received
+ *   from the server-side API.
  * @returns {object} An object with three properties extracted from the model:
  *   uri, domain and title.
  *
@@ -179,8 +179,6 @@ function updateViewModel(domainModel, vm, permissions) {
 
   vm.annotationURI = new URL(
     '/a/' + domainModel.id, vm.baseURI).href;
-
-  vm.document = extractDocumentMetadata(domainModel);
 }
 
 /** Return truthy if the given annotation is valid, falsy otherwise.
@@ -274,10 +272,6 @@ function AnnotationController(
     /** The baseURI for the website, e.g. 'https://hypothes.is/'. */
     vm.baseURI = $document.prop('baseURI');
 
-    /** An object that will contain some metadata about the annotated document.
-     */
-    vm.document = null;
-
     /** Give the template access to the feature flags. */
     vm.feature = features.flagEnabled;
 
@@ -475,6 +469,11 @@ function AnnotationController(
     }, true);
   };
 
+  /** Return some metadata extracted from the annotated document. */
+  vm.document = function() {
+    return extractDocumentMetadata(domainModel);
+  };
+
   /**
     * @ngdoc method
     * @name annotation.AnnotationController#edit
@@ -842,7 +841,6 @@ module.exports = {
   // FIXME: The code should be refactored to enable unit testing without having
   // to do this.
   extractDocumentMetadata: extractDocumentMetadata,
-  updateViewModel: updateViewModel,
   updateDomainModel: updateDomainModel,
   validate: validate,
 

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

@@ -149,141 +149,6 @@ describe('annotation.js', function() {
     });
   });
 
-  describe('updateViewModel()', function() {
-    var updateViewModel = require('../annotation').updateViewModel;
-    var sandbox;
-
-    beforeEach(function() {
-      sandbox = sinon.sandbox.create();
-    });
-
-    afterEach(function() {
-      sandbox.restore();
-    });
-
-    it('copies model.document.title to vm.document.title', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com/example.html',
-        document: {
-          title: 'A special document'
-        }
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(vm.document.title, 'A special document');
-    });
-
-    it('uses the first title when there are more than one', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com/example.html',
-        document: {
-          title: ['first title', 'second title']
-        }
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(vm.document.title, 'first title');
-    });
-
-    it('truncates long titles', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com/example.html',
-        document: {
-          title: 'A very very very long title that really\nshouldn\'t be found on a page on the internet.'
-        }
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(
-        vm.document.title, 'A very very very long title th…');
-    });
-
-    it('copies model.uri to vm.document.uri', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com/example.html',
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(vm.document.uri, 'http://example.com/example.html');
-    });
-
-    it('copies the hostname from model.uri to vm.document.domain', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com/example.html',
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(vm.document.domain, 'example.com');
-    });
-
-    it('uses the domain for the title if the title is not present', function() {
-      var vm = {};
-      var model = {
-        uri: 'http://example.com',
-        document: {}
-      };
-
-      updateViewModel(model, vm);
-
-      assert.equal(vm.document.title, 'example.com');
-    });
-
-    it(
-      'still sets the uri correctly if the annotation has no document',
-      function() {
-        var vm = {};
-        var model = {
-          uri: 'http://example.com',
-          document: undefined
-        };
-
-        updateViewModel(model, vm);
-
-        assert(vm.document.uri === 'http://example.com');
-      }
-    );
-
-    it(
-      'still sets the domain correctly if the annotation has no document',
-      function() {
-        var vm = {};
-        var model = {
-          uri: 'http://example.com',
-          document: undefined
-        };
-
-        updateViewModel(model, vm);
-
-        assert(vm.document.domain === 'example.com');
-      }
-    );
-
-    it(
-      'uses the domain for the title when the annotation has no document',
-      function() {
-        var vm = {};
-        var model = {
-          uri: 'http://example.com',
-          document: undefined
-        };
-
-        updateViewModel(model, vm);
-
-        assert(vm.document.title === 'example.com');
-      }
-    );
-  });
-
   describe('updateDomainModel()', function() {
     var updateDomainModel = require('../annotation').updateDomainModel;
 

h/templates/client/annotation.html

@@ -32,11 +32,11 @@
         </span>
         <i class="h-icon-border-color" ng-show="vm.isHighlight() && !vm.editing()" title="This is a highlight. Click 'edit' to add a note or tag."></i>
         <span class="annotation-citation"
-              ng-bind-html="vm.document | documentTitle"
+              ng-bind-html="vm.document() | documentTitle"
               ng-if="!vm.isSidebar">
         </span>
         <span class="annotation-citation-domain"
-              ng-bind-html="vm.document | documentDomain"
+              ng-bind-html="vm.document() | documentDomain"
               ng-if="!vm.isSidebar">
         </span>
       </span>