From fddf2ee42fabe5c675695ab409fa6a47d9b05883 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Wed, 9 Jul 2014 22:11:04 +0000 Subject: [PATCH] Fix active theme selector. Add validation to API. Closes #3226 - Remove dependent property from the computed content property that is used to build the active theme selector. - Add validation to the Settings model so that it rejects attempts to set an activeTheme that is not installed. --- core/client/controllers/settings/general.js | 2 +- core/server/data/validation/index.js | 31 +++++++++++++++++-- core/server/models/settings.js | 14 +++++++-- .../test/integration/api/api_settings_spec.js | 13 ++++++++ core/test/integration/api/api_themes_spec.js | 8 ++--- 5 files changed, 59 insertions(+), 9 deletions(-) diff --git a/core/client/controllers/settings/general.js b/core/client/controllers/settings/general.js index 4b1157ac55..2a85bc96fb 100644 --- a/core/client/controllers/settings/general.js +++ b/core/client/controllers/settings/general.js @@ -24,7 +24,7 @@ var SettingsGeneralController = Ember.ObjectController.extend({ return themes; }, []); - }.property('availableThemes').readOnly(), + }.property().readOnly(), actions: { save: function () { diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index c5e24f6674..3153490171 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -3,10 +3,15 @@ var schema = require('../schema').tables, validator = require('validator'), when = require('when'), errors = require('../../errors'), + config = require('../../config'), + requireTree = require('../../require-tree').readAll, validateSchema, validateSettings, - validate; + validateActiveTheme, + validate, + + availableThemes; // Provide a few custom validators // @@ -89,6 +94,27 @@ validateSettings = function (defaultSettings, model) { return when.resolve(); }; +// A Promise that will resolve to an object with a property for each installed theme. +// This is necessary because certain configuration data is only available while Ghost +// is running and at times the validations are used when it's not (e.g. tests) +availableThemes = requireTree(config().paths.themePath); + +validateActiveTheme = function (themeName) { + // If Ghost is running and its availableThemes collection exists + // give it priority. + if (Object.keys(config().paths.availableThemes).length > 0) { + availableThemes = when(config().paths.availableThemes); + } + + return availableThemes.then(function (themes) { + if (!themes.hasOwnProperty(themeName)) { + return when.reject(new errors.ValidationError(themeName + ' cannot be activated because it is not currently installed.', 'activeTheme')); + } + + return when.resolve(); + }); +}; + // Validate default settings using the validator module. // Each validation's key is a method name and its value is an array of options // @@ -134,5 +160,6 @@ validate = function (value, key, validations) { module.exports = { validateSchema: validateSchema, - validateSettings: validateSettings + validateSettings: validateSettings, + validateActiveTheme: validateActiveTheme }; diff --git a/core/server/models/settings.js b/core/server/models/settings.js index af0727870e..bd1cf4f15d 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -48,9 +48,19 @@ Settings = ghostBookshelf.Model.extend({ }, validate: function () { - var self = this; - return validation.validateSchema(self.tableName, self.toJSON()).then(function () { + var self = this, + setting = this.toJSON(); + + return validation.validateSchema(self.tableName, setting).then(function () { return validation.validateSettings(getDefaultSettings(), self); + }).then(function () { + var themeName = setting.value || ''; + + if (setting.key !== 'activeTheme') { + return when.resolve(); + } + + return validation.validateActiveTheme(themeName); }); }, diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js index e70087cfdc..19751fac0e 100644 --- a/core/test/integration/api/api_settings_spec.js +++ b/core/test/integration/api/api_settings_spec.js @@ -208,4 +208,17 @@ describe('Settings API', function () { done(); }).catch(done); }); + + it('does not allow an active theme which is not installed', function (done) { + return callApiWithContext(defaultContext, 'edit', 'activeTheme', { settings: [{ key: 'activeTheme', value: 'rasper' }] }) + .then(function (response) { + done(new Error('Allowed to set an active theme which is not installed')); + }).catch(function (err) { + should.exist(err); + + err.type.should.eql('ValidationError'); + + done(); + }).catch(done); + }); }); diff --git a/core/test/integration/api/api_themes_spec.js b/core/test/integration/api/api_themes_spec.js index 7228358bd6..9fdb859c1e 100644 --- a/core/test/integration/api/api_themes_spec.js +++ b/core/test/integration/api/api_themes_spec.js @@ -87,15 +87,15 @@ describe('Themes API', function () { _.extend(configStub, config); ThemeAPI.__set__('config', configStub); - ThemeAPI.edit({themes: [{uuid: 'rasper', active: true }]}, {context: {user: 1}}).then(function (result) { + ThemeAPI.edit({themes: [{uuid: 'casper', active: true }]}, {context: {user: 1}}).then(function (result) { should.exist(result); should.exist(result.themes); result.themes.length.should.be.above(0); testUtils.API.checkResponse(result.themes[0], 'theme'); - result.themes[0].uuid.should.equal('rasper'); + result.themes[0].uuid.should.equal('casper'); done(); }).catch(function (error) { done(new Error(JSON.stringify(error))); }).catch(done); - }) -}); \ No newline at end of file + }); +});