From f8ca7b8c021671e2e14c37029c8de346961b5129 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 26 Nov 2024 15:47:39 +0100 Subject: [PATCH 1/4] catalog-react: useFacetsEntities use facets endpoint Signed-off-by: Vincenzo Scamporlino --- .../useFacetsEntities.test.ts | 182 ++++++------------ .../EntityOwnerPicker/useFacetsEntities.ts | 52 ++--- 2 files changed, 83 insertions(+), 151 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts index a3901825b3..44ff6bf21c 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts @@ -82,9 +82,6 @@ describe('useFacetsEntities', () => { mockCatalogApi.getEntityFacets.mockResolvedValue( facetsFromEntityRefs(entityRefs), ); - mockCatalogApi.getEntitiesByRefs.mockResolvedValue( - entitiesFromEntityRefs(entityRefs), - ); const { result } = renderHook(() => useFacetsEntities({ enabled: true })); @@ -110,48 +107,20 @@ describe('useFacetsEntities', () => { }); }); - it(`should return the owners sorted by namespace, (displayName or title or name) and kind`, async () => { + it(`should return the owners sorted by kind, namespace and name`, async () => { const entityRefs = [ 'group:namespace/team-b', - 'component:default/c', + 'user:default/c', 'group:default/a', - 'component:default/a', - 'component:default/b', + 'user:default/a', + 'user:default/b', 'group:default/d', 'group:default/e', ]; - const enrichedEntities: { [key: string]: Entity } = { - 'group:default/a': { - apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { name: 'a', namespace: 'default', title: 'My title A' }, - }, - 'component:default/a': { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'a', namespace: 'default', title: 'My title B' }, - }, - 'group:default/d': { - apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { name: 'd', namespace: 'default' }, - spec: { profile: { displayName: 'My display name D' } }, - }, - 'group:default/e': { - apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { name: 'e', namespace: 'default' }, - spec: { profile: { displayName: 'My display name E' } }, - }, - }; - mockCatalogApi.getEntityFacets.mockResolvedValue( facetsFromEntityRefs(entityRefs), ); - mockCatalogApi.getEntitiesByRefs.mockResolvedValue( - entitiesFromEntityRefs(entityRefs, enrichedEntities), - ); const { result } = renderHook(() => useFacetsEntities({ enabled: true })); @@ -162,49 +131,48 @@ describe('useFacetsEntities', () => { items: [ { apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'b', namespace: 'default' }, - }, - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'c', namespace: 'default' }, + kind: 'group', + metadata: { name: 'a', namespace: 'default' }, }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', metadata: { name: 'd', namespace: 'default' }, - spec: { profile: { displayName: 'My display name D' } }, }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', metadata: { name: 'e', namespace: 'default' }, - spec: { profile: { displayName: 'My display name E' } }, - }, - { - apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { - name: 'a', - namespace: 'default', - title: 'My title A', - }, - }, - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { - name: 'a', - namespace: 'default', - title: 'My title B', - }, }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', metadata: { name: 'team-b', namespace: 'namespace' }, }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { + name: 'a', + namespace: 'default', + }, + }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { + name: 'b', + namespace: 'default', + }, + }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { + name: 'c', + namespace: 'default', + }, + }, ], }, loading: false, @@ -215,18 +183,15 @@ describe('useFacetsEntities', () => { it(`should paginate the data accordingly`, async () => { const entityRefs = [ 'group:namespace/team-b', - 'component:default/c', + 'user:default/c', 'group:default/a', - 'component:default/a', - 'component:default/b', + 'user:default/a', + 'user:default/b', ]; mockCatalogApi.getEntityFacets.mockResolvedValue( facetsFromEntityRefs(entityRefs), ); - mockCatalogApi.getEntitiesByRefs.mockResolvedValue( - entitiesFromEntityRefs(entityRefs), - ); const { result } = renderHook(() => useFacetsEntities({ enabled: true })); @@ -237,13 +202,13 @@ describe('useFacetsEntities', () => { items: [ { apiVersion: 'backstage.io/v1beta1', - kind: 'component', + kind: 'group', metadata: { name: 'a', namespace: 'default' }, }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', - metadata: { name: 'a', namespace: 'default' }, + metadata: { name: 'team-b', namespace: 'namespace' }, }, ], cursor: 'eyJ0ZXh0IjoiIiwic3RhcnQiOjJ9', @@ -257,11 +222,6 @@ describe('useFacetsEntities', () => { expect(result.current[0]).toEqual({ value: { items: [ - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'a', namespace: 'default' }, - }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', @@ -269,13 +229,18 @@ describe('useFacetsEntities', () => { }, { apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'b', namespace: 'default' }, + kind: 'group', + metadata: { name: 'team-b', namespace: 'namespace' }, }, { apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'c', namespace: 'default' }, + kind: 'user', + metadata: { name: 'a', namespace: 'default' }, + }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { name: 'b', namespace: 'default' }, }, ], cursor: 'eyJ0ZXh0IjoiIiwic3RhcnQiOjR9', @@ -289,31 +254,31 @@ describe('useFacetsEntities', () => { expect(result.current[0]).toEqual({ value: { items: [ - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'a', namespace: 'default' }, - }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', metadata: { name: 'a', namespace: 'default' }, }, - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'b', namespace: 'default' }, - }, - { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'c', namespace: 'default' }, - }, { apiVersion: 'backstage.io/v1beta1', kind: 'group', metadata: { name: 'team-b', namespace: 'namespace' }, }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { name: 'a', namespace: 'default' }, + }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { name: 'b', namespace: 'default' }, + }, + { + apiVersion: 'backstage.io/v1beta1', + kind: 'user', + metadata: { name: 'c', namespace: 'default' }, + }, ], }, loading: false, @@ -337,34 +302,15 @@ describe('useFacetsEntities', () => { mockCatalogApi.getEntityFacets.mockResolvedValue( facetsFromEntityRefs(entityRefs), ); - const enrichedEntities: { [key: string]: Entity } = { - 'group:default/go': { - apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { name: 'go', namespace: 'default', title: 'Hidden Spider' }, - }, - 'component:default/lemon': { - apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'lemon', namespace: 'default' }, - spec: { - profile: { displayName: 'Lemon Spider' }, - }, - }, - }; - mockCatalogApi.getEntitiesByRefs.mockResolvedValue( - entitiesFromEntityRefs(entityRefs, enrichedEntities), - ); const { result } = renderHook(() => useFacetsEntities({ enabled: true })); result.current[1]({ text: 'der ' }); + await waitFor(() => { expect(result.current[0]).toEqual({ value: { items: [ - enrichedEntities['group:default/go'], - enrichedEntities['component:default/lemon'], { apiVersion: 'backstage.io/v1beta1', kind: 'component', @@ -372,13 +318,13 @@ describe('useFacetsEntities', () => { }, { apiVersion: 'backstage.io/v1beta1', - kind: 'group', - metadata: { name: 'spiderman', namespace: 'namespace' }, + kind: 'component', + metadata: { name: 'a-component', namespace: 'spiders' }, }, { apiVersion: 'backstage.io/v1beta1', - kind: 'component', - metadata: { name: 'a-component', namespace: 'spiders' }, + kind: 'group', + metadata: { name: 'spiderman', namespace: 'namespace' }, }, { apiVersion: 'backstage.io/v1beta1', diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.ts b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.ts index f43bba97c7..a1d7693c1d 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.ts +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.ts @@ -17,8 +17,7 @@ import { useApi } from '@backstage/core-plugin-api'; import useAsyncFn from 'react-use/esm/useAsyncFn'; import { catalogApiRef } from '../../api'; import { useState } from 'react'; -import { Entity } from '@backstage/catalog-model'; -import get from 'lodash/get'; +import { Entity, parseEntityRef } from '@backstage/catalog-model'; type FacetsCursor = { start: number; @@ -34,15 +33,13 @@ type FacetsInitialRequest = { text: string; }; -const maybeString = (value: unknown): string | undefined => - typeof value === 'string' ? value : undefined; - /** * This hook asynchronously loads the entity owners using the facets endpoint. * EntityOwnerPicker uses this hook when mode="owners-only" is passed as prop. * All the owners are kept internally in memory and rendered in batches once requested * by the frontend. The values returned by this hook are compatible with `useQueryEntities` * hook, which is also used by EntityOwnerPicker. + * In this mode, the EntityOwnerPicker won't show detailed information of the owners. */ export function useFacetsEntities({ enabled }: { enabled: boolean }) { const catalogApi = useApi(catalogApiRef); @@ -52,37 +49,30 @@ export function useFacetsEntities({ enabled }: { enabled: boolean }) { return []; } const facet = 'relations.ownedBy'; - const facetsResponse = await catalogApi.getEntityFacets({ - facets: [facet], - }); - const entityRefs = facetsResponse.facets[facet]?.map(e => e.value) ?? []; return catalogApi - .getEntitiesByRefs({ entityRefs }) - .then(resp => - resp.items - .filter(entity => entity !== undefined) - .map(entity => entity as Entity) + .getEntityFacets({ facets: [facet] }) + .then(response => + response.facets[facet] + .map(e => e.value) + .map(ref => { + const { kind, name, namespace } = parseEntityRef(ref); + return { + apiVersion: 'backstage.io/v1beta1', + kind, + metadata: { name, namespace }, + }; + }) .sort( (a, b) => - (a.metadata.namespace || '').localeCompare( - b.metadata.namespace || '', + a.kind.localeCompare(b.kind, 'en-US') || + a.metadata.namespace.localeCompare( + b.metadata.namespace, 'en-US', ) || - ( - maybeString(get(a, 'spec.profile.displayName')) || - a.metadata.title || - a.metadata.name - ).localeCompare( - maybeString(get(b, 'spec.profile.displayName')) || - b.metadata.title || - b.metadata.name, - 'en-US', - ) || - a.kind.localeCompare(b.kind, 'en-US'), + a.metadata.name.localeCompare(b.metadata.name, 'en-US'), ), ) - .then(entities => entities) .catch(() => []); }); @@ -161,10 +151,6 @@ function filterEntity(text: string, entity: Entity) { return ( entity.kind.includes(normalizedText) || entity.metadata.namespace?.includes(normalizedText) || - entity.metadata.name.includes(normalizedText) || - entity.metadata.title?.includes(normalizedText) || - (get(entity, 'spec.profile.displayName') as unknown as string)?.includes( - normalizedText, - ) + entity.metadata.name.includes(normalizedText) ); } From 4a433987d9ef1fdcfd56ceee3ffc2c559390abc5 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 26 Nov 2024 15:47:54 +0100 Subject: [PATCH 2/4] catalog:react changeset useFacetsEntities Signed-off-by: Vincenzo Scamporlino --- .changeset/strange-brooms-check.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/strange-brooms-check.md diff --git a/.changeset/strange-brooms-check.md b/.changeset/strange-brooms-check.md new file mode 100644 index 0000000000..b6cb897170 --- /dev/null +++ b/.changeset/strange-brooms-check.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-react': patch +--- + +Fixed an issue where the `EntityOwnerPicker` component failed to load when the `mode` prop was set to `owners-only`. In this mode, the `EntityOwnerPicker` does not load details about the owners, such as `displayName` or `title`. To display these details, use `mode=all` instead. From d1e89162e9d3bd9ef46450c485cef59f4a18f306 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 26 Nov 2024 16:17:36 +0100 Subject: [PATCH 3/4] catalog-react: remove unused Signed-off-by: Vincenzo Scamporlino --- .../useFacetsEntities.test.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts index 44ff6bf21c..a714b31327 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/useFacetsEntities.test.ts @@ -17,7 +17,6 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useFacetsEntities } from './useFacetsEntities'; import { catalogApiMock } from '@backstage/plugin-catalog-react/testUtils'; -import { Entity, parseEntityRef } from '@backstage/catalog-model'; const mockCatalogApi = catalogApiMock.mock(); @@ -37,25 +36,6 @@ describe('useFacetsEntities', () => { }, }); - const entitiesFromEntityRefs = ( - entityRefs: string[], - enrichedEntities: { [key: string]: Entity } = {}, - ) => ({ - items: entityRefs.map(ref => { - const compoundRef = parseEntityRef(ref); - return ( - enrichedEntities[ref] || { - apiVersion: 'backstage.io/v1beta1', - kind: compoundRef.kind, - metadata: { - name: compoundRef.name, - namespace: compoundRef.namespace, - }, - } - ); - }), - }); - it(`should return empty items when facets are loading`, () => { mockCatalogApi.getEntityFacets.mockReturnValue(new Promise(() => {})); const { result } = renderHook(() => useFacetsEntities({ enabled: true })); From 04bcdbddfb168cf2e7917c626473585627b5c533 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 26 Nov 2024 16:37:59 +0100 Subject: [PATCH 4/4] catalog-react: fix tests Signed-off-by: Vincenzo Scamporlino --- .../EntityOwnerPicker.test.tsx | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index 9d9c25a233..7591d4c3ae 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -402,17 +402,16 @@ describe('', () => { fireEvent.click(screen.getByTestId('owner-picker-expand')); + // // some-owner, some-owner-2, another-owner, another-owner-2 await waitFor(() => - expect(screen.getByText('Another Owner')).toBeInTheDocument(), + expect(screen.getByText('some-owner')).toBeInTheDocument(), ); - [ - 'some-owner', - 'Some Owner 2', - 'Another Owner in Another Namespace', - ].forEach(owner => { - expect(screen.getByText(owner)).toBeInTheDocument(); - }); + ['some-owner-2', 'another-owner', 'test-namespace/another-owner-2'].forEach( + owner => { + expect(screen.getByText(owner)).toBeInTheDocument(); + }, + ); expect(mockCatalogApi.getEntityFacets).toHaveBeenCalledTimes(1); @@ -423,9 +422,9 @@ describe('', () => { ); [ - 'some-owner-batch-2', - 'Some Owner Batch 2', - 'Another Owner in Another Namespace Batch 2', + 'some-owner-2-batch-2', + 'another-owner-batch-2', + 'test-namespace/another-owner-2-batch-2', ].forEach(owner => { expect(screen.getByText(owner)).toBeInTheDocument(); }); @@ -465,10 +464,8 @@ describe('', () => { , ); - expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ - entityRefs: [...ownerEntitiesBatch1, ...ownerEntitiesBatch2].map(entity => - stringifyEntityRef(entity), - ), + expect(mockCatalogApi.getEntityFacets).toHaveBeenCalledWith({ + facets: ['relations.ownedBy'], }); expect(updateFilters).toHaveBeenLastCalledWith({ owners: undefined,