refactor(playlists): address review comments

Signed-off-by: Phil Kuang <pkuang@factset.com>
This commit is contained in:
Phil Kuang
2022-09-06 17:36:17 -04:00
parent d3737da337
commit 2028a73c96
27 changed files with 330 additions and 201 deletions
+1 -1
View File
@@ -2,4 +2,4 @@
'@backstage/core-components': patch
---
Export `HeaderActionMenu` and expose default `Table` icons via `Table.tableIcons`
Export `HeaderActionMenu` and expose default `Table` icons via `Table.icons`
+4
View File
@@ -16,6 +16,8 @@ yarn.lock @backstage/reviewers @backst
/docs/plugins/integrating-search-into-plugins.md @backstage/reviewers @backstage/techdocs-core
/packages/techdocs-cli @backstage/reviewers @backstage/techdocs-core
/packages/techdocs-cli-embedded-app @backstage/reviewers @backstage/techdocs-core
/plugins/adr @backstage/reviewers @kuangp
/plugins/adr-* @backstage/reviewers @kuangp
/plugins/allure @backstage/reviewers @deepak-bhardwaj-ps
/plugins/apache-airflow @backstage/reviewers @cmpadden
/plugins/api-docs @backstage/reviewers @backstage/sda-se-reviewers
@@ -47,6 +49,8 @@ yarn.lock @backstage/reviewers @backst
/plugins/kubernetes @backstage/reviewers @backstage/warpspeed
/plugins/kubernetes-* @backstage/reviewers @backstage/warpspeed
/plugins/newrelic-dashboard @backstage/reviewers @mufaddal7
/plugins/playlist @backstage/reviewers @kuangp
/plugins/playlist-* @backstage/reviewers @kuangp
/plugins/scaffolder-backend-module-rails @backstage/reviewers @angeliski
/plugins/scaffolder-backend-module-yeoman @backstage/reviewers @pawelmitka
/plugins/search @backstage/reviewers @backstage/techdocs-core
Binary file not shown.

Before

Width:  |  Height:  |  Size: 14 KiB

After

Width:  |  Height:  |  Size: 14 KiB

