diff --git a/ghost/core/core/server/api/endpoints/comments-comments.js b/ghost/core/core/server/api/endpoints/comments-comments.js index 35f98dbf01..a84cae6397 100644 --- a/ghost/core/core/server/api/endpoints/comments-comments.js +++ b/ghost/core/core/server/api/endpoints/comments-comments.js @@ -3,7 +3,7 @@ const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); const models = require('../../models'); const db = require('../../data/db'); -const service = require('../../services/comments'); +const commentsService = require('../../services/comments'); const ALLOWED_INCLUDES = ['post', 'member', 'likes', 'replies']; const UNSAFE_ATTRS = ['status']; @@ -52,17 +52,8 @@ module.exports = { } }, permissions: true, - query(frame) { - return models.Comment.findOne(frame.data, frame.options) - .then((model) => { - if (!model) { - return Promise.reject(new errors.NotFoundError({ - message: tpl(messages.commentNotFound) - })); - } - - return model; - }); + async query(frame) { + return await commentsService.api.getCommentByID(frame.data.id, frame.options); } }, @@ -83,17 +74,21 @@ module.exports = { } }, permissions: true, - query(frame) { - return models.Comment.edit(frame.data.comments[0], frame.options) - .then((model) => { - if (!model) { - return Promise.reject(new errors.NotFoundError({ - message: tpl(messages.commentNotFound) - })); - } + async query(frame) { + if (frame.data.comments[0].status === 'deleted') { + return await commentsService.api.deleteComment( + frame.options.id, + frame?.options?.context?.member?.id, + frame.options + ); + } - return model; - }); + return await commentsService.api.editCommentContent( + frame.options.id, + frame?.options?.context?.member?.id, + frame.data.comments[0].html, + frame.options + ); } }, @@ -116,20 +111,24 @@ module.exports = { permissions: { unsafeAttrs: UNSAFE_ATTRS }, - query(frame) { - // TODO: move to comment service + async query(frame) { const data = frame.data.comments[0]; - if (frame.options?.context?.member?.id) { - data.member_id = frame.options.context.member.id; - - // todo: add validation that the parent comment is on the same post, and not deleted - return models.Comment.add(data, frame.options); - } else { - return Promise.reject(new errors.NotFoundError({ - message: tpl(messages.memberNotFound) - })); + if (data.parent_id) { + return await commentsService.api.replyToComment( + data.parent_id, + frame.options.context.member.id, + data.html, + frame.options + ); } + + return await commentsService.api.commentOnPost( + data.post_id, + frame.options.context.member.id, + data.html, + frame.options + ); } }, @@ -260,7 +259,7 @@ module.exports = { })); } - await service.api.reportComment(frame.options.id, frame.options?.context?.member); + await commentsService.api.reportComment(frame.options.id, frame.options?.context?.member); } } }; diff --git a/ghost/core/core/server/services/comments/service.js b/ghost/core/core/server/services/comments/service.js index ba7c74ec2b..097dfdb10a 100644 --- a/ghost/core/core/server/services/comments/service.js +++ b/ghost/core/core/server/services/comments/service.js @@ -1,3 +1,14 @@ +const tpl = require('@tryghost/tpl'); +const errors = require('@tryghost/errors'); + +const messages = { + commentNotFound: 'Comment could not be found', + memberNotFound: 'Unable to find member', + likeNotFound: 'Unable to find like', + alreadyLiked: 'This comment was liked already', + replyToReply: 'Can not reply to a reply' +}; + class CommentsService { constructor({config, logging, models, mailer, settingsCache, urlService, urlUtils}) { this.config = config; @@ -42,6 +53,142 @@ class CommentsService { await this.emails.notifiyReport(comment, reporter); } + + /** + * @param {string} id - The ID of the Comment to get + * @param {any} options + */ + async getCommentByID(id, options) { + const model = await this.models.Comment.findOne({id}, options); + + if (!model) { + throw new errors.NotFoundError({ + messages: tpl(messages.commentNotFound) + }); + } + + return model; + } + + /** + * @param {string} post - The ID of the Post to comment on + * @param {string} member - The ID of the Member to comment as + * @param {string} comment - The HTML content of the Comment + * @param {any} options + */ + async commentOnPost(post, member, comment, options) { + await this.models.Member.findOne({ + id: member + }, { + require: true, + ...options + }); + + const model = await this.models.Comment.add({ + post_id: post, + member_id: member, + parent_id: null, + html: comment, + status: 'published' + }, options); + + return model; + } + + /** + * @param {string} parent - The ID of the Comment to reply to + * @param {string} member - The ID of the Member to comment as + * @param {string} comment - The HTML content of the Comment + * @param {any} options + */ + async replyToComment(parent, member, comment, options) { + await this.models.Member.findOne({ + id: member + }, { + require: true, + ...options + }); + + const parentComment = await this.getCommentByID(parent, options); + if (!parentComment) { + throw new errors.BadRequestError({ + message: tpl(messages.commentNotFound) + }); + } + + if (parentComment.get('parent_id') !== null) { + throw new errors.BadRequestError({ + message: tpl(messages.replyToReply) + }); + } + + const model = await this.models.Comment.add({ + post_id: parentComment.get('post_id'), + member_id: member, + parent_id: parentComment.id, + html: comment, + status: 'published' + }, options); + + return model; + } + + /** + * @param {string} id - The ID of the Comment to delete + * @param {string} member - The ID of the Member to delete as + * @param {any} options + */ + async deleteComment(id, member, options) { + const existingComment = await this.getCommentByID(id, options); + + if (existingComment.get('member_id') !== member) { + throw new errors.NoPermissionError({ + // todo fix message + message: tpl(messages.memberNotFound) + }); + } + + const model = await this.models.Comment.edit({ + status: 'deleted' + }, { + id, + require: true, + ...options + }); + + return model; + } + + /** + * @param {string} id - The ID of the Comment to edit + * @param {string} member - The ID of the Member to edit as + * @param {string} comment - The new HTML content of the Comment + * @param {any} options + */ + async editCommentContent(id, member, comment, options) { + const existingComment = await this.getCommentByID(id, options); + + if (!comment) { + return existingComment; + } + + if (existingComment.get('member_id') !== member) { + throw new errors.NoPermissionError({ + // todo fix message + message: tpl(messages.memberNotFound) + }); + } + + const model = await this.models.Comment.edit({ + html: comment + }, { + id, + require: true, + ...options + }); + + return model; + } } module.exports = CommentsService; diff --git a/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap b/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap index bcf4ce9971..5b890b12ec 100644 --- a/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap +++ b/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap @@ -110,6 +110,59 @@ Object { } `; +exports[`Comments API when authenticated Can edit a comment on a post 1: [body] 1`] = ` +Object { + "comments": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "edited_at": null, + "html": "Updated comment", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": Any, + "likes_count": Any, + "member": Object { + "avatar_image": null, + "bio": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": null, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + }, + "replies": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "edited_at": null, + "html": "This is a reply", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": Any, + "likes_count": Any, + "member": Object { + "avatar_image": null, + "bio": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": null, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + }, + "status": "published", + }, + ], + "status": "published", + }, + ], +} +`; + +exports[`Comments API when authenticated Can edit a comment on a post 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "624", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Comments API when authenticated Can like a comment 1: [body] 1`] = ` Object { "comments": Array [ @@ -256,6 +309,155 @@ Object { } `; +exports[`Comments API when authenticated Can not edit a comment as a member who is not you 1: [body] 1`] = ` +Object { + "comments": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "edited_at": null, + "html": "Illegal comment update", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": Any, + "likes_count": Any, + "member": Object { + "avatar_image": null, + "bio": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": null, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + }, + "replies": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "edited_at": null, + "html": "This is a reply", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": Any, + "likes_count": Any, + "member": Object { + "avatar_image": null, + "bio": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": null, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + }, + "status": "published", + }, + ], + "status": "published", + }, + ], +} +`; + +exports[`Comments API when authenticated Can not edit a comment as a member who is not you 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "631", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Comments API when authenticated Can not edit a comment post_id 1: [body] 1`] = ` +Object { + "comments": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "edited_at": null, + "html": "Updated comment", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": Any, + "likes_count": Any, + "member": Object { + "avatar_image": null, + "bio": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": null, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + }, + "replies": Array [], + "status": "published", + }, + ], +} +`; + +exports[`Comments API when authenticated Can not edit a comment post_id 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "326", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Comments API when authenticated Can not edit a comment which does not belong to you 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "You do not have permission to edit comments", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Permission error, cannot edit comment.", + "property": null, + "type": "NoPermissionError", + }, + ], +} +`; + +exports[`Comments API when authenticated Can not edit a comment which does not belong to you 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "269", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Comments API when authenticated Can not reply to a reply 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Can not reply to a reply", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Request not understood error, cannot save comment.", + "property": null, + "type": "BadRequestError", + }, + ], +} +`; + +exports[`Comments API when authenticated Can not reply to a reply 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "260", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Comments API when authenticated Can remove a like 1: [headers] 1`] = ` Object { "access-control-allow-origin": "*", diff --git a/ghost/core/test/e2e-api/members-comments/comments.test.js b/ghost/core/test/e2e-api/members-comments/comments.test.js index c7f3f144f1..d4b6d198ad 100644 --- a/ghost/core/test/e2e-api/members-comments/comments.test.js +++ b/ghost/core/test/e2e-api/members-comments/comments.test.js @@ -1,9 +1,10 @@ +const assert = require('assert'); const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework'); const {anyEtag, anyObjectId, anyLocationFor, anyISODateTime, anyUuid, anyNumber, anyBoolean} = matchers; const should = require('should'); const models = require('../../../core/server/models'); -let membersAgent, member, postId, commentId; +let membersAgent, membersAgent2, member, postId, commentId; const commentMatcherNoMember = { id: anyObjectId, @@ -42,6 +43,7 @@ async function sleep(ms) { describe('Comments API', function () { before(async function () { membersAgent = await agentProvider.getMembersAPIAgent(); + membersAgent2 = await agentProvider.getMembersAPIAgent(); await fixtureManager.init('posts', 'members', 'comments'); @@ -91,6 +93,7 @@ describe('Comments API', function () { before(async function () { await membersAgent.loginAs('member@example.com'); member = await models.Member.findOne({email: 'member@example.com'}, {require: true}); + await membersAgent2.loginAs('member2@example.com'); }); it('Can comment on a post', async function () { @@ -113,7 +116,7 @@ describe('Comments API', function () { // Wait for the emails (because this happens async) await sleep(100); - + // Check if author got an email mockManager.assert.sentEmailCount(1); mockManager.assert.sentEmail({ @@ -292,7 +295,7 @@ describe('Comments API', function () { mockManager.assert.sentEmail({ subject: '🚩 A comment has been reported on your post', to: fixtureManager.get('users', 0).email - }); + }); }); it('Cannot report a comment twice', async function () { @@ -314,5 +317,180 @@ describe('Comments API', function () { mockManager.assert.sentEmailCount(0); }); + + it('Can edit a comment on a post', async function () { + await membersAgent + .put(`/api/comments/${commentId}`) + .body({comments: [{ + html: 'Updated comment' + }]}) + .expectStatus(200) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + comments: [commentMatcherWithReply] + }); + }); + + it('Can not edit a comment post_id', async function () { + const anotherPostId = fixtureManager.get('posts', 1).id; + await membersAgent + .put(`/api/comments/${commentId}`) + .body({comments: [{ + post_id: anotherPostId + }]}); + + const {body} = await membersAgent + .get(`/api/comments/?filter=post_id:${anotherPostId}`); + + assert(!body.comments.find(comment => comment.id === commentId), 'The comment should not have moved post'); + }); + + it('Can not edit a comment which does not belong to you', async function () { + await membersAgent2 + .put(`/api/comments/${commentId}`) + .body({comments: [{ + html: 'Illegal comment update' + }]}) + .expectStatus(403) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + errors: [{ + type: 'NoPermissionError', + id: anyUuid + }] + }); + }); + + it('Can not edit a comment as a member who is not you', async function () { + const memberId = fixtureManager.get('members', 1).id; + await membersAgent + .put(`/api/comments/${commentId}`) + .body({comments: [{ + html: 'Illegal comment update', + member_id: memberId + }]}); + + const { + body: { + comments: [ + comment + ] + } + } = await membersAgent.get(`/api/comments/${commentId}`) + .expectStatus(200) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + comments: [commentMatcherWithReply] + }); + + assert(comment.member.id !== memberId); + }); + + it('Can not reply to a reply', async function () { + const { + body: { + comments: [{ + id: parentId + }] + } + } = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + html: 'Parent' + }]}); + + const { + body: { + comments: [{ + id: replyId + }] + } + } = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + parent_id: parentId, + html: 'Reply' + }]}); + + await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + parent_id: replyId, + html: 'Reply to a reply!' + }]}) + .expectStatus(400) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + errors: [{ + type: 'BadRequestError', + id: anyUuid + }] + }); + }); + + it('Can not edit a replies parent', async function () { + const { + body: { + comments: [{ + id: parentId + }] + } + } = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + html: 'Parent' + }]}); + + const { + body: { + comments: [{ + id: newParentId + }] + } + } = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + html: 'New Parent' + }]}); + + const { + body: { + comments: [{ + id: replyId + }] + } + } = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + parent_id: parentId, + html: 'Reply' + }]}); + + // Attempt to edit the parent + await membersAgent + .put(`/api/comments/${replyId}/`) + .body({comments: [{ + parent_id: newParentId, + html: 'Changed parent' + }]}); + + const {body: {comments: [comment]}} = await membersAgent.get(`api/comments/${newParentId}`); + + assert(comment.replies.length === 0, 'The parent comment should not have changed'); + }); }); });