From 0e1fde885ba8e0c7247f5a41eed41d79993c8677 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Thu, 20 Oct 2022 13:09:29 +0200 Subject: [PATCH] Added support for filtering email events by post_id refs https://github.com/TryGhost/Team/issues/2093 --- .../components/gh-member-details-activity.hbs | 4 +- .../app/components/gh-member-details.hbs | 4 +- .../app/components/gh-post-settings-menu.hbs | 6 +- .../app/components/modal-import-members.hbs | 4 +- .../core/server/models/email-recipient.js | 14 ++++ ghost/core/core/server/models/email.js | 6 +- .../__snapshots__/activity-feed.test.js.snap | 84 +++++++++++++++++-- .../test/e2e-api/admin/activity-feed.test.js | 56 +++++++++++-- .../test/utils/fixtures/data-generator.js | 19 ++++- ghost/members-api/lib/repositories/event.js | 9 ++ 10 files changed, 180 insertions(+), 26 deletions(-) diff --git a/ghost/admin/app/components/gh-member-details-activity.hbs b/ghost/admin/app/components/gh-member-details-activity.hbs index d0785a7a82df..d1d61f31d00a 100644 --- a/ghost/admin/app/components/gh-member-details-activity.hbs +++ b/ghost/admin/app/components/gh-member-details-activity.hbs @@ -16,11 +16,11 @@

