From 9a84878117c815c04513ea89e4e9a77a1b44715e Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 2 Aug 2017 10:32:50 -0700 Subject: [PATCH 1/3] [ui/routes] add failing tests --- .../public/routes/__tests__/_route_manager.js | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/ui/public/routes/__tests__/_route_manager.js b/src/ui/public/routes/__tests__/_route_manager.js index ef53b44dc442d..90e8f3ad5f507 100644 --- a/src/ui/public/routes/__tests__/_route_manager.js +++ b/src/ui/public/routes/__tests__/_route_manager.js @@ -113,4 +113,70 @@ describe('routes/route_manager', function () { expect($rp.when.lastCall.args[1]).to.have.property('requireDefaultIndex', true); }); }); + + describe('#defaults()', () => { + it('adds defaults to routes with matching paths', () => { + routes.when('/foo', { name: 'foo' }); + routes.when('/bar', { name: 'bar' }); + routes.when('/baz', { name: 'baz' }); + routes.defaults(/^\/ba/, { + withDefaults: true + }); + routes.config($rp); + + sinon.assert.calledWithExactly($rp.when, '/foo', sinon.match({ name: 'foo', withDefaults: undefined })); + sinon.assert.calledWithExactly($rp.when, '/bar', sinon.match({ name: 'bar', withDefaults: true })); + sinon.assert.calledWithExactly($rp.when, '/baz', sinon.match({ name: 'baz', withDefaults: true })); + }); + + it('does not override values specified in the route', () => { + routes.when('/foo', { name: 'foo' }); + routes.defaults(/./, { name: 'bar' }); + routes.config($rp); + + sinon.assert.calledWithExactly($rp.when, '/foo', sinon.match({ name: 'foo' })); + }); + + it('does not assign defaults by reference, to prevent accidentally merging unrelated defaults together', () => { + routes.when('/foo', { name: 'foo' }); + routes.when('/bar', { name: 'bar' }); + routes.when('/baz', { name: 'baz', funcs: { bazFunc() {} } }); + + // multiple defaults must be defined that, when applied correctly, will + // create a new object property on all routes that is unique to all of them + routes.defaults(/./, { funcs: { all() {} } }); + routes.defaults(/^\/foo/, { funcs: { fooFunc() {} } }); + routes.defaults(/^\/bar/, { funcs: { barFunc() {} } }); + routes.config($rp); + + sinon.assert.calledThrice($rp.when); + sinon.assert.calledWithExactly($rp.when, '/foo', sinon.match({ + name: 'foo', + funcs: sinon.match({ + all: sinon.match.func, + fooFunc: sinon.match.func, + barFunc: undefined, + bazFunc: undefined, + }) + })); + sinon.assert.calledWithExactly($rp.when, '/bar', sinon.match({ + name: 'bar', + funcs: sinon.match({ + all: sinon.match.func, + fooFunc: undefined, + barFunc: sinon.match.func, + bazFunc: undefined, + }) + })); + sinon.assert.calledWithExactly($rp.when, '/baz', sinon.match({ + name: 'baz', + funcs: sinon.match({ + all: sinon.match.func, + fooFunc: undefined, + barFunc: undefined, + bazFunc: sinon.match.func, + }) + })); + }); + }); }); From e3805966195aae620d694980c544e6f66a5a46bd Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 2 Aug 2017 10:50:34 -0700 Subject: [PATCH 2/3] [ui/routes] clone defaults before applying them --- src/ui/public/routes/route_manager.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/routes/route_manager.js b/src/ui/public/routes/route_manager.js index 356525aabb4fb..4b17fbfdbdffb 100644 --- a/src/ui/public/routes/route_manager.js +++ b/src/ui/public/routes/route_manager.js @@ -1,4 +1,4 @@ -import { defaultsDeep, wrap } from 'lodash'; +import { cloneDeep, defaultsDeep, wrap } from 'lodash'; import { wrapRouteWithPrep } from './wrap_route_with_prep'; import { RouteSetupManager } from './route_setup_manager'; @@ -19,7 +19,7 @@ export default function RouteManager() { defaults.forEach(def => { if (def.regex.test(path)) { - defaultsDeep(route, def.value); + defaultsDeep(route, cloneDeep(def.value)); } }); From e1df81633c959dbde43434f616e9e436ce98b885 Mon Sep 17 00:00:00 2001 From: Spencer Date: Wed, 2 Aug 2017 11:40:53 -0700 Subject: [PATCH 3/3] Mention issue in related tests --- src/ui/public/routes/__tests__/_route_manager.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ui/public/routes/__tests__/_route_manager.js b/src/ui/public/routes/__tests__/_route_manager.js index 90e8f3ad5f507..1291aacb815e0 100644 --- a/src/ui/public/routes/__tests__/_route_manager.js +++ b/src/ui/public/routes/__tests__/_route_manager.js @@ -137,6 +137,7 @@ describe('routes/route_manager', function () { sinon.assert.calledWithExactly($rp.when, '/foo', sinon.match({ name: 'foo' })); }); + // See https://github.com/elastic/kibana/issues/13294 it('does not assign defaults by reference, to prevent accidentally merging unrelated defaults together', () => { routes.when('/foo', { name: 'foo' }); routes.when('/bar', { name: 'bar' });