Ensure hidden settings return falsy when used with the @custom helper (#17920)

no refs

When a theme setting is hidden, it should return a falsy value when used

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();
     }
 

ghost/admin/app/components/settings/design/theme-settings-form.hbs

@@ -1,20 +1,22 @@
 <div class="gh-stack">
     <form>
         {{#each @themeSettings as |setting index|}}
-            {{#if (eq setting.type "select")}}
+            {{#if setting.visible}}
-                <CustomThemeSettings::Select @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{#if (eq setting.type "select")}}
-            {{/if}}
+                    <CustomThemeSettings::Select @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
-            {{#if (eq setting.type "boolean")}}
+                {{/if}}
-                <CustomThemeSettings::Boolean @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{#if (eq setting.type "boolean")}}
-            {{/if}}
+                    <CustomThemeSettings::Boolean @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
-            {{#if (eq setting.type "color")}}
+                {{/if}}
-                <CustomThemeSettings::Color @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{#if (eq setting.type "color")}}
-            {{/if}}
+                    <CustomThemeSettings::Color @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
-            {{#if (eq setting.type "text")}}
+                {{/if}}
-                <CustomThemeSettings::Text @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{#if (eq setting.type "text")}}
-            {{/if}}
+                    <CustomThemeSettings::Text @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
-            {{#if (eq setting.type "image")}}
+                {{/if}}
-                <CustomThemeSettings::Image @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{#if (eq setting.type "image")}}
+                    <CustomThemeSettings::Image @setting={{setting}} @index={{index}} @onChange={{@onChange}} />
+                {{/if}}
             {{/if}}
         {{/each}}
     </form>

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})
 });

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() {
+    updateSettingsVisibility() {
-        this.settingGroups = this._buildSettingGroups(this.settings);
+        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
+            const groupSettings = settings.filter(setting => setting.group === knownGroup.key);
-                .filter(setting => setting.group === knownGroup.key)
-                .filter((setting) => {
-                    if (setting.visibility) {
-                        return nql(setting.visibility).queryJSON(this.keyValueObject);
-                    }
-
-                    return true;
-                });
 
             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);
+    }
 }

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
+            };
+        });
+    }
 };

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"
   }

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
+            });
+        });
     });
 });