Fix: thread save incorrectly rejects when no dialogue exists (#247)

* Fix: thread save incorrectly rejects when no dialogue exists * Update: tests for updating local annotations on annotationthreads * Update: annotation save tests and variable name updates
box · Aug 1, 2017 · 6b361ed · 6b361ed
1 parent 2ee12a3
commit 6b361ed
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 20 deletions.
diff --git a/src/lib/annotations/AnnotationThread.js b/src/lib/annotations/AnnotationThread.js
@@ -146,29 +146,10 @@ class AnnotationThread extends EventEmitter {
 
         // Changing state from pending
         this.state = STATES.hover;
-
         // Save annotation on server
         this.annotationService
             .create(annotationData)
-            .then((savedAnnotation) => {
-                // If no temporary annotation is found, save to thread normally
-                const tempIdx = this.annotations.indexOf(tempAnnotation);
-                if (tempIdx === -1) {
-                    this.saveAnnotationToThread(savedAnnotation);
-                }
-
-                // Add thread number to associated dialog and thread
-                this.threadNumber = this.threadNumber || savedAnnotation.threadNumber;
-                this.dialog.element.dataset.threadNumber = this.threadNumber;
-
-                // Otherwise, replace temporary annotation with annotation saved to server
-                this.annotations[tempIdx] = savedAnnotation;
-
-                if (this.dialog) {
-                    this.dialog.addAnnotation(savedAnnotation);
-                    this.dialog.removeAnnotation(tempAnnotationID);
-                }
-            })
+            .then((savedAnnotation) => this.updateTemporaryAnnotation(tempAnnotation, savedAnnotation))
             .catch(() => {
                 // Remove temporary annotation
                 this.deleteAnnotation(tempAnnotationID, /* useServer */ false);
@@ -396,6 +377,42 @@ class AnnotationThread extends EventEmitter {
     // Private
     //--------------------------------------------------------------------------
 
+    /**
+     * Update a temporary annotation with the annotation saved on the backend. Set the threadNumber if it has not
+     * yet been set. Propogate the threadnumber to an attached dialog if applicable.
+     *
+     * @private
+     * @param {Annotation} tempAnnotation - The locally stored placeholder for the server validated annotation
+     * @param {Annotation} savedAnnotation - The annotation determined by the backend to be used as the source of truth
+     * @return {void}
+     */
+    updateTemporaryAnnotation(tempAnnotation, savedAnnotation) {
+        const tempIdx = this.annotations.indexOf(tempAnnotation);
+        if (tempIdx === -1) {
+            // If no temporary annotation is found, save to thread normally
+            this.saveAnnotationToThread(savedAnnotation);
+        } else {
+            // Otherwise, replace temporary annotation with annotation saved to server
+            this.annotations[tempIdx] = savedAnnotation;
+        }
+
+        // Set threadNumber if the savedAnnotation is the first annotation of the thread
+        if (!this.threadNumber && savedAnnotation && savedAnnotation.threadNumber) {
+            this.threadNumber = savedAnnotation.threadNumber;
+        }
+
+        if (this.dialog) {
+            // Add thread number to associated dialog and thread
+            if (this.dialog.element && this.dialog.element.dataset) {
+                this.dialog.element.dataset.threadNumber = this.threadNumber;
+            }
+
+            // Remove temporary annotation and replace it with the saved annotation
+            this.dialog.addAnnotation(savedAnnotation);
+            this.dialog.removeAnnotation(tempAnnotation.annotationID);
+        }
+    }
+
     /**
      * Creates the HTML for the annotation indicator.
      *

diff --git a/src/lib/annotations/__tests__/AnnotationThread-test.js b/src/lib/annotations/__tests__/AnnotationThread-test.js
@@ -174,6 +174,52 @@ describe('lib/annotations/AnnotationThread', () => {
         });
     });
 
+    describe('updateTemporaryAnnotation()', () => {
+        let annotationService;
+
+        beforeEach(() => {
+            annotationService = {
+                create: () => {}
+            };
+
+            thread = new AnnotationThread({
+                annotatedElement: document.querySelector('.annotated-element'),
+                annotations: [],
+                annotationService,
+                fileVersionId: '1',
+                location: {},
+                threadID: '2',
+                threadNumber: '1',
+                type: 'point'
+            });
+
+            stubs.create = sandbox.stub(annotationService, 'create');
+            stubs.saveAnnotationToThread = sandbox.stub(thread, 'saveAnnotationToThread');
+        });
+
+        it('should save annotation to thread if it does not exist in annotations array', () => {
+            const serverAnnotation = 'real annotation';
+            const tempAnnotation = serverAnnotation;
+
+            thread.updateTemporaryAnnotation(tempAnnotation, serverAnnotation);
+
+            expect(stubs.saveAnnotationToThread).to.be.called;
+        });
+
+        it('should overwrite a local annotation to the thread if it does exist as an associated annotation', () => {
+            const serverAnnotation = 'real annotation';
+            const tempAnnotation = 'placeholder annotation';
+            const isServerAnnotation = (annotation => (annotation === serverAnnotation));
+
+            thread.annotations.push(tempAnnotation)
+            expect(thread.annotations.find(isServerAnnotation)).to.be.undefined;
+            thread.updateTemporaryAnnotation(tempAnnotation, serverAnnotation);
+            expect(stubs.saveAnnotationToThread).to.not.be.called;
+            expect(thread.annotations.find(isServerAnnotation)).to.not.be.undefined;
+        });
+
+    })
+
     describe('deleteAnnotation()', () => {
         let annotationService;