From 3285a5ec29a7e6482f246ba9b5d53bfedcc0e015 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 12:16:13 -0700 Subject: [PATCH 1/9] add status to Comment. /comments/pending and /comments/rejected ONLY return the respective comments --- client/coral-admin/src/actions/comments.js | 45 +++++++++- .../ModerationQueue/ModerationContainer.js | 18 +++- models/comment.js | 80 ++++-------------- routes/api/queue/index.js | 83 ++++++++++--------- 4 files changed, 119 insertions(+), 107 deletions(-) diff --git a/client/coral-admin/src/actions/comments.js b/client/coral-admin/src/actions/comments.js index e8276a9da..d897af3a5 100644 --- a/client/coral-admin/src/actions/comments.js +++ b/client/coral-admin/src/actions/comments.js @@ -6,10 +6,11 @@ import * as actionTypes from '../constants/actions'; export const fetchModerationQueueComments = () => { return dispatch => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); + return Promise.all([ coralApi('/queue/comments/pending'), - coralApi('/comments?status=rejected'), - coralApi('/comments?action_type=flag') + coralApi('/queue/comments/rejected'), + coralApi('/queue/comments/flagged') ]) .then(([pending, rejected, flagged]) => { @@ -27,11 +28,49 @@ export const fetchModerationQueueComments = () => { dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users}); dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments}); dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions}); - }); }; }; +export const fetchPendingQueue = () => { + return dispatch => { + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); + + return coralApi('/queue/comments/pending') + .then(pending => { + dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: pending.users}); + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: pending.comments}); + dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: pending.actions}); + }); + }; +}; + +export const fetchRejectedQueue = () => { + return dispatch => { + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); + + return coralApi('/queue/comments/rejected') + .then(rejected => { + dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: rejected.users}); + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: rejected.comments}); + dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: rejected.actions}); + }); + }; +}; + +export const fetchFlaggedQueue = () => { + return dispatch => { + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); + + return coralApi('/queue/comments/flagged') + .then(flagged => { + dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: flagged.users}); + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: flagged.comments}); + dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: flagged.actions}); + }); + }; +}; + // Create a new comment export const createComment = (name, body) => { return (dispatch) => { diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js index 8a684184a..d2c888e9e 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js @@ -6,7 +6,10 @@ import { updateStatus, showBanUserDialog, hideBanUserDialog, - fetchModerationQueueComments + fetchPendingQueue, + fetchRejectedQueue, + fetchFlaggedQueue, + fetchModerationQueueComments, } from 'actions/comments'; import {userStatusUpdate} from 'actions/users'; import {fetchSettings} from 'actions/settings'; @@ -54,6 +57,16 @@ class ModerationContainer extends React.Component { onTabClick(activeTab) { this.setState({activeTab}); + + if (activeTab === 'pending') { + this.props.fetchPendingQueue(); + } else if (activeTab === 'rejected') { + this.props.fetchRejectedQueue(); + } else if (activeTab === 'flagged') { + this.props.fetchFlaggedQueue(); + } else { + this.props.fetchModerationQueueComments(); + } } onClose() { @@ -90,6 +103,9 @@ const mapDispatchToProps = dispatch => { return { fetchSettings: () => dispatch(fetchSettings()), fetchModerationQueueComments: () => dispatch(fetchModerationQueueComments()), + fetchPendingQueue: () => dispatch(fetchPendingQueue()), + fetchRejectedQueue: () => dispatch(fetchRejectedQueue()), + fetchFlaggedQueue: () => dispatch(fetchFlaggedQueue()), showBanUserDialog: (userId, userName, commentId) => dispatch(showBanUserDialog(userId, userName, commentId)), hideBanUserDialog: () => dispatch(hideBanUserDialog(false)), banUser: (userId, commentId) => dispatch(userStatusUpdate('banned', userId, commentId)).then(() => { diff --git a/models/comment.js b/models/comment.js index 3f9ae1a65..9bd9163c4 100644 --- a/models/comment.js +++ b/models/comment.js @@ -47,6 +47,7 @@ const CommentSchema = new Schema({ asset_id: String, author_id: String, status_history: [StatusSchema], + status: {type: String, default: null}, parent_id: String }, { timestamps: { @@ -84,24 +85,6 @@ CommentSchema.method('filterForUser', function(user = false) { 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_history. - */ -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; -}); - /** * Creates a new Comment that came from a public source. * @param {Mixed} comment either a single comment or an array of comments. @@ -118,7 +101,7 @@ CommentSchema.statics.publicCreate = (comment) => { body, asset_id, parent_id, - status = false, + status = null, author_id } = comment; @@ -130,6 +113,7 @@ CommentSchema.statics.publicCreate = (comment) => { type: status, created_at: new Date() }] : [], + status, author_id }); @@ -160,7 +144,7 @@ CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({ */ CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ asset_id, - 'status_history.type': 'accepted' + status: 'accepted' }); /** @@ -171,14 +155,8 @@ CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({ asset_id, $or: [ - { - 'status_history.type': 'accepted' - }, - { - status_history: { - $size: 0 - } - } + {status: 'accepted'}, + {status: null} ] }); @@ -205,28 +183,20 @@ CommentSchema.statics.findIdsByActionType = (action_type) => Action .then((actions) => actions.map(a => a.item_id)); /** - * Find comments by their status_history. - * @param {String} status the status of the comment to search for - * @return {Promise} + * Find comments by current status + * @param {String} status status of the comment to search for + * @return {Promise} resovles to comment array */ -CommentSchema.statics.findByStatus = (status = false) => { - let q = {}; - - if (status) { - q['status_history.type'] = status; - } else { - q.status_history = {$size: 0}; - } - - return Comment.find(q); +CommentSchema.statics.findByStatus = (status = null) => { + return Comment.find({status}); }; /** * Find comments that need to be moderated (aka moderation queue). - * @param {String} moderationValue pre or post moderation setting. If it is undefined then look at the settings. + * @param {String} asset_id * @return {Promise} */ -CommentSchema.statics.moderationQueue = (moderation, asset_id = false) => { +CommentSchema.statics.moderationQueue = (status, asset_id = null) => { /** * This adds the asset_id requirement to the query if the asset_id is defined. @@ -239,25 +209,8 @@ CommentSchema.statics.moderationQueue = (moderation, asset_id = false) => { return query; }; - // 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 - } - }))); - } + // Pre-moderation: New comments are shown in the moderator queues immediately. + let comments = assetIDWrap(Comment.findByStatus(status)); return comments; }; @@ -277,7 +230,8 @@ CommentSchema.statics.pushStatus = (id, status, assigned_by = null) => Comment.u created_at: new Date(), assigned_by } - } + }, + $set: {status} }); /** diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index 9c85fbe94..c629233f1 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -4,10 +4,22 @@ const User = require('../../../models/user'); const Action = require('../../../models/action'); const Setting = require('../../../models/setting'); const Asset = require('../../../models/asset'); +const authorization = require('../../../middleware/authorization'); const _ = require('lodash'); const router = express.Router(); +function gatherActionsAndUsers (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) + ])) + ]); +} + //============================================================================== // Get Routes //============================================================================== @@ -16,49 +28,40 @@ const router = express.Router(); // depending on the settings. The :moderation overwrites this settings. // Pre-moderation: New comments are shown in the moderator queues immediately. // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. -router.get('/comments/pending', (req, res, next) => { +router.get('/comments/pending', authorization.needed('admin'), (req, res, next) => { - const { - asset_id - } = req.query; + 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) - ])) - ]); - }) + Comment.moderationQueue('premod', asset_id) + .then(gatherActionsAndUsers) .then(([comments, users, actions]) => { - res.json({ - comments, - users, - actions - }); + res.json({comments, users, actions}); + }) + .catch(error => { + next(error); + }); +}); + +router.get('/comments/rejected', /*authorization.needed('admin'),*/ (req, res, next) => { + const {asset_id} = req.query; + + Comment.moderationQueue('rejected', asset_id) + .then(gatherActionsAndUsers) + .then(([comments, users, actions]) => { + res.json({comments, users, actions}); + }) + .catch(error => { + next(error); + }); +}); + +router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) => { + const {asset_id} = req.query; + + Comment.moderationQueue('rejected', asset_id) + .then(gatherActionsAndUsers) + .then(([comments, users, actions]) => { + res.json({comments, users, actions}) }) .catch(error => { next(error); From f1272338cdd37c609c77e6ee53f7d20e7f32e430 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 13:30:53 -0700 Subject: [PATCH 2/9] re-use dispatch function --- client/coral-admin/src/actions/comments.js | 32 +++++++--------------- routes/api/queue/index.js | 8 +++--- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/client/coral-admin/src/actions/comments.js b/client/coral-admin/src/actions/comments.js index d897af3a5..2dae1ef00 100644 --- a/client/coral-admin/src/actions/comments.js +++ b/client/coral-admin/src/actions/comments.js @@ -2,6 +2,12 @@ import coralApi from '../../../coral-framework/helpers/response'; import * as commentTypes from '../constants/comments'; import * as actionTypes from '../constants/actions'; +function addUsersCommentsActions (dispatch, {comments, users, actions}) { + dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users}); + dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments}); + dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions}); +} + // Get comments to fill each of the three lists on the mod queue export const fetchModerationQueueComments = () => { return dispatch => { @@ -22,13 +28,7 @@ export const fetchModerationQueueComments = () => { actions: [...pending.actions, ...rejected.actions, ...flagged.actions] }; }) - .then(({comments, users, actions}) => { - - /* Post comments and users to redux store. Actions will be posted when they are needed. */ - dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users}); - dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments}); - dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions}); - }); + .then(addUsersCommentsActions.bind(this, dispatch)); }; }; @@ -37,11 +37,7 @@ export const fetchPendingQueue = () => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); return coralApi('/queue/comments/pending') - .then(pending => { - dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: pending.users}); - dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: pending.comments}); - dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: pending.actions}); - }); + .then(addUsersCommentsActions.bind(this, dispatch)); }; }; @@ -50,11 +46,7 @@ export const fetchRejectedQueue = () => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); return coralApi('/queue/comments/rejected') - .then(rejected => { - dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: rejected.users}); - dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: rejected.comments}); - dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: rejected.actions}); - }); + .then(addUsersCommentsActions.bind(this, dispatch)); }; }; @@ -63,11 +55,7 @@ export const fetchFlaggedQueue = () => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); return coralApi('/queue/comments/flagged') - .then(flagged => { - dispatch({type: commentTypes.USERS_MODERATION_QUEUE_FETCH_SUCCESS, users: flagged.users}); - dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_SUCCESS, comments: flagged.comments}); - dispatch({type: actionTypes.ACTIONS_MODERATION_QUEUE_FETCH_SUCCESS, actions: flagged.actions}); - }); + .then(addUsersCommentsActions.bind(this, dispatch)); }; }; diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index c629233f1..3edcbe1b4 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -58,10 +58,10 @@ router.get('/comments/rejected', /*authorization.needed('admin'),*/ (req, res, n router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) => { const {asset_id} = req.query; - Comment.moderationQueue('rejected', asset_id) - .then(gatherActionsAndUsers) - .then(([comments, users, actions]) => { - res.json({comments, users, actions}) + Comment.findIdsByActionType('flagged') + .then(ids => { + console.log(ids); + res.json(ids); }) .catch(error => { next(error); From b66efb7f2a9621c65ce8773b73e3cb95b2758a5a Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 13:44:10 -0700 Subject: [PATCH 3/9] return flagged comments --- routes/api/comments/index.js | 2 +- routes/api/queue/index.js | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 21426d217..f37b507f9 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -56,7 +56,7 @@ router.get('/', (req, res, next) => { .then((ids) => assetIDWrap(Comment.find({ id: { $in: ids - }, + } }))); } else { query = assetIDWrap(Comment.all()); diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index 3edcbe1b4..1d788136a 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -58,10 +58,20 @@ router.get('/comments/rejected', /*authorization.needed('admin'),*/ (req, res, n router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) => { const {asset_id} = req.query; - Comment.findIdsByActionType('flagged') - .then(ids => { - console.log(ids); - res.json(ids); + const assetIDWrap = (query) => { + if (asset_id) { + query = query.where('asset_id', asset_id); + } + + return query; + }; + + Comment.findIdsByActionType('flag') + .then(ids => assetIDWrap(Comment.find({ + id: { $in: ids } + }))) + .then(([comments, users, actions]) => { + res.json({comments, users, actions}); }) .catch(error => { next(error); From 44b41b031efe2bd1f753baeeda611977eff3b028 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 14:03:58 -0700 Subject: [PATCH 4/9] lint --- routes/api/queue/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index 1d788136a..ff9e31d76 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -2,8 +2,6 @@ const express = require('express'); 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 authorization = require('../../../middleware/authorization'); const _ = require('lodash'); @@ -42,7 +40,7 @@ router.get('/comments/pending', authorization.needed('admin'), (req, res, next) }); }); -router.get('/comments/rejected', /*authorization.needed('admin'),*/ (req, res, next) => { +router.get('/comments/rejected', authorization.needed('admin'), (req, res, next) => { const {asset_id} = req.query; Comment.moderationQueue('rejected', asset_id) @@ -68,7 +66,7 @@ router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) Comment.findIdsByActionType('flag') .then(ids => assetIDWrap(Comment.find({ - id: { $in: ids } + id: {$in: ids} }))) .then(([comments, users, actions]) => { res.json({comments, users, actions}); From 6e89734e127b16976aea04455da36d745f712008 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 12 Jan 2017 14:08:52 -0700 Subject: [PATCH 5/9] Moved errors to errors.js, std error msgs/types --- app.js | 40 +++++---- errors.js | 126 +++++++++++++++++++++++------ middleware/authorization.js | 12 +-- models/setting.js | 2 +- routes/api/auth/index.js | 5 +- routes/api/comments/index.js | 9 ++- routes/api/stream/index.js | 6 +- tests/routes/api/comments/index.js | 4 +- 8 files changed, 140 insertions(+), 64 deletions(-) diff --git a/app.js b/app.js index b128eb283..9cdad4ea3 100644 --- a/app.js +++ b/app.js @@ -9,6 +9,7 @@ const enabled = require('debug').enabled; const RedisStore = require('connect-redis')(session); const redis = require('./services/redis'); const csrf = require('csurf'); +const errors = require('./errors'); const app = express(); @@ -109,40 +110,47 @@ app.use('/', require('./routes')); // ERROR HANDLING //============================================================================== -const ErrNotFound = new Error('Not Found'); -ErrNotFound.status = 404; - // Catch 404 and forward to error handler. app.use((req, res, next) => { - next(ErrNotFound); + next(errors.ErrNotFound); }); // General error handler. Respond with the message and error if we have it while // returning a status code that makes sense. app.use('/api', (err, req, res, next) => { - if (err !== ErrNotFound) { + if (err !== errors.ErrNotFound) { if (app.get('env') !== 'test' || enabled('talk:errors')) { console.error(err); } } - res.status(err.status || 500); - res.json({ - message: err.message, - error: app.get('env') === 'development' ? err : {} - }); + if (err instanceof errors.APIError) { + res.status(err.status).json({ + message: err.message, + error: err + }); + } else { + res.status(500).json({}); + } }); app.use('/', (err, req, res, next) => { - if (err !== ErrNotFound) { + if (err !== errors.ErrNotFound) { console.error(err); } - res.status(err.status || 500); - res.render('error', { - message: err.message, - error: app.get('env') === 'development' ? err : {} - }); + if (err instanceof errors.APIError) { + res.status(err.status); + res.render('error', { + message: err.message, + error: app.get('env') === 'development' ? err : {} + }); + } else { + res.render('error', { + message: err.message, + error: app.get('env') === 'development' ? err : {} + }); + } }); module.exports = app; diff --git a/errors.js b/errors.js index 0651b4fff..b5b217b61 100644 --- a/errors.js +++ b/errors.js @@ -1,45 +1,123 @@ +/** + * ExtendableError provides a base Error class to source off of that does not + * break the inheritence chain. + */ +class ExtendableError { + constructor(message = null) { + this.message = message; + this.stack = (new Error()).stack; + } +} + +/** + * APIError is the base error that all application issued errors originate, they + * are composed of data used by the front end and backend to handle errors + * consistently. + */ +class APIError extends ExtendableError { + constructor(message, {status = 500, translation_key = null}, metadata = {}) { + super(message); + + this.status = status; + this.translation_key = translation_key; + this.metadata = metadata; + } + + toJSON() { + return { + message: this.message, + status: this.status, + translation_key: this.translation_key, + metadata: this.metadata + }; + } +} + // ErrPasswordTooShort is returned when the password length is too short. -const ErrPasswordTooShort = new Error('password must be at least 8 characters'); -ErrPasswordTooShort.translation_key = 'PASSWORD_LENGTH'; -ErrPasswordTooShort.status = 400; +const ErrPasswordTooShort = new APIError('password must be at least 8 characters', { + status: 400, + translation_key: 'PASSWORD_LENGTH' +}); -const ErrMissingEmail = new Error('email is required'); -ErrMissingEmail.translation_key = 'EMAIL_REQUIRED'; -ErrMissingEmail.status = 400; +const ErrMissingEmail = new APIError('email is required', { + translation_key: 'EMAIL_REQUIRED', + status: 400 +}); -const ErrMissingPassword = new Error('password is required'); -ErrMissingPassword.translation_key = 'PASSWORD_REQUIRED'; -ErrMissingPassword.status = 400; +const ErrMissingPassword = new APIError('password is required', { + translation_key: 'PASSWORD_REQUIRED', + status: 400 +}); -const ErrEmailTaken = new Error('Email address already in use'); -ErrEmailTaken.translation_key = 'EMAIL_IN_USE'; -ErrEmailTaken.status = 400; +const ErrEmailTaken = new APIError('Email address already in use', { + translation_key: 'EMAIL_IN_USE', + status: 400 +}); -const ErrSpecialChars = new Error('No special characters are allowed in a display name'); -ErrSpecialChars.translation_key = 'NO_SPECIAL_CHARACTERS'; -ErrSpecialChars.status = 400; +const ErrSpecialChars = new APIError('No special characters are allowed in a display name', { + translation_key: 'NO_SPECIAL_CHARACTERS', + status: 400 +}); -const ErrMissingDisplay = new Error('A display name is required to create a user'); -ErrMissingDisplay.translation_key = 'DISPLAY_NAME_REQUIRED'; -ErrMissingDisplay.status = 400; +const ErrMissingDisplay = new APIError('A display name is required to create a user', { + translation_key: 'DISPLAY_NAME_REQUIRED', + status: 400 +}); // ErrMissingToken is returned in the event that the password reset is requested // without a token. -const ErrMissingToken = new Error('token is required'); -ErrMissingToken.status = 400; +const ErrMissingToken = new APIError('token is required', { + status: 400 +}); + +// ErrAssetCommentingClosed is returned when a comment or action is attempted on +// a stream where commenting has been closed. +class ErrAssetCommentingClosed extends APIError { + constructor(closedMessage = null) { + super('asset commenting is closed', { + status: 400 + }, { + + // Include the closedMessage in the metadata piece of the error. + closedMessage + }); + } +} // ErrContainsProfanity is returned in the event that the middleware detects // profanity/wordlisted words in the payload. -const ErrContainsProfanity = new Error('Suspected profanity. If you think this in error, please let us know!'); -ErrContainsProfanity.translation_key = 'PROFANITY_ERROR'; -ErrContainsProfanity.status = 400; +const ErrContainsProfanity = new APIError('Suspected profanity. If you think this in error, please let us know!', { + translation_key: 'PROFANITY_ERROR', + status: 400 +}); + +const ErrNotFound = new APIError('not found', { + status: 404 +}); + +const ErrInvalidAssetURL = new APIError('asset_url is invalid', { + status: 400 +}); + +// ErrNotAuthorized is an error that is returned in the event an operation is +// deemed not authorized. +const ErrNotAuthorized = new APIError('not authorized', { + status: 401 +}); module.exports = { + ExtendableError, + APIError, ErrPasswordTooShort, ErrMissingEmail, ErrMissingPassword, + ErrMissingToken, ErrEmailTaken, ErrSpecialChars, ErrMissingDisplay, - ErrContainsProfanity + ErrContainsProfanity, + ErrAssetCommentingClosed, + ErrNotFound, + ErrInvalidAssetURL, + ErrNotAuthorized }; diff --git a/middleware/authorization.js b/middleware/authorization.js index 65f77b753..efe4b1034 100644 --- a/middleware/authorization.js +++ b/middleware/authorization.js @@ -7,17 +7,7 @@ const authorization = module.exports = { }; const debug = require('debug')('talk:middleware:authorization'); - -/** - * ErrNotAuthorized is an error that is returned in the event an operation is - * deemed not authorized. - * @type {Error} - */ -const ErrNotAuthorized = new Error('not authorized'); -ErrNotAuthorized.status = 401; - -// Add the ErrNotAuthorized error to the authorization object to be exported. -authorization.ErrNotAuthorized = ErrNotAuthorized; +const ErrNotAuthorized = require('../errors').ErrNotAuthorized; /** * has returns true if the user has all the roles specified, otherwise it will diff --git a/models/setting.js b/models/setting.js index ad9fa6599..ed41acfa6 100644 --- a/models/setting.js +++ b/models/setting.js @@ -42,7 +42,7 @@ const SettingSchema = new Schema({ }, closedMessage: { type: String, - default: '' + default: 'Expired' }, wordlist: WordlistSchema, charCount: { diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index 6a84c6049..050aa1135 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -1,6 +1,7 @@ const express = require('express'); const passport = require('../../../services/passport'); const authorization = require('../../../middleware/authorization'); +const errors = require('../../../errors'); const router = express.Router(); @@ -42,7 +43,7 @@ const HandleAuthCallback = (req, res, next) => (err, user) => { } if (!user) { - return next(authorization.ErrNotAuthorized); + return next(errors.ErrNotAuthorized); } // Perform the login of the user! @@ -65,7 +66,7 @@ const HandleAuthPopupCallback = (req, res, next) => (err, user) => { } if (!user) { - return res.render('auth-callback', {err: JSON.stringify(authorization.ErrNotAuthorized), data: null}); + return res.render('auth-callback', {err: JSON.stringify(errors.ErrNotAuthorized), data: null}); } // Perform the login of the user! diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 21426d217..226f772be 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -1,4 +1,5 @@ const express = require('express'); +const errors = require('../../../errors'); const Comment = require('../../../models/comment'); const Asset = require('../../../models/asset'); const User = require('../../../models/user'); @@ -20,13 +21,13 @@ router.get('/', (req, res, next) => { // everything on this route requires admin privileges besides listing comments for owner of said comments if (!authorization.has(req.user, 'admin') && !user_id) { - next(authorization.ErrNotAuthorized); + next(errors.ErrNotAuthorized); return; } // if the user is not an admin, only return comment list for the owner of the comments if (req.user.id !== user_id && !authorization.has(req.user, 'admin')) { - next(authorization.ErrNotAuthorized); + next(errors.ErrNotAuthorized); return; } @@ -102,14 +103,14 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { status = Asset .rectifySettings(Asset.findById(asset_id).then((asset) => { if (!asset) { - return Promise.reject(new Error('asset referenced is not found')); + return Promise.reject(errors.ErrNotFound); } // 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 Promise.reject(new errors.ErrAssetCommentingClosed(asset.closedMessage)); } return asset; diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 68d0f0ad9..540e36e52 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -1,6 +1,7 @@ const express = require('express'); const _ = require('lodash'); const scraper = require('../../../services/scraper'); +const errors = require('../../../errors'); const url = require('url'); const Comment = require('../../../models/comment'); @@ -9,9 +10,6 @@ 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) => { @@ -21,7 +19,7 @@ router.get('/', (req, res, next) => { // Verify that the asset_url is parsable. let parsed_asset_url = url.parse(asset_url); if (!parsed_asset_url.protocol) { - return next(ErrInvalidAssetURL); + return next(errors.ErrInvalidAssetURL); } // Get the asset_id for this url (or create it if it doesn't exist) diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 360daa5dc..246da24a7 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -101,7 +101,7 @@ describe('/api/v1/comments', () => { expect(res).to.be.empty; }) .catch((err) => { - expect(err).to.have.property('status', 401); + expect(err).to.have.status(401); }); }); @@ -287,7 +287,7 @@ describe('/api/v1/comments', () => { .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!'); + expect(err.response.body.error.metadata.closedMessage).to.be.equal('tests said expired!'); }); }); From 0eefd3364b95e5f9ea80d6a5332dae5a056c8f5a Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 15:25:36 -0700 Subject: [PATCH 6/9] update tests --- client/coral-admin/src/actions/comments.js | 4 ++++ .../ModerationQueue/ModerationQueue.js | 2 +- routes/api/comments/index.js | 2 +- routes/api/queue/index.js | 1 + tests/models/comment.js | 17 ++++++++--------- tests/routes/api/comments/index.js | 2 ++ tests/routes/api/queue/index.js | 4 ++++ tests/routes/api/stream/index.js | 4 ++++ 8 files changed, 25 insertions(+), 11 deletions(-) diff --git a/client/coral-admin/src/actions/comments.js b/client/coral-admin/src/actions/comments.js index 2dae1ef00..f37c79b4f 100644 --- a/client/coral-admin/src/actions/comments.js +++ b/client/coral-admin/src/actions/comments.js @@ -55,6 +55,10 @@ export const fetchFlaggedQueue = () => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); return coralApi('/queue/comments/flagged') + .then(comments => { + comments.forEach(comment => comment.flagged = true); + return comments; + }) .then(addUsersCommentsActions.bind(this, dispatch)); }; }; diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js index 55a2e705e..d46a9b35d 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js @@ -56,7 +56,7 @@ export default ({onTabClick, ...props}) => (
{ if (charCountEnable && body.length > charCount) { return 'rejected'; } - return moderation === 'pre' ? 'premod' : ''; + return moderation === 'pre' ? 'premod' : null; }); } diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index ff9e31d76..34902557c 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -68,6 +68,7 @@ router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) .then(ids => assetIDWrap(Comment.find({ id: {$in: ids} }))) + .then(gatherActionsAndUsers) .then(([comments, users, actions]) => { res.json({comments, users, actions}); }) diff --git a/tests/models/comment.js b/tests/models/comment.js index 91f51ccb8..20590dc3d 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -21,6 +21,7 @@ describe('models.Comment', () => { status_history: [{ type: 'accepted' }], + status: 'accepted', parent_id: '', author_id: '123', id: '2' @@ -37,6 +38,7 @@ describe('models.Comment', () => { status_history: [{ type: 'rejected' }], + status: 'rejected', parent_id: '', author_id: '456', id: '4' @@ -46,6 +48,7 @@ describe('models.Comment', () => { status_history: [{ type: 'premod' }], + status: 'premod', parent_id: '', author_id: '456', id: '5' @@ -55,6 +58,7 @@ describe('models.Comment', () => { status_history: [{ type: 'premod' }], + status: 'premod', parent_id: '', author_id: '456', id: '6' @@ -184,20 +188,12 @@ describe('models.Comment', () => { describe('#moderationQueue()', () => { it('should find an array of new comments to moderate when pre-moderation', () => { - return Comment.moderationQueue('pre').then((result) => { + return Comment.moderationQueue('premod').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; - expect(result).to.have.lengthOf(1); - expect(result[0]).to.have.property('body', 'comment 30'); - }); - }); - }); describe('#removeAction', () => { @@ -227,6 +223,7 @@ describe('models.Comment', () => { .then(() => Comment.findById(comment_id)) .then((c) => { expect(c).to.have.property('status'); + expect(c.status).to.equal('rejected'); 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,6 +235,8 @@ describe('models.Comment', () => { .then(() => Comment.findById(comments[1].id)) .then((c) => { expect(c).to.have.property('status_history'); + expect(c).to.have.property('status'); + expect(c.status).to.equal('rejected'); 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); diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 360daa5dc..65b52e246 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -34,12 +34,14 @@ describe('/api/v1/comments', () => { body: 'comment 20', asset_id: 'asset', author_id: '456', + status: 'rejected', status_history: [{ type: 'rejected' }] }, { body: 'comment 30', asset_id: '456', + status: 'accepted', status_history: [{ type: 'accepted' }] diff --git a/tests/routes/api/queue/index.js b/tests/routes/api/queue/index.js index 378899967..6a7507618 100644 --- a/tests/routes/api/queue/index.js +++ b/tests/routes/api/queue/index.js @@ -21,6 +21,7 @@ describe('/api/v1/queue', () => { body: 'comment 10', asset_id: 'asset', author_id: '123', + status: 'rejected', status_history: [{ type: 'rejected' }] @@ -29,6 +30,7 @@ describe('/api/v1/queue', () => { body: 'comment 20', asset_id: 'asset', author_id: '456', + status: 'premod', status_history: [{ type: 'premod' }] @@ -36,6 +38,7 @@ describe('/api/v1/queue', () => { id: 'hij', body: 'comment 30', asset_id: '456', + status: 'accepted', status_history: [{ type: 'accepted' }] @@ -89,6 +92,7 @@ describe('/api/v1/queue', () => { .set(passport.inject({roles: ['admin']})) .then((res) => { expect(res).to.have.status(200); + expect(res.body.comments).to.have.length(1); 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'); diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index 5d192e976..d9b59d6fe 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -29,6 +29,7 @@ describe('/api/v1/stream', () => { body: 'comment 10', author_id: '', parent_id: '', + status: 'accepted', status_history: [{ type: 'accepted' }] @@ -37,6 +38,7 @@ describe('/api/v1/stream', () => { body: 'comment 20', author_id: '', parent_id: '', + status: null, status_history: [] }, { id: 'uio', @@ -44,6 +46,7 @@ describe('/api/v1/stream', () => { asset_id: 'asset', author_id: '456', parent_id: '', + status: 'accepted', status_history: [{ type: 'accepted' }] @@ -51,6 +54,7 @@ describe('/api/v1/stream', () => { id: 'hij', body: 'comment 40', asset_id: '456', + status: 'rejected', status_history: [{ type: 'rejected' }] From f56e30c1a1ab2c42728ce10f5e9b1cea1891112e Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 13 Jan 2017 10:08:35 -0500 Subject: [PATCH 7/9] Updating docs for .env functionality --- README.md | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 239483628..6b5105a54 100644 --- a/README.md +++ b/README.md @@ -17,30 +17,32 @@ To launch a Talk server of your own from your browser without any need to muck a ### Configuration -The Talk application requires specific configuration options to be available -inside the environment in order to run, those variables are listed here: +The Talk application looks for the following configuration values either as +environment variables or via an `.env` file: - `TALK_MONGO_URL` (*required*) - the database connection string for the MongoDB database. - `TALK_REDIS_URL` (*required*) - the database connection string for the Redis database. - `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_EMAIL` (*required*) - the address to send emails from using the + +- `TALK_FACEBOOK_APP_ID` (*required for login via fb*) - the Facebook app id for your Facebook +Login enabled app. +- `TALK_FACEBOOK_APP_SECRET` (*required for login via fb*) - the Facebook app secret for your +Facebook Login enabled app. + +- `TALK_SMTP_EMAIL` (*required for email*) - the address to send emails from using the SMTP provider. -- `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_USERNAME` (*required for email*) - username of the SMTP provider you are using. +- `TALK_SMTP_PASSWORD` (*required for email*) - password for the SMTP provider you are using. +- `TALK_SMTP_HOST` (*required for email*) - SMTP host url with format `smtp.domain.com`. +- `TALK_SMTP_PORT` (*required for email*) - SMTP port. +We recommend using environment variables for configuration in staging and +production environments. -### Install from Source - -If you want to run Talk in development mode from source (without docker) you can read the [INSTALL file](INSTALL.md). +For more information about configuration via `.env`, please [visit our wiki](https://github.com/coralproject/talk/wiki/Configuration-via-.env). ### License From 7c95781072efb77fc9b5a86dd1f70b76f0984899 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Fri, 13 Jan 2017 10:21:12 -0500 Subject: [PATCH 8/9] Update README.md --- README.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6b5105a54..e005c314f 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,7 @@ To launch a Talk server of your own from your browser without any need to muck a ### Configuration -The Talk application looks for the following configuration values either as -environment variables or via an `.env` file: +The Talk application looks for the following configuration values either as environment variables: - `TALK_MONGO_URL` (*required*) - the database connection string for the MongoDB database. - `TALK_REDIS_URL` (*required*) - the database connection string for the Redis database. @@ -39,10 +38,7 @@ Facebook Login enabled app. - `TALK_SMTP_HOST` (*required for email*) - SMTP host url with format `smtp.domain.com`. - `TALK_SMTP_PORT` (*required for email*) - SMTP port. -We recommend using environment variables for configuration in staging and -production environments. - -For more information about configuration via `.env`, please [visit our wiki](https://github.com/coralproject/talk/wiki/Configuration-via-.env). +During development mode, you can use the command `npm run dev-start` which will load environment variables from a file named .env in the present working directory. ### License From 39a4d031c71c989940419f8299bd1706c8cc0874 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 13 Jan 2017 08:24:12 -0700 Subject: [PATCH 9/9] Adjusted for new wiki page --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e005c314f..79c523925 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,8 @@ Facebook Login enabled app. - `TALK_SMTP_HOST` (*required for email*) - SMTP host url with format `smtp.domain.com`. - `TALK_SMTP_PORT` (*required for email*) - SMTP port. -During development mode, you can use the command `npm run dev-start` which will load environment variables from a file named .env in the present working directory. +Refer to the wiki page on [Configuration Loading](https://github.com/coralproject/talk/wiki/Configuration-Loading) for +alternative methods of loading configuration during development. ### License