From 0d9955538d588fd577f479b25eeeac8ec3c80d4a Mon Sep 17 00:00:00 2001 From: Sag Date: Thu, 8 Aug 2024 17:47:51 +0200 Subject: [PATCH] Removed fallback Ghost icon in staff notifications (#20731) ref https://linear.app/tryghost/issue/PLG-150 - if the publication has no custom icon, staff notifications do not render the Ghost icon as fallback anymore --- ghost/core/core/frontend/meta/blog-logo.js | 2 +- ghost/core/core/server/lib/image/BlogIcon.js | 18 +++++++++++++----- ghost/core/core/server/services/slack.js | 6 +++--- .../unit/server/lib/image/blog-icon.test.js | 16 +++++++++++++--- ghost/staff-service/lib/StaffServiceEmails.js | 10 +++++----- 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/ghost/core/core/frontend/meta/blog-logo.js b/ghost/core/core/frontend/meta/blog-logo.js index e472b68be3..dd054ec8e3 100644 --- a/ghost/core/core/frontend/meta/blog-logo.js +++ b/ghost/core/core/frontend/meta/blog-logo.js @@ -11,7 +11,7 @@ function getBlogLogo() { // CASE: no publication logo is updated. We can try to use either an uploaded publication icon // or use the default one to make // Google happy with it. See https://github.com/TryGhost/Ghost/issues/7558 - logo.url = blogIcon.getIconUrl(true); + logo.url = blogIcon.getIconUrl({absolute: true}); } return logo; diff --git a/ghost/core/core/server/lib/image/BlogIcon.js b/ghost/core/core/server/lib/image/BlogIcon.js index af673419c0..2238dfc04f 100644 --- a/ghost/core/core/server/lib/image/BlogIcon.js +++ b/ghost/core/core/server/lib/image/BlogIcon.js @@ -98,13 +98,17 @@ class BlogIcon { } /** - * Return URL for Blog icon: [subdirectory or not]favicon.[ico, jpeg, or png] - * Always returns {string} getIconUrl - * @returns {string} [subdirectory or not]favicon.[ico, jpeg, or png] + * Return URL for blog icon, if available: [subdirectory or not]favicon.[ico, jpeg, or png] + * Otherwise, fallbacks to the default Ghost favicon.ico file, if requested + * Otherwise, returns null + * @param {Object} [options] + * @param {boolean} [options.absolute] - if true, return absolute URL. Default: false + * @param {boolean} [options.fallbackToDefault] - if true, fallbacks to Ghost's default favicon.ico when no blog icon is found. Default: true + * @returns {string|null} [subdirectory or not]favicon.[ico, jpeg, or png] or null * @description Checks if we have a custom uploaded icon and the extension of it. If no custom uploaded icon * exists, we're returning the default `favicon.ico` */ - getIconUrl(absolute) { + getIconUrl({absolute = false, fallbackToDefault = true} = {}) { const blogIcon = this.settingsCache.get('icon'); if (blogIcon) { @@ -124,9 +128,13 @@ class BlogIcon { const sizedIcon = blogIcon.replace(/\/content\/images\//, '/content/images/size/w256h256/'); return this.urlUtils.urlFor({relativeUrl: sizedIcon}, absolute ? true : undefined); - } else { + } + + if (fallbackToDefault) { return this.urlUtils.urlFor({relativeUrl: '/favicon.ico'}, absolute ? true : undefined); } + + return null; } /** diff --git a/ghost/core/core/server/services/slack.js b/ghost/core/core/server/services/slack.js index be9916e4bc..e25d2a34a3 100644 --- a/ghost/core/core/server/services/slack.js +++ b/ghost/core/core/server/services/slack.js @@ -110,7 +110,7 @@ function ping(post) { // if it is a post or a test message to check webhook working. text: `Notification from *${blogTitle}* :ghost:`, unfurl_links: true, - icon_url: blogIcon.getIconUrl(true), + icon_url: blogIcon.getIconUrl({absolute: true}), username: slackSettings.username, // We don't want to send attachment if it is a test notification. attachments: [ @@ -141,7 +141,7 @@ function ping(post) { } ], footer: blogTitle, - footer_icon: blogIcon.getIconUrl(true), + footer_icon: blogIcon.getIconUrl({absolute: true}), ts: moment().unix() } ] @@ -150,7 +150,7 @@ function ping(post) { slackData = { text: message, unfurl_links: true, - icon_url: blogIcon.getIconUrl(true), + icon_url: blogIcon.getIconUrl({absolute: true}), username: slackSettings.username }; } diff --git a/ghost/core/test/unit/server/lib/image/blog-icon.test.js b/ghost/core/test/unit/server/lib/image/blog-icon.test.js index 5d3e9de9c1..4e9e9996f2 100644 --- a/ghost/core/test/unit/server/lib/image/blog-icon.test.js +++ b/ghost/core/test/unit/server/lib/image/blog-icon.test.js @@ -51,7 +51,7 @@ describe('lib/image: blog icon', function () { } } }}); - blogIcon.getIconUrl(true).should.deepEqual([{relativeUrl: '/content/images/2017/04/my-icon.ico'}, true]); + blogIcon.getIconUrl({absolute: true}).should.deepEqual([{relativeUrl: '/content/images/2017/04/my-icon.ico'}, true]); }); it('custom uploaded png blog icon', function () { @@ -64,7 +64,7 @@ describe('lib/image: blog icon', function () { } } }}); - blogIcon.getIconUrl(true).should.deepEqual([{relativeUrl: '/content/images/size/w256h256/2017/04/my-icon.png'}, true]); + blogIcon.getIconUrl({absolute: true}).should.deepEqual([{relativeUrl: '/content/images/size/w256h256/2017/04/my-icon.png'}, true]); }); it('default ico blog icon', function () { @@ -73,7 +73,17 @@ describe('lib/image: blog icon', function () { }, settingsCache: { get: () => {} }}); - blogIcon.getIconUrl(true).should.deepEqual([{relativeUrl: '/favicon.ico'}, true]); + blogIcon.getIconUrl({absolute: true}).should.deepEqual([{relativeUrl: '/favicon.ico'}, true]); + }); + + it('returns null if no fallback is requested', function () { + const blogIcon = new BlogIcon({config: {}, storageUtils: {}, urlUtils: { + urlFor: (key, boolean) => [key, boolean] + }, settingsCache: { + get: () => {} + }}); + + should.equal(blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), null); }); }); }); diff --git a/ghost/staff-service/lib/StaffServiceEmails.js b/ghost/staff-service/lib/StaffServiceEmails.js index fa15117087..4022de8ae6 100644 --- a/ghost/staff-service/lib/StaffServiceEmails.js +++ b/ghost/staff-service/lib/StaffServiceEmails.js @@ -45,7 +45,7 @@ class StaffServiceEmails { attributionUrl: attribution?.url || '', referrerSource: attribution?.referrerSource, siteTitle: this.settingsCache.get('title'), - siteIconUrl: this.blogIcon.getIconUrl(true), + siteIconUrl: this.blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), siteUrl: this.urlUtils.getSiteUrl(), siteDomain: this.siteDomain, accentColor: this.settingsCache.get('accent_color'), @@ -105,7 +105,7 @@ class StaffServiceEmails { offerData, subscriptionData, siteTitle: this.settingsCache.get('title'), - siteIconUrl: this.blogIcon.getIconUrl(true), + siteIconUrl: this.blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), siteUrl: this.urlUtils.getSiteUrl(), siteDomain: this.siteDomain, accentColor: this.settingsCache.get('accent_color'), @@ -156,7 +156,7 @@ class StaffServiceEmails { tierData, subscriptionData, siteTitle: this.settingsCache.get('title'), - siteIconUrl: this.blogIcon.getIconUrl(true), + siteIconUrl: this.blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), siteUrl: this.urlUtils.getSiteUrl(), siteDomain: this.siteDomain, accentColor: this.settingsCache.get('accent_color'), @@ -186,7 +186,7 @@ class StaffServiceEmails { return { siteTitle: this.settingsCache.get('title'), - siteIconUrl: this.blogIcon.getIconUrl(true), + siteIconUrl: this.blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), siteUrl: this.urlUtils.getSiteUrl(), siteDomain: this.siteDomain, accentColor: this.settingsCache.get('accent_color'), @@ -287,7 +287,7 @@ class StaffServiceEmails { const templateData = { siteTitle: this.settingsCache.get('title'), siteUrl: this.urlUtils.getSiteUrl(), - siteIconUrl: this.blogIcon.getIconUrl(true), + siteIconUrl: this.blogIcon.getIconUrl({absolute: true, fallbackToDefault: false}), siteDomain: this.siteDomain, fromEmail: this.fromEmailAddress, toEmail: to,