From e5ce16aa44a81acb373ab39570349f94d62c378e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 18 Aug 2023 13:25:19 +0200 Subject: [PATCH] address comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/heavy-experts-accept.md | 2 +- .../defaultEntityPresentation.ts | 46 ++++++++++--------- .../EntityRefLink/EntityRefLink.test.tsx | 18 ++++---- .../UnregisterEntityDialog.test.tsx | 8 ++-- .../TemplateCard/TemplateCard.test.tsx | 13 +++--- .../General/UserSettingsIdentityCard.test.tsx | 4 +- 6 files changed, 49 insertions(+), 42 deletions(-) diff --git a/.changeset/heavy-experts-accept.md b/.changeset/heavy-experts-accept.md index ec232d1c4d..e6f2f435b8 100644 --- a/.changeset/heavy-experts-accept.md +++ b/.changeset/heavy-experts-accept.md @@ -8,7 +8,7 @@ iconography etc) are represented in the user interface. Usage of this API is initially added to the `EntityRefLink` and `EntityRefLinks` components, so that they can render richer, more correct representation of -entity refs. There's also a new `EntityName` component, which works just like +entity refs. There's also a new `EntityDisplayName` component, which works just like the `EntityRefLink` but without the link. Along with that change, the `fetchEntities` and `getTitle` props of diff --git a/plugins/catalog-react/src/apis/EntityPresentationApi/defaultEntityPresentation.ts b/plugins/catalog-react/src/apis/EntityPresentationApi/defaultEntityPresentation.ts index 31ed742a8d..ab9ea68060 100644 --- a/plugins/catalog-react/src/apis/EntityPresentationApi/defaultEntityPresentation.ts +++ b/plugins/catalog-react/src/apis/EntityPresentationApi/defaultEntityPresentation.ts @@ -85,9 +85,9 @@ export function defaultEntityPresentation( const shortRef = getShortRef({ kind, namespace, name, context }); - const primary = [displayName, title, shortRef].filter( + const primary = [displayName, title, shortRef].find( candidate => candidate && typeof candidate === 'string', - )[0]!; + )!; const secondary = [ primary !== entityRef ? entityRef : undefined, @@ -134,35 +134,35 @@ function getParts(entityOrRef: Entity | CompoundEntityRef | string): { } if (typeof entityOrRef === 'object' && entityOrRef !== null) { - const kind = [get(entityOrRef, 'kind')].filter( + const kind = [get(entityOrRef, 'kind')].find( candidate => candidate && typeof candidate === 'string', - )[0]; + ); const namespace = [ get(entityOrRef, 'metadata.namespace'), get(entityOrRef, 'namespace'), - ].filter(candidate => candidate && typeof candidate === 'string')[0]; + ].find(candidate => candidate && typeof candidate === 'string'); const name = [ get(entityOrRef, 'metadata.name'), get(entityOrRef, 'name'), - ].filter(candidate => candidate && typeof candidate === 'string')[0]; + ].find(candidate => candidate && typeof candidate === 'string'); - const title = [get(entityOrRef, 'metadata.title')].filter( + const title = [get(entityOrRef, 'metadata.title')].find( candidate => candidate && typeof candidate === 'string', - )[0]; + ); - const description = [get(entityOrRef, 'metadata.description')].filter( + const description = [get(entityOrRef, 'metadata.description')].find( candidate => candidate && typeof candidate === 'string', - )[0]; + ); - const displayName = [get(entityOrRef, 'spec.profile.displayName')].filter( + const displayName = [get(entityOrRef, 'spec.profile.displayName')].find( candidate => candidate && typeof candidate === 'string', - )[0]; + ); - const type = [get(entityOrRef, 'spec.type')].filter( + const type = [get(entityOrRef, 'spec.type')].find( candidate => candidate && typeof candidate === 'string', - )[0]; + ); return { kind, namespace, name, title, description, displayName, type }; } @@ -177,23 +177,27 @@ function getShortRef(options: { context?: { defaultKind?: string; defaultNamespace?: string }; }): string { const kind = options.kind?.toLocaleLowerCase('en-US') || 'unknown'; - const namespace = - options.namespace?.toLocaleLowerCase('en-US') || DEFAULT_NAMESPACE; - const name = options.name?.toLocaleLowerCase('en-US') || 'unknown'; - const defaultKind = options.context?.defaultKind?.toLocaleLowerCase('en-US'); - const defaultNamespace = + const namespace = options.namespace || DEFAULT_NAMESPACE; + const name = options.name || 'unknown'; + const defaultKindLower = + options.context?.defaultKind?.toLocaleLowerCase('en-US'); + const defaultNamespaceLower = options.context?.defaultNamespace?.toLocaleLowerCase('en-US'); let result = name; if ( - (defaultNamespace && namespace !== defaultNamespace) || + (defaultNamespaceLower && + namespace.toLocaleLowerCase('en-US') !== defaultNamespaceLower) || namespace !== DEFAULT_NAMESPACE ) { result = `${namespace}/${result}`; } - if (defaultKind && kind !== defaultKind) { + if ( + defaultKindLower && + kind.toLocaleLowerCase('en-US') !== defaultKindLower + ) { result = `${kind}:${result}`; } diff --git a/plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.test.tsx b/plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.test.tsx index 65d9be2190..c1a37b9682 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.test.tsx +++ b/plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.test.tsx @@ -40,7 +40,7 @@ describe('', () => { '/catalog/:namespace/:kind/:name/*': entityRouteRef, }, }); - expect(screen.getByText('component:software').closest('a')).toHaveAttribute( + expect(screen.getByText('software').closest('a')).toHaveAttribute( 'href', '/catalog/default/component/software', ); @@ -65,9 +65,10 @@ describe('', () => { '/catalog/:namespace/:kind/:name/*': entityRouteRef, }, }); - expect( - screen.getByText('component:test/software').closest('a'), - ).toHaveAttribute('href', '/catalog/test/component/software'); + expect(screen.getByText('test/software').closest('a')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); it('renders link for entity and hides default kind', async () => { @@ -109,7 +110,7 @@ describe('', () => { '/catalog/:namespace/:kind/:name/*': entityRouteRef, }, }); - expect(screen.getByText('component:software').closest('a')).toHaveAttribute( + expect(screen.getByText('software').closest('a')).toHaveAttribute( 'href', '/catalog/default/component/software', ); @@ -126,9 +127,10 @@ describe('', () => { '/catalog/:namespace/:kind/:name/*': entityRouteRef, }, }); - expect( - screen.getByText('component:test/software').closest('a'), - ).toHaveAttribute('href', '/catalog/test/component/software'); + expect(screen.getByText('test/software').closest('a')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); it('renders link for entity name and hides default kind', async () => { diff --git a/plugins/catalog-react/src/components/UnregisterEntityDialog/UnregisterEntityDialog.test.tsx b/plugins/catalog-react/src/components/UnregisterEntityDialog/UnregisterEntityDialog.test.tsx index 7f139854f6..7c8ddb3afb 100644 --- a/plugins/catalog-react/src/components/UnregisterEntityDialog/UnregisterEntityDialog.test.tsx +++ b/plugins/catalog-react/src/components/UnregisterEntityDialog/UnregisterEntityDialog.test.tsx @@ -285,8 +285,8 @@ describe('UnregisterEntityDialog', () => { expect( screen.getByText(/will unregister the following entities/), ).toBeInTheDocument(); - expect(screen.getByText(/k1:ns1\/n1/)).toBeInTheDocument(); - expect(screen.getByText(/k2:ns2\/n2/)).toBeInTheDocument(); + expect(screen.getByText(/ns1\/n1/)).toBeInTheDocument(); + expect(screen.getByText(/ns2\/n2/)).toBeInTheDocument(); }); await userEvent.click(screen.getByText('Unregister Location')); @@ -333,8 +333,8 @@ describe('UnregisterEntityDialog', () => { expect( screen.getByText(/will unregister the following entities/), ).toBeInTheDocument(); - expect(screen.getByText(/k1:ns1\/n1/)).toBeInTheDocument(); - expect(screen.getByText(/k2:ns2\/n2/)).toBeInTheDocument(); + expect(screen.getByText(/ns1\/n1/)).toBeInTheDocument(); + expect(screen.getByText(/ns2\/n2/)).toBeInTheDocument(); }); await userEvent.click(screen.getByText('Advanced Options')); diff --git a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx index 2d5b203cfb..5c373411fa 100644 --- a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx +++ b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx @@ -89,7 +89,7 @@ describe('TemplateCard', () => { expect(description.querySelector('strong')).toBeInTheDocument(); }); - it('should render no descroption if none is provided through the template', async () => { + it('should render no description if none is provided through the template', async () => { const mockTemplate: TemplateEntityV1beta3 = { apiVersion: 'scaffolder.backstage.io/v1beta3', kind: 'Template', @@ -186,11 +186,12 @@ describe('TemplateCard', () => { }, ); - expect(getByRole('link', { name: 'my-test-user' })).toBeInTheDocument(); - expect(getByRole('link', { name: 'my-test-user' })).toHaveAttribute( - 'href', - '/catalog/group/default/my-test-user', - ); + expect( + getByRole('link', { name: 'group:default/my-test-user' }), + ).toBeInTheDocument(); + expect( + getByRole('link', { name: 'group:default/my-test-user' }), + ).toHaveAttribute('href', '/catalog/group/default/my-test-user'); }); it('should call the onSelected handler when clicking the choose button', async () => { diff --git a/plugins/user-settings/src/components/General/UserSettingsIdentityCard.test.tsx b/plugins/user-settings/src/components/General/UserSettingsIdentityCard.test.tsx index 0129542b03..1be797980b 100644 --- a/plugins/user-settings/src/components/General/UserSettingsIdentityCard.test.tsx +++ b/plugins/user-settings/src/components/General/UserSettingsIdentityCard.test.tsx @@ -45,7 +45,7 @@ describe('', () => { }, ); - expect(screen.getByText('user:test-ownership')).toBeInTheDocument(); - expect(screen.getByText('foo:bar/foobar')).toBeInTheDocument(); + expect(screen.getByText('test-ownership')).toBeInTheDocument(); + expect(screen.getByText('bar/foobar')).toBeInTheDocument(); }); });