Revert "fix: ensure we initialise activeTheme on bootstrap (#6950)"

This reverts commit 8f2afeed03.

core/server/api/settings.js

@@ -52,7 +52,6 @@ updateConfigCache = function () {
 
     config.set({
         theme: {
-            activeTheme: settingsCache.activeTheme.value,
             title: (settingsCache.title && settingsCache.title.value) || '',
             description: (settingsCache.description && settingsCache.description.value) || '',
             logo: (settingsCache.logo && settingsCache.logo.value) || '',

core/server/middleware/index.js

@@ -1,21 +1,20 @@
 var bodyParser      = require('body-parser'),
     compress        = require('compression'),
+    config          = require('../config'),
+    errors          = require('../errors'),
     express         = require('express'),
     hbs             = require('express-hbs'),
     logger          = require('morgan'),
-    multer          = require('multer'),
-    netjet          = require('netjet'),
     path            = require('path'),
+    routes          = require('../routes'),
     serveStatic     = require('express').static,
     slashes         = require('connect-slashes'),
-    tmpdir          = require('os').tmpdir,
-    passport        = require('passport'),
-    config          = require('../config'),
-    errors          = require('../errors'),
-    routes          = require('../routes'),
     storage         = require('../storage'),
+    passport        = require('passport'),
     utils           = require('../utils'),
     sitemapHandler  = require('../data/xml/sitemap/handler'),
+    multer          = require('multer'),
+    tmpdir          = require('os').tmpdir,
     authStrategies   = require('./auth-strategies'),
     auth             = require('./auth'),
     cacheControl     = require('./cache-control'),
@@ -31,6 +30,7 @@ var bodyParser      = require('body-parser'),
     maintenance     = require('./maintenance'),
     versionMatch     = require('./api/version-match'),
     cors             = require('./cors'),
+    netjet           = require('netjet'),
     labs             = require('./labs'),
     helpers          = require('../helpers'),
 
@@ -129,7 +129,7 @@ setupMiddleware = function setupMiddleware(blogApp) {
 
     // First determine whether we're serving admin or theme content
     blogApp.use(decideIsAdmin);
-    blogApp.use(themeHandler.updateActiveTheme(blogApp));
+    blogApp.use(themeHandler.updateActiveTheme);
     blogApp.use(themeHandler.configHbsForContext);
 
     // Admin only config

core/server/middleware/theme-handler.js

@@ -48,6 +48,7 @@ themeHandler = {
         // Pass 'secure' flag to the view engine
         // so that templates can choose 'url' vs 'urlSSL'
         res.locals.secure = req.secure;
+
         next();
     },
 
@@ -88,37 +89,40 @@ themeHandler = {
     // Updates the blogApp's activeTheme variable and subsequently
     // activates that theme's views with the hbs templating engine if it
     // is not yet activated.
-    //
+    updateActiveTheme: function updateActiveTheme(req, res, next) {
-    // on server bootstrap we activate the default theme (which is casper)
+        var blogApp = req.app;
-    updateActiveTheme: function updateActiveTheme(blog) {
-        themeHandler.activateTheme(blog, config.theme.activeTheme);
 
-        return function updateActiveThemeDynamically(req, res, next) {
+        api.settings.read({context: {internal: true}, key: 'activeTheme'}).then(function then(response) {
-            var blogApp = req.app;
+            var activeTheme = response.settings[0];
 
-            api.settings.read({context: {internal: true}, key: 'activeTheme'}).then(function then(response) {
+            // Check if the theme changed
-                var activeTheme = response.settings[0];
+            if (activeTheme.value !== blogApp.get('activeTheme')) {
-
+                // Change theme
-                // Check if the theme changed
+                if (!config.paths.availableThemes.hasOwnProperty(activeTheme.value)) {
-                if (activeTheme.value !== blogApp.get('activeTheme')) {
+                    if (!res.isAdmin) {
-                    // Change theme
+                        // Throw an error if the theme is not available, but not on the admin UI
-                    if (!config.paths.availableThemes.hasOwnProperty(activeTheme.value)) {
+                        return errors.throwError(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value}));
-                        if (!res.isAdmin) {
-                            // Throw an error if the theme is not available, but not on the admin UI
-                            return errors.throwError(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value}));
-                        } else {
-                            errors.logWarn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value}));
-                            return next();
-                        }
                     } else {
-                        themeHandler.activateTheme(blogApp, activeTheme.value);
+                        // At this point the activated theme is not present and the current
+                        // request is for the admin client.  In order to allow the user access
+                        // to the admin client we set an hbs instance on the app so that middleware
+                        // processing can continue.
+                        blogApp.engine('hbs', hbs.express3());
+                        errors.logWarn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value}));
+
+                        return next();
                     }
+                } else {
+                    themeHandler.activateTheme(blogApp, activeTheme.value);
                 }
-                next();
+            }
-            }).catch(function handleError(err) {
+            next();
-                next(err);
+        }).catch(function handleError(err) {
-            });
+            // Trying to start up without the active theme present, setup a simple hbs instance
-        };
+            // and render an error page straight away.
+            blogApp.engine('hbs', hbs.express3());
+            next(err);
+        });
     }
 };
 

core/test/unit/middleware/theme-handler_spec.js

@@ -127,34 +127,19 @@ describe('Theme Handler', function () {
     });
 
     describe('updateActiveTheme', function () {
-        it('sets active theme on bootstrap', function () {
-            should.not.exist(blogApp.get('activeTheme'));
-
-            var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme');
-            configUtils.set({theme: {activeTheme: 'casper'}});
-            configUtils.set({paths: {availableThemes: {casper: {}}}});
-
-            themeHandler.updateActiveTheme(blogApp);
-            activateThemeSpy.calledOnce.should.eql(true);
-            blogApp.get('activeTheme').should.eql('casper');
-        });
-
         it('updates the active theme if changed', function (done) {
             var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme');
-
             sandbox.stub(api.settings, 'read').withArgs(sandbox.match.has('key', 'activeTheme')).returns(Promise.resolve({
                 settings: [{
-                    key: 'activateTheme',
+                    key: 'activeKey',
-                    value: 'new-theme'
+                    value: 'casper'
                 }]
             }));
+            blogApp.set('activeTheme', 'not-casper');
+            configUtils.set({paths: {availableThemes: {casper: {}}}});
 
-            configUtils.set({theme: {activeTheme: 'casper'}});
+            themeHandler.updateActiveTheme(req, res, function () {
-            configUtils.set({paths: {availableThemes: {casper: {}, 'new-theme': {}}}});
+                activateThemeSpy.called.should.be.true();
-
-            themeHandler.updateActiveTheme(blogApp)(req, res, function () {
-                activateThemeSpy.calledTwice.should.be.true();
-                blogApp.get('activeTheme').should.eql('new-theme');
                 done();
             });
         });
@@ -163,17 +148,15 @@ describe('Theme Handler', function () {
             var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme');
             sandbox.stub(api.settings, 'read').withArgs(sandbox.match.has('key', 'activeTheme')).returns(Promise.resolve({
                 settings: [{
-                    key: 'activateTheme',
+                    key: 'activeKey',
                     value: 'casper'
                 }]
             }));
-
+            blogApp.set('activeTheme', 'casper');
-            configUtils.set({theme: {activeTheme: 'casper'}});
             configUtils.set({paths: {availableThemes: {casper: {}}}});
 
-            themeHandler.updateActiveTheme(blogApp)(req, res, function () {
+            themeHandler.updateActiveTheme(req, res, function () {
-                activateThemeSpy.calledOnce.should.be.true();
+                activateThemeSpy.called.should.be.false();
-                blogApp.get('activeTheme').should.eql('casper');
                 done();
             });
         });
@@ -188,14 +171,13 @@ describe('Theme Handler', function () {
                     value: 'rasper'
                 }]
             }));
-
+            blogApp.set('activeTheme', 'not-casper');
-            configUtils.set({theme: {activeTheme: 'casper'}});
             configUtils.set({paths: {availableThemes: {casper: {}}}});
 
-            themeHandler.updateActiveTheme(blogApp)(req, res, function (err) {
+            themeHandler.updateActiveTheme(req, res, function (err) {
                 should.exist(err);
                 errorSpy.called.should.be.true();
-                activateThemeSpy.calledOnce.should.be.true();
+                activateThemeSpy.called.should.be.false();
                 err.message.should.eql('The currently active theme "rasper" is missing.');
                 done();
             });
@@ -212,14 +194,13 @@ describe('Theme Handler', function () {
                     value: 'rasper'
                 }]
             }));
-
             res.isAdmin = true;
-            configUtils.set({theme: {activeTheme: 'casper'}});
+            blogApp.set('activeTheme', 'not-casper');
             configUtils.set({paths: {availableThemes: {casper: {}}}});
 
-            themeHandler.updateActiveTheme(blogApp)(req, res, function () {
+            themeHandler.updateActiveTheme(req, res, function () {
                 errorSpy.called.should.be.false();
-                activateThemeSpy.calledOnce.should.be.true();
+                activateThemeSpy.called.should.be.false();
                 warnSpy.called.should.be.true();
                 warnSpy.calledWith('The currently active theme "rasper" is missing.').should.be.true();
                 done();