From 33fd03a42905c59e90da6723f1d985cbadf0b3e9 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Tue, 12 May 2020 16:38:15 -0700 Subject: [PATCH 1/2] fix(annotations): Fix annotations control shows if no create permission --- src/lib/viewers/BaseViewer.js | 13 ++++++++++ src/lib/viewers/__tests__/BaseViewer-test.js | 24 +++++++++++++++++++ src/lib/viewers/doc/DocBaseViewer.js | 4 ++-- .../doc/__tests__/DocBaseViewer-test.js | 20 ++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 9238166c4..52122414a 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -1006,6 +1006,19 @@ class BaseViewer extends EventEmitter { ); } + /** + * Returns whether or not user has permissions to create annotations on the current file + * + * @param {Object} permissions Permissions on the current file + * @return {boolean} Whether or not user has create permission + */ + hasAnnotationCreatePermission(permissions = this.options.file.permissions) { + if (!permissions) { + return false; + } + return permissions.can_annotate || permissions.can_create_annotations; + } + /** * Returns whether or not annotations are enabled for this viewer. * diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 076448093..06360caab 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -1195,6 +1195,30 @@ describe('lib/viewers/BaseViewer', () => { }); }); + describe('hasAnnotationCreatePermission()', () => { + let permissions = {}; + beforeEach(() => { + permissions = { + can_annotate: false, + can_create_annotations: false, + }; + }); + + it('should return false if both create permissions are false', () => { + expect(base.hasAnnotationCreatePermission(permissions)).to.be.false; + }); + + it('should return true if it has old create permission', () => { + permissions.can_annotate = true; + expect(base.hasAnnotationCreatePermission(permissions)).to.be.true; + }); + + it('should return true if it has new create permission', () => { + permissions.can_create_annotations = true; + expect(base.hasAnnotationCreatePermission(permissions)).to.be.true; + }); + }); + describe('hasAnnotationPermissions()', () => { const permissions = { can_annotate: false, // Old diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 9ec8f16d0..f4b66b5ff 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -1015,7 +1015,7 @@ class DocBaseViewer extends BaseViewer { this.controls = new Controls(this.containerEl); this.pageControls = new PageControls(this.controls, this.docEl); this.zoomControls = new ZoomControls(this.controls); - if (this.areNewAnnotationsEnabled()) { + if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { this.annotationControls = new AnnotationControls(this.controls); } this.pageControls.addListener('pagechange', this.setPage); @@ -1100,7 +1100,7 @@ class DocBaseViewer extends BaseViewer { ); this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); - if (this.areNewAnnotationsEnabled()) { + if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { this.annotationControls.init({ onRegionClick: this.regionClickHandler, }); diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 4568644bc..74a308dcd 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -5,6 +5,7 @@ import DocFindBar from '../DocFindBar'; import Browser from '../../../Browser'; import BaseViewer from '../../BaseViewer'; import Controls from '../../../Controls'; +import AnnotationControls from '../../../AnnotationControls'; import PageControls from '../../../PageControls'; import ZoomControls from '../../../ZoomControls'; import fullscreen from '../../../Fullscreen'; @@ -1637,6 +1638,15 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(docBase.zoomControls instanceof ZoomControls).to.be.true; expect(docBase.pageControls.contentEl).to.equal(docBase.docEl); }); + + it('should add annotations controls', () => { + sandbox.stub(docBase, 'bindControlListeners'); + sandbox.stub(docBase, 'areNewAnnotationsEnabled').returns(true); + sandbox.stub(docBase, 'hasAnnotationCreatePermission').returns(true); + + docBase.loadUI(); + expect(docBase.annotationControls instanceof AnnotationControls).to.be.true; + }); }); describe('bindDOMListeners()', () => { @@ -2242,7 +2252,14 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { removeListener: sandbox.stub(), }; + docBase.annotationControls = { + init: sandbox.stub(), + destroy: sandbox.stub(), + }; + stubs.isFindDisabled = sandbox.stub(docBase, 'isFindDisabled'); + sandbox.stub(docBase, 'areNewAnnotationsEnabled').returns(true); + sandbox.stub(docBase, 'hasAnnotationCreatePermission').returns(true); }); it('should add the correct controls', () => { @@ -2285,6 +2302,9 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT, ); + expect(docBase.annotationControls.init).to.be.calledWith({ + onRegionClick: docBase.regionClickHandler, + }); }); it('should not add the toggle thumbnails control if the option is not enabled', () => { From 95e49c36400a669e781018707802658523b5b805 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Tue, 12 May 2020 17:49:41 -0700 Subject: [PATCH 2/2] fix(annotations): Add tests --- .../viewers/doc/__tests__/DocBaseViewer-test.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 74a308dcd..46a53e23f 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -2258,8 +2258,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }; stubs.isFindDisabled = sandbox.stub(docBase, 'isFindDisabled'); - sandbox.stub(docBase, 'areNewAnnotationsEnabled').returns(true); - sandbox.stub(docBase, 'hasAnnotationCreatePermission').returns(true); + stubs.areNewAnnotationsEnabled = sandbox.stub(docBase, 'areNewAnnotationsEnabled').returns(true); + stubs.hasCreatePermission = sandbox.stub(docBase, 'hasAnnotationCreatePermission').returns(true); }); it('should add the correct controls', () => { @@ -2307,6 +2307,18 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); }); + it('should not add annotationControls if no create permission', () => { + stubs.hasCreatePermission.returns(false); + + expect(docBase.annotationControls.init).not.to.be.called; + }); + + it('should not add annotationControls if new annotations is not enabled', () => { + stubs.areNewAnnotationsEnabled.returns(false); + + expect(docBase.annotationControls.init).not.to.be.called; + }); + it('should not add the toggle thumbnails control if the option is not enabled', () => { // Create a new instance that has enableThumbnailsSidebar as false docBase.options.enableThumbnailsSidebar = false;