Show upgrade form even after failure (#2130)

vmware-tanzu · Nov 5, 2020 · 664b919 · 664b919
1 parent 0909220
commit 664b919
Show file tree

Hide file tree

Showing 16 changed files with 110 additions and 78 deletions.
diff --git a/dashboard/src/actions/apps.test.tsx b/dashboard/src/actions/apps.test.tsx
@@ -203,7 +203,7 @@ describe("delete applications", () => {
     const error = new Error("something went wrong!");
     const expectedActions = [
       { type: getType(actions.apps.requestDeleteApp) },
-      { type: getType(actions.apps.errorDeleteApp), payload: error },
+      { type: getType(actions.apps.errorApp), payload: error },
     ];
     deleteAppMock.mockImplementation(() => {
       throw error;
@@ -260,7 +260,7 @@ describe("deploy chart", () => {
     const expectedActions = [
       { type: getType(actions.apps.requestDeployApp) },
       {
-        type: getType(actions.apps.errorApps),
+        type: getType(actions.apps.errorApp),
         payload: new UnprocessableEntity(
           "Namespace not selected. Please select a namespace using the selector in the top right corner.",
         ),
@@ -286,7 +286,7 @@ describe("deploy chart", () => {
     const expectedActions = [
       { type: getType(actions.apps.requestDeployApp) },
       {
-        type: getType(actions.apps.errorApps),
+        type: getType(actions.apps.errorApp),
         payload: new UnprocessableEntity(
           "The given values don't match the required format. The following errors were found:\n  - .foo: should be string",
         ),
@@ -333,7 +333,7 @@ describe("upgradeApp", () => {
     const expectedActions = [
       { type: getType(actions.apps.requestUpgradeApp) },
       {
-        type: getType(actions.apps.errorApps),
+        type: getType(actions.apps.errorApp),
         payload: new Error("Boom!"),
       },
     ];
@@ -361,7 +361,7 @@ describe("upgradeApp", () => {
     const expectedActions = [
       { type: getType(actions.apps.requestUpgradeApp) },
       {
-        type: getType(actions.apps.errorApps),
+        type: getType(actions.apps.errorApp),
         payload: new UnprocessableEntity(
           "The given values don't match the required format. The following errors were found:\n  - .foo: should be string",
         ),
@@ -399,7 +399,7 @@ describe("rollbackApp", () => {
     const expectedActions = [
       { type: getType(actions.apps.requestRollbackApp) },
       {
-        type: getType(actions.apps.errorApps),
+        type: getType(actions.apps.errorApp),
         payload: new Error("Boom!"),
       },
     ];

diff --git a/dashboard/src/actions/apps.ts b/dashboard/src/actions/apps.ts
@@ -8,12 +8,17 @@ import { hapi } from "../shared/hapi/release";
 import { definedNamespaces } from "../shared/Namespace";
 import { validate } from "../shared/schema";
 import {
+  CreateError,
+  DeleteError,
+  FetchError,
   IAppOverview,
   IChartUpdateInfo,
   IChartVersion,
   IRelease,
   IStoreState,
+  RollbackError,
   UnprocessableEntity,
+  UpgradeError,
 } from "../shared/types";
 
 export const requestApps = createAction("REQUEST_APPS");
@@ -50,12 +55,9 @@ export const requestRollbackApp = createAction("REQUEST_ROLLBACK_APP");
 
 export const receiveRollbackApp = createAction("RECEIVE_ROLLBACK_APP_CONFIRMATION");
 
-export const errorApps = createAction("ERROR_APPS", resolve => {
-  return (err: Error) => resolve(err);
-});
-
-export const errorDeleteApp = createAction("ERROR_DELETE_APP", resolve => {
-  return (err: Error) => resolve(err);
+export const errorApp = createAction("ERROR_APP", resolve => {
+  return (err: FetchError | CreateError | UpgradeError | RollbackError | DeleteError) =>
+    resolve(err);
 });
 
 export const selectApp = createAction("SELECT_APP", resolve => {
@@ -77,8 +79,7 @@ const allActions = [
   receiveUpgradeApp,
   requestRollbackApp,
   receiveRollbackApp,
-  errorApps,
-  errorDeleteApp,
+  errorApp,
   selectApp,
 ];
 
@@ -96,7 +97,7 @@ export function getApp(
       dispatch(selectApp(app));
       return app;
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new FetchError(e.message)));
       return;
     }
   };
@@ -185,7 +186,7 @@ export function getAppWithUpdateInfo(
         );
       }
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new FetchError(e.message)));
     }
   };
 }
@@ -203,7 +204,7 @@ export function deleteApp(
       dispatch(receiveDeleteApp());
       return true;
     } catch (e) {
-      dispatch(errorDeleteApp(e));
+      dispatch(errorApp(new DeleteError(e.message)));
       return false;
     }
   };
@@ -224,7 +225,7 @@ export function fetchApps(
       dispatch(receiveAppList(apps));
       return apps;
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new FetchError(e.message)));
       return [];
     }
   };
@@ -291,7 +292,7 @@ export function deployChart(
       dispatch(receiveDeployApp());
       return true;
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new CreateError(e.message)));
       return false;
     }
   };
@@ -324,7 +325,7 @@ export function upgradeApp(
       dispatch(receiveUpgradeApp());
       return true;
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new UpgradeError(e.message)));
       return false;
     }
   };
@@ -344,7 +345,7 @@ export function rollbackApp(
       dispatch(getAppWithUpdateInfo(cluster, namespace, releaseName));
       return true;
     } catch (e) {
-      dispatch(errorApps(e));
+      dispatch(errorApp(new RollbackError(e.message)));
       return false;
     }
   };

diff --git a/dashboard/src/components/AppList/AppList.test.tsx b/dashboard/src/components/AppList/AppList.test.tsx
@@ -5,7 +5,7 @@ import * as React from "react";
 import SearchFilter from "components/SearchFilter/SearchFilter";
 import { BrowserRouter as Router } from "react-router-dom";
 import itBehavesLike from "../../shared/specs";
-import { IAppOverview, IAppState } from "../../shared/types";
+import { FetchError, IAppOverview, IAppState } from "../../shared/types";
 import Alert from "../js/Alert";
 import AppList, { IAppListProps } from "./AppList";
 import AppListItem from "./AppListItem";
@@ -116,7 +116,7 @@ context("when fetched but not apps available", () => {
 
 context("when an error is present", () => {
   beforeEach(() => {
-    props = { ...defaultProps, apps: { error: new Error("Boom!") } };
+    props = { ...defaultProps, apps: { error: new FetchError("Boom!") } };
   });
 
   it("matches the snapshot", () => {

diff --git a/dashboard/src/components/AppUpgrade/AppUpgrade.test.tsx b/dashboard/src/components/AppUpgrade/AppUpgrade.test.tsx
@@ -5,7 +5,14 @@ import * as React from "react";
 import Alert from "components/js/Alert";
 import { hapi } from "shared/hapi/release";
 import { defaultStore, mountWrapper } from "shared/specs/mountWrapper";
-import { IAppRepository, IChartState, IChartVersion, IRelease } from "shared/types";
+import {
+  FetchError,
+  IAppRepository,
+  IChartState,
+  IChartVersion,
+  IRelease,
+  UpgradeError,
+} from "shared/types";
 import itBehavesLike from "../../shared/specs";
 import SelectRepoForm from "../SelectRepoForm/SelectRepoForm";
 import UpgradeForm from "../UpgradeForm/UpgradeForm";
@@ -103,7 +110,7 @@ context("when an error exists", () => {
     const wrapper = shallow(
       <AppUpgrade
         {...defaultProps}
-        appsError={new Error("foo does not exist")}
+        error={new FetchError("foo does not exist")}
         repos={[repo]}
         repo={repo}
       />,
@@ -147,6 +154,37 @@ context("when an error exists", () => {
         .text(),
     ).toContain("Chart repositories not found");
   });
+
+  it("still renders the upgrade form even if there is an upgrade error", () => {
+    const repo = {
+      metadata: { name: "stable" },
+    } as IAppRepository;
+    const upgradeError = new UpgradeError("foo upgrade failed");
+    const wrapper = shallow(
+      <AppUpgrade
+        {...defaultProps}
+        error={upgradeError}
+        repos={[repo]}
+        repo={repo}
+        app={
+          {
+            chart: {
+              metadata: {
+                name: "bar",
+                version: "1.0.0",
+              },
+            },
+            name: "foo",
+            updateInfo: { repository: {} },
+          } as IRelease
+        }
+        repoName="foobar"
+      />,
+    );
+
+    expect(wrapper.find(UpgradeForm)).toExist();
+    expect(wrapper.find(UpgradeForm).prop("error")).toEqual(upgradeError);
+  });
 });
 
 it("renders the upgrade form when the repo is available", () => {

diff --git a/dashboard/src/components/AppUpgrade/AppUpgrade.tsx b/dashboard/src/components/AppUpgrade/AppUpgrade.tsx
@@ -3,7 +3,14 @@ import React, { useEffect } from "react";
 import Alert from "components/js/Alert";
 import { RouterAction } from "connected-react-router";
 import { JSONSchema4 } from "json-schema";
-import { IAppRepository, IChartState, IChartVersion, IRelease } from "../../shared/types";
+import {
+  FetchError,
+  IAppRepository,
+  IChartState,
+  IChartVersion,
+  IRelease,
+  UpgradeError,
+} from "../../shared/types";
 import LoadingWrapper from "../LoadingWrapper/LoadingWrapper";
 import SelectRepoForm from "../SelectRepoForm/SelectRepoForm";
 import UpgradeForm from "../UpgradeForm/UpgradeForm";
@@ -12,7 +19,7 @@ export interface IAppUpgradeProps {
   app?: IRelease;
   appsIsFetching: boolean;
   chartsIsFetching: boolean;
-  appsError: Error | undefined;
+  error?: FetchError | UpgradeError;
   namespace: string;
   cluster: string;
   releaseName: string;
@@ -53,7 +60,7 @@ function AppUpgrade({
   app,
   appsIsFetching,
   chartsIsFetching,
-  appsError,
+  error,
   namespace,
   cluster,
   releaseName,
@@ -67,13 +74,6 @@ function AppUpgrade({
   getChartVersion,
   getDeployedChartVersion,
   push,
-  reposIsFetching,
-  repoError,
-  chartsError,
-  repo,
-  repos,
-  checkChart,
-  fetchRepositories,
 }: IAppUpgradeProps) {
   useEffect(() => {
     getAppWithUpdateInfo(cluster, namespace, releaseName);
@@ -95,13 +95,10 @@ function AppUpgrade({
     }
   }, [getDeployedChartVersion, app, chart, repoName, repoNamespace, cluster]);
 
-  if (appsError) {
-    return (
-      <Alert theme="danger">
-        An error occurred while processing the application: {appsError.message}
-      </Alert>
-    );
+  if (error && error.constructor === FetchError) {
+    return <Alert theme="danger">Unable to retrieve the current app: {error.message}</Alert>;
   }
+
   if (appsIsFetching || !app || !app.updateInfo) {
     return <LoadingWrapper loaded={false} />;
   }
@@ -125,6 +122,7 @@ function AppUpgrade({
           deployed={deployed}
           upgradeApp={upgradeApp}
           push={push}
+          error={error}
           fetchChartVersions={fetchChartVersions}
           getChartVersion={getChartVersion}
         />

diff --git a/dashboard/src/components/AppView/AppControls/DeleteButton/DeleteButton.test.tsx b/dashboard/src/components/AppView/AppControls/DeleteButton/DeleteButton.test.tsx
@@ -6,6 +6,7 @@ import * as React from "react";
 import { act } from "react-dom/test-utils";
 import * as ReactRedux from "react-redux";
 import { defaultStore, getStore, mountWrapper } from "shared/specs/mountWrapper";
+import { DeleteError } from "shared/types";
 import DeleteButton from "./DeleteButton";
 
 const defaultProps = {
@@ -54,7 +55,7 @@ it("deletes an application", async () => {
 });
 
 it("renders an error", async () => {
-  const store = getStore({ apps: { deleteError: new Error("Boom!") } });
+  const store = getStore({ apps: { error: new DeleteError("Boom!") } });
   const wrapper = mountWrapper(store, <DeleteButton {...defaultProps} />);
   // Open modal
   act(() => {

diff --git a/dashboard/src/components/AppView/AppControls/DeleteButton/DeleteButton.tsx b/dashboard/src/components/AppView/AppControls/DeleteButton/DeleteButton.tsx
@@ -20,7 +20,7 @@ export default function DeleteButton({ cluster, namespace, releaseName }: IDelet
   const [modalIsOpen, setModal] = useState(false);
   const [deleting, setDeleting] = useState(false);
   const dispatch: ThunkDispatch<IStoreState, null, Action> = useDispatch();
-  const error = useSelector((state: IStoreState) => state.apps.deleteError);
+  const error = useSelector((state: IStoreState) => state.apps.error);
 
   const openModal = () => setModal(true);
   const closeModal = () => setModal(false);

diff --git a/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.test.tsx b/dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.test.tsx
@@ -6,6 +6,7 @@ import * as React from "react";
 import { act } from "react-dom/test-utils";
 import * as ReactRedux from "react-redux";
 import { defaultStore, getStore, mountWrapper } from "shared/specs/mountWrapper";
+import { RollbackError } from "shared/types";
 import RollbackButton from "./RollbackButton";
 
 const defaultProps = {
@@ -55,7 +56,7 @@ it("rolls back an application", async () => {
 });
 
 it("renders an error", async () => {
-  const store = getStore({ apps: { error: new Error("Boom!") } });
+  const store = getStore({ apps: { error: new RollbackError("Boom!") } });
   const wrapper = mountWrapper(store, <RollbackButton {...defaultProps} />);
   // Open modal
   act(() => {

diff --git a/dashboard/src/components/AppView/AppView.test.tsx b/dashboard/src/components/AppView/AppView.test.tsx
@@ -4,7 +4,7 @@ import * as React from "react";
 import Alert from "components/js/Alert";
 import LoadingWrapper from "components/LoadingWrapper/LoadingWrapper";
 import { defaultStore, mountWrapper } from "shared/specs/mountWrapper";
-import { IResource } from "shared/types";
+import { DeleteError, IResource } from "shared/types";
 import ApplicationStatusContainer from "../../containers/ApplicationStatusContainer";
 import { hapi } from "../../shared/hapi/release";
 import ResourceRef from "../../shared/ResourceRef";
@@ -32,7 +32,6 @@ describe("AppViewComponent", () => {
   const validProps = {
     app: appRelease,
     deleteApp: jest.fn(),
-    deleteError: undefined,
     error: undefined,
     getAppWithUpdateInfo: jest.fn(),
     namespace: "my-happy-place",
@@ -237,7 +236,7 @@ describe("AppViewComponent", () => {
     it("renders a delete-error", () => {
       const wrapper = mountWrapper(
         defaultStore,
-        <AppViewComponent {...validProps} deleteError={new Error("Boom!")} />,
+        <AppViewComponent {...validProps} error={new DeleteError("Boom!")} />,
       );
       const err = wrapper.find(Alert);
       expect(err).toExist();