From 1b0aa0abd8225ab7e6068c4688849d7bf220f478 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 1 Jul 2021 17:55:44 +0100 Subject: [PATCH] Fixed `feature_image_caption` needing triple-curlies in themes (#13107) refs https://github.com/TryGhost/Team/issues/845 refs https://github.com/TryGhost/Ghost/commit/517d2abc5caedfed988d7e015c5170c6b215d81e - updated router response formatting functions and `{{#get}}` helper response handling to make any `feature_image_caption` properties in the response a `SafeString` instance so triple-curlies are not needed when using the property in themes --- core/frontend/helpers/get.js | 9 ++- core/frontend/services/proxy.js | 12 +++- .../routing/helpers/format-response.js | 6 ++ test/unit/helpers/get_spec.js | 31 ++++++++++ .../routing/helpers/format-response_spec.js | 60 ++++++++++++++----- 5 files changed, 99 insertions(+), 19 deletions(-) diff --git a/core/frontend/helpers/get.js b/core/frontend/helpers/get.js index af1122a796..06b480cbb7 100644 --- a/core/frontend/helpers/get.js +++ b/core/frontend/helpers/get.js @@ -1,7 +1,7 @@ // # Get Helper // Usage: `{{#get "posts" limit="5"}}`, `{{#get "tags" limit="all"}}` // Fetches data from the API -const {config, logging, errors, i18n, hbs, api} = require('../services/proxy'); +const {config, logging, errors, i18n, hbs, api, prepareContextResource} = require('../services/proxy'); const _ = require('lodash'); const Promise = require('bluebird'); const jsonpath = require('jsonpath'); @@ -141,14 +141,17 @@ module.exports = function get(resource, options) { // @TODO: https://github.com/TryGhost/Ghost/issues/10548 return controller[action](apiOptions).then(function success(result) { - let blockParams; + // prepare data properties for use with handlebars + if (result[resource] && result[resource].length) { + result[resource].forEach(prepareContextResource); + } // used for logging details of slow requests returnedRowsCount = result[resource] && result[resource].length; // block params allows the theme developer to name the data using something like // `{{#get "posts" as |result pageInfo|}}` - blockParams = [result[resource]]; + const blockParams = [result[resource]]; if (result.meta && result.meta.pagination) { result.pagination = result.meta.pagination; blockParams.push(result.meta.pagination); diff --git a/core/frontend/services/proxy.js b/core/frontend/services/proxy.js index 4f5f85add5..9c8e0cb6aa 100644 --- a/core/frontend/services/proxy.js +++ b/core/frontend/services/proxy.js @@ -60,5 +60,15 @@ module.exports = { blogIcon: require('../../server/lib/image').blogIcon, urlService: require('./url'), urlUtils: require('../../shared/url-utils'), - localUtils: require('./theme-engine/handlebars/utils') + localUtils: require('./theme-engine/handlebars/utils'), + + // Used by router service and {{get}} helper to prepare data for optimal usage in themes + prepareContextResource(data) { + (Array.isArray(data) ? data : [data]).forEach((resource) => { + // feature_image_caption contains HTML, making it a SafeString spares theme devs from triple-curlies + if (resource.feature_image_caption) { + resource.feature_image_caption = new hbs.SafeString(resource.feature_image_caption); + } + }); + } }; diff --git a/core/frontend/services/routing/helpers/format-response.js b/core/frontend/services/routing/helpers/format-response.js index 349cb9eeed..eef78ef1f3 100644 --- a/core/frontend/services/routing/helpers/format-response.js +++ b/core/frontend/services/routing/helpers/format-response.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const {prepareContextResource} = require('../../proxy'); /** * @description Formats API response into handlebars/theme format. @@ -10,6 +11,7 @@ function formatPageResponse(result) { if (result.posts) { response.posts = result.posts; + prepareContextResource(response.posts); } if (result.meta && result.meta.pagination) { @@ -17,6 +19,8 @@ function formatPageResponse(result) { } _.each(result.data, function (data, name) { + prepareContextResource(data); + if (data.meta) { // Move pagination to be a top level key response[name] = data; @@ -43,6 +47,8 @@ function formatPageResponse(result) { * @return {Object} containing page variables */ function formatResponse(post) { + prepareContextResource(post); + return { post: post }; diff --git a/test/unit/helpers/get_spec.js b/test/unit/helpers/get_spec.js index 15b2d95718..3d27427e28 100644 --- a/test/unit/helpers/get_spec.js +++ b/test/unit/helpers/get_spec.js @@ -1,6 +1,7 @@ const should = require('should'); const sinon = require('sinon'); const Promise = require('bluebird'); +const {SafeString} = require('../../../core/frontend/services/proxy'); // Stuff we are testing const helpers = require('../../../core/frontend/helpers'); @@ -28,6 +29,36 @@ describe('{{#get}} helper', function () { sinon.restore(); }); + describe('context preparation', function () { + let browsePostsStub; + const meta = {pagination: {}}; + + beforeEach(function () { + locals = {root: {_locals: {apiVersion: 'canary'}}}; + + browsePostsStub = sinon.stub(api.canary, 'postsPublic').get(() => { + return { + browse: sinon.stub().resolves({posts: [{feature_image_caption: 'A link'}], meta: meta}) + }; + }); + }); + + it('converts html strings to SafeString', function (done) { + helpers.get.call( + {}, + 'posts', + {hash: {}, data: locals, fn: fn, inverse: inverse} + ).then(function () { + fn.called.should.be.true(); + fn.firstCall.args[0].should.be.an.Object().with.property('posts'); + + fn.firstCall.args[0].posts[0].feature_image_caption.should.be.an.instanceOf(SafeString); + + done(); + }).catch(done); + }); + }); + describe('authors v2', function () { let browseAuthorsStub; const meta = {pagination: {}}; diff --git a/test/unit/services/routing/helpers/format-response_spec.js b/test/unit/services/routing/helpers/format-response_spec.js index 9fdf5a66fd..a40bbb304e 100644 --- a/test/unit/services/routing/helpers/format-response_spec.js +++ b/test/unit/services/routing/helpers/format-response_spec.js @@ -1,6 +1,7 @@ const should = require('should'); const testUtils = require('../../../../utils'); const helpers = require('../../../../../core/frontend/services/routing/helpers'); +const {SafeString} = require('../../../../../core/frontend/services/proxy'); describe('Unit - services/routing/helpers/format-response', function () { let posts; @@ -18,26 +19,31 @@ describe('Unit - services/routing/helpers/format-response', function () { describe('entry', function () { it('should return the post object wrapped in a post key', function () { - let formatted; - let postObject = posts[0]; + const postObject = posts[0]; - formatted = helpers.formatResponse.entry(postObject); + const formatted = helpers.formatResponse.entry(postObject); formatted.should.be.an.Object().with.property('post'); formatted.post.should.eql(postObject); }); + + it('should return the post object with html strings converted to SafeString', function () { + const postObject = testUtils.DataGenerator.forKnex.createPost({slug: 'with-caption', feature_image_caption: 'A link'}); + + const formatted = helpers.formatResponse.entry(postObject); + + formatted.post.feature_image_caption.should.be.an.instanceof(SafeString); + }); }); describe('entries', function () { it('should return posts and posts pagination as top level keys', function () { - let formatted; - - let data = { + const data = { posts: posts, meta: {pagination: {}} }; - formatted = helpers.formatResponse.entries(data); + const formatted = helpers.formatResponse.entries(data); formatted.should.be.an.Object().with.properties('posts', 'pagination'); formatted.posts.should.eql(data.posts); @@ -45,24 +51,20 @@ describe('Unit - services/routing/helpers/format-response', function () { }); it('should flatten api read responses which have no pagination data', function () { - let formatted; - - let data = { + const data = { posts: posts, meta: {pagination: {}}, data: {tag: tags} }; - formatted = helpers.formatResponse.entries(data); + const formatted = helpers.formatResponse.entries(data); formatted.should.be.an.Object().with.properties('posts', 'pagination', 'tag'); formatted.tag.should.eql(data.data.tag[0]); }); it('should remove the meta key from api browse responses', function () { - let formatted; - - let data = { + const data = { posts: posts, meta: {pagination: {}}, data: { @@ -73,10 +75,38 @@ describe('Unit - services/routing/helpers/format-response', function () { } }; - formatted = helpers.formatResponse.entries(data); + const formatted = helpers.formatResponse.entries(data); formatted.should.be.an.Object().with.properties('posts', 'pagination', 'featured'); formatted.featured.should.be.an.Object().with.properties('posts', 'pagination'); }); + + it('should return post objects with html strings converted to SafeString', function () { + // `data` contains arrays that have extra properties, need to create them first because the extra props can't be added inline + const featured_multiple = [ + testUtils.DataGenerator.forKnex.createPost({slug: 'featured-two', feature_image_caption: 'Featured link two'}), + testUtils.DataGenerator.forKnex.createPost({slug: 'featured-three', feature_image_caption: 'Featured link three'}) + ]; + featured_multiple.meta = {pagination: {}}; + + const data = { + posts: [ + testUtils.DataGenerator.forKnex.createPost({slug: 'one', feature_image_caption: 'Link one'}), + testUtils.DataGenerator.forKnex.createPost({slug: 'two', feature_image_caption: 'Link two'}) + ], + data: { + featured_single: [testUtils.DataGenerator.forKnex.createPost({slug: 'featured-one', feature_image_caption: 'Featured link one'})], + featured_multiple + } + }; + + const formatted = helpers.formatResponse.entries(data); + + formatted.posts[0].feature_image_caption.should.be.an.instanceof(SafeString); + formatted.posts[1].feature_image_caption.should.be.an.instanceof(SafeString); + formatted.featured_single.feature_image_caption.should.be.an.instanceof(SafeString); + formatted.featured_multiple[0].feature_image_caption.should.be.an.instanceof(SafeString); + formatted.featured_multiple[1].feature_image_caption.should.be.an.instanceof(SafeString); + }); }); });