From b2e871f4a66fc02be63320800c9ff9975188d0a9 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sat, 19 Aug 2017 10:49:50 -0600 Subject: [PATCH] added event for edit to manage reply count --- models/comment.js | 8 ++ package.json | 2 + services/comments.js | 180 +++++++++++++++++++------------ services/events/constants.js | 2 + test/server/services/comments.js | 74 +++++-------- yarn.lock | 71 ++++++++++++ 6 files changed, 218 insertions(+), 119 deletions(-) diff --git a/models/comment.js b/models/comment.js index 972f37e46..e30cd091f 100644 --- a/models/comment.js +++ b/models/comment.js @@ -70,6 +70,9 @@ const CommentSchema = new Schema({ // parent_id is the id of the parent comment (null if there is none). parent_id: String, + // The number of replies to this comment directly. + reply_count: Number, + // Counts to store related to actions taken on the given comment. action_counts: { default: {}, @@ -123,6 +126,11 @@ CommentSchema.virtual('edited').get(function() { return this.body_history.length > 1; }); +// Visable is true when the comment is visible to the public. +CommentSchema.virtual('visible').get(function() { + return ['ACCEPTED', 'NONE'].includes(this.status); +}); + // Comment model. const Comment = mongoose.model('Comment', CommentSchema); diff --git a/package.json b/package.json index 29a8bdcaa..6e57aa221 100644 --- a/package.json +++ b/package.json @@ -198,6 +198,8 @@ "mocha-junit-reporter": "^1.12.1", "nodemon": "^1.11.0", "pre-git": "^3.10.0", + "sinon": "^3.2.1", + "sinon-chai": "^2.13.0", "supertest": "^2.0.1" }, "engines": { diff --git a/services/comments.js b/services/comments.js index 8db14966b..9f3bce0ea 100644 --- a/services/comments.js +++ b/services/comments.js @@ -10,6 +10,8 @@ const events = require('./events'); const { ACTIONS_NEW, ACTIONS_DELETE, + COMMENTS_NEW, + COMMENTS_EDIT, } = require('./events/constants'); module.exports = class CommentsService { @@ -19,7 +21,7 @@ module.exports = class CommentsService { * @param {Mixed} comment either a single comment or an array of comments. * @return {Promise} */ - static publicCreate(comment) { + static async publicCreate(comment) { // Check to see if this is an array of comments, if so map it out. if (Array.isArray(comment)) { @@ -41,7 +43,12 @@ module.exports = class CommentsService { }] }, comment)); - return commentModel.save(); + const savedCommentModel = await commentModel.save(); + + // Emit that the comment was created! + events.emitAsync(COMMENTS_NEW, savedCommentModel); + + return savedCommentModel; } /** @@ -54,7 +61,10 @@ module.exports = class CommentsService { static async edit(id, author_id, {body, status, ignoreEditWindow = false}) { const query = { id, - author_id + author_id, + status: { + $in: ['NONE', 'PREMOD'], + }, }; // Establish the edit window (if it exists) and add the condition to the @@ -68,9 +78,7 @@ module.exports = class CommentsService { }; } - const { - value: comment - } = await CommentModel.findOneAndUpdate(query, { + const originalComment = await CommentModel.findOneAndUpdate(query, { $set: { body, status, @@ -85,12 +93,9 @@ module.exports = class CommentsService { created_at: new Date(), } }, - }, { - new: true, - rawResult: true }); - if (comment === null) { + if (originalComment === null) { // Try to get the comment. const comment = await CommentsService.findById(id); @@ -103,6 +108,11 @@ module.exports = class CommentsService { throw errors.ErrNotAuthorized; } + // Check to see if the comment had a status that was editable. + if (!['NONE', 'PREMOD'].includes(comment.status)) { + throw errors.ErrNotAuthorized; + } + // Check to see if the edit window expired. if (!ignoreEditWindow && comment.created_at <= lastEditableCommentCreatedAt) { throw errors.ErrEditWindowHasEnded; @@ -111,7 +121,22 @@ module.exports = class CommentsService { throw new Error('comment edit failed for an unexpected reason'); } - return comment; + // Mutate the comment like Mongo would have. + const editedComment = originalComment; + editedComment.status = status; + editedComment.body = body; + editedComment.body_history.push({ + body, + created_at: new Date(), + }); + editedComment.status_history.push({ + type: status, + created_at: new Date(), + }); + + events.emitAsync(COMMENTS_EDIT, originalComment, editedComment); + + return editedComment; } /** @@ -211,21 +236,36 @@ module.exports = class CommentsService { * moderation action * @return {Promise} */ - static pushStatus(id, status, assigned_by = null) { - return CommentModel.findOneAndUpdate({id}, { + static async pushStatus(id, status, assigned_by = null) { + const created_at = new Date(); + const originalComment = await CommentModel.findOneAndUpdate({id}, { $push: { status_history: { type: status, - created_at: new Date(), - assigned_by + created_at, + assigned_by, } }, $set: {status} - }, { - - // return modified comment. - new: true, }); + + if (originalComment === null) { + throw errors.ErrNotFound; + } + + const editedComment = new CommentModel(originalComment.toObject()); + editedComment.status_history.push({ + type: status, + created_at, + assigned_by, + }); + editedComment.status = status; + + // Emit that the comment was edited, and pass the original comment and the + // edited comment. + events.emitAsync(COMMENTS_EDIT, originalComment, editedComment); + + return editedComment; } /** @@ -244,57 +284,6 @@ module.exports = class CommentsService { metadata }); } - - /** - * 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} - */ - static removeById(id) { - return CommentModel.remove({id}); - } - - /** - * Remove an action from the comment. - * @param {String} id identifier of the comment (uuid) - * @param {String} action_type the type of the action to be removed - * @param {String} user_id the id of the user performing the action - * @return {Promise} - */ - static removeAction(item_id, user_id, action_type) { - return ActionModel.remove({ - action_type, - item_type: 'COMMENTS', - item_id, - user_id - }); - } - - /** - * Returns all the comments in the collection. - * @return {Promise} - */ - static all() { - return CommentModel.find({}); - } - - /** - * Returns all the comments by user - * probably to be paginated at some point in the future - * @return {Promise} array resolves to an array of comments by that user - */ - static findByUserId(author_id, admin = false) { - - // do not return un-published comments for non-admins - let query = {author_id}; - - if (!admin) { - query.$nor = [{status: 'PREMOD'}, {status: 'REJECTED'}]; - } - - return CommentModel.find(query); - } }; //============================================================================== @@ -334,7 +323,7 @@ events.on(ACTIONS_NEW, async (action) => { return incrActionCounts(action, 1); }); -// When an action is deleted, modify the comment. +// When an action is deleted, remove the action count on the comment. events.on(ACTIONS_DELETE, async (action) => { if (!action || action.item_type !== 'COMMENTS') { return; @@ -342,3 +331,52 @@ events.on(ACTIONS_DELETE, async (action) => { return incrActionCounts(action, -1); }); + +const incrReplyCount = async (comment, value) => { + try { + await CommentModel.update({ + id: comment.parent_id, + }, { + $inc: { + reply_count: value, + }, + }); + } catch (err) { + console.error('Can\'t mutate the reply count:', err); + } +}; + +// When a comment is created, if it is a reply, increment the reply count on the +// parent's document. +events.on(COMMENTS_NEW, async (comment) => { + if ( + !comment || // Check that the comment is defined. + (comment.parent_id === null || 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. + ) { + return; + } + + return incrReplyCount(comment, 1); +}); + +// When a comment is edited, if the visability changed publicly, then modify the +// comment. +events.on(COMMENTS_EDIT, async (originalComment, editedComment) => { + if ( + !editedComment || // Check that the comment is defined. + (editedComment.parent_id === null || editedComment.parent_id.length === 0) // Check that the comment has a parent (is a reply). + ) { + return; + } + + // If the comment was visible before, and now it isn't, decrement the count; + if (originalComment.visible && !editedComment.visible) { + return incrReplyCount(editedComment, -1); + } + + // If the comment was not visible before, and now it is, increment the count. + if (!originalComment.visible && editedComment.visible) { + return incrReplyCount(editedComment, 1); + } +}); diff --git a/services/events/constants.js b/services/events/constants.js index bc6b962b9..a1da8dce6 100644 --- a/services/events/constants.js +++ b/services/events/constants.js @@ -1,2 +1,4 @@ module.exports.ACTIONS_DELETE = 'actions.delete'; module.exports.ACTIONS_NEW = 'actions.new'; +module.exports.COMMENTS_NEW = 'comments.new'; +module.exports.COMMENTS_EDIT = 'comments.edit'; diff --git a/test/server/services/comments.js b/test/server/services/comments.js index 6aab7dad4..8be3e3f00 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -1,7 +1,8 @@ const CommentModel = require('../../../models/comment'); const ActionModel = require('../../../models/action'); -const ActionsService = require('../../../services/actions'); +const events = require('../../../services/events'); +const {COMMENTS_EDIT} = require('../../../services/events/constants'); const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); const CommentsService = require('../../../services/comments'); @@ -10,8 +11,11 @@ const settings = {id: '1', moderation: 'PRE', wordlist: {banned: ['bad words'], const chai = require('chai'); chai.use(require('chai-as-promised')); +chai.use(require('sinon-chai')); const expect = chai.expect; +const sinon = require('sinon'); + describe('services.CommentsService', () => { const comments = [{ body: 'comment 10', @@ -188,58 +192,32 @@ describe('services.CommentsService', () => { }); - describe('#removeAction', () => { - - it('should remove an action', () => { - return CommentsService - .removeAction('3', '123', 'flag') - .then(() => { - return ActionsService.findByItemIdArray(['123']); - }) - .then((actions) => { - expect(actions.length).to.equal(0); - }); - }); - }); - - describe('#findByUserId', () => { - it('should return all comments if admin', () => { - return CommentsService - .findByUserId('456', true) - .then((comments) => { - expect(comments).to.have.length(4); - }); - }); - - it('should not return premod and rejected comments if not admin', () => { - return CommentsService - .findByUserId('456') - .then((comments) => { - expect(comments).to.have.length(1); - }); - }); - - }); - describe('#changeStatus', () => { - it('should change the status of a comment from no status', () => { + it('should change the status of a comment from no status', async () => { let comment_id = comments[0].id; - return CommentsService.findById(comment_id) - .then((c) => { - expect(c.status).to.be.equal('NONE'); + let c = await CommentsService.findById(comment_id); + expect(c.status).to.be.equal('NONE'); - return CommentsService.pushStatus(comment_id, 'REJECTED', '123'); - }) - .then(() => CommentsService.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'); - }); + const spy = sinon.spy(); + events.once(COMMENTS_EDIT, () => spy()); + + let c2 = await CommentsService.pushStatus(comment_id, 'REJECTED', '123'); + expect(c2).to.have.property('status'); + expect(c2.status).to.equal('REJECTED'); + expect(c2.status_history).to.have.length(1); + expect(c2.status_history[0]).to.have.property('type', 'REJECTED'); + expect(c2.status_history[0]).to.have.property('assigned_by', '123'); + + expect(spy).to.have.been.called; + + let c3 = await CommentsService.findById(comment_id); + expect(c3).to.have.property('status'); + expect(c3.status).to.equal('REJECTED'); + expect(c3.status_history).to.have.length(1); + expect(c3.status_history[0]).to.have.property('type', 'REJECTED'); + expect(c3.status_history[0]).to.have.property('assigned_by', '123'); }); it('should change the status of a comment from accepted', () => { diff --git a/yarn.lock b/yarn.lock index 0b4d2f04a..0420dc269 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2143,6 +2143,10 @@ diff@3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/diff/-/diff-3.2.0.tgz#c9ce393a4b7cbd0b058a725c93df299027868ff9" +diff@^3.1.0: + version "3.3.0" + resolved "https://registry.yarnpkg.com/diff/-/diff-3.3.0.tgz#056695150d7aa93237ca7e378ac3b1682b7963b9" + diffie-hellman@^5.0.0: version "5.0.2" resolved "https://registry.yarnpkg.com/diffie-hellman/-/diffie-hellman-5.0.2.tgz#b5835739270cfe26acf632099fded2a07f209e5e" @@ -2918,6 +2922,12 @@ form-data@^2.1.2, form-data@~2.1.1: combined-stream "^1.0.5" mime-types "^2.1.12" +formatio@1.2.0, formatio@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/formatio/-/formatio-1.2.0.tgz#f3b2167d9068c4698a8d51f4f760a39a54d818eb" + dependencies: + samsam "1.x" + formidable@^1.0.17: version "1.1.1" resolved "https://registry.yarnpkg.com/formidable/-/formidable-1.1.1.tgz#96b8886f7c3c3508b932d6bd70c4d3a88f35f1a9" @@ -4260,6 +4270,10 @@ jsx-ast-utils@^1.3.4: version "1.4.1" resolved "https://registry.yarnpkg.com/jsx-ast-utils/-/jsx-ast-utils-1.4.1.tgz#3867213e8dd79bf1e8f2300c0cfc1efb182c0df1" +just-extend@^1.1.22: + version "1.1.22" + resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-1.1.22.tgz#3330af756cab6a542700c64b2e4e4aa062d52fff" + jwa@^1.1.4: version "1.1.5" resolved "https://registry.yarnpkg.com/jwa/-/jwa-1.1.5.tgz#a0552ce0220742cd52e153774a32905c30e756e5" @@ -4591,6 +4605,14 @@ lodash@^4.0.0, lodash@^4.1.0, lodash@^4.14.0, lodash@^4.15.0, lodash@^4.16.6, lo version "4.17.4" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae" +lolex@^1.6.0: + version "1.6.0" + resolved "https://registry.yarnpkg.com/lolex/-/lolex-1.6.0.tgz#3a9a0283452a47d7439e72731b9e07d7386e49f6" + +lolex@^2.1.2: + version "2.1.2" + resolved "https://registry.yarnpkg.com/lolex/-/lolex-2.1.2.tgz#2694b953c9ea4d013e5b8bfba891c991025b2629" + longest@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/longest/-/longest-1.0.1.tgz#30a0b2da38f73770e8294a0d22e6625ed77d0097" @@ -4908,6 +4930,10 @@ nan@^2.0.0, nan@^2.3.0, nan@^2.4.0: version "2.5.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.5.0.tgz#aa8f1e34531d807e9e27755b234b4a6ec0c152a8" +native-promise-only@^0.8.1: + version "0.8.1" + resolved "https://registry.yarnpkg.com/native-promise-only/-/native-promise-only-0.8.1.tgz#20a318c30cb45f71fe7adfbf7b21c99c1472ef11" + natural-compare@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7" @@ -4945,6 +4971,15 @@ nib@~1.1.2: dependencies: stylus "0.54.5" +nise@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/nise/-/nise-1.0.1.tgz#0da92b10a854e97c0f496f6c2845a301280b3eef" + dependencies: + formatio "^1.2.0" + just-extend "^1.1.22" + lolex "^1.6.0" + path-to-regexp "^1.7.0" + no-case@^2.2.0: version "2.3.1" resolved "https://registry.yarnpkg.com/no-case/-/no-case-2.3.1.tgz#7aeba1c73a52184265554b7dc03baf720df80081" @@ -5413,6 +5448,12 @@ path-to-regexp@0.1.7: version "0.1.7" resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" +path-to-regexp@^1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.7.0.tgz#59fde0f435badacba103a84e9d3bc64e96b9937d" + dependencies: + isarray "0.0.1" + path-type@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-1.1.0.tgz#59c44f7ee491da704da415da5a4070ba4f8fe441" @@ -6751,6 +6792,10 @@ safe-buffer@^5.0.1, safe-buffer@~5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.0.1.tgz#d263ca54696cd8a306b5ca6551e92de57918fbe7" +samsam@1.x, samsam@^1.1.3: + version "1.2.1" + resolved "https://registry.yarnpkg.com/samsam/-/samsam-1.2.1.tgz#edd39093a3184370cb859243b2bdf255e7d8ea67" + sax@0.5.x: version "0.5.8" resolved "https://registry.yarnpkg.com/sax/-/sax-0.5.8.tgz#d472db228eb331c2506b0e8c15524adb939d12c1" @@ -6877,6 +6922,24 @@ simplemde@^1.11.2: codemirror-spell-checker "*" marked "*" +sinon-chai@^2.13.0: + version "2.13.0" + resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-2.13.0.tgz#b9a42e801c20234bfc2f43b29e6f4f61b60990c4" + +sinon@^3.2.1: + version "3.2.1" + resolved "https://registry.yarnpkg.com/sinon/-/sinon-3.2.1.tgz#d8adabd900730fd497788a027049c64b08be91c2" + dependencies: + diff "^3.1.0" + formatio "1.2.0" + lolex "^2.1.2" + native-promise-only "^0.8.1" + nise "^1.0.1" + path-to-regexp "^1.7.0" + samsam "^1.1.3" + text-encoding "0.6.4" + type-detect "^4.0.0" + slash@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/slash/-/slash-1.0.0.tgz#c41f2f6c39fc16d1cd17ad4b5d896114ae470d55" @@ -7276,6 +7339,10 @@ tcomb@^2.5.1: version "2.7.0" resolved "https://registry.yarnpkg.com/tcomb/-/tcomb-2.7.0.tgz#10d62958041669a5d53567b9a4ee8cde22b1c2b0" +text-encoding@0.6.4: + version "0.6.4" + resolved "https://registry.yarnpkg.com/text-encoding/-/text-encoding-0.6.4.tgz#e399a982257a276dae428bb92845cb71bdc26d19" + text-table@~0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4" @@ -7433,6 +7500,10 @@ type-detect@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-1.0.0.tgz#762217cc06db258ec48908a1298e8b95121e8ea2" +type-detect@^4.0.0: + version "4.0.3" + resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.3.tgz#0e3f2670b44099b0b46c284d136a7ef49c74c2ea" + type-is@~1.6.14: version "1.6.15" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.15.tgz#cab10fb4909e441c82842eafe1ad646c81804410"