Fix rollback bug (#1748)

vmware-tanzu · Jun 2, 2020 · 3249a2c · 3249a2c
1 parent a68fa6c
commit 3249a2c
Show file tree

Hide file tree

Showing 16 changed files with 57 additions and 54 deletions.
diff --git a/dashboard/src/actions/apps.test.tsx b/dashboard/src/actions/apps.test.tsx
@@ -183,22 +183,22 @@ describe("delete applications", () => {
     App.delete = deleteAppOrig;
   });
   it("delete an application", async () => {
-    await store.dispatch(actions.apps.deleteApp("foo", "default", false));
+    await store.dispatch(actions.apps.deleteApp("default", "foo", false));
     const expectedActions = [
       { type: getType(actions.apps.requestDeleteApp) },
       { type: getType(actions.apps.receiveDeleteApp) },
     ];
     expect(store.getActions()).toEqual(expectedActions);
-    expect(deleteAppMock.mock.calls[0]).toEqual(["foo", "default", false]);
+    expect(deleteAppMock.mock.calls[0]).toEqual(["default", "foo", false]);
   });
   it("delete and purge an application", async () => {
-    await store.dispatch(actions.apps.deleteApp("foo", "default", true));
+    await store.dispatch(actions.apps.deleteApp("default", "foo", true));
     const expectedActions = [
       { type: getType(actions.apps.requestDeleteApp) },
       { type: getType(actions.apps.receiveDeleteApp) },
     ];
     expect(store.getActions()).toEqual(expectedActions);
-    expect(deleteAppMock.mock.calls[0]).toEqual(["foo", "default", true]);
+    expect(deleteAppMock.mock.calls[0]).toEqual(["default", "foo", true]);
   });
   it("delete and throw an error", async () => {
     const error = new Error("something went wrong!");
@@ -209,7 +209,7 @@ describe("delete applications", () => {
     deleteAppMock.mockImplementation(() => {
       throw error;
     });
-    expect(await store.dispatch(actions.apps.deleteApp("foo", "default", true))).toBe(false);
+    expect(await store.dispatch(actions.apps.deleteApp("default", "foo", true))).toBe(false);
     expect(store.getActions()).toEqual(expectedActions);
   });
 });
@@ -221,12 +221,12 @@ describe("deploy chart", () => {
 
   it("returns true if namespace is correct and deployment is successful", async () => {
     const res = await store.dispatch(
-      actions.apps.deployChart("my-version" as any, "chart-namespace", "my-release", "default"),
+      actions.apps.deployChart("my-version" as any, "chart-namespace", "default", "my-release"),
     );
     expect(res).toBe(true);
     expect(App.create).toHaveBeenCalledWith(
-      "my-release",
       "default",
+      "my-release",
       "chart-namespace",
       "my-version",
       undefined,
@@ -243,8 +243,8 @@ describe("deploy chart", () => {
       actions.apps.deployChart(
         "my-version" as any,
         "chart-namespace",
-        "my-release",
         definedNamespaces.all,
+        "my-release",
       ),
     );
     expect(res).toBe(false);
@@ -264,8 +264,8 @@ describe("deploy chart", () => {
       actions.apps.deployChart(
         "my-version" as any,
         "chart-namespace",
-        "my-release",
         "default",
+        "my-release",
         "foo: 1",
         {
           properties: { foo: { type: "string" } },
@@ -290,8 +290,8 @@ describe("upgradeApp", () => {
   const provisionCMD = actions.apps.upgradeApp(
     "my-version" as any,
     "kubeapps-ns",
-    "my-release",
     definedNamespaces.all,
+    "my-release",
   );
 
   it("calls ServiceBinding.delete and returns true if no error", async () => {
@@ -305,8 +305,8 @@ describe("upgradeApp", () => {
     ];
     expect(store.getActions()).toEqual(expectedActions);
     expect(App.upgrade).toHaveBeenCalledWith(
-      "my-release",
       definedNamespaces.all,
+      "my-release",
       "kubeapps-ns",
       "my-version" as any,
       undefined,
@@ -335,8 +335,8 @@ describe("upgradeApp", () => {
       actions.apps.upgradeApp(
         "my-version" as any,
         "kubeapps-ns",
-        "my-release",
         "default",
+        "my-release",
         "foo: 1",
         {
           properties: { foo: { type: "string" } },
@@ -359,20 +359,23 @@ describe("upgradeApp", () => {
 });
 
 describe("rollbackApp", () => {
-  const provisionCMD = actions.apps.rollbackApp("my-release", "default", 1);
+  const provisionCMD = actions.apps.rollbackApp("default", "my-release", 1);
 
   it("success and re-request apps info", async () => {
     App.rollback = jest.fn().mockImplementationOnce(() => true);
+    App.getRelease = jest.fn().mockImplementationOnce(() => true);
     const res = await store.dispatch(provisionCMD);
     expect(res).toBe(true);
 
     const expectedActions = [
       { type: getType(actions.apps.requestRollbackApp) },
       { type: getType(actions.apps.receiveRollbackApp) },
       { type: getType(actions.apps.requestApps) },
+      { type: getType(actions.apps.selectApp), payload: true },
     ];
     expect(store.getActions()).toEqual(expectedActions);
-    expect(App.rollback).toHaveBeenCalledWith("my-release", "default", 1);
+    expect(App.rollback).toHaveBeenCalledWith("default", "my-release", 1);
+    expect(App.getRelease).toHaveBeenCalledWith("default", "my-release");
   });
 
   it("dispatches an error", async () => {

diff --git a/dashboard/src/actions/apps.ts b/dashboard/src/actions/apps.ts
@@ -87,8 +87,8 @@ const allActions = [
 export type AppsAction = ActionType<typeof allActions[number]>;
 
 export function getApp(
-  releaseName: string,
   namespace: string,
+  releaseName: string,
 ): ThunkAction<Promise<hapi.release.Release | undefined>, IStoreState, null, AppsAction> {
   return async dispatch => {
     dispatch(requestApps());
@@ -164,7 +164,7 @@ export function getAppWithUpdateInfo(
 ): ThunkAction<Promise<void>, IStoreState, null, AppsAction> {
   return async dispatch => {
     try {
-      const app = await dispatch(getApp(releaseName, namespace));
+      const app = await dispatch(getApp(namespace, releaseName));
       if (
         app &&
         app.chart &&
@@ -189,14 +189,14 @@ export function getAppWithUpdateInfo(
 }
 
 export function deleteApp(
-  releaseName: string,
   namespace: string,
+  releaseName: string,
   purge: boolean,
 ): ThunkAction<Promise<boolean>, IStoreState, null, AppsAction> {
   return async dispatch => {
     dispatch(requestDeleteApp());
     try {
-      await App.delete(releaseName, namespace, purge);
+      await App.delete(namespace, releaseName, purge);
       dispatch(receiveDeleteApp());
       return true;
     } catch (e) {
@@ -249,8 +249,8 @@ export function fetchAppsWithUpdateInfo(
 export function deployChart(
   chartVersion: IChartVersion,
   chartNamespace: string,
-  releaseName: string,
   namespace: string,
+  releaseName: string,
   values?: string,
   schema?: JSONSchema4,
 ): ThunkAction<Promise<boolean>, IStoreState, null, AppsAction> {
@@ -274,7 +274,7 @@ export function deployChart(
           );
         }
       }
-      await App.create(releaseName, namespace, chartNamespace, chartVersion, values);
+      await App.create(namespace, releaseName, chartNamespace, chartVersion, values);
       dispatch(receiveDeployApp());
       return true;
     } catch (e) {
@@ -287,8 +287,8 @@ export function deployChart(
 export function upgradeApp(
   chartVersion: IChartVersion,
   chartNamespace: string,
-  releaseName: string,
   namespace: string,
+  releaseName: string,
   values?: string,
   schema?: JSONSchema4,
 ): ThunkAction<Promise<boolean>, IStoreState, null, AppsAction> {
@@ -306,7 +306,7 @@ export function upgradeApp(
           );
         }
       }
-      await App.upgrade(releaseName, namespace, chartNamespace, chartVersion, values);
+      await App.upgrade(namespace, releaseName, chartNamespace, chartVersion, values);
       dispatch(receiveUpgradeApp());
       return true;
     } catch (e) {
@@ -317,16 +317,16 @@ export function upgradeApp(
 }
 
 export function rollbackApp(
-  releaseName: string,
   namespace: string,
+  releaseName: string,
   revision: number,
 ): ThunkAction<Promise<boolean>, IStoreState, null, AppsAction> {
   return async (dispatch, getState) => {
     dispatch(requestRollbackApp());
     try {
-      await App.rollback(releaseName, namespace, revision);
+      await App.rollback(namespace, releaseName, revision);
       dispatch(receiveRollbackApp());
-      dispatch(getAppWithUpdateInfo(releaseName, namespace));
+      dispatch(getAppWithUpdateInfo(namespace, releaseName));
       return true;
     } catch (e) {
       dispatch(errorApps(e));

diff --git a/dashboard/src/components/AppUpgrade/AppUpgrade.tsx b/dashboard/src/components/AppUpgrade/AppUpgrade.tsx
@@ -21,8 +21,8 @@ interface IAppUpgradeProps {
   upgradeApp: (
     version: IChartVersion,
     chartNamespace: string,
-    releaseName: string,
     namespace: string,
+    releaseName: string,
     values?: string,
     schema?: JSONSchema4,
   ) => Promise<boolean>;

diff --git a/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.test.tsx b/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.test.tsx
@@ -27,5 +27,5 @@ it("should perform the rollback", async () => {
   expect(dialog).toExist();
   const onConfirm = dialog.prop("onConfirm") as (revision: number) => Promise<any>;
   await onConfirm(1);
-  expect(rollbackApp).toBeCalledWith(defaultProps.releaseName, defaultProps.namespace, 1);
+  expect(rollbackApp).toBeCalledWith(defaultProps.namespace, defaultProps.releaseName, 1);
 });
diff --git a/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.tsx b/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.tsx
@@ -6,7 +6,7 @@ interface IRollbackButtonProps {
   releaseName: string;
   namespace: string;
   revision: number;
-  rollbackApp: (releaseName: string, namespace: string, revision: number) => Promise<boolean>;
+  rollbackApp: (namespace: string, releaseName: string, revision: number) => Promise<boolean>;
   loading: boolean;
   error?: Error;
 }
@@ -60,8 +60,8 @@ class RollbackButton extends React.Component<IRollbackButtonProps> {
   private handleRollback = async (revision: number) => {
     this.setState({ loading: true });
     const success = await this.props.rollbackApp(
-      this.props.releaseName,
       this.props.namespace,
+      this.props.releaseName,
       revision,
     );
     // If there is an error it's catched at AppView level

diff --git a/dashboard/src/components/AppView/AppView.tsx b/dashboard/src/components/AppView/AppView.tsx
@@ -24,7 +24,7 @@ export interface IAppViewProps {
   error: Error | undefined;
   deleteError: Error | undefined;
   getAppWithUpdateInfo: (namespace: string, releaseName: string) => void;
-  deleteApp: (releaseName: string, namespace: string, purge: boolean) => Promise<boolean>;
+  deleteApp: (namespace: string, releaseName: string, purge: boolean) => Promise<boolean>;
   push: (location: string) => RouterAction;
 }
 
@@ -240,7 +240,7 @@ class AppView extends React.Component<IAppViewProps, IAppViewState> {
   }
 
   private deleteApp = (purge: boolean) => {
-    return this.props.deleteApp(this.props.releaseName, this.props.namespace, purge);
+    return this.props.deleteApp(this.props.namespace, this.props.releaseName, purge);
   };
 }
 

diff --git a/dashboard/src/components/DeploymentForm/DeploymentForm.test.tsx b/dashboard/src/components/DeploymentForm/DeploymentForm.test.tsx
@@ -185,8 +185,8 @@ it("triggers a deployment when submitting the form", done => {
   expect(deployChart).toHaveBeenCalledWith(
     versions[0],
     defaultProps.chartNamespace,
-    releaseName,
     namespace,
+    releaseName,
     appValues,
     schema,
   );

diff --git a/dashboard/src/components/DeploymentForm/DeploymentForm.tsx b/dashboard/src/components/DeploymentForm/DeploymentForm.tsx
@@ -25,8 +25,8 @@ export interface IDeploymentFormProps {
   deployChart: (
     version: IChartVersion,
     chartNamespace: string,
-    releaseName: string,
     namespace: string,
+    releaseName: string,
     values?: string,
     schema?: JSONSchema4,
   ) => Promise<boolean>;
@@ -163,8 +163,8 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
       const deployed = await deployChart(
         selected.version,
         chartNamespace,
-        releaseName,
         namespace,
+        releaseName,
         appValues,
         selected.schema,
       );

diff --git a/dashboard/src/components/UpgradeForm/UpgradeForm.test.tsx b/dashboard/src/components/UpgradeForm/UpgradeForm.test.tsx
@@ -114,8 +114,8 @@ it("triggers an upgrade when submitting the form", done => {
   expect(upgradeApp).toHaveBeenCalledWith(
     versions[0],
     "kubeapps",
-    releaseName,
     namespace,
+    releaseName,
     appValues,
     schema,
   );

diff --git a/dashboard/src/components/UpgradeForm/UpgradeForm.tsx b/dashboard/src/components/UpgradeForm/UpgradeForm.tsx
@@ -24,8 +24,8 @@ export interface IUpgradeFormProps {
   upgradeApp: (
     version: IChartVersion,
     chartNamespace: string,
-    releaseName: string,
     namespace: string,
+    releaseName: string,
     values?: string,
     schema?: JSONSchema4,
   ) => Promise<boolean>;
@@ -151,8 +151,8 @@ class UpgradeForm extends React.Component<IUpgradeFormProps, IUpgradeFormState>
       const deployed = await upgradeApp(
         selected.version,
         repoNamespace,
-        releaseName,
         namespace,
+        releaseName,
         appValues,
         selected.schema,
       );

diff --git a/dashboard/src/containers/AppNewContainer/AppNewContainer.tsx b/dashboard/src/containers/AppNewContainer/AppNewContainer.tsx
@@ -40,13 +40,13 @@ function mapDispatchToProps(dispatch: ThunkDispatch<IStoreState, null, Action>)
     deployChart: (
       version: IChartVersion,
       chartNamespace: string,
-      releaseName: string,
       namespace: string,
+      releaseName: string,
       values?: string,
       schema?: JSONSchema4,
     ) =>
       dispatch(
-        actions.apps.deployChart(version, chartNamespace, releaseName, namespace, values, schema),
+        actions.apps.deployChart(version, chartNamespace, namespace, releaseName, values, schema),
       ),
     fetchChartVersions: (namespace: string, id: string) =>
       dispatch(actions.charts.fetchChartVersions(namespace, id)),

diff --git a/dashboard/src/containers/AppUpgradeContainer/AppUpgradeContainer.tsx b/dashboard/src/containers/AppUpgradeContainer/AppUpgradeContainer.tsx
@@ -62,13 +62,13 @@ function mapDispatchToProps(dispatch: ThunkDispatch<IStoreState, null, Action>)
     upgradeApp: (
       version: IChartVersion,
       chartNamespace: string,
-      releaseName: string,
       namespace: string,
+      releaseName: string,
       values?: string,
       schema?: JSONSchema4,
     ) =>
       dispatch(
-        actions.apps.upgradeApp(version, chartNamespace, releaseName, namespace, values, schema),
+        actions.apps.upgradeApp(version, chartNamespace, namespace, releaseName, values, schema),
       ),
     getDeployedChartVersion: (namespace: string, id: string, version: string) =>
       dispatch(actions.charts.getDeployedChartVersion(namespace, id, version)),

diff --git a/dashboard/src/containers/AppViewContainer/AppViewContainer.tsx b/dashboard/src/containers/AppViewContainer/AppViewContainer.tsx
@@ -29,8 +29,8 @@ function mapStateToProps({ apps, kube, charts }: IStoreState, { match: { params
 
 function mapDispatchToProps(dispatch: ThunkDispatch<IStoreState, null, Action>) {
   return {
-    deleteApp: (releaseName: string, ns: string, purge: boolean) =>
-      dispatch(actions.apps.deleteApp(releaseName, ns, purge)),
+    deleteApp: (namespace: string, releaseName: string, purge: boolean) =>
+      dispatch(actions.apps.deleteApp(namespace, releaseName, purge)),
     getAppWithUpdateInfo: (namespace: string, releaseName: string) =>
       dispatch(actions.apps.getAppWithUpdateInfo(namespace, releaseName)),
     // TODO: remove once WebSockets are moved to Redux store (#882)

diff --git a/dashboard/src/containers/RollbackButtonContainer/RollbackButtonContainer.tsx b/dashboard/src/containers/RollbackButtonContainer/RollbackButtonContainer.tsx
@@ -23,8 +23,8 @@ function mapStateToProps({ apps }: IStoreState, props: IButtonProps) {
 
 function mapDispatchToProps(dispatch: ThunkDispatch<IStoreState, null, Action>) {
   return {
-    rollbackApp: (releaseName: string, namespace: string, revision: number) =>
-      dispatch(actions.apps.rollbackApp(releaseName, namespace, revision)),
+    rollbackApp: (namespace: string, releaseName: string, revision: number) =>
+      dispatch(actions.apps.rollbackApp(namespace, releaseName, revision)),
   };
 }