From 97ea664d4d6f24717564454183dd7238e6990239 Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Wed, 8 Jan 2020 14:43:21 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20empty=20html/plaintext?= =?UTF-8?q?=20fields=20for=20narrow=20fields=20parameter=20(#11505)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../canary/utils/serializers/input/pages.js | 8 ++++ .../canary/utils/serializers/input/posts.js | 8 ++++ .../utils/serializers/output/utils/clean.js | 4 ++ .../api/v2/utils/serializers/input/pages.js | 8 ++++ .../api/v2/utils/serializers/input/posts.js | 8 ++++ .../api/canary/content/posts_spec.js | 37 +++++++++++++++++-- .../regression/api/v2/content/posts_spec.js | 37 +++++++++++++++++-- .../regression/api/v3/content/posts_spec.js | 37 +++++++++++++++++-- 8 files changed, 135 insertions(+), 12 deletions(-) diff --git a/core/server/api/canary/utils/serializers/input/pages.js b/core/server/api/canary/utils/serializers/input/pages.js index 105bbac9c4..1d0c648a44 100644 --- a/core/server/api/canary/utils/serializers/input/pages.js +++ b/core/server/api/canary/utils/serializers/input/pages.js @@ -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; @@ -100,6 +106,7 @@ module.exports = { if (localUtils.isContentAPI(frame)) { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { @@ -119,6 +126,7 @@ module.exports = { if (localUtils.isContentAPI(frame)) { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { diff --git a/core/server/api/canary/utils/serializers/input/posts.js b/core/server/api/canary/utils/serializers/input/posts.js index 7c9875d8f3..2d01102fc7 100644 --- a/core/server/api/canary/utils/serializers/input/posts.js +++ b/core/server/api/canary/utils/serializers/input/posts.js @@ -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; @@ -108,6 +114,7 @@ module.exports = { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { @@ -135,6 +142,7 @@ module.exports = { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { 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 7a87293af2..3c55ffbc89 100644 --- a/core/server/api/canary/utils/serializers/output/utils/clean.js +++ b/core/server/api/canary/utils/serializers/output/utils/clean.js @@ -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; } diff --git a/core/server/api/v2/utils/serializers/input/pages.js b/core/server/api/v2/utils/serializers/input/pages.js index 7eb7ecfec9..61de084c6e 100644 --- a/core/server/api/v2/utils/serializers/input/pages.js +++ b/core/server/api/v2/utils/serializers/input/pages.js @@ -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; @@ -100,6 +106,7 @@ module.exports = { if (localUtils.isContentAPI(frame)) { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { @@ -119,6 +126,7 @@ module.exports = { if (localUtils.isContentAPI(frame)) { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { diff --git a/core/server/api/v2/utils/serializers/input/posts.js b/core/server/api/v2/utils/serializers/input/posts.js index c82a962ca5..07d7e9e587 100644 --- a/core/server/api/v2/utils/serializers/input/posts.js +++ b/core/server/api/v2/utils/serializers/input/posts.js @@ -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; @@ -108,6 +114,7 @@ module.exports = { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { @@ -135,6 +142,7 @@ module.exports = { removeMobiledocFormat(frame); setDefaultOrder(frame); + forceVisibilityColumn(frame); } if (!localUtils.isContentAPI(frame)) { diff --git a/core/test/regression/api/canary/content/posts_spec.js b/core/test/regression/api/canary/content/posts_spec.js index f36422f496..feb0d5bdde 100644 --- a/core/test/regression/api/canary/content/posts_spec.js +++ b/core/test/regression/api/canary/content/posts_spec.js @@ -224,6 +224,7 @@ describe('api/canary/content/posts', function () { }); describe('content gating', function () { + let publicPost; let membersPost; let paidPost; @@ -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', @@ -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}`)) @@ -319,7 +346,7 @@ 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); @@ -327,17 +354,19 @@ describe('api/canary/content/posts', function () { // 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); diff --git a/core/test/regression/api/v2/content/posts_spec.js b/core/test/regression/api/v2/content/posts_spec.js index 2244afafa7..2bd8fb9b41 100644 --- a/core/test/regression/api/v2/content/posts_spec.js +++ b/core/test/regression/api/v2/content/posts_spec.js @@ -204,6 +204,7 @@ describe('api/v2/content/posts', function () { }); describe('content gating', function () { + let publicPost; let membersPost; let paidPost; @@ -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', @@ -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}`)) @@ -299,7 +326,7 @@ 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); @@ -307,17 +334,19 @@ describe('api/v2/content/posts', function () { // 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); diff --git a/core/test/regression/api/v3/content/posts_spec.js b/core/test/regression/api/v3/content/posts_spec.js index bebd2580cd..76668138a3 100644 --- a/core/test/regression/api/v3/content/posts_spec.js +++ b/core/test/regression/api/v3/content/posts_spec.js @@ -224,6 +224,7 @@ describe('api/v3/content/posts', function () { }); describe('content gating', function () { + let publicPost; let membersPost; let paidPost; @@ -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', @@ -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}`)) @@ -319,7 +346,7 @@ 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); @@ -327,17 +354,19 @@ describe('api/v3/content/posts', function () { // 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);