Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Added feature to allow authors to restore deleted modules #1916

23 changes: 23 additions & 0 deletions packages/app/obojobo-express/__tests__/models/draft.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,29 @@ describe('Draft Model', () => {
return expect(DraftModel.deleteByIdAndUser('draft_id', 'user_id')).rejects.toBe('mock-error')
})

test('restoreByIdAndUser reverts delete flag', () => {
expect.hasAssertions()
const oboEvents = require('../../server/obo_events')

db.none.mockResolvedValueOnce()

return DraftModel.restoreByIdAndUser('draft_id', 'user_id').then(voidResult => {
expect(voidResult).toBe(undefined) //eslint-disable-line no-undefined
expect(oboEvents.emit).toHaveBeenCalledWith('EVENT_DRAFT_RESTORED', {
id: 'draft_id',
userId: 'user_id'
})
})
})

test('restoreByIdAndUser fails as expected', () => {
expect.hasAssertions()

db.none.mockRejectedValueOnce('mock-error')

return expect(DraftModel.restoreByIdAndUser('draft_id', 'user_id')).rejects.toBe('mock-error')
})

test('getChildNodesByType returns all nodes with a matching type', () => {
expect.hasAssertions()

Expand Down
16 changes: 16 additions & 0 deletions packages/app/obojobo-express/__tests__/routes/api/drafts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -867,4 +867,20 @@ describe('api draft route', () => {
expect(response.body.value).toHaveProperty('type', 'unexpected')
})
})

// restore draft

