diff --git a/core/server/api/canary/posts.js b/core/server/api/canary/posts.js index c87297c4ec..f571be5ec3 100644 --- a/core/server/api/canary/posts.js +++ b/core/server/api/canary/posts.js @@ -2,7 +2,7 @@ const models = require('../../models'); const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); const getPostServiceInstance = require('../../services/posts/posts-service'); -const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email', 'tiers']; +const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email', 'tiers', 'newsletter']; const unsafeAttrs = ['status', 'authors', 'visibility']; const messages = { diff --git a/core/server/api/canary/utils/serializers/input/posts.js b/core/server/api/canary/utils/serializers/input/posts.js index e8c7f15e9b..c5c1b427a8 100644 --- a/core/server/api/canary/utils/serializers/input/posts.js +++ b/core/server/api/canary/utils/serializers/input/posts.js @@ -38,7 +38,7 @@ function defaultRelations(frame) { return false; } - frame.options.withRelated = ['tags', 'authors', 'authors.roles', 'email', 'tiers']; + frame.options.withRelated = ['tags', 'authors', 'authors.roles', 'email', 'tiers', 'newsletter']; } function setDefaultOrder(frame) { diff --git a/core/server/api/canary/utils/serializers/output/mappers/posts.js b/core/server/api/canary/utils/serializers/output/mappers/posts.js index 3a9c708ff7..7a96f1d2ef 100644 --- a/core/server/api/canary/utils/serializers/output/mappers/posts.js +++ b/core/server/api/canary/utils/serializers/output/mappers/posts.js @@ -96,6 +96,10 @@ module.exports = async (model, frame, options = {}) => { if (relation === 'email' && _.isEmpty(jsonModel.email)) { jsonModel.email = null; } + + if (relation === 'newsletter' && _.isEmpty(jsonModel.newsletter)) { + jsonModel.newsletter = null; + } }); } diff --git a/core/server/api/canary/utils/serializers/output/utils/clean.js b/core/server/api/canary/utils/serializers/output/utils/clean.js index b8c8ac6f78..05a7b5d21f 100644 --- a/core/server/api/canary/utils/serializers/output/utils/clean.js +++ b/core/server/api/canary/utils/serializers/output/utils/clean.js @@ -77,6 +77,7 @@ const post = (attrs, frame) => { // delete attrs.page; delete attrs.status; delete attrs.email_only; + delete attrs.newsletter; // We are standardising on returning null from the Content API for any empty values if (attrs.twitter_title === '') { @@ -124,6 +125,7 @@ const post = (attrs, frame) => { delete attrs.locale; delete attrs.author; delete attrs.type; + delete attrs.newsletter_id; return attrs; }; diff --git a/package.json b/package.json index f6f3295e2b..54eca544da 100644 --- a/package.json +++ b/package.json @@ -57,11 +57,11 @@ "dependencies": { "@sentry/node": "6.19.7", "@tryghost/adapter-manager": "0.2.30", - "@tryghost/admin-api-schema": "2.14.1", + "@tryghost/admin-api-schema": "2.15.0", "@tryghost/api-version-compatibility-service": "0.2.0", "@tryghost/bookshelf-plugins": "0.4.1", "@tryghost/bootstrap-socket": "0.2.19", - "@tryghost/color-utils": "0.1.14", + "@tryghost/color-utils": "0.1.15", "@tryghost/config-url-helpers": "0.1.7", "@tryghost/constants": "1.0.4", "@tryghost/custom-theme-settings-service": "0.3.2", @@ -73,7 +73,7 @@ "@tryghost/email-content-generator": "0.1.1", "@tryghost/errors": "1.2.12", "@tryghost/express-dynamic-redirects": "0.2.11", - "@tryghost/helpers": "1.1.65", + "@tryghost/helpers": "1.1.66", "@tryghost/image-transform": "1.0.31", "@tryghost/job-manager": "0.8.23", "@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 3cef5d473f..4ad0f2b46e 100644 --- a/test/e2e-api/admin/posts.test.js +++ b/test/e2e-api/admin/posts.test.js @@ -22,6 +22,11 @@ describe('Posts API', function () { * can't be overwritten after an email record is created. */ await localUtils.doAuth(request, 'users:extra', 'posts', 'emails', 'newsletters', 'members'); + + // Assign a newsletter to one of the posts + const newsletterId = testUtils.DataGenerator.Content.newsletters[0].id; + const postId = testUtils.DataGenerator.Content.posts[0].id; + await models.Post.edit({newsletter_id: newsletterId}, {id: postId}); }); afterEach(function () { @@ -60,6 +65,14 @@ describe('Posts API', function () { jsonResponse.posts[2].authors.length.should.eql(1); jsonResponse.posts[2].tags[0].url.should.eql(`${config.get('url')}/tag/getting-started/`); jsonResponse.posts[2].authors[0].url.should.eql(`${config.get('url')}/author/ghost/`); + + // Check if the newsletter relation is loaded by default and newsletter_id is not returned + jsonResponse.posts[12].id.should.eql(testUtils.DataGenerator.Content.posts[0].id); + jsonResponse.posts[12].newsletter.id.should.eql(testUtils.DataGenerator.Content.newsletters[0].id); + should.not.exist(jsonResponse.posts[12].newsletter_id); + + should(jsonResponse.posts[0].newsletter).be.null(); + should.not.exist(jsonResponse.posts[0].newsletter_id); }); it('Can retrieve multiple post formats', async function () { @@ -98,7 +111,7 @@ describe('Posts API', function () { jsonResponse.posts[0], 'post', null, - ['authors', 'primary_author', 'email', 'tiers'] + ['authors', 'primary_author', 'email', 'tiers', 'newsletter'] ); localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); @@ -162,6 +175,33 @@ describe('Posts API', function () { _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); testUtils.API.isISO8601(jsonResponse.posts[0].created_at).should.be.true(); + + // Check if the newsletter relation is loaded by default and newsletter_id is not returned + jsonResponse.posts[0].newsletter.id.should.eql(testUtils.DataGenerator.Content.newsletters[0].id); + should.not.exist(jsonResponse.posts[0].newsletter_id); + }); + + it('Can request a post by id without newsletter', async function () { + const res = await request.get(localUtils.API.getApiQuery('posts/' + testUtils.DataGenerator.Content.posts[1].id + '/')) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200); + + should.not.exist(res.headers['x-cache-invalidate']); + const jsonResponse = res.body; + should.exist(jsonResponse); + should.exist(jsonResponse.posts); + localUtils.API.checkResponse(jsonResponse.posts[0], 'post'); + jsonResponse.posts[0].id.should.equal(testUtils.DataGenerator.Content.posts[1].id); + + _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + + testUtils.API.isISO8601(jsonResponse.posts[0].created_at).should.be.true(); + + // Newsletter should be returned as null + should(jsonResponse.posts[0].newsletter).be.null(); + should.not.exist(jsonResponse.posts[0].newsletter_id); }); it('Can retrieve a post by slug', async function () { @@ -179,11 +219,15 @@ describe('Posts API', function () { jsonResponse.posts[0].slug.should.equal('welcome'); _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + + // Newsletter should be returned as null + should(jsonResponse.posts[0].newsletter).be.null(); + should.not.exist(jsonResponse.posts[0].newsletter_id); }); it('Can include relations for a single post', async function () { const res = await request - .get(localUtils.API.getApiQuery('posts/' + testUtils.DataGenerator.Content.posts[0].id + '/?include=authors,tags,email,tiers')) + .get(localUtils.API.getApiQuery('posts/' + testUtils.DataGenerator.Content.posts[0].id + '/?include=authors,tags,email,tiers,newsletter')) .set('Origin', config.get('url')) .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) @@ -204,6 +248,9 @@ describe('Posts API', function () { jsonResponse.posts[0].email.should.be.an.Object(); localUtils.API.checkResponse(jsonResponse.posts[0].email, 'email'); + + jsonResponse.posts[0].newsletter.id.should.eql(testUtils.DataGenerator.Content.newsletters[0].id); + should.not.exist(jsonResponse.posts[0].newsletter_id); }); it('Can add a post', async function () { @@ -235,6 +282,10 @@ describe('Posts API', function () { should.exist(res.headers.location); res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('posts/')}${res.body.posts[0].id}/`); + // Newsletter should be returned as null + should(res.body.posts[0].newsletter).be.null(); + should.not.exist(res.body.posts[0].newsletter_id); + const model = await models.Post.findOne({ id: res.body.posts[0].id, status: 'draft' @@ -352,6 +403,10 @@ describe('Posts API', function () { .expect(200); res2.headers['x-cache-invalidate'].should.eql('/p/' + res2.body.posts[0].uuid + '/'); + + // Newsletter should be returned as null + should(res2.body.posts[0].newsletter).be.null(); + should.not.exist(res2.body.posts[0].newsletter_id); }); it('Can update and force re-render', async function () { @@ -422,12 +477,18 @@ 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 () { + 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 postId = testUtils.DataGenerator.Content.posts[2].id; + + const modelBefore = await models.Post.findOne({ + id: postId + }, testUtils.context.internal); + + should(modelBefore.get('newsletter_id')).eql(null, 'This test requires the initial post to not have a newsletter'); const res = await request .get(localUtils.API.getApiQuery(`posts/${postId}/?`)) @@ -436,7 +497,44 @@ describe('Posts API', function () { post.updated_at = res.body.posts[0].updated_at; - await request + const res2 = 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(422); + + const model = await models.Post.findOne({ + id: postId + }, testUtils.context.internal); + + should(model.get('newsletter_id')).eql(null); + }); + + it(`Can't change the newsletter 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[2].id; + + const modelBefore = await models.Post.findOne({ + id: postId + }, testUtils.context.internal); + + should(modelBefore.get('newsletter_id')).eql(null, 'This test requires the initial post to not have a newsletter'); + + 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; + + const res2 = await request .put(localUtils.API.getApiQuery('posts/' + postId + '/')) .set('Origin', config.get('url')) .send({posts: [post]}) @@ -451,6 +549,45 @@ describe('Posts API', function () { should(model.get('newsletter_id')).eql(null); }); + it('Cannot change the newsletter via body when adding', async function () { + const post = { + title: 'My newsletter post', + status: 'draft', + feature_image_alt: 'Testing newsletter', + 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(), + newsletter: { + // This should be ignored, the default one should be used instead + id: testUtils.DataGenerator.Content.newsletters[0].id + } + }; + + 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); + + // Check that the default newsletter is used instead of the one in body (not allowed) + should(res.body.posts[0].status).eql('draft'); + should(res.body.posts[0].newsletter).eql(null); + should.not.exist(res.body.posts[0].newsletter_id); + + const id = res.body.posts[0].id; + + const model = await models.Post.findOne({ + id, + status: 'draft' // Fix for default filter + }, 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', @@ -471,6 +608,10 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(201); + // Check newsletter relation is loaded, but null in response. + should(res.body.posts[0].newsletter).eql(null); + should.not.exist(res.body.posts[0].newsletter_id); + const id = res.body.posts[0].id; const updatedPost = res.body.posts[0]; @@ -487,6 +628,10 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + should(finalPost.body.posts[0].newsletter.id).eql(newsletterId); + should.not.exist(finalPost.body.posts[0].newsletter_id); + const model = await models.Post.findOne({ id }, testUtils.context.internal); @@ -495,6 +640,8 @@ describe('Posts API', function () { }); it('Defaults to the default newsletter when publishing without a newsletter_id', async function () { + const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); + const post = { title: 'My post without newsletter_id', status: 'draft', @@ -521,7 +668,7 @@ describe('Posts API', function () { updated_at: res.body.posts[0].updated_at }; - await request + const finalPost = await request .put(localUtils.API.getApiQuery('posts/' + id + '/?email_recipient_filter=all')) .set('Origin', config.get('url')) .send({posts: [updatedPost]}) @@ -529,14 +676,14 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + should(finalPost.body.posts[0].newsletter.id).eql(defaultNewsletter.get('id')); + should.not.exist(finalPost.body.posts[0].newsletter_id); + const model = await models.Post.findOne({ id }, testUtils.context.internal); - const defaultNewsletter = await models.Newsletter.findOne({ - sort_order: 0 - }, testUtils.context.internal); - should(model.get('newsletter_id')).eql(defaultNewsletter.get('id')); }); @@ -578,6 +725,10 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + should(res2.body.posts[0].newsletter.id).eql(newsletterId); + should.not.exist(res2.body.posts[0].newsletter_id); + model = await models.Post.findOne({ id: id, status: 'published' @@ -597,6 +748,10 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + should(res3.body.posts[0].newsletter).eql(null); + should.not.exist(res3.body.posts[0].newsletter_id); + model = await models.Post.findOne({ id: id, status: 'draft' @@ -611,7 +766,7 @@ describe('Posts API', function () { updated_at: res3.body.posts[0].updated_at }; - await request + const res4 = await request .put(localUtils.API.getApiQuery('posts/' + id + '/?email_recipient_filter=all&newsletter_id=' + newsletterId2)) .set('Origin', config.get('url')) .send({posts: [republished]}) @@ -619,6 +774,11 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + // + did update the newsletter id + should(res4.body.posts[0].newsletter.id).eql(newsletterId2); + should.not.exist(res4.body.posts[0].newsletter_id); + model = await models.Post.findOne({ id: id, status: 'published' @@ -626,7 +786,7 @@ describe('Posts API', function () { should(model.get('newsletter_id')).eql(newsletterId2); // Should not change if status remains published - await request + const res5 = await request .put(localUtils.API.getApiQuery('posts/' + id + '/?email_recipient_filter=all&newsletter_id=' + newsletterId)) .set('Origin', config.get('url')) .send({posts: [republished]}) @@ -634,6 +794,11 @@ describe('Posts API', function () { .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); + // Check newsletter relation is loaded in response + // + did not update the newsletter id + should(res5.body.posts[0].newsletter.id).eql(newsletterId2); + should.not.exist(res5.body.posts[0].newsletter_id); + model = await models.Post.findOne({ id: id, status: 'published' diff --git a/test/e2e-api/admin/utils.js b/test/e2e-api/admin/utils.js index aeee1f0e41..e06f5296c5 100644 --- a/test/e2e-api/admin/utils.js +++ b/test/e2e-api/admin/utils.js @@ -71,7 +71,7 @@ const expectedProperties = { 'frontmatter', 'email_only', 'tiers', - 'newsletter_id' + 'newsletter' ], page: [ diff --git a/test/regression/api/admin/utils.js b/test/regression/api/admin/utils.js index 0c81a74357..2e01e1fdd6 100644 --- a/test/regression/api/admin/utils.js +++ b/test/regression/api/admin/utils.js @@ -65,7 +65,7 @@ const expectedProperties = { 'frontmatter', 'email_only', 'tiers', - 'newsletter_id' + 'newsletter' ], user: [ 'id', diff --git a/yarn.lock b/yarn.lock index c4608c58a5..f5e8ad83e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1719,10 +1719,10 @@ dependencies: "@tryghost/errors" "^1.2.1" -"@tryghost/admin-api-schema@2.14.1": - version "2.14.1" - resolved "https://registry.yarnpkg.com/@tryghost/admin-api-schema/-/admin-api-schema-2.14.1.tgz#cc9485847671014b8e86b31b73ef7b96f7d22e83" - integrity sha512-VnrwzOlIjIIIYJi64iQzINMqBNQe1wbafe4yXQsL0leaHlBCrucEGYZVMpq25pISkBpCg8kGqYksuJdyfIQ4KA== +"@tryghost/admin-api-schema@2.15.0": + version "2.15.0" + resolved "https://registry.yarnpkg.com/@tryghost/admin-api-schema/-/admin-api-schema-2.15.0.tgz#fb2a14af9dd5d418d3a2f5a7ce89b349ac092b2b" + integrity sha512-zh1aod5J3DwKuhiJ1JbkMT8ybs0DfHjs7ubwSc+GUkWaa6w8yyDFukzDC59P3PPV3DMTyKwzJlEqv2S86QQmOw== dependencies: "@tryghost/errors" "^1.0.0" lodash "^4.17.11" @@ -1838,10 +1838,10 @@ dependencies: long-timeout "^0.1.1" -"@tryghost/color-utils@0.1.14": - version "0.1.14" - resolved "https://registry.yarnpkg.com/@tryghost/color-utils/-/color-utils-0.1.14.tgz#4b794bf79a1b813608725e0456b100f28f881e7d" - integrity sha512-3mIs+z3P+BR2nF4a6zCWN2GEeZlKwK6JfxEVhrmRcwsIvWVtpPaMAT4FjozuXlBz5JskIOLZDfXS6AcP+cc3nw== +"@tryghost/color-utils@0.1.15": + version "0.1.15" + resolved "https://registry.yarnpkg.com/@tryghost/color-utils/-/color-utils-0.1.15.tgz#814f082ae4ee65c96b0429ab196af2772b7b25c4" + integrity sha512-LI8UKboknIwCewL3z1l5FS4V0W84Nfc4snA1n62F3a8casS6V+imtqdGb1qgu+Y/0IhVxLbMSM5/qOpJzZoPiQ== dependencies: color "^3.2.1" @@ -1971,10 +1971,10 @@ cookiejar "^2.1.3" reqresnext "^1.7.0" -"@tryghost/helpers@1.1.65": - version "1.1.65" - resolved "https://registry.yarnpkg.com/@tryghost/helpers/-/helpers-1.1.65.tgz#02d17739c35d50bf1d7fd1cc12ff5c0ed0f40efd" - integrity sha512-IOtGa638BtIq4W0k/D9+RbawRjMrYtvwHdfS+GdJaMHAQQWa68jxTQfHBIinPLk5n4nCJHNaXScSiLbakQ/j0w== +"@tryghost/helpers@1.1.66": + version "1.1.66" + resolved "https://registry.yarnpkg.com/@tryghost/helpers/-/helpers-1.1.66.tgz#dcd485975209e4aff78d4f0ae598855852c7b9e1" + integrity sha512-GizIDYroVSLKxSTTMZ2OSrljB5eze59ZkkJM8pvabedrdV+km2uL0dubOiOtpJKcXzNj6UU7KEhoip6f4BoAnA== dependencies: lodash-es "^4.17.11"