Improve upgradeView, reduce API calls (#3540)

* Initial working version. Now just 3 calls are required Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Remove many params in UpgradeForm Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Simplify contexts and refs Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Use the state in the upgradeForm Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Use elements instead of strings in diff view Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix AppUpgrade tests Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Use new api operation Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Remove unused action Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix pkg loading name Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Avoid re-requesting the pkg versions Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix tests after the refactor Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
vmware-tanzu · Oct 11, 2021 · e9a6b9c · e9a6b9c
1 parent 3f6d1d0
commit e9a6b9c
Show file tree

Hide file tree

Showing 15 changed files with 607 additions and 611 deletions.
diff --git a/dashboard/src/actions/packages.test.tsx b/dashboard/src/actions/packages.test.tsx
@@ -481,46 +481,3 @@ describe("fetchAndSelectAvailablePackageDetail", () => {
     expect(store.getActions()).toEqual(expectedActions);
   });
 });
-
-describe("fetchDeployedAvailablePackageDetail", () => {
-  it("should request a deployed package", async () => {
-    const response: GetAvailablePackageDetailResponse = {
-      availablePackageDetail: defaultAvailablePackageDetail,
-    };
-    const mockGetAvailablePackageDetail = jest
-      .fn()
-      .mockImplementation(() => Promise.resolve(response));
-    jest
-      .spyOn(PackagesService, "getAvailablePackageDetail")
-      .mockImplementation(mockGetAvailablePackageDetail);
-
-    const expectedActions = [
-      { type: getType(actions.packages.requestDeployedAvailablePackageDetail) },
-      {
-        type: getType(actions.packages.receiveDeployedAvailablePackageDetail),
-        payload: { availablePackageDetail: defaultAvailablePackageDetail },
-      },
-    ];
-
-    await store.dispatch(
-      actions.packages.fetchDeployedAvailablePackageDetail(
-        {
-          context: { cluster: cluster, namespace: namespace },
-          identifier: "foo",
-          plugin: { name: "my.plugin", version: "0.0.1" } as Plugin,
-        } as AvailablePackageReference,
-        "1.0.0",
-      ),
-    );
-
-    expect(store.getActions()).toEqual(expectedActions);
-    expect(mockGetAvailablePackageDetail.mock.calls[0]).toEqual([
-      {
-        context: { cluster: cluster, namespace: namespace },
-        identifier: "foo",
-        plugin: { name: "my.plugin", version: "0.0.1" } as Plugin,
-      } as AvailablePackageReference,
-      "1.0.0",
-    ]);
-  });
-});
diff --git a/dashboard/src/actions/packages.ts b/dashboard/src/actions/packages.ts
@@ -70,24 +70,6 @@ export const receiveSelectedAvailablePackageVersions = createAction(
 
 // No reset action
 
-// ** DeployedAvailablePackage actions **
-// related to the already deployed package in the state
-
-// Request action
-export const requestDeployedAvailablePackageDetail = createAction(
-  "REQUEST_DEPLOYED_AVAILABLE_PACKAGE_DETAIL",
-);
-
-// Receive action
-export const receiveDeployedAvailablePackageDetail = createAction(
-  "RECEIVE_DEPLOYED_AVAILABLE_PACKAGE_DETAIL",
-  resolve => {
-    return (availablePackageDetail: AvailablePackageDetail) => resolve({ availablePackageDetail });
-  },
-);
-
-// No reset action
-
 // ** Error actions **
 // for handling the erros thrown by the rest of the actions
 
@@ -108,8 +90,6 @@ const allActions = [
   resetSelectedAvailablePackageDetail,
   requestSelectedAvailablePackageVersions,
   receiveSelectedAvailablePackageVersions,
-  requestDeployedAvailablePackageDetail,
-  receiveDeployedAvailablePackageDetail,
   createErrorPackage,
   clearErrorPackage,
 ];
@@ -177,23 +157,3 @@ export function fetchAndSelectAvailablePackageDetail(
     }
   };
 }
