Remove empty annotations when a new annotation is created

When a new annotation is created, remove any empty annotation cards that
currently exist. This fixes an issue where it was easy to accidentally
create a large number of blank annotations or empty replies.

Any annotations which are not new (ie. have an ID assigned) or have tags
or text entered by the user are kept.

This logic applies to both annotations and replies.

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');
@@ -257,7 +259,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);
@@ -306,6 +312,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();
@@ -360,7 +372,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 {
@@ -559,7 +571,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();
@@ -594,7 +606,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);
         });
@@ -607,7 +619,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': {
@@ -1112,7 +1114,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();
           });
         }
@@ -1323,7 +1325,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');
       });
@@ -1336,12 +1338,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,