Merge pull request #3214 from hypothesis/295-remove-empty-annotations

Remove empty annotations when a new annotation is created

h/static/scripts/directive/annotation.js

 /* jshint node: true */
 'use strict';
 
+var angular = require('angular');
+
 var annotationMetadata = require('../annotation-metadata');
 var dateUtil = require('../date-util');
 var documentDomain = require('../filter/document-domain');
@@ -260,7 +262,11 @@ function AnnotationController(
     // successfully saved to the server, and also when changes to the
     // annotation made by another client are received by this client from the
     // server.
-    $rootScope.$on('annotationUpdated', onAnnotationUpdated);
+    $rootScope.$on(events.ANNOTATION_UPDATED, onAnnotationUpdated);
+
+    // When a new annotation is created, remove any existing annotations that
+    // are empty
+    $rootScope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty);
 
     // Call `onDestroy()` when this AnnotationController's scope is removed.
     $scope.$on('$destroy', onDestroy);
@@ -309,6 +315,12 @@ function AnnotationController(
     }
   }
 
+  function deleteIfNewAndEmpty() {
+    if (isNew(domainModel) && !vm.form.text && vm.form.tags.length === 0) {
+      vm.revert();
+    }
+  }
+
   function onDestroy() {
     if (vm.cancelTimestampRefresh) {
       vm.cancelTimestampRefresh();
@@ -363,7 +375,7 @@ function AnnotationController(
       // Highlights are always private.
       domainModel.permissions = permissions.private();
       domainModel.$create().then(function() {
-        $rootScope.$emit('annotationCreated', domainModel);
+        $rootScope.$emit(events.ANNOTATION_CREATED, domainModel);
         updateView(domainModel);
       });
     } else {
@@ -562,7 +574,7 @@ function AnnotationController(
   vm.revert = function() {
     drafts.remove(domainModel);
     if (vm.action === 'create') {
-      $rootScope.$emit('annotationDeleted', domainModel);
+      $rootScope.$emit(events.ANNOTATION_DELETED, domainModel);
     } else {
       updateView(domainModel);
       view();
@@ -597,7 +609,7 @@ function AnnotationController(
       case 'create':
         updateDomainModel(domainModel, vm, permissions);
         saved = domainModel.$create().then(function () {
-          $rootScope.$emit('annotationCreated', domainModel);
+          $rootScope.$emit(events.ANNOTATION_CREATED, domainModel);
           updateView(domainModel);
           drafts.remove(domainModel);
         });
@@ -610,7 +622,7 @@ function AnnotationController(
           id: updatedModel.id
         }).then(function () {
           drafts.remove(domainModel);
-          $rootScope.$emit('annotationUpdated', updatedModel);
+          $rootScope.$emit(events.ANNOTATION_UPDATED, updatedModel);
         });
         break;
 

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

@@ -6,6 +6,7 @@ var proxyquire = require('proxyquire');
 
 var events = require('../../events');
 var fixtures = require('../../test/annotation-fixtures');
+var testUtil = require('../../test/util');
 var util = require('./util');
 
 var inject = angular.mock.inject;
@@ -17,6 +18,7 @@ function annotationDirective() {
   var noop = function () { return ''; };
 
   var annotation = proxyquire('../annotation', {
+    angular: testUtil.noCallThru(angular),
     '../filter/document-domain': noop,
     '../filter/document-title': noop,
     '../filter/persona': {
@@ -1100,7 +1102,7 @@ describe('annotation', function() {
           sandbox.spy($rootScope, '$emit');
           annotation.$create.returns(Promise.resolve());
           controller.save().then(function() {
-            assert($rootScope.$emit.calledWith('annotationCreated'));
+            assert($rootScope.$emit.calledWith(events.ANNOTATION_CREATED));
             done();
           });
         }
@@ -1311,7 +1313,7 @@ describe('annotation', function() {
           text: 'new text',
         };
 
-        $rootScope.$emit('annotationUpdated', updatedModel);
+        $rootScope.$emit(events.ANNOTATION_UPDATED, updatedModel);
 
         assert.equal(parts.controller.form.text, 'new text');
       });
@@ -1324,12 +1326,49 @@ describe('annotation', function() {
           text: 'new text',
         };
 
-        $rootScope.$emit('annotationUpdated', updatedModel);
+        $rootScope.$emit(events.ANNOTATION_UPDATED, updatedModel);
 
         assert.equal(parts.controller.form.text, 'original text');
       });
     });
 
+    describe('when another new annotation is created', function () {
+      it('removes the current annotation if empty', function () {
+        var annotation = fixtures.newEmptyAnnotation();
+        createDirective(annotation);
+        $rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
+          fixtures.newAnnotation());
+        assert.calledWith(fakeDrafts.remove, annotation);
+      });
+
+      it('does not remove the current annotation if is is not new', function () {
+        var parts = createDirective(fixtures.defaultAnnotation());
+        parts.controller.form.text = '';
+        parts.controller.form.tags = [];
+        $rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
+          fixtures.newAnnotation());
+        assert.notCalled(fakeDrafts.remove);
+      });
+
+      it('does not remove the current annotation if it has text', function () {
+        var annotation = fixtures.newAnnotation();
+        var parts = createDirective(annotation);
+        parts.controller.form.text = 'An incomplete thought';
+        $rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
+          fixtures.newAnnotation());
+        assert.notCalled(fakeDrafts.remove);
+      });
+
+      it('does not remove the current annotation if it has tags', function () {
+        var annotation = fixtures.newAnnotation();
+        var parts = createDirective(annotation);
+        parts.controller.form.tags = [{text: 'a-tag'}];
+        $rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
+          fixtures.newAnnotation());
+        assert.notCalled(fakeDrafts.remove);
+      });
+    });
+
     describe('onGroupFocused()', function() {
       it('if the annotation is being edited it updates drafts', function() {
         var parts = createDirective();

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

@@ -30,6 +30,18 @@ function newAnnotation() {
   };
 }
 
+/** Return a new annotation which has no tags or text. */
+function newEmptyAnnotation() {
+  return {
+    id: undefined,
+    $highlight: undefined,
+    target: ['foo'],
+    references: [],
+    text: '',
+    tags: [],
+  };
+}
+
 /** Return an annotation domain model object for a new highlight
  * (newly-created client-side, not yet saved to the server).
  */
@@ -97,6 +109,7 @@ function oldReply() {
 module.exports = {
   defaultAnnotation: defaultAnnotation,
   newAnnotation: newAnnotation,
+  newEmptyAnnotation: newEmptyAnnotation,
   newHighlight: newHighlight,
   oldAnnotation: oldAnnotation,
   oldHighlight: oldHighlight,