From 81e6a7c5bd331321701423bb57171cfde1af039c Mon Sep 17 00:00:00 2001 From: "e.baidakova" Date: Wed, 2 Nov 2022 09:47:20 +0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Add=20ability=20to=20cache=20com?= =?UTF-8?q?ments=20count=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes TryGhost/Team#2094 - Comment counts request was changed from `post` to `get` to enable request caching. --- .../src/comment-counts/js/comment-counts.js | 7 +- .../server/api/endpoints/comments-members.js | 3 + .../server/services/comments/controller.js | 16 +- ghost/core/core/server/web/comments/routes.js | 9 +- ghost/core/core/shared/config/defaults.json | 5 +- .../__snapshots__/comments.test.js.snap | 2 +- .../e2e-api/members-comments/comments.test.js | 150 +++++++++--------- 7 files changed, 101 insertions(+), 91 deletions(-) diff --git a/ghost/core/core/frontend/src/comment-counts/js/comment-counts.js b/ghost/core/core/frontend/src/comment-counts/js/comment-counts.js index 6f494369a4..c128a8cbae 100644 --- a/ghost/core/core/frontend/src/comment-counts/js/comment-counts.js +++ b/ghost/core/core/frontend/src/comment-counts/js/comment-counts.js @@ -67,13 +67,12 @@ return; } - const rawRes = await fetch(api, { - method: 'POST', + const rawRes = await fetch(`${api}?ids=${ids.join(',')}`, { + method: 'GET', headers: { Accept: 'application/json', 'Content-Type': 'application/json' - }, - body: JSON.stringify({ids}) + } }); if (rawRes.status !== 200) { diff --git a/ghost/core/core/server/api/endpoints/comments-members.js b/ghost/core/core/server/api/endpoints/comments-members.js index 8a7c025535..1a05a27449 100644 --- a/ghost/core/core/server/api/endpoints/comments-members.js +++ b/ghost/core/core/server/api/endpoints/comments-members.js @@ -132,6 +132,9 @@ module.exports = { counts: { permissions: false, + options: [ + 'ids' + ], async query(frame) { return commentsService.controller.count(frame); } diff --git a/ghost/core/core/server/services/comments/controller.js b/ghost/core/core/server/services/comments/controller.js index 18f9182a2c..613aeb7307 100644 --- a/ghost/core/core/server/services/comments/controller.js +++ b/ghost/core/core/server/services/comments/controller.js @@ -105,11 +105,13 @@ module.exports = class CommentsController { } async count(frame) { - if (!frame?.data?.ids) { + if (!frame?.options?.ids) { return await this.stats.getAllCounts(); } - return await this.stats.getCountsByPost(frame.data.ids); + const ids = frame?.options?.ids.split(','); + + return await this.stats.getCountsByPost(ids); } /** @@ -119,8 +121,8 @@ module.exports = class CommentsController { this.#checkMember(frame); return await this.service.likeComment( - frame.options.id, - frame.options?.context?.member, + frame.options.id, + frame.options?.context?.member, frame.options ); } @@ -132,8 +134,8 @@ module.exports = class CommentsController { this.#checkMember(frame); return await this.service.unlikeComment( - frame.options.id, - frame.options?.context?.member, + frame.options.id, + frame.options?.context?.member, frame.options ); } @@ -145,7 +147,7 @@ module.exports = class CommentsController { this.#checkMember(frame); return await this.service.reportComment( - frame.options.id, + frame.options.id, frame.options?.context?.member ); } diff --git a/ghost/core/core/server/web/comments/routes.js b/ghost/core/core/server/web/comments/routes.js index 0e3cfa795e..c0b8eacabd 100644 --- a/ghost/core/core/server/web/comments/routes.js +++ b/ghost/core/core/server/web/comments/routes.js @@ -1,19 +1,24 @@ const express = require('../../../shared/express'); +const config = require('../../../shared/config'); const api = require('../../api').endpoints; const {http} = require('@tryghost/api-framework'); +const shared = require('../shared'); const bodyParser = require('body-parser'); const membersService = require('../../../server/services/members'); module.exports = function apiRoutes() { const router = express.Router('comment api'); - router.use(bodyParser.json({limit: '50mb'})); // Global handling for member session, ensures a member is logged in to the frontend router.use(membersService.middleware.loadMemberSession); - router.post('/counts', http(api.commentsMembers.counts)); + const countsCache = shared.middleware.cacheControl( + 'public', + {maxAge: config.get('caching:commentsCountAPI:maxAge')} + ); + router.get('/counts', countsCache, http(api.commentsMembers.counts)); router.get('/', http(api.commentsMembers.browse)); router.get('/:id', http(api.commentsMembers.read)); diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index 06b23bc89d..67ac6d8cbd 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -140,6 +140,9 @@ }, "contentAPI": { "maxAge": 0 + }, + "commentsCountAPI": { + "maxAge": 0 } }, "imageOptimization": { @@ -168,7 +171,7 @@ "comments": { "url": "https://cdn.jsdelivr.net/ghost/comments-ui@~{version}/umd/comments-ui.min.js", "styles": "https://cdn.jsdelivr.net/ghost/comments-ui@~{version}/umd/main.css", - "version": "0.10" + "version": "0.11" }, "editor": { "url": "https://unpkg.com/@tryghost/koenig-lexical@~{version}/dist/koenig-lexical.umd.js", 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 fe7c6e2621..0b1ec71bc2 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 @@ -1863,7 +1863,7 @@ Object { exports[`Comments API when commenting enabled for all when authenticated Can fetch counts 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", + "cache-control": "public, max-age=0", "content-length": "89", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, 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 0f7cf7fc15..4d59bbc479 100644 --- a/ghost/core/test/e2e-api/members-comments/comments.test.js +++ b/ghost/core/test/e2e-api/members-comments/comments.test.js @@ -12,7 +12,7 @@ let membersAgent, membersAgent2, postId, postTitle, commentId; async function getPaidProduct() { return await models.Product.findOne({type: 'paid'}); -} +} const commentMatcher = { id: anyObjectId, @@ -61,7 +61,7 @@ function escapeRegExp(string) { async function testCanCommentOnPost(member) { await models.Member.edit({last_seen_at: null, last_commented_at: null}, {id: member.get('id')}); - + const {body} = await membersAgent .post(`/api/comments/`) .body({comments: [{ @@ -181,7 +181,7 @@ async function testCannotReply(status = 403) { describe('Comments API', function () { let member; - + before(async function () { membersAgent = await agentProvider.getMembersAPIAgent(); membersAgent2 = await agentProvider.getMembersAPIAgent(); @@ -212,11 +212,11 @@ describe('Comments API', function () { return getStub.wrappedMethod.call(settingsCache, key, options); }); }); - + afterEach(async function () { sinon.restore(); }); - + it('Can browse all comments of a post', async function () { const {body} = await membersAgent .get(`/api/comments/?filter=post_id:${postId}`) @@ -236,10 +236,10 @@ describe('Comments API', function () { it('cannot reply on a post', async function () { await testCannotReply(401); }); - + it('cannot report a comment', async function () { commentId = fixtureManager.get('comments', 0).id; - + // Create a temporary comment await membersAgent .post(`/api/comments/${commentId}/report/`) @@ -292,7 +292,7 @@ describe('Comments API', function () { await membersAgent.loginAs('member@example.com'); member = await models.Member.findOne({email: 'member@example.com'}, {require: true}); await membersAgent2.loginAs('member2@example.com'); - + // Wait before we mock emails from newly created members // todo: in the future we need a way to wait for DomainEvents to be fired and handled correctly await sleep(200); @@ -307,15 +307,15 @@ describe('Comments API', function () { return getStub.wrappedMethod.call(settingsCache, key, options); }); }); - + afterEach(async function () { sinon.restore(); }); - + it('Can comment on a post', async function () { await testCanCommentOnPost(member); }); - + it('Can browse all comments of a post', async function () { const {body} = await membersAgent .get(`/api/comments/?filter=post_id:${postId}`) @@ -327,13 +327,13 @@ describe('Comments API', function () { comments: [commentMatcherWithReplies({replies: 1}), commentMatcher] }); }); - + it('Can reply to your own comment', async function () { // Should not update last_seen_at or last_commented_at when both are already set to a value on the same day const timezone = settingsCache.get('timezone'); const date = moment.utc(new Date()).tz(timezone).startOf('day').toDate(); await models.Member.edit({last_seen_at: date, last_commented_at: date}, {id: member.get('id')}); - + const {body} = await membersAgent .post(`/api/comments/`) .body({comments: [{ @@ -349,25 +349,25 @@ describe('Comments API', function () { .matchBodySnapshot({ comments: [commentMatcher] }); - + // Check only the author got an email (because we are the author of this parent comment) mockManager.assert.sentEmailCount(1); mockManager.assert.sentEmail({ subject: '💬 New comment on your post: ' + postTitle, to: fixtureManager.get('users', 0).email }); - + // Wait for the dispatched events (because this happens async) await sleep(200); - + // Check last updated_at is not changed? member = await models.Member.findOne({id: member.id}); should.equal(member.get('last_seen_at').getTime(), date.getTime(), 'The member should not update `last_seen_at` if last seen at is same day'); - + // Check last_commented_at changed? should.equal(member.get('last_commented_at').getTime(), date.getTime(), 'The member should not update `last_commented_at` f last seen at is same day'); }); - + it('Can reply to a comment', async function () { await testCanReply(member); }); @@ -375,7 +375,7 @@ describe('Comments API', function () { let testReplyId; it('Limits returned replies to 3', async function () { const parentId = fixtureManager.get('comments', 0).id; - + // Check initial status: two replies before test await membersAgent .get(`/api/comments/${parentId}/`) @@ -389,7 +389,7 @@ describe('Comments API', function () { .expect(({body}) => { body.comments[0].count.replies.should.eql(2); }); - + // Add some replies for (let index = 0; index < 3; index++) { const {body: reply} = await membersAgent @@ -411,7 +411,7 @@ describe('Comments API', function () { testReplyId = reply.comments[0].id; } } - + // Check if we have count.replies = 4, and replies.length == 3 await membersAgent .get(`/api/comments/${parentId}/`) @@ -446,7 +446,7 @@ describe('Comments API', function () { }); await testCanReply(member, {from: 'support@example.com'}); }); - + it('Can like a comment', async function () { // Check not liked await membersAgent @@ -462,7 +462,7 @@ describe('Comments API', function () { body.comments[0].liked.should.eql(false); body.comments[0].count.likes.should.eql(0); }); - + // Create a temporary comment await membersAgent .post(`/api/comments/${commentId}/like/`) @@ -471,7 +471,7 @@ describe('Comments API', function () { etag: anyEtag }) .expectEmptyBody(); - + // Check liked await membersAgent .get(`/api/comments/${commentId}/`) @@ -487,7 +487,7 @@ describe('Comments API', function () { body.comments[0].count.likes.should.eql(1); }); }); - + it('Cannot like a comment multiple times', async function () { // Create a temporary comment await membersAgent @@ -502,7 +502,7 @@ describe('Comments API', function () { }] }); }); - + it('Can like a reply', async function () { // Check initial status: two replies before test await membersAgent @@ -512,7 +512,7 @@ describe('Comments API', function () { etag: anyEtag }) .expectEmptyBody(); - + // Check liked await membersAgent .get(`/api/comments/${testReplyId}/`) @@ -528,10 +528,10 @@ describe('Comments API', function () { body.comments[0].count.likes.should.eql(1); }); }); - + it('Can return replies', async function () { const parentId = fixtureManager.get('comments', 0).id; - + // Check initial status: two replies before test await membersAgent .get(`/api/comments/${parentId}/replies/`) @@ -546,17 +546,17 @@ describe('Comments API', function () { should(body.comments[0].count.replies).be.undefined(); should(body.meta.pagination.total).eql(7); should(body.meta.pagination.next).eql(null); - + // Check liked + likes working for replies too should(body.comments[2].id).eql(testReplyId); should(body.comments[2].count.likes).eql(1); should(body.comments[2].liked).eql(true); }); }); - + it('Can request last page of replies', async function () { const parentId = fixtureManager.get('comments', 0).id; - + // Check initial status: two replies before test await membersAgent .get(`/api/comments/${parentId}/replies/?page=3&limit=3`) @@ -573,7 +573,7 @@ describe('Comments API', function () { should(body.meta.pagination.next).eql(null); }); }); - + it('Can remove a like (unlike)', async function () { // Unlike await membersAgent @@ -583,7 +583,7 @@ describe('Comments API', function () { etag: anyEtag }) .expectEmptyBody(); - + // Check not liked await membersAgent .get(`/api/comments/${commentId}/`) @@ -599,7 +599,7 @@ describe('Comments API', function () { body.comments[0].count.likes.should.eql(0); }); }); - + it('Cannot unlike a comment if it has not been liked', async function () { // Remove like await membersAgent @@ -614,7 +614,7 @@ describe('Comments API', function () { }] }); }); - + it('Can report a comment', async function () { // Create a temporary comment await membersAgent @@ -624,14 +624,14 @@ describe('Comments API', function () { etag: anyEtag }) .expectEmptyBody(); - + // Check report const reports = await models.CommentReport.findAll({filter: 'comment_id:' + commentId}); reports.models.length.should.eql(1); - + const report = reports.models[0]; report.get('member_id').should.eql(member.id); - + mockManager.assert.sentEmail({ subject: '🚩 A comment has been reported on your post', to: fixtureManager.get('users', 0).email, @@ -639,7 +639,7 @@ describe('Comments API', function () { text: new RegExp(escapeRegExp('This is a message\n\nNew line')) }); }); - + it('Cannot report a comment twice', async function () { // Create a temporary comment await membersAgent @@ -649,17 +649,17 @@ describe('Comments API', function () { etag: anyEtag }) .expectEmptyBody(); - + // Check report should be the same (no extra created) const reports = await models.CommentReport.findAll({filter: 'comment_id:' + commentId}); reports.models.length.should.eql(1); - + const report = reports.models[0]; report.get('member_id').should.eql(member.id); - + mockManager.assert.sentEmailCount(0); }); - + it('Can edit a comment on a post', async function () { const {body} = await await membersAgent .put(`/api/comments/${commentId}`) @@ -676,10 +676,10 @@ describe('Comments API', function () { edited_at: anyISODateTime }] }); - + assert(body.comments[0].edited_at, 'The edited_at field should be populated'); }); - + it('Can not edit a comment post_id', async function () { const anotherPostId = fixtureManager.get('posts', 1).id; await membersAgent @@ -687,13 +687,13 @@ describe('Comments API', function () { .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}`) @@ -711,7 +711,7 @@ describe('Comments API', function () { }] }); }); - + it('Can not edit a comment as a member who is not you', async function () { const memberId = fixtureManager.get('members', 1).id; await membersAgent @@ -720,7 +720,7 @@ describe('Comments API', function () { html: 'Illegal comment update', member_id: memberId }]}); - + const { body: { comments: [ @@ -738,10 +738,10 @@ describe('Comments API', function () { edited_at: anyISODateTime }] }); - + assert(comment.member.id !== memberId); }); - + it('Can not reply to a reply', async function () { const { body: { @@ -755,7 +755,7 @@ describe('Comments API', function () { post_id: postId, html: 'Parent' }]}); - + const { body: { comments: [{ @@ -769,7 +769,7 @@ describe('Comments API', function () { parent_id: parentId, html: 'Reply' }]}); - + await membersAgent .post(`/api/comments/`) .body({comments: [{ @@ -788,7 +788,7 @@ describe('Comments API', function () { }] }); }); - + it('Can not edit a replies parent', async function () { const { body: { @@ -802,7 +802,7 @@ describe('Comments API', function () { post_id: postId, html: 'Parent' }]}); - + const { body: { comments: [{ @@ -815,7 +815,7 @@ describe('Comments API', function () { post_id: postId, html: 'New Parent' }]}); - + const { body: { comments: [{ @@ -829,7 +829,7 @@ describe('Comments API', function () { parent_id: parentId, html: 'Reply' }]}); - + // Attempt to edit the parent await membersAgent .put(`/api/comments/${replyId}/`) @@ -837,29 +837,27 @@ describe('Comments API', function () { 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'); }); - + it('Can fetch counts', async function () { + const ids = [ + fixtureManager.get('posts', 0).id, + fixtureManager.get('posts', 1).id, + fixtureManager.get('posts', 2).id + ]; await membersAgent - .post(`api/comments/counts`) - .body({ - ids: [ - fixtureManager.get('posts', 0).id, - fixtureManager.get('posts', 1).id, - fixtureManager.get('posts', 2).id - ] - }) + .get(`api/comments/counts/?ids=${ids.join(',')}`) .expectStatus(200) .matchHeaderSnapshot({ etag: anyEtag }) .matchBodySnapshot(); }); - + it('Can delete a comment, and it is redacted from', async function () { const { body: { @@ -873,7 +871,7 @@ describe('Comments API', function () { post_id: postId, html: 'Comment to delete' }]}); - + const { body: { comments: [deletedComment] @@ -883,7 +881,7 @@ describe('Comments API', function () { .body({comments: [{ status: 'deleted' }]}); - + assert(!deletedComment.html); }); }); @@ -937,7 +935,7 @@ describe('Comments API', function () { member = await models.Member.findOne({email: 'paid@example.com'}, {require: true}); const product = await getPaidProduct(); - + // Attach comped subscription to this member await models.Member.edit({ status: 'comped', @@ -976,7 +974,7 @@ describe('Comments API', function () { // Only allow members with access to a given post to comment on that post describe('Tier-only posts', function () { let post; - let product; + let product; before(async function () { product = await getPaidProduct(); @@ -1012,7 +1010,7 @@ describe('Comments API', function () { before(async function () { await membersAgent.loginAs('member-premium@example.com'); member = await models.Member.findOne({email: 'member-premium@example.com'}, {require: true}); - + // Attach comped subscription to this member await models.Member.edit({ status: 'comped',