From 45d4c79c265fe8f8290142a29a6d1db2791fd275 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 16 Jan 2018 12:30:38 -0700 Subject: [PATCH 1/2] added reply bug --- errors.js | 16 +++++- locales/en.yml | 1 + locales/es.yml | 1 + services/comments.js | 36 ++++++++---- test/.eslintrc.json | 2 +- test/server/graph/mutations/createComment.js | 2 + test/server/services/comments.js | 58 ++++++++++++++++++++ 7 files changed, 100 insertions(+), 16 deletions(-) diff --git a/errors.js b/errors.js index 868be0488..a19afe3c6 100644 --- a/errors.js +++ b/errors.js @@ -265,6 +265,15 @@ const ErrCannotIgnoreStaff = new APIError('Cannot ignore staff members.', { status: 400, }); +// ErrParentDoesNotVisible is returned when the user tries to reply to a comment +// that isn't visible. +const ErrParentDoesNotVisible = new APIError( + 'Cannot reply to a comment that is not visible', + { + translation_key: 'COMMENT_PARENT_NOT_VISIBLE', + } +); + module.exports = { APIError, ErrAlreadyExists, @@ -276,24 +285,25 @@ module.exports = { ErrContainsProfanity, ErrEditWindowHasEnded, ErrEmailTaken, + ErrEmailVerificationToken, ErrInstallLock, ErrInvalidAssetURL, ErrLoginAttemptMaximumExceeded, ErrMaxRateLimit, ErrMissingEmail, ErrMissingPassword, - ErrEmailVerificationToken, - ErrPasswordResetToken, ErrMissingUsername, ErrNotAuthorized, ErrNotFound, ErrNotVerified, + ErrParentDoesNotVisible, + ErrPasswordResetToken, ErrPasswordTooShort, ErrPermissionUpdateUsername, + ErrSameUsernameProvided, ErrSettingsInit, ErrSettingsNotInit, ErrSpecialChars, ErrUsernameTaken, - ErrSameUsernameProvided, ExtendableError, }; diff --git a/locales/en.yml b/locales/en.yml index 0da121202..39cd3f0d9 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -198,6 +198,7 @@ en: embedlink: copy: "Copy to Clipboard" error: + COMMENT_PARENT_NOT_VISIBLE: "The comment that you're replying to has been removed or doesn't exist." EMAIL_VERIFICATION_TOKEN_INVALID: "Email verification token is invalid." PASSWORD_RESET_TOKEN_INVALID: "Your password reset link is invalid." COMMENT_TOO_SHORT: "Comments should be more than one character, please revise your comment and try again." diff --git a/locales/es.yml b/locales/es.yml index 584e83f7b..010119728 100644 --- a/locales/es.yml +++ b/locales/es.yml @@ -182,6 +182,7 @@ es: embedlink: copy: "Copiar al portapapeles" error: + COMMENT_PARENT_NOT_VISIBLE: "El comentario a la que estás contestando ha sido eliminado o no existe." COMMENT_TOO_SHORT: "Tu comentario debe tener algo escrito" COMMENTING_CLOSED: "Los comentarios ya estan cerrados" confirm_password: "Las contraseñas no coinciden. Inténtelo nuevamente" diff --git a/services/comments.js b/services/comments.js index 344115054..d82f5046d 100644 --- a/services/comments.js +++ b/services/comments.js @@ -7,24 +7,35 @@ const SettingsService = require('./settings'); const cloneDeep = require('lodash/cloneDeep'); const errors = require('../errors'); const events = require('./events'); +const merge = require('lodash/merge'); const { COMMENTS_NEW, COMMENTS_EDIT } = require('./events/constants'); module.exports = class CommentsService { /** * Creates a new Comment that came from a public source. - * @param {Mixed} comment either a single comment or an array of comments. + * @param {Mixed} input either a single comment or an array of comments. * @return {Promise} */ - static async publicCreate(comment) { + static async publicCreate(input) { // Check to see if this is an array of comments, if so map it out. - if (Array.isArray(comment)) { - return Promise.all(comment.map(CommentsService.publicCreate)); + if (Array.isArray(input)) { + return Promise.all(input.map(CommentsService.publicCreate)); } - const { status = 'NONE' } = comment; + const { status = 'NONE', parent_id = null } = input; - const commentModel = new CommentModel( - Object.assign( + // Check to see if we are replying to a comment, and if that comment is + // visible. + if (parent_id !== null) { + const parent = await CommentModel.findOne({ id: parent_id }); + if (parent === null || !parent.visible) { + throw errors.ErrParentDoesNotVisible; + } + } + + // Turn the comment into a new object. + const comment = new CommentModel( + merge( { status_history: status ? [ @@ -36,21 +47,22 @@ module.exports = class CommentsService { : [], body_history: [ { - body: comment.body, + body: input.body, created_at: new Date(), }, ], }, - comment + input ) ); - const savedCommentModel = await commentModel.save(); + // Save the comment to the database. + await comment.save(); // Emit that the comment was created! - await events.emitAsync(COMMENTS_NEW, savedCommentModel); + await events.emitAsync(COMMENTS_NEW, comment); - return savedCommentModel; + return comment; } /** diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 027872c7d..36b8a3078 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -7,6 +7,6 @@ ], "extends": "../.eslintrc.json", "rules": { - "mocha/no-exclusive-tests": "warn" + "mocha/no-exclusive-tests": "error" } } diff --git a/test/server/graph/mutations/createComment.js b/test/server/graph/mutations/createComment.js index 88ca3415a..8a5a98aa1 100644 --- a/test/server/graph/mutations/createComment.js +++ b/test/server/graph/mutations/createComment.js @@ -116,6 +116,8 @@ describe('graph.mutations.createComment', () => { } expect(data.createComment).to.have.property('errors').null; expect(data.createComment).to.have.property('comment').not.null; + expect(data.createComment.comment).to.have.property('id').not + .null; } } ); diff --git a/test/server/services/comments.js b/test/server/services/comments.js index c9ec1315d..b0af0b0e0 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -131,6 +131,64 @@ describe('services.CommentsService', () => { }); describe('#publicCreate()', () => { + describe('does not allow replies to comments that are not visible', () => { + it('parent not found', async () => { + try { + await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'ACCEPTED', + parent_id: 'does not exist', + }); + throw new Error('comment should not have been created'); + } catch (err) { + expect(err).to.have.property( + 'translation_key', + 'COMMENT_PARENT_NOT_VISIBLE' + ); + } + }); + + it('parent REJECTED', async () => { + try { + const parent = await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'REJECTED', + }); + await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'ACCEPTED', + parent_id: parent.id, + }); + throw new Error('comment should not have been created'); + } catch (err) { + expect(err).to.have.property( + 'translation_key', + 'COMMENT_PARENT_NOT_VISIBLE' + ); + } + }); + + it('parent SYSTEM_WITHHELD', async () => { + try { + const parent = await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'SYSTEM_WITHHELD', + }); + await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'ACCEPTED', + parent_id: parent.id, + }); + throw new Error('comment should not have been created'); + } catch (err) { + expect(err).to.have.property( + 'translation_key', + 'COMMENT_PARENT_NOT_VISIBLE' + ); + } + }); + }); + it('creates a new comment', async () => { const c = await CommentsService.publicCreate({ body: 'This is a comment!', From b709cb485e3ceec5e3cdca030ff1524b685e6848 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 16 Jan 2018 14:11:24 -0700 Subject: [PATCH 2/2] refactored publicCreate --- services/comments.js | 15 +++------- test/server/graph/queries/asset.js | 14 +++++---- test/server/services/assets.js | 46 ++++++++++++++++-------------- test/server/services/comments.js | 25 ---------------- 4 files changed, 36 insertions(+), 64 deletions(-) diff --git a/services/comments.js b/services/comments.js index d82f5046d..e18e04970 100644 --- a/services/comments.js +++ b/services/comments.js @@ -13,15 +13,11 @@ const { COMMENTS_NEW, COMMENTS_EDIT } = require('./events/constants'); module.exports = class CommentsService { /** * Creates a new Comment that came from a public source. - * @param {Mixed} input either a single comment or an array of comments. + * @param {Object} input either a single comment or an array of comments. * @return {Promise} */ static async publicCreate(input) { - // Check to see if this is an array of comments, if so map it out. - if (Array.isArray(input)) { - return Promise.all(input.map(CommentsService.publicCreate)); - } - + // Extract the parent_id from the comment, if there is one. const { status = 'NONE', parent_id = null } = input; // Check to see if we are replying to a comment, and if that comment is @@ -33,8 +29,8 @@ module.exports = class CommentsService { } } - // Turn the comment into a new object. - const comment = new CommentModel( + // Create the comment in the database. + const comment = await CommentModel.create( merge( { status_history: status @@ -56,9 +52,6 @@ module.exports = class CommentsService { ) ); - // Save the comment to the database. - await comment.save(); - // Emit that the comment was created! await events.emitAsync(COMMENTS_NEW, comment); diff --git a/test/server/graph/queries/asset.js b/test/server/graph/queries/asset.js index af19e4f69..f976bd37d 100644 --- a/test/server/graph/queries/asset.js +++ b/test/server/graph/queries/asset.js @@ -34,12 +34,14 @@ describe('graph.queries.asset', () => { username: 'usernameC', }, ]); - comments = await CommentsService.publicCreate( - [0, 0, 1, 1].map(idx => ({ - author_id: users[idx].id, - asset_id: assets[idx].id, - body: `hello there! ${String(Math.random()).slice(2)}`, - })) + comments = await Promise.all( + [0, 0, 1, 1].map(idx => + CommentsService.publicCreate({ + author_id: users[idx].id, + asset_id: assets[idx].id, + body: `hello there! ${String(Math.random()).slice(2)}`, + }) + ) ); }); diff --git a/test/server/services/assets.js b/test/server/services/assets.js index c071b5634..923f28751 100644 --- a/test/server/services/assets.js +++ b/test/server/services/assets.js @@ -176,28 +176,30 @@ describe('services.AssetsService', () => { ); // Create some comments on both assets. - await CommentsService.publicCreate([ - { - asset_id: '1', - body: 'This is a comment!', - status: 'ACCEPTED', - }, - { - asset_id: '1', - body: 'This is a comment!', - status: 'ACCEPTED', - }, - { - asset_id: '2', - body: 'This is a comment!', - status: 'ACCEPTED', - }, - { - asset_id: '2', - body: 'This is a comment!', - status: 'ACCEPTED', - }, - ]); + await Promise.all( + [ + { + asset_id: '1', + body: 'This is a comment!', + status: 'ACCEPTED', + }, + { + asset_id: '1', + body: 'This is a comment!', + status: 'ACCEPTED', + }, + { + asset_id: '2', + body: 'This is a comment!', + status: 'ACCEPTED', + }, + { + asset_id: '2', + body: 'This is a comment!', + status: 'ACCEPTED', + }, + ].map(comment => CommentsService.publicCreate(comment)) + ); // Merge all the comments from asset 1 into asset 2, followed by deleting // asset 1. diff --git a/test/server/services/comments.js b/test/server/services/comments.js index b0af0b0e0..2d0b37163 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -200,31 +200,6 @@ describe('services.CommentsService', () => { expect(c.id).to.be.uuid; expect(c.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(c1).to.not.be.null; - expect(c1.status).to.be.equal('ACCEPTED'); - - 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('#edit', () => {