Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-01 02:41:39 -05:00
Code Issues Activity

🐛Fixed auto redirect for extra data queries on post and page resources (#9828)

Browse source 
closes #9791

- we only made use of the redirect middleware, who detects if a redirect should happen, for taxonomies (tags, authors)
- `data: page.team` will now redirect too
- `data: post.team` will now redirect too
- you can disable the redirect using the long form
This commit is contained in:
Katharina Irrgang 2018-12-03 20:31:48 +01:00 • committed by GitHub
parent 65a66ac007
commit fc21b25895
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 160 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
core/server/services/routing/CollectionRouter.js
View file

@ -73,6 +73,9 @@ class CollectionRouter extends ParentRouter {
// REGISTER: context middleware for entries
this.router().use(this._prepareEntryContext.bind(this));
// REGISTER: page/post resource redirects
this.router().param('slug', this._respectDominantRouter.bind(this));
// REGISTER: permalinks e.g. /:slug/, /podcast/:slug
this.mountRoute(this.permalinks.getValue({withUrlOptions: true}), controllers.entry);

5
core/server/services/routing/ParentRouter.js
View file

@ -78,7 +78,10 @@ class ParentRouter extends EventEmitter {
if (targetRoute) {
debug('_respectDominantRouter');
const matchPath = this.permalinks.getValue().replace(':slug', '[a-zA-Z0-9-_]+');
// CASE: transform /tag/:slug/ -> /tag/[a-zA-Z0-9-_]+/ to able to find url pieces to append
// e.g. /tag/bacon/page/2/ -> 'page/2' (to append)
// e.g. /bacon/welcome/ -> '' (nothing to append)
const matchPath = this.permalinks.getValue().replace(/:\w+/g, '[a-zA-Z0-9-_]+');
const toAppend = req.url.replace(new RegExp(matchPath), '');
return urlService.utils.redirect301(res, url.format({

11
core/server/services/settings/validate.js
View file

@ -75,9 +75,9 @@ _private.validateData = function validateData(object) {
const requiredQueryFields = ['type', 'resource'];
const allowedQueryValues = {
type: ['read', 'browse'],
resource: _.map(RESOURCE_CONFIG.QUERY, 'resource')
resource: _.union(_.map(RESOURCE_CONFIG.QUERY, 'resource'), _.map(RESOURCE_CONFIG.QUERY, 'alias'))
};
const allowedQueryOptions = ['limit', 'order', 'filter', 'include', 'slug', 'visibility', 'status'];
const allowedQueryOptions = ['limit', 'order', 'filter', 'include', 'slug', 'visibility', 'status', 'page'];
const allowedRouterOptions = ['redirect', 'slug'];
const defaultRouterOptions = {
redirect: true
@ -146,7 +146,12 @@ _private.validateData = function validateData(object) {
data.query[key][option] = object.data[key][option];
});
const DEFAULT_RESOURCE = _.find(RESOURCE_CONFIG.QUERY, {resource: data.query[key].resource});
const DEFAULT_RESOURCE = _.find(RESOURCE_CONFIG.QUERY, {alias: data.query[key].resource}) || _.find(RESOURCE_CONFIG.QUERY, {resource: data.query[key].resource});
// CASE: you define resource:pages and the alias is "pages". We need to load the internal alias/resource structure, otherwise we break api versions.
data.query[key].alias = DEFAULT_RESOURCE.alias;
data.query[key].resource = DEFAULT_RESOURCE.resource;
data.query[key] = _.defaults(data.query[key], _.omit(DEFAULT_RESOURCE, 'options'));
data.query[key].options = _.pick(object.data[key], allowedQueryOptions);

114
core/test/integration/web/site_spec.js
View file

@ -1306,10 +1306,52 @@ describe('Integration - Web - Site', function () {
users: [{redirect: false, slug: 'joe-bloggs'}]
}
}
},
'/channel6/': {
controller: 'channel',
data: {
query: {
post: {
resource: 'posts',
type: 'read',
options: {
slug: 'html-ipsum',
redirect: true
}
}
},
router: {
posts: [{redirect: true, slug: 'html-ipsum'}]
}
}
},
'/channel7/': {
controller: 'channel',
data: {
query: {
post: {
resource: 'posts',
type: 'read',
options: {
slug: 'static-page-test',
redirect: true
}
}
},
router: {
posts: [{redirect: true, slug: 'static-page-test'}]
}
}
}
},
collections: {},
collections: {
'/': {
permalink: '/:slug/'
}
},
taxonomies: {
tag: '/tag/:slug/',
@ -1447,7 +1489,45 @@ describe('Integration - Web - Site', function () {
});
});
it('serve kitching-sink', function () {
it('serve channel 6', function () {
const req = {
secure: true,
method: 'GET',
url: '/channel6/',
host: 'example.com'
};
return testUtils.mocks.express.invoke(app, req)
.then(function (response) {
const $ = cheerio.load(response.body);
response.statusCode.should.eql(200);
response.template.should.eql('index');
$('.post-card').length.should.equal(4);
});
});
it('serve channel 7', function () {
const req = {
secure: true,
method: 'GET',
url: '/channel7/',
host: 'example.com'
};
return testUtils.mocks.express.invoke(app, req)
.then(function (response) {
const $ = cheerio.load(response.body);
response.statusCode.should.eql(200);
response.template.should.eql('index');
$('.post-card').length.should.equal(4);
});
});
it('serve kitching-sink: redirect', function () {
const req = {
secure: true,
method: 'GET',
@ -1462,6 +1542,36 @@ describe('Integration - Web - Site', function () {
});
});
it('serve html-ipsum: redirect', function () {
const req = {
secure: true,
method: 'GET',
url: '/html-ipsum/',
host: 'example.com'
};
return testUtils.mocks.express.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(301);
response.headers.location.should.eql('/channel6/');
});
});
it('serve html-ipsum: redirect', function () {
const req = {
secure: true,
method: 'GET',
url: '/static-page-test/',
host: 'example.com'
};
return testUtils.mocks.express.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(301);
response.headers.location.should.eql('/channel7/');
});
});
it('serve chorizo: no redirect', function () {
const req = {
secure: true,

3
core/test/unit/services/routing/CollectionRouter_spec.js
View file

@ -1,5 +1,6 @@
const should = require('should'),
sinon = require('sinon'),
express = require('express'),
settingsCache = require('../../../../server/services/settings/cache'),
common = require('../../../../server/lib/common'),
controllers = require('../../../../server/services/routing/controllers'),
@ -16,6 +17,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
sandbox.spy(CollectionRouter.prototype, 'mountRoute');
sandbox.spy(CollectionRouter.prototype, 'mountRouter');
sandbox.spy(CollectionRouter.prototype, 'unmountRoute');
sandbox.spy(express.Router, 'param');
req = sandbox.stub();
res = sandbox.stub();
@ -45,6 +47,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
common.events.on.calledTwice.should.be.false();
collectionRouter.mountRoute.callCount.should.eql(3);
express.Router.param.callCount.should.eql(3);
// parent route
collectionRouter.mountRoute.args[0][0].should.eql('/');

30
core/test/unit/services/routing/ParentRouter_spec.js
View file

@ -250,6 +250,36 @@ describe('UNIT - services/routing/ParentRouter', function () {
next.called.should.eql(true);
urlService.utils.redirect301.called.should.be.false();
});
it('redirect primary tag permalink', function () {
const parentRouter = new ParentRouter('index');
parentRouter.getResourceType = sandbox.stub().returns('posts');
parentRouter.permalinks = {
getValue: sandbox.stub().returns('/:primary_tag/:slug/')
};
req.url = '/bacon/welcome/';
req.originalUrl = `${req.url}?x=y`;
req.app._router.stack = [{
name: 'SiteRouter',
handle: {
stack: [{
name: 'StaticRoutesRouter',
handle: {
parent: {
isRedirectEnabled: sandbox.stub().returns(true),
getRoute: sandbox.stub().returns('/route/')
}
}
}]
}
}];
parentRouter._respectDominantRouter(req, res, next, 'welcome');
next.called.should.eql(false);
urlService.utils.redirect301.withArgs(res, '/route/?x=y').calledOnce.should.be.true();
});
});
describe('fn: isRedirectEnabled', function () {