diff --git a/core/server/data/meta/rss_url.js b/core/server/data/meta/rss_url.js index 4fa6731cd9..91d63bc4cc 100644 --- a/core/server/data/meta/rss_url.js +++ b/core/server/data/meta/rss_url.js @@ -1,21 +1,7 @@ const routingService = require('../../services/routing'); -/** - * https://github.com/TryGhost/Team/issues/65#issuecomment-393622816 - * - * For now we output only the default rss feed link. And this is the first collection. - * If the first collection has rss disabled, we output nothing. - * - * @TODO: We are currently investigating this. - */ function getRssUrl(data, absolute) { - const firstCollection = routingService.registry.getFirstCollectionRouter(); - - if (!firstCollection) { - return null; - } - - return firstCollection.getRssUrl({ + return routingService.registry.getRssUrl({ secure: data.secure, absolute: absolute }); diff --git a/core/server/services/routing/CollectionRouter.js b/core/server/services/routing/CollectionRouter.js index 339561dcbf..541ebd6356 100644 --- a/core/server/services/routing/CollectionRouter.js +++ b/core/server/services/routing/CollectionRouter.js @@ -9,16 +9,14 @@ const middlewares = require('./middlewares'); const RSSRouter = require('./RSSRouter'); class CollectionRouter extends ParentRouter { - constructor(indexRoute, object, options) { - options = options || {}; - + constructor(mainRoute, object) { super('CollectionRouter'); - this.firstCollection = options.firstCollection; + this.routerName = mainRoute === '/' ? 'index' : mainRoute.replace(/\//g, ''); // NOTE: index/parent route e.g. /, /podcast/, /magic/ ;) this.route = { - value: indexRoute + value: mainRoute }; this.permalinks = { @@ -52,14 +50,9 @@ class CollectionRouter extends ParentRouter { return this.permalinks.value; }; - // the main post listening collection get's the index context - if (this.firstCollection) { - this.context = ['index']; - } else { - this.context = []; - } + this.context = [this.routerName]; - debug(this.route, this.permalinks); + debug(this.name, this.route, this.permalinks); this._registerRoutes(); this._listeners(); @@ -67,7 +60,7 @@ class CollectionRouter extends ParentRouter { _registerRoutes() { // REGISTER: context middleware for this collection - this.router().use(this._prepareIndexContext.bind(this)); + this.router().use(this._prepareEntriesContext.bind(this)); // REGISTER: collection route e.g. /, /podcast/ this.mountRoute(this.route.value, controllers.collection); @@ -96,7 +89,7 @@ class CollectionRouter extends ParentRouter { * * @TODO: Why do we need two context objects? O_O - refactor this out */ - _prepareIndexContext(req, res, next) { + _prepareEntriesContext(req, res, next) { res.locals.routerOptions = { filter: this.filter, permalinks: this.permalinks.getValue({withUrlOptions: true}), @@ -104,7 +97,8 @@ class CollectionRouter extends ParentRouter { context: this.context, frontPageTemplate: 'home', templates: this.templates, - identifier: this.identifier + identifier: this.identifier, + name: this.routerName }; res._route = { diff --git a/core/server/services/routing/StaticRoutesRouter.js b/core/server/services/routing/StaticRoutesRouter.js index bb10f2537a..701953b540 100644 --- a/core/server/services/routing/StaticRoutesRouter.js +++ b/core/server/services/routing/StaticRoutesRouter.js @@ -4,10 +4,10 @@ const helpers = require('./helpers'); const ParentRouter = require('./ParentRouter'); class StaticRoutesRouter extends ParentRouter { - constructor(key, object) { + constructor(mainRoute, object) { super('StaticRoutesRouter'); - this.route = {value: key}; + this.route = {value: mainRoute}; this.templates = object.templates || []; debug(this.route.value, this.templates); diff --git a/core/server/services/routing/bootstrap.js b/core/server/services/routing/bootstrap.js index 85b3fa1ba4..1f0cd1aa68 100644 --- a/core/server/services/routing/bootstrap.js +++ b/core/server/services/routing/bootstrap.js @@ -47,12 +47,8 @@ module.exports = function bootstrap() { }); _.each(dynamicRoutes.collections, (value, key) => { - const collectionRouter = new CollectionRouter(key, value, { - firstCollection: Object.keys(dynamicRoutes.collections).indexOf(key) === 0 - }); - + const collectionRouter = new CollectionRouter(key, value); siteRouter.mountRouter(collectionRouter.router()); - registry.setRouter(collectionRouter.identifier, collectionRouter); }); diff --git a/core/server/services/routing/helpers/templates.js b/core/server/services/routing/helpers/templates.js index c0936fe92a..9869f3ad2b 100644 --- a/core/server/services/routing/helpers/templates.js +++ b/core/server/services/routing/helpers/templates.js @@ -48,7 +48,7 @@ _private.getErrorTemplateHierarchy = function getErrorTemplateHierarchy(statusCo _private.getCollectionTemplateHierarchy = function getCollectionTemplateHierarchy(routerOptions, requestOptions) { const templateList = ['index']; - // CASE: author, tag + // CASE: author, tag, custom collection name if (routerOptions.name && routerOptions.name !== 'index') { templateList.unshift(routerOptions.name); diff --git a/core/server/services/routing/registry.js b/core/server/services/routing/registry.js index 15de7b56fb..88538d1033 100644 --- a/core/server/services/routing/registry.js +++ b/core/server/services/routing/registry.js @@ -19,14 +19,40 @@ module.exports = { return routers[name]; }, - getFirstCollectionRouter() { - return _.find(routers, (router) => { - if (router.name === 'CollectionRouter' && router.firstCollection) { - return router; - } + /** + * https://github.com/TryGhost/Team/issues/65#issuecomment-393622816 + * + * Hierarchy for primary rss url: + * + * - index collection (/) + * - if you only have one collection, we take this rss url + */ + getRssUrl(options) { + let rssUrl = null; - return false; - }); + const collectionIndexRouter = _.find(routers, {name: 'CollectionRouter', routerName: 'index'}); + + if (collectionIndexRouter) { + rssUrl = collectionIndexRouter.getRssUrl(options); + + // CASE: is rss enabled? + if (rssUrl) { + return rssUrl; + } + } + + const collectionRouters = _.filter(routers, {name: 'CollectionRouter'}); + + if (collectionRouters && collectionRouters.length === 1) { + rssUrl = collectionRouters[0].getRssUrl(options); + + // CASE: is rss enabled? + if (rssUrl) { + return rssUrl; + } + } + + return rssUrl; }, resetAllRoutes() { diff --git a/core/test/integration/helpers/ghost_head_spec.js b/core/test/integration/helpers/ghost_head_spec.js index 4ecddcafa8..4d3e04f247 100644 --- a/core/test/integration/helpers/ghost_head_spec.js +++ b/core/test/integration/helpers/ghost_head_spec.js @@ -16,7 +16,7 @@ const should = require('should'), * either move to integration tests or rewrite!!! */ describe('{{ghost_head}} helper', function () { - let posts = [], tags = [], users = [], firstCollection; + let posts = [], tags = [], users = []; before(function () { testUtils.integrationTesting.defaultMocks(sandbox); @@ -28,9 +28,7 @@ describe('{{ghost_head}} helper', function () { before(function () { models.init(); - firstCollection = sandbox.stub(); - firstCollection.getRssUrl = sandbox.stub().returns('http://localhost:65530/rss/'); - sandbox.stub(routing.registry, 'getFirstCollectionRouter').returns(firstCollection); + sandbox.stub(routing.registry, 'getRssUrl').returns('http://localhost:65530/rss/'); settingsCache.get.withArgs('title').returns('Ghost'); settingsCache.get.withArgs('description').returns('blog description'); @@ -1253,11 +1251,11 @@ describe('{{ghost_head}} helper', function () { url: 'http://localhost:65530/blog/' }); - firstCollection.getRssUrl.returns('http://localhost:65530/blog/rss/'); + routing.registry.getRssUrl.returns('http://localhost:65530/blog/rss/'); }); after(function () { - firstCollection.getRssUrl.returns('http://localhost:65530/rss/'); + routing.registry.getRssUrl.returns('http://localhost:65530/rss/'); }); it('returns correct rss url with subdirectory', function (done) { diff --git a/core/test/unit/data/meta/rss_url_spec.js b/core/test/unit/data/meta/rss_url_spec.js index c068a66563..30c1948962 100644 --- a/core/test/unit/data/meta/rss_url_spec.js +++ b/core/test/unit/data/meta/rss_url_spec.js @@ -5,15 +5,9 @@ const should = require('should'), getRssUrl = require('../../../../server/data/meta/rss_url'); describe('getRssUrl', function () { - let firstCollection; - beforeEach(function () { sandbox.restore(); - - firstCollection = sandbox.stub(); - firstCollection.getRssUrl = sandbox.stub().returns('/rss/'); - - sandbox.stub(routing.registry, 'getFirstCollectionRouter').returns(firstCollection); + sandbox.stub(routing.registry, 'getRssUrl').returns('/rss/'); }); it('should return rss url', function () { @@ -29,6 +23,6 @@ describe('getRssUrl', function () { secure: false }, true); - firstCollection.getRssUrl.calledWith({secure: false, absolute: true}).should.be.true(); + routing.registry.getRssUrl.calledWith({secure: false, absolute: true}).should.be.true(); }); }); diff --git a/core/test/unit/services/routing/CollectionRouter_spec.js b/core/test/unit/services/routing/CollectionRouter_spec.js index adde371496..f7624f8505 100644 --- a/core/test/unit/services/routing/CollectionRouter_spec.js +++ b/core/test/unit/services/routing/CollectionRouter_spec.js @@ -9,27 +9,28 @@ const should = require('should'), describe('UNIT - services/routing/CollectionRouter', function () { let req, res, next; + beforeEach(function () { + sandbox.stub(settingsCache, 'get').withArgs('permalinks').returns('/:slug/'); + + sandbox.stub(common.events, 'emit'); + sandbox.stub(common.events, 'on'); + + sandbox.spy(CollectionRouter.prototype, 'mountRoute'); + sandbox.spy(CollectionRouter.prototype, 'mountRouter'); + sandbox.spy(CollectionRouter.prototype, 'unmountRoute'); + + req = sandbox.stub(); + res = sandbox.stub(); + next = sandbox.stub(); + + res.locals = {}; + }); + + afterEach(function () { + sandbox.restore(); + }); + describe('instantiate', function () { - beforeEach(function () { - sandbox.stub(settingsCache, 'get').withArgs('permalinks').returns('/:slug/'); - - sandbox.stub(common.events, 'emit'); - sandbox.stub(common.events, 'on'); - - sandbox.spy(CollectionRouter.prototype, 'mountRoute'); - sandbox.spy(CollectionRouter.prototype, 'mountRouter'); - - req = sandbox.stub(); - res = sandbox.stub(); - next = sandbox.stub(); - - res.locals = {}; - }); - - afterEach(function () { - sandbox.restore(); - }); - it('default', function () { const collectionRouter = new CollectionRouter('/', {permalink: '/:slug/'}); @@ -64,15 +65,18 @@ describe('UNIT - services/routing/CollectionRouter', function () { collectionRouter.mountRouter.args[0][1].should.eql(collectionRouter.rssRouter.router()); }); - it('first collection option', function () { - const collectionRouter1 = new CollectionRouter('/', {permalink: '/:slug/'}, {firstCollection: true}); - const collectionRouter2 = new CollectionRouter('/', {permalink: '/:slug/'}, {firstCollection: false}); + it('router name', function () { + const collectionRouter1 = new CollectionRouter('/', {permalink: '/:slug/'}); + const collectionRouter2 = new CollectionRouter('/podcast/', {permalink: '/:slug/'}); + const collectionRouter3 = new CollectionRouter('/hello/world/', {permalink: '/:slug/'}); - collectionRouter1.firstCollection.should.be.true(); - collectionRouter2.firstCollection.should.be.false(); + collectionRouter1.routerName.should.eql('index'); + collectionRouter2.routerName.should.eql('podcast'); + collectionRouter3.routerName.should.eql('helloworld'); collectionRouter1.context.should.eql(['index']); - collectionRouter2.context.should.eql([]); + collectionRouter2.context.should.eql(['podcast']); + collectionRouter3.context.should.eql(['helloworld']); }); it('collection lives under /blog/', function () { @@ -136,11 +140,29 @@ describe('UNIT - services/routing/CollectionRouter', function () { }); }); - describe('fn: _prepareIndexContext', function () { - it('default', function () { + describe('fn: _prepareEntriesContext', function () { + it('index collection', function () { + const collectionRouter = new CollectionRouter('/', {permalink: '/:slug/'}); + + collectionRouter._prepareEntriesContext(req, res, next); + + next.calledOnce.should.be.true(); + res.locals.routerOptions.should.eql({ + filter: 'page:false', + permalinks: '/:slug/:options(edit)?/', + frontPageTemplate: 'home', + templates: [], + identifier: collectionRouter.identifier, + context: ['index'], + name: 'index', + type: 'posts' + }); + }); + + it('with templates, no index collection', function () { const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/', templates: ['home', 'index']}); - collectionRouter._prepareIndexContext(req, res, next); + collectionRouter._prepareEntriesContext(req, res, next); next.calledOnce.should.be.true(); res.locals.routerOptions.should.eql({ @@ -149,27 +171,14 @@ describe('UNIT - services/routing/CollectionRouter', function () { frontPageTemplate: 'home', templates: ['index', 'home'], identifier: collectionRouter.identifier, - context: [], + context: ['magic'], + name: 'magic', type: 'posts' }); }); }); describe('permalink in database changes', function () { - beforeEach(function () { - sandbox.stub(settingsCache, 'get').withArgs('permalinks').returns('/:slug/'); - - sandbox.stub(common.events, 'emit'); - sandbox.stub(common.events, 'on'); - - sandbox.spy(CollectionRouter.prototype, 'mountRoute'); - sandbox.spy(CollectionRouter.prototype, 'unmountRoute'); - }); - - afterEach(function () { - sandbox.restore(); - }); - it('permalink placeholder: flat', function () { const collectionRouter = new CollectionRouter('/magic/', {permalink: '{globals.permalinks}'}); diff --git a/core/test/unit/services/routing/helpers/templates_spec.js b/core/test/unit/services/routing/helpers/templates_spec.js index eb5b723e3a..1e5a90b270 100644 --- a/core/test/unit/services/routing/helpers/templates_spec.js +++ b/core/test/unit/services/routing/helpers/templates_spec.js @@ -22,6 +22,18 @@ describe('templates', function () { _private.getCollectionTemplateHierarchy({name: 'index'}).should.eql(['index']); }); + it('should return custom templates even if the collection is index', function () { + _private.getCollectionTemplateHierarchy({name: 'index', templates: ['something']}).should.eql(['something', 'index']); + }); + + it('should return collection name', function () { + _private.getCollectionTemplateHierarchy({name: 'podcast'}).should.eql(['podcast', 'index']); + }); + + it('should return custom templates', function () { + _private.getCollectionTemplateHierarchy({name: 'podcast', templates: ['mozart']}).should.eql(['mozart', 'podcast', 'index']); + }); + it('should return just index if collection name is index even if slug is set', function () { _private.getCollectionTemplateHierarchy({name: 'index', slugTemplate: true}, {slugParam: 'test'}).should.eql(['index']); }); diff --git a/core/test/unit/services/routing/registry_spec.js b/core/test/unit/services/routing/registry_spec.js new file mode 100644 index 0000000000..a51f779f2c --- /dev/null +++ b/core/test/unit/services/routing/registry_spec.js @@ -0,0 +1,59 @@ +const should = require('should'), + sinon = require('sinon'), + rewire = require('rewire'), + registry = rewire('../../../../server/services/routing/registry'), + sandbox = sinon.sandbox.create(); + +describe('UNIT: services/routing/registry', function () { + let getRssUrlStub; + + beforeEach(function () { + getRssUrlStub = sandbox.stub(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + describe('fn: getRssUrl', function () { + it('no url available', function () { + should.not.exist(registry.getRssUrl()); + }); + + it('single collection, no index collection', function () { + registry.setRouter('CollectionRouter', { + name: 'CollectionRouter', + routerName: 'podcast', + getRssUrl: sandbox.stub().returns('/podcast/rss/') + }); + + registry.getRssUrl().should.eql('/podcast/rss/'); + }); + + it('single collection, no index collection, rss disabled', function () { + registry.setRouter('CollectionRouter', { + name: 'CollectionRouter', + routerName: 'podcast', + getRssUrl: sandbox.stub().returns(null) + }); + + should.not.exist(registry.getRssUrl()); + }); + + it('index collection', function () { + registry.setRouter('CollectionRouter', { + name: 'CollectionRouter', + routerName: 'podcast', + getRssUrl: sandbox.stub().returns('/podcast/rss/') + }); + + registry.setRouter('CollectionRouter', { + name: 'CollectionRouter', + routerName: 'index', + getRssUrl: sandbox.stub().returns('/rss/') + }); + + registry.getRssUrl().should.eql('/rss/'); + }); + }); +});