diff --git a/core/server/api/canary/posts.js b/core/server/api/canary/posts.js index da69387beb..bb7b9f8e41 100644 --- a/core/server/api/canary/posts.js +++ b/core/server/api/canary/posts.js @@ -129,6 +129,7 @@ module.exports = { 'formats', 'source', 'email_recipient_filter', + 'newsletter_id', 'send_email_when_published', 'force_rerender', // NOTE: only for internal context diff --git a/core/server/models/newsletter.js b/core/server/models/newsletter.js index 7e806babfd..4cae64454e 100644 --- a/core/server/models/newsletter.js +++ b/core/server/models/newsletter.js @@ -37,7 +37,12 @@ const Newsletter = ghostBookshelf.Model.extend({ } } } - +}, { + orderDefaultOptions: function orderDefaultOptions() { + return { + sort_order: 'ASC' + }; + } }); module.exports = { diff --git a/core/server/models/post.js b/core/server/models/post.js index 87473f1b14..36e7038413 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -674,6 +674,13 @@ Post = ghostBookshelf.Model.extend({ } } + // newsletter_id is read-only and should only be set using a query param when publishing/scheduling + if (options.newsletter_id + && this.hasChanged('status') + && (newStatus === 'published' || newStatus === 'scheduled')) { + this.set('newsletter_id', options.newsletter_id); + } + // email_recipient_filter is read-only and should only be set using a query param when publishing/scheduling if (options.email_recipient_filter && (options.email_recipient_filter !== 'none') @@ -1019,7 +1026,7 @@ Post = ghostBookshelf.Model.extend({ findPage: ['status'], findAll: ['columns', 'filter'], destroy: ['destroyAll', 'destroyBy'], - edit: ['filter', 'email_recipient_filter', 'force_rerender'] + edit: ['filter', 'email_recipient_filter', 'force_rerender', 'newsletter_id'] }; // 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 44020e7018..134d057fe6 100644 --- a/core/server/services/mega/mega.js +++ b/core/server/services/mega/mega.js @@ -159,6 +159,7 @@ const transformEmailRecipientFilter = (emailRecipientFilter, {errorProperty = 'e * @param {object} postModel Post Model Object * @param {object} options * @param {ValidAPIVersion} options.apiVersion - api version to be used when serializing email data + * @param {string} options.newsletter_id - the newsletter_id to send the email to */ const addEmail = async (postModel, options) => { @@ -211,7 +212,8 @@ const addEmail = async (postModel, options) => { plaintext: emailData.plaintext, submitted_at: moment().toDate(), track_opens: !!settingsCache.get('email_track_opens'), - recipient_filter: emailRecipientFilter + recipient_filter: emailRecipientFilter, + newsletter_id: options.newsletter_id }, knexOptions); } else { return existing; diff --git a/core/server/services/posts/posts-service.js b/core/server/services/posts/posts-service.js index 5a3eb7dd57..f77c516ee7 100644 --- a/core/server/services/posts/posts-service.js +++ b/core/server/services/posts/posts-service.js @@ -4,7 +4,8 @@ const tpl = require('@tryghost/tpl'); const messages = { invalidEmailRecipientFilter: 'Invalid filter in email_recipient_filter param.', - invalidVisibilityFilter: 'Invalid visibility filter.' + invalidVisibilityFilter: 'Invalid visibility filter.', + invalidNewsletterId: 'The newsletter_id parameter doesn\'t match any active newsletter.' }; class PostsService { @@ -19,6 +20,16 @@ class PostsService { async editPost(frame) { let model; + // Make sure the newsletter_id is matching an active newsletter + if (frame.options.newsletter_id) { + const newsletter = await this.models.Newsletter.findOne({id: frame.options.newsletter_id, status: 'active'}, {transacting: frame.options.transacting}); + if (!newsletter) { + throw new BadRequestError({ + message: messages.invalidNewsletterId + }); + } + } + if (!frame.options.email_recipient_filter && frame.options.send_email_when_published) { await this.models.Base.transaction(async (transacting) => { const options = { @@ -65,6 +76,14 @@ class PostsService { const sendEmail = model.wasChanged() && this.shouldSendEmail(model.get('status'), model.previous('status')); if (sendEmail) { + // Set the newsletter_id if it isn't passed to the API + if (!frame.options.newsletter_id) { + const newsletters = await this.models.Newsletter.findPage({status: 'active', limit: 1, columns: ['id']}, {transacting: frame.options.transacting}); + if (newsletters.data.length > 0) { + frame.options.newsletter_id = newsletters.models[0].id; + } + } + let postEmail = model.relations.email; if (!postEmail) { diff --git a/package.json b/package.json index c66439547b..7a33fb9f64 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "dependencies": { "@sentry/node": "6.19.6", "@tryghost/adapter-manager": "0.2.28", - "@tryghost/admin-api-schema": "2.13.0", + "@tryghost/admin-api-schema": "2.14.0", "@tryghost/bookshelf-plugins": "0.3.18", "@tryghost/bootstrap-socket": "0.2.17", "@tryghost/color-utils": "0.1.12", @@ -71,7 +71,7 @@ "@tryghost/email-analytics-service": "1.0.6", "@tryghost/errors": "1.2.10", "@tryghost/express-dynamic-redirects": "0.2.8", - "@tryghost/helpers": "1.1.62", + "@tryghost/helpers": "1.1.63", "@tryghost/image-transform": "1.0.29", "@tryghost/job-manager": "0.8.21", "@tryghost/kg-card-factory": "3.1.3", diff --git a/test/e2e-api/admin/posts.test.js b/test/e2e-api/admin/posts.test.js index 425b275943..eed21b1497 100644 --- a/test/e2e-api/admin/posts.test.js +++ b/test/e2e-api/admin/posts.test.js @@ -17,7 +17,7 @@ describe('Posts API', function () { before(async function () { await localUtils.startGhost(); request = supertest.agent(config.get('url')); - await localUtils.doAuth(request, 'users:extra', 'posts', 'emails'); + await localUtils.doAuth(request, 'users:extra', 'posts', 'emails', 'newsletters'); }); afterEach(function () { @@ -418,6 +418,78 @@ describe('Posts API', function () { res2.body.posts[0].status.should.eql('draft'); }); + it('Can\'t change the newsletter_id of a post from the post body', async function () { + const post = { + newsletter_id: testUtils.DataGenerator.Content.newsletters[0].id + }; + + const postId = testUtils.DataGenerator.Content.posts[0].id; + + const res = await request + .get(localUtils.API.getApiQuery(`posts/${postId}/?`)) + .set('Origin', config.get('url')) + .expect(200); + + post.updated_at = res.body.posts[0].updated_at; + + await request + .put(localUtils.API.getApiQuery('posts/' + postId + '/')) + .set('Origin', config.get('url')) + .send({posts: [post]}) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200); + + const model = await models.Post.findOne({ + id: postId + }, testUtils.context.internal); + + should(model.get('newsletter_id')).eql(null); + }); + + it('Can change the newsletter_id of a post when publishing', async function () { + const post = { + title: 'My newsletter_id post', + status: 'draft', + feature_image_alt: 'Testing newsletter_id', + feature_image_caption: 'Testing feature image caption', + mobiledoc: testUtils.DataGenerator.markdownToMobiledoc('my post'), + created_at: moment().subtract(2, 'days').toDate(), + updated_at: moment().subtract(2, 'days').toDate(), + created_by: ObjectId().toHexString(), + updated_by: ObjectId().toHexString() + }; + + const res = await request.post(localUtils.API.getApiQuery('posts')) + .set('Origin', config.get('url')) + .send({posts: [post]}) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + const id = res.body.posts[0].id; + + const updatedPost = res.body.posts[0]; + + updatedPost.status = 'published'; + + const newsletterId = testUtils.DataGenerator.Content.newsletters[0].id; + + const finalPost = await request + .put(localUtils.API.getApiQuery('posts/' + id + '/?email_recipient_filter=all&newsletter_id=' + newsletterId)) + .set('Origin', config.get('url')) + .send({posts: [updatedPost]}) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200); + + const model = await models.Post.findOne({ + id + }, testUtils.context.internal); + + should(model.get('newsletter_id')).eql(newsletterId); + }); + it('Can destroy a post', async function () { const res = await request .del(localUtils.API.getApiQuery('posts/' + testUtils.DataGenerator.Content.posts[0].id + '/')) diff --git a/yarn.lock b/yarn.lock index 3ef0d9c937..a53966cb12 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1597,10 +1597,10 @@ dependencies: "@tryghost/errors" "^1.2.1" -"@tryghost/admin-api-schema@2.13.0": - version "2.13.0" - resolved "https://registry.yarnpkg.com/@tryghost/admin-api-schema/-/admin-api-schema-2.13.0.tgz#2871d44b1d9c7961b38c9f4b366f7c9ee5daa921" - integrity sha512-OOFkOwcy+sYbGMO5vp/kYZPq/nY/CaXgZndbU1iYsr8PV2h4PF+SdRhcU2G7M8XZZdtQWmR8+eMCk30V9MM3+w== +"@tryghost/admin-api-schema@2.14.0": + version "2.14.0" + resolved "https://registry.yarnpkg.com/@tryghost/admin-api-schema/-/admin-api-schema-2.14.0.tgz#9e95358b8dae4fbf71d4668b7b8fbcaeedd46a14" + integrity sha512-sZ6f1udiFNdhZOg0eoMBDnldJNwnd6+mi/Kob2Zh4ZyNQjWBpdVZDOdeGfYO8G8ZzcRejspUrSVQX5W7k+S5Wg== dependencies: "@tryghost/errors" "^1.0.0" lodash "^4.17.11" @@ -1831,10 +1831,10 @@ cookiejar "^2.1.3" reqresnext "^1.7.0" -"@tryghost/helpers@1.1.62": - version "1.1.62" - resolved "https://registry.yarnpkg.com/@tryghost/helpers/-/helpers-1.1.62.tgz#6e339d3d11df7851200f81d55a6b89f02906eaea" - integrity sha512-vL5qfzjADPfDXknlVZ1zEHWynV52ETjZsnkKUUUPlIm2sxYrMHxblGaAV4rdrNx/tyQ9uD3WkO9gD8rqzDf1jA== +"@tryghost/helpers@1.1.63": + version "1.1.63" + resolved "https://registry.yarnpkg.com/@tryghost/helpers/-/helpers-1.1.63.tgz#7f9ba6d734f9a8d63e036b030fb034a9c76c6234" + integrity sha512-BTePojAzyF9sgzDrJYnGoX5tLjYfuYuvsQtat1epJdOOWTOU7rNgJi+BLwjY+cv6XWMODoZRLmf+jV3CWgjn+g== dependencies: lodash-es "^4.17.11"