Update urls.app.apps.list to require cluster. (#1830)

* Update urls.app.apps.list to require cluster. * Fix header test. * Fix arg order in list url helper used in shared/urls itself.
vmware-tanzu · Jun 30, 2020 · 9c14870 · 9c14870
1 parent 3ebb2e5
commit 9c14870
Show file tree

Hide file tree

Showing 12 changed files with 89 additions and 55 deletions.
diff --git a/dashboard/src/components/AppView/AppControls/AppControls.test.tsx b/dashboard/src/components/AppView/AppControls/AppControls.test.tsx
@@ -10,16 +10,20 @@ import * as url from "../../../shared/url";
 import ConfirmDialog from "../../ConfirmDialog";
 
 import { IRelease } from "shared/types";
-import AppControls from "./AppControls";
+import AppControls, { IAppControlsProps } from "./AppControls";
 import UpgradeButton from "./UpgradeButton";
 
+const namespace = "bar";
+const defaultProps = {
+  cluster: "default",
+  app: new hapi.release.Release({ name: "foo", namespace }),
+  deleteApp: jest.fn(),
+  push: jest.fn(),
+} as IAppControlsProps;
+
 it("calls delete function when clicking the button", done => {
-  const name = "foo";
-  const namespace = "bar";
-  const app = new hapi.release.Release({ name, namespace });
-  const wrapper = shallow(
-    <AppControls app={app} deleteApp={jest.fn().mockReturnValue(true)} push={jest.fn()} />,
-  );
+  const props = { ...defaultProps, deleteApp: jest.fn().mockReturnValue(true) };
+  const wrapper = shallow(<AppControls {...props} />);
   const button = wrapper
     .find(".AppControls")
     .children()
@@ -41,19 +45,18 @@ it("calls delete function when clicking the button", done => {
     wrapper.update();
     const redirect = wrapper.find(Redirect);
     expect(redirect.props()).toMatchObject({
-      to: url.app.apps.list(namespace),
+      to: url.app.apps.list(defaultProps.cluster, namespace),
     } as any);
     done();
   }, 1);
 });
 
 it("calls delete function with additional purge", () => {
-  const name = "foo";
-  const namespace = "bar";
-  const app = new hapi.release.Release({ name, namespace });
-  const deleteApp = jest.fn().mockReturnValue(false); // Return "false" to avoid redirect when mounting
+  // Return "false" to avoid redirect when mounting
+  const deleteApp = jest.fn().mockReturnValue(false);
+  const props = { ...defaultProps, deleteApp };
   // mount() is necessary to render the Modal
-  const wrapper = mount(<AppControls app={app} deleteApp={deleteApp} push={jest.fn()} />);
+  const wrapper = mount(<AppControls {...props} />);
   Modal.setAppElement(document.createElement("div"));
   wrapper.setState({ modalIsOpen: true });
   wrapper.update();
@@ -74,6 +77,7 @@ it("calls delete function with additional purge", () => {
 
 context("when name or namespace do not exist", () => {
   const props = {
+    ...defaultProps,
     app: new hapi.release.Release({ name: "name", namespace: "my-ns" }),
   };
 
@@ -89,12 +93,13 @@ context("when name or namespace do not exist", () => {
 
 context("when the application has been already deleted", () => {
   const props = {
+    ...defaultProps,
     app: new hapi.release.Release({ name: "name", namespace: "my-ns", info: { deleted: {} } }),
     deleteApp: jest.fn().mockReturnValue(false), // Return "false" to avoid redirect when mounting
   };
 
   it("should show Purge instead of Delete in the button title", () => {
-    const wrapper = shallow(<AppControls {...props} push={jest.fn()} />);
+    const wrapper = shallow(<AppControls {...props} />);
     const button = wrapper.find(".button-danger");
     expect(button.text()).toBe("Purge");
   });
@@ -115,7 +120,7 @@ context("when the application has been already deleted", () => {
   it("should purge when clicking on delete", () => {
     // mount() is necessary to render the Modal
     const deleteApp = jest.fn().mockReturnValue(false);
-    const wrapper = mount(<AppControls {...props} deleteApp={deleteApp} push={jest.fn()} />);
+    const wrapper = mount(<AppControls {...props} deleteApp={deleteApp} />);
     Modal.setAppElement(document.createElement("div"));
     wrapper.setState({ modalIsOpen: true, purge: false });
     wrapper.update();
@@ -140,7 +145,6 @@ context("when the application has been already deleted", () => {
 context("when there is a new version available", () => {
   it("should forward the latest version", () => {
     const name = "foo";
-    const namespace = "bar";
     const app = {
       name,
       namespace,
@@ -150,7 +154,8 @@ context("when there is a new version available", () => {
         appLatestVersion: "1.0.0",
       },
     } as IRelease;
-    const wrapper = shallow(<AppControls app={app} deleteApp={jest.fn()} push={jest.fn()} />);
+    const props = { ...defaultProps, app };
+    const wrapper = shallow(<AppControls {...props} />);
 
     expect(wrapper.find(UpgradeButton).prop("newVersion")).toBe(true);
   });
@@ -159,7 +164,6 @@ context("when there is a new version available", () => {
 context("when the application is up to date", () => {
   it("should not forward the latest version", () => {
     const name = "foo";
-    const namespace = "bar";
     const app = {
       name,
       namespace,
@@ -169,7 +173,8 @@ context("when the application is up to date", () => {
         appLatestVersion: "1.1.0",
       },
     } as IRelease;
-    const wrapper = shallow(<AppControls app={app} deleteApp={jest.fn()} push={jest.fn()} />);
+    const props = { ...defaultProps, app };
+    const wrapper = shallow(<AppControls {...props} />);
 
     expect(wrapper.find(UpgradeButton).prop("updateVersion")).toBe(undefined);
   });
@@ -178,6 +183,7 @@ context("when the application is up to date", () => {
 context("Rollback button", () => {
   it("should show the RollbackButton when there is more than one revision", () => {
     const props = {
+      ...defaultProps,
       app: new hapi.release.Release({
         name: "name",
         namespace: "my-ns",
@@ -186,12 +192,13 @@ context("Rollback button", () => {
       }),
       deleteApp: jest.fn().mockReturnValue(false), // Return "false" to avoid redirect when mounting
     };
-    const wrapper = shallow(<AppControls {...props} push={jest.fn()} />);
+    const wrapper = shallow(<AppControls {...props} />);
     const button = wrapper.find(RollbackButtonContainer);
     expect(button).toExist();
   });
   it("should not show the RollbackButton when there is only one revision", () => {
     const props = {
+      ...defaultProps,
       app: new hapi.release.Release({
         name: "name",
         namespace: "my-ns",

diff --git a/dashboard/src/components/AppView/AppControls/AppControls.tsx b/dashboard/src/components/AppView/AppControls/AppControls.tsx
@@ -10,7 +10,8 @@ import LoadingWrapper from "../../LoadingWrapper";
 import "./AppControls.css";
 import UpgradeButton from "./UpgradeButton";
 
-interface IAppControlsProps {
+export interface IAppControlsProps {
+  cluster: string;
   app: IRelease;
   deleteApp: (purge: boolean) => Promise<boolean>;
   push: (location: string) => RouterAction;
@@ -36,7 +37,7 @@ class AppControls extends React.Component<IAppControlsProps, IAppControlsState>
   };
 
   public render() {
-    const { app, push } = this.props;
+    const { app, cluster, push } = this.props;
     const { name, namespace } = app;
     const deleted = app.info && app.info.deleted;
     if (!name || !namespace) {
@@ -84,7 +85,7 @@ class AppControls extends React.Component<IAppControlsProps, IAppControlsState>
             )
           }
         />
-        {this.state.redirectToAppList && <Redirect to={url.app.apps.list(namespace)} />}
+        {this.state.redirectToAppList && <Redirect to={url.app.apps.list(cluster, namespace)} />}
       </div>
     );
   }

diff --git a/dashboard/src/components/AppView/AppView.tsx b/dashboard/src/components/AppView/AppView.tsx
@@ -18,6 +18,7 @@ import ChartInfo from "./ChartInfo";
 import ResourceTable from "./ResourceTable";
 
 export interface IAppViewProps {
+  cluster: string;
   namespace: string;
   releaseName: string;
   app?: IRelease;
@@ -129,7 +130,7 @@ class AppView extends React.Component<IAppViewProps, IAppViewState> {
   }
 
   public appInfo(app: IRelease, info: hapi.release.IInfo) {
-    const { push } = this.props;
+    const { cluster, push } = this.props;
     const {
       serviceRefs,
       ingressRefs,
@@ -167,7 +168,12 @@ class AppView extends React.Component<IAppViewProps, IAppViewState> {
                     />
                   </div>
                   <div className="col-8 text-r">
-                    <AppControls app={app} deleteApp={this.deleteApp} push={push} />
+                    <AppControls
+                      cluster={cluster}
+                      app={app}
+                      deleteApp={this.deleteApp}
+                      push={push}
+                    />
                   </div>
                 </div>
                 <AccessURLTable serviceRefs={serviceRefs} ingressRefs={ingressRefs} />

diff --git a/dashboard/src/components/Header/Header.test.tsx b/dashboard/src/components/Header/Header.test.tsx
@@ -1,17 +1,22 @@
 import { shallow } from "enzyme";
 import * as React from "react";
-import { IClusterState } from "../../reducers/cluster";
+import { IClustersState } from "../../reducers/cluster";
 import { app } from "../../shared/url";
 import Header from "./Header";
 
 const defaultProps = {
   authenticated: true,
   fetchNamespaces: jest.fn(),
   logout: jest.fn(),
-  cluster: {
-    currentNamespace: "default",
-    namespaces: ["default", "other"],
-  } as IClusterState,
+  clusters: {
+    currentCluster: "default",
+    clusters: {
+      default: {
+        currentNamespace: "default",
+        namespaces: ["default", "other"],
+      },
+    },
+  } as IClustersState,
   defaultNamespace: "kubeapps-user",
   pathname: "",
   push: jest.fn(),
@@ -25,7 +30,7 @@ it("renders the header links and titles", () => {
   const menubar = wrapper.find(".header__nav__menu").first();
   const items = menubar.children().map(p => p.props().children.props);
   const expectedItems = [
-    { children: "Applications", to: app.apps.list("default") },
+    { children: "Applications", to: app.apps.list("default", "default") },
     { children: "Catalog", to: app.catalog("default") },
     { children: "Service Instances (alpha)", to: app.servicesInstances("default") },
   ];
@@ -90,7 +95,7 @@ it("renders the namespace switcher", () => {
   expect(namespaceSelector.props()).toEqual(
     expect.objectContaining({
       defaultNamespace: defaultProps.defaultNamespace,
-      cluster: defaultProps.cluster,
+      cluster: defaultProps.clusters.clusters.default,
     }),
   );
 });
@@ -99,15 +104,20 @@ it("call setNamespace and getNamespace when selecting a namespace", () => {
   const setNamespace = jest.fn();
   const createNamespace = jest.fn();
   const getNamespace = jest.fn();
-  const cluster = {
-    currentNamespace: "foo",
-    namespaces: ["foo", "bar"],
+  const clusters = {
+    ...defaultProps.clusters,
+    clusters: {
+      default: {
+        currentNamespace: "foo",
+        namespaces: ["foo", "bar"],
+      },
+    },
   };
   const wrapper = shallow(
     <Header
       {...defaultProps}
       setNamespace={setNamespace}
-      cluster={cluster}
+      clusters={clusters}
       createNamespace={createNamespace}
       getNamespace={getNamespace}
     />,
@@ -126,15 +136,20 @@ it("call setNamespace and getNamespace when selecting a namespace", () => {
 it("doesn't call getNamespace when selecting all namespaces", () => {
   const setNamespace = jest.fn();
   const getNamespace = jest.fn();
-  const cluster = {
-    currentNamespace: "foo",
-    namespaces: ["foo", "bar"],
+  const clusters = {
+    ...defaultProps.clusters,
+    clusters: {
+      default: {
+        currentNamespace: "foo",
+        namespaces: ["foo", "bar"],
+      },
+    },
   };
   const wrapper = shallow(
     <Header
       {...defaultProps}
       setNamespace={setNamespace}
-      cluster={cluster}
+      clusters={clusters}
       getNamespace={getNamespace}
     />,
   );

diff --git a/dashboard/src/components/Header/Header.tsx b/dashboard/src/components/Header/Header.tsx
@@ -4,7 +4,7 @@ import { NavLink } from "react-router-dom";
 import "react-select/dist/react-select.css";
 import { IFeatureFlags } from "shared/Config";
 import logo from "../../logo.svg";
-import { IClusterState } from "../../reducers/cluster";
+import { IClustersState } from "../../reducers/cluster";
 import { definedNamespaces } from "../../shared/Namespace";
 import { app } from "../../shared/url";
 import "./Header.css";
@@ -15,7 +15,7 @@ interface IHeaderProps {
   authenticated: boolean;
   fetchNamespaces: () => void;
   logout: () => void;
-  cluster: IClusterState;
+  clusters: IClustersState;
   defaultNamespace: string;
   pathname: string;
   push: (path: string) => void;
@@ -52,12 +52,13 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
   public render() {
     const {
       fetchNamespaces,
-      cluster,
+      clusters,
       defaultNamespace,
       authenticated: showNav,
       createNamespace,
       getNamespace,
     } = this.props;
+    const cluster = clusters.clusters[clusters.currentCluster];
     const header = `header ${this.state.mobileOpen ? "header-open" : ""}`;
     const submenu = `header__nav__submenu ${
       this.state.configOpen ? "header__nav__submenu-open" : ""
@@ -88,7 +89,10 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
                 </button>
                 <ul className="header__nav__menu" role="menubar">
                   <li>
-                    <HeaderLink to={app.apps.list(cluster.currentNamespace)} exact={true}>
+                    <HeaderLink
+                      to={app.apps.list(clusters.currentCluster, cluster.currentNamespace)}
+                      exact={true}
+                    >
                       Applications
                     </HeaderLink>
                   </li>

diff --git a/dashboard/src/components/OperatorInstance/OperatorInstance.test.tsx b/dashboard/src/components/OperatorInstance/OperatorInstance.test.tsx
@@ -15,8 +15,8 @@ import OperatorInstance, { IOperatorInstanceProps } from "./OperatorInstance";
 
 const defaultProps: IOperatorInstanceProps = {
   isFetching: false,
-  namespace: "default",
   cluster: "default",
+  namespace: "default",
   csvName: "foo",
   crdName: "foo.kubeapps.com",
   instanceName: "foo-cluster",
@@ -109,7 +109,7 @@ describe("renders a resource", () => {
     expect(deleteResource).toHaveBeenCalledWith(defaultProps.namespace, "foo", resource);
     // wait async calls
     await new Promise(r => r());
-    expect(push).toHaveBeenCalledWith(app.apps.list(defaultProps.namespace));
+    expect(push).toHaveBeenCalledWith(app.apps.list(defaultProps.cluster, defaultProps.namespace));
   });
 
   it("updates the state with the CRD resources", () => {

diff --git a/dashboard/src/components/OperatorInstance/OperatorInstance.tsx b/dashboard/src/components/OperatorInstance/OperatorInstance.tsx
@@ -262,12 +262,12 @@ class OperatorInstance extends React.Component<IOperatorInstanceProps, IOperator
   };
 
   private handleDeleteClick = async () => {
-    const { namespace, resource } = this.props;
+    const { cluster, namespace, resource } = this.props;
     const { crd } = this.state;
     const deleted = await this.props.deleteResource(namespace, crd!.name.split(".")[0], resource!);
     this.closeModal();
     if (deleted) {
-      this.props.push(app.apps.list(namespace));
+      this.props.push(app.apps.list(cluster, namespace));
     }
   };
 }

diff --git a/dashboard/src/containers/AppViewContainer/AppViewContainer.tsx b/dashboard/src/containers/AppViewContainer/AppViewContainer.tsx
@@ -10,6 +10,7 @@ import { IResource, IStoreState } from "../../shared/types";
 interface IRouteProps {
   match: {
     params: {
+      cluster: string;
       namespace: string;
       releaseName: string;
     };
@@ -22,6 +23,7 @@ function mapStateToProps({ apps, kube, charts }: IStoreState, { match: { params
     deleteError: apps.deleteError,
     resources: kube.items,
     error: apps.error,
+    cluster: params.cluster,
     namespace: params.namespace,
     releaseName: params.releaseName,
   };