TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-27 00:52:36 +03:00
Code Issues Projects Releases Wiki Activity

Added members.last_commented_at and last_seen_at update when commenting (#15088)

Browse Source 
refs https://github.com/TryGhost/Team/issues/1717

- Updates last_commented_at and last_seen_at (only once a day)
- Used the LastSeenAtUpdater, so we can combine updating last_commented_at and last_seen_at in one query + used same pattern
- Updated comments service to await emails in order to make E2E tests more stable (as we don't have any method to await emails and test emails otherwise). This removed the email sending logic from the `onCreated` hook of the model.
This commit is contained in:
Simon Backx 2022-07-25 17:35:46 +02:00 committed by GitHub
parent 57a743e3aa
commit 31a4135fec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 235 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ghost/core/core/server/models/comment.js
View File

@ -2,7 +2,6 @@ const ghostBookshelf = require('./base');
const _ = require('lodash');
const errors = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');
const commentsService = require('../services/comments');
const messages = {
commentNotFound: 'Comment could not be found',
@ -70,10 +69,6 @@ const Comment = ghostBookshelf.Model.extend({
onCreated: function onCreated(model, options) {
ghostBookshelf.Model.prototype.onCreated.apply(this, arguments);
if (!options.context.internal) {
commentsService.api.sendNewCommentNotifications(model);
}
model.emitChange('added', options);
},

4
ghost/core/core/server/services/comments/emails.js
View File

@ -49,7 +49,7 @@ class CommentsServiceEmails {
const {html, text} = await this.renderEmailTemplate('new-comment', templateData);
this.sendMail({
await this.sendMail({
to,
subject,
html,
@ -148,7 +148,7 @@ class CommentsServiceEmails {
const {html, text} = await this.renderEmailTemplate('report', templateData);
this.sendMail({
await this.sendMail({
to,
subject,
html,

26
ghost/core/core/server/services/comments/service.js
View File

@ -1,5 +1,7 @@
const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const {MemberCommentEvent} = require('@tryghost/member-events');
const DomainEvents = require('@tryghost/domain-events');
const messages = {
commentNotFound: 'Comment could not be found',
@ -24,10 +26,10 @@ class CommentsService {
}
async sendNewCommentNotifications(comment) {
this.emails.notifyPostAuthors(comment);
await this.emails.notifyPostAuthors(comment);
if (comment.get('parent_id')) {
this.emails.notifyParentCommentAuthor(comment);
await this.emails.notifyParentCommentAuthor(comment);
}
}
@ -92,6 +94,16 @@ class CommentsService {
status: 'published'
}, options);
if (!options.context.internal) {
await this.sendNewCommentNotifications(model);
}
DomainEvents.dispatch(MemberCommentEvent.create({
memberId: member,
postId: post,
commentId: model.id
}));
return model;
}
@ -130,6 +142,16 @@ class CommentsService {
status: 'published'
}, options);
if (!options.context.internal) {
await this.sendNewCommentNotifications(model);
}
DomainEvents.dispatch(MemberCommentEvent.create({
memberId: member,
postId: parentComment.get('post_id'),
commentId: model.id
}));
return model;
}

50
ghost/core/test/e2e-api/members-comments/comments.test.js
View File

@ -3,6 +3,8 @@ const {agentProvider, mockManager, fixtureManager, matchers} = require('../../ut
const {anyEtag, anyObjectId, anyLocationFor, anyISODateTime, anyUuid, anyNumber, anyBoolean} = matchers;
const should = require('should');
const models = require('../../../core/server/models');
const moment = require('moment-timezone');
const settingsCache = require('../../../core/shared/settings-cache');
let membersAgent, membersAgent2, member, postId, commentId;
@ -101,6 +103,8 @@ describe('Comments API', function () {
});
it('Can comment on a post', async function () {
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: [{
@ -118,9 +122,6 @@ 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({
@ -129,6 +130,16 @@ describe('Comments API', function () {
// Note that the <strong> tag is removed by the sanitizer
html: new RegExp(escapeRegExp('<p>This is a message</p><p>New line</p>'))
});
// Wait for the dispatched events (because this happens async)
await sleep(200);
// Check last_updated_at changed?
member = await models.Member.findOne({id: member.id});
should.notEqual(member.get('last_seen_at'), null, 'The member should have a `last_seen_at` property after posting a comment.');
// Check last_commented_at changed?
should.notEqual(member.get('last_commented_at'), null, 'The member should have a `last_commented_at` property after posting a comment.');
});
it('Can browse all comments of a post', async function () {
@ -144,6 +155,11 @@ describe('Comments API', function () {
});
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: [{
@ -160,18 +176,28 @@ describe('Comments API', function () {
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
});
// 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 () {
const date = new Date(0);
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: [{
@ -188,8 +214,6 @@ describe('Comments API', function () {
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',
@ -200,6 +224,16 @@ describe('Comments API', function () {
subject: '💬 You have a new reply on one of your comments',
to: fixtureManager.get('members', 0).email
});
// Wait for the dispatched events (because this happens async)
await sleep(250);
// Check last_updated_at changed?
member = await models.Member.findOne({id: member.id});
should.notEqual(member.get('last_seen_at').getTime(), date.getTime(), 'Should update `last_seen_at` property after posting a comment.');
// Check last_commented_at changed?
should.notEqual(member.get('last_commented_at').getTime(), date.getTime(), 'Should update `last_commented_at` property after posting a comment.');
});
it('Can like a comment', async function () {

3
ghost/member-events/index.js
View File

@ -6,5 +6,6 @@ module.exports = {
MemberPaidConverstionEvent: require('./lib/MemberPaidConversionEvent'),
MemberPaidCancellationEvent: require('./lib/MemberPaidCancellationEvent'),
MemberPageViewEvent: require('./lib/MemberPageViewEvent'),
SubscriptionCreatedEvent: require('./lib/SubscriptionCreatedEvent')
SubscriptionCreatedEvent: require('./lib/SubscriptionCreatedEvent'),
MemberCommentEvent: require('./lib/MemberCommentEvent')
};

28
ghost/member-events/lib/MemberCommentEvent.js Normal file
View File

@ -0,0 +1,28 @@
/**
* @typedef {object} MemberCommentEventData
* @prop {string} memberId
* @prop {string} commentId
* @prop {string} postId
*/
/**
* Server-side event firing on page views (page, post, tags...)
*/
module.exports = class MemberCommentEvent {
/**
* @param {MemberCommentEventData} data
* @param {Date} timestamp
*/
constructor(data, timestamp) {
this.data = data;
this.timestamp = timestamp;
}
/**
* @param {MemberCommentEventData} data
* @param {Date} [timestamp]
*/
static create(data, timestamp) {
return new MemberCommentEvent(data, timestamp || new Date);
}
};

3
ghost/members-api/lib/repositories/member.js
View File

@ -318,7 +318,8 @@ module.exports = class MemberRepository {
'products',
'newsletters',
'enable_comment_notifications',
'last_seen_at'
'last_seen_at',
'last_commented_at'
]);
// Determine if we need to fetch the initial member with relations

32
ghost/members-events-service/lib/last-seen-at-updater.js
View File

@ -1,4 +1,4 @@
const {MemberPageViewEvent} = require('@tryghost/member-events');
const {MemberPageViewEvent, MemberCommentEvent} = require('@tryghost/member-events');
const moment = require('moment-timezone');
const {IncorrectUsageError} = require('@tryghost/errors');
@ -32,6 +32,10 @@ class LastSeenAtUpdater {
this._domainEventsService.subscribe(MemberPageViewEvent, async (event) => {
await this.updateLastSeenAt(event.data.memberId, event.data.memberLastSeenAt, event.timestamp);
});
this._domainEventsService.subscribe(MemberCommentEvent, async (event) => {
await this.updateLastCommentedAt(event.data.memberId, event.timestamp);
});
}
/**
@ -54,6 +58,32 @@ class LastSeenAtUpdater {
});
}
}
/**
* Updates the member.last_seen_at field if it wasn't updated in the current day yet (in the publication timezone)
* Example: current time is 2022-02-28 18:00:00
* - memberLastSeenAt is 2022-02-27 23:00:00, timestamp is current time, then `last_seen_at` is set to the current time
* - memberLastSeenAt is 2022-02-28 01:00:00, timestamp is current time, then `last_seen_at` isn't changed
* @param {string} memberId The id of the member to be udpated
* @param {Date} timestamp The event timestamp
*/
async updateLastCommentedAt(memberId, timestamp) {
const membersApi = await this._getMembersApi();
const member = await membersApi.members.get({id: memberId}, {require: true});
const timezone = this._settingsCacheService.get('timezone');
const memberLastSeenAt = member.get('last_seen_at');
const memberLastCommentedAt = member.get('last_commented_at');
if (memberLastSeenAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastSeenAt) || memberLastCommentedAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastCommentedAt)) {
await membersApi.members.update({
last_seen_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss'),
last_commented_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss')
}, {
id: memberId
});
}
}
}
module.exports = LastSeenAtUpdater;

105
ghost/members-events-service/test/last-seen-at-updater.test.js
View File

@ -6,7 +6,7 @@ const assert = require('assert');
const sinon = require('sinon');
const {LastSeenAtUpdater} = require('../');
const DomainEvents = require('@tryghost/domain-events');
const {MemberPageViewEvent} = require('@tryghost/member-events');
const {MemberPageViewEvent, MemberCommentEvent} = require('@tryghost/member-events');
const moment = require('moment');
describe('LastSeenAtUpdater', function () {
@ -35,6 +35,30 @@ describe('LastSeenAtUpdater', function () {
assert(updater.updateLastSeenAt.calledOnceWithExactly('1', previousLastSeen, now.toDate()));
});
it('Calls updateLastCommentedAt on MemberCommentEvents', async function () {
const now = moment('2022-02-28T18:00:00Z').utc();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Etc/UTC');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
},
async getMembersApi() {
return {
members: {
update: stub
}
};
}
});
sinon.stub(updater, 'updateLastCommentedAt');
DomainEvents.dispatch(MemberCommentEvent.create({memberId: '1'}, now.toDate()));
assert(updater.updateLastCommentedAt.calledOnceWithExactly('1', now.toDate()));
});
it('works correctly on another timezone (not updating last_seen_at)', async function () {
const now = moment('2022-02-28T04:00:00Z').utc();
const previousLastSeen = moment('2022-02-27T20:00:00Z').toISOString();
@ -59,6 +83,38 @@ describe('LastSeenAtUpdater', function () {
assert(stub.notCalled, 'The LastSeenAtUpdater should attempt a member update when the new timestamp is within the same day in the publication timezone.');
});
it('works correctly on another timezone (not updating last_commented_at)', async function () {
const now = moment('2022-02-28T04:00:00Z').utc();
const previousLastSeen = moment('2022-02-27T20:00:00Z').toISOString();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Asia/Bangkok');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
},
async getMembersApi() {
return {
members: {
update: stub,
get: () => {
return {
id: '1',
get: () => {
return previousLastSeen;
}
};
}
}
};
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.notCalled, 'The LastSeenAtUpdater should attempt a member update when the new timestamp is within the same day in the publication timezone.');
});
it('works correctly on another timezone (updating last_seen_at)', async function () {
const now = moment('2022-02-28T04:00:00Z').utc();
const previousLastSeen = moment('2022-02-27T20:00:00Z').toISOString();
@ -111,6 +167,38 @@ describe('LastSeenAtUpdater', function () {
assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member when the previous last_seen_at is close to the event timestamp.');
});
it('Doesn\'t update when last_commented_at is too recent', async function () {
const now = moment('2022-02-28T18:00:00Z');
const previousLastSeen = moment('2022-02-28T00:00:00Z').toISOString();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Etc/UTC');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
},
async getMembersApi() {
return {
members: {
update: stub,
get: () => {
return {
id: '1',
get: () => {
return previousLastSeen;
}
};
}
}
};
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member when the previous last_seen_at is close to the event timestamp.');
});
it('Doesn\'t fire on other events', async function () {
const now = moment('2022-02-28T18:00:00Z');
const stub = sinon.stub().resolves();
@ -133,4 +221,19 @@ describe('LastSeenAtUpdater', function () {
await updater.updateLastSeenAt('1', undefined, now.toDate());
assert(stub.notCalled, 'The LastSeenAtUpdater should never fire on MemberPageViewEvent events.');
});
it('throws if getMembersApi is not passed to LastSeenAtUpdater', async function () {
const settingsCache = sinon.stub().returns('Asia/Bangkok');
should.throws(() => {
new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
});
}, 'Missing option getMembersApi');
});
});