Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: Fix using 'ui-like' KVs when using an empty default nspace #7734

Merged
merged 1 commit into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions ui-v2/app/adapters/application.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import Adapter from './http';
import { inject as service } from '@ember/service';
// TODO: This should be changed to use env
import config from 'consul-ui/config/environment';
import { env } from 'consul-ui/env';

export const DATACENTER_QUERY_PARAM = 'dc';
export const NSPACE_QUERY_PARAM = 'ns';
export default Adapter.extend({
repo: service('settings'),
client: service('client/http'),
formatNspace: function(nspace) {
if (config.CONSUL_NSPACES_ENABLED) {
if (env('CONSUL_NSPACES_ENABLED')) {
return nspace !== '' ? { [NSPACE_QUERY_PARAM]: nspace } : undefined;
}
},
Expand Down
38 changes: 8 additions & 30 deletions ui-v2/app/helpers/href-mut.js
Original file line number Diff line number Diff line change
@@ -1,39 +1,17 @@
import Helper from '@ember/component/helper';
import { inject as service } from '@ember/service';
import { hrefTo } from 'consul-ui/helpers/href-to';
import { getOwner } from '@ember/application';
import transitionable from 'consul-ui/utils/routing/transitionable';

const getRouteParams = function(route, params = {}) {
return route.paramNames.map(function(item) {
if (typeof params[item] !== 'undefined') {
return params[item];
}
return route.params[item];
});
};
export default Helper.extend({
router: service('router'),
compute([params], hash) {
let current = this.router.currentRoute;
let parent;
let atts = getRouteParams(current, params);
// walk up the entire route/s replacing any instances
// of the specified params with the values specified
while ((parent = current.parent)) {
atts = atts.concat(getRouteParams(parent, params));
current = parent;
}
let route = this.router.currentRoute.name;
// TODO: this is specific to consul/nspaces
// 'ideally' we could try and do this elsewhere
// not super important though.
// This will turn an URL that has no nspace (/ui/dc-1/nodes) into one
// that does have a namespace (/ui/~nspace/dc-1/nodes) if you href-mut with
// a nspace parameter
if (typeof params.nspace !== 'undefined' && route.startsWith('dc.')) {
route = `nspace.${route}`;
atts.push(params.nspace);
}
//
return hrefTo(this, this.router, [route, ...atts.reverse()], hash);
return hrefTo(
this,
this.router,
transitionable(this.router.currentRoute, params, getOwner(this)),
hash
);
},
});
33 changes: 33 additions & 0 deletions ui-v2/app/routes/nspace.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,43 @@
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';
import { hash } from 'rsvp';
import { getOwner } from '@ember/application';
import { env } from 'consul-ui/env';
import transitionable from 'consul-ui/utils/routing/transitionable';

const DEFAULT_NSPACE_PARAM = '~default';
export default Route.extend({
repo: service('repository/dc'),
router: service('router'),
// The ember router seems to change the priority of individual routes
// depending on whether they are wildcard routes or not.
// This means that the namespace routes will be recognized before kv ones
// even though we define namespace routes after kv routes (kv routes are
// wildcard routes)
// Therefore here whenever we detect that ember has recognized a nspace route
// when it shouldn't (we know this as there is no ~ in the nspace param)
// we recalculate the route it should have caught by generating the nspace
// equivalent route for the url (/dc-1/kv/services > /~default/dc-1/kv/services)
// and getting the information for that route. We then remove the nspace specific
// information that we generated onto the route, which leaves us with the route
// we actually want. Using this final route information we redirect the user
// to where they wanted to go.
beforeModel: function(transition) {
if (!this.paramsFor('nspace').nspace.startsWith('~')) {
const url = `${env('rootURL')}${DEFAULT_NSPACE_PARAM}${transition.intent.url}`;
const route = this.router.recognize(url);
const [name, ...params] = transitionable(route, {}, getOwner(this));
this.replaceWith.apply(this, [
// remove the 'nspace.' from the routeName
name
.split('.')
.slice(1)
.join('.'),
// remove the nspace param from the params
...params.slice(1),
]);
}
},
model: function(params) {
return hash({
item: this.repo.getActive(),
Expand Down
35 changes: 35 additions & 0 deletions ui-v2/app/utils/routing/transitionable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const filter = function(routeName, atts, params) {
if (typeof params.nspace !== 'undefined' && routeName.startsWith('dc.')) {
routeName = `nspace.${routeName}`;
atts = [params.nspace].concat(atts);
}
return [routeName, ...atts];
};
const replaceRouteParams = function(route, params = {}) {
return (route.paramNames || [])
.map(function(item) {
if (typeof params[item] !== 'undefined') {
return params[item];
}
return route.params[item];
})
.reverse();
};
export default function(route, params = {}, container) {
if (route === null) {
route = container.lookup('route:application');
}
let atts = replaceRouteParams(route, params);
// walk up the entire route/s replacing any instances
// of the specified params with the values specified
let current = route;
let parent;
while ((parent = current.parent)) {
atts = atts.concat(replaceRouteParams(parent, params));
current = parent;
}
// Reverse atts here so it doen't get confusing whilst debugging
// (.reverse is destructive)
atts.reverse();
return filter(route.name || 'application', atts, params);
}
21 changes: 21 additions & 0 deletions ui-v2/tests/acceptance/dc/kvs/edit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,24 @@ Feature: dc / kvs / edit: KV Viewing
kv: another-key
---
Then I don't see ID on the session
# Make sure we can view KVs that have similar names to sections in the UI
Scenario: I have KV called [Page]
Given 1 datacenter model with the value "datacenter"
And 1 kv model from yaml
---
Key: [Page]
---
When I visit the kv page for yaml
---
dc: datacenter
kv: [Page]
---
Then the url should be /datacenter/kv/[Page]/edit
Where:
--------------
| Page |
| services |
| nodes |
| intentions |
| kvs |
--------------
62 changes: 62 additions & 0 deletions ui-v2/tests/unit/utils/routing/transitionable-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import transitionable from 'consul-ui/utils/routing/transitionable';
import { module, test } from 'qunit';

const makeRoute = function(name, params = {}, parent) {
return {
name: name,
paramNames: Object.keys(params),
params: params,
parent: parent,
};
};
module('Unit | Utility | routing/transitionable', function() {
test('it walks up the route tree to resolve all the required parameters', function(assert) {
const expected = ['dc.service.instance', 'dc-1', 'service-0', 'node-0', 'service-instance-0'];
const dc = makeRoute('dc', { dc: 'dc-1' });
const service = makeRoute('dc.service', { service: 'service-0' }, dc);
const instance = makeRoute(
'dc.service.instance',
{ node: 'node-0', id: 'service-instance-0' },
service
);
const actual = transitionable(instance, {});
assert.deepEqual(actual, expected);
});
test('it walks up the route tree to resolve all the required parameters whilst nspaced', function(assert) {
const expected = [
'nspace.dc.service.instance',
'team-1',
'dc-1',
'service-0',
'node-0',
'service-instance-0',
];
const dc = makeRoute('dc', { dc: 'dc-1' });
const service = makeRoute('dc.service', { service: 'service-0' }, dc);
const instance = makeRoute(
'dc.service.instance',
{ node: 'node-0', id: 'service-instance-0' },
service
);
const actual = transitionable(instance, { nspace: 'team-1' });
assert.deepEqual(actual, expected);
});
test('it walks up the route tree to resolve all the required parameters whilst replacing specified params', function(assert) {
const expected = [
'dc.service.instance',
'dc-1',
'service-0',
'different-node',
'service-instance-0',
];
const dc = makeRoute('dc', { dc: 'dc-1' });
const service = makeRoute('dc.service', { service: 'service-0' }, dc);
const instance = makeRoute(
'dc.service.instance',
{ node: 'node-0', id: 'service-instance-0' },
service
);
const actual = transitionable(instance, { node: 'different-node' });
assert.deepEqual(actual, expected);
});
});