From 4556e1df0a6d116f178ef24195298ac4f768d4bf Mon Sep 17 00:00:00 2001 From: Johan Stenehall Date: Wed, 5 Mar 2014 23:03:39 +0100 Subject: [PATCH] Rss support for tags closes #2260 - added routes for /tag/:slug/rss and /tag/:slug/rss/:page - added support for tag in the rss controller - added route tests for each extra case - fixing a tiny typo in some test descriptions --- core/server/controllers/frontend.js | 63 +++++++++++++------- core/server/routes/frontend.js | 2 + core/test/functional/routes/frontend_test.js | 55 +++++++++++++++-- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/core/server/controllers/frontend.js b/core/server/controllers/frontend.js index 0d8b8b390d..7c220e0325 100644 --- a/core/server/controllers/frontend.js +++ b/core/server/controllers/frontend.js @@ -252,11 +252,16 @@ frontendControllers = { 'rss': function (req, res, next) { // Initialize RSS var pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1, - feed; + tagParam = req.params.slug; // No negative pages, or page 1 - if (isNaN(pageParam) || pageParam < 1 || (pageParam === 1 && req.route.path === '/rss/:page/')) { - return res.redirect(config().paths.subdir + '/rss/'); + if (isNaN(pageParam) || pageParam < 1 || + (pageParam === 1 && (req.route.path === '/rss/:page/' || req.route.path === '/tag/:slug/rss/:page/'))) { + if (tagParam !== undefined) { + return res.redirect(config().paths.subdir + '/tag/' + tagParam + '/rss/'); + } else { + return res.redirect(config().paths.subdir + '/rss/'); + } } // TODO: needs refactor for multi user to not use first user as default @@ -266,25 +271,37 @@ frontendControllers = { api.settings.read('description'), api.settings.read('permalinks') ]).then(function (result) { - var user = result[0].value, - title = result[1].value.value, - description = result[2].value.value, - permalinks = result[3].value, - siteUrl = config.urlFor('home', null, true), - feedUrl = config.urlFor('rss', null, true); - feed = new RSS({ - title: title, - description: description, - generator: 'Ghost v' + res.locals.version, - feed_url: feedUrl, - site_url: siteUrl, - ttl: '60' - }); + var options = {}; + if (pageParam) { options.page = pageParam; } + if (tagParam) { options.tag = tagParam; } + + return api.posts.browse(options).then(function (page) { + + var user = result[0].value, + title = result[1].value.value, + description = result[2].value.value, + permalinks = result[3].value, + siteUrl = config.urlFor('home', null, true), + feedUrl = config.urlFor('rss', null, true), + maxPage = page.pages, + feedItems = [], + feed; + + if (tagParam) { + title = page.aspect.tag.name + ' - ' + title; + feedUrl = feedUrl + 'tag/' + page.aspect.tag.slug + '/'; + } + + feed = new RSS({ + title: title, + description: description, + generator: 'Ghost v' + res.locals.version, + feed_url: feedUrl, + site_url: siteUrl, + ttl: '60' + }); - return api.posts.browse({page: pageParam}).then(function (page) { - var maxPage = page.pages, - feedItems = []; // A bit of a hack for situations with no content. if (maxPage === 0) { @@ -294,7 +311,11 @@ frontendControllers = { // If page is greater than number of pages we have, redirect to last page if (pageParam > maxPage) { - return res.redirect(config().paths.subdir + '/rss/' + maxPage + '/'); + if (tagParam) { + return res.redirect(config().paths.subdir + '/tag/' + tagParam + '/rss/' + maxPage + '/'); + } else { + return res.redirect(config().paths.subdir + '/rss/' + maxPage + '/'); + } } filters.doFilter('prePostsRender', page.posts).then(function (posts) { diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index f3149b195a..84aa5cf358 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -6,6 +6,8 @@ module.exports = function (server) { // ### Frontend routes server.get('/rss/', frontend.rss); server.get('/rss/:page/', frontend.rss); + server.get('/tag/:slug/rss/', frontend.rss); + server.get('/tag/:slug/rss/:page/', frontend.rss); server.get('/tag/:slug/page/:page/', frontend.tag); server.get('/tag/:slug/', frontend.tag); server.get('/page/:page/', frontend.homepage); diff --git a/core/test/functional/routes/frontend_test.js b/core/test/functional/routes/frontend_test.js index dddab36c58..01be21894a 100644 --- a/core/test/functional/routes/frontend_test.js +++ b/core/test/functional/routes/frontend_test.js @@ -171,7 +171,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to last page is page too high', function (done) { + it('should redirect to last page if page too high', function (done) { request.get('/page/4/') .expect('Location', '/page/3/') .expect('Cache-Control', cacheRules['public']) @@ -179,7 +179,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to first page is page too low', function (done) { + it('should redirect to first page if page too low', function (done) { request.get('/page/0/') .expect('Location', '/') .expect('Cache-Control', cacheRules['public']) @@ -214,7 +214,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to last page is page too high', function (done) { + it('should redirect to last page if page too high', function (done) { request.get('/rss/3/') .expect('Location', '/rss/2/') .expect('Cache-Control', cacheRules['public']) @@ -222,7 +222,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to first page is page too low', function (done) { + it('should redirect to first page if page too low', function (done) { request.get('/rss/0/') .expect('Location', '/rss/') .expect('Cache-Control', cacheRules['public']) @@ -231,6 +231,49 @@ describe('Frontend Routing', function () { }); }); + describe('Tag based RSS pages', function () { + it('should redirect without slash', function (done) { + request.get('/tag/getting-started/rss') + .expect('Location', '/tag/getting-started/rss/') + .expect('Cache-Control', cacheRules.year) + .expect(301) + .end(doEnd(done)); + }); + + it('should respond with xml', function (done) { + request.get('/tag/getting-started/rss/') + .expect('Content-Type', /xml/) + .expect('Cache-Control', cacheRules['public']) + .expect(200) + .end(doEnd(done)); + }); + + it('should redirect page 1', function (done) { + request.get('/tag/getting-started/rss/1/') + .expect('Location', '/tag/getting-started/rss/') + .expect('Cache-Control', cacheRules['public']) + // TODO: This should probably be a 301? + .expect(302) + .end(doEnd(done)); + }); + + it('should redirect to last page if page too high', function (done) { + request.get('/tag/getting-started/rss/2/') + .expect('Location', '/tag/getting-started/rss/1/') + .expect('Cache-Control', cacheRules['public']) + .expect(302) + .end(doEnd(done)); + }); + + it('should redirect to first page if page too low', function (done) { + request.get('/tag/getting-started/rss/0/') + .expect('Location', '/tag/getting-started/rss/') + .expect('Cache-Control', cacheRules['public']) + .expect(302) + .end(doEnd(done)); + }); + }); + describe('Static page', function () { it('should redirect without slash', function (done) { request.get('/static-page-test') @@ -335,7 +378,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to last page is page too high', function (done) { + it('should redirect to last page if page too high', function (done) { request.get('/tag/injection/page/4/') .expect('Location', '/tag/injection/page/2/') .expect('Cache-Control', cacheRules['public']) @@ -343,7 +386,7 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); - it('should redirect to first page is page too low', function (done) { + it('should redirect to first page if page too low', function (done) { request.get('/tag/injection/page/0/') .expect('Location', '/tag/injection/') .expect('Cache-Control', cacheRules['public'])