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

Fix currentNamespace #1192

Merged
merged 1 commit into from Sep 30, 2019
Merged

Conversation

andresmgot
Copy link
Contributor

Found a couple of issues while doing some QA:

  • After Use auth default namespace in namespace reducer #1190, the namespace in Routes.tsx is always defined (and is pointing to default since the token has not been read yet). So, regardless of the content of the token once logged in, it was redirecting me to the default namespace. To avoid that I have added the additional check to only redirect to /apps/ns/{ns} when accessing the root path (/) if the user is already authenticated.
  • After login out the current namespace was cleared to the default value (default). This caused that after login out and in again the namespace changed from whatever it was to default. To avoid this, we should not remove the current namespace from the state.

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

I miss CI :/ Thanks for doing the manual QA and fixing!

@@ -32,7 +32,8 @@ const namespaceReducer = (
case getType(actions.namespace.errorNamespaces):
return { ...state, errorMsg: action.payload.err.message };
case getType(actions.namespace.clearNamespaces):
return { ...initialState };
// Clear namespaces info but keep "current" to avoid unexpected redirections
return { ...initialState, current: state.current };
Copy link
Contributor

Choose a reason for hiding this comment

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

After login out the current namespace was cleared to the default value (default). This caused that after login out and in again the namespace changed from whatever it was to default.

Isn't that the expected behaviour - that if I log out there is no state, so that when I log back in, I'll be put on the default namespace? I'm not sure what I'm missing on this one, but +1'ing so we can start those release images :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really, the state is better cleaned up when reloading the page.

Let me put an example of the issue.

  1. I got my token for the namespace user-1. I login in Kubeapps with that token and go to to my namespace.
  2. I go to the /catalog page, which doesn't have the namespace in the route.
  3. I click on "logout" and the current namespace is set back to default.
  4. I login again, and since I haven't changed the namespace in the selector I got again in the /catalog page but this time, the selected namespace is default.

@andresmgot andresmgot merged commit 2142c83 into vmware-tanzu:master Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants