From 254b598e6f826cfccc283c3b32d18babcfe77815 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Mon, 27 Aug 2018 15:08:01 +0100 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20auto=20redirect=20ex?= =?UTF-8?q?tra=20data=20queries=20on=20post=20and=20page=20resources?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 longform --- .../services/routing/CollectionRouter.js | 3 + core/test/integration/web/site_spec.js | 114 +++++++++++++++++- .../services/routing/CollectionRouter_spec.js | 3 + 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/core/server/services/routing/CollectionRouter.js b/core/server/services/routing/CollectionRouter.js index 93eb153f3746..3daba46daef4 100644 --- a/core/server/services/routing/CollectionRouter.js +++ b/core/server/services/routing/CollectionRouter.js @@ -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); diff --git a/core/test/integration/web/site_spec.js b/core/test/integration/web/site_spec.js index 6b716684f9ea..1e590ed67366 100644 --- a/core/test/integration/web/site_spec.js +++ b/core/test/integration/web/site_spec.js @@ -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, diff --git a/core/test/unit/services/routing/CollectionRouter_spec.js b/core/test/unit/services/routing/CollectionRouter_spec.js index f169f6e4f6b4..1f03aae2de73 100644 --- a/core/test/unit/services/routing/CollectionRouter_spec.js +++ b/core/test/unit/services/routing/CollectionRouter_spec.js @@ -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('/'); From 1bab0192e88f32ae8d1ea28be74762c92d33cbf9 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Fri, 31 Aug 2018 12:10:12 +0100 Subject: [PATCH 2/4] TODO: - /{primary_tag}/{slug}/ as permalink - /route/: data: post.welcome - does not work --- core/server/services/routing/ParentRouter.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/server/services/routing/ParentRouter.js b/core/server/services/routing/ParentRouter.js index 1896bad09d2f..e211aef94dd3 100644 --- a/core/server/services/routing/ParentRouter.js +++ b/core/server/services/routing/ParentRouter.js @@ -75,12 +75,15 @@ class ParentRouter extends EventEmitter { return true; }); + console.log(this.permalinks, targetRoute); if (targetRoute) { debug('_respectDominantRouter'); const matchPath = this.permalinks.getValue().replace(':slug', '[a-zA-Z0-9-_]+'); const toAppend = req.url.replace(new RegExp(matchPath), ''); + console.log(toAppend); + return urlService.utils.redirect301(res, url.format({ pathname: urlService.utils.createUrl(urlService.utils.urlJoin(targetRoute, toAppend), false, false, true), search: url.parse(req.originalUrl).search From 69887d77ab73267fc5df2cae321622ec2746b210 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Mon, 17 Sep 2018 16:51:19 +0200 Subject: [PATCH 3/4] Fixed todo --- core/server/services/routing/ParentRouter.js | 8 ++--- .../services/routing/ParentRouter_spec.js | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/core/server/services/routing/ParentRouter.js b/core/server/services/routing/ParentRouter.js index e211aef94dd3..19d730575a11 100644 --- a/core/server/services/routing/ParentRouter.js +++ b/core/server/services/routing/ParentRouter.js @@ -75,15 +75,15 @@ class ParentRouter extends EventEmitter { return true; }); - console.log(this.permalinks, targetRoute); 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), ''); - console.log(toAppend); - return urlService.utils.redirect301(res, url.format({ pathname: urlService.utils.createUrl(urlService.utils.urlJoin(targetRoute, toAppend), false, false, true), search: url.parse(req.originalUrl).search diff --git a/core/test/unit/services/routing/ParentRouter_spec.js b/core/test/unit/services/routing/ParentRouter_spec.js index 8deb4f2fd3c8..59ab76d1b5d0 100644 --- a/core/test/unit/services/routing/ParentRouter_spec.js +++ b/core/test/unit/services/routing/ParentRouter_spec.js @@ -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 () { From f6700f7916108335cd3f0b2362ecfb5afac46c2d Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Mon, 12 Nov 2018 23:02:34 +0100 Subject: [PATCH 4/4] Fixes for long form --- core/server/services/settings/validate.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/server/services/settings/validate.js b/core/server/services/settings/validate.js index 7c89ca5554cf..0eb8f9825683 100644 --- a/core/server/services/settings/validate.js +++ b/core/server/services/settings/validate.js @@ -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);