0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-06 22:40:14 -05:00

Included newsletter relation by default in posts (#14723)

refs https://github.com/TryGhost/Team/issues/1569

**Changes in admin-api-schema:**
- https://github.com/TryGhost/SDK/compare/%40tryghost/admin-api-schema%402.14.1...%40tryghost/admin-api-schema%402.15.0
- Ignore `newsletter` when used in input

**Changes**
- Added the `newsletter` relation as a default include for posts
- Removed the newsletter_id from the API output

**Tests**
- Test the newsletter relation is always loaded for browse, read, add and edit, unless the include option is added explicitly

Co-authored-by: Matt Hanley <git@matthanley.co.uk>
This commit is contained in:
Simon Backx 2022-05-09 11:06:59 +02:00 committed by GitHub
parent 96e7187e8d
commit 21d9d20e3e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 202 additions and 31 deletions

View file

@ -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 = {

View file

@ -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) {

View file

@ -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;
}
});
}

View file

@ -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;
};

View file

@ -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",

View file

@ -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 <b>feature image caption</b>',
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'

View file

@ -71,7 +71,7 @@ const expectedProperties = {
'frontmatter',
'email_only',
'tiers',
'newsletter_id'
'newsletter'
],
page: [

View file

@ -65,7 +65,7 @@ const expectedProperties = {
'frontmatter',
'email_only',
'tiers',
'newsletter_id'
'newsletter'
],
user: [
'id',

View file

@ -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"