From b82f1304542c1ab148bf8ce1f22dfa231682b6dc Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 6 Dec 2016 14:44:15 -0500 Subject: [PATCH 01/16] Adds a new closedAt time for assets to prevent new comments from being created --- models/asset.js | 15 +++++++ models/setting.js | 10 +++++ routes/api/comments/index.js | 15 ++++++- tests/routes/api/comments/index.js | 63 ++++++++++++++++++++++++++---- 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/models/asset.js b/models/asset.js index a0619576f..7b0717674 100644 --- a/models/asset.js +++ b/models/asset.js @@ -29,6 +29,14 @@ const AssetSchema = new Schema({ type: Schema.Types.Mixed, default: null }, + closedAt: { + type: Date, + default: null + }, + closedMessage: { + type: String, + default: null + }, title: String, description: String, image: String, @@ -60,6 +68,13 @@ AssetSchema.index({ background: true }); +/** + * Returns true if the asset is closed, false else. + */ +AssetSchema.virtual('isClosed').get(function() { + return this.closedAt && this.closedAt.getTime() <= new Date().getTime(); +}); + /** * Finds an asset by its id. * @param {String} id identifier of the asset (uuid). diff --git a/models/setting.js b/models/setting.js index 22b2b103f..5d71b8c97 100644 --- a/models/setting.js +++ b/models/setting.js @@ -28,6 +28,16 @@ const SettingSchema = new Schema({ type: String, default: '' }, + closedTimeout: { + type: Number, + + // Two weeks default expiry. + default: 60 * 60 * 24 * 7 * 2 + }, + closedMessage: { + type: String, + default: '' + }, wordlist: [String] }, { timestamps: { diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index b82c1faf3..748f64f5b 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -80,7 +80,20 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { status = Promise.resolve('rejected'); } else { status = Asset - .rectifySettings(Asset.findById(asset_id)) + .rectifySettings(Asset.findById(asset_id).then((asset) => { + if (!asset) { + return Promise.reject(new Error('asset referenced is not found')); + } + + // Check to see if the asset has closed commenting... + if (asset.isClosed) { + + // They have, ensure that we send back an error. + return Promise.reject(new Error(`asset has commenting closed because: ${asset.closedMessage}`)); + } + + return asset; + })) // Return `premod` if pre-moderation is enabled and an empty "new" status // in the event that it is not in pre-moderation mode. diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index bf7a06ff6..70912eac7 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -19,6 +19,14 @@ const settings = {id: '1', moderation: 'pre'}; describe('/api/v1/comments', () => { + // Ensure that the settings are always available. + beforeEach(() => Promise.all([ + Setting.init(settings), + wordlist.insert([ + 'bad words' + ]) + ])); + describe('#get', () => { const comments = [{ body: 'comment 10', @@ -75,11 +83,7 @@ describe('/api/v1/comments', () => { return Action.create(actions); }), - User.createLocalUsers(users), - wordlist.insert([ - 'bad words' - ]), - Setting.init(settings) + User.createLocalUsers(users) ]); }); @@ -142,11 +146,19 @@ describe('/api/v1/comments', () => { describe('#post', () => { + let asset_id; + + beforeEach(() => Asset.findOrCreateByUrl('https://coralproject.net/section/article-is-the-best').then((asset) => { + + // Update the asset id. + asset_id = asset.id; + })); + 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': ''}) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset_id, 'parent_id': ''}) .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); @@ -157,7 +169,7 @@ describe('/api/v1/comments', () => { 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': ''}) + .send({'body': 'bad words are the baddest', 'author_id': '123', 'asset_id': asset_id, 'parent_id': ''}) .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); @@ -188,6 +200,43 @@ describe('/api/v1/comments', () => { expect(res.body.status[0]).to.have.property('type', 'premod'); }); }); + + it('shouldn\'t create a comment when the asset has expired commenting', () => { + return Asset.create({ + closedAt: new Date().setDate(0), + closedMessage: 'tests said expired!' + }) + .then((asset) => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''}); + }) + .then((res) => { + expect(res).to.have.status(500); + }) + .catch((err) => { + expect(err.response.body).to.not.be.null; + expect(err.response.body).to.have.property('message'); + expect(err.response.body.message).to.contain('tests said expired!'); + }); + }); + + it('should create a comment when the asset has not expired yet', () => { + return Asset.create({ + closedAt: new Date().setDate(32), + closedMessage: 'tests said expired!' + }) + .then((asset) => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''}); + }) + .then((res) => { + expect(res).to.have.status(201); + }); + }); }); }); From fb392422eae88c96be5016c1e8d26bcd21a53e30 Mon Sep 17 00:00:00 2001 From: Dan Zajdband Date: Thu, 8 Dec 2016 12:44:02 -0500 Subject: [PATCH 02/16] Added textarea for closed message on admin --- .../src/containers/Configure/Configure.js | 16 ++++++++++++++++ client/coral-admin/src/translations.json | 8 ++++++-- client/coral-embed-stream/src/CommentStream.js | 4 ++-- client/coral-framework/actions/config.js | 1 + client/coral-framework/reducers/config.js | 3 ++- models/setting.js | 2 +- 6 files changed, 28 insertions(+), 6 deletions(-) diff --git a/client/coral-admin/src/containers/Configure/Configure.js b/client/coral-admin/src/containers/Configure/Configure.js index a9e2f3ef2..e171fc27d 100644 --- a/client/coral-admin/src/containers/Configure/Configure.js +++ b/client/coral-admin/src/containers/Configure/Configure.js @@ -28,6 +28,7 @@ class Configure extends React.Component { // 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.updateClosedMessage = this.updateClosedMessage.bind(this); this.saveSettings = this.saveSettings.bind(this); } @@ -51,6 +52,11 @@ class Configure extends React.Component { this.props.dispatch(updateSettings({infoBoxContent})); } + updateClosedMessage (event) { + const closedMessage = event.target.value; + this.props.dispatch(updateSettings({closedMessage})); + } + saveSettings () { this.props.dispatch(saveSettingsToServer()); } @@ -78,6 +84,16 @@ class Configure extends React.Component {

+ + + {lang.t('configure.closed-comments-desc')} + + + @@ -114,7 +114,7 @@ class CommentStream extends Component { author={user} /> - :

Comments are closed for this thread.

+ :

{this.props.config.closedMessage}

} {!loggedIn && } { diff --git a/client/coral-framework/actions/config.js b/client/coral-framework/actions/config.js index 6dc880434..e004fa1eb 100644 --- a/client/coral-framework/actions/config.js +++ b/client/coral-framework/actions/config.js @@ -5,6 +5,7 @@ import coralApi from '../helpers/response'; * Action name constants */ +export const UPDATE_SETTINGS = 'UPDATE_SETTINGS'; export const OPEN_COMMENTS = 'OPEN_COMMENTS'; export const CLOSE_COMMENTS = 'CLOSE_COMMENTS'; export const ADD_ITEM = 'ADD_ITEM'; diff --git a/client/coral-framework/reducers/config.js b/client/coral-framework/reducers/config.js index 9620f5b97..7fbccaa49 100644 --- a/client/coral-framework/reducers/config.js +++ b/client/coral-framework/reducers/config.js @@ -5,7 +5,8 @@ import * as actions from '../actions/config'; const initialState = Map({ features: Map({}), - status: 'open' + status: 'open', + closedMessage: '' }); export default (state = initialState, action) => { diff --git a/models/setting.js b/models/setting.js index 5d71b8c97..2414381fa 100644 --- a/models/setting.js +++ b/models/setting.js @@ -99,7 +99,7 @@ SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { * @param {Object} settings the source settings object * @return {Object} the filtered settings object */ -SettingService.public = (settings) => _.pick(settings, ['moderation', 'infoBoxEnable', 'infoBoxContent']); +SettingService.public = (settings) => _.pick(settings, ['moderation', 'infoBoxEnable', 'infoBoxContent', 'closedMessage']); /** * This is run once when the app starts to ensure settings are populated. From 7c4208fa163c17086ddcff0dc411109f38f0ac7a Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Thu, 8 Dec 2016 10:04:10 -1000 Subject: [PATCH 03/16] Update config notes --- README.md | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0f217d89c..848dd989a 100644 --- a/README.md +++ b/README.md @@ -19,13 +19,19 @@ Runs Talk. The Talk application requires specific configuration options to be available inside the environment in order to run, those variables are listed here: -- `TALK_SESSION_SECRET` (*required*) - a random string which will be used to - secure cookies -- `TALK_FACEBOOK_APP_ID` (*required*) - the Facebook app id for your Facebook - Login enabled app. -- `TALK_FACEBOOK_APP_SECRET` (*required*) - the Facebook app secret for your - Facebook Login enabled app. -- `TALK_ROOT_URL` (*required*) - Root url of the installed application externally available in the format: `://` without the path. +- `TALK_SESSION_SECRET` (*required*) - a random string which will be used to +secure cookies. +`TALK_FACEBOOK_APP_ID` (*required*) - the Facebook app id for your Facebook +Login enabled app. +`TALK_FACEBOOK_APP_SECRET` (*required*) - the Facebook app secret for your +Facebook Login enabled app. +`TALK_ROOT_URL` (*required*) - root url of the installed application externally +available in the format: `://` without the path. +`TALK_SMTP_PROVIDER` (*required*) - SMTP provider name. +`TALK_SMTP_USERNAME` (*required*) - username of the SMTP provider you are using. +`TALK_SMTP_PASSWORD` (*required*) - password for the SMTP provider you are using. +`TALK_SMTP_HOST` (*required*) - SMTP host url with format `smtp.domain.com`. +`TALK_SMTP_PORT` (*required*) - SMTP port. ### Running with Docker Make sure you have Docker running first and then run `docker-compose up -d` From 1c5dffc94cd975d4b082db9b8ac71c44b0cf22ae Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Thu, 8 Dec 2016 10:05:38 -1000 Subject: [PATCH 04/16] Add product links --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 848dd989a..6589cddb9 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,11 @@ A commenting platform from The Coral Project. [https://coralproject.net](https:/ ## Contributing to Talk +### Product Roadmap +You can view what the Coral Team is working on next here: https://www.pivotaltracker.com/n/projects/1863625 + +You can view product ideas and our longer term roadmap here: https://trello.com/b/ILND751a/talk + ### Local Dependencies Node Mongo From 3d35183e6781b15cc081b5dcc1b13258e43f087f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Dec 2016 15:56:52 -0500 Subject: [PATCH 05/16] Add payload filtering and status_history updates --- app.js | 4 +- middleware/payload-filter.js | 73 ++++++++++++++++++++++++++++++ models/asset.js | 8 +--- models/comment.js | 61 +++++++++++++------------ models/setting.js | 48 ++++++++++++++++---- models/user.js | 22 ++++----- routes/api/comments/index.js | 2 +- routes/api/index.js | 4 ++ routes/api/stream/index.js | 17 +++++-- routes/api/user/index.js | 47 ++++++++----------- services/scraper.js | 4 +- tests/models/comment.js | 44 ++++++++---------- tests/models/setting.js | 14 ++++++ tests/routes/api/comments/index.js | 27 ++++++----- tests/routes/api/queue/index.js | 6 +-- tests/routes/api/stream/index.js | 19 ++++++-- 16 files changed, 265 insertions(+), 135 deletions(-) create mode 100644 middleware/payload-filter.js diff --git a/app.js b/app.js index dd58e9883..fe392932b 100644 --- a/app.js +++ b/app.js @@ -94,7 +94,9 @@ app.use((req, res, next) => { // returning a status code that makes sense. app.use('/api', (err, req, res, next) => { if (err !== ErrNotFound) { - console.error(err); + if (app.get('env') !== 'test') { + console.error(err); + } } res.status(err.status || 500); diff --git a/middleware/payload-filter.js b/middleware/payload-filter.js new file mode 100644 index 000000000..8ce3d39d5 --- /dev/null +++ b/middleware/payload-filter.js @@ -0,0 +1,73 @@ +// The maximum depth to recurse into nested objects checking for mongoose +// objects. +const maxRecursion = 3; + +/** + * Middleware to wrap the `res.json` function to ensure that we can filter the + * payload response first based on user and role. + */ +module.exports = (req, res, next) => { + /** + * Updates the original document based on filtering out for roles. + * @param {Mixed} o original object to be modified + * @param {Integer} l current level of depth in the first object + * @return {Mixed} (possibly) modified object + */ + const wrap = (o, d = 0) => { + if (d > maxRecursion) { + return o; + } + + // If this is an array, we need to walk over all the object's elements. + if (Array.isArray(o)) { + + // Map each of the array elements. + return o.map((ob) => wrap(ob, d + 1)); + + } else if (o && o.constructor && o.constructor.name === 'model') { + + // The object here is definitly a mongoose model. + + // Check to see if it has a `filterForUser` method. + if (typeof o.filterForUser === 'function') { + + // The object here actually has the `filterForUser` function, so filter + // the object! + o = o.filterForUser(req.user); + } + + } else if (typeof o === 'object') { + + // Itterate over the props, find only properties owned by the object. + for (let prop in o) { + + // If and only if the object owns the property do we actually pull the + // property out. + if (typeof o.hasOwnProperty === 'function' && o.hasOwnProperty(prop)) { + + // Wrap the property with one more layer down. + o[prop] = wrap(o[prop], d + 1); + } + } + + } + + return o; + }; + + // Save a reference to the original json function. + const json = res.json; + + // Override the original json function. + res.json = (payload) => { + + // Restore the old pointer. + res.json = json; + + // Send it down the pipe after we've filtered it. + res.json(wrap(payload)); + }; + + // Now that we've overridden the `res.json`, let's hand it down. + next(); +}; diff --git a/models/asset.js b/models/asset.js index a0619576f..0cdc77d7b 100644 --- a/models/asset.js +++ b/models/asset.js @@ -36,11 +36,7 @@ const AssetSchema = new Schema({ subsection: String, author: String, publication_date: Date, - modified_date: Date, - status: { - type: String, - default: 'open' - } + modified_date: Date }, { versionKey: false, timestamps: { @@ -85,7 +81,7 @@ AssetSchema.statics.rectifySettings = (assetQuery) => Promise.all([ // If the asset exists and has settings then return the merged object. if (asset && asset.settings) { - return Object.assign({}, settings, asset.settings); + settings.merge(asset.settings); } return settings; diff --git a/models/comment.js b/models/comment.js index af9aa4cb1..53e8bbfb0 100644 --- a/models/comment.js +++ b/models/comment.js @@ -1,5 +1,6 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; +const _ = require('lodash'); const uuid = require('uuid'); const Action = require('./action'); @@ -45,7 +46,7 @@ const CommentSchema = new Schema({ }, asset_id: String, author_id: String, - status: [StatusSchema], + status_history: [StatusSchema], parent_id: String }, { timestamps: { @@ -59,22 +60,7 @@ const CommentSchema = new Schema({ * output. */ CommentSchema.options.toJSON = {}; -CommentSchema.options.toJSON.hide = '_id status'; -CommentSchema.options.toJSON.transform = (doc, ret, options) => { - if (options.hide) { - options.hide.split(' ').forEach((prop) => { - delete ret[prop]; - }); - } - - return ret; -}; - -/** - * toJSON overrides to remove fields from the json - * output. - */ -CommentSchema.options.toJSON = {}; +CommentSchema.options.toJSON.virtuals = true; CommentSchema.options.toJSON.hide = '_id'; CommentSchema.options.toJSON.transform = (doc, ret, options) => { if (options.hide) { @@ -86,14 +72,31 @@ CommentSchema.options.toJSON.transform = (doc, ret, options) => { return ret; }; +/** + * Filters the object for the given user only allowing those with the allowed + * roles/permissions to access particular parameters. + */ +CommentSchema.method('filterForUser', function(user = false) { + if (!user || !user.roles.includes('admin')) { + return _.pick(this.toJSON(), ['id', 'body', 'asset_id', 'author_id', 'parent_id', 'status']); + } + + return this.toJSON(); +}); + /** * Sets up a virtual getter function on a comment such that when you try and * access the `comment.last_status` it returns the last status in the array - * of status's on the comment, or `null` if there was no status. + * of status's on the comment, or `null` if there was no status_history. */ -CommentSchema.virtual('last_status').get(function() { - if (this.status && this.status.length > 0) { - return this.status[this.status.length - 1].type; +CommentSchema.virtual('status').get(function() { + + // Here we are taking advantage of the fact that when documents are inserted + // for the new status on a comment that they are always appended to the end + // of the list in the order that they are inserted, hence, the last status + // is always the most recent. + if (this.status_history && this.status_history.length > 0) { + return this.status_history[this.status_history.length - 1].type; } return null; @@ -123,7 +126,7 @@ CommentSchema.statics.publicCreate = (comment) => { body, asset_id, parent_id, - status: status ? [{ + status_history: status ? [{ type: status, created_at: new Date() }] : [], @@ -157,7 +160,7 @@ CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({ */ CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ asset_id, - 'status.type': 'accepted' + 'status_history.type': 'accepted' }); /** @@ -169,10 +172,10 @@ CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({ asset_id, $or: [ { - 'status.type': 'accepted' + 'status_history.type': 'accepted' }, { - status: { + status_history: { $size: 0 } } @@ -202,7 +205,7 @@ CommentSchema.statics.findIdsByActionType = (action_type) => Action .then((actions) => actions.map(a => a.item_id)); /** - * Find comments by their status. + * Find comments by their status_history. * @param {String} status the status of the comment to search for * @return {Promise} */ @@ -210,9 +213,9 @@ CommentSchema.statics.findByStatus = (status = false) => { let q = {}; if (status) { - q['status.type'] = status; + q['status_history.type'] = status; } else { - q.status = {$size: 0}; + q.status_history = {$size: 0}; } return Comment.find(q); @@ -269,7 +272,7 @@ CommentSchema.statics.moderationQueue = (moderation, asset_id = false) => { */ CommentSchema.statics.pushStatus = (id, status, assigned_by = null) => Comment.update({id}, { $push: { - status: { + status_history: { type: status, created_at: new Date(), assigned_by diff --git a/models/setting.js b/models/setting.js index 22b2b103f..86dc761f3 100644 --- a/models/setting.js +++ b/models/setting.js @@ -36,6 +36,42 @@ const SettingSchema = new Schema({ } }); +/** + * toJSON provides settings overrides to this object's serialization methods. + */ +SettingSchema.options.toJSON = {}; +SettingSchema.options.toJSON.virtuals = true; + +/** + * Merges two settings objects. + */ +SettingSchema.method('merge', function(src) { + SettingSchema.eachPath((path) => { + + // Exclude internal fields... + if (['id', '_id', '__v', 'created_at', 'updated_at'].includes(path)) { + return; + } + + // If the source object contains the path, shallow copy it. + if (path in src) { + this[path] = src[path]; + } + }); +}); + +/** + * Filters the object for the given user only allowing those with the allowed + * roles/permissions to access particular parameters. + */ +SettingSchema.method('filterForUser', function(user = false) { + if (!user || !user.roles.includes('admin')) { + return _.pick(this.toJSON(), ['moderation', 'infoBoxEnable', 'infoBoxContent']); + } + + return this.toJSON(); +}); + /** * The Mongo Mongoose object. */ @@ -62,7 +98,9 @@ const EXPIRY_TIME = 60 * 2; * Gets the entire settings record and sends it back * @return {Promise} settings the whole settings record */ -SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => Setting.findOne(selector)); +SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => { + return Setting.findOne(selector); +}).then((setting) => new Setting(setting)); /** * This will update the settings object with whatever you pass in @@ -83,14 +121,6 @@ SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { .then(() => settings); }); -/** - * Filters the document to ensure that the resulting document is indeed ready - * for non authenticated users. - * @param {Object} settings the source settings object - * @return {Object} the filtered settings object - */ -SettingService.public = (settings) => _.pick(settings, ['moderation', 'infoBoxEnable', 'infoBoxContent']); - /** * This is run once when the app starts to ensure settings are populated. * @return {Promise} null initialize the global settings object diff --git a/models/user.js b/models/user.js index 336486a54..10a3a5e05 100644 --- a/models/user.js +++ b/models/user.js @@ -1,5 +1,6 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); +const _ = require('lodash'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); @@ -105,7 +106,8 @@ UserSchema.index({ * output. */ UserSchema.options.toJSON = {}; -UserSchema.options.toJSON.hide = '_id password profiles roles disabled'; +UserSchema.options.toJSON.hide = '_id password'; +UserSchema.options.toJSON.virtuals = true; UserSchema.options.toJSON.transform = (doc, ret, options) => { if (options.hide) { options.hide.split(' ').forEach((prop) => { @@ -117,20 +119,16 @@ UserSchema.options.toJSON.transform = (doc, ret, options) => { }; /** - * toObject overrides to remove the password field from the toObject - * output. + * Filters the object for the given user only allowing those with the allowed + * roles/permissions to access particular parameters. */ -UserSchema.options.toObject = {}; -UserSchema.options.toObject.hide = 'password'; -UserSchema.options.toObject.transform = (doc, ret, options) => { - if (options.hide) { - options.hide.split(' ').forEach((prop) => { - delete ret[prop]; - }); +UserSchema.method('filterForUser', function(user = false) { + if (!user || !user.roles.includes('admin')) { + return _.pick(this.toJSON(), ['id', 'displayName', 'settings', 'created_at', 'updated_at']); } - return ret; -}; + return this.toJSON(); +}); // Create the User model. const UserModel = mongoose.model('User', UserSchema); diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index b82c1faf3..00908e8e8 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -97,7 +97,7 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { .then((comment) => { // The comment was created! Send back the created comment. - res.status(201).send(comment); + res.status(201).json(comment); }) .catch((err) => { next(err); diff --git a/routes/api/index.js b/routes/api/index.js index 9b4f0432b..2c36e86bb 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -1,8 +1,12 @@ const express = require('express'); const authorization = require('../../middleware/authorization'); +const payloadFilter = require('../../middleware/payload-filter'); const router = express.Router(); +// Filter all content going down the pipe based on user roles. +router.use(payloadFilter); + router.use('/asset', authorization.needed('admin'), require('./asset')); router.use('/settings', authorization.needed('admin'), require('./settings')); router.use('/queue', authorization.needed('admin'), require('./queue')); diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 26947be09..8d70adbb4 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -1,21 +1,32 @@ const express = require('express'); const _ = require('lodash'); const scraper = require('../../../services/scraper'); +const url = require('url'); 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 ErrInvalidAssetURL = new Error('asset_url is invalid'); +ErrInvalidAssetURL.status = 400; const router = express.Router(); router.get('/', (req, res, next) => { + let asset_url = decodeURIComponent(req.query.asset_url); + + // Verify that the asset_url is parsable. + let parsed_asset_url = url.parse(asset_url); + if (!parsed_asset_url.protocol) { + return next(ErrInvalidAssetURL); + } + // Get the asset_id for this url (or create it if it doesn't exist) Promise.all([ // Find or create the asset by url. - Asset.findOrCreateByUrl(decodeURIComponent(req.query.asset_url)) + Asset.findOrCreateByUrl(asset_url) // Add the found asset to the scraper if it's not already scraped. .then((asset) => { @@ -34,7 +45,7 @@ router.get('/', (req, res, next) => { // Merge the asset specific settings with the returned settings object in // the event that the asset that was returned also had settings. if (asset && asset.settings) { - settings = Object.assign({}, settings, asset.settings); + settings.merge(asset.settings); } // Fetch the appropriate comments stream. @@ -98,7 +109,7 @@ router.get('/', (req, res, next) => { comments, users, actions, - settings: Setting.public(settings) + settings }); }) .catch(error => { diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 1765809ad..bd83563dc 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -26,26 +26,15 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { .limit(limit), User.count() ]) - .then(([data, count]) => { - const users = data.map((user) => { - const {id, displayName, created_at} = user; - return { - id, - displayName, - created_at, - profiles: user.toObject().profiles, - roles: user.toObject().roles - }; - }); + .then(([result, count]) => { res.json({ - result: users, + result, limit: Number(limit), count, page: Number(page), - totalPages: Math.ceil(count / limit) + totalPages: Math.ceil(count / (limit === 0 ? 1 : limit)) }); - }) .catch(next); }); @@ -53,8 +42,8 @@ router.get('/', authorization.needed('admin'), (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 => { - res.send(role); + .then(() => { + res.status(204).end(); }) .catch(next); }); @@ -65,13 +54,17 @@ router.post('/', (req, res, next) => { User .createLocalUser(email, password, displayName) .then(user => { - res.status(201).send(user); + + res.status(201).json(user); }) .catch(err => { next(err); }); }); +const ErrPasswordTooShort = new Error('password must be at least 8 characters'); +ErrPasswordTooShort.status = 400; + /** * expects 2 fields in the body of the request * 1) the token that was in the url of the email link {String} @@ -81,7 +74,7 @@ router.post('/update-password', (req, res, next) => { const {token, password} = req.body; if (!password || password.length < 8) { - return res.status(400).send('Password must be at least 8 characters'); + return next(ErrPasswordTooShort); } User.verifyPasswordResetToken(token) @@ -93,7 +86,8 @@ router.post('/update-password', (req, res, next) => { }) .catch(error => { console.error(error); - res.status(401).send('Not Authorized'); + + next(authorization.ErrNotAuthorized); }); }); @@ -133,10 +127,8 @@ router.post('/request-password-reset', (req, res, next) => { // if we fail on missing emails, it would reveal if people are registered or not. res.status(204).end(); }) - .catch(error => { - const errorMsg = typeof error === 'string' ? error : error.message; - - res.status(500).json({error: errorMsg}); + .catch((err) => { + next(err); }); }); @@ -150,10 +142,11 @@ router.put('/:user_id/bio', (req, res, next) => { User .addBio(user_id, bio) - .then(user => res.status(200).send(user)) - .catch(error => { - const errorMsg = typeof error === 'string' ? error : error.message; - res.status(500).json({error: errorMsg}); + .then(user => { + res.json(user); + }) + .catch((err) => { + next(err); }); }); diff --git a/services/scraper.js b/services/scraper.js index 922ef77bc..10e0e4aa8 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -23,7 +23,7 @@ const scraper = { title: `Scrape for asset ${asset.id}`, asset_id: asset.id }) - .attempts(10) + .attempts(3) .delay(1000) .backoff({type: 'exponential'}) .save((err) => { @@ -106,7 +106,7 @@ const scraper = { // Handle errors that occur. .catch((err) => { - console.error(`Failed to scrape on Job[${job.id}] for Asset[${job.data.asset_id}]:`, err); + debug(`Failed to scrape on Job[${job.id}] for Asset[${job.data.asset_id}]:`, err); done(err); }); diff --git a/tests/models/comment.js b/tests/models/comment.js index a9c2f4787..3db7d67a2 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -11,14 +11,14 @@ describe('models.Comment', () => { const comments = [{ body: 'comment 10', asset_id: '123', - status: [], + status_history: [], parent_id: '', author_id: '123', id: '1' }, { body: 'comment 20', asset_id: '123', - status: [{ + status_history: [{ type: 'accepted' }], parent_id: '', @@ -27,14 +27,14 @@ describe('models.Comment', () => { }, { body: 'comment 30', asset_id: '456', - status: [], + status_history: [], parent_id: '', author_id: '456', id: '3' }, { body: 'comment 40', asset_id: '123', - status: [{ + status_history: [{ type: 'rejected' }], parent_id: '', @@ -43,7 +43,7 @@ describe('models.Comment', () => { }, { body: 'comment 50', asset_id: '1234', - status: [{ + status_history: [{ type: 'premod' }], parent_id: '', @@ -52,7 +52,7 @@ describe('models.Comment', () => { }, { body: 'comment 60', asset_id: '1234', - status: [{ + status_history: [{ type: 'premod' }], parent_id: '', @@ -99,8 +99,7 @@ describe('models.Comment', () => { expect(c).to.not.be.null; expect(c.id).to.not.be.null; expect(c.id).to.be.uuid; - expect(c.status).to.have.length(1); - expect(c.status[0]).to.have.property('type', 'accepted'); + expect(c.status).to.be.equal('accepted'); }); }); @@ -116,17 +115,15 @@ describe('models.Comment', () => { }]).then(([c1, c2, c3]) => { expect(c1).to.not.be.null; expect(c1.id).to.be.uuid; - expect(c1.status).to.have.length(1); - expect(c1.status[0]).to.have.property('type', 'accepted'); + expect(c1.status).to.be.equal('accepted'); expect(c2).to.not.be.null; expect(c2.id).to.be.uuid; - expect(c2.status).to.have.length(0); + expect(c2.status).to.be.null; expect(c3).to.not.be.null; expect(c3.id).to.be.uuid; - expect(c3.status).to.have.length(1); - expect(c3.status[0]).to.have.property('type', 'rejected'); + expect(c3.status).to.be.equal('rejected'); }); }); @@ -220,17 +217,16 @@ describe('models.Comment', () => { return Comment.findById(comment_id) .then((c) => { - expect(c).to.have.property('status'); - expect(c.status).to.have.length(0); + expect(c.status).to.be.null; return Comment.pushStatus(comment_id, 'rejected', '123'); }) .then(() => Comment.findById(comment_id)) .then((c) => { expect(c).to.have.property('status'); - expect(c.status).to.have.length(1); - expect(c.status[0]).to.have.property('type', 'rejected'); - expect(c.status[0]).to.have.property('assigned_by', '123'); + expect(c.status_history).to.have.length(1); + expect(c.status_history[0]).to.have.property('type', 'rejected'); + expect(c.status_history[0]).to.have.property('assigned_by', '123'); }); }); @@ -238,13 +234,13 @@ describe('models.Comment', () => { return Comment.pushStatus(comments[1].id, 'rejected', '123') .then(() => Comment.findById(comments[1].id)) .then((c) => { - expect(c).to.have.property('status'); - expect(c.status).to.have.length(2); - expect(c.status[0]).to.have.property('type', 'accepted'); - expect(c.status[0]).to.have.property('assigned_by', null); + expect(c).to.have.property('status_history'); + expect(c.status_history).to.have.length(2); + expect(c.status_history[0]).to.have.property('type', 'accepted'); + expect(c.status_history[0]).to.have.property('assigned_by', null); - expect(c.status[1]).to.have.property('type', 'rejected'); - expect(c.status[1]).to.have.property('assigned_by', '123'); + expect(c.status_history[1]).to.have.property('type', 'rejected'); + expect(c.status_history[1]).to.have.property('assigned_by', '123'); }); }); diff --git a/tests/models/setting.js b/tests/models/setting.js index 3e77297fc..fbd5bf77f 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -39,4 +39,18 @@ describe('models.Setting', () => { }); }); }); + + describe('#merge', () => { + it('should merge a settings object and it\'s overrides', () => { + return Setting + .retrieve() + .then((settings) => { + let ovrSett = {moderation: 'post'}; + + settings.merge(ovrSett); + + expect(settings).to.have.property('moderation', 'post'); + }); + }); + }); }); diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index bf7a06ff6..988320e34 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -19,6 +19,11 @@ const settings = {id: '1', moderation: 'pre'}; describe('/api/v1/comments', () => { + beforeEach(() => Promise.all([ + wordlist.insert(['bad words']), + Setting.init(settings) + ])); + describe('#get', () => { const comments = [{ body: 'comment 10', @@ -32,13 +37,13 @@ describe('/api/v1/comments', () => { body: 'comment 20', asset_id: 'asset', author_id: '456', - status: [{ + status_history: [{ type: 'rejected' }] }, { body: 'comment 30', asset_id: '456', - status: [{ + status_history: [{ type: 'accepted' }] }]; @@ -75,11 +80,7 @@ describe('/api/v1/comments', () => { return Action.create(actions); }), - User.createLocalUsers(users), - wordlist.insert([ - 'bad words' - ]), - Setting.init(settings) + User.createLocalUsers(users) ]); }); @@ -161,8 +162,7 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('status').and.to.have.length(1); - expect(res.body.status[0]).to.have.property('type', 'rejected'); + expect(res.body).to.have.property('status', 'rejected'); }); }); @@ -184,8 +184,7 @@ describe('/api/v1/comments', () => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); expect(res.body).to.have.property('asset_id'); - expect(res.body).to.have.property('status').and.to.have.length(1); - expect(res.body.status[0]).to.have.property('type', 'premod'); + expect(res.body).to.have.property('status', 'premod'); }); }); }); @@ -301,20 +300,20 @@ describe('/api/v1/comments/:comment_id/actions', () => { body: 'comment 10', asset_id: 'asset', author_id: '123', - status: [] + status_history: [] }, { id: 'def', body: 'comment 20', asset_id: 'asset', author_id: '456', - status: [{ + status_history: [{ type: 'rejected' }] }, { id: 'hij', body: 'comment 30', asset_id: '456', - status: [{ + status_history: [{ type: 'accepted' }] }]; diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index d64423013..ce55836ab 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -21,7 +21,7 @@ describe('/api/v1/queue', () => { body: 'comment 10', asset_id: 'asset', author_id: '123', - status: [{ + status_history: [{ type: 'rejected' }] }, { @@ -29,14 +29,14 @@ describe('/api/v1/queue', () => { body: 'comment 20', asset_id: 'asset', author_id: '456', - status: [{ + status_history: [{ type: 'premod' }] }, { id: 'hij', body: 'comment 30', asset_id: '456', - status: [{ + status_history: [{ type: 'accepted' }] }]; diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index a128938ba..67088e37e 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -25,7 +25,7 @@ describe('/api/v1/stream', () => { body: 'comment 10', author_id: '', parent_id: '', - status: [{ + status_history: [{ type: 'accepted' }] }, { @@ -33,21 +33,21 @@ describe('/api/v1/stream', () => { body: 'comment 20', author_id: '', parent_id: '', - status: [] + status_history: [] }, { id: 'uio', body: 'comment 30', asset_id: 'asset', author_id: '456', parent_id: '', - status: [{ + status_history: [{ type: 'accepted' }] }, { id: 'hij', body: 'comment 40', asset_id: '456', - status: [{ + status_history: [{ type: 'rejected' }] }]; @@ -116,6 +116,16 @@ describe('/api/v1/stream', () => { }); }); + it('should reject requests without a scheme in the asset_url', () => { + return chai.request(app) + .get('/api/v1/stream') + .query({asset_url: 'test.com'}) + .catch((err) => { + expect(err).to.have.status(400); + expect(err.response.body.message).to.contain('asset_url is invalid'); + }); + }); + it('should merge the settings when the asset contains settings to override it with', () => { return chai.request(app) .get('/api/v1/stream') @@ -126,6 +136,7 @@ describe('/api/v1/stream', () => { expect(res.body.comments.length).to.equal(1); expect(res.body.users.length).to.equal(1); expect(res.body.settings).to.have.property('moderation', 'pre'); + expect(res.body.settings).to.not.have.property('wordlist'); }); }); }); From 9ea6c667443559c437dd007d5a3ad0c810ac2445 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Thu, 8 Dec 2016 10:59:56 -1000 Subject: [PATCH 06/16] Revert "Status history" --- bin/cli-serve | 4 - cache.js | 15 -- models/action.js | 29 --- models/asset.js | 26 +-- models/comment.js | 296 +++++++++-------------------- models/setting.js | 117 ++++-------- models/user.js | 3 +- package.json | 1 - routes/api/comments/index.js | 87 +++------ routes/api/queue/index.js | 68 ++----- routes/api/settings/index.js | 22 +-- routes/api/stream/index.js | 20 +- services/wordlist.js | 2 +- tests/.eslintrc.json | 6 +- tests/models/action.js | 47 ++--- tests/models/asset.js | 2 +- tests/models/comment.js | 152 +++------------ tests/models/setting.js | 26 ++- tests/models/user.js | 2 +- tests/routes/api/comments/index.js | 166 +++++++--------- tests/routes/api/queue/index.js | 124 ++++++------ tests/routes/api/settings/index.js | 4 +- tests/routes/api/stream/index.js | 62 +++--- 23 files changed, 435 insertions(+), 846 deletions(-) diff --git a/bin/cli-serve b/bin/cli-serve index dd682f5ad..a6790c6a0 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -96,10 +96,6 @@ function startApp() { server.listen(port); server.on('error', onError); server.on('listening', onListening); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); }); } diff --git a/cache.js b/cache.js index 1d090e46d..9345f8cde 100644 --- a/cache.js +++ b/cache.js @@ -76,21 +76,6 @@ cache.get = (key) => new Promise((resolve, reject) => { }); }); -/** - * This invalidates a cached entry in the cache. - * @param {Mixed} key Either an array of items composing a key or a string - * @return {Promise} - */ -cache.invalidate = (key) => new Promise((resolve, reject) => { - cache.client.del(keyfunc(key), (err) => { - if (err) { - return reject(err); - } - - resolve(); - }); -}); - /** * This sets a value on the key with the expiry and then resolves once it is * done. diff --git a/models/action.js b/models/action.js index b5d2ab8b6..891dbd4f5 100644 --- a/models/action.js +++ b/models/action.js @@ -1,6 +1,5 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); -const _ = require('lodash'); const Schema = mongoose.Schema; const ActionSchema = new Schema({ @@ -67,34 +66,6 @@ ActionSchema.statics.findByItemIdArray = function(item_ids) { }); }; -/** - * Fetches the action summaries for the given asset, and comments around the - * given user id. - * @param {[type]} asset_id [description] - * @param {[type]} comments [description] - * @param {String} [current_user_id=''] [description] - * @return {[type]} [description] - */ -ActionSchema.statics.getActionSummariesFromComments = (asset_id = '', comments, current_user_id = '') => { - - // 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 actions for pretty much everything at this point. - return Action.getActionSummaries(_.uniq([ - - // Actions can be on assets... - asset_id, - - // Comments... - ...comments.map((comment) => comment.id), - - // Or Authors... - ...userIDs - ].filter((e) => e)), current_user_id); -}; - /** * Returns summaries of actions for an array of ids * @param {String} ids array of user identifiers (uuid) diff --git a/models/asset.js b/models/asset.js index a0619576f..165ebbc6c 100644 --- a/models/asset.js +++ b/models/asset.js @@ -1,8 +1,6 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; -const Setting = require('./setting'); - const uuid = require('uuid'); const AssetSchema = new Schema({ @@ -60,6 +58,11 @@ AssetSchema.index({ 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). @@ -72,25 +75,6 @@ AssetSchema.statics.findById = (id) => Asset.findOne({id}); */ AssetSchema.statics.findByUrl = (url) => Asset.findOne({url}); -/** - * Retrieves the settings given an asset query and rectifies it against the - * global settings. - * @param {Promise} assetQuery an asset query that returns a single asset. - * @return {Promise} - */ -AssetSchema.statics.rectifySettings = (assetQuery) => Promise.all([ - Setting.retrieve(), - assetQuery -]).then(([settings, asset]) => { - - // If the asset exists and has settings then return the merged object. - if (asset && asset.settings) { - return Object.assign({}, settings, asset.settings); - } - - return settings; -}); - /** * Finds a asset by its url. * diff --git a/models/comment.js b/models/comment.js index af9aa4cb1..c1a32cf1b 100644 --- a/models/comment.js +++ b/models/comment.js @@ -1,37 +1,9 @@ const mongoose = require('../mongoose'); -const Schema = mongoose.Schema; const uuid = require('uuid'); const Action = require('./action'); -/** - * The Mongo schema for a Comment Status. - * @type {Schema} - */ -const StatusSchema = new Schema({ - type: { - type: String, - enum: [ - 'accepted', - 'rejected', - 'premod', - ], - }, +const Schema = mongoose.Schema; - // The User ID of the user that assigned the status. - assigned_by: { - type: String, - default: null - }, - - created_at: Date -}, { - _id: false -}); - -/** - * The Mongo schema for a Comment. - * @type {Schema} - */ const CommentSchema = new Schema({ id: { type: String, @@ -45,7 +17,11 @@ const CommentSchema = new Schema({ }, asset_id: String, author_id: String, - status: [StatusSchema], + status: { + type: String, + enum: ['accepted', 'rejected', ''], + default: '' + }, parent_id: String }, { timestamps: { @@ -54,168 +30,90 @@ const CommentSchema = new Schema({ } }); -/** - * toJSON overrides to remove fields from the json - * output. - */ -CommentSchema.options.toJSON = {}; -CommentSchema.options.toJSON.hide = '_id status'; -CommentSchema.options.toJSON.transform = (doc, ret, options) => { - if (options.hide) { - options.hide.split(' ').forEach((prop) => { - delete ret[prop]; - }); - } - - return ret; -}; - -/** - * toJSON overrides to remove fields from the json - * output. - */ -CommentSchema.options.toJSON = {}; -CommentSchema.options.toJSON.hide = '_id'; -CommentSchema.options.toJSON.transform = (doc, ret, options) => { - if (options.hide) { - options.hide.split(' ').forEach((prop) => { - delete ret[prop]; - }); - } - - return ret; -}; - -/** - * Sets up a virtual getter function on a comment such that when you try and - * access the `comment.last_status` it returns the last status in the array - * of status's on the comment, or `null` if there was no status. - */ -CommentSchema.virtual('last_status').get(function() { - if (this.status && this.status.length > 0) { - return this.status[this.status.length - 1].type; - } - - return null; -}); - -/** - * Creates a new Comment that came from a public source. - * @param {Mixed} comment either a single comment or an array of comments. - * @return {Promise} - */ -CommentSchema.statics.publicCreate = (comment) => { - - // Check to see if this is an array of comments, if so map it out. - if (Array.isArray(comment)) { - return Promise.all(comment.map(Comment.publicCreate)); - } - - const { - body, - asset_id, - parent_id, - status = false, - author_id - } = comment; - - comment = new Comment({ - body, - asset_id, - parent_id, - status: status ? [{ - type: status, - created_at: new Date() - }] : [], - author_id - }); - - return comment.save(); -}; +//============================================================================== +// Find Statics +//============================================================================== /** * Finds a comment by the id. * @param {String} id identifier of comment (uuid) * @return {Promise} */ -CommentSchema.statics.findById = (id) => Comment.findOne({id}); +CommentSchema.statics.findById = function(id) { + return Comment.findOne({'id': id}); +}; /** * Finds ALL the comments by the asset_id. * @param {String} asset_id identifier of the asset which owns this comment (uuid) * @return {Promise} */ -CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({ - asset_id -}); +CommentSchema.statics.findByAssetId = function(asset_id) { + return Comment.find({asset_id}); +}; /** - * Finds the accepted comments by the asset_id get the comments that are - * accepted. + * Finds the accepted comments by the asset_id. + * get the comments that are accepted. * @param {String} asset_id identifier of the asset which owns the comments (uuid) * @return {Promise} */ -CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ - asset_id, - 'status.type': 'accepted' -}); +CommentSchema.statics.findAcceptedByAssetId = function(asset_id) { + return Comment.find({asset_id: asset_id, status:'accepted'}); +}; /** * Finds the new and accepted comments by the asset_id. * @param {String} asset_id identifier of the asset which owns the comments (uuid) * @return {Promise} */ -CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({ - asset_id, - $or: [ - { - 'status.type': 'accepted' - }, - { - status: { - $size: 0 - } - } - ] -}); +CommentSchema.statics.findAcceptedAndNewByAssetId = function(asset_id) { + return Comment.find({asset_id: asset_id, status: {'$in': ['accepted', '']}}); +}; /** * Find comments by an action that was performed on them. * @param {String} action_type the type of action that was performed on the comment * @return {Promise} */ -CommentSchema.statics.findByActionType = (action_type) => Action - .findCommentsIdByActionType(action_type, 'comment') - .then((actions) => Comment.find({ - id: { - $in: actions.map((a) => a.item_id) - } - })); +CommentSchema.statics.findByActionType = function(action_type) { + return Action + .findCommentsIdByActionType(action_type, 'comment') + .then((actions) => { + return Comment.find({'id': {'$in': actions.map(function(a){ + return a.item_id;})} + }); + }); +}; /** - * Find comment id's where the action type matches the argument. + * Find not moderated comments by an action that was performed on them. * @param {String} action_type the type of action that was performed on the comment + * @param {String} status the status of the comment to search for * @return {Promise} */ -CommentSchema.statics.findIdsByActionType = (action_type) => Action - .findCommentsIdByActionType(action_type, 'comment') - .then((actions) => actions.map(a => a.item_id)); +CommentSchema.statics.findByStatusByActionType = function(status, action_type) { + return Action + .findCommentsIdByActionType(action_type, 'comment') + .then((actions) => { + return Comment.find({ + status: status, + id: { + $in: actions.map(a => a.item_id) + } + }); + }); +}; /** * Find comments by their status. * @param {String} status the status of the comment to search for * @return {Promise} */ -CommentSchema.statics.findByStatus = (status = false) => { - let q = {}; - - if (status) { - q['status.type'] = status; - } else { - q.status = {$size: 0}; - } - - return Comment.find(q); +CommentSchema.statics.findByStatus = function(status) { + return Comment.find({ + status: status === 'new' ? '' : status + }); }; /** @@ -223,59 +121,39 @@ CommentSchema.statics.findByStatus = (status = false) => { * @param {String} moderationValue pre or post moderation setting. If it is undefined then look at the settings. * @return {Promise} */ -CommentSchema.statics.moderationQueue = (moderation, asset_id = false) => { +CommentSchema.statics.moderationQueue = function(moderation) { + switch(moderation){ - /** - * This adds the asset_id requirement to the query if the asset_id is defined. - */ - const assetIDWrap = (query) => { - if (asset_id) { - query = query.where('asset_id', asset_id); - } + // Pre-moderation: New comments are shown in the moderator queues immediately. + case 'pre': + return Comment.findByStatus('').then((comments) => { + return comments; + }); - return query; - }; + // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. + case 'post': + return Comment.findByStatusByActionType('', 'flag').then((comments) => { + return comments; + }); - // Decide on whether or not we need to load extended options for the - // moderation based on the moderation options. - let comments; - - if (moderation === 'pre') { - - // Pre-moderation: New comments are shown in the moderator queues immediately. - comments = assetIDWrap(CommentSchema.statics.findByStatus('premod')); - - } else { - - // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. - comments = CommentSchema.statics.findIdsByActionType('flag') - .then((ids) => assetIDWrap(Comment.find({ - id: { - $in: ids - } - }))); + default: + return Promise.reject(Error('Moderation setting not found.')); } - - return comments; }; +//============================================================================== +// Update Statics +//============================================================================== + /** - * Pushes a new status in for the user. - * @param {String} id identifier of the comment (uuid) - * @param {String} status the new status of the comment - * @param {String} assigned_by the user id for the user who performed the - * moderation action + * Change the status of a comment. + * @param {String} id identifier of the comment (uuid) + * @param {String} status the new status of the comment * @return {Promise} */ -CommentSchema.statics.pushStatus = (id, status, assigned_by = null) => Comment.update({id}, { - $push: { - status: { - type: status, - created_at: new Date(), - assigned_by - } - } -}); +CommentSchema.statics.changeStatus = function(id, status) { + return Comment.findOneAndUpdate({'id': id}, {$set: {'status': status}}); +}; /** * Add an action to the comment. @@ -291,13 +169,19 @@ CommentSchema.statics.addAction = (item_id, user_id, action_type) => Action.inse action_type }); +//============================================================================== +// Remove Statics +//============================================================================== + /** * Change the status of a comment. * @param {String} id identifier of the comment (uuid) * @param {String} status the new status of the comment * @return {Promise} */ -CommentSchema.statics.removeById = (id) => Comment.remove({id}); +CommentSchema.statics.removeById = function(id) { + return Comment.remove({'id': id}); +}; /** * Remove an action from the comment. @@ -306,18 +190,22 @@ CommentSchema.statics.removeById = (id) => Comment.remove({id}); * @param {String} user_id the id of the user performing the action * @return {Promise} */ -CommentSchema.statics.removeAction = (item_id, user_id, action_type) => Action.remove({ - action_type, - item_type: 'comment', - item_id, - user_id -}); +CommentSchema.statics.removeAction = function(item_id, user_id, action_type) { + return Action.remove({ + action_type, + item_type: 'comment', + item_id, + user_id + }); +}; /** * Returns all the comments in the collection. * @return {Promise} */ -CommentSchema.statics.all = () => Comment.find(); +CommentSchema.statics.all = () => { + return Comment.find(); +}; // Comment model. const Comment = mongoose.model('Comment', CommentSchema); diff --git a/models/setting.js b/models/setting.js index 22b2b103f..bf5ac290a 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,33 +1,18 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; -const _ = require('lodash'); -const cache = require('../cache'); /** - * SettingSchema manages application settings that get used on front and backend. + * this Schema manages application settings that get used on front and backend + * NOTE: when you set a setting here, it will not automatically be exposed to + * the front end. You must add it to the whitelist in the settings route + * in /routes/api/settings/index.js * @type {Schema} */ const SettingSchema = new Schema({ - id: { - type: String, - default: '1' - }, - moderation: { - type: String, - enum: [ - 'pre', - 'post' - ], - default: 'pre' - }, - infoBoxEnable: { - type: Boolean, - default: false - }, - infoBoxContent: { - type: String, - default: '' - }, + id: {type: String, default: '1'}, + moderation: {type: String, enum: ['pre', 'post'], default: 'pre'}, + infoBoxEnable: {type: Boolean, default: false}, + infoBoxContent: {type: String, default: ''}, wordlist: [String] }, { timestamps: { @@ -37,70 +22,48 @@ const SettingSchema = new Schema({ }); /** - * The Mongo Mongoose object. + * this is run once when the app starts to ensure settings are populated + * @return {Promise} null initialize the global settings object */ -const Setting = mongoose.model('Setting', SettingSchema); - -/** - * The Setting Service object exposing the Setting model. - * @type {Object} - */ -const SettingService = module.exports = {}; - -/** - * The selector used to uniquely identify the settings document. - */ -const selector = {id: '1'}; - -/** - * Cache expiry time in seconds for when the cached entry of the settings object - * expires. 2 minutes. - */ -const EXPIRY_TIME = 60 * 2; +SettingSchema.statics.init = function (defaults) { + return this.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); +}; /** * Gets the entire settings record and sends it back * @return {Promise} settings the whole settings record */ -SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => Setting.findOne(selector)); +SettingSchema.statics.getSettings = function () { + return this.findOne({id: '1'}); +}; + +/** + * Gets the settings visible to the public + * @return {Promise} moderation the settings for how to moderate comments + */ +SettingSchema.statics.getPublicSettings = function () { + return this.findOne({id: '1'}).select('moderation infoBoxEnable infoBoxContent'); +}; + +/** + * Gets the info box settings and sends it back + * @return {Promise} content the content of the info Box + */ +SettingSchema.statics.getInfoBoxSetting = function () { + return this.findOne({id: '1'}).select('infoBoxEnable infoBoxContent'); +}; /** * This will update the settings object with whatever you pass in * @param {object} setting a hash of whatever settings you want to update * @return {Promise} settings Promise that resolves to the entire (updated) settings object. */ -SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { - $set: settings -}, { - upsert: true, - new: true, - setDefaultsOnInsert: true -}).then((settings) => { - - // Invalidate the settings cache. - return cache - .set('settings', settings, EXPIRY_TIME) - .then(() => settings); -}); - -/** - * Filters the document to ensure that the resulting document is indeed ready - * for non authenticated users. - * @param {Object} settings the source settings object - * @return {Object} the filtered settings object - */ -SettingService.public = (settings) => _.pick(settings, ['moderation', 'infoBoxEnable', 'infoBoxContent']); - -/** - * This is run once when the app starts to ensure settings are populated. - * @return {Promise} null initialize the global settings object - */ -SettingService.init = (defaults) => { - - // Inject the defaults on top of the passed in defaults to ensure that the new - // settings conform to the required selector. - defaults = Object.assign({}, defaults, selector); - - // Actually update the settings collection. - return SettingService.update(defaults); +SettingSchema.statics.updateSettings = function (setting) { + // There should only ever be one record unless something has gone wrong. + // In the future we may have multiple records for custom settings for objects/users. + return this.findOneAndUpdate({id: '1'}, {$set: setting}, {new: true}); }; + +const Setting = mongoose.model('Setting', SettingSchema); + +module.exports = Setting; diff --git a/models/user.js b/models/user.js index 336486a54..86908ab42 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' ]; @@ -105,7 +106,7 @@ UserSchema.index({ * output. */ UserSchema.options.toJSON = {}; -UserSchema.options.toJSON.hide = '_id password profiles roles disabled'; +UserSchema.options.toJSON.hide = 'password profiles roles disabled'; UserSchema.options.toJSON.transform = (doc, ret, options) => { if (options.hide) { options.hide.split(' ').forEach((prop) => { diff --git a/package.json b/package.json index c54bd6723..be6c2b153 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,6 @@ "eslint-config-standard": "^6.2.1", "eslint-plugin-flowtype": "^2.25.0", "eslint-plugin-import": "^2.2.0", - "eslint-plugin-mocha": "^4.7.0", "eslint-plugin-promise": "^3.3.1", "eslint-plugin-react": "^6.6.0", "eslint-plugin-standard": "^2.0.1", diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index b82c1faf3..3bdf4afc6 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -1,6 +1,5 @@ const express = require('express'); const Comment = require('../../../models/comment'); -const Asset = require('../../../models/asset'); const User = require('../../../models/user'); const Action = require('../../../models/action'); const wordlist = require('../../../services/wordlist'); @@ -10,45 +9,24 @@ const _ = require('lodash'); const router = express.Router(); router.get('/', authorization.needed('admin'), (req, res, next) => { - - const { - status = null, - action_type = null, - asset_id = null - } = req.query; - - /** - * This adds the asset_id requirement to the query if the asset_id is defined. - */ - const assetIDWrap = (query) => { - if (asset_id) { - query = query.where('asset_id', asset_id); - } - - return query; - }; - let query; - if (status) { - query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status)); - } else if (action_type) { - query = Comment - .findIdsByActionType(action_type) - .then((ids) => assetIDWrap(Comment.find({ - id: { - $in: ids - }, - }))); + if (req.query.status) { + query = Comment.findByStatus(req.query.status); + } else if (req.query.action_type) { + query = Comment.findByActionType(req.query.action_type); } else { - query = assetIDWrap(Comment.all()); + query = Comment.all(); } query.then((comments) => { return Promise.all([ comments, User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), - Action.getActionSummariesFromComments(asset_id, comments, req.user ? req.user.id : false) + Action.getActionSummaries(_.uniq([ + ...comments.map((comment) => comment.id), + ...comments.map((comment) => comment.author_id) + ])) ]); }) .then(([comments, users, actions])=> @@ -70,38 +48,21 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { parent_id } = req.body; - // Decide the status based on whether or not the current asset/settings - // has pre-mod enabled or not. If the comment was rejected based on the - // wordlist, then reject it, otherwise if the moderation setting is - // premod, set it to `premod`. - let status; + Comment + .create({ + body, + asset_id, + parent_id, + status: req.wordlist.matched ? 'rejected' : '', + author_id: req.user.id + }) + .then((comment) => { - if (req.wordlist.matched) { - status = Promise.resolve('rejected'); - } else { - status = Asset - .rectifySettings(Asset.findById(asset_id)) - - // Return `premod` if pre-moderation is enabled and an empty "new" status - // in the event that it is not in pre-moderation mode. - .then(({moderation}) => moderation === 'pre' ? 'premod' : ''); - } - - status.then((status) => Comment.publicCreate({ - body, - asset_id, - parent_id, - status, - author_id: req.user.id - })) - .then((comment) => { - - // The comment was created! Send back the created comment. - res.status(201).send(comment); - }) - .catch((err) => { - next(err); - }); + res.status(201).send(comment); + }) + .catch((err) => { + next(err); + }); }); router.get('/:comment_id', authorization.needed('admin'), (req, res, next) => { @@ -138,7 +99,7 @@ router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next } = req.body; Comment - .pushStatus(req.params.comment_id, status, req.user.id) + .changeStatus(req.params.comment_id, status) .then(() => { res.status(204).end(); }) diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index 9c85fbe94..b38d1b763 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -3,7 +3,6 @@ const Comment = require('../../../models/comment'); const User = require('../../../models/user'); const Action = require('../../../models/action'); const Setting = require('../../../models/setting'); -const Asset = require('../../../models/asset'); const _ = require('lodash'); const router = express.Router(); @@ -17,52 +16,27 @@ const router = express.Router(); // 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) => { - - const { - asset_id - } = req.query; - - let settings = Setting.retrieve(); - - if (asset_id) { - - // In the event that we have an asset_id, we should fetch the asset settings - // in order to actually determine if there is additional comments to parse. - settings = Promise.all([ - settings, - Asset.findById(asset_id).select('settings') - ]).then(([{moderation}, asset]) => { - if (asset.settings && asset.settings.moderation) { - return {moderation: asset.settings.moderation}; - } - - return {moderation}; - }); - } - - settings - .then(({moderation}) => { - return Comment.moderationQueue(moderation); - }).then((comments) => { - return Promise.all([ - comments, - User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), - Action.getActionSummaries(_.uniq([ - ...comments.map((comment) => comment.id), - ...comments.map((comment) => comment.author_id) - ])) - ]); - }) - .then(([comments, users, actions]) => { - res.json({ - comments, - users, - actions - }); - }) - .catch(error => { - next(error); - }); + Setting.getPublicSettings().then(({moderation}) => + Comment.moderationQueue(moderation)) + .then((comments) => { + return Promise.all([ + comments, + User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), + Action.getActionSummaries(_.uniq([ + ...comments.map((comment) => comment.id), + ...comments.map((comment) => comment.author_id) + ])) + ]); + }) + .then(([comments, users, actions])=> + res.status(200).json({ + comments, + users, + actions + })) + .catch(error => { + next(error); + }); }); module.exports = router; diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 76d16c7ec..910524fb2 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -4,21 +4,19 @@ const Setting = require('../../../models/setting'); const router = express.Router(); router.get('/', (req, res, next) => { - Setting.retrieve().then((settings) => { - res.json(settings); - }) - .catch((err) => { - next(err); - }); + Setting + .getSettings() + .then(settings => { + res.json(settings); + }) + .catch(next); }); router.put('/', (req, res, next) => { - Setting.update(req.body).then(() => { - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); + Setting + .updateSettings(req.body) + .then(() => res.status(204).end()) + .catch(next); }); module.exports = router; diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 26947be09..4e8373999 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -26,14 +26,14 @@ router.get('/', (req, res, next) => { return asset; }), - // Get the moderation setting from the settings. - Setting.retrieve() + // Get the public settings. + Setting.getPublicSettings() ]) .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 && asset.settings) { + if (asset.settings) { settings = Object.assign({}, settings, asset.settings); } @@ -70,7 +70,17 @@ router.get('/', (req, res, next) => { let users = userIDs.length > 0 ? User.findByIdArray(userIDs) : []; // Fetch the actions for pretty much everything at this point. - let actions = Action.getActionSummariesFromComments(asset.id, comments, req.user ? req.user.id : false); + 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([ @@ -98,7 +108,7 @@ router.get('/', (req, res, next) => { comments, users, actions, - settings: Setting.public(settings) + settings }); }) .catch(error => { diff --git a/services/wordlist.js b/services/wordlist.js index 59a842817..e6f2ad668 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -20,7 +20,7 @@ const wordlist = { */ wordlist.init = () => { return Setting - .retrieve() + .getSettings() .then((settings) => { // Insert the settings wordlist. diff --git a/tests/.eslintrc.json b/tests/.eslintrc.json index b188e92cf..363ef5a28 100644 --- a/tests/.eslintrc.json +++ b/tests/.eslintrc.json @@ -4,12 +4,8 @@ "node": true, "mocha": true }, - "plugins": [ - "mocha" - ], "extends": "../.eslintrc.json", "rules": { - "no-undef": [0], - "mocha/no-exclusive-tests": "warn" + "no-undef": [0] } } diff --git a/tests/models/action.js b/tests/models/action.js index 5f05dea71..c354b8f83 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -1,34 +1,35 @@ const Action = require('../../models/action'); const expect = require('chai').expect; -describe('models.Action', () => { - let mockActions = []; +describe('Action: models', () => { + let mockActions; - beforeEach(() => Action.create([{ - action_type: 'flag', - item_id: '123', - item_type: 'comment', - user_id: 'flagginguserid' - }, { - action_type: 'flag', - item_id: '456', - item_type: 'comment' - }, { - action_type: 'flag', - item_id: '123', - item_type: 'comment' - }, { - action_type: 'like', - item_id: '123', - item_type: 'comment' - }]).then((actions) => { - mockActions = actions; - })); + beforeEach(() => { + return Action.create([{ + action_type: 'flag', + item_id: '123', + item_type: 'comment', + user_id: 'flagginguserid' + }, { + action_type: 'flag', + item_id: '456', + item_type: 'comment' + }, { + action_type: 'flag', + item_id: '123', + item_type: 'comment' + }, { + action_type: 'like', + item_id: '123', + item_type: 'comment' + }]).then((actions) => { + mockActions = actions; + }); + }); describe('#findById()', () => { it('should find an action by id', () => { return Action.findById(mockActions[0].id).then((result) => { - expect(result).to.not.be.null; expect(result).to.have.property('action_type', 'flag'); }); }); diff --git a/tests/models/asset.js b/tests/models/asset.js index c9eff819e..69bf99f9d 100644 --- a/tests/models/asset.js +++ b/tests/models/asset.js @@ -6,7 +6,7 @@ const expect = chai.expect; // Use the chai should. chai.should(); -describe('models.Asset', () => { +describe('Asset: model', () => { beforeEach(() => { const defaults = {url:'http://test.com'}; diff --git a/tests/models/comment.js b/tests/models/comment.js index a9c2f4787..07834a186 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -7,57 +7,35 @@ const settings = {id: '1', moderation: 'pre'}; const expect = require('chai').expect; -describe('models.Comment', () => { +describe('Comment: models', () => { const comments = [{ body: 'comment 10', asset_id: '123', - status: [], + status: '', parent_id: '', author_id: '123', id: '1' }, { body: 'comment 20', asset_id: '123', - status: [{ - type: 'accepted' - }], + status: 'accepted', parent_id: '', author_id: '123', id: '2' }, { body: 'comment 30', asset_id: '456', - status: [], + status: '', parent_id: '', author_id: '456', id: '3' }, { body: 'comment 40', asset_id: '123', - status: [{ - type: 'rejected' - }], + status: 'rejected', parent_id: '', author_id: '456', id: '4' - }, { - body: 'comment 50', - asset_id: '1234', - status: [{ - type: 'premod' - }], - parent_id: '', - author_id: '456', - id: '5' - }, { - body: 'comment 60', - asset_id: '1234', - status: [{ - type: 'premod' - }], - parent_id: '', - author_id: '456', - id: '6' }]; const users = [{ @@ -82,69 +60,25 @@ describe('models.Comment', () => { user_id: '456' }]; - beforeEach(() => Promise.all([ - Setting.init(settings), - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ])); - - describe('#publicCreate()', () => { - - it('creates a new comment', () => { - return Comment.publicCreate({ - body: 'This is a comment!', - status: 'accepted' - }).then((c) => { - expect(c).to.not.be.null; - expect(c.id).to.not.be.null; - expect(c.id).to.be.uuid; - expect(c.status).to.have.length(1); - expect(c.status[0]).to.have.property('type', 'accepted'); - }); - }); - - it('creates many new comments', () => { - return Comment.publicCreate([{ - body: 'This is a comment!', - status: 'accepted' - }, { - body: 'This is another comment!' - }, { - body: 'This is a rejected comment!', - status: 'rejected' - }]).then(([c1, c2, c3]) => { - expect(c1).to.not.be.null; - expect(c1.id).to.be.uuid; - expect(c1.status).to.have.length(1); - expect(c1.status[0]).to.have.property('type', 'accepted'); - - expect(c2).to.not.be.null; - expect(c2.id).to.be.uuid; - expect(c2.status).to.have.length(0); - - expect(c3).to.not.be.null; - expect(c3.id).to.be.uuid; - expect(c3.status).to.have.length(1); - expect(c3.status[0]).to.have.property('type', 'rejected'); - }); - }); - + beforeEach(() => { + return Promise.all([ + Setting.create(settings), + Comment.create(comments), + User.createLocalUsers(users), + Action.create(actions) + ]); }); describe('#findById()', () => { - it('should find a comment by id', () => { return Comment.findById('1').then((result) => { expect(result).to.not.be.null; expect(result).to.have.property('body', 'comment 10'); }); }); - }); describe('#findByAssetId()', () => { - it('should find an array of all comments by asset id', () => { return Comment.findByAssetId('123').then((result) => { expect(result).to.have.length(3); @@ -157,7 +91,6 @@ describe('models.Comment', () => { expect(result[2]).to.have.property('body', 'comment 40'); }); }); - it('should find an array of accepted comments by asset id', () => { return Comment.findAcceptedByAssetId('123').then((result) => { expect(result).to.have.length(1); @@ -168,7 +101,6 @@ describe('models.Comment', () => { expect(result[0]).to.have.property('body', 'comment 20'); }); }); - it('should find an array of new and accepted comments by asset id', () => { return Comment.findAcceptedAndNewByAssetId('123').then((result) => { expect(result).to.have.length(2); @@ -180,16 +112,13 @@ describe('models.Comment', () => { }); }); }); - describe('#moderationQueue()', () => { - it('should find an array of new comments to moderate when pre-moderation', () => { return Comment.moderationQueue('pre').then((result) => { expect(result).to.not.be.null; expect(result).to.have.lengthOf(2); }); }); - it('should find an array of new comments to moderate when post-moderation', () => { return Comment.moderationQueue('post').then((result) => { expect(result).to.not.be.null; @@ -197,56 +126,21 @@ describe('models.Comment', () => { expect(result[0]).to.have.property('body', 'comment 30'); }); }); - + // it('should fail when the moderation is not pre or post', () => { + // return Comment.moderationQueue('any').catch(function(error) { + // expect(error).to.not.be.null; + // }); + // }); }); describe('#removeAction', () => { - it('should remove an action', () => { - return Comment.removeAction('3', '123', 'flag') - .then(() => { - return Action.findByItemIdArray(['123']); - }) - .then((actions) => { - expect(actions.length).to.equal(0); - }); + return Comment.removeAction('3', '123', 'flag').then(() => { + return Action.findByItemIdArray(['123']); + }) + .then((actions) => { + expect(actions.length).to.equal(0); + }); }); }); - - describe('#changeStatus', () => { - - it('should change the status of a comment from no status', () => { - let comment_id = comments[0].id; - - return Comment.findById(comment_id) - .then((c) => { - expect(c).to.have.property('status'); - expect(c.status).to.have.length(0); - - return Comment.pushStatus(comment_id, 'rejected', '123'); - }) - .then(() => Comment.findById(comment_id)) - .then((c) => { - expect(c).to.have.property('status'); - expect(c.status).to.have.length(1); - expect(c.status[0]).to.have.property('type', 'rejected'); - expect(c.status[0]).to.have.property('assigned_by', '123'); - }); - }); - - it('should change the status of a comment from accepted', () => { - return Comment.pushStatus(comments[1].id, 'rejected', '123') - .then(() => Comment.findById(comments[1].id)) - .then((c) => { - expect(c).to.have.property('status'); - expect(c.status).to.have.length(2); - expect(c.status[0]).to.have.property('type', 'accepted'); - expect(c.status[0]).to.have.property('assigned_by', null); - - expect(c.status[1]).to.have.property('type', 'rejected'); - expect(c.status[1]).to.have.property('assigned_by', '123'); - }); - }); - - }); }); diff --git a/tests/models/setting.js b/tests/models/setting.js index 3e77297fc..186a65245 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -1,29 +1,34 @@ const Setting = require('../../models/setting'); const expect = require('chai').expect; -describe('models.Setting', () => { +describe('Setting: model', () => { - beforeEach(() => Setting.init({moderation: 'pre'})); + beforeEach(() => { + const defaults = { + id: 1 + }; + return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); + }); - describe('#retrieve()', () => { + describe('#getSettings()', () => { it('should have a moderation field defined', () => { - return Setting.retrieve().then(settings => { + return Setting.getSettings().then(settings => { expect(settings).to.have.property('moderation').and.to.equal('pre'); }); }); it('should have two infoBox fields defined', () => { - return Setting.retrieve().then(settings => { + return Setting.getSettings().then(settings => { expect(settings).to.have.property('infoBoxEnable').and.to.equal(false); expect(settings).to.have.property('infoBoxContent').and.to.equal(''); }); }); }); - describe('#update()', () => { + describe('#updateSettings()', () => { it('should update the settings with a passed object', () => { const mockSettings = {moderation: 'post', infoBoxEnable: true, infoBoxContent: 'yeah'}; - return Setting.update(mockSettings).then(updatedSettings => { + 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); @@ -32,10 +37,13 @@ describe('models.Setting', () => { }); }); - describe('#get', () => { + describe('#getPublicSettings', () => { it('should return the moderation settings', () => { - return Setting.retrieve().then(({moderation}) => { + return Setting.getPublicSettings().then(({moderation, infoBoxEnable, infoBoxContent, wordlist}) => { expect(moderation).not.to.be.null; + expect(infoBoxEnable).not.to.be.null; + expect(infoBoxContent).not.to.be.null; + expect(wordlist).to.be.undefined; }); }); }); diff --git a/tests/models/user.js b/tests/models/user.js index 5e1677016..c8af92054 100644 --- a/tests/models/user.js +++ b/tests/models/user.js @@ -1,7 +1,7 @@ const User = require('../../models/user'); const expect = require('chai').expect; -describe('models.User', () => { +describe('User: models', () => { let mockUsers; beforeEach(() => { return User.createLocalUsers([{ diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index bf7a06ff6..bc8b67bc1 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -10,7 +10,6 @@ chai.use(require('chai-http')); const wordlist = require('../../../../services/wordlist'); const Comment = require('../../../../models/comment'); -const Asset = require('../../../../models/asset'); const Action = require('../../../../models/action'); const User = require('../../../../models/user'); @@ -18,71 +17,63 @@ const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; describe('/api/v1/comments', () => { + 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: 'def-rejected', + body: 'comment 20', + asset_id: 'asset', + author_id: '456', + status: 'rejected' + }, { + id: 'hij', + body: 'comment 30', + asset_id: '456', + author_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', + item_type: 'comment' + }, { + action_type: 'like', + item_id: 'hij', + item_type: 'comment' + }]; + + beforeEach(() => { + return Promise.all([ + Comment.create(comments), + User.createLocalUsers(users), + Action.create(actions), + wordlist.insert([ + 'bad words' + ]), + Setting.create(settings) + ]); + }); describe('#get', () => { - const comments = [{ - body: 'comment 10', - asset_id: 'asset', - author_id: '123' - }, { - body: 'comment 20', - asset_id: 'asset', - author_id: '456' - }, { - body: 'comment 20', - asset_id: 'asset', - author_id: '456', - status: [{ - type: 'rejected' - }] - }, { - body: 'comment 30', - asset_id: '456', - status: [{ - type: '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', - item_type: 'comment' - }, { - action_type: 'like', - item_id: 'hij', - item_type: 'comment' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments).then((newComments) => { - newComments.forEach((comment, i) => { - comments[i].id = comment.id; - }); - - actions[0].item_id = comments[0].id; - actions[1].item_id = comments[1].id; - - return Action.create(actions); - }), - User.createLocalUsers(users), - wordlist.insert([ - 'bad words' - ]), - Setting.init(settings) - ]); - }); - it('should return all the comments', () => { return chai.request(app) .get('/api/v1/comments') @@ -100,9 +91,7 @@ describe('/api/v1/comments', () => { .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); - expect(res.body).to.have.property('comments'); - expect(res.body.comments).to.have.length(1); - expect(res.body.comments[0]).to.have.property('id', comments[2].id); + expect(res.body.comments[0]).to.have.property('id', 'def-rejected'); }); }); @@ -113,7 +102,7 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(200); expect(res.body.comments).to.have.length(1); - expect(res.body.comments[0]).to.have.property('id', comments[3].id); + expect(res.body.comments[0]).to.have.property('id', 'hij'); }); }); @@ -135,7 +124,8 @@ describe('/api/v1/comments', () => { expect(res).to.have.status(200); expect(res.body.comments).to.have.length(1); - expect(res.body.comments[0]).to.have.property('id', comments[0].id); + expect(res.body.comments[0]).to.have.property('id', 'abc'); + }); }); }); @@ -161,31 +151,7 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('status').and.to.have.length(1); - expect(res.body.status[0]).to.have.property('type', 'rejected'); - }); - }); - - it('should create a comment with a premod status if it\'s asset is has pre-moderation enabled', () => { - return Asset - .findOrCreateByUrl('https://coralproject.net/article1') - .then((asset) => { - return Asset - .overrideSettings(asset.id, {moderation: 'pre'}) - .then(() => asset); - }) - .then((asset) => { - return chai.request(app) - .post('/api/v1/comments') - .set(passport.inject({roles: []})) - .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''}); - }) - .then((res) => { - expect(res).to.have.status(201); - expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('asset_id'); - expect(res.body).to.have.property('status').and.to.have.length(1); - expect(res.body.status[0]).to.have.property('type', 'premod'); + expect(res.body).to.have.property('status', 'rejected'); }); }); }); @@ -301,22 +267,18 @@ describe('/api/v1/comments/:comment_id/actions', () => { body: 'comment 10', asset_id: 'asset', author_id: '123', - status: [] + status: '' }, { id: 'def', body: 'comment 20', asset_id: 'asset', author_id: '456', - status: [{ - type: 'rejected' - }] + status: 'rejected' }, { id: 'hij', body: 'comment 30', asset_id: '456', - status: [{ - type: 'accepted' - }] + status: 'accepted' }]; const users = [{ @@ -354,8 +316,10 @@ describe('/api/v1/comments/:comment_id/actions', () => { .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 d64423013..1e99ae61e 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -15,54 +15,56 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -describe('/api/v1/queue', () => { - const comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - author_id: '123', - status: [{ - type: 'rejected' - }] - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset', - author_id: '456', - status: [{ - type: 'premod' - }] - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456', - status: [{ - type: 'accepted' - }] - }]; +beforeEach(() => { + return Setting.create(settings); +}); - const users = [{ - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; +describe('Get moderation queues rejected, pending, flags', () => { - const actions = [{ - action_type: 'flag', - item_id: 'abc', - item_type: 'comment' - }, { - action_type: 'like', - item_id: 'hij', - item_type: 'comment' - }]; + describe('/api/v1/queue', () => { + let comments; - beforeEach(() => { - return User.createLocalUsers(users) + const users = [{ + id: '456', + displayName: 'Ana', + email: 'ana@gmail.com', + password: '123' + }, { + id: '123', + displayName: 'Maria', + email: 'maria@gmail.com', + password: '123' + }]; + + let actions; + + beforeEach(() => { + + comments = [{ + id: 'abc', + body: 'comment 10', + asset_id: 'asset', + status: 'rejected' + }, { + id: 'def', + body: 'comment 20', + asset_id: 'asset' + }, { + id: 'hij', + body: 'comment 30', + asset_id: '456', + status: 'accepted' + }]; + + actions = [{ + action_type: 'flag', + item_type: 'comment' + }, { + action_type: 'like', + item_type: 'comment' + }]; + + return User.createLocalUsers(users) .then((u) => { comments[0].author_id = u[0].id; comments[1].author_id = u[1].id; @@ -74,24 +76,22 @@ describe('/api/v1/queue', () => { actions[0].item_id = c[0].id; actions[1].item_id = c[1].id; - return Promise.all([ - Action.create(actions), - Setting.init(settings) - ]); + return Action.create(actions); }); - }); + }); - it('should return all the pending comments, users and actions', 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.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'); - done(); - }); + it('should return all the pending comments, users and actions', 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.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'); + done(); + }); + }); }); }); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index 318af81f4..d1a7ba81b 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -12,7 +12,7 @@ const defaults = {id: '1', moderation: 'pre'}; describe('/api/v1/settings', () => { - beforeEach(() => Setting.init(defaults)); + beforeEach(() => Setting.create(defaults)); describe('#get', () => { @@ -40,7 +40,7 @@ describe('/api/v1/settings', () => { .then((res) => { expect(res).to.have.status(204); - return Setting.retrieve(); + 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 a128938ba..6484fdc12 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -20,37 +20,7 @@ describe('/api/v1/stream', () => { moderation: 'post' }; - const comments = [{ - id: 'abc', - body: 'comment 10', - author_id: '', - parent_id: '', - status: [{ - type: 'accepted' - }] - }, { - id: 'def', - body: 'comment 20', - author_id: '', - parent_id: '', - status: [] - }, { - id: 'uio', - body: 'comment 30', - asset_id: 'asset', - author_id: '456', - parent_id: '', - status: [{ - type: 'accepted' - }] - }, { - id: 'hij', - body: 'comment 40', - asset_id: '456', - status: [{ - type: 'rejected' - }] - }]; + let comments; const users = [{ displayName: 'Ana', @@ -71,6 +41,33 @@ describe('/api/v1/stream', () => { }]; beforeEach(() => { + + comments = [{ + id: 'abc', + body: 'comment 10', + author_id: '', + parent_id: '', + status: 'accepted' + }, { + id: 'def', + body: 'comment 20', + author_id: '', + parent_id: '', + status: '' + }, { + id: 'uio', + body: 'comment 30', + asset_id: 'asset', + author_id: '456', + parent_id: '', + status: 'accepted' + }, { + id: 'hij', + body: 'comment 40', + asset_id: '456', + status: 'rejected' + }]; + return Promise.all([ User.createLocalUsers(users), Asset.findOrCreateByUrl('http://test.com'), @@ -97,11 +94,10 @@ describe('/api/v1/stream', () => { return Promise.all([ Comment.create(comments), Action.create(actions), - Setting.init(settings) + Setting.init().then(() => Setting.updateSettings(settings)) ]); }); }); - it('should return a stream with comments, users and actions for an existing asset', () => { return chai.request(app) .get('/api/v1/stream') From aaebbabef9dfed3be977912a4794299d232bb695 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Dec 2016 16:12:44 -0500 Subject: [PATCH 07/16] Revert "Revert "Status history"" --- bin/cli-serve | 4 + cache.js | 15 ++ models/action.js | 29 +++ models/asset.js | 26 ++- models/comment.js | 296 ++++++++++++++++++++--------- models/setting.js | 117 ++++++++---- models/user.js | 3 +- package.json | 1 + routes/api/comments/index.js | 87 ++++++--- routes/api/queue/index.js | 68 +++++-- routes/api/settings/index.js | 22 ++- routes/api/stream/index.js | 20 +- services/wordlist.js | 2 +- tests/.eslintrc.json | 6 +- tests/models/action.js | 47 +++-- tests/models/asset.js | 2 +- tests/models/comment.js | 152 ++++++++++++--- tests/models/setting.js | 26 +-- tests/models/user.js | 2 +- tests/routes/api/comments/index.js | 166 +++++++++------- tests/routes/api/queue/index.js | 124 ++++++------ tests/routes/api/settings/index.js | 4 +- tests/routes/api/stream/index.js | 62 +++--- 23 files changed, 846 insertions(+), 435 deletions(-) diff --git a/bin/cli-serve b/bin/cli-serve index a6790c6a0..dd682f5ad 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -96,6 +96,10 @@ function startApp() { server.listen(port); server.on('error', onError); server.on('listening', onListening); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); }); } diff --git a/cache.js b/cache.js index 9345f8cde..1d090e46d 100644 --- a/cache.js +++ b/cache.js @@ -76,6 +76,21 @@ cache.get = (key) => new Promise((resolve, reject) => { }); }); +/** + * This invalidates a cached entry in the cache. + * @param {Mixed} key Either an array of items composing a key or a string + * @return {Promise} + */ +cache.invalidate = (key) => new Promise((resolve, reject) => { + cache.client.del(keyfunc(key), (err) => { + if (err) { + return reject(err); + } + + resolve(); + }); +}); + /** * This sets a value on the key with the expiry and then resolves once it is * done. diff --git a/models/action.js b/models/action.js index 891dbd4f5..b5d2ab8b6 100644 --- a/models/action.js +++ b/models/action.js @@ -1,5 +1,6 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); +const _ = require('lodash'); const Schema = mongoose.Schema; const ActionSchema = new Schema({ @@ -66,6 +67,34 @@ ActionSchema.statics.findByItemIdArray = function(item_ids) { }); }; +/** + * Fetches the action summaries for the given asset, and comments around the + * given user id. + * @param {[type]} asset_id [description] + * @param {[type]} comments [description] + * @param {String} [current_user_id=''] [description] + * @return {[type]} [description] + */ +ActionSchema.statics.getActionSummariesFromComments = (asset_id = '', comments, current_user_id = '') => { + + // 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 actions for pretty much everything at this point. + return Action.getActionSummaries(_.uniq([ + + // Actions can be on assets... + asset_id, + + // Comments... + ...comments.map((comment) => comment.id), + + // Or Authors... + ...userIDs + ].filter((e) => e)), current_user_id); +}; + /** * Returns summaries of actions for an array of ids * @param {String} ids array of user identifiers (uuid) diff --git a/models/asset.js b/models/asset.js index 165ebbc6c..a0619576f 100644 --- a/models/asset.js +++ b/models/asset.js @@ -1,6 +1,8 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; +const Setting = require('./setting'); + const uuid = require('uuid'); const AssetSchema = new Schema({ @@ -58,11 +60,6 @@ AssetSchema.index({ 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). @@ -75,6 +72,25 @@ AssetSchema.statics.findById = (id) => Asset.findOne({id}); */ AssetSchema.statics.findByUrl = (url) => Asset.findOne({url}); +/** + * Retrieves the settings given an asset query and rectifies it against the + * global settings. + * @param {Promise} assetQuery an asset query that returns a single asset. + * @return {Promise} + */ +AssetSchema.statics.rectifySettings = (assetQuery) => Promise.all([ + Setting.retrieve(), + assetQuery +]).then(([settings, asset]) => { + + // If the asset exists and has settings then return the merged object. + if (asset && asset.settings) { + return Object.assign({}, settings, asset.settings); + } + + return settings; +}); + /** * Finds a asset by its url. * diff --git a/models/comment.js b/models/comment.js index c1a32cf1b..af9aa4cb1 100644 --- a/models/comment.js +++ b/models/comment.js @@ -1,9 +1,37 @@ const mongoose = require('../mongoose'); +const Schema = mongoose.Schema; const uuid = require('uuid'); const Action = require('./action'); -const Schema = mongoose.Schema; +/** + * The Mongo schema for a Comment Status. + * @type {Schema} + */ +const StatusSchema = new Schema({ + type: { + type: String, + enum: [ + 'accepted', + 'rejected', + 'premod', + ], + }, + // The User ID of the user that assigned the status. + assigned_by: { + type: String, + default: null + }, + + created_at: Date +}, { + _id: false +}); + +/** + * The Mongo schema for a Comment. + * @type {Schema} + */ const CommentSchema = new Schema({ id: { type: String, @@ -17,11 +45,7 @@ const CommentSchema = new Schema({ }, asset_id: String, author_id: String, - status: { - type: String, - enum: ['accepted', 'rejected', ''], - default: '' - }, + status: [StatusSchema], parent_id: String }, { timestamps: { @@ -30,90 +54,168 @@ const CommentSchema = new Schema({ } }); -//============================================================================== -// Find Statics -//============================================================================== +/** + * toJSON overrides to remove fields from the json + * output. + */ +CommentSchema.options.toJSON = {}; +CommentSchema.options.toJSON.hide = '_id status'; +CommentSchema.options.toJSON.transform = (doc, ret, options) => { + if (options.hide) { + options.hide.split(' ').forEach((prop) => { + delete ret[prop]; + }); + } + + return ret; +}; + +/** + * toJSON overrides to remove fields from the json + * output. + */ +CommentSchema.options.toJSON = {}; +CommentSchema.options.toJSON.hide = '_id'; +CommentSchema.options.toJSON.transform = (doc, ret, options) => { + if (options.hide) { + options.hide.split(' ').forEach((prop) => { + delete ret[prop]; + }); + } + + return ret; +}; + +/** + * Sets up a virtual getter function on a comment such that when you try and + * access the `comment.last_status` it returns the last status in the array + * of status's on the comment, or `null` if there was no status. + */ +CommentSchema.virtual('last_status').get(function() { + if (this.status && this.status.length > 0) { + return this.status[this.status.length - 1].type; + } + + return null; +}); + +/** + * Creates a new Comment that came from a public source. + * @param {Mixed} comment either a single comment or an array of comments. + * @return {Promise} + */ +CommentSchema.statics.publicCreate = (comment) => { + + // Check to see if this is an array of comments, if so map it out. + if (Array.isArray(comment)) { + return Promise.all(comment.map(Comment.publicCreate)); + } + + const { + body, + asset_id, + parent_id, + status = false, + author_id + } = comment; + + comment = new Comment({ + body, + asset_id, + parent_id, + status: status ? [{ + type: status, + created_at: new Date() + }] : [], + author_id + }); + + return comment.save(); +}; /** * Finds a comment by the id. * @param {String} id identifier of comment (uuid) * @return {Promise} */ -CommentSchema.statics.findById = function(id) { - return Comment.findOne({'id': id}); -}; +CommentSchema.statics.findById = (id) => Comment.findOne({id}); /** * Finds ALL the comments by the asset_id. * @param {String} asset_id identifier of the asset which owns this comment (uuid) * @return {Promise} */ -CommentSchema.statics.findByAssetId = function(asset_id) { - return Comment.find({asset_id}); -}; +CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({ + asset_id +}); /** - * Finds the accepted comments by the asset_id. - * get the comments that are accepted. + * Finds the accepted comments by the asset_id get the comments that are + * accepted. * @param {String} asset_id identifier of the asset which owns the comments (uuid) * @return {Promise} */ -CommentSchema.statics.findAcceptedByAssetId = function(asset_id) { - return Comment.find({asset_id: asset_id, status:'accepted'}); -}; +CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ + asset_id, + 'status.type': 'accepted' +}); /** * Finds the new and accepted comments by the asset_id. * @param {String} asset_id identifier of the asset which owns the comments (uuid) * @return {Promise} */ -CommentSchema.statics.findAcceptedAndNewByAssetId = function(asset_id) { - return Comment.find({asset_id: asset_id, status: {'$in': ['accepted', '']}}); -}; +CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({ + asset_id, + $or: [ + { + 'status.type': 'accepted' + }, + { + status: { + $size: 0 + } + } + ] +}); /** * Find comments by an action that was performed on them. * @param {String} action_type the type of action that was performed on the comment * @return {Promise} */ -CommentSchema.statics.findByActionType = function(action_type) { - return Action - .findCommentsIdByActionType(action_type, 'comment') - .then((actions) => { - return Comment.find({'id': {'$in': actions.map(function(a){ - return a.item_id;})} - }); - }); -}; +CommentSchema.statics.findByActionType = (action_type) => Action + .findCommentsIdByActionType(action_type, 'comment') + .then((actions) => Comment.find({ + id: { + $in: actions.map((a) => a.item_id) + } + })); /** - * Find not moderated comments by an action that was performed on them. + * Find comment id's where the action type matches the argument. * @param {String} action_type the type of action that was performed on the comment - * @param {String} status the status of the comment to search for * @return {Promise} */ -CommentSchema.statics.findByStatusByActionType = function(status, action_type) { - return Action - .findCommentsIdByActionType(action_type, 'comment') - .then((actions) => { - return Comment.find({ - status: status, - id: { - $in: actions.map(a => a.item_id) - } - }); - }); -}; +CommentSchema.statics.findIdsByActionType = (action_type) => Action + .findCommentsIdByActionType(action_type, 'comment') + .then((actions) => actions.map(a => a.item_id)); /** * Find comments by their status. * @param {String} status the status of the comment to search for * @return {Promise} */ -CommentSchema.statics.findByStatus = function(status) { - return Comment.find({ - status: status === 'new' ? '' : status - }); +CommentSchema.statics.findByStatus = (status = false) => { + let q = {}; + + if (status) { + q['status.type'] = status; + } else { + q.status = {$size: 0}; + } + + return Comment.find(q); }; /** @@ -121,39 +223,59 @@ CommentSchema.statics.findByStatus = function(status) { * @param {String} moderationValue pre or post moderation setting. If it is undefined then look at the settings. * @return {Promise} */ -CommentSchema.statics.moderationQueue = function(moderation) { - switch(moderation){ +CommentSchema.statics.moderationQueue = (moderation, asset_id = false) => { - // Pre-moderation: New comments are shown in the moderator queues immediately. - case 'pre': - return Comment.findByStatus('').then((comments) => { - return comments; - }); + /** + * This adds the asset_id requirement to the query if the asset_id is defined. + */ + const assetIDWrap = (query) => { + if (asset_id) { + query = query.where('asset_id', asset_id); + } - // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. - case 'post': - return Comment.findByStatusByActionType('', 'flag').then((comments) => { - return comments; - }); + return query; + }; - default: - return Promise.reject(Error('Moderation setting not found.')); + // Decide on whether or not we need to load extended options for the + // moderation based on the moderation options. + let comments; + + if (moderation === 'pre') { + + // Pre-moderation: New comments are shown in the moderator queues immediately. + comments = assetIDWrap(CommentSchema.statics.findByStatus('premod')); + + } else { + + // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. + comments = CommentSchema.statics.findIdsByActionType('flag') + .then((ids) => assetIDWrap(Comment.find({ + id: { + $in: ids + } + }))); } -}; -//============================================================================== -// Update Statics -//============================================================================== + return comments; +}; /** - * Change the status of a comment. - * @param {String} id identifier of the comment (uuid) - * @param {String} status the new status of the comment + * Pushes a new status in for the user. + * @param {String} id identifier of the comment (uuid) + * @param {String} status the new status of the comment + * @param {String} assigned_by the user id for the user who performed the + * moderation action * @return {Promise} */ -CommentSchema.statics.changeStatus = function(id, status) { - return Comment.findOneAndUpdate({'id': id}, {$set: {'status': status}}); -}; +CommentSchema.statics.pushStatus = (id, status, assigned_by = null) => Comment.update({id}, { + $push: { + status: { + type: status, + created_at: new Date(), + assigned_by + } + } +}); /** * Add an action to the comment. @@ -169,19 +291,13 @@ CommentSchema.statics.addAction = (item_id, user_id, action_type) => Action.inse action_type }); -//============================================================================== -// Remove Statics -//============================================================================== - /** * Change the status of a comment. * @param {String} id identifier of the comment (uuid) * @param {String} status the new status of the comment * @return {Promise} */ -CommentSchema.statics.removeById = function(id) { - return Comment.remove({'id': id}); -}; +CommentSchema.statics.removeById = (id) => Comment.remove({id}); /** * Remove an action from the comment. @@ -190,22 +306,18 @@ CommentSchema.statics.removeById = function(id) { * @param {String} user_id the id of the user performing the action * @return {Promise} */ -CommentSchema.statics.removeAction = function(item_id, user_id, action_type) { - return Action.remove({ - action_type, - item_type: 'comment', - item_id, - user_id - }); -}; +CommentSchema.statics.removeAction = (item_id, user_id, action_type) => Action.remove({ + action_type, + item_type: 'comment', + item_id, + user_id +}); /** * Returns all the comments in the collection. * @return {Promise} */ -CommentSchema.statics.all = () => { - return Comment.find(); -}; +CommentSchema.statics.all = () => Comment.find(); // Comment model. const Comment = mongoose.model('Comment', CommentSchema); diff --git a/models/setting.js b/models/setting.js index bf5ac290a..22b2b103f 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,18 +1,33 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; +const _ = require('lodash'); +const cache = require('../cache'); /** - * this Schema manages application settings that get used on front and backend - * NOTE: when you set a setting here, it will not automatically be exposed to - * the front end. You must add it to the whitelist in the settings route - * in /routes/api/settings/index.js + * SettingSchema manages application settings that get used on front and backend. * @type {Schema} */ const SettingSchema = new Schema({ - id: {type: String, default: '1'}, - moderation: {type: String, enum: ['pre', 'post'], default: 'pre'}, - infoBoxEnable: {type: Boolean, default: false}, - infoBoxContent: {type: String, default: ''}, + id: { + type: String, + default: '1' + }, + moderation: { + type: String, + enum: [ + 'pre', + 'post' + ], + default: 'pre' + }, + infoBoxEnable: { + type: Boolean, + default: false + }, + infoBoxContent: { + type: String, + default: '' + }, wordlist: [String] }, { timestamps: { @@ -22,48 +37,70 @@ const SettingSchema = new Schema({ }); /** - * this is run once when the app starts to ensure settings are populated - * @return {Promise} null initialize the global settings object + * The Mongo Mongoose object. */ -SettingSchema.statics.init = function (defaults) { - return this.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); -}; +const Setting = mongoose.model('Setting', SettingSchema); + +/** + * The Setting Service object exposing the Setting model. + * @type {Object} + */ +const SettingService = module.exports = {}; + +/** + * The selector used to uniquely identify the settings document. + */ +const selector = {id: '1'}; + +/** + * Cache expiry time in seconds for when the cached entry of the settings object + * expires. 2 minutes. + */ +const EXPIRY_TIME = 60 * 2; /** * Gets the entire settings record and sends it back * @return {Promise} settings the whole settings record */ -SettingSchema.statics.getSettings = function () { - return this.findOne({id: '1'}); -}; - -/** - * Gets the settings visible to the public - * @return {Promise} moderation the settings for how to moderate comments - */ -SettingSchema.statics.getPublicSettings = function () { - return this.findOne({id: '1'}).select('moderation infoBoxEnable infoBoxContent'); -}; - -/** - * Gets the info box settings and sends it back - * @return {Promise} content the content of the info Box - */ -SettingSchema.statics.getInfoBoxSetting = function () { - return this.findOne({id: '1'}).select('infoBoxEnable infoBoxContent'); -}; +SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => Setting.findOne(selector)); /** * This will update the settings object with whatever you pass in * @param {object} setting a hash of whatever settings you want to update * @return {Promise} settings Promise that resolves to the entire (updated) settings object. */ -SettingSchema.statics.updateSettings = function (setting) { - // There should only ever be one record unless something has gone wrong. - // In the future we may have multiple records for custom settings for objects/users. - return this.findOneAndUpdate({id: '1'}, {$set: setting}, {new: true}); +SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { + $set: settings +}, { + upsert: true, + new: true, + setDefaultsOnInsert: true +}).then((settings) => { + + // Invalidate the settings cache. + return cache + .set('settings', settings, EXPIRY_TIME) + .then(() => settings); +}); + +/** + * Filters the document to ensure that the resulting document is indeed ready + * for non authenticated users. + * @param {Object} settings the source settings object + * @return {Object} the filtered settings object + */ +SettingService.public = (settings) => _.pick(settings, ['moderation', 'infoBoxEnable', 'infoBoxContent']); + +/** + * This is run once when the app starts to ensure settings are populated. + * @return {Promise} null initialize the global settings object + */ +SettingService.init = (defaults) => { + + // Inject the defaults on top of the passed in defaults to ensure that the new + // settings conform to the required selector. + defaults = Object.assign({}, defaults, selector); + + // Actually update the settings collection. + return SettingService.update(defaults); }; - -const Setting = mongoose.model('Setting', SettingSchema); - -module.exports = Setting; diff --git a/models/user.js b/models/user.js index 86908ab42..336486a54 100644 --- a/models/user.js +++ b/models/user.js @@ -9,7 +9,6 @@ const SALT_ROUNDS = 10; // USER_ROLES is the array of roles that is permissible as a user role. const USER_ROLES = [ - '', 'admin', 'moderator' ]; @@ -106,7 +105,7 @@ UserSchema.index({ * output. */ UserSchema.options.toJSON = {}; -UserSchema.options.toJSON.hide = 'password profiles roles disabled'; +UserSchema.options.toJSON.hide = '_id password profiles roles disabled'; UserSchema.options.toJSON.transform = (doc, ret, options) => { if (options.hide) { options.hide.split(' ').forEach((prop) => { diff --git a/package.json b/package.json index be6c2b153..c54bd6723 100644 --- a/package.json +++ b/package.json @@ -89,6 +89,7 @@ "eslint-config-standard": "^6.2.1", "eslint-plugin-flowtype": "^2.25.0", "eslint-plugin-import": "^2.2.0", + "eslint-plugin-mocha": "^4.7.0", "eslint-plugin-promise": "^3.3.1", "eslint-plugin-react": "^6.6.0", "eslint-plugin-standard": "^2.0.1", diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 3bdf4afc6..b82c1faf3 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -1,5 +1,6 @@ const express = require('express'); const Comment = require('../../../models/comment'); +const Asset = require('../../../models/asset'); const User = require('../../../models/user'); const Action = require('../../../models/action'); const wordlist = require('../../../services/wordlist'); @@ -9,24 +10,45 @@ const _ = require('lodash'); const router = express.Router(); router.get('/', authorization.needed('admin'), (req, res, next) => { + + const { + status = null, + action_type = null, + asset_id = null + } = req.query; + + /** + * This adds the asset_id requirement to the query if the asset_id is defined. + */ + const assetIDWrap = (query) => { + if (asset_id) { + query = query.where('asset_id', asset_id); + } + + return query; + }; + let query; - if (req.query.status) { - query = Comment.findByStatus(req.query.status); - } else if (req.query.action_type) { - query = Comment.findByActionType(req.query.action_type); + if (status) { + query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status)); + } else if (action_type) { + query = Comment + .findIdsByActionType(action_type) + .then((ids) => assetIDWrap(Comment.find({ + id: { + $in: ids + }, + }))); } else { - query = Comment.all(); + query = assetIDWrap(Comment.all()); } query.then((comments) => { return Promise.all([ comments, User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), - Action.getActionSummaries(_.uniq([ - ...comments.map((comment) => comment.id), - ...comments.map((comment) => comment.author_id) - ])) + Action.getActionSummariesFromComments(asset_id, comments, req.user ? req.user.id : false) ]); }) .then(([comments, users, actions])=> @@ -48,21 +70,38 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { parent_id } = req.body; - Comment - .create({ - body, - asset_id, - parent_id, - status: req.wordlist.matched ? 'rejected' : '', - author_id: req.user.id - }) - .then((comment) => { + // Decide the status based on whether or not the current asset/settings + // has pre-mod enabled or not. If the comment was rejected based on the + // wordlist, then reject it, otherwise if the moderation setting is + // premod, set it to `premod`. + let status; - res.status(201).send(comment); - }) - .catch((err) => { - next(err); - }); + if (req.wordlist.matched) { + status = Promise.resolve('rejected'); + } else { + status = Asset + .rectifySettings(Asset.findById(asset_id)) + + // Return `premod` if pre-moderation is enabled and an empty "new" status + // in the event that it is not in pre-moderation mode. + .then(({moderation}) => moderation === 'pre' ? 'premod' : ''); + } + + status.then((status) => Comment.publicCreate({ + body, + asset_id, + parent_id, + status, + author_id: req.user.id + })) + .then((comment) => { + + // The comment was created! Send back the created comment. + res.status(201).send(comment); + }) + .catch((err) => { + next(err); + }); }); router.get('/:comment_id', authorization.needed('admin'), (req, res, next) => { @@ -99,7 +138,7 @@ router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next } = req.body; Comment - .changeStatus(req.params.comment_id, status) + .pushStatus(req.params.comment_id, status, req.user.id) .then(() => { res.status(204).end(); }) diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index b38d1b763..9c85fbe94 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -3,6 +3,7 @@ const Comment = require('../../../models/comment'); const User = require('../../../models/user'); const Action = require('../../../models/action'); const Setting = require('../../../models/setting'); +const Asset = require('../../../models/asset'); const _ = require('lodash'); const router = express.Router(); @@ -16,27 +17,52 @@ const router = express.Router(); // 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) => { - Setting.getPublicSettings().then(({moderation}) => - Comment.moderationQueue(moderation)) - .then((comments) => { - return Promise.all([ - comments, - User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), - Action.getActionSummaries(_.uniq([ - ...comments.map((comment) => comment.id), - ...comments.map((comment) => comment.author_id) - ])) - ]); - }) - .then(([comments, users, actions])=> - res.status(200).json({ - comments, - users, - actions - })) - .catch(error => { - next(error); - }); + + const { + asset_id + } = req.query; + + let settings = Setting.retrieve(); + + if (asset_id) { + + // In the event that we have an asset_id, we should fetch the asset settings + // in order to actually determine if there is additional comments to parse. + settings = Promise.all([ + settings, + Asset.findById(asset_id).select('settings') + ]).then(([{moderation}, asset]) => { + if (asset.settings && asset.settings.moderation) { + return {moderation: asset.settings.moderation}; + } + + return {moderation}; + }); + } + + settings + .then(({moderation}) => { + return Comment.moderationQueue(moderation); + }).then((comments) => { + return Promise.all([ + comments, + User.findByIdArray(_.uniq(comments.map((comment) => comment.author_id))), + Action.getActionSummaries(_.uniq([ + ...comments.map((comment) => comment.id), + ...comments.map((comment) => comment.author_id) + ])) + ]); + }) + .then(([comments, users, actions]) => { + res.json({ + comments, + users, + actions + }); + }) + .catch(error => { + next(error); + }); }); module.exports = router; diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 910524fb2..76d16c7ec 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -4,19 +4,21 @@ const Setting = require('../../../models/setting'); const router = express.Router(); router.get('/', (req, res, next) => { - Setting - .getSettings() - .then(settings => { - res.json(settings); - }) - .catch(next); + Setting.retrieve().then((settings) => { + res.json(settings); + }) + .catch((err) => { + next(err); + }); }); router.put('/', (req, res, next) => { - Setting - .updateSettings(req.body) - .then(() => res.status(204).end()) - .catch(next); + Setting.update(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 4e8373999..26947be09 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -26,14 +26,14 @@ router.get('/', (req, res, next) => { return asset; }), - // Get the public settings. - Setting.getPublicSettings() + // Get the moderation setting from the settings. + Setting.retrieve() ]) .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) { + if (asset && asset.settings) { settings = Object.assign({}, settings, asset.settings); } @@ -70,17 +70,7 @@ router.get('/', (req, res, next) => { 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); + let actions = Action.getActionSummariesFromComments(asset.id, comments, req.user ? req.user.id : false); return Promise.all([ @@ -108,7 +98,7 @@ router.get('/', (req, res, next) => { comments, users, actions, - settings + settings: Setting.public(settings) }); }) .catch(error => { diff --git a/services/wordlist.js b/services/wordlist.js index e6f2ad668..59a842817 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -20,7 +20,7 @@ const wordlist = { */ wordlist.init = () => { return Setting - .getSettings() + .retrieve() .then((settings) => { // Insert the settings wordlist. diff --git a/tests/.eslintrc.json b/tests/.eslintrc.json index 363ef5a28..b188e92cf 100644 --- a/tests/.eslintrc.json +++ b/tests/.eslintrc.json @@ -4,8 +4,12 @@ "node": true, "mocha": true }, + "plugins": [ + "mocha" + ], "extends": "../.eslintrc.json", "rules": { - "no-undef": [0] + "no-undef": [0], + "mocha/no-exclusive-tests": "warn" } } diff --git a/tests/models/action.js b/tests/models/action.js index c354b8f83..5f05dea71 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -1,35 +1,34 @@ const Action = require('../../models/action'); const expect = require('chai').expect; -describe('Action: models', () => { - let mockActions; +describe('models.Action', () => { + let mockActions = []; - beforeEach(() => { - return Action.create([{ - action_type: 'flag', - item_id: '123', - item_type: 'comment', - user_id: 'flagginguserid' - }, { - action_type: 'flag', - item_id: '456', - item_type: 'comment' - }, { - action_type: 'flag', - item_id: '123', - item_type: 'comment' - }, { - action_type: 'like', - item_id: '123', - item_type: 'comment' - }]).then((actions) => { - mockActions = actions; - }); - }); + beforeEach(() => Action.create([{ + action_type: 'flag', + item_id: '123', + item_type: 'comment', + user_id: 'flagginguserid' + }, { + action_type: 'flag', + item_id: '456', + item_type: 'comment' + }, { + action_type: 'flag', + item_id: '123', + item_type: 'comment' + }, { + action_type: 'like', + item_id: '123', + item_type: 'comment' + }]).then((actions) => { + mockActions = actions; + })); describe('#findById()', () => { it('should find an action by id', () => { return Action.findById(mockActions[0].id).then((result) => { + expect(result).to.not.be.null; expect(result).to.have.property('action_type', 'flag'); }); }); diff --git a/tests/models/asset.js b/tests/models/asset.js index 69bf99f9d..c9eff819e 100644 --- a/tests/models/asset.js +++ b/tests/models/asset.js @@ -6,7 +6,7 @@ const expect = chai.expect; // Use the chai should. chai.should(); -describe('Asset: model', () => { +describe('models.Asset', () => { beforeEach(() => { const defaults = {url:'http://test.com'}; diff --git a/tests/models/comment.js b/tests/models/comment.js index 07834a186..a9c2f4787 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -7,35 +7,57 @@ const settings = {id: '1', moderation: 'pre'}; const expect = require('chai').expect; -describe('Comment: models', () => { +describe('models.Comment', () => { const comments = [{ body: 'comment 10', asset_id: '123', - status: '', + status: [], parent_id: '', author_id: '123', id: '1' }, { body: 'comment 20', asset_id: '123', - status: 'accepted', + status: [{ + type: 'accepted' + }], parent_id: '', author_id: '123', id: '2' }, { body: 'comment 30', asset_id: '456', - status: '', + status: [], parent_id: '', author_id: '456', id: '3' }, { body: 'comment 40', asset_id: '123', - status: 'rejected', + status: [{ + type: 'rejected' + }], parent_id: '', author_id: '456', id: '4' + }, { + body: 'comment 50', + asset_id: '1234', + status: [{ + type: 'premod' + }], + parent_id: '', + author_id: '456', + id: '5' + }, { + body: 'comment 60', + asset_id: '1234', + status: [{ + type: 'premod' + }], + parent_id: '', + author_id: '456', + id: '6' }]; const users = [{ @@ -60,25 +82,69 @@ describe('Comment: models', () => { user_id: '456' }]; - beforeEach(() => { - return Promise.all([ - Setting.create(settings), - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions) - ]); + beforeEach(() => Promise.all([ + Setting.init(settings), + Comment.create(comments), + User.createLocalUsers(users), + Action.create(actions) + ])); + + describe('#publicCreate()', () => { + + it('creates a new comment', () => { + return Comment.publicCreate({ + body: 'This is a comment!', + status: 'accepted' + }).then((c) => { + expect(c).to.not.be.null; + expect(c.id).to.not.be.null; + expect(c.id).to.be.uuid; + expect(c.status).to.have.length(1); + expect(c.status[0]).to.have.property('type', 'accepted'); + }); + }); + + it('creates many new comments', () => { + return Comment.publicCreate([{ + body: 'This is a comment!', + status: 'accepted' + }, { + body: 'This is another comment!' + }, { + body: 'This is a rejected comment!', + status: 'rejected' + }]).then(([c1, c2, c3]) => { + expect(c1).to.not.be.null; + expect(c1.id).to.be.uuid; + expect(c1.status).to.have.length(1); + expect(c1.status[0]).to.have.property('type', 'accepted'); + + expect(c2).to.not.be.null; + expect(c2.id).to.be.uuid; + expect(c2.status).to.have.length(0); + + expect(c3).to.not.be.null; + expect(c3.id).to.be.uuid; + expect(c3.status).to.have.length(1); + expect(c3.status[0]).to.have.property('type', 'rejected'); + }); + }); + }); describe('#findById()', () => { + it('should find a comment by id', () => { return Comment.findById('1').then((result) => { expect(result).to.not.be.null; expect(result).to.have.property('body', 'comment 10'); }); }); + }); describe('#findByAssetId()', () => { + it('should find an array of all comments by asset id', () => { return Comment.findByAssetId('123').then((result) => { expect(result).to.have.length(3); @@ -91,6 +157,7 @@ describe('Comment: models', () => { expect(result[2]).to.have.property('body', 'comment 40'); }); }); + it('should find an array of accepted comments by asset id', () => { return Comment.findAcceptedByAssetId('123').then((result) => { expect(result).to.have.length(1); @@ -101,6 +168,7 @@ describe('Comment: models', () => { expect(result[0]).to.have.property('body', 'comment 20'); }); }); + it('should find an array of new and accepted comments by asset id', () => { return Comment.findAcceptedAndNewByAssetId('123').then((result) => { expect(result).to.have.length(2); @@ -112,13 +180,16 @@ describe('Comment: models', () => { }); }); }); + describe('#moderationQueue()', () => { + it('should find an array of new comments to moderate when pre-moderation', () => { return Comment.moderationQueue('pre').then((result) => { expect(result).to.not.be.null; expect(result).to.have.lengthOf(2); }); }); + it('should find an array of new comments to moderate when post-moderation', () => { return Comment.moderationQueue('post').then((result) => { expect(result).to.not.be.null; @@ -126,21 +197,56 @@ describe('Comment: models', () => { expect(result[0]).to.have.property('body', 'comment 30'); }); }); - // it('should fail when the moderation is not pre or post', () => { - // return Comment.moderationQueue('any').catch(function(error) { - // expect(error).to.not.be.null; - // }); - // }); + }); describe('#removeAction', () => { + it('should remove an action', () => { - return Comment.removeAction('3', '123', 'flag').then(() => { - return Action.findByItemIdArray(['123']); - }) - .then((actions) => { - expect(actions.length).to.equal(0); - }); + return Comment.removeAction('3', '123', 'flag') + .then(() => { + return Action.findByItemIdArray(['123']); + }) + .then((actions) => { + expect(actions.length).to.equal(0); + }); }); }); + + describe('#changeStatus', () => { + + it('should change the status of a comment from no status', () => { + let comment_id = comments[0].id; + + return Comment.findById(comment_id) + .then((c) => { + expect(c).to.have.property('status'); + expect(c.status).to.have.length(0); + + return Comment.pushStatus(comment_id, 'rejected', '123'); + }) + .then(() => Comment.findById(comment_id)) + .then((c) => { + expect(c).to.have.property('status'); + expect(c.status).to.have.length(1); + expect(c.status[0]).to.have.property('type', 'rejected'); + expect(c.status[0]).to.have.property('assigned_by', '123'); + }); + }); + + it('should change the status of a comment from accepted', () => { + return Comment.pushStatus(comments[1].id, 'rejected', '123') + .then(() => Comment.findById(comments[1].id)) + .then((c) => { + expect(c).to.have.property('status'); + expect(c.status).to.have.length(2); + expect(c.status[0]).to.have.property('type', 'accepted'); + expect(c.status[0]).to.have.property('assigned_by', null); + + expect(c.status[1]).to.have.property('type', 'rejected'); + expect(c.status[1]).to.have.property('assigned_by', '123'); + }); + }); + + }); }); diff --git a/tests/models/setting.js b/tests/models/setting.js index 186a65245..3e77297fc 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -1,34 +1,29 @@ const Setting = require('../../models/setting'); const expect = require('chai').expect; -describe('Setting: model', () => { +describe('models.Setting', () => { - beforeEach(() => { - const defaults = { - id: 1 - }; - return Setting.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); - }); + beforeEach(() => Setting.init({moderation: 'pre'})); - describe('#getSettings()', () => { + describe('#retrieve()', () => { it('should have a moderation field defined', () => { - return Setting.getSettings().then(settings => { + return Setting.retrieve().then(settings => { expect(settings).to.have.property('moderation').and.to.equal('pre'); }); }); it('should have two infoBox fields defined', () => { - return Setting.getSettings().then(settings => { + return Setting.retrieve().then(settings => { expect(settings).to.have.property('infoBoxEnable').and.to.equal(false); expect(settings).to.have.property('infoBoxContent').and.to.equal(''); }); }); }); - describe('#updateSettings()', () => { + describe('#update()', () => { it('should update the settings with a passed object', () => { const mockSettings = {moderation: 'post', infoBoxEnable: true, infoBoxContent: 'yeah'}; - return Setting.updateSettings(mockSettings).then(updatedSettings => { + return Setting.update(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); @@ -37,13 +32,10 @@ describe('Setting: model', () => { }); }); - describe('#getPublicSettings', () => { + describe('#get', () => { it('should return the moderation settings', () => { - return Setting.getPublicSettings().then(({moderation, infoBoxEnable, infoBoxContent, wordlist}) => { + return Setting.retrieve().then(({moderation}) => { expect(moderation).not.to.be.null; - expect(infoBoxEnable).not.to.be.null; - expect(infoBoxContent).not.to.be.null; - expect(wordlist).to.be.undefined; }); }); }); diff --git a/tests/models/user.js b/tests/models/user.js index c8af92054..5e1677016 100644 --- a/tests/models/user.js +++ b/tests/models/user.js @@ -1,7 +1,7 @@ const User = require('../../models/user'); const expect = require('chai').expect; -describe('User: models', () => { +describe('models.User', () => { let mockUsers; beforeEach(() => { return User.createLocalUsers([{ diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index bc8b67bc1..bf7a06ff6 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -10,6 +10,7 @@ chai.use(require('chai-http')); const wordlist = require('../../../../services/wordlist'); const Comment = require('../../../../models/comment'); +const Asset = require('../../../../models/asset'); const Action = require('../../../../models/action'); const User = require('../../../../models/user'); @@ -17,63 +18,71 @@ const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; describe('/api/v1/comments', () => { - 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: 'def-rejected', - body: 'comment 20', - asset_id: 'asset', - author_id: '456', - status: 'rejected' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456', - author_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', - item_type: 'comment' - }, { - action_type: 'like', - item_id: 'hij', - item_type: 'comment' - }]; - - beforeEach(() => { - return Promise.all([ - Comment.create(comments), - User.createLocalUsers(users), - Action.create(actions), - wordlist.insert([ - 'bad words' - ]), - Setting.create(settings) - ]); - }); describe('#get', () => { + const comments = [{ + body: 'comment 10', + asset_id: 'asset', + author_id: '123' + }, { + body: 'comment 20', + asset_id: 'asset', + author_id: '456' + }, { + body: 'comment 20', + asset_id: 'asset', + author_id: '456', + status: [{ + type: 'rejected' + }] + }, { + body: 'comment 30', + asset_id: '456', + status: [{ + type: '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', + item_type: 'comment' + }, { + action_type: 'like', + item_id: 'hij', + item_type: 'comment' + }]; + + beforeEach(() => { + return Promise.all([ + Comment.create(comments).then((newComments) => { + newComments.forEach((comment, i) => { + comments[i].id = comment.id; + }); + + actions[0].item_id = comments[0].id; + actions[1].item_id = comments[1].id; + + return Action.create(actions); + }), + User.createLocalUsers(users), + wordlist.insert([ + 'bad words' + ]), + Setting.init(settings) + ]); + }); + it('should return all the comments', () => { return chai.request(app) .get('/api/v1/comments') @@ -91,7 +100,9 @@ describe('/api/v1/comments', () => { .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); - expect(res.body.comments[0]).to.have.property('id', 'def-rejected'); + expect(res.body).to.have.property('comments'); + expect(res.body.comments).to.have.length(1); + expect(res.body.comments[0]).to.have.property('id', comments[2].id); }); }); @@ -102,7 +113,7 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(200); expect(res.body.comments).to.have.length(1); - expect(res.body.comments[0]).to.have.property('id', 'hij'); + expect(res.body.comments[0]).to.have.property('id', comments[3].id); }); }); @@ -124,8 +135,7 @@ describe('/api/v1/comments', () => { expect(res).to.have.status(200); expect(res.body.comments).to.have.length(1); - expect(res.body.comments[0]).to.have.property('id', 'abc'); - + expect(res.body.comments[0]).to.have.property('id', comments[0].id); }); }); }); @@ -151,7 +161,31 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); - expect(res.body).to.have.property('status', 'rejected'); + expect(res.body).to.have.property('status').and.to.have.length(1); + expect(res.body.status[0]).to.have.property('type', 'rejected'); + }); + }); + + it('should create a comment with a premod status if it\'s asset is has pre-moderation enabled', () => { + return Asset + .findOrCreateByUrl('https://coralproject.net/article1') + .then((asset) => { + return Asset + .overrideSettings(asset.id, {moderation: 'pre'}) + .then(() => asset); + }) + .then((asset) => { + return chai.request(app) + .post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''}); + }) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + expect(res.body).to.have.property('asset_id'); + expect(res.body).to.have.property('status').and.to.have.length(1); + expect(res.body.status[0]).to.have.property('type', 'premod'); }); }); }); @@ -267,18 +301,22 @@ describe('/api/v1/comments/:comment_id/actions', () => { body: 'comment 10', asset_id: 'asset', author_id: '123', - status: '' + status: [] }, { id: 'def', body: 'comment 20', asset_id: 'asset', author_id: '456', - status: 'rejected' + status: [{ + type: 'rejected' + }] }, { id: 'hij', body: 'comment 30', asset_id: '456', - status: 'accepted' + status: [{ + type: 'accepted' + }] }]; const users = [{ @@ -316,10 +354,8 @@ describe('/api/v1/comments/:comment_id/actions', () => { .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 1e99ae61e..d64423013 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -15,56 +15,54 @@ const User = require('../../../../models/user'); const Setting = require('../../../../models/setting'); const settings = {id: '1', moderation: 'pre'}; -beforeEach(() => { - return Setting.create(settings); -}); +describe('/api/v1/queue', () => { + const comments = [{ + id: 'abc', + body: 'comment 10', + asset_id: 'asset', + author_id: '123', + status: [{ + type: 'rejected' + }] + }, { + id: 'def', + body: 'comment 20', + asset_id: 'asset', + author_id: '456', + status: [{ + type: 'premod' + }] + }, { + id: 'hij', + body: 'comment 30', + asset_id: '456', + status: [{ + type: 'accepted' + }] + }]; -describe('Get moderation queues rejected, pending, flags', () => { + const users = [{ + displayName: 'Ana', + email: 'ana@gmail.com', + password: '123' + }, { + displayName: 'Maria', + email: 'maria@gmail.com', + password: '123' + }]; - describe('/api/v1/queue', () => { - let comments; + const actions = [{ + action_type: 'flag', + item_id: 'abc', + item_type: 'comment' + }, { + action_type: 'like', + item_id: 'hij', + item_type: 'comment' + }]; - const users = [{ - id: '456', - displayName: 'Ana', - email: 'ana@gmail.com', - password: '123' - }, { - id: '123', - displayName: 'Maria', - email: 'maria@gmail.com', - password: '123' - }]; - - let actions; - - beforeEach(() => { - - comments = [{ - id: 'abc', - body: 'comment 10', - asset_id: 'asset', - status: 'rejected' - }, { - id: 'def', - body: 'comment 20', - asset_id: 'asset' - }, { - id: 'hij', - body: 'comment 30', - asset_id: '456', - status: 'accepted' - }]; - - actions = [{ - action_type: 'flag', - item_type: 'comment' - }, { - action_type: 'like', - item_type: 'comment' - }]; - - return User.createLocalUsers(users) + beforeEach(() => { + return User.createLocalUsers(users) .then((u) => { comments[0].author_id = u[0].id; comments[1].author_id = u[1].id; @@ -76,22 +74,24 @@ describe('Get moderation queues rejected, pending, flags', () => { actions[0].item_id = c[0].id; actions[1].item_id = c[1].id; - return Action.create(actions); + return Promise.all([ + Action.create(actions), + Setting.init(settings) + ]); }); - }); + }); - it('should return all the pending comments, users and actions', 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.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'); - done(); - }); - }); + it('should return all the pending comments, users and actions', 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.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'); + done(); + }); }); }); diff --git a/tests/routes/api/settings/index.js b/tests/routes/api/settings/index.js index d1a7ba81b..318af81f4 100644 --- a/tests/routes/api/settings/index.js +++ b/tests/routes/api/settings/index.js @@ -12,7 +12,7 @@ const defaults = {id: '1', moderation: 'pre'}; describe('/api/v1/settings', () => { - beforeEach(() => Setting.create(defaults)); + beforeEach(() => Setting.init(defaults)); describe('#get', () => { @@ -40,7 +40,7 @@ describe('/api/v1/settings', () => { .then((res) => { expect(res).to.have.status(204); - return Setting.getSettings(); + return Setting.retrieve(); }) .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 6484fdc12..a128938ba 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -20,7 +20,37 @@ describe('/api/v1/stream', () => { moderation: 'post' }; - let comments; + const comments = [{ + id: 'abc', + body: 'comment 10', + author_id: '', + parent_id: '', + status: [{ + type: 'accepted' + }] + }, { + id: 'def', + body: 'comment 20', + author_id: '', + parent_id: '', + status: [] + }, { + id: 'uio', + body: 'comment 30', + asset_id: 'asset', + author_id: '456', + parent_id: '', + status: [{ + type: 'accepted' + }] + }, { + id: 'hij', + body: 'comment 40', + asset_id: '456', + status: [{ + type: 'rejected' + }] + }]; const users = [{ displayName: 'Ana', @@ -41,33 +71,6 @@ describe('/api/v1/stream', () => { }]; beforeEach(() => { - - comments = [{ - id: 'abc', - body: 'comment 10', - author_id: '', - parent_id: '', - status: 'accepted' - }, { - id: 'def', - body: 'comment 20', - author_id: '', - parent_id: '', - status: '' - }, { - id: 'uio', - body: 'comment 30', - asset_id: 'asset', - author_id: '456', - parent_id: '', - status: 'accepted' - }, { - id: 'hij', - body: 'comment 40', - asset_id: '456', - status: 'rejected' - }]; - return Promise.all([ User.createLocalUsers(users), Asset.findOrCreateByUrl('http://test.com'), @@ -94,10 +97,11 @@ describe('/api/v1/stream', () => { return Promise.all([ Comment.create(comments), Action.create(actions), - Setting.init().then(() => Setting.updateSettings(settings)) + Setting.init(settings) ]); }); }); + it('should return a stream with comments, users and actions for an existing asset', () => { return chai.request(app) .get('/api/v1/stream') From fed41e1fba80250545f8192ca1c7e7fb7ec7eb5d Mon Sep 17 00:00:00 2001 From: Dan Zajdband Date: Thu, 8 Dec 2016 16:59:29 -0500 Subject: [PATCH 08/16] Linted code --- client/coral-embed-stream/src/CommentStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js index 69fc5f73f..7f3b1b30f 100644 --- a/client/coral-embed-stream/src/CommentStream.js +++ b/client/coral-embed-stream/src/CommentStream.js @@ -114,7 +114,7 @@ class CommentStream extends Component { author={user} /> - :

{this.props.config.closedMessage}

+ :

{closedMessage}

} {!loggedIn && } { From 413db123c4e99606336fdb80593b02948a06a2c1 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 8 Dec 2016 12:18:19 -1000 Subject: [PATCH 09/16] Type on comment. --- bin/cli-settings | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/cli-settings b/bin/cli-settings index c639a5ca5..e7ba30151 100755 --- a/bin/cli-settings +++ b/bin/cli-settings @@ -15,7 +15,7 @@ const mongoose = require('../mongoose'); const Setting = require('../models/setting'); const util = require('../util'); -// Regeister the shutdown criteria. +// Register the shutdown criteria. util.onshutdown([ () => mongoose.disconnect() ]); From a86b5f84ac2fb6196ec111cbda1c997a19ca99d9 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 8 Dec 2016 12:18:53 -1000 Subject: [PATCH 10/16] Adds a role option when creating the user. --- bin/cli-users | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/bin/cli-users b/bin/cli-users index 2e0dc84e6..e77259301 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -34,6 +34,7 @@ function createUser(options) { email: options.email, password: options.password, displayName: options.name, + role: options.role }); } @@ -62,6 +63,11 @@ function createUser(options) { name: 'displayName', description: 'Display Name', required: true + }, + { + name: 'role', + description: 'User Role', + required: false } ], (err, result) => { if (err) { @@ -76,15 +82,24 @@ function createUser(options) { }); }) .then((result) => { - return User.createLocalUser(result.email.trim(), result.password.trim(), result.displayName.trim()); - }) - .then((user) => { - console.log(`Created user ${user.id}.`); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(); + return User.createLocalUser(result.email.trim(), result.password.trim(), result.displayName.trim()) + .then((user) => { + console.log(`Created user ${user.id}.`); + User + .addRoleToUser(user.id, result.role.trim()) + .then(() => { + console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); + util.shutdown(); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); + }); + }) + .catch((err) => { + console.error(err); + util.shutdown(); + }); }); } @@ -330,6 +345,7 @@ program .option('--email [email]', 'Email to use') .option('--password [password]', 'Password to use') .option('--name [name]', 'Name to use') + .option('--role [role]', 'Role to add') .option('-f, --flag_mode', 'Source from flags instead of prompting') .description('create a new user') .action(createUser); From 4572d8e68ef9570e6f1ba324ec655ec889c1e6f7 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 8 Dec 2016 12:20:54 -1000 Subject: [PATCH 11/16] filter by premod status --- .../src/containers/ModerationQueue/ModerationQueue.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js index e5670d169..3774ed5f9 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js @@ -81,9 +81,11 @@ class ModerationQueue extends React.Component { singleView={singleView} commentIds={ comments.get('ids') - .filter(id => !comments.get('byId') + .filter(id => + comments + .get('byId') .get(id) - .get('status')) + .get('status') === 'premod') } comments={comments.get('byId')} users={users.get('byId')} From bc88cd1e9d231b85a871140626e91f29fe298fd8 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Dec 2016 17:23:31 -0500 Subject: [PATCH 12/16] Adjusted promise --- bin/cli-users | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bin/cli-users b/bin/cli-users index e77259301..012eba9d4 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -85,15 +85,12 @@ function createUser(options) { return User.createLocalUser(result.email.trim(), result.password.trim(), result.displayName.trim()) .then((user) => { console.log(`Created user ${user.id}.`); - User + + return User .addRoleToUser(user.id, result.role.trim()) .then(() => { console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); }); }) .catch((err) => { From c8d0fae0970fa0bb9fe0639c9077ce125b272856 Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Thu, 8 Dec 2016 12:50:12 -1000 Subject: [PATCH 13/16] Fix formatting --- README.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 6589cddb9..51c0d1ab9 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ You can view product ideas and our longer term roadmap here: https://trello.com/ ### Local Dependencies Node + Mongo ### Getting Started @@ -26,17 +27,17 @@ inside the environment in order to run, those variables are listed here: - `TALK_SESSION_SECRET` (*required*) - a random string which will be used to secure cookies. -`TALK_FACEBOOK_APP_ID` (*required*) - the Facebook app id for your Facebook +- `TALK_FACEBOOK_APP_ID` (*required*) - the Facebook app id for your Facebook Login enabled app. -`TALK_FACEBOOK_APP_SECRET` (*required*) - the Facebook app secret for your +- `TALK_FACEBOOK_APP_SECRET` (*required*) - the Facebook app secret for your Facebook Login enabled app. -`TALK_ROOT_URL` (*required*) - root url of the installed application externally +- `TALK_ROOT_URL` (*required*) - root url of the installed application externally available in the format: `://` without the path. -`TALK_SMTP_PROVIDER` (*required*) - SMTP provider name. -`TALK_SMTP_USERNAME` (*required*) - username of the SMTP provider you are using. -`TALK_SMTP_PASSWORD` (*required*) - password for the SMTP provider you are using. -`TALK_SMTP_HOST` (*required*) - SMTP host url with format `smtp.domain.com`. -`TALK_SMTP_PORT` (*required*) - SMTP port. +- `TALK_SMTP_PROVIDER` (*required*) - SMTP provider name. +- `TALK_SMTP_USERNAME` (*required*) - username of the SMTP provider you are using. +- `TALK_SMTP_PASSWORD` (*required*) - password for the SMTP provider you are using. +- `TALK_SMTP_HOST` (*required*) - SMTP host url with format `smtp.domain.com`. +- `TALK_SMTP_PORT` (*required*) - SMTP port. ### Running with Docker Make sure you have Docker running first and then run `docker-compose up -d` From 500dafbafa5d3e513641b3092274ed58ef2760cb Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 8 Dec 2016 12:51:45 -1000 Subject: [PATCH 14/16] riley being anal --- middleware/payload-filter.js | 2 +- tests/models/setting.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/payload-filter.js b/middleware/payload-filter.js index 8ce3d39d5..0cafd91fe 100644 --- a/middleware/payload-filter.js +++ b/middleware/payload-filter.js @@ -38,7 +38,7 @@ module.exports = (req, res, next) => { } else if (typeof o === 'object') { - // Itterate over the props, find only properties owned by the object. + // Iterate over the props, find only properties owned by the object. for (let prop in o) { // If and only if the object owns the property do we actually pull the diff --git a/tests/models/setting.js b/tests/models/setting.js index fbd5bf77f..8fd23d6a0 100644 --- a/tests/models/setting.js +++ b/tests/models/setting.js @@ -41,7 +41,7 @@ describe('models.Setting', () => { }); describe('#merge', () => { - it('should merge a settings object and it\'s overrides', () => { + it('should merge a settings object and its overrides', () => { return Setting .retrieve() .then((settings) => { From 7ce44ccd8c40d7098d2283a2d8bed836a232239b Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Thu, 8 Dec 2016 12:53:39 -1000 Subject: [PATCH 15/16] More formatting --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 51c0d1ab9..0b77e95b7 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,11 @@ Make sure you have Docker running first and then run `docker-compose up -d` `npm run lint` ### Helpful URLs -Bare comment stream: http://localhost:5000/client/coral-embed-stream/ -Comment stream embedded on sample article: http://localhost:5000/client/coral-embed-stream/samplearticle.html -Moderator view: http://localhost:5000/admin/ +Comment stream: http://localhost:3000/ + +Comment stream embedded on sample article: http://localhost:3000/assets/samplearticle.html + +Moderator view: http://localhost:3000/admin ### Docs `swagger.yaml` From dd5c114dc2ea82bd521ff6327db1ee129328ca32 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 8 Dec 2016 18:36:20 -1000 Subject: [PATCH 16/16] don't use well-known email settings --- docker-compose.yml | 1 + services/mailer.js | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index d7a180277..16df60095 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,6 +7,7 @@ services: restart: always ports: - "5000:5000" + - "2525:2525" environment: - "TALK_PORT=5000" - "TALK_MONGO_URL=mongodb://mongo" diff --git a/services/mailer.js b/services/mailer.js index b7d621954..fe07f063b 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -3,7 +3,7 @@ const nodemailer = require('nodemailer'); const smtpRequiredProps = [ 'TALK_SMTP_USERNAME', 'TALK_SMTP_PASSWORD', - 'TALK_SMTP_PROVIDER' + 'TALK_SMTP_HOST' ]; smtpRequiredProps.forEach(prop => { @@ -13,9 +13,7 @@ smtpRequiredProps.forEach(prop => { }); const options = { - // list of providers here: - // https://github.com/nodemailer/nodemailer-wellknown#supported-services - service: process.env.TALK_SMTP_PROVIDER, + host: process.env.TALK_SMTP_HOST, auth: { user: process.env.TALK_SMTP_USERNAME, pass: process.env.TALK_SMTP_PASSWORD @@ -24,10 +22,8 @@ const options = { if (process.env.TALK_SMTP_PORT) { options.port = process.env.TALK_SMTP_PORT; -} - -if (process.env.TALK_SMTP_HOST) { - options.host = process.env.TALK_SMTP_HOST; +} else { + options.port = 25; } const defaultTransporter = nodemailer.createTransport(options);