diff --git a/core/server/controllers/channel.js b/core/server/controllers/channel.js new file mode 100644 index 000000000000..d3d8caa124d5 --- /dev/null +++ b/core/server/controllers/channel.js @@ -0,0 +1,46 @@ +var _ = require('lodash'), + errors = require('../errors'), + i18n = require('../i18n'), + filters = require('../filters'), + safeString = require('../utils/index').safeString, + handleError = require('./frontend/error'), + fetchData = require('./frontend/fetch-data'), + setRequestIsSecure = require('./frontend/secure'), + renderChannel = require('./frontend/render-channel'); + +module.exports = function channelController(req, res, next) { + // Parse the parameters we need from the URL + var channelOpts = res.locals.channel, + pageParam = req.params.page !== undefined ? req.params.page : 1, + slugParam = req.params.slug ? safeString(req.params.slug) : undefined; + + // Ensure we at least have an empty object for postOptions + channelOpts.postOptions = channelOpts.postOptions || {}; + // Set page on postOptions for the query made later + channelOpts.postOptions.page = pageParam; + channelOpts.slugParam = slugParam; + + // Call fetchData to get everything we need from the API + return fetchData(channelOpts).then(function handleResult(result) { + // If page is greater than number of pages we have, go straight to 404 + if (pageParam > result.meta.pagination.pages) { + return next(new errors.NotFoundError({message: i18n.t('errors.errors.pageNotFound')})); + } + + // Format data 1 + // @TODO: figure out if this can be removed, it's supposed to ensure that absolutely URLs get generated + // correctly for the various objects, but I believe it doesn't work and a different approach is needed. + setRequestIsSecure(req, result.posts); + _.each(result.data, function (data) { + setRequestIsSecure(req, data); + }); + + // @TODO: properly design these filters + filters.doFilter('prePostsRender', result.posts, res.locals) + .then(function (posts) { + result.posts = posts; + return result; + }) + .then(renderChannel(req, res)); + }).catch(handleError(next)); +}; diff --git a/core/server/controllers/channels/router.js b/core/server/controllers/channels/router.js index ef100d9993a2..1256d68df79a 100644 --- a/core/server/controllers/channels/router.js +++ b/core/server/controllers/channels/router.js @@ -6,7 +6,7 @@ var express = require('express'), rss = require('../../data/xml/rss'), utils = require('../../utils'), channelLoader = require('./loader'), - renderChannel = require('../frontend/render-channel'), + channelController = require('../channel'), rssRouter, channelRouter, channelsRouter; @@ -70,11 +70,11 @@ channelRouter = function channelRouter(channel) { pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page(\\d+)/'), middleware = [channelConfigMiddleware(channel)]; - channelRouter.get(baseRoute, middleware, renderChannel); + channelRouter.get(baseRoute, middleware, channelController); if (channel.isPaged) { channelRouter.param('page', handlePageParam); - channelRouter.get(pageRoute, middleware, renderChannel); + channelRouter.get(pageRoute, middleware, channelController); } if (channel.hasRSS) { diff --git a/core/server/controllers/frontend/render-channel.js b/core/server/controllers/frontend/render-channel.js index 808175a85284..7ee20f5b0caf 100644 --- a/core/server/controllers/frontend/render-channel.js +++ b/core/server/controllers/frontend/render-channel.js @@ -1,56 +1,17 @@ var debug = require('ghost-ignition').debug('channels:render'), - _ = require('lodash'), - errors = require('../../errors'), - i18n = require('../../i18n'), - filters = require('../../filters'), - safeString = require('../../utils/index').safeString, - handleError = require('./error'), - fetchData = require('./fetch-data'), formatResponse = require('./format-response'), setResponseContext = require('./context'), - setRequestIsSecure = require('./secure'), templates = require('./templates'); -function renderChannel(req, res, next) { +module.exports = function renderChannel(req, res) { debug('renderChannel called'); - // Parse the parameters we need from the URL - var channelOpts = res.locals.channel, - pageParam = req.params.page !== undefined ? req.params.page : 1, - slugParam = req.params.slug ? safeString(req.params.slug) : undefined; + return function renderChannel(result) { + var view = templates.channel(res.locals.channel); - // Ensure we at least have an empty object for postOptions - channelOpts.postOptions = channelOpts.postOptions || {}; - // Set page on postOptions for the query made later - channelOpts.postOptions.page = pageParam; - channelOpts.slugParam = slugParam; + result = formatResponse.channel(result); - // Call fetchData to get everything we need from the API - return fetchData(channelOpts).then(function handleResult(result) { - // If page is greater than number of pages we have, go straight to 404 - if (pageParam > result.meta.pagination.pages) { - return next(new errors.NotFoundError({message: i18n.t('errors.errors.pageNotFound')})); - } - - // @TODO: figure out if this can be removed, it's supposed to ensure that absolutely URLs get generated - // correctly for the various objects, but I believe it doesn't work and a different approach is needed. - setRequestIsSecure(req, result.posts); - _.each(result.data, function (data) { - setRequestIsSecure(req, data); - }); - - // @TODO: properly design these filters - filters.doFilter('prePostsRender', result.posts, res.locals).then(function then(posts) { - var view = templates.channel(channelOpts); - - // Do final data formatting and then render - result.posts = posts; - result = formatResponse.channel(result); - - setResponseContext(req, res); - debug('Rendering view: ' + view); - res.render(view, result); - }); - }).catch(handleError(next)); -} - -module.exports = renderChannel; + setResponseContext(req, res); + debug('Rendering view: ' + view); + res.render(view, result); + }; +}; diff --git a/core/server/controllers/preview.js b/core/server/controllers/preview.js index d8acd7f4e95d..c5e1c118f933 100644 --- a/core/server/controllers/preview.js +++ b/core/server/controllers/preview.js @@ -5,7 +5,7 @@ var api = require('../api'), renderPost = require('./frontend/render-post'), setRequestIsSecure = require('./frontend/secure'); -module.exports = function preview(req, res, next) { +module.exports = function previewController(req, res, next) { var params = { uuid: req.params.uuid, status: 'all', diff --git a/core/server/controllers/single.js b/core/server/controllers/single.js index ad24cdf64f04..c4973d332957 100644 --- a/core/server/controllers/single.js +++ b/core/server/controllers/single.js @@ -5,7 +5,7 @@ var utils = require('../utils'), renderPost = require('./frontend/render-post'), setRequestIsSecure = require('./frontend/secure'); -module.exports = function single(req, res, next) { +module.exports = function singleController(req, res, next) { // Query database to find post return postLookup(req.path).then(function then(lookup) { var post = lookup ? lookup.post : false; diff --git a/core/test/unit/controllers/channels/custom_channels_spec.js b/core/test/unit/controllers/channels/custom_channels_spec.js index 6f4faac1eb10..0fcdfa286b75 100644 --- a/core/test/unit/controllers/channels/custom_channels_spec.js +++ b/core/test/unit/controllers/channels/custom_channels_spec.js @@ -11,10 +11,11 @@ var should = require('should'), // jshint ignore:line sandbox = sinon.sandbox.create(); /** - * These tests are a bit weird, - * need to test express private API - * Would be better to be testing our own internal API - * E.g. setupRSS.calledOnce, rather than router stack! + * These tests are a bit weird, because we are testing the express private API + * Would be better to be testing our own internal API E.g. setupRSS.calledOnce, rather than router stack! + * + * This is partly because router_spec.js is testing a full stack of behaviour, including the loader + * And we need to differentiate more between testing the default channels, and channels in general */ describe('Custom Channels', function () { var channelLoaderStub; @@ -52,7 +53,7 @@ describe('Custom Channels', function () { routeStack[0].should.be.a.DispatchLayer({ route: { path: '/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -61,7 +62,7 @@ describe('Custom Channels', function () { keys: ['page'], route: { path: '/page/:page(\\d+)/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -114,7 +115,7 @@ describe('Custom Channels', function () { routeStack[0].should.be.a.DispatchLayer({ route: { path: '/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -123,7 +124,7 @@ describe('Custom Channels', function () { keys: ['page'], route: { path: '/page/:page(\\d+)/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -159,7 +160,7 @@ describe('Custom Channels', function () { routeStack[0].should.be.a.DispatchLayer({ route: { path: '/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -168,7 +169,7 @@ describe('Custom Channels', function () { keys: ['page'], route: { path: '/page/:page(\\d+)/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); @@ -200,7 +201,7 @@ describe('Custom Channels', function () { routeStack[0].should.be.a.DispatchLayer({ route: { path: '/', - stack: ['doChannelConfig', 'renderChannel'] + stack: ['doChannelConfig', 'channelController'] } }); diff --git a/core/test/unit/controllers/channels/loader_spec.js b/core/test/unit/controllers/channels/loader_spec.js index 6a892458b8d9..c160369b0832 100644 --- a/core/test/unit/controllers/channels/loader_spec.js +++ b/core/test/unit/controllers/channels/loader_spec.js @@ -2,30 +2,44 @@ var should = require('should'), // jshint ignore:line _ = require('lodash'), rewire = require('rewire'), channelUtils = require('../../../utils/channelUtils'), - channelLoader = rewire('../../../../server/controllers/channels/loader'), - Channel = channelUtils.Channel; + channelLoader = rewire('../../../../server/controllers/channels/loader'); -describe('Channel Config', function () { - var channelReset; +describe('Channels', function () { + describe('Loader', function () { + // This is a bit of a weird test. because the loader functionality is TEMPORARY + // If you have a local config, that gets loaded instead of the default. + // This just tests that either way, we get a JSON object. + it('should load a JSON object', function () { + var loadConfig = channelLoader.__get__('loadConfig'), + result = loadConfig(); - before(function () { - channelReset = channelLoader.__set__('loadConfig', function () { - return channelUtils.getDefaultChannels(); + result.should.be.an.Object(); }); }); - after(function () { - channelReset(); - }); + describe('Default config', function () { + var channelReset; + + before(function () { + // Change the channels we get returned + channelReset = channelLoader.__set__('loadConfig', function () { + return channelUtils.getDefaultChannels(); + }); + }); + + after(function () { + channelReset(); + }); - it('should build a list of channels', function () { - var channels = channelLoader.list(); - channels.should.be.an.Array().with.lengthOf(3); + it('[default] list() should return a list of channel objects', function () { + var channels = channelLoader.list(); + channels.should.be.an.Array().with.lengthOf(3); - _.map(channels, 'name').should.eql(['index', 'tag', 'author']); + _.map(channels, 'name').should.eql(['index', 'tag', 'author']); - _.each(channels, function (channel) { - channel.should.be.a.Channel(); + _.each(channels, function (channel) { + channel.should.be.a.Channel(); + }); }); }); }); diff --git a/core/test/unit/controllers/channels/router_spec.js b/core/test/unit/controllers/channels/router_spec.js index a421e2cabb6c..e79da818cefa 100644 --- a/core/test/unit/controllers/channels/router_spec.js +++ b/core/test/unit/controllers/channels/router_spec.js @@ -9,6 +9,13 @@ var should = require('should'), themes = require('../../../../server/themes'), sandbox = sinon.sandbox.create(); +/** + * Note: this tests the following all in one go: + * - Channel Router controllers/channels/router + * - Channel Controller controllers/channel.js + * - Channel Renderer controllers/frontend/render-channel.js + * This is because the refactor is in progress! + */ describe('Channels', function () { var channelRouter, req, res, hasTemplateStub, themeConfigStub; @@ -168,6 +175,7 @@ describe('Channels', function () { this.locals.context.should.containEql('index'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({page: 1, limit: 5, include: 'author,tags'}).should.be.true(); }, done); }); @@ -183,6 +191,7 @@ describe('Channels', function () { this.locals.context.should.containEql('index'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({page: 1, limit: 5, include: 'author,tags'}).should.be.true(); }, done); }); @@ -197,6 +206,7 @@ describe('Channels', function () { this.locals.context.should.containEql('index'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({page: 2, limit: 5, include: 'author,tags'}).should.be.true(); }, done); }); @@ -210,6 +220,7 @@ describe('Channels', function () { this.locals.context.should.containEql('index'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({page: 2, limit: 5, include: 'author,tags'}).should.be.true(); }, done); }); @@ -223,6 +234,7 @@ describe('Channels', function () { this.locals.context.should.containEql('index'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({page: 3, limit: 5, include: 'author,tags'}).should.be.true(); }, done); }); @@ -313,7 +325,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 1, limit: 5, include: 'author,tags'}).should.be.true(); tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); @@ -329,7 +343,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 1, limit: 5, include: 'author,tags'}).should.be.true(); tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); @@ -346,7 +362,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 1, limit: 5, include: 'author,tags'}).should.be.true(); tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); @@ -371,6 +389,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 2, limit: 5, include: 'author,tags'}).should.be.true(); + tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); @@ -387,6 +408,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 2, limit: 5, include: 'author,tags'}).should.be.true(); + tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); @@ -408,6 +432,9 @@ describe('Channels', function () { this.locals.context.should.containEql('tag'); postAPIStub.calledOnce.should.be.true(); + postAPIStub.calledWith({filter: 'tags:\'my-tag\'+tags.visibility:public', page: 3, limit: 5, include: 'author,tags'}).should.be.true(); + tagAPIStub.calledOnce.should.be.true(); + tagAPIStub.calledWith({slug: 'my-tag', visibility: 'public'}).should.be.true(); }, done); }); diff --git a/core/test/unit/controllers/frontend/render-channel_spec.js b/core/test/unit/controllers/frontend/render-channel_spec.js deleted file mode 100644 index 6a0ff226f695..000000000000 --- a/core/test/unit/controllers/frontend/render-channel_spec.js +++ /dev/null @@ -1,55 +0,0 @@ -/*jshint expr:true*/ -var should = require('should'), // jshint ignore:line - sinon = require('sinon'), - rewire = require('rewire'), - - channelUtils = require('../../../utils/channelUtils'), - - // stuff being tested - renderChannel = rewire('../../../../server/controllers/frontend/render-channel'), - - sandbox = sinon.sandbox.create(), - originalFetchData; - -describe('Render Channel', function () { - beforeEach(function () { - originalFetchData = renderChannel.__get__('fetchData'); - }); - - afterEach(function () { - sandbox.restore(); - - renderChannel.__set__('fetchData', originalFetchData); - }); - - describe('Tag config', function () { - var req = { - params: {} - }, - res = { - locals: { - channel: channelUtils.getTestChannel('tag') - } - }, - promise = { - then: function () { - return { - catch: function () { - } - }; - } - }; - - it('should return correct tag config', function () { - renderChannel.__set__('fetchData', function (channelOpts) { - channelOpts.name.should.eql('tag'); - channelOpts.postOptions.filter.should.eql('tags:\'%s\'+tags.visibility:public'); - channelOpts.data.tag.options.should.eql({slug: '%s', visibility: 'public'}); - - return promise; - }); - - renderChannel(req, res); - }); - }); -});