+8 -14
View File
@@ -14,24 +14,18 @@
* limitations under the License.
*/
import { IdentityClient } from '@backstage/plugin-auth-node';
import { createRouter } from '@backstage/plugin-playlist-backend';
import { Router } from 'express';
import { PluginEnvironment } from '../types';
export default async function createPlugin({
database,
discovery,
logger,
permissions,
}: PluginEnvironment): Promise<Router> {
export default async function createPlugin(
env: PluginEnvironment,
): Promise<Router> {
return await createRouter({
database,
identity: IdentityClient.create({
discovery,
issuer: await discovery.getExternalBaseUrl('auth'),
}),
logger,
permissions,
database: env.database,
discovery: env.discovery,
identity: env.identity,
logger: env.logger,
permissions: env.permissions,
});
}
+11 -13
View File
@@ -16,7 +16,6 @@ import { CardHeaderProps } from '@material-ui/core/CardHeader';
import { Column } from '@material-table/core';
import { ComponentClass } from 'react';
import { ComponentProps } from 'react';
import { ComponentType } from 'react';
import { default as CSS_2 } from 'csstype';
import { CSSProperties } from 'react';
import { ElementType } from 'react';
@@ -53,16 +52,6 @@ import { Theme } from '@material-ui/core/styles';
import { TooltipProps } from '@material-ui/core/Tooltip';
import { WithStyles } from '@material-ui/core/styles';
// @public (undocumented)
export type ActionItemProps = {
label?: ListItemTextProps['primary'];
secondaryLabel?: ListItemTextProps['secondary'];
icon?: ReactElement;
disabled?: boolean;
onClick?: (event: React_2.MouseEvent<HTMLDivElement>) => void;
WrapperComponent?: ComponentType;
};
// @public (undocumented)
export function AlertDisplay(props: AlertDisplayProps): JSX.Element | null;
@@ -448,9 +437,18 @@ export function Header(props: PropsWithChildren<Props_14>): JSX.Element;
// @public (undocumented)
export function HeaderActionMenu(props: HeaderActionMenuProps): JSX.Element;
// @public (undocumented)
export type HeaderActionMenuItem = {
label?: ListItemTextProps['primary'];
secondaryLabel?: ListItemTextProps['secondary'];
icon?: ReactElement;
disabled?: boolean;
onClick?: (event: React_2.MouseEvent<HTMLElement>) => void;
};
// @public (undocumented)
export type HeaderActionMenuProps = {
actionItems: ActionItemProps[];
actionItems: HeaderActionMenuItem[];
};
// @public (undocumented)
@@ -1357,7 +1355,7 @@ export function Table<T extends object = {}>(props: TableProps<T>): JSX.Element;
// @public (undocumented)
export namespace Table {
var // (undocumented)
tableIcons: Readonly<Icons>;
icons: Readonly<Icons>;
}
// Warning: (ae-missing-release-tag) "TableClassKey" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -523,4 +523,4 @@ export function Table<T extends object = {}>(props: TableProps<T>) {
);
}
Table.tableIcons = Object.freeze(tableIcons);
Table.icons = Object.freeze(tableIcons);
@@ -60,7 +60,7 @@ describe('<ComponentContextMenu />', () => {
);
});
it('Test wrapper, and secondary label', async () => {
it('Secondary label', async () => {
const onClickFunction = jest.fn();
const rendered = await renderInTestApp(
<HeaderActionMenu
@@ -68,9 +68,7 @@ describe('<ComponentContextMenu />', () => {
{
label: 'Some label',
secondaryLabel: 'Secondary label',
WrapperComponent: ({ children }) => (
<button onClick={onClickFunction}>{children}</button>
),
onClick: onClickFunction,
},
]}
/>,
@@ -14,7 +14,7 @@
* limitations under the License.
*/
import React, { Fragment, ReactElement, ComponentType } from 'react';
import React, { Fragment, ReactElement } from 'react';
import IconButton from '@material-ui/core/IconButton';
import List from '@material-ui/core/List';
import ListItem from '@material-ui/core/ListItem';
@@ -28,13 +28,12 @@ import MoreVert from '@material-ui/icons/MoreVert';
/**
* @public
*/
export type ActionItemProps = {
export type HeaderActionMenuItem = {
label?: ListItemTextProps['primary'];
secondaryLabel?: ListItemTextProps['secondary'];
icon?: ReactElement;
disabled?: boolean;
onClick?: (event: React.MouseEvent<HTMLDivElement>) => void;
WrapperComponent?: ComponentType;
onClick?: (event: React.MouseEvent<HTMLElement>) => void;
};
const ActionItem = ({
@@ -43,10 +42,9 @@ const ActionItem = ({
icon,
disabled = false,
onClick,
WrapperComponent = React.Fragment,
}: ActionItemProps) => {
}: HeaderActionMenuItem) => {
return (
<WrapperComponent>
<React.Fragment>
<ListItem
data-testid="header-action-item"
disabled={disabled}
@@ -60,7 +58,7 @@ const ActionItem = ({
{icon && <ListItemIcon>{icon}</ListItemIcon>}
<ListItemText primary={label} secondary={secondaryLabel} />
</ListItem>
</WrapperComponent>
</React.Fragment>
);
};
@@ -68,7 +66,7 @@ const ActionItem = ({
* @public
*/
export type HeaderActionMenuProps = {
actionItems: ActionItemProps[];
actionItems: HeaderActionMenuItem[];
};
/**
@@ -16,6 +16,6 @@
export { HeaderActionMenu } from './HeaderActionMenu';
export type {
ActionItemProps,
HeaderActionMenuItem,
HeaderActionMenuProps,
} from './HeaderActionMenu';
+8 -13
View File
@@ -21,20 +21,15 @@ import { createRouter } from '@backstage/plugin-playlist-backend';
import { Router } from 'express';
import { PluginEnvironment } from '../types';
export default async function createPlugin({
database,
discovery,
logger,
permissions,
}: PluginEnvironment): Promise<Router> {
export default async function createPlugin(
env: PluginEnvironment,
): Promise<Router> {
return await createRouter({
database,
identity: IdentityClient.create({
discovery,
issuer: await discovery.getExternalBaseUrl('auth'),
}),
logger,
permissions,
database: env.database,
discovery: env.discovery,
identity: env.identity,
logger: env.logger,
permissions: env.permissions,
});
}
```
+5 -2
View File
@@ -7,7 +7,7 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common';
import { Conditions } from '@backstage/plugin-permission-node';
import express from 'express';
import { IdentityClient } from '@backstage/plugin-auth-node';
import { IdentityApi } from '@backstage/plugin-auth-node';
import { Logger } from 'winston';
import { Permission } from '@backstage/plugin-permission-common';
import { PermissionCondition } from '@backstage/plugin-permission-common';
@@ -17,6 +17,7 @@ import { PermissionPolicy } from '@backstage/plugin-permission-node';
import { PermissionRule } from '@backstage/plugin-permission-node';
import { PlaylistMetadata } from '@backstage/plugin-playlist-common';
import { PluginDatabaseManager } from '@backstage/backend-common';
import { PluginEndpointDiscovery } from '@backstage/backend-common';
import { PolicyDecision } from '@backstage/plugin-permission-common';
import { PolicyQuery } from '@backstage/plugin-permission-node';
import { ResourcePermission } from '@backstage/plugin-permission-common';
@@ -84,7 +85,9 @@ export interface RouterOptions {
// (undocumented)
database: PluginDatabaseManager;
// (undocumented)
identity: IdentityClient;
discovery: PluginEndpointDiscovery;
// (undocumented)
identity: IdentityApi;
// (undocumented)
logger: Logger;
// (undocumented)
@@ -37,7 +37,9 @@ exports.up = async function up(knex) {
.onDelete('CASCADE')
.comment('The id of the playlist this entity belongs to');
table.string('entity_ref').notNullable().comment('A entity ref');
table.unique(['playlist_id', 'entity_ref']);
table.unique(['playlist_id', 'entity_ref'], {
indexName: 'playlist_entity_composite_index',
});
});
await knex.schema.createTable('followers', table => {
@@ -49,7 +51,9 @@ exports.up = async function up(knex) {
.onDelete('CASCADE')
.comment('The id of the playlist being followed');
table.string('user_ref').notNullable().comment('A user entity ref');
table.unique(['playlist_id', 'user_ref']);
table.unique(['playlist_id', 'user_ref'], {
indexName: 'playlist_follower_composite_index',
});
});
};
+10 -8
View File
@@ -22,13 +22,15 @@
"postpack": "backstage-cli package postpack"
},
"dependencies": {
"@backstage/backend-common": "^0.15.1-next.2",
"@backstage/backend-test-utils": "^0.1.28-next.2",
"@backstage/config": "^1.0.1",
"@backstage/errors": "^1.1.0",
"@backstage/plugin-auth-node": "^0.2.5-next.2",
"@backstage/plugin-permission-common": "^0.6.4-next.1",
"@backstage/plugin-permission-node": "^0.6.5-next.2",
"@backstage/backend-common": "^0.15.1-next.3",
"@backstage/backend-test-utils": "^0.1.28-next.3",
"@backstage/catalog-client": "1.1.0-next.2",
"@backstage/catalog-model": "^1.1.1-next.0",
"@backstage/config": "^1.0.2-next.0",
"@backstage/errors": "^1.1.1-next.0",
"@backstage/plugin-auth-node": "^0.2.5-next.3",
"@backstage/plugin-permission-common": "^0.6.4-next.2",
"@backstage/plugin-permission-node": "^0.6.5-next.3",
"@backstage/plugin-playlist-common": "^0.0.0",
"@types/express": "*",
"express": "^4.17.1",
@@ -40,7 +42,7 @@
"yn": "^4.0.0"
},
"devDependencies": {
"@backstage/cli": "^0.19.0-next.2",
"@backstage/cli": "^0.19.0-next.3",
"@types/supertest": "^2.0.8",
"msw": "^0.47.0",
"supertest": "^6.1.3"
@@ -73,7 +73,7 @@ export function parseListPlaylistsFilterString(
return undefined;
}
const filtersByKey: Record<string, ListPlaylistsMatchFilter> = {};
const filtersByKey = new Map<string, ListPlaylistsMatchFilter>();
for (const statement of statements) {
const equalsIndex = statement.indexOf('=');
@@ -89,13 +89,12 @@ export function parseListPlaylistsFilterString(
);
}
const f =
key in filtersByKey
? filtersByKey[key]
: (filtersByKey[key] = { key, values: [] });
const f = filtersByKey.has(key)
? filtersByKey.get(key)
: filtersByKey.set(key, { key, values: [] }).get(key);
f.values.push(value);
f!.values.push(value);
}
return Object.values(filtersByKey);
return [...filtersByKey.values()];
}
@@ -14,9 +14,13 @@
* limitations under the License.
*/
import { DatabaseManager, getVoidLogger } from '@backstage/backend-common';
import {
DatabaseManager,
getVoidLogger,
PluginEndpointDiscovery,
} from '@backstage/backend-common';
import { ConfigReader } from '@backstage/config';
import { IdentityClient } from '@backstage/plugin-auth-node';
import { IdentityApi } from '@backstage/plugin-auth-node';
import { AuthorizeResult } from '@backstage/plugin-permission-common';
import { permissions } from '@backstage/plugin-playlist-common';
import express from 'express';
@@ -24,8 +28,39 @@ import request from 'supertest';
import { createRouter } from './router';
const sampleEntities = [
{
kind: 'system',
metadata: {
namespace: 'default',
name: 'test-ent-system',
title: 'Test Ent',
description: 'test ent description',
},
},
{
kind: 'component',
metadata: {
namespace: 'default',
name: 'test-ent',
title: 'Test Ent 2',
description: 'test ent description 2',
},
spec: {
type: 'library',
},
},
];
const mockGetEntties = jest
.fn()
.mockImplementation(async () => ({ items: sampleEntities }));
jest.mock('@backstage/catalog-client', () => ({
CatalogClient: jest
.fn()
.mockImplementation(() => ({ getEntities: mockGetEntties })),
}));
jest.mock('@backstage/plugin-auth-node', () => ({
...jest.requireActual('@backstage/plugin-auth-node'),
getBearerTokenFromAuthorizationHeader: () => 'token',
}));
@@ -99,14 +134,20 @@ describe('createRouter', () => {
userEntityRef: 'user:default/me',
};
const mockIdentityClient = {
authenticate: jest
getIdentity: jest
.fn()
.mockImplementation(async () => ({ identity: mockUser })),
} as unknown as IdentityClient;
} as unknown as IdentityApi;
const discovery: jest.Mocked<PluginEndpointDiscovery> = {
getBaseUrl: jest.fn(),
getExternalBaseUrl: jest.fn(),
};
beforeEach(async () => {
const router = await createRouter({
database: createDatabase(),
discovery,
identity: mockIdentityClient,
logger: getVoidLogger(),
permissions: mockPermissionEvaluator,
@@ -398,6 +439,7 @@ describe('createRouter', () => {
{ token: 'token' },
);
expect(mockDbHandler.getPlaylistEntities).not.toHaveBeenCalled();
expect(mockGetEntties).not.toHaveBeenCalled();
expect(response.status).toEqual(403);
});
@@ -406,8 +448,25 @@ describe('createRouter', () => {
expect(mockDbHandler.getPlaylistEntities).toHaveBeenCalledWith(
'playlist-id',
);
expect(mockGetEntties).toHaveBeenCalledWith(
{
filter: [
{
kind: 'component',
'metadata.namespace': 'default',
'metadata.name': 'test-ent',
},
{
kind: 'system',
'metadata.namespace': 'default',
'metadata.name': 'test-ent-system',
},
],
},
{ token: 'token' },
);
expect(response.status).toEqual(200);
expect(response.body).toEqual(mockEntities);
expect(response.body).toEqual(sampleEntities);
});
});
+48 -7
View File
@@ -14,11 +14,17 @@
* limitations under the License.
*/
import { errorHandler, PluginDatabaseManager } from '@backstage/backend-common';
import {
errorHandler,
PluginDatabaseManager,
PluginEndpointDiscovery,
} from '@backstage/backend-common';
import { CatalogClient } from '@backstage/catalog-client';
import { parseEntityRef } from '@backstage/catalog-model';
import { NotAllowedError } from '@backstage/errors';
import {
getBearerTokenFromAuthorizationHeader,
IdentityClient,
IdentityApi,
} from '@backstage/plugin-auth-node';
import {
AuthorizePermissionRequest,
@@ -44,7 +50,8 @@ import { parseListPlaylistsFilterParams } from './ListPlaylistsFilter';
*/
export interface RouterOptions {
database: PluginDatabaseManager;
identity: IdentityClient;
discovery: PluginEndpointDiscovery;
identity: IdentityApi;
logger: Logger;
permissions: PermissionEvaluator;
}
@@ -57,6 +64,7 @@ export async function createRouter(
): Promise<express.Router> {
const {
database,
discovery,
identity,
logger,
permissions: permissionEvaluator,
@@ -64,19 +72,23 @@ export async function createRouter(
logger.info('Initializing Playlist backend');
const catalogClient = new CatalogClient({ discoveryApi: discovery });
const db = await database.getClient();
const dbHandler = await DatabaseHandler.create({ database: db });
const evaluateRequestPermission = async (
req: express.Request,
request: express.Request,
permission: AuthorizePermissionRequest | QueryPermissionRequest,
conditional: boolean = false,
) => {
const token = getBearerTokenFromAuthorizationHeader(
req.header('authorization'),
request.header('authorization'),
);
const user = await identity.authenticate(token);
const user = await identity.getIdentity({ request });
if (!user) {
throw new NotAllowedError('Unauthorized');
}
const decision = conditional
? (
@@ -196,7 +208,36 @@ export async function createRouter(
permission: permissions.playlistListRead,
resourceRef: req.params.playlistId,
});
const entities = await dbHandler.getPlaylistEntities(req.params.playlistId);
const entityRefs = await dbHandler.getPlaylistEntities(
req.params.playlistId,
);
if (!entityRefs.length) {
res.json([]);
return;
}
const filter = entityRefs.map(ref => {
const compoundRef = parseEntityRef(ref);
return {
kind: compoundRef.kind,
'metadata.namespace': compoundRef.namespace,
'metadata.name': compoundRef.name,
};
});
const token = getBearerTokenFromAuthorizationHeader(
req.header('authorization'),
);
// TODO(kuanpg): entities in this playlist that no longer exist in the catalog will be
// excluded from this response, we need a way to clean up these orphaned refs potentially
// via catalog events (https://github.com/backstage/backstage/issues/8219)
//
// Note: This will also enforce catalog permissions and will only return entities for which the current user has access to
const entities = (await catalogClient.getEntities({ filter }, { token }))
.items;
res.json(entities);
});
@@ -23,7 +23,7 @@ import {
useHotMemoize,
} from '@backstage/backend-common';
import { ConfigReader } from '@backstage/config';
import { IdentityClient } from '@backstage/plugin-auth-node';
import { DefaultIdentityClient } from '@backstage/plugin-auth-node';
import { ServerPermissionClient } from '@backstage/plugin-permission-node';
import { Server } from 'http';
import { Logger } from 'winston';
@@ -53,7 +53,7 @@ export async function startStandaloneServer(
return manager.forPlugin('playlist');
});
const identity = IdentityClient.create({
const identity = DefaultIdentityClient.create({
discovery,
issuer: await discovery.getExternalBaseUrl('auth'),
});
@@ -69,6 +69,7 @@ export async function startStandaloneServer(
logger.debug('Starting application server...');
const router = await createRouter({
database,
discovery,
identity,
logger,
permissions,
+2 -2
View File
@@ -23,10 +23,10 @@
"postpack": "backstage-cli package postpack"
},
"dependencies": {
"@backstage/plugin-permission-common": "^0.6.4-next.1"
"@backstage/plugin-permission-common": "^0.6.4-next.2"
},
"devDependencies": {
"@backstage/cli": "^0.19.0-next.2"
"@backstage/cli": "^0.19.0-next.3"
},
"files": [
"dist"
+6 -6
View File
@@ -8,16 +8,16 @@
import { ApiRef } from '@backstage/core-plugin-api';
import { BackstagePlugin } from '@backstage/core-plugin-api';
import { DiscoveryApi } from '@backstage/core-plugin-api';
import { Entity } from '@backstage/catalog-model';
import { FetchApi } from '@backstage/core-plugin-api';
import { Playlist } from '@backstage/plugin-playlist-common';
import { PlaylistMetadata } from '@backstage/plugin-playlist-common';
import { RouteRef } from '@backstage/core-plugin-api';
// @public (undocumented)
export const EntityPlaylistDialog: ({
open,
onClose,
}: EntityPlaylistDialogProps) => JSX.Element;
export const EntityPlaylistDialog: (
props: EntityPlaylistDialogProps,
) => JSX.Element;
// @public (undocumented)
export type EntityPlaylistDialogProps = {
@@ -49,7 +49,7 @@ export interface PlaylistApi {
// (undocumented)
getPlaylist(playlistId: string): Promise<Playlist>;
// (undocumented)
getPlaylistEntities(playlistId: string): Promise<string[]>;
getPlaylistEntities(playlistId: string): Promise<Entity[]>;
// (undocumented)
removePlaylistEntities(
playlistId: string,
@@ -80,7 +80,7 @@ export class PlaylistClient implements PlaylistApi {
// (undocumented)
getPlaylist(playlistId: string): Promise<Playlist>;
// (undocumented)
getPlaylistEntities(playlistId: string): Promise<string[]>;
getPlaylistEntities(playlistId: string): Promise<Entity[]>;
// (undocumented)
removePlaylistEntities(
playlistId: string,
+11 -12
View File
@@ -22,14 +22,14 @@
"postpack": "backstage-cli package postpack"
},
"dependencies": {
"@backstage/catalog-model": "^1.1.0",
"@backstage/core-components": "^0.11.1-next.2",
"@backstage/core-plugin-api": "^1.0.6-next.2",
"@backstage/errors": "^1.1.0",
"@backstage/catalog-model": "^1.1.1-next.0",
"@backstage/core-components": "^0.11.1-next.3",
"@backstage/core-plugin-api": "^1.0.6-next.3",
"@backstage/errors": "^1.1.1-next.0",
"@backstage/plugin-catalog-common": "^1.0.6-next.0",
"@backstage/plugin-catalog-react": "^1.1.4-next.1",
"@backstage/plugin-permission-common": "^0.6.4-next.1",
"@backstage/plugin-permission-react": "^0.4.5-next.1",
"@backstage/plugin-catalog-react": "^1.1.4-next.2",
"@backstage/plugin-permission-common": "^0.6.4-next.2",
"@backstage/plugin-permission-react": "^0.4.5-next.2",
"@backstage/plugin-playlist-common": "^0.0.0",
"@backstage/plugin-search-react": "^1.1.0-next.2",
"@backstage/theme": "^0.2.16",
@@ -47,15 +47,14 @@
"react-router-dom": "6.0.0-beta.0 || ^6.3.0"
},
"devDependencies": {
"@backstage/cli": "^0.19.0-next.2",
"@backstage/core-app-api": "^1.1.0-next.2",
"@backstage/dev-utils": "^1.0.6-next.1",
"@backstage/test-utils": "^1.2.0-next.2",
"@backstage/cli": "^0.19.0-next.3",
"@backstage/core-app-api": "^1.1.0-next.3",
"@backstage/dev-utils": "^1.0.6-next.2",
"@backstage/test-utils": "^1.2.0-next.3",
"@testing-library/jest-dom": "^5.10.1",
"@testing-library/react": "^12.1.3",
"@testing-library/react-hooks": "^8.0.0",
"@testing-library/user-event": "^14.0.0",
"@types/jest": "*",
"@types/node": "*",
"cross-fetch": "^3.1.5",
"msw": "^0.47.0",
+2 -1
View File
@@ -14,6 +14,7 @@
* limitations under the License.
*/
import { Entity } from '@backstage/catalog-model';
import { createApiRef } from '@backstage/core-plugin-api';
import { Playlist, PlaylistMetadata } from '@backstage/plugin-playlist-common';
@@ -62,7 +63,7 @@ export interface PlaylistApi {
addPlaylistEntities(playlistId: string, entityRefs: string[]): Promise<void>;
getPlaylistEntities(playlistId: string): Promise<string[]>;
getPlaylistEntities(playlistId: string): Promise<Entity[]>;
removePlaylistEntities(
playlistId: string,
@@ -215,7 +215,29 @@ describe('PlaylistClient', () => {
});
it('getPlaylistEntities', async () => {
const entities = ['component:default/ent1', 'component:default/ent2'];
const entities = [
{
kind: 'system',
metadata: {
namespace: 'default',
name: 'test-ent',
title: 'Test Ent',
description: 'test ent description',
},
},
{
kind: 'component',
metadata: {
namespace: 'foo',
name: 'test-ent2',
title: 'Test Ent 2',
description: 'test ent description 2',
},
spec: {
type: 'library',
},
},
];
server.use(
rest.get(`${mockBaseUrl}/id/entities`, (_, res, ctx) =>
+2 -1
View File
@@ -14,6 +14,7 @@
* limitations under the License.
*/
import { Entity } from '@backstage/catalog-model';
import { DiscoveryApi, FetchApi } from '@backstage/core-plugin-api';
import { ResponseError } from '@backstage/errors';
import { Playlist, PlaylistMetadata } from '@backstage/plugin-playlist-common';
@@ -142,7 +143,7 @@ export class PlaylistClient implements PlaylistApi {
}
}
async getPlaylistEntities(playlistId: string): Promise<string[]> {
async getPlaylistEntities(playlistId: string): Promise<Entity[]> {
const baseUrl = await this.discoveryApi.getBaseUrl('playlist');
const resp = await this.fetchApi.fetch(
`${baseUrl}/${playlistId}/entities`,
@@ -78,10 +78,9 @@ export type EntityPlaylistDialogProps = {
onClose: () => void;
};
export const EntityPlaylistDialog = ({
open,
onClose,
}: EntityPlaylistDialogProps) => {
export const EntityPlaylistDialog = (props: EntityPlaylistDialogProps) => {
const { open, onClose } = props;
const classes = useStyles();
const navigate = useNavigate();
const { entity } = useAsyncEntity();
@@ -15,11 +15,7 @@
*/
import { ErrorApi, errorApiRef } from '@backstage/core-plugin-api';
import {
CatalogApi,
catalogApiRef,
entityRouteRef,
} from '@backstage/plugin-catalog-react';
import { entityRouteRef } from '@backstage/plugin-catalog-react';
import { AuthorizeResult } from '@backstage/plugin-permission-common';
import {
PermissionApi,
@@ -47,17 +43,6 @@ jest.mock('./AddEntitiesDrawer', () => ({
describe('PlaylistEntitiesTable', () => {
const errorApi: Partial<ErrorApi> = { post: jest.fn() };
const playlistApi: Partial<PlaylistApi> = {
getPlaylistEntities: jest
.fn()
.mockImplementation(async () => [
'system:default/test-ent',
'component:foo/test-ent2',
]),
addPlaylistEntities: jest.fn().mockImplementation(async () => {}),
removePlaylistEntities: jest.fn().mockImplementation(async () => {}),
};
const sampleEntities = [
{
kind: 'system',
@@ -81,10 +66,12 @@ describe('PlaylistEntitiesTable', () => {
},
},
];
const catalogApi: Partial<CatalogApi> = {
getEntities: jest
const playlistApi: Partial<PlaylistApi> = {
getPlaylistEntities: jest
.fn()
.mockImplementation(async () => ({ items: sampleEntities })),
.mockImplementation(async () => sampleEntities),
addPlaylistEntities: jest.fn().mockImplementation(async () => {}),
removePlaylistEntities: jest.fn().mockImplementation(async () => {}),
};
const mockAuthorize = jest
@@ -97,7 +84,6 @@ describe('PlaylistEntitiesTable', () => {
<SWRConfig value={{ provider: () => new Map() }}>
<TestApiProvider
apis={[
[catalogApiRef, catalogApi],
[errorApiRef, errorApi],
[permissionApiRef, permissionApi],
[playlistApiRef, playlistApi],
@@ -121,28 +107,6 @@ describe('PlaylistEntitiesTable', () => {
const rendered = await render();
expect(playlistApi.getPlaylistEntities).toHaveBeenCalledWith('playlist-id');
expect(catalogApi.getEntities).toHaveBeenCalledWith({
filter: [
{
kind: 'system',
'metadata.namespace': 'default',
'metadata.name': 'test-ent',
},
{
kind: 'component',
'metadata.namespace': 'foo',
'metadata.name': 'test-ent2',
},
],
fields: [
'kind',
'metadata.namespace',
'metadata.name',
'metadata.title',
'metadata.description',
'spec.type',
],
});
expect(rendered.getByText('Test Ent')).toBeInTheDocument();
expect(rendered.getByText('system')).toBeInTheDocument();
@@ -14,11 +14,7 @@
* limitations under the License.
*/
import {
Entity,
parseEntityRef,
stringifyEntityRef,
} from '@backstage/catalog-model';
import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
import {
ErrorPanel,
SubvalueCell,
@@ -26,7 +22,7 @@ import {
TableFilter,
} from '@backstage/core-components';
import { errorApiRef, useApi } from '@backstage/core-plugin-api';
import { catalogApiRef, EntityRefLink } from '@backstage/plugin-catalog-react';
import { EntityRefLink } from '@backstage/plugin-catalog-react';
import { usePermission } from '@backstage/plugin-permission-react';
import { permissions } from '@backstage/plugin-playlist-common';
import AddBoxIcon from '@material-ui/icons/AddBox';
@@ -43,7 +39,6 @@ export const PlaylistEntitiesTable = ({
}: {
playlistId: string;
}) => {
const catalogApi = useApi(catalogApiRef);
const errorApi = useApi(errorApiRef);
const playlistApi = useApi(playlistApiRef);
const [openAddEntitiesDrawer, setOpenAddEntitiesDrawer] = useState(false);
@@ -53,39 +48,10 @@ export const PlaylistEntitiesTable = ({
resourceRef: playlistId,
});
const [{ value: entities, loading, error }, loadEntities] =
useAsyncFn(async () => {
const entityRefs = await playlistApi.getPlaylistEntities(playlistId);
if (!entityRefs.length) {
return [];
}
const filter = entityRefs.map(ref => {
const compoundRef = parseEntityRef(ref);
return {
kind: compoundRef.kind,
'metadata.namespace': compoundRef.namespace,
'metadata.name': compoundRef.name,
};
});
// TODO(kuanpg): entities in this playlist that no longer exist in the catalog will be
// excluded from this response, we need a way to clean up these orphaned refs potentially
// via catalog events (https://github.com/backstage/backstage/issues/8219)
return (
await catalogApi.getEntities({
filter,
fields: [
'kind',
'metadata.namespace',
'metadata.name',
'metadata.title',
'metadata.description',
'spec.type',
],
})
).items;
}, [catalogApi, playlistApi]);
const [{ value: entities, loading, error }, loadEntities] = useAsyncFn(
() => playlistApi.getPlaylistEntities(playlistId),
[playlistApi],
);
useEffect(() => {
loadEntities();
@@ -190,7 +156,7 @@ export const PlaylistEntitiesTable = ({
data={entities ?? []}
filters={filters}
icons={{
...Table.tableIcons,
...Table.icons,
Search: forwardRef((props, ref) => (
<SearchIcon {...props} ref={ref} />
)),
+82 -1
View File
@@ -3010,7 +3010,7 @@ __metadata:
languageName: unknown
linkType: soft
"@backstage/catalog-client@^1.1.0-next.2, @backstage/catalog-client@workspace:packages/catalog-client":
"@backstage/catalog-client@1.1.0-next.2, @backstage/catalog-client@^1.1.0-next.2, @backstage/catalog-client@workspace:packages/catalog-client":
version: 0.0.0-use.local
resolution: "@backstage/catalog-client@workspace:packages/catalog-client"
dependencies:
@@ -6222,6 +6222,85 @@ __metadata:
languageName: node
linkType: hard
"@backstage/plugin-playlist-backend@^0.0.0, @backstage/plugin-playlist-backend@workspace:plugins/playlist-backend":
version: 0.0.0-use.local
resolution: "@backstage/plugin-playlist-backend@workspace:plugins/playlist-backend"
dependencies:
"@backstage/backend-common": ^0.15.1-next.3
"@backstage/backend-test-utils": ^0.1.28-next.3
"@backstage/catalog-client": 1.1.0-next.2
"@backstage/catalog-model": ^1.1.1-next.0
"@backstage/cli": ^0.19.0-next.3
"@backstage/config": ^1.0.2-next.0
"@backstage/errors": ^1.1.1-next.0
"@backstage/plugin-auth-node": ^0.2.5-next.3
"@backstage/plugin-permission-common": ^0.6.4-next.2
"@backstage/plugin-permission-node": ^0.6.5-next.3
"@backstage/plugin-playlist-common": ^0.0.0
"@types/express": "*"
"@types/supertest": ^2.0.8
express: ^4.17.1
express-promise-router: ^4.1.0
knex: ^2.0.0
msw: ^0.47.0
node-fetch: ^2.6.7
supertest: ^6.1.3
uuid: ^8.2.0
winston: ^3.2.1
yn: ^4.0.0
languageName: unknown
linkType: soft
"@backstage/plugin-playlist-common@^0.0.0, @backstage/plugin-playlist-common@workspace:plugins/playlist-common":
version: 0.0.0-use.local
resolution: "@backstage/plugin-playlist-common@workspace:plugins/playlist-common"
dependencies:
"@backstage/cli": ^0.19.0-next.3
"@backstage/plugin-permission-common": ^0.6.4-next.2
languageName: unknown
linkType: soft
"@backstage/plugin-playlist@^0.0.0, @backstage/plugin-playlist@workspace:plugins/playlist":
version: 0.0.0-use.local
resolution: "@backstage/plugin-playlist@workspace:plugins/playlist"
dependencies:
"@backstage/catalog-model": ^1.1.1-next.0
"@backstage/cli": ^0.19.0-next.3
"@backstage/core-app-api": ^1.1.0-next.3
"@backstage/core-components": ^0.11.1-next.3
"@backstage/core-plugin-api": ^1.0.6-next.3
"@backstage/dev-utils": ^1.0.6-next.2
"@backstage/errors": ^1.1.1-next.0
"@backstage/plugin-catalog-common": ^1.0.6-next.0
"@backstage/plugin-catalog-react": ^1.1.4-next.2
"@backstage/plugin-permission-common": ^0.6.4-next.2
"@backstage/plugin-permission-react": ^0.4.5-next.2
"@backstage/plugin-playlist-common": ^0.0.0
"@backstage/plugin-search-react": ^1.1.0-next.2
"@backstage/test-utils": ^1.2.0-next.3
"@backstage/theme": ^0.2.16
"@material-ui/core": ^4.9.13
"@material-ui/icons": ^4.9.1
"@material-ui/lab": ^4.0.0-alpha.57
"@testing-library/jest-dom": ^5.10.1
"@testing-library/react": ^12.1.3
"@testing-library/react-hooks": ^8.0.0
"@testing-library/user-event": ^14.0.0
"@types/node": "*"
cross-fetch: ^3.1.5
lodash: ^4.17.21
msw: ^0.47.0
qs: ^6.9.4
react-hook-form: ^7.13.0
react-use: ^17.2.4
swr: ^1.1.2
peerDependencies:
react: ^16.13.1 || ^17.0.0
react-router: 6.0.0-beta.0 || ^6.3.0
react-router-dom: 6.0.0-beta.0 || ^6.3.0
languageName: unknown
linkType: soft
"@backstage/plugin-proxy-backend@^0.2.30-next.2, @backstage/plugin-proxy-backend@workspace:plugins/proxy-backend":
version: 0.0.0-use.local
resolution: "@backstage/plugin-proxy-backend@workspace:plugins/proxy-backend"
@@ -22470,6 +22549,7 @@ __metadata:
"@backstage/plugin-org": ^0.5.9-next.3
"@backstage/plugin-pagerduty": 0.5.2-next.3
"@backstage/plugin-permission-react": ^0.4.5-next.2
"@backstage/plugin-playlist": ^0.0.0
"@backstage/plugin-rollbar": ^0.4.9-next.3
"@backstage/plugin-scaffolder": ^1.6.0-next.3
"@backstage/plugin-search": ^1.0.2-next.3
@@ -22557,6 +22637,7 @@ __metadata:
"@backstage/plugin-permission-backend": ^0.5.11-next.2
"@backstage/plugin-permission-common": ^0.6.4-next.2
"@backstage/plugin-permission-node": ^0.6.5-next.3
"@backstage/plugin-playlist-backend": ^0.0.0
"@backstage/plugin-proxy-backend": ^0.2.30-next.2
"@backstage/plugin-rollbar-backend": ^0.1.33-next.3
"@backstage/plugin-scaffolder-backend": ^1.6.0-next.3