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

feat(permissions): Add checks for new permission values on file object #485

Merged
merged 2 commits into from
May 11, 2020
Merged

feat(permissions): Add checks for new permission values on file object #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9de4d93
feat(permissions): Add checks for new permission values on file object
jstoffan May 11, 2020
f3f99f7
Merge branch 'master' into add-permissions
mergify[bot] May 11, 2020
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
4 changes: 4 additions & 0 deletions src/@types/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
export enum PERMISSIONS {
CAN_ANNOTATE = 'can_annotate',
CAN_CREATE_ANNOTATIONS = 'can_create_annotations',
CAN_VIEW_ANNOTATIONS = 'can_view_annotations',
CAN_VIEW_ANNOTATIONS_ALL = 'can_view_annotations_all',
CAN_VIEW_ANNOTATIONS_SELF = 'can_view_annotations_self',
}

export interface Permissions {
[index: string]: boolean | undefined;
[PERMISSIONS.CAN_ANNOTATE]?: boolean;
[PERMISSIONS.CAN_CREATE_ANNOTATIONS]?: boolean;
[PERMISSIONS.CAN_VIEW_ANNOTATIONS]?: boolean;
[PERMISSIONS.CAN_VIEW_ANNOTATIONS_ALL]?: boolean;
[PERMISSIONS.CAN_VIEW_ANNOTATIONS_SELF]?: boolean;
}
Expand Down
8 changes: 5 additions & 3 deletions src/BoxAnnotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ class BoxAnnotations {
}

return (
!!permissions[PERMISSIONS.CAN_ANNOTATE] ||
!!permissions[PERMISSIONS.CAN_VIEW_ANNOTATIONS_ALL] ||
!!permissions[PERMISSIONS.CAN_VIEW_ANNOTATIONS_SELF]
!!permissions[PERMISSIONS.CAN_ANNOTATE] || // TODO: Remove once permissions are fully migrated
!!permissions[PERMISSIONS.CAN_CREATE_ANNOTATIONS] ||
!!permissions[PERMISSIONS.CAN_VIEW_ANNOTATIONS] ||
!!permissions[PERMISSIONS.CAN_VIEW_ANNOTATIONS_ALL] || // TODO: Remove once permissions are fully migrated
!!permissions[PERMISSIONS.CAN_VIEW_ANNOTATIONS_SELF] // TODO: Remove once permissions are fully migrated
);
}

