From 3c36f68947758791a4ac856897c6275ce4e255a9 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Mon, 26 Feb 2018 11:51:10 -0800 Subject: [PATCH 1/5] Fix: Cleanup baseViewer.hasAnnotationPermissions() and tests --- src/lib/viewers/BaseViewer.js | 6 ++++-- src/lib/viewers/__tests__/BaseViewer-test.js | 20 ++++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 9f2a20a8f..599601f16 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -811,8 +811,10 @@ class BaseViewer extends EventEmitter { return false; } - const canViewAnnotations = !!(permissions.can_view_annotations_all || permissions.can_view_annotations_self); - return !permissions.can_annotate && !canViewAnnotations; + const { can_annotate, can_view_annotations_all, can_view_annotations_self } = permissions; + + // eslint-disable-next-line + return can_annotate || can_view_annotations_all || can_view_annotations_self; } /** diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 3cd829cca..b075fb21a 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -86,6 +86,7 @@ describe('lib/viewers/BaseViewer', () => { base.isMobile = true; sandbox.stub(base, 'loadAnnotator'); sandbox.stub(base, 'finishLoadingSetup'); + sandbox.stub(base, 'areAnnotationsEnabled').returns(true); base.setup(); @@ -382,8 +383,8 @@ describe('lib/viewers/BaseViewer', () => { sinon.assert.failException; }) .catch((error) => { - expect(error).equals(sinon.match.error); - expect(error.message).equals('message'); + expect(error).to.equal(sinon.match.error); + expect(error.message).to.equal('message'); }); }); }); @@ -954,28 +955,29 @@ describe('lib/viewers/BaseViewer', () => { }; it('does nothing if file permissions are undefined', () => { - expect(base.hasAnnotationPermissions()).to.be.falsy; + expect(base.hasAnnotationPermissions()).to.equal(false); }); it('should return false if the user can neither annotate nor view all or their own annotations', () => { - expect(base.hasAnnotationPermissions(permissions)).to.be.falsy; + expect(base.hasAnnotationPermissions(permissions)).to.equal(false); }); it('should return true if the user can at least view all annotations', () => { permissions.can_view_annotations_all = true; - expect(base.hasAnnotationPermissions(permissions)).to.be.truthy; + expect(base.hasAnnotationPermissions(permissions)).to.equal(true); }); it('should return true if the user can at least view their own annotations', () => { permissions.can_view_annotations_all = false; permissions.can_view_annotations_self = true; - expect(base.hasAnnotationPermissions(permissions)).to.be.truthy; + expect(base.hasAnnotationPermissions(permissions)).to.equal(true); }); }); describe('areAnnotationsEnabled()', () => { beforeEach(() => { stubs.getViewerOption = sandbox.stub(base, 'getViewerOption').withArgs('annotations').returns(false); + stubs.hasPermissions = sandbox.stub(base, 'hasAnnotationPermissions').returns(true); base.options.file = { permissions: { can_annotate: true @@ -983,8 +985,14 @@ describe('lib/viewers/BaseViewer', () => { }; }); + it('should return false if the user cannot create/view annotations', () => { + stubs.hasPermissions.returns(false); + expect(base.areAnnotationsEnabled()).to.equal(false); + }); + it('should return true if viewer option is set to true', () => { expect(base.areAnnotationsEnabled()).to.equal(false); + stubs.getViewerOption.returns(true); expect(base.areAnnotationsEnabled()).to.equal(true); }); From 40f7ca73f7d717b66f01e4e7195fb391dfc8ed3a Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Tue, 27 Feb 2018 12:56:08 -0800 Subject: [PATCH 2/5] Chore: Force boolean --- src/lib/viewers/BaseViewer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 599601f16..88c37e299 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -814,7 +814,7 @@ class BaseViewer extends EventEmitter { const { can_annotate, can_view_annotations_all, can_view_annotations_self } = permissions; // eslint-disable-next-line - return can_annotate || can_view_annotations_all || can_view_annotations_self; + return !!(can_annotate || can_view_annotations_all || can_view_annotations_self); } /** From f21c735f741e52d47b72586ed087869170c785d9 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Tue, 27 Feb 2018 16:00:52 -0800 Subject: [PATCH 3/5] Update: Use chai --- src/lib/viewers/__tests__/BaseViewer-test.js | 40 ++++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index b075fb21a..5b2715db7 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -575,7 +575,7 @@ describe('lib/viewers/BaseViewer', () => { stubs.isIOS.returns(true); base.mobileZoomStartHandler(event); - expect(base._scaling).to.equal(true); + expect(base._scaling).to.be.true; expect(event.stopPropagation).to.be.called; expect(event.preventDefault).to.be.called; }); @@ -584,7 +584,7 @@ describe('lib/viewers/BaseViewer', () => { stubs.isIOS.returns(false); base.mobileZoomStartHandler(event); - expect(base._scaling).to.equal(true); + expect(base._scaling).to.be.true; expect(base._pinchScale).to.not.equal(undefined); expect(event.stopPropagation).to.be.called; expect(event.preventDefault).to.be.called; @@ -595,7 +595,7 @@ describe('lib/viewers/BaseViewer', () => { event.touches = [0]; base.mobileZoomStartHandler(event); - expect(base._scaling).to.equal(false); + expect(base._scaling).to.be.false; expect(base._pinchScale).to.equal(undefined); expect(event.stopPropagation).to.not.be.called; expect(event.preventDefault).to.not.be.called; @@ -667,7 +667,7 @@ describe('lib/viewers/BaseViewer', () => { base.mobileZoomEndHandler(event); expect(base.zoomIn).to.be.called; - expect(base._scaling).to.equal(false); + expect(base._scaling).to.be.false; expect(base._pincScale).to.equal(undefined); }); @@ -691,7 +691,7 @@ describe('lib/viewers/BaseViewer', () => { base.mobileZoomEndHandler(event); expect(base.zoomOut).to.be.called; expect(base.zoomIn).to.not.be.called; - expect(base._scaling).to.equal(false); + expect(base._scaling).to.be.false; expect(base._pincScale).to.equal(undefined); }); }); @@ -955,22 +955,22 @@ describe('lib/viewers/BaseViewer', () => { }; it('does nothing if file permissions are undefined', () => { - expect(base.hasAnnotationPermissions()).to.equal(false); + expect(base.hasAnnotationPermissions()).to.be.false; }); it('should return false if the user can neither annotate nor view all or their own annotations', () => { - expect(base.hasAnnotationPermissions(permissions)).to.equal(false); + expect(base.hasAnnotationPermissions(permissions)).to.be.false; }); it('should return true if the user can at least view all annotations', () => { permissions.can_view_annotations_all = true; - expect(base.hasAnnotationPermissions(permissions)).to.equal(true); + expect(base.hasAnnotationPermissions(permissions)).to.be.true; }); it('should return true if the user can at least view their own annotations', () => { permissions.can_view_annotations_all = false; permissions.can_view_annotations_self = true; - expect(base.hasAnnotationPermissions(permissions)).to.equal(true); + expect(base.hasAnnotationPermissions(permissions)).to.be.true; }); }); @@ -987,30 +987,30 @@ describe('lib/viewers/BaseViewer', () => { it('should return false if the user cannot create/view annotations', () => { stubs.hasPermissions.returns(false); - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; }); it('should return true if viewer option is set to true', () => { - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; stubs.getViewerOption.returns(true); - expect(base.areAnnotationsEnabled()).to.equal(true); + expect(base.areAnnotationsEnabled()).to.be.true; }); it('should use the global showAnnotations boolean if the viewer param is not specified', () => { stubs.getViewerOption.withArgs('annotations').returns(null); base.options.showAnnotations = true; - expect(base.areAnnotationsEnabled()).to.equal(true); + expect(base.areAnnotationsEnabled()).to.be.true; base.options.showAnnotations = false; - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; }); 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; - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; base.options.viewer = { NAME: 'viewerName' }; base.options.boxAnnotations = sinon.createStubInstance(window.BoxAnnotations); @@ -1019,29 +1019,29 @@ describe('lib/viewers/BaseViewer', () => { // No enabled annotators in options boxAnnotations.options = { 'nope': 'wrong options type' }; boxAnnotations.viewerOptions = undefined; - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; // All default types enabled boxAnnotations.viewerOptions = { 'viewerName': { enabled: true } }; - expect(base.areAnnotationsEnabled()).to.equal(true); + expect(base.areAnnotationsEnabled()).to.be.true; // No specified enabled types boxAnnotations.viewerOptions = { 'viewerName': { enabledTypes: [] } }; - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; // Specified types enabled boxAnnotations.viewerOptions = { 'viewerName': { enabledTypes: [ 'point' ] } }; - expect(base.areAnnotationsEnabled()).to.equal(true); + expect(base.areAnnotationsEnabled()).to.be.true; // No passed in version of BoxAnnotations window.BoxAnnotations = undefined; - expect(base.areAnnotationsEnabled()).to.equal(false); + expect(base.areAnnotationsEnabled()).to.be.false; }); }); From 6127c6e58f0c99184df59e69a714fe7473c0efd1 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Mon, 26 Mar 2018 16:01:03 -0700 Subject: [PATCH 4/5] Chore: Cleanup --- src/lib/viewers/BaseViewer.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index b7115c75d..9be243d39 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -873,10 +873,11 @@ class BaseViewer extends EventEmitter { return false; } - const { can_annotate, can_view_annotations_all, can_view_annotations_self } = permissions; - - // eslint-disable-next-line - return !!(can_annotate || can_view_annotations_all || can_view_annotations_self); + return !!( + permissions.can_annotate || + permissions.can_view_annotations_all || + permissions.can_view_annotations_self + ); } /** @@ -886,7 +887,8 @@ class BaseViewer extends EventEmitter { */ areAnnotationsEnabled() { // Do not attempt to fetch annotations if the user cannot create or view annotations - if (!this.hasAnnotationPermissions(this.options.file)) { + const { permissions } = this.options.file; + if (!this.hasAnnotationPermissions(permissions)) { return false; } From 6686eaf5b97c6c9a6a92116a5e7a3f7006198eed Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Mon, 26 Mar 2018 16:08:45 -0700 Subject: [PATCH 5/5] Update BaseViewer-test.js --- src/lib/viewers/__tests__/BaseViewer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 6388d552c..775c1293e 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -500,7 +500,7 @@ describe('lib/viewers/BaseViewer', () => { sinon.assert.failException; }) .catch((error) => { - expect(error).to.equal(sinon.match.error); + expect(error).to.be.an('error'); expect(error.message).to.equal('message'); }); });