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

When using namespaces, the 'default' namespace is a little special in
that we wanted the option for all our URLs to stay the same when using
namespaces if you are using the default namespace, with the option of
also being able to explicitly specify `~default` as a namespace.

In other words both `ui/services/service-name` and
`ui/~default/services/service-name` show the same thing.

This means that if you switch between OSS and Enterprise, all of your
URLs stay the same, but you can still specifically link to the default
namespace itself.

Our routing configuration is duplicated in order to achieve this:

```
- :dc
  - :service
  - :kv
    - :edit
- :nspace
  - :dc
    - :service
    - :kv
      - :edit
```

Secondly, ember routing resolves/matches routes in the order that you specify
them, unless, its seems, when using wildcard routes, like we do in the
KV area.

When not using the wildcard routes the above routing configuration
resolves/matches a `/dc-1/kv/service` to the `dc.kv.edit` route correctly
(dc:dc-1, kv:services), that route having been configured in a higher
priority than the nspace routes.

However when configured with wildcards (required in the KV area), note
the asterisk below:

```
- :dc
    :service
  - :kv
    - *edit
- :nspace
  - :dc
    - :service
    - :kv
      - *edit
```

Given something like `/dc-1/kv/services` the router instead matches the
`nspace.dc.service` (nspace:dc-1, dc:kv, service:services) route first even
though the `dc.kv.edit` route should still match first.
Changing the `dc.kv.edit` route back to use a non-wildcard route
(:edit instead of *edit), returns the router to match the routes in the
correct order.

In order to work around this, we catch any incorrectly matched routes
(those being directed to the nspace Route but not having a `~`
character in the nspace parameter), and then recalculate the correct
route name and parameters. Lastly we use this recalculated route to
direct the user/app to the correct route.

This route recalcation requires walking up the route to gather up all of
the required route parameters, and although this feels like something
that could already exist in ember, it doesn't seem to. We had already
done a lot of this work a while ago when implementing our `href-mut`
helper. This commit therefore repurposes that work slighlty and externalizes
it outside of the helper itself into a more usable util so we can import
it where we need it. Tests have been added before refactoring it down
to make the code easier to follow.

ui-v2/app/adapters/application.js

@@ -1,7 +1,6 @@
 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';
@@ -9,7 +8,7 @@ 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;
     }
   },

ui-v2/app/helpers/href-mut.js

@@ -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
+    );
   },
 });

ui-v2/app/routes/nspace.js

@@ -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(),

ui-v2/app/utils/routing/transitionable.js

@@ -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);
+}

ui-v2/tests/acceptance/dc/kvs/edit.feature

@@ -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        |
+    --------------

ui-v2/tests/unit/utils/routing/transitionable-test.js

@@ -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);
+  });
+});