Eliminate `vm.action` flag in AnnotationController

Instead of maintaining a state flag indicating whether an annotation is
being viewed, edited or created, derive that state from whether the
annotation has an ID and whether it has an unsaved draft or not.

In the process this commit simplifies the tests for reverting edits and
adds a missing test that the annotation is deleted if new when clicking
the Cancel button.

h/static/scripts/directive/annotation.js

@@ -162,9 +162,6 @@ function AnnotationController(
     * can call the methods.
     */
   function init() {
-    /** The currently active action - 'view', 'create' or 'edit'. */
-    vm.action = 'view';
-
     /** vm.form is the read-write part of vm for the templates: it contains
      *  the variables that the templates will write changes to via ng-model. */
     vm.form = {};
@@ -324,14 +321,6 @@ function AnnotationController(
     }
   }
 
-  /** Switches the view to a viewer, closing the editor controls if they're
-   *  open.
-    * @name annotation.AnnotationController#view
-    */
-  function view() {
-    vm.action = 'view';
-  }
-
   /**
     * @ngdoc method
     * @name annotation.AnnotationController#authorize
@@ -376,8 +365,15 @@ function AnnotationController(
     * @description Switches the view to an editor.
     */
   vm.edit = function() {
+    if (!drafts.get(vm.annotation)) {
+      drafts.update(vm.annotation, {
+        tags: vm.annotation.tags,
+        text: vm.annotation.text,
+        isPrivate: permissions.isPrivate(vm.annotation.permissions,
+          session.state.userid),
+      });
+    }
     restoreFromDrafts(drafts, vm);
-    vm.action = isNew(vm.annotation) ? 'create' : 'edit';
   };
 
   /**
@@ -387,11 +383,7 @@ function AnnotationController(
    *   (i.e. the annotation editor form should be open), `false` otherwise.
    */
   vm.editing = function() {
-    if (vm.action === 'create' || vm.action === 'edit') {
-      return true;
-    } else {
-      return false;
-    }
+    return drafts.get(vm.annotation) && !vm.isSaving;
   };
 
   /**
@@ -502,11 +494,10 @@ function AnnotationController(
     */
   vm.revert = function() {
     drafts.remove(vm.annotation);
-    if (vm.action === 'create') {
+    if (isNew(vm.annotation)) {
       $rootScope.$emit(events.ANNOTATION_DELETED, vm.annotation);
     } else {
       updateView();
-      view();
     }
   };
 
@@ -520,8 +511,7 @@ function AnnotationController(
       flash.info('Please sign in to save your annotations.');
       return Promise.resolve();
     }
-    if ((vm.action === 'create' || vm.action === 'edit') &&
-        !vm.hasContent() && vm.isShared()) {
+    if (!vm.hasContent() && vm.isShared()) {
       flash.info('Please add text or a tag before publishing.');
       return Promise.resolve();
     }
@@ -546,7 +536,6 @@ function AnnotationController(
     // optimistically switch back to view mode and display the saving
     // indicator
     vm.isSaving = true;
-    view();
 
     return saved.then(function () {
       vm.isSaving = false;

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

@@ -380,12 +380,22 @@ describe('annotation', function() {
         assert.notCalled(fakeStore.annotation.create);
       });
 
-      it('edits new annotations on initialization', function() {
+      it('creates drafts for new annotations on initialization', function() {
         var annotation = fixtures.newAnnotation();
+        createDirective(annotation);
+        assert.calledWith(fakeDrafts.update, annotation, {
+          isPrivate: false,
+          tags: annotation.tags,
+          text: annotation.text,
+        });
+      });
 
+      it('does not create drafts for new highlights on initialization', function() {
+        var annotation = fixtures.newHighlight();
         var controller = createDirective(annotation).controller;
 
-        assert.isTrue(controller.editing());
+        assert.notOk(controller.editing());
+        assert.notCalled(fakeDrafts.update);
       });
 
       it('edits annotations with drafts on initialization', function() {
@@ -397,47 +407,25 @@ describe('annotation', function() {
 
         assert.isTrue(controller.editing());
       });
-
-      it('does not edit new highlights on initialization', function() {
-        var annotation = fixtures.newHighlight();
-
-        var controller = createDirective(annotation).controller;
-
-        assert.isFalse(controller.editing());
-      });
-
-      it('edits highlights with drafts on initialization', function() {
-        var annotation = fixtures.oldHighlight();
-        // You can edit a highlight, enter some text or tags, and save it (the
-        // highlight then becomes an annotation). You can also edit a highlight
-        // and then change focus to another group and back without saving the
-        // highlight, in which case the highlight will have draft edits.
-        // This highlight has draft edits.
-        fakeDrafts.get.returns({text: '', tags: []});
-
-        var controller = createDirective(annotation).controller;
-
-        assert.isTrue(controller.editing());
-      });
     });
 
-    describe('.editing()', function() {
-      it('returns true if action is "create"', function() {
+    describe('#editing()', function() {
+      it('returns false if the annotation does not have a draft', function () {
         var controller = createDirective().controller;
-        controller.action = 'create';
-        assert.equal(controller.editing(), true);
+        assert.notOk(controller.editing());
       });
 
-      it('returns true if action is "edit"', function() {
+      it('returns true if the annotation has a draft', function () {
         var controller = createDirective().controller;
-        controller.action = 'edit';
-        assert.equal(controller.editing(), true);
+        fakeDrafts.get.returns({tags: [], text: '', isPrivate: false});
+        assert.isTrue(controller.editing());
       });
 
-      it('returns false if action is "view"', function() {
+      it('returns false if the annotation has a draft but is being saved', function () {
         var controller = createDirective().controller;
-        controller.action = 'view';
-        assert.equal(controller.editing(), false);
+        fakeDrafts.get.returns({tags: [], text: '', isPrivate: false});
+        controller.isSaving = true;
+        assert.isFalse(controller.editing());
       });
     });
 
@@ -795,21 +783,24 @@ describe('annotation', function() {
 
       function controllerWithActionCreate() {
         var controller = createDirective(annotation).controller;
-        controller.action = 'create';
         controller.form.text = 'new annotation';
         return controller;
       }
 
-      it(
-        'emits annotationCreated when saving an annotation succeeds',
-        function() {
-          var controller = controllerWithActionCreate();
-          sandbox.spy($rootScope, '$emit');
-          return controller.save().then(function() {
-            assert.calledWith($rootScope.$emit, events.ANNOTATION_CREATED);
-          });
-        }
-      );
+      it('removes the draft when saving an annotation succeeds', function () {
+        var controller = controllerWithActionCreate();
+        return controller.save().then(function () {
+          assert.calledWith(fakeDrafts.remove, annotation);
+        });
+      });
+
+      it('emits annotationCreated when saving an annotation succeeds', function () {
+        var controller = controllerWithActionCreate();
+        sandbox.spy($rootScope, '$emit');
+        return controller.save().then(function() {
+          assert.calledWith($rootScope.$emit, events.ANNOTATION_CREATED);
+        });
+      });
 
       it('flashes a generic error if the server can\'t be reached', function() {
         var controller = controllerWithActionCreate();
@@ -849,14 +840,13 @@ describe('annotation', function() {
         }));
         var saved = controller.save();
         assert.equal(controller.isSaving, true);
-        assert.equal(controller.action, 'view');
         create();
         return saved.then(function () {
           assert.equal(controller.isSaving, false);
         });
       });
 
-      it('reverts to edit mode if saving fails', function () {
+      it('does not remove the draft if saving fails', function () {
         var controller = controllerWithActionCreate();
         var failCreation;
         fakeStore.annotation.create = sinon.stub().returns(new Promise(function (resolve, reject) {
@@ -867,7 +857,7 @@ describe('annotation', function() {
         failCreation({status: -1});
         return saved.then(function () {
           assert.equal(controller.isSaving, false);
-          assert.ok(controller.editing());
+          assert.notCalled(fakeDrafts.remove);
         });
       });
 
@@ -882,7 +872,6 @@ describe('annotation', function() {
             text: 'foo',
           };
           var controller = createDirective(annotation).controller;
-          controller.action = 'create';
           return controller.save().then(function() {
             assert.calledWith(fakeStore.annotation.create, sinon.match({}),
               sinon.match({group: 'test-id'}));
@@ -900,7 +889,6 @@ describe('annotation', function() {
 
       function controllerWithActionEdit() {
         var controller = createDirective(annotation).controller;
-        controller.action = 'edit';
         controller.form.text = 'updated text';
         return controller;
       }
@@ -1106,71 +1094,34 @@ describe('annotation', function() {
 
 
     describe('reverting edits', function () {
-      // Simulate what happens when the user edits an annotation,
-      // clicks Save, gets an error because the server fails to save the
-      // annotation, then clicks Cancel - in the frontend the annotation should
-      // be restored to its original value, the edits lost.
       it('restores the original text', function() {
         var controller = createDirective({
           id: 'test-annotation-id',
           user: 'acct:bill@localhost',
           text: 'Initial annotation body text',
         }).controller;
-        fakeStore.annotation.update = function () {
-          return Promise.reject({
-            status: 500,
-            statusText: 'Server Error',
-            data: {}
-          });
-        };
-        var originalText = controller.form.text;
-        // Simulate the user clicking the Edit button on the annotation.
         controller.edit();
-        // Simulate the user typing some text into the annotation editor textarea.
         controller.form.text = 'changed by test code';
-        // Simulate the user hitting the Save button and wait for the
-        // (unsuccessful) response from the server.
-        controller.save();
-        // At this point the annotation editor controls are still open, and the
-        // annotation's text is still the modified (unsaved) text.
-        assert.equal(controller.form.text, 'changed by test code');
-        // Simulate the user clicking the Cancel button.
         controller.revert();
-        assert.equal(controller.form.text, originalText);
+        assert.equal(controller.form.text, controller.annotation.text);
       });
 
-      // Test that editing reverting changes to an annotation with
-      // no text resets the text to be empty.
       it('clears the text if the text was originally empty', function() {
         var controller = createDirective({
           id: 'test-annotation-id',
           user: 'acct:bill@localhost',
         }).controller;
         controller.edit();
-        assert.equal(controller.action, 'edit');
         controller.form.text = 'this should be reverted';
         controller.revert();
         assert.equal(controller.form.text, '');
       });
 
-      it('reverts to the most recently saved version', function () {
-        var controller = createDirective({
-          id: 'new-annot',
-          user: 'acct:bill@localhost',
-          text: 'saved-text',
-        }).controller;
-        controller.edit();
-        controller.form.text = 'New annotation text';
-
-        return controller.save().then(function () {
-          controller.edit();
-          controller.form.text = 'Updated annotation text';
-          return controller.save();
-        }).then(function () {
-          controller.edit();
-          controller.revert();
-          assert.equal(controller.form.text, controller.annotation.text);
-        });
+      it('deletes the annotation if it was new', function () {
+        var controller = createDirective(fixtures.newAnnotation()).controller;
+        sandbox.spy($rootScope, '$emit');
+        controller.revert();
+        assert.calledWith($rootScope.$emit, events.ANNOTATION_DELETED);
       });
     });