-
-export function fetchDeployedAvailablePackageDetail(
-  availablePackageReference?: AvailablePackageReference,
-  version?: string,
-): ThunkAction<Promise<void>, IStoreState, null, PackagesAction> {
-  return async dispatch => {
-    try {
-      dispatch(requestDeployedAvailablePackageDetail());
-      const response = await PackagesService.getAvailablePackageDetail(
-        availablePackageReference,
-        version,
-      );
-      if (response.availablePackageDetail) {
-        dispatch(receiveDeployedAvailablePackageDetail(response.availablePackageDetail));
-      }
-    } catch (e: any) {
-      dispatch(createErrorPackage(new FetchError(e.message)));
-    }
-  };
-}
diff --git a/dashboard/src/components/AppUpgrade/AppUpgrade.test.tsx b/dashboard/src/components/AppUpgrade/AppUpgrade.test.tsx
@@ -1,9 +1,10 @@
-import actions from "actions";
 import Alert from "components/js/Alert";
 import LoadingWrapper from "components/LoadingWrapper";
 import {
+  AvailablePackageDetail,
   AvailablePackageReference,
   Context,
+  InstalledPackageDetail,
   InstalledPackageReference,
   InstalledPackageStatus,
   InstalledPackageStatus_StatusReason,
@@ -21,8 +22,10 @@ import {
   FetchError,
   IAppRepository,
   IAppState,
+  IPackageState,
   UpgradeError,
 } from "shared/types";
+import { PluginNames } from "shared/utils";
 import SelectRepoForm from "../SelectRepoForm/SelectRepoForm";
 import UpgradeForm from "../UpgradeForm/UpgradeForm";
 import AppUpgrade from "./AppUpgrade";
@@ -64,6 +67,20 @@ const installedPackage1 = {
   } as InstalledPackageStatus,
 } as CustomInstalledPackageDetail;
 
