some code review comments addressed

Signed-off-by: Brian Fletcher <brian@roadie.io>
This commit is contained in:
Brian Fletcher
2022-12-07 14:45:47 +00:00
parent f39bec7b58
commit f877061abc
14 changed files with 106 additions and 197 deletions
@@ -21,7 +21,7 @@ spec:
# Intentional no displayName for testing
email: breanna-davison@example.com
picture: https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg?background=%23fff
memberOf: [team-a, team-not-exist]
memberOf: [team-a]
---
apiVersion: backstage.io/v1alpha1
kind: User
@@ -147,6 +147,7 @@ export async function createRouter(
res.status(204).end();
})
.get('/entities/by-name/:kind/:namespace/:name', async (req, res) => {
await new Promise(resolve => setTimeout(resolve, 3000));
const { kind, namespace, name } = req.params;
const { entities } = await entitiesCatalog.entities({
filter: basicEntityFilter({
@@ -14,20 +14,11 @@
* limitations under the License.
*/
import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils';
import { renderInTestApp } from '@backstage/test-utils';
import { screen } from '@testing-library/react';
import React from 'react';
import { entityRouteRef } from '../../routes';
import { EntityRefLink } from './EntityRefLink';
import { catalogApiRef } from '../../api';
import { CatalogApi } from '@backstage/catalog-client';
import { ApiProvider } from '@backstage/core-app-api';
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
const apis = TestApiRegistry.from([catalogApiRef, catalogApi]);
describe('<EntityRefLink />', () => {
it('renders link for entity in default namespace', async () => {
@@ -43,16 +34,11 @@ describe('<EntityRefLink />', () => {
lifecycle: 'production',
},
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entity} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLink entityRef={entity} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText('component:software')).toHaveAttribute(
'href',
@@ -74,16 +60,11 @@ describe('<EntityRefLink />', () => {
lifecycle: 'production',
},
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entity} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLink entityRef={entity} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText('component:test/software')).toHaveAttribute(
'href',
'/catalog/test/component/software',
@@ -105,9 +86,7 @@ describe('<EntityRefLink />', () => {
},
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entity} defaultKind="Component" />
</ApiProvider>,
<EntityRefLink entityRef={entity} defaultKind="Component" />,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
@@ -126,16 +105,11 @@ describe('<EntityRefLink />', () => {
namespace: 'default',
name: 'software',
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entityName} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLink entityRef={entityName} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText('component:software')).toHaveAttribute(
'href',
'/catalog/default/component/software',
@@ -148,16 +122,11 @@ describe('<EntityRefLink />', () => {
namespace: 'test',
name: 'software',
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entityName} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLink entityRef={entityName} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText('component:test/software')).toHaveAttribute(
'href',
'/catalog/test/component/software',
@@ -171,9 +140,7 @@ describe('<EntityRefLink />', () => {
name: 'software',
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entityName} defaultKind="component" />
</ApiProvider>,
<EntityRefLink entityRef={entityName} defaultKind="component" />,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
@@ -193,11 +160,9 @@ describe('<EntityRefLink />', () => {
name: 'software',
};
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLink entityRef={entityName} defaultKind="component">
Custom Children
</EntityRefLink>
</ApiProvider>,
<EntityRefLink entityRef={entityName} defaultKind="component">
Custom Children
</EntityRefLink>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
@@ -25,8 +25,8 @@ import {
import React, { forwardRef } from 'react';
import { entityRouteRef } from '../../routes';
import { humanizeEntityRef } from './humanize';
import { Link, LinkProps } from '@backstage/core-components';
import { useApi, useRouteRef } from '@backstage/core-plugin-api';
import { Link, LinkProps, Progress } from '@backstage/core-components';
import { useApiHolder, useRouteRef } from '@backstage/core-plugin-api';
import {
Button,
Tooltip,
@@ -80,7 +80,7 @@ export const PeekAheadPopover = ({
}: PeekAheadPopoverProps) => {
const entityRoute = useRouteRef(entityRouteRef);
const classes = useStyles();
const catalogApi = useApi(catalogApiRef);
const apiHolder = useApiHolder();
const {
value: entity,
@@ -88,18 +88,17 @@ export const PeekAheadPopover = ({
error,
} = useAsync(async () => {
if (popupState.isOpen) {
const retrievedEntity = await catalogApi.getEntityByRef(entityRef);
if (!retrievedEntity) {
throw new Error(`${entityRef.name} was not found`);
const catalogApi = apiHolder.get(catalogApiRef);
if (catalogApi) {
const retrievedEntity = await catalogApi.getEntityByRef(entityRef);
if (!retrievedEntity) {
throw new Error(`${entityRef.name} was not found`);
}
return retrievedEntity;
}
return retrievedEntity;
}
return undefined;
}, [popupState]);
if (loading) {
return null;
}
}, [popupState.isOpen, apiHolder, entityRef]);
return (
<HoverPopover
@@ -117,12 +116,13 @@ export const PeekAheadPopover = ({
}}
>
<Card>
{loading && <Progress />}
<CardContent>
<Typography gutterBottom>{entityRef.namespace}</Typography>
<Typography color="textSecondary">{entityRef.namespace}</Typography>
<Typography variant="h5" component="div">
{entityRef.name}
</Typography>
<Typography>{entityRef.kind}</Typography>
<Typography color="textSecondary">{entityRef.kind}</Typography>
<Typography variant="body2">
{error && <Alert severity="warning">{error.message}</Alert>}
{entity && (
@@ -201,25 +201,17 @@ export const EntityRefLink = forwardRef<any, EntityRefLinkProps>(
{ defaultKind },
);
const link = (
<Link
{...bindHover(popupState)}
{...linkProps}
ref={ref}
to={entityRoute(routeParams)}
>
{children}
{!children && (title ?? formattedEntityRefTitle)}
</Link>
);
return (
<>
{title ? (
<Tooltip title={formattedEntityRefTitle}>{link}</Tooltip>
) : (
link
)}
<Link
{...bindHover(popupState)}
{...linkProps}
ref={ref}
to={entityRoute(routeParams)}
>
{children}
{!children && (title ?? formattedEntityRefTitle)}
</Link>
<PeekAheadPopover
popupState={popupState}
@@ -14,20 +14,11 @@
* limitations under the License.
*/
import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils';
import { renderInTestApp } from '@backstage/test-utils';
import { screen } from '@testing-library/react';
import React from 'react';
import { entityRouteRef } from '../../routes';
import { EntityRefLinks } from './EntityRefLinks';
import { catalogApiRef } from '../../api';
import { CatalogApi } from '@backstage/catalog-client';
import { ApiProvider } from '@backstage/core-app-api';
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
const apis = TestApiRegistry.from([catalogApiRef, catalogApi]);
describe('<EntityRefLinks />', () => {
it('renders a single link', async () => {
@@ -38,16 +29,11 @@ describe('<EntityRefLinks />', () => {
name: 'software',
},
];
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLinks entityRefs={entityNames} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLinks entityRefs={entityNames} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText('component:software')).toHaveAttribute(
'href',
'/catalog/default/component/software',
@@ -67,16 +53,11 @@ describe('<EntityRefLinks />', () => {
name: 'interface',
},
];
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityRefLinks entityRefs={entityNames} />
</ApiProvider>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
await renderInTestApp(<EntityRefLinks entityRefs={entityNames} />, {
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
},
);
});
expect(screen.getByText(',')).toBeInTheDocument();
expect(screen.getByText('component:software')).toHaveAttribute(
'href',
@@ -20,21 +20,12 @@ import {
RELATION_PART_OF,
SystemEntity,
} from '@backstage/catalog-model';
import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils';
import { renderInTestApp } from '@backstage/test-utils';
import { waitFor, screen } from '@testing-library/react';
import React from 'react';
import { entityRouteRef } from '../../routes';
import { EntityTable } from './EntityTable';
import { componentEntityColumns, systemEntityColumns } from './presets';
import { catalogApiRef } from '../../api';
import { CatalogApi } from '@backstage/catalog-client';
import { ApiProvider } from '@backstage/core-app-api';
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
const apis = TestApiRegistry.from([catalogApiRef, catalogApi]);
describe('systemEntityColumns', () => {
it('shows systems', async () => {
@@ -64,14 +55,12 @@ describe('systemEntityColumns', () => {
];
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityTable
title="My Systems"
entities={entities}
emptyContent={<div>EMPTY</div>}
columns={systemEntityColumns}
/>
</ApiProvider>,
<EntityTable
title="My Systems"
entities={entities}
emptyContent={<div>EMPTY</div>}
columns={systemEntityColumns}
/>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
@@ -118,14 +107,12 @@ describe('componentEntityColumns', () => {
];
await renderInTestApp(
<ApiProvider apis={apis}>
<EntityTable
title="My Components"
entities={entities}
emptyContent={<div>EMPTY</div>}
columns={componentEntityColumns}
/>
</ApiProvider>,
<EntityTable
title="My Components"
entities={entities}
emptyContent={<div>EMPTY</div>}
columns={componentEntityColumns}
/>,
{
mountedRoutes: {
'/catalog/:namespace/:kind/:name/*': entityRouteRef,
+1 -2
View File
@@ -1,6 +1,6 @@
{
"name": "@backstage/plugin-playlist",
"version": "0.1.3-next.1",
"version": "0.1.3-next.2",
"main": "src/index.ts",
"types": "src/index.ts",
"license": "Apache-2.0",
@@ -52,7 +52,6 @@
"react-router-dom": "6.0.0-beta.0 || ^6.3.0"
},
"devDependencies": {
"@backstage/catalog-client": "workspace:^",
"@backstage/cli": "workspace:^",
"@backstage/core-app-api": "workspace:^",
"@backstage/dev-utils": "workspace:^",
@@ -14,19 +14,11 @@
* limitations under the License.
*/
import { entityRouteRef, catalogApiRef } from '@backstage/plugin-catalog-react';
import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils';
import { entityRouteRef } from '@backstage/plugin-catalog-react';
import { renderInTestApp } from '@backstage/test-utils';
import { lightTheme } from '@backstage/theme';
import { ThemeProvider } from '@material-ui/core';
import React from 'react';
import { CatalogApi } from '@backstage/catalog-client';
import { ApiProvider } from '@backstage/core-app-api';
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
const apis = TestApiRegistry.from([catalogApiRef, catalogApi]);
import { rootRouteRef } from '../../routes';
import { PlaylistCard } from './PlaylistCard';
@@ -35,20 +27,18 @@ describe('<PlaylistCard/>', () => {
it('renders playlist info', async () => {
const rendered = await renderInTestApp(
<ThemeProvider theme={lightTheme}>
<ApiProvider apis={apis}>
<PlaylistCard
playlist={{
id: 'id1',
name: 'playlist-1',
description: 'test description',
owner: 'group:default/some-owner',
public: true,
entities: 3,
followers: 2,
isFollowing: false,
}}
/>
</ApiProvider>
<PlaylistCard
playlist={{
id: 'id1',
name: 'playlist-1',
description: 'test description',
owner: 'group:default/some-owner',
public: true,
entities: 3,
followers: 2,
isFollowing: false,
}}
/>
</ThemeProvider>,
{
mountedRoutes: {
@@ -1,6 +1,6 @@
{
"name": "@backstage/plugin-techdocs-addons-test-utils",
"version": "1.0.7-next.1",
"version": "1.0.7-next.2",
"main": "src/index.ts",
"types": "src/index.ts",
"license": "Apache-2.0",
@@ -32,13 +32,11 @@
"postpack": "backstage-cli package postpack"
},
"dependencies": {
"@backstage/catalog-client": "workspace:^",
"@backstage/core-app-api": "workspace:^",
"@backstage/core-components": "workspace:^",
"@backstage/core-plugin-api": "workspace:^",
"@backstage/integration-react": "workspace:^",
"@backstage/plugin-catalog": "workspace:^",
"@backstage/plugin-catalog-react": "workspace:^",
"@backstage/plugin-search-react": "workspace:^",
"@backstage/plugin-techdocs": "workspace:^",
"@backstage/plugin-techdocs-react": "workspace:^",
@@ -22,8 +22,6 @@ import { screen } from 'testing-library__dom';
import { renderToStaticMarkup } from 'react-dom/server';
import { Route } from 'react-router-dom';
import { act, render } from '@testing-library/react';
import { CatalogApi } from '@backstage/catalog-client';
import { catalogApiRef } from '@backstage/plugin-catalog-react';
import { wrapInTestApp, TestApiProvider } from '@backstage/test-utils';
import { FlatRoutes } from '@backstage/core-app-api';
@@ -61,10 +59,6 @@ const scmIntegrationsApi = {
fromConfig: jest.fn().mockReturnValue({}),
};
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
/** @ignore */
type TechDocsAddonTesterTestApiPair<TApi> = TApi extends infer TImpl
? readonly [ApiRef<TApi>, Partial<TImpl>]
@@ -204,7 +198,6 @@ export class TechDocsAddonTester {
[techdocsStorageApiRef, techdocsStorageApi],
[searchApiRef, searchApi],
[scmIntegrationsApiRef, scmIntegrationsApi],
[catalogApiRef, catalogApi],
...this.options.apis,
];
@@ -1,7 +1,7 @@
{
"name": "@backstage/plugin-techdocs-module-addons-contrib",
"description": "Plugin module for contributed TechDocs Addons",
"version": "1.0.7-next.1",
"version": "1.0.7-next.2",
"main": "src/index.ts",
"types": "src/index.ts",
"license": "Apache-2.0",
@@ -42,7 +42,7 @@
"@material-ui/core": "^4.9.13",
"@material-ui/icons": "^4.9.1",
"@material-ui/lab": "4.0.0-alpha.57",
"@react-hookz/web": "^19.0.0",
"@react-hookz/web": "^20.0.0",
"git-url-parse": "^13.0.0",
"react-use": "^17.2.4"
},
@@ -50,7 +50,6 @@
"react": "^16.13.1 || ^17.0.0"
},
"devDependencies": {
"@backstage/catalog-client": "workspace:^",
"@backstage/cli": "workspace:^",
"@backstage/core-app-api": "workspace:^",
"@backstage/dev-utils": "workspace:^",
+1 -2
View File
@@ -1,7 +1,7 @@
{
"name": "@backstage/plugin-techdocs",
"description": "The Backstage plugin that renders technical documentation for your components",
"version": "1.4.1-next.1",
"version": "1.4.1-next.2",
"main": "src/index.ts",
"types": "src/index.ts",
"license": "Apache-2.0",
@@ -65,7 +65,6 @@
"react-router-dom": "6.0.0-beta.0 || ^6.3.0"
},
"devDependencies": {
"@backstage/catalog-client": "workspace:^",
"@backstage/cli": "workspace:^",
"@backstage/core-app-api": "workspace:^",
"@backstage/dev-utils": "workspace:^",
@@ -17,10 +17,9 @@ import React from 'react';
import { act } from '@testing-library/react';
import { ThemeProvider } from '@material-ui/core';
import { scmIntegrationsApiRef } from '@backstage/integration-react';
import { CatalogApi } from '@backstage/catalog-client';
import { lightTheme } from '@backstage/theme';
import { catalogApiRef, entityRouteRef } from '@backstage/plugin-catalog-react';
import { entityRouteRef } from '@backstage/plugin-catalog-react';
import { renderInTestApp, TestApiProvider } from '@backstage/test-utils';
import { techdocsApiRef, techdocsStorageApiRef } from '../../../api';
@@ -67,10 +66,6 @@ const techdocsStorageApiMock: jest.Mocked<typeof techdocsStorageApiRef.T> = {
syncEntityDocs: jest.fn(),
};
const catalogApi: jest.Mocked<CatalogApi> = {
getEntityByRef: jest.fn(),
} as any;
const Wrapper = ({ children }: { children: React.ReactNode }) => {
return (
<ThemeProvider theme={lightTheme}>
@@ -79,7 +74,6 @@ const Wrapper = ({ children }: { children: React.ReactNode }) => {
[scmIntegrationsApiRef, {}],
[techdocsApiRef, techdocsApiMock],
[techdocsStorageApiRef, techdocsStorageApiMock],
[catalogApiRef, catalogApi],
]}
>
{children}
+17 -6
View File
@@ -7189,7 +7189,6 @@ __metadata:
version: 0.0.0-use.local
resolution: "@backstage/plugin-playlist@workspace:plugins/playlist"
dependencies:
"@backstage/catalog-client": "workspace:^"
"@backstage/catalog-model": "workspace:^"
"@backstage/cli": "workspace:^"
"@backstage/core-app-api": "workspace:^"
@@ -8059,7 +8058,6 @@ __metadata:
version: 0.0.0-use.local
resolution: "@backstage/plugin-techdocs-addons-test-utils@workspace:plugins/techdocs-addons-test-utils"
dependencies:
"@backstage/catalog-client": "workspace:^"
"@backstage/cli": "workspace:^"
"@backstage/core-app-api": "workspace:^"
"@backstage/core-components": "workspace:^"
@@ -8067,7 +8065,6 @@ __metadata:
"@backstage/dev-utils": "workspace:^"
"@backstage/integration-react": "workspace:^"
"@backstage/plugin-catalog": "workspace:^"
"@backstage/plugin-catalog-react": "workspace:^"
"@backstage/plugin-search-react": "workspace:^"
"@backstage/plugin-techdocs": "workspace:^"
"@backstage/plugin-techdocs-react": "workspace:^"
@@ -8129,7 +8126,6 @@ __metadata:
version: 0.0.0-use.local
resolution: "@backstage/plugin-techdocs-module-addons-contrib@workspace:plugins/techdocs-module-addons-contrib"
dependencies:
"@backstage/catalog-client": "workspace:^"
"@backstage/cli": "workspace:^"
"@backstage/core-app-api": "workspace:^"
"@backstage/core-components": "workspace:^"
@@ -8144,7 +8140,7 @@ __metadata:
"@material-ui/core": ^4.9.13
"@material-ui/icons": ^4.9.1
"@material-ui/lab": 4.0.0-alpha.57
"@react-hookz/web": ^19.0.0
"@react-hookz/web": ^20.0.0
"@testing-library/jest-dom": ^5.10.1
"@testing-library/react": ^12.1.3
"@testing-library/user-event": ^14.0.0
@@ -8233,7 +8229,6 @@ __metadata:
version: 0.0.0-use.local
resolution: "@backstage/plugin-techdocs@workspace:plugins/techdocs"
dependencies:
"@backstage/catalog-client": "workspace:^"
"@backstage/catalog-model": "workspace:^"
"@backstage/cli": "workspace:^"
"@backstage/config": "workspace:^"
@@ -12454,6 +12449,22 @@ __metadata:
languageName: node
linkType: hard
"@react-hookz/web@npm:^20.0.0":
version: 20.0.0
resolution: "@react-hookz/web@npm:20.0.0"
dependencies:
"@react-hookz/deep-equal": ^1.0.3
peerDependencies:
js-cookie: ^3.0.1
react: ^16.8 || ^17 || ^18
react-dom: ^16.8 || ^17 || ^18
peerDependenciesMeta:
js-cookie:
optional: true
checksum: 475d03cdd9a9131b7094549602c8a3ab52ad33e99d7ef408ba2ee7897bfdf332ab5228f46187b9a4c42535b4aaaf808e2c9c882949b2ec32d5ad42d53487a723
languageName: node
linkType: hard
"@remix-run/router@npm:1.0.4":
version: 1.0.4
resolution: "@remix-run/router@npm:1.0.4"