Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Disable annotations if user has neither annotate nor view permissions #598

Merged
merged 4 commits into from
Jan 22, 2018
Merged

Fix: Disable annotations if user has neither annotate nor view permissions #598

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78942ba
Fix: Disable annotations if user has no annotate or view permissions
pramodsum Jan 22, 2018
ede6866
Fix: Tests
pramodsum Jan 22, 2018
e419b85
Chore: Move annotations permissions check into own method
pramodsum Jan 22, 2018
c87eb62
Merge branch 'master' into master
pramodsum Jan 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/lib/viewers/BaseViewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,15 @@ class BaseViewer extends EventEmitter {
* @return {boolean} Whether or not viewer is annotatable
*/
areAnnotationsEnabled() {
// Do not attempt to fetch annotations if the user cannot create or view annotations
const { permissions } = this.options.file;
if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we abstract this into a function? hasAnnotationPermissions()?

!permissions.can_annotate &&
!(permissions.can_view_annotations_all || permissions.can_view_annotations_self)
) {
return false;
}

// Respect viewer-specific annotation option if it is set
if (
window.BoxAnnotations &&
Expand Down
35 changes: 33 additions & 2 deletions src/lib/viewers/__tests__/BaseViewer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ describe('lib/viewers/BaseViewer', () => {
base = new BaseViewer({
container: containerEl,
file: {
id: '0'
id: '0',
permissions: {
can_annotate: false
}
}
});
});
Expand All @@ -62,7 +65,10 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.options).to.deep.equal({
container: containerEl,
file: {
id: '0'
id: '0',
permissions: {
can_annotate: false
}
},
showAnnotations: true
});
Expand Down Expand Up @@ -292,6 +298,7 @@ describe('lib/viewers/BaseViewer', () => {
stubs.fullscreenAddListener = sandbox.stub(fullscreen, 'addListener');
stubs.baseAddListener = sandbox.spy(base, 'addListener');
stubs.documentAddEventListener = sandbox.stub(document.defaultView, 'addEventListener');
base.containerEl = document;
});

it('should append common event listeners', () => {
Expand Down Expand Up @@ -517,6 +524,7 @@ describe('lib/viewers/BaseViewer', () => {
}
});
sandbox.stub(base, 'loadAssets').returns(Promise.resolve());
sandbox.stub(base, 'areAnnotationsEnabled').returns(false);
sandbox.stub(base, 'loadAnnotator');
sandbox.stub(base, 'finishLoadingSetup');
base.setup();
Expand Down Expand Up @@ -905,6 +913,29 @@ describe('lib/viewers/BaseViewer', () => {
describe('areAnnotationsEnabled()', () => {
beforeEach(() => {
stubs.getViewerOption = sandbox.stub(base, 'getViewerOption').withArgs('annotations').returns(false);
base.options.file = {
permissions: {
can_annotate: true
}
};
});

it('should return false if the user can neither annotate nor view all or their own annotations', () => {
stubs.getViewerOption.returns(true);

base.options.file.permissions = {
can_annotate: false,
can_view_annotations_all: false,
can_view_annotations_self: false
};
expect(base.areAnnotationsEnabled()).to.equal(false);

base.options.file.permissions.can_view_annotations_all = true;
expect(base.areAnnotationsEnabled()).to.equal(true);

base.options.file.permissions.can_view_annotations_all = false;
base.options.file.permissions.can_view_annotations_self = true;
expect(base.areAnnotationsEnabled()).to.equal(true);
});

it('should return true if viewer option is set to true', () => {
Expand Down