From 326aa376ef3c4e0ea846b7981180e97ab541ef95 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 25 Apr 2023 22:47:00 +0200 Subject: [PATCH 1/9] catalog-react: humanizeEntity add defaultName Signed-off-by: Vincenzo Scamporlino --- .../components/EntityRefLink/humanize.test.ts | 18 ++++++++++------ .../src/components/EntityRefLink/humanize.ts | 21 +++++++------------ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts b/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts index e7d1649a29..1182a7778d 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts +++ b/plugins/catalog-react/src/components/EntityRefLink/humanize.test.ts @@ -215,9 +215,12 @@ describe('humanizeEntityRef', () => { 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), + humanizeEntity( + { + metadata: { name: 'my-entity', title: 'My Title' }, + } as Entity, + 'default', + ), ).toBe('My Title'); }); @@ -257,13 +260,16 @@ describe('humanizeEntity', () => { ])( 'gives a readable name for kind %s when one is provided at spec.profile.displayName', (_, entity: Entity, expected) => { - expect(humanizeEntity(entity)).toBe(expected); + expect(humanizeEntity(entity, 'default')).toBe(expected); }, ); it('should pass through to humanizeEntityRef when nothing matches', () => { expect( - humanizeEntity({ kind: 'Group', metadata: { name: 'test' } } as Entity), - ).toBe('group:test'); + humanizeEntity( + { kind: 'Group', metadata: { name: 'test' } } as Entity, + 'default', + ), + ).toBe('default'); }); }); diff --git a/plugins/catalog-react/src/components/EntityRefLink/humanize.ts b/plugins/catalog-react/src/components/EntityRefLink/humanize.ts index 7f1501280a..478f0dce9d 100644 --- a/plugins/catalog-react/src/components/EntityRefLink/humanize.ts +++ b/plugins/catalog-react/src/components/EntityRefLink/humanize.ts @@ -25,7 +25,8 @@ import get from 'lodash/get'; * @param defaultNamespace - if set to false then namespace is never omitted, * if set to string which matches namespace of entity then omitted * - * @public */ + * @public + **/ export function humanizeEntityRef( entityRef: Entity | CompoundEntityRef, opts?: { @@ -73,27 +74,19 @@ export function humanizeEntityRef( * If an entity is either User or Group, this will be its `spec.profile.displayName`. * Otherwise, this is `metadata.title`. * - * If neither of those are found or populated, fallback to `humanizeEntityRef`. + * If neither of those are found or populated, fallback to `defaultName`. * * @param entity - Entity to convert. - * @param opts - If entity readable name is not available, opts will be used to specify humanizeEntityRef options. - * @returns Readable name, defaults to unique identifier. + * @param defaultName - If entity readable name is not available, `defaultName` will be returned. + * @returns Readable name, defaults to `defaultName`. * - * @public */ -export function humanizeEntity( - entity: Entity, - opts?: { - defaultKind?: string; - defaultNamespace?: string | false; - }, -) { +export function humanizeEntity(entity: Entity, defaultName: string) { for (const path of ['spec.profile.displayName', 'metadata.title']) { const value = get(entity, path); if (value && typeof value === 'string') { return value; } } - - return humanizeEntityRef(entity, opts); + return defaultName; } From 1271c0f71a31e784f48643e0b2b012e58451d522 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 25 Apr 2023 22:49:37 +0200 Subject: [PATCH 2/9] catalog-react: EntityOwnerPicker paginate entities Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/package.json | 1 + .../EntityOwnerPicker.test.tsx | 169 ++++++------ .../EntityOwnerPicker/EntityOwnerPicker.tsx | 243 +++++++++--------- yarn.lock | 17 ++ 4 files changed, 217 insertions(+), 213 deletions(-) diff --git a/plugins/catalog-react/package.json b/plugins/catalog-react/package.json index d5808553cb..ce3e5db783 100644 --- a/plugins/catalog-react/package.json +++ b/plugins/catalog-react/package.json @@ -60,6 +60,7 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.61", + "@react-hookz/web": "^23.0.0", "@types/react": "^16.13.1 || ^17.0.0", "classnames": "^2.2.6", "jwt-decode": "^3.1.0", diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index e4dc322c7c..86101d3579 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -14,8 +14,8 @@ * limitations under the License. */ -import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; -import { fireEvent, screen } from '@testing-library/react'; +import { Entity } from '@backstage/catalog-model'; +import { fireEvent, screen, waitFor } from '@testing-library/react'; import React from 'react'; import { MockEntityListContextProvider } from '../../testUtils/providers'; import { EntityOwnerFilter } from '../../filters'; @@ -28,8 +28,9 @@ import { } from '@backstage/test-utils'; import { catalogApiRef, CatalogApi } from '../..'; import { errorApiRef } from '@backstage/core-plugin-api'; +import { QueryEntitiesCursorRequest } from '@backstage/catalog-client'; -const ownerEntities: Entity[] = [ +const ownerEntitiesBatch1: Entity[] = [ { apiVersion: '1', kind: 'Group', @@ -68,64 +69,52 @@ const ownerEntities: Entity[] = [ }, ]; -const sampleEntities: Entity[] = [ +const ownerEntitiesBatch2: Entity[] = [ { apiVersion: '1', - kind: 'Component', + kind: 'Group', metadata: { - name: 'component-1', + name: 'some-owner-batch-2', }, - relations: [ - { - type: 'ownedBy', - targetRef: 'group:default/some-owner', - }, - { - type: 'ownedBy', - targetRef: 'group:default/some-owner-2', - }, - ], }, { apiVersion: '1', - kind: 'Component', + kind: 'Group', metadata: { - name: 'component-2', + name: 'some-owner-2-batch-2', }, - relations: [ - { - type: 'ownedBy', - targetRef: 'group:default/another-owner', + spec: { + profile: { + displayName: 'Some Owner Batch 2', }, - { - type: 'ownedBy', - targetRef: 'group:test-namespace/another-owner-2', - }, - ], + }, }, { apiVersion: '1', - kind: 'Component', + kind: 'Group', metadata: { - name: 'component-3', + name: 'another-owner-batch-2', + title: 'Another Owner Batch 2', + }, + }, + { + apiVersion: '1', + kind: 'Group', + metadata: { + namespace: 'test-namespace', + name: 'another-owner-2-batch-2', + title: 'Another Owner in Another Namespace Batch 2', }, - relations: [ - { - type: 'ownedBy', - targetRef: 'group:default/some-owner', - }, - ], }, ]; -const getEntitiesByRefs = jest.fn(async ({ entityRefs }) => ({ - items: entityRefs.map((e: string) => - ownerEntities.find(f => stringifyEntityRef(f) === e), - ), -})); +const mockedQueryEntities: jest.MockedFn = + jest.fn(); + const mockCatalogApi: Partial = { - getEntitiesByRefs, + queryEntities: mockedQueryEntities, }; + const mockErrorApi = new MockErrorApi(); describe('', () => { @@ -134,12 +123,33 @@ describe('', () => { [errorApiRef, mockErrorApi], ); + beforeEach(() => { + jest.resetAllMocks(); + + mockedQueryEntities.mockImplementation(async request => { + const totalItems = + ownerEntitiesBatch1.length + ownerEntitiesBatch2.length; + if ((request as QueryEntitiesCursorRequest).cursor) { + return { + items: ownerEntitiesBatch2, + pageInfo: {}, + totalItems, + }; + } + return { + items: ownerEntitiesBatch1, + pageInfo: { + nextCursor: 'nextCursor', + }, + totalItems, + }; + }); + }); + it('renders all owners', async () => { await renderWithEffects( - + , @@ -147,36 +157,36 @@ describe('', () => { expect(screen.getByText('Owner')).toBeInTheDocument(); fireEvent.click(screen.getByTestId('owner-picker-expand')); + + await waitFor(() => + expect(screen.getByText('Another Owner')).toBeInTheDocument(), + ); + [ - 'Another Owner', 'some-owner', 'Some Owner 2', 'Another Owner in Another Namespace', ].forEach(owner => { expect(screen.getByText(owner)).toBeInTheDocument(); }); - }); - it('renders unique owners in alphabetical order', async () => { - await renderWithEffects( - - - - - , + expect(mockedQueryEntities).toHaveBeenCalledTimes(1); + + fireEvent.scroll(screen.getByTestId('owner-picker-listbox')); + + await waitFor(() => + expect(screen.getByText('some-owner-batch-2')).toBeInTheDocument(), ); - expect(screen.getByText('Owner')).toBeInTheDocument(); - fireEvent.click(screen.getByTestId('owner-picker-expand')); + [ + 'some-owner-batch-2', + 'Some Owner Batch 2', + 'Another Owner in Another Namespace Batch 2', + ].forEach(owner => { + expect(screen.getByText(owner)).toBeInTheDocument(); + }); - expect(screen.getAllByRole('option').map(o => o.textContent)).toEqual([ - 'Another Owner', - 'Another Owner in Another Namespace', - 'some-owner', - 'Some Owner 2', - ]); + expect(mockedQueryEntities).toHaveBeenCalledTimes(2); }); it('respects the query parameter filter value', async () => { @@ -186,8 +196,6 @@ describe('', () => { ', () => { @@ -222,6 +228,8 @@ describe('', () => { }); fireEvent.click(screen.getByTestId('owner-picker-expand')); + await waitFor(() => screen.getByText('some-owner')); + fireEvent.click(screen.getByText('some-owner')); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['group:default/some-owner']), @@ -234,8 +242,6 @@ describe('', () => { ', () => { }); fireEvent.click(screen.getByTestId('owner-picker-expand')); - expect(screen.getByLabelText('some-owner')).toBeChecked(); + await waitFor(() => + expect(screen.getByLabelText('some-owner')).toBeChecked(), + ); fireEvent.click(screen.getByLabelText('some-owner')); expect(updateFilters).toHaveBeenLastCalledWith({ @@ -265,7 +273,6 @@ describe('', () => { value={{ updateFilters, queryParameters: { owners: ['team-a'] }, - backendEntities: sampleEntities, }} > @@ -281,7 +288,6 @@ describe('', () => { value={{ updateFilters, queryParameters: { owners: ['team-b'] }, - backendEntities: sampleEntities, }} > @@ -292,23 +298,4 @@ describe('', () => { owners: new EntityOwnerFilter(['group:default/team-b']), }); }); - it('removes owners from filters if there are none available', async () => { - const updateFilters = jest.fn(); - 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 8c4f2bfea0..e5ca1b86f4 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -14,17 +14,11 @@ * limitations under the License. */ -import { - Entity, - parseEntityRef, - RELATION_OWNED_BY, - stringifyEntityRef, -} from '@backstage/catalog-model'; +import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Box, Checkbox, FormControlLabel, - makeStyles, TextField, Typography, } from '@material-ui/core'; @@ -35,38 +29,69 @@ import { Autocomplete } from '@material-ui/lab'; import React, { useEffect, useMemo, useState } from 'react'; import { useEntityList } from '../../hooks/useEntityListProvider'; import { EntityOwnerFilter } from '../../filters'; -import { getEntityRelations } from '../../utils'; -import useAsync from 'react-use/lib/useAsync'; -import { errorApiRef, useApi } from '@backstage/core-plugin-api'; +import { useApi } from '@backstage/core-plugin-api'; import { catalogApiRef } from '../../api'; -import { humanizeEntity, humanizeEntityRef } from '../EntityRefLink/humanize'; +import useAsyncFn from 'react-use/lib/useAsyncFn'; +import { useDebouncedEffect } from '@react-hookz/web'; +import PersonIcon from '@material-ui/icons/Person'; +import GroupIcon from '@material-ui/icons/Group'; +import { humanizeEntity } from '../EntityRefLink/humanize'; /** @public */ export type CatalogReactEntityOwnerPickerClassKey = 'input'; -const useStyles = makeStyles( - { - input: {}, - }, - { - name: 'CatalogReactEntityOwnerPicker', - }, -); - const icon = ; const checkedIcon = ; /** @public */ export const EntityOwnerPicker = () => { - const classes = useStyles(); const { updateFilters, - backendEntities, filters, queryParameters: { owners: ownersParameter }, } = useEntityList(); + const catalogApi = useApi(catalogApiRef); - const errorApi = useApi(errorApiRef); + const [text, setText] = useState(''); + + const [{ value, loading }, handleFetch] = useAsyncFn( + async (request: { text: string } | { cursor: string; prev: Entity[] }) => { + const initialRequest = request as { text: string }; + const cursorRequest = request as { cursor: string; prev: Entity[] }; + const limit = 20; + + if (cursorRequest.cursor) { + const response = await catalogApi.queryEntities({ + cursor: cursorRequest.cursor, + limit, + }); + return { + ...response, + items: [...cursorRequest.prev, ...response.items], + }; + } + + return catalogApi.queryEntities({ + fullTextFilter: { + term: initialRequest.text || '', + fields: [ + 'metadata.name', + 'kind', + 'spec.profile.displayname', + 'metadata.title', + ], + }, + filter: { kind: ['User', 'Group'] }, + orderFields: [{ field: 'metadata.name', order: 'asc' }], + limit, + }); + }, + [text], + ); + + useDebouncedEffect(() => handleFetch({ text }), [text], 250); + + const availableOwners = value?.items || []; const queryParamOwners = useMemo( () => [ownersParameter].flat().filter(Boolean) as string[], @@ -74,72 +99,9 @@ export const EntityOwnerPicker = () => { ); const [selectedOwners, setSelectedOwners] = useState( - queryParamOwners.length - ? new EntityOwnerFilter(queryParamOwners).values - : filters.owners?.values ?? [], + queryParamOwners.length ? queryParamOwners : filters.owners?.values ?? [], ); - const { - loading, - error, - value: ownerEntities, - } = useAsync(async () => { - const ownerEntityRefs = [ - ...new Set( - backendEntities - .flatMap((e: Entity) => - getEntityRelations(e, RELATION_OWNED_BY).map(o => - stringifyEntityRef(o), - ), - ) - .filter(Boolean) as string[], - ), - ]; - const { items: ownerEntitiesOrNull } = await catalogApi.getEntitiesByRefs({ - entityRefs: ownerEntityRefs, - fields: [ - 'kind', - 'metadata.name', - 'metadata.title', - 'metadata.namespace', - 'spec.profile.displayName', - ], - }); - const owners = ownerEntitiesOrNull.map((entity, index) => { - if (entity) { - return { - label: humanizeEntity(entity, { defaultKind: 'Group' }), - entityRef: stringifyEntityRef(entity), - }; - } - return { - label: humanizeEntityRef(parseEntityRef(ownerEntityRefs[index]), { - defaultKind: 'group', - }), - entityRef: ownerEntityRefs[index], - }; - }); - - return owners.sort((a, b) => - a.label.localeCompare(b.label, 'en-US', { - ignorePunctuation: true, - caseFirst: 'upper', - }), - ); - }, [backendEntities]); - - useEffect(() => { - if (error) { - errorApi.post( - { - ...error, - message: `EntityOwnerPicker failed to initialize: ${error.message}`, - }, - {}, - ); - } - }, [error, errorApi]); - // Set selected owners on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { @@ -150,17 +112,12 @@ export const EntityOwnerPicker = () => { }, [queryParamOwners]); useEffect(() => { - if (!loading && ownerEntities) { - updateFilters({ - owners: - selectedOwners.length && ownerEntities.length - ? new EntityOwnerFilter(selectedOwners) - : undefined, - }); - } - }, [selectedOwners, updateFilters, ownerEntities, loading]); - - if (!loading && !ownerEntities?.length) return null; + updateFilters({ + owners: selectedOwners.length + ? new EntityOwnerFilter(selectedOwners) + : undefined, + }); + }, [selectedOwners, updateFilters]); return ( @@ -170,38 +127,80 @@ export const EntityOwnerPicker = () => { multiple disableCloseOnSelect loading={loading} - options={ownerEntities || []} - value={ - ownerEntities?.filter(e => - selectedOwners.some((f: string) => f === e.entityRef), - ) ?? [] - } - onChange={(_: object, value: { entityRef: string }[]) => - setSelectedOwners(value.map(e => e.entityRef)) - } - getOptionLabel={option => option.label} - renderOption={(option, { selected }) => ( - - } - onClick={event => event.preventDefault()} - label={option.label} - /> - )} + options={availableOwners} + value={selectedOwners as unknown as Entity[]} + getOptionSelected={(o, v) => { + if (typeof v === 'string') { + return stringifyEntityRef(o) === v; + } + return o === v; + }} + onChange={(_: object, owners) => { + setText(''); + setSelectedOwners( + owners.map(e => + typeof e === 'string' ? e : stringifyEntityRef(e), + ), + ); + }} + filterOptions={x => x} + renderOption={(entity, { selected }) => { + const isGroup = entity.kind === 'Group'; + + return ( + + } + onClick={event => event.preventDefault()} + label={ + + {isGroup ? ( + + ) : ( + + )} +   + {humanizeEntity(entity, entity.metadata.name)} + + } + /> + ); + }} size="small" popupIcon={} renderInput={params => ( { + setText(e.currentTarget.value); + }} variant="outlined" /> )} + ListboxProps={{ + onScroll: (e: React.MouseEvent) => { + const element = e.currentTarget; + const hasReachedEnd = + Math.abs( + element.scrollHeight - + element.clientHeight - + element.scrollTop, + ) < 1; + + if (hasReachedEnd && value?.pageInfo.nextCursor) { + handleFetch({ + cursor: value.pageInfo.nextCursor, + prev: value.items, + }); + } + }, + 'data-testid': 'owner-picker-listbox', + }} /> diff --git a/yarn.lock b/yarn.lock index f91790c2a5..beab34f402 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5823,6 +5823,7 @@ __metadata: "@material-ui/core": ^4.12.2 "@material-ui/icons": ^4.9.1 "@material-ui/lab": 4.0.0-alpha.61 + "@react-hookz/web": ^23.0.0 "@testing-library/dom": ^8.0.0 "@testing-library/jest-dom": ^5.10.1 "@testing-library/react": ^12.1.3 @@ -13863,6 +13864,22 @@ __metadata: languageName: node linkType: hard +"@react-hookz/web@npm:^23.0.0": + version: 23.0.0 + resolution: "@react-hookz/web@npm:23.0.0" + dependencies: + "@react-hookz/deep-equal": ^1.0.4 + peerDependencies: + js-cookie: ^3.0.1 + react: ^16.8 || ^17 || ^18 + react-dom: ^16.8 || ^17 || ^18 + peerDependenciesMeta: + js-cookie: + optional: true + checksum: 230bff62291bcafa65e0b9adcc3f0769fd63fabc226c77149e7797d1ea67362e9ad581ceff3cdfe2949698798d2deefb223c05bdfed1e465b5d92298d48cf3e5 + languageName: node + linkType: hard + "@remix-run/router@npm:1.3.2": version: 1.3.2 resolution: "@remix-run/router@npm:1.3.2" From cb4c15989b6bfee4903a1881df56bda4c2dcecdf Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 25 Apr 2023 23:03:31 +0200 Subject: [PATCH 3/9] catalog-react: add EntityOwnerPicker changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/little-boats-think.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/little-boats-think.md diff --git a/.changeset/little-boats-think.md b/.changeset/little-boats-think.md new file mode 100644 index 0000000000..96bf9685e5 --- /dev/null +++ b/.changeset/little-boats-think.md @@ -0,0 +1,9 @@ +--- +'@backstage/plugin-catalog-react': minor +--- + +`EntityOwnerPicker` now loads entities asynchronously + +**BREAKING**: In order to improve the performance of the component, the users and groups displayed by `EntityOwnerPicker` aren't inferred +anymore by the entities available in the `EntityListContext` and are not affected by the filters applied to the `EntityListContext`. +Instead, the entities are displayed in batches, loaded asynchronously on scroll and filtered server side. From a2f46987f7729cd1966eaa7a018d6d263941112f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 25 Apr 2023 23:52:00 +0200 Subject: [PATCH 4/9] catalog-react: hide EntityOwnerPicker if user or group kind is selected Signed-off-by: Vincenzo Scamporlino --- .../components/EntityOwnerPicker/EntityOwnerPicker.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index e5ca1b86f4..7eae97b73a 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -119,6 +119,14 @@ export const EntityOwnerPicker = () => { }); }, [selectedOwners, updateFilters]); + if ( + ['user', 'group'].includes( + filters.kind?.value.toLocaleLowerCase('en-US') || '', + ) + ) { + return null; + } + return ( From 85b6e588ebabd850ac8e149cf92305ca70f64479 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 27 Apr 2023 15:20:04 +0200 Subject: [PATCH 5/9] Update .changeset/little-boats-think.md Co-authored-by: Aramis <34432188+sennyeya@users.noreply.github.com> Signed-off-by: Vincenzo Scamporlino --- .changeset/little-boats-think.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/little-boats-think.md b/.changeset/little-boats-think.md index 96bf9685e5..a17cee4ec7 100644 --- a/.changeset/little-boats-think.md +++ b/.changeset/little-boats-think.md @@ -5,5 +5,5 @@ `EntityOwnerPicker` now loads entities asynchronously **BREAKING**: In order to improve the performance of the component, the users and groups displayed by `EntityOwnerPicker` aren't inferred -anymore by the entities available in the `EntityListContext` and are not affected by the filters applied to the `EntityListContext`. +anymore by the entities available in the `EntityListContext` and will no longer react to changes in the filters of `EntityListContext`. Instead, the entities are displayed in batches, loaded asynchronously on scroll and filtered server side. From 3648a4ebb86796f74bb319158ce6f160dcd6c819 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 15 May 2023 14:53:43 +0200 Subject: [PATCH 6/9] catalog-react: humanize display selected entities Signed-off-by: Vincenzo Scamporlino --- .../EntityOwnerPicker.test.tsx | 74 ++++++++++++++++++- .../EntityOwnerPicker/EntityOwnerPicker.tsx | 58 ++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index 86101d3579..e5497d022a 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -111,8 +111,12 @@ const ownerEntitiesBatch2: Entity[] = [ const mockedQueryEntities: jest.MockedFn = jest.fn(); +const mockedGetEntitiesByRef: jest.MockedFn = + jest.fn(); + const mockCatalogApi: Partial = { queryEntities: mockedQueryEntities, + getEntitiesByRefs: mockedGetEntitiesByRef, }; const mockErrorApi = new MockErrorApi(); @@ -146,7 +150,7 @@ describe('', () => { }); }); - it('renders all owners', async () => { + it('renders all users and groups', async () => { await renderWithEffects( @@ -171,6 +175,7 @@ describe('', () => { }); expect(mockedQueryEntities).toHaveBeenCalledTimes(1); + expect(mockedGetEntitiesByRef).not.toHaveBeenCalled(); fireEvent.scroll(screen.getByTestId('owner-picker-listbox')); @@ -205,11 +210,71 @@ describe('', () => { , ); + expect(mockedGetEntitiesByRef).toHaveBeenCalledWith({ + entityRefs: ['another-owner'], + }); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['group:default/another-owner']), }); }); + it('should display the selected owners as humanized entities', async () => { + const updateFilters = jest.fn(); + const queryParameters = { owners: ['another-owner'] }; + + mockedGetEntitiesByRef.mockResolvedValue({ + items: [ + { + metadata: { + name: 'another-owner', + title: 'Beautiful display name', + namespace: 'default', + }, + apiVersion: '1', + kind: 'group', + }, + ], + }); + await renderWithEffects( + + + + + , + ); + + await waitFor(() => + expect( + screen.getByRole('button', { + name: 'Beautiful display name', + }), + ).toBeInTheDocument(), + ); + + expect(mockedGetEntitiesByRef).toHaveBeenCalledWith({ + entityRefs: ['another-owner'], + }); + + fireEvent.click(screen.getByTestId('owner-picker-expand')); + await waitFor(() => screen.getByText('Some Owner 2')); + fireEvent.click(screen.getByText('Some Owner 2')); + + expect(mockedGetEntitiesByRef).toHaveBeenCalledTimes(1); + + await waitFor(() => + expect( + screen.getByRole('button', { + name: 'Some Owner 2', + }), + ).toBeInTheDocument(), + ); + }); + it('adds owners to filters', async () => { const updateFilters = jest.fn(); await renderWithEffects( @@ -223,6 +288,7 @@ describe('', () => { , ); + expect(mockedGetEntitiesByRef).not.toHaveBeenCalled(); expect(updateFilters).toHaveBeenLastCalledWith({ owners: undefined, }); @@ -250,6 +316,9 @@ describe('', () => { , ); + expect(mockedGetEntitiesByRef).toHaveBeenCalledWith({ + entityRefs: ['group:default/some-owner'], + }); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['group:default/some-owner']), }); @@ -279,6 +348,9 @@ describe('', () => { , ); + expect(mockedGetEntitiesByRef).toHaveBeenCalledWith({ + entityRefs: ['team-a'], + }); expect(updateFilters).toHaveBeenLastCalledWith({ owners: new EntityOwnerFilter(['group:default/team-a']), }); diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 7eae97b73a..2b85b49434 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -26,11 +26,12 @@ import CheckBoxIcon from '@material-ui/icons/CheckBox'; import CheckBoxOutlineBlankIcon from '@material-ui/icons/CheckBoxOutlineBlank'; import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; import { Autocomplete } from '@material-ui/lab'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; import { useEntityList } from '../../hooks/useEntityListProvider'; import { EntityOwnerFilter } from '../../filters'; import { useApi } from '@backstage/core-plugin-api'; import { catalogApiRef } from '../../api'; +import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn'; import { useDebouncedEffect } from '@react-hookz/web'; import PersonIcon from '@material-ui/icons/Person'; @@ -102,6 +103,8 @@ export const EntityOwnerPicker = () => { queryParamOwners.length ? queryParamOwners : filters.owners?.values ?? [], ); + const { getEntity, setEntity } = useSelectedOwners(selectedOwners); + // Set selected owners on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { @@ -143,12 +146,26 @@ export const EntityOwnerPicker = () => { } return o === v; }} + getOptionLabel={o => { + const entity = typeof o === 'string' ? getEntity(o) || o : o; + + return typeof entity === 'string' + ? entity + : humanizeEntity(entity, entity.metadata.name); + }} onChange={(_: object, owners) => { setText(''); setSelectedOwners( - owners.map(e => - typeof e === 'string' ? e : stringifyEntityRef(e), - ), + owners.map(e => { + const entityRef = + typeof e === 'string' ? e : stringifyEntityRef(e); + + if (typeof e !== 'string') { + setEntity(e); + } + + return entityRef; + }), ); }} filterOptions={x => x} @@ -214,3 +231,36 @@ export const EntityOwnerPicker = () => { ); }; + +/** + * Hook used for storing the full entity of the specified owners + * in order to display users and group using the information contained on each entity. + * When a component is rendered for the first time, it loads the content of the entities + * specified by `initialSelectedOwnersRefs` and export the `getEntity` and `setEntity` + * utilities, used to retrieve and modify the owners. + */ +function useSelectedOwners(initialSelectedOwnersRefs: string[]) { + const allEntities = useRef>({}); + const catalogApi = useApi(catalogApiRef); + + useAsync(async () => { + if (initialSelectedOwnersRefs.length === 0) { + return; + } + const initialSelectedEntities = await catalogApi.getEntitiesByRefs({ + entityRefs: initialSelectedOwnersRefs, + }); + initialSelectedEntities.items.forEach(e => { + if (e) { + allEntities.current[stringifyEntityRef(e)] = e; + } + }); + }, []); + + return { + getEntity: (entityRef: string) => allEntities.current[entityRef], + setEntity: (entity: Entity) => { + allEntities.current[stringifyEntityRef(entity)] = entity; + }, + }; +} From 2da0dcea62aef213aa6830149e2c4b1c5ba09259 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 15 May 2023 21:11:12 +0200 Subject: [PATCH 7/9] catalog-react: rollback styles Signed-off-by: Vincenzo Scamporlino --- .../EntityOwnerPicker/EntityOwnerPicker.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 2b85b49434..8b223c50b1 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -21,6 +21,7 @@ import { FormControlLabel, TextField, Typography, + makeStyles, } from '@material-ui/core'; import CheckBoxIcon from '@material-ui/icons/CheckBox'; import CheckBoxOutlineBlankIcon from '@material-ui/icons/CheckBoxOutlineBlank'; @@ -41,11 +42,21 @@ import { humanizeEntity } from '../EntityRefLink/humanize'; /** @public */ export type CatalogReactEntityOwnerPickerClassKey = 'input'; +const useStyles = makeStyles( + { + input: {}, + }, + { + name: 'CatalogReactEntityOwnerPicker', + }, +); + const icon = ; const checkedIcon = ; /** @public */ export const EntityOwnerPicker = () => { + const classes = useStyles(); const { updateFilters, filters, @@ -201,6 +212,7 @@ export const EntityOwnerPicker = () => { renderInput={params => ( { setText(e.currentTarget.value); }} From 73f9b42c11d4bb83afcc0f5ef5a82d706190ef94 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 15 May 2023 21:16:34 +0200 Subject: [PATCH 8/9] catalog-react: patch changeset and rewording Signed-off-by: Vincenzo Scamporlino --- .changeset/little-boats-think.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.changeset/little-boats-think.md b/.changeset/little-boats-think.md index a17cee4ec7..7c26c8d37c 100644 --- a/.changeset/little-boats-think.md +++ b/.changeset/little-boats-think.md @@ -1,9 +1,8 @@ --- -'@backstage/plugin-catalog-react': minor +'@backstage/plugin-catalog-react': patch --- -`EntityOwnerPicker` now loads entities asynchronously +The `EntityOwnerPicker` component has undergone improvements to enhance its performance. -**BREAKING**: In order to improve the performance of the component, the users and groups displayed by `EntityOwnerPicker` aren't inferred -anymore by the entities available in the `EntityListContext` and will no longer react to changes in the filters of `EntityListContext`. -Instead, the entities are displayed in batches, loaded asynchronously on scroll and filtered server side. +The component now loads entities asynchronously, resulting in improved performance and responsiveness. Instead of loading all entities upfront, they are now loaded in batches as the user scrolls. +The previous implementation inferred users and groups displayed by the `EntityOwnerPicker` component based on the entities available in the `EntityListContext`. The updated version no longer relies on the `EntityListContext` for inference, allowing for better decoupling and improved performance. From 03f618ebe0455db5be1d2cc52bf11f8bf695f3dc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 22 May 2023 11:33:41 +0200 Subject: [PATCH 9/9] Update .changeset/little-boats-think.md Signed-off-by: Patrik Oldsberg --- .changeset/little-boats-think.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/little-boats-think.md b/.changeset/little-boats-think.md index 7c26c8d37c..d389bfbbb1 100644 --- a/.changeset/little-boats-think.md +++ b/.changeset/little-boats-think.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-catalog-react': patch +'@backstage/plugin-catalog-react': minor --- The `EntityOwnerPicker` component has undergone improvements to enhance its performance.