Unified ThemeValidationError generation code

- All the code for creating these errors is now replaced with a single function
- This is useful DRY as it helps make code more readable
- This gets rid of the override of the error type to ThemeWorksButHasErrors - which is both weird and afaict not used anywhere
This commit is contained in:
Hannah Wolfe 2021-07-07 13:40:10 +01:00
parent 362140b31e
commit 6a39d0a011
No known key found for this signature in database
GPG Key ID: 9F8C7532D0A6BA55
3 changed files with 20 additions and 31 deletions

View File

@ -1,4 +1,3 @@
const _ = require('lodash');
const debug = require('@tryghost/debug')('themes');
const logging = require('@tryghost/logging');
const errors = require('@tryghost/errors');
@ -10,10 +9,8 @@ const list = require('./list');
const settingsCache = require('../../../shared/settings-cache');
const messages = {
missingTheme: 'The currently active theme "{theme}" is missing.',
themeCannotBeActivated: '{themeName} cannot be activated because it is not currently installed.',
invalidTheme: 'The currently active theme "{theme}" is invalid.',
themeHasErrors: 'The currently active theme "{theme}" has errors, but will still work.'
activeThemeIsMissing: 'The currently active theme "{theme}" is missing.',
themeCannotBeActivated: '{themeName} cannot be activated because it is not currently installed.'
};
module.exports = {
@ -32,25 +29,10 @@ module.exports = {
.check(theme)
.then(function validationSuccess(checkedTheme) {
if (!validate.canActivate(checkedTheme)) {
logging.error(new errors.ThemeValidationError({
message: tpl(messages.invalidTheme, {theme: activeThemeName}),
errorDetails: Object.assign(
_.pick(checkedTheme, ['checkedVersion', 'name', 'path', 'version']), {
errors: checkedTheme.results.error
}
)
}));
// CASE: inform that the theme has errors, but not fatal (theme still works)
logging.error(validate.getThemeValidationError('activeThemeHasFatalErrors', activeThemeName, checkedTheme));
} else if (checkedTheme.results.error.length) {
logging.warn(new errors.ThemeValidationError({
errorType: 'ThemeWorksButHasErrors',
message: tpl(messages.themeHasErrors, {theme: activeThemeName}),
errorDetails: Object.assign(
_.pick(checkedTheme, ['checkedVersion', 'name', 'path', 'version']), {
errors: checkedTheme.results.error
}
)
}));
// CASE: inform that the theme has errors, but not fatal (theme still works)
logging.warn(validate.getThemeValidationError('activeThemeHasErrors', activeThemeName, checkedTheme));
}
debug('Activating theme (method A on boot)', activeThemeName);
@ -60,7 +42,7 @@ module.exports = {
.catch(function (err) {
if (err instanceof errors.NotFoundError) {
// CASE: active theme is missing, we don't want to exit because the admin panel will still work
err.message = tpl(messages.missingTheme, {theme: activeThemeName});
err.message = tpl(messages.activeThemeIsMissing, {theme: activeThemeName});
}
// CASE: theme threw an odd error, we don't want to exit because the admin panel will still work
@ -79,7 +61,7 @@ module.exports = {
}));
}
return validate.checkSafe(loadedTheme)
return validate.checkSafe(themeName, loadedTheme)
.then((checkedTheme) => {
debug('Activating theme (method B on API "activate")', themeName);
bridge.activateTheme(loadedTheme, checkedTheme);

View File

@ -61,7 +61,7 @@ module.exports = {
let renamedExisting = false;
try {
checkedTheme = await validate.checkSafe(zip, true);
checkedTheme = await validate.checkSafe(shortName, zip, true);
const themeExists = await getStorage().exists(shortName);
// CASE: move the existing theme to a backup folder
if (themeExists) {

View File

@ -7,7 +7,9 @@ const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const messages = {
invalidTheme: 'Theme is not compatible or contains errors.'
themeHasErrors: 'Theme {name} is not compatible or contains errors.',
activeThemeHasFatalErrors: 'The currently active theme "{theme}" has fatal errors.',
activeThemeHasErrors: 'The currently active theme "{theme}" has errors, but will still work.'
};
const canActivate = function canActivate(checkedTheme) {
@ -44,7 +46,7 @@ const check = async function check(theme, isZip) {
return checkedTheme;
};
const checkSafe = async function checkSafe(theme, isZip) {
const checkSafe = async function checkSafe(themeName, theme, isZip) {
const checkedTheme = await check(theme, isZip);
if (canActivate(checkedTheme)) {
@ -61,16 +63,21 @@ const checkSafe = async function checkSafe(theme, isZip) {
fs.remove(checkedTheme.path);
}
return Promise.reject(new errors.ThemeValidationError({
message: tpl(messages.invalidTheme),
return Promise.reject(getThemeValidationError('themeHasErrors', themeName, checkedTheme));
};
const getThemeValidationError = (message, themeName, checkedTheme) => {
return new errors.ThemeValidationError({
message: tpl(messages[message], {theme: themeName}),
errorDetails: Object.assign(
_.pick(checkedTheme, ['checkedVersion', 'name', 'path', 'version']), {
errors: checkedTheme.results.error
}
)
}));
});
};
module.exports.check = check;
module.exports.checkSafe = checkSafe;
module.exports.canActivate = canActivate;
module.exports.getThemeValidationError = getThemeValidationError;