test('restore draft returns successfully', () => {
expect.assertions(4)
DraftModel.restoreByIdAndUser.mockResolvedValueOnce('mock-db-result')
mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canDeleteDrafts' } // mock current logged in user
return request(app)
.put('/api/drafts/restore/00000000-0000-0000-0000-000000000000')
.then(response => {
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-db-result')
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ MockDraft.createWithContent = jest
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.mockGetChildNodeById = mockGetChildNodeById
Expand Down
26 changes: 25 additions & 1 deletion packages/app/obojobo-express/server/models/draft.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,30 @@ class Draft {
oboEvents.emit(Draft.EVENT_DRAFT_DELETED, { id, userId })
})
.catch(error => {
logger.logError('Draft fetchById Error', error)
logger.logError('Draft deleteByIdAndUser Error', error)
throw error
})
}

static restoreByIdAndUser(id, userId) {
return db
.none(
`
UPDATE drafts
SET deleted = FALSE
WHERE id = $[id]
AND user_id = $[userId]
`,
{
id,
userId
}
)
.then(() => {
oboEvents.emit(Draft.EVENT_DRAFT_RESTORED, { id, userId })
})
.catch(error => {
logger.logError('Draft restoreByIdAndUser Error', error)
throw error
})
}
Expand Down Expand Up @@ -293,5 +316,6 @@ class Draft {
Draft.EVENT_NEW_DRAFT_CREATED = 'EVENT_NEW_DRAFT_CREATED'
Draft.EVENT_DRAFT_DELETED = 'EVENT_DRAFT_DELETED'
Draft.EVENT_DRAFT_UPDATED = 'EVENT_DRAFT_UPDATED'
Draft.EVENT_DRAFT_RESTORED = 'EVENT_DRAFT_RESTORED'

module.exports = Draft
2 changes: 2 additions & 0 deletions packages/app/obojobo-express/server/obo_express.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mockRouter.all = jest.fn().mockReturnValue(mockRouter)
mockRouter.get = jest.fn().mockReturnValue(mockRouter)
mockRouter.post = jest.fn().mockReturnValue(mockRouter)
mockRouter.delete = jest.fn().mockReturnValue(mockRouter)
mockRouter.put = jest.fn().mockReturnValue(mockRouter)

const mockExpress = (mockOn = false, mockStatic = false) => {
jest.mock(
Expand All @@ -39,6 +40,7 @@ const mockExpress = (mockOn = false, mockStatic = false) => {
get: mockRouter.get,
post: mockRouter.post,
delete: mockRouter.delete,
put: mockRouter.put,
static: mockStatic ? mockStatic : jest.fn()
})
module.Router = () => mockRouter
Expand Down
11 changes: 11 additions & 0 deletions packages/app/obojobo-express/server/routes/api/drafts.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,4 +223,15 @@ router
.catch(res.unexpected)
})

// Restore a Draft
// mounted as /api/drafts/restore/:draftId
router
.route('/restore/:draftId')
.put([requireCanDeleteDrafts, requireDraftId, checkValidationRules])
.put((req, res) => {
return DraftModel.restoreByIdAndUser(req.params.draftId, req.currentUser.id)
.then(res.success)
.catch(res.unexpected)
})

module.exports = router
17 changes: 0 additions & 17 deletions packages/app/obojobo-repository/server/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,6 @@ oboEvents.on(DraftModel.EVENT_NEW_DRAFT_CREATED, newDraft => {
.catch(logger.error)
})

// when a draft is deleted, remove all of it's permissions
oboEvents.on(DraftModel.EVENT_DRAFT_DELETED, draft => {
// remove all permissions for all deleted drafts
return db
.none(
`
DELETE FROM repository_map_user_to_draft
USING drafts
WHERE
drafts.id = repository_map_user_to_draft.draft_id
AND
drafts.deleted = TRUE`
)
.then(logger.info(`User ${draft.userId} deleted module ${draft.id}`))
.catch(logger.error)
})

// 401 Error Page
oboEvents.on('HTTP_NOT_AUTHORIZED', ({ req, res, next }) => {
req.responseHandled = true
Expand Down
23 changes: 5 additions & 18 deletions packages/app/obojobo-repository/server/events.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('Server Events', () => {

require('./events')

expect(oboEvents.on).toHaveBeenCalledTimes(5)
expect(oboEvents.on).toHaveBeenCalledTimes(4)
expect(oboEvents.on).toHaveBeenCalledWith('HTTP_NOT_AUTHORIZED', expect.any(Function))
expect(oboEvents.on).toHaveBeenCalledWith('HTTP_NOT_FOUND', expect.any(Function))
expect(oboEvents.on).toHaveBeenCalledWith('HTTP_UNEXPECTED', expect.any(Function))
Expand All @@ -36,7 +36,7 @@ describe('Server Events', () => {
expect(oboEvents.on).toHaveBeenCalledWith(DraftModel.EVENT_DRAFT_DELETED, expect.any(Function))
})

test('maps a user to a module when its created', () => {
test("maps a user to a module when it's created", () => {
// verify we have the right callback
const [eventName, newDraftListener] = oboEvents.on.mock.calls[0]
expect(eventName).toBe(DraftModel.EVENT_NEW_DRAFT_CREATED)
Expand All @@ -49,22 +49,9 @@ describe('Server Events', () => {
expect(db.none.mock.calls[0][0]).toContain('INSERT INTO repository_map_user_to_draft')
})

test('cleans ownership when a draft is deleted', () => {
// verify we have the right callback
const [eventName, deleteDraftListener] = oboEvents.on.mock.calls[1]
expect(eventName).toBe(DraftModel.EVENT_DRAFT_DELETED)
expect(deleteDraftListener.length).toBe(1) // callback function arguments
expect(db.none).toHaveBeenCalledTimes(0)

// call the callback
deleteDraftListener({ id: 2, userId: 3 })
expect(db.none).toHaveBeenCalledTimes(1)
expect(db.none.mock.calls[0][0]).toContain('DELETE FROM repository_map_user_to_draft')
})

test('HTTP_NOT_AUTHORIZED events render a page', () => {
// verify we have the right callback
const [eventName, notAuthorizedListener] = oboEvents.on.mock.calls[2]
const [eventName, notAuthorizedListener] = oboEvents.on.mock.calls[1]
expect(eventName).toBe('HTTP_NOT_AUTHORIZED')
expect(notAuthorizedListener.length).toBe(1) // callback function arguments

Expand All @@ -86,7 +73,7 @@ describe('Server Events', () => {

test('HTTP_NOT_FOUND events render a page', () => {
// verify we have the right callback
const [eventName, notFoundListener] = oboEvents.on.mock.calls[3]
const [eventName, notFoundListener] = oboEvents.on.mock.calls[2]
expect(eventName).toBe('HTTP_NOT_FOUND')
expect(notFoundListener.length).toBe(1) // callback function arguments

Expand All @@ -108,7 +95,7 @@ describe('Server Events', () => {

test('HTTP_UNEXPECTED events render a page', () => {
// verify we have the right callback
const [eventName, unexpectedListener] = oboEvents.on.mock.calls[4]
const [eventName, unexpectedListener] = oboEvents.on.mock.calls[3]
expect(eventName).toBe('HTTP_UNEXPECTED')
expect(unexpectedListener.length).toBe(1) // callback function arguments

Expand Down
17 changes: 14 additions & 3 deletions packages/app/obojobo-repository/server/models/draft_summary.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const db = require('obojobo-express/server/db')
const logger = require('obojobo-express/server/logger')

const buildQueryWhere = (whereSQL, joinSQL = '') => {
const buildQueryWhere = (whereSQL, joinSQL = '', deleted = 'FALSE') => {
return `
SELECT
DISTINCT drafts_content.draft_id AS draft_id,
Expand All @@ -16,7 +16,7 @@ const buildQueryWhere = (whereSQL, joinSQL = '') => {
JOIN drafts_content
ON drafts_content.draft_id = drafts.id
${joinSQL}
WHERE drafts.deleted = FALSE
WHERE drafts.deleted = ${deleted}
AND ${whereSQL}
WINDOW wnd AS (
PARTITION BY drafts_content.draft_id ORDER BY drafts_content.created_at
Expand Down Expand Up @@ -82,9 +82,20 @@ class DraftSummary {
)
}

static fetchDeletedByUserId(userId) {
return DraftSummary.fetchAndJoinWhere(
`JOIN repository_map_user_to_draft
ON repository_map_user_to_draft.draft_id = drafts.id`,
`repository_map_user_to_draft.user_id = $[userId]`,
{ userId, deleted: 'TRUE' }
)
}

static fetchAndJoinWhere(joinSQL, whereSQL, queryValues) {
const query = buildQueryWhere(whereSQL, joinSQL, queryValues.deleted)

return db
.any(buildQueryWhere(whereSQL, joinSQL), queryValues)
.any(query, queryValues)
.then(DraftSummary.resultsToObjects)
.catch(error => {
throw logger.logError('Error loading DraftSummary by query', error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('DraftSummary Model', () => {
//NOTE:
// 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 = '') =>
const queryBuilder = (whereSQL, joinSQL = '', deleted = 'FALSE') =>
`
SELECT
DISTINCT drafts_content.draft_id AS draft_id,
Expand All @@ -94,7 +94,7 @@ describe('DraftSummary Model', () => {
JOIN drafts_content
ON drafts_content.draft_id = drafts.id
${joinSQL}
WHERE drafts.deleted = FALSE
WHERE drafts.deleted = ${deleted}
AND ${whereSQL}
WINDOW wnd AS (
PARTITION BY drafts_content.draft_id ORDER BY drafts_content.created_at
Expand Down Expand Up @@ -505,4 +505,21 @@ describe('DraftSummary Model', () => {
expect(summaries[1].draftId).toBe('mockDraftId2')
expect(summaries[2].draftId).toBe('mockDraftId3')
})

test('fetchDeletedByUserId generates correct query and retrieves deleted drafts', () => {
expect.hasAssertions()

db.any = jest.fn()
db.any.mockResolvedValueOnce(mockRawDraftSummary)

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, 'TRUE')

return DraftSummary.fetchDeletedByUserId(0).then(summary => {
expect(db.any).toHaveBeenCalledWith(query, { userId: 0, deleted: 'TRUE' })
expectIsMockSummary(summary)
})
})
})
9 changes: 9 additions & 0 deletions packages/app/obojobo-repository/server/routes/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ router
.catch(res.unexpected)
})

router
.route('/drafts-deleted')
.get([requireCurrentUser, requireCanPreviewDrafts])
.get((req, res) => {
return DraftSummary.fetchDeletedByUserId(req.currentUser.id)
.then(res.success)
.catch(res.unexpected)
})

router
.route('/drafts/:draftId/revisions')
.get([
Expand Down
21 changes: 21 additions & 0 deletions packages/app/obojobo-repository/server/routes/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,27 @@ describe('repository api route', () => {
})
})

test('get /drafts-deleted returns the expected response', () => {
const mockResult = [
{ draftId: 'mockDraftId1', title: 'whatever1' },
{ draftId: 'mockDraftId2', title: 'whatever2' },
{ draftId: 'mockDraftId3', title: 'whatever3' }
]

DraftSummary.fetchDeletedByUserId = jest.fn()
DraftSummary.fetchDeletedByUserId.mockResolvedValueOnce(mockResult)

expect.hasAssertions()

return request(app)
.get('/drafts-deleted')
.then(response => {
expect(DraftSummary.fetchDeletedByUserId).toHaveBeenCalled()
expect(response.statusCode).toBe(200)
expect(response.body).toEqual(mockResult)
})
})

test('get /drafts/:draftId/revisions/ returns the expected response, no after - hasMoreResults false', () => {
const mockResult = {
revisions: [
Expand Down
Loading