From cabf78e938d3f7b190bf0d6226aa293a6117f56f Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Mon, 3 Jan 2022 17:45:16 +0000 Subject: [PATCH] Cleaned up `customThemeSettings` labs flag closes https://github.com/TryGhost/Team/issues/1164 - `customThemeSettings` feature is GA so any conditionals can be cleaned up - removed conditional loading of custom theme settings and associated API routes - removed event trigger for reloading custom theme settings when the feature flag is toggled - removed flag from labs GA list --- .../update-local-template-options.js | 7 +------ .../services/themes/activation-bridge.js | 13 +++--------- core/server/services/themes/index.js | 21 ------------------- core/server/web/api/canary/admin/routes.js | 4 ++-- core/shared/labs.js | 1 - .../services/theme-engine/middleware.test.js | 1 - 6 files changed, 6 insertions(+), 41 deletions(-) diff --git a/core/frontend/services/theme-engine/middleware/update-local-template-options.js b/core/frontend/services/theme-engine/middleware/update-local-template-options.js index b01a43834e..4d06331f24 100644 --- a/core/frontend/services/theme-engine/middleware/update-local-template-options.js +++ b/core/frontend/services/theme-engine/middleware/update-local-template-options.js @@ -2,7 +2,6 @@ const _ = require('lodash'); const hbs = require('../engine'); const urlUtils = require('../../../../shared/url-utils'); const customThemeSettingsCache = require('../../../../shared/custom-theme-settings-cache'); -const labs = require('../../../../shared/labs'); const preview = require('../preview'); function updateLocalTemplateOptions(req, res, next) { @@ -17,12 +16,8 @@ function updateLocalTemplateOptions(req, res, next) { const previewData = preview.handle(req, Object.keys(customThemeSettingsCache.getAll())); // strip custom off of preview data so it doesn't get merged into @site - const customThemeSettingsPreviewData = previewData.custom; + const customData = previewData.custom; delete previewData.custom; - let customData = {}; - if (labs.isSet('customThemeSettings')) { - customData = customThemeSettingsPreviewData; - } // update site data with any preview values from the request Object.assign(siteData, previewData); diff --git a/core/server/services/themes/activation-bridge.js b/core/server/services/themes/activation-bridge.js index e67c5efd54..f497a571eb 100644 --- a/core/server/services/themes/activation-bridge.js +++ b/core/server/services/themes/activation-bridge.js @@ -1,6 +1,5 @@ const debug = require('@tryghost/debug')('themes'); const bridge = require('../../../bridge'); -const labs = require('../../../shared/labs'); const customThemeSettings = require('../custom-theme-settings'); /** @@ -11,25 +10,19 @@ module.exports = { activateFromBoot: async (themeName, theme, checkedTheme) => { debug('Activating theme (method A on boot)', themeName); // TODO: probably a better place for this to happen - after successful activation / when reloading site? - if (labs.isSet('customThemeSettings')) { - await customThemeSettings.api.activateTheme(themeName, checkedTheme); - } + await customThemeSettings.api.activateTheme(themeName, checkedTheme); await bridge.activateTheme(theme, checkedTheme); }, activateFromAPI: async (themeName, theme, checkedTheme) => { debug('Activating theme (method B on API "activate")', themeName); // TODO: probably a better place for this to happen - after successful activation / when reloading site? - if (labs.isSet('customThemeSettings')) { - await customThemeSettings.api.activateTheme(themeName, checkedTheme); - } + await customThemeSettings.api.activateTheme(themeName, checkedTheme); await bridge.activateTheme(theme, checkedTheme); }, activateFromAPIOverride: async (themeName, theme, checkedTheme) => { debug('Activating theme (method C on API "override")', themeName); // TODO: probably a better place for this to happen - after successful activation / when reloading site? - if (labs.isSet('customThemeSettings')) { - await customThemeSettings.api.activateTheme(themeName, checkedTheme); - } + await customThemeSettings.api.activateTheme(themeName, checkedTheme); await bridge.activateTheme(theme, checkedTheme); } }; diff --git a/core/server/services/themes/index.js b/core/server/services/themes/index.js index 49462aa421..9f7d20ae3a 100644 --- a/core/server/services/themes/index.js +++ b/core/server/services/themes/index.js @@ -6,12 +6,6 @@ const installer = require('./installer'); const settingsCache = require('../../../shared/settings-cache'); -// Needed for theme re-activation after customThemeSettings flag is toggled -// @TODO: remove when customThemeSettings flag is removed -const labs = require('../../../shared/labs'); -const events = require('../../lib/common/events'); -let _lastLabsValue; - module.exports = { /* * Load the currently active theme @@ -19,21 +13,6 @@ module.exports = { init: async () => { const themeName = settingsCache.get('active_theme'); - /** - * When customThemeSettings labs flag is toggled we need to re-validate and activate - * the active theme so that it's settings are read and synced - * - * @TODO: remove when customThemeSettings labs flag is removed - */ - _lastLabsValue = labs.isSet('customThemeSettings'); - events.on('settings.labs.edited', () => { - if (labs.isSet('customThemeSettings') !== _lastLabsValue) { - _lastLabsValue = labs.isSet('customThemeSettings'); - - activate.activate(settingsCache.get('active_theme')); - } - }); - return activate.loadAndActivate(themeName); }, /** diff --git a/core/server/web/api/canary/admin/routes.js b/core/server/web/api/canary/admin/routes.js index 64b884fefb..d07a162cbb 100644 --- a/core/server/web/api/canary/admin/routes.js +++ b/core/server/web/api/canary/admin/routes.js @@ -303,8 +303,8 @@ module.exports = function apiRoutes() { router.del('/snippets/:id', mw.authAdminApi, http(api.snippets.destroy)); // ## Custom theme settings - router.get('/custom_theme_settings', mw.authAdminApi, labs.enabledMiddleware('customThemeSettings'), http(api.customThemeSettings.browse)); - router.put('/custom_theme_settings', mw.authAdminApi, labs.enabledMiddleware('customThemeSettings'), http(api.customThemeSettings.edit)); + router.get('/custom_theme_settings', mw.authAdminApi, http(api.customThemeSettings.browse)); + router.put('/custom_theme_settings', mw.authAdminApi, http(api.customThemeSettings.edit)); return router; }; diff --git a/core/shared/labs.js b/core/shared/labs.js index 16aec5dcb4..40237a54f4 100644 --- a/core/shared/labs.js +++ b/core/shared/labs.js @@ -15,7 +15,6 @@ const messages = { // flags in this list always return `true`, allows quick global enable prior to full flag removal const GA_FEATURES = [ - 'customThemeSettings', 'nftCard', 'calloutCard', 'videoCard', diff --git a/test/unit/frontend/services/theme-engine/middleware.test.js b/test/unit/frontend/services/theme-engine/middleware.test.js index 9fab51ddeb..d91697d927 100644 --- a/test/unit/frontend/services/theme-engine/middleware.test.js +++ b/test/unit/frontend/services/theme-engine/middleware.test.js @@ -56,7 +56,6 @@ describe('Themes middleware', function () { // if we want to compare it // we will need some unique content members: true, - customThemeSettings: true, offers: true };