Skip to content

Commit

Permalink
馃悰 Fixed empty html/plaintext fields for narrow fields parameter (#11505)
Browse files Browse the repository at this point in the history 
refs https://forum.ghost.org/t/plaintext-value-is-empty-using-the-api/10537

- The `plaintext`/`html` fields were empty because `visibility` attribute was not present in response body on output serialization stage. `visibility` field is always needed for content gating to work as expected 
- Added `visibility` field in the input serialization layer as it wouldn't be possible to use content gating if added on model layer through `defaultColumnsToFetch`
- Added test cases covering a bug
  • Loading branch information
@naz
naz committed Jan 8, 2020
1 parent db8a598 commit 97ea664
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 12 deletions.
8 changes: 8 additions & 0 deletions core/server/api/canary/utils/serializers/input/pages.js
View file
Expand Up @@ -53,6 +53,12 @@ function setDefaultOrder(frame) {
}
}

function forceVisibilityColumn(frame) {
if (frame.options.columns && !frame.options.columns.includes('visibility')) {
frame.options.columns.push('visibility');
}
}

function defaultFormat(frame) {
if (frame.options.formats) {
return;
Expand Down Expand Up @@ -100,6 +106,7 @@ module.exports = {
if (localUtils.isContentAPI(frame)) {
removeMobiledocFormat(frame);
setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand All @@ -119,6 +126,7 @@ module.exports = {
if (localUtils.isContentAPI(frame)) {
removeMobiledocFormat(frame);
setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down
8 changes: 8 additions & 0 deletions core/server/api/canary/utils/serializers/input/posts.js
View file
Expand Up @@ -53,6 +53,12 @@ function setDefaultOrder(frame) {
}
}

function forceVisibilityColumn(frame) {
if (frame.options.columns && !frame.options.columns.includes('visibility')) {
frame.options.columns.push('visibility');
}
}

function defaultFormat(frame) {
if (frame.options.formats) {
return;
Expand Down Expand Up @@ -108,6 +114,7 @@ module.exports = {
removeMobiledocFormat(frame);

setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down Expand Up @@ -135,6 +142,7 @@ module.exports = {
removeMobiledocFormat(frame);

setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down
4 changes: 4 additions & 0 deletions core/server/api/canary/utils/serializers/output/utils/clean.js
View file
Expand Up @@ -95,6 +95,10 @@ const post = (attrs, frame) => {
if (attrs.og_description === '') {
attrs.og_description = null;
}
// NOTE: the visibility column has to be always present in Content API response to perform content gating
if (frame.options.columns && frame.options.columns.includes('visibility') && !frame.original.query.fields.includes('visibility')) {
delete attrs.visibility;
}
} else {
delete attrs.page;
}
Expand Down
8 changes: 8 additions & 0 deletions core/server/api/v2/utils/serializers/input/pages.js
View file
Expand Up @@ -53,6 +53,12 @@ function setDefaultOrder(frame) {
}
}

function forceVisibilityColumn(frame) {
if (frame.options.columns && !frame.options.columns.includes('visibility')) {
frame.options.columns.push('visibility');
}
}

function defaultFormat(frame) {
if (frame.options.formats) {
return;
Expand Down Expand Up @@ -100,6 +106,7 @@ module.exports = {
if (localUtils.isContentAPI(frame)) {
removeMobiledocFormat(frame);
setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand All @@ -119,6 +126,7 @@ module.exports = {
if (localUtils.isContentAPI(frame)) {
removeMobiledocFormat(frame);
setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down
8 changes: 8 additions & 0 deletions core/server/api/v2/utils/serializers/input/posts.js
View file
Expand Up @@ -53,6 +53,12 @@ function setDefaultOrder(frame) {
}
}

function forceVisibilityColumn(frame) {
if (frame.options.columns && !frame.options.columns.includes('visibility')) {
frame.options.columns.push('visibility');
}
}

function defaultFormat(frame) {
if (frame.options.formats) {
return;
Expand Down Expand Up @@ -108,6 +114,7 @@ module.exports = {
removeMobiledocFormat(frame);

setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down Expand Up @@ -135,6 +142,7 @@ module.exports = {
removeMobiledocFormat(frame);

setDefaultOrder(frame);
forceVisibilityColumn(frame);
}

if (!localUtils.isContentAPI(frame)) {
Expand Down
37 changes: 33 additions & 4 deletions core/test/regression/api/canary/content/posts_spec.js
View file
Expand Up @@ -224,6 +224,7 @@ describe('api/canary/content/posts', function () {
});

describe('content gating', function () {
let publicPost;
let membersPost;
let paidPost;

Expand All @@ -233,6 +234,12 @@ describe('api/canary/content/posts', function () {
});

before (function () {
publicPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'free-to-see',
visibility: 'public',
published_at: moment().add(15, 'seconds').toDate() // here to ensure sorting is not modified
});

membersPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'thou-shalt-not-be-seen',
visibility: 'members',
Expand All @@ -246,11 +253,31 @@ describe('api/canary/content/posts', function () {
});

return testUtils.fixtures.insertPosts([
publicPost,
membersPost,
paidPost
]);
});

it('public post fields are always visible', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${publicPost.id}/?key=${validKey}&fields=slug,html,plaintext&formats=html,plaintext`))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.then((res) => {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
const post = jsonResponse.posts[0];

localUtils.API.checkResponse(post, 'post', null, null, ['id', 'slug', 'html', 'plaintext']);
post.slug.should.eql('free-to-see');
post.html.should.not.eql('');
post.plaintext.should.not.eql('');
});
});

it('cannot read members only post content', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${membersPost.id}/?key=${validKey}`))
Expand Down Expand Up @@ -319,25 +346,27 @@ describe('api/canary/content/posts', function () {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
localUtils.API.checkResponse(jsonResponse, 'posts');
jsonResponse.posts.should.have.length(13);
jsonResponse.posts.should.have.length(14);
localUtils.API.checkResponse(jsonResponse.posts[0], 'post');
localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination');
_.isBoolean(jsonResponse.posts[0].featured).should.eql(true);

// Default order 'published_at desc' check
jsonResponse.posts[0].slug.should.eql('thou-shalt-not-be-seen');
jsonResponse.posts[1].slug.should.eql('thou-shalt-be-paid-for');
jsonResponse.posts[6].slug.should.eql('organising-content');
jsonResponse.posts[2].slug.should.eql('free-to-see');
jsonResponse.posts[7].slug.should.eql('organising-content');

jsonResponse.posts[0].html.should.eql('');
jsonResponse.posts[1].html.should.eql('');
jsonResponse.posts[6].html.should.not.eql('');
jsonResponse.posts[2].html.should.not.eql('');
jsonResponse.posts[7].html.should.not.eql('');

// check meta response for this test
jsonResponse.meta.pagination.page.should.eql(1);
jsonResponse.meta.pagination.limit.should.eql(15);
jsonResponse.meta.pagination.pages.should.eql(1);
jsonResponse.meta.pagination.total.should.eql(13);
jsonResponse.meta.pagination.total.should.eql(14);
jsonResponse.meta.pagination.hasOwnProperty('next').should.be.true();
jsonResponse.meta.pagination.hasOwnProperty('prev').should.be.true();
should.not.exist(jsonResponse.meta.pagination.next);
Expand Down
37 changes: 33 additions & 4 deletions core/test/regression/api/v2/content/posts_spec.js
View file
Expand Up @@ -204,6 +204,7 @@ describe('api/v2/content/posts', function () {
});

describe('content gating', function () {
let publicPost;
let membersPost;
let paidPost;

Expand All @@ -213,6 +214,12 @@ describe('api/v2/content/posts', function () {
});

before (function () {
publicPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'free-to-see',
visibility: 'public',
published_at: moment().add(15, 'seconds').toDate() // here to ensure sorting is not modified
});

membersPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'thou-shalt-not-be-seen',
visibility: 'members',
Expand All @@ -226,11 +233,31 @@ describe('api/v2/content/posts', function () {
});

return testUtils.fixtures.insertPosts([
publicPost,
membersPost,
paidPost
]);
});

it('public post fields are always visible', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${publicPost.id}/?key=${validKey}&fields=slug,html,plaintext&formats=html,plaintext`))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.then((res) => {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
const post = jsonResponse.posts[0];

localUtils.API.checkResponse(post, 'post', null, null, ['id', 'slug', 'html', 'plaintext']);
post.slug.should.eql('free-to-see');
post.html.should.not.eql('');
post.plaintext.should.not.eql('');
});
});

it('cannot read members only post content', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${membersPost.id}/?key=${validKey}`))
Expand Down Expand Up @@ -299,25 +326,27 @@ describe('api/v2/content/posts', function () {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
localUtils.API.checkResponse(jsonResponse, 'posts');
jsonResponse.posts.should.have.length(13);
jsonResponse.posts.should.have.length(14);
localUtils.API.checkResponse(jsonResponse.posts[0], 'post');
localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination');
_.isBoolean(jsonResponse.posts[0].featured).should.eql(true);

// Default order 'published_at desc' check
jsonResponse.posts[0].slug.should.eql('thou-shalt-not-be-seen');
jsonResponse.posts[1].slug.should.eql('thou-shalt-be-paid-for');
jsonResponse.posts[6].slug.should.eql('organising-content');
jsonResponse.posts[2].slug.should.eql('free-to-see');
jsonResponse.posts[7].slug.should.eql('organising-content');

jsonResponse.posts[0].html.should.eql('');
jsonResponse.posts[1].html.should.eql('');
jsonResponse.posts[6].html.should.not.eql('');
jsonResponse.posts[2].html.should.not.eql('');
jsonResponse.posts[7].html.should.not.eql('');

// check meta response for this test
jsonResponse.meta.pagination.page.should.eql(1);
jsonResponse.meta.pagination.limit.should.eql(15);
jsonResponse.meta.pagination.pages.should.eql(1);
jsonResponse.meta.pagination.total.should.eql(13);
jsonResponse.meta.pagination.total.should.eql(14);
jsonResponse.meta.pagination.hasOwnProperty('next').should.be.true();
jsonResponse.meta.pagination.hasOwnProperty('prev').should.be.true();
should.not.exist(jsonResponse.meta.pagination.next);
Expand Down
37 changes: 33 additions & 4 deletions core/test/regression/api/v3/content/posts_spec.js
View file
Expand Up @@ -224,6 +224,7 @@ describe('api/v3/content/posts', function () {
});

describe('content gating', function () {
let publicPost;
let membersPost;
let paidPost;

Expand All @@ -233,6 +234,12 @@ describe('api/v3/content/posts', function () {
});

before (function () {
publicPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'free-to-see',
visibility: 'public',
published_at: moment().add(15, 'seconds').toDate() // here to ensure sorting is not modified
});

membersPost = testUtils.DataGenerator.forKnex.createPost({
slug: 'thou-shalt-not-be-seen',
visibility: 'members',
Expand All @@ -246,11 +253,31 @@ describe('api/v3/content/posts', function () {
});

return testUtils.fixtures.insertPosts([
publicPost,
membersPost,
paidPost
]);
});

it('public post fields are always visible', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${publicPost.id}/?key=${validKey}&fields=slug,html,plaintext&formats=html,plaintext`))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.then((res) => {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
const post = jsonResponse.posts[0];

localUtils.API.checkResponse(post, 'post', null, null, ['id', 'slug', 'html', 'plaintext']);
post.slug.should.eql('free-to-see');
post.html.should.not.eql('');
post.plaintext.should.not.eql('');
});
});

it('cannot read members only post content', function () {
return request
.get(localUtils.API.getApiQuery(`posts/${membersPost.id}/?key=${validKey}`))
Expand Down Expand Up @@ -319,25 +346,27 @@ describe('api/v3/content/posts', function () {
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
localUtils.API.checkResponse(jsonResponse, 'posts');
jsonResponse.posts.should.have.length(13);
jsonResponse.posts.should.have.length(14);
localUtils.API.checkResponse(jsonResponse.posts[0], 'post');
localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination');
_.isBoolean(jsonResponse.posts[0].featured).should.eql(true);

// Default order 'published_at desc' check
jsonResponse.posts[0].slug.should.eql('thou-shalt-not-be-seen');
jsonResponse.posts[1].slug.should.eql('thou-shalt-be-paid-for');
jsonResponse.posts[6].slug.should.eql('organising-content');
jsonResponse.posts[2].slug.should.eql('free-to-see');
jsonResponse.posts[7].slug.should.eql('organising-content');

jsonResponse.posts[0].html.should.eql('');
jsonResponse.posts[1].html.should.eql('');
jsonResponse.posts[6].html.should.not.eql('');
jsonResponse.posts[2].html.should.not.eql('');
jsonResponse.posts[7].html.should.not.eql('');

// check meta response for this test
jsonResponse.meta.pagination.page.should.eql(1);
jsonResponse.meta.pagination.limit.should.eql(15);
jsonResponse.meta.pagination.pages.should.eql(1);
jsonResponse.meta.pagination.total.should.eql(13);
jsonResponse.meta.pagination.total.should.eql(14);
jsonResponse.meta.pagination.hasOwnProperty('next').should.be.true();
jsonResponse.meta.pagination.hasOwnProperty('prev').should.be.true();
should.not.exist(jsonResponse.meta.pagination.next);
Expand Down

0 comments on commit 97ea664

Please sign in to comment.