Skip to content

Commit

Permalink
fix(annotations): Fix annotations control shows if no create permissi…
Browse files Browse the repository at this point in the history
…on (#1209)

* fix(annotations): Fix annotations control shows if no create permission

* fix(annotations): Add tests

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
Mingze and mergify[bot] authored May 13, 2020
1 parent 19675be commit 57d9bb4
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 2 deletions.
13 changes: 13 additions & 0 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
24 changes: 24 additions & 0 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
});
Expand Down
32 changes: 32 additions & 0 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -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');
stubs.areNewAnnotationsEnabled = sandbox.stub(docBase, 'areNewAnnotationsEnabled').returns(true);
stubs.hasCreatePermission = sandbox.stub(docBase, 'hasAnnotationCreatePermission').returns(true);
});

it('should add the correct controls', () => {
Expand Down Expand Up @@ -2285,6 +2302,21 @@ 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 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', () => {
Expand Down

0 comments on commit 57d9bb4

Please sign in to comment.