Skip to content

Commit

Permalink
Merge pull request #2074 from FrenjaminBanklin/issue/2005-read-only-c…
Browse files Browse the repository at this point in the history
…opies

Add optional read-only mode for copied modules and optional update auto-sync for read-only copies.
  • Loading branch information
FrenjaminBanklin authored Aug 23, 2023
2 parents efa2060 + d4b7bf2 commit 7112d66
Show file tree
Hide file tree
Showing 50 changed files with 3,307 additions and 266 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,33 @@ describe('EditorAPI', () => {
expect(res).toBe(mockJsonResult)
})

test('copyDraft fetches with the correct args', async () => {
test('copyDraft fetches with the correct args - allow default readOnly', async () => {
const res = await EditorAPI.copyDraft('draft-id', 'new-title')

expect(post).toHaveBeenCalledWith('/api/drafts/draft-id/copy', { title: 'new-title' })
expect(post).toHaveBeenCalledWith('/api/drafts/draft-id/copy', {
title: 'new-title',
readOnly: false
})
expect(res).toBe(mockJsonResult)
})

test('copyDraft fetches with the correct args - readOnly true', async () => {
const res = await EditorAPI.copyDraft('draft-id', 'new-title', true)

expect(post).toHaveBeenCalledWith('/api/drafts/draft-id/copy', {
title: 'new-title',
readOnly: true
})
expect(res).toBe(mockJsonResult)
})

test('copyDraft fetches with the correct args - readOnly false', async () => {
const res = await EditorAPI.copyDraft('draft-id', 'new-title', false)

expect(post).toHaveBeenCalledWith('/api/drafts/draft-id/copy', {
title: 'new-title',
readOnly: false
})
expect(res).toBe(mockJsonResult)
})

Expand Down Expand Up @@ -147,7 +170,10 @@ describe('EditorAPI', () => {
expect.hasAssertions()

return EditorAPI.copyDraft('mock-draft-id', 'new title').then(result => {
expect(post).toHaveBeenCalledWith('/api/drafts/mock-draft-id/copy', { title: 'new title' })
expect(post).toHaveBeenCalledWith('/api/drafts/mock-draft-id/copy', {
title: 'new title',
readOnly: false
})
expect(result).toEqual(mockJsonResult)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

exports[`File Menu File Menu node 1`] = `
"<div class=\\"visual-editor--drop-down-menu\\"><div class=\\"dropdown is-not-open\\" tabindex=\\"-1\\"><button class=\\"menu-title\\">File</button><div class=\\"menu-items\\"><button>Save Module<span>
CTRL+S</span></button><button>New</button><button>Make a copy...</button><div class=\\"dropdown is-not-open\\" tabindex=\\"-1\\"><button class=\\"menu-title\\">Download</button><div class=\\"menu-items\\"><button>XML Document (.xml)</button><button>JSON Document (.json)</button></div></div><button>Import from file...</button><button disabled=\\"\\">Delete Module...</button><button>Copy LTI Link</button></div></div></div>"
CTRL+S</span></button><button>New</button><button>Make a copy...</button><button>Make a read-only copy...</button><div class=\\"dropdown is-not-open\\" tabindex=\\"-1\\"><button class=\\"menu-title\\">Download</button><div class=\\"menu-items\\"><button>XML Document (.xml)</button><button>JSON Document (.json)</button></div></div><button>Import from file...</button><button disabled=\\"\\">Delete Module...</button><button>Copy LTI Link</button></div></div></div>"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,20 @@ describe('File Menu', () => {
expect(ModalUtil.show).toHaveBeenCalled()
})

test('FileMenu calls Copy (Read-Only)', () => {
const model = {
title: 'mockTitle'
}

const component = mount(<FileMenu draftId="mockDraft" model={model} />)

component
.findWhere(n => n.type() === 'button' && n.html().includes('Make a read-only copy...'))
.simulate('click')

expect(ModalUtil.show).toHaveBeenCalled()
})

test('FileMenu calls Download', done => {
// setup
const model = {
Expand Down Expand Up @@ -243,7 +257,49 @@ describe('File Menu', () => {
.instance()
.copyModule('new title')
.then(() => {
expect(EditorAPI.copyDraft).toHaveBeenCalledWith('mockDraftId', 'new title')
expect(EditorAPI.copyDraft).toHaveBeenCalledWith('mockDraftId', 'new title', false)
})
})

test('copyModuleReadOnly calls copyDraft api', () => {
expect.hasAssertions()
const model = {
flatJSON: () => ({ children: [] }),
children: [
{
get: () => CONTENT_NODE,
flatJSON: () => ({ children: [] }),
children: { models: [{ get: () => 'mockValue' }] }
},
{
get: () => ASSESSMENT_NODE
}
]
}

const exportToJSON = jest.fn()

const component = mount(
<FileMenu
draftId="mockDraftId"
model={model}
exportToJSON={exportToJSON}
onSave={jest.fn()}
/>
)

EditorAPI.copyDraft.mockResolvedValueOnce({
status: 'ok',
value: {
draftId: 'new-copy-draft-id'
}
})

return component
.instance()
.copyModuleReadOnly('new title')
.then(() => {
expect(EditorAPI.copyDraft).toHaveBeenCalledWith('mockDraftId', 'new title', true)
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class FileMenu extends React.PureComponent {
constructor(props) {
super(props)
this.copyModule = this.copyModule.bind(this)
this.copyModuleReadOnly = this.copyModuleReadOnly.bind(this)
this.deleteModule = this.deleteModule.bind(this)
this.buildFileSelector = this.buildFileSelector.bind(this)
}
Expand All @@ -27,13 +28,37 @@ class FileMenu extends React.PureComponent {
})
}

copyModule(newTitle) {
copyModule(newTitle, readOnly = false) {
ModalUtil.hide()
return EditorAPI.copyDraft(this.props.draftId, newTitle).then(result => {
window.open(window.location.origin + '/editor/visual/' + result.value.draftId, '_blank')
return EditorAPI.copyDraft(this.props.draftId, newTitle, readOnly).then(result => {
if (readOnly) {
const buttons = [
{
value: 'OK',
onClick: ModalUtil.hide,
default: true
}
]
ModalUtil.show(
<Dialog buttons={buttons} title="Read-Only Copy Created">
A read-only copy of this module has been created.
<br />
Read-only copies can not be edited directly.
<br />
View the read-only copy in the dashboard to optionally synchronize any edits made to
this module.
</Dialog>
)
} else {
window.open(window.location.origin + '/editor/visual/' + result.value.draftId, '_blank')
}
})
}

copyModuleReadOnly(newTitle) {
return this.copyModule(newTitle, true)
}

processFileContent(id, content, type) {
EditorAPI.postDraft(
id,
Expand Down Expand Up @@ -101,6 +126,19 @@ class FileMenu extends React.PureComponent {
/>
)
},
{
name: 'Make a read-only copy...',
type: 'action',
action: () =>
ModalUtil.show(
<Prompt
title="Copy Module (Read-Only)"
message="Enter the title for the copied module:"
value={this.props.title + ' - Copy'}
onConfirm={this.copyModuleReadOnly}
/>
)
},
{
name: 'Download',
type: 'sub-menu',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ const EditorAPI = {
return API.delete(`/api/drafts/${draftId}`).then(API.processJsonResults)
},

copyDraft(draftId, newTitle) {
return API.post(`/api/drafts/${draftId}/copy`, { title: newTitle }).then(API.processJsonResults)
copyDraft(draftId, newTitle, readOnly = false) {
return API.post(`/api/drafts/${draftId}/copy`, { title: newTitle, readOnly }).then(
API.processJsonResults
)
},

requestEditLock(draftId, contentId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ const documentFunctions = ['setCurrentDocument', 'requireCurrentDocument', 'rese

jest.mock('test_node')
jest.mock('../server/models/draft')
jest.mock('obojobo-repository/server/models/drafts_metadata')

const DraftDocument = oboRequire('server/models/draft')
const DraftsMetadata = require('obojobo-repository/server/models/drafts_metadata')

describe('current document middleware', () => {
beforeAll(() => {})
Expand Down Expand Up @@ -32,6 +34,7 @@ describe('current document middleware', () => {
mockNext.mockClear()
mockStatus.mockClear()
mockJson.mockClear()
DraftsMetadata.getByDraftIdAndKey.mockClear()
})

test('calls next', () => {
Expand Down Expand Up @@ -168,4 +171,58 @@ describe('current document middleware', () => {
done()
})
})

test('requireDraftWritable rejects when no draftId is available', done => {
expect.assertions(1)

return mockArgs.req
.requireDraftWritable()
.then(() => {
expect(false).toBe('never_called')
done()
})
.catch(err => {
expect(err.message).toBe('DraftDocument Required')
done()
})
})

test('requireDraftWritable rejects when corresponding draft is read-only', done => {
expect.assertions(3)
DraftsMetadata.getByDraftIdAndKey.mockResolvedValue(true)

const { req } = mockArgs
req.params = {
draftId: 1
}

return req
.requireDraftWritable()
.then(() => {
expect(false).toBe('never_called')
done()
})
.catch(err => {
expect(DraftsMetadata.getByDraftIdAndKey).toHaveBeenCalledTimes(1)
expect(DraftsMetadata.getByDraftIdAndKey).toHaveBeenCalledWith(1, 'read_only')
expect(err.message).toBe('Requested document is read-only')
done()
})
})

test('requireDraftWritable resolves when corresponding draft is not read-only', done => {
expect.assertions(2)
DraftsMetadata.getByDraftIdAndKey.mockResolvedValue(false)

const { req } = mockArgs
req.params = {
draftId: 1
}

return req.requireDraftWritable().then(() => {
expect(DraftsMetadata.getByDraftIdAndKey).toHaveBeenCalledTimes(1)
expect(DraftsMetadata.getByDraftIdAndKey).toHaveBeenCalledWith(1, 'read_only')
done()
})
})
})
31 changes: 31 additions & 0 deletions packages/app/obojobo-express/__tests__/express_validators.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

jest.mock('test_node')
jest.mock('../server/models/user')
jest.mock('../server/logger')

let mockRes
let mockReq
Expand All @@ -10,11 +11,14 @@ let mockUser
let mockDocument

const Validators = oboRequire('server/express_validators')
const logger = oboRequire('server/logger')

describe('current user middleware', () => {
beforeAll(() => {})
afterAll(() => {})
beforeEach(() => {
logger.error.mockClear()

mockUser = {
id: 1,
username: 'mock-user',
Expand Down Expand Up @@ -163,6 +167,33 @@ describe('current user middleware', () => {
})
})

// requireDraftWritable tests

test('requireDraftWritable resolves', () => {
mockReq.requireDraftWritable = jest.fn().mockResolvedValue()

return expect(
Validators.requireDraftWritable(mockReq, mockRes, mockNext)
).resolves.toBeUndefined()
})

test('requireDraftWritable calls next when valid', () => {
mockReq.requireDraftWritable = jest.fn().mockResolvedValue()

return Validators.requireDraftWritable(mockReq, mockRes, mockNext).then(() => {
expect(mockNext).toHaveBeenCalled()
})
})

test('requireDraftWritable calls missing when invalid', () => {
mockReq.requireDraftWritable = jest.fn().mockRejectedValue()

return Validators.requireDraftWritable(mockReq, mockRes, mockNext).then(() => {
expect(logger.error).toHaveBeenCalledWith('User tried editing read-only module')
expect(mockRes.missing).toHaveBeenCalled()
})
})

// requireDraftId tests

test('requireDraftId resolves in body', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/app/obojobo-express/__tests__/routes/editor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ jest.mock('../../server/express_current_document', () => (req, res, next) => {
req.currentDocument = mockCurrentDocument
return Promise.resolve(mockCurrentDocument)
}
req.requireDraftWritable = () => {
return Promise.resolve(true)
}
next()
})

Expand Down
Loading

0 comments on commit 7112d66

Please sign in to comment.