From 3416c050645e27c78a49b2317ae32d473029c140 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 14 Mar 2017 17:03:36 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Posts=20per=20page=20as=20theme-con?= =?UTF-8?q?fig=20(#8149)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #8131 - Remove ppp from default-settings.json - Remove ppp from meta (unused?\!) - ✨ Basic concept of theme config - use theme config ppp setting - ✨ Make @config.posts_per_page helper available - rather than @blog.posts_per_page, we now have @config.posts_per_page - 🚨 Test updates - Adding TODO note --- .../server/controllers/frontend/fetch-data.js | 4 ++-- core/server/data/meta/index.js | 1 - core/server/data/schema/default-settings.json | 8 ------- core/server/middleware/theme-handler.js | 17 +++++++++------ core/server/themes/active.js | 20 ++++++++++++++++++ core/server/themes/defaults.json | 3 +++ .../controllers/frontend/channels_spec.js | 7 +++++-- .../controllers/frontend/fetch-data_spec.js | 21 +++++++++---------- .../unit/middleware/theme-handler_spec.js | 4 +++- 9 files changed, 54 insertions(+), 31 deletions(-) create mode 100644 core/server/themes/defaults.json diff --git a/core/server/controllers/frontend/fetch-data.js b/core/server/controllers/frontend/fetch-data.js index 03f01b00053a..d6fd6b5a7235 100644 --- a/core/server/controllers/frontend/fetch-data.js +++ b/core/server/controllers/frontend/fetch-data.js @@ -5,7 +5,7 @@ var api = require('../../api'), _ = require('lodash'), Promise = require('bluebird'), - settingsCache = require('../../settings/cache'), + themes = require('../../themes'), queryDefaults, defaultPostQuery = {}; @@ -33,7 +33,7 @@ _.extend(defaultPostQuery, queryDefaults, { function fetchPostsPerPage(options) { options = options || {}; - var postsPerPage = parseInt(settingsCache.get('postsPerPage')); + var postsPerPage = parseInt(themes.getActive().config('posts_per_page')); // No negative posts per page, must be number if (!isNaN(postsPerPage) && postsPerPage > 0) { diff --git a/core/server/data/meta/index.js b/core/server/data/meta/index.js index 7a2c5b3be256..ee59e100010f 100644 --- a/core/server/data/meta/index.js +++ b/core/server/data/meta/index.js @@ -54,7 +54,6 @@ function getMetaData(data, root) { twitter: settingsCache.get('twitter'), timezone: settingsCache.get('activeTimezone'), navigation: settingsCache.get('navigation'), - posts_per_page: settingsCache.get('postsPerPage'), icon: settingsCache.get('icon'), cover: settingsCache.get('cover'), logo: settingsCache.get('logo'), diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index 60b579e34f78..04d4ba6437ae 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -35,14 +35,6 @@ "isEmpty": false } }, - "postsPerPage": { - "defaultValue": "5", - "validations": { - "isEmpty": false, - "isInt": true, - "isLength": [1, 1000] - } - }, "activeTimezone": { "defaultValue": "Etc/UTC", "validations": { diff --git a/core/server/middleware/theme-handler.js b/core/server/middleware/theme-handler.js index d15004dc6722..4fd7ca86eeda 100644 --- a/core/server/middleware/theme-handler.js +++ b/core/server/middleware/theme-handler.js @@ -14,24 +14,28 @@ themeHandler = { configHbsForContext: function configHbsForContext(req, res, next) { // Static information, same for every request unless the settings change // @TODO: bind this once and then update based on events? - var themeData = { + var blogData = { title: settingsCache.get('title'), description: settingsCache.get('description'), facebook: settingsCache.get('facebook'), twitter: settingsCache.get('twitter'), timezone: settingsCache.get('activeTimezone'), navigation: settingsCache.get('navigation'), - posts_per_page: settingsCache.get('postsPerPage'), icon: settingsCache.get('icon'), cover: settingsCache.get('cover'), logo: settingsCache.get('logo'), amp: settingsCache.get('amp') }, - labsData = _.cloneDeep(settingsCache.get('labs')); + labsData = _.cloneDeep(settingsCache.get('labs')), + themeData = {}; + + if (themeUtils.getActive()) { + themeData.posts_per_page = themeUtils.getActive().config('posts_per_page'); + } // Request-specific information // These things are super dependent on the request, so they need to be in middleware - themeData.url = utils.url.urlFor('home', {secure: req.secure}, true); + blogData.url = utils.url.urlFor('home', {secure: req.secure}, true); // Pass 'secure' flag to the view engine // so that templates can choose to render https or http 'url', see url utility @@ -40,8 +44,9 @@ themeHandler = { // @TODO: only do this if something changed? hbs.updateTemplateOptions({ data: { - blog: themeData, - labs: labsData + blog: blogData, + labs: labsData, + config: themeData } }); diff --git a/core/server/themes/active.js b/core/server/themes/active.js index 3526ffb23989..b7fe1d5aa710 100644 --- a/core/server/themes/active.js +++ b/core/server/themes/active.js @@ -18,9 +18,22 @@ */ var _ = require('lodash'), join = require('path').join, + defaultConfig = require('./defaults.json'), // Current instance of ActiveTheme currentActiveTheme; +// @TODO: will clean this code up later, honest! (and add tests) +function tempConfigHandler(packageJson) { + var config = _.cloneDeep(defaultConfig), + allowedKeys = ['posts_per_page']; + + if (packageJson && packageJson.hasOwnProperty('config')) { + config = _.assign(config, _.pick(packageJson.config, allowedKeys)); + } + + return config; +} + class ActiveTheme { /** * @TODO this API needs to be simpler, but for now should work! @@ -43,6 +56,9 @@ class ActiveTheme { } return templates; }, []); + + // Do something with config here + this._config = tempConfigHandler(this._packageInfo); } get name() { @@ -64,6 +80,10 @@ class ActiveTheme { hasTemplate(templateName) { return this._templates.indexOf(templateName) > -1; } + + config(key) { + return this._config[key]; + } } module.exports = { diff --git a/core/server/themes/defaults.json b/core/server/themes/defaults.json new file mode 100644 index 000000000000..daf034df4d68 --- /dev/null +++ b/core/server/themes/defaults.json @@ -0,0 +1,3 @@ +{ + "posts_per_page": 5 +} diff --git a/core/test/unit/controllers/frontend/channels_spec.js b/core/test/unit/controllers/frontend/channels_spec.js index e2d57e5d1c46..2d49d2a9f354 100644 --- a/core/test/unit/controllers/frontend/channels_spec.js +++ b/core/test/unit/controllers/frontend/channels_spec.js @@ -10,7 +10,7 @@ var should = require('should'), sandbox = sinon.sandbox.create(); describe('Channels', function () { - var channelRouter, req, res, hasTemplateStub; + var channelRouter, req, res, hasTemplateStub, themeConfigStub; // Initialise 'req' with the bare minimum properties function setupRequest() { @@ -100,8 +100,11 @@ describe('Channels', function () { hasTemplateStub = sandbox.stub().returns(false); hasTemplateStub.withArgs('index').returns(true); + themeConfigStub = sandbox.stub().withArgs('posts_per_page').returns(5); + sandbox.stub(themes, 'getActive').returns({ - hasTemplate: hasTemplateStub + hasTemplate: hasTemplateStub, + config: themeConfigStub }); } diff --git a/core/test/unit/controllers/frontend/fetch-data_spec.js b/core/test/unit/controllers/frontend/fetch-data_spec.js index 71ba2c0041b4..e25083fe46e3 100644 --- a/core/test/unit/controllers/frontend/fetch-data_spec.js +++ b/core/test/unit/controllers/frontend/fetch-data_spec.js @@ -3,7 +3,7 @@ var should = require('should'), Promise = require('bluebird'), // Stuff we are testing api = require('../../../../server/api'), - settingsCache = require('../../../../server/settings/cache'), + themes = require('../../../../server/themes'), fetchData = require('../../../../server/controllers/frontend/fetch-data'), configUtils = require('../../../utils/configUtils'), sandbox = sinon.sandbox.create(); @@ -11,7 +11,8 @@ var should = require('should'), describe('fetchData', function () { var apiPostsStub, apiTagStub, - apiUserStub; + apiUserStub, + themeConfigStub; beforeEach(function () { apiPostsStub = sandbox.stub(api.posts, 'browse') @@ -19,6 +20,12 @@ describe('fetchData', function () { apiTagStub = sandbox.stub(api.tags, 'read').returns(new Promise.resolve({tags: []})); apiUserStub = sandbox.stub(api.users, 'read').returns(new Promise.resolve({users: []})); + + themeConfigStub = sandbox.stub().withArgs('posts_per_page').returns(10); + + sandbox.stub(themes, 'getActive').returns({ + config: themeConfigStub + }); }); afterEach(function () { @@ -27,10 +34,6 @@ describe('fetchData', function () { }); describe('channel config', function () { - beforeEach(function () { - sandbox.stub(settingsCache, 'get').returns(10); - }); - it('should handle no post options', function (done) { fetchData({}).then(function (result) { should.exist(result); @@ -154,10 +157,6 @@ describe('fetchData', function () { }); describe('valid postsPerPage', function () { - beforeEach(function () { - sandbox.stub(settingsCache, 'get').returns(10); - }); - it('Adds limit & includes to options by default', function (done) { fetchData({}).then(function () { apiPostsStub.calledOnce.should.be.true(); @@ -171,7 +170,7 @@ describe('fetchData', function () { describe('invalid postsPerPage', function () { beforeEach(function () { - sandbox.stub(settingsCache, 'get').returns(-1); + themeConfigStub.withArgs('posts_per_page').returns(-1); }); it('Will not add limit if postsPerPage is not valid', function (done) { diff --git a/core/test/unit/middleware/theme-handler_spec.js b/core/test/unit/middleware/theme-handler_spec.js index e6fa1beb3591..b16bda3cb365 100644 --- a/core/test/unit/middleware/theme-handler_spec.js +++ b/core/test/unit/middleware/theme-handler_spec.js @@ -19,7 +19,9 @@ describe('Theme Handler', function () { blogApp = express(); req.app = blogApp; - getActiveThemeStub = sandbox.stub(themeUtils, 'getActive').returns({}); + getActiveThemeStub = sandbox.stub(themeUtils, 'getActive').returns({ + config: sandbox.stub() + }); }); afterEach(function () {