From d9fef821709a581639224aed6e39979b21c31cef Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Thu, 25 Jul 2019 11:08:29 +0200 Subject: [PATCH] Added global site SEO fields to be used in theme helpers (#10930) #10921 - Changed {{meta_title}} helper to use site meta_title' field - Changed {{meta_description}} helper to use site 'meta_description' field - Changed {{og_image}} helper to use site 'og_image' field - Added site title handling for og/twitter metadata - Refactored use of 'blog' in variable name in favor of 'site' - Extended meta_description test suite with 'home' context cases - Changed {{twitter_image}} helper to use site 'twitter_image' field - Added ghost_head test for site metadata - Renamed blog->site in variable names for touched files --- core/frontend/meta/description.js | 23 +- core/frontend/meta/og_image.js | 16 +- core/frontend/meta/title.js | 34 +- core/frontend/meta/twitter_image.js | 16 +- core/test/unit/data/meta/description_spec.js | 67 +++- core/test/unit/data/meta/og_image_spec.js | 43 ++- core/test/unit/data/meta/title_spec.js | 74 +++-- .../test/unit/data/meta/twitter_image_spec.js | 43 ++- core/test/unit/helpers/ghost_head_spec.js | 48 +++ .../unit/helpers/meta_description_spec.js | 250 ++++++++------- core/test/unit/helpers/meta_title_spec.js | 290 +++++++++++------- 11 files changed, 618 insertions(+), 286 deletions(-) diff --git a/core/frontend/meta/description.js b/core/frontend/meta/description.js index 128c7dbe3c..0697244174 100644 --- a/core/frontend/meta/description.js +++ b/core/frontend/meta/description.js @@ -1,22 +1,27 @@ -var _ = require('lodash'), - settingsCache = require('../../server/services/settings/cache'); +const _ = require('lodash'); +const settingsCache = require('../../server/services/settings/cache'); function getDescription(data, root, options) { - var description = '', - postSdDescription, - context = root ? root.context : null, - blogDescription = settingsCache.get('description'); + const context = root ? root.context : null; + const siteDescription = settingsCache.get('meta_description') || settingsCache.get('description'); + + let description = ''; + let postSdDescription; options = options ? options : {}; - // We only return meta_description if provided. Only exception is the Blog - // description, which doesn't rely on meta_description. + // We only return meta_description if provided if (data.meta_description) { description = data.meta_description; } else if (_.includes(context, 'paged')) { description = ''; } else if (_.includes(context, 'home')) { - description = blogDescription; + if (options && options.property) { + const siteSdDescription = options.property + '_description'; + description = settingsCache.get(siteSdDescription) || ''; + } else { + description = siteDescription; + } } else if (_.includes(context, 'author') && data.author) { // The usage of meta data fields for author is currently not implemented. // We do have meta_description and meta_title fields diff --git a/core/frontend/meta/og_image.js b/core/frontend/meta/og_image.js index d33608947d..b34a97b9af 100644 --- a/core/frontend/meta/og_image.js +++ b/core/frontend/meta/og_image.js @@ -1,10 +1,12 @@ -var urlUtils = require('../../server/lib/url-utils'), - getContextObject = require('./context_object.js'), - _ = require('lodash'); +const _ = require('lodash'); +const getContextObject = require('./context_object.js'); +const urlUtils = require('../../server/lib/url-utils'); +const settingsCache = require('../../server/services/settings/cache'); function getOgImage(data) { - var context = data.context ? data.context : null, - contextObject = getContextObject(data, context); + const context = data.context ? data.context : null; + const contextObject = getContextObject(data, context); + const siteOgImage = settingsCache.get('og_image'); if (_.includes(context, 'post') || _.includes(context, 'page') || _.includes(context, 'amp')) { if (contextObject.og_image) { @@ -14,6 +16,10 @@ function getOgImage(data) { } } + if (_.includes(context, 'home') && siteOgImage) { + return urlUtils.urlFor('image', {image: siteOgImage}, true); + } + return null; } diff --git a/core/frontend/meta/title.js b/core/frontend/meta/title.js index 26d5f79bf7..7ddd7082b6 100644 --- a/core/frontend/meta/title.js +++ b/core/frontend/meta/title.js @@ -1,13 +1,14 @@ -var _ = require('lodash'), - settingsCache = require('../../server/services/settings/cache'); +const _ = require('lodash'); +const settingsCache = require('../../server/services/settings/cache'); function getTitle(data, root, options) { - var title = '', - context = root ? root.context : null, - postSdTitle, - blogTitle = settingsCache.get('title'), - pagination = root ? root.pagination : null, - pageString = ''; + const context = root ? root.context : null; + const siteTitle = settingsCache.get('meta_title') || settingsCache.get('title'); + const pagination = root ? root.pagination : null; + + let title = ''; + let postSdTitle; + let pageString = ''; options = options ? options : {}; @@ -20,19 +21,24 @@ function getTitle(data, root, options) { title = data.meta_title; // Home title } else if (_.includes(context, 'home')) { - title = blogTitle; + if (options && options.property) { + const siteSdTitle = options.property + '_title'; + title = settingsCache.get(siteSdTitle) || ''; + } else { + title = siteTitle; + } // Author title, paged } else if (_.includes(context, 'author') && data.author && _.includes(context, 'paged')) { - title = data.author.name + ' - ' + blogTitle + pageString; + title = data.author.name + ' - ' + siteTitle + pageString; // Author title, index } else if (_.includes(context, 'author') && data.author) { - title = data.author.name + ' - ' + blogTitle; + title = data.author.name + ' - ' + siteTitle; // Tag title, paged } else if (_.includes(context, 'tag') && data.tag && _.includes(context, 'paged')) { - title = data.tag.meta_title || data.tag.name + ' - ' + blogTitle + pageString; + title = data.tag.meta_title || data.tag.name + ' - ' + siteTitle + pageString; // Tag title, index } else if (_.includes(context, 'tag') && data.tag) { - title = data.tag.meta_title || data.tag.name + ' - ' + blogTitle; + title = data.tag.meta_title || data.tag.name + ' - ' + siteTitle; // Post title } else if (_.includes(context, 'post') && data.post) { if (options && options.property) { @@ -59,7 +65,7 @@ function getTitle(data, root, options) { } // Fallback } else { - title = blogTitle + pageString; + title = siteTitle + pageString; } return (title || '').trim(); diff --git a/core/frontend/meta/twitter_image.js b/core/frontend/meta/twitter_image.js index cfafd594b3..20b96efa70 100644 --- a/core/frontend/meta/twitter_image.js +++ b/core/frontend/meta/twitter_image.js @@ -1,10 +1,12 @@ -var urlUtils = require('../../server/lib/url-utils'), - getContextObject = require('./context_object.js'), - _ = require('lodash'); +const _ = require('lodash'); +const urlUtils = require('../../server/lib/url-utils'); +const getContextObject = require('./context_object.js'); +const settingsCache = require('../../server/services/settings/cache'); function getTwitterImage(data) { - var context = data.context ? data.context : null, - contextObject = getContextObject(data, context); + const context = data.context ? data.context : null; + const contextObject = getContextObject(data, context); + const siteTwitterImage = settingsCache.get('twitter_image'); if (_.includes(context, 'post') || _.includes(context, 'page') || _.includes(context, 'amp')) { if (contextObject.twitter_image) { @@ -14,6 +16,10 @@ function getTwitterImage(data) { } } + if (_.includes(context, 'home') && siteTwitterImage) { + return urlUtils.urlFor('image', {image: siteTwitterImage}, true); + } + return null; } diff --git a/core/test/unit/data/meta/description_spec.js b/core/test/unit/data/meta/description_spec.js index cfcebb8260..43adbb0ac9 100644 --- a/core/test/unit/data/meta/description_spec.js +++ b/core/test/unit/data/meta/description_spec.js @@ -1,7 +1,70 @@ -var should = require('should'), - getMetaDescription = require('../../../../frontend/meta/description'); +const should = require('should'); +const sinon = require('sinon'); +const getMetaDescription = require('../../../../frontend/meta/description'); +const settingsCache = require('../../../../server/services/settings/cache'); describe('getMetaDescription', function () { + let localSettingsCache = {}; + + before(function () { + sinon.stub(settingsCache, 'get').callsFake(function (key) { + return localSettingsCache[key]; + }); + }); + + after(function () { + sinon.restore(); + }); + + afterEach(function () { + localSettingsCache = {}; + }); + + it('should return site description when in home context', function () { + localSettingsCache.description = 'Site description'; + + var description = getMetaDescription({ + }, { + context: ['home'] + }); + description.should.equal('Site description'); + }); + + it('should return site OG description when in home context', function () { + localSettingsCache.og_description = 'My site Facebook description'; + + var description = getMetaDescription({ + }, { + context: ['home'] + }, { + property: 'og' + }); + description.should.equal('My site Facebook description'); + }); + + it('should return site twitter description when in home context', function () { + localSettingsCache.twitter_description = 'My site Twitter description'; + + var description = getMetaDescription({ + }, { + context: ['home'] + }, { + property: 'twitter' + }); + description.should.equal('My site Twitter description'); + }); + + it('should return site meta_description when it is defined and in home context', function () { + localSettingsCache.description = 'Site description'; + localSettingsCache.meta_description = 'Site meta description'; + + var description = getMetaDescription({ + }, { + context: ['home'] + }); + description.should.equal('Site meta description'); + }); + it('should return meta_description if on data root', function () { var description = getMetaDescription({ meta_description: 'My test description.' diff --git a/core/test/unit/data/meta/og_image_spec.js b/core/test/unit/data/meta/og_image_spec.js index 2a8ef756c6..5889932092 100644 --- a/core/test/unit/data/meta/og_image_spec.js +++ b/core/test/unit/data/meta/og_image_spec.js @@ -1,8 +1,45 @@ -var should = require('should'), - getOgImage = require('../../../../frontend/meta/og_image'); +const should = require('should'); +const sinon = require('sinon'); +const getOgImage = require('../../../../frontend/meta/og_image'); +const settingsCache = require('../../../../server/services/settings/cache'); describe('getOgImage', function () { - it('[home] should return null if not post context [home]', function () { + describe('[home]', function () { + it('should return null if [home] context and no og_image set', function () { + sinon.stub(settingsCache, 'get').callsFake(function (key) { + return { + og_image: null + }[key]; + }); + + var ogImageUrl = getOgImage({ + context: ['home'], + home: {} + }); + should(ogImageUrl).equal(null); + + sinon.restore(); + }); + + it('should return image URL if [home] context and og_image set', function () { + sinon.stub(settingsCache, 'get').callsFake(function (key) { + return { + og_image: '/content/images/home-og.jpg' + }[key]; + }); + + var ogImageUrl = getOgImage({ + context: ['home'], + home: {} + }); + ogImageUrl.should.not.equal('/content/images/home-og.jpg'); + ogImageUrl.should.match(/\/content\/images\/home-og\.jpg$/); + + sinon.restore(); + }); + }); + + it('[home] should return null if not post context [home] and no og_image set', function () { var ogImageUrl = getOgImage({ context: ['home'], home: {} diff --git a/core/test/unit/data/meta/title_spec.js b/core/test/unit/data/meta/title_spec.js index 31db341b1e..811d11b88d 100644 --- a/core/test/unit/data/meta/title_spec.js +++ b/core/test/unit/data/meta/title_spec.js @@ -25,15 +25,51 @@ describe('getTitle', function () { title.should.equal('My test title'); }); - it('should return blog title if on home', function () { - localSettingsCache.title = 'My blog title'; + it('should return site title if on home', function () { + localSettingsCache.title = 'My site title'; var title = getTitle({}, {context: 'home'}); - title.should.equal('My blog title'); + title.should.equal('My site title'); }); - it('should return author name - blog title if on data author page', function () { - localSettingsCache.title = 'My blog title 2'; + it('should return site meta_title if on home and mata_title present', function () { + localSettingsCache.title = 'My site title'; + localSettingsCache.meta_title = 'My site meta title'; + + var title = getTitle({}, {context: 'home'}); + title.should.equal('My site meta title'); + }); + + it('should return facebook site title if in home context', function () { + localSettingsCache.title = 'My site title'; + localSettingsCache.og_title = 'My site facebook meta title'; + + var title = getTitle({ + }, { + context: ['home'] + }, { + property: 'og' + }); + + title.should.equal('My site facebook meta title'); + }); + + it('should return twitter site title if in home context', function () { + localSettingsCache.title = 'My site title'; + localSettingsCache.twitter_title = 'My site twitter meta title'; + + var title = getTitle({ + }, { + context: ['home'] + }, { + property: 'twitter' + }); + + title.should.equal('My site twitter meta title'); + }); + + it('should return author name - site title if on data author page', function () { + localSettingsCache.title = 'My site title 2'; var title = getTitle({ author: { @@ -41,11 +77,11 @@ describe('getTitle', function () { } }, {context: ['author']}); - title.should.equal('Author Name - My blog title 2'); + title.should.equal('Author Name - My site title 2'); }); it('should return author page title if on data author page with more then one page', function () { - localSettingsCache.title = 'My blog title 2'; + localSettingsCache.title = 'My site title 2'; var title = getTitle({ author: { @@ -59,11 +95,11 @@ describe('getTitle', function () { } }); - title.should.equal('Author Name - My blog title 2 (Page 3)'); + title.should.equal('Author Name - My site title 2 (Page 3)'); }); - it('should return tag name - blog title if on data tag page no meta_title', function () { - localSettingsCache.title = 'My blog title 3'; + it('should return tag name - site title if on data tag page no meta_title', function () { + localSettingsCache.title = 'My site title 3'; var title = getTitle({ tag: { @@ -71,11 +107,11 @@ describe('getTitle', function () { } }, {context: ['tag']}); - title.should.equal('Tag Name - My blog title 3'); + title.should.equal('Tag Name - My site title 3'); }); - it('should return tag name - blog title if on data tag page no meta_title (Page #)', function () { - localSettingsCache.title = 'My blog title 3'; + it('should return tag name - site title if on data tag page no meta_title (Page #)', function () { + localSettingsCache.title = 'My site title 3'; var title = getTitle({ tag: { @@ -89,11 +125,11 @@ describe('getTitle', function () { } }); - title.should.equal('Tag Name - My blog title 3 (Page 39)'); + title.should.equal('Tag Name - My site title 3 (Page 39)'); }); it('should return translated pagination-string if passed in options object', function () { - localSettingsCache.title = 'This is my blog title'; + localSettingsCache.title = 'This is my site title'; var title = getTitle({ tag: { @@ -111,7 +147,7 @@ describe('getTitle', function () { } }); - title.should.equal('Tag Name - This is my blog title p.23'); + title.should.equal('Tag Name - This is my site title p.23'); }); it('should return tag meta_title if in tag data', function () { @@ -242,8 +278,8 @@ describe('getTitle', function () { title.should.equal('My Tag Meta Title Post!'); }); - it('should return blog title with page if unknown type', function () { - localSettingsCache.title = 'My blog title 4'; + it('should return site title with page if unknown type', function () { + localSettingsCache.title = 'My site title 4'; var title = getTitle({}, { context: ['paged'], @@ -253,6 +289,6 @@ describe('getTitle', function () { } }); - title.should.equal('My blog title 4 (Page 35)'); + title.should.equal('My site title 4 (Page 35)'); }); }); diff --git a/core/test/unit/data/meta/twitter_image_spec.js b/core/test/unit/data/meta/twitter_image_spec.js index 2da61f43b2..ee6fde9116 100644 --- a/core/test/unit/data/meta/twitter_image_spec.js +++ b/core/test/unit/data/meta/twitter_image_spec.js @@ -1,13 +1,42 @@ -var should = require('should'), - getTwitterImage = require('../../../../frontend/meta/twitter_image'); +const should = require('should'); +const sinon = require('sinon'); +const getTwitterImage = require('../../../../frontend/meta/twitter_image'); +const settingsCache = require('../../../../server/services/settings/cache'); describe('getTwitterImage', function () { - it('[home] should return null if not post context [home]', function () { - var twitterImageUrl = getTwitterImage({ - context: ['home'], - home: {} + describe('[home]', function () { + it('should return null if [home] context and no twitter_image set', function () { + sinon.stub(settingsCache, 'get').callsFake(function (key) { + return { + twitter_image: null + }[key]; + }); + + var twitterImageUrl = getTwitterImage({ + context: ['home'], + home: {} + }); + should(twitterImageUrl).equal(null); + + sinon.restore(); + }); + + it('should return image URL if [home] context and twitter_image set', function () { + sinon.stub(settingsCache, 'get').callsFake(function (key) { + return { + twitter_image: '/content/images/home-twitter.jpg' + }[key]; + }); + + var twitterImageUrl = getTwitterImage({ + context: ['home'], + home: {} + }); + twitterImageUrl.should.not.equal('/content/images/home-twitter.jpg'); + twitterImageUrl.should.match(/\/content\/images\/home-twitter\.jpg$/); + + sinon.restore(); }); - should(twitterImageUrl).equal(null); }); it('should return null if not post context [author]', function () { diff --git a/core/test/unit/helpers/ghost_head_spec.js b/core/test/unit/helpers/ghost_head_spec.js index cf7e8bfe96..b0ec46431f 100644 --- a/core/test/unit/helpers/ghost_head_spec.js +++ b/core/test/unit/helpers/ghost_head_spec.js @@ -369,6 +369,54 @@ describe('{{ghost_head}} helper', function () { }).catch(done); }); + it('returns meta structured data on homepage with site metadata defined', function (done) { + settingsCache.get.withArgs('meta_description').returns('site SEO description'); + + settingsCache.get.withArgs('og_title').returns('facebook site title'); + settingsCache.get.withArgs('og_description').returns('facebook site description'); + settingsCache.get.withArgs('og_image').returns('/content/images/facebook-image.png'); + + settingsCache.get.withArgs('twitter_title').returns('twitter site title'); + settingsCache.get.withArgs('twitter_description').returns('twitter site description'); + settingsCache.get.withArgs('twitter_image').returns('/content/images/twitter-image.png'); + + helpers.ghost_head(testUtils.createHbsResponse({ + locals: { + relativeUrl: '/', + context: ['home', 'index'], + safeVersion: '0.3' + } + })).then(function (rendered) { + should.exist(rendered); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(//); + rendered.string.should.match(/