Fix: Ensure deleteSuccessHandler destroys thread when necessary (#289)

box · Nov 15, 2018 · e972a72 · e972a72
1 parent e64b3f8
commit e972a72
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 2 deletions.
diff --git a/src/AnnotationThread.js b/src/AnnotationThread.js
@@ -364,8 +364,12 @@ class AnnotationThread extends EventEmitter {
      * @return {void}
      */
     deleteSuccessHandler = () => {
-        // Broadcast annotation deletion event
-        this.emit(THREAD_EVENT.delete);
+        if (this.threadID) {
+            this.renderAnnotationPopover();
+        } else {
+            this.emit(THREAD_EVENT.delete);
+            this.destroy();
+        }
     };
 
     /**

diff --git a/src/__tests__/AnnotationThread-test.js b/src/__tests__/AnnotationThread-test.js
@@ -277,6 +277,65 @@ describe('AnnotationThread', () => {
         });
     });
 
+    describe('cleanupAnnotationOnDelete()', () => {
+        let comment;
+        beforeEach(() => {
+            comment = {
+                id: '123'
+            };
+            thread.comments = [comment];
+            thread.threadID = '123abc';
+            thread.renderAnnotationPopover = jest.fn();
+            thread.canDelete = false;
+        });
+
+        it('should delete the appropriate comment', () => {
+            thread.cleanupAnnotationOnDelete(comment.id);
+            expect(thread.comments.length).toEqual(0);
+        });
+
+        it('should not destroy the thread if the user does not have canDelete permissions', () => {
+            thread.cleanupAnnotationOnDelete(comment.id);
+            expect(thread.threadID).not.toBeNull();
+        });
+
+        it('should destroy the the thread if the comment was the last one in the annotation', () => {
+            thread.canDelete = true;
+            thread.cleanupAnnotationOnDelete(comment.id);
+            expect(thread.threadID).toBeNull();
+        });
+
+        it('should re-render the popover if the comment was NOTthe last one in the annotation', () => {
+            thread.cleanupAnnotationOnDelete(comment.id);
+            expect(thread.threadID).not.toBeNull();
+            expect(thread.renderAnnotationPopover).toBeCalled();
+        });
+    });
+
+    describe('deleteSuccessHAndler()', () => {
+        beforeEach(() => {
+            thread.renderAnnotationPopover = jest.fn();
+            thread.destroy = jest.fn();
+            thread.emit = jest.fn();
+        });
+
+        it('should re-render the popover if the thread is still valid', () => {
+            thread.threadID = '123';
+            thread.deleteSuccessHandler();
+            expect(thread.renderAnnotationPopover).toBeCalled();
+            expect(thread.emit).not.toBeCalledWith(THREAD_EVENT.delete);
+            expect(thread.destroy).not.toBeCalled();
+        });
+
+        it('should properly destroy the thread if the thread should be completely deleted', () => {
+            thread.threadID = null;
+            thread.deleteSuccessHandler();
+            expect(thread.renderAnnotationPopover).not.toBeCalled();
+            expect(thread.emit).toBeCalledWith(THREAD_EVENT.delete);
+            expect(thread.destroy).toBeCalled();
+        });
+    });
+
     describe('scrollIntoView()', () => {
         it('should scroll to annotation page and center annotation in viewport', () => {
             thread.scrollToPage = jest.fn();