Merge pull request #1916 from maufcost/issue/1852-restore-modules

Added feature to allow authors to restore deleted modules

packages/app/obojobo-express/__tests__/models/draft.test.js

@@ -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()
 

packages/app/obojobo-express/__tests__/routes/api/drafts.test.js

@@ -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')
+			})
+	})
 })

packages/app/obojobo-express/server/models/__mocks__/draft.js

@@ -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

packages/app/obojobo-express/server/models/draft.js

@@ -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
 			})
 	}
@@ -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

packages/app/obojobo-express/server/obo_express.test.js

@@ -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(
@@ -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

packages/app/obojobo-express/server/routes/api/drafts.js

@@ -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

packages/app/obojobo-repository/server/events.js

@@ -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

packages/app/obojobo-repository/server/events.test.js

@@ -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))
@@ -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)
@@ -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
 
@@ -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
 
@@ -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
 

packages/app/obojobo-repository/server/models/draft_summary.js

@@ -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,
@@ -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
@@ -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)

packages/app/obojobo-repository/server/models/draft_summary.test.js

@@ -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,
@@ -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
@@ -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)
+		})
+	})
 })

packages/app/obojobo-repository/server/routes/api.js

@@ -55,6 +55,15 @@ router
 		}
 	})
 
+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([

packages/app/obojobo-repository/server/routes/api.test.js

@@ -213,6 +213,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: [