From 516b527c6565152c7afa7f1fdab8579db2cb16a8 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Wed, 20 Jul 2022 15:25:38 +0200 Subject: [PATCH] Fixed comment notification emails for replying on your own comment fixes https://github.com/TryGhost/Team/issues/1697 - Don't send a notification email when you reply on your own comment - Added some tests for comment emails (requires some async waiting, maybe we can improve this in the future) --- core/server/services/comments/emails.js | 5 + .../__snapshots__/comments.test.js.snap | 125 +++++++++++++++++- .../e2e-api/members-comments/comments.test.js | 88 +++++++++++- 3 files changed, 207 insertions(+), 11 deletions(-) diff --git a/core/server/services/comments/emails.js b/core/server/services/comments/emails.js index adca2add34..d327fe73e2 100644 --- a/core/server/services/comments/emails.js +++ b/core/server/services/comments/emails.js @@ -65,6 +65,11 @@ class CommentsServiceEmails { return; } + // Don't send a notification if you reply to your own comment + if (parentMember.id === reply.get('member_id')) { + return; + } + const to = parentMember.get('email'); const subject = '💬 You have a new reply on one of your comments'; diff --git a/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap b/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap index 5b93eb351c..2255e2049b 100644 --- a/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap +++ b/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap @@ -15,7 +15,7 @@ Object { "bio": null, "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, "name": "Mr Egg", - "uuid": "f6f91461-d7d8-4a3f-aa5d-8e582c40b340", + "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 { @@ -127,7 +127,24 @@ Object { "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 [], + "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", }, ], @@ -169,7 +186,7 @@ exports[`Comments API when authenticated Can like a comment 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": "328", + "content-length": "626", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Encoding", @@ -203,7 +220,24 @@ Object { "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 [], + "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", }, ], @@ -214,7 +248,7 @@ exports[`Comments API when authenticated Can like a comment 5: [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": "327", + "content-length": "625", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Encoding", @@ -248,7 +282,24 @@ Object { "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 [], + "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", }, ], @@ -259,7 +310,7 @@ exports[`Comments API when authenticated Can remove a like 3: [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": "328", + "content-length": "626", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Encoding", @@ -267,6 +318,66 @@ Object { } `; +exports[`Comments API when authenticated Can reply to a comment 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": "This is a reply", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": false, + "likes_count": 0, + "member": null, + "status": "published", + }, + ], +} +`; + +exports[`Comments API when authenticated Can reply to a comment 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": "195", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/comments\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Comments API when authenticated Can reply to your own comment 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": "This is a reply", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "liked": false, + "likes_count": 0, + "member": null, + "status": "published", + }, + ], +} +`; + +exports[`Comments API when authenticated Can reply to your own comment 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": "195", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/comments\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Comments API when authenticated Cannot like a comment multiple times 1: [body] 1`] = ` Object { "errors": Array [ diff --git a/test/e2e-api/members-comments/comments.test.js b/test/e2e-api/members-comments/comments.test.js index 781f2a3516..882cf9e39f 100644 --- a/test/e2e-api/members-comments/comments.test.js +++ b/test/e2e-api/members-comments/comments.test.js @@ -24,13 +24,20 @@ const commentMatcherWithReply = { id: anyObjectId, created_at: anyISODateTime, member: { - id: anyObjectId + id: anyObjectId, + uuid: anyUuid }, likes_count: anyNumber, liked: anyBoolean, replies: [commentMatcher] }; +async function sleep(ms) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + describe('Comments API', function () { before(async function () { membersAgent = await agentProvider.getMembersAPIAgent(); @@ -40,6 +47,10 @@ describe('Comments API', function () { postId = fixtureManager.get('posts', 0).id; }); + beforeEach(function () { + mockManager.mockMail(); + }); + afterEach(function () { mockManager.restore(); }); @@ -70,6 +81,16 @@ describe('Comments API', function () { }); // Save for other tests commentId = body.comments[0].id; + + // Wait for the emails (because this happens async) + await sleep(100); + + // Check if author got an email + mockManager.assert.sentEmailCount(1); + mockManager.assert.sentEmail({ + subject: '💬 You have a new comment on one of your posts', + to: fixtureManager.get('users', 0).email + }); }); it('Can browse all comments of a post', async function () { @@ -84,6 +105,65 @@ describe('Comments API', function () { }); }); + it('Can reply to your own comment', async function () { + const {body} = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + parent_id: commentId, + html: 'This is a reply' + }]}) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('comments') + }) + .matchBodySnapshot({ + comments: [commentMatcherNoMember] + }); + + // Wait for the emails (because this happens async) + await sleep(100); + + // Check only the author got an email (because we are the author of this parent comment) + mockManager.assert.sentEmailCount(1); + mockManager.assert.sentEmail({ + subject: '💬 You have a new comment on one of your posts', + to: fixtureManager.get('users', 0).email + }); + }); + + it('Can reply to a comment', async function () { + const {body} = await membersAgent + .post(`/api/comments/`) + .body({comments: [{ + post_id: postId, + parent_id: fixtureManager.get('comments', 0).id, + html: 'This is a reply' + }]}) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('comments') + }) + .matchBodySnapshot({ + comments: [commentMatcherNoMember] + }); + + // Wait for the emails (because this happens async) + await sleep(100); + mockManager.assert.sentEmailCount(2); + mockManager.assert.sentEmail({ + subject: '💬 You have a new comment on one of your posts', + to: fixtureManager.get('users', 0).email + }); + + mockManager.assert.sentEmail({ + subject: '💬 You have a new reply on one of your comments', + to: fixtureManager.get('members', 0).email + }); + }); + it('Can like a comment', async function () { // Check not liked await membersAgent @@ -93,7 +173,7 @@ describe('Comments API', function () { etag: anyEtag }) .matchBodySnapshot({ - comments: new Array(1).fill(commentMatcher) + comments: new Array(1).fill(commentMatcherWithReply) }) .expect(({body}) => { body.comments[0].liked.should.eql(false); @@ -116,7 +196,7 @@ describe('Comments API', function () { etag: anyEtag }) .matchBodySnapshot({ - comments: new Array(1).fill(commentMatcher) + comments: new Array(1).fill(commentMatcherWithReply) }) .expect(({body}) => { body.comments[0].liked.should.eql(true); @@ -156,7 +236,7 @@ describe('Comments API', function () { etag: anyEtag }) .matchBodySnapshot({ - comments: new Array(1).fill(commentMatcher) + comments: new Array(1).fill(commentMatcherWithReply) }) .expect(({body}) => { body.comments[0].liked.should.eql(false);