+const installedPackageDetail = {
+  availablePackageRef: {
+    context: { cluster: "default", namespace: "my-ns" },
+    identifier: "test",
+    plugin: { name: PluginNames.PACKAGES_HELM, version: "0.0.1" } as Plugin,
+  },
+  currentVersion: { appVersion: "4.5.6", pkgVersion: "1.2.3" },
+} as InstalledPackageDetail;
+
+const selectedPackage = {
+  versions: [{ appVersion: "10.0.0", pkgVersion: "1.2.3" }],
+  availablePackageDetail: { name: "test" } as AvailablePackageDetail,
+} as IPackageState["selected"];
+
 const repo1 = {
   metadata: {
     name: defaultProps.repo,
@@ -98,7 +115,7 @@ it("renders the repo selection form if not introduced", () => {
     } as IAppState,
   };
   const wrapper = mountWrapper(
-    getStore({ ...defaultStore, apps: { ...state.apps } }),
+    getStore({ ...defaultStore, ...state }),
     <MemoryRouter initialEntries={[routePathParam]}>
       <Route path={routePath}>
         <AppUpgrade />,
@@ -118,8 +135,7 @@ it("renders the repo selection form if not introduced when the app is loaded", (
   const wrapper = mountWrapper(
     getStore({
       ...defaultStore,
-      repos: { ...state.repos },
-      apps: { ...state.apps },
+      ...state,
     }),
     <MemoryRouter initialEntries={[routePathParam]}>
       <Route path={routePath}>
@@ -142,7 +158,7 @@ describe("when an error exists", () => {
     const wrapper = mountWrapper(
       getStore({
         ...defaultStore,
-        apps: { ...state.apps },
+        ...state,
       }),
       <MemoryRouter initialEntries={[routePathParam]}>
         <Route path={routePath}>
@@ -168,8 +184,7 @@ describe("when an error exists", () => {
     const wrapper = mountWrapper(
       getStore({
         ...defaultStore,
-        repos: { ...state.repos },
-        apps: { ...state.apps },
+        ...state,
       }),
       <MemoryRouter initialEntries={[routePathParam]}>
         <Route path={routePath}>
@@ -189,12 +204,15 @@ describe("when an error exists", () => {
       apps: {
         error: upgradeError,
         selected: installedPackage1,
-      } as IAppState,
+        selectedDetails: installedPackageDetail,
+      } as unknown as IAppState,
+      packages: { selected: selectedPackage } as IPackageState,
     };
+
     const wrapper = mountWrapper(
       getStore({
         ...defaultStore,
-        apps: { ...state.apps },
+        ...state,
       }),
       <MemoryRouter initialEntries={[routePathParam]}>
         <Route path={routePath}>
@@ -203,26 +221,27 @@ describe("when an error exists", () => {
       </MemoryRouter>,
     );
     expect(wrapper.find(UpgradeForm)).toExist();
-    expect(wrapper.find(UpgradeForm).prop("error")).toEqual(upgradeError);
+    expect(wrapper.find(UpgradeForm).find(Alert)).toIncludeText(upgradeError.message);
   });
 });
 
 it("renders the upgrade form when the repo is available", () => {
   const state = {
     apps: {
       selected: installedPackage1,
-    } as IAppState,
+      selectedDetails: installedPackageDetail,
+    } as unknown as IAppState,
     repos: {
       repo: repo1,
       repos: [repo1],
       isFetching: false,
     } as IAppRepositoryState,
+    packages: { selected: selectedPackage } as IPackageState,
   };
   const wrapper = mountWrapper(
     getStore({
       ...defaultStore,
-      apps: { ...state.apps },
-      repos: { ...state.repos },
+      ...state,
     }),
     <MemoryRouter initialEntries={[routePathParam]}>
       <Route path={routePath}>
@@ -239,18 +258,19 @@ it("skips the repo selection form if the app contains upgrade info", () => {
   const state = {
     apps: {
       selected: installedPackage1,
-    } as IAppState,
+      selectedDetails: installedPackageDetail,
+    } as unknown as IAppState,
     repos: {
       repo: repo1,
       repos: [repo1],
       isFetching: false,
     } as IAppRepositoryState,
+    packages: { selected: selectedPackage } as IPackageState,
   };
   const wrapper = mountWrapper(
     getStore({
       ...defaultStore,
-      apps: { ...state.apps },
-      repos: { ...state.repos },
+      ...state,
     }),
     <MemoryRouter initialEntries={[routePathParam]}>
       <Route path={routePath}>
@@ -262,78 +282,3 @@ it("skips the repo selection form if the app contains upgrade info", () => {
   expect(wrapper.find(Alert)).not.toExist();
   expect(wrapper.find(SelectRepoForm)).not.toExist();
 });
-
-describe("when receiving new props", () => {
-  it("should request the deployed package when the app and repo are populated", () => {
-    const fetchDeployedAvailablePackageDetail = jest.fn();
-    actions.packages.fetchDeployedAvailablePackageDetail = fetchDeployedAvailablePackageDetail;
-
-    const state = {
-      apps: {
-        selected: installedPackage1,
-      } as IAppState,
-      repos: {
-        repo: repo1,
-        repos: [repo1],
-        isFetching: false,
-      } as IAppRepositoryState,
-    };
-    mountWrapper(
-      getStore({
-        ...defaultStore,
-        apps: { ...state.apps },
-        repos: { ...state.repos },
-      }),
-      <MemoryRouter initialEntries={[routePathParam]}>
-        <Route path={routePath}>
-          <AppUpgrade />,
-        </Route>
-      </MemoryRouter>,
-    );
-
-    expect(fetchDeployedAvailablePackageDetail).toHaveBeenCalledWith(
-      {
-        context: { cluster: defaultProps.cluster, namespace: defaultProps.repoNamespace },
-        identifier: "stable/bar",
-        plugin: defaultProps.plugin,
-      } as AvailablePackageReference,
-      "1.0.0",
-    );
-  });
-
-  it("should request the deployed package when the repo is populated later", () => {
-    const fetchDeployedAvailablePackageDetail = jest.fn();
-    actions.packages.fetchDeployedAvailablePackageDetail = fetchDeployedAvailablePackageDetail;
-
-    const state = {
-      apps: {
-        selected: installedPackage1,
-      } as IAppState,
-      repos: {
-        repo: repo1,
-        repos: [repo1],
-        isFetching: false,
-      } as IAppRepositoryState,
-    };
-    mountWrapper(
-      getStore({
-        ...defaultStore,
-        apps: { ...state.apps },
-        repos: { ...state.repos },
-      }),
-      <MemoryRouter initialEntries={[routePathParam]}>
-        <Route path={routePath}>
-          <AppUpgrade />,
-        </Route>
-      </MemoryRouter>,
-    );
-    expect(fetchDeployedAvailablePackageDetail).toHaveBeenCalledWith(
-      {
-        context: { cluster: defaultProps.cluster, namespace: defaultProps.repoNamespace },
-        identifier: "stable/bar",
-        plugin: defaultProps.plugin,
-      } as AvailablePackageReference,
-      "1.0.0",
-    );
-  });
-});