Use two separate types for IChartCatalogItem and IOperatorCatalogItem. (

#1806) It's not perfect - there's more that could be DRY'd up and encapsulated, but with this change you can actually use TypeScript's type system to ensure the correct fields are present. Fixes #1803
vmware-tanzu · Jun 22, 2020 · 99225a5 · 99225a5
1 parent ad182b5
commit 99225a5
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 72 deletions.
diff --git a/dashboard/src/components/Catalog/Catalog.test.tsx b/dashboard/src/components/Catalog/Catalog.test.tsx
@@ -200,7 +200,6 @@ describe("renderization", () => {
           name: "bar",
           namespace: "chart-namespace",
         },
-        type: "chart",
         version: "v2.0.0",
       };
       const expectedItem2 = {
@@ -212,7 +211,6 @@ describe("renderization", () => {
           name: "foo",
           namespace: "chart-namespace",
         },
-        type: "chart",
         version: "v1.0.0",
       };
       expect(
@@ -248,7 +246,6 @@ describe("renderization", () => {
           name: "foo",
           namespace: "chart-namespace",
         },
-        type: "chart",
         version: "v1.0.0",
       };
       expect(
@@ -294,7 +291,6 @@ describe("renderization", () => {
           id: "foo-cluster",
           name: "foo-cluster",
           namespace: "kubeapps",
-          type: "operator",
           version: "v1.0.0",
         };
         const csvCard = cardGrid

diff --git a/dashboard/src/components/Catalog/Catalog.tsx b/dashboard/src/components/Catalog/Catalog.tsx
@@ -9,7 +9,7 @@ import { MessageAlert } from "../ErrorAlert";
 import LoadingWrapper from "../LoadingWrapper";
 import PageHeader from "../PageHeader";
 import SearchFilter from "../SearchFilter";
-import CatalogItem, { ICatalogItem } from "./CatalogItem";
+import CatalogItem, { ICatalogItemProps, IChartCatalogItem, IOperatorCatalogItem } from "./CatalogItem";
 
 interface ICatalogProps {
   charts: IChartState;
@@ -102,9 +102,10 @@ class Catalog extends React.Component<ICatalogProps, ICatalogState> {
     const filteredCharts = this.filteredCharts(allItems);
     const filteredCSVs = this.shouldRenderOperators() ? this.filteredCSVs(csvs) : [];
     const catalogItems = this.getCatalogItems(filteredCharts, filteredCSVs);
-    const items = catalogItems.map(c => (
-      <CatalogItem key={`${c.type}/${c.repo?.name || c.csv}/${c.name}`} item={c} />
-    ));
+    const items = catalogItems.map(c => {
+      const keyComponent = c.type === "operator" ? (c.item as IOperatorCatalogItem).csv : (c.item as IChartCatalogItem).repo.name;
+      return <CatalogItem type={c.type} key={`${c.type}/${keyComponent}/${c.item.name}`} item={c.item} />
+    });
     return (
       <section className="Catalog">
         <PageHeader>
@@ -169,35 +170,39 @@ class Catalog extends React.Component<ICatalogProps, ICatalogState> {
     return csvs.filter(c => new RegExp(escapeRegExp(filter), "i").test(c.metadata.name));
   }
 
-  private getCatalogItems(charts: IChart[], csvs: IClusterServiceVersion[]): ICatalogItem[] {
-    let result: ICatalogItem[] = [];
+  private getCatalogItems(charts: IChart[], csvs: IClusterServiceVersion[]): ICatalogItemProps[] {
+    let result: ICatalogItemProps[] = [];
     charts.forEach(c => {
       result = result.concat({
-        id: c.id,
-        name: c.attributes.name,
-        icon: c.attributes.icon ? `api/assetsvc/${c.attributes.icon}` : undefined,
-        version: c.relationships.latestChartVersion.data.app_version,
-        description: c.attributes.description,
         type: "chart",
-        repo: c.attributes.repo,
-        namespace: this.props.namespace,
+        item: {
+          id: c.id,
+          name: c.attributes.name,
+          icon: c.attributes.icon ? `api/assetsvc/${c.attributes.icon}` : undefined,
+          version: c.relationships.latestChartVersion.data.app_version,
+          description: c.attributes.description,
+          repo: c.attributes.repo,
+          namespace: this.props.namespace,
+        },
       });
     });
     csvs.forEach(csv => {
       csv.spec.customresourcedefinitions.owned.forEach(crd => {
         result = result.concat({
-          id: crd.name,
-          name: crd.displayName,
-          icon: `data:${csv.spec.icon[0].mediatype};base64,${csv.spec.icon[0].base64data}`,
-          version: crd.version,
-          description: crd.description,
           type: "operator",
-          csv: csv.metadata.name,
-          namespace: this.props.namespace,
+          item: {
+            id: crd.name,
+            name: crd.displayName,
+            icon: `data:${csv.spec.icon[0].mediatype};base64,${csv.spec.icon[0].base64data}`,
+            version: crd.version,
+            description: crd.description,
+            csv: csv.metadata.name,
+            namespace: this.props.namespace,
+          },
         });
       });
     });
-    return result.sort((a, b) => (a.name.toLowerCase() > b.name.toLowerCase() ? 1 : -1));
+    return result.sort((a, b) => (a.item.name.toLowerCase() > b.item.name.toLowerCase() ? 1 : -1));
   }
 
   private toggleListCharts = () => {

diff --git a/dashboard/src/components/Catalog/CatalogItem.test.tsx b/dashboard/src/components/Catalog/CatalogItem.test.tsx
@@ -6,7 +6,7 @@ import * as React from "react";
 import { IRepo } from "../../shared/types";
 import { CardIcon } from "../Card";
 import InfoCard from "../InfoCard";
-import CatalogItem, { ICatalogItem } from "./CatalogItem";
+import CatalogItem, { IChartCatalogItem, IOperatorCatalogItem } from "./CatalogItem";
 
 jest.mock("../../placeholder.png", () => "placeholder.png");
 
@@ -22,10 +22,10 @@ const defaultItem = {
   } as IRepo,
   namespace: "repo-namespace",
   icon: "icon.png",
-} as ICatalogItem;
+} as IChartCatalogItem;
 
 it("should render a chart item in a namespace", () => {
-  const wrapper = shallow(<CatalogItem item={defaultItem} />);
+  const wrapper = shallow(<CatalogItem item={defaultItem} type="chart" />);
   expect(wrapper).toMatchSnapshot();
 });
 
@@ -37,14 +37,14 @@ it("should render a global chart item in a namespace", () => {
       namespace: "kubeapps",
     } as IRepo,
   };
-  const wrapper = shallow(<CatalogItem item={globalItem} />);
+  const wrapper = shallow(<CatalogItem item={globalItem} type="chart" />);
   expect(wrapper).toMatchSnapshot();
 });
 
 it("should use the default placeholder for the icon if it doesn't exist", () => {
   const chartWithoutIcon = cloneDeep(defaultItem);
   chartWithoutIcon.icon = undefined;
-  const wrapper = shallow(<CatalogItem item={chartWithoutIcon} />);
+  const wrapper = shallow(<CatalogItem item={chartWithoutIcon} type="chart" />);
   // Importing an image returns "undefined"
   expect(
     wrapper
@@ -58,7 +58,7 @@ it("should use the default placeholder for the icon if it doesn't exist", () =>
 it("should place a dash if the version is not avaliable", () => {
   const chartWithoutVersion = cloneDeep(defaultItem);
   chartWithoutVersion.version = "";
-  const wrapper = shallow(<CatalogItem item={chartWithoutVersion} />);
+  const wrapper = shallow(<CatalogItem item={chartWithoutVersion} type="chart" />);
   expect(
     wrapper
       .find(InfoCard)
@@ -71,7 +71,7 @@ it("should place a dash if the version is not avaliable", () => {
 it("show the chart description", () => {
   const chartWithDescription = cloneDeep(defaultItem);
   chartWithDescription.description = "This is a description";
-  const wrapper = shallow(<CatalogItem item={chartWithDescription} />);
+  const wrapper = shallow(<CatalogItem item={chartWithDescription} type="chart" />);
   expect(
     wrapper
       .find(InfoCard)
@@ -86,7 +86,7 @@ context("when the description is too long", () => {
     const chartWithDescription = cloneDeep(defaultItem);
     chartWithDescription.description =
       "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum ultrices velit leo, quis pharetra mi vestibulum quis.";
-    const wrapper = shallow(<CatalogItem item={chartWithDescription} />);
+    const wrapper = shallow(<CatalogItem item={chartWithDescription} type="chart" />);
     expect(
       wrapper
         .find(InfoCard)
@@ -101,18 +101,17 @@ context("when the item is a catalog", () => {
   const catalogItem = {
     ...defaultItem,
     csv: "foo-cluster",
-    type: "operator",
-  } as ICatalogItem;
+  } as IOperatorCatalogItem;
 
   it("shows the proper tag", () => {
-    const wrapper = shallow(<CatalogItem item={catalogItem} />);
+    const wrapper = shallow(<CatalogItem item={catalogItem} type={"operator"} />);
     expect((wrapper.find(InfoCard).prop("tag1Content") as JSX.Element).props.children).toEqual(
       "foo-cluster",
     );
   });
 
   it("has the proper link", () => {
-    const wrapper = shallow(<CatalogItem item={catalogItem} />);
+    const wrapper = shallow(<CatalogItem item={catalogItem} type={"operator"} />);
     expect(wrapper.find(InfoCard).prop("link")).toEqual(
       `/ns/${defaultItem.namespace}/operators-instances/new/foo-cluster/foo1`,
     );

diff --git a/dashboard/src/components/Catalog/CatalogItem.tsx b/dashboard/src/components/Catalog/CatalogItem.tsx
@@ -8,22 +8,26 @@ import * as url from "../../shared/url";
 import InfoCard from "../InfoCard";
 import "./CatalogItem.css";
 
-
-
-interface ICatalogItemProps {
-  item: ICatalogItem;
-}
-
 export interface ICatalogItem {
   id: string;
   name: string;
   version: string;
   description: string;
-  type: "chart" | "operator";
   namespace: string;
   icon?: string;
-  repo?: IRepo;
-  csv?: string;
+}
+
+export interface IChartCatalogItem extends ICatalogItem {
+  repo: IRepo;
+}
+
+export interface IOperatorCatalogItem extends ICatalogItem {
+  csv: string;
+}
+
+export interface ICatalogItemProps {
+  type: string;
+  item: IChartCatalogItem | IOperatorCatalogItem;
 }
 
 // 3 lines description max
@@ -38,32 +42,55 @@ function trimDescription(desc: string): string {
 }
 
 const CatalogItem: React.SFC<ICatalogItemProps> = props => {
-  const { item } = props;
-  const { icon, name, repo, version, description, type, namespace, id, csv } = item;
-  const iconSrc = icon || placeholder;
-  let link;
-  let tag1;
-  let subIcon;
-  if (type === "chart") {
-    tag1 = (
-      // TODO(#1803): See #1803 regarding the work-arounds below for the fact
-      // that repo is required if we're dealing with a chart here.
-      <Link
-        className="ListItem__content__info_tag_link"
-        to={url.app.repo(repo?.name || "", namespace)}
-      >
-        {repo?.name}
-      </Link>
-    );
-    link = url.app.charts.get(name, repo || {} as IRepo, namespace);
-    subIcon = helmIcon;
+  if (props.type === "operator") {
+    const item = props.item as IOperatorCatalogItem;
+    return OperatorCatalogItem(item); 
   } else {
-    // Cosmetic change, remove the version from the csv name
-    const csvName = csv?.split(".v")[0];
-    tag1 = <span>{csvName}</span>;
-    link = `/ns/${namespace}/operators-instances/new/${csv}/${id}`;
-    subIcon = operatorIcon;
+    const item = props.item as IChartCatalogItem;
+    return ChartCatalogItem(item);
   }
+};
+
+const OperatorCatalogItem: React.SFC<IOperatorCatalogItem> = props => {
+  const { icon, name, csv, version, description, namespace, id } = props;
+  const iconSrc = icon || placeholder;
+  // Cosmetic change, remove the version from the csv name
+  const csvName = props.csv.split(".v")[0];
+  const tag1 = <span>{csvName}</span>;
+  const link = `/ns/${namespace}/operators-instances/new/${csv}/${id}`;
+  const subIcon = operatorIcon;
+  const descriptionC = (
+    <div className="ListItem__content__description">{trimDescription(description)}</div>
+  );
+  return (
+    <InfoCard
+      key={id}
+      title={name}
+      link={link}
+      info={version || "-"}
+      icon={iconSrc}
+      description={descriptionC}
+      tag1Content={tag1}
+      tag1Class={""}
+      subIcon={subIcon}
+    />
+  );
+};
+
+const ChartCatalogItem: React.SFC<IChartCatalogItem> = props => {
+  const { icon, name, repo, version, description, namespace, id } = props;
+  const iconSrc = icon || placeholder;
+  const tag1 = (
+    <Link
+      className="ListItem__content__info_tag_link"
+      to={url.app.repo(repo.name, namespace)}
+    >
+      {repo.name}
+    </Link>
+  );
+  const link = url.app.charts.get(name, repo || {} as IRepo, namespace);
+  const subIcon = helmIcon;
+
   const descriptionC = (
     <div className="ListItem__content__description">{trimDescription(description)}</div>
   );

diff --git a/dashboard/src/components/Catalog/__snapshots__/Catalog.test.tsx.snap b/dashboard/src/components/Catalog/__snapshots__/Catalog.test.tsx.snap
@@ -39,11 +39,11 @@ exports[`renderization when charts available should render the list of charts 1`
                   "name": "bar",
                   "namespace": "chart-namespace",
                 },
-                "type": "chart",
                 "version": "v2.0.0",
               }
             }
             key="chart/bar/bar"
+            type="chart"
           />
           <CatalogItem
             item={
@@ -57,11 +57,11 @@ exports[`renderization when charts available should render the list of charts 1`
                   "name": "foo",
                   "namespace": "chart-namespace",
                 },
-                "type": "chart",
                 "version": "v1.0.0",
               }
             }
             key="chart/foo/foo"
+            type="chart"
           />
         </CardGrid>
       </div>