From 78f751344ac2f2c152e5ed2656c20210b3d0ca7c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 22 Nov 2016 17:01:48 -0700 Subject: [PATCH 01/28] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c5ec0d6bf..dbfdfb175 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -5,5 +5,3 @@ - click a button - view the cat - see the cat meow - -@coralproject/tech From 325162cb3ef40c07ca8ac8d083151b52aaf9d80d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 22 Nov 2016 17:15:14 -0700 Subject: [PATCH 02/28] Added authorization pieces + test functions --- middleware/authorization.js | 44 +++++++++++++-------- models/action.js | 16 +++++++- routes/api/actions/index.js | 3 +- routes/api/asset/index.js | 11 ------ routes/api/auth/index.js | 2 +- routes/api/comments/index.js | 9 +++-- routes/api/index.js | 7 ++-- routes/api/queue/index.js | 4 +- routes/api/settings/index.js | 5 ++- routes/api/stream/index.js | 2 +- routes/api/user/index.js | 5 ++- tests/models/action.js | 61 +++++++++++++++++++++-------- tests/routes/api/assets/index.js | 63 +----------------------------- tests/routes/api/comments/index.js | 30 +++++++++++--- tests/routes/api/queue/index.js | 2 + tests/routes/api/settings/index.js | 54 +++++++++++++------------ tests/utils/passport.js | 25 ++++++++++++ 17 files changed, 190 insertions(+), 153 deletions(-) create mode 100644 tests/utils/passport.js diff --git a/middleware/authorization.js b/middleware/authorization.js index 238219a06..65f77b753 100644 --- a/middleware/authorization.js +++ b/middleware/authorization.js @@ -2,7 +2,9 @@ * authorization contains the references to the authorization middleware. * @type {Object} */ -const authorization = module.exports = {}; +const authorization = module.exports = { + middleware: [] +}; const debug = require('debug')('talk:middleware:authorization'); @@ -33,21 +35,29 @@ authorization.has = (user, ...roles) => roles.every((role) => user.roles.indexOf * @param {Array} roles all the roles that a user must have * @return {Callback} connect middleware */ -authorization.needed = (...roles) => (req, res, next) => { - // All routes that are wrapepd with this middleware actually require a role. - if (!req.user) { - debug(`No user on request, returning with ${ErrNotAuthorized}`); - return next(ErrNotAuthorized); - } +authorization.needed = (...roles) => [ - // Check to see if the current user has all the roles requested for the given - // array of roles requested, if one is not on the user, then this will - // evaluate to true. - if (!authorization.has(req.user, ...roles)) { - debug('User does not have all the required roles to access this page'); - return next(ErrNotAuthorized); - } + // Insert the pre-needed middlware. + ...authorization.middleware, - // Looks like they're allowed! - return next(); -}; + // Insert the actual middleware to check for the required role. + (req, res, next) => { + + // All routes that are wrapepd with this middleware actually require a role. + if (!req.user) { + debug(`No user on request, returning with ${ErrNotAuthorized}`); + return next(ErrNotAuthorized); + } + + // Check to see if the current user has all the roles requested for the given + // array of roles requested, if one is not on the user, then this will + // evaluate to true. + if (!authorization.has(req.user, ...roles)) { + debug('User does not have all the required roles to access this page'); + return next(ErrNotAuthorized); + } + + // Looks like they're allowed! + return next(); + } +]; diff --git a/models/action.js b/models/action.js index 91378b0b0..48c0a9a18 100644 --- a/models/action.js +++ b/models/action.js @@ -41,7 +41,7 @@ ActionSchema.statics.findByItemIdArray = function(item_ids) { * Returns summaries of actions for an array of ids * @param {String} ids array of user identifiers (uuid) */ -ActionSchema.statics.getActionSummaries = function(item_ids) { +ActionSchema.statics.getActionSummaries = function(item_ids, current_user_id = '') { return Action.aggregate([ { @@ -71,6 +71,18 @@ ActionSchema.statics.getActionSummaries = function(item_ids) { // just grabbing the last instance of the item type here. item_type: { $last: '$item_type' + }, + + current_user: { + $max: { + $cond: { + if: { + $eq: ['$user_id', current_user_id], + }, + then: '$$CURRENT', + else: null + } + } } } }, @@ -89,7 +101,7 @@ ActionSchema.statics.getActionSummaries = function(item_ids) { item_type: '$item_type', // set the current user to false here - current_user: {$literal: false} + current_user: '$current_user' } } ]) diff --git a/routes/api/actions/index.js b/routes/api/actions/index.js index f2f9be390..9724f5a73 100644 --- a/routes/api/actions/index.js +++ b/routes/api/actions/index.js @@ -6,7 +6,8 @@ const router = express.Router(); router.delete('/:action_id', (req, res, next) => { Action .findOneAndRemove({ - id: req.params.action_id + id: req.params.action_id, + user_id: req.user.id }) .then(() => { res.status(204).end(); diff --git a/routes/api/asset/index.js b/routes/api/asset/index.js index b261da883..217c49547 100644 --- a/routes/api/asset/index.js +++ b/routes/api/asset/index.js @@ -30,15 +30,4 @@ router.get('/:id', (req, res, next) => { }); -// Upsert an asset and return the affected document. -router.put('/', (req, res, next) => { - - Asset.upsert(req.body) - .then((asset) => { - res.json(asset); - }) - .catch(next); - -}); - module.exports = router; diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index ae6fba01c..b88b6fe12 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -14,7 +14,7 @@ router.get('/', authorization.needed(), (req, res) => { /** * This destroys the session of a user, if they have one. */ -router.delete('/', (req, res) => { +router.delete('/', authorization.needed(), (req, res) => { req.session.destroy(() => { res.status(204).end(); }); diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index d318857d6..1507e82aa 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -1,10 +1,11 @@ const express = require('express'); const Comment = require('../../../models/comment'); const wordlist = require('../../../services/wordlist'); +const authorization = require('../../../middleware/authorization'); const router = express.Router(); -router.get('/', (req, res, next) => { +router.get('/', authorization.needed('admin'), (req, res, next) => { let query; if (req.query.status) { @@ -49,7 +50,7 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { }); }); -router.get('/:comment_id', (req, res, next) => { +router.get('/:comment_id', authorization.needed('admin'), (req, res, next) => { Comment .findById(req.params.comment_id) .then(comment => { @@ -65,7 +66,7 @@ router.get('/:comment_id', (req, res, next) => { }); }); -router.delete('/:comment_id', (req, res, next) => { +router.delete('/:comment_id', authorization.needed('admin'), (req, res, next) => { Comment .removeById(req.params.comment_id) .then(() => { @@ -76,7 +77,7 @@ router.delete('/:comment_id', (req, res, next) => { }); }); -router.put('/:comment_id/status', (req, res, next) => { +router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next) => { const { status diff --git a/routes/api/index.js b/routes/api/index.js index 7192a1324..8c6014ed8 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -1,14 +1,15 @@ const express = require('express'); +const authorization = require('../../middleware/authorization'); const router = express.Router(); router.use('/asset', require('./asset')); router.use('/auth', require('./auth')); -router.use('/comments', require('./comments')); +router.use('/comments', authorization.needed(), require('./comments')); router.use('/queue', require('./queue')); -router.use('/settings', require('./settings')); +router.use('/settings', authorization.needed('admin'), require('./settings')); router.use('/stream', require('./stream')); router.use('/user', require('./user')); -router.use('/actions', require('./actions')); +router.use('/actions', authorization.needed(), require('./actions')); module.exports = router; diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index f8ab95d6b..edc5602ef 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -1,7 +1,7 @@ const express = require('express'); const Comment = require('../../../models/comment'); - const Setting = require('../../../models/setting'); +const authorization = require('../../../middleware/authorization'); const router = express.Router(); @@ -13,7 +13,7 @@ const router = express.Router(); // depending on the settings. The :moderation overwrites this settings. // Pre-moderation: New comments are shown in the moderator queues immediately. // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. -router.get('/comments/pending', (req, res, next) => { +router.get('/comments/pending', authorization.needed('admin'), (req, res, next) => { Setting.getModerationSetting().then(function({moderation}){ Comment.moderationQueue(moderation).then((comments) => { res.status(200).json(comments); diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 2665cacc8..6e8c64d4f 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -1,7 +1,8 @@ -const _ = require('lodash'); const express = require('express'); -const router = express.Router(); const Setting = require('../../../models/setting'); +const _ = require('lodash'); + +const router = express.Router(); router.get('/', (req, res, next) => { Setting diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index acbfe3d77..a48548608 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -41,7 +41,7 @@ router.get('/', (req, res, next) => { asset.id, ...comments.map((comment) => comment.id), ...comments.map((comment) => comment.author_id) - ])) + ]), req.user ? req.user.id : '') ]); }) .then(([assets, comments, users, actions]) => { diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 3d5b49d93..3251e0608 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -7,8 +7,9 @@ const fs = require('fs'); const path = require('path'); const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/password-reset-email.ejs')); const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); +const authorization = require('../../../middleware/authorization'); -router.get('/', (req, res, next) => { +router.get('/', authorization.needed('admin'), (req, res, next) => { const { value = '', field = 'created_at', @@ -49,7 +50,7 @@ router.get('/', (req, res, next) => { .catch(next); }); -router.post('/:user_id/role', (req, res, next) => { +router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => { User .addRoleToUser(req.params.user_id, req.body.role) .then(role => { diff --git a/tests/models/action.js b/tests/models/action.js index 78f5fd1d0..87d53b7da 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -5,23 +5,25 @@ const expect = require('chai').expect; describe('Action: models', () => { let mockActions; + beforeEach(() => { return Action.create([{ action_type: 'flag', item_id: '123', - item_type: 'comments' + item_type: 'comment', + user_id: 'flagginguserid' }, { action_type: 'flag', item_id: '456', - item_type: 'comments' + item_type: 'comment' }, { action_type: 'flag', item_id: '123', - item_type: 'comments' + item_type: 'comment' }, { action_type: 'like', item_id: '123', - item_type: 'comments' + item_type: 'comment' }]).then((actions) => { mockActions = actions; }); @@ -30,8 +32,7 @@ describe('Action: models', () => { describe('#findById()', () => { it('should find an action by id', () => { return Action.findById(mockActions[0].id).then((result) => { - expect(result).to.have.property('action_type') - .and.to.equal('flag'); + expect(result).to.have.property('action_type', 'flag'); }); }); }); @@ -46,27 +47,55 @@ describe('Action: models', () => { describe('#getActionSummaries()', () => { it('should return properly formatted summaries from an array of item_ids', () => { - return Action.getActionSummaries(['123', '789']).then((result) => { - expect(result).to.have.length(2); + return Action.getActionSummaries(['123', '789']).then((summaries) => { + expect(summaries).to.have.length(2); - const sorted = result.sort((a, b) => a.count - b.count); - - expect(sorted[0]).to.deep.equal({ + expect(summaries).to.deep.include({ action_type: 'like', count: 1, item_id: '123', - item_type: 'comments', - current_user: false + item_type: 'comment', + current_user: null }); - expect(sorted[1]).to.deep.equal({ + expect(summaries).to.deep.include({ action_type: 'flag', count: 2, item_id: '123', - item_type: 'comments', - current_user: false + item_type: 'comment', + current_user: null }); }); }); + + it('should include a current user when one is passed', () => { + return Action + .getActionSummaries(['123'], 'flagginguserid') + .then((summaries) => { + expect(summaries).to.have.length(2); + + let summary = summaries.find((s) => s.item_id === '123' && s.action_type === 'flag'); + + expect(summary).to.not.be.undefined; + expect(summary.current_user).to.not.be.null; + expect(summary.current_user).to.have.property('item_id', '123'); + expect(summary.current_user).to.have.property('item_type', 'comment'); + expect(summary.current_user).to.have.property('user_id', 'flagginguserid'); + expect(summary.current_user).to.have.property('action_type', 'flag'); + }); + }); + + it('should not include a current user when one is passed for a user that doesn\'t have an action', () => { + return Action + .getActionSummaries(['123'], 'flagginguserid2') + .then((summaries) => { + expect(summaries).to.have.length(2); + + summaries.forEach((summary) => { + expect(summary).to.not.be.undefined; + expect(summary).to.have.property('current_user', null); + }); + }); + }); }); }); diff --git a/tests/routes/api/assets/index.js b/tests/routes/api/assets/index.js index aa764e214..542e88c42 100644 --- a/tests/routes/api/assets/index.js +++ b/tests/routes/api/assets/index.js @@ -1,22 +1,13 @@ require('../../../utils/mongoose'); +const passport = require('../../../utils/passport'); const chai = require('chai'); -const expect = chai.expect; const server = require('../../../../app'); // Setup chai. chai.should(); chai.use(require('chai-http')); -let fixture = { - 'url': 'http://hhgg.com/total-perspective-vortex', - 'type': 'article', - 'headline': 'The Total Perspective Vortex', - 'summary': 'You are an insignificant dot on an insignificant dot.', - 'section': 'Everything', - 'authors': ['Ford Prefect'] -}; - describe('Asset: routes', () => { describe('/GET Asset', () => { @@ -25,6 +16,7 @@ describe('Asset: routes', () => { chai.request(server) .get('/api/v1/asset') + .set(passport.inject({roles: ['admin']})) .end((err, res) => { if (err) { @@ -41,55 +33,4 @@ describe('Asset: routes', () => { }); }); - // This test checks PUT and read - describe('/PUT Asset', () => { - describe('#put', () => { - it('It should save an asset and load it again.', (done) => { - - chai.request(server) - .put('/api/v1/asset') - .send(fixture) - .end((err, res) => { - - if (err) { - throw new Error(err); - } - - res.should.have.status(200); - res.body.should.be.a('object'); - - // Id should be generated by the model if absent. - res.body.should.have.property('id'); - - // Save the asset id to compare with GET result. - let assetId = res.body.id; - - // Load the asset to make sure it's really there. - chai.request(server) - .get(`/api/v1/asset?url=${encodeURIComponent(fixture.url)}`) - .end((err, res) => { - - if (err) { - throw new Error(err); - } - - res.should.have.status(200); - res.body.should.be.an('array'); - - let asset = res.body[0]; - - expect(asset).to.have.property('id'); - - // Ensure the asset has the same id as above. - // This tests the single url per Id concept. - expect(assetId).to.equal(asset.id); - - done(); - - }); - }); - }); - }); - }); // End describe /PUT Asset - }); diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index ec9e05d03..be3d14f69 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -1,6 +1,7 @@ process.env.NODE_ENV = 'test'; require('../../../utils/mongoose'); +const passport = require('../../../utils/passport'); const app = require('../../../../app'); const chai = require('chai'); @@ -68,6 +69,7 @@ describe('Get /comments', () => { it('should return all the comments', () => { return chai.request(app) .get('/api/v1/comments') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); @@ -126,6 +128,7 @@ describe('Get comments by status and action', () => { it('should return all the rejected comments', () => { return chai.request(app) .get('/api/v1/comments?status=rejected') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); expect(res.body[0]).to.have.property('id', 'abc'); @@ -135,6 +138,7 @@ describe('Get comments by status and action', () => { it('should return all the approved comments', () => { return chai.request(app) .get('/api/v1/comments?status=accepted') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); expect(res.body[0]).to.have.property('id', 'hij'); @@ -144,6 +148,7 @@ describe('Get comments by status and action', () => { it('should return all the new comments', () => { return chai.request(app) .get('/api/v1/comments?status=new') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); expect(res.body[0]).to.have.property('id', 'def'); @@ -153,6 +158,7 @@ describe('Get comments by status and action', () => { it('should return all the flagged comments', () => { return chai.request(app) .get('/api/v1/comments?action_type=flag') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); @@ -195,6 +201,7 @@ describe('Post /comments', () => { it('should create a comment', () => { return chai.request(app) .post('/api/v1/comments') + .set(passport.inject({roles: []})) .send({'body': 'Something body.', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) .then((res) => { expect(res).to.have.status(201); @@ -205,6 +212,7 @@ describe('Post /comments', () => { it('should create a comment with a rejected status if it contains a bad word', () => { return chai.request(app) .post('/api/v1/comments') + .set(passport.inject({roles: []})) .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) .then((res) => { expect(res).to.have.status(201); @@ -262,6 +270,7 @@ describe('Get /:comment_id', () => { it('should return the right comment for the comment_id', () => { return chai.request(app) .get('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); expect(res).to.have.property('body'); @@ -318,6 +327,7 @@ describe('Remove /:comment_id', () => { it('it should remove comment', () => { return chai.request(app) .delete('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(204); @@ -329,11 +339,6 @@ describe('Remove /:comment_id', () => { }); }); -process.on('unhandledRejection', (reason) => { - console.error('Reason: '); - console.error(reason); -}); - describe('Put /:comment_id/status', () => { const comments = [{ @@ -384,12 +389,26 @@ describe('Put /:comment_id/status', () => { it('it should update status', function() { return chai.request(app) .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: ['admin']})) .send({status: 'accepted'}) .then((res) => { expect(res).to.have.status(204); expect(res.body).to.be.empty; }); }); + + it('it should not allow a non-admin to update status', () => { + return chai.request(app) + .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: []})) + .send({status: 'accepted'}) + .then((res) => { + expect(res).to.be.empty; + }) + .catch((err) => { + expect(err).to.have.property('status', 401); + }); + }); }); describe('Post /:comment_id/actions', () => { @@ -442,6 +461,7 @@ describe('Post /:comment_id/actions', () => { it('it should update actions', () => { return chai.request(app) .post('/api/v1/comments/abc/actions') + .set(passport.inject({roles: ['admin']})) .send({'user_id': '456', 'action_type': 'flag'}) .then((res) => { expect(res).to.have.status(201); diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index f21fe1331..4eff3d384 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -1,6 +1,7 @@ process.env.NODE_ENV = 'test'; require('../../../utils/mongoose'); +const passport = require('../../../utils/passport'); const app = require('../../../../app'); const chai = require('chai'); @@ -71,6 +72,7 @@ describe('Get moderation queues rejected, pending, flags', () => { it('should return all the pending comments', function(done){ chai.request(app) .get('/api/v1/queue/comments/pending') + .set(passport.inject({roles: ['admin']})) .end(function(err, res){ expect(err).to.be.null; expect(res).to.have.status(200); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index 41c37828e..86feca1eb 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -1,13 +1,15 @@ process.env.NODE_ENV = 'test'; require('../../../utils/mongoose'); +const passport = require('../../../utils/passport'); const app = require('../../../../app'); const chai = require('chai'); -const chaiHttp = require('chai-http'); -chai.use(chaiHttp); const expect = chai.expect; +chai.should(); +chai.use(require('chai-http')); + const Setting = require('../../../../models/setting'); const defaults = {id: '1', moderation: 'pre'}; @@ -17,15 +19,16 @@ describe('GET /settings', () => { return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); }); - it('should return a settings object', done => { - chai.request(app) + it('should return a settings object', () => { + return chai.request(app) .get('/api/v1/settings') - .end((err, res) => { - expect(err).to.be.null; + .set(passport.inject({ + roles: ['admin'] + })) + .then((res) => { expect(res).to.have.status(200); expect(res).to.be.json; expect(res.body).to.have.property('moderation', 'pre'); - done(err); }); }); }); @@ -33,25 +36,26 @@ describe('GET /settings', () => { // update the settings. describe('update settings', () => { it('should respond ok to a PUT', () => { - return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}) - .then(() => { - return chai.request(app) - .put('/api/v1/settings') - .send({moderation: 'post'}) - .then(res => { - expect(res).to.have.status(204); + return Setting + .update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}) + .then(() => { + return chai.request(app) + .put('/api/v1/settings') + .set(passport.inject({ + roles: ['admin'] + })) + .send({moderation: 'post'}); + }) + .then(res => { + expect(res).to.have.status(204); - return Setting.getSettings(); + return Setting.getSettings(); + }) + .then(settings => { - }) - .then(settings => { - // confirm updated settings in db - expect(settings).to.have.property('moderation'); - expect(settings.moderation).to.equal('post'); - }) - .catch(err => { - throw err; - }); - }); + // confirm updated settings in db + expect(settings).to.have.property('moderation'); + expect(settings.moderation).to.equal('post'); + }); }); }); diff --git a/tests/utils/passport.js b/tests/utils/passport.js new file mode 100644 index 000000000..401e50b77 --- /dev/null +++ b/tests/utils/passport.js @@ -0,0 +1,25 @@ +const authorization = require('../../middleware/authorization'); + +// Add the passport middleware here before it's setup. +authorization.middleware.push((req, res, next) => { + req.user = JSON.parse(new Buffer(req.get('X-Mock-Authorization'), 'base64').toString('ascii')); + + next(); +}); + +const MockStrategy = { + + /** + * Injects the new user into the request header for the mock middleware to + * interpret. + * @param {Object} user the user to inject + * @return {Object} the headers to add to the request + */ + inject(user) { + return { + 'X-Mock-Authorization': new Buffer(JSON.stringify(user)).toString('base64') + }; + } +}; + +module.exports = MockStrategy; From e5c02c427418433cc4fed21f9723c1aa55bb3082 Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 22 Nov 2016 17:14:24 -0800 Subject: [PATCH 03/28] The field is action_type and not action. --- client/coral-admin/src/AppRouter.js | 6 +++--- .../containers/{ => CommentStream}/CommentStream.css | 0 .../containers/{ => CommentStream}/CommentStream.js | 2 +- .../src/containers/{ => Configure}/Configure.css | 0 .../src/containers/{ => Configure}/Configure.js | 9 ++++++--- .../{ => ModerationQueue}/ModerationQueue.css | 0 .../{ => ModerationQueue}/ModerationQueue.js | 12 +++++++++--- client/coral-admin/src/services/talk-adapter.js | 2 +- 8 files changed, 20 insertions(+), 11 deletions(-) rename client/coral-admin/src/containers/{ => CommentStream}/CommentStream.css (100%) rename client/coral-admin/src/containers/{ => CommentStream}/CommentStream.js (98%) rename client/coral-admin/src/containers/{ => Configure}/Configure.css (100%) rename client/coral-admin/src/containers/{ => Configure}/Configure.js (96%) rename client/coral-admin/src/containers/{ => ModerationQueue}/ModerationQueue.css (100%) rename client/coral-admin/src/containers/{ => ModerationQueue}/ModerationQueue.js (95%) diff --git a/client/coral-admin/src/AppRouter.js b/client/coral-admin/src/AppRouter.js index a0b43361d..6d55d7b18 100644 --- a/client/coral-admin/src/AppRouter.js +++ b/client/coral-admin/src/AppRouter.js @@ -1,9 +1,9 @@ import React from 'react'; import {Router, Route, IndexRoute, browserHistory} from 'react-router'; -import ModerationQueue from 'containers/ModerationQueue'; -import CommentStream from 'containers/CommentStream'; -import Configure from 'containers/Configure'; +import ModerationQueue from 'containers/ModerationQueue/ModerationQueue'; +import CommentStream from 'containers/CommentStream/CommentStream'; +import Configure from 'containers/Configure/Configure'; import CommunityContainer from 'containers/Community/CommunityContainer'; import LayoutContainer from 'containers/LayoutContainer'; diff --git a/client/coral-admin/src/containers/CommentStream.css b/client/coral-admin/src/containers/CommentStream/CommentStream.css similarity index 100% rename from client/coral-admin/src/containers/CommentStream.css rename to client/coral-admin/src/containers/CommentStream/CommentStream.css diff --git a/client/coral-admin/src/containers/CommentStream.js b/client/coral-admin/src/containers/CommentStream/CommentStream.js similarity index 98% rename from client/coral-admin/src/containers/CommentStream.js rename to client/coral-admin/src/containers/CommentStream/CommentStream.js index da4d03a22..b1e002549 100644 --- a/client/coral-admin/src/containers/CommentStream.js +++ b/client/coral-admin/src/containers/CommentStream/CommentStream.js @@ -31,7 +31,7 @@ class CommentStream extends React.Component { // The only action for now is flagging onClickAction (action, id) { - if (action === 'flagged') { + if (action === 'flag') { this.props.dispatch(flagComment(id)); clearTimeout(this._snackTimeout); this.setState({snackbar: true, snackbarMsg: 'Thank you for reporting this comment. Our moderation team has been notified and will review it shortly.'}); diff --git a/client/coral-admin/src/containers/Configure.css b/client/coral-admin/src/containers/Configure/Configure.css similarity index 100% rename from client/coral-admin/src/containers/Configure.css rename to client/coral-admin/src/containers/Configure/Configure.css diff --git a/client/coral-admin/src/containers/Configure.js b/client/coral-admin/src/containers/Configure/Configure.js similarity index 96% rename from client/coral-admin/src/containers/Configure.js rename to client/coral-admin/src/containers/Configure/Configure.js index 2058c66c8..a9e2f3ef2 100644 --- a/client/coral-admin/src/containers/Configure.js +++ b/client/coral-admin/src/containers/Configure/Configure.js @@ -1,7 +1,6 @@ - import React from 'react'; import {connect} from 'react-redux'; -import {fetchSettings, updateSettings, saveSettingsToServer} from '../actions/settings'; +import {fetchSettings, updateSettings, saveSettingsToServer} from '../../actions/settings'; import { List, ListItem, @@ -14,7 +13,7 @@ import { } from 'react-mdl'; import styles from './Configure.css'; import I18n from 'coral-framework/modules/i18n/i18n'; -import translations from '../translations.json'; +import translations from '../../translations.json'; class Configure extends React.Component { constructor (props) { @@ -23,9 +22,13 @@ class Configure extends React.Component { this.state = {activeSection: 'comments', copied: false}; this.copyToClipBoard = this.copyToClipBoard.bind(this); + + // Update settings this.updateModeration = this.updateModeration.bind(this); + // InfoBox has two settings. Enable or not and the content of it if it is enable. this.updateInfoBoxEnable = this.updateInfoBoxEnable.bind(this); this.updateInfoBoxContent = this.updateInfoBoxContent.bind(this); + this.saveSettings = this.saveSettings.bind(this); } diff --git a/client/coral-admin/src/containers/ModerationQueue.css b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.css similarity index 100% rename from client/coral-admin/src/containers/ModerationQueue.css rename to client/coral-admin/src/containers/ModerationQueue/ModerationQueue.css diff --git a/client/coral-admin/src/containers/ModerationQueue.js b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js similarity index 95% rename from client/coral-admin/src/containers/ModerationQueue.js rename to client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js index 88150012a..a4d82fddc 100644 --- a/client/coral-admin/src/containers/ModerationQueue.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js @@ -1,16 +1,22 @@ import React from 'react'; import {connect} from 'react-redux'; +import key from 'keymaster'; + import ModerationKeysModal from 'components/ModerationKeysModal'; import CommentList from 'components/CommentList'; + import {updateStatus} from 'actions/comments'; import styles from './ModerationQueue.css'; -import key from 'keymaster'; + import I18n from 'coral-framework/modules/i18n/i18n'; -import translations from '../translations.json'; +import translations from '../../translations.json'; /* * Renders the moderation queue as a tabbed layout with 3 moderation - * queues filtered by status (Untouched, Rejected and Approved) + * queues : + * * pending: filtered by status Untouched + * * rejected: filtered by status Rejected + * * flagged: with a flagged action on them */ class ModerationQueue extends React.Component { diff --git a/client/coral-admin/src/services/talk-adapter.js b/client/coral-admin/src/services/talk-adapter.js index 86878723e..361a6479e 100644 --- a/client/coral-admin/src/services/talk-adapter.js +++ b/client/coral-admin/src/services/talk-adapter.js @@ -37,7 +37,7 @@ const fetchModerationQueueComments = store => Promise.all([ fetch('/api/v1/queue/comments/pending'), fetch('/api/v1/comments?status=rejected'), - fetch('/api/v1/comments?action=flag') + fetch('/api/v1/comments?action_type=flag') ]) .then(res => Promise.all(res.map(r => r.json()))) .then(res => { From 7a6ea34468a4356a53e2f94c310a0fc0da3cc843 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 25 Nov 2016 10:42:38 -0500 Subject: [PATCH 04/28] Remove the preview link --- routes/admin/index.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/routes/admin/index.js b/routes/admin/index.js index 9d3cbd0a9..2931626be 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,12 +1,8 @@ const express = require('express'); const router = express.Router(); -router.get('/embed/stream/preview', (req, res) => { - res.render('embed-stream', {basePath: '/client/embed/stream'}); -}); - -// this route is expecting there to be a token in the hash -// see /views/password-reset-email.ejs +// GET /password-reset expects an OpenID token in the hash. +// Links to this endpoit are generated in /views/password-reset-email.ejs. router.get('/password-reset', (req, res, next) => { // TODO: store the redirect uri in the token or something fancy // admins and regular users should probably be redirected to different places. From 93cd4ea74c7a9654319f63ea9da4c8eb3c6da8d9 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 25 Nov 2016 10:43:32 -0500 Subject: [PATCH 05/28] Remove non-standard stream embed template --- views/embed/stream.ejs | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 views/embed/stream.ejs diff --git a/views/embed/stream.ejs b/views/embed/stream.ejs deleted file mode 100644 index 5cdef0811..000000000 --- a/views/embed/stream.ejs +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - -
- - - From a022ef8a52fa53dd74af5e562da9d4efe32d1dc4 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 25 Nov 2016 10:50:30 -0500 Subject: [PATCH 06/28] Restore embed/stream.ejs --- views/embed/stream.ejs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 views/embed/stream.ejs diff --git a/views/embed/stream.ejs b/views/embed/stream.ejs new file mode 100644 index 000000000..f87318db6 --- /dev/null +++ b/views/embed/stream.ejs @@ -0,0 +1,13 @@ + + + + + + + + +
+ + + \ No newline at end of file From cb31caed4b0378a3d9fc5784e3f0b6cd53604ef1 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 25 Nov 2016 11:47:00 -0500 Subject: [PATCH 07/28] Update comments --- routes/admin/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routes/admin/index.js b/routes/admin/index.js index 2931626be..03852c375 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,10 +1,10 @@ const express = require('express'); const router = express.Router(); -// GET /password-reset expects an OpenID token in the hash. -// Links to this endpoit are generated in /views/password-reset-email.ejs. +// Get /password-reset expects a signed token (JWT) in the hash. +// Links to this endpoint are generated by /views/password-reset-email.ejs. router.get('/password-reset', (req, res, next) => { - // TODO: store the redirect uri in the token or something fancy + // TODO: store the redirect uri in the token or something fancy. // admins and regular users should probably be redirected to different places. res.render('password-reset', {redirectUri: process.env.TALK_ROOT_URL}); }); From 3ebdca33f34d14e395dd1b4b93dac1d0c1d15ad6 Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Sun, 27 Nov 2016 19:37:17 -0300 Subject: [PATCH 08/28] Logout func --- client/coral-admin/src/actions/auth.js | 14 ++++++++ .../src/components/FullLoading.css | 12 +++++++ .../coral-admin/src/components/FullLoading.js | 13 ++++++++ .../coral-admin/src/components/ui/Header.css | 32 +++++++++++++++++-- .../coral-admin/src/components/ui/Header.js | 29 +++++++++++++---- .../coral-admin/src/components/ui/Layout.js | 4 +-- client/coral-admin/src/components/ui/Logo.css | 5 ++- .../src/containers/LayoutContainer.js | 26 ++++++--------- client/coral-admin/src/reducers/auth.js | 8 ++++- 9 files changed, 113 insertions(+), 30 deletions(-) create mode 100644 client/coral-admin/src/components/FullLoading.css create mode 100644 client/coral-admin/src/components/FullLoading.js diff --git a/client/coral-admin/src/actions/auth.js b/client/coral-admin/src/actions/auth.js index c5a03132f..2c77ffce7 100644 --- a/client/coral-admin/src/actions/auth.js +++ b/client/coral-admin/src/actions/auth.js @@ -17,3 +17,17 @@ export const checkLogin = () => dispatch => { }) .catch(error => dispatch(checkLoginFailure(error))); }; + +// LogOut Actions + +const logOutRequest = () => ({type: actions.LOGOUT_REQUEST}); +const logOutSuccess = () => ({type: actions.LOGOUT_SUCCESS}); +const logOutFailure = () => ({type: actions.LOGOUT_FAILURE}); + +export const logout = () => dispatch => { + dispatch(logOutRequest()); + fetch(`${base}/auth`, getInit('DELETE')) + .then(handleResp) + .then(() => dispatch(logOutSuccess())) + .catch(error => dispatch(logOutFailure(error))); +}; diff --git a/client/coral-admin/src/components/FullLoading.css b/client/coral-admin/src/components/FullLoading.css new file mode 100644 index 000000000..8d850d381 --- /dev/null +++ b/client/coral-admin/src/components/FullLoading.css @@ -0,0 +1,12 @@ +.layout { + max-width: 800px; + margin: 0 auto; +} + +.layout h1 { + font-size: 40px; +} + +.layout img { + width: 100%; +} diff --git a/client/coral-admin/src/components/FullLoading.js b/client/coral-admin/src/components/FullLoading.js new file mode 100644 index 000000000..dee584aed --- /dev/null +++ b/client/coral-admin/src/components/FullLoading.js @@ -0,0 +1,13 @@ +import React from 'react'; +import {Layout} from 'react-mdl'; +import styles from './FullLoading.css'; +import {CoralLogo} from 'coral-ui'; + +export const FullLoading = () => ( + +
+

Loading

+ +
+ +); diff --git a/client/coral-admin/src/components/ui/Header.css b/client/coral-admin/src/components/ui/Header.css index 3d4e7dc77..a0d7dd30d 100644 --- a/client/coral-admin/src/components/ui/Header.css +++ b/client/coral-admin/src/components/ui/Header.css @@ -1,6 +1,5 @@ .header { background: #505050; - overflow: hidden; } .header > div { @@ -14,8 +13,35 @@ background: #232323; } -.version { +.rightPanel { position: absolute; right: 0; - width: 50px; + width: 170px; +} + +.rightPanel ul { + list-style: none; + line-height: 38px; +} + +.rightPanel li { + display: inline-block; + float: right; + margin-left: 15px; +} + +.rightPanel .settings { + vertical-align: middle; + border-radius: 3px; + border: solid 1px #9e9e9e; + line-height: 10px; +} + +.rightPanel .settings > div { + position: relative; +} + +.rightPanel .settings:hover { + background: rgba(158, 158, 158, 0.69); + cursor: pointer; } diff --git a/client/coral-admin/src/components/ui/Header.js b/client/coral-admin/src/components/ui/Header.js index 7ba88d25a..e4d151d30 100644 --- a/client/coral-admin/src/components/ui/Header.js +++ b/client/coral-admin/src/components/ui/Header.js @@ -1,21 +1,36 @@ import React from 'react'; -import {Navigation, Header} from 'react-mdl'; +import {Navigation, Header, IconButton, MenuItem, Menu} from 'react-mdl'; import {Link, IndexLink} from 'react-router'; import styles from './Header.css'; import I18n from 'coral-framework/modules/i18n/i18n'; import translations from '../../translations.json'; import {Logo} from './Logo'; -export default () => ( +export default ({handleLogout}) => (
- {lang.t('configure.moderate')} - {lang.t('configure.community')} - {lang.t('configure.configure')} + {lang.t('configure.moderate')} + {lang.t('configure.community')} + {lang.t('configure.configure')} -
- {`v${process.env.VERSION}`} +
+
    +
  • +
    + + + Sign Out + +
    +
    • +
  • + {`v${process.env.VERSION}`} +
    • +
); diff --git a/client/coral-admin/src/components/ui/Layout.js b/client/coral-admin/src/components/ui/Layout.js index 3e1b9cf2d..46c7aa7fa 100644 --- a/client/coral-admin/src/components/ui/Layout.js +++ b/client/coral-admin/src/components/ui/Layout.js @@ -4,9 +4,9 @@ import Header from './Header'; import Drawer from './Drawer'; import styles from './Layout.css'; -export const Layout = ({children}) => ( +export const Layout = ({children, ...props}) => ( -
+
{children} diff --git a/client/coral-admin/src/components/ui/Logo.css b/client/coral-admin/src/components/ui/Logo.css index e764af627..f89bf3d5d 100644 --- a/client/coral-admin/src/components/ui/Logo.css +++ b/client/coral-admin/src/components/ui/Logo.css @@ -1,7 +1,9 @@ .logo h1 { color: #272727; font-size: 20px; - padding: 0 30px; + margin: 0; + line-height: 60px; + padding: 0 20px; } .logo span { @@ -13,6 +15,7 @@ .logo { background: #E5E5E5; + height: 100%; } diff --git a/client/coral-admin/src/containers/LayoutContainer.js b/client/coral-admin/src/containers/LayoutContainer.js index 5f3cb0cff..f263c33f5 100644 --- a/client/coral-admin/src/containers/LayoutContainer.js +++ b/client/coral-admin/src/containers/LayoutContainer.js @@ -1,37 +1,31 @@ import React, {Component} from 'react'; import {connect} from 'react-redux'; import {Layout} from '../components/ui/Layout'; -import {checkLogin} from '../actions/auth'; -import {NotFound} from '../components/NotFound'; +import {checkLogin, logout} from '../actions/auth'; +import {FullLoading} from '../components/FullLoading'; import {PermissionRequired} from '../components/PermissionRequired'; class LayoutContainer extends Component { componentWillMount () { - this.props.checkLogin(); + const {checkLogin} = this.props; + checkLogin(); } render () { - const {isAdmin, loggedIn} = this.props.auth; - - if (!loggedIn) { - return ; - } - - if (!isAdmin && loggedIn) { - return ; - } - - return ; + const {isAdmin, loggedIn, loadingUser} = this.props.auth; + if (loadingUser) { return ; } + if (!isAdmin) { return ; } + if (isAdmin && loggedIn) { return ; } + return ; } } -LayoutContainer.propTypes = {}; - const mapStateToProps = state => ({ auth: state.auth.toJS() }); const mapDispatchToProps = dispatch => ({ checkLogin: () => dispatch(checkLogin()), + handleLogout: () => dispatch(logout()) }); export default connect( diff --git a/client/coral-admin/src/reducers/auth.js b/client/coral-admin/src/reducers/auth.js index 59dccac5e..f897c1bae 100644 --- a/client/coral-admin/src/reducers/auth.js +++ b/client/coral-admin/src/reducers/auth.js @@ -9,19 +9,25 @@ const initialState = Map({ export default function auth (state = initialState, action) { switch (action.type) { + case actions.CHECK_LOGIN_REQUEST: + return state + .set('loadingUser', true); case actions.CHECK_LOGIN_FAILURE: return state .set('loggedIn', false) + .set('loadingUser', false) .set('user', null); case actions.CHECK_LOGIN_SUCCESS: return state .set('loggedIn', true) + .set('loadingUser', false) .set('isAdmin', action.isAdmin) .set('user', action.user); case actions.LOGOUT_SUCCESS: return state .set('loggedIn', false) - .set('user', null); + .set('user', null) + .set('isAdmin', false); default : return state; } From 9d8968f1ce37e332cc979291d4c61dd9b64e2dcc Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 10:14:13 -0700 Subject: [PATCH 09/28] Corrected action upserting functionality --- models/comment.js | 25 ++++++++++++++++++------- routes/api/comments/index.js | 8 +++----- tests/routes/api/comments/index.js | 2 +- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/models/comment.js b/models/comment.js index 2ac130978..73f8250d7 100644 --- a/models/comment.js +++ b/models/comment.js @@ -161,15 +161,26 @@ CommentSchema.statics.changeStatus = function(id, status) { * @param {String} action the new action to the comment * @return {Promise} */ -CommentSchema.statics.addAction = function(id, user_id, action_type) { - // check that the comment exist - let action = new Action({ - action_type: action_type, +CommentSchema.statics.addAction = function(item_id, user_id, action_type) { + const action = { + item_id, item_type: 'comment', - item_id: id, - user_id: user_id + user_id, + action_type + }; + + // Update/Create the action for the user. + return Action.findOneAndUpdate(action, action, { + + // Ensure that if it's new, we return the new object created. + new: true, + + // Perform an upsert in the event that this doesn't exist. + upsert: true, + + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true }); - return action.save(); }; //============================================================================== diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 1507e82aa..bfb3d67ab 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -29,8 +29,7 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { const { body, asset_id, - parent_id, - author_id + parent_id } = req.body; Comment @@ -39,7 +38,7 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { asset_id, parent_id, status: req.wordlist.matched ? 'rejected' : '', - author_id + author_id: req.user.id }) .then((comment) => { @@ -96,12 +95,11 @@ router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next router.post('/:comment_id/actions', (req, res, next) => { const { - user_id, action_type } = req.body; Comment - .addAction(req.params.comment_id, user_id, action_type) + .addAction(req.params.comment_id, req.user.id, action_type) .then((action) => { res.status(201).json(action); }) diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index be3d14f69..29775d3d0 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -461,7 +461,7 @@ describe('Post /:comment_id/actions', () => { it('it should update actions', () => { return chai.request(app) .post('/api/v1/comments/abc/actions') - .set(passport.inject({roles: ['admin']})) + .set(passport.inject({id: '456', roles: ['admin']})) .send({'user_id': '456', 'action_type': 'flag'}) .then((res) => { expect(res).to.have.status(201); From d34cbc7870206717c66f461bdb24bbfca8718980 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 10:19:33 -0700 Subject: [PATCH 10/28] Moved the action insert into the action model. --- models/action.js | 29 +++++++++++++++++++++++++++++ models/comment.js | 30 ++++++++---------------------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/models/action.js b/models/action.js index 48c0a9a18..891dbd4f5 100644 --- a/models/action.js +++ b/models/action.js @@ -27,6 +27,35 @@ ActionSchema.statics.findById = function(id) { return Action.findOne({id}); }; +/** + * Add an action. + * @param {String} item_id identifier of the comment (uuid) + * @param {String} user_id user id of the action (uuid) + * @param {String} action the new action to the comment + * @return {Promise} + */ +ActionSchema.statics.insertUserAction = ({item_id, item_type, user_id, action_type}) => { + const action = { + item_id, + item_type, + user_id, + action_type + }; + + // Create/Update the action. + return Action.findOneAndUpdate(action, action, { + + // Ensure that if it's new, we return the new object created. + new: true, + + // Perform an upsert in the event that this doesn't exist. + upsert: true, + + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true + }); +}; + /** * Finds actions in an array of ids. * @param {String} ids array of user identifiers (uuid) diff --git a/models/comment.js b/models/comment.js index 73f8250d7..c1a32cf1b 100644 --- a/models/comment.js +++ b/models/comment.js @@ -157,31 +157,17 @@ CommentSchema.statics.changeStatus = function(id, status) { /** * Add an action to the comment. - * @param {String} id identifier of the comment (uuid) + * @param {String} item_id identifier of the comment (uuid) + * @param {String} user_id user id of the action (uuid) * @param {String} action the new action to the comment * @return {Promise} */ -CommentSchema.statics.addAction = function(item_id, user_id, action_type) { - const action = { - item_id, - item_type: 'comment', - user_id, - action_type - }; - - // Update/Create the action for the user. - return Action.findOneAndUpdate(action, action, { - - // Ensure that if it's new, we return the new object created. - new: true, - - // Perform an upsert in the event that this doesn't exist. - upsert: true, - - // Set the default values if not provided based on the mongoose models. - setDefaultsOnInsert: true - }); -}; +CommentSchema.statics.addAction = (item_id, user_id, action_type) => Action.insertUserAction({ + item_id, + item_type: 'comment', + user_id, + action_type +}); //============================================================================== // Remove Statics From c0eb20285889f8eeaadc33884c19f72acdf86810 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 28 Nov 2016 10:57:15 -0700 Subject: [PATCH 11/28] small refactor on items actions --- client/coral-framework/actions/items.js | 31 +++++++++++-------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index f922280c1..4af6dd1b1 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -104,20 +104,18 @@ export function getStream (assetUrl) { .then((json) => { /* Add items to the store */ - const itemTypes = Object.keys(json); - for (let i = 0; i < itemTypes.length; i++ ) { - if (itemTypes[i] === 'actions') { - for (let j = 0; j < json[itemTypes[i]].length; j++ ) { - let action = json[itemTypes[i]][j]; + Object.keys(json).forEach(type => { + if (type === 'actions') { + json[type].forEach(action => { action.id = `${action.action_type}_${action.item_id}`; dispatch(addItem(action, 'actions')); - } + }); } else { - for (let j = 0; j < json[itemTypes[i]].length; j++ ) { - dispatch(addItem(json[itemTypes[i]][j], itemTypes[i])); - } + json[type].forEach(item => { + dispatch(addItem(item, type)); + }); } - } + }); const assetId = json.assets[0].id; @@ -140,15 +138,14 @@ export function getStream (assetUrl) { dispatch(updateItem(assetId, 'comments', rels.rootComments, 'assets')); - const childKeys = Object.keys(rels.childComments); - for (let i = 0; i < childKeys.length; i++ ) { - dispatch(updateItem(childKeys[i], 'children', rels.childComments[childKeys[i]].reverse(), 'comments')); - } + Object.keys(rels.childComments).forEach(key => { + dispatch(updateItem(key, 'children', rels.childComments[key].reverse(), 'comments')); + }); /* Hydrate actions on comments */ - for (let i = 0; i < json.actions.length; i++ ) { - dispatch(updateItem(json.actions[i].item_id, json.actions[i].action_type, json.actions[i].id, 'comments')); - } + json.actions.forEach(action => { + dispatch(updateItem(action.item_id, action.action_type, action.id, 'comments')); + }); return (json); }); From ee0e0ea9ed081fc17b1cd6340c636162b71c8f7a Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 28 Nov 2016 11:17:41 -0700 Subject: [PATCH 12/28] import belen's fetch helpers --- client/coral-admin/src/actions/settings.js | 30 ++-------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/client/coral-admin/src/actions/settings.js b/client/coral-admin/src/actions/settings.js index f71730663..b71a63e39 100644 --- a/client/coral-admin/src/actions/settings.js +++ b/client/coral-admin/src/actions/settings.js @@ -1,3 +1,5 @@ +import {base, handleResp, getInit} from '../helpers/response'; + export const SETTINGS_LOADING = 'SETTINGS_LOADING'; export const SETTINGS_RECEIVED = 'SETTINGS_RECEIVED'; export const SETTINGS_FETCH_ERROR = 'SETTINGS_FETCH_ERROR'; @@ -8,34 +10,6 @@ export const SAVE_SETTINGS_LOADING = 'SAVE_SETTINGS_LOADING'; export const SAVE_SETTINGS_SUCCESS = 'SAVE_SETTINGS_SUCCESS'; export const SAVE_SETTINGS_FAILED = 'SAVE_SETTINGS_FAILED'; -const base = '/api/v1'; - -const getInit = (method, body) => { - const headers = new Headers({ - 'Content-Type': 'application/json', - 'Accept': 'application/json' - }); - - const init = {method, headers}; - if (method.toLowerCase() !== 'get') { - init.body = JSON.stringify(body); - } - - return init; -}; - -const handleResp = res => { - if (res.status === 401) { - throw new Error('Not Authorized to make this request'); - } else if (res.status > 399) { - throw new Error('Error! Status ', res.status); - } else if (res.status === 204) { - return res.text(); - } else { - return res.json(); - } -}; - export const fetchSettings = () => dispatch => { dispatch({type: SETTINGS_LOADING}); fetch(`${base}/settings`, getInit('GET')) From b0f01cf00f73f986c74144dfb7f92039e79b4fa7 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 11:29:39 -0700 Subject: [PATCH 13/28] Initial commit of asset queuing (#97) * Initial commit of asset queuing * Addresssing comments --- .eslintrc.json | 41 ++--- bin/cli | 3 + bin/cli-assets | 76 ++++++++++ bin/cli-jobs | 49 ++++++ bin/cli-serve | 132 +++++++++++++++++ bin/cli-settings | 15 +- bin/cli-users | 85 ++++------- bin/www | 97 ------------ .../coral-embed-stream/src/CommentStream.js | 8 +- client/coral-framework/actions/auth.js | 8 +- models/asset.js | 89 ++++------- package.json | 12 +- routes/api/asset/index.js | 78 ++++++++-- routes/api/auth/index.js | 13 +- routes/api/index.js | 17 ++- routes/api/queue/index.js | 3 +- routes/api/stream/index.js | 91 +++++++++--- routes/api/user/index.js | 5 +- routes/index.js | 4 +- services/scraper.js | 140 ++++++++++++++++++ swagger.yaml | 87 +++++++++++ tests/index.js | 9 -- tests/models/action.js | 2 - tests/models/asset.js | 35 ----- tests/models/comment.js | 2 - tests/models/setting.js | 4 - tests/models/user.js | 2 - tests/{utils => }/mongoose.js | 6 +- tests/{utils => }/passport.js | 2 +- tests/routes/api/assets/index.js | 34 +---- tests/routes/api/auth/index.js | 2 - tests/routes/api/comments/index.js | 5 +- tests/routes/api/queue/index.js | 5 +- tests/routes/api/settings/index.js | 5 +- tests/routes/api/stream/index.js | 6 +- tests/services/scraper.js | 22 +++ util.js | 42 ++++++ views/article.ejs | 4 +- 38 files changed, 832 insertions(+), 408 deletions(-) create mode 100755 bin/cli-assets create mode 100755 bin/cli-jobs create mode 100755 bin/cli-serve delete mode 100755 bin/www create mode 100644 services/scraper.js delete mode 100644 tests/index.js rename tests/{utils => }/mongoose.js (71%) rename tests/{utils => }/passport.js (90%) create mode 100644 tests/services/scraper.js create mode 100644 util.js diff --git a/.eslintrc.json b/.eslintrc.json index 6ac5a08e6..035a86189 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -5,25 +5,15 @@ }, "extends": "eslint:recommended", "rules": { - "indent": [ - "error", + "indent": ["error", 2 ], "no-console": [ 0 ], - "linebreak-style": [ - "error", - "unix" - ], - "quotes": [ - "error", - "single" - ], - "semi": [ - "error", - "always" - ], + "linebreak-style": ["error", "unix"], + "quotes": ["error", "single"], + "semi": ["error", "always"], "no-template-curly-in-string": [1], "no-unsafe-negation": [1], "array-callback-return": [1], @@ -35,7 +25,6 @@ "no-throw-literal": [2], "yoda": [1], "no-path-concat": [2], - "no-process-exit": [2], "eol-last": [1], "no-continue": [1], "no-nested-ternary": [1], @@ -46,20 +35,20 @@ "no-const-assign": [2], "no-duplicate-imports": [2], "prefer-template": [1], - "comma-spacing": [ - "error", - { + "comma-spacing": ["error", { "after": true - } - ], + }], "no-var": [2], "no-lonely-if": [2], "curly": [2], - "no-unused-vars": ["error", { "argsIgnorePattern": "next" }], - "no-multiple-empty-lines": [ - "error", - {"max": 1} - ], - "newline-per-chained-call": ["error", { "ignoreChainWithDepth": 2 }] + "no-unused-vars": ["error", { + "argsIgnorePattern": "next" + }], + "no-multiple-empty-lines": ["error", { + "max": 1 + }], + "newline-per-chained-call": ["error", { + "ignoreChainWithDepth": 2 + }] } } diff --git a/bin/cli b/bin/cli index bec5ea84f..d8148282f 100755 --- a/bin/cli +++ b/bin/cli @@ -19,7 +19,10 @@ const pkg = require('../package.json'); program .version(pkg.version) + .command('serve', 'serve the application') + .command('assets', 'interact with assets') .command('settings', 'work with the application settings') + .command('jobs', 'work with the job queues') .command('users', 'work with the application auth') .parse(process.argv); diff --git a/bin/cli-assets b/bin/cli-assets new file mode 100755 index 000000000..99965c6d9 --- /dev/null +++ b/bin/cli-assets @@ -0,0 +1,76 @@ +#!/usr/bin/env node + +/** + * Setup the debug paramater. + */ + +process.env.DEBUG = process.env.TALK_DEBUG; + +/** + * Module dependencies. + */ + +const program = require('commander'); +const pkg = require('../package.json'); +const Table = require('cli-table'); +const Asset = require('../models/asset'); +const mongoose = require('../mongoose'); +const util = require('../util'); + +// Register the shutdown criteria. +util.onshutdown([ + () => mongoose.disconnect() +]); + +/** + * Lists all the assets registered in the database. + */ +function listAssets() { + Asset + .find({}) + .sort({'created_at': 1}) + .then((asset) => { + let table = new Table({ + head: [ + 'ID', + 'Title', + 'URL' + ] + }); + + asset.forEach((asset) => { + table.push([ + asset.id, + asset.title ? asset.title : '', + asset.url ? asset.url : '' + ]); + }); + + console.log(table.toString()); + util.shutdown(); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); + }); +} + +//============================================================================== +// Setting up the program command line arguments. +//============================================================================== + +program + .version(pkg.version); + +program + .command('list') + .description('list all the assets in the database') + .action(listAssets); + +program.parse(process.argv); + +// If there is no command listed, output help. +if (!process.argv.slice(2).length) { + program.outputHelp(); + util.shutdown(); +} diff --git a/bin/cli-jobs b/bin/cli-jobs new file mode 100755 index 000000000..f14276d38 --- /dev/null +++ b/bin/cli-jobs @@ -0,0 +1,49 @@ +#!/usr/bin/env node + +/** + * Setup the debug paramater. + */ + +process.env.DEBUG = process.env.TALK_DEBUG; + +/** + * Module dependencies. + */ + +const program = require('commander'); +const scraper = require('../services/scraper'); +const util = require('../util'); +const mongoose = require('../mongoose'); + +util.onshutdown([ + () => mongoose.disconnect() +]); + +function processJobs() { + + // Start the processor. + scraper.process(); + + // The scraper only needs to shutdown when the scraper has actually been + // started. + util.onshutdown([ + () => scraper.shutdown() + ]); +} + +//============================================================================== +// Setting up the program command line arguments. +//============================================================================== + +program + .command('process') + .description('starts job processing') + .action(processJobs); + +program.parse(process.argv); + +// If there is no command listed, output help. +if (process.argv.length <= 2) { + program.outputHelp(); + util.shutdown(); +} diff --git a/bin/cli-serve b/bin/cli-serve new file mode 100755 index 000000000..14c1261d7 --- /dev/null +++ b/bin/cli-serve @@ -0,0 +1,132 @@ +#!/usr/bin/env node + +/** + * Setup the debug paramater. + */ + +process.env.DEBUG = process.env.TALK_DEBUG; + +const app = require('../app'); +const debug = require('debug')('talk:server'); +const http = require('http'); +const init = require('../init'); +const scraper = require('../services/scraper'); +const mongoose = require('../mongoose'); +const util = require('../util'); + +/** +* Get port from environment and store in Express. +*/ +const port = normalizePort(process.env.TALK_PORT || '3000'); + +app.set('port', port); + +/** +* Create HTTP server. +*/ +const server = http.createServer(app); + +/** + * Event listener for HTTP server "error" event. + */ +function onError(error) { + if (error.syscall !== 'listen') { + throw error; + } + + let bind = typeof port === 'string' + ? `Pipe ${port}` + : `Port ${port}`; + + // handle specific listen errors with friendly messages + switch (error.code) { + case 'EACCES': + console.error(`${bind} requires elevated privileges`); + break; + case 'EADDRINUSE': + console.error(`${bind} is already in use`); + break; + } + + throw error; +} + +/** + * Normalize a port into a number, string, or false. + */ + +function normalizePort(val) { + let port = parseInt(val, 10); + + if (isNaN(port)) { + // named pipe + return val; + } + + if (port >= 0) { + // port number + return port; + } + + return false; +} + +/** + * Event listener for HTTP server "listening" event. + */ + +function onListening() { + let addr = server.address(); + let bind = typeof addr === 'string' + ? `pipe ${ addr}` + : `port ${ addr.port}`; + debug(`Listening on ${ bind}`); +} + +/** + * Start the app. + */ +function startApp() { + init().then(() => { + + /** + * Listen on provided port, on all network interfaces. + */ + server.listen(port); + server.on('error', onError); + server.on('listening', onListening); + }); +} + +/** + * Module dependencies. + */ + +const program = require('commander'); + +//============================================================================== +// Setting up the program command line arguments. +//============================================================================== + +program + .option('-j, --jobs', 'enable job processing on this thread') + .parse(process.argv); + +// Start the application serving. +startApp(); + +// Enable job processing on the thread if enabled. +if (program.jobs) { + + // Start the processor. + scraper.process(); +} + +// Define a safe shutdown function to call in the event we need to shutdown +// because the node hooks are below which will interrupt the shutdown process. +// Shutdown the mongoose connection, the app server, and the scraper. +util.onshutdown([ + () => program.jobs ? scraper.shutdown() : null, + () => mongoose.disconnect(), + () => server.close() +]); diff --git a/bin/cli-settings b/bin/cli-settings index cff6c04ed..c639a5ca5 100755 --- a/bin/cli-settings +++ b/bin/cli-settings @@ -11,6 +11,14 @@ process.env.DEBUG = process.env.TALK_DEBUG; */ const program = require('commander'); +const mongoose = require('../mongoose'); +const Setting = require('../models/setting'); +const util = require('../util'); + +// Regeister the shutdown criteria. +util.onshutdown([ + () => mongoose.disconnect() +]); //============================================================================== // Setting up the program command line arguments. @@ -20,19 +28,17 @@ program .command('init') .description('initilizes the talk settings') .action(() => { - const mongoose = require('../mongoose'); - const Setting = require('../models/setting'); const defaults = {id: '1', moderation: 'pre'}; Setting .update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}) .then(() => { console.log('Created settings object.'); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(`failed to create the settings object ${JSON.stringify(err)}`); - throw new Error(err); // just to be safe + util.shutdown(1); }); }); @@ -41,4 +47,5 @@ program.parse(process.argv); // If there is no command listed, output help. if (!process.argv.slice(2).length) { program.outputHelp(); + util.shutdown(); } diff --git a/bin/cli-users b/bin/cli-users index 54d7f2927..2e0dc84e6 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -13,14 +13,20 @@ process.env.DEBUG = process.env.TALK_DEBUG; const program = require('commander'); const pkg = require('../package.json'); const prompt = require('prompt'); +const User = require('../models/user'); +const mongoose = require('../mongoose'); +const util = require('../util'); +const Table = require('cli-table'); + +// Regeister the shutdown criteria. +util.onshutdown([ + () => mongoose.disconnect() +]); /** * Prompts for input and registers a user based on those. */ function createUser(options) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - return new Promise((resolve, reject) => { if (options.flag_mode) { @@ -74,11 +80,11 @@ function createUser(options) { }) .then((user) => { console.log(`Created user ${user.id}.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(); }); } @@ -86,20 +92,17 @@ function createUser(options) { * Deletes a user. */ function deleteUser(userID) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .findOneAndRemove({ id: userID }) .then(() => { console.log('Deleted user'); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(); }); } @@ -107,9 +110,6 @@ function deleteUser(userID) { * Changes the password for a user. */ function passwd(userID) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - prompt.start(); prompt.get([ @@ -128,13 +128,13 @@ function passwd(userID) { ], (err, result) => { if (err) { console.error(err); - mongoose.disconnect(); + util.shutdown(); return; } if (result.password !== result.confirmPassword) { console.error(new Error('Password mismatch')); - mongoose.disconnect(); + util.shutdown(1); return; } @@ -142,11 +142,11 @@ function passwd(userID) { .changePassword(userID, result.password) .then(() => { console.log('Password changed.'); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); }); } @@ -155,9 +155,6 @@ function passwd(userID) { * Updates the user from the options array. */ function updateUser(userID, options) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - const updates = []; if (options.email && typeof options.email === 'string' && options.email.length > 0) { @@ -189,11 +186,11 @@ function updateUser(userID, options) { .all(updates.map((q) => q.exec())) .then(() => { console.log(`User ${userID} updated.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -201,10 +198,6 @@ function updateUser(userID, options) { * Lists all the users registered in the database. */ function listUsers() { - const Table = require('cli-table'); - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .all() .then((users) => { @@ -229,11 +222,11 @@ function listUsers() { }); console.log(table.toString()); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -243,18 +236,15 @@ function listUsers() { * @param {String} srcUserID id of the user to which is the source of the merge */ function mergeUsers(dstUserID, srcUserID) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .mergeUsers(dstUserID, srcUserID) .then(() => { console.log(`User ${srcUserID} was merged into user ${dstUserID}.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -264,18 +254,15 @@ function mergeUsers(dstUserID, srcUserID) { * @param {String} role the role to add */ function addRole(userID, role) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .addRoleToUser(userID, role) .then(() => { console.log(`Added the ${role} role to User ${userID}.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -285,18 +272,15 @@ function addRole(userID, role) { * @param {String} role the role to remove */ function removeRole(userID, role) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .removeRoleFromUser(userID, role) .then(() => { console.log(`Removed the ${role} role from User ${userID}.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -305,18 +289,15 @@ function removeRole(userID, role) { * @param {String} userID the ID of a user to disable */ function disableUser(userID) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .disableUser(userID) .then(() => { console.log(`User ${userID} was disabled.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -325,18 +306,15 @@ function disableUser(userID) { * @param {String} userID the ID of a user to enable */ function enableUser(userID) { - const User = require('../models/user'); - const mongoose = require('../mongoose'); - User .enableUser(userID) .then(() => { console.log(`User ${userID} was enabled.`); - mongoose.disconnect(); + util.shutdown(); }) .catch((err) => { console.error(err); - mongoose.disconnect(); + util.shutdown(1); }); } @@ -408,4 +386,5 @@ program.parse(process.argv); // If there is no command listed, output help. if (!process.argv.slice(2).length) { program.outputHelp(); + util.shutdown(); } diff --git a/bin/www b/bin/www deleted file mode 100755 index 3e9e20918..000000000 --- a/bin/www +++ /dev/null @@ -1,97 +0,0 @@ -#!/usr/bin/env node - -/** - * Setup the debug paramater. - */ - -process.env.DEBUG = process.env.TALK_DEBUG; - -/** - * Module dependencies. - */ - -const app = require('../app'); -const debug = require('debug')('talk:server'); -const http = require('http'); -const init = require('../init'); -const port = normalizePort(process.env.TALK_PORT || '3000'); - -let server; - -init().then(() => { - - /** - * Get port from environment and store in Express. - */ - app.set('port', port); - - /** - * Create HTTP server. - */ - server = http.createServer(app); - - /** - * Listen on provided port, on all network interfaces. - */ - server.listen(port); - server.on('error', onError); - server.on('listening', onListening); -}); - -/** - * Normalize a port into a number, string, or false. - */ - -function normalizePort(val) { - let port = parseInt(val, 10); - - if (isNaN(port)) { - // named pipe - return val; - } - - if (port >= 0) { - // port number - return port; - } - - return false; -} - -/** - * Event listener for HTTP server "error" event. - */ - -function onError(error) { - if (error.syscall !== 'listen') { - throw error; - } - - let bind = typeof port === 'string' - ? `Pipe ${ port}` - : `Port ${ port}`; - - // handle specific listen errors with friendly messages - switch (error.code) { - case 'EACCES': - console.error(`${bind} requires elevated privileges`); - break; - case 'EADDRINUSE': - console.error(`${bind} is already in use`); - break; - } - - throw error; -} - -/** - * Event listener for HTTP server "listening" event. - */ - -function onListening() { - let addr = server.address(); - let bind = typeof addr === 'string' - ? `pipe ${ addr}` - : `port ${ addr.port}`; - debug(`Listening on ${ bind}`); -} diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js index a0eaca044..40d249088 100644 --- a/client/coral-embed-stream/src/CommentStream.js +++ b/client/coral-embed-stream/src/CommentStream.js @@ -61,8 +61,12 @@ class CommentStream extends Component { // Set up messaging between embedded Iframe an parent component // Using recommended Pym init code which violates .eslint standards const pym = new Pym.Child({polling: 100}); - const path = /https?\:\/\/([^?]+)/.exec(pym.parentUrl); - this.props.getStream(path && path[1] || window.location); + + if (/https?\:\/\/([^?]+)/.test(pym.parentUrl)) { + this.props.getStream(pym.parentUrl); + } else { + this.props.getStream(window.location); + } } render () { diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 7313bb30f..1058edbbb 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -127,7 +127,13 @@ const checkLoginFailure = error => ({type: actions.CHECK_LOGIN_FAILURE, error}); export const checkLogin = () => dispatch => { dispatch(checkLoginRequest()); fetch(`${base}/auth`, getInit('GET')) - .then(handleResp) + .then((res) => { + if (res.status !== 200) { + throw new Error('not logged in'); + } + + return res.json(); + }) .then(user => dispatch(checkLoginSuccess(user))) .catch(error => dispatch(checkLoginFailure(error))); }; diff --git a/models/asset.js b/models/asset.js index 1092cd3e0..b5321cec3 100644 --- a/models/asset.js +++ b/models/asset.js @@ -3,7 +3,6 @@ const uuid = require('uuid'); const Schema = mongoose.Schema; const AssetSchema = new Schema({ - id: { type: String, default: uuid.v4, @@ -19,12 +18,18 @@ const AssetSchema = new Schema({ type: String, default: 'article' }, - headline: String, - summary: String, + scraped: { + type: Date, + default: null + }, + title: String, + description: String, + image: String, section: String, subsection: String, - authors: [String], - publication_date: Date + author: String, + publication_date: Date, + modified_date: Date }, { versionKey: false, timestamps: { @@ -36,80 +41,42 @@ const AssetSchema = new Schema({ /** * Search for assets. Currently only returns all. */ -AssetSchema.statics.search = function(query) { - - return Asset.find(query).exec(); - -}; +AssetSchema.statics.search = (query) => Asset.find(query); /** * Finds an asset by its id. * @param {String} id identifier of the asset (uuid). */ -AssetSchema.statics.findById = function(id) { - - return Asset.findOne({id}).exec(); - -}; +AssetSchema.statics.findById = (id) => Asset.findOne({id}); /** * Finds a asset by its url. * @param {String} url identifier of the asset (uuid). */ -AssetSchema.statics.findByUrl = function(url) { - - return Asset.findOne({'url': url}).exec(); - -}; +AssetSchema.statics.findByUrl = (url) => Asset.findOne({url}); /** * Finds a asset by its url. + * + * NOTE: This function has scalability concerns regarding mongoose's decision + * always write {updated_at: new Date()} on every call to findOneAndUpdate + * even though the update document exactly matches the query document... In + * the future this function should never update, only findOneAndCreate but this + * is not possible with the mongoose driver. + * * @param {String} url identifier of the asset (uuid). */ -AssetSchema.statics.findOrCreateByUrl = function(url) { +AssetSchema.statics.findOrCreateByUrl = (url) => Asset.findOneAndUpdate({url}, {url}, { - return Asset.findOne({url}) - .then((asset) => asset ? asset - : Asset.upsert({url})); -}; + // Ensure that if it's new, we return the new object created. + new: true, -/** - * Upserts an asset. -*/ -AssetSchema.statics.upsert = function(data) { - // If an id is not sent, create one. - if (typeof data.id === 'undefined') { - data.id = uuid.v4(); - } + // Perform an upsert in the event that this doesn't exist. + upsert: true, - // Perform the upsert against the id field. - let updatePromise = Asset.update({id: data.id}, data, {upsert: true}).exec() - .then(() => { - - // Pull the freshly minted asset out and return. - return Asset.findById(data.id); - - }) - .catch((err) => { - - console.error('Error upserting asset.', err); - //return new Promise(); // ??? what do we return on error? - - }); - - return updatePromise; - -}; - -/** - * Remove assets from the db. - * @param {String} query bson query to identify assets to be removed. -*/ -AssetSchema.statics.removeAll = function(query) { - - return Asset.remove(query).exec(); - -}; + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true +}); const Asset = mongoose.model('Asset', AssetSchema); diff --git a/package.json b/package.json index 9300c347f..bc2f72f20 100644 --- a/package.json +++ b/package.json @@ -4,14 +4,14 @@ "description": "A commenting platform from The Coral Project. https://coralproject.net", "main": "app.js", "scripts": { - "start": "./bin/www", + "start": "./bin/cli serve --jobs", "build": "NODE_ENV=production webpack --config webpack.config.js --bail", "build-watch": "NODE_ENV=development webpack --config webpack.config.dev.js --watch", "lint": "eslint bin/* .", "lint-fix": "eslint . --fix", - "test": "mocha --compilers js:babel-core/register --recursive tests", - "test-watch": "mocha --compilers js:babel-core/register --recursive -w tests", - "embed-start": "NODE_ENV=development npm run build && ./bin/www" + "test": "NODE_ENV=test mocha --compilers js:babel-core/register --recursive tests", + "test-watch": "NODE_ENV=test mocha --compilers js:babel-core/register --recursive -w tests", + "embed-start": "NODE_ENV=development npm run build && ./bin/cli serve --jobs" }, "config": { "pre-git": { @@ -45,14 +45,16 @@ "express-session": "^1.14.2", "helmet": "^3.1.0", "jsonwebtoken": "^7.1.9", + "kue": "^0.11.5", "lodash": "^4.16.6", + "metascraper": "^1.0.6", "mongoose": "^4.6.5", "morgan": "^1.7.0", + "natural": "^0.4.0", "nodemailer": "^2.6.4", "passport": "^0.3.2", "passport-facebook": "^2.1.1", "passport-local": "^1.0.0", - "natural": "^0.4.0", "prompt": "^1.0.0", "react-linkify": "^0.1.3", "redis": "^2.6.3", diff --git a/routes/api/asset/index.js b/routes/api/asset/index.js index 217c49547..18fd0b7ec 100644 --- a/routes/api/asset/index.js +++ b/routes/api/asset/index.js @@ -1,33 +1,81 @@ const express = require('express'); const router = express.Router(); -const Asset = require('../../../models/asset'); -// Search assets. +const Asset = require('../../../models/asset'); +const scraper = require('../../../services/scraper'); + +// List assets. router.get('/', (req, res, next) => { - let query = {}; + const { + limit = 20, + skip = 0, + sort = 'asc', + field = 'created_at' + } = req.query; - if (typeof req.query.url !== 'undefined') { - query.url = req.query.url; - } + // Find all the assets. + Promise.all([ + Asset + .find({}) + .sort({[field]: (sort === 'asc') ? 1 : -1}) + .skip(skip) + .limit(limit), + Asset.count() + ]) + .then(([result, count]) => { - Asset.search(query) - .then((asset) => { - res.json(asset); - }) - .catch(next); + // Send back the asset data. + res.json({ + result, + count + }); + }) + .catch((err) => { + next(err); + }); }); -// Get an asset by id -router.get('/:id', (req, res, next) => { +// Get an asset by id. +router.get('/:asset_id', (req, res, next) => { - Asset.findById(req.params.id) + // Send back the asset. + Asset + .findById(req.params.asset_id) .then((asset) => { + if (!asset) { + return res.status(404).end(); + } + res.json(asset); }) - .catch(next); + .catch((err) => { + next(err); + }); +}); +// Adds the asset id to the queue to be scraped. +router.post('/:asset_id/scrape', (req, res, next) => { + + // Create a new asset scrape job. + Asset + .findById(req.params.asset_id) + .then((asset) => { + if (!asset) { + return res.status(404).end(); + } + + return scraper.create(asset); + }) + .then((job) => { + + // Send the job back for monitoring. + res.status(201).json(job); + }) + .catch((err) => { + next(err); + }); }); module.exports = router; diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index b88b6fe12..369e5ec77 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -7,7 +7,18 @@ const router = express.Router(); /** * This returns the user if they are logged in. */ -router.get('/', authorization.needed(), (req, res) => { +router.get('/', (req, res, next) => { + if (req.user) { + return next(); + } + + // When there is no user on the request, then just send back a 204 to this + // request. It's not really "an error" if what they asked for isn't available, + // but it could be. + res.status(204).end(); +}, (req, res) => { + + // Send back the user object. res.json(req.user.toObject()); }); diff --git a/routes/api/index.js b/routes/api/index.js index 8c6014ed8..8da3f791b 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -3,13 +3,18 @@ const authorization = require('../../middleware/authorization'); const router = express.Router(); -router.use('/asset', require('./asset')); -router.use('/auth', require('./auth')); -router.use('/comments', authorization.needed(), require('./comments')); -router.use('/queue', require('./queue')); +router.use('/asset', authorization.needed('admin'), require('./asset')); router.use('/settings', authorization.needed('admin'), require('./settings')); -router.use('/stream', require('./stream')); -router.use('/user', require('./user')); +router.use('/queue', authorization.needed('admin'), require('./queue')); + +router.use('/comments', authorization.needed(), require('./comments')); router.use('/actions', authorization.needed(), require('./actions')); +router.use('/auth', require('./auth')); +router.use('/stream', require('./stream')); +router.use('/user', require('./user')); + +// Bind the kue handler to the /kue path. +router.use('/kue', authorization.needed('admin'), require('kue').app); + module.exports = router; diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index edc5602ef..f661992f1 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -1,7 +1,6 @@ const express = require('express'); const Comment = require('../../../models/comment'); const Setting = require('../../../models/setting'); -const authorization = require('../../../middleware/authorization'); const router = express.Router(); @@ -13,7 +12,7 @@ const router = express.Router(); // depending on the settings. The :moderation overwrites this settings. // Pre-moderation: New comments are shown in the moderator queues immediately. // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. -router.get('/comments/pending', authorization.needed('admin'), (req, res, next) => { +router.get('/comments/pending', (req, res, next) => { Setting.getModerationSetting().then(function({moderation}){ Comment.moderationQueue(moderation).then((comments) => { res.status(200).json(comments); diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index a48548608..0ef97aad2 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -1,52 +1,99 @@ const express = require('express'); const _ = require('lodash'); +const scraper = require('../../../services/scraper'); const Comment = require('../../../models/comment'); const User = require('../../../models/user'); const Action = require('../../../models/action'); const Asset = require('../../../models/asset'); - const Setting = require('../../../models/setting'); const router = express.Router(); -// Find all the comments by a specific asset_url. -// . if pre: get the comments that are accepted. -// . if post: get the comments that are new and accepted. router.get('/', (req, res, next) => { // Get the asset_id for this url (or create it if it doesn't exist) Promise.all([ - Asset.findOrCreateByUrl(decodeURIComponent(req.query.asset_url)), + + // Find or create the asset by url. + Asset.findOrCreateByUrl(decodeURIComponent(req.query.asset_url)) + + // Add the found asset to the scraper if it's not already scraped. + .then((asset) => { + if (!asset.scraped) { + return scraper.create(asset).then(() => asset); + } + + return asset; + }), + + // Get the moderation setting from the settings. Setting.getModerationSetting() ]) .then(([asset, {moderation}]) => { - // Get the sitewide moderation setting and return the appropriate comments - switch(moderation){ - case 'pre': - return Promise.all([Comment.findAcceptedByAssetId(asset.id), asset]); - case 'post': - return Promise.all([Comment.findAcceptedAndNewByAssetId(asset.id), asset]); - default: - return Promise.reject(new Error('Moderation setting not found.')); + let comments; + + if (moderation === 'post') { + comments = Comment.findAcceptedByAssetId(asset.id); + } else { + + // Defaults to 'pre' moderation. + comments = Comment.findAcceptedAndNewByAssetId(asset.id); } + + return Promise.all([ + + // This is the promised component... Fetch the comments based on the + // moderation settings. + comments, + + // Send back the reference to the asset. + asset + ]); }) // Get all the users and actions for those comments. .then(([comments, asset]) => { + + // Get the user id's from the author id's as a unique array that gets + // sorted. + let userIDs = _.uniq(comments.map((comment) => comment.author_id)).sort(); + + // Fetch the users for which there is a comment available for them. + let users = userIDs.length > 0 ? User.findByIdArray(userIDs) : []; + + // Fetch the actions for pretty much everything at this point. + let actions = Action.getActionSummaries(_.uniq([ + + // Actions can be on assets... + asset.id, + + // Comments... + ...comments.map((comment) => comment.id), + + // Or Authors... + ...userIDs + ]), req.user ? req.user.id : false); + return Promise.all([ - [asset], + + // Pass back the asset that we loaded... + asset, + + // It's comments... comments, - User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), - Action.getActionSummaries(_.uniq([ - asset.id, - ...comments.map((comment) => comment.id), - ...comments.map((comment) => comment.author_id) - ]), req.user ? req.user.id : '') + + // All the users/authors of those comments... + users, + + // And all actions about the asset, comments, and users. + actions ]); }) - .then(([assets, comments, users, actions]) => { + .then(([asset, comments, users, actions]) => { + + // Send back the payload containing all this data. res.json({ - assets, + assets: [asset], comments, users, actions diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 3251e0608..43d3b8ac0 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -16,7 +16,7 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { page = 1, asc = 'false', limit = 50 // Total Per Page - } = req.query; + } = req.query; Promise.all([ User @@ -128,9 +128,10 @@ router.post('/request-password-reset', (req, res, next) => { return mailer.sendSimple(options); }) .then(() => { + // we want to send a 204 regardless of the user being found in the db // if we fail on missing emails, it would reveal if people are registered or not. - res.status(204).send('OK'); + res.status(204).end(); }) .catch(error => { const errorMsg = typeof error === 'string' ? error : error.message; diff --git a/routes/index.js b/routes/index.js index 0c03edcc5..a875d5127 100644 --- a/routes/index.js +++ b/routes/index.js @@ -6,11 +6,11 @@ router.use('/admin', require('./admin')); router.use('/embed', require('./embed')); router.get('/', (req, res) => { - return res.render('article', {title: 'Coral Talk'}); + res.render('article', {title: 'Coral Talk'}); }); router.get('/assets/:asset_title', (req, res) => { - return res.render('article', {title: req.params.asset_title.split('-').join(' ')}); + res.render('article', {title: req.params.asset_title.split('-').join(' ')}); }); module.exports = router; diff --git a/services/scraper.js b/services/scraper.js new file mode 100644 index 000000000..665d4fc66 --- /dev/null +++ b/services/scraper.js @@ -0,0 +1,140 @@ +const kue = require('kue'); +const queue = kue.createQueue(); +const debug = require('debug')('talk:services:scraper'); +const Asset = require('../models/asset'); +const JOB_NAME = 'scraper'; + +const metascraper = require('metascraper'); + +/** + * Exposes a service object to allow operations to execute against the scraper. + * @type {Object} + */ +const scraper = { + + /** + * creates a new scraper job and scrapes the url when it gets processed. + */ + create(asset) { + return new Promise((resolve, reject) => { + debug(`Creating job for Asset[${asset.id}]`); + + let job = queue + .create(JOB_NAME, { + title: `Scrape for asset ${asset.id}`, + asset_id: asset.id + }) + .attempts(10) + .delay(1000) + .backoff({type: 'exponential'}) + .save((err) => { + if (err) { + return reject(err); + } + + debug(`Created Job[${job.id}] for Asset[${asset.id}]`); + + return resolve(job); + }); + }); + }, + + /** + * Scrapes the given asset for metadata. + */ + scrape(asset) { + return metascraper.scrapeUrl(asset.url, Object.assign({}, metascraper.RULES, { + section: ($) => $('meta[property="article:section"]').attr('content'), + modified: ($) => $('meta[property="article:modified"]').attr('content') + })); + }, + + update(id, meta) { + return Asset.update({id}, { + $set: { + title: meta.title || '', + description: meta.description || '', + image: meta.image ? meta.image : '', + author: meta.author || '', + publication_date: meta.date || '', + modified_date: meta.modified || '', + section: meta.section || '', + scraped: new Date() + } + }); + }, + + /** + * Start the queue processor for the scraper job. + */ + process() { + + debug(`Now processing ${JOB_NAME} jobs`); + + // Process jobs with the processJob function. + queue.process(JOB_NAME, (job, done) => { + + debug(`Starting on Job[${job.id}] for Asset[${job.data.asset_id}]`); + + Asset + + // Find the asset, or complain that it doesn't exist. + .findById(job.data.asset_id) + .then((asset) => { + if (!asset) { + throw new Error('asset not found'); + } + + return asset; + }) + + // Scrape the metadata from the asset. + .then(scraper.scrape) + + // Assign the metadata retrieved for the asset to the db. + .then((meta) => { + debug(`Scraped ${JSON.stringify(meta)} on Job[${job.id}] for Asset[${job.data.asset_id}]`); + + return scraper.update(job.data.asset_id, meta); + }) + + // Finish the job because we just handled our scraping + updating the + // asset in the database. + .then(() => { + debug(`Finished on Job[${job.id}] for Asset[${job.data.asset_id}]`); + done(); + }) + + // Handle errors that occur. + .catch((err) => { + console.error(`Failed to scrape on Job[${job.id}] for Asset[${job.data.asset_id}]:`, err); + + done(err); + }); + }); + }, + + /** + * Shuts down the current queue to ensure that the application can shutdown + * cleanly. + */ + shutdown() { + return new Promise((resolve, reject) => { + + // Shutdown and give the queue 5 seconds to shutdown before we start + // killing jobs. + queue.shutdown(5000, (err) => { + if (err) { + return reject(err); + } + + debug(`Processing for ${JOB_NAME} jobs stopped`); + + resolve(); + }); + }); + } + +}; + +module.exports = scraper; diff --git a/swagger.yaml b/swagger.yaml index fd3b0012b..09adb97d6 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -256,6 +256,86 @@ paths: description: An error occured. schema: $ref: '#/definitions/Error' + /asset: + get: + parameters: + - name: limit + in: query + type: number + format: int32 + description: Limit the listing results + - name: skip + in: query + type: number + format: int32 + description: Skip the listing results + - name: sort + in: query + enum: + - asc + - desc + type: string + description: Sorting method + - name: field + in: query + type: string + description: Field to sort by. + responses: + 200: + description: Assets listed. + schema: + type: object + properties: + count: + type: number + description: Total number of assets found. + result: + type: array + items: + $ref: '#/definitions/Asset' + + /asset/{asset_id}: + get: + parameters: + - name: asset_id + in: path + required: true + type: string + format: uuid + responses: + 200: + description: The requested asset. + schema: + $ref: '#/definitions/Asset' + 404: + description: The asset was not found. + 500: + description: An error occured. + schema: + $ref: '#/definitions/Error' + + + /asset/{asset_id}/scrape: + post: + parameters: + - name: asset_id + in: path + required: true + type: string + format: uuid + responses: + 201: + description: The job that was created. + schema: + $ref: '#/definitions/Job' + 404: + description: The asset was not found. + 500: + description: An error occured. + schema: + $ref: '#/definitions/Error' + + /stream: get: tags: @@ -420,3 +500,10 @@ definitions: type: object Settings: type: object + Job: + type: object + properties: + id: + format: number + state: + format: string diff --git a/tests/index.js b/tests/index.js deleted file mode 100644 index 0c3d947ce..000000000 --- a/tests/index.js +++ /dev/null @@ -1,9 +0,0 @@ -const expect = require('chai').expect; - -describe('Comment', () => { - describe('#add', () => { - it('should add a comment', () => { - expect(0).to.be.equal(0); - }); - }); -}); diff --git a/tests/models/action.js b/tests/models/action.js index 87d53b7da..c354b8f83 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -1,5 +1,3 @@ -require('../utils/mongoose'); - const Action = require('../../models/action'); const expect = require('chai').expect; diff --git a/tests/models/asset.js b/tests/models/asset.js index d0c3d3429..a5c4ae62b 100644 --- a/tests/models/asset.js +++ b/tests/models/asset.js @@ -1,7 +1,3 @@ -/* eslint-env node, mocha */ - -require('../utils/mongoose'); - const Asset = require('../../models/asset'); const expect = require('chai').expect; @@ -74,35 +70,4 @@ describe('Asset: model', () => { }); }); }); - - describe('#upsert', ()=> { - it('should insert an asset with no id', () => { - return Asset.upsert({url: 'http://newasset.test.com'}) - .then((asset) => { - expect(asset).to.have.property('id'); - }); - }); - - it('should update an asset when the id exists', () => { - return Asset.upsert({id: 1, url: 'http://new.test.com'}) - .then((asset) => { - expect(asset).to.have.property('id') - .and.to.equal('1'); - expect(asset).to.have.property('url') - .and.to.equal('http://new.test.com'); - }); - }); - }); - - describe('#removeAll', ()=> { - it('should insert an asset with no id', () => { - return Asset.removeAll({id:1}) - .then(() => { - return Asset.findById(1); - }) - .then((result) => { - expect(result).to.be.null; - }); - }); - }); }); diff --git a/tests/models/comment.js b/tests/models/comment.js index f8dd7f9f5..07834a186 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -1,5 +1,3 @@ -require('../utils/mongoose'); - const Comment = require('../../models/comment'); const User = require('../../models/user'); const Action = require('../../models/action'); diff --git a/tests/models/setting.js b/tests/models/setting.js index 3f4dc27fa..a36b363fb 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -1,7 +1,3 @@ -/* eslint-env node, mocha */ - -require('../utils/mongoose'); - const Setting = require('../../models/setting'); const expect = require('chai').expect; diff --git a/tests/models/user.js b/tests/models/user.js index f77543b4b..c8af92054 100644 --- a/tests/models/user.js +++ b/tests/models/user.js @@ -1,5 +1,3 @@ -require('../utils/mongoose'); - const User = require('../../models/user'); const expect = require('chai').expect; diff --git a/tests/utils/mongoose.js b/tests/mongoose.js similarity index 71% rename from tests/utils/mongoose.js rename to tests/mongoose.js index 1549440a6..9d3ba1195 100644 --- a/tests/utils/mongoose.js +++ b/tests/mongoose.js @@ -1,8 +1,4 @@ -const mongoose = require('../../mongoose'); - -// Ensure the NODE_ENV is set to 'test', -// this is helpful when you would like to change behavior when testing. -process.env.NODE_ENV = 'test'; +const mongoose = require('../mongoose'); beforeEach(function (done) { function clearDB() { diff --git a/tests/utils/passport.js b/tests/passport.js similarity index 90% rename from tests/utils/passport.js rename to tests/passport.js index 401e50b77..2d1c53ab9 100644 --- a/tests/utils/passport.js +++ b/tests/passport.js @@ -1,4 +1,4 @@ -const authorization = require('../../middleware/authorization'); +const authorization = require('../middleware/authorization'); // Add the passport middleware here before it's setup. authorization.middleware.push((req, res, next) => { diff --git a/tests/routes/api/assets/index.js b/tests/routes/api/assets/index.js index 542e88c42..fb67b7b48 100644 --- a/tests/routes/api/assets/index.js +++ b/tests/routes/api/assets/index.js @@ -1,36 +1,10 @@ -require('../../../utils/mongoose'); -const passport = require('../../../utils/passport'); +describe('/assets', () => { -const chai = require('chai'); -const server = require('../../../../app'); + describe('GET', () => { -// Setup chai. -chai.should(); -chai.use(require('chai-http')); + it('should return assets that we search for'); + it('should not return assets that we do not search for'); -describe('Asset: routes', () => { - - describe('/GET Asset', () => { - describe('#get', () => { - it('It should get an empty array when there are no assets.', (done) => { - - chai.request(server) - .get('/api/v1/asset') - .set(passport.inject({roles: ['admin']})) - .end((err, res) => { - - if (err) { - throw new Error(err); - } - - res.should.have.status(200); - res.body.should.be.a('array'); - res.body.length.should.be.eql(0); - done(); - }); - - }); - }); }); }); diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index dd49a1472..ddca63fe8 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -1,5 +1,3 @@ -require('../../../utils/mongoose'); - const app = require('../../../../app'); const chai = require('chai'); const expect = chai.expect; diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 29775d3d0..ff7f3a9d4 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -1,7 +1,4 @@ -process.env.NODE_ENV = 'test'; - -require('../../../utils/mongoose'); -const passport = require('../../../utils/passport'); +const passport = require('../../../passport'); const app = require('../../../../app'); const chai = require('chai'); diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index 4eff3d384..74137a49a 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -1,7 +1,4 @@ -process.env.NODE_ENV = 'test'; - -require('../../../utils/mongoose'); -const passport = require('../../../utils/passport'); +const passport = require('../../../passport'); const app = require('../../../../app'); const chai = require('chai'); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index 86feca1eb..9f4466a7f 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -1,7 +1,4 @@ -process.env.NODE_ENV = 'test'; - -require('../../../utils/mongoose'); -const passport = require('../../../utils/passport'); +const passport = require('../../../passport'); const app = require('../../../../app'); const chai = require('chai'); diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index 009184b6b..7d05c9708 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -1,5 +1,3 @@ -require('../../../utils/mongoose'); - const app = require('../../../../app'); const chai = require('chai'); const expect = chai.expect; @@ -92,8 +90,8 @@ describe('api/stream: routes', () => { .then(res => { expect(res).to.have.status(200); expect(res.body.assets.length).to.equal(1); - expect(res.body.comments.length).to.equal(1); - expect(res.body.users.length).to.equal(1); + expect(res.body.comments.length).to.equal(2); + expect(res.body.users.length).to.equal(2); expect(res.body.actions.length).to.equal(1); }); }); diff --git a/tests/services/scraper.js b/tests/services/scraper.js new file mode 100644 index 000000000..f3b5d006c --- /dev/null +++ b/tests/services/scraper.js @@ -0,0 +1,22 @@ +describe('scraper: services', () => { + describe('#create', () => { + it('should create a new kue job'); + }); + + describe('#scrape', () => { + it('should scrape complete information'); + it('should scrape what it can'); + }); + + describe('#update', () => { + it('should update the database record entries from the meta'); + }); + + describe('#process', () => { + it('should start the processor to scrape assets'); + }); + + describe('#shutdown', () => { + it('should shutdown the job processor'); + }); +}); diff --git a/util.js b/util.js new file mode 100644 index 000000000..6907d90cd --- /dev/null +++ b/util.js @@ -0,0 +1,42 @@ +const util = module.exports = {}; + +/** + * Stores an array of functions that should be executed in the event that the + * application needs to shutdown. + * @type {Array} + */ +util.toshutdown = []; + +/** + * Calls all the shutdown functions and then ends the process. + * @param {Number} [defaultCode=0] default return code upon sucesfull shutdown. + */ +util.shutdown = (defaultCode = 0) => { + Promise + .all(util.toshutdown.map((func) => func ? func() : null).filter((func) => func)) + .then(() => { + process.exit(defaultCode); + }) + .catch((err) => { + console.error(err); + + process.exit(1); + }); +}; + +/** + * Waits until an event is triggered by the node runtime and elevates a series + * of jobs to be ran in the event we need to shutdown. + * @param {Array} jobs Array of promise capable shutdown functions that are + * executed. + */ +util.onshutdown = (jobs) => { + + // Add the new jobs to shutdown to the object reference. + util.toshutdown = util.toshutdown.concat(jobs); +}; + +// Attach to the SIGTERM + SIGINT handles to ensure a clean shutdown in the +// event that we have an external event. +process.on('SIGTERM', () => util.shutdown()); +process.on('SIGINT', () => util.shutdown()); diff --git a/views/article.ejs b/views/article.ejs index 7d168f960..71db59593 100644 --- a/views/article.ejs +++ b/views/article.ejs @@ -1,8 +1,10 @@ - + + + From 0d7014f77a0c980c188501a704a70e6c1aeac2cf Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 28 Nov 2016 11:50:25 -0700 Subject: [PATCH 14/28] add helpers to talk-adapter --- .../coral-admin/src/services/talk-adapter.js | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/client/coral-admin/src/services/talk-adapter.js b/client/coral-admin/src/services/talk-adapter.js index 361a6479e..efbbf6ddc 100644 --- a/client/coral-admin/src/services/talk-adapter.js +++ b/client/coral-admin/src/services/talk-adapter.js @@ -1,3 +1,4 @@ +import {base, handleResp, getInit} from '../helpers/response'; /** * The adapter is a redux middleware that interecepts the actions that need @@ -35,11 +36,11 @@ export default store => next => action => { const fetchModerationQueueComments = store => Promise.all([ - fetch('/api/v1/queue/comments/pending'), - fetch('/api/v1/comments?status=rejected'), - fetch('/api/v1/comments?action_type=flag') + fetch(`${base}/queue/comments/pending`, getInit('GET')), + fetch(`${base}/comments?status=rejected`, getInit('GET')), + fetch(`${base}/comments?action_type=flag`, getInit('GET')) ]) -.then(res => Promise.all(res.map(r => r.json()))) +.then(res => Promise.all(res.map(handleResp))) .then(res => { res[2] = res[2].map(comment => { comment.flagged = true; return comment; }); return res.reduce((prev, curr) => prev.concat(curr), []); @@ -51,26 +52,22 @@ Promise.all([ // Update a comment. Now to update a comment we need to send back the whole object const updateComment = (store, comment) => { - fetch(`/api/v1/comments/${comment.get('id')}/status`, { - method: 'PUT', - headers: jsonHeader, - body: JSON.stringify({status: comment.get('status')}) - }) - .then(res => res.json()) + fetch(`${base}/comments/${comment.get('id')}/status`, getInit('PUT', {status: comment.get('status')})) + .then(handleResp) .then(res => store.dispatch({type: 'COMMENT_UPDATE_SUCCESS', res})) .catch(error => store.dispatch({type: 'COMMENT_UPDATE_FAILED', error})); }; // Create a new comment -const createComment = (store, name, comment) => -fetch('/api/v1/comments', { - method: 'POST', - body: JSON.stringify({ +const createComment = (store, name, comment) => { + const body = { status: 'Untouched', body: comment, name: name, createdAt: Date.now() - }) -}).then(res => res.json()) -.then(res => store.dispatch({type: 'COMMENT_CREATE_SUCCESS', comment: res})) -.catch(error => store.dispatch({type: 'COMMENT_CREATE_FAILED', error})); + }; + return fetch(`${base}/comments`, getInit('POST', body)) + .then(handleResp) + .then(res => store.dispatch({type: 'COMMENT_CREATE_SUCCESS', comment: res})) + .catch(error => store.dispatch({type: 'COMMENT_CREATE_FAILED', error})); +}; From 552e174b0d5895a19915dd7234d30f475a9e96a1 Mon Sep 17 00:00:00 2001 From: Dan Zajdband Date: Mon, 28 Nov 2016 14:23:22 -0500 Subject: [PATCH 15/28] fix(embed): Disabled framing middleware enabling placing the comment box in third-party sites. In the future we will count with a whitelist so we can adjust the middleware --- app.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app.js b/app.js index 8008cb01e..48ede6e63 100644 --- a/app.js +++ b/app.js @@ -22,7 +22,10 @@ if (app.get('env') !== 'test') { //============================================================================== app.set('trust proxy', 1); -app.use(helmet()); +// We disable frameward on helmet to allow crossdomain injection of the embed +app.use(helmet({ + frameguard: false +})); app.use(bodyParser.json()); app.use('/client', express.static(path.join(__dirname, 'dist'))); app.set('views', path.join(__dirname, 'views')); From 25d53116f71c88665a6824a616ce46a04705bebe Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 28 Nov 2016 12:49:29 -0700 Subject: [PATCH 16/28] remove unused var --- client/coral-admin/src/services/talk-adapter.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/client/coral-admin/src/services/talk-adapter.js b/client/coral-admin/src/services/talk-adapter.js index efbbf6ddc..eeb799452 100644 --- a/client/coral-admin/src/services/talk-adapter.js +++ b/client/coral-admin/src/services/talk-adapter.js @@ -8,9 +8,6 @@ import {base, handleResp, getInit} from '../helpers/response'; * for the coral but also for wordpress comments, disqus and many more. */ -// Default headers for json payloads. -const jsonHeader = new Headers({'Content-Type': 'application/json'}); - // Intercept redux actions and act over the ones we are interested export default store => next => action => { From f6e423e9455a7786c499da1036b13c5d50d86d03 Mon Sep 17 00:00:00 2001 From: Dan Zajdband Date: Mon, 28 Nov 2016 15:09:26 -0500 Subject: [PATCH 17/28] Fixed linting issues --- app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.js b/app.js index 48ede6e63..a30d1a202 100644 --- a/app.js +++ b/app.js @@ -24,7 +24,7 @@ if (app.get('env') !== 'test') { app.set('trust proxy', 1); // We disable frameward on helmet to allow crossdomain injection of the embed app.use(helmet({ - frameguard: false + frameguard: false })); app.use(bodyParser.json()); app.use('/client', express.static(path.join(__dirname, 'dist'))); From 616dd534d9cb1f4a7439f7b83c20780c307afd36 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 14:05:06 -0700 Subject: [PATCH 18/28] Moved kue redis connector --- app.js | 2 +- cache.js | 8 +++--- kue.js | 11 ++++++++ redis.js | 62 ++++++++++++++++++++++++--------------------- routes/api/index.js | 2 +- services/scraper.js | 9 +++---- 6 files changed, 55 insertions(+), 39 deletions(-) create mode 100644 kue.js diff --git a/app.js b/app.js index 8008cb01e..e84eb4b79 100644 --- a/app.js +++ b/app.js @@ -45,7 +45,7 @@ const session_opts = { }, store: new RedisStore({ ttl: 1800, - client: redis, + client: redis.createClient(), }) }; diff --git a/cache.js b/cache.js index efe689f9c..9345f8cde 100644 --- a/cache.js +++ b/cache.js @@ -1,6 +1,8 @@ const redis = require('./redis'); -const cache = module.exports = {}; +const cache = module.exports = { + client: redis.createClient() +}; /** * This collects a key that may either be an array or a string and creates a @@ -51,7 +53,7 @@ cache.wrap = (key, expiry, work) => { * @return {Promise} */ cache.get = (key) => new Promise((resolve, reject) => { - redis.get(keyfunc(key), (err, reply) => { + cache.client.get(keyfunc(key), (err, reply) => { if (err) { return reject(err); } @@ -87,7 +89,7 @@ cache.set = (key, value, expiry) => new Promise((resolve, reject) => { // Serialize the value as JSON. let reply = JSON.stringify(value); - redis.set(keyfunc(key), reply, 'EX', expiry, (err) => { + cache.client.set(keyfunc(key), reply, 'EX', expiry, (err) => { if (err) { return reject(err); } diff --git a/kue.js b/kue.js new file mode 100644 index 000000000..e2229d424 --- /dev/null +++ b/kue.js @@ -0,0 +1,11 @@ +const kue = require('kue'); +const redis = require('./redis'); + +module.exports = { + queue: kue.createQueue({ + redis: { + createClientFactory: () => redis.createClient() + } + }), + kue +}; diff --git a/redis.js b/redis.js index c37fcc64e..9f67c34bb 100644 --- a/redis.js +++ b/redis.js @@ -2,38 +2,42 @@ const redis = require('redis'); const debug = require('debug')('talk:redis'); const url = process.env.TALK_REDIS_URL || 'redis://localhost'; -const client = redis.createClient(url, { - retry_strategy: function(options) { - if (options.error && options.error.code === 'ECONNREFUSED') { +module.exports = { + createClient() { + let client = redis.createClient(url, { + retry_strategy: function(options) { + if (options.error && options.error.code === 'ECONNREFUSED') { - // End reconnecting on a specific error and flush all commands with a individual error - return new Error('The server refused the connection'); - } - if (options.total_retry_time > 1000 * 60 * 60) { + // End reconnecting on a specific error and flush all commands with a individual error + return new Error('The server refused the connection'); + } + if (options.total_retry_time > 1000 * 60 * 60) { - // End reconnecting after a specific timeout and flush all commands with a individual error - return new Error('Retry time exhausted'); - } + // End reconnecting after a specific timeout and flush all commands with a individual error + return new Error('Retry time exhausted'); + } - if (options.times_connected > 10) { + if (options.times_connected > 10) { - // End reconnecting with built in error - return undefined; - } + // End reconnecting with built in error + return undefined; + } - // reconnect after - return Math.max(options.attempt * 100, 3000); + // reconnect after + return Math.max(options.attempt * 100, 3000); + } + }); + + client.ping((err) => { + if (err) { + console.error('Can\'t ping the redis server!'); + + throw err; + } + + debug('connection established'); + }); + + return client; } -}); - -client.ping((err) => { - if (err) { - console.error('Can\'t ping the redis server!'); - - throw err; - } - - debug('connection established'); -}); - -module.exports = client; +}; diff --git a/routes/api/index.js b/routes/api/index.js index 8da3f791b..9b4f0432b 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -15,6 +15,6 @@ router.use('/stream', require('./stream')); router.use('/user', require('./user')); // Bind the kue handler to the /kue path. -router.use('/kue', authorization.needed('admin'), require('kue').app); +router.use('/kue', authorization.needed('admin'), require('../../kue').kue.app); module.exports = router; diff --git a/services/scraper.js b/services/scraper.js index 665d4fc66..922ef77bc 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -1,5 +1,4 @@ -const kue = require('kue'); -const queue = kue.createQueue(); +const kue = require('../kue'); const debug = require('debug')('talk:services:scraper'); const Asset = require('../models/asset'); const JOB_NAME = 'scraper'; @@ -19,7 +18,7 @@ const scraper = { return new Promise((resolve, reject) => { debug(`Creating job for Asset[${asset.id}]`); - let job = queue + let job = kue.queue .create(JOB_NAME, { title: `Scrape for asset ${asset.id}`, asset_id: asset.id @@ -72,7 +71,7 @@ const scraper = { debug(`Now processing ${JOB_NAME} jobs`); // Process jobs with the processJob function. - queue.process(JOB_NAME, (job, done) => { + kue.queue.process(JOB_NAME, (job, done) => { debug(`Starting on Job[${job.id}] for Asset[${job.data.asset_id}]`); @@ -123,7 +122,7 @@ const scraper = { // Shutdown and give the queue 5 seconds to shutdown before we start // killing jobs. - queue.shutdown(5000, (err) => { + kue.queue.shutdown(5000, (err) => { if (err) { return reject(err); } From ca5049feeb2effd7a8001303b6dd3c67f8cedf6a Mon Sep 17 00:00:00 2001 From: David Jay Date: Mon, 28 Nov 2016 16:52:00 -0500 Subject: [PATCH 19/28] Adding settings to stream. --- routes/api/stream/index.js | 20 +++++++++++--------- tests/routes/api/stream/index.js | 9 +++++---- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index acbfe3d77..a22fdd691 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -18,21 +18,21 @@ router.get('/', (req, res, next) => { // Get the asset_id for this url (or create it if it doesn't exist) Promise.all([ Asset.findOrCreateByUrl(decodeURIComponent(req.query.asset_url)), - Setting.getModerationSetting() + Setting.getSettings() ]) - .then(([asset, {moderation}]) => { + .then(([asset, settings]) => { // Get the sitewide moderation setting and return the appropriate comments - switch(moderation){ + switch(settings.moderation){ case 'pre': - return Promise.all([Comment.findAcceptedByAssetId(asset.id), asset]); + return Promise.all([Comment.findAcceptedByAssetId(asset.id), asset, settings]); case 'post': - return Promise.all([Comment.findAcceptedAndNewByAssetId(asset.id), asset]); + return Promise.all([Comment.findAcceptedAndNewByAssetId(asset.id), asset, settings]); default: return Promise.reject(new Error('Moderation setting not found.')); } }) // Get all the users and actions for those comments. - .then(([comments, asset]) => { + .then(([comments, asset, settings]) => { return Promise.all([ [asset], comments, @@ -41,15 +41,17 @@ router.get('/', (req, res, next) => { asset.id, ...comments.map((comment) => comment.id), ...comments.map((comment) => comment.author_id) - ])) + ])), + settings ]); }) - .then(([assets, comments, users, actions]) => { + .then(([assets, comments, users, actions, settings]) => { res.json({ assets, comments, users, - actions + actions, + settings }); }) .catch(error => { diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index 009184b6b..3f032c1d7 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -91,10 +91,11 @@ describe('api/stream: routes', () => { .query({'asset_url': 'http://test.com'}) .then(res => { expect(res).to.have.status(200); - expect(res.body.assets.length).to.equal(1); - expect(res.body.comments.length).to.equal(1); - expect(res.body.users.length).to.equal(1); - expect(res.body.actions.length).to.equal(1); + expect(res.body.assets[0]).to.have.property('url'); + expect(res.body.comments[0]).to.have.property('body'); + expect(res.body.users[0]).to.have.property('displayName'); + expect(res.body.actions[0]).to.have.property('action_type'); + expect(res.body.settings).to.have.property('moderation'); }); }); }); From ab46c020d9c1ea1d58003d176840f8add0e8ea15 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 15:01:13 -0700 Subject: [PATCH 20/28] Added asset searching --- bin/cli-assets | 38 +++++++++++++++ bin/cli-jobs | 69 ++++++++++++++++++++++++++++ models/asset.js | 32 +++++++++++-- package.json | 17 +++++-- routes/api/asset/index.js | 9 ++-- tests/mongoose.js | 4 +- tests/routes/api/assets/index.js | 79 +++++++++++++++++++++++++++++++- 7 files changed, 234 insertions(+), 14 deletions(-) diff --git a/bin/cli-assets b/bin/cli-assets index 99965c6d9..9ab36685a 100755 --- a/bin/cli-assets +++ b/bin/cli-assets @@ -12,9 +12,11 @@ process.env.DEBUG = process.env.TALK_DEBUG; const program = require('commander'); const pkg = require('../package.json'); +const parseDuration = require('parse-duration'); const Table = require('cli-table'); const Asset = require('../models/asset'); const mongoose = require('../mongoose'); +const scraper = require('../services/scraper'); const util = require('../util'); // Register the shutdown criteria. @@ -55,6 +57,37 @@ function listAssets() { }); } +function refreshAssets(ageString) { + const now = new Date().getTime(); + const ageMs = parseDuration(ageString); + const age = new Date(now - ageMs); + + Asset.find({ + $or: [ + { + scraped: { + $lte: age + } + }, + { + scraped: null + } + ] + }) + + // Queue all the assets for scraping. + .then((assets) => Promise.all(assets.map(scraper.create))) + + .then(() => { + console.log('Assets were queued to be scraped'); + util.shutdown(); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); + }); +} + //============================================================================== // Setting up the program command line arguments. //============================================================================== @@ -67,6 +100,11 @@ program .description('list all the assets in the database') .action(listAssets); +program + .command('refresh ') + .description('queues the assets that exceed the age requested') + .action(refreshAssets); + program.parse(process.argv); // If there is no command listed, output help. diff --git a/bin/cli-jobs b/bin/cli-jobs index f14276d38..bc320d14b 100755 --- a/bin/cli-jobs +++ b/bin/cli-jobs @@ -14,11 +14,15 @@ const program = require('commander'); const scraper = require('../services/scraper'); const util = require('../util'); const mongoose = require('../mongoose'); +const kue = require('../kue'); util.onshutdown([ () => mongoose.disconnect() ]); +/** + * Starts the job processor. + */ function processJobs() { // Start the processor. @@ -31,6 +35,65 @@ function processJobs() { ]); } +/** + * Removes a single job. + * @param {Object} job the job to be removed + * @return {Promise} + */ +function removeJob(job) { + return new Promise((resolve, reject) => job.remove((err) => { + if (err) { + return reject(err); + } + + return resolve(job); + })); +} + +/** + * Removes the jobs passed in and returns a promise. + * @param {Array} jobs array of jobs + * @return {Promise} + */ +function removeJobs(jobs) { + return Promise.all(jobs.map(removeJob)); +} + +/** + * Get the top n jobs with a specific state. + * @param {String} [state='complete'] state to list jobs by + * @param {Number} limit limit of jobs to load + * @return {Promise} + */ +function rangeJobsByState(state = 'complete', limit) { + return new Promise((resolve, reject) => { + kue.Job.rangeByState(state, 0, limit, 'asc', (err, jobs) => { + if (err) { + return reject(err); + } + + resolve(jobs); + }); + }); +} + +/** + * Cleans up the jobs that are in the queue. + */ +function cleanupJobs(options) { + const n = 100; + + Promise.all([ + rangeJobsByState('complete', n), + options.stuck ? rangeJobsByState('failed', n) : false + ]) + .then((joblists) => joblists.filter((jobs) => jobs).map(removeJobs)) + .then(() => { + util.shutdown(); + console.log('Removed old jobs'); + }); +} + //============================================================================== // Setting up the program command line arguments. //============================================================================== @@ -40,6 +103,12 @@ program .description('starts job processing') .action(processJobs); +program + .command('cleanup') + .option('-s, --stuck', 'cleans up jobs that have been stuck', false) + .description('cleans up inactive jobs') + .action(cleanupJobs); + program.parse(process.argv); // If there is no command listed, output help. diff --git a/models/asset.js b/models/asset.js index b5321cec3..14d5675df 100644 --- a/models/asset.js +++ b/models/asset.js @@ -38,21 +38,32 @@ const AssetSchema = new Schema({ } }); +AssetSchema.index({ + title: 'text', + url: 'text', + description: 'text', + section: 'text', + subsection: 'text', + author: 'text' +}, { + background: true +}); + /** * Search for assets. Currently only returns all. -*/ + */ AssetSchema.statics.search = (query) => Asset.find(query); /** * Finds an asset by its id. * @param {String} id identifier of the asset (uuid). -*/ + */ AssetSchema.statics.findById = (id) => Asset.findOne({id}); /** * Finds a asset by its url. * @param {String} url identifier of the asset (uuid). -*/ + */ AssetSchema.statics.findByUrl = (url) => Asset.findOne({url}); /** @@ -65,7 +76,8 @@ AssetSchema.statics.findByUrl = (url) => Asset.findOne({url}); * is not possible with the mongoose driver. * * @param {String} url identifier of the asset (uuid). -*/ + * @return {Promise} + */ AssetSchema.statics.findOrCreateByUrl = (url) => Asset.findOneAndUpdate({url}, {url}, { // Ensure that if it's new, we return the new object created. @@ -78,6 +90,18 @@ AssetSchema.statics.findOrCreateByUrl = (url) => Asset.findOneAndUpdate({url}, { setDefaultsOnInsert: true }); +/** + * Finds assets matching keywords on the model. If `value` is an empty string, + * then it will not even perform a text search query. + * @param {String} value string to search by. + * @return {Promise} + */ +AssetSchema.statics.search = (value) => value.length === 0 ? Asset.find({}) : Asset.find({ + $text: { + $search: value + } +}); + const Asset = mongoose.model('Asset', AssetSchema); module.exports = Asset; diff --git a/package.json b/package.json index bc2f72f20..8d0256af2 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,13 @@ "config": { "pre-git": { "commit-msg": [], - "pre-commit": ["npm run lint", "npm test"], - "pre-push": ["npm test"], + "pre-commit": [ + "npm run lint", + "npm test" + ], + "pre-push": [ + "npm test" + ], "post-commit": [], "post-merge": [] } @@ -26,7 +31,12 @@ "type": "git", "url": "git+https://github.com/coralproject/talk.git" }, - "keywords": ["talk", "coral", "coralproject", "ask"], + "keywords": [ + "talk", + "coral", + "coralproject", + "ask" + ], "author": "", "license": "Apache-2.0", "bugs": { @@ -52,6 +62,7 @@ "morgan": "^1.7.0", "natural": "^0.4.0", "nodemailer": "^2.6.4", + "parse-duration": "^0.1.1", "passport": "^0.3.2", "passport-facebook": "^2.1.1", "passport-local": "^1.0.0", diff --git a/routes/api/asset/index.js b/routes/api/asset/index.js index 18fd0b7ec..0c740d32f 100644 --- a/routes/api/asset/index.js +++ b/routes/api/asset/index.js @@ -11,17 +11,20 @@ router.get('/', (req, res, next) => { limit = 20, skip = 0, sort = 'asc', - field = 'created_at' + field = 'created_at', + search = '' } = req.query; // Find all the assets. Promise.all([ Asset - .find({}) + .search(search) .sort({[field]: (sort === 'asc') ? 1 : -1}) .skip(skip) .limit(limit), - Asset.count() + Asset + .search(search) + .count() ]) .then(([result, count]) => { diff --git a/tests/mongoose.js b/tests/mongoose.js index 9d3ba1195..74a0b04f7 100644 --- a/tests/mongoose.js +++ b/tests/mongoose.js @@ -2,8 +2,8 @@ const mongoose = require('../mongoose'); beforeEach(function (done) { function clearDB() { - for (let i in mongoose.connection.collections) { - mongoose.connection.collections[i].remove(function() {}); + for (let collection in mongoose.connection.collections) { + mongoose.connection.collections[collection].remove(function() {}); } return done(); } diff --git a/tests/routes/api/assets/index.js b/tests/routes/api/assets/index.js index fb67b7b48..667bec0ed 100644 --- a/tests/routes/api/assets/index.js +++ b/tests/routes/api/assets/index.js @@ -1,9 +1,84 @@ +const passport = require('../../../passport'); + +const app = require('../../../../app'); +const chai = require('chai'); +const expect = chai.expect; + +// Setup chai. +chai.should(); +chai.use(require('chai-http')); + +const Asset = require('../../../../models/asset'); + describe('/assets', () => { + beforeEach(() => { + return Asset.create([ + { + url: 'https://coralproject.net/news/asset1', + title: 'Asset 1', + description: 'term1' + }, + { + url: 'https://coralproject.net/news/asset2', + title: 'Asset 2', + description: 'term2' + } + ]); + }); + describe('GET', () => { - it('should return assets that we search for'); - it('should not return assets that we do not search for'); + it('should return all assets without a search query', () => { + return chai.request(app) + .get('/api/v1/asset') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + const body = res.body; + + expect(body).to.have.property('count', 2); + expect(body).to.have.property('result'); + + const assets = body.result; + + expect(assets).to.have.length(2); + }); + }); + + it('should return assets that we search for', () => { + return chai.request(app) + .get('/api/v1/asset?search=term2') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + const body = res.body; + + expect(body).to.have.property('count', 1); + expect(body).to.have.property('result'); + + const assets = body.result; + + expect(assets).to.have.length(1); + + const asset = assets[0]; + + expect(asset).to.have.property('url', 'https://coralproject.net/news/asset2'); + expect(asset).to.have.property('title', 'Asset 2'); + }); + }); + + it('should not return assets that we do not search for', () => { + return chai.request(app) + .get('/api/v1/asset?search=term3') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + const body = res.body; + + expect(body).to.have.property('count', 0); + expect(body).to.have.property('result'); + + expect(body.result).to.be.empty; + }); + }); }); From 823c223cf12c0be75c515915288d06ea8a7dde36 Mon Sep 17 00:00:00 2001 From: David Jay Date: Mon, 28 Nov 2016 17:11:37 -0500 Subject: [PATCH 21/28] Adding settings to redux store from stream. --- client/coral-embed-stream/src/index.js | 4 +-- client/coral-framework/actions/config.js | 35 ----------------------- client/coral-framework/actions/items.js | 17 +++++++++++ client/coral-framework/index.js | 2 -- client/coral-framework/reducers/config.js | 13 +++------ 5 files changed, 22 insertions(+), 49 deletions(-) delete mode 100644 client/coral-framework/actions/config.js diff --git a/client/coral-embed-stream/src/index.js b/client/coral-embed-stream/src/index.js index f2a584ed1..de14edbc7 100644 --- a/client/coral-embed-stream/src/index.js +++ b/client/coral-embed-stream/src/index.js @@ -2,9 +2,7 @@ import React from 'react'; import {render} from 'react-dom'; import CommentStream from './CommentStream'; import {Provider} from 'react-redux'; -import {fetchConfig, store} from '../../coral-framework'; - -store.dispatch(fetchConfig()); +import {store} from '../../coral-framework'; render( diff --git a/client/coral-framework/actions/config.js b/client/coral-framework/actions/config.js deleted file mode 100644 index d8fd886be..000000000 --- a/client/coral-framework/actions/config.js +++ /dev/null @@ -1,35 +0,0 @@ -import {fromJS} from 'immutable'; - -/** - * Action name constants - */ - -export const FETCH_CONFIG_REQUEST = 'FETCH_CONFIG_REQUEST'; -export const FETCH_CONFIG_FAILED = 'FETCH_CONFIG_FAILED'; -export const FETCH_CONFIG_SUCCESS = 'FETCH_CONFIG_SUCCESS'; - -/** - * Action creators - */ - -export function fetchConfig () { - return (dispatch) => { - - dispatch({type: FETCH_CONFIG_REQUEST}); - - return fetch('/api/v1/settings') - .then( - response => { - return response.ok ? response.json() - : Promise.reject(`${response.status} ${response.statusText}`); - } - ) - .then((json) => { - return dispatch({type: FETCH_CONFIG_SUCCESS, config: fromJS(json)}); - }) - .catch((error) => { - dispatch({type: FETCH_CONFIG_FAILED, error}); - }); - - }; -} diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index f922280c1..5ca358811 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -1,3 +1,4 @@ +import {fromJS} from 'immutable'; /* Item Actions */ /** @@ -6,6 +7,7 @@ export const ADD_ITEM = 'ADD_ITEM'; export const UPDATE_ITEM = 'UPDATE_ITEM'; +export const UPDATE_SETTINGS = 'UPDATE_SETTINGS'; export const APPEND_ITEM_ARRAY = 'APPEND_ITEM_ARRAY'; const getInit = (method, body) => { @@ -61,6 +63,7 @@ export const addItem = (item, item_type) => { * id - the id of the item to be posted * property - the property to be updated * value - the value that the property should be set to +* item_type - the type of the item being updated (users, comments, etc) * */ export const updateItem = (id, property, value, item_type) => { @@ -73,6 +76,18 @@ export const updateItem = (id, property, value, item_type) => { }; }; +/* +* Appends data to an array in an item in the local store without posting it to the server +* Useful for adding a recently posted reply to a comment, etc. +* +* @params +* id - the id of the item to be posted +* property - the property to be updated (should be an array) +* value - the value that should be added to the array +* add_to_front - boolean that defines whether value is added at the beginning (unshift) or end (push) +* item_type - the type of the item being updated (users, comments, etc) +* +*/ export const appendItemArray = (id, property, value, add_to_front, item_type) => { return { type: APPEND_ITEM_ARRAY, @@ -112,6 +127,8 @@ export function getStream (assetUrl) { action.id = `${action.action_type}_${action.item_id}`; dispatch(addItem(action, 'actions')); } + } else if (itemTypes[i] === 'settings') { + return dispatch({type: UPDATE_SETTINGS, config: fromJS(json[itemTypes[i]])}); } else { for (let j = 0; j < json[itemTypes[i]].length; j++ ) { dispatch(addItem(json[itemTypes[i]][j], itemTypes[i])); diff --git a/client/coral-framework/index.js b/client/coral-framework/index.js index 4c6ae741f..837c8956a 100644 --- a/client/coral-framework/index.js +++ b/client/coral-framework/index.js @@ -1,6 +1,5 @@ import Notification from './modules/notification/Notification'; import store from './store'; -import {fetchConfig} from './actions/config'; import * as itemActions from './actions/items'; import I18n from './modules/i18n/i18n'; import * as notificationActions from './actions/notification'; @@ -9,7 +8,6 @@ import * as authActions from './actions/auth'; export { Notification, store, - fetchConfig, itemActions, I18n, notificationActions, diff --git a/client/coral-framework/reducers/config.js b/client/coral-framework/reducers/config.js index cbc131fe6..6521d92a3 100644 --- a/client/coral-framework/reducers/config.js +++ b/client/coral-framework/reducers/config.js @@ -1,7 +1,7 @@ /* @flow */ import {Map} from 'immutable'; -import * as actions from '../actions/config'; +import * as actions from '../actions/items'; const initialState = Map({ features: Map({}) @@ -9,15 +9,10 @@ const initialState = Map({ export default (state = initialState, action) => { switch(action.type) { - case actions.FETCH_CONFIG_REQUEST: - return state.set('loading', true); - case actions.FETCH_CONFIG_FAILED: - return state.set('loading', false); - - // Override config if worked - case actions.FETCH_CONFIG_SUCCESS: - return action.config.set('loading', false); + // Override config if worked + case actions.UPDATE_SETTINGS: + return action.config; default: return state; From 461e80e26045885dd27a69380920dd1f77ed3e3d Mon Sep 17 00:00:00 2001 From: David Jay Date: Mon, 28 Nov 2016 17:43:24 -0500 Subject: [PATCH 22/28] Removing switch in steam endpoint. --- routes/api/stream/index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 17965387b..7b793cdb6 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -31,14 +31,14 @@ router.get('/', (req, res, next) => { ]) .then(([asset, settings]) => { // Get the sitewide moderation setting and return the appropriate comments - switch(settings.moderation){ - case 'pre': - return Promise.all([Comment.findAcceptedByAssetId(asset.id), asset, settings]); - case 'post': - return Promise.all([Comment.findAcceptedAndNewByAssetId(asset.id), asset, settings]); - default: - return Promise.reject(new Error('Moderation setting not found.')); + let comments; + if (settings.moderation === 'pre') { + comments = Comment.findAcceptedByAssetId(asset.id); + } else { + comments = Comment.findAcceptedAndNewByAssetId(asset.id); } + + return Promise.all([comments, asset, settings]); }) .then(([comments, asset, settings]) => { From d5ff0f8f281c9a328f1c6acf21756d341eafc122 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 28 Nov 2016 16:18:19 -0700 Subject: [PATCH 23/28] keep it DRY. add getInit all over the place. update tests --- client/coral-admin/src/actions/auth.js | 2 +- client/coral-admin/src/actions/community.js | 2 +- client/coral-admin/src/actions/settings.js | 2 +- .../coral-admin/src/services/talk-adapter.js | 2 +- client/coral-framework/actions/items.js | 42 ++++++------------- client/coral-framework/helpers/response.js | 4 +- .../coral-framework/store/itemActions.spec.js | 1 + 7 files changed, 19 insertions(+), 36 deletions(-) diff --git a/client/coral-admin/src/actions/auth.js b/client/coral-admin/src/actions/auth.js index 2c77ffce7..2f8f1041e 100644 --- a/client/coral-admin/src/actions/auth.js +++ b/client/coral-admin/src/actions/auth.js @@ -1,5 +1,5 @@ import * as actions from '../constants/auth'; -import {base, handleResp, getInit} from '../helpers/response'; +import {base, handleResp, getInit} from '../../../coral-framework/helpers/response'; // Check Login diff --git a/client/coral-admin/src/actions/community.js b/client/coral-admin/src/actions/community.js index 7a4112f8b..5921573d1 100644 --- a/client/coral-admin/src/actions/community.js +++ b/client/coral-admin/src/actions/community.js @@ -9,7 +9,7 @@ import { SET_ROLE } from '../constants/community'; -import {base, getInit, handleResp} from '../helpers/response'; +import {base, getInit, handleResp} from '../../../coral-framework/helpers/response'; export const fetchCommenters = (query = {}) => dispatch => { dispatch(requestFetchCommenters()); diff --git a/client/coral-admin/src/actions/settings.js b/client/coral-admin/src/actions/settings.js index b71a63e39..6a133ddb5 100644 --- a/client/coral-admin/src/actions/settings.js +++ b/client/coral-admin/src/actions/settings.js @@ -1,4 +1,4 @@ -import {base, handleResp, getInit} from '../helpers/response'; +import {base, handleResp, getInit} from '../../../coral-framework/helpers/response'; export const SETTINGS_LOADING = 'SETTINGS_LOADING'; export const SETTINGS_RECEIVED = 'SETTINGS_RECEIVED'; diff --git a/client/coral-admin/src/services/talk-adapter.js b/client/coral-admin/src/services/talk-adapter.js index eeb799452..6b872d12d 100644 --- a/client/coral-admin/src/services/talk-adapter.js +++ b/client/coral-admin/src/services/talk-adapter.js @@ -1,4 +1,4 @@ -import {base, handleResp, getInit} from '../helpers/response'; +import {base, handleResp, getInit} from '../../../coral-framework/helpers/response'; /** * The adapter is a redux middleware that interecepts the actions that need diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index 4af6dd1b1..e7d28fbe4 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -1,3 +1,5 @@ +import {getInit, base, handleResp} from '../../coral-framework/helpers/response'; + /* Item Actions */ /** @@ -8,26 +10,6 @@ export const ADD_ITEM = 'ADD_ITEM'; export const UPDATE_ITEM = 'UPDATE_ITEM'; export const APPEND_ITEM_ARRAY = 'APPEND_ITEM_ARRAY'; -const getInit = (method, body) => { - const headers = { - 'Content-Type': 'application/json', - 'Accept': 'application/json' - }; - - const init = {method, headers}; - if (body) { - init.body = JSON.stringify(body); - } - - return init; -}; - -const responseHandler = response => { - if (response.status === 204) { - return; - } - return response.ok ? response.json() : Promise.reject(`${response.status} ${response.statusText}`); -}; /** * Action creators */ @@ -99,8 +81,8 @@ export const appendItemArray = (id, property, value, add_to_front, item_type) => */ export function getStream (assetUrl) { return (dispatch) => { - return fetch(`/api/v1/stream?asset_url=${encodeURIComponent(assetUrl)}`) - .then(responseHandler) + return fetch(`${base}/stream?asset_url=${encodeURIComponent(assetUrl)}`) + .then(handleResp) .then((json) => { /* Add items to the store */ @@ -168,8 +150,8 @@ export function getStream (assetUrl) { export function getItemsArray (ids) { return (dispatch) => { - return fetch(`/v1/item/${ids}`, getInit('GET')) - .then(responseHandler) + return fetch(`${base}/item/${ids}`, getInit('GET')) + .then(handleResp) .then((json) => { for (let i = 0; i < json.items.length; i++) { dispatch(addItem(json.items[i])); @@ -198,8 +180,8 @@ export function postItem (item, type, id) { if (id) { item.id = id; } - return fetch(`/api/v1/${type}`, getInit('POST', item)) - .then(responseHandler) + return fetch(`${base}/${type}`, getInit('POST', item)) + .then(handleResp) .then((json) => { dispatch(addItem({...item, id:json.id}, type)); return json.id; @@ -229,8 +211,8 @@ export function postAction (item_id, action_type, user_id, item_type) { user_id }; - return fetch(`/api/v1/${item_type}/${item_id}/actions`, getInit('POST', action)) - .then(responseHandler); + return fetch(`${base}/${item_type}/${item_id}/actions`, getInit('POST', action)) + .then(handleResp); }; } @@ -251,7 +233,7 @@ export function postAction (item_id, action_type, user_id, item_type) { export function deleteAction (action_id) { return () => { - return fetch(`/api/v1/actions/${action_id}`, {method: 'DELETE'}) - .then(responseHandler); + return fetch(`${base}/actions/${action_id}`, {method: 'DELETE'}) + .then(handleResp); }; } diff --git a/client/coral-framework/helpers/response.js b/client/coral-framework/helpers/response.js index bccfc5a04..83f51e3ec 100644 --- a/client/coral-framework/helpers/response.js +++ b/client/coral-framework/helpers/response.js @@ -3,10 +3,10 @@ export const base = '/api/v1'; export const getInit = (method, body) => { let init = { method, - headers: new Headers({ + headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' - }), + }, credentials: 'same-origin' }; diff --git a/tests/client/coral-framework/store/itemActions.spec.js b/tests/client/coral-framework/store/itemActions.spec.js index 340ec9549..45c41025f 100644 --- a/tests/client/coral-framework/store/itemActions.spec.js +++ b/tests/client/coral-framework/store/itemActions.spec.js @@ -127,6 +127,7 @@ describe('itemActions', () => { 'Accept': 'application/json', 'Content-Type':'application/json' }, + credentials: 'same-origin', body: JSON.stringify(item.data) } ); From c0a9ea9905819e94f4b2709a91651e6f5e9c9931 Mon Sep 17 00:00:00 2001 From: David Jay Date: Mon, 28 Nov 2016 18:19:07 -0500 Subject: [PATCH 24/28] Returning all settings from api/v1/settings. --- routes/api/settings/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 6e8c64d4f..910524fb2 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -1,6 +1,5 @@ const express = require('express'); const Setting = require('../../../models/setting'); -const _ = require('lodash'); const router = express.Router(); @@ -8,8 +7,7 @@ router.get('/', (req, res, next) => { Setting .getSettings() .then(settings => { - const whitelist = ['moderation']; - res.json(_.pick(settings, whitelist)); + res.json(settings); }) .catch(next); }); From 3448752f6fbd2dd142bfa1fe72aaf4cbc92503ff Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 17:13:11 -0700 Subject: [PATCH 25/28] Added asset settings + improved tests for routes --- models/asset.js | 19 +- models/user.js | 1 + routes/api/asset/index.js | 15 + routes/api/stream/index.js | 29 +- swagger.yaml | 22 ++ tests/models/asset.js | 28 +- tests/models/setting.js | 2 + tests/routes/api/assets/index.js | 4 +- tests/routes/api/auth/index.js | 57 ++-- tests/routes/api/comments/index.js | 459 ++++++++++------------------- tests/routes/api/queue/index.js | 31 +- tests/routes/api/settings/index.js | 71 ++--- tests/routes/api/stream/index.js | 66 +++-- 13 files changed, 396 insertions(+), 408 deletions(-) diff --git a/models/asset.js b/models/asset.js index 14d5675df..00251985a 100644 --- a/models/asset.js +++ b/models/asset.js @@ -1,7 +1,8 @@ const mongoose = require('../mongoose'); -const uuid = require('uuid'); const Schema = mongoose.Schema; +const uuid = require('uuid'); + const AssetSchema = new Schema({ id: { type: String, @@ -22,6 +23,10 @@ const AssetSchema = new Schema({ type: Date, default: null }, + settings: { + type: Schema.Types.Mixed, + default: null + }, title: String, description: String, image: String, @@ -90,6 +95,18 @@ AssetSchema.statics.findOrCreateByUrl = (url) => Asset.findOneAndUpdate({url}, { setDefaultsOnInsert: true }); +/** + * Updates the settings for the asset. + * @param {[type]} id [description] + * @param {[type]} settings [description] + * @return {[type]} [description] + */ +AssetSchema.statics.overrideSettings = (id, settings) => Asset.update({id}, { + $set: { + settings + } +}); + /** * Finds assets matching keywords on the model. If `value` is an empty string, * then it will not even perform a text search query. diff --git a/models/user.js b/models/user.js index bde2363c2..845b84d19 100644 --- a/models/user.js +++ b/models/user.js @@ -9,6 +9,7 @@ const SALT_ROUNDS = 10; // USER_ROLES is the array of roles that is permissible as a user role. const USER_ROLES = [ + '', 'admin', 'moderator' ]; diff --git a/routes/api/asset/index.js b/routes/api/asset/index.js index 0c740d32f..96b83a969 100644 --- a/routes/api/asset/index.js +++ b/routes/api/asset/index.js @@ -81,4 +81,19 @@ router.post('/:asset_id/scrape', (req, res, next) => { }); }); +router.put('/:asset_id/settings', (req, res, next) => { + + // Override the settings for the asset. + Asset + .overrideSettings(req.params.asset_id, req.body) + .then(() => { + + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); + +}); + module.exports = router; diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 0ef97aad2..95ec4774f 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -30,10 +30,18 @@ router.get('/', (req, res, next) => { // Get the moderation setting from the settings. Setting.getModerationSetting() ]) - .then(([asset, {moderation}]) => { + .then(([asset, settings]) => { + + // Merge the asset specific settings with the returned settings object in + // the event that the asset that was returned also had settings. + if (asset.settings) { + settings = Object.assign(settings, asset.settings); + } + + // Fetch the appropriate comments stream. let comments; - if (moderation === 'post') { + if (settings.moderation === 'post') { comments = Comment.findAcceptedByAssetId(asset.id); } else { @@ -48,11 +56,14 @@ router.get('/', (req, res, next) => { comments, // Send back the reference to the asset. - asset + asset, + + // Send back the settings to the stream. + settings ]); }) // Get all the users and actions for those comments. - .then(([comments, asset]) => { + .then(([comments, asset, settings]) => { // Get the user id's from the author id's as a unique array that gets // sorted. @@ -86,17 +97,21 @@ router.get('/', (req, res, next) => { users, // And all actions about the asset, comments, and users. - actions + actions, + + // Pass back the settings that we loaded. + settings ]); }) - .then(([asset, comments, users, actions]) => { + .then(([asset, comments, users, actions, settings]) => { // Send back the payload containing all this data. res.json({ assets: [asset], comments, users, - actions + actions, + settings }); }) .catch(error => { diff --git a/swagger.yaml b/swagger.yaml index 09adb97d6..e819f451c 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -335,6 +335,28 @@ paths: schema: $ref: '#/definitions/Error' + /asset/{asset_id}/settings: + put: + parameters: + - name: asset_id + in: path + required: true + type: string + format: uuid + - name: body + in: body + required: true + schema: + $ref: '#/definitions/Settings' + responses: + 204: + description: The asset settings were updated. + 404: + description: The asset was not found. + 500: + description: An error occured. + schema: + $ref: '#/definitions/Error' /stream: get: diff --git a/tests/models/asset.js b/tests/models/asset.js index a5c4ae62b..69bf99f9d 100644 --- a/tests/models/asset.js +++ b/tests/models/asset.js @@ -1,5 +1,10 @@ const Asset = require('../../models/asset'); -const expect = require('chai').expect; + +const chai = require('chai'); +const expect = chai.expect; + +// Use the chai should. +chai.should(); describe('Asset: model', () => { @@ -53,6 +58,27 @@ describe('Asset: model', () => { }); }); + describe('#overrideSettings', () => { + it('should update the settings', () => { + return Asset + .findOrCreateByUrl('https://override.test.com/asset') + .then((asset) => { + expect(asset).to.have.property('settings'); + expect(asset.settings).to.be.null; + + return Asset.overrideSettings(asset.id, {moderation: 'pre'}); + }) + .then(() => { + return Asset.findOrCreateByUrl('https://override.test.com/asset'); + }) + .then((asset) => { + expect(asset).to.have.property('settings'); + expect(asset.settings).is.an('object'); + expect(asset.settings).to.have.property('moderation', 'pre'); + }); + }); + }); + describe('#findOrCreateByUrl', ()=> { it('should find an asset by a url', () => { return Asset.findOrCreateByUrl('http://test.com') diff --git a/tests/models/setting.js b/tests/models/setting.js index a36b363fb..08a47d7a1 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -14,6 +14,7 @@ describe('Setting: model', () => { expect(settings).to.have.property('moderation').and.to.equal('pre'); }); }); + it('should have two infoBox fields defined', () => { return Setting.getSettings().then(settings => { expect(settings).to.have.property('infoBoxEnable').and.to.equal(false); @@ -26,6 +27,7 @@ describe('Setting: model', () => { it('should update the settings with a passed object', () => { const mockSettings = {moderation: 'post', infoBoxEnable: true, infoBoxContent: 'yeah'}; return Setting.updateSettings(mockSettings).then(updatedSettings => { + expect(updatedSettings).to.be.an('object'); expect(updatedSettings).to.have.property('moderation').and.to.equal('post'); expect(updatedSettings).to.have.property('infoBoxEnable', true); expect(updatedSettings).to.have.property('infoBoxContent', 'yeah'); diff --git a/tests/routes/api/assets/index.js b/tests/routes/api/assets/index.js index 667bec0ed..c56e5b1ba 100644 --- a/tests/routes/api/assets/index.js +++ b/tests/routes/api/assets/index.js @@ -10,7 +10,7 @@ chai.use(require('chai-http')); const Asset = require('../../../../models/asset'); -describe('/assets', () => { +describe('/api/v1/assets', () => { beforeEach(() => { return Asset.create([ @@ -27,7 +27,7 @@ describe('/assets', () => { ]); }); - describe('GET', () => { + describe('#get', () => { it('should return all assets without a search query', () => { return chai.request(app) diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index ddca63fe8..dd408d135 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -6,32 +6,47 @@ chai.use(require('chai-http')); const User = require('../../../../models/user'); -describe('POST /auth/local', () => { +describe('/api/v1/auth', () => { + describe('#get', () => { + it('should return nothing when no user is logged in', () => { + return chai.request(app) + .get('/api/v1/auth') + .then((res) => { + expect(res.status).to.be.equal(204); + expect(res.body).to.be.empty; + }); + }); + }); +}); + +describe('/api/v1/auth/local', () => { beforeEach(() => { return User.createLocalUser('maria@gmail.com', 'password!', 'Maria'); }); - it('should send back the user on a successful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!'}) - .catch((res) => { - expect(res).to.have.status(200); - expect(res).to.be.json; - expect(res.body).to.have.property('user'); - expect(res.body.user).to.have.property('displayName', 'Maria'); - }); - }); + describe('#post', () => { + it('should send back the user on a successful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!'}) + .catch((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('user'); + expect(res.body.user).to.have.property('displayName', 'Maria'); + }); + }); - it('should not send back the user on a unsuccessful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!3'}) - .catch((err) => { - expect(err).to.not.be.null; - expect(err.response).to.have.status(401); - expect(err.response.body).to.have.property('message', 'not authorized'); - }); + it('should not send back the user on a unsuccessful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!3'}) + .catch((err) => { + expect(err).to.not.be.null; + expect(err.response).to.have.status(401); + expect(err.response.body).to.have.property('message', 'not authorized'); + }); + }); }); }); diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index ff7f3a9d4..5ac9af7b7 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -16,11 +16,7 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -beforeEach(() => { - return Setting.create(settings); -}); - -describe('Get /comments', () => { +describe('/api/v1/comments', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -32,61 +28,11 @@ describe('Get /comments', () => { asset_id: 'asset', author_id: '456' }, { - id: 'hij', - body: 'comment 30', - asset_id: '456' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('should return all the comments', () => { - return chai.request(app) - .get('/api/v1/comments') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - - expect(res).to.have.status(200); - - }); - }); -}); - -describe('Get comments by status and action', () => { - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123', - status: 'rejected' - }, { - id: 'def', + id: 'def-rejected', body: 'comment 20', asset_id: 'asset', - author_id: '456' + author_id: '456', + status: 'rejected' }, { id: 'hij', body: 'comment 30', @@ -117,109 +63,100 @@ describe('Get comments by status and action', () => { beforeEach(() => { return Promise.all([ Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('should return all the rejected comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=rejected') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'abc'); - }); - }); - - it('should return all the approved comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=accepted') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'hij'); - }); - }); - - it('should return all the new comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=new') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'def'); - }); - }); - - it('should return all the flagged comments', () => { - return chai.request(app) - .get('/api/v1/comments?action_type=flag') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - - expect(res.body.length).to.equal(1); - expect(res.body[0]).to.have.property('id', 'abc'); - - }); - }); -}); - -describe('Post /comments', () => { - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ User.createLocalUsers(users), Action.create(actions), wordlist.insert([ 'bad words' - ]) + ]), + Setting.create(settings) ]); }); - it('should create a comment', () => { - return chai.request(app) - .post('/api/v1/comments') - .set(passport.inject({roles: []})) - .send({'body': 'Something body.', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) - .then((res) => { - expect(res).to.have.status(201); - expect(res.body).to.have.property('id'); - }); + describe('#get', () => { + it('should return all the comments', () => { + return chai.request(app) + .get('/api/v1/comments') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + + expect(res).to.have.status(200); + + }); + }); + + it('should return all the rejected comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=rejected') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body[0]).to.have.property('id', 'def-rejected'); + }); + }); + + it('should return all the approved comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=accepted') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body).to.have.length(1); + expect(res.body[0]).to.have.property('id', 'hij'); + }); + }); + + it('should return all the new comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=new') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body).to.have.length(2); + }); + }); + + it('should return all the flagged comments', () => { + return chai.request(app) + .get('/api/v1/comments?action_type=flag') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + + expect(res.body).to.have.length(1); + expect(res.body[0]).to.have.property('id', 'abc'); + + }); + }); }); - it('should create a comment with a rejected status if it contains a bad word', () => { - return chai.request(app) - .post('/api/v1/comments') - .set(passport.inject({roles: []})) - .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) - .then((res) => { - expect(res).to.have.status(201); - expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('status', 'rejected'); - }); + describe('#post', () => { + + it('should create a comment', () => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + }); + }); + + it('should create a comment with a rejected status if it contains a bad word', () => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + expect(res.body).to.have.property('status', 'rejected'); + }); + }); }); }); -describe('Get /:comment_id', () => { +describe('/api/v1/comments/:comment_id', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -264,79 +201,65 @@ describe('Get /:comment_id', () => { ]); }); - it('should return the right comment for the comment_id', () => { - return chai.request(app) - .get('/api/v1/comments/abc') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res).to.have.property('body'); - expect(res.body).to.have.property('body', 'comment 10'); + describe('#get', () => { - }); + it('should return the right comment for the comment_id', () => { + return chai.request(app) + .get('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.have.property('body'); + expect(res.body).to.have.property('body', 'comment 10'); + + }); + }); + }); + + describe('#delete', () => { + it('it should remove comment', () => { + return chai.request(app) + .delete('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(204); + + return Comment.findById('abc'); + }) + .then((comment) => { + expect(comment).to.be.null; + }); + }); + }); + + describe('#put', () => { + it('it should update status', function() { + return chai.request(app) + .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: ['admin']})) + .send({status: 'accepted'}) + .then((res) => { + expect(res).to.have.status(204); + expect(res.body).to.be.empty; + }); + }); + + it('it should not allow a non-admin to update status', () => { + return chai.request(app) + .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: []})) + .send({status: 'accepted'}) + .then((res) => { + expect(res).to.be.empty; + }) + .catch((err) => { + expect(err).to.have.property('status', 401); + }); + }); }); }); -describe('Remove /:comment_id', () => { - - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123' - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset', - author_id: '456' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('it should remove comment', () => { - return chai.request(app) - .delete('/api/v1/comments/abc') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(204); - - return Comment.findById('abc'); - }) - .then((comment) => { - expect(comment).to.be.null; - }); - }); -}); - -describe('Put /:comment_id/status', () => { +describe('/api/v1/comments/:comment_id/actions', () => { const comments = [{ id: 'abc', @@ -383,90 +306,20 @@ describe('Put /:comment_id/status', () => { ]); }); - it('it should update status', function() { - return chai.request(app) - .put('/api/v1/comments/abc/status') - .set(passport.inject({roles: ['admin']})) - .send({status: 'accepted'}) - .then((res) => { - expect(res).to.have.status(204); - expect(res.body).to.be.empty; - }); - }); - - it('it should not allow a non-admin to update status', () => { - return chai.request(app) - .put('/api/v1/comments/abc/status') - .set(passport.inject({roles: []})) - .send({status: 'accepted'}) - .then((res) => { - expect(res).to.be.empty; - }) - .catch((err) => { - expect(err).to.have.property('status', 401); - }); - }); -}); - -describe('Post /:comment_id/actions', () => { - - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123', - status: '' - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset', - author_id: '456', - status: 'rejected' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456', - status: 'accepted' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('it should update actions', () => { - return chai.request(app) - .post('/api/v1/comments/abc/actions') - .set(passport.inject({id: '456', roles: ['admin']})) - .send({'user_id': '456', 'action_type': 'flag'}) - .then((res) => { - expect(res).to.have.status(201); - expect(res).to.have.body; - expect(res.body).to.have.property('item_type', 'comment'); - expect(res.body).to.have.property('action_type', 'flag'); - expect(res.body).to.have.property('item_id', 'abc'); - expect(res.body).to.have.property('user_id', '456'); - }); + describe('#post', () => { + it('it should update actions', () => { + return chai.request(app) + .post('/api/v1/comments/abc/actions') + .set(passport.inject({id: '456', roles: ['admin']})) + .send({'user_id': '456', 'action_type': 'flag'}) + .then((res) => { + expect(res).to.have.status(201); + expect(res).to.have.body; + expect(res.body).to.have.property('item_type', 'comment'); + expect(res.body).to.have.property('action_type', 'flag'); + expect(res.body).to.have.property('item_id', 'abc'); + expect(res.body).to.have.property('user_id', '456'); + }); + }); }); }); diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index 74137a49a..733e5a1be 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -15,11 +15,7 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -beforeEach(() => { - return Setting.create(settings); -}); - -describe('Get moderation queues rejected, pending, flags', () => { +describe('/api/v1/queue', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -62,19 +58,22 @@ describe('Get moderation queues rejected, pending, flags', () => { return Promise.all([ Comment.create(comments), User.createLocalUsers(users), - Action.create(actions) + Action.create(actions), + Setting.create(settings) ]); }); - it('should return all the pending comments', function(done){ - chai.request(app) - .get('/api/v1/queue/comments/pending') - .set(passport.inject({roles: ['admin']})) - .end(function(err, res){ - expect(err).to.be.null; - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'def'); - done(); - }); + describe('#get', () => { + it('should return all the pending comments', function(done){ + chai.request(app) + .get('/api/v1/queue/comments/pending') + .set(passport.inject({roles: ['admin']})) + .end(function(err, res){ + expect(err).to.be.null; + expect(res).to.have.status(200); + expect(res.body[0]).to.have.property('id', 'def'); + done(); + }); + }); }); }); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index 9f4466a7f..d1a7ba81b 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -10,49 +10,42 @@ chai.use(require('chai-http')); const Setting = require('../../../../models/setting'); const defaults = {id: '1', moderation: 'pre'}; -describe('GET /settings', () => { +describe('/api/v1/settings', () => { - beforeEach(() => { - return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); + beforeEach(() => Setting.create(defaults)); + + describe('#get', () => { + + it('should return a settings object', () => { + return chai.request(app) + .get('/api/v1/settings') + .set(passport.inject({ + roles: ['admin'] + })) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('moderation', 'pre'); + }); + }); }); - it('should return a settings object', () => { - return chai.request(app) - .get('/api/v1/settings') - .set(passport.inject({ - roles: ['admin'] - })) - .then((res) => { - expect(res).to.have.status(200); - expect(res).to.be.json; - expect(res.body).to.have.property('moderation', 'pre'); - }); - }); -}); + describe('#put', () => { -// update the settings. -describe('update settings', () => { - it('should respond ok to a PUT', () => { - return Setting - .update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}) - .then(() => { - return chai.request(app) - .put('/api/v1/settings') - .set(passport.inject({ - roles: ['admin'] - })) - .send({moderation: 'post'}); - }) - .then(res => { - expect(res).to.have.status(204); + it('should update the settings', () => { + return chai.request(app) + .put('/api/v1/settings') + .set(passport.inject({roles: ['admin']})) + .send({moderation: 'post'}) + .then((res) => { + expect(res).to.have.status(204); - return Setting.getSettings(); - }) - .then(settings => { - - // confirm updated settings in db - expect(settings).to.have.property('moderation'); - expect(settings.moderation).to.equal('post'); - }); + return Setting.getSettings(); + }) + .then((settings) => { + expect(settings).to.have.property('moderation', 'post'); + }); + }); }); + }); diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index 7d05c9708..2b86f00ff 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -13,9 +13,12 @@ const Asset = require('../../../../models/asset'); const Setting = require('../../../../models/setting'); -describe('api/stream: routes', () => { +describe('/api/v1/stream', () => { - const settings = {id: '1', moderation: 'pre'}; + const settings = { + id: '1', + moderation: 'pre' + }; const comments = [{ id: 'abc', @@ -35,7 +38,7 @@ describe('api/stream: routes', () => { asset_id: 'asset', author_id: '456', parent_id: '', - status: '' + status: 'accepted' }, { id: 'hij', body: 'comment 40', @@ -65,15 +68,26 @@ describe('api/stream: routes', () => { return Promise.all([ User.createLocalUsers(users), - Asset.findOrCreateByUrl('http://test.com') + Asset.findOrCreateByUrl('http://test.com'), + Asset + .findOrCreateByUrl('http://coralproject.net/asset2') + .then((asset) => { + return Asset + .overrideSettings(asset.id, {moderation: 'post'}) + .then(() => asset); + }) ]) - .then(([users, asset]) => { + .then(([users, asset1, asset2]) => { comments[0].author_id = users[0].id; comments[1].author_id = users[1].id; + comments[2].author_id = users[0].id; + comments[3].author_id = users[1].id; - comments[0].asset_id = asset.id; - comments[1].asset_id = asset.id; + comments[0].asset_id = asset1.id; + comments[1].asset_id = asset1.id; + comments[2].asset_id = asset2.id; + comments[3].asset_id = asset2.id; return Promise.all([ Comment.create(comments), @@ -83,16 +97,32 @@ describe('api/stream: routes', () => { }); }); - it('should return a stream with comments, users and actions for an existing asset', () => { - return chai.request(app) - .get('/api/v1/stream') - .query({'asset_url': 'http://test.com'}) - .then(res => { - expect(res).to.have.status(200); - expect(res.body.assets.length).to.equal(1); - expect(res.body.comments.length).to.equal(2); - expect(res.body.users.length).to.equal(2); - expect(res.body.actions.length).to.equal(1); - }); + describe('#get', () => { + it('should return a stream with comments, users and actions for an existing asset', () => { + return chai.request(app) + .get('/api/v1/stream') + .query({'asset_url': 'http://test.com'}) + .then(res => { + expect(res).to.have.status(200); + expect(res.body.assets.length).to.equal(1); + expect(res.body.comments.length).to.equal(2); + expect(res.body.users.length).to.equal(2); + expect(res.body.actions.length).to.equal(1); + expect(res.body.settings).to.have.property('moderation', 'pre'); + }); + }); + + it('should merge the settings when the asset contains settings to override it with', () => { + return chai.request(app) + .get('/api/v1/stream') + .query({'asset_url': 'http://coralproject.net/asset2'}) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body.assets.length).to.equal(1); + expect(res.body.comments.length).to.equal(1); + expect(res.body.users.length).to.equal(1); + expect(res.body.settings).to.have.property('moderation', 'post'); + }); + }); }); }); From ea2ee68ed629fa928a5bfc33f800c4f341133144 Mon Sep 17 00:00:00 2001 From: David Jay Date: Tue, 29 Nov 2016 11:44:54 -0500 Subject: [PATCH 26/28] Test db clear (#116) * Adding headers to stream request. * Switching mongodb when node_env === test. --- client/coral-framework/actions/items.js | 4 ++-- mongoose.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index 4587b174b..00deba5ef 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -1,4 +1,4 @@ -import {getInit, base, handleResp} from '../../coral-framework/helpers/response'; +import {getInit, base, handleResp} from '../helpers/response'; import {fromJS} from 'immutable'; /* Item Actions */ @@ -95,7 +95,7 @@ export const appendItemArray = (id, property, value, add_to_front, item_type) => */ export function getStream (assetUrl) { return (dispatch) => { - return fetch(`${base}/stream?asset_url=${encodeURIComponent(assetUrl)}`) + return fetch(`${base}/stream?asset_url=${encodeURIComponent(assetUrl)}`, getInit('GET')) .then(handleResp) .then((json) => { diff --git a/mongoose.js b/mongoose.js index 712b2fcb0..0121a203d 100644 --- a/mongoose.js +++ b/mongoose.js @@ -1,7 +1,11 @@ const mongoose = require('mongoose'); const debug = require('debug')('talk:db'); const enabled = require('debug').enabled; -const url = process.env.TALK_MONGO_URL || 'mongodb://localhost'; +let url = process.env.TALK_MONGO_URL || 'mongodb://localhost'; + +if (process.env.NODE_ENV === 'test') { + url = 'mongodb://localhost/coral-test'; +} // Use native promises mongoose.Promise = global.Promise; From ac17fc2cc601d60ea4d97c369e1a75a6e3739dfd Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Nov 2016 17:13:11 -0700 Subject: [PATCH 27/28] Added asset settings + improved tests for routes --- models/asset.js | 19 +- models/user.js | 1 + routes/api/asset/index.js | 15 + routes/api/stream/index.js | 32 +- swagger.yaml | 22 ++ tests/models/asset.js | 28 +- tests/models/setting.js | 2 + tests/routes/api/assets/index.js | 4 +- tests/routes/api/auth/index.js | 57 ++-- tests/routes/api/comments/index.js | 459 ++++++++++------------------- tests/routes/api/queue/index.js | 31 +- tests/routes/api/settings/index.js | 71 ++--- tests/routes/api/stream/index.js | 67 +++-- 13 files changed, 401 insertions(+), 407 deletions(-) diff --git a/models/asset.js b/models/asset.js index 14d5675df..00251985a 100644 --- a/models/asset.js +++ b/models/asset.js @@ -1,7 +1,8 @@ const mongoose = require('../mongoose'); -const uuid = require('uuid'); const Schema = mongoose.Schema; +const uuid = require('uuid'); + const AssetSchema = new Schema({ id: { type: String, @@ -22,6 +23,10 @@ const AssetSchema = new Schema({ type: Date, default: null }, + settings: { + type: Schema.Types.Mixed, + default: null + }, title: String, description: String, image: String, @@ -90,6 +95,18 @@ AssetSchema.statics.findOrCreateByUrl = (url) => Asset.findOneAndUpdate({url}, { setDefaultsOnInsert: true }); +/** + * Updates the settings for the asset. + * @param {[type]} id [description] + * @param {[type]} settings [description] + * @return {[type]} [description] + */ +AssetSchema.statics.overrideSettings = (id, settings) => Asset.update({id}, { + $set: { + settings + } +}); + /** * Finds assets matching keywords on the model. If `value` is an empty string, * then it will not even perform a text search query. diff --git a/models/user.js b/models/user.js index bde2363c2..845b84d19 100644 --- a/models/user.js +++ b/models/user.js @@ -9,6 +9,7 @@ const SALT_ROUNDS = 10; // USER_ROLES is the array of roles that is permissible as a user role. const USER_ROLES = [ + '', 'admin', 'moderator' ]; diff --git a/routes/api/asset/index.js b/routes/api/asset/index.js index 0c740d32f..96b83a969 100644 --- a/routes/api/asset/index.js +++ b/routes/api/asset/index.js @@ -81,4 +81,19 @@ router.post('/:asset_id/scrape', (req, res, next) => { }); }); +router.put('/:asset_id/settings', (req, res, next) => { + + // Override the settings for the asset. + Asset + .overrideSettings(req.params.asset_id, req.body) + .then(() => { + + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); + +}); + module.exports = router; diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 7b793cdb6..ddab5b87b 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -30,16 +30,36 @@ router.get('/', (req, res, next) => { Setting.getModerationSetting() ]) .then(([asset, settings]) => { - // Get the sitewide moderation setting and return the appropriate comments + + // Merge the asset specific settings with the returned settings object in + // the event that the asset that was returned also had settings. + if (asset.settings) { + settings = Object.assign(settings, asset.settings); + } + + // Fetch the appropriate comments stream. let comments; - if (settings.moderation === 'pre') { + + if (settings.moderation === 'post') { comments = Comment.findAcceptedByAssetId(asset.id); } else { comments = Comment.findAcceptedAndNewByAssetId(asset.id); } - return Promise.all([comments, asset, settings]); + return Promise.all([ + + // This is the promised component... Fetch the comments based on the + // moderation settings. + comments, + + // Send back the reference to the asset. + asset, + + // Send back the settings to the stream. + settings + ]); }) + // Get all the users and actions for those comments. .then(([comments, asset, settings]) => { // Get the user id's from the author id's as a unique array that gets @@ -73,14 +93,16 @@ router.get('/', (req, res, next) => { // The users who wrote those comments users, - // The actions on the above items + // And all actions about the asset, comments, and users. actions, - // And the relevant settings + // Pass back the settings that we loaded. settings ]); }) .then(([asset, comments, users, actions, settings]) => { + + // Send back the payload containing all this data. res.json({ assets: [asset], comments, diff --git a/swagger.yaml b/swagger.yaml index 09adb97d6..e819f451c 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -335,6 +335,28 @@ paths: schema: $ref: '#/definitions/Error' + /asset/{asset_id}/settings: + put: + parameters: + - name: asset_id + in: path + required: true + type: string + format: uuid + - name: body + in: body + required: true + schema: + $ref: '#/definitions/Settings' + responses: + 204: + description: The asset settings were updated. + 404: + description: The asset was not found. + 500: + description: An error occured. + schema: + $ref: '#/definitions/Error' /stream: get: diff --git a/tests/models/asset.js b/tests/models/asset.js index a5c4ae62b..69bf99f9d 100644 --- a/tests/models/asset.js +++ b/tests/models/asset.js @@ -1,5 +1,10 @@ const Asset = require('../../models/asset'); -const expect = require('chai').expect; + +const chai = require('chai'); +const expect = chai.expect; + +// Use the chai should. +chai.should(); describe('Asset: model', () => { @@ -53,6 +58,27 @@ describe('Asset: model', () => { }); }); + describe('#overrideSettings', () => { + it('should update the settings', () => { + return Asset + .findOrCreateByUrl('https://override.test.com/asset') + .then((asset) => { + expect(asset).to.have.property('settings'); + expect(asset.settings).to.be.null; + + return Asset.overrideSettings(asset.id, {moderation: 'pre'}); + }) + .then(() => { + return Asset.findOrCreateByUrl('https://override.test.com/asset'); + }) + .then((asset) => { + expect(asset).to.have.property('settings'); + expect(asset.settings).is.an('object'); + expect(asset.settings).to.have.property('moderation', 'pre'); + }); + }); + }); + describe('#findOrCreateByUrl', ()=> { it('should find an asset by a url', () => { return Asset.findOrCreateByUrl('http://test.com') diff --git a/tests/models/setting.js b/tests/models/setting.js index a36b363fb..08a47d7a1 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -14,6 +14,7 @@ describe('Setting: model', () => { expect(settings).to.have.property('moderation').and.to.equal('pre'); }); }); + it('should have two infoBox fields defined', () => { return Setting.getSettings().then(settings => { expect(settings).to.have.property('infoBoxEnable').and.to.equal(false); @@ -26,6 +27,7 @@ describe('Setting: model', () => { it('should update the settings with a passed object', () => { const mockSettings = {moderation: 'post', infoBoxEnable: true, infoBoxContent: 'yeah'}; return Setting.updateSettings(mockSettings).then(updatedSettings => { + expect(updatedSettings).to.be.an('object'); expect(updatedSettings).to.have.property('moderation').and.to.equal('post'); expect(updatedSettings).to.have.property('infoBoxEnable', true); expect(updatedSettings).to.have.property('infoBoxContent', 'yeah'); diff --git a/tests/routes/api/assets/index.js b/tests/routes/api/assets/index.js index 667bec0ed..c56e5b1ba 100644 --- a/tests/routes/api/assets/index.js +++ b/tests/routes/api/assets/index.js @@ -10,7 +10,7 @@ chai.use(require('chai-http')); const Asset = require('../../../../models/asset'); -describe('/assets', () => { +describe('/api/v1/assets', () => { beforeEach(() => { return Asset.create([ @@ -27,7 +27,7 @@ describe('/assets', () => { ]); }); - describe('GET', () => { + describe('#get', () => { it('should return all assets without a search query', () => { return chai.request(app) diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index ddca63fe8..dd408d135 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -6,32 +6,47 @@ chai.use(require('chai-http')); const User = require('../../../../models/user'); -describe('POST /auth/local', () => { +describe('/api/v1/auth', () => { + describe('#get', () => { + it('should return nothing when no user is logged in', () => { + return chai.request(app) + .get('/api/v1/auth') + .then((res) => { + expect(res.status).to.be.equal(204); + expect(res.body).to.be.empty; + }); + }); + }); +}); + +describe('/api/v1/auth/local', () => { beforeEach(() => { return User.createLocalUser('maria@gmail.com', 'password!', 'Maria'); }); - it('should send back the user on a successful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!'}) - .catch((res) => { - expect(res).to.have.status(200); - expect(res).to.be.json; - expect(res.body).to.have.property('user'); - expect(res.body.user).to.have.property('displayName', 'Maria'); - }); - }); + describe('#post', () => { + it('should send back the user on a successful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!'}) + .catch((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('user'); + expect(res.body.user).to.have.property('displayName', 'Maria'); + }); + }); - it('should not send back the user on a unsuccessful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!3'}) - .catch((err) => { - expect(err).to.not.be.null; - expect(err.response).to.have.status(401); - expect(err.response.body).to.have.property('message', 'not authorized'); - }); + it('should not send back the user on a unsuccessful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!3'}) + .catch((err) => { + expect(err).to.not.be.null; + expect(err.response).to.have.status(401); + expect(err.response.body).to.have.property('message', 'not authorized'); + }); + }); }); }); diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index ff7f3a9d4..5ac9af7b7 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -16,11 +16,7 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -beforeEach(() => { - return Setting.create(settings); -}); - -describe('Get /comments', () => { +describe('/api/v1/comments', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -32,61 +28,11 @@ describe('Get /comments', () => { asset_id: 'asset', author_id: '456' }, { - id: 'hij', - body: 'comment 30', - asset_id: '456' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('should return all the comments', () => { - return chai.request(app) - .get('/api/v1/comments') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - - expect(res).to.have.status(200); - - }); - }); -}); - -describe('Get comments by status and action', () => { - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123', - status: 'rejected' - }, { - id: 'def', + id: 'def-rejected', body: 'comment 20', asset_id: 'asset', - author_id: '456' + author_id: '456', + status: 'rejected' }, { id: 'hij', body: 'comment 30', @@ -117,109 +63,100 @@ describe('Get comments by status and action', () => { beforeEach(() => { return Promise.all([ Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('should return all the rejected comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=rejected') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'abc'); - }); - }); - - it('should return all the approved comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=accepted') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'hij'); - }); - }); - - it('should return all the new comments', () => { - return chai.request(app) - .get('/api/v1/comments?status=new') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'def'); - }); - }); - - it('should return all the flagged comments', () => { - return chai.request(app) - .get('/api/v1/comments?action_type=flag') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - - expect(res.body.length).to.equal(1); - expect(res.body[0]).to.have.property('id', 'abc'); - - }); - }); -}); - -describe('Post /comments', () => { - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ User.createLocalUsers(users), Action.create(actions), wordlist.insert([ 'bad words' - ]) + ]), + Setting.create(settings) ]); }); - it('should create a comment', () => { - return chai.request(app) - .post('/api/v1/comments') - .set(passport.inject({roles: []})) - .send({'body': 'Something body.', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) - .then((res) => { - expect(res).to.have.status(201); - expect(res.body).to.have.property('id'); - }); + describe('#get', () => { + it('should return all the comments', () => { + return chai.request(app) + .get('/api/v1/comments') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + + expect(res).to.have.status(200); + + }); + }); + + it('should return all the rejected comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=rejected') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body[0]).to.have.property('id', 'def-rejected'); + }); + }); + + it('should return all the approved comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=accepted') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body).to.have.length(1); + expect(res.body[0]).to.have.property('id', 'hij'); + }); + }); + + it('should return all the new comments', () => { + return chai.request(app) + .get('/api/v1/comments?status=new') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body).to.have.length(2); + }); + }); + + it('should return all the flagged comments', () => { + return chai.request(app) + .get('/api/v1/comments?action_type=flag') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + + expect(res.body).to.have.length(1); + expect(res.body[0]).to.have.property('id', 'abc'); + + }); + }); }); - it('should create a comment with a rejected status if it contains a bad word', () => { - return chai.request(app) - .post('/api/v1/comments') - .set(passport.inject({roles: []})) - .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) - .then((res) => { - expect(res).to.have.status(201); - expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('status', 'rejected'); - }); + describe('#post', () => { + + it('should create a comment', () => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + }); + }); + + it('should create a comment with a rejected status if it contains a bad word', () => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': '1', 'parent_id': ''}) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + expect(res.body).to.have.property('status', 'rejected'); + }); + }); }); }); -describe('Get /:comment_id', () => { +describe('/api/v1/comments/:comment_id', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -264,79 +201,65 @@ describe('Get /:comment_id', () => { ]); }); - it('should return the right comment for the comment_id', () => { - return chai.request(app) - .get('/api/v1/comments/abc') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(200); - expect(res).to.have.property('body'); - expect(res.body).to.have.property('body', 'comment 10'); + describe('#get', () => { - }); + it('should return the right comment for the comment_id', () => { + return chai.request(app) + .get('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.have.property('body'); + expect(res.body).to.have.property('body', 'comment 10'); + + }); + }); + }); + + describe('#delete', () => { + it('it should remove comment', () => { + return chai.request(app) + .delete('/api/v1/comments/abc') + .set(passport.inject({roles: ['admin']})) + .then((res) => { + expect(res).to.have.status(204); + + return Comment.findById('abc'); + }) + .then((comment) => { + expect(comment).to.be.null; + }); + }); + }); + + describe('#put', () => { + it('it should update status', function() { + return chai.request(app) + .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: ['admin']})) + .send({status: 'accepted'}) + .then((res) => { + expect(res).to.have.status(204); + expect(res.body).to.be.empty; + }); + }); + + it('it should not allow a non-admin to update status', () => { + return chai.request(app) + .put('/api/v1/comments/abc/status') + .set(passport.inject({roles: []})) + .send({status: 'accepted'}) + .then((res) => { + expect(res).to.be.empty; + }) + .catch((err) => { + expect(err).to.have.property('status', 401); + }); + }); }); }); -describe('Remove /:comment_id', () => { - - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123' - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset', - author_id: '456' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('it should remove comment', () => { - return chai.request(app) - .delete('/api/v1/comments/abc') - .set(passport.inject({roles: ['admin']})) - .then((res) => { - expect(res).to.have.status(204); - - return Comment.findById('abc'); - }) - .then((comment) => { - expect(comment).to.be.null; - }); - }); -}); - -describe('Put /:comment_id/status', () => { +describe('/api/v1/comments/:comment_id/actions', () => { const comments = [{ id: 'abc', @@ -383,90 +306,20 @@ describe('Put /:comment_id/status', () => { ]); }); - it('it should update status', function() { - return chai.request(app) - .put('/api/v1/comments/abc/status') - .set(passport.inject({roles: ['admin']})) - .send({status: 'accepted'}) - .then((res) => { - expect(res).to.have.status(204); - expect(res.body).to.be.empty; - }); - }); - - it('it should not allow a non-admin to update status', () => { - return chai.request(app) - .put('/api/v1/comments/abc/status') - .set(passport.inject({roles: []})) - .send({status: 'accepted'}) - .then((res) => { - expect(res).to.be.empty; - }) - .catch((err) => { - expect(err).to.have.property('status', 401); - }); - }); -}); - -describe('Post /:comment_id/actions', () => { - - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123', - status: '' - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset', - author_id: '456', - status: 'rejected' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456', - status: 'accepted' - }]; - - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - const actions = [{ - action_type: 'flag', - item_id: 'abc' - }, { - action_type: 'like', - item_id: 'hij' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); - }); - - it('it should update actions', () => { - return chai.request(app) - .post('/api/v1/comments/abc/actions') - .set(passport.inject({id: '456', roles: ['admin']})) - .send({'user_id': '456', 'action_type': 'flag'}) - .then((res) => { - expect(res).to.have.status(201); - expect(res).to.have.body; - expect(res.body).to.have.property('item_type', 'comment'); - expect(res.body).to.have.property('action_type', 'flag'); - expect(res.body).to.have.property('item_id', 'abc'); - expect(res.body).to.have.property('user_id', '456'); - }); + describe('#post', () => { + it('it should update actions', () => { + return chai.request(app) + .post('/api/v1/comments/abc/actions') + .set(passport.inject({id: '456', roles: ['admin']})) + .send({'user_id': '456', 'action_type': 'flag'}) + .then((res) => { + expect(res).to.have.status(201); + expect(res).to.have.body; + expect(res.body).to.have.property('item_type', 'comment'); + expect(res.body).to.have.property('action_type', 'flag'); + expect(res.body).to.have.property('item_id', 'abc'); + expect(res.body).to.have.property('user_id', '456'); + }); + }); }); }); diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index 74137a49a..733e5a1be 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -15,11 +15,7 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -beforeEach(() => { - return Setting.create(settings); -}); - -describe('Get moderation queues rejected, pending, flags', () => { +describe('/api/v1/queue', () => { const comments = [{ id: 'abc', body: 'comment 10', @@ -62,19 +58,22 @@ describe('Get moderation queues rejected, pending, flags', () => { return Promise.all([ Comment.create(comments), User.createLocalUsers(users), - Action.create(actions) + Action.create(actions), + Setting.create(settings) ]); }); - it('should return all the pending comments', function(done){ - chai.request(app) - .get('/api/v1/queue/comments/pending') - .set(passport.inject({roles: ['admin']})) - .end(function(err, res){ - expect(err).to.be.null; - expect(res).to.have.status(200); - expect(res.body[0]).to.have.property('id', 'def'); - done(); - }); + describe('#get', () => { + it('should return all the pending comments', function(done){ + chai.request(app) + .get('/api/v1/queue/comments/pending') + .set(passport.inject({roles: ['admin']})) + .end(function(err, res){ + expect(err).to.be.null; + expect(res).to.have.status(200); + expect(res.body[0]).to.have.property('id', 'def'); + done(); + }); + }); }); }); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index 9f4466a7f..d1a7ba81b 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -10,49 +10,42 @@ chai.use(require('chai-http')); const Setting = require('../../../../models/setting'); const defaults = {id: '1', moderation: 'pre'}; -describe('GET /settings', () => { +describe('/api/v1/settings', () => { - beforeEach(() => { - return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); + beforeEach(() => Setting.create(defaults)); + + describe('#get', () => { + + it('should return a settings object', () => { + return chai.request(app) + .get('/api/v1/settings') + .set(passport.inject({ + roles: ['admin'] + })) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('moderation', 'pre'); + }); + }); }); - it('should return a settings object', () => { - return chai.request(app) - .get('/api/v1/settings') - .set(passport.inject({ - roles: ['admin'] - })) - .then((res) => { - expect(res).to.have.status(200); - expect(res).to.be.json; - expect(res.body).to.have.property('moderation', 'pre'); - }); - }); -}); + describe('#put', () => { -// update the settings. -describe('update settings', () => { - it('should respond ok to a PUT', () => { - return Setting - .update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}) - .then(() => { - return chai.request(app) - .put('/api/v1/settings') - .set(passport.inject({ - roles: ['admin'] - })) - .send({moderation: 'post'}); - }) - .then(res => { - expect(res).to.have.status(204); + it('should update the settings', () => { + return chai.request(app) + .put('/api/v1/settings') + .set(passport.inject({roles: ['admin']})) + .send({moderation: 'post'}) + .then((res) => { + expect(res).to.have.status(204); - return Setting.getSettings(); - }) - .then(settings => { - - // confirm updated settings in db - expect(settings).to.have.property('moderation'); - expect(settings.moderation).to.equal('post'); - }); + return Setting.getSettings(); + }) + .then((settings) => { + expect(settings).to.have.property('moderation', 'post'); + }); + }); }); + }); diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index 2e419a8e1..2b86f00ff 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -13,9 +13,12 @@ const Asset = require('../../../../models/asset'); const Setting = require('../../../../models/setting'); -describe('api/stream: routes', () => { +describe('/api/v1/stream', () => { - const settings = {id: '1', moderation: 'pre'}; + const settings = { + id: '1', + moderation: 'pre' + }; const comments = [{ id: 'abc', @@ -35,7 +38,7 @@ describe('api/stream: routes', () => { asset_id: 'asset', author_id: '456', parent_id: '', - status: '' + status: 'accepted' }, { id: 'hij', body: 'comment 40', @@ -65,15 +68,26 @@ describe('api/stream: routes', () => { return Promise.all([ User.createLocalUsers(users), - Asset.findOrCreateByUrl('http://test.com') + Asset.findOrCreateByUrl('http://test.com'), + Asset + .findOrCreateByUrl('http://coralproject.net/asset2') + .then((asset) => { + return Asset + .overrideSettings(asset.id, {moderation: 'post'}) + .then(() => asset); + }) ]) - .then(([users, asset]) => { + .then(([users, asset1, asset2]) => { comments[0].author_id = users[0].id; comments[1].author_id = users[1].id; + comments[2].author_id = users[0].id; + comments[3].author_id = users[1].id; - comments[0].asset_id = asset.id; - comments[1].asset_id = asset.id; + comments[0].asset_id = asset1.id; + comments[1].asset_id = asset1.id; + comments[2].asset_id = asset2.id; + comments[3].asset_id = asset2.id; return Promise.all([ Comment.create(comments), @@ -83,17 +97,32 @@ describe('api/stream: routes', () => { }); }); - it('should return a stream with comments, users and actions for an existing asset', () => { - return chai.request(app) - .get('/api/v1/stream') - .query({'asset_url': 'http://test.com'}) - .then(res => { - expect(res).to.have.status(200); - expect(res.body.assets[0]).to.have.property('url'); - expect(res.body.comments[0]).to.have.property('body'); - expect(res.body.users[0]).to.have.property('displayName'); - expect(res.body.actions[0]).to.have.property('action_type'); - expect(res.body.settings).to.have.property('moderation'); - }); + describe('#get', () => { + it('should return a stream with comments, users and actions for an existing asset', () => { + return chai.request(app) + .get('/api/v1/stream') + .query({'asset_url': 'http://test.com'}) + .then(res => { + expect(res).to.have.status(200); + expect(res.body.assets.length).to.equal(1); + expect(res.body.comments.length).to.equal(2); + expect(res.body.users.length).to.equal(2); + expect(res.body.actions.length).to.equal(1); + expect(res.body.settings).to.have.property('moderation', 'pre'); + }); + }); + + it('should merge the settings when the asset contains settings to override it with', () => { + return chai.request(app) + .get('/api/v1/stream') + .query({'asset_url': 'http://coralproject.net/asset2'}) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body.assets.length).to.equal(1); + expect(res.body.comments.length).to.equal(1); + expect(res.body.users.length).to.equal(1); + expect(res.body.settings).to.have.property('moderation', 'post'); + }); + }); }); }); From 20ef93c0afbcd73e0b501d9a82d180fbc29a0802 Mon Sep 17 00:00:00 2001 From: David Jay Date: Tue, 29 Nov 2016 11:44:54 -0500 Subject: [PATCH 28/28] Test db clear (#116) * Adding headers to stream request. * Switching mongodb when node_env === test. --- client/coral-framework/actions/items.js | 4 ++-- mongoose.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index 4587b174b..00deba5ef 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -1,4 +1,4 @@ -import {getInit, base, handleResp} from '../../coral-framework/helpers/response'; +import {getInit, base, handleResp} from '../helpers/response'; import {fromJS} from 'immutable'; /* Item Actions */ @@ -95,7 +95,7 @@ export const appendItemArray = (id, property, value, add_to_front, item_type) => */ export function getStream (assetUrl) { return (dispatch) => { - return fetch(`${base}/stream?asset_url=${encodeURIComponent(assetUrl)}`) + return fetch(`${base}/stream?asset_url=${encodeURIComponent(assetUrl)}`, getInit('GET')) .then(handleResp) .then((json) => { diff --git a/mongoose.js b/mongoose.js index 712b2fcb0..0121a203d 100644 --- a/mongoose.js +++ b/mongoose.js @@ -1,7 +1,11 @@ const mongoose = require('mongoose'); const debug = require('debug')('talk:db'); const enabled = require('debug').enabled; -const url = process.env.TALK_MONGO_URL || 'mongodb://localhost'; +let url = process.env.TALK_MONGO_URL || 'mongodb://localhost'; + +if (process.env.NODE_ENV === 'test') { + url = 'mongodb://localhost/coral-test'; +} // Use native promises mongoose.Promise = global.Promise;