From ff03fd55dee6b85b63ee79bdd8a1930433b46f8a Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 20 Apr 2024 12:03:21 -0300 Subject: [PATCH 01/10] feat: allow promise in EntityPresentationApi for all the asynchronous processes working under the hood Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../EntityPresentationApi.ts | 5 ++++ .../DefaultEntityPresentationApi.ts | 29 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts index 945bce1d27..de5449917a 100644 --- a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts +++ b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts @@ -109,6 +109,11 @@ export interface EntityRefPresentation { * elsewhere. */ update$?: Observable; + + /* The `promise` property in the `EntityRefPresentation` interface is defining a property named + `promise` that holds a promise. This promise resolves to an array of + `EntityRefPresentationSnapshot` objects. */ + promise: Promise; } /** diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts index 82862be435..bbd5540d15 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts @@ -298,10 +298,37 @@ export class DefaultEntityPresentationApi implements EntityPresentationApi { }; }); - return { + const entityRefPresentation: EntityRefPresentation = { snapshot: initialSnapshot, update$: observable, + get promise() { + return new Promise( + (resolve, reject) => { + if (!observable) { + resolve([initialSnapshot]); + } else { + const res: EntityRefPresentationSnapshot[] = []; + const subscription = observable.subscribe({ + next: snapshot => { + res.push(snapshot); + }, + error: error => { + initialSnapshot = { + primaryTitle: entityRef, + entityRef: entityRef, + }; + }, + complete() { + subscription.unsubscribe(); + resolve(res); + }, + }); + } + }, + ); + }, }; + return entityRefPresentation; } #getEntityForInitialRender(entityOrRef: Entity | string): { From 929cb2611873cee3d562530e0313b2ca9c727aa3 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 20 Apr 2024 12:09:43 -0300 Subject: [PATCH 02/10] feat: MultiEntityPicker uses entityPresentationApi to display entity instead of humanizeEntityRef Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../MultiEntityPicker.test.tsx | 17 ++++- .../MultiEntityPicker/MultiEntityPicker.tsx | 76 +++++++++++-------- 2 files changed, 60 insertions(+), 33 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx index bc0a5ed32d..4be07b139d 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx @@ -16,7 +16,11 @@ import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; -import { CatalogApi, catalogApiRef } from '@backstage/plugin-catalog-react'; +import { + CatalogApi, + catalogApiRef, + entityPresentationApiRef, +} from '@backstage/plugin-catalog-react'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { fireEvent, screen } from '@testing-library/react'; @@ -24,6 +28,7 @@ import React from 'react'; import { MultiEntityPicker } from './MultiEntityPicker'; import { MultiEntityPickerProps } from './schema'; import { ScaffolderRJSFFieldProps as FieldProps } from '@backstage/plugin-scaffolder-react'; +import { DefaultEntityPresentationApi } from '@backstage/plugin-catalog'; const makeEntity = (kind: string, namespace: string, name: string): Entity => ({ apiVersion: 'scaffolder.backstage.io/v1beta3', @@ -59,7 +64,15 @@ describe('', () => { ]; Wrapper = ({ children }: { children?: React.ReactNode }) => ( - + {children} ); diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index d9804a8cba..0a0fcfdc2b 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -25,7 +25,9 @@ import { import { useApi } from '@backstage/core-plugin-api'; import { catalogApiRef, - humanizeEntityRef, + entityPresentationApiRef, + EntityRefPresentationSnapshot, + EntityDisplayName, } from '@backstage/plugin-catalog-react'; import TextField from '@material-ui/core/TextField'; import FormControl from '@material-ui/core/FormControl'; @@ -64,40 +66,35 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { uiSchema['ui:options']?.defaultNamespace || undefined; const catalogApi = useApi(catalogApiRef); - + const entityPresentationApi = useApi(entityPresentationApiRef); const { value: entities, loading } = useAsync(async () => { const { items } = await catalogApi.getEntities( catalogFilter ? { filter: catalogFilter } : undefined, ); - return items; + const primaryTitles: string[] = []; + for (const item of items) { + const entityPresentation = (await entityPresentationApi.forEntity(item) + ?.promise) as EntityRefPresentationSnapshot[]; + entityPresentation.map(e => primaryTitles.push(e.primaryTitle)); + } + + return { items, primaryTitles }; }); const allowArbitraryValues = uiSchema['ui:options']?.allowArbitraryValues ?? true; - const getLabel = useCallback( - (ref: string) => { - try { - return humanizeEntityRef( - parseEntityRef(ref, { defaultKind, defaultNamespace }), - { - defaultKind, - defaultNamespace, - }, - ); - } catch (err) { - return ref; - } - }, - [defaultKind, defaultNamespace], - ); - const onSelect = useCallback( (_: any, refs: (string | Entity)[], reason: AutocompleteChangeReason) => { const values = refs .map(ref => { if (typeof ref !== 'string') { // if ref does not exist: pass 'undefined' to trigger validation for required value - return ref ? stringifyEntityRef(ref as Entity) : undefined; + return ref + ? entityPresentationApi.forEntity(ref, { + defaultKind, + defaultNamespace, + }).snapshot.entityRef + : undefined; } if (reason === 'blur' || reason === 'create-option') { // Add in default namespace, etc. @@ -126,14 +123,21 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { onChange(values); }, - [onChange, formData, defaultKind, defaultNamespace, allowArbitraryValues], + [ + onChange, + formData, + defaultKind, + defaultNamespace, + allowArbitraryValues, + entityPresentationApi, + ], ); useEffect(() => { - if (entities?.length === 1) { - onChange([stringifyEntityRef(entities[0])]); + if (entities?.items?.length === 1) { + onChange([stringifyEntityRef(entities.items[0])]); } - }, [entities, onChange]); + }, [entities?.items, onChange]); return ( { formData && formData.includes(stringifyEntityRef(e)), - ) ?? (allowArbitraryValues && formData ? formData.map(getLabel) : []) + ) ?? + (allowArbitraryValues && formData + ? entities?.primaryTitles || [] + : []) } loading={loading} onChange={onSelect} - options={entities || []} + options={entities?.items || []} + renderOption={option => } getOptionLabel={option => // option can be a string due to freeSolo. typeof option === 'string' ? option - : humanizeEntityRef(option, { defaultKind, defaultNamespace })! + : entityPresentationApi.forEntity(option, { + defaultKind, + defaultNamespace, + }).snapshot.entityRef! } autoSelect freeSolo={allowArbitraryValues} @@ -170,7 +181,10 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { label={title} margin="dense" helperText={description} - FormHelperTextProps={{ margin: 'dense', style: { marginLeft: 0 } }} + FormHelperTextProps={{ + margin: 'dense', + style: { marginLeft: 0 }, + }} variant="outlined" required={required} InputProps={params.InputProps} From 4268696352ec81671b543059ce732b1442b6ee92 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sun, 21 Apr 2024 21:57:36 -0300 Subject: [PATCH 03/10] chore: changeset added Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/olive-rockets-drum.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/olive-rockets-drum.md diff --git a/.changeset/olive-rockets-drum.md b/.changeset/olive-rockets-drum.md new file mode 100644 index 0000000000..12f80dab5c --- /dev/null +++ b/.changeset/olive-rockets-drum.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-react': minor +'@backstage/plugin-scaffolder': minor +'@backstage/plugin-catalog': minor +--- + +`MultiEntityPicker` uses `entityPresentationApi` instead of `humanizeEntityRef` to display entity. Also, `EntityPresentationApi` now allows `promise` getter for under asynchronous process of presentation api From ac6cff672e9dd1e8194a342610e77f4a2d3eeec1 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Mon, 22 Apr 2024 22:23:01 -0300 Subject: [PATCH 04/10] testcase updated for new promise getter in api Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../DefaultEntityPresentationApi.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts index 42ff777cbd..475bfe1660 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts @@ -35,6 +35,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, + promise: new Promise((resolve, reject) => resolve({})), }); expect( @@ -48,6 +49,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, + promise: new Promise((resolve, reject) => resolve({})), }); expect( @@ -63,6 +65,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, + promise: new Promise((resolve, reject) => resolve({})), }); const entity: Entity = { @@ -85,6 +88,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, + promise: new Promise((resolve, reject) => resolve({})), }); }); @@ -150,6 +154,47 @@ describe('DefaultEntityPresentationApi', () => { }), ); }); + + it('returns the correct snapshots via promise', async () => { + const catalogApi = { + getEntitiesByRefs: jest.fn(), + }; + const api = DefaultEntityPresentationApi.create({ + catalogApi: catalogApi as Partial as any, + }); + + catalogApi.getEntitiesByRefs.mockResolvedValueOnce({ + items: [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test', + namespace: 'default', + etag: 'something', + }, + spec: { + type: 'service', + }, + }, + ], + }); + + const entityRef = 'component:default/test'; + const entitySnapshot = { + entityRef: entityRef, + primaryTitle: 'test', + secondaryTitle: 'component:default/test | service', + Icon: expect.anything(), + }; + + const promise = api.forEntity(entityRef).promise; + + const snapshots = await promise; + + expect(snapshots.length).toEqual(1); // Only one snapshot expected + expect(snapshots[0]).toEqual(entitySnapshot); // Snapshot should match the simulated one + }); }); async function consumePresentation( From 30e92e77097540746d66d2d3b0330e8cf30119d9 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Mon, 22 Apr 2024 23:18:55 -0300 Subject: [PATCH 05/10] chore: lint errors resolved Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../EntityPresentationApi.ts | 2 +- .../DefaultEntityPresentationApi.test.ts | 12 ++--- .../DefaultEntityPresentationApi.ts | 46 +++++++++---------- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts index de5449917a..f4f1193718 100644 --- a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts +++ b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts @@ -113,7 +113,7 @@ export interface EntityRefPresentation { /* The `promise` property in the `EntityRefPresentation` interface is defining a property named `promise` that holds a promise. This promise resolves to an array of `EntityRefPresentationSnapshot` objects. */ - promise: Promise; + promise?: Promise; } /** diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts index 475bfe1660..c37efb204f 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts @@ -35,7 +35,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise((resolve, reject) => resolve({})), + promise: new Promise(resolve => resolve({})), }); expect( @@ -49,7 +49,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise((resolve, reject) => resolve({})), + promise: new Promise(resolve => resolve({})), }); expect( @@ -65,7 +65,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise((resolve, reject) => resolve({})), + promise: new Promise(resolve => resolve({})), }); const entity: Entity = { @@ -88,7 +88,7 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise((resolve, reject) => resolve({})), + promise: new Promise(resolve => resolve({})), }); }); @@ -192,8 +192,8 @@ describe('DefaultEntityPresentationApi', () => { const snapshots = await promise; - expect(snapshots.length).toEqual(1); // Only one snapshot expected - expect(snapshots[0]).toEqual(entitySnapshot); // Snapshot should match the simulated one + expect(snapshots?.length).toEqual(1); // Only one snapshot expected + expect(snapshots?.[0]).toEqual(entitySnapshot); // Snapshot should match the simulated one }); }); diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts index bbd5540d15..c7572cca3b 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts @@ -302,30 +302,28 @@ export class DefaultEntityPresentationApi implements EntityPresentationApi { snapshot: initialSnapshot, update$: observable, get promise() { - return new Promise( - (resolve, reject) => { - if (!observable) { - resolve([initialSnapshot]); - } else { - const res: EntityRefPresentationSnapshot[] = []; - const subscription = observable.subscribe({ - next: snapshot => { - res.push(snapshot); - }, - error: error => { - initialSnapshot = { - primaryTitle: entityRef, - entityRef: entityRef, - }; - }, - complete() { - subscription.unsubscribe(); - resolve(res); - }, - }); - } - }, - ); + return new Promise(resolve => { + if (!observable) { + resolve([initialSnapshot]); + } else { + const res: EntityRefPresentationSnapshot[] = []; + const subscription = observable.subscribe({ + next: snapshot => { + res.push(snapshot); + }, + error: () => { + initialSnapshot = { + primaryTitle: entityRef, + entityRef: entityRef, + }; + }, + complete() { + subscription.unsubscribe(); + resolve(res); + }, + }); + } + }); }, }; return entityRefPresentation; From 3383f82c58e0f344a7e530bb6096e43b11a6a763 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Fri, 26 Apr 2024 01:28:40 -0300 Subject: [PATCH 06/10] feat: no need to pass value in autocomplete as we have renderOptions Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../MultiEntityPicker/MultiEntityPicker.tsx | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index 0a0fcfdc2b..073b364720 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -71,14 +71,8 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { const { items } = await catalogApi.getEntities( catalogFilter ? { filter: catalogFilter } : undefined, ); - const primaryTitles: string[] = []; - for (const item of items) { - const entityPresentation = (await entityPresentationApi.forEntity(item) - ?.promise) as EntityRefPresentationSnapshot[]; - entityPresentation.map(e => primaryTitles.push(e.primaryTitle)); - } - return { items, primaryTitles }; + return items; }); const allowArbitraryValues = uiSchema['ui:options']?.allowArbitraryValues ?? true; @@ -134,10 +128,10 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { ); useEffect(() => { - if (entities?.items?.length === 1) { - onChange([stringifyEntityRef(entities.items[0])]); + if (entities?.length === 1) { + onChange([stringifyEntityRef(entities[0])]); } - }, [entities?.items, onChange]); + }, [entities, onChange]); return ( { formData && formData.includes(stringifyEntityRef(e)), - ) ?? - (allowArbitraryValues && formData - ? entities?.primaryTitles || [] - : []) - } loading={loading} onChange={onSelect} - options={entities?.items || []} + options={entities || []} renderOption={option => } getOptionLabel={option => // option can be a string due to freeSolo. From 90db1ce4410c44dc3574340072791aca7ca80c1f Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Fri, 26 Apr 2024 01:33:16 -0300 Subject: [PATCH 07/10] chore: eslint error resolved Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../components/fields/MultiEntityPicker/MultiEntityPicker.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index 073b364720..85e08740df 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -26,7 +26,6 @@ import { useApi } from '@backstage/core-plugin-api'; import { catalogApiRef, entityPresentationApiRef, - EntityRefPresentationSnapshot, EntityDisplayName, } from '@backstage/plugin-catalog-react'; import TextField from '@material-ui/core/TextField'; From 45446a0bfa7e1d8b902dccc9f0ed19e0e5ffdc6d Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Thu, 2 May 2024 16:08:36 -0300 Subject: [PATCH 08/10] feat: revert chnages done in EntityPresentationApi Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/olive-rockets-drum.md | 2 +- .../EntityPresentationApi.ts | 5 --- .../DefaultEntityPresentationApi.test.ts | 45 ------------------- .../DefaultEntityPresentationApi.ts | 27 +---------- 4 files changed, 2 insertions(+), 77 deletions(-) diff --git a/.changeset/olive-rockets-drum.md b/.changeset/olive-rockets-drum.md index 12f80dab5c..b5e6c2cefe 100644 --- a/.changeset/olive-rockets-drum.md +++ b/.changeset/olive-rockets-drum.md @@ -4,4 +4,4 @@ '@backstage/plugin-catalog': minor --- -`MultiEntityPicker` uses `entityPresentationApi` instead of `humanizeEntityRef` to display entity. Also, `EntityPresentationApi` now allows `promise` getter for under asynchronous process of presentation api +`MultiEntityPicker` uses `EntityDisplayName` instead of `humanizeEntityRef` to display entity. diff --git a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts index f4f1193718..945bce1d27 100644 --- a/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts +++ b/plugins/catalog-react/src/apis/EntityPresentationApi/EntityPresentationApi.ts @@ -109,11 +109,6 @@ export interface EntityRefPresentation { * elsewhere. */ update$?: Observable; - - /* The `promise` property in the `EntityRefPresentation` interface is defining a property named - `promise` that holds a promise. This promise resolves to an array of - `EntityRefPresentationSnapshot` objects. */ - promise?: Promise; } /** diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts index c37efb204f..42ff777cbd 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.test.ts @@ -35,7 +35,6 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise(resolve => resolve({})), }); expect( @@ -49,7 +48,6 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise(resolve => resolve({})), }); expect( @@ -65,7 +63,6 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise(resolve => resolve({})), }); const entity: Entity = { @@ -88,7 +85,6 @@ describe('DefaultEntityPresentationApi', () => { Icon: expect.anything(), }, update$: undefined, - promise: new Promise(resolve => resolve({})), }); }); @@ -154,47 +150,6 @@ describe('DefaultEntityPresentationApi', () => { }), ); }); - - it('returns the correct snapshots via promise', async () => { - const catalogApi = { - getEntitiesByRefs: jest.fn(), - }; - const api = DefaultEntityPresentationApi.create({ - catalogApi: catalogApi as Partial as any, - }); - - catalogApi.getEntitiesByRefs.mockResolvedValueOnce({ - items: [ - { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - name: 'test', - namespace: 'default', - etag: 'something', - }, - spec: { - type: 'service', - }, - }, - ], - }); - - const entityRef = 'component:default/test'; - const entitySnapshot = { - entityRef: entityRef, - primaryTitle: 'test', - secondaryTitle: 'component:default/test | service', - Icon: expect.anything(), - }; - - const promise = api.forEntity(entityRef).promise; - - const snapshots = await promise; - - expect(snapshots?.length).toEqual(1); // Only one snapshot expected - expect(snapshots?.[0]).toEqual(entitySnapshot); // Snapshot should match the simulated one - }); }); async function consumePresentation( diff --git a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts index c7572cca3b..82862be435 100644 --- a/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts +++ b/plugins/catalog/src/apis/EntityPresentationApi/DefaultEntityPresentationApi.ts @@ -298,35 +298,10 @@ export class DefaultEntityPresentationApi implements EntityPresentationApi { }; }); - const entityRefPresentation: EntityRefPresentation = { + return { snapshot: initialSnapshot, update$: observable, - get promise() { - return new Promise(resolve => { - if (!observable) { - resolve([initialSnapshot]); - } else { - const res: EntityRefPresentationSnapshot[] = []; - const subscription = observable.subscribe({ - next: snapshot => { - res.push(snapshot); - }, - error: () => { - initialSnapshot = { - primaryTitle: entityRef, - entityRef: entityRef, - }; - }, - complete() { - subscription.unsubscribe(); - resolve(res); - }, - }); - } - }); - }, }; - return entityRefPresentation; } #getEntityForInitialRender(entityOrRef: Entity | string): { From de6bcac9aeebd67b971144fa448d845caebad7e2 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 4 May 2024 22:29:09 -0300 Subject: [PATCH 09/10] feat: review requested changes Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/olive-rockets-drum.md | 2 -- .../MultiEntityPicker/MultiEntityPicker.tsx | 16 ++-------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/.changeset/olive-rockets-drum.md b/.changeset/olive-rockets-drum.md index b5e6c2cefe..e28ba6101a 100644 --- a/.changeset/olive-rockets-drum.md +++ b/.changeset/olive-rockets-drum.md @@ -1,7 +1,5 @@ --- -'@backstage/plugin-catalog-react': minor '@backstage/plugin-scaffolder': minor -'@backstage/plugin-catalog': minor --- `MultiEntityPicker` uses `EntityDisplayName` instead of `humanizeEntityRef` to display entity. diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index 85e08740df..fb18564456 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -82,12 +82,7 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { .map(ref => { if (typeof ref !== 'string') { // if ref does not exist: pass 'undefined' to trigger validation for required value - return ref - ? entityPresentationApi.forEntity(ref, { - defaultKind, - defaultNamespace, - }).snapshot.entityRef - : undefined; + return ref ? stringifyEntityRef(ref as Entity) : undefined; } if (reason === 'blur' || reason === 'create-option') { // Add in default namespace, etc. @@ -116,14 +111,7 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { onChange(values); }, - [ - onChange, - formData, - defaultKind, - defaultNamespace, - allowArbitraryValues, - entityPresentationApi, - ], + [onChange, formData, defaultKind, defaultNamespace, allowArbitraryValues], ); useEffect(() => { From 71f18a999eb4cef13167b45991d9bf9117902c50 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Thu, 9 May 2024 23:46:02 -0300 Subject: [PATCH 10/10] chore: use promise getter of entityPresentationApi Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../MultiEntityPicker/MultiEntityPicker.tsx | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index d2ad90aa10..f8fc831976 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -27,6 +27,7 @@ import { catalogApiRef, entityPresentationApiRef, EntityDisplayName, + EntityRefPresentationSnapshot, } from '@backstage/plugin-catalog-react'; import TextField from '@material-ui/core/TextField'; import FormControl from '@material-ui/core/FormControl'; @@ -70,8 +71,22 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { const { items } = await catalogApi.getEntities( catalogFilter ? { filter: catalogFilter } : undefined, ); - - return items; + const entityRefToPresentation = new Map< + string, + EntityRefPresentationSnapshot + >( + await Promise.all( + items.map(async item => { + const presentation = await entityPresentationApi.forEntity(item) + .promise; + return [stringifyEntityRef(item), presentation] as [ + string, + EntityRefPresentationSnapshot, + ]; + }), + ), + ); + return { entities: items, entityRefToPresentation }; }); const allowArbitraryValues = uiSchema['ui:options']?.allowArbitraryValues ?? true; @@ -115,8 +130,8 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { ); useEffect(() => { - if (entities?.length === 1) { - onChange([stringifyEntityRef(entities[0])]); + if (entities?.entities?.length === 1) { + onChange([stringifyEntityRef(entities?.entities[0])]); } }, [entities, onChange]); @@ -129,20 +144,18 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { } getOptionLabel={option => // option can be a string due to freeSolo. typeof option === 'string' ? option - : entityPresentationApi.forEntity(option, { - defaultKind, - defaultNamespace, - }).snapshot.entityRef! + : entities?.entityRefToPresentation.get(stringifyEntityRef(option)) + ?.entityRef! } autoSelect freeSolo={allowArbitraryValues}