Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00
Code Issues Activity

✨ Added ability to send a newsletter to members with a certain label or product (#12932)

Browse source 
refs https://github.com/TryGhost/Team/issues/581
refs https://github.com/TryGhost/Team/issues/582

When publishing a post via the API it was possible to send it using `?email_recipient_filter=all/free/paid` which allowed you to send to members only based on their payment status which is quite limiting for some sites.

This PR updates the `?email_recipient_filter` query param to support Ghost's `?filter` param syntax which enables more specific recipient lists, eg:

`?email_recipient_filter=status:free` = free members only
`?email_recipient_filter=status:paid` = paid members only
`?email_recipient_filter=label:vip` = members that have the `vip` label attached
`?email_recipient_filter=status:paid,label:vip` = paid members and members that have the `vip` label attached

The older `free/paid` values are still supported by the API for backwards compatibility.

- updates `Post` and `Email` models to transform legacy `free` and `paid` values to their NQL equivalents on read/write
  - lets us not worry about supporting legacy values elsewhere in the code
  - cleanup migration to transform all rows slated for 5.0
- removes schema and API `isIn` validations for recipient filters so allow free-form filters
- updates posts API input serializers to transform `free` and `paid` values in the `?email_recipient_filter` param to their NQL equivalents for backwards compatibility
- updates Post API controllers `edit` methods to run a query using the supplied filter to verify that it's valid
- updates `mega` service to use the filter directly when selecting recipients
This commit is contained in:
Kevin Ansfield 2021-05-07 11:56:41 +01:00 • committed by GitHub
parent 1ee97ccfbc
commit 322664a145
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 351 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
core/server/api/canary/posts.js
View file

@ -3,6 +3,7 @@ const i18n = require('../../../shared/i18n');
const errors = require('@tryghost/errors');
const urlUtils = require('../../../shared/url-utils');
const {mega} = require('../../services/mega');
const {BadRequestError} = require('@tryghost/errors');
const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email'];
const unsafeAttrs = ['status', 'authors', 'visibility'];
@ -141,9 +142,6 @@ module.exports = {
source: {
values: ['html']
},
email_recipient_filter: {
values: ['none', 'free', 'paid', 'all']
},
send_email_when_published: {
values: [true, false]
}
@ -183,7 +181,20 @@ module.exports = {
}
/**Handle newsletter email */
if (model.get('email_recipient_filter') !== 'none') {
const emailRecipientFilter = model.get('email_recipient_filter');
if (emailRecipientFilter !== 'none') {
if (emailRecipientFilter !== 'all') {
// check filter is valid
try {
await models.Member.findPage({filter: `subscribed:true+${emailRecipientFilter}`, limit: 1});
} catch (err) {
return Promise.reject(new BadRequestError({
message: i18n.t('errors.api.posts.invalidEmailRecipientFilter'),
context: err.message
}));
}
}
const postPublished = model.wasChanged() && (model.get('status') === 'published') && (model.previous('status') !== 'published');
if (postPublished) {
let postEmail = model.relations.email;

11
core/server/api/canary/utils/serializers/input/posts.js
View file

@ -102,6 +102,15 @@ const forceStatusFilter = (frame) => {
}
};
const transformLegacyEmailRecipientFilters = (frame) => {
if (frame.options.email_recipient_filter === 'free') {
frame.options.email_recipient_filter = 'status:free';
}
if (frame.options.email_recipient_filter === 'paid') {
frame.options.email_recipient_filter = 'status:-free';
}
};
module.exports = {
browse(apiConfig, frame) {
debug('browse');
@ -196,6 +205,7 @@ module.exports = {
});
}
transformLegacyEmailRecipientFilters(frame);
handlePostsMeta(frame);
defaultFormat(frame);
defaultRelations(frame);
@ -205,6 +215,7 @@ module.exports = {
debug('edit');
this.add(apiConfig, frame, {add: false});
transformLegacyEmailRecipientFilters(frame);
handlePostsMeta(frame);
forceStatusFilter(frame);
forcePageFilter(frame);

19
core/server/api/v3/posts.js
View file

@ -3,6 +3,7 @@ const i18n = require('../../../shared/i18n');
const errors = require('@tryghost/errors');
const urlUtils = require('../../../shared/url-utils');
const {mega} = require('../../services/mega');
const {BadRequestError} = require('@tryghost/errors');
const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email'];
const unsafeAttrs = ['status', 'authors', 'visibility'];
@ -141,9 +142,6 @@ module.exports = {
source: {
values: ['html']
},
email_recipient_filter: {
values: ['none', 'free', 'paid', 'all']
},
send_email_when_published: {
values: [true, false]
}
@ -183,7 +181,20 @@ module.exports = {
}
/**Handle newsletter email */
if (model.get('email_recipient_filter') !== 'none') {
const emailRecipientFilter = model.get('email_recipient_filter');
if (emailRecipientFilter !== 'none') {
if (emailRecipientFilter !== 'all') {
// check filter is valid
try {
await models.Member.findPage({filter: `subscribed:true+${emailRecipientFilter}`, limit: 1});
} catch (err) {
return Promise.reject(new BadRequestError({
message: i18n.t('errors.api.posts.invalidEmailRecipientFilter'),
context: err.message
}));
}
}
const postPublished = model.wasChanged() && (model.get('status') === 'published') && (model.previous('status') !== 'published');
if (postPublished) {
let postEmail = model.relations.email;

11
core/server/api/v3/utils/serializers/input/posts.js
View file

@ -102,6 +102,15 @@ const forceStatusFilter = (frame) => {
}
};
const transformLegacyEmailRecipientFilters = (frame) => {
if (frame.options.email_recipient_filter === 'free') {
frame.options.email_recipient_filter = 'status:free';
}
if (frame.options.email_recipient_filter === 'paid') {
frame.options.email_recipient_filter = 'status:-free';
}
};
module.exports = {
browse(apiConfig, frame) {
debug('browse');
@ -204,6 +213,7 @@ module.exports = {
});
}
transformLegacyEmailRecipientFilters(frame);
handlePostsMeta(frame);
defaultFormat(frame);
defaultRelations(frame);
@ -213,6 +223,7 @@ module.exports = {
debug('edit');
this.add(apiConfig, frame, {add: false});
transformLegacyEmailRecipientFilters(frame);
handlePostsMeta(frame);
forceStatusFilter(frame);
forcePageFilter(frame);

6
core/server/data/schema/schema.js
View file

@ -34,8 +34,7 @@ module.exports = {
type: 'string',
maxlength: 50,
nullable: false,
defaultTo: 'none',
validations: {isIn: [['none', 'all', 'free', 'paid']]}
defaultTo: 'none'
},
/**
* @deprecated: single authors was superceded by multiple authors in Ghost 1.22.0
@ -529,8 +528,7 @@ module.exports = {
type: 'string',
maxlength: 50,
nullable: false,
defaultTo: 'paid',
validations: {isIn: [['all', 'free', 'paid']]}
defaultTo: 'status:-free'
},
error: {type: 'string', maxlength: 2000, nullable: true},
error_data: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},

30
core/server/models/email.js
View file

@ -8,7 +8,7 @@ const Email = ghostBookshelf.Model.extend({
return {
uuid: uuid.v4(),
status: 'pending',
recipient_filter: 'paid',
recipient_filter: 'status:-free',
track_opens: false,
delivered_count: 0,
opened_count: 0,
@ -16,12 +16,40 @@ const Email = ghostBookshelf.Model.extend({
};
},
parse() {
const attrs = ghostBookshelf.Model.prototype.parse.apply(this, arguments);
// update legacy recipient_filter values to proper NQL
if (attrs.recipient_filter === 'free') {
attrs.recipient_filter = 'status:free';
}
if (attrs.recipient_filter === 'paid') {
attrs.recipient_filter = 'status:-free';
}
return attrs;
},
formatOnWrite(attrs) {
// update legacy recipient_filter values to proper NQL
if (attrs.recipient_filter === 'free') {
attrs.recipient_filter = 'status:free';
}
if (attrs.recipient_filter === 'paid') {
attrs.recipient_filter = 'status:-free';
}
return attrs;
},
post() {
return this.belongsTo('Post', 'post_id');
},
emailBatches() {
return this.hasMany('EmailBatch', 'email_id');
},
recipients() {
return this.hasMany('EmailRecipient', 'email_id');
},

16
core/server/models/post.js
View file

@ -101,6 +101,14 @@ Post = ghostBookshelf.Model.extend({
}
});
// update legacy email_recipient_filter values to proper NQL
if (attrs.email_recipient_filter === 'free') {
attrs.email_recipient_filter = 'status:free';
}
if (attrs.email_recipient_filter === 'paid') {
attrs.email_recipient_filter = 'status:-free';
}
return attrs;
},
@ -139,6 +147,14 @@ Post = ghostBookshelf.Model.extend({
}
});
// update legacy email_recipient_filter values to proper NQL
if (attrs.email_recipient_filter === 'free') {
attrs.email_recipient_filter = 'status:free';
}
if (attrs.email_recipient_filter === 'paid') {
attrs.email_recipient_filter = 'status:-free';
}
return attrs;
},

20
core/server/services/mega/mega.js
View file

@ -117,19 +117,17 @@ const addEmail = async (postModel, options) => {
const emailRecipientFilter = postModel.get('email_recipient_filter');
switch (emailRecipientFilter) {
// `paid` and `free` were swapped out for NQL filters in 4.5.0, we shouldn't see them here now
case 'paid':
filterOptions.filter = 'subscribed:true+status:-free';
break;
case 'free':
filterOptions.filter = 'subscribed:true+status:free';
break;
throw new Error(`Unexpected email_recipient_filter value "${emailRecipientFilter}", expected an NQL equivalent`);
case 'all':
filterOptions.filter = 'subscribed:true';
break;
case 'none':
throw new Error('Cannot sent email to "none" email_recipient_filter');
default:
throw new Error(`Unknown email_recipient_filter ${emailRecipientFilter}`);
filterOptions.filter = `subscribed:true+${emailRecipientFilter}`;
}
const startRetrieve = Date.now();
@ -314,24 +312,22 @@ async function sendEmailJob({emailModel, options}) {
// instantiations and associated processing and event loop blocking
async function getEmailMemberRows({emailModel, options}) {
const knexOptions = _.pick(options, ['transacting', 'forUpdate']);
// 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);
const recipientFilter = emailModel.get('recipient_filter');
switch (recipientFilter) {
// `paid` and `free` were swapped out for NQL filters in 4.5.0, we shouldn't see them here now
case 'paid':
filterOptions.filter = 'subscribed:true+status:-free';
break;
case 'free':
filterOptions.filter = 'subscribed:true+status:free';
break;
throw new Error(`Unexpected recipient_filter value "${recipientFilter}", expected an NQL equivalent`);
case 'all':
filterOptions.filter = 'subscribed:true';
break;
case 'none':
throw new Error('Cannot sent email to "none" recipient_filter');
default:
throw new Error(`Unknown recipient_filter ${recipientFilter}`);
filterOptions.filter = `subscribed:true+${recipientFilter}`;
}
const startRetrieve = Date.now();

3
core/shared/i18n/translations/en.json
View file

@ -355,7 +355,8 @@
"notificationDoesNotExist": "Notification does not exist."
},
"posts": {
"postNotFound": "Post not found."
"postNotFound": "Post not found.",
"invalidEmailRecipientFilter": "Invalid filter in email_recipient_filter param."
},
"authors": {
"notFound": "Author not found."

34
test/regression/api/canary/admin/posts_spec.js
View file

@ -604,6 +604,40 @@ describe('Posts API (canary)', function () {
should.equal(res.body.posts[0].plaintext, undefined);
});
});
it('errors with invalid email recipient filter', function () {
return request
.post(localUtils.API.getApiQuery('posts/'))
.set('Origin', config.get('url'))
.send({
posts: [{
title: 'Ready to be emailed',
status: 'draft'
}]
})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(201)
.then((res) => {
return request
.put(`${localUtils.API.getApiQuery(`posts/${res.body.posts[0].id}/`)}?email_recipient_filter=not a filter`)
.set('Origin', config.get('url'))
.send({
posts: [{
title: res.body.posts[0].title,
mobilecdoc: res.body.posts[0].mobilecdoc,
updated_at: res.body.posts[0].updated_at,
status: 'published'
}]
})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(400);
})
.then((res) => {
res.text.should.match(/invalid filter/i);
});
});
});
describe('Destroy', function () {

34
test/regression/api/v3/admin/posts_spec.js
View file

@ -604,6 +604,40 @@ describe('Posts API (v3)', function () {
should.equal(res.body.posts[0].plaintext, undefined);
});
});
it('errors with invalid email recipient filter', function () {
return request
.post(localUtils.API.getApiQuery('posts/'))
.set('Origin', config.get('url'))
.send({
posts: [{
title: 'Ready to be emailed',
status: 'draft'
}]
})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(201)
.then((res) => {
return request
.put(`${localUtils.API.getApiQuery(`posts/${res.body.posts[0].id}/`)}?email_recipient_filter=not a filter`)
.set('Origin', config.get('url'))
.send({
posts: [{
title: res.body.posts[0].title,
mobilecdoc: res.body.posts[0].mobilecdoc,
updated_at: res.body.posts[0].updated_at,
status: 'published'
}]
})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(400);
})
.then((res) => {
res.text.should.match(/invalid filter/i);
});
});
});
describe('Destroy', function () {

41
test/regression/models/model_posts_spec.js
View file

@ -60,6 +60,29 @@ describe('Post Model', function () {
});
});
describe('findOne', function () {
it('transforms legacy email_recipient_filter values on read', function (done) {
const postId = testUtils.DataGenerator.Content.posts[0].id;
db.knex('posts').where({id: postId}).update({
email_recipient_filter: 'paid'
}).then(() => {
return db.knex('posts').where({id: postId});
}).then((knexResult) => {
const [knexPost] = knexResult;
knexPost.email_recipient_filter.should.equal('paid');
return models.Post.findOne({id: postId});
}).then((result) => {
should.exist(result);
const post = result.toJSON();
post.email_recipient_filter.should.equal('status:-free');
done();
}).catch(done);
});
});
describe('findPage', function () {
// @TODO: this test case fails for mysql currently if you run all regression tests, the test does not fail if you run this as a single test
describe.skip('with more posts/tags', function () {
@ -692,6 +715,24 @@ describe('Post Model', function () {
done();
}).catch(done);
});
it('transforms legacy email_recipient_filter values on save', function (done) {
const postId = testUtils.DataGenerator.Content.posts[3].id;
models.Post.findOne({id: postId}).then(() => {
return models.Post.edit({
email_recipient_filter: 'free'
}, _.extend({}, context, {id: postId}));
}).then((edited) => {
edited.attributes.email_recipient_filter.should.equal('status:free');
return db.knex('posts').where({id: edited.id});
}).then((knexResult) => {
const [knexPost] = knexResult;
knexPost.email_recipient_filter.should.equal('status:free');
done();
}).catch(done);
});
});
describe('add', function () {

66
test/unit/api/canary/utils/serializers/input/posts_spec.js
View file

@ -361,5 +361,71 @@ describe('Unit: canary/utils/serializers/input/posts', function () {
frame.data.posts[0].tags.should.eql([{name: 'name1'}, {name: 'name2'}]);
});
});
describe('transforms legacy email recipient filter values', function () {
it('free becomes status:free', function () {
const frame = {
options: {
email_recipient_filter: 'free'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.edit({}, frame);
frame.options.email_recipient_filter.should.eql('status:free');
});
it('paid becomes status:-free', function () {
const frame = {
options: {
email_recipient_filter: 'paid'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.edit({}, frame);
frame.options.email_recipient_filter.should.eql('status:-free');
});
});
});
describe('add', function () {
describe('transforms legacy email recipient filter values', function () {
it('free becomes status:free', function () {
const frame = {
options: {
email_recipient_filter: 'free'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.add({}, frame);
frame.options.email_recipient_filter.should.eql('status:free');
});
it('paid becomes status:-free', function () {
const frame = {
options: {
email_recipient_filter: 'paid'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.add({}, frame);
frame.options.email_recipient_filter.should.eql('status:-free');
});
});
});
});

66
test/unit/api/v3/utils/serializers/input/posts_spec.js
View file

@ -361,5 +361,71 @@ describe('Unit: v3/utils/serializers/input/posts', function () {
frame.data.posts[0].tags.should.eql([{name: 'name1'}, {name: 'name2'}]);
});
});
describe('transforms legacy email recipient filter values', function () {
it('free becomes status:free', function () {
const frame = {
options: {
email_recipient_filter: 'free'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.edit({}, frame);
frame.options.email_recipient_filter.should.eql('status:free');
});
it('paid becomes status:-free', function () {
const frame = {
options: {
email_recipient_filter: 'paid'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.edit({}, frame);
frame.options.email_recipient_filter.should.eql('status:-free');
});
});
});
describe('add', function () {
describe('transforms legacy email recipient filter values', function () {
it('free becomes status:free', function () {
const frame = {
options: {
email_recipient_filter: 'free'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.add({}, frame);
frame.options.email_recipient_filter.should.eql('status:free');
});
it('paid becomes status:-free', function () {
const frame = {
options: {
email_recipient_filter: 'paid'
},
data: {
posts: [{id: '1'}]
}
};
serializers.input.posts.add({}, frame);
frame.options.email_recipient_filter.should.eql('status:-free');
});
});
});
});

2
test/unit/data/schema/integrity_spec.js
View file

@ -32,7 +32,7 @@ const defaultSettings = require('../../../../core/server/data/schema/default-set
*/
describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = 'b7bca80554f3946cd2f83e0e99ff3532';
const currentSchemaHash = 'bcb57235883ddb9765f9abf8ab878cd7';
const currentFixturesHash = '8671672598d2a62e53418c4b91aa79a3';
const currentSettingsHash = 'c202cf5780aa77d8730a82680e2b142e';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';