From c5ba27e2b5641db3d2a08e037d76a278d5fbae7c Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 10 May 2022 21:49:38 +0100 Subject: [PATCH] Added initial concept of calculated settings (#14766) refs: https://github.com/TryGhost/Team/issues/626 - calculated settings are simplified settings (booleans) that are based on other settings or data - they make it easier for us to determine what state features are in elsewhere in ghost e.g. admin and themes - this duplicates some of the members config concepts in the settings service --- .../services/settings/settings-service.js | 101 +++++++++++++++++- core/shared/settings-cache/cache.js | 33 +++++- core/shared/settings-cache/public.js | 5 +- .../admin/__snapshots__/settings.test.js.snap | 29 ++++- test/e2e-api/admin/settings.test.js | 2 +- .../__snapshots__/settings.test.js.snap | 7 +- test/e2e-api/content/settings.test.js | 5 +- .../shared/__snapshots__/version.test.js.snap | 14 ++- test/regression/api/admin/settings.test.js | 8 +- test/unit/shared/settings-cache.test.js | 2 +- 10 files changed, 189 insertions(+), 17 deletions(-) diff --git a/core/server/services/settings/settings-service.js b/core/server/services/settings/settings-service.js index 10f4396c7c..9ee1cec70d 100644 --- a/core/server/services/settings/settings-service.js +++ b/core/server/services/settings/settings-service.js @@ -2,13 +2,23 @@ * Settings Lib * A collection of utilities for handling settings including a cache */ +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); + const events = require('../../lib/common/events'); const models = require('../../models'); const labs = require('../../../shared/labs'); +const config = require('../../../shared/config'); const SettingsCache = require('../../../shared/settings-cache'); const SettingsBREADService = require('./settings-bread-service'); const {obfuscatedSetting, isSecretSetting, hideValueIfSecret} = require('./settings-utils'); +const ObjectId = require('bson-objectid'); + +const messages = { + incorrectKeyType: 'type must be one of "direct" or "connect".' +}; + /** * @returns {SettingsBREADService} instance of the PostsService */ @@ -20,13 +30,36 @@ const getSettingsBREADServiceInstance = () => { }); }; +class CalculatedField { + constructor({key, type, group, fn, dependents}) { + this.key = key; + this.type = type; + this.group = group; + this.fn = fn; + this.dependents = dependents; + } + + getSetting() { + return { + key: this.key, + type: this.type, + group: this.group, + value: this.fn(), + // @TODO: remove this hack + id: ObjectId().toHexString(), + created_at: new Date().toISOString().replace(/\d{3}Z$/, '000Z'), + updated_at: new Date().toISOString().replace(/\d{3}Z$/, '000Z') + }; + } +} + module.exports = { /** * Initialize the cache, used in boot and in testing */ async init() { const settingsCollection = await models.Settings.populateDefaults(); - SettingsCache.init(events, settingsCollection); + SettingsCache.init(events, settingsCollection, this.getCalculatedFields()); }, /** @@ -36,6 +69,72 @@ module.exports = { SettingsCache.reset(events); }, + isMembersEnabled() { + return SettingsCache.get('members_signup_access') !== 'none'; + }, + + isMembersInviteOnly() { + return SettingsCache.get('members_signup_access') === 'invite'; + }, + + /** + * @param {'direct' | 'connect'} type - The "type" of keys to fetch from settings + * @returns {{publicKey: string, secretKey: string} | null} + */ + getStripeKeys(type) { + if (type !== 'direct' && type !== 'connect') { + throw new errors.IncorrectUsageError({message: tpl(messages.incorrectKeyType)}); + } + + const secretKey = SettingsCache.get(`stripe_${type === 'connect' ? 'connect_' : ''}secret_key`); + const publicKey = SettingsCache.get(`stripe_${type === 'connect' ? 'connect_' : ''}publishable_key`); + + if (!secretKey || !publicKey) { + return null; + } + + return { + secretKey, + publicKey + }; + }, + + /** + * @returns {{publicKey: string, secretKey: string} | null} + */ + getActiveStripeKeys() { + const stripeDirect = config.get('stripeDirect'); + + if (stripeDirect) { + return this.getStripeKeys('direct'); + } + + const connectKeys = this.getStripeKeys('connect'); + + if (!connectKeys) { + return this.getStripeKeys('direct'); + } + + return connectKeys; + }, + + isStripeConnected() { + return this.isMembersEnabled() && this.getActiveStripeKeys() !== null; + }, + + /** + * + */ + getCalculatedFields() { + const fields = []; + + fields.push(new CalculatedField({key: 'members_enabled', type: 'boolean', group: 'members', fn: this.isMembersEnabled.bind(this), dependents: ['members_signup_access']})); + fields.push(new CalculatedField({key: 'members_invite_only', type: 'boolean', group: 'members', fn: this.isMembersInviteOnly.bind(this), dependents: ['members_signup_access']})); + fields.push(new CalculatedField({key: 'paid_members_enabled', type: 'boolean', group: 'members', fn: this.isStripeConnected.bind(this), dependents: ['members_signup_access', 'stripe_secret_key', 'stripe_publishable_key', 'stripe_connect_secret_key', 'stripe_connect_publishable_key']})); + + return fields; + }, + /** * Handles synchronization of routes.yaml hash loaded in the frontend with * the value stored in the settings table. diff --git a/core/shared/settings-cache/cache.js b/core/shared/settings-cache/cache.js index 0e973694a5..c31a0d7fef 100644 --- a/core/shared/settings-cache/cache.js +++ b/core/shared/settings-cache/cache.js @@ -3,6 +3,7 @@ // circular dependency bugs. const debug = require('@tryghost/debug')('settings:cache'); const _ = require('lodash'); + const publicSettings = require('./public'); // Local function, only ever used for initializing @@ -12,6 +13,13 @@ const updateSettingFromModel = function updateSettingFromModel(settingModel) { module.exports.set(settingModel.get('key'), settingModel.toJSON()); }; +const updateCalculatedField = function updateCalculatedField(field) { + return () => { + debug('Auto updating', field.key); + module.exports.set(field.key, field.getSetting()); + }; +}; + /** * ## Cache * Holds cached settings @@ -20,6 +28,7 @@ const updateSettingFromModel = function updateSettingFromModel(settingModel) { * @type {{}} - object of objects */ let settingsCache = {}; +let _calculatedFields = []; const doGet = (key, options) => { if (!settingsCache[key]) { @@ -116,10 +125,11 @@ module.exports = { * * @param {EventEmitter} events * @param {Bookshelf.Collection} settingsCollection + * @param {Array} calculatedFields * @return {object} */ - init(events, settingsCollection) { - // First, reset the cache and listeners + init(events, settingsCollection, calculatedFields) { + // First, reset the cache and this.reset(events); // // if we have been passed a collection of settings, use this to populate the cache @@ -127,11 +137,21 @@ module.exports = { _.each(settingsCollection.models, updateSettingFromModel); } + _calculatedFields = Array.isArray(calculatedFields) ? calculatedFields : []; + // Bind to events to automatically keep up-to-date events.on('settings.edited', updateSettingFromModel); events.on('settings.added', updateSettingFromModel); events.on('settings.deleted', updateSettingFromModel); + // set and bind calculated fields + _calculatedFields.forEach((field) => { + updateCalculatedField(field)(); + field.dependents.forEach((dependent) => { + events.on(`settings.${dependent}.edited`, updateCalculatedField(field)); + }); + }); + return settingsCache; }, @@ -145,5 +165,14 @@ module.exports = { events.removeListener('settings.edited', updateSettingFromModel); events.removeListener('settings.added', updateSettingFromModel); events.removeListener('settings.deleted', updateSettingFromModel); + + //unbind calculated fields + _calculatedFields.forEach((field) => { + field.dependents.forEach((dependent) => { + events.removeListener(`settings.${dependent}.edited`, updateCalculatedField(field)); + }); + }); + + _calculatedFields = []; } }; diff --git a/core/shared/settings-cache/public.js b/core/shared/settings-cache/public.js index 37b4ba21f1..8e10d79317 100644 --- a/core/shared/settings-cache/public.js +++ b/core/shared/settings-cache/public.js @@ -26,5 +26,8 @@ module.exports = { twitter_image: 'twitter_image', twitter_title: 'twitter_title', twitter_description: 'twitter_description', - members_support_address: 'members_support_address' + members_support_address: 'members_support_address', + members_enabled: 'members_enabled', + members_invite_only: 'members_invite_only', + paid_members_enabled: 'paid_members_enabled' }; diff --git a/test/e2e-api/admin/__snapshots__/settings.test.js.snap b/test/e2e-api/admin/__snapshots__/settings.test.js.snap index 4aca8dca5d..f06549676d 100644 --- a/test/e2e-api/admin/__snapshots__/settings.test.js.snap +++ b/test/e2e-api/admin/__snapshots__/settings.test.js.snap @@ -1055,6 +1055,33 @@ Object { "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "value": false, }, + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "group": "members", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "key": "members_enabled", + "type": "boolean", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "value": true, + }, + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "group": "members", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "key": "members_invite_only", + "type": "boolean", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "value": false, + }, + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "group": "members", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "key": "paid_members_enabled", + "type": "boolean", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "value": true, + }, Object { "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "flags": null, @@ -1115,7 +1142,7 @@ exports[`Settings API Can request all settings 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "18299", + "content-length": "18867", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Origin, Accept-Encoding", diff --git a/test/e2e-api/admin/settings.test.js b/test/e2e-api/admin/settings.test.js index edf47597a7..3f72307cf5 100644 --- a/test/e2e-api/admin/settings.test.js +++ b/test/e2e-api/admin/settings.test.js @@ -2,7 +2,7 @@ const assert = require('assert'); const {agentProvider, fixtureManager, mockManager, matchers} = require('../../utils/e2e-framework'); const {stringMatching, anyEtag, anyObjectId, anyISODateTime} = matchers; -const CURRENT_SETTINGS_COUNT = 86; +const CURRENT_SETTINGS_COUNT = 89; const settingsMatcher = { id: anyObjectId, diff --git a/test/e2e-api/content/__snapshots__/settings.test.js.snap b/test/e2e-api/content/__snapshots__/settings.test.js.snap index 23c9d12df4..ec764a2413 100644 --- a/test/e2e-api/content/__snapshots__/settings.test.js.snap +++ b/test/e2e-api/content/__snapshots__/settings.test.js.snap @@ -14,6 +14,8 @@ Object { "lang": "en", "locale": "en", "logo": null, + "members_enabled": true, + "members_invite_only": false, "members_support_address": "noreply", "meta_description": null, "meta_title": null, @@ -42,6 +44,7 @@ Object { "og_description": null, "og_image": null, "og_title": null, + "paid_members_enabled": true, "secondary_navigation": Array [ Object { "label": "Data & privacy", @@ -83,9 +86,9 @@ exports[`Settings Content API Can request settings 2: [headers] 1`] = ` Object { "access-control-allow-origin": "*", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "942", + "content-length": "1021", "content-type": "application/json; charset=utf-8", - "etag": "W/\\"3ae-FBGPtlUjSvGtTGLOj2sW5Rbn33s\\"", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Accept-Encoding", "x-powered-by": "Express", } diff --git a/test/e2e-api/content/settings.test.js b/test/e2e-api/content/settings.test.js index 1caa152c7b..680f92625c 100644 --- a/test/e2e-api/content/settings.test.js +++ b/test/e2e-api/content/settings.test.js @@ -1,4 +1,5 @@ const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework'); +const {anyEtag} = matchers; describe('Settings Content API', function () { let agent; @@ -12,7 +13,9 @@ describe('Settings Content API', function () { it('Can request settings', async function () { await agent.get('settings/') .expectStatus(200) - .matchHeaderSnapshot() + .matchHeaderSnapshot({ + etag: anyEtag + }) .matchBodySnapshot(); }); }); diff --git a/test/e2e-api/shared/__snapshots__/version.test.js.snap b/test/e2e-api/shared/__snapshots__/version.test.js.snap index a0db4deb36..ad67a11518 100644 --- a/test/e2e-api/shared/__snapshots__/version.test.js.snap +++ b/test/e2e-api/shared/__snapshots__/version.test.js.snap @@ -492,6 +492,8 @@ Object { "lang": "en", "locale": "en", "logo": null, + "members_enabled": true, + "members_invite_only": false, "members_support_address": "noreply", "meta_description": null, "meta_title": null, @@ -520,6 +522,7 @@ Object { "og_description": null, "og_image": null, "og_title": null, + "paid_members_enabled": true, "secondary_navigation": Array [ Object { "label": "Data & privacy", @@ -549,10 +552,10 @@ exports[`API Versioning Content API responds with current content version header Object { "access-control-allow-origin": "*", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "942", + "content-length": "1021", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, - "etag": "W/\\"3ae-FBGPtlUjSvGtTGLOj2sW5Rbn33s\\"", + "etag": "W/\\"3fd-PXBX1gYn1ftAeK+GoVbEnAsmVAE\\"", "vary": "Accept-Encoding", "x-powered-by": "Express", } @@ -572,6 +575,8 @@ Object { "lang": "en", "locale": "en", "logo": null, + "members_enabled": true, + "members_invite_only": false, "members_support_address": "noreply", "meta_description": null, "meta_title": null, @@ -600,6 +605,7 @@ Object { "og_description": null, "og_image": null, "og_title": null, + "paid_members_enabled": true, "secondary_navigation": Array [ Object { "label": "Data & privacy", @@ -629,9 +635,9 @@ exports[`API Versioning Content API responds with no content version header when Object { "access-control-allow-origin": "*", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "942", + "content-length": "1021", "content-type": "application/json; charset=utf-8", - "etag": "W/\\"3ae-FBGPtlUjSvGtTGLOj2sW5Rbn33s\\"", + "etag": "W/\\"3fd-PXBX1gYn1ftAeK+GoVbEnAsmVAE\\"", "vary": "Accept-Encoding", "x-powered-by": "Express", } diff --git a/test/regression/api/admin/settings.test.js b/test/regression/api/admin/settings.test.js index a7008cf246..030d23f3de 100644 --- a/test/regression/api/admin/settings.test.js +++ b/test/regression/api/admin/settings.test.js @@ -441,6 +441,8 @@ const defaultSettingsKeyTypes = [ } ]; +const calculatedSettingsTypes = ['members_enabled', 'members_invite_only', 'paid_members_enabled']; + describe('Settings API (canary)', function () { let request; @@ -467,7 +469,7 @@ describe('Settings API (canary)', function () { jsonResponse.settings.should.be.an.Object(); const settings = jsonResponse.settings; - should.equal(settings.length, defaultSettingsKeyTypes.length); + should.equal(settings.length, (defaultSettingsKeyTypes.length + calculatedSettingsTypes.length)); for (const defaultSetting of defaultSettingsKeyTypes) { should.exist(settings.find((setting) => { return (setting.key === defaultSetting.key) @@ -500,7 +502,7 @@ describe('Settings API (canary)', function () { jsonResponse.settings.should.be.an.Object(); const settings = jsonResponse.settings; // Returns all settings - should.equal(settings.length, defaultSettingsKeyTypes.length); + should.equal(settings.length, (defaultSettingsKeyTypes.length + calculatedSettingsTypes.length)); for (const defaultSetting of defaultSettingsKeyTypes) { should.exist(settings.find((setting) => { return setting.key === defaultSetting.key && setting.type === defaultSetting.type; @@ -553,7 +555,7 @@ describe('Settings API (canary)', function () { jsonResponse.settings.should.be.an.Object(); const settings = jsonResponse.settings; - Object.keys(settings).length.should.equal(defaultSettingsKeyTypes.length); + Object.keys(settings).length.should.equal((defaultSettingsKeyTypes.length + calculatedSettingsTypes.length)); localUtils.API.checkResponse(jsonResponse, 'settings'); }); diff --git a/test/unit/shared/settings-cache.test.js b/test/unit/shared/settings-cache.test.js index 234fd20379..d6d40f803b 100644 --- a/test/unit/shared/settings-cache.test.js +++ b/test/unit/shared/settings-cache.test.js @@ -11,7 +11,7 @@ should.equal(true, true); describe('UNIT: settings cache', function () { beforeEach(function () { - cache.init(events, {}); + cache.init(events, {}, []); }); afterEach(function () {