Expand Down
44 changes: 28 additions & 16 deletions src/__tests__/BoxAnnotations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,49 +21,61 @@ describe('BoxAnnotations', () => {
beforeEach(() => {
permissions = {
can_annotate: false,
can_create_annotations: false,
can_view_annotations: false,
can_view_annotations_all: false,
can_view_annotations_self: false,
};
});

it('should return false if permissions do not exist', () => {
test('should return false if permissions do not exist', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the alias?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The convention is it = Karma and test = Jest. Most of the other tests use test.

expect(loader.canLoad()).toBe(false);
});

it('should return true if user has at least can_annotate permissions', () => {
test('should return true if user has at least can_annotate permissions', () => {
permissions.can_annotate = true;
expect(loader.canLoad(permissions)).toBe(true);
});

it('should return true if user has at least can_view_annotations_all permissions', () => {
test('should return true if user has at least can_create_annotations permissions', () => {
permissions.can_create_annotations = true;
expect(loader.canLoad(permissions)).toBe(true);
});

test('should return true if user has at least can_view_annotations permissions', () => {
permissions.can_view_annotations = true;
expect(loader.canLoad(permissions)).toBe(true);
});

test('should return true if user has at least can_view_annotations_all permissions', () => {
permissions.can_view_annotations_all = true;
expect(loader.canLoad(permissions)).toBe(true);
});

it('should return true if user has at least can_view_annotations_self permissions', () => {
test('should return true if user has at least can_view_annotations_self permissions', () => {
permissions.can_view_annotations_self = true;
expect(loader.canLoad(permissions)).toBe(true);
});
});

describe('getAnnotators()', () => {
it("should return the loader's annotators", () => {
test("should return the loader's annotators", () => {
expect(loader.getAnnotators()).toStrictEqual(loader.annotators);
});

it("should return an empty array if the loader doesn't have annotators", () => {
test("should return an empty array if the loader doesn't have annotators", () => {
loader.annotators = [];
expect(loader.getAnnotators()).toStrictEqual([]);
});
});

describe('getAnnotatorsForViewer()', () => {
it('should return undefined if the annotator does not exist', () => {
test('should return undefined if the annotator does not exist', () => {
const annotator = loader.getAnnotatorsForViewer('not_supported_type');
expect(annotator).toBeUndefined();
});

it('should return the correct annotator for the viewer name', () => {
test('should return the correct annotator for the viewer name', () => {
const name = 'Document';
const annotator = loader.getAnnotatorsForViewer(name);
expect(annotator.NAME).toEqual(name); // First entry is Document annotator
Expand Down Expand Up @@ -93,14 +105,14 @@ describe('BoxAnnotations', () => {
};
});

it('should use the specified types from options', () => {
test('should use the specified types from options', () => {
loader.viewerOptions = {
Document: { enabledTypes: ['region'] },
};
expect(loader.determineAnnotator(options).TYPES).toStrictEqual(['region']);
});

it('should filter and only keep allowed types of annotations', () => {
test('should filter and only keep allowed types of annotations', () => {
const viewerConfig = { enabledTypes: ['region', 'timestamp'] };
expect(loader.determineAnnotator(options, viewerConfig).TYPES).toStrictEqual(['region']);

Expand All @@ -112,30 +124,30 @@ describe('BoxAnnotations', () => {
expect(loader.determineAnnotator(options).TYPES).toStrictEqual(['region']);
});

it('should respect default annotators if none provided', () => {
test('should respect default annotators if none provided', () => {
expect(loader.determineAnnotator(options).TYPES).toStrictEqual(['region']);
});

it('should not return an annotator if the user has incorrect permissions/scopes', () => {
test('should not return an annotator if the user has incorrect permissions/scopes', () => {
loader.canLoad = jest.fn().mockReturnValue(false);
loader.getAnnotatorsForViewer = jest.fn().mockReturnValue(annotator);
expect(loader.determineAnnotator(options)).toBeNull();
});

it('should choose the first annotator that matches the viewer', () => {
test('should choose the first annotator that matches the viewer', () => {
loader.getAnnotatorsForViewer = jest.fn().mockReturnValue(annotator);

const result = loader.determineAnnotator(options);
expect(result.NAME).toEqual('Document');
expect(loader.getAnnotatorsForViewer).toBeCalled();
});

it('should not return an annotator if no matching annotator is found', () => {
test('should not return an annotator if no matching annotator is found', () => {
loader.getAnnotatorsForViewer = jest.fn().mockReturnValue(null);
expect(loader.determineAnnotator(options)).toBeNull();
});

it('should return a copy of the annotator that matches', () => {
test('should return a copy of the annotator that matches', () => {
const docAnnotator = {
NAME: 'Document',
VIEWER: ['Document'],
Expand All @@ -149,7 +161,7 @@ describe('BoxAnnotations', () => {
expect(result.NAME).not.toEqual(annotator.NAME);
});

it('should return null if the config for the viewer disables annotations', () => {
test('should return null if the config for the viewer disables annotations', () => {
const config = {
enabled: false,
};
Expand Down
2 changes: 2 additions & 0 deletions src/document/__tests__/DocumentAnnotator-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ describe('DocumentAnnotator', () => {
file_version: { id: '98765' },
permissions: {
can_annotate: true,
can_create_annotations: true,
can_view_annotations: true,
can_view_annotations_all: true,
can_view_annotations_self: true,
},
Expand Down