From 408b4950f385c15043f8646d091f74dbe03239aa Mon Sep 17 00:00:00 2001
From: Sumedha Pramod <pramods@umich.edu>
Date: Mon, 23 Apr 2018 13:56:39 -0400
Subject: [PATCH] Fix: Replace {1} with user name on drawing save (#179)

---
 src/doc/DocDrawingDialog.js                | 12 +++----
 src/doc/__tests__/DocDrawingDialog-test.js | 38 +++++++++++++++++-----
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/src/doc/DocDrawingDialog.js b/src/doc/DocDrawingDialog.js
index c1d367a80..4621075cf 100644
--- a/src/doc/DocDrawingDialog.js
+++ b/src/doc/DocDrawingDialog.js
@@ -63,6 +63,7 @@ class DocDrawingDialog extends AnnotationDialog {
         if (!this.element) {
             this.setup([annotation]);
         }
+        this.assignDrawingLabel(annotation);
     }
 
     /**
@@ -145,7 +146,7 @@ class DocDrawingDialog extends AnnotationDialog {
 
         const firstAnnotation = util.getFirstAnnotation(annotations);
         if (firstAnnotation) {
-            this.assignDrawingLabel(firstAnnotation);
+            this.addAnnotation(firstAnnotation);
         }
 
         this.element.appendChild(this.drawingDialogEl);
@@ -249,17 +250,16 @@ class DocDrawingDialog extends AnnotationDialog {
      * the username does not exist.
      *
      * @private
-     * @param {Annotation} savedAnnotation The annotation data to populate the label with.
+     * @param {Annotation} annotation The annotation data to populate the label with.
      * @return {void}
      */
-    assignDrawingLabel(savedAnnotation) {
-        if (!savedAnnotation || !this.drawingDialogEl) {
+    assignDrawingLabel(annotation) {
+        if (!annotation || !this.drawingDialogEl || annotation.user.id === '0') {
             return;
         }
 
         const drawingLabelEl = this.drawingDialogEl.querySelector(`.${constants.CLASS_ANNOTATION_DRAWING_LABEL}`);
-        const username = savedAnnotation.user ? savedAnnotation.user.name : constants.USER_ANONYMOUS;
-        drawingLabelEl.textContent = util.replacePlaceholders(this.localized.whoDrew, [username]);
+        drawingLabelEl.textContent = util.replacePlaceholders(this.localized.whoDrew, [annotation.user.name]);
 
         util.showElement(drawingLabelEl);
     }
diff --git a/src/doc/__tests__/DocDrawingDialog-test.js b/src/doc/__tests__/DocDrawingDialog-test.js
index e602e73ba..fca3d1471 100644
--- a/src/doc/__tests__/DocDrawingDialog-test.js
+++ b/src/doc/__tests__/DocDrawingDialog-test.js
@@ -45,6 +45,23 @@ describe('doc/DocDrawingDialog', () => {
         });
     });
 
+    describe('addAnnotation()', () => {
+        it('should setup the element if not already done', () => {
+            dialog.element = undefined;
+            sandbox.stub(dialog, 'setup');
+            sandbox.stub(dialog, 'assignDrawingLabel');
+
+            dialog.addAnnotation({});
+            expect(dialog.setup).to.be.calledWith([{}]);
+            expect(dialog.assignDrawingLabel).to.be.calledWith({});
+
+            dialog.element = document.createElement('div');
+            dialog.addAnnotation({});
+            expect(dialog.setup).to.be.calledWith([{}]);
+            expect(dialog.assignDrawingLabel).to.be.calledWith({});
+        });
+    });
+
     describe('postDrawing()', () => {
         it('should emit annotation create to indicate that the save button was pressed', () => {
             const event = {
@@ -134,7 +151,7 @@ describe('doc/DocDrawingDialog', () => {
 
             sandbox.stub(dialog, 'generateDialogEl').returns(drawingDialogEl);
             sandbox.stub(dialog, 'bindDOMListeners');
-            sandbox.stub(dialog, 'assignDrawingLabel');
+            sandbox.stub(dialog, 'addAnnotation');
         });
 
         it('should setup the dialog element without a commit button when given an annotation', () => {
@@ -148,7 +165,7 @@ describe('doc/DocDrawingDialog', () => {
             dialog.setup([annotation]);
             expect(dialog.generateDialogEl).to.be.called;
             expect(dialog.bindDOMListeners).to.be.called;
-            expect(dialog.assignDrawingLabel).to.be.called;
+            expect(dialog.addAnnotation).to.be.called;
             expect(dialog.element.contains(dialog.drawingDialogEl));
             expect(dialog.commitButtonEl).to.be.null;
         });
@@ -157,7 +174,7 @@ describe('doc/DocDrawingDialog', () => {
             dialog.setup([]);
             expect(dialog.generateDialogEl).to.be.called;
             expect(dialog.bindDOMListeners).to.be.called;
-            expect(dialog.assignDrawingLabel).to.not.be.called;
+            expect(dialog.addAnnotation).to.not.be.called;
             expect(dialog.element.contains(dialog.drawingDialogEl));
             expect(dialog.commitButtonEl).to.not.be.undefined;
             expect(dialog.element.contains(dialog.commitButtonEl));
@@ -276,7 +293,7 @@ describe('doc/DocDrawingDialog', () => {
             sandbox.stub(util, 'replacePlaceholders').returns(notYaoMing);
             sandbox.stub(util, 'showElement');
 
-            dialog.assignDrawingLabel('non empty');
+            dialog.assignDrawingLabel({ user: { id: '1' } });
             expect(drawingLabelEl.textContent).to.equal(notYaoMing);
             expect(dialog.drawingDialogEl.querySelector).to.be.called;
             expect(util.replacePlaceholders).to.be.called;
@@ -284,11 +301,16 @@ describe('doc/DocDrawingDialog', () => {
         });
 
         it('should do nothing when given an invalid annotation or does not have a dialog', () => {
-            expect(dialog.assignDrawingLabel, 'not empty').to.not.throw();
+            sandbox.stub(util, 'showElement');
+
+            dialog.drawingDialogEl = document.createElement('div');
+            dialog.assignDrawingLabel(null);
+            dialog.assignDrawingLabel({ user: { id: '0' } });
+
+            dialog.drawingDialogEl = undefined;
+            dialog.assignDrawingLabel({ user: { id: '1' } });
 
-            dialog.drawingDialogEl = 'not empty';
-            expect(dialog.assignDrawingLabel, undefined).to.not.throw();
-            expect(dialog.assignDrawingLabel, null).to.not.throw();
+            expect(util.showElement).to.not.be.called;
         });
     });
 });