From c1f0bc06d9cb3af9c8f596cfc9644bf2083e2016 Mon Sep 17 00:00:00 2001
From: Sumedha Pramod <pramods@umich.edu>
Date: Wed, 20 Dec 2017 14:42:58 -0800
Subject: [PATCH] Chore: Add safety check for annotatorConfig (#554)

---
 src/lib/viewers/BaseViewer.js                |  6 +--
 src/lib/viewers/__tests__/BaseViewer-test.js | 53 +++++++++++++++-----
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js
index 0ed30f4d8..8959e88a1 100644
--- a/src/lib/viewers/BaseViewer.js
+++ b/src/lib/viewers/BaseViewer.js
@@ -390,9 +390,9 @@ class BaseViewer extends EventEmitter {
         }
 
         if (this.annotationsLoadPromise) {
-            this.annotationsLoadPromise.then(this.annotationsLoadHandler).catch(() => {
+            this.annotationsLoadPromise.then(this.annotationsLoadHandler).catch((err) => {
                 /* eslint-disable no-console */
-                console.error('Annotation assets failed to load');
+                console.error('Annotation assets failed to load', err);
                 /* eslint-enable no-console */
             });
         }
@@ -763,7 +763,7 @@ class BaseViewer extends EventEmitter {
             const { boxAnnotations, viewer } = this.options;
             const annotatorConfig = boxAnnotations.options[viewer.NAME];
             this.viewerConfig = {
-                enabled: annotatorConfig.enabled || !!annotatorConfig.enabledTypes
+                enabled: annotatorConfig && (annotatorConfig.enabled || annotatorConfig.enabledTypes.length > 0)
             };
         } else {
             this.viewerConfig = this.getViewerAnnotationsConfig();
diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js
index 4d959d36a..d5da37b5c 100644
--- a/src/lib/viewers/__tests__/BaseViewer-test.js
+++ b/src/lib/viewers/__tests__/BaseViewer-test.js
@@ -326,26 +326,36 @@ describe('lib/viewers/BaseViewer', () => {
     describe('viewerLoadHandler()', () => {
         beforeEach(() => {
             base.annotationsLoadPromise = Promise.resolve();
-            sandbox.stub(base, 'annotationsLoadHandler');
+            stubs.annotationsLoadHandler = sandbox.stub(base, 'annotationsLoadHandler');
         });
 
         it('should set the scale if it exists', () => {
-            base.viewerLoadHandler({
-                scale: 1.5
-            });
-
+            base.viewerLoadHandler({ scale: 1.5 });
             expect(base.scale).to.equal(1.5);
         });
 
         it('should handle the annotations load promise', () => {
-             base.viewerLoadHandler({
-                scale: 1.5
-            });
-
+            base.viewerLoadHandler({ scale: 1.5 });
             return base.annotationsLoadPromise.then(() => {
                 expect(base.annotationsLoadHandler).to.be.called;
             });
         });
+
+        it('should handle the annotations load promise', () => {
+            stubs.annotationsLoadHandler.callsFake(() => {
+                throw new Error('message');
+            });
+
+            base.viewerLoadHandler({ scale: 1.5 });
+            return base.annotationsLoadPromise
+                .then(() => {
+                    sinon.assert.failException;
+                })
+                .catch((error) => {
+                    expect(error).equals(sinon.match.error);
+                    expect(error.message).equals('message');
+                });
+        });
     });
 
     describe('toggleFullscreen()', () => {
@@ -886,7 +896,7 @@ describe('lib/viewers/BaseViewer', () => {
             expect(base.areAnnotationsEnabled()).to.equal(true);
         });
 
-        it('should use the global show annotationsBoolean if the viewer param is not specified', () => {
+        it('should use the global show annotations boolean if the viewer param is not specified', () => {
             stubs.getViewerOption.withArgs('annotations').returns(null);
             base.options.showAnnotations = true;
             expect(base.areAnnotationsEnabled()).to.equal(true);
@@ -895,7 +905,7 @@ describe('lib/viewers/BaseViewer', () => {
             expect(base.areAnnotationsEnabled()).to.equal(false);
         });
 
-        it('should user BoxAnnotations options if an instance of BoxAnnotations is passed into Preview', () => {
+        it('should use BoxAnnotations options if an instance of BoxAnnotations is passed into Preview', () => {
             stubs.getViewerOption.withArgs('annotations').returns(null);
             base.options.showAnnotations = false;
             base.options.boxAnnotations = undefined;
@@ -903,11 +913,30 @@ describe('lib/viewers/BaseViewer', () => {
 
             base.options.viewer = { NAME: 'viewerName' };
             base.options.boxAnnotations = sinon.createStubInstance(window.BoxAnnotations);
+
+            // No enabled annotators in options
+            base.options.boxAnnotations.options = {};
+            expect(base.areAnnotationsEnabled()).to.equal(false);
+
+            // All default types enabled
             base.options.boxAnnotations.options = {
                 'viewerName': { enabled: true }
-            }
+            };
+            expect(base.areAnnotationsEnabled()).to.equal(true);
+
+            // No specified enabled types
+            base.options.boxAnnotations.options = {
+                'viewerName': { enabledTypes: [] }
+            };
+            expect(base.areAnnotationsEnabled()).to.equal(false);
+
+            // Specified types enabled
+            base.options.boxAnnotations.options = {
+                'viewerName': { enabledTypes: [ 'point' ] }
+            };
             expect(base.areAnnotationsEnabled()).to.equal(true);
 
+            // No passed in version of BoxAnnotations
             window.BoxAnnotations = undefined;
             expect(base.areAnnotationsEnabled()).to.equal(false);
         });