Skip to content

Commit

Permalink
Fix routing of posts and static pages
Browse files Browse the repository at this point in the history
closes #1757 and #1773

- switches routes.frontend for posts and pages
to use a regex with two capturing groups.  This removes
the need to dynamically remove an express route at a
later point, leaving the decision making to frontend
controller.

- added unit tests for all routing conditions that 
can arise for posts and pages.

- updated functional tests to also test for same thing
in unit tests

- removes old code from server/api/index that used
to fix this issue, but is no longer needed

- removed some un-needed require statements in routes/admin
  • Loading branch information
hswolff committed Dec 30, 2013
1 parent f3a8073 commit 91ca4a4
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 117 deletions.
37 changes: 9 additions & 28 deletions core/server/api/index.js
Expand Up @@ -3,11 +3,11 @@

var _ = require('underscore'),
when = require('when'),
config = require('../config'),
errors = require('../errorHandling'),
db = require('./db'),
settings = require('./settings'),
notifications = require('./notifications'),
config = require('../config'),
posts = require('./posts'),
users = require('./users'),
tags = require('./tags'),
Expand Down Expand Up @@ -49,34 +49,15 @@ requestHandler = function (apiMethod) {
var options = _.extend(req.body, req.query, req.params),
apiContext = {
user: req.session && req.session.user
},
postRouteIndex,
i;

settings.read('permalinks').then(function (permalinks) {
// If permalinks have changed, find old post route
if (req.body.permalinks && req.body.permalinks !== permalinks) {
for (i = 0; i < req.app.routes.get.length; i += 1) {
if (req.app.routes.get[i].path === config.paths().subdir + permalinks) {
postRouteIndex = i;
break;
}
}
}

return apiMethod.call(apiContext, options).then(function (result) {
// Reload post route
if (postRouteIndex) {
req.app.get(permalinks, req.app.routes.get.splice(postRouteIndex, 1)[0].callbacks);
}
};

invalidateCache(req, res, result);
res.json(result || {});
}, function (error) {
var errorCode = error.errorCode || 500,
errorMsg = {error: _.isString(error) ? error : (_.isObject(error) ? error.message : 'Unknown API Error')};
res.json(errorCode, errorMsg);
});
return apiMethod.call(apiContext, options).then(function (result) {
invalidateCache(req, res, result);
res.json(result || {});
}, function (error) {
var errorCode = error.errorCode || 500,
errorMsg = {error: _.isString(error) ? error : (_.isObject(error) ? error.message : 'Unknown API Error')};
res.json(errorCode, errorMsg);
});
};
};
Expand Down
55 changes: 37 additions & 18 deletions core/server/controllers/frontend.js
Expand Up @@ -69,36 +69,55 @@ frontendControllers = {
});
},
'single': function (req, res, next) {
api.posts.read(_.pick(req.params, ['id', 'slug', 'page'])).then(function (post) {
if (post) {
// From route check if a date was parsed
// from the regex
var dateInSlug = req.params[0] !== '';
when.join(
api.settings.read('permalinks'),
api.posts.read({slug: req.params[1]})
).then(function (promises) {
var permalink = promises[0].value,
post = promises[1];

function render() {
filters.doFilter('prePostsRender', post).then(function (post) {
api.settings.read('activeTheme').then(function (activeTheme) {
var paths = config.paths().availableThemes[activeTheme.value];
if (post.page && paths.hasOwnProperty('page')) {
res.render('page', {post: post});
} else {
res.render('post', {post: post});
}
var paths = config.paths().availableThemes[activeTheme.value],
view = post.page && paths.hasOwnProperty('page') ? 'page' : 'post';
res.render(view, {post: post});
});
});
} else {
next();
}

if (!post) {
return next();
}

// A page can only be rendered when there is no date in the url.
// A post can either be rendered with a date in the url
// depending on the permalink setting.
// For all other conditions return 404.
if (post.page === 1 && dateInSlug === false) {
return render();
}

if (post.page === 0) {
// Handle post rendering
if ((permalink === '/:slug/' && dateInSlug === false) ||
(permalink !== '/:slug/' && dateInSlug === true)) {
return render();
}
}

next();


}).otherwise(function (err) {
var e = new Error(err.message);
e.status = err.errorCode;
return next(e);
});
},
'post': function (req, res, next) {
req.params.page = 0;
return frontendControllers.single(req, res, next);
},
'page': function (req, res, next) {
req.params.page = 1;
return frontendControllers.single(req, res, next);
},
'rss': function (req, res, next) {
// Initialize RSS
var siteUrl = config().url,
Expand Down
4 changes: 1 addition & 3 deletions core/server/routes/admin.js
@@ -1,8 +1,6 @@
var admin = require('../controllers/admin'),
api = require('../api'),
config = require('../config'),
middleware = require('../middleware').middleware,
url = require('url');
middleware = require('../middleware').middleware;

module.exports = function (server) {
var subdir = config.paths().subdir;
Expand Down
24 changes: 12 additions & 12 deletions core/server/routes/frontend.js
@@ -1,19 +1,19 @@
var frontend = require('../controllers/frontend'),
api = require('../api');
var frontend = require('../controllers/frontend');

module.exports = function (server) {
/*jslint regexp: true */

// ### Frontend routes
/* TODO: dynamic routing, homepage generator, filters ETC ETC */
server.get('/rss/', frontend.rss);
server.get('/rss/:page/', frontend.rss);
server.get('/page/:page/', frontend.homepage);
// Only capture the :slug part of the URL
// This regex will always have two capturing groups,
// one for date, and one for the slug.
// Examples:
// Given `/plain-slug/` the req.params would be ['', 'plain-slug']
// Given `/2012/12/24/plain-slug/` the req.params would be ['2012/12/24', 'plain-slug']
server.get(/^\/([0-9\/]*)([^\/.]*)\/$/, frontend.single);
server.get('/', frontend.homepage);

api.settings.read('permalinks').then(function (permalinks) {
if (permalinks.value !== '/:slug/') {
server.get('/:slug/', frontend.page);
server.get(permalinks.value, frontend.post);
} else {
server.get(permalinks.value, frontend.single);
}
});
};
};
14 changes: 13 additions & 1 deletion core/test/functional/base.js
Expand Up @@ -209,6 +209,17 @@ CasperTest.Routines = (function () {
}, id);
}

function togglePermalinks(test) {
casper.thenOpen(url + "ghost/settings/");
casper.thenClick('#permalinks');
casper.thenClick('.button-save');
casper.waitFor(function successNotification() {
return this.evaluate(function () {
return document.querySelectorAll('.js-bb-notification section').length > 0;
});
});
}

function _createRunner(fn) {
fn.run = function run(test) {
var routine = this;
Expand All @@ -225,7 +236,8 @@ CasperTest.Routines = (function () {
register: _createRunner(register),
login: _createRunner(login),
logout: _createRunner(logout),
deletePost: _createRunner(deletePost)
deletePost: _createRunner(deletePost),
togglePermalinks: _createRunner(togglePermalinks)
};

}());
14 changes: 2 additions & 12 deletions core/test/functional/frontend/feed_test.js
Expand Up @@ -28,18 +28,8 @@ CasperTest.begin('Ensure that RSS is available', 11, function suite(test) {
});
});

CasperTest.begin('Ensures dated permalinks works with RSS', 4, function suite(test) {
casper.thenOpen(url + "ghost/settings/", function testTitleAndUrl() {
test.assertTitle("Ghost Admin", "Ghost admin has no title");
test.assertUrlMatch(/ghost\/settings\/general\/$/, "Ghost doesn't require login this time");
});
casper.thenClick('#permalinks');
casper.thenClick('.button-save');
casper.waitFor(function successNotification() {
return this.evaluate(function () {
return document.querySelectorAll('.js-bb-notification section').length > 0;
});
});
CasperTest.begin('Ensures dated permalinks works with RSS', 2, function suite(test) {
CasperTest.Routines.togglePermalinks.run(test);
casper.thenOpen(url + 'rss/', function (response) {
var content = this.getPageContent(),
today = new Date(),
Expand Down
14 changes: 14 additions & 0 deletions core/test/functional/frontend/post_test.js
Expand Up @@ -3,7 +3,21 @@
*/

/*globals CasperTest, casper, __utils__, url, testPost, falseUser, email */

// Tests when permalinks is set to date (changed in feed_test)
CasperTest.begin('Post page does not load as slug', 2, function suite(test) {
casper.start(url + 'welcome-to-ghost', function then(response) {
test.assertTitle('404 — Page Not Found', 'The post should return 404 page');
test.assertElementCount('.content .post', 0, 'There is no post on this page');
});
}, true);

CasperTest.begin('Toggle permalinks', 0, function suite(test) {
CasperTest.Routines.togglePermalinks.run(test);
});

CasperTest.begin('Post page loads', 3, function suite(test) {
CasperTest.Routines.togglePermalinks.run(test);
casper.start(url + 'welcome-to-ghost', function then(response) {
test.assertTitle('Welcome to Ghost', 'The post should have a title and it should be "Welcome to Ghost"');
test.assertElementCount('.content .post', 1, 'There is exactly one post on this page');
Expand Down

0 comments on commit 91ca4a4

Please sign in to comment.