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

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Apr 29, 2020

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/dc-1/services/service-name and
/ui/~default/dc-1/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, and of course link to other namespaces without
changing our URL structure.

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 recalculation 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 slightly 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
and simplifying it slightly to make the code easier to follow.

The result is a simple function that can be passed easily to anything transition-like:

router.transitionTo(...transitionable(route, potentiallyReplacingParams, owner))
router.replaceWith(...transitionable(route, potentiallyReplacingParams, owner))
// etc

A transitionable being an array with the first element being the name of the route to transition to and all the other elements being the parameter values for the route:

// /:dc/services/:service
['dc.service.show', 'dc-1', 'service-0']

We had also been using this code in another upcoming branch in place of refresh so that we could 'refresh' error pages back to their original route.

route.refresh() // doesn't seem to work if there was an error
///
const route = getOwner(this).lookup('route:${this.router.currentRoute.name});
route.transitionTo(...transitionable(route, {}, getOwner(this))) // works on error pages

As well as obviously using this for our original href-mut helper (just change a single param somewhere in the URL)

Further work potentially on the end of this PR are further moving the nspace specific code out of the transitionable util so that it is no longer project specific, and deciding on whether to require passing the ember owner/container into the util.

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.
@johncowen johncowen added theme/ui Anything related to the UI backport/1.7 labels Apr 29, 2020
@johncowen johncowen added this to the 1.7.3 milestone Apr 29, 2020
@johncowen johncowen requested a review from a team April 29, 2020 18:52
Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johncowen johncowen merged commit 604de87 into master Apr 30, 2020
@johncowen johncowen deleted the ui/bugfix/kv-ui-like-keynames branch April 30, 2020 08:28
hashicorp-ci pushed a commit that referenced this pull request Apr 30, 2020
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.
@johncowen johncowen added the type/bug Feature does not function as expected label Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants