From ce3e29813bd8566250dc95ed532bd6e4370c0880 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 8 Sep 2017 15:41:43 -0600 Subject: [PATCH] implemented status switch when mismatched --- graph/mutators/comment.js | 30 +++-- services/assets.js | 25 +++-- services/comments.js | 103 +++++++++++------ test/server/services/comments.js | 187 +++++++++++++++++-------------- 4 files changed, 208 insertions(+), 137 deletions(-) diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 9142687f6..a75e319f4 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -194,16 +194,32 @@ const createComment = async (context, {tags = [], body, asset_id, parent_id = nu * @param {String} [asset_id] id of asset comment is posted on * @return {Object} resolves to the wordlist results */ -const filterNewComment = (context, {body, asset_id}) => { +const filterNewComment = async (context, {body, asset_id}) => { + + // Load the settings. + const [ + settings, + asset, + ] = await Promise.all([ + context.loaders.Settings.load(), + context.loaders.Assets.getByID.load(asset_id), + ]); // Create a new instance of the Wordlist. const wl = new Wordlist(); + // Load the wordlist. + wl.upsert(settings.wordlist); + // Load the wordlist and filter the comment content. - return Promise.all([ - wl.load().then(() => wl.scan('body', body)), - asset_id && AssetsService.rectifySettings(AssetsService.findById(asset_id)) - ]); + return [ + + // Scan the word. + wl.scan('body', body), + + // Return the asset's settings. + AssetsService.rectifySettings(asset, settings) + ]; }; /** @@ -247,7 +263,7 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli // Return `premod` if pre-moderation is enabled and an empty "new" status // in the event that it is not in pre-moderation mode. - let {moderation, charCountEnable, charCount} = await AssetsService.rectifySettings(asset); + let {moderation, charCountEnable, charCount} = await AssetsService.rectifySettings(asset, settings); // Reject if the comment is too long if (charCountEnable && body.length > charCount) { @@ -365,7 +381,7 @@ const edit = async (context, {id, asset_id, edit: {body}}) => { const status = await resolveNewCommentStatus(context, {asset_id, body}, wordlist, settings); // Execute the edit. - const comment = await CommentsService.edit(id, context.user.id, {body, status}); + const comment = await CommentsService.edit({id, author_id: context.user.id, body, status}); // Publish the edited comment via the subscription. context.pubsub.publish('commentEdited', comment); diff --git a/services/assets.js b/services/assets.js index 34c8ff465..cddfcfb7c 100644 --- a/services/assets.js +++ b/services/assets.js @@ -3,6 +3,7 @@ const AssetModel = require('../models/asset'); const SettingsService = require('./settings'); const domainlist = require('./domainlist'); const errors = require('../errors'); +const merge = require('lodash/merge'); module.exports = class AssetsService { @@ -28,19 +29,21 @@ module.exports = class AssetsService { * @param {Promise} assetQuery an asset query that returns a single asset. * @return {Promise} */ - static rectifySettings(assetQuery) { - return Promise.all([ - SettingsService.retrieve(), - assetQuery - ]).then(([settings, asset]) => { + static async rectifySettings(assetQuery, settings = null) { + const [ + globalSettings, + asset, + ] = await Promise.all([ + settings || SettingsService.retrieve(), + assetQuery, + ]); - // If the asset exists and has settings then return the merged object. - if (asset && asset.settings) { - settings.merge(asset.settings); - } + // If the asset exists and has settings then return the merged object. + if (asset && asset.settings) { + settings = merge({}, globalSettings, asset.settings); + } - return settings; - }); + return settings; } /** diff --git a/services/comments.js b/services/comments.js index db0714233..1c39fdafb 100644 --- a/services/comments.js +++ b/services/comments.js @@ -5,6 +5,7 @@ const ActionsService = require('./actions'); const SettingsService = require('./settings'); const sc = require('snake-case'); +const cloneDeep = require('lodash/cloneDeep'); const errors = require('../errors'); const events = require('./events'); const { @@ -52,13 +53,34 @@ module.exports = class CommentsService { } /** - * Edit a Comment - * @param {String} id comment.id you want to edit (or its ID) - * @param {String} author_id user.id of the user trying to edit the comment (will err if not comment author) + * lastUnmoderatedStatus will retrieve the last status before this one. + * + * @param {Object} comment the comment to get the last status of + */ + static lastUnmoderatedStatus(comment) { + const UNMODERATED_STATUSES = [ + 'NONE', + 'PREMOD', + ]; + + for (let i = comment.status_history.length - 1; i >= 0; i--) { + const {type} = comment.status_history[i]; + + if (UNMODERATED_STATUSES.includes(type)) { + return type; + } + } + } + + /** + * Edit a Comment. + * + * @param {String} id comment.id you want to edit (or its ID) + * @param {String} author_id user.id of the user trying to edit the comment (will err if not comment author) * @param {String} body the new Comment body * @param {String} status the new Comment status */ - static async edit(id, author_id, {body, status, ignoreEditWindow = false}) { + static async edit({id, author_id, body, status}) { const query = { id, author_id, @@ -69,14 +91,11 @@ module.exports = class CommentsService { // Establish the edit window (if it exists) and add the condition to the // original query. - let lastEditableCommentCreatedAt; - if (!ignoreEditWindow) { - const {editCommentWindowLength: editWindowMs} = await SettingsService.retrieve(); - lastEditableCommentCreatedAt = new Date((new Date()).getTime() - editWindowMs); - query.created_at = { - $gt: lastEditableCommentCreatedAt, - }; - } + const {editCommentWindowLength: editWindowMs} = await SettingsService.retrieve(); + const lastEditableCommentCreatedAt = new Date(Date.now() - editWindowMs); + query.created_at = { + $gt: lastEditableCommentCreatedAt, + }; const originalComment = await CommentModel.findOneAndUpdate(query, { $set: { @@ -117,7 +136,7 @@ module.exports = class CommentsService { } // Check to see if the edit window expired. - if (!ignoreEditWindow && comment.created_at <= lastEditableCommentCreatedAt) { + if (comment.created_at <= lastEditableCommentCreatedAt) { debug('rejecting comment edit because outside edit time window'); throw errors.ErrEditWindowHasEnded; } @@ -126,7 +145,7 @@ module.exports = class CommentsService { } // Mutate the comment like Mongo would have. - const editedComment = originalComment; + const editedComment = cloneDeep(originalComment); editedComment.status = status; editedComment.body = body; editedComment.body_history.push({ @@ -138,6 +157,43 @@ module.exports = class CommentsService { created_at: new Date(), }); + // We should adjust the comment's status such that if it was approved + // previously, we should mark the comment as 'NONE' or 'PREMOD', which ever + // was most recent if the new comment is destined to be `NONE` or `PREMOD`. + if (originalComment.status === 'ACCEPTED' && ['NONE', 'PREMOD'].includes(status)) { + + const lastUnmoderatedStatus = CommentsService.lastUnmoderatedStatus(originalComment); + + // If the last moderated status was found and the current comment doesn't + // match this already. + if (lastUnmoderatedStatus && status !== lastUnmoderatedStatus) { + + // Update the comment model (if at this point, the status is still + // accepted) with the previously unmoderated status + await CommentModel.update({ + id, + status, + }, { + $set: { + status: lastUnmoderatedStatus, + }, + $push: { + status_history: { + type: lastUnmoderatedStatus, + created_at: new Date(), + } + }, + }); + + // Update the returned comment. + editedComment.status = lastUnmoderatedStatus; + editedComment.status_history.push({ + type: lastUnmoderatedStatus, + created_at: new Date(), + }); + } + } + await events.emitAsync(COMMENTS_EDIT, originalComment, editedComment); return editedComment; @@ -215,23 +271,6 @@ module.exports = class CommentsService { return CommentModel.find({status}); } - /** - * Find comments that need to be moderated (aka moderation queue). - * @param {String} asset_id - * @return {Promise} - */ - static moderationQueue(status = 'NONE', asset_id = null) { - - // Fetch the comments with statuses. - let comments = CommentModel.find({status}); - - if (asset_id) { - comments = comments.where({asset_id}); - } - - return comments; - } - /** * Pushes a new status in for the user. * @param {String} id identifier of the comment (uuid) @@ -356,7 +395,7 @@ events.on(COMMENTS_NEW, async (comment) => { if ( !comment || // Check that the comment is defined. (!comment.parent_id || comment.parent_id.length === 0) || // Check that the comment has a parent (is a reply). - !(comment.status === 'NONE' || comment.status === 'APPROVED') // Check that the comment is visible. + !(comment.status === 'NONE' || comment.status === 'ACCEPTED') // Check that the comment is visible. ) { return; } diff --git a/test/server/services/comments.js b/test/server/services/comments.js index e33089325..2be05f87c 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -10,7 +10,6 @@ const CommentsService = require('../../../services/comments'); const settings = {id: '1', moderation: 'PRE', wordlist: {banned: ['bad words'], suspect: ['suspect words']}}; const chai = require('chai'); -chai.use(require('chai-as-promised')); chai.use(require('sinon-chai')); const expect = chai.expect; @@ -96,102 +95,118 @@ describe('services.CommentsService', () => { user_id: '456' }]; - beforeEach(() => { - return SettingsService.init(settings).then(() => { - return Promise.all([ - CommentModel.create(comments), - UsersService.createLocalUsers(users), - ActionModel.create(actions) - ]); - }); + beforeEach(async () => { + await SettingsService.init(settings); + + await Promise.all([ + CommentModel.create(comments), + UsersService.createLocalUsers(users), + ActionModel.create(actions) + ]); }); describe('#publicCreate()', () => { - it('creates a new comment', () => { - return CommentsService - .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.be.equal('ACCEPTED'); - }); + it('creates a new comment', async () => { + const c = await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'ACCEPTED' + }); + + expect(c).to.not.be.null; + expect(c.id).to.not.be.null; + expect(c.id).to.be.uuid; + expect(c.status).to.be.equal('ACCEPTED'); }); - it('creates many new comments', () => { - return CommentsService - .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.be.equal('ACCEPTED'); + it('creates many new comments', async () => { + const [ + c1, + c2, + c3, + ] = await CommentsService.publicCreate([{ + body: 'This is a comment!', + status: 'ACCEPTED' + }, { + body: 'This is another comment!' + }, { + body: 'This is a rejected comment!', + status: 'REJECTED' + }]); - expect(c2).to.not.be.null; - expect(c2.id).to.be.uuid; - expect(c2.status).to.be.equal('NONE'); + expect(c1).to.not.be.null; + expect(c1.status).to.be.equal('ACCEPTED'); - expect(c3).to.not.be.null; - expect(c3.id).to.be.uuid; - expect(c3.status).to.be.equal('REJECTED'); - }); + expect(c2).to.not.be.null; + expect(c2.status).to.be.equal('NONE'); + + expect(c3).to.not.be.null; + expect(c3.status).to.be.equal('REJECTED'); }); + }); + describe.only('#edit', () => { + it('changes the comment status back to premod if it was accepted', async () => { + const originalComment = await CommentsService.publicCreate({ + body: 'this is a body!', + status: 'PREMOD', + author_id: '123', + }); + + expect(originalComment.status_history).to.have.length(1); + + await CommentsService.pushStatus(originalComment.id, 'ACCEPTED'); + + let retrivedComment = await CommentsService.findById(originalComment.id); + + expect(retrivedComment).to.have.property('status', 'ACCEPTED'); + expect(retrivedComment.status_history).to.have.length(2); + expect(retrivedComment.status_history[1]).to.have.property('type', 'ACCEPTED'); + + const editedComment = await CommentsService.edit({ + id: originalComment.id, + author_id: '123', + body: 'This is a body!', + status: 'NONE', + }); + + expect(editedComment).to.have.property('status', 'PREMOD'); + expect(editedComment.status_history).to.have.length(4); + expect(editedComment.status_history[3]).to.have.property('type', 'PREMOD'); + + retrivedComment = await CommentsService.findById(originalComment.id); + + expect(retrivedComment).to.have.property('status', 'PREMOD'); + expect(retrivedComment.status_history).to.have.length(4); + expect(retrivedComment.status_history[3]).to.have.property('type', 'PREMOD'); + }); }); describe('#findById()', () => { - it('should find a comment by id', () => { - return CommentsService - .findById('1') - .then((result) => { - expect(result).to.not.be.null; - expect(result).to.have.property('body', 'comment 10'); - }); + it('should find a comment by id', async () => { + const comment = await CommentsService.findById('1'); + expect(comment).to.not.be.null; + expect(comment).to.have.property('body', 'comment 10'); }); }); describe('#findByAssetId()', () => { - it('should find an array of all comments by asset id', () => { - return CommentsService - .findByAssetId('123') - .then((result) => { - expect(result).to.have.length(3); - result.sort((a, b) => { - if (a.body < b.body) {return -1;} - else {return 1;} - }); - expect(result[0]).to.have.property('body', 'comment 10'); - expect(result[1]).to.have.property('body', 'comment 20'); - expect(result[2]).to.have.property('body', 'comment 40'); - }); + it('should find an array of all comments by asset id', async () => { + const comments = await CommentsService.findByAssetId('123'); + expect(comments).to.have.length(3); + comments.sort((a, b) => { + if (a.body < b.body) {return -1;} + else {return 1;} + }); + expect(comments[0]).to.have.property('body', 'comment 10'); + expect(comments[1]).to.have.property('body', 'comment 20'); + expect(comments[2]).to.have.property('body', 'comment 40'); }); }); - describe('#moderationQueue()', () => { - - it('should find an array of new comments to moderate when pre-moderation', () => { - return CommentsService - .moderationQueue('PREMOD') - .then((result) => { - expect(result).to.not.be.null; - expect(result).to.have.lengthOf(2); - }); - }); - - }); - describe('#changeStatus', () => { it('should change the status of a comment from no status', async () => { @@ -220,20 +235,18 @@ describe('services.CommentsService', () => { expect(c3.status_history[0]).to.have.property('assigned_by', '123'); }); - it('should change the status of a comment from accepted', () => { - return CommentsService.pushStatus(comments[1].id, 'REJECTED', '123') - .then(() => CommentsService.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); + it('should change the status of a comment from accepted', async () => { + await CommentsService.pushStatus(comments[1].id, 'REJECTED', '123'); + const c = await CommentsService.findById(comments[1].id); + 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); - expect(c.status_history[1]).to.have.property('type', 'REJECTED'); - expect(c.status_history[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'); }); }); });