{{or @member.name @member.email}} - {{#unless (or @member.name @member.email)}} + {{#if (not (or @member.name @member.email))}} {{#if @member.isNew}} New member {{/if}} - {{/unless}} + {{/if}}

{{#if (and @member.name @member.email)}} diff --git a/ghost/admin/app/components/gh-member-details.hbs b/ghost/admin/app/components/gh-member-details.hbs index 178eff4c8be2..42c7d93cd183 100644 --- a/ghost/admin/app/components/gh-member-details.hbs +++ b/ghost/admin/app/components/gh-member-details.hbs @@ -15,11 +15,11 @@

{{or @member.name @member.email}} - {{#unless (or @member.name @member.email)}} + {{#if (not (or @member.name @member.email))}} {{#if @member.isNew}} New member {{/if}} - {{/unless}} + {{/if}}

{{#if (and @member.name @member.email)}} diff --git a/ghost/admin/app/components/gh-post-settings-menu.hbs b/ghost/admin/app/components/gh-post-settings-menu.hbs index 73684aab0e22..2d1938373c54 100644 --- a/ghost/admin/app/components/gh-post-settings-menu.hbs +++ b/ghost/admin/app/components/gh-post-settings-menu.hbs @@ -61,9 +61,9 @@ @disabled={{this.post.isScheduled}} @isActive={{not this.isViewingSubview}} /> - {{#unless (or this.post.isDraft this.post.isPublished this.post.pastScheduledTime this.post.isSent)}} + {{#if (not (or this.post.isDraft this.post.isPublished this.post.pastScheduledTime this.post.isSent))}}

Use the publish menu to re-schedule

- {{/unless}} + {{/if}}
{{#unless this.session.user.isContributor}} @@ -187,7 +187,7 @@
{{! .post-settings-menu }} {{#if this.isViewingSubview}} -
+
{{#if (eq this.subview "meta-data")}}
diff --git a/ghost/admin/app/components/modal-import-members.hbs b/ghost/admin/app/components/modal-import-members.hbs index 99b785fc75a4..aa04f0b22b23 100644 --- a/ghost/admin/app/components/modal-import-members.hbs +++ b/ghost/admin/app/components/modal-import-members.hbs @@ -80,12 +80,12 @@
{{/if}} {{#if this.importResponse.errorCount}} - {{#unless (eq this.importResponse.importedCount 0)}} + {{#if (not (eq this.importResponse.importedCount 0))}}

{{format-number this.importResponse.errorCount}} {{gh-pluralize this.importResponse.errorCount "member" without-count=true}} {{if (eq this.importResponse.errorCount 1) "was" "were"}} skipped due to the following errors:

- {{/unless}} + {{/if}}
    {{#each this.importResponse.errorList as |error|}} diff --git a/ghost/core/core/server/models/email-recipient.js b/ghost/core/core/server/models/email-recipient.js index 894094923eff..46b9e5944309 100644 --- a/ghost/core/core/server/models/email-recipient.js +++ b/ghost/core/core/server/models/email-recipient.js @@ -3,6 +3,20 @@ const ghostBookshelf = require('./base'); const EmailRecipient = ghostBookshelf.Model.extend({ tableName: 'email_recipients', hasTimestamps: false, + + filterRelations: function filterRelations() { + return { + email: { + // Mongo-knex doesn't support belongsTo relations + tableName: 'emails', + tableNameAs: 'email', + type: 'manyToMany', + joinTable: 'email_recipients', + joinFrom: 'id', + joinTo: 'email_id' + } + }; + }, email() { return this.belongsTo('Email', 'email_id'); diff --git a/ghost/core/core/server/models/email.js b/ghost/core/core/server/models/email.js index 0d49e2ccb057..a0788b8fe699 100644 --- a/ghost/core/core/server/models/email.js +++ b/ghost/core/core/server/models/email.js @@ -81,11 +81,7 @@ const Email = ghostBookshelf.Model.extend({ model.emitChange('deleted', options); } -}, { - post() { - return this.belongsTo('Post'); - } -}); +}, {}); const Emails = ghostBookshelf.Collection.extend({ model: Email diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap index 0ab04a66629a..e0e4be361810 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap @@ -35,6 +35,14 @@ Object { "data": Any, "type": Any, }, + Object { + "data": Any, + "type": Any, + }, + Object { + "data": Any, + "type": Any, + }, ], "meta": Object { "pagination": Object { @@ -43,7 +51,7 @@ Object { "page": null, "pages": 1, "prev": null, - "total": 8, + "total": 10, }, }, } @@ -53,7 +61,7 @@ exports[`Activity Feed API Can filter events by post id 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "15074", + "content-length": "17358", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Version, Origin, Accept-Encoding", @@ -78,9 +86,9 @@ Object { "limit": "2", "next": null, "page": null, - "pages": 4, + "pages": 5, "prev": null, - "total": 8, + "total": 10, }, }, } @@ -90,7 +98,7 @@ exports[`Activity Feed API Can limit events 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "1239", + "content-length": "1240", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Version, Origin, Accept-Encoding", @@ -348,6 +356,72 @@ Object { } `; +exports[`Activity Feed API Returns email delivered events in activity feed 1: [body] 1`] = ` +Object { + "events": Array [ + Object { + "data": Any, + "type": Any, + }, + ], + "meta": Object { + "pagination": Object { + "limit": 10, + "next": null, + "page": null, + "pages": 1, + "prev": null, + "total": 1, + }, + }, +} +`; + +exports[`Activity Feed API Returns email delivered events in activity feed 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "1246", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Activity Feed API Returns email opened events in activity feed 1: [body] 1`] = ` +Object { + "events": Array [ + Object { + "data": Any, + "type": Any, + }, + ], + "meta": Object { + "pagination": Object { + "limit": 10, + "next": null, + "page": null, + "pages": 1, + "prev": null, + "total": 1, + }, + }, +} +`; + +exports[`Activity Feed API Returns email opened events in activity feed 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "1243", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Activity Feed API Returns feedback events in activity feed 1: [body] 1`] = ` Object { "events": Array [ diff --git a/ghost/core/test/e2e-api/admin/activity-feed.test.js b/ghost/core/test/e2e-api/admin/activity-feed.test.js index c1607bab6410..1074806c5452 100644 --- a/ghost/core/test/e2e-api/admin/activity-feed.test.js +++ b/ghost/core/test/e2e-api/admin/activity-feed.test.js @@ -1,5 +1,6 @@ const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework'); const {anyEtag, anyObjectId, anyUuid, anyISODate, anyString, anyObject, anyNumber} = matchers; +const models = require('../../../core/server/models'); const assert = require('assert'); @@ -7,7 +8,7 @@ let agent; describe('Activity Feed API', function () { before(async function () { agent = await agentProvider.getAdminAPIAgent(); - await fixtureManager.init('posts', 'newsletters', 'members:newsletters', 'comments', 'redirects', 'clicks', 'feedback'); + await fixtureManager.init('posts', 'newsletters', 'members:newsletters', 'comments', 'redirects', 'clicks', 'feedback', 'members:emails'); await agent.loginAsOwner(); }); @@ -125,8 +126,49 @@ describe('Activity Feed API', function () { }); }); + it('Returns email delivered events in activity feed', async function () { + // Check activity feed + await agent + .get(`/members/events?filter=type:email_delivered_event`) + .expectStatus(200) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + events: new Array(1).fill({ + type: anyString, + data: anyObject + }) + }) + .expect(({body}) => { + assert(body.events.find(e => e.type === 'email_delivered_event'), 'Expected an email delivered event'); + assert(!body.events.find(e => e.type !== 'email_delivered_event'), 'Expected only email delivered events'); + }); + }); + + it('Returns email opened events in activity feed', async function () { + // Check activity feed + await agent + .get(`/members/events?filter=type:email_opened_event`) + .expectStatus(200) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + events: new Array(1).fill({ + type: anyString, + data: anyObject + }) + }) + .expect(({body}) => { + assert(body.events.find(e => e.type === 'email_opened_event'), 'Expected an email opened event'); + assert(!body.events.find(e => e.type !== 'email_opened_event'), 'Expected only email opened events'); + }); + }); + it('Can filter events by post id', async function () { const postId = fixtureManager.get('posts', 0).id; + await agent .get(`/members/events?filter=data.post_id:${postId}`) .expectStatus(200) @@ -134,13 +176,13 @@ describe('Activity Feed API', function () { etag: anyEtag }) .matchBodySnapshot({ - events: new Array(8).fill({ + events: new Array(10).fill({ type: anyString, data: anyObject }) }) .expect(({body}) => { - assert(!body.events.find(e => (e.data?.post?.id ?? e.data?.attribution?.id) !== postId), 'Should only return events for the post'); + assert(!body.events.find(e => (e.data?.post?.id ?? e.data?.attribution?.id ?? e.data?.email?.post_id) !== postId), 'Should only return events for the post'); // Check all post_id event types are covered by this test assert(body.events.find(e => e.type === 'click_event'), 'Expected a click event'); @@ -148,9 +190,11 @@ describe('Activity Feed API', function () { assert(body.events.find(e => e.type === 'feedback_event'), 'Expected a feedback event'); assert(body.events.find(e => e.type === 'signup_event'), 'Expected a signup event'); assert(body.events.find(e => e.type === 'subscription_event'), 'Expected a subscription event'); + assert(body.events.find(e => e.type === 'email_delivered_event'), 'Expected an email delivered event'); + assert(body.events.find(e => e.type === 'email_opened_event'), 'Expected an email opened event'); // Assert total is correct - assert.equal(body.meta.pagination.total, 8); + assert.equal(body.meta.pagination.total, 10); }); }); @@ -169,10 +213,10 @@ describe('Activity Feed API', function () { }) }) .expect(({body}) => { - assert(!body.events.find(e => (e.data?.post?.id ?? e.data?.attribution?.id) !== postId), 'Should only return events for the post'); + assert(!body.events.find(e => (e.data?.post?.id ?? e.data?.attribution?.id ?? e.data?.email?.post_id) !== postId), 'Should only return events for the post'); // Assert total is correct - assert.equal(body.meta.pagination.total, 8); + assert.equal(body.meta.pagination.total, 10); }); }); }); diff --git a/ghost/core/test/utils/fixtures/data-generator.js b/ghost/core/test/utils/fixtures/data-generator.js index ae7daa0e9615..6a527ac92af0 100644 --- a/ghost/core/test/utils/fixtures/data-generator.js +++ b/ghost/core/test/utils/fixtures/data-generator.js @@ -785,6 +785,19 @@ DataGenerator.Content = { member_uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b343', member_email: 'member1@test.com', member_name: 'Mr Egg' + }, + { + id: ObjectId().toHexString(), + email_id: null, // emails[0] relation added later + member_id: null, // members[4] relation added later + batch_id: null, // email_batches[0] relation added later + processed_at: moment().toDate(), + delivered_at: moment().toDate(), + opened_at: moment().toDate(), + failed_at: null, + member_uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b344', + member_email: 'member4@test.com', + member_name: 'Mr Egg' } ], @@ -850,6 +863,9 @@ DataGenerator.Content.email_recipients[2].member_id = DataGenerator.Content.memb DataGenerator.Content.email_recipients[3].batch_id = DataGenerator.Content.email_batches[0].id; DataGenerator.Content.email_recipients[3].email_id = DataGenerator.Content.email_batches[0].email_id; DataGenerator.Content.email_recipients[3].member_id = DataGenerator.Content.members[3].id; +DataGenerator.Content.email_recipients[4].batch_id = DataGenerator.Content.email_batches[0].id; +DataGenerator.Content.email_recipients[4].email_id = DataGenerator.Content.email_batches[0].email_id; +DataGenerator.Content.email_recipients[4].member_id = DataGenerator.Content.members[4].id; DataGenerator.Content.members_stripe_customers[0].member_id = DataGenerator.Content.members[2].id; DataGenerator.Content.members_stripe_customers[1].member_id = DataGenerator.Content.members[3].id; DataGenerator.Content.members_stripe_customers[2].member_id = DataGenerator.Content.members[4].id; @@ -1533,7 +1549,8 @@ DataGenerator.forKnex = (function () { createEmailRecipient(DataGenerator.Content.email_recipients[0]), createEmailRecipient(DataGenerator.Content.email_recipients[1]), createEmailRecipient(DataGenerator.Content.email_recipients[2]), - createEmailRecipient(DataGenerator.Content.email_recipients[3]) + createEmailRecipient(DataGenerator.Content.email_recipients[3]), + createEmailRecipient(DataGenerator.Content.email_recipients[4]) ]; const members = [ diff --git a/ghost/members-api/lib/repositories/event.js b/ghost/members-api/lib/repositories/event.js index d341ffa98d89..52c9c28bfcf3 100644 --- a/ghost/members-api/lib/repositories/event.js +++ b/ghost/members-api/lib/repositories/event.js @@ -443,6 +443,9 @@ module.exports = class EventRepository { if (filters['data.member_id']) { options.filter.push(filters['data.member_id'].replace(/data.member_id:/g, 'member_id:')); } + if (filters['data.post_id']) { + options.filter.push(filters['data.post_id'].replace(/data.post_id:/g, 'email.post_id:')); + } options.filter = options.filter.join('+'); options.order = options.order.replace(/created_at/g, 'delivered_at'); @@ -480,6 +483,9 @@ module.exports = class EventRepository { if (filters['data.member_id']) { options.filter.push(filters['data.member_id'].replace(/data.member_id:/g, 'member_id:')); } + if (filters['data.post_id']) { + options.filter.push(filters['data.post_id'].replace(/data.post_id:/g, 'email.post_id:')); + } options.filter = options.filter.join('+'); options.order = options.order.replace(/created_at/g, 'opened_at'); @@ -517,6 +523,9 @@ module.exports = class EventRepository { if (filters['data.member_id']) { options.filter.push(filters['data.member_id'].replace(/data.member_id:/g, 'member_id:')); } + if (filters['data.post_id']) { + options.filter.push(filters['data.post_id'].replace(/data.post_id:/g, 'email.post_id:')); + } options.filter = options.filter.join('+'); options.order = options.order.replace(/created_at/g, 'failed_at');