Review comments

Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
This commit is contained in:
Oliver Sand
2021-09-20 12:42:31 +02:00
parent aec9385859
commit 075808940c
26 changed files with 107 additions and 83 deletions
-8
View File
@@ -1,8 +0,0 @@
---
'@backstage/plugin-catalog-graph': patch
---
Add new plugin `@backstage/plugin-catalog-graph`. The catalog graph visualizes
the relations between entities, like ownership, grouping or API relationships.
For more details on adding the plugin to your Backstage instance, [see the README](https://github.com/backstage/backstage/blob/master/plugins/catalog-graph/README.md).
@@ -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" />
+7 -7
View File
@@ -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][];
```
+3 -1
View File
@@ -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",
@@ -56,6 +56,7 @@ describe('<CatalogGraphCard/>', () => {
getLocationByEntity: jest.fn(),
addLocation: jest.fn(),
removeLocationById: jest.fn(),
refreshEntity: jest.fn(),
};
apis = ApiRegistry.with(catalogApiRef, catalog);
@@ -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,7 +67,7 @@ export const CatalogGraphCard = ({
kinds?: string[];
relations?: string[];
direction?: Direction;
maxHeight?: number;
height?: number;
title?: string;
}) => {
const { entity } = useEntity();
@@ -75,14 +75,14 @@ export const CatalogGraphCard = ({
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);
@@ -86,6 +86,7 @@ describe('<CatalogGraphPage/>', () => {
getLocationByEntity: jest.fn(),
addLocation: jest.fn(),
removeLocationById: jest.fn(),
refreshEntity: jest.fn(),
};
const apis = ApiRegistry.with(catalogApiRef, catalog);
@@ -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);
@@ -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={() => {}} />,
);
@@ -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],
);
@@ -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}
/>,
@@ -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[]) => {
@@ -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({}));
@@ -104,7 +104,7 @@ export function useCatalogGraphPage({
(Array.isArray(query.selectedKinds)
? query.selectedKinds
: initialState?.selectedKinds
)?.map(k => k.toLowerCase()),
)?.map(k => k.toLocaleLowerCase('en-US')),
);
const [unidirectional, setUnidirectional] = useState<boolean>(() =>
typeof query.unidirectional === 'string'
@@ -40,7 +40,7 @@ describe('<EntityRelationsGraph/>', () => {
});
beforeEach(() => {
const entities: { [key: string]: Entity } = {
const entities: { [ref: string]: Entity } = {
'b:d/c': {
apiVersion: 'a',
kind: 'b',
@@ -155,6 +155,7 @@ describe('<EntityRelationsGraph/>', () => {
getLocationByEntity: jest.fn(),
addLocation: jest.fn(),
removeLocationById: jest.fn(),
refreshEntity: jest.fn(),
};
const apis = ApiRegistry.with(catalogApiRef, catalog);
@@ -24,7 +24,7 @@ import classNames from 'classnames';
import React, { MouseEvent, useEffect, useMemo } from 'react';
import { CustomLabel } from './CustomLabel';
import { CustomNode } from './CustomNode';
import { RelationPairs, RELATION_PAIRS } from './relations';
import { RelationPairs, ALL_RELATION_PAIRS } from './relations';
import { Direction, EntityNode } from './types';
import { useEntityRelationNodesAndEdges } from './useEntityRelationNodesAndEdges';
@@ -73,7 +73,7 @@ export const EntityRelationsGraph = ({
relations,
direction = Direction.LEFT_RIGHT,
onNodeClick,
relationPairs = RELATION_PAIRS,
relationPairs = ALL_RELATION_PAIRS,
className,
}: {
rootEntityNames: EntityName | EntityName[];
@@ -83,7 +83,7 @@ export const EntityRelationsGraph = ({
kinds?: string[];
relations?: string[];
direction?: Direction;
onNodeClick?: (value: EntityNode, event: MouseEvent<SVGElement>) => void;
onNodeClick?: (value: EntityNode, event: MouseEvent<unknown>) => void;
relationPairs?: RelationPairs;
className?: string;
}) => {
@@ -14,7 +14,7 @@
* limitations under the License.
*/
export { EntityRelationsGraph } from './EntityRelationsGraph';
export { RELATION_PAIRS } from './relations';
export { ALL_RELATION_PAIRS } from './relations';
export type { RelationPairs } from './relations';
export { Direction } from './types';
export type { EntityEdge, EntityNode } from './types';
@@ -49,7 +49,7 @@ export type RelationPairs = [string, string][];
*
* @public
*/
export const RELATION_PAIRS: RelationPairs = [
export const ALL_RELATION_PAIRS: RelationPairs = [
[RELATION_OWNER_OF, RELATION_OWNED_BY],
[RELATION_CONSUMES_API, RELATION_API_CONSUMED_BY],
[RELATION_API_PROVIDED_BY, RELATION_PROVIDES_API],
@@ -67,7 +67,7 @@ export type EntityNode = DependencyGraphTypes.DependencyNode<{
/**
* Optional click handler.
*/
onClick?: MouseEventHandler<SVGGElement>;
onClick?: MouseEventHandler<unknown>;
}>;
export type GraphEdge = DependencyGraphTypes.GraphEdge<EntityEdge>;
@@ -26,7 +26,9 @@ import { useEntityStore as useEntityStoreMocked } from './useEntityStore';
jest.mock('./useEntityStore');
const useEntityStore = useEntityStoreMocked as jest.Mock;
const useEntityStore = useEntityStoreMocked as jest.Mock<
ReturnType<typeof useEntityStoreMocked>
>;
describe('useEntityRelationGraph', () => {
const requestEntities = jest.fn();
@@ -33,7 +33,7 @@ export function useEntityRelationGraph({
kinds?: string[];
};
}): {
entities?: { [key: string]: Entity };
entities?: { [ref: string]: Entity };
loading: boolean;
error?: Error;
} {
@@ -63,7 +63,8 @@ export function useEntityRelationGraph({
for (const rel of entity.relations) {
if (
(!relations || relations.includes(rel.type)) &&
(!kinds || kinds.includes(rel.target.kind.toLowerCase()))
(!kinds ||
kinds.includes(rel.target.kind.toLocaleLowerCase('en-US')))
) {
const relationEntityRef = stringifyEntityRef(rel.target);
@@ -28,11 +28,13 @@ import { useEntityRelationNodesAndEdges } from './useEntityRelationNodesAndEdges
jest.mock('./useEntityRelationGraph');
const useEntityRelationGraph = useEntityRelationGraphMocked as jest.Mock;
const useEntityRelationGraph = useEntityRelationGraphMocked as jest.Mock<
ReturnType<typeof useEntityRelationGraphMocked>
>;
describe('useEntityRelationNodesAndEdges', () => {
beforeEach(() => {
const entities: { [key: string]: Entity } = {
const entities: { [ref: string]: Entity } = {
'b:d/c': {
apiVersion: 'a',
kind: 'b',
@@ -19,7 +19,7 @@ import {
} from '@backstage/catalog-model';
import { MouseEvent, useState } from 'react';
import { useDebounce } from 'react-use';
import { RelationPairs, RELATION_PAIRS } from './relations';
import { RelationPairs, ALL_RELATION_PAIRS } from './relations';
import { EntityEdge, EntityNode } from './types';
import { useEntityRelationGraph } from './useEntityRelationGraph';
@@ -34,7 +34,7 @@ export function useEntityRelationNodesAndEdges({
kinds,
relations,
onNodeClick,
relationPairs = RELATION_PAIRS,
relationPairs = ALL_RELATION_PAIRS,
}: {
rootEntityRefs: string[];
maxDepth?: number;
@@ -42,7 +42,7 @@ export function useEntityRelationNodesAndEdges({
mergeRelations?: boolean;
kinds?: string[];
relations?: string[];
onNodeClick?: (value: EntityNode, event: MouseEvent<SVGElement>) => void;
onNodeClick?: (value: EntityNode, event: MouseEvent<unknown>) => void;
relationPairs?: RelationPairs;
}): {
loading: boolean;
@@ -112,7 +112,10 @@ export function useEntityRelationNodesAndEdges({
return;
}
if (kinds && !kinds.includes(rel.target.kind.toLowerCase())) {
if (
kinds &&
!kinds.includes(rel.target.kind.toLocaleLowerCase('en-US'))
) {
return;
}
@@ -14,13 +14,15 @@
* limitations under the License.
*/
import { Entity } from '@backstage/catalog-model';
import { useApi } from '@backstage/core-plugin-api';
import { useApi as useApiMocked } from '@backstage/core-plugin-api';
import { CatalogApi } from '@backstage/plugin-catalog-react';
import { act, renderHook } from '@testing-library/react-hooks';
import { useEntityStore } from './useEntityStore';
jest.mock('@backstage/core-plugin-api');
const useApi = useApiMocked as jest.Mocked<any>;
describe('useEntityStore', () => {
let catalogApi: jest.Mocked<CatalogApi>;
@@ -34,9 +36,10 @@ describe('useEntityStore', () => {
getLocationByEntity: jest.fn(),
addLocation: jest.fn(),
removeLocationById: jest.fn(),
refreshEntity: jest.fn(),
};
(useApi as jest.Mock<any>).mockReturnValue(catalogApi);
useApi.mockReturnValue(catalogApi);
});
afterEach(() => jest.resetAllMocks());
@@ -23,11 +23,13 @@ import { useAsyncFn } from 'react-use';
// TODO: This is a good use case for a graphql API, once it is available in the
// future.
const limiter = limiterFactory(10);
/**
* Ensures that a set of requested entities is loaded.
*/
export function useEntityStore(): {
entities: { [key: string]: Entity };
entities: { [ref: string]: Entity };
loading: boolean;
error?: Error;
requestEntities: Dispatch<string[]>;
@@ -39,12 +41,12 @@ export function useEntityStore(): {
cachedEntities: new Map<string, Entity>(),
});
const [entities, setEntities] = useState<{
[key: string]: Entity;
[ref: string]: Entity;
}>({});
const updateEntities = useCallback(() => {
const { cachedEntities, requestedEntities } = state.current;
const filteredEntities: { [key: string]: Entity } = {};
const filteredEntities: { [ref: string]: Entity } = {};
requestedEntities.forEach(entityRef => {
const entity = cachedEntities.get(entityRef);
@@ -56,7 +58,6 @@ export function useEntityStore(): {
}, [state, setEntities]);
const [asyncState, fetch] = useAsyncFn(async () => {
const limiter = limiterFactory(10);
const { requestedEntities, outstandingEntities, cachedEntities } =
state.current;
+1 -1
View File
@@ -21,7 +21,7 @@ import { catalogEntityRouteRef, catalogGraphRouteRef } from './routes';
* @public
*/
export const catalogGraphPlugin = createPlugin({
id: '@internal/catalog-graph',
id: 'catalog-graph',
routes: {
catalogGraph: catalogGraphRouteRef,
},