Merge pull request #2884 from hypothesis/canceling-edits-loses-changes

Fix canceling edits reverting to the first-loaded version of an annotation

h/static/scripts/directive/annotation.js

@@ -384,6 +384,7 @@ function AnnotationController(
 
   function onAnnotationUpdated(event, updatedDomainModel) {
     if (updatedDomainModel.id === domainModel.id) {
+      domainModel = updatedDomainModel;
       updateView(updatedDomainModel);
     }
   }
@@ -699,9 +700,9 @@ function AnnotationController(
         }
 
         onFulfilled = function() {
+          drafts.remove(domainModel);
           $rootScope.$emit('annotationUpdated', updatedModel);
           view();
-          drafts.remove(domainModel);
         };
         onRejected = function(reason) {
           flash.error(

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

@@ -1925,7 +1925,7 @@ describe('annotation', function() {
       var controller = createAnnotationDirective({
         annotation: {
           id: 'test-annotation-id',
-          user: 'acct:bill@localhost'
+          user: 'acct:bill@localhost',
         }
       }).controller;
       controller.edit();
@@ -1934,5 +1934,33 @@ describe('annotation', function() {
       controller.revert();
       assert.equal(controller.form.text, void 0);
     });
+
+    it('reverts to the most recently saved version when canceling changes',
+      function () {
+
+      var controller = createAnnotationDirective({
+        annotation: {
+          user: 'acct:bill@localhost',
+          $create: function () {
+            this.id = 'new-annotation-id';
+            return Promise.resolve();
+          },
+          $update: function () {
+            return Promise.resolve(this);
+          },
+        },
+      }).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, 'Updated annotation text');
+      });
+    });
   });
 });

h/static/scripts/drafts.js

@@ -17,10 +17,13 @@
 function DraftStore() {
   this._drafts = [];
 
-  // returns true if 'draft' is a draft for a given
-  // annotation. Annotations are matched by ID
-  // and annotation instance (for unsaved annotations
-  // which have no ID)
+  /**
+   * Returns true if 'draft' is a draft for a given
+   * annotation.
+   *
+   * Annotations are matched by ID and annotation instance (for unsaved
+   * annotations which have no ID)
+   */
   function match(draft, model) {
     return draft.model === model ||
            (draft.model.id && model.id === draft.model.id);
@@ -49,11 +52,12 @@ function DraftStore() {
   /** Retrieve the draft changes for an annotation. */
   this.get = function get(model) {
     for (var i = 0; i < this._drafts.length; i++) {
-      if (match(this._drafts[i], model)) {
+      var draft = this._drafts[i];
+      if (match(draft, model)) {
         return {
-          isPrivate: this._drafts[i].isPrivate,
-          tags: this._drafts[i].tags,
-          text: this._drafts[i].text,
+          isPrivate: draft.isPrivate,
+          tags: draft.tags,
+          text: draft.text,
         };
       }
     }
@@ -81,6 +85,7 @@ function DraftStore() {
     });
   };
 
+  /** Remove all drafts. */
   this.discard = function discard() {
     this._drafts = [];
   };