Skip to content

Commit

Permalink
✨ Posts per page as theme-config (#8149)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ErisDS authored and kirrg001 committed Mar 14, 2017
1 parent b8162b1 commit 3416c05
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 31 deletions.
4 changes: 2 additions & 2 deletions core/server/controllers/frontend/fetch-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
var api = require('../../api'),
_ = require('lodash'),
Promise = require('bluebird'),
settingsCache = require('../../settings/cache'),
themes = require('../../themes'),
queryDefaults,
defaultPostQuery = {};

Expand Down Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion core/server/data/meta/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
8 changes: 0 additions & 8 deletions core/server/data/schema/default-settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@
"isEmpty": false
}
},
"postsPerPage": {
"defaultValue": "5",
"validations": {
"isEmpty": false,
"isInt": true,
"isLength": [1, 1000]
}
},
"activeTimezone": {
"defaultValue": "Etc/UTC",
"validations": {
Expand Down
17 changes: 11 additions & 6 deletions core/server/middleware/theme-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
});

Expand Down
20 changes: 20 additions & 0 deletions core/server/themes/active.js
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -43,6 +56,9 @@ class ActiveTheme {
}
return templates;
}, []);

// Do something with config here
this._config = tempConfigHandler(this._packageInfo);
}

get name() {
Expand All @@ -64,6 +80,10 @@ class ActiveTheme {
hasTemplate(templateName) {
return this._templates.indexOf(templateName) > -1;
}

config(key) {
return this._config[key];
}
}

module.exports = {
Expand Down
3 changes: 3 additions & 0 deletions core/server/themes/defaults.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"posts_per_page": 5
}
7 changes: 5 additions & 2 deletions core/test/unit/controllers/frontend/channels_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
});
}

Expand Down
21 changes: 10 additions & 11 deletions core/test/unit/controllers/frontend/fetch-data_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,29 @@ 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();

describe('fetchData', function () {
var apiPostsStub,
apiTagStub,
apiUserStub;
apiUserStub,
themeConfigStub;

beforeEach(function () {
apiPostsStub = sandbox.stub(api.posts, 'browse')
.returns(new Promise.resolve({posts: [], meta: {pagination: {}}}));

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 () {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion core/test/unit/middleware/theme-handler_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down

0 comments on commit 3416c05

Please sign in to comment.