From e3c82c1643a41f5abcdf4836b2966a6fb8ff2c65 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 2 Mar 2017 22:05:35 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20Use=20settingsCache=20in=20theme?= =?UTF-8?q?=20handler=20(#8091)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - we already have the settingsCache here, makes no sense to call the API --- core/server/middleware/theme-handler.js | 57 +++++++++---------- .../unit/middleware/theme-handler_spec.js | 34 +++-------- 2 files changed, 33 insertions(+), 58 deletions(-) diff --git a/core/server/middleware/theme-handler.js b/core/server/middleware/theme-handler.js index 6b6c6eddfb..65e20f06ce 100644 --- a/core/server/middleware/theme-handler.js +++ b/core/server/middleware/theme-handler.js @@ -2,13 +2,12 @@ var _ = require('lodash'), fs = require('fs'), path = require('path'), hbs = require('express-hbs'), - api = require('../api'), - settingsCache = require('../settings/cache'), config = require('../config'), utils = require('../utils'), logging = require('../logging'), errors = require('../errors'), i18n = require('../i18n'), + settingsCache = require('../settings/cache'), themeList = require('../themes').list, themeHandler; @@ -88,39 +87,35 @@ themeHandler = { // activates that theme's views with the hbs templating engine if it // is not yet activated. updateActiveTheme: function updateActiveTheme(req, res, next) { - var blogApp = req.app; + var blogApp = req.app, + activeTheme = settingsCache.get('activeTheme'); - api.settings.read({context: {internal: true}, key: 'activeTheme'}).then(function then(response) { - var activeTheme = response.settings[0]; - - // Check if the theme changed - if (activeTheme.value !== blogApp.get('activeTheme')) { - // Change theme - if (!themeList.get(activeTheme.value)) { - if (!res.isAdmin) { - return next(new errors.NotFoundError({ - message: i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value}) - })); - } else { - // 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()); - logging.warn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value})); - return next(); - } + // Check if the theme changed + if (activeTheme !== blogApp.get('activeTheme')) { + // Change theme + if (!themeList.get(activeTheme)) { + if (!res.isAdmin) { + // 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()); + return next(new errors.NotFoundError({ + message: i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme}) + })); } 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()); + logging.warn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme})); + return next(); } + } else { + themeHandler.activateTheme(blogApp, activeTheme); } - next(); - }).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); - }); + } + + next(); } }; diff --git a/core/test/unit/middleware/theme-handler_spec.js b/core/test/unit/middleware/theme-handler_spec.js index b1fa4ba547..33c47df4f3 100644 --- a/core/test/unit/middleware/theme-handler_spec.js +++ b/core/test/unit/middleware/theme-handler_spec.js @@ -1,13 +1,13 @@ var sinon = require('sinon'), should = require('should'), express = require('express'), - Promise = require('bluebird'), fs = require('fs'), hbs = require('express-hbs'), themeList = require('../../../server/themes').list, themeHandler = require('../../../server/middleware/theme-handler'), logging = require('../../../server/logging'), - api = require('../../../server/api'), + settingsCache = require('../../../server/settings/cache'), + sandbox = sinon.sandbox.create(); describe('Theme Handler', function () { @@ -96,12 +96,7 @@ describe('Theme Handler', function () { 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: 'activeKey', - value: 'casper' - }] - })); + sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('casper'); blogApp.set('activeTheme', 'not-casper'); themeHandler.updateActiveTheme(req, res, function () { @@ -112,12 +107,8 @@ describe('Theme Handler', function () { it('does not update the active theme if not 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: 'activeKey', - value: 'casper' - }] - })); + + sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('casper'); blogApp.set('activeTheme', 'casper'); themeHandler.updateActiveTheme(req, res, function () { @@ -129,13 +120,7 @@ describe('Theme Handler', function () { it('throws error if theme is missing', function (done) { var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme'); - sandbox.stub(api.settings, 'read').withArgs(sandbox.match.has('key', 'activeTheme')).returns(Promise.resolve({ - settings: [{ - key: 'activeKey', - value: 'rasper' - }] - })); - + sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('rasper'); blogApp.set('activeTheme', 'not-casper'); themeHandler.updateActiveTheme(req, res, function (err) { @@ -150,12 +135,7 @@ describe('Theme Handler', function () { var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme'), loggingWarnStub = sandbox.spy(logging, 'warn'); - sandbox.stub(api.settings, 'read').withArgs(sandbox.match.has('key', 'activeTheme')).returns(Promise.resolve({ - settings: [{ - key: 'activeKey', - value: 'rasper' - }] - })); + sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('rasper'); res.isAdmin = true; blogApp.set('activeTheme', 'not-casper');