From 6140a98351c23ba028a3fc3fb4486fd969d5fac8 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Fri, 6 Nov 2020 17:32:23 +0000 Subject: [PATCH] Updated newsletter functionality to use `email_recipient_filter` (#12343) no-issue * Used email_recipient_filter in MEGA This officially decouples the newsletter recipients from the post visibility allowing us to send emails to free members only * Supported enum for send_email_when_published in model This allows us to migrate from the previously used boolean to an enum when we eventually rename the email_recipient_filter column to send_email_when_published * Updated the posts API to handle email_recipient_filter We now no longer rely on the send_email_when_published property to send newsletters, meaning we can remove the column and start cleaning up the new columns name * Handled draft status changes when emails not sent We want to reset any concept of sending an email when a post is transition to the draft status, if and only if, and email has not already been sent. If an email has been sent, we should leave the email related fields as they were. * Removed send_email_when_published from add method This is not supported at the model layer * Removed email_recipient_filter from v2&Content API This should not be exposed on previous api versions, or publicly * Removed reference to send_email_when_published This allows us to move completely to the email_recipient_filter property, keeping the code clean and allowing us to delete the send_email_when_published column in the database. We plan to then migrate _back_ to the send_email_when_published name at both the database and api level. --- core/server/api/canary/posts.js | 12 ++++--- .../utils/serializers/output/utils/mapper.js | 1 + .../utils/serializers/output/utils/mapper.js | 1 + core/server/models/post.js | 20 ++++++------ core/server/services/mega/mega.js | 32 ++++++++++++++++--- test/api-acceptance/admin/utils.js | 1 + test/regression/api/v2/admin/utils.js | 1 + test/regression/api/v2/content/utils.js | 1 + 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/core/server/api/canary/posts.js b/core/server/api/canary/posts.js index 2d198d9d5d..890e074fee 100644 --- a/core/server/api/canary/posts.js +++ b/core/server/api/canary/posts.js @@ -88,8 +88,7 @@ module.exports = { options: [ 'include', 'formats', - 'source', - 'send_email_when_published' + 'source' ], validation: { options: { @@ -125,7 +124,7 @@ module.exports = { 'id', 'formats', 'source', - 'send_email_when_published', + 'email_recipient_filter', 'force_rerender', // NOTE: only for internal context 'forUpdate', @@ -141,6 +140,9 @@ module.exports = { }, source: { values: ['html'] + }, + email_recipient_filter: { + values: ['none', 'free', 'paid', 'all'] } } }, @@ -149,14 +151,14 @@ module.exports = { }, async query(frame) { /**Check host limits for members when send email is true**/ - if (frame.options.send_email_when_published) { + if (frame.options.email_recipient_filter && frame.options.email_recipient_filter !== 'none') { await membersService.checkHostLimit(); } let model = await models.Post.edit(frame.data.posts[0], frame.options); /**Handle newsletter email */ - if (model.get('send_email_when_published')) { + if (model.get('email_recipient_filter') !== 'none') { const postPublished = model.wasChanged() && (model.get('status') === 'published') && (model.previous('status') !== 'published'); if (postPublished) { let postEmail = model.relations.email; diff --git a/core/server/api/canary/utils/serializers/output/utils/mapper.js b/core/server/api/canary/utils/serializers/output/utils/mapper.js index dfb2fb7b2f..d6deaac5fb 100644 --- a/core/server/api/canary/utils/serializers/output/utils/mapper.js +++ b/core/server/api/canary/utils/serializers/output/utils/mapper.js @@ -89,6 +89,7 @@ const mapPage = (model, frame) => { delete jsonModel.email_subject; delete jsonModel.send_email_when_published; + delete jsonModel.email_recipient_filter; return jsonModel; }; diff --git a/core/server/api/v2/utils/serializers/output/utils/mapper.js b/core/server/api/v2/utils/serializers/output/utils/mapper.js index 9eb8b034e1..aaea5d13ee 100644 --- a/core/server/api/v2/utils/serializers/output/utils/mapper.js +++ b/core/server/api/v2/utils/serializers/output/utils/mapper.js @@ -76,6 +76,7 @@ const mapPost = (model, frame) => { delete jsonModel.posts_meta; delete jsonModel.send_email_when_published; + delete jsonModel.email_recipient_filter; delete jsonModel.email_subject; return jsonModel; diff --git a/core/server/models/post.js b/core/server/models/post.js index 58bc1cb321..dea20aba53 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -505,19 +505,21 @@ Post = ghostBookshelf.Model.extend({ } } - // send_email_when_published is read-only and should only be set using a query param when publishing/scheduling - if (options.send_email_when_published && this.hasChanged('status') && (newStatus === 'published' || newStatus === 'scheduled')) { - this.set('send_email_when_published', true); + // email_recipient_filter is read-only and should only be set using a query param when publishing/scheduling + if (this.hasChanged('status') && (newStatus === 'published' || newStatus === 'scheduled')) { + if (typeof options.email_recipient_filter === 'undefined') { + this.set('email_recipient_filter', 'none'); + } else { + this.set('email_recipient_filter', options.email_recipient_filter); + } } - // ensure draft posts have the send_email_when_published reset unless an email has already been sent + // ensure draft posts have the email_recipient_filter reset unless an email has already been sent if (newStatus === 'draft' && this.hasChanged('status')) { ops.push(function ensureSendEmailWhenPublishedIsUnchanged() { return self.related('email').fetch({transacting: options.transacting}).then((email) => { - if (email) { - self.set('send_email_when_published', true); - } else { - self.set('send_email_when_published', false); + if (!email) { + self.set('email_recipient_filter', 'none'); } }); }); @@ -843,7 +845,7 @@ Post = ghostBookshelf.Model.extend({ findPage: ['status'], findAll: ['columns', 'filter'], destroy: ['destroyAll', 'destroyBy'], - edit: ['filter', 'send_email_when_published', 'force_rerender'] + edit: ['filter', 'email_recipient_filter', 'force_rerender'] }; // The post model additionally supports having a formats option diff --git a/core/server/services/mega/mega.js b/core/server/services/mega/mega.js index c28d27231c..bf831459b5 100644 --- a/core/server/services/mega/mega.js +++ b/core/server/services/mega/mega.js @@ -95,8 +95,21 @@ const addEmail = async (postModel, options) => { const knexOptions = _.pick(options, ['transacting', 'forUpdate']); const filterOptions = Object.assign({}, knexOptions, {filter: 'subscribed:true', limit: 1}); - if (postModel.get('visibility') === 'paid') { + const emailRecipientFilter = postModel.get('email_recipient_filter'); + + switch (emailRecipientFilter) { + case 'paid': filterOptions.paid = true; + break; + case 'free': + filterOptions.paid = false; + break; + case 'all': + break; + case 'none': + throw new Error('Cannot sent email to "none" email_recipient_filter'); + default: + throw new Error(`Unknown email_recipient_filter ${emailRecipientFilter}`); } const startRetrieve = Date.now(); @@ -127,7 +140,8 @@ const addEmail = async (postModel, options) => { html: emailData.html, plaintext: emailData.plaintext, submitted_at: moment().toDate(), - track_opens: config.get('enableDeveloperExperiments') + track_opens: config.get('enableDeveloperExperiments'), + recipient_filter: emailRecipientFilter }, knexOptions); } else { return existing; @@ -264,13 +278,23 @@ async function sendEmailJob({emailModel, options}) { // instantiations and associated processing and event loop blocking async function getEmailMemberRows({emailModel, options}) { const knexOptions = _.pick(options, ['transacting', 'forUpdate']); - const postModel = await models.Post.findOne({id: emailModel.get('post_id')}, knexOptions); // TODO: this will clobber a user-assigned filter if/when we allow emails to be sent to filtered member lists const filterOptions = Object.assign({}, knexOptions, {filter: 'subscribed:true'}); - if (postModel.get('visibility') === 'paid') { + const recipientFilter = emailModel.get('recipient_filter'); + + switch (recipientFilter) { + case 'paid': filterOptions.paid = true; + break; + case 'free': + filterOptions.paid = false; + break; + case 'all': + break; + default: + throw new Error(`Unknown recipient_filter ${recipientFilter}`); } const startRetrieve = Date.now(); diff --git a/test/api-acceptance/admin/utils.js b/test/api-acceptance/admin/utils.js index 8c2d607788..b777c14a73 100644 --- a/test/api-acceptance/admin/utils.js +++ b/test/api-acceptance/admin/utils.js @@ -58,6 +58,7 @@ const expectedProperties = { .without('author_id', 'author') // pages are not sent as emails .without('send_email_when_published') + .without('email_recipient_filter') // always returns computed properties .concat('url', 'primary_tag', 'primary_author', 'excerpt') // returned by default diff --git a/test/regression/api/v2/admin/utils.js b/test/regression/api/v2/admin/utils.js index 39d5b6b790..36049916ae 100644 --- a/test/regression/api/v2/admin/utils.js +++ b/test/regression/api/v2/admin/utils.js @@ -28,6 +28,7 @@ const expectedProperties = { .without('author_id', 'author') // emails are not supported in API v2 .without('send_email_when_published') + .without('email_recipient_filter') // always returns computed properties .concat('url', 'primary_tag', 'primary_author', 'excerpt') .concat('authors', 'tags') diff --git a/test/regression/api/v2/content/utils.js b/test/regression/api/v2/content/utils.js index e42d30797b..915a78ff65 100644 --- a/test/regression/api/v2/content/utils.js +++ b/test/regression/api/v2/content/utils.js @@ -23,6 +23,7 @@ const expectedProperties = { .without('locale', 'visibility') // emails are not supported in API v2 .without('send_email_when_published') + .without('email_recipient_filter') // These fields aren't useful as they always have known values .without('status') .concat('page')