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

clear namespaces on logout and properly render nested namespaces in the namepace picker #7186

Merged
merged 5 commits into from
Aug 1, 2019

Conversation

meirish
Copy link
Contributor

@meirish meirish commented Jul 24, 2019

This fixes two small namespace bugs:

  • We weren't clearing the cache when you logged out, and an empty response resulted in an ignored error, so a list of namespaces would be present if one user logged out and another user logged in.
  • If you logged into a namespace by typing its name with and initial /, the namespace-picker would not show any namespaces nested further down because the tree construction would try to match /foo with foo.

This PR fixes both of these issues.

@meirish meirish added this to the 1.2.1 milestone Jul 24, 2019
@meirish meirish force-pushed the b-clear-namespaces branch from bab5f35 to 37231db Compare July 26, 2019 03:18
@meirish meirish marked this pull request as ready for review July 26, 2019 14:52
@meirish meirish requested a review from a team July 26, 2019 14:54
this.get('controlGroup').deleteTokens();
this.get('console').set('isOpen', false);
this.get('console').clearLog(true);
this.auth.deleteCurrentToken();
Copy link
Contributor

Choose a reason for hiding this comment

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

💅 💅 💅

<LinkTo
@params={{array "vault.cluster.secrets" (query-params namespace=normalizedNamespace)}}
class={{concat "is-block " class}}
data-test-namespace-link={{this.normalizedNamespace}}
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need the this. now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not required, but it's helpful to clarify what's state on the component, what's an argument to a component (@foo), and what's a helper. https://github.com/emberjs/rfcs/blob/master/text/0276-named-args.md has more about that. I should probably update the other use of normalizedNamespace too haha.

madalynrose
madalynrose previously approved these changes Jul 26, 2019
Copy link
Contributor

@madalynrose madalynrose left a comment

Choose a reason for hiding this comment

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

small question but this looks great!

@meirish meirish force-pushed the b-clear-namespaces branch from defc5c5 to 86e0991 Compare August 1, 2019 21:23
@meirish meirish merged commit aaefdc1 into master Aug 1, 2019
@meirish meirish deleted the b-clear-namespaces branch August 1, 2019 21:54
@jefferai jefferai removed this from the 1.2.1 milestone Aug 5, 2019
meirish added a commit that referenced this pull request Aug 5, 2019
…he namepace picker (#7186)

* reset namespace cache when a user logs out

* fix issue where if you log in to a namespace with an initial /, nested namespaces would not show up in the navigation

* set an empty list of namespaces instead of ignoring error

* add tests for namespace bugs

* use this consistently in template
meirish added a commit that referenced this pull request Aug 5, 2019
* ancestorKeysForKey always returns an array (#6205)

* clear namespaces on logout and properly render nested namespaces in the namepace picker (#7186)

* reset namespace cache when a user logs out

* fix issue where if you log in to a namespace with an initial /, nested namespaces would not show up in the navigation

* set an empty list of namespaces instead of ignoring error

* add tests for namespace bugs

* use this consistently in template
@chrishoffman chrishoffman added this to the 1.2.1 milestone Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants