diff --git a/.changeset/calm-snakes-tap.md b/.changeset/calm-snakes-tap.md index 6b853da53d..6fdf092ede 100644 --- a/.changeset/calm-snakes-tap.md +++ b/.changeset/calm-snakes-tap.md @@ -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` diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f00771d29d..b0f1e828cd 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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 diff --git a/microsite/static/img/playlist-logo.png b/microsite/static/img/playlist-logo.png index b5c8d99401..333a9f832a 100644 Binary files a/microsite/static/img/playlist-logo.png and b/microsite/static/img/playlist-logo.png differ diff --git a/packages/backend/src/plugins/playlist.ts b/packages/backend/src/plugins/playlist.ts index 0d31158842..6cae6a30ab 100644 --- a/packages/backend/src/plugins/playlist.ts +++ b/packages/backend/src/plugins/playlist.ts @@ -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 { +export default async function createPlugin( + env: PluginEnvironment, +): Promise { 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, }); } diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 20e14312bb..99d18b9b12 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -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) => void; - WrapperComponent?: ComponentType; -}; - // @public (undocumented) export function AlertDisplay(props: AlertDisplayProps): JSX.Element | null; @@ -448,9 +437,18 @@ export function Header(props: PropsWithChildren): 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) => void; +}; + // @public (undocumented) export type HeaderActionMenuProps = { - actionItems: ActionItemProps[]; + actionItems: HeaderActionMenuItem[]; }; // @public (undocumented) @@ -1357,7 +1355,7 @@ export function Table(props: TableProps): JSX.Element; // @public (undocumented) export namespace Table { var // (undocumented) - tableIcons: Readonly; + icons: Readonly; } // Warning: (ae-missing-release-tag) "TableClassKey" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/packages/core-components/src/components/Table/Table.tsx b/packages/core-components/src/components/Table/Table.tsx index b04c247b2f..49538b680a 100644 --- a/packages/core-components/src/components/Table/Table.tsx +++ b/packages/core-components/src/components/Table/Table.tsx @@ -523,4 +523,4 @@ export function Table(props: TableProps) { ); } -Table.tableIcons = Object.freeze(tableIcons); +Table.icons = Object.freeze(tableIcons); diff --git a/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.test.tsx b/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.test.tsx index 3337571d4e..92ce241aa8 100644 --- a/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.test.tsx +++ b/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.test.tsx @@ -60,7 +60,7 @@ describe('', () => { ); }); - it('Test wrapper, and secondary label', async () => { + it('Secondary label', async () => { const onClickFunction = jest.fn(); const rendered = await renderInTestApp( ', () => { { label: 'Some label', secondaryLabel: 'Secondary label', - WrapperComponent: ({ children }) => ( - - ), + onClick: onClickFunction, }, ]} />, diff --git a/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.tsx b/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.tsx index 125363fe0a..e164244ea7 100644 --- a/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.tsx +++ b/packages/core-components/src/layout/HeaderActionMenu/HeaderActionMenu.tsx @@ -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) => void; - WrapperComponent?: ComponentType; + onClick?: (event: React.MouseEvent) => void; }; const ActionItem = ({ @@ -43,10 +42,9 @@ const ActionItem = ({ icon, disabled = false, onClick, - WrapperComponent = React.Fragment, -}: ActionItemProps) => { +}: HeaderActionMenuItem) => { return ( - + {icon}} - + ); }; @@ -68,7 +66,7 @@ const ActionItem = ({ * @public */ export type HeaderActionMenuProps = { - actionItems: ActionItemProps[]; + actionItems: HeaderActionMenuItem[]; }; /** diff --git a/packages/core-components/src/layout/HeaderActionMenu/index.ts b/packages/core-components/src/layout/HeaderActionMenu/index.ts index 6fa74d9d09..4fd9552fa7 100644 --- a/packages/core-components/src/layout/HeaderActionMenu/index.ts +++ b/packages/core-components/src/layout/HeaderActionMenu/index.ts @@ -16,6 +16,6 @@ export { HeaderActionMenu } from './HeaderActionMenu'; export type { - ActionItemProps, + HeaderActionMenuItem, HeaderActionMenuProps, } from './HeaderActionMenu'; diff --git a/plugins/playlist-backend/README.md b/plugins/playlist-backend/README.md index 75e0281a08..14a39ca741 100644 --- a/plugins/playlist-backend/README.md +++ b/plugins/playlist-backend/README.md @@ -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 { +export default async function createPlugin( + env: PluginEnvironment, +): Promise { 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, }); } ``` diff --git a/plugins/playlist-backend/api-report.md b/plugins/playlist-backend/api-report.md index 5c86b6ae45..c7109efbfb 100644 --- a/plugins/playlist-backend/api-report.md +++ b/plugins/playlist-backend/api-report.md @@ -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) diff --git a/plugins/playlist-backend/migrations/20220701011329_init.js b/plugins/playlist-backend/migrations/20220701011329_init.js index 17a71e8438..be08363dec 100644 --- a/plugins/playlist-backend/migrations/20220701011329_init.js +++ b/plugins/playlist-backend/migrations/20220701011329_init.js @@ -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', + }); }); }; diff --git a/plugins/playlist-backend/package.json b/plugins/playlist-backend/package.json index bbea8163fb..b760611548 100644 --- a/plugins/playlist-backend/package.json +++ b/plugins/playlist-backend/package.json @@ -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" diff --git a/plugins/playlist-backend/src/service/ListPlaylistsFilter.ts b/plugins/playlist-backend/src/service/ListPlaylistsFilter.ts index bff14b2a58..a746286136 100644 --- a/plugins/playlist-backend/src/service/ListPlaylistsFilter.ts +++ b/plugins/playlist-backend/src/service/ListPlaylistsFilter.ts @@ -73,7 +73,7 @@ export function parseListPlaylistsFilterString( return undefined; } - const filtersByKey: Record = {}; + const filtersByKey = new Map(); 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()]; } diff --git a/plugins/playlist-backend/src/service/router.test.ts b/plugins/playlist-backend/src/service/router.test.ts index 06185acae8..3f90a8b8af 100644 --- a/plugins/playlist-backend/src/service/router.test.ts +++ b/plugins/playlist-backend/src/service/router.test.ts @@ -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 = { + 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); }); }); diff --git a/plugins/playlist-backend/src/service/router.ts b/plugins/playlist-backend/src/service/router.ts index 7ca41c0ac1..bf0b789b7a 100644 --- a/plugins/playlist-backend/src/service/router.ts +++ b/plugins/playlist-backend/src/service/router.ts @@ -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 { 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); }); diff --git a/plugins/playlist-backend/src/service/standaloneServer.ts b/plugins/playlist-backend/src/service/standaloneServer.ts index 46d6f7ad8d..3b6acfd09b 100644 --- a/plugins/playlist-backend/src/service/standaloneServer.ts +++ b/plugins/playlist-backend/src/service/standaloneServer.ts @@ -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, diff --git a/plugins/playlist-common/package.json b/plugins/playlist-common/package.json index 67a97b1237..b9e8637f17 100644 --- a/plugins/playlist-common/package.json +++ b/plugins/playlist-common/package.json @@ -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" diff --git a/plugins/playlist/api-report.md b/plugins/playlist/api-report.md index ddfec2948d..7a732a4a53 100644 --- a/plugins/playlist/api-report.md +++ b/plugins/playlist/api-report.md @@ -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; // (undocumented) - getPlaylistEntities(playlistId: string): Promise; + getPlaylistEntities(playlistId: string): Promise; // (undocumented) removePlaylistEntities( playlistId: string, @@ -80,7 +80,7 @@ export class PlaylistClient implements PlaylistApi { // (undocumented) getPlaylist(playlistId: string): Promise; // (undocumented) - getPlaylistEntities(playlistId: string): Promise; + getPlaylistEntities(playlistId: string): Promise; // (undocumented) removePlaylistEntities( playlistId: string, diff --git a/plugins/playlist/package.json b/plugins/playlist/package.json index 51cffcca9f..f59fce18db 100644 --- a/plugins/playlist/package.json +++ b/plugins/playlist/package.json @@ -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", diff --git a/plugins/playlist/src/api/PlaylistApi.ts b/plugins/playlist/src/api/PlaylistApi.ts index d0b87ec28a..9a9da1badb 100644 --- a/plugins/playlist/src/api/PlaylistApi.ts +++ b/plugins/playlist/src/api/PlaylistApi.ts @@ -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; - getPlaylistEntities(playlistId: string): Promise; + getPlaylistEntities(playlistId: string): Promise; removePlaylistEntities( playlistId: string, diff --git a/plugins/playlist/src/api/PlaylistClient.test.ts b/plugins/playlist/src/api/PlaylistClient.test.ts index f3b62c19ac..20d4a0bcc8 100644 --- a/plugins/playlist/src/api/PlaylistClient.test.ts +++ b/plugins/playlist/src/api/PlaylistClient.test.ts @@ -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) => diff --git a/plugins/playlist/src/api/PlaylistClient.ts b/plugins/playlist/src/api/PlaylistClient.ts index b77aa79733..425b185de8 100644 --- a/plugins/playlist/src/api/PlaylistClient.ts +++ b/plugins/playlist/src/api/PlaylistClient.ts @@ -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 { + async getPlaylistEntities(playlistId: string): Promise { const baseUrl = await this.discoveryApi.getBaseUrl('playlist'); const resp = await this.fetchApi.fetch( `${baseUrl}/${playlistId}/entities`, diff --git a/plugins/playlist/src/components/EntityPlaylistDialog/EntityPlaylistDialog.tsx b/plugins/playlist/src/components/EntityPlaylistDialog/EntityPlaylistDialog.tsx index 474a9abb1f..247f737f3f 100644 --- a/plugins/playlist/src/components/EntityPlaylistDialog/EntityPlaylistDialog.tsx +++ b/plugins/playlist/src/components/EntityPlaylistDialog/EntityPlaylistDialog.tsx @@ -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(); diff --git a/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.test.tsx b/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.test.tsx index 3dfd89fda9..f7720b25c1 100644 --- a/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.test.tsx +++ b/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.test.tsx @@ -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 = { post: jest.fn() }; - const playlistApi: Partial = { - 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 = { - getEntities: jest + const playlistApi: Partial = { + 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', () => { new Map() }}> { 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(); diff --git a/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.tsx b/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.tsx index 0fa78b19bd..f499f59a61 100644 --- a/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.tsx +++ b/plugins/playlist/src/components/PlaylistPage/PlaylistEntitiesTable.tsx @@ -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) => ( )), diff --git a/yarn.lock b/yarn.lock index d55434a22a..02659263e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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