Added name argument to .activateTheme()

no issue

Ghost allows multiple themes with the same `name` value in `package.json` so it assigns custom names based on the file name. When syncing we were only looking at the package.json `name` value so there could be a syncing clash when swapping between active themes.

- added `name` as the first argument to `.activateTheme(name, theme)` so that the service's consumer is in control of settings sync applying to it's known theme name
This commit is contained in:
Kevin Ansfield 2021-10-20 11:04:44 +01:00
parent 38e9f84d41
commit 2951da002c
2 changed files with 31 additions and 31 deletions

View File

@ -29,13 +29,14 @@ module.exports = class CustomThemeSettingsService {
* The service only deals with one theme at a time,
* that theme is changed by calling this method with the output from gscan
*
* @param {string} name - the name of the theme (Ghost has different names to themes with duplicate package.json names)
* @param {Object} theme - checked theme output from gscan
*/
async activateTheme(theme) {
this.activeThemeName = theme.name;
async activateTheme(name, theme) {
this.activeThemeName = name;
// add/remove/edit key/value records in the respository to match theme settings
const settings = await this._syncRepositoryWithTheme(theme);
const settings = await this._syncRepositoryWithTheme(name, theme);
// populate the shared cache with all key/value pairs for this theme
this._populateValueCacheForTheme(theme, settings);
@ -127,10 +128,10 @@ module.exports = class CustomThemeSettingsService {
* @returns {Array} - list of stored theme record objects
* @private
*/
async _syncRepositoryWithTheme(theme) {
async _syncRepositoryWithTheme(name, theme) {
const themeSettings = theme.customSettings || {};
const settingsCollection = await this._repository.browse({filter: `theme:${theme.name}`});
const settingsCollection = await this._repository.browse({filter: `theme:${name}`});
let knownSettings = settingsCollection.toJSON();
// exit early if there's nothing to sync for this theme
@ -148,7 +149,7 @@ module.exports = class CustomThemeSettingsService {
const hasChangedType = themeSetting && themeSetting.type !== knownSetting.type;
if (hasBeenRemoved || hasChangedType) {
debug(`Removing custom theme setting '${theme.name}.${knownSetting.key}' - ${hasBeenRemoved ? 'not found in theme' : 'type changed'}`);
debug(`Removing custom theme setting '${name}.${knownSetting.key}' - ${hasBeenRemoved ? 'not found in theme' : 'type changed'}`);
await this._repository.destroy({id: knownSetting.id});
removedIds.push(knownSetting.id);
continue;
@ -156,7 +157,7 @@ module.exports = class CustomThemeSettingsService {
// replace value with default if it's not a valid select option
if (themeSetting.options && !themeSetting.options.includes(knownSetting.value)) {
debug(`Resetting custom theme setting value '${theme.name}.${themeSetting.key}' - "${knownSetting.value}" is not a valid option`);
debug(`Resetting custom theme setting value '${name}.${themeSetting.key}' - "${knownSetting.value}" is not a valid option`);
await this._repository.edit({value: themeSetting.default}, {id: knownSetting.id});
}
}
@ -170,18 +171,18 @@ module.exports = class CustomThemeSettingsService {
for (const [key, setting] of Object.entries(themeSettings)) {
if (!knownSettingsKeys.includes(key)) {
const newSettingValues = {
theme: theme.name,
theme: name,
key,
type: setting.type,
value: setting.default
};
debug(`Adding custom theme setting '${theme.name}.${key}'`);
debug(`Adding custom theme setting '${name}.${key}'`);
await this._repository.add(newSettingValues);
}
}
const updatedSettingsCollection = await this._repository.browse({filter: `theme:${theme.name}`});
const updatedSettingsCollection = await this._repository.browse({filter: `theme:${name}`});
return updatedSettingsCollection.toJSON();
}

View File

@ -92,16 +92,17 @@ describe('Service', function () {
});
describe('activateTheme()', function () {
it('sets .activeThemeName', function () {
it('sets .activeThemeName correctly', function () {
should(service.activeThemeName).equal(null);
service.activateTheme({name: 'test'});
// theme names do not always match the name in package.json
service.activateTheme('Test-test', {name: 'test'});
service.activeThemeName.should.equal('test');
service.activeThemeName.should.equal('Test-test');
});
it('handles known settings not seen in theme', async function () {
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
// 'one' custom setting no longer exists
@ -139,7 +140,7 @@ describe('Service', function () {
});
it('handles known settings that change type', async function () {
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
// no change
@ -192,7 +193,7 @@ describe('Service', function () {
});
it('handles value of select not matching updated options', async function () {
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
// no change
@ -217,7 +218,7 @@ describe('Service', function () {
});
it('handles new settings', async function () {
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
// no change
@ -284,7 +285,7 @@ describe('Service', function () {
it('handles activation of new theme when already activated', async function () {
// activate known theme
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -301,7 +302,7 @@ describe('Service', function () {
});
// activate new theme
await service.activateTheme({
await service.activateTheme('new', {
name: 'new',
customSettings: {
typography: {
@ -366,9 +367,7 @@ describe('Service', function () {
});
it('exits early if both repository and theme have no settings', async function () {
await service.activateTheme({
name: 'no-custom'
});
await service.activateTheme('no-custom', {name: 'no-custom'});
model.findAll.callCount.should.equal(1);
});
@ -383,7 +382,7 @@ describe('Service', function () {
describe('updateSettings()', function () {
it('saves new values', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -468,7 +467,7 @@ describe('Service', function () {
it('ignores everything except keys and values', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -553,7 +552,7 @@ describe('Service', function () {
it('errors on unknown setting', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -591,7 +590,7 @@ describe('Service', function () {
it('errors on unallowed select value', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -622,7 +621,7 @@ describe('Service', function () {
it('allows any valid color value', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -653,7 +652,7 @@ describe('Service', function () {
it('errors on invalid color values', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -677,7 +676,7 @@ describe('Service', function () {
it('allows any valid boolean value', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -708,7 +707,7 @@ describe('Service', function () {
it('errors on invalid boolean values', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {
@ -732,7 +731,7 @@ describe('Service', function () {
it('allows any text value', async function () {
// activate theme so settings are loaded in internal cache
await service.activateTheme({
await service.activateTheme('test', {
name: 'test',
customSettings: {
one: {