Merge pull request #16320 from sennyeya/async-selects

Attempt to show readable names in `EntityOwnerPicker` by loading from the `by-refs` endpoint.
This commit is contained in:
Fredrik Adelöw
2023-04-06 21:25:16 +02:00
committed by GitHub
11 changed files with 490 additions and 133 deletions
+19
View File
@@ -0,0 +1,19 @@
---
'@backstage/plugin-catalog-react': minor
---
Attempt to load entity owner names in the EntityOwnerPicker through the `by-refs` endpoint. If `spec.profile.displayName` or `metadata.title` are populated, we now attempt to show those before showing the `humanizeEntityRef`'d version.
**BREAKING**: `EntityOwnerFilter` now uses the full entity ref instead of the `humanizeEntityRef`. If you rely on `EntityOwnerFilter.values` or the `queryParameters.owners` of `useEntityList`, you will need to adjust your code like the following.
```tsx
const { queryParameters: { owners } } = useEntityList();
// or
const { filter: { owners } } = useEntityList();
// Instead of,
if (owners.some(ref => ref === humanizeEntityRef(myEntity))) ...
// You'll need to use,
if (owners.some(ref => ref === stringifyEntityRef(myEntity))) ...
```
@@ -64,6 +64,7 @@ describe('DefaultApiExplorerPage', () => {
}),
getLocationByRef: () =>
Promise.resolve({ id: 'id', type: 'url', target: 'url' }),
getEntitiesByRefs: () => Promise.resolve({ items: [] }),
getEntityByRef: async entityRef => {
return {
apiVersion: 'backstage.io/v1alpha1',
-1
View File
@@ -242,7 +242,6 @@ export class EntityOwnerFilter implements EntityFilter {
constructor(values: string[]);
// (undocumented)
filterEntity(entity: Entity): boolean;
// (undocumented)
toQueryValue(): string[];
// (undocumented)
readonly values: string[];
@@ -14,12 +14,59 @@
* limitations under the License.
*/
import { Entity, parseEntityRef } from '@backstage/catalog-model';
import { fireEvent, render, screen } from '@testing-library/react';
import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
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 {
MockErrorApi,
renderWithEffects,
TestApiRegistry,
} from '@backstage/test-utils';
import { catalogApiRef, CatalogApi } from '../..';
import { errorApiRef } from '@backstage/core-plugin-api';
const ownerEntities: Entity[] = [
{
apiVersion: '1',
kind: 'Group',
metadata: {
name: 'some-owner',
},
},
{
apiVersion: '1',
kind: 'Group',
metadata: {
name: 'some-owner-2',
},
spec: {
profile: {
displayName: 'Some Owner 2',
},
},
},
{
apiVersion: '1',
kind: 'Group',
metadata: {
name: 'another-owner',
title: 'Another Owner',
},
},
{
apiVersion: '1',
kind: 'Group',
metadata: {
namespace: 'test-namespace',
name: 'another-owner-2',
title: 'Another Owner in Another Namespace',
},
},
];
const sampleEntities: Entity[] = [
{
@@ -50,6 +97,10 @@ const sampleEntities: Entity[] = [
type: 'ownedBy',
targetRef: 'group:default/another-owner',
},
{
type: 'ownedBy',
targetRef: 'group:test-namespace/another-owner-2',
},
],
},
{
@@ -67,77 +118,104 @@ const sampleEntities: Entity[] = [
},
];
const getEntitiesByRefs = jest.fn(async ({ entityRefs }) => ({
items: entityRefs.map((e: string) =>
ownerEntities.find(f => stringifyEntityRef(f) === e),
),
}));
const mockCatalogApi: Partial<CatalogApi> = {
getEntitiesByRefs,
};
const mockErrorApi = new MockErrorApi();
describe('<EntityOwnerPicker/>', () => {
it('renders all owners', () => {
render(
<MockEntityListContextProvider
value={{ entities: sampleEntities, backendEntities: sampleEntities }}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
const mockApis = TestApiRegistry.from(
[catalogApiRef, mockCatalogApi],
[errorApiRef, mockErrorApi],
);
it('renders all owners', async () => {
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{ entities: sampleEntities, backendEntities: sampleEntities }}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
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(owner as string)).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', () => {
render(
<MockEntityListContextProvider
value={{ entities: sampleEntities, backendEntities: sampleEntities }}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
it('renders unique owners in alphabetical order', async () => {
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{ entities: sampleEntities, backendEntities: sampleEntities }}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(screen.getByText('Owner')).toBeInTheDocument();
fireEvent.click(screen.getByTestId('owner-picker-expand'));
expect(screen.getAllByRole('option').map(o => o.textContent)).toEqual([
'another-owner',
'Another Owner',
'Another Owner in Another Namespace',
'some-owner',
'some-owner-2',
'Some Owner 2',
]);
});
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(
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
queryParameters,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
queryParameters,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: new EntityOwnerFilter(['another-owner']),
owners: new EntityOwnerFilter(['group:default/another-owner']),
});
});
it('adds owners to filters', () => {
it('adds owners to filters', async () => {
const updateFilters = jest.fn();
render(
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: undefined,
@@ -146,28 +224,31 @@ describe('<EntityOwnerPicker/>', () => {
fireEvent.click(screen.getByTestId('owner-picker-expand'));
fireEvent.click(screen.getByText('some-owner'));
expect(updateFilters).toHaveBeenLastCalledWith({
owners: new EntityOwnerFilter(['some-owner']),
owners: new EntityOwnerFilter(['group:default/some-owner']),
});
});
it('removes owners from filters', () => {
it('removes owners from filters', async () => {
const updateFilters = jest.fn();
render(
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
filters: { owners: new EntityOwnerFilter(['some-owner']) },
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
entities: sampleEntities,
backendEntities: sampleEntities,
updateFilters,
filters: { owners: new EntityOwnerFilter(['some-owner']) },
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: new EntityOwnerFilter(['some-owner']),
owners: new EntityOwnerFilter(['group:default/some-owner']),
});
fireEvent.click(screen.getByTestId('owner-picker-expand'));
expect(screen.getByLabelText('some-owner')).toBeChecked();
fireEvent.click(screen.getByLabelText('some-owner'));
@@ -176,49 +257,55 @@ describe('<EntityOwnerPicker/>', () => {
});
});
it('responds to external queryParameters changes', () => {
it('responds to external queryParameters changes', async () => {
const updateFilters = jest.fn();
const rendered = render(
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-a'] },
backendEntities: sampleEntities,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
const rendered = await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-a'] },
backendEntities: sampleEntities,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: new EntityOwnerFilter(['team-a']),
owners: new EntityOwnerFilter(['group:default/team-a']),
});
rendered.rerender(
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-b'] },
backendEntities: sampleEntities,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-b'] },
backendEntities: sampleEntities,
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: new EntityOwnerFilter(['team-b']),
owners: new EntityOwnerFilter(['group:default/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(
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-a'] },
backendEntities: [],
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>,
await renderWithEffects(
<ApiProvider apis={mockApis}>
<MockEntityListContextProvider
value={{
updateFilters,
queryParameters: { owners: ['team-a'] },
backendEntities: [],
}}
>
<EntityOwnerPicker />
</MockEntityListContextProvider>
</ApiProvider>,
);
expect(updateFilters).toHaveBeenLastCalledWith({
owners: undefined,
@@ -14,7 +14,12 @@
* limitations under the License.
*/
import { Entity, RELATION_OWNED_BY } from '@backstage/catalog-model';
import {
Entity,
parseEntityRef,
RELATION_OWNED_BY,
stringifyEntityRef,
} from '@backstage/catalog-model';
import {
Box,
Checkbox,
@@ -31,7 +36,10 @@ import React, { useEffect, useMemo, useState } from 'react';
import { useEntityList } from '../../hooks/useEntityListProvider';
import { EntityOwnerFilter } from '../../filters';
import { getEntityRelations } from '../../utils';
import { humanizeEntityRef } from '../EntityRefLink';
import useAsync from 'react-use/lib/useAsync';
import { errorApiRef, useApi } from '@backstage/core-plugin-api';
import { catalogApiRef } from '../../api';
import { humanizeEntity, humanizeEntityRef } from '../EntityRefLink/humanize';
/** @public */
export type CatalogReactEntityOwnerPickerClassKey = 'input';
@@ -57,6 +65,8 @@ export const EntityOwnerPicker = () => {
filters,
queryParameters: { owners: ownersParameter },
} = useEntityList();
const catalogApi = useApi(catalogApiRef);
const errorApi = useApi(errorApiRef);
const queryParamOwners = useMemo(
() => [ownersParameter].flat().filter(Boolean) as string[],
@@ -64,43 +74,93 @@ export const EntityOwnerPicker = () => {
);
const [selectedOwners, setSelectedOwners] = useState(
queryParamOwners.length ? queryParamOwners : filters.owners?.values ?? [],
queryParamOwners.length
? new EntityOwnerFilter(queryParamOwners).values
: 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(() => {
if (queryParamOwners.length) {
setSelectedOwners(queryParamOwners);
const filter = new EntityOwnerFilter(queryParamOwners);
setSelectedOwners(filter.values);
}
}, [queryParamOwners]);
const availableOwners = useMemo(
() =>
[
...new Set(
backendEntities
.flatMap((e: Entity) =>
getEntityRelations(e, RELATION_OWNED_BY).map(o =>
humanizeEntityRef(o, { defaultKind: 'group' }),
),
)
.filter(Boolean) as string[],
),
].sort(),
[backendEntities],
);
useEffect(() => {
updateFilters({
owners:
selectedOwners.length && availableOwners.length
? new EntityOwnerFilter(selectedOwners)
: undefined,
});
}, [selectedOwners, updateFilters, availableOwners]);
if (!loading && ownerEntities) {
updateFilters({
owners:
selectedOwners.length && ownerEntities.length
? new EntityOwnerFilter(selectedOwners)
: undefined,
});
}
}, [selectedOwners, updateFilters, ownerEntities, loading]);
if (!availableOwners.length) return null;
if (!loading && !ownerEntities?.length) return null;
return (
<Box pb={1} pt={1}>
@@ -109,9 +169,17 @@ export const EntityOwnerPicker = () => {
<Autocomplete
multiple
disableCloseOnSelect
options={availableOwners}
value={selectedOwners}
onChange={(_: object, value: string[]) => setSelectedOwners(value)}
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 }) => (
<FormControlLabel
control={
@@ -122,7 +190,7 @@ export const EntityOwnerPicker = () => {
/>
}
onClick={event => event.preventDefault()}
label={option}
label={option.label}
/>
)}
size="small"
@@ -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');
});
});
@@ -19,6 +19,7 @@ import {
CompoundEntityRef,
DEFAULT_NAMESPACE,
} from '@backstage/catalog-model';
import get from 'lodash/get';
/**
* @param defaultNamespace - if set to false then namespace is never omitted,
@@ -65,3 +66,34 @@ export function humanizeEntityRef(
: kind;
return `${kind ? `${kind}:` : ''}${namespace ? `${namespace}/` : ''}${name}`;
}
/**
* Convert an entity to its more readable name if available.
*
* 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`.
*
* @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.
*
* @public
*/
export function humanizeEntity(
entity: Entity,
opts?: {
defaultKind?: string;
defaultNamespace?: string | false;
},
) {
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);
}
+64 -1
View File
@@ -15,11 +15,12 @@
*/
import { AlphaEntity } from '@backstage/catalog-model/alpha';
import { Entity } from '@backstage/catalog-model';
import { Entity, RELATION_OWNED_BY } from '@backstage/catalog-model';
import { TemplateEntityV1beta3 } from '@backstage/plugin-scaffolder-common';
import {
EntityErrorFilter,
EntityOrphanFilter,
EntityOwnerFilter,
EntityTextFilter,
} from './filters';
@@ -143,3 +144,65 @@ describe('EntityErrorFilter', () => {
expect(filter.filterEntity(entities[1])).toBeFalsy();
});
});
describe('EntityOwnerFilter', () => {
it('should handle humanizedEntityRefs', () => {
const filter = new EntityOwnerFilter(['my-user']);
expect(
filter.filterEntity({
relations: [
{
type: RELATION_OWNED_BY,
targetRef: 'group:default/my-user',
},
],
} as Entity),
).toBeTruthy();
expect(filter.values).toStrictEqual(['group:default/my-user']);
});
it('should handle full entityRefs', () => {
const filter = new EntityOwnerFilter(['group:default/my-user']);
expect(
filter.filterEntity({
relations: [
{
type: RELATION_OWNED_BY,
targetRef: 'group:default/my-user',
},
],
} as Entity),
).toBeTruthy();
expect(filter.values).toStrictEqual(['group:default/my-user']);
});
it('should gracefully reject non-entity refs', () => {
const filter = new EntityOwnerFilter(['group:default/my-user', '']);
expect(
filter.filterEntity({
relations: [
{
type: RELATION_OWNED_BY,
targetRef: 'group:default/my-user',
},
],
} as Entity),
).toBeTruthy();
expect(filter.values).toStrictEqual(['group:default/my-user']);
});
it('should handle non group full entity refs', () => {
const filter = new EntityOwnerFilter(['user:default/my-user', '']);
expect(
filter.filterEntity({
relations: [
{
type: RELATION_OWNED_BY,
targetRef: 'user:default/my-user',
},
],
} as Entity),
).toBeTruthy();
expect(filter.values).toStrictEqual(['user:default/my-user']);
});
});
+27 -4
View File
@@ -14,9 +14,13 @@
* limitations under the License.
*/
import { Entity, RELATION_OWNED_BY } from '@backstage/catalog-model';
import {
Entity,
parseEntityRef,
RELATION_OWNED_BY,
stringifyEntityRef,
} from '@backstage/catalog-model';
import { AlphaEntity } from '@backstage/catalog-model/alpha';
import { humanizeEntityRef } from './components/EntityRefLink';
import { EntityFilter, UserListFilterKind } from './types';
import { getEntityRelations } from './utils';
@@ -113,18 +117,37 @@ export class EntityTextFilter implements EntityFilter {
/**
* Filter matching entities that are owned by group.
* @public
*
* CAUTION: This class may contain both full and partial entity refs.
*/
export class EntityOwnerFilter implements EntityFilter {
constructor(readonly values: string[]) {}
readonly values: string[];
constructor(values: string[]) {
this.values = values.reduce((fullRefs, ref) => {
// Attempt to remove bad entity references here.
try {
fullRefs.push(
stringifyEntityRef(parseEntityRef(ref, { defaultKind: 'Group' })),
);
return fullRefs;
} catch (err) {
return fullRefs;
}
}, [] as string[]);
}
filterEntity(entity: Entity): boolean {
return this.values.some(v =>
getEntityRelations(entity, RELATION_OWNED_BY).some(
o => humanizeEntityRef(o, { defaultKind: 'group' }) === v,
o => stringifyEntityRef(o) === v,
),
);
}
/**
* Get the URL query parameter value. May be a mix of full and humanized entity refs.
* @returns list of entity refs.
*/
toQueryValue(): string[] {
return this.values;
}
@@ -119,6 +119,13 @@ describe('DefaultCatalogPage', () => {
],
};
},
/**
* For the purposes of this test case, use existing functionality. The picker
* isn't being tested, just needs this method to render correctly.
*/
getEntitiesByRefs: async refs => {
return { items: refs.entityRefs.map(() => undefined) };
},
};
const testProfile: Partial<ProfileInfo> = {
displayName: 'Display Name',
@@ -38,6 +38,7 @@ import { DefaultTechDocsHome } from './DefaultTechDocsHome';
const mockCatalogApi = {
getEntityByRef: () => Promise.resolve(),
getEntitiesByRefs: () => Promise.resolve({ items: [] }),
getEntities: async () => ({
items: [
{