Review comments

Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
backstage · Sep 20, 2021 · 0758089 · 0758089
1 parent aec9385
commit 0758089
Show file tree

Hide file tree

Showing 26 changed files with 107 additions and 83 deletions.
diff --git a/.changeset/wicked-pugs-speak.md b/.changeset/wicked-pugs-speak.md
diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx
@@ -47,7 +47,6 @@ import {
   EntityOrphanWarning,
   EntityProcessingErrorsPanel,
   EntitySwitch,
-  EntitySystemDiagramCard,
   hasCatalogProcessingErrors,
   isComponentType,
   isKind,
@@ -265,7 +264,7 @@ const overviewContent = (
     </Grid>
 
     <Grid item md={6} xs={12}>
-      <EntityCatalogGraphCard variant="gridItem" maxHeight={400} />
+      <EntityCatalogGraphCard variant="gridItem" height={400} />
     </Grid>
 
     <EntitySwitch>
@@ -476,7 +475,7 @@ const apiPage = (
           <EntityAboutCard />
         </Grid>
         <Grid item md={6} xs={12}>
-          <EntityCatalogGraphCard variant="gridItem" maxHeight={400} />
+          <EntityCatalogGraphCard variant="gridItem" height={400} />
         </Grid>
         <Grid item xs={12}>
           <Grid container>
@@ -545,7 +544,7 @@ const systemPage = (
           <EntityAboutCard variant="gridItem" />
         </Grid>
         <Grid item md={6} xs={12}>
-          <EntityCatalogGraphCard variant="gridItem" maxHeight={400} />
+          <EntityCatalogGraphCard variant="gridItem" height={400} />
         </Grid>
         <Grid item md={6}>
           <EntityHasComponentsCard variant="gridItem" />
@@ -559,14 +558,11 @@ const systemPage = (
       </Grid>
     </EntityLayout.Route>
     <EntityLayout.Route path="/diagram" title="Diagram">
-      <EntitySystemDiagramCard />
-    </EntityLayout.Route>
-    <EntityLayout.Route path="/relations" title="Diagram (new)">
       <EntityCatalogGraphCard
         variant="gridItem"
         direction={Direction.TOP_BOTTOM}
         title="System Diagram"
-        maxHeight={700}
+        height={700}
         relations={[
           RELATION_PART_OF,
           RELATION_HAS_PART,
@@ -592,7 +588,7 @@ const domainPage = (
           <EntityAboutCard variant="gridItem" />
         </Grid>
         <Grid item md={6} xs={12}>
-          <EntityCatalogGraphCard variant="gridItem" maxHeight={400} />
+          <EntityCatalogGraphCard variant="gridItem" height={400} />
         </Grid>
         <Grid item md={6}>
           <EntityHasSystemsCard variant="gridItem" />

diff --git a/plugins/catalog-graph/api-report.md b/plugins/catalog-graph/api-report.md
@@ -14,6 +14,9 @@ import { MouseEvent as MouseEvent_2 } from 'react';
 import { MouseEventHandler } from 'react';
 import { RouteRef } from '@backstage/core-plugin-api';
 
+// @public
+export const ALL_RELATION_PAIRS: RelationPairs;
+
 // @public
 export const CatalogGraphPage: ({
   relationPairs,
@@ -72,7 +75,7 @@ export const EntityCatalogGraphCard: ({
   kinds,
   relations,
   direction,
-  maxHeight,
+  height,
   title,
 }: {
   variant?: InfoCardVariants | undefined;
@@ -83,7 +86,7 @@ export const EntityCatalogGraphCard: ({
   kinds?: string[] | undefined;
   relations?: string[] | undefined;
   direction?: Direction | undefined;
-  maxHeight?: number | undefined;
+  height?: number | undefined;
   title?: string | undefined;
 }) => JSX.Element;
 
@@ -101,7 +104,7 @@ export type EntityNode = DependencyGraphTypes.DependencyNode<{
   namespace: string;
   focused?: boolean;
   color?: 'primary' | 'secondary' | 'default';
-  onClick?: MouseEventHandler<SVGGElement>;
+  onClick?: MouseEventHandler<unknown>;
 }>;
 
 // @public
@@ -125,15 +128,12 @@ export const EntityRelationsGraph: ({
   relations?: string[] | undefined;
   direction?: Direction | undefined;
   onNodeClick?:
-    | ((value: EntityNode, event: MouseEvent_2<SVGElement>) => void)
+    | ((value: EntityNode, event: MouseEvent_2<unknown>) => void)
     | undefined;
   relationPairs?: RelationPairs | undefined;
   className?: string | undefined;
 }) => JSX.Element;
 
-// @public
-export const RELATION_PAIRS: RelationPairs;
-
 // @public
 export type RelationPairs = [string, string][];
 ```
diff --git a/plugins/catalog-graph/package.json b/plugins/catalog-graph/package.json
@@ -3,7 +3,8 @@
   "version": "0.1.0",
   "main": "src/index.ts",
   "types": "src/index.ts",
-  "private": true,
+  "license": "Apache-2.0",
+  "private": false,
   "publishConfig": {
     "access": "public",
     "main": "dist/index.esm.js",
@@ -29,6 +30,7 @@
     "@material-ui/core": "^4.12.2",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.57",
+    "@types/react": "*",
     "react": "^16.13.1",
     "react-dom": "^16.13.1",
     "react-use": "^17.2.4",

diff --git a/plugins/catalog-graph/src/components/CatalogGraphCard/CatalogGraphCard.test.tsx b/plugins/catalog-graph/src/components/CatalogGraphCard/CatalogGraphCard.test.tsx
@@ -56,6 +56,7 @@ describe('<CatalogGraphCard/>', () => {
       getLocationByEntity: jest.fn(),
       addLocation: jest.fn(),
       removeLocationById: jest.fn(),
+      refreshEntity: jest.fn(),
     };
     apis = ApiRegistry.with(catalogApiRef, catalog);
 

diff --git a/plugins/catalog-graph/src/components/CatalogGraphCard/CatalogGraphCard.tsx b/plugins/catalog-graph/src/components/CatalogGraphCard/CatalogGraphCard.tsx
@@ -31,15 +31,15 @@ import {
   EntityNode,
   EntityRelationsGraph,
   RelationPairs,
-  RELATION_PAIRS,
+  ALL_RELATION_PAIRS,
 } from '../EntityRelationsGraph';
 
-const useStyles = makeStyles<Theme, { maxHeight: number | undefined }>({
-  card: ({ maxHeight }) => ({
+const useStyles = makeStyles<Theme, { height: number | undefined }>({
+  card: ({ height }) => ({
     display: 'flex',
     flexDirection: 'column',
-    maxHeight,
-    minHeight: 0,
+    maxHeight: height,
+    minHeight: height,
   }),
   graph: {
     flex: 1,
@@ -49,14 +49,14 @@ const useStyles = makeStyles<Theme, { maxHeight: number | undefined }>({
 
 export const CatalogGraphCard = ({
   variant = 'gridItem',
-  relationPairs = RELATION_PAIRS,
+  relationPairs = ALL_RELATION_PAIRS,
   maxDepth = 1,
   unidirectional = true,
   mergeRelations = true,
   kinds,
   relations,
   direction = Direction.LEFT_RIGHT,
-  maxHeight,
+  height,
   title = 'Relations',
 }: {
   variant?: InfoCardVariants;
@@ -67,22 +67,22 @@ export const CatalogGraphCard = ({
   kinds?: string[];
   relations?: string[];
   direction?: Direction;
-  maxHeight?: number;
+  height?: number;
   title?: string;
 }) => {
   const { entity } = useEntity();
   const entityName = getEntityName(entity);
   const catalogEntityRoute = useRouteRef(catalogEntityRouteRef);
   const catalogGraphRoute = useRouteRef(catalogGraphRouteRef);
   const navigate = useNavigate();
-  const classes = useStyles({ maxHeight });
+  const classes = useStyles({ height });
 
   const onNodeClick = useCallback(
-    (node: EntityNode, _: MouseEvent) => {
+    (node: EntityNode, _: MouseEvent<unknown>) => {
       const nodeEntityName = parseEntityRef(node.id);
       const path = catalogEntityRoute({
-        kind: nodeEntityName.kind.toLowerCase(),
-        namespace: nodeEntityName.namespace.toLowerCase(),
+        kind: nodeEntityName.kind.toLocaleLowerCase('en-US'),
+        namespace: nodeEntityName.namespace.toLocaleLowerCase('en-US'),
         name: nodeEntityName.name,
       });
       navigate(path);

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/CatalogGraphPage.test.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/CatalogGraphPage.test.tsx
@@ -86,6 +86,7 @@ describe('<CatalogGraphPage/>', () => {
       getLocationByEntity: jest.fn(),
       addLocation: jest.fn(),
       removeLocationById: jest.fn(),
+      refreshEntity: jest.fn(),
     };
     const apis = ApiRegistry.with(catalogApiRef, catalog);
 

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/CatalogGraphPage.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/CatalogGraphPage.tsx
@@ -35,7 +35,7 @@ import {
   EntityNode,
   EntityRelationsGraph,
   RelationPairs,
-  RELATION_PAIRS,
+  ALL_RELATION_PAIRS,
 } from '../EntityRelationsGraph';
 import { DirectionFilter } from './DirectionFilter';
 import { MaxDepthFilter } from './MaxDepthFilter';
@@ -97,7 +97,7 @@ const useStyles = makeStyles(theme => ({
 }));
 
 export const CatalogGraphPage = ({
-  relationPairs = RELATION_PAIRS,
+  relationPairs = ALL_RELATION_PAIRS,
   initialState,
 }: {
   relationPairs?: RelationPairs;
@@ -134,13 +134,13 @@ export const CatalogGraphPage = ({
     toggleShowFilters,
   } = useCatalogGraphPage({ initialState });
   const onNodeClick = useCallback(
-    (node: EntityNode, event: MouseEvent) => {
+    (node: EntityNode, event: MouseEvent<unknown>) => {
       const nodeEntityName = parseEntityRef(node.id);
 
       if (event.shiftKey) {
         const path = catalogEntityRoute({
-          kind: nodeEntityName.kind.toLowerCase(),
-          namespace: nodeEntityName.namespace.toLowerCase(),
+          kind: nodeEntityName.kind.toLocaleLowerCase('en-US'),
+          namespace: nodeEntityName.namespace.toLocaleLowerCase('en-US'),
           name: nodeEntityName.name,
         });
         navigate(path);

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedKindsFilter.test.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedKindsFilter.test.tsx
@@ -23,21 +23,29 @@ import { SelectedKindsFilter } from './SelectedKindsFilter';
 jest.mock('@backstage/core-plugin-api');
 jest.mock('@backstage/plugin-catalog-react');
 
-const useApi = useApiMocked as jest.Mock;
-const useEntityKinds = useEntityKindsMocked as jest.Mock;
+const useApi = useApiMocked as jest.Mock<ReturnType<typeof useApiMocked>>;
+const useEntityKinds = useEntityKindsMocked as jest.Mock<
+  ReturnType<typeof useEntityKindsMocked>
+>;
 
 describe('<SelectedKindsFilter/>', () => {
   beforeEach(() => {
     useApi.mockReturnValue({});
     useEntityKinds.mockReturnValue({
+      loading: false,
       kinds: ['API', 'Component', 'System', 'Domain', 'Resource'],
+      error: undefined,
     });
   });
 
   afterEach(() => jest.resetAllMocks());
 
   test('should not explode while loading', () => {
-    useEntityKinds.mockReturnValue({});
+    useEntityKinds.mockReturnValue({
+      loading: true,
+      kinds: undefined,
+      error: undefined,
+    });
     const { baseElement } = render(
       <SelectedKindsFilter value={['api', 'component']} onChange={() => {}} />,
     );

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedKindsFilter.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedKindsFilter.tsx
@@ -55,7 +55,7 @@ export const SelectedKindsFilter = ({ value, onChange }: Props) => {
   }, [error, alertApi]);
 
   const normalizedKinds = useMemo(
-    () => (kinds ? kinds.map(k => k.toLowerCase()) : kinds),
+    () => (kinds ? kinds.map(k => k.toLocaleLowerCase('en-US')) : kinds),
     [kinds],
   );
 

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedRelationsFilter.test.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedRelationsFilter.test.tsx
@@ -21,14 +21,14 @@ import {
 import { render, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
-import { RELATION_PAIRS } from '../EntityRelationsGraph';
+import { ALL_RELATION_PAIRS } from '../EntityRelationsGraph';
 import { SelectedRelationsFilter } from './SelectedRelationsFilter';
 
 describe('<SelectedRelationsFilter/>', () => {
   test('should render current value', () => {
     const { getByText } = render(
       <SelectedRelationsFilter
-        relationPairs={RELATION_PAIRS}
+        relationPairs={ALL_RELATION_PAIRS}
         value={[RELATION_OWNED_BY, RELATION_CHILD_OF]}
         onChange={() => {}}
       />,
@@ -42,7 +42,7 @@ describe('<SelectedRelationsFilter/>', () => {
     const onChange = jest.fn();
     const { getByText, getByLabelText } = render(
       <SelectedRelationsFilter
-        relationPairs={RELATION_PAIRS}
+        relationPairs={ALL_RELATION_PAIRS}
         value={[RELATION_OWNED_BY, RELATION_CHILD_OF]}
         onChange={onChange}
       />,
@@ -69,8 +69,8 @@ describe('<SelectedRelationsFilter/>', () => {
     const onChange = jest.fn();
     const { getByText, getByLabelText } = render(
       <SelectedRelationsFilter
-        relationPairs={RELATION_PAIRS}
-        value={RELATION_PAIRS.flatMap(p => p).filter(
+        relationPairs={ALL_RELATION_PAIRS}
+        value={ALL_RELATION_PAIRS.flatMap(p => p).filter(
           r => r !== RELATION_HAS_MEMBER,
         )}
         onChange={onChange}
@@ -94,7 +94,7 @@ describe('<SelectedRelationsFilter/>', () => {
     const onChange = jest.fn();
     const { getByRole } = render(
       <SelectedRelationsFilter
-        relationPairs={RELATION_PAIRS}
+        relationPairs={ALL_RELATION_PAIRS}
         value={[]}
         onChange={onChange}
       />,

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedRelationsFilter.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/SelectedRelationsFilter.tsx
@@ -46,10 +46,7 @@ export const SelectedRelationsFilter = ({
   onChange,
 }: Props) => {
   const classes = useStyles();
-  const relations = useMemo(
-    () => relationPairs.flatMap(r => r),
-    [relationPairs],
-  );
+  const relations = useMemo(() => relationPairs.flat(), [relationPairs]);
 
   const handleChange = useCallback(
     (_: unknown, v: string[]) => {

diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.ts b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.ts
@@ -26,14 +26,24 @@ jest.mock('react-router', () => ({
 jest.spyOn(window.history, 'replaceState');
 jest.spyOn(window.history, 'pushState');
 
-const useLocation = useLocationMocked as jest.Mock;
-const windowHistoryReplaceState = window.history.replaceState as jest.Mock;
-const windowHistoryPushState = window.history.replaceState as jest.Mock;
+const useLocation = useLocationMocked as jest.Mock<
+  ReturnType<typeof useLocationMocked>
+>;
+const windowHistoryReplaceState = window.history.replaceState as jest.Mock<
+  ReturnType<typeof window.history.replaceState>
+>;
+const windowHistoryPushState = window.history.pushState as jest.Mock<
+  ReturnType<typeof window.history.pushState>
+>;
 
 describe('useCatalogGraphPage', () => {
   beforeEach(() => {
     useLocation.mockReturnValue({
       search: '?',
+      state: {},
+      key: '',
+      pathname: '',
+      hash: '',
     });
   });
 
@@ -71,6 +81,10 @@ describe('useCatalogGraphPage', () => {
     useLocation.mockReturnValueOnce({
       search:
         '?rootEntityRefs[]=b:d/c&maxDepth=2&direction=RL&mergeRelations=false&unidirectional=false&showFilters=false&selectedKinds[]=api&selectedRelations[]=memberOf',
+      state: {},
+      key: '',
+      pathname: '',
+      hash: '',
     });
 
     const { result } = renderHook(() => useCatalogGraphPage({}));