From c8bb6081ab70660ce50978582537084354d9c979 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sat, 10 Oct 2015 18:51:38 +0100 Subject: [PATCH] Frontend controller refactor & test improvements no issue - Split context out of frontend controller - Add 100% test coverage for context - Add tests for preview & improve other bits of coverage - Further refactors (WIP) will make it easier to reach 100% coverage on the frontend --- core/server/controllers/frontend/context.js | 60 +++++ .../{frontend.js => frontend/index.js} | 51 +--- .../unit/controllers/frontend/context_spec.js | 248 ++++++++++++++++++ .../frontend/index_spec.js} | 244 ++++++++++++++++- 4 files changed, 551 insertions(+), 52 deletions(-) create mode 100644 core/server/controllers/frontend/context.js rename core/server/controllers/{frontend.js => frontend/index.js} (86%) create mode 100644 core/test/unit/controllers/frontend/context_spec.js rename core/test/unit/{frontend_spec.js => controllers/frontend/index_spec.js} (86%) diff --git a/core/server/controllers/frontend/context.js b/core/server/controllers/frontend/context.js new file mode 100644 index 0000000000..b79fa75ea5 --- /dev/null +++ b/core/server/controllers/frontend/context.js @@ -0,0 +1,60 @@ +/** + * # Response context + * + * Figures out which context we are currently serving. The biggest challenge with determining this + * is that the only way to determine whether or not we are a post, or a page, is with data after all the + * data for the template has been retrieved. + * + * Contexts are determined based on 3 pieces of information + * 1. res.locals.relativeUrl - which never includes the subdirectory + * 2. req.params.page - always has the page parameter, regardless of if the URL contains a keyword (RSS pages don't) + * 3. data - used for telling the difference between posts and pages + */ + +var config = require('../../config'), + + // Context patterns, should eventually come from Channel configuration + tagPattern = new RegExp('^\\/' + config.routeKeywords.tag + '\\/.+'), + authorPattern = new RegExp('^\\/' + config.routeKeywords.author + '\\/.+'), + privatePattern = new RegExp('^\\/' + config.routeKeywords.private + '\\/'), + indexPattern = new RegExp('^\\/' + config.routeKeywords.page + '\\/'), + rssPattern = new RegExp('^\\/rss\\/'), + homePattern = new RegExp('^\\/$'); + +function setResponseContext(req, res, data) { + var pageParam = req.params && req.params.page !== undefined ? parseInt(req.params.page, 10) : 1; + + res.locals = res.locals || {}; + res.locals.context = []; + + // If we don't have a relativeUrl, we can't detect the context, so return + if (!res.locals.relativeUrl) { + return; + } + + // paged context + if (!isNaN(pageParam) && pageParam > 1) { + res.locals.context.push('paged'); + } + + if (indexPattern.test(res.locals.relativeUrl)) { + res.locals.context.push('index'); + } else if (homePattern.test(res.locals.relativeUrl)) { + res.locals.context.push('home'); + res.locals.context.push('index'); + } else if (rssPattern.test(res.locals.relativeUrl)) { + res.locals.context.push('rss'); + } else if (privatePattern.test(res.locals.relativeUrl)) { + res.locals.context.push('private'); + } else if (tagPattern.test(res.locals.relativeUrl)) { + res.locals.context.push('tag'); + } else if (authorPattern.test(res.locals.relativeUrl)) { + res.locals.context.push('author'); + } else if (data && data.post && data.post.page) { + res.locals.context.push('page'); + } else { + res.locals.context.push('post'); + } +} + +module.exports = setResponseContext; diff --git a/core/server/controllers/frontend.js b/core/server/controllers/frontend/index.js similarity index 86% rename from core/server/controllers/frontend.js rename to core/server/controllers/frontend/index.js index 60dc10570a..c5a6febde9 100644 --- a/core/server/controllers/frontend.js +++ b/core/server/controllers/frontend/index.js @@ -5,16 +5,17 @@ /*global require, module */ var _ = require('lodash'), - api = require('../api'), - rss = require('../data/xml/rss'), + api = require('../../api'), + rss = require('../../data/xml/rss'), path = require('path'), - config = require('../config'), - errors = require('../errors'), - filters = require('../filters'), + config = require('../../config'), + errors = require('../../errors'), + filters = require('../../filters'), Promise = require('bluebird'), - template = require('../helpers/template'), + template = require('../../helpers/template'), routeMatch = require('path-match')(), - safeString = require('../utils/index').safeString, + safeString = require('../../utils/index').safeString, + setResponseContext = require('./context'), frontendControllers, staticPostPermalink = routeMatch('/:slug/:edit?'); @@ -69,42 +70,6 @@ function handleError(next) { }; } -function setResponseContext(req, res, data) { - var contexts = [], - pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1, - tagPattern = new RegExp('^\\/' + config.routeKeywords.tag + '\\/.+'), - authorPattern = new RegExp('^\\/' + config.routeKeywords.author + '\\/.+'), - privatePattern = new RegExp('^\\/' + config.routeKeywords.private + '\\/'), - indexPattern = new RegExp('^\\/' + config.routeKeywords.page + '\\/'), - homePattern = new RegExp('^\\/$'); - - // paged context - if (!isNaN(pageParam) && pageParam > 1) { - contexts.push('paged'); - } - - if (indexPattern.test(res.locals.relativeUrl)) { - contexts.push('index'); - } else if (homePattern.test(res.locals.relativeUrl)) { - contexts.push('home'); - contexts.push('index'); - } else if (/^\/rss\//.test(res.locals.relativeUrl)) { - contexts.push('rss'); - } else if (privatePattern.test(res.locals.relativeUrl)) { - contexts.push('private'); - } else if (tagPattern.test(res.locals.relativeUrl)) { - contexts.push('tag'); - } else if (authorPattern.test(res.locals.relativeUrl)) { - contexts.push('author'); - } else if (data && data.post && data.post.page) { - contexts.push('page'); - } else { - contexts.push('post'); - } - - res.locals.context = contexts; -} - // Add Request context parameter to the data object // to be passed down to the templates function setReqCtx(req, data) { diff --git a/core/test/unit/controllers/frontend/context_spec.js b/core/test/unit/controllers/frontend/context_spec.js new file mode 100644 index 0000000000..c533321563 --- /dev/null +++ b/core/test/unit/controllers/frontend/context_spec.js @@ -0,0 +1,248 @@ +/*globals describe, beforeEach, it*/ +var should = require('should'), + + // Stuff we are testing + setResponseContext = require('../../../../server/controllers/frontend/context'); + +describe('Contexts', function () { + var req, res, data; + + beforeEach(function () { + req = { + params: {} + }; + res = { + locals: {} + }; + data = {}; + }); + + describe('Unknown', function () { + it('should return empty array with no error if all parameters are empty', function () { + // Reset all parameters to empty; + req = {}; + res = {}; + data = {}; + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(0); + }); + + it('should return empty array with no error with basic parameters', function () { + // BeforeEach sets each of these to the bare minimum that should be provided for determining context + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(0); + }); + }); + + describe('Index', function () { + it('should correctly identify `/` as index', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('home'); + res.locals.context[1].should.eql('index'); + }); + + it('should correctly identify `/` as home & index', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('home'); + res.locals.context[1].should.eql('index'); + }); + + it('will not identify `/page/2/` as index & paged without page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/page/2/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('index'); + }); + + it('should identify `/page/2/` as index & paged with page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/page/2/'; + req.params.page = 2; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('paged'); + res.locals.context[1].should.eql('index'); + }); + }); + + describe('RSS', function () { + it('should correctly identify `/rss/` as rss', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/rss/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('rss'); + }); + + it('will not identify `/rss/2/` as rss & paged without page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/rss/2/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('rss'); + }); + + it('should correctly identify `/rss/2/` as rss & paged with page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/rss/2/'; + req.params.page = 2; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('paged'); + res.locals.context[1].should.eql('rss'); + }); + }); + + describe('Tag', function () { + it('should correctly identify `/tag/getting-started/` as tag', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/tag/getting-started/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('tag'); + }); + + it('should not identify just `/tag/` as being the tag context', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/tag/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('post'); + }); + + it('will not identify `/tag/getting-started/page/2/ as paged without page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/tag/getting-started/page/2/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('tag'); + }); + + it('should correctly identify `/tag/getting-started/page/2/ as paged with page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/tag/getting-started/page/2/'; + req.params.page = 2; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('paged'); + res.locals.context[1].should.eql('tag'); + }); + }); + + describe('Author', function () { + it('should correctly identify `/author/pat/` as author', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/author/pat/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('author'); + }); + + it('should not identify just `/author/` as being the author context', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/author/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('post'); + }); + + it('will not identify `/author/pat/page/2/ as paged without page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/author/pat/page/2/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('author'); + }); + + it('should correctly identify `/author/pat/page/2/ as paged with page param', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/author/pat/page/2/'; + req.params.page = 2; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(2); + res.locals.context[0].should.eql('paged'); + res.locals.context[1].should.eql('author'); + }); + }); + + describe('Posts & Pages', function () { + it('should correctly identify a post', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/welcome-to-ghost/'; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('post'); + }); + + it('should correctly idenfity a page', function () { + // Setup test by setting relativeUrl + res.locals.relativeUrl = '/about/'; + data.post = {page: true}; + + setResponseContext(req, res, data); + + should.exist(res.locals.context); + res.locals.context.should.be.an.Array.with.lengthOf(1); + res.locals.context[0].should.eql('page'); + }); + }); +}); diff --git a/core/test/unit/frontend_spec.js b/core/test/unit/controllers/frontend/index_spec.js similarity index 86% rename from core/test/unit/frontend_spec.js rename to core/test/unit/controllers/frontend/index_spec.js index d460b04c8f..edeabbeebc 100644 --- a/core/test/unit/frontend_spec.js +++ b/core/test/unit/controllers/frontend/index_spec.js @@ -8,12 +8,12 @@ var moment = require('moment'), _ = require('lodash'), // Stuff we are testing - api = require('../../server/api'), + api = require('../../../../server/api'), - frontend = rewire('../../server/controllers/frontend'), - config = require('../../server/config'), + frontend = rewire('../../../../server/controllers/frontend'), + config = require('../../../../server/config'), origConfig = _.cloneDeep(config), - defaultConfig = require('../../../config.example')[process.env.NODE_ENV]; + defaultConfig = require('../../../../../config.example')[process.env.NODE_ENV]; // To stop jshint complaining should.equal(true, true); @@ -31,7 +31,7 @@ describe('Frontend Controller', function () { sandbox = sinon.sandbox.create(); // Reset frontend controller for next test - frontend = rewire('../../server/controllers/frontend'); + frontend = rewire('../../../../server/controllers/frontend'); }); afterEach(function () { @@ -43,7 +43,8 @@ describe('Frontend Controller', function () { // from failing via timeout when they // should just immediately fail function failTest(done, msg) { - return function () { + return function (stuf) { + console.log(msg, stuf); done(new Error(msg)); }; } @@ -574,9 +575,10 @@ describe('Frontend Controller', function () { beforeEach(function () { sandbox.stub(api.posts, 'read', function (args) { - return Promise.resolve(_.find(mockPosts, function (mock) { + var post = _.find(mockPosts, function (mock) { return mock.posts[0].slug === args.slug; - })); + }); + return Promise.resolve(post || {posts: []}); }); apiSettingsStub = sandbox.stub(api.settings, 'read'); @@ -945,6 +947,24 @@ describe('Frontend Controller', function () { done(); }); }); + + it('should call next if post is not found', function (done) { + var req = { + path: '/unknown/' + }, + res = { + locals: {}, + render: sinon.spy(), + redirect: sinon.spy() + }; + + frontend.single(req, res, function (err) { + should.not.exist(err); + res.render.called.should.be.false; + res.redirect.called.should.be.false; + done(); + }); + }); }); describe('permalink set to date', function () { @@ -1331,6 +1351,38 @@ describe('Frontend Controller', function () { }); }); }); + + describe('permalink set to custom format no slash', function () { + beforeEach(function () { + apiSettingsStub.withArgs('permalinks').returns(Promise.resolve({ + settings: [{ + value: '/:year/:slug' + }] + })); + + var date = moment(mockPosts[1].posts[0].published_at).format('YYYY'); + mockPosts[1].posts[0].url = '/' + date + '/' + mockPosts[1].posts[0].slug + '/'; + }); + + // Handle Edit append + it('will redirect post to admin edit page via /:year/:id/edit/', function (done) { + var date = moment(mockPosts[1].posts[0].published_at).format('YYYY'), + req = { + path: '/' + [date, mockPosts[1].posts[0].slug, 'edit'].join('/') + '/' + }, + res = { + locals: {}, + render: sinon.spy(), + redirect: function (arg) { + res.render.called.should.be.false; + arg.should.eql(adminEditPagePath + mockPosts[1].posts[0].id + '/'); + done(); + } + }; + + frontend.single(req, res, failTest(done)); + }); + }); }); }); @@ -1342,7 +1394,8 @@ describe('Frontend Controller', function () { route: {path: '/private/?r=/'}, query: {r: ''}, params: {} - }, + }; + config = { paths: { adminViews: '/core/server/views', @@ -1406,4 +1459,177 @@ describe('Frontend Controller', function () { frontend.private(req, res, failTest(done)); }); }); + + describe('preview', function () { + var mockPosts = [{ + posts: [{ + status: 'draft', + uuid: 'abc-1234-01', + id: 1, + title: 'Test static page', + slug: 'test-static-page', + markdown: 'Test static page content', + page: 1, + author: { + id: 1, + name: 'Test User', + slug: 'test', + email: 'test@ghost.org' + }, + url: '/test-static-page/' + }] + }, { + posts: [{ + status: 'draft', + uuid: 'abc-1234-02', + id: 2, + title: 'Test normal post', + slug: 'test-normal-post', + markdown: 'The test normal post content', + page: 0, + author: { + id: 1, + name: 'Test User', + slug: 'test', + email: 'test@ghost.org' + } + }] + }, { + posts: [{ + status: 'published', + uuid: 'abc-1234-03', + id: 3, + title: 'Getting started', + slug: 'about', + markdown: 'This is a blog post', + page: 0, + published_at: new Date('2014/1/30').getTime(), + author: { + id: 1, + name: 'Test User', + slug: 'test', + email: 'test@ghost.org' + }, + url: '/getting-started/' + }] + }]; + + beforeEach(function () { + sandbox.stub(api.posts, 'read', function (args) { + var post = _.find(mockPosts, function (mock) { + return mock.posts[0].uuid === args.uuid; + }); + return Promise.resolve(post || {posts: []}); + }); + + apiSettingsStub = sandbox.stub(api.settings, 'read'); + + apiSettingsStub.withArgs(sinon.match.has('key', 'activeTheme')).returns(Promise.resolve({ + settings: [{ + key: 'activeTheme', + value: 'casper' + }] + })); + + frontend.__set__('config', { + paths: { + subdir: '', + availableThemes: { + casper: { + assets: null, + 'default.hbs': '/content/themes/casper/default.hbs', + 'index.hbs': '/content/themes/casper/index.hbs', + 'page.hbs': '/content/themes/casper/page.hbs', + 'page-about.hbs': '/content/themes/casper/page-about.hbs', + 'post.hbs': '/content/themes/casper/post.hbs' + } + } + }, + routeKeywords: { + page: 'page', + tag: 'tag', + author: 'author' + }, + urlFor: function (type, posts) { + return posts.post.url; + } + }); + }); + + it('should render draft post', function (done) { + var req = { + params: { + uuid: 'abc-1234-02' + } + }, + res = { + render: function (view, context) { + view.should.equal('post'); + should.exist(context.post); + context.post.should.equal(mockPosts[1].posts[0]); + done(); + } + }; + + frontend.preview(req, res, failTest(done)); + }); + + it('should render draft page', function (done) { + var req = { + params: { + uuid: 'abc-1234-01' + } + }, + res = { + render: function (view, context) { + view.should.equal('page'); + should.exist(context.post); + context.post.should.equal(mockPosts[0].posts[0]); + done(); + } + }; + + frontend.preview(req, res, failTest(done)); + }); + + it('should call next if post is not found', function (done) { + var req = { + params: { + uuid: 'abc-1234-04' + } + }, + res = { + locals: {}, + render: sinon.spy(), + redirect: sinon.spy() + }; + + frontend.preview(req, res, function (err) { + should.not.exist(err); + res.render.called.should.be.false; + res.redirect.called.should.be.false; + done(); + }); + }); + + it('should call redirect if post is published', function (done) { + var req = { + params: { + uuid: 'abc-1234-03' + } + }, + res = { + locals: {}, + render: sinon.spy(), + redirect: function (status, url) { + res.render.called.should.be.false; + status.should.eql(301); + url.should.eql('/getting-started/'); + done(); + } + }; + + frontend.preview(req, res, failTest(done)); + }); + }); });