New: Separate create and view permissions for annotations (#358)

Enables annotations to be shown in a view-only setting with the
correct scopes

Annotations permissions will be as follows:
- can_annotate - whether a user can add any new annotations
- can_view_annotations_all - whether a user can view all annotations
- can_view_annotations_self - whether a user can view their own
  annotations (This is not yet supported as Preview is not aware of
  who the user previewing the file is in order to distinguish which
  annotations belong to the current user vs. other users)

src/lib/annotations/AnnotationThread.js

@@ -50,6 +50,7 @@ class AnnotationThread extends EventEmitter {
         this.type = data.type;
         this.locale = data.locale;
         this.isMobile = data.isMobile;
+        this.permissions = data.permissions;
 
         this.setup();
     }

src/lib/annotations/Annotator.js

@@ -52,7 +52,6 @@ class Annotator extends EventEmitter {
     constructor(data) {
         super();
 
-        this.canAnnotate = data.canAnnotate;
         this.container = data.container;
         this.options = data.options;
         this.fileVersionId = data.fileVersionId;
@@ -65,6 +64,14 @@ class Annotator extends EventEmitter {
 
         const { CONTROLLERS } = this.options.annotator || {};
         this.modeControllers = CONTROLLERS || {};
+
+        // Get annotation permissions
+        const permissions = data.options.permissions || {};
+        this.permissions = {
+            canAnnotate: permissions.can_annotate || false,
+            canViewAllAnnotations: permissions.can_view_annotations_all || false,
+            canViewOwnAnnotations: permissions.can_view_annotations_self || false
+        };
     }
 
     /**
@@ -112,8 +119,7 @@ class Annotator extends EventEmitter {
         this.annotationService = new AnnotationService({
             apiHost,
             fileId,
-            token,
-            canAnnotate: this.canAnnotate
+            token
         });
 
         // Set up mobile annotations dialog
@@ -162,7 +168,7 @@ class Annotator extends EventEmitter {
      */
     showModeAnnotateButton(currentMode) {
         const mode = this.modeButtons[currentMode];
-        if (!mode || !this.isModeAnnotatable(currentMode)) {
+        if (!mode || !this.permissions.canAnnotate || !this.isModeAnnotatable(currentMode)) {
             return;
         }
 
@@ -314,7 +320,7 @@ class Annotator extends EventEmitter {
 
         // Only show/hide point annotation button if user has the
         // appropriate permissions
-        if (!this.canAnnotate) {
+        if (!this.permissions.canAnnotate) {
             return;
         }
 
@@ -534,11 +540,18 @@ class Annotator extends EventEmitter {
     fetchAnnotations() {
         this.threads = {};
 
+        // Do not load any pre-existing annotations if the user does not have
+        // the correct permissions
+        if (!this.permissions.canViewAllAnnotations || !this.permissions.canViewOwnAnnotations) {
+            return this.threads;
+        }
+
         return this.annotationService.getThreadMap(this.fileVersionId).then((threadMap) => {
             // Generate map of page to threads
             Object.keys(threadMap).forEach((threadID) => {
                 const annotations = threadMap[threadID];
                 const firstAnnotation = annotations[0];
+
                 if (!firstAnnotation || !this.isModeAnnotatable(firstAnnotation.type)) {
                     return;
                 }

src/lib/annotations/__tests__/Annotator-test.js

@@ -119,7 +119,7 @@ describe('lib/annotations/Annotator', () => {
             stubs.setupMobileDialog = sandbox.stub(annotator, 'setupMobileDialog');
             stubs.showButton = sandbox.stub(annotator, 'showModeAnnotateButton');
 
-            annotator.canAnnotate = true;
+            annotator.permissions.canAnnotate = true;
             annotator.modeButtons = {
                 point: { selector: 'point_btn' },
                 draw: { selector: 'draw_btn' }
@@ -242,7 +242,7 @@ describe('lib/annotations/Annotator', () => {
 
         describe('rotateAnnotations()', () => {
             beforeEach(() => {
-                annotator.canAnnotate = true;
+                annotator.permissions.canAnnotate = true;
                 stubs.hide = sandbox.stub(annotatorUtil, 'hideElement');
                 stubs.show = sandbox.stub(annotatorUtil, 'showElement');
                 stubs.render = sandbox.stub(annotator, 'renderAnnotations');
@@ -258,7 +258,7 @@ describe('lib/annotations/Annotator', () => {
             });
 
             it('should only render annotations if user cannot annotate', () => {
-                annotator.canAnnotate = false;
+                annotator.permissions.canAnnotate = false;
                 annotator.rotateAnnotations();
                 expect(stubs.hide).to.not.be.called;
                 expect(stubs.show).to.not.be.called;
@@ -419,12 +419,32 @@ describe('lib/annotations/Annotator', () => {
                     someID2: [{}]
                 };
                 stubs.threadPromise = Promise.resolve(threadMap);
-                stubs.serviceMock.expects('getThreadMap').returns(stubs.threadPromise);
                 sandbox.stub(annotator, 'emit');
                 sandbox.stub(annotator, 'isModeAnnotatable').returns(true);
+
+                annotator.permissions = {
+                    canViewAllAnnotations: true,
+                    canViewOwnAnnotations: true
+                };
+            });
+
+            it('should not fetch existing annotations if the user does not have correct permissions', () => {
+                stubs.serviceMock.expects('getThreadMap').never();
+                annotator.permissions = {
+                    canViewAllAnnotations: false,
+                    canViewOwnAnnotations: true
+                };
+                annotator.fetchAnnotations();
+
+                annotator.permissions = {
+                    canViewAllAnnotations: true,
+                    canViewOwnAnnotations: false
+                };
+                annotator.fetchAnnotations();
             });
 
             it('should reset and create a new thread map by fetching annotation data from the server', () => {
+                stubs.serviceMock.expects('getThreadMap').returns(stubs.threadPromise);
                 stubs.createThread = sandbox.stub(annotator, 'createAnnotationThread');
                 stubs.createThread.onFirstCall();
                 stubs.createThread.onSecondCall().returns(stubs.thread);
@@ -440,6 +460,7 @@ describe('lib/annotations/Annotator', () => {
             });
 
             it('should emit a message to indicate that all annotations have been fetched', () => {
+                stubs.serviceMock.expects('getThreadMap').returns(stubs.threadPromise);
                 annotator.fetchAnnotations();
                 return stubs.threadPromise.then(() => {
                     expect(annotator.emit).to.be.calledWith('annotationsfetched');
@@ -911,13 +932,22 @@ describe('lib/annotations/Annotator', () => {
                         selector: '.bp-btn-annotate'
                     }
                 };
+                annotator.permissions.canAnnotate = true;
             });
 
             afterEach(() => {
                 annotator.modeButtons = {};
                 annotator.container = document;
             });
 
+            it('should do nothing if user cannot annotate', () => {
+                annotator.permissions.canAnnotate = false;
+                sandbox.stub(annotator, 'isModeAnnotatable').returns(true);
+                sandbox.stub(annotator, 'getAnnotationModeClickHandler');
+                annotator.showModeAnnotateButton(TYPES.point);
+                expect(annotator.getAnnotationModeClickHandler).to.not.be.called;
+            });
+
             it('should do nothing if the mode does not require a button', () => {
                 sandbox.stub(annotator, 'getAnnotationModeClickHandler');
                 sandbox.stub(annotator, 'isModeAnnotatable').returns(true);

src/lib/annotations/doc/DocAnnotator.js

@@ -292,7 +292,8 @@ class DocAnnotator extends Annotator {
             isMobile: this.isMobile,
             locale: this.locale,
             location,
-            type
+            type,
+            permissions: this.permissions
         };
 
         // Set existing thread ID if created with annotations
@@ -455,10 +456,6 @@ class DocAnnotator extends Annotator {
 
         this.annotatedElement.addEventListener('mouseup', this.highlightMouseupHandler);
 
-        if (!this.canAnnotate) {
-            return;
-        }
-
         if (this.hasTouch && this.isMobile) {
             document.addEventListener('selectionchange', this.onSelectionChange);
             this.annotatedElement.addEventListener('touchstart', this.drawingSelectionHandler);
@@ -488,10 +485,6 @@ class DocAnnotator extends Annotator {
             this.highlightThrottleHandle = null;
         }
 
-        if (!this.canAnnotate) {
-            return;
-        }
-
         if (this.hasTouch && this.isMobile) {
             document.removeEventListener('selectionchange', this.onSelectionChange);
             this.annotatedElement.removeEventListener('touchstart', this.drawingSelectionHandler);

src/lib/annotations/doc/DocHighlightDialog.js

@@ -293,9 +293,12 @@ class DocHighlightDialog extends AnnotationDialog {
             ]);
             annotatorUtil.showElement(highlightLabelEl);
 
-            // Hide delete button on plain highlights if user doesn't have
-            // permissions
-            if (annotations[0].permissions && !annotations[0].permissions.can_delete) {
+            if (!this.canAnnotate) {
+                // Hide all action buttons if user cannot annotate
+                const highlightButtons = this.highlightDialogEl.querySelector(constants.SELECTOR_HIGHLIGHT_BTNS);
+                annotatorUtil.hideElement(highlightButtons);
+            } else if (annotations[0].permissions && !annotations[0].permissions.can_delete) {
+                // Hide delete button on plain highlights if user doesn't have permissions
                 const addHighlightBtn = this.highlightDialogEl.querySelector(constants.SELECTOR_ADD_HIGHLIGHT_BTN);
                 annotatorUtil.hideElement(addHighlightBtn);
             }
@@ -306,7 +309,7 @@ class DocHighlightDialog extends AnnotationDialog {
             this.addAnnotationElement(annotation);
         });
 
-        if (!this.isMobile) {
+        if (!this.isMobile && this.canAnnotate) {
             this.bindDOMListeners();
         }
     }

src/lib/annotations/doc/DocHighlightThread.js

@@ -273,7 +273,7 @@ class DocHighlightThread extends AnnotationThread {
             annotations: this.annotations,
             locale: this.locale,
             location: this.location,
-            canAnnotate: this.annotationService.canAnnotate
+            canAnnotate: this.permissions.canAnnotate
         });
 
         // Ensures that previously created annotations have the right type

src/lib/annotations/doc/DocPointThread.js

@@ -24,7 +24,7 @@ class DocPointThread extends AnnotationThread {
      */
     showDialog() {
         // Don't show dialog if user can annotate and there is a current selection
-        if (this.annotationService.canAnnotate && docAnnotatorUtil.isSelectionPresent()) {
+        if (this.permissions.canAnnotate && docAnnotatorUtil.isSelectionPresent()) {
             return;
         }
 
@@ -78,7 +78,7 @@ class DocPointThread extends AnnotationThread {
             annotations: this.annotations,
             locale: this.locale,
             location: this.location,
-            canAnnotate: this.annotationService.canAnnotate
+            canAnnotate: this.permissions.canAnnotate
         });
     }
 }

src/lib/annotations/doc/__tests__/DocAnnotator-test.js

@@ -530,19 +530,6 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             stubs.elMock = sandbox.mock(annotator.annotatedElement);
         });
 
-        it('shouldn\'t bind DOM listeners if user cannot annotate except mouseup', () => {
-            annotator.canAnnotate = false;
-
-            stubs.elMock.expects('addEventListener').withArgs('mouseup', sinon.match.func);
-            stubs.elMock.expects('addEventListener').withArgs('dblclick', sinon.match.func).never();
-            stubs.elMock.expects('addEventListener').withArgs('mousedown', sinon.match.func).never();
-            stubs.elMock.expects('addEventListener').withArgs('contextmenu', sinon.match.func).never();
-            stubs.elMock.expects('addEventListener').withArgs('mousemove', sinon.match.func).never();
-            stubs.elMock.expects('addEventListener').withArgs('touchstart', sinon.match.func).never();
-            stubs.elMock.expects('addEventListener').withArgs('click', sinon.match.func).never();
-            annotator.bindDOMListeners();
-        });
-
         it('should bind DOM listeners if user can annotate', () => {
             annotator.canAnnotate = true;
 
@@ -556,7 +543,7 @@ describe('lib/annotations/doc/DocAnnotator', () => {
         });
 
         it('should bind selectionchange event, on the document, if on mobile and can annotate', () => {
-            annotator.annotationService.canAnnotate = true;
+            annotator.permissions.canAnnotate = true;
             annotator.isMobile = true;
             annotator.hasTouch = true;
             const docListen = sandbox.spy(document, 'addEventListener');
@@ -578,18 +565,6 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             annotator.highlightMousemoveHandler = () => {};
         });
 
-        it('should not unbind DOM listeners if user cannot annotate except mouseup', () => {
-            annotator.canAnnotate = false;
-
-            stubs.elMock.expects('removeEventListener').withArgs('mouseup', sinon.match.func);
-            stubs.elMock.expects('removeEventListener').withArgs('mousedown', sinon.match.func).never();
-            stubs.elMock.expects('removeEventListener').withArgs('contextmenu', sinon.match.func).never();
-            stubs.elMock.expects('removeEventListener').withArgs('mousemove', sinon.match.func).never();
-            stubs.elMock.expects('removeEventListener').withArgs('dblclick', sinon.match.func).never();
-            stubs.elMock.expects('removeEventListener').withArgs('click', sinon.match.func).never();
-            annotator.unbindDOMListeners();
-        });
-
         it('should unbind DOM listeners if user can annotate', () => {
             annotator.canAnnotate = true;
 
@@ -616,7 +591,7 @@ describe('lib/annotations/doc/DocAnnotator', () => {
         });
 
         it('should unbind selectionchange event, on the document, if on mobile, has touch and can annotate', () => {
-            annotator.annotationService.canAnnotate = true;
+            annotator.permissions.canAnnotate = true;
             annotator.isMobile = true;
             annotator.hasTouch = true;
             const docStopListen = sandbox.spy(document, 'removeEventListener');

src/lib/annotations/doc/__tests__/DocHighlightDialog-test.js

@@ -399,7 +399,20 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             expect(stubs.show).to.be.called;
         });
 
-        it('should hide delete button on plain highlights if user does not have permissions', () => {
+        it('should hide all buttons on plain highlights if user does not have canAnnotate permissions', () => {
+            sandbox.stub(annotatorUtil, 'isPlainHighlight').returns(true);
+            dialog.canAnnotate = false;
+
+            dialog.setup([stubs.annotation]);
+            const highlightLabelEl = dialog.highlightDialogEl.querySelector(`.${CLASS_HIGHLIGHT_LABEL}`);
+            const highlightBtns = dialog.highlightDialogEl.querySelector(constants.SELECTOR_HIGHLIGHT_BTNS);
+            const addHighlightBtn = dialog.highlightDialogEl.querySelector(constants.SELECTOR_ADD_HIGHLIGHT_BTN);
+            expect(stubs.show).to.be.calledWith(highlightLabelEl);
+            expect(stubs.hide).to.be.calledWith(highlightBtns);
+            expect(stubs.hide).to.not.be.calledWith(addHighlightBtn);
+        });
+
+        it('should hide delete button on plain highlights if user does not have delete permissions', () => {
             sandbox.stub(annotatorUtil, 'isPlainHighlight').returns(true);
             stubs.annotation.permissions.can_delete = false;
 

src/lib/annotations/doc/__tests__/DocHighlightThread-test.js

@@ -33,7 +33,11 @@ describe('lib/annotations/doc/DocHighlightThread', () => {
             fileVersionId: 1,
             location: {},
             threadID: 2,
-            type: 'highlight'
+            type: 'highlight',
+            permissions: {
+                canAnnotate: true,
+                canViewAllAnnotations: true
+            }
         });
         highlightThread.dialog.setup([]);
     });
@@ -151,7 +155,11 @@ describe('lib/annotations/doc/DocHighlightThread', () => {
                 fileVersionId: 1,
                 location: {},
                 threadID: 2,
-                type: 'highlight'
+                type: 'highlight',
+                permissions: {
+                    canAnnotate: true,
+                    canViewAllAnnotations: true
+                }
             });
             plainHighlightThread.dialog.setup([]);
 
@@ -178,7 +186,11 @@ describe('lib/annotations/doc/DocHighlightThread', () => {
                 fileVersionId: 1,
                 location: {},
                 threadID: 2,
-                type: 'highlight'
+                type: 'highlight',
+                permissions: {
+                    canAnnotate: true,
+                    canViewAllAnnotations: true
+                }
             });
 
             Object.defineProperty(Object.getPrototypeOf(DocHighlightThread.prototype), 'deleteAnnotation', {