diff --git a/packages/app/obojobo-express/__tests__/express_current_document.test.js b/packages/app/obojobo-express/__tests__/express_current_document.test.js index 11c07818a5..09661f9e32 100644 --- a/packages/app/obojobo-express/__tests__/express_current_document.test.js +++ b/packages/app/obojobo-express/__tests__/express_current_document.test.js @@ -7,6 +7,8 @@ jest.mock('../server/models/draft') const DraftDocument = oboRequire('server/models/draft') describe('current document middleware', () => { + const standardPartialMockUser = { createdAt: Date.now() } + beforeAll(() => {}) afterAll(() => {}) beforeEach(() => { @@ -105,7 +107,7 @@ describe('current document middleware', () => { }) }) - test('requireCurrentDocument loads the draft document from params when one is avalible', done => { + test('requireCurrentDocument loads a draft document from params when one is available', done => { expect.assertions(2) const { req } = mockArgs @@ -120,7 +122,7 @@ describe('current document middleware', () => { }) }) - test('requireCurrentDocument loads the draft document from body when one is avalible', done => { + test('requireCurrentDocument loads a draft document from body when one is available', done => { expect.assertions(2) const { req } = mockArgs @@ -135,7 +137,7 @@ describe('current document middleware', () => { }) }) - test('requireCurrentDocument loads the draft document from body.event when one is avalible', done => { + test('requireCurrentDocument loads a draft document from body.event when one is available', done => { expect.assertions(2) const { req } = mockArgs @@ -152,6 +154,122 @@ describe('current document middleware', () => { }) }) + test('requireCurrentDocument loads a draft document from params when multiple are available', done => { + expect.assertions(2) + const mockDocument = new DraftDocument({ + draftId: 'mockDraftId1', + contentId: 'mockContentId' + }) + + // The actual result of this will depend on the current user's creation timestamp, + // but for simplicity of this test we'll just mock the result + DraftDocument.fetchById = jest.fn().mockResolvedValue(mockDocument) + + const { req } = mockArgs + req.params = { + draftA: 'mockDraftId1', + draftB: 'mockDraftId2' + } + req.currentUser = standardPartialMockUser + + return req.requireCurrentDocument().then(draftDocument => { + expect(draftDocument.draftId).toBe('mockDraftId1') + expect(draftDocument).toBeInstanceOf(DraftDocument) + done() + }) + }) + + test('requireCurrentDocument loads a draft document from body when multiple are available', done => { + expect.assertions(2) + const mockDocument = new DraftDocument({ + draftId: 'mockDraftId1', + contentId: 'mockContentId' + }) + + // The actual result of this will depend on the current user's creation timestamp, + // but for simplicity of this test we'll just mock the result + DraftDocument.fetchById = jest.fn().mockResolvedValue(mockDocument) + + const { req } = mockArgs + req.body = { + draftA: 'mockDraftId1', + draftB: 'mockDraftId2' + } + req.currentUser = standardPartialMockUser + + return req.requireCurrentDocument().then(draftDocument => { + expect(draftDocument.draftId).toBe('mockDraftId1') + expect(draftDocument).toBeInstanceOf(DraftDocument) + done() + }) + }) + + test('requireCurrentDocument loads a draft document from body.event when multiple are available', done => { + expect.assertions(2) + const mockDocument = new DraftDocument({ + draftId: 'mockDraftId1', + contentId: 'mockContentId' + }) + + // The actual result of this will depend on the current user's creation timestamp, + // but for simplicity of this test we'll just mock the result + DraftDocument.fetchById = jest.fn().mockResolvedValue(mockDocument) + + const { req } = mockArgs + req.body = { + event: { + draftA: 'mockDraftId1', + draftB: 'mockDraftId2' + } + } + req.currentUser = standardPartialMockUser + + return req.requireCurrentDocument().then(draftDocument => { + expect(draftDocument.draftId).toBe('mockDraftId1') + expect(draftDocument).toBeInstanceOf(DraftDocument) + done() + }) + }) + + // This may be subject to change in the future + // Currently, the module chosen in a split-run event is determined by the user's creation date + // Even numbers will go to draftA, odd numbers will go to draftB + test('DraftDocument.fetchById is provided the correct values in split-run requests based on user creation date', done => { + const { req } = mockArgs + + const expectedEvenDraftId = 'mockDraftId1' + const expectedOddDraftId = 'mockDraftId2' + + req.body = { + draftA: expectedEvenDraftId, + draftB: expectedOddDraftId + } + + // Start by forcing a creation date that ends in an odd number + let mockCreationDate = Date.now() + if (mockCreationDate % 2 === 0) mockCreationDate++ + + req.currentUser = { createdAt: mockCreationDate } + + return req + .requireCurrentDocument() + .then(() => { + expect(DraftDocument.fetchById).toBeCalledWith(expectedOddDraftId) + DraftDocument.fetchById.mockClear() + // Change the user's creation date to an even number - should be odd already + mockCreationDate++ + req.currentUser = { createdAt: mockCreationDate } + // Remove the currentDocument so the full process runs again + req.currentDocument = null + // return req.requireCurrentDocument() + }) + .then(req.requireCurrentDocument) + .then(() => { + expect(DraftDocument.fetchById).toBeCalledWith(expectedEvenDraftId) + done() + }) + }) + test('requireCurrentDocument rejects when no DraftDocument is set', done => { expect.assertions(1) 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 88f36509cd..c5fdf4079e 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 @@ -11,7 +11,7 @@ Array [ first_name = $[firstName], last_name = $[lastName], roles = $[roles] - RETURNING id + RETURNING id, created_at ", Object { "accessLevel": undefined, @@ -70,7 +70,7 @@ Array [ first_name = $[firstName], last_name = $[lastName], roles = $[roles] - RETURNING id + RETURNING id, created_at ", Object { "accessLevel": undefined, diff --git a/packages/app/obojobo-express/__tests__/models/user.test.js b/packages/app/obojobo-express/__tests__/models/user.test.js index d138fff5bf..3e4a7a5e41 100644 --- a/packages/app/obojobo-express/__tests__/models/user.test.js +++ b/packages/app/obojobo-express/__tests__/models/user.test.js @@ -8,6 +8,8 @@ const User = require('../../server/models/user') const db = require('../../server/db') const oboEvents = require('../../server/obo_events') +const nowForTests = Date.now() + describe('user model', () => { beforeAll(() => { Date.now = () => 'mockNowDate' @@ -137,7 +139,7 @@ describe('user model', () => { test('creates a new user', () => { expect.hasAssertions() - db.one.mockResolvedValueOnce({ id: 3 }) + db.one.mockResolvedValueOnce({ id: 3, created_at: nowForTests }) const u = new User({ firstName: 'Roger', @@ -149,6 +151,7 @@ describe('user model', () => { return u.saveOrCreate().then(user => { expect(user).toBeInstanceOf(User) expect(user.id).toBe(3) + expect(user.createdAt).toBe(nowForTests) expect(db.one).toHaveBeenCalledTimes(1) expect(db.one.mock.calls[0]).toMatchSnapshot() expect(oboEvents.emit).toHaveBeenCalledTimes(1) @@ -159,7 +162,7 @@ describe('user model', () => { test('saves an existing user', () => { expect.hasAssertions() - db.one.mockResolvedValueOnce({ id: 10 }) + db.one.mockResolvedValueOnce({ id: 10, created_at: nowForTests }) const u = new User({ id: 10, @@ -172,6 +175,7 @@ describe('user model', () => { return u.saveOrCreate().then(user => { expect(user).toBeInstanceOf(User) expect(user.id).toBe(10) + expect(user.createdAt).toBe(nowForTests) expect(db.one).toHaveBeenCalledTimes(1) expect(db.one.mock.calls[0]).toMatchSnapshot() expect(oboEvents.emit).toHaveBeenCalledTimes(1) diff --git a/packages/app/obojobo-express/__tests__/routes/__snapshots__/view-split.test.js.snap b/packages/app/obojobo-express/__tests__/routes/__snapshots__/view-split.test.js.snap new file mode 100644 index 0000000000..4cddec3a86 --- /dev/null +++ b/packages/app/obojobo-express/__tests__/routes/__snapshots__/view-split.test.js.snap @@ -0,0 +1,45 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`view-split route launch visit allows EVENT_BEFORE_NEW_VISIT to alter req 1`] = ` +Array [ + Object { + "action": "visit:create", + "actorTime": "2016-09-22T16:57:14.500Z", + "contentId": undefined, + "draftId": "00000000-0000-0000-0000-000000000000", + "eventVersion": "1.1.0", + "ip": "::ffff:127.0.0.1", + "isPreview": false, + "metadata": Object {}, + "payload": Object { + "deactivatedVisitId": "mocked-deactivated-visit-id", + "visitId": "mocked-visit-id", + }, + "resourceLinkId": 3, + "userId": 4, + "visitId": "mocked-visit-id", + }, +] +`; + +exports[`view-split route launch visit inserts event \`visit:create\` 1`] = ` +Array [ + Object { + "action": "visit:create", + "actorTime": "2016-09-22T16:57:14.500Z", + "contentId": undefined, + "draftId": "00000000-0000-0000-0000-000000000000", + "eventVersion": "1.1.0", + "ip": "::ffff:127.0.0.1", + "isPreview": false, + "metadata": Object {}, + "payload": Object { + "deactivatedVisitId": "mocked-deactivated-visit-id", + "visitId": "mocked-visit-id", + }, + "resourceLinkId": 3, + "userId": 4, + "visitId": "mocked-visit-id", + }, +] +`; diff --git a/packages/app/obojobo-express/__tests__/routes/view-split.test.js b/packages/app/obojobo-express/__tests__/routes/view-split.test.js new file mode 100644 index 0000000000..cd35e23383 --- /dev/null +++ b/packages/app/obojobo-express/__tests__/routes/view-split.test.js @@ -0,0 +1,257 @@ +jest.mock('../../server/models/visit') +jest.mock('../../server/insert_event') +jest.mock('../../server/obo_events') +jest.mock( + '../../server/asset_resolver', + () => ({ + assetForEnv: path => path, + webpackAssetPath: path => path + }), + { virtual: true } +) +// make sure all Date objects use a static date +mockStaticDate() + +jest.mock('../../server/db') +jest.unmock('fs') // need fs working for view rendering +jest.unmock('express') // we'll use supertest + express for this + +// override requireCurrentUser to provide our own +let mockCurrentUser +let mockCurrentVisit +let mockSaveSessionSuccess = true +jest.mock('../../server/express_current_user', () => (req, res, next) => { + req.requireCurrentUser = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + req.saveSessionPromise = () => { + if (mockSaveSessionSuccess) return Promise.resolve() + return Promise.reject() + } + req.getCurrentVisitFromRequest = () => { + req.currentVisit = mockCurrentVisit + return Promise.resolve() + } + next() +}) + +// ovveride requireCurrentDocument to provide our own +let mockCurrentDocument +jest.mock('../../server/express_current_document', () => (req, res, next) => { + req.requireCurrentDocument = () => { + if (!mockCurrentDocument) return Promise.reject() + req.currentDocument = mockCurrentDocument + return Promise.resolve(mockCurrentDocument) + } + res.render = jest + .fn() + .mockImplementation((template, message) => + res.send(`mock template rendered: ${template} with message: ${message}`) + ) + next() +}) + +// ovveride requireCurrentDocument to provide our own +let mockLtiLaunch +jest.mock('../../server/express_lti_launch', () => ({ + assignment: (req, res, next) => { + req.lti = { body: mockLtiLaunch } + req.oboLti = { + launchId: 'mock-launch-id', + body: mockLtiLaunch + } + next() + } +})) + +// setup express server +const request = require('supertest') +const express = require('express') +const app = express() +app.set('view engine', 'ejs') +app.set('views', __dirname + '../../../server/views/') +app.use(oboRequire('server/express_current_user')) +app.use(oboRequire('server/express_current_document')) +app.use('/', oboRequire('server/express_response_decorator')) +app.use('/', oboRequire('server/routes/view-split')) + +describe('view-split route', () => { + const insertEvent = oboRequire('server/insert_event') + const VisitModel = oboRequire('server/models/visit') + const oboEvents = oboRequire('server/obo_events') + + const mockLtiObj = { + resource_link_id: 3, + draftA: 'mockDraftId1', + draftB: 'mockDraftId2' + } + + beforeAll(() => {}) + afterAll(() => {}) + beforeEach(() => { + mockCurrentVisit = { is_preview: false } + mockCurrentUser = { id: 4, hasPermission: perm => perm === 'canViewAsStudent' } + insertEvent.mockReset() + VisitModel.createVisit.mockReset() + VisitModel.fetchById.mockResolvedValue(mockCurrentVisit) + }) + afterEach(() => {}) + + test('launch visit requires current user in form requests', () => { + expect.assertions(3) + mockCurrentUser = null + return request(app) + .post(`/`) + .type('application/x-www-form-urlencoded') + .then(response => { + expect(response.header['content-type']).toContain('text/html') + expect(response.statusCode).toBe(401) + expect(response.text).toBe('Not Authorized') + }) + }) + + test('launch visit requires current user in json requests', () => { + expect.assertions(5) + mockCurrentUser = null + return request(app) + .post(`/`) + .type('application/json') + .set('Accept', 'application/json') + .then(response => { + expect(response.header['content-type']).toContain('application/json') + expect(response.statusCode).toBe(401) + expect(response.body).toHaveProperty('status', 'error') + expect(response.body).toHaveProperty('value') + expect(response.body.value).toHaveProperty('type', 'notAuthorized') + }) + }) + + test('launch visit requires a currentDocument', () => { + expect.assertions(2) + mockCurrentDocument = null + + return request(app) + .post(`/`) + .type('application/x-www-form-urlencoded') + .then(response => { + expect(response.header['content-type']).toContain('text/html') + expect(response.statusCode).toBe(404) + }) + }) + + test('launch visit redirects to view for students', () => { + expect.assertions(3) + + VisitModel.createVisit.mockResolvedValueOnce({ + visitId: 'mocked-visit-id', + deactivatedVisitId: 'mocked-deactivated-visit-id' + }) + + mockCurrentDocument = { draftId: validUUID() } + mockLtiLaunch = mockLtiObj + + return request(app) + .post(`/`) + .then(response => { + expect(response.header['content-type']).toContain('text/plain') + expect(response.statusCode).toBe(302) + expect(response.text).toBe( + 'Found. Redirecting to /view/' + validUUID() + '/visit/mocked-visit-id' + ) + }) + }) + + test('launch visit emits a SPLIT_RUN_PREVIEW event', () => { + expect.assertions(2) + mockCurrentUser.hasPermission = () => false + + mockCurrentDocument = { draftId: validUUID() } + mockLtiLaunch = mockLtiObj + + // For some reason this test will hang unless the response is handled + // We can spoof the end result of the oboEvents.emit call to move things along + oboEvents.emit.mockImplementation(({ res }) => { + res.render() + }) + + return request(app) + .post(`/`) + .then(() => { + expect(oboEvents.emit).toHaveBeenCalledWith( + 'SPLIT_RUN_PREVIEW', + expect.objectContaining({ moduleOptionIds: ['mockDraftId1', 'mockDraftId2'] }) + ) + expect(VisitModel.createVisit).not.toHaveBeenCalled() + + oboEvents.emit.mockReset() + }) + }) + + test('launch visit inserts event `visit:create`', () => { + expect.assertions(4) + VisitModel.createVisit.mockResolvedValueOnce({ + visitId: 'mocked-visit-id', + deactivatedVisitId: 'mocked-deactivated-visit-id' + }) + + mockCurrentDocument = { draftId: validUUID() } + mockLtiLaunch = mockLtiObj + + return request(app) + .post(`/`) + .then(response => { + expect(response.header['content-type']).toContain('text/plain') + expect(response.statusCode).toBe(302) + expect(insertEvent).toHaveBeenCalledTimes(1) + expect(insertEvent.mock.calls[0]).toMatchSnapshot() + }) + }) + + test('launch visit allows EVENT_BEFORE_NEW_VISIT to alter req', () => { + expect.assertions(4) + VisitModel.createVisit.mockResolvedValueOnce({ + visitId: 'mocked-visit-id', + deactivatedVisitId: 'mocked-deactivated-visit-id' + }) + + // use event listener to alter req + oboEvents.emit.mockImplementation((event, options) => { + options.req.visitOptions = {} + }) + + mockCurrentDocument = { draftId: validUUID() } + mockLtiLaunch = mockLtiObj + + return request(app) + .post(`/`) + .then(response => { + expect(response.header['content-type']).toContain('text/plain') + expect(response.statusCode).toBe(302) + expect(insertEvent).toHaveBeenCalledTimes(1) + expect(insertEvent.mock.calls[0]).toMatchSnapshot() + }) + }) + + test('launch visit doesnt redirect with session errors', () => { + expect.assertions(3) + + mockSaveSessionSuccess = false + + VisitModel.createVisit.mockResolvedValueOnce({ + visitId: 'mocked-visit-id', + deactivatedVisitId: 'mocked-deactivated-visit-id' + }) + + mockCurrentDocument = { draftId: validUUID() } + mockLtiLaunch = mockLtiObj + + return request(app) + .post(`/`) + .then(response => { + expect(response.header['content-type']).toContain('text/html') + expect(VisitModel.createVisit).toHaveBeenCalledTimes(1) + expect(response.statusCode).toBe(500) + }) + }) +}) diff --git a/packages/app/obojobo-express/server/express_current_document.js b/packages/app/obojobo-express/server/express_current_document.js index 1a9c7e558b..7f8d296d10 100644 --- a/packages/app/obojobo-express/server/express_current_document.js +++ b/packages/app/obojobo-express/server/express_current_document.js @@ -12,6 +12,8 @@ const resetCurrentDocument = req => { req.currentDocument = null } +const chooseSplitRunDraft = (user, draftA, draftB) => (user.createdAt % 2 === 0 ? draftA : draftB) + const requireCurrentDocument = req => { if (req.currentDocument) { return Promise.resolve(req.currentDocument) @@ -19,7 +21,18 @@ const requireCurrentDocument = req => { // Figure out where the draftId is in this request let draftId = null - if (req.params && req.params.draftId) { + + // Check first if this is a split-run request, in which case + // there will be two drafts indicated as options and we need + // to choose one + // Otherwise this will be a single-module request + if (req.params && req.params.draftA && req.params.draftB) { + draftId = chooseSplitRunDraft(req.currentUser, req.params.draftA, req.params.draftB) + } else if (req.body && req.body.draftA && req.body.draftB) { + draftId = chooseSplitRunDraft(req.currentUser, req.body.draftA, req.body.draftB) + } else if (req.body && req.body.event && req.body.event.draftA && req.body.event.draftB) { + draftId = chooseSplitRunDraft(req.currentUser, req.body.event.draftA, req.body.event.draftB) + } else if (req.params && req.params.draftId) { draftId = req.params.draftId } else if (req.body && req.body.draftId) { draftId = req.body.draftId diff --git a/packages/app/obojobo-express/server/models/user.js b/packages/app/obojobo-express/server/models/user.js index e2c420667a..ab9a6d7bfd 100644 --- a/packages/app/obojobo-express/server/models/user.js +++ b/packages/app/obojobo-express/server/models/user.js @@ -114,7 +114,7 @@ class User { first_name = $[firstName], last_name = $[lastName], roles = $[roles] - RETURNING id + RETURNING id, created_at `, this ) @@ -125,6 +125,8 @@ class User { // populate my id from the result this.id = insertUserResult.id } + // populate createdAt time from the result + this.createdAt = insertUserResult.created_at oboEvents.emit(eventName, this) return this }) diff --git a/packages/app/obojobo-express/server/obo_express.js b/packages/app/obojobo-express/server/obo_express.js index 23efca6b93..20b84ef6ac 100644 --- a/packages/app/obojobo-express/server/obo_express.js +++ b/packages/app/obojobo-express/server/obo_express.js @@ -34,6 +34,7 @@ app.on('mount', app => { // =========== ROUTING & CONTROLLERS =========== app.use('/preview', oboRequire('server/routes/preview')) app.use('/view', oboRequire('server/routes/viewer')) + app.use('/view-split', oboRequire('server/routes/view-split')) app.use('/editor', oboRequire('server/routes/editor')) app.use('/lti', oboRequire('server/routes/lti')) app.use('/api/drafts', oboRequire('server/routes/api/drafts')) diff --git a/packages/app/obojobo-express/server/obo_express_dev.js b/packages/app/obojobo-express/server/obo_express_dev.js index c608054d65..fb96dc967e 100644 --- a/packages/app/obojobo-express/server/obo_express_dev.js +++ b/packages/app/obojobo-express/server/obo_express_dev.js @@ -83,6 +83,7 @@ const renderLtiLaunch = (paramsIn, method, endpoint, res) => { oauth_version: '1.0' } const params = { ...paramsIn, ...oauthParams } + const hmac_sha1 = sig.generate(method, endpoint, params, oauthSecret, '', { encodeSignature: false }) @@ -132,6 +133,17 @@ module.exports = app => { .map(draft => ``) .join('') + // in support of quickly testing the split-run feature + // create a copy of the draft options, but auto-select the last one so it's different from the first option + const draftOptions2 = drafts + .map((draft, index) => { + if (index === drafts.length - 1) { + return `` + } + return `` + }) + .join('') + let userSelectRender = '
No users found. Create a student or instructor with the buttons above.
' if (userOptions && userOptions.length) { @@ -162,142 +174,174 @@ module.exports = app => { -