From fef0aa44d31f9ab4cfdbbea45d991a0d1c4cbaf5 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Tue, 12 Mar 2019 12:13:47 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20meta=20twitter=5Fimage?= =?UTF-8?q?=20output=20when=20using=20`data:=20post.{slug}`=20for=20routes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #10082 ``` routes: /news/: data: post.news ``` The twitter_image was not available, because the context is [news, post] and the data is in `data.post`. The context helper was incorrect. I think it is still not fully correct, but only focused on this use case. The meta layer needs a full refactoring. --- core/server/data/meta/context_object.js | 31 +++++++++++++------ .../test/unit/data/meta/twitter_image_spec.js | 12 +++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/core/server/data/meta/context_object.js b/core/server/data/meta/context_object.js index 52eabd45ffb..d8b7495497f 100644 --- a/core/server/data/meta/context_object.js +++ b/core/server/data/meta/context_object.js @@ -5,16 +5,29 @@ function getContextObject(data, context) { /** * If the data object does not contain the requested context, we return the fallback object. */ - var blog = { - cover_image: settingsCache.get('cover_image'), - twitter: settingsCache.get('twitter'), - facebook: settingsCache.get('facebook') - }, - contextObject; + const blog = { + cover_image: settingsCache.get('cover_image'), + twitter: settingsCache.get('twitter'), + facebook: settingsCache.get('facebook') + }; - context = _.includes(context, 'page') || _.includes(context, 'amp') ? 'post' : context; - contextObject = data[context] || blog; - return contextObject; + let chosenContext; + + // @TODO: meta layer is very broken, it's really hard to understand what it's doing + // The problem is that handlebars root object is structured differently. Sometimes the object is flat on data + // and sometimes the object is part of a key e.g. data.post. This needs to be prepared at the very first stage and not in each helper. + if (_.includes(context, 'page') || _.includes(context, 'amp') && data.post) { + chosenContext = data.post; + } else if (_.includes(context, 'post') && data.post) { + chosenContext = data.post; + } else if (data[context]) { + // @NOTE: This is confusing as hell. It tries to get data[['author']], which works, but coincidence? + chosenContext = data[context]; + } else { + chosenContext = blog; + } + + return chosenContext; } module.exports = getContextObject; diff --git a/core/test/unit/data/meta/twitter_image_spec.js b/core/test/unit/data/meta/twitter_image_spec.js index 9b26f60d111..a8ecddf33e2 100644 --- a/core/test/unit/data/meta/twitter_image_spec.js +++ b/core/test/unit/data/meta/twitter_image_spec.js @@ -38,6 +38,18 @@ describe('getTwitterImage', function () { twitterImageUrl.should.match(/\/content\/images\/my-special-twitter-image\.jpg$/); }); + it('should return absolute url for Twitter image in post context', function () { + var twitterImageUrl = getTwitterImage({ + context: ['news', 'post'], + post: { + feature_image: '/content/images/my-test-image.jpg', + twitter_image: '/content/images/my-special-twitter-image.jpg' + } + }); + twitterImageUrl.should.not.equal('/content/images/my-special-twitter-image.jpg'); + twitterImageUrl.should.match(/\/content\/images\/my-special-twitter-image\.jpg$/); + }); + it('should return absolute url for feature image in post context', function () { var twitterImageUrl = getTwitterImage({ context: ['post'],