🔥 🎨 Themes & settings misc cleanup (#8061)

no issue

🔥 remove unused loadThemes API method
🚨 Add tests for themes.readOne
🔥 Don't update settings cache for imports
- this isn't needed as of #8057
- settings.edit fires an event, that will result in the update happening automatically
🎨 Move validation to themes
- slowly collecting all theme-related code together
🔥 Reduce DEBUG output
- all this info is a bit tooooo much!

core/server/api/db.js

@@ -63,9 +63,6 @@ db = {
 
         function importContent(options) {
             return importer.importFromFile(options)
-                .then(function () {
-                    api.settings.updateSettingsCache();
-                })
                 .return({db: []});
         }
 

core/server/api/themes.js

@@ -23,13 +23,6 @@ var Promise = require('bluebird'),
  * **See:** [API Methods](index.js.html#api%20methods)
  */
 themes = {
-    loadThemes: function () {
-        return utils.readThemes(config.getContentPath('themes'))
-            .then(function (result) {
-                config.set('paths:availableThemes', result);
-            });
-    },
-
     browse: function browse() {
         return Promise.resolve({themes: settingsCache.get('availableThemes')});
     },

core/server/data/validation/index.js

@@ -5,12 +5,10 @@ var schema    = require('../schema').tables,
     assert    = require('assert'),
     Promise   = require('bluebird'),
     errors    = require('../../errors'),
-    config    = require('../../config'),
-    i18n        = require('../../i18n'),
+    i18n      = require('../../i18n'),
 
     validateSchema,
     validateSettings,
-    validateActiveTheme,
     validate;
 
 function assertString(input) {
@@ -127,20 +125,6 @@ validateSettings = function validateSettings(defaultSettings, model) {
     return Promise.resolve();
 };
 
-validateActiveTheme = function validateActiveTheme(themeName) {
-    // @TODO come up with something way better here - we should probably attempt to read the theme from the
-    // File system at this point and validate the theme using gscan rather than just checking if it's in a cache object
-    if (!config.get('paths').availableThemes || Object.keys(config.get('paths').availableThemes).length === 0) {
-        // We haven't yet loaded all themes, this is probably being called early?
-        return Promise.resolve();
-    }
-
-    // Else, if we have a list, check if the theme is in it
-    if (!config.get('paths').availableThemes.hasOwnProperty(themeName)) {
-        return Promise.reject(new errors.ValidationError({message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), context: 'activeTheme'}));
-    }
-};
-
 // Validate default settings using the validator module.
 // Each validation's key is a method name and its value is an array of options
 //
@@ -191,6 +175,5 @@ module.exports = {
     validate: validate,
     validator: validator,
     validateSchema: validateSchema,
-    validateSettings: validateSettings,
-    validateActiveTheme: validateActiveTheme
+    validateSettings: validateSettings
 };

core/server/models/settings.js

@@ -7,6 +7,7 @@ var Settings,
     events         = require('../events'),
     i18n           = require('../i18n'),
     validation     = require('../data/validation'),
+    themes         = require('../themes'),
 
     internalContext = {context: {internal: true}},
 
@@ -89,7 +90,7 @@ Settings = ghostBookshelf.Model.extend({
                 return;
             }
 
-            return validation.validateActiveTheme(themeName);
+            return themes.validate.activeTheme(themeName);
         });
     }
 }, {

core/server/themes/index.js

@@ -2,5 +2,6 @@ var themeLoader = require('./loader');
 
 module.exports = {
     init: themeLoader.init,
-    load: themeLoader.load
+    load: themeLoader.load,
+    validate: require('./validate')
 };

core/server/themes/loader.js

@@ -9,7 +9,7 @@ var debug = require('debug')('ghost:themes:loader'),
     initThemes;
 
 updateConfigAndCache = function updateConfigAndCache(themes) {
-    debug('loading themes', themes);
+    debug('loading themes', Object.keys(themes));
     config.set('paths:availableThemes', themes);
     settingsApi.updateSettingsCache();
 };

core/server/themes/validate.js

@@ -0,0 +1,21 @@
+var Promise = require('bluebird'),
+    config = require('../config'),
+    errors = require('../errors'),
+    i18n = require('../i18n'),
+    validateActiveTheme;
+
+// @TODO replace this with something PROPER - we should probably attempt to read the theme from the
+// File system at this point and validate the theme using gscan rather than just checking if it's in a cache object
+validateActiveTheme = function validateActiveTheme(themeName) {
+    if (!config.get('paths').availableThemes || Object.keys(config.get('paths').availableThemes).length === 0) {
+        // We haven't yet loaded all themes, this is probably being called early?
+        return Promise.resolve();
+    }
+
+    // Else, if we have a list, check if the theme is in it
+    if (!config.get('paths').availableThemes.hasOwnProperty(themeName)) {
+        return Promise.reject(new errors.ValidationError({message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), context: 'activeTheme'}));
+    }
+};
+
+module.exports.activeTheme = validateActiveTheme;

core/test/unit/themes_spec.js

@@ -8,7 +8,7 @@ var should          = require('should'),
 should.equal(true, true);
 
 describe('Themes', function () {
-    describe('Read', function () {
+    describe('Read All', function () {
         it('should read directory and include only folders', function (done) {
             var themesPath = tmp.dirSync({unsafeCleanup: true});
 
@@ -21,8 +21,46 @@ describe('Themes', function () {
             fs.mkdirSync(join(themesPath.name, 'casper', 'partials'));
             fs.writeFileSync(join(themesPath.name, 'casper', 'index.hbs'));
             fs.writeFileSync(join(themesPath.name, 'casper', 'partials', 'navigation.hbs'));
+            fs.mkdirSync(join(themesPath.name, 'not-casper'));
+            fs.writeFileSync(join(themesPath.name, 'not-casper', 'index.hbs'));
 
             readThemes.all(themesPath.name)
+                .then(function (tree) {
+                    tree.should.eql({
+                        casper: {
+                            partials: {
+                                'navigation.hbs': join(themesPath.name, 'casper', 'partials', 'navigation.hbs')
+                            },
+                            'index.hbs': join(themesPath.name, 'casper', 'index.hbs')
+                        },
+                        'not-casper': {
+                            'index.hbs': join(themesPath.name, 'not-casper', 'index.hbs')
+                        }
+                    });
+
+                    done();
+                })
+                .catch(done)
+                .finally(themesPath.removeCallback);
+        });
+    });
+    describe('Read One', function () {
+        it('should read directory and include only single requested theme', function (done) {
+            var themesPath = tmp.dirSync({unsafeCleanup: true});
+
+            // create trash
+            fs.writeFileSync(join(themesPath.name, 'casper.zip'));
+            fs.writeFileSync(join(themesPath.name, '.DS_Store'));
+
+            // create actual theme
+            fs.mkdirSync(join(themesPath.name, 'casper'));
+            fs.mkdirSync(join(themesPath.name, 'casper', 'partials'));
+            fs.writeFileSync(join(themesPath.name, 'casper', 'index.hbs'));
+            fs.writeFileSync(join(themesPath.name, 'casper', 'partials', 'navigation.hbs'));
+            fs.mkdirSync(join(themesPath.name, 'not-casper'));
+            fs.writeFileSync(join(themesPath.name, 'not-casper', 'index.hbs'));
+
+            readThemes.one(themesPath.name, 'casper')
                 .then(function (tree) {
                     tree.should.eql({
                         casper: {
@@ -38,5 +76,22 @@ describe('Themes', function () {
                 .catch(done)
                 .finally(themesPath.removeCallback);
         });
+
+        it('should return empty object if theme cannot be found', function (done) {
+            var themesPath = tmp.dirSync({unsafeCleanup: true});
+
+            // create trash
+            fs.writeFileSync(join(themesPath.name, 'casper.zip'));
+            fs.writeFileSync(join(themesPath.name, '.DS_Store'));
+
+            readThemes.one(themesPath.name, 'casper')
+                .then(function (tree) {
+                    tree.should.eql({});
+
+                    done();
+                })
+                .catch(done)
+                .finally(themesPath.removeCallback);
+        });
     });
 });

core/test/unit/validation_spec.js

@@ -8,13 +8,12 @@ describe('Validation', function () {
         should.exist(validation);
 
         validation.should.have.properties(
-            ['validate', 'validator', 'validateSchema', 'validateSettings', 'validateActiveTheme']
+            ['validate', 'validator', 'validateSchema', 'validateSettings']
         );
 
         validation.validate.should.be.a.Function();
         validation.validateSchema.should.be.a.Function();
         validation.validateSettings.should.be.a.Function();
-        validation.validateActiveTheme.should.be.a.Function();
 
         validation.validator.should.have.properties(['empty', 'notContains', 'isTimezone', 'isEmptyOrURL', 'isSlug']);
     });