diff --git a/ghost/admin/app/components/gh-nav-menu/design.js b/ghost/admin/app/components/gh-nav-menu/design.js index e33c2eb5e0..bf1d2834a3 100644 --- a/ghost/admin/app/components/gh-nav-menu/design.js +++ b/ghost/admin/app/components/gh-nav-menu/design.js @@ -72,7 +72,7 @@ export default class DesignMenuComponent extends Component { @action handleThemeSettingChange() { - this.customThemeSettings.rebuildSettingGroups(); + this.customThemeSettings.updateSettingsVisibility(); this.themeManagement.updatePreviewHtmlTask.perform(); } diff --git a/ghost/admin/app/components/settings/design/theme-settings-form.hbs b/ghost/admin/app/components/settings/design/theme-settings-form.hbs index a3cc9b2919..e872626ebb 100644 --- a/ghost/admin/app/components/settings/design/theme-settings-form.hbs +++ b/ghost/admin/app/components/settings/design/theme-settings-form.hbs @@ -1,20 +1,22 @@
{{#each @themeSettings as |setting index|}} - {{#if (eq setting.type "select")}} - - {{/if}} - {{#if (eq setting.type "boolean")}} - - {{/if}} - {{#if (eq setting.type "color")}} - - {{/if}} - {{#if (eq setting.type "text")}} - - {{/if}} - {{#if (eq setting.type "image")}} - + {{#if setting.visible}} + {{#if (eq setting.type "select")}} + + {{/if}} + {{#if (eq setting.type "boolean")}} + + {{/if}} + {{#if (eq setting.type "color")}} + + {{/if}} + {{#if (eq setting.type "text")}} + + {{/if}} + {{#if (eq setting.type "image")}} + + {{/if}} {{/if}} {{/each}} diff --git a/ghost/admin/app/models/custom-theme-setting.js b/ghost/admin/app/models/custom-theme-setting.js index b93eb88cba..1dee91bc2e 100644 --- a/ghost/admin/app/models/custom-theme-setting.js +++ b/ghost/admin/app/models/custom-theme-setting.js @@ -8,5 +8,6 @@ export default Model.extend({ default: attr('string'), value: attr(), group: attr('string'), - visibility: attr('string') + visibility: attr('string'), + visible: attr('boolean', {defaultValue: true}) }); diff --git a/ghost/admin/app/services/custom-theme-settings.js b/ghost/admin/app/services/custom-theme-settings.js index 6826626f7d..5008dc75f6 100644 --- a/ghost/admin/app/services/custom-theme-settings.js +++ b/ghost/admin/app/services/custom-theme-settings.js @@ -5,6 +5,8 @@ import {run} from '@ember/runloop'; import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; +const HIDDEN_SETTING_VALUE = null; + export default class CustomThemeSettingsServices extends Service { @service store; @@ -34,7 +36,7 @@ export default class CustomThemeSettingsServices extends Service { const keyValue = {}; this.settings.forEach((setting) => { - keyValue[setting.key] = setting.value; + keyValue[setting.key] = this._isSettingVisible(setting) ? setting.value : HIDDEN_SETTING_VALUE; }); return keyValue; @@ -64,6 +66,8 @@ export default class CustomThemeSettingsServices extends Service { this.settings = settings; this.settingGroups = this._buildSettingGroups(settings); + this.updateSettingsVisibility(); + this._hasLoaded = true; return this.settings; @@ -93,8 +97,10 @@ export default class CustomThemeSettingsServices extends Service { this.settings.forEach(setting => setting.rollbackAttributes()); } - rebuildSettingGroups() { - this.settingGroups = this._buildSettingGroups(this.settings); + updateSettingsVisibility() { + this.settings.forEach((setting) => { + setting.visible = this._isSettingVisible(setting); + }); } _buildSettingGroups(settings) { @@ -116,15 +122,7 @@ export default class CustomThemeSettingsServices extends Service { } this.KNOWN_GROUPS.forEach((knownGroup) => { - const groupSettings = settings - .filter(setting => setting.group === knownGroup.key) - .filter((setting) => { - if (setting.visibility) { - return nql(setting.visibility).queryJSON(this.keyValueObject); - } - - return true; - }); + const groupSettings = settings.filter(setting => setting.group === knownGroup.key); if (groupSettings.length) { groups.push(Object.assign({}, knownGroup, {settings: groupSettings})); @@ -133,4 +131,14 @@ export default class CustomThemeSettingsServices extends Service { return groups; } + + _isSettingVisible(setting) { + if (!setting.visibility) { + return true; + } + + const settingsMap = this.settings.reduce((map, {key, value}) => ({...map, [key]: value}), {}); + + return nql(setting.visibility).queryJSON(settingsMap); + } } diff --git a/ghost/custom-theme-settings-service/lib/CustomThemeSettingsService.js b/ghost/custom-theme-settings-service/lib/CustomThemeSettingsService.js index 1b5e252b8c..4689843afe 100644 --- a/ghost/custom-theme-settings-service/lib/CustomThemeSettingsService.js +++ b/ghost/custom-theme-settings-service/lib/CustomThemeSettingsService.js @@ -1,5 +1,6 @@ const _ = require('lodash'); const BREAD = require('./CustomThemeSettingsBREADService'); +const nql = require('@tryghost/nql'); const tpl = require('@tryghost/tpl'); const {ValidationError} = require('@tryghost/errors'); const debug = require('@tryghost/debug')('custom-theme-settings-service'); @@ -10,6 +11,8 @@ const messages = { invalidValueForSetting: 'Invalid value for \'{key}\'. The value must follow this format: {format}.' }; +const HIDDEN_SETTING_VALUE = null; + module.exports = class CustomThemeSettingsService { /** * @param {Object} options @@ -150,11 +153,15 @@ module.exports = class CustomThemeSettingsService { this._activeThemeSettings[setting.key].value = setting.value; } + const settingsObjects = this.listSettings(); + // update the public cache - this._valueCache.populate(this.listSettings()); + this._valueCache.populate( + this._computeCachedSettings(settingsObjects) + ); // return full setting objects - return this.listSettings(); + return settingsObjects; } // Private ----------------------------------------------------------------- @@ -233,7 +240,9 @@ module.exports = class CustomThemeSettingsService { return; } - this._valueCache.populate(settings); + this._valueCache.populate( + this._computeCachedSettings(settings) + ); } /** @@ -264,4 +273,26 @@ module.exports = class CustomThemeSettingsService { this._activeThemeSettings = activeThemeSettings; } + + /** + * Compute the settings to cache, taking into account visibility rules + * + * @param {Object[]} settings - list of setting objects + * @returns {Object[]} - list of setting objects with visibility rules applied + * @private + */ + _computeCachedSettings(settings) { + const settingsMap = settings.reduce((map, {key, value}) => ({...map, [key]: value}), {}); + + return settings.map((setting) => { + return { + ...setting, + // If a setting is not visible, set the value to HIDDEN_SETTING_VALUE so that it is not exposed in the cache + // (meaning it also won't be exposed in the theme when rendering) + value: setting.visibility && nql(setting.visibility).queryJSON(settingsMap) === false + ? HIDDEN_SETTING_VALUE + : setting.value + }; + }); + } }; diff --git a/ghost/custom-theme-settings-service/package.json b/ghost/custom-theme-settings-service/package.json index d6ff8e864f..7b4ef23d86 100644 --- a/ghost/custom-theme-settings-service/package.json +++ b/ghost/custom-theme-settings-service/package.json @@ -27,6 +27,7 @@ "dependencies": { "@tryghost/debug": "0.1.24", "@tryghost/errors": "1.2.24", + "@tryghost/nql": "0.11.0", "@tryghost/tpl": "0.1.24", "lodash": "4.17.21" } diff --git a/ghost/custom-theme-settings-service/test/service.test.js b/ghost/custom-theme-settings-service/test/service.test.js index 56df5c8a7c..9a46eefd3d 100644 --- a/ghost/custom-theme-settings-service/test/service.test.js +++ b/ghost/custom-theme-settings-service/test/service.test.js @@ -875,5 +875,36 @@ describe('Service', function () { }] ).should.be.resolved(); }); + + it('does not expose hidden settings in the public cache', async function () { + const HIDDEN_SETTING_VALUE = null; + + const settingName = 'foo'; + const settingDefinition = { + type: 'select', + options: ['Foo', 'Bar', 'Baz'], + default: 'Foo', + visibility: 'some_other_setting:bar' + }; + + await service.activateTheme('test', { + name: 'test', + customSettings: { + [settingName]: settingDefinition + } + }); + + await service.updateSettings( + [{ + key: settingName, + value: 'Foo', + ...settingDefinition + }] + ); + + cache.getAll().should.deepEqual({ + [settingName]: HIDDEN_SETTING_VALUE + }); + }); }); });