Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-01 02:41:39 -05:00
Code Issues Activity

Removed method complexity in settings API controller

Browse source 
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13

- The controller code is not meant to contain complex business logic. Removed complexity in settings.edit method
- Have separated the regular editing from "Stripe Data" editing to keep the dependency on the members service still in the controller reducing coupling of the settings BREAD service to the minimum.
- The stripeConnectData passed into `edit` method still feels out of place (maybe it should be passed as an array already that's ready to be merged with the rest of settings, but that was left for another refactor in the future)
This commit is contained in:
Naz 2021-09-20 19:06:16 +02:00 • committed by naz
parent 85ee721157
commit 6b25b91fa4
3 changed files with 104 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
core/server/api/canary/settings.js
View file

@ -5,7 +5,7 @@ const models = require('../../models');
const routeSettings = require('../../services/route-settings');
const frontendSettings = require('../../../frontend/services/settings');
const i18n = require('../../../shared/i18n');
const {BadRequestError, NoPermissionError, NotFoundError} = require('@tryghost/errors');
const {BadRequestError, NoPermissionError} = require('@tryghost/errors');
const settingsService = require('../../services/settings');
const settingsCache = require('../../../shared/settings-cache');
const membersService = require('../../services/members');
@ -208,76 +208,20 @@ module.exports = {
}
},
async query(frame) {
let stripeConnectData;
const stripeConnectIntegrationToken = frame.data.settings.find(setting => setting.key === 'stripe_connect_integration_token');
const settings = frame.data.settings.filter((setting) => {
// The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
return ![
'stripe_connect_integration_token',
'stripe_connect_publishable_key',
'stripe_connect_secret_key',
'stripe_connect_livemode',
'stripe_connect_account_id',
'stripe_connect_display_name'
].includes(setting.key)
// Remove obfuscated settings
&& !(setting.value === settingsService.obfuscatedSetting && settingsService.isSecretSetting(setting));
});
const getSetting = setting => settingsCache.get(setting.key, {resolve: false});
const firstUnknownSetting = settings.find(setting => !getSetting(setting));
if (firstUnknownSetting) {
throw new NotFoundError({
message: i18n.t('errors.api.settings.problemFindingSetting', {
key: firstUnknownSetting.key
})
});
}
if (!(frame.options.context && frame.options.context.internal)) {
const firstCoreSetting = settings.find(setting => getSetting(setting).group === 'core');
if (firstCoreSetting) {
throw new NoPermissionError({
message: i18n.t('errors.api.settings.accessCoreSettingFromExtReq')
});
}
}
if (stripeConnectIntegrationToken && stripeConnectIntegrationToken.value) {
const getSessionProp = prop => frame.original.session[prop];
try {
const data = await membersService.stripeConnect.getStripeConnectTokenData(stripeConnectIntegrationToken.value, getSessionProp);
settings.push({
key: 'stripe_connect_publishable_key',
value: data.public_key
});
settings.push({
key: 'stripe_connect_secret_key',
value: data.secret_key
});
settings.push({
key: 'stripe_connect_livemode',
value: data.livemode
});
settings.push({
key: 'stripe_connect_display_name',
value: data.display_name
});
settings.push({
key: 'stripe_connect_account_id',
value: data.account_id
});
} catch (err) {
throw new BadRequestError({
err,
message: 'The Stripe Connect token could not be parsed.'
});
}
stripeConnectData = await settingsBREADService.getStripeConnectData(
stripeConnectIntegrationToken,
getSessionProp,
membersService.stripeConnect.getStripeConnectTokenData
);
}
return models.Settings.edit(settings, frame.options);
return await settingsBREADService.edit(frame.data.settings, frame.options, stripeConnectData);
}
},

1
core/server/services/settings/index.js
View file

@ -29,6 +29,7 @@ function hideValueIfSecret(setting) {
*/
const getSettingsBREADServiceInstance = () => {
return new SettingsBREADService({
SettingsModel: models.Settings,
settingsCache: SettingsCache
});
};

96
core/server/services/settings/settings-bread-service.js
View file

@ -1,5 +1,5 @@
const tpl = require('@tryghost/tpl');
const {NotFoundError, NoPermissionError} = require('@tryghost/errors');
const {NotFoundError, NoPermissionError, BadRequestError} = require('@tryghost/errors');
const messages = {
problemFindingSetting: 'Problem finding setting: {key}',
@ -26,9 +26,11 @@ class SettingsBREADService {
/**
*
* @param {Object} options
* @param {Object} options.SettingsModel
* @param {Object} options.settingsCache - SettingsCache instance
*/
constructor({settingsCache}) {
constructor({SettingsModel, settingsCache}) {
this.SettingsModel = SettingsModel;
this.settingsCache = settingsCache;
}
@ -76,6 +78,96 @@ class SettingsBREADService {
[key]: setting
};
}
/**
*
* @param {Object[]} settings
* @param {Object} options
* @param {Object} [options.context]
* @param {Object} [stripeConnectData]
* @returns
*/
async edit(settings, options, stripeConnectData) {
const filteredSettings = settings.filter((setting) => {
// The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
return ![
'stripe_connect_integration_token',
'stripe_connect_publishable_key',
'stripe_connect_secret_key',
'stripe_connect_livemode',
'stripe_connect_account_id',
'stripe_connect_display_name'
].includes(setting.key)
// Remove obfuscated settings
&& !(setting.value === obfuscatedSetting && isSecretSetting(setting));
});
const getSetting = setting => this.settingsCache.get(setting.key, {resolve: false});
const firstUnknownSetting = filteredSettings.find(setting => !getSetting(setting));
if (firstUnknownSetting) {
throw new NotFoundError({
message: tpl(messages.problemFindingSetting, {
key: firstUnknownSetting.key
})
});
}
if (!(options.context && options.context.internal)) {
const firstCoreSetting = filteredSettings.find(setting => getSetting(setting).group === 'core');
if (firstCoreSetting) {
throw new NoPermissionError({
message: tpl(messages.accessCoreSettingFromExtReq)
});
}
}
if (stripeConnectData) {
filteredSettings.push({
key: 'stripe_connect_publishable_key',
value: stripeConnectData.public_key
});
filteredSettings.push({
key: 'stripe_connect_secret_key',
value: stripeConnectData.secret_key
});
filteredSettings.push({
key: 'stripe_connect_livemode',
value: stripeConnectData.livemode
});
filteredSettings.push({
key: 'stripe_connect_display_name',
value: stripeConnectData.display_name
});
filteredSettings.push({
key: 'stripe_connect_account_id',
value: stripeConnectData.account_id
});
}
return this.SettingsModel.edit(filteredSettings, options);
}
/**
*
* @param {Object} stripeConnectIntegrationToken
* @param {Function} getSessionProp sync function fetching property from session store
* @param {Function} getStripeConnectTokenData async function retreiving Stripe Connect data for settings
* @returns {Promise<Object>} resolves with an object with following keys: public_key, secret_key, livemode, display_name, account_id
*/
async getStripeConnectData(stripeConnectIntegrationToken, getSessionProp, getStripeConnectTokenData) {
if (stripeConnectIntegrationToken && stripeConnectIntegrationToken.value) {
try {
return await getStripeConnectTokenData(stripeConnectIntegrationToken.value, getSessionProp);
} catch (err) {
throw new BadRequestError({
err,
message: 'The Stripe Connect token could not be parsed.'
});
}
}
}
}
module.exports = SettingsBREADService;