From c66531a8ebc78ca510fdc46120e107fdc7c9a11a Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 10 Feb 2023 18:46:00 -0500 Subject: [PATCH] Adding test cases and cleaning up. Signed-off-by: Aramis Sennyey --- .../EntityOwnerPicker.test.tsx | 257 ++++++++++++------ .../EntityOwnerPicker/EntityOwnerPicker.tsx | 26 +- .../components/EntityRefLink/humanize.test.ts | 59 +++- .../src/components/EntityRefLink/humanize.ts | 26 ++ .../src/components/EntityRefLink/index.ts | 2 +- 5 files changed, 280 insertions(+), 90 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index b6e928b519..652555cf64 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -15,11 +15,38 @@ */ import { Entity, parseEntityRef } from '@backstage/catalog-model'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, screen } from '@testing-library/react'; import React from 'react'; import { MockEntityListContextProvider } from '../../testUtils/providers'; import { EntityOwnerFilter } from '../../filters'; import { EntityOwnerPicker } from './EntityOwnerPicker'; +import { ApiProvider } from '@backstage/core-app-api'; +import { renderWithEffects, TestApiRegistry } from '@backstage/test-utils'; +import { catalogApiRef, CatalogApi } from '../..'; + +const ownerEntities: Entity[] = [ + { + apiVersion: '1', + kind: 'Group', + metadata: { + name: 'some-owner', + }, + }, + { + apiVersion: '1', + kind: 'Group', + metadata: { + name: 'some-owner-2', + }, + }, + { + apiVersion: '1', + kind: 'Group', + metadata: { + name: 'another-owner', + }, + }, +]; const sampleEntities: Entity[] = [ { @@ -67,14 +94,68 @@ const sampleEntities: Entity[] = [ }, ]; +const getEntitiesByRefs = jest.fn(async ({ entityRefs }) => ({ + items: entityRefs.map((e: string) => + ownerEntities.find(f => f.metadata.name === e), + ), +})); +const mockCatalogApi: Partial = { + getEntitiesByRefs, +}; + describe('', () => { - it('renders all owners', () => { - render( - - - , + const mockApis = TestApiRegistry.from([catalogApiRef, mockCatalogApi]); + + it('renders display name when available', async () => { + const names: Record = { + 'some-owner': 'Some Team', + 'some-owner-2': 'Other Team', + 'another-owner': 'AnotherTeam', + }; + getEntitiesByRefs.mockResolvedValueOnce({ + items: ownerEntities.map(e => { + e.spec = { + ...e.spec, + profile: { + displayName: names[e.metadata.name], + }, + }; + return e; + }), + }); + await renderWithEffects( + + + + + , + ); + expect(screen.getByText('Owner')).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId('owner-picker-expand')); + sampleEntities + .flatMap(e => e.relations?.map(r => parseEntityRef(r.targetRef).name)) + .forEach(owner => { + expect(screen.getByText(names[owner as string])).toBeInTheDocument(); + }); + }); + + /** + * All previous test cases are still applicable for the case where there is no + * owner entity returned from the API or the owner entity returned does not have + * a display name. + */ + it('renders all owners', async () => { + await renderWithEffects( + + + + + , ); expect(screen.getByText('Owner')).toBeInTheDocument(); @@ -86,13 +167,15 @@ describe('', () => { }); }); - it('renders unique owners in alphabetical order', () => { - render( - - - , + it('renders unique owners in alphabetical order', async () => { + await renderWithEffects( + + + + + , ); expect(screen.getByText('Owner')).toBeInTheDocument(); @@ -105,20 +188,22 @@ describe('', () => { ]); }); - it('respects the query parameter filter value', () => { + it('respects the query parameter filter value', async () => { const updateFilters = jest.fn(); const queryParameters = { owners: ['another-owner'] }; - render( - - - , + await renderWithEffects( + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ @@ -126,18 +211,20 @@ describe('', () => { }); }); - it('adds owners to filters', () => { + it('adds owners to filters', async () => { const updateFilters = jest.fn(); - render( - - - , + await renderWithEffects( + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ owners: undefined, @@ -150,19 +237,21 @@ describe('', () => { }); }); - it('removes owners from filters', () => { + it('removes owners from filters', async () => { const updateFilters = jest.fn(); - render( - - - , + await renderWithEffects( + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['some-owner']), @@ -176,49 +265,55 @@ describe('', () => { }); }); - it('responds to external queryParameters changes', () => { + it('responds to external queryParameters changes', async () => { const updateFilters = jest.fn(); - const rendered = render( - - - , + const rendered = await renderWithEffects( + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['team-a']), }); rendered.rerender( - - - , + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['team-b']), }); }); - it('removes owners from filters if there are none available', () => { + it('removes owners from filters if there are none available', async () => { const updateFilters = jest.fn(); - render( - - - , + await renderWithEffects( + + + + + , ); expect(updateFilters).toHaveBeenLastCalledWith({ owners: undefined, diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 72ced126e5..ac4304ce60 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -40,6 +40,7 @@ import { humanizeEntityRef } from '../EntityRefLink'; import useAsync from 'react-use/lib/useAsync'; import { useApi } from '@backstage/core-plugin-api'; import { catalogApiRef } from '../../api'; +import { humanizeEntity } from '../EntityRefLink/humanize'; /** @public */ export type CatalogReactEntityOwnerPickerClassKey = 'input'; @@ -109,9 +110,16 @@ export const EntityOwnerPicker = () => { entityRefs: availableOwners.map(ref => stringifyEntityRef(parseEntityRef(ref, { defaultKind: 'Group' })), ), + fields: [ + 'kind', + 'metadata.name', + 'metadata.title', + 'spec.profile.displayName', + ], }); return availableOwners.map( - (e, i) => items.at(i) || ({ metadata: { name: e } } as Entity), + (e, i) => + items.at(i) || ({ metadata: { name: e }, kind: 'Group' } as Entity), ); }, [availableOwners]); @@ -136,15 +144,19 @@ export const EntityOwnerPicker = () => { disableCloseOnSelect loading={loading} options={owners || []} - getOptionLabel={option => - option.metadata.title || option.metadata.name + value={ + owners?.filter(e => + selectedOwners.some( + f => f === humanizeEntityRef(e, { defaultKind: 'Group' }), + ), + ) ?? [] } - value={owners?.filter(e => - selectedOwners.some(f => f === e.metadata.name), - )} onChange={(_: object, value: Entity[]) => setSelectedOwners(value.map(e => e.metadata.name)) } + getOptionLabel={option => + humanizeEntity(option, { defaultKind: 'Group' }) + } renderOption={(option, { selected }) => ( { /> } onClick={event => event.preventDefault()} - label={option.metadata.title || option.metadata.name} + label={humanizeEntity(option, { defaultKind: 'Group' })} /> )} size="small" diff --git a/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts b/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts index 80ce2100f6..e7d1649a29 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts +++ b/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { humanizeEntityRef } from './humanize'; +import { Entity } from '@backstage/catalog-model'; +import { humanizeEntity, humanizeEntityRef } from './humanize'; describe('humanizeEntityRef', () => { it('formats entity in default namespace', () => { @@ -210,3 +211,59 @@ describe('humanizeEntityRef', () => { expect(title).toEqual('component:default/software'); }); }); + +describe('humanizeEntity', () => { + it('gives a readable name when one is provided at metadata.title', () => { + expect( + humanizeEntity({ + metadata: { name: 'my-entity', title: 'My Title' }, + } as Entity), + ).toBe('My Title'); + }); + + it.each([ + [ + 'User', + { + apiVersion: '1', + kind: 'User', + metadata: { + name: 'user-name', + }, + spec: { + profile: { + displayName: 'User Name', + }, + }, + }, + 'User Name', + ], + [ + 'Group', + { + apiVersion: '1', + kind: 'User', + metadata: { + name: 'team-name', + }, + spec: { + profile: { + displayName: 'Team Name', + }, + }, + }, + 'Team Name', + ], + ])( + 'gives a readable name for kind %s when one is provided at spec.profile.displayName', + (_, entity: Entity, expected) => { + expect(humanizeEntity(entity)).toBe(expected); + }, + ); + + it('should pass through to humanizeEntityRef when nothing matches', () => { + expect( + humanizeEntity({ kind: 'Group', metadata: { name: 'test' } } as Entity), + ).toBe('group:test'); + }); +}); diff --git a/plugins/catalog-react/src/components/EntityRefLink/humanize.ts b/plugins/catalog-react/src/components/EntityRefLink/humanize.ts index bfc8e2af84..3781935ad5 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/humanize.ts +++ b/plugins/catalog-react/src/components/EntityRefLink/humanize.ts @@ -18,6 +18,7 @@ import { Entity, CompoundEntityRef, DEFAULT_NAMESPACE, + UserEntity, } from '@backstage/catalog-model'; /** @@ -65,3 +66,28 @@ export function humanizeEntityRef( : kind; return `${kind ? `${kind}:` : ''}${namespace ? `${namespace}/` : ''}${name}`; } + +/** + * Convert an entity to its more common name. + * + * @param entity Entity to convert. + * @returns Readable name, defaults to unique identifier. + */ +export function humanizeEntity( + entity: Entity, + opts?: { + defaultKind?: string; + defaultNamespace?: string | false; + }, +) { + let title: string | undefined = undefined; + switch (entity.kind) { + case 'User': + case 'Group': + title = (entity as UserEntity).spec?.profile?.displayName; + break; + default: + title = entity.metadata.title; + } + return title || humanizeEntityRef(entity, opts); +} diff --git a/plugins/catalog-react/src/components/EntityRefLink/index.ts b/plugins/catalog-react/src/components/EntityRefLink/index.ts index 50ac3d4534..4d71e8b10e 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/index.ts +++ b/plugins/catalog-react/src/components/EntityRefLink/index.ts @@ -18,4 +18,4 @@ export { EntityRefLink } from './EntityRefLink'; export type { EntityRefLinkProps } from './EntityRefLink'; export { EntityRefLinks } from './EntityRefLinks'; export type { EntityRefLinksProps } from './EntityRefLinks'; -export { humanizeEntityRef } from './humanize'; +export { humanizeEntityRef, humanizeEntity } from './humanize';