diff --git a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/__snapshots__/visual-editor.test.js.snap b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/__snapshots__/visual-editor.test.js.snap index 8d774d8e8b..4bca214452 100644 --- a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/__snapshots__/visual-editor.test.js.snap +++ b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/__snapshots__/visual-editor.test.js.snap @@ -534,6 +534,7 @@ exports[`VisualEditor VisualEditor component 1`] = `
" +CTRL+S
" `; diff --git a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/toolbars/file-menu.test.js b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/toolbars/file-menu.test.js index 5acec0c863..d3ba4ddb26 100644 --- a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/toolbars/file-menu.test.js +++ b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/toolbars/file-menu.test.js @@ -16,6 +16,8 @@ jest.mock('../../../../src/scripts/oboeditor/stores/editor-store', () => ({ import { downloadDocument } from '../../../../src/scripts/common/util/download-document' jest.mock('../../../../src/scripts/common/util/download-document') +import { FULL, PARTIAL } from 'obojobo-express/server/constants' + const CONTENT_NODE = 'ObojoboDraft.Sections.Content' const ASSESSMENT_NODE = 'ObojoboDraft.Sections.Assessment' @@ -166,7 +168,7 @@ describe('File Menu', () => { title: 'mockTitle' } - const component = mount() + const component = mount() component .findWhere(n => n.type() === 'button' && n.html().includes('Delete Module...')) @@ -175,6 +177,20 @@ describe('File Menu', () => { expect(ModalUtil.show).toHaveBeenCalled() }) + test('FileMenu does not call Delete if accessLevel is not "Full"', () => { + const model = { + title: 'mockTitle' + } + + const component = mount() + + component + .findWhere(n => n.type() === 'button' && n.html().includes('Delete Module...')) + .simulate('click') + + expect(ModalUtil.show).not.toHaveBeenCalled() + }) + test('FileMenu calls Copy LTI Link', () => { const model = { title: 'mockTitle' diff --git a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/visual-editor.test.js b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/visual-editor.test.js index 05429bce40..972e159715 100644 --- a/packages/app/obojobo-document-engine/__tests__/oboeditor/components/visual-editor.test.js +++ b/packages/app/obojobo-document-engine/__tests__/oboeditor/components/visual-editor.test.js @@ -9,6 +9,7 @@ import EditorUtil from 'src/scripts/oboeditor/util/editor-util' import ModalStore from '../../../src/scripts/common/stores/modal-store' import Dispatcher from '../../../src/scripts/common/flux/dispatcher' import OboModel from 'src/scripts/common/models/obo-model' +import { FULL } from 'obojobo-express/server/constants' import { Editor, Transforms } from 'slate' import { ReactEditor } from 'slate-react' @@ -43,6 +44,36 @@ const BREAK_NODE = 'ObojoboDraft.Chunks.Break' let restoreConsole describe('VisualEditor', () => { + let props + + const defaultProps = { + page: { + id: 122, + set: jest.fn(), + attributes: { + children: [ + { + type: BREAK_NODE, + content: {}, + children: [] + } + ] + }, + get: jest.fn().mockReturnValue(ASSESSMENT_NODE), + toJSON: () => ({ + children: [ + { + type: BREAK_NODE, + content: {}, + children: [] + } + ] + }) + }, + model: { title: 'Mock Title' }, + draft: { accessLevel: FULL } + } + beforeEach(() => { jest.clearAllMocks() jest.restoreAllMocks() @@ -60,37 +91,30 @@ describe('VisualEditor', () => { ]) ) Editor.marks = jest.fn().mockReturnValue({}) - }) - afterEach(() => { - restoreConsole() - }) - - test('VisualEditor component', () => { - const props = { + props = { insertableItems: 'mock-insertable-items', page: { attributes: { children: [{ type: 'mockNode' }] }, get: jest.fn(), toJSON: () => ({ children: [{ type: 'mockNode' }] }) }, - model: { title: 'Mock Title' } + model: { title: 'Mock Title' }, + draft: { accessLevel: FULL } } + }) + + afterEach(() => { + restoreConsole() + }) + + test('VisualEditor component', () => { const component = renderer.create() expect(component.toJSON()).toMatchSnapshot() }) test('VisualEditor component handles triple click', () => { const spy = jest.spyOn(Transforms, 'move').mockReturnValueOnce(true) - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = mount() // Double click @@ -120,15 +144,6 @@ describe('VisualEditor', () => { test('VisualEditor component - editor is disable when modal is opened', () => { ModalStore.init() - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = renderer.create() Dispatcher.trigger('modal:show', { value: 'mockValue' }) @@ -138,15 +153,6 @@ describe('VisualEditor', () => { test('VisualEditor component - editor is disable when modal is closed', () => { ModalStore.init() - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = renderer.create() Dispatcher.trigger('modal:hide', { value: 'mockValue' }) @@ -154,15 +160,6 @@ describe('VisualEditor', () => { }) test('VisualEditor component with decoration', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } jest.spyOn(Common.Registry, 'getItemForType').mockReturnValue({ plugins: { decorate: () => [ @@ -187,15 +184,6 @@ describe('VisualEditor', () => { }) test('VisualEditor component with Elements', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } jest.spyOn(Common.Registry, 'getItemForType').mockReturnValue({ plugins: { renderNode: () =>

Mock Content

//eslint-disable-line react/display-name @@ -226,22 +214,15 @@ describe('VisualEditor', () => { }) test('VisualEditor component with no page', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: null, - model: { title: 'Mock Title' } - } + props.page = null const component = renderer.create() expect(component.toJSON()).toMatchSnapshot() }) test('updating a component with no page doesnt update state', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: null, - model: { title: 'Mock Title' } - } + props.page = null + const thing = renderer.create() const instance = thing.getInstance() @@ -253,15 +234,8 @@ describe('VisualEditor', () => { }) test('updating a component from no page to a page updates state', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }], type: ASSESSMENT_NODE }) - }, - model: { title: 'Mock Title' } - } + props.page.toJSON = () => ({ children: [{ type: 'mockNode' }], type: ASSESSMENT_NODE }) + jest.spyOn(Common.Registry, 'getItemForType').mockReturnValue({ oboToSlate: () => ({ text: '' }) }) @@ -278,11 +252,8 @@ describe('VisualEditor', () => { }) test('updating a component from a page to no page updates state', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: null, - model: { title: 'Mock Title' } - } + props.page = null + const thing = renderer.create() const instance = thing.getInstance() @@ -296,18 +267,14 @@ describe('VisualEditor', () => { }) test('VisualEditor component changes pages', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - id: 1, - set: jest.fn(), - attributes: { - children: [{ type: 'mockNode' }] - }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) + props.page = { + id: 1, + set: jest.fn(), + attributes: { + children: [{ type: 'mockNode' }] }, - model: { title: 'Mock Title' } + get: jest.fn(), + toJSON: () => ({ children: [{ type: 'mockNode' }] }) } let thing @@ -326,58 +293,15 @@ describe('VisualEditor', () => { }) test('VisualEditor component with deleted page to another page', () => { - const prevProps = { - page: { - id: 122, - set: jest.fn(), - attributes: { - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }, - get: jest.fn().mockReturnValue(ASSESSMENT_NODE), - toJSON: () => ({ - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }) - }, - model: { title: 'Mock Title' } - } + const prevProps = defaultProps const newProps = { + ...defaultProps, page: { + ...defaultProps.page, id: 123, - set: jest.fn(), - attributes: { - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }, - get: jest.fn(), - toJSON: () => ({ - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }) - }, - model: { title: 'Mock Title' } + get: jest.fn() + } } const spy = jest.spyOn(VisualEditor.prototype, 'exportToJSON').mockReturnValueOnce() @@ -394,58 +318,15 @@ describe('VisualEditor', () => { test('VisualEditor component with page updating to another page', () => { OboModel.models = { 122: 'mock content' } - const prevProps = { - page: { - id: 122, - set: jest.fn(), - attributes: { - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }, - get: jest.fn().mockReturnValue(ASSESSMENT_NODE), - toJSON: () => ({ - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }) - }, - model: { title: 'Mock Title' } - } + const prevProps = defaultProps const newProps = { + ...defaultProps, page: { + ...defaultProps.page, id: 123, - set: jest.fn(), - attributes: { - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }, - get: jest.fn(), - toJSON: () => ({ - children: [ - { - type: BREAK_NODE, - content: {}, - children: [] - } - ] - }) - }, - model: { title: 'Mock Title' } + get: jest.fn() + } } const spy = jest.spyOn(VisualEditor.prototype, 'exportToJSON') @@ -468,14 +349,7 @@ describe('VisualEditor', () => { }) test('VisualEditor component refocuses on editor', () => { - const props = { - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } + props.insertableItems = null jest.spyOn(ReactEditor, 'focus').mockReturnValue(true) const component = mount() @@ -488,15 +362,6 @@ describe('VisualEditor', () => { }) test('VisualEditor component alters value majorly', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = mount() const value = [ @@ -510,15 +375,6 @@ describe('VisualEditor', () => { }) test('VisualEditor component alters value majorly with multi-select', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } jest.spyOn(ReactEditor, 'isFocused').mockReturnValue(true) window.getSelection = jest.fn().mockReturnValue({ getRangeAt: () => ({ @@ -543,15 +399,6 @@ describe('VisualEditor', () => { }) test('VisualEditor component alters value majorly with latex', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } jest.spyOn(ReactEditor, 'isFocused').mockReturnValue(true) window.getSelection = jest.fn().mockReturnValue({ getRangeAt: () => ({ @@ -576,15 +423,6 @@ describe('VisualEditor', () => { }) test('toggleEditable changes the state', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = mount() const inst = component.instance() @@ -606,15 +444,6 @@ describe('VisualEditor', () => { }) test('markUnsaved changes the state', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = mount() const inst = component.instance() @@ -677,7 +506,8 @@ describe('VisualEditor', () => { flatJSON: () => { return { content: {}, children: [] } } - } + }, + draft: { accessLevel: FULL } } const saveModule = jest.spyOn(VisualEditor.prototype, 'saveModule') @@ -739,7 +569,8 @@ describe('VisualEditor', () => { flatJSON: () => { return { content: {}, children: [] } } - } + }, + draft: { accessLevel: FULL } } const saveModule = jest.spyOn(VisualEditor.prototype, 'saveModule') @@ -824,7 +655,8 @@ describe('VisualEditor', () => { flatJSON: () => { return { content: {}, children: [] } } - } + }, + draft: { accessLevel: FULL } } const component = mount() const plugins = component.instance().plugins @@ -862,16 +694,6 @@ describe('VisualEditor', () => { }) } - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mock node' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mock node' }] }) - }, - model: { title: 'Mock Title' } - } - const value = { document: { nodes: { @@ -921,16 +743,6 @@ describe('VisualEditor', () => { toJSON: () => ({ children: [{ type: 'mock node' }] }) } - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mock node' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mock node' }] }) - }, - model: { title: 'Mock Title' } - } - const value = ['node-one', 'node-two'] const thing = mount() @@ -963,15 +775,6 @@ describe('VisualEditor', () => { }) test('exportToJSON returns undefined for null page', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mock node' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mock node' }] }) - }, - model: { title: 'Mock Title' } - } const thing = mount() expect(thing.instance()).toHaveProperty('exportToJSON', expect.any(Function)) @@ -996,7 +799,8 @@ describe('VisualEditor', () => { get: jest.fn(), toJSON: () => ({ children: [{ type: 'mock node' }] }) }, - model: { title: 'Mock Title' } + model: { title: 'Mock Title' }, + draft: { accessLevel: FULL } } const component = mount() @@ -1027,7 +831,8 @@ describe('VisualEditor', () => { title: 'Mock Title', flatJSON: () => ({ content: {} }), children: [] - } + }, + draft: { accessLevel: FULL } } const component = mount() @@ -1058,7 +863,8 @@ describe('VisualEditor', () => { title: 'Mock Title', flatJSON: () => ({ content: {} }), children: [] - } + }, + draft: { accessLevel: FULL } } const component = mount() @@ -1109,7 +915,8 @@ describe('VisualEditor', () => { title: 'Mock Title', flatJSON: () => ({ content: {} }), children: [] - } + }, + draft: { accessLevel: FULL } } const component = mount() @@ -1231,7 +1038,8 @@ describe('VisualEditor', () => { title: 'Mock Title', flatJSON: () => ({ content: {} }), children: [] - } + }, + draft: { accessLevel: FULL } } const component = mount() @@ -1293,7 +1101,8 @@ describe('VisualEditor', () => { title: 'Mock Title', flatJSON: () => ({ content: {} }), children: [] - } + }, + draft: { accessLevel: FULL } } const component = mount() @@ -1336,11 +1145,7 @@ describe('VisualEditor', () => { Object.defineProperty(window, 'location', { value: { reload: jest.fn() } }) - const props = { - insertableItems: 'mock-insertable-items', - page: { toJSON: () => ({ children: [{ type: 'mock node' }] }) }, - model: { title: 'Mock Title' } - } + const component = renderer.create() component.getInstance().reload() @@ -1350,15 +1155,6 @@ describe('VisualEditor', () => { }) test('onResized sets state.contentRect', () => { - const props = { - insertableItems: 'mock-insertable-items', - page: { - attributes: { children: [{ type: 'mockNode' }] }, - get: jest.fn(), - toJSON: () => ({ children: [{ type: 'mockNode' }] }) - }, - model: { title: 'Mock Title' } - } const component = mount() expect(component.state().contentRect).toBe(null) diff --git a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-menu.js b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-menu.js index 2bb7567c27..ceda023c9a 100644 --- a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-menu.js +++ b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-menu.js @@ -4,7 +4,7 @@ import Common from 'obojobo-document-engine/src/scripts/common' import ClipboardUtil from '../../util/clipboard-util' import EditorAPI from 'obojobo-document-engine/src/scripts/viewer/util/editor-api' import { downloadDocument } from '../../../common/util/download-document' - +import { FULL } from 'obojobo-express/server/constants' import DropDownMenu from './drop-down-menu' const { Prompt } = Common.components.modal @@ -143,6 +143,7 @@ class FileMenu extends React.PureComponent { { name: 'Delete Module...', type: 'action', + disabled: this.props.accessLevel !== FULL, action: () => { const buttons = [ { diff --git a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-toolbar.js b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-toolbar.js index d7d573d16b..15b3bd3c86 100644 --- a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-toolbar.js +++ b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/toolbars/file-toolbar.js @@ -78,6 +78,7 @@ const FileToolbar = props => { onSave={props.onSave} reload={props.reload} mode={props.mode} + accessLevel={props.accessLevel} />
diff --git a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js index 3a059e3bb0..c2c9f6ce9d 100644 --- a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js +++ b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js @@ -560,6 +560,7 @@ class VisualEditor extends React.Component { { beforeEach(() => { mockArgs = (() => { const res = {} - const req = { session: {} } + const req = { session: {}, currentUser: {} } const mockJson = jest.fn().mockImplementation(() => { return true }) diff --git a/packages/app/obojobo-express/__tests__/models/__snapshots__/user.test.js.snap b/packages/app/obojobo-express/__tests__/models/__snapshots__/user.test.js.snap index 920a128a7a..88f36509cd 100644 --- a/packages/app/obojobo-express/__tests__/models/__snapshots__/user.test.js.snap +++ b/packages/app/obojobo-express/__tests__/models/__snapshots__/user.test.js.snap @@ -14,6 +14,7 @@ Array [ RETURNING id ", Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": 3, @@ -32,6 +33,7 @@ Array [ exports[`user model fetchById fetches one from the database 1`] = ` Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/7cd5405dd878f2f032418e1bd8dfba0c?s=120&d=retro", "firstName": "Guest", "id": 5, @@ -71,6 +73,7 @@ Array [ RETURNING id ", Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": 10, diff --git a/packages/app/obojobo-express/__tests__/models/draft.test.js b/packages/app/obojobo-express/__tests__/models/draft.test.js index 274f0aa403..4d73019b84 100644 --- a/packages/app/obojobo-express/__tests__/models/draft.test.js +++ b/packages/app/obojobo-express/__tests__/models/draft.test.js @@ -299,27 +299,26 @@ describe('Draft Model', () => { }) }) - test('deleteByIdAndUser sets delete flag', () => { + test('deleteById sets delete flag', () => { expect.hasAssertions() const oboEvents = require('../../server/obo_events') db.none.mockResolvedValueOnce() - return DraftModel.deleteByIdAndUser('draft_id', 'user_id').then(voidResult => { + return DraftModel.deleteById('draft_id').then(voidResult => { expect(voidResult).toBe(undefined) //eslint-disable-line no-undefined expect(oboEvents.emit).toHaveBeenCalledWith('EVENT_DRAFT_DELETED', { - id: 'draft_id', - userId: 'user_id' + id: 'draft_id' }) }) }) - test('deleteByIdAndUser fails as expected', () => { + test('deleteById fails as expected', () => { expect.hasAssertions() db.none.mockRejectedValueOnce('mock-error') - return expect(DraftModel.deleteByIdAndUser('draft_id', 'user_id')).rejects.toBe('mock-error') + return expect(DraftModel.deleteById('draft_id')).rejects.toBe('mock-error') }) test('restoreByIdAndUser reverts delete flag', () => { diff --git a/packages/app/obojobo-express/__tests__/models/user.test.js b/packages/app/obojobo-express/__tests__/models/user.test.js index 8701876c98..d138fff5bf 100644 --- a/packages/app/obojobo-express/__tests__/models/user.test.js +++ b/packages/app/obojobo-express/__tests__/models/user.test.js @@ -255,6 +255,7 @@ describe('user model', () => { expect(users).toMatchInlineSnapshot(` Array [ Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": null, @@ -264,6 +265,7 @@ describe('user model', () => { "username": "someusername", }, Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": null, @@ -291,6 +293,7 @@ describe('user model', () => { expect(users).toMatchInlineSnapshot(` Array [ Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": null, @@ -300,6 +303,7 @@ describe('user model', () => { "username": "someusername", }, Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/19c0a7d712a610200eb3b9686aa0de4a?s=120&d=retro", "firstName": "Roger", "id": null, diff --git a/packages/app/obojobo-express/__tests__/routes/api/drafts.test.js b/packages/app/obojobo-express/__tests__/routes/api/drafts.test.js index 434a050944..d782ce042d 100644 --- a/packages/app/obojobo-express/__tests__/routes/api/drafts.test.js +++ b/packages/app/obojobo-express/__tests__/routes/api/drafts.test.js @@ -8,6 +8,7 @@ jest.mock('obojobo-document-json-parser/json-to-xml-parser') jest.mock('obojobo-repository/server/models/draft_permissions') import DraftModel from '../../../server/models/draft' +import { FULL, PARTIAL, MINIMAL } from '../../../server/constants' const CollectionModel = require('obojobo-repository/server/models/collection') const xml = require('obojobo-document-xml-parser/xml-to-draft-object') const jsonToXml = require('obojobo-document-json-parser/json-to-xml-parser') @@ -67,8 +68,7 @@ describe('api draft route', () => { db.any.mockReset() xml.mockReset() jsonToXml.mockReset() - DraftPermissions.userHasPermissionToDraft.mockReset() - DraftPermissions.userHasPermissionToDraft.mockResolvedValue(true) + DraftPermissions.getUserAccessLevelToDraft.mockReset() CollectionModel.addModule.mockReset() }) afterEach(() => {}) @@ -81,8 +81,9 @@ describe('api draft route', () => { // mock the document returned by fetchById const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: { title: 'mock-document-json' }, + authorId: 99, + accessLevel: FULL } // mock the xmlDocument Getter @@ -91,6 +92,7 @@ describe('api draft route', () => { }) DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) return request(app) .get('/api/drafts/00000000-0000-0000-0000-000000000000/full') @@ -110,14 +112,18 @@ describe('api draft route', () => { // mock a yell function that returns a document const mockYell = jest.fn() + const mockDocument = { title: 'mock-document-json' } + // mock the document returned by fetchById const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) jsonToXml.mockReturnValueOnce('mock-xml') @@ -133,7 +139,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/xml') expect(response.statusCode).toBe(200) expect(response.text).toContain('mock-xml') - expect(jsonToXml).toHaveBeenCalledWith('mock-document-json') + expect(jsonToXml).toHaveBeenCalledWith(mockDocument) }) }) @@ -144,13 +150,18 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + + const mockDocument = { title: 'mock-document-json' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -164,7 +175,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/json') expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('status', 'ok') - expect(response.body).toHaveProperty('value', 'mock-document-json') + expect(response.body).toHaveProperty('value', mockDocument) expect(jsonToXml).not.toHaveBeenCalled() }) }) @@ -176,13 +187,17 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + const mockDocument = { title: 'mock-document' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchDraftByVersion.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -198,7 +213,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/json') expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('status', 'ok') - expect(response.body).toHaveProperty('value', 'mock-document-json') + expect(response.body).toHaveProperty('value', mockDocument) expect(DraftModel.fetchDraftByVersion).toHaveBeenCalledWith( '00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-000000000001' @@ -214,13 +229,17 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + const mockDocument = { title: 'mock-document' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchDraftByVersion.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -250,13 +269,17 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + const mockDocument = { title: 'mock-document' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -270,7 +293,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/json') expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('status', 'ok') - expect(response.body).toHaveProperty('value', 'mock-document-json') + expect(response.body).toHaveProperty('value', mockDocument) expect(jsonToXml).not.toHaveBeenCalled() }) }) @@ -282,13 +305,17 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + const mockDocument = { title: 'mock-document' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -302,7 +329,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/json') expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('status', 'ok') - expect(response.body).toHaveProperty('value', 'mock-document-json') + expect(response.body).toHaveProperty('value', mockDocument) }) }) @@ -313,13 +340,17 @@ describe('api draft route', () => { const mockYell = jest.fn() // mock the document returned by fetchById + const mockDocument = { title: 'mock-document' } + const mockDraftModel = { root: { yell: mockYell }, - document: 'mock-document-json', - authorId: 99 + document: mockDocument, + authorId: 99, + accessLevel: FULL } DraftModel.fetchById.mockResolvedValueOnce(mockDraftModel) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) // mock the xmlDocument Getter Object.defineProperty(mockDraftModel, 'xmlDocument', { @@ -333,7 +364,7 @@ describe('api draft route', () => { expect(response.header['content-type']).toContain('application/json') expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('status', 'ok') - expect(response.body).toHaveProperty('value', 'mock-document-json') + expect(response.body).toHaveProperty('value', mockDocument) }) }) @@ -358,9 +389,9 @@ describe('api draft route', () => { }) }) - test('get full draft returns 401 if user is not the author', () => { + test('get full draft returns 401 if user does not have access level with editing permission', () => { expect.assertions(5) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(MINIMAL) mockCurrentUser = { id: 88, hasPermission: perm => perm === 'canViewEditor' } // mock current logged in user // mock a yell function that returns a document const mockYell = jest.fn() @@ -379,14 +410,14 @@ describe('api draft route', () => { expect(response.body.value).toHaveProperty('type', 'notAuthorized') expect(response.body.value).toHaveProperty( 'message', - 'You must be the author of this draft to retrieve this information' + 'Your access level must be "Partial" or higher to retrieve this information.' ) }) }) - test('get full draft returns 401 if user does not have canViewEditor rights AND is not the author', () => { + test('get full draft returns 401 if user does not have canViewEditor rights NOR access level with editing permission', () => { expect.assertions(4) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(MINIMAL) mockCurrentUser = { id: 88, hasPermission: () => false } // mock current logged in user return request(app) @@ -402,6 +433,7 @@ describe('api draft route', () => { test('get full draft returns 404 when no items found', () => { expect.assertions(5) mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canViewEditor' } // mock current logged in user + // mock a failure to find the draft const mockError = new pgp.errors.QueryResultError( pgp.errors.queryResultErrorCode.noData, @@ -410,6 +442,8 @@ describe('api draft route', () => { 'mockValues' ) DraftModel.fetchById.mockRejectedValueOnce(mockError) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) + return request(app) .get('/api/drafts/00000000-0000-0000-0000-000000000000/full') .then(response => { @@ -425,6 +459,7 @@ describe('api draft route', () => { expect.assertions(5) // mock a failure to find the draft DraftModel.fetchById.mockRejectedValueOnce('mock-other-error') + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) return request(app) .get('/api/drafts/00000000-0000-0000-0000-000000000000/full') .then(response => { @@ -497,7 +532,6 @@ describe('api draft route', () => { expect(response.body.value).toHaveProperty('message', 'mock-other-error') }) }) - // new draft test('new draft returns success', () => { @@ -955,7 +989,8 @@ describe('api draft route', () => { test('delete draft returns successfully', () => { expect.assertions(4) - DraftModel.deleteByIdAndUser.mockResolvedValueOnce('mock-db-result') + DraftModel.deleteById.mockResolvedValueOnce('mock-db-result') + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canDeleteDrafts' } // mock current logged in user return request(app) .delete('/api/drafts/00000000-0000-0000-0000-000000000000') @@ -969,7 +1004,10 @@ describe('api draft route', () => { test('delete 500s when the database errors', () => { expect.assertions(5) - DraftModel.deleteByIdAndUser.mockRejectedValueOnce('oh no') + + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) + DraftModel.deleteById.mockRejectedValueOnce('oh no') + mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canDeleteDrafts' } // mock current logged in user return request(app) .delete('/api/drafts/00000000-0000-0000-0000-000000000000') @@ -982,10 +1020,10 @@ describe('api draft route', () => { }) }) - test('delete 401s when a user tries deleting a draft they do not own', () => { + test('delete 401s when a user tries deleting a draft they do not have Full access to', () => { expect.assertions(5) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(PARTIAL) mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canDeleteDrafts' } // mock current logged in user return request(app) @@ -997,7 +1035,7 @@ describe('api draft route', () => { expect(response.body.value).toHaveProperty('type', 'notAuthorized') expect(response.body.value).toHaveProperty( 'message', - 'You must be the author of this draft to delete it' + 'You must have "Full" access to this draft to delete it' ) }) }) diff --git a/packages/app/obojobo-express/__tests__/routes/api/locks.test.js b/packages/app/obojobo-express/__tests__/routes/api/locks.test.js index f80be7e3f6..2b03fc3066 100644 --- a/packages/app/obojobo-express/__tests__/routes/api/locks.test.js +++ b/packages/app/obojobo-express/__tests__/routes/api/locks.test.js @@ -20,6 +20,8 @@ jest.mock('../../../server/express_validators', () => ({ requireCanViewEditor: mockValidatorThatPasses })) +import { FULL, MINIMAL } from '../../../server/constants' + describe('Route api/locks', () => { let request let bodyParser @@ -73,7 +75,7 @@ describe('Route api/locks', () => { test('post lock calls expected Validators', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) EditLock.fetchByDraftId.mockReturnValueOnce({ userId: mockCurrentUser.id }) EditLock.create.mockReturnValueOnce({ userId: mockCurrentUser.id }) expect(mockValidatorThatPasses).toHaveBeenCalledTimes(0) @@ -88,7 +90,7 @@ describe('Route api/locks', () => { test('post lock returns expected results', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) EditLock.fetchByDraftId.mockReturnValueOnce({ userId: mockCurrentUser.id }) EditLock.create.mockReturnValueOnce({ userId: mockCurrentUser.id }) expect(mockValidatorThatPasses).toHaveBeenCalledTimes(0) @@ -102,9 +104,9 @@ describe('Route api/locks', () => { }) }) - test('post lock returns not authorized when user doesnt have permission to draft', () => { + test('post lock returns not authorized when user doesnt have "Full" or "Partial" access level to draft', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(MINIMAL) return request(app) .post('/api/locks/mock-draft-id') @@ -115,14 +117,14 @@ describe('Route api/locks', () => { expect(response.body).toHaveProperty('value') expect(response.body.value).toHaveProperty( 'message', - 'You do not have the required access to edit this module.' + 'You do not have the required access level to edit this module.' ) }) }) test('post lock returns not authorized when a different user has a lock', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) EditLock.fetchByDraftId.mockReturnValueOnce({ userId: 'someone-else' }) return request(app) @@ -140,7 +142,7 @@ describe('Route api/locks', () => { test('post lock returns not authorized when creating a lock doesnt work', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) EditLock.fetchByDraftId.mockReturnValueOnce({ userId: mockCurrentUser.id }) EditLock.create.mockReturnValueOnce({ userId: 'some-other-user' }) @@ -159,7 +161,7 @@ describe('Route api/locks', () => { test('post lock returns a 403 when the contentId does not match', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockImplementationOnce(() => { + DraftPermissions.getUserAccessLevelToDraft.mockImplementationOnce(() => { throw new Error('Current version of draft does not match requested lock.') }) @@ -179,7 +181,7 @@ describe('Route api/locks', () => { test('post lock returns a 500 when an unexpected error occurs', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockImplementationOnce(() => { + DraftPermissions.getUserAccessLevelToDraft.mockImplementationOnce(() => { throw new Error('mock-error') }) @@ -199,7 +201,7 @@ describe('Route api/locks', () => { test('post lock calls EditLock.deleteExpiredLocks', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) EditLock.fetchByDraftId.mockReturnValueOnce({ userId: mockCurrentUser.id }) EditLock.create.mockReturnValueOnce({ userId: mockCurrentUser.id }) diff --git a/packages/app/obojobo-express/server/constants.js b/packages/app/obojobo-express/server/constants.js new file mode 100644 index 0000000000..4a90d708a7 --- /dev/null +++ b/packages/app/obojobo-express/server/constants.js @@ -0,0 +1,23 @@ +const FULL = 1 +const PARTIAL = 2 +const MINIMAL = 3 + +const levelName = { + [FULL]: 'Full', + [PARTIAL]: 'Partial', + [MINIMAL]: 'Minimal' +} + +const levelNumber = { + Full: 1, + Partial: 2, + Minimal: 3 +} + +module.exports = { + FULL, + PARTIAL, + MINIMAL, + levelName, + levelNumber +} diff --git a/packages/app/obojobo-express/server/migrations/20220321130020-add-access-levels-to-draft.js b/packages/app/obojobo-express/server/migrations/20220321130020-add-access-levels-to-draft.js new file mode 100644 index 0000000000..b332ebd570 --- /dev/null +++ b/packages/app/obojobo-express/server/migrations/20220321130020-add-access-levels-to-draft.js @@ -0,0 +1,39 @@ +'use strict' + +var dbm +var type +var seed + +/** + * We receive the dbmigrate dependency from dbmigrate initially. + * This enables us to not have to rely on NODE_PATH. + */ +exports.setup = function(options, seedLink) { + dbm = options.dbmigrate + type = dbm.dataType + seed = seedLink +} + +exports.up = function(db) { + return db + .addColumn('repository_map_user_to_draft', 'access_level', { + type: 'int', + notNull: true, + defaultValue: 1 + }) + .then(() => { + return db.addIndex('repository_map_user_to_draft', 'user_access_level_index', [ + 'access_level' + ]) + }) +} + +exports.down = function(db) { + return db + .removeIndex('repository_map_user_to_draft', 'user_access_level_index') + .then(() => db.removeColumn('repository_map_user_to_draft', 'access_level')) +} + +exports._meta = { + version: 1 +} diff --git a/packages/app/obojobo-express/server/models/__mocks__/draft.js b/packages/app/obojobo-express/server/models/__mocks__/draft.js index 4ddda0bbf2..44ab995453 100644 --- a/packages/app/obojobo-express/server/models/__mocks__/draft.js +++ b/packages/app/obojobo-express/server/models/__mocks__/draft.js @@ -22,7 +22,7 @@ MockDraft.updateContent = jest.fn().mockResolvedValue('mockUpdatedContentId') MockDraft.findDuplicateIds = jest.fn().mockReturnValue(null) MockDraft.fetchDraftByVersion = jest.fn().mockImplementation(() => Promise.resolve(new MockDraft())) MockDraft.restoreByIdAndUser = jest.fn().mockImplementation(() => Promise.resolve(new MockDraft())) -MockDraft.deleteByIdAndUser = jest.fn().mockResolvedValue(null) +MockDraft.deleteById = jest.fn().mockResolvedValue(null) MockDraft.mockGetChildNodeById = mockGetChildNodeById MockDraft.__setMockYell = newMock => { diff --git a/packages/app/obojobo-express/server/models/draft.js b/packages/app/obojobo-express/server/models/draft.js index 836b192f9a..d88402e9f5 100644 --- a/packages/app/obojobo-express/server/models/draft.js +++ b/packages/app/obojobo-express/server/models/draft.js @@ -80,25 +80,23 @@ class Draft { return draftNode } - static deleteByIdAndUser(id, userId) { + static deleteById(id) { return db .none( ` UPDATE drafts SET deleted = TRUE WHERE id = $[id] - AND user_id = $[userId] `, { - id, - userId + id } ) .then(() => { - oboEvents.emit(Draft.EVENT_DRAFT_DELETED, { id, userId }) + oboEvents.emit(Draft.EVENT_DRAFT_DELETED, { id }) }) .catch(error => { - logger.logError('Draft deleteByIdAndUser Error', error) + logger.logError('Draft deleteById Error', error) throw error }) } @@ -110,7 +108,6 @@ class Draft { UPDATE drafts SET deleted = FALSE WHERE id = $[id] - AND user_id = $[userId] `, { id, diff --git a/packages/app/obojobo-express/server/models/user.js b/packages/app/obojobo-express/server/models/user.js index b4d37e5fca..e2c420667a 100644 --- a/packages/app/obojobo-express/server/models/user.js +++ b/packages/app/obojobo-express/server/models/user.js @@ -173,7 +173,16 @@ class User { } toJSON() { - const allowedKeys = ['id', 'firstName', 'lastName', 'username', 'roles', 'perms', 'avatarUrl'] + const allowedKeys = [ + 'id', + 'firstName', + 'lastName', + 'username', + 'roles', + 'perms', + 'avatarUrl', + 'accessLevel' + ] const safeUser = {} copyAttributesFn(safeUser, this, allowedKeys) return safeUser diff --git a/packages/app/obojobo-express/server/routes/api/drafts.js b/packages/app/obojobo-express/server/routes/api/drafts.js index 01f72723a3..2868f52bea 100644 --- a/packages/app/obojobo-express/server/routes/api/drafts.js +++ b/packages/app/obojobo-express/server/routes/api/drafts.js @@ -25,6 +25,7 @@ const isNoDataFromQueryError = e => { e instanceof pgp.errors.QueryResultError && e.code === pgp.errors.queryResultErrorCode.noData ) } +const { levelName, FULL, PARTIAL } = require('../../constants') // Get a complete Draft Document Tree (for editing) // optional query variable: contentId= @@ -35,14 +36,16 @@ router .get(async (req, res) => { try { // @TODO: checking permissions should probably be more dynamic, not hard-coded to the repository - const hasPerms = await DraftPermissions.userHasPermissionToDraft( + const access_level = await DraftPermissions.getUserAccessLevelToDraft( req.currentUser.id, req.params.draftId ) + const hasPerms = access_level === FULL || access_level === PARTIAL + if (!hasPerms) { return res.notAuthorized( - 'You must be the author of this draft to retrieve this information' + 'Your access level must be "Partial" or higher to retrieve this information.' ) } @@ -54,7 +57,11 @@ router // get the current version draftModel = await DraftModel.fetchById(req.params.draftId) } + + // Get the draft document and attach the user's access level for use in editor const draftDocument = draftModel.document + draftDocument.accessLevel = access_level + res.format({ 'application/xml': async () => { let xml = await draftModel.xmlDocument @@ -259,16 +266,20 @@ router .route('/:draftId') .delete([requireCanDeleteDrafts, requireDraftId, checkValidationRules]) .delete(async (req, res) => { - const hasPerms = await DraftPermissions.userHasPermissionToDraft( + const access_level = await DraftPermissions.getUserAccessLevelToDraft( req.currentUser.id, req.params.draftId ) + const hasPerms = access_level === FULL + if (!hasPerms) { - return res.notAuthorized('You must be the author of this draft to delete it') + return res.notAuthorized( + 'You must have "' + levelName[FULL] + '" access to this draft to delete it' + ) } - return DraftModel.deleteByIdAndUser(req.params.draftId, req.currentUser.id) + return DraftModel.deleteById(req.params.draftId) .then(res.success) .catch(res.unexpected) }) diff --git a/packages/app/obojobo-express/server/routes/api/locks.js b/packages/app/obojobo-express/server/routes/api/locks.js index 13db0829d0..ab9732e6c0 100644 --- a/packages/app/obojobo-express/server/routes/api/locks.js +++ b/packages/app/obojobo-express/server/routes/api/locks.js @@ -6,6 +6,7 @@ const DraftPermissions = require('obojobo-repository/server/models/draft_permiss const { checkValidationRules, requireDraftId, requireContentId, requireCanViewEditor } = oboRequire( 'server/express_validators' ) +const { FULL, PARTIAL } = require('../../constants') // CHECK A LOCK // mounted as /api/locks/:draftId @@ -24,13 +25,15 @@ router .post([requireDraftId, requireCanViewEditor, requireContentId, checkValidationRules]) .post(async (req, res) => { try { - const hasPerms = await DraftPermissions.userHasPermissionToDraft( + const access_level = await DraftPermissions.getUserAccessLevelToDraft( req.currentUser.id, req.params.draftId ) + const hasPerms = access_level === FULL || access_level === PARTIAL + if (!hasPerms) { - return res.notAuthorized('You do not have the required access to edit this module.') + return res.notAuthorized('You do not have the required access level to edit this module.') } // attempt to lock diff --git a/packages/app/obojobo-repository/server/models/collection.js b/packages/app/obojobo-repository/server/models/collection.js index 5f8e131ce2..de859e30bd 100644 --- a/packages/app/obojobo-repository/server/models/collection.js +++ b/packages/app/obojobo-repository/server/models/collection.js @@ -129,7 +129,7 @@ class Collection { const whereSQL = `repository_collections.id = $[collectionId]` - return DraftSummary.fetchAndJoinWhere(joinOn, whereSQL, { collectionId: this.id }) + return DraftSummary.fetchAndJoinWhere('', joinOn, whereSQL, { collectionId: this.id }) .then(draftSummaries => { this.drafts = draftSummaries return this diff --git a/packages/app/obojobo-repository/server/models/collection.test.js b/packages/app/obojobo-repository/server/models/collection.test.js index d592e38dee..abd21e4a46 100644 --- a/packages/app/obojobo-repository/server/models/collection.test.js +++ b/packages/app/obojobo-repository/server/models/collection.test.js @@ -226,6 +226,8 @@ describe('Collection Model', () => { db.one.mockResolvedValueOnce(mockRawCollection) DraftSummary.fetchAndJoinWhere.mockResolvedValueOnce(mockDrafts) + const selectSQL = '' + const joinSQL = ` JOIN repository_map_drafts_to_collections ON repository_map_drafts_to_collections.draft_id = drafts.id @@ -237,7 +239,7 @@ describe('Collection Model', () => { return CollectionModel.fetchById('mockCollectionId') .then(collection => collection.loadRelatedDrafts()) .then(collection => { - expect(DraftSummary.fetchAndJoinWhere).toHaveBeenCalledWith(joinSQL, whereSQL, { + expect(DraftSummary.fetchAndJoinWhere).toHaveBeenCalledWith(selectSQL, joinSQL, whereSQL, { collectionId: 'mockCollectionId' }) expect(collection.drafts).toEqual(mockDrafts) @@ -254,6 +256,8 @@ describe('Collection Model', () => { db.one.mockResolvedValueOnce(mockRawCollection) DraftSummary.fetchAndJoinWhere.mockRejectedValueOnce(mockError) + const selectSQL = '' + const joinSQL = ` JOIN repository_map_drafts_to_collections ON repository_map_drafts_to_collections.draft_id = drafts.id @@ -265,7 +269,7 @@ describe('Collection Model', () => { return CollectionModel.fetchById('mockCollectionId') .then(collection => collection.loadRelatedDrafts()) .catch(error => { - expect(DraftSummary.fetchAndJoinWhere).toHaveBeenCalledWith(joinSQL, whereSQL, { + expect(DraftSummary.fetchAndJoinWhere).toHaveBeenCalledWith(selectSQL, joinSQL, whereSQL, { collectionId: 'mockCollectionId' }) expect(error).toBe(mockError) diff --git a/packages/app/obojobo-repository/server/models/draft_permissions.js b/packages/app/obojobo-repository/server/models/draft_permissions.js index bf2fda065a..7c631f3994 100644 --- a/packages/app/obojobo-repository/server/models/draft_permissions.js +++ b/packages/app/obojobo-repository/server/models/draft_permissions.js @@ -18,6 +18,20 @@ class DraftPermissions { }) } + static updateAccessLevel(draftId, userId, accessLevel) { + return db + .none( + `UPDATE repository_map_user_to_draft + SET access_level = $[accessLevel] + WHERE draft_id = $[draftId] + AND user_id = $[userId]`, + { accessLevel, userId, draftId } + ) + .catch(error => { + throw logger.logError('Error updateAccessLevel', error) + }) + } + static removeOwnerFromDraft(draftId, userId) { return db .none( @@ -42,7 +56,8 @@ class DraftPermissions { users.email, users.username, users.created_at, - users.roles + users.roles, + access_level FROM repository_map_user_to_draft JOIN users ON users.id = user_id @@ -51,28 +66,31 @@ class DraftPermissions { { draftId } ) .then(results => { - return results.map(r => User.dbResultToModel(r)) + return results.map(r => { + const u = User.dbResultToModel(r) + u.accessLevel = r.access_level + return u + }) }) .catch(error => { throw logger.logError('Error getDraftOwners', error) }) } - // returns a boolean - static async userHasPermissionToDraft(userId, draftId) { + // returns an int (or null) + static async getUserAccessLevelToDraft(userId, draftId) { try { const result = await db.oneOrNone( - `SELECT user_id + `SELECT access_level FROM repository_map_user_to_draft WHERE draft_id = $[draftId] AND user_id = $[userId]`, { userId, draftId } ) - // oneOrNone returns null when there are no results - return result !== null + return result ? result.access_level : null } catch (error) { - throw logger.logError('Error userHasPermissionToDraft', error) + throw logger.logError('Error getUserAccessLevelToDraft', error) } } @@ -81,10 +99,10 @@ class DraftPermissions { try { const results = await Promise.all([ DraftPermissions.draftIsPublic(draftId), - DraftPermissions.userHasPermissionToDraft(userId, draftId) + DraftPermissions.getUserAccessLevelToDraft(userId, draftId) ]) - return results[0] === true || results[1] === true + return results[0] === true || results[1] !== null } catch (error) { throw logger.logError('Error userHasPermissionToCopy', error) } diff --git a/packages/app/obojobo-repository/server/models/draft_permissions.test.js b/packages/app/obojobo-repository/server/models/draft_permissions.test.js index e03c306b2f..d4d6e5e07e 100644 --- a/packages/app/obojobo-repository/server/models/draft_permissions.test.js +++ b/packages/app/obojobo-repository/server/models/draft_permissions.test.js @@ -1,3 +1,5 @@ +import { FULL, PARTIAL } from '../../../obojobo-express/server/constants' + describe('DraftPermissions Model', () => { jest.mock('obojobo-express/server/db') jest.mock('obojobo-express/server/logger') @@ -33,7 +35,8 @@ describe('DraftPermissions Model', () => { expect(DraftPermissions).toHaveProperty('addOwnerToDraft') expect(DraftPermissions).toHaveProperty('removeOwnerFromDraft') expect(DraftPermissions).toHaveProperty('getDraftOwners') - expect(DraftPermissions).toHaveProperty('userHasPermissionToDraft') + expect(DraftPermissions).toHaveProperty('getUserAccessLevelToDraft') + expect(DraftPermissions).toHaveProperty('updateAccessLevel') }) test('addOwnerToDraft retrieves a data from the database', () => { @@ -62,6 +65,33 @@ describe('DraftPermissions Model', () => { }) }) + test('updateAccessLevel retrieves a data from the database', () => { + expect.hasAssertions() + + db.none.mockResolvedValueOnce('mock-db-results') + + return DraftPermissions.updateAccessLevel('MDID', 'MUID', PARTIAL).then(model => { + expect(model).toBe('mock-db-results') + const [query, options] = db.none.mock.calls[0] + expect(query).toContain('UPDATE repository_map_user_to_draft') + expect(options).toEqual({ + userId: 'MUID', + draftId: 'MDID', + accessLevel: PARTIAL + }) + }) + }) + + test('updateAccessLevel throws and logs error', () => { + expect.hasAssertions() + db.none.mockRejectedValueOnce(mockError) + + return DraftPermissions.updateAccessLevel('MDID', 'MUID', PARTIAL).catch(error => { + expect(logger.logError).toHaveBeenCalledWith('Error updateAccessLevel', mockError) + expect(error).toBe(mockError) + }) + }) + test('removeOwnerFromDraft retrieves a data from the database', () => { expect.hasAssertions() @@ -98,6 +128,7 @@ describe('DraftPermissions Model', () => { expect(users).toHaveLength(1) expect(users[0]).toMatchInlineSnapshot(` Object { + "accessLevel": undefined, "avatarUrl": "https://secure.gravatar.com/avatar/340b0dabf1ff06d15fd57dfe757bdbde?s=120&d=retro", "firstName": "Jeffrey", "id": 1, @@ -128,13 +159,13 @@ describe('DraftPermissions Model', () => { }) }) - test('userHasPermissionToDraft retrieves a data from the database', () => { + test('getUserAccessLevelToDraft retrieves a data from the database', () => { expect.hasAssertions() - db.oneOrNone.mockResolvedValueOnce('mock-db-results') + db.oneOrNone.mockResolvedValueOnce({ access_level: FULL }) - return DraftPermissions.userHasPermissionToDraft('MUID', 'MDID').then(hasPermissions => { - expect(hasPermissions).toBe(true) + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').then(access_level => { + expect(access_level).toBe(FULL) const [query, options] = db.oneOrNone.mock.calls[0] expect(query).toContain('SELECT') expect(query).toContain('FROM repository_map_user_to_draft') @@ -145,13 +176,57 @@ describe('DraftPermissions Model', () => { }) }) - test('userHasPermissionToDraft retrieves a data from the database', () => { + test('getUserAccessLevelToDraft retrieves null data from the database', () => { expect.hasAssertions() db.oneOrNone.mockResolvedValueOnce(null) - return DraftPermissions.userHasPermissionToDraft('MUID', 'MDID').then(hasPermissions => { - expect(hasPermissions).toBe(false) + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').then(results => { + expect(results).toBe(null) + const [query, options] = db.oneOrNone.mock.calls[0] + expect(query).toContain('SELECT') + expect(query).toContain('FROM repository_map_user_to_draft') + expect(options).toEqual({ + userId: 'MUID', + draftId: 'MDID' + }) + }) + }) + + test('getUserAccessLevelToDraft throws and logs error', () => { + expect.hasAssertions() + db.oneOrNone.mockRejectedValueOnce(mockError) + + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').catch(error => { + expect(logger.logError).toHaveBeenCalledWith('Error getUserAccessLevelToDraft', mockError) + expect(error).toBe(mockError) + }) + }) + + test('getUserAccessLevel retrieves a data from the database when user exists', () => { + expect.hasAssertions() + + db.oneOrNone.mockResolvedValueOnce({ access_level: FULL }) + + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').then(accessLevel => { + expect(accessLevel).toEqual(FULL) + const [query, options] = db.oneOrNone.mock.calls[0] + expect(query).toContain('SELECT') + expect(query).toContain('FROM repository_map_user_to_draft') + expect(options).toEqual({ + userId: 'MUID', + draftId: 'MDID' + }) + }) + }) + + test('getUserAccessLevel returns null from the database when user does not exist', () => { + expect.hasAssertions() + + db.oneOrNone.mockResolvedValueOnce(null) + + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').then(accessLevel => { + expect(accessLevel).toEqual(null) const [query, options] = db.oneOrNone.mock.calls[0] expect(query).toContain('SELECT') expect(query).toContain('FROM repository_map_user_to_draft') @@ -162,12 +237,12 @@ describe('DraftPermissions Model', () => { }) }) - test('userHasPermissionToDraft throws and logs error', () => { + test('getUserAccessLevel throws and logs error', () => { expect.hasAssertions() db.oneOrNone.mockRejectedValueOnce(mockError) - return DraftPermissions.userHasPermissionToDraft('MUID', 'MDID').catch(error => { - expect(logger.logError).toHaveBeenCalledWith('Error userHasPermissionToDraft', mockError) + return DraftPermissions.getUserAccessLevelToDraft('MUID', 'MDID').catch(error => { + expect(logger.logError).toHaveBeenCalledWith('Error getUserAccessLevelToDraft', mockError) expect(error).toBe(mockError) }) }) @@ -200,18 +275,18 @@ describe('DraftPermissions Model', () => { }) test.each` - draftIsPublic | userHasPermissionToDraft | expected - ${null} | ${'mock-db-result'} | ${true} - ${'mock-db-result'} | ${null} | ${true} - ${null} | ${null} | ${false} - ${'mock-db-result'} | ${'mock-db-result'} | ${true} + draftIsPublic | getUserAccessLevelToDraft | expected + ${null} | ${'mock-db-result'} | ${true} + ${'mock-db-result'} | ${null} | ${true} + ${null} | ${null} | ${false} + ${'mock-db-result'} | ${'mock-db-result'} | ${true} `( - 'userHasPermissionToCopy returns $expected with db results $draftIsPublic and $userHasPermissionToDraft', - ({ draftIsPublic, userHasPermissionToDraft, expected }) => { + 'userHasPermissionToCopy returns $expected with db results $draftIsPublic and $getUserAccessLevelToDraft', + ({ draftIsPublic, getUserAccessLevelToDraft, expected }) => { expect.hasAssertions() db.oneOrNone.mockResolvedValueOnce(draftIsPublic) // draftIsPublic call - db.oneOrNone.mockResolvedValueOnce(userHasPermissionToDraft) // userHasPermissionToDraft call + db.oneOrNone.mockResolvedValueOnce(getUserAccessLevelToDraft) // getUserAccessLevelToDraft call return DraftPermissions.userHasPermissionToCopy('MUID', 'MDID').then(hasPermissions => { expect(hasPermissions).toBe(expected) @@ -238,7 +313,7 @@ describe('DraftPermissions Model', () => { test('userHasPermissionToCopy throws and logs error', () => { expect.hasAssertions() db.oneOrNone.mockResolvedValueOnce('mock-db-results') // draftIsPublic call - db.oneOrNone.mockRejectedValueOnce(mockError) // userHasPermissionToDraft call + db.oneOrNone.mockRejectedValueOnce(mockError) // getUserAccessLevelToDraft call return DraftPermissions.userHasPermissionToCopy('MUID', 'MDID').catch(error => { expect(logger.logError).toHaveBeenCalledWith('Error userHasPermissionToCopy', mockError) @@ -249,7 +324,7 @@ describe('DraftPermissions Model', () => { test('userHasPermissionToCopy throws and logs error', () => { expect.hasAssertions() db.oneOrNone.mockRejectedValueOnce(mockError) // draftIsPublic call - db.oneOrNone.mockResolvedValueOnce('mock-db-results') // userHasPermissionToDraft call + db.oneOrNone.mockResolvedValueOnce('mock-db-results') // getUserAccessLevelToDraft call return DraftPermissions.userHasPermissionToCopy('MUID', 'MDID').catch(error => { expect(logger.logError).toHaveBeenCalledWith('Error userHasPermissionToCopy', mockError) diff --git a/packages/app/obojobo-repository/server/models/draft_summary.js b/packages/app/obojobo-repository/server/models/draft_summary.js index abf49aa0ed..273b12ab25 100644 --- a/packages/app/obojobo-repository/server/models/draft_summary.js +++ b/packages/app/obojobo-repository/server/models/draft_summary.js @@ -1,7 +1,14 @@ const db = require('obojobo-express/server/db') const logger = require('obojobo-express/server/logger') +const { FULL } = require('obojobo-express/server/constants') -const buildQueryWhere = (whereSQL, joinSQL = '', deleted = 'FALSE', limitSQL = '') => { +const buildQueryWhere = ( + whereSQL, + joinSQL = '', + selectSQL = '', + deleted = 'FALSE', + limitSQL = '' +) => { return ` SELECT DISTINCT drafts_content.draft_id AS draft_id, @@ -11,6 +18,7 @@ const buildQueryWhere = (whereSQL, joinSQL = '', deleted = 'FALSE', limitSQL = ' count(drafts_content.id) OVER wnd as revision_count, COALESCE(last_value(drafts_content.content->'content'->>'title') OVER wnd, '') as "title", drafts.user_id AS user_id, + ${selectSQL} 'visual' AS editor FROM drafts JOIN drafts_content @@ -40,11 +48,13 @@ class DraftSummary { content, id, first_name, - last_name + last_name, + access_level }) { this.draftId = draft_id this.title = title this.userId = user_id + this.accessLevel = access_level this.createdAt = created_at this.updatedAt = updated_at this.latestVersion = latest_version @@ -76,6 +86,7 @@ class DraftSummary { static fetchByUserId(userId) { return DraftSummary.fetchAndJoinWhere( + `repository_map_user_to_draft.access_level AS access_level,`, `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id`, `repository_map_user_to_draft.user_id = $[userId]`, @@ -85,6 +96,7 @@ class DraftSummary { static fetchRecentByUserId(userId) { return DraftSummary.fetchAndJoinWhereLimit( + `repository_map_user_to_draft.access_level AS access_level,`, `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id`, `repository_map_user_to_draft.user_id = $[userId]`, @@ -95,6 +107,7 @@ class DraftSummary { static fetchAllInCollection(collectionId) { return DraftSummary.fetchAndJoinWhere( + '', `JOIN repository_map_drafts_to_collections ON repository_map_drafts_to_collections.draft_id = drafts.id`, `repository_map_drafts_to_collections.collection_id = $[collectionId]`, @@ -104,6 +117,7 @@ class DraftSummary { static fetchAllInCollectionForUser(collectionId, userId) { return DraftSummary.fetchAndJoinWhere( + `repository_map_user_to_draft.access_level AS access_level,`, `JOIN repository_map_drafts_to_collections ON repository_map_drafts_to_collections.draft_id = drafts.id JOIN repository_map_user_to_draft @@ -141,14 +155,15 @@ class DraftSummary { }) } - static fetchAndJoinWhereLimit(joinSQL, whereSQL, limitSQL, queryValues) { + static fetchAndJoinWhereLimit(selectSQL, joinSQL, whereSQL, limitSQL, queryValues) { return db - .any(buildQueryWhere(whereSQL, joinSQL, 'FALSE', limitSQL), queryValues) + .any(buildQueryWhere(whereSQL, joinSQL, selectSQL, 'FALSE', limitSQL), queryValues) .then(DraftSummary.resultsToObjects) .catch(error => { logger.error( 'fetchAndJoinWhereLimit Error', error.message, + selectSQL, joinSQL, whereSQL, limitSQL, @@ -160,15 +175,16 @@ class DraftSummary { static fetchDeletedByUserId(userId) { return DraftSummary.fetchAndJoinWhere( + `repository_map_user_to_draft.access_level AS access_level,`, `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id`, - `repository_map_user_to_draft.user_id = $[userId]`, + `repository_map_user_to_draft.user_id = $[userId] AND access_level = ${FULL}`, { userId, deleted: 'TRUE' } ) } - static fetchAndJoinWhere(joinSQL, whereSQL, queryValues) { - const query = buildQueryWhere(whereSQL, joinSQL, queryValues.deleted) + static fetchAndJoinWhere(selectSQL, joinSQL, whereSQL, queryValues) { + const query = buildQueryWhere(whereSQL, joinSQL, selectSQL, queryValues.deleted) return db .any(query, queryValues) diff --git a/packages/app/obojobo-repository/server/models/draft_summary.test.js b/packages/app/obojobo-repository/server/models/draft_summary.test.js index b35aa5d864..f311179aca 100644 --- a/packages/app/obojobo-repository/server/models/draft_summary.test.js +++ b/packages/app/obojobo-repository/server/models/draft_summary.test.js @@ -1,3 +1,5 @@ +import { FULL } from 'obojobo-express/server/constants' + describe('DraftSummary Model', () => { jest.mock('obojobo-express/server/db') jest.mock('obojobo-express/server/logger') @@ -90,7 +92,7 @@ describe('DraftSummary Model', () => { // This is just the DraftSummary non-public 'buildQuery' method. // Not sure if there's a good way of exposing that, so will just use this. - const queryBuilder = (whereSQL, joinSQL = '', deleted = 'FALSE', limitSQL = '') => + const queryBuilder = (whereSQL, joinSQL = '', selectSQL = '', deleted = 'FALSE', limitSQL = '') => ` SELECT DISTINCT drafts_content.draft_id AS draft_id, @@ -100,6 +102,7 @@ describe('DraftSummary Model', () => { count(drafts_content.id) OVER wnd as revision_count, COALESCE(last_value(drafts_content.content->'content'->>'title') OVER wnd, '') as "title", drafts.user_id AS user_id, + ${selectSQL} 'visual' AS editor FROM drafts JOIN drafts_content @@ -220,7 +223,8 @@ describe('DraftSummary Model', () => { const whereSQL = 'repository_map_user_to_draft.user_id = $[userId]' const joinSQL = `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id` - const query = queryBuilder(whereSQL, joinSQL) + const selectSQL = 'repository_map_user_to_draft.access_level AS access_level,' + const query = queryBuilder(whereSQL, joinSQL, selectSQL) return DraftSummary.fetchByUserId(0).then(summary => { const [actualQuery, options] = db.any.mock.calls[0] @@ -237,8 +241,9 @@ describe('DraftSummary Model', () => { const whereSQL = 'repository_map_user_to_draft.user_id = $[userId]' const joinSQL = `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id` + const selectSQL = 'repository_map_user_to_draft.access_level AS access_level,' const limitSQL = 'LIMIT 5' - const query = queryBuilder(whereSQL, joinSQL, 'FALSE', limitSQL) + const query = queryBuilder(whereSQL, joinSQL, selectSQL, 'FALSE', limitSQL) return DraftSummary.fetchRecentByUserId(0).then(summary => { const [actualQuery, options] = db.any.mock.calls[0] @@ -273,7 +278,8 @@ describe('DraftSummary Model', () => { ON repository_map_drafts_to_collections.draft_id = drafts.id JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id` - const query = queryBuilder(whereSQL, joinSQL) + const selectSQL = `repository_map_user_to_draft.access_level AS access_level,` + const query = queryBuilder(whereSQL, joinSQL, selectSQL) return DraftSummary.fetchAllInCollectionForUser('mockCollectionId', 0).then(summary => { expect(db.any).toHaveBeenCalledWith(query, { collectionId: 'mockCollectionId', userId: 0 }) @@ -328,22 +334,28 @@ describe('DraftSummary Model', () => { const whereSQL = '' const joinSQL = '' + const selectSQL = '' const limitSQL = '' const mockQueryValues = { id: 'mockDraftId' } - return DraftSummary.fetchAndJoinWhereLimit(whereSQL, joinSQL, limitSQL, mockQueryValues).catch( - err => { - expect(logger.error).toHaveBeenCalledWith( - 'fetchAndJoinWhereLimit Error', - 'not found in db', - whereSQL, - joinSQL, - limitSQL, - mockQueryValues - ) - expect(err).toBe('Error loading DraftSummary by query') - } - ) + return DraftSummary.fetchAndJoinWhereLimit( + whereSQL, + joinSQL, + selectSQL, + limitSQL, + mockQueryValues + ).catch(err => { + expect(logger.error).toHaveBeenCalledWith( + 'fetchAndJoinWhereLimit Error', + 'not found in db', + whereSQL, + joinSQL, + selectSQL, + limitSQL, + mockQueryValues + ) + expect(err).toBe('Error loading DraftSummary by query') + }) }) test('fetchAndJoinWhere catches database errors', () => { @@ -354,12 +366,18 @@ describe('DraftSummary Model', () => { const whereSQL = '' const joinSQL = '' + const selectSQL = '' const mockQueryValues = { id: 'mockDraftId' } - return DraftSummary.fetchAndJoinWhere(whereSQL, joinSQL, mockQueryValues).catch(err => { - expect(logger.logError).toHaveBeenCalledWith('Error loading DraftSummary by query', mockError) - expect(err).toBe(mockError) - }) + return DraftSummary.fetchAndJoinWhere(selectSQL, whereSQL, joinSQL, mockQueryValues).catch( + err => { + expect(logger.logError).toHaveBeenCalledWith( + 'Error loading DraftSummary by query', + mockError + ) + expect(err).toBe(mockError) + } + ) }) test('fetchWhere catches database errors', () => { @@ -643,10 +661,12 @@ describe('DraftSummary Model', () => { db.any = jest.fn() db.any.mockResolvedValueOnce(mockRawDraftSummary) - const whereSQL = 'repository_map_user_to_draft.user_id = $[userId]' + const whereSQL = `repository_map_user_to_draft.user_id = $[userId] AND access_level = ${FULL}` const joinSQL = `JOIN repository_map_user_to_draft ON repository_map_user_to_draft.draft_id = drafts.id` - const query = queryBuilder(whereSQL, joinSQL, 'TRUE') + const selectSQL = 'repository_map_user_to_draft.access_level AS access_level,' + + const query = queryBuilder(whereSQL, joinSQL, selectSQL, 'TRUE') return DraftSummary.fetchDeletedByUserId(0).then(summary => { expect(db.any).toHaveBeenCalledWith(query, { userId: 0, deleted: 'TRUE' }) diff --git a/packages/app/obojobo-repository/server/routes/api.js b/packages/app/obojobo-repository/server/routes/api.js index 9fb75cd19a..28d2afdd53 100644 --- a/packages/app/obojobo-repository/server/routes/api.js +++ b/packages/app/obojobo-repository/server/routes/api.js @@ -22,6 +22,8 @@ const { fetchAllCollectionsForDraft } = require('../services/collections') const { getUserModuleCount } = require('../services/count') const publicLibCollectionId = require('../../shared/publicLibCollectionId') +const { levelName, levelNumber, FULL } = require('../../../obojobo-express/server/constants') + // List public drafts router.route('/drafts-public').get((req, res) => { return Collection.fetchById(publicLibCollectionId) @@ -241,9 +243,10 @@ router const draftId = req.currentDocument.draftId // check currentUser's permissions - const canShare = await DraftPermissions.userHasPermissionToDraft(req.currentUser.id, draftId) + const canShare = + (await DraftPermissions.getUserAccessLevelToDraft(req.currentUser.id, draftId)) === FULL if (!canShare) { - res.notAuthorized('Current User has no permissions to selected draft') + res.notAuthorized('Current User does not have permission to share this draft') return } @@ -259,6 +262,46 @@ router } }) +// update a user's access level +router + .route('/drafts/:draftId/permission/update') + .post([requireCurrentUser, requireCurrentDocument]) + .post(async (req, res) => { + try { + const userId = req.body.userId + const draftId = req.currentDocument.draftId + const targetLevel = req.body.accessLevel + + // Guard against invalid access levels + if (!levelNumber[targetLevel]) { + const msg = 'Invalid access level: ' + targetLevel + res.status(400).send(msg) + return + } + + // check if same access level + const currentLevel = await DraftPermissions.getUserAccessLevelToDraft( + req.body.userId, + draftId + ) + + if (levelName[currentLevel] === targetLevel) { + res.success() + return + } + + // make sure the target userId exists + // fetchById will throw if not found + await UserModel.fetchById(userId) + + // add permissions + await DraftPermissions.updateAccessLevel(draftId, userId, levelNumber[targetLevel]) + res.success() + } catch (error) { + res.unexpected(error) + } + }) + // delete a permission for a user to a draft router .route('/drafts/:draftId/permission/:userId') @@ -269,7 +312,8 @@ router const draftId = req.currentDocument.draftId // check currentUser's permissions - const canShare = await DraftPermissions.userHasPermissionToDraft(req.currentUser.id, draftId) + const canShare = + (await DraftPermissions.getUserAccessLevelToDraft(req.currentUser.id, draftId)) === FULL if (!canShare) { res.notAuthorized('Current User has no permissions to selected draft') return diff --git a/packages/app/obojobo-repository/server/routes/api.test.js b/packages/app/obojobo-repository/server/routes/api.test.js index 0c5b3366ab..54594f0da1 100644 --- a/packages/app/obojobo-repository/server/routes/api.test.js +++ b/packages/app/obojobo-repository/server/routes/api.test.js @@ -78,6 +78,8 @@ app.use(require('obojobo-express/server/express_current_document')) app.use('/', require('obojobo-express/server/express_response_decorator')) app.use('/', require('obojobo-repository/server/routes/api')) +import { FULL, PARTIAL, MINIMAL, levelName } from '../../../obojobo-express/server/constants' + describe('repository api route', () => { beforeEach(() => { jest.resetAllMocks() @@ -639,10 +641,74 @@ describe('repository api route', () => { }) }) - test('post /drafts/:draftId/permission runs correctly when current user has perms to draft', () => { + // update draft access levels + test('post /drafts/:draftId/permission/update does not call updateAccessLevel if target access level matches current access level', () => { + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(PARTIAL) + + return request(app) + .post('/drafts/mockDraftId/permission/update') + .send({ accessLevel: levelName[PARTIAL], userId: 99 }) + .then(response => { + expect(DraftPermissions.updateAccessLevel).not.toHaveBeenCalled() + expect(response.statusCode).toBe(200) + }) + }) + + test('post /drafts/:draftId/permission/update correctly sets new access level ', () => { + DraftPermissions.getUserAccessLevelToDraft + .mockResolvedValueOnce(FULL) + .mockResolvedValueOnce(PARTIAL) + + UserModel.fetchById = jest.fn() + UserModel.fetchById.mockResolvedValueOnce(true) + + return request(app) + .post('/drafts/mockDraftId/permission/update') + .send({ accessLevel: levelName[MINIMAL], userId: 99 }) + .then(response => { + expect(DraftPermissions.updateAccessLevel).toHaveBeenCalled() + expect(DraftPermissions.updateAccessLevel).toHaveBeenCalledWith( + mockCurrentDocument.draftId, + 99, + MINIMAL + ) + expect(response.statusCode).toBe(200) + }) + }) + + test('post /drafts/:draftId/permission/update handles unknown access level', () => { + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) + + UserModel.fetchById = jest.fn() + UserModel.fetchById.mockResolvedValueOnce(true) + return request(app) + .post('/drafts/mockDraftId/permission/update') + .send({ accessLevel: 'Unknown', userId: 99 }) + .then(response => { + expect(DraftPermissions.updateAccessLevel).toHaveBeenCalledTimes(0) + expect(response.statusCode).toBe(400) + expect(response.text).toBe('Invalid access level: Unknown') + }) + }) + + test('post /drafts/:draftId/permission/update handles thrown errors', () => { + expect.hasAssertions() + + DraftPermissions.getUserAccessLevelToDraft.mockRejectedValueOnce('database error') + + return request(app) + .post('/drafts/mockDraftId/permission/update') + .send({ accessLevel: levelName[PARTIAL], userId: 99 }) + .then(response => { + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledTimes(1) + expect(response.statusCode).toBe(500) + }) + }) + + test('post /drafts/:draftId/permission runs correctly when current user has "Full" access level to draft', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) DraftPermissions.addOwnerToDraft.mockResolvedValueOnce() UserModel.fetchById = jest.fn() @@ -652,7 +718,7 @@ describe('repository api route', () => { .post('/drafts/mockDraftId/permission') .send({ userId: 1 }) .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) @@ -665,17 +731,17 @@ describe('repository api route', () => { }) }) - test('post /drafts/:draftId/permission runs correctly when current user does not have perms to draft', () => { + test('post /drafts/:draftId/permission runs correctly when current user does not have "Full" access level to draft', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(PARTIAL) UserModel.fetchById = jest.fn() return request(app) .post('/drafts/mockDraftId/permission') .send({ userId: 1 }) .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) @@ -688,7 +754,7 @@ describe('repository api route', () => { test('post /drafts/:draftId/permission catches unexpected errors correctly', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) UserModel.fetchById = jest.fn() UserModel.fetchById.mockResolvedValueOnce(true) DraftPermissions.addOwnerToDraft.mockRejectedValueOnce('database error') @@ -697,7 +763,7 @@ describe('repository api route', () => { .post('/drafts/mockDraftId/permission') .send({ userId: 1 }) .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) @@ -713,10 +779,10 @@ describe('repository api route', () => { }) }) - test('delete /drafts/:draftId/permission/:userId runs correctly when current user has perms to draft', () => { + test('delete /drafts/:draftId/permission/:userId runs correctly when current user has "Full" access level to draft', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) UserModel.fetchById = jest.fn() UserModel.fetchById.mockResolvedValueOnce({ id: 1 }) DraftPermissions.removeOwnerFromDraft.mockResolvedValueOnce(true) @@ -724,7 +790,7 @@ describe('repository api route', () => { return request(app) .delete('/drafts/mockDraftId/permission/1') .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) @@ -737,16 +803,16 @@ describe('repository api route', () => { }) }) - test('delete /drafts/:draftId/permission/:userId runs correctly when current user does not have perms to draft', () => { + test('delete /drafts/:draftId/permission/:userId runs correctly when current user does not have "Full" access level to draft', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(MINIMAL) UserModel.fetchById = jest.fn() return request(app) .delete('/drafts/mockDraftId/permission/1') .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) @@ -759,7 +825,7 @@ describe('repository api route', () => { test('delete /drafts/:draftId/permission/:userId catches unexpected errors correctly', () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) UserModel.fetchById = jest.fn() UserModel.fetchById.mockResolvedValueOnce({ id: 1 }) DraftPermissions.removeOwnerFromDraft.mockRejectedValueOnce('database error') @@ -767,7 +833,7 @@ describe('repository api route', () => { return request(app) .delete('/drafts/mockDraftId/permission/1') .then(response => { - expect(DraftPermissions.userHasPermissionToDraft).toHaveBeenCalledWith( + expect(DraftPermissions.getUserAccessLevelToDraft).toHaveBeenCalledWith( mockCurrentUser.id, mockCurrentDocument.draftId ) diff --git a/packages/app/obojobo-repository/server/routes/dashboard.js b/packages/app/obojobo-repository/server/routes/dashboard.js index a40a7a928f..260933d8b7 100644 --- a/packages/app/obojobo-repository/server/routes/dashboard.js +++ b/packages/app/obojobo-repository/server/routes/dashboard.js @@ -54,7 +54,7 @@ const renderDashboard = (req, res, options) => { switch (options.mode) { case MODE_COLLECTION: pageTitle = 'View Collection' - return DraftSummary.fetchAllInCollection(options.collection.id, req.currentUser.id) + return DraftSummary.fetchAllInCollectionForUser(options.collection.id, req.currentUser.id) case MODE_ALL: return DraftSummary.fetchByUserId(req.currentUser.id) case MODE_DELETED: diff --git a/packages/app/obojobo-repository/server/routes/dashboard.test.js b/packages/app/obojobo-repository/server/routes/dashboard.test.js index 09758c7344..fc290f2f6c 100644 --- a/packages/app/obojobo-repository/server/routes/dashboard.test.js +++ b/packages/app/obojobo-repository/server/routes/dashboard.test.js @@ -325,12 +325,12 @@ describe('repository dashboard route', () => { CollectionSummary.fetchByUserId = jest.fn() CollectionSummary.fetchByUserId.mockResolvedValueOnce(mockCollectionSummary) - DraftSummary.fetchAllInCollection = jest.fn() + DraftSummary.fetchAllInCollectionForUser = jest.fn() DraftSummary.fetchByUserId = jest.fn() DraftSummary.fetchDeletedByUserId = jest.fn() DraftSummary.fetchRecentByUserId = jest.fn() - DraftSummary.fetchAllInCollection.mockResolvedValueOnce(mockModuleSummary) + DraftSummary.fetchAllInCollectionForUser.mockResolvedValueOnce(mockModuleSummary) const path = 'collections/mock-collection-safe-name-mockCollectionShortId' @@ -356,8 +356,8 @@ describe('repository dashboard route', () => { expect(CollectionSummary.fetchByUserId).toHaveBeenCalledTimes(1) expect(CollectionSummary.fetchByUserId).toHaveBeenCalledWith(mockCurrentUser.id) - expect(DraftSummary.fetchAllInCollection).toHaveBeenCalledTimes(1) - expect(DraftSummary.fetchAllInCollection).toHaveBeenCalledWith( + expect(DraftSummary.fetchAllInCollectionForUser).toHaveBeenCalledTimes(1) + expect(DraftSummary.fetchAllInCollectionForUser).toHaveBeenCalledWith( mockSingleCollection.id, mockCurrentUser.id ) diff --git a/packages/app/obojobo-repository/shared/actions/dashboard-actions.js b/packages/app/obojobo-repository/shared/actions/dashboard-actions.js index 0a70b11b40..c9071c48eb 100644 --- a/packages/app/obojobo-repository/shared/actions/dashboard-actions.js +++ b/packages/app/obojobo-repository/shared/actions/dashboard-actions.js @@ -42,6 +42,15 @@ const apiAddPermissionsToModule = (draftId, userId) => { return fetch(`/api/drafts/${draftId}/permission`, options).then(res => res.json()) } +const apiUpdatePermissionsForModule = (draftId, userId, accessLevel) => { + const options = { + ...defaultOptions(), + method: 'POST', + body: `{"userId":${userId}, "accessLevel":"${accessLevel}"}` + } + return fetch(`/api/drafts/${draftId}/permission/update`, options).then(res => res.json()) +} + const apiGetPermissionsForModule = draftId => { return fetch(`/api/drafts/${draftId}/permission`, defaultOptions()).then(res => res.json()) } @@ -262,6 +271,14 @@ const addUserToModule = (draftId, userId) => ({ ) }) +const CHANGE_ACCESS_LEVEL = 'CHANGE_ACCESS_LEVEL' +const changeAccessLevel = (draftId, userId, accessLevel) => ({ + type: CHANGE_ACCESS_LEVEL, + promise: apiUpdatePermissionsForModule(draftId, userId, accessLevel).then(() => { + apiGetPermissionsForModule(draftId) + }) +}) + const DELETE_MODULE_PERMISSIONS = 'DELETE_MODULE_PERMISSIONS' const deleteModulePermissions = (draftId, userId, options = { ...defaultModuleModeOptions }) => { let apiModuleGetCall @@ -340,11 +357,11 @@ const bulkDeleteModules = draftIds => ({ }) const BULK_ADD_MODULES_TO_COLLECTIONS = 'BULK_ADD_MODULES_TO_COLLECTIONS' -const bulkAddModulesToCollection = (draftIds, collectionIds) => { +const bulkAddModulesToCollection = (drafts, collectionIds) => { const allPromises = [] - draftIds.forEach(draftId => { + drafts.forEach(draft => { collectionIds.forEach(collectionId => { - allPromises.push(apiAddModuleToCollection(draftId, collectionId)) + allPromises.push(apiAddModuleToCollection(draft.draftId, collectionId)) }) }) @@ -613,6 +630,7 @@ module.exports = { LOAD_USER_SEARCH, CLOSE_MODAL, ADD_USER_TO_MODULE, + CHANGE_ACCESS_LEVEL, LOAD_USERS_FOR_MODULE, CREATE_NEW_MODULE, CLEAR_PEOPLE_SEARCH_RESULTS, @@ -661,6 +679,7 @@ module.exports = { deleteModulePermissions, searchForUser, addUserToModule, + changeAccessLevel, createNewCollection, createNewModule, showModulePermissions, diff --git a/packages/app/obojobo-repository/shared/actions/dashboard-actions.test.js b/packages/app/obojobo-repository/shared/actions/dashboard-actions.test.js index ddf5f996b1..97c8e4460c 100644 --- a/packages/app/obojobo-repository/shared/actions/dashboard-actions.test.js +++ b/packages/app/obojobo-repository/shared/actions/dashboard-actions.test.js @@ -219,6 +219,36 @@ describe('Dashboard Actions', () => { }) }) + test('changeAccessLevel returns the expected output and calls other functions correctly', () => { + global.fetch.mockResolvedValueOnce(standardFetchResponse) + + const actionReply = DashboardActions.changeAccessLevel('mockDraftId', 99, 'Partial') + + expect(global.fetch).toHaveBeenCalledWith('/api/drafts/mockDraftId/permission/update', { + ...defaultFetchOptions, + method: 'POST', + body: '{"userId":99, "accessLevel":"Partial"}' + }) + global.fetch.mockReset() + global.fetch.mockResolvedValueOnce({ + json: () => ({ value: 'mockSecondaryPermissionsVal' }) + }) + + expect(actionReply).toEqual({ + type: DashboardActions.CHANGE_ACCESS_LEVEL, + promise: expect.any(Object) + }) + + // should get draft permissions after changing them + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + expect(global.fetch).toHaveBeenCalledWith( + '/api/drafts/mockDraftId/permission', + defaultFetchOptions + ) + }) + }) + // three (plus one default) ways of calling deleteModulePermissions const assertDeleteModulePermissionsRunsWithOptions = (secondaryLookupUrl, options) => { global.fetch.mockResolvedValueOnce(standardFetchResponse) @@ -429,10 +459,14 @@ describe('Dashboard Actions', () => { test('bulkAddModulesToCollection calls other functions', () => { global.fetch.mockResolvedValue(standardFetchResponse) - const mockDraftIds = ['draft-id-1', 'draft-id-2', 'draft-id-3'] + const mockDrafts = [ + { draftId: 'draft-id-1' }, + { draftId: 'draft-id-2' }, + { draftId: 'draft-id-3' } + ] const mockCollectionIds = ['collection-id-1', 'collection-id-2'] - const actionReply = DashboardActions.bulkAddModulesToCollection(mockDraftIds, mockCollectionIds) + const actionReply = DashboardActions.bulkAddModulesToCollection(mockDrafts, mockCollectionIds) expect(actionReply).toEqual({ type: DashboardActions.BULK_ADD_MODULES_TO_COLLECTIONS, @@ -440,11 +474,11 @@ describe('Dashboard Actions', () => { }) actionReply.promise.then(() => { - const expectedNumberOfCalls = mockDraftIds.length * mockCollectionIds.length + const expectedNumberOfCalls = mockDrafts.length * mockCollectionIds.length expect(global.fetch).toHaveBeenCalledTimes(expectedNumberOfCalls) let callIndex = 0 - mockDraftIds.forEach(draftId => { + mockDrafts.forEach(draft => { mockCollectionIds.forEach(collectionId => { const expectedCall = global.fetch.mock.calls[callIndex++] const expectedApiUrl = `/api/collections/${collectionId}/modules/add` @@ -452,7 +486,7 @@ describe('Dashboard Actions', () => { expect(expectedCall[1]).toEqual({ ...defaultFetchOptions, method: 'POST', - body: `{"draftId":"${draftId}"}` + body: `{"draftId":"${draft.draftId}"}` }) }) }) diff --git a/packages/app/obojobo-repository/shared/components/__snapshots__/module-menu.test.js.snap b/packages/app/obojobo-repository/shared/components/__snapshots__/module-menu.test.js.snap index 2a8e006757..2c6438472f 100644 --- a/packages/app/obojobo-repository/shared/components/__snapshots__/module-menu.test.js.snap +++ b/packages/app/obojobo-repository/shared/components/__snapshots__/module-menu.test.js.snap @@ -1,11 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ModuleMenu ModuleMenu renders correctly with className prop 1`] = ` +exports[`ModuleMenu ModuleMenu renders correctly with "Full" access level 1`] = `
`; -exports[`ModuleMenu ModuleMenu renders correctly with standard expected props 1`] = ` +exports[`ModuleMenu ModuleMenu renders correctly with "Minimal" access level 1`] = ` +
+
+ + Preview + +
+ +
+
+`; + +exports[`ModuleMenu ModuleMenu renders correctly with "Partial" access level 1`] = `
@@ -63,12 +89,74 @@ exports[`ModuleMenu ModuleMenu renders correctly with standard expected props 1` > Edit +
+
+
+`; + +exports[`ModuleMenu ModuleMenu renders correctly with className prop 1`] = ` +
+
+ + Preview + + + Edit + +
+ +
+
+`; + +exports[`ModuleMenu ModuleMenu renders correctly with standard expected props 1`] = ` +
+
+ + Preview + + + Edit +
+
+ Add or remove collaborators. +
+
- Write, edit, and update. + +
+ View scores by student. +
- + +
+ View and restore previous versions. +
+
- Add or remove collaborators. + +
+ Add to or remove from private collections. +
- + +
+ Download a copy in JSON format. +
+
- View scores by student. + +
+ Download a copy in XML format. +
- + + Public Page + +
+ Visit this modules public page. +
+
- View and restore previous versions. + +
+ Say farewell. +
- + + +`; + +exports[`ModuleOptionsDialog renders correctly with Minimal access level 1`] = ` +
+
+
+ +
+
+ Mock Module Title +
+ +
+
+

+ Module Options +

+

+ Your Access Level: + Minimal +

+
+
- Manage Collections - + + Preview + +
+ View with preview controls. +
+
- Add to or remove from private collections. + +
+ View scores by student. +
- + +
+ Add to or remove from private collections. +
+
- Download a copy in JSON format. + + Public Page + +
+ Visit this modules public page. +
- +
+
+`; + +exports[`ModuleOptionsDialog renders correctly with Partial access level 1`] = ` +
+
+
+ +
+
+ Mock Module Title +
+ +
+
+

+ Module Options +

+

+ Your Access Level: + Partial +

+
+
+ + Preview + +
+ View with preview controls. +
+
+
+ + Edit + +
+ Write, edit, and update. +
+
+
+ +
+ View scores by student. +
+
+
+ +
+ View and restore previous versions. +
+
+
+ +
+ Add to or remove from private collections. +
+
+
+ +
+ Download a copy in JSON format. +
+
+
+ +
+ Download a copy in XML format. +
+
+
+ + Public Page + +
+ Visit this modules public page. +
+
+
+ +
+
+`; + +exports[`ModuleOptionsDialog renders correctly with standard expected props 1`] = ` +
+
+
+ +
+
+ Mock Module Title +
+ +
+
+

+ Module Options +

+

+ Your Access Level: + Full +

+
+
- Download XML - + + Preview + +
+ View with preview controls. +
+
- Download a copy in XML format. + + Edit + +
+ Write, edit, and update. +
- - Public Page - + +
+ Add or remove collaborators. +
+
- Visit this modules public page. + +
+ View scores by student. +
- + +
+ View and restore previous versions. +
+
+
+ +
+ Add to or remove from private collections. +
+
+
+ +
+ Download a copy in JSON format. +
+
+
+ +
+ Download a copy in XML format. +
+
+
+ + Public Page + +
+ Visit this modules public page. +
+
- Say farewell. + +
+ Say farewell. +
@@ -712,13 +791,12 @@ function Dashboard(props) { mainControlBarRender = (
- {getModuleCount(props.selectedModules)} + + {getModuleCount(numFullSelected, numPartialSelected, numMinimalSelected)} + {bulkCollectionActionButton} {bulkActionButton} -
diff --git a/packages/app/obojobo-repository/shared/components/dashboard.test.js b/packages/app/obojobo-repository/shared/components/dashboard.test.js index c23347c6af..7d7faf17b7 100644 --- a/packages/app/obojobo-repository/shared/components/dashboard.test.js +++ b/packages/app/obojobo-repository/shared/components/dashboard.test.js @@ -67,6 +67,7 @@ import VersionHistoryDialog from './version-history-dialog' import AssessmentScoreDataDialog from './assessment-score-data-dialog' const { MODE_RECENT, MODE_ALL, MODE_COLLECTION, MODE_DELETED } = require('../repository-constants') +const { FULL, PARTIAL, MINIMAL } = require('obojobo-express/server/constants') describe('Dashboard', () => { const mockShortFromUUID = jest.fn() @@ -108,30 +109,35 @@ describe('Dashboard', () => { { draftId: 'mockDraftId', title: 'D Module Title ', + accessLevel: FULL, createdAt: new Date(10000000000).toISOString(), updatedAt: new Date(200000000000).toISOString() }, { draftId: 'mockDraftId2', title: 'A Module Title 2', + accessLevel: FULL, createdAt: new Date(20000000000).toISOString(), updatedAt: new Date(400000000000).toISOString() }, { draftId: 'mockDraftId3', title: 'C Module Title 3', + accessLevel: PARTIAL, createdAt: new Date(30000000000).toISOString(), updatedAt: new Date(300000000000).toISOString() }, { draftId: 'mockDraftId4', title: 'B Module Title 4', + accessLevel: PARTIAL, createdAt: new Date(40000000000).toISOString(), updatedAt: new Date(100000000000).toISOString() }, { draftId: 'mockDraftId5', title: 'E Module Title 5', + accessLevel: MINIMAL, createdAt: new Date(50000000000).toISOString(), updatedAt: new Date(500000000000).toISOString() } @@ -913,7 +919,9 @@ describe('Dashboard', () => { test('renders selected module count properly when one or multiple modules are selected', () => { dashboardProps.myModules = [...standardMyModules] dashboardProps.multiSelectMode = true - dashboardProps.selectedModules = [standardMyModules[0].draftId, standardMyModules[1].draftId] + dashboardProps.selectedModules = [standardMyModules[0], standardMyModules[1]] + dashboardProps.numFullSelected = 2 + let component act(() => { component = create() @@ -922,15 +930,137 @@ describe('Dashboard', () => { const expectedControlBarClasses = 'repository--main-content--control-bar is-multi-select-mode' let controlBar = component.root.findByProps({ className: expectedControlBarClasses }) - expect(controlBar.children[0].children[0]).toEqual('2 Modules Selected:') + expect(controlBar.children[0].children[0]).toEqual( + '2 Modules Selected (2 Full, 0 Partial, 0 Minimal):' + ) + + dashboardProps.selectedModules = [standardMyModules[0]] + dashboardProps.numFullSelected = 1 - dashboardProps.selectedModules = [standardMyModules[0].draftId] act(() => { component = create() }) controlBar = component.root.findByProps({ className: expectedControlBarClasses }) - expect(controlBar.children[0].children[0]).toEqual('1 Module Selected:') + expect(controlBar.children[0].children[0]).toEqual( + '1 Module Selected (1 Full, 0 Partial, 0 Minimal):' + ) + + component.unmount() + }) + + test('updates selected module count properly when new module is selected', () => { + dashboardProps.myModules = [...standardMyModules] + dashboardProps.multiSelectMode = true + dashboardProps.selectedModules = [standardMyModules[0]] + dashboardProps.numFullSelected = 1 + dashboardProps.selectModules = jest.fn() + + let component + act(() => { + component = create() + }) + + const expectedControlBarClasses = 'repository--main-content--control-bar is-multi-select-mode' + let controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + + // Begin with one full access level module selected + expect(controlBar.children[0].children[0]).toEqual( + '1 Module Selected (1 Full, 0 Partial, 0 Minimal):' + ) + + const moduleComponents = component.root.findAllByType(Module) + + // Select a partial access level module + act(() => { + const mockClickEvent = { + shiftKey: false + } + moduleComponents[1].props.onSelect(mockClickEvent) + }) + + dashboardProps.selectedModules = [standardMyModules[0], standardMyModules[3]] + + controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + expect(controlBar.children[0].children[0]).toEqual( + '2 Modules Selected (1 Full, 1 Partial, 0 Minimal):' + ) + + // Select a minimal access level module + act(() => { + const mockClickEvent = { + shiftKey: false + } + moduleComponents[4].props.onSelect(mockClickEvent) + }) + + dashboardProps.selectedModules = [standardMyModules[0], standardMyModules[3]] + + controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + expect(controlBar.children[0].children[0]).toEqual( + '3 Modules Selected (1 Full, 1 Partial, 1 Minimal):' + ) + + component.unmount() + }) + + test('updates selected module count properly when module is deselected', () => { + dashboardProps.myModules = [...standardMyModules] + dashboardProps.multiSelectMode = true + dashboardProps.selectedModules = [ + standardMyModules[0], + standardMyModules[3], + standardMyModules[4] + ] + dashboardProps.numFullSelected = 1 + dashboardProps.numPartialSelected = 1 + dashboardProps.numMinimalSelected = 1 + dashboardProps.deselectModules = jest.fn() + + let component + act(() => { + component = create() + }) + + const expectedControlBarClasses = 'repository--main-content--control-bar is-multi-select-mode' + let controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + + // Begin with one of each access level selected + expect(controlBar.children[0].children[0]).toEqual( + '3 Modules Selected (1 Full, 1 Partial, 1 Minimal):' + ) + + const moduleComponents = component.root.findAllByType(Module) + + // Deselect a partial access level module + act(() => { + const mockClickEvent = { + shiftKey: false + } + moduleComponents[1].props.onSelect(mockClickEvent) + }) + + dashboardProps.selectedModules = [standardMyModules[0], standardMyModules[4]] + + controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + expect(controlBar.children[0].children[0]).toEqual( + '2 Modules Selected (1 Full, 0 Partial, 1 Minimal):' + ) + + // Deselect a minimal access level module + act(() => { + const mockClickEvent = { + shiftKey: false + } + moduleComponents[4].props.onSelect(mockClickEvent) + }) + + dashboardProps.selectedModules = [standardMyModules[0]] + + controlBar = component.root.findByProps({ className: expectedControlBarClasses }) + expect(controlBar.children[0].children[0]).toEqual( + '1 Module Selected (1 Full, 0 Partial, 0 Minimal):' + ) component.unmount() }) @@ -1093,7 +1223,7 @@ describe('Dashboard', () => { }) test('"Add All To Collection" button calls functions appropriately', () => { - const mockSelectedModules = ['mockId', 'mockId2'] + const mockSelectedModules = [standardMyModules[0], standardMyModules[1]] dashboardProps.showCollectionBulkAddModulesDialog = jest.fn(() => Promise.resolve()) @@ -1130,13 +1260,14 @@ describe('Dashboard', () => { // This will only be available in MODE_COLLECTION test('"Remove All From Collection" button calls functions appropriately', async () => { - const mockSelectedModules = ['mockId', 'mockId2'] + const mockSelectedModules = [standardMyModules[1], standardMyModules[2]] dashboardProps.bulkRemoveModulesFromCollection = jest.fn(() => Promise.resolve()) dashboardProps.myModules = [...standardMyModules] dashboardProps.multiSelectMode = true dashboardProps.selectedModules = mockSelectedModules + dashboardProps.deselectModules = jest.fn() dashboardProps.mode = MODE_COLLECTION dashboardProps.collection = { @@ -1172,7 +1303,7 @@ describe('Dashboard', () => { }) expect(dashboardProps.bulkRemoveModulesFromCollection).toHaveBeenCalledTimes(1) expect(dashboardProps.bulkRemoveModulesFromCollection).toHaveBeenCalledWith( - mockSelectedModules, + mockSelectedModules.map(draft => draft.draftId), 'mockCollectionId' ) @@ -1181,8 +1312,9 @@ describe('Dashboard', () => { test('"Delete All" button calls functions appropriately', async () => { dashboardProps.bulkDeleteModules = jest.fn(() => Promise.resolve()) - dashboardProps.selectedModules = ['mockId', 'mockId2'] + dashboardProps.selectedModules = [standardMyModules[0], standardMyModules[1]] dashboardProps.multiSelectMode = true + dashboardProps.deselectModules = jest.fn() const reusableComponent = let component act(() => { @@ -1262,6 +1394,7 @@ describe('Dashboard', () => { dashboardProps.myModules = [...standardMyModules] dashboardProps.selectModules = jest.fn() dashboardProps.deselectModules = jest.fn() + let component act(() => { component = create() @@ -1277,9 +1410,9 @@ describe('Dashboard', () => { moduleComponents[0].props.onSelect(mockClickEvent) }) expect(dashboardProps.selectModules).toHaveBeenCalledTimes(1) - expect(dashboardProps.selectModules).toHaveBeenCalledWith(['mockDraftId2']) + expect(dashboardProps.selectModules).toHaveBeenCalledWith([standardMyModules[1]]) - dashboardProps.selectedModules = ['mockDraftId2'] + dashboardProps.selectedModules = [standardMyModules[1]] dashboardProps.multiSelectMode = true act(() => { @@ -1294,7 +1427,7 @@ describe('Dashboard', () => { moduleComponents[0].props.onSelect(mockClickEvent) }) expect(dashboardProps.deselectModules).toHaveBeenCalledTimes(1) - expect(dashboardProps.deselectModules).toHaveBeenCalledWith(['mockDraftId2']) + expect(dashboardProps.deselectModules).toHaveBeenCalledWith([standardMyModules[1]]) component.unmount() }) @@ -1343,7 +1476,7 @@ describe('Dashboard', () => { moduleComponents[2].props.onSelect(mockClickEvent) }) expect(dashboardProps.selectModules).toHaveBeenCalledTimes(1) - expect(dashboardProps.selectModules).toHaveBeenCalledWith(['mockDraftId3']) + expect(dashboardProps.selectModules).toHaveBeenCalledWith([standardMyModules[2]]) dashboardProps.selectModules.mockReset() component.unmount() @@ -1382,7 +1515,10 @@ describe('Dashboard', () => { }) expect(dashboardProps.selectModules).toHaveBeenCalledTimes(1) // this seems a bit magical without knowing the sort order based on last updated times - maybe replace with some variables and math later - expect(dashboardProps.selectModules).toHaveBeenCalledWith(['mockDraftId4', 'mockDraftId3']) + expect(dashboardProps.selectModules).toHaveBeenCalledWith([ + standardMyModules[3], + standardMyModules[2] + ]) dashboardProps.selectModules.mockReset() component.unmount() @@ -1391,7 +1527,7 @@ describe('Dashboard', () => { test('shift-clicking a module when modules are already selected properly selects additonal modules', () => { dashboardProps.myModules = [...standardMyModules] dashboardProps.selectModules = jest.fn() - dashboardProps.selectedModules = ['mockDraftId4'] + dashboardProps.selectedModules = [standardMyModules[3]] let component act(() => { component = create() @@ -1407,7 +1543,7 @@ describe('Dashboard', () => { }) expect(dashboardProps.selectModules).toHaveBeenCalledTimes(1) // mockDraftId4 is already in 'selectedModules', so it will not be passed to 'selectModules' - expect(dashboardProps.selectModules).toHaveBeenCalledWith(['mockDraftId2']) + expect(dashboardProps.selectModules).toHaveBeenCalledWith([standardMyModules[1]]) dashboardProps.selectModules.mockReset() }) @@ -1947,12 +2083,13 @@ describe('Dashboard', () => { const originalAlert = global.alert global.alert = jest.fn() - const mockSelectedModules = [standardMyModules[0].draftId, standardMyModules[1].draftId] + const mockSelectedModules = [standardMyModules[0], standardMyModules[1]] dashboardProps.mode = MODE_DELETED dashboardProps.myModules = [...standardMyModules] dashboardProps.multiSelectMode = true dashboardProps.selectedModules = mockSelectedModules + dashboardProps.deselectModules = jest.fn() dashboardProps.bulkRestoreModules = jest.fn().mockResolvedValue(true) @@ -1971,7 +2108,9 @@ describe('Dashboard', () => { restoreAllButton.props.onClick(mockClickEvent) }) expect(dashboardProps.bulkRestoreModules).toHaveBeenCalled() - expect(dashboardProps.bulkRestoreModules).toHaveBeenCalledWith(mockSelectedModules) + expect(dashboardProps.bulkRestoreModules).toHaveBeenCalledWith( + mockSelectedModules.map(module => module.draftId) + ) component.unmount() diff --git a/packages/app/obojobo-repository/shared/components/module-menu.jsx b/packages/app/obojobo-repository/shared/components/module-menu.jsx index cbd2c1cf33..a910cd946c 100644 --- a/packages/app/obojobo-repository/shared/components/module-menu.jsx +++ b/packages/app/obojobo-repository/shared/components/module-menu.jsx @@ -4,6 +4,7 @@ const React = require('react') const ButtonLink = require('./button-link') const Button = require('./button') const { urlForEditor } = require('../repository-utils') +const { FULL, MINIMAL } = require('obojobo-express/server/constants') const ModuleMenu = props => { const onShare = () => { @@ -20,10 +21,12 @@ const ModuleMenu = props => { Preview - - Edit - - + {props.accessLevel !== MINIMAL && ( + + Edit + + )} + {props.accessLevel === FULL && }
-
Add or remove collaborators.
+ {props.accessLevel === FULL && ( +
+ +
Add or remove collaborators.
+
+ )} - -
View scores by student.
+
+ +
View scores by student.
+
- -
View and restore previous versions.
+ {props.accessLevel !== MINIMAL && ( +
+ +
View and restore previous versions.
+
+ )} - -
Add to or remove from private collections.
+
+ +
Add to or remove from private collections.
+
- -
Download a copy in JSON format.
+ {props.accessLevel !== MINIMAL && ( +
+ +
Download a copy in JSON format.
+
+ )} - -
Download a copy in XML format.
+ {props.accessLevel !== MINIMAL && ( +
+ +
Download a copy in XML format.
+
+ )} - - Public Page - -
Visit this modules public page.
+
+ + Public Page + +
Visit this modules public page.
+
- -
Say farewell.
+ {props.accessLevel === FULL && ( +
+ +
Say farewell.
+
+ )} @@ -95,7 +121,7 @@ class ModulePermissionsDialog extends React.Component {

Module Access

-
People who can edit this module
+
People who can access this module
diff --git a/packages/app/obojobo-repository/shared/components/module-permissions-dialog.scss b/packages/app/obojobo-repository/shared/components/module-permissions-dialog.scss index 46243b642e..2eb5914ff4 100644 --- a/packages/app/obojobo-repository/shared/components/module-permissions-dialog.scss +++ b/packages/app/obojobo-repository/shared/components/module-permissions-dialog.scss @@ -52,6 +52,11 @@ .wrapper { padding-left: 1.25em; padding-right: 1.25em; + + p { + font-size: 0.8em; + margin: 0; + } } .title { @@ -75,49 +80,78 @@ $dimensions-avatar: 1.8em; $dimensions-close-button: 1.2em; - text-align: left; + // text-align: left; height: 11.5em; - overflow-y: scroll; + overflow-y: auto; list-style: none; font-size: 0.9em; padding: 0; margin: 0; li { - max-width: 75%; + max-width: 95%; &:first-child { - margin-top: 1em; + margin-top: 0.5em; } &:last-child { - margin-bottom: 1em; + margin-bottom: 0.5em; } } } - .delete-button, - .delete-label { - margin-top: 1em; + .access-level-dropdown { + display: flex; + justify-content: center; + align-items: center; + margin: 5px; + font-size: 0.8em; + flex: 2; + + select { + font-family: 'Libre Franklin', Arial, sans-serif; + border-radius: 0.25em; + background-color: #ffffff; + padding: 0.7em 3em 0.7em 0.5em; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + + /* prettier-ignore */ + background-image: + linear-gradient( + 45deg, + transparent 50%, + gray 50% + ), + linear-gradient(135deg, gray 50%, transparent 50%); + + /* prettier-ignore */ + background-position: + calc(100% - 1.5em) 1.1em, + calc(100% - 1.1em) 1.1em, + calc(100% - 2.5em) 0.5em; + + // stylelint-disable unit-disallowed-list + background-size: 0.4em 0.4em, 0.4em 0.4em, 0.1em 1.5em; + background-repeat: no-repeat; + cursor: pointer; + position: relative; + align-self: center; + margin-left: 10px; + font-size: 0.9em; + + &:disabled { + cursor: not-allowed; + } + } } - .buttons-with-labels { - text-align: left; - margin: 2em 0; + .button-label-group { display: grid; grid-template-columns: 8em auto; - grid-gap: 0.6em; + margin: 0.6em; align-items: center; - - .label { - font-size: 0.8em; - } - - .repository--button { - min-width: 10.5em; - display: inline-block; - text-align: center; - margin-right: 1em; - } } } diff --git a/packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js b/packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js index 9053b90bda..9ce93ff56b 100644 --- a/packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js +++ b/packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js @@ -9,6 +9,8 @@ import React from 'react' import ReactModal from 'react-modal' import { create, act } from 'react-test-renderer' +import { FULL, PARTIAL, MINIMAL } from 'obojobo-express/server/constants' + import ModulePermissionsDialog from './module-permissions-dialog' import PeopleSearchDialog from './people-search-dialog-hoc' import PeopleListItem from './people-list-item' @@ -30,6 +32,7 @@ describe('ModulePermissionsDialog', () => { loadUsersForModule: jest.fn(), addUserToModule: jest.fn(), deleteModulePermissions: jest.fn(), + changeAccessLevel: jest.fn(), onClose: jest.fn() } }) @@ -209,6 +212,33 @@ describe('ModulePermissionsDialog', () => { expect(defaultProps.deleteModulePermissions).toHaveBeenCalledWith('mockDraftId', 1) }) + test('props.changeAccessLevel is called when a peopleListItem access level is changed', () => { + defaultProps.draftPermissions['mockDraftId'] = { + items: [{ id: 1, accessLevel: FULL }, { id: 99, accessLevel: MINIMAL }] + } + const reusableComponent = + let component + act(() => { + component = create(reusableComponent) + }) + + expectLoadUsersForModuleToBeCalledOnceWithId() + + const peopleListItems = component.root.findAllByType(PeopleListItem) + expect(peopleListItems.length).toBe(2) + expect(peopleListItems[0].props.id).toBe(1) + expect(peopleListItems[0].props.isMe).toBe(false) + expect(peopleListItems[1].props.id).toBe(99) + expect(peopleListItems[1].props.isMe).toBe(true) + + act(() => { + peopleListItems[0].findByType('select').props.onChange({ target: { value: PARTIAL } }) + }) + + expect(defaultProps.changeAccessLevel).toHaveBeenCalledTimes(1) + expect(defaultProps.changeAccessLevel).toHaveBeenCalledWith('mockDraftId', 1, PARTIAL) + }) + test('confirmation window denied when "x" button is clicked on peopleList item for current user', () => { window.confirm = jest.fn() window.confirm.mockReturnValue(false) diff --git a/packages/app/obojobo-repository/shared/components/module.jsx b/packages/app/obojobo-repository/shared/components/module.jsx index f365668aa1..497e205b2d 100644 --- a/packages/app/obojobo-repository/shared/components/module.jsx +++ b/packages/app/obojobo-repository/shared/components/module.jsx @@ -67,7 +67,12 @@ const Module = props => { )} {isMenuOpen && !props.isDeleted ? ( - + ) : null} {props.children}
diff --git a/packages/app/obojobo-repository/shared/components/people-list-item.jsx b/packages/app/obojobo-repository/shared/components/people-list-item.jsx index 1caae1d866..d3d1b7a991 100644 --- a/packages/app/obojobo-repository/shared/components/people-list-item.jsx +++ b/packages/app/obojobo-repository/shared/components/people-list-item.jsx @@ -3,18 +3,20 @@ require('./people-list-item.scss') const React = require('react') const Avatar = require('./avatar') -const PeopleListItem = props => ( -
  • - -
    -
    - {`${props.firstName} ${props.lastName}`} {props.isMe ? (me) : null} +const PeopleListItem = props => { + return ( +
  • + +
    +
    + {`${props.firstName} ${props.lastName}`} {props.isMe ? (me) : null} +
    +
    {props.username}
    -
    {props.username}
    - - {props.children} -
    • -) + {props.children} + + ) +} PeopleListItem.defaultProps = { firstName: '', diff --git a/packages/app/obojobo-repository/shared/components/people-list-item.scss b/packages/app/obojobo-repository/shared/components/people-list-item.scss index ee3f14d998..5b18999259 100644 --- a/packages/app/obojobo-repository/shared/components/people-list-item.scss +++ b/packages/app/obojobo-repository/shared/components/people-list-item.scss @@ -4,10 +4,12 @@ $dimensions-avatar: 1.8em; $dimensions-close-button: 1.2em; - padding: 0.3em; + display: flex; margin: 0 auto; position: relative; border-radius: 0.25em; + text-align: left; + align-items: center; &:hover { background-color: lighten($color-banner-bg, 2%); @@ -38,6 +40,7 @@ .user-info { padding-left: 2.5em; padding-right: 2.5em; + flex: 2; } > button { @@ -52,9 +55,7 @@ min-width: $dimensions-close-button; line-height: $dimensions-close-button; background-color: inherit; - position: absolute; - right: 0.5em; - top: 50%; transform: translate(0, -50%); + flex: 1; } } diff --git a/packages/obonode/obojobo-sections-assessment/server/express.js b/packages/obonode/obojobo-sections-assessment/server/express.js index b9c892a451..0fb760fd11 100644 --- a/packages/obonode/obojobo-sections-assessment/server/express.js +++ b/packages/obonode/obojobo-sections-assessment/server/express.js @@ -214,9 +214,9 @@ router .get((req, res) => { let currentUserHasPermissionToDraft - return DraftPermissions.userHasPermissionToDraft(req.currentUser.id, req.params.draftId) + return DraftPermissions.getUserAccessLevelToDraft(req.currentUser.id, req.params.draftId) .then(result => { - currentUserHasPermissionToDraft = result + currentUserHasPermissionToDraft = result !== null // Users must either have some level of permissions to this draft, or have // the canViewSystemStats permission diff --git a/packages/obonode/obojobo-sections-assessment/server/express.test.js b/packages/obonode/obojobo-sections-assessment/server/express.test.js index 74930f2e16..37e3103aa4 100644 --- a/packages/obonode/obojobo-sections-assessment/server/express.test.js +++ b/packages/obonode/obojobo-sections-assessment/server/express.test.js @@ -42,6 +42,7 @@ const assessmentExpress = require('./express') const express_response_decorator = require('obojobo-express/server/express_response_decorator') const express = require('express') const { ERROR_INVALID_ATTEMPT_END, ERROR_INVALID_ATTEMPT_RESUME } = require('./error-constants') +const { FULL } = require('obojobo-express/server/constants') const DraftPermissions = require('../../../app/obojobo-repository/server/models/draft_permissions') let app @@ -488,7 +489,7 @@ describe('server/express', () => { } next() }) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) AssessmentModel.fetchAttemptHistoryDetails.mockResolvedValueOnce(mockReturnValue) const response = await request(app) @@ -530,7 +531,7 @@ describe('server/express', () => { } next() }) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) AssessmentModel.fetchAttemptHistoryDetails.mockResolvedValueOnce(mockReturnValue) const response = await request(app) @@ -587,7 +588,7 @@ describe('server/express', () => { } next() }) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(null) AssessmentModel.fetchAttemptHistoryDetails.mockResolvedValueOnce(mockReturnValue) const response = await request(app) @@ -642,7 +643,7 @@ describe('server/express', () => { } next() }) - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(false) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(null) AssessmentModel.fetchAttemptHistoryDetails.mockResolvedValueOnce(mockReturnValue) const response = await request(app) @@ -662,7 +663,7 @@ describe('server/express', () => { test('GET /api/assessments/:draftId/details fails', async () => { expect.hasAssertions() - DraftPermissions.userHasPermissionToDraft.mockResolvedValueOnce(true) + DraftPermissions.getUserAccessLevelToDraft.mockResolvedValueOnce(FULL) AssessmentModel.fetchAttemptHistoryDetails.mockRejectedValueOnce(Error('Oops!')) const response = await request(app)