From 6ffa47bb0ac7527730211bcb124b73f58f77965e Mon Sep 17 00:00:00 2001 From: Gustaf Lundh Date: Thu, 27 Oct 2022 19:34:58 +0200 Subject: [PATCH] Catalog kind filter fixes & clean-up Various fixes: * CatalogDefaultPage cannot handle Capitalized kinds from the kind query parameter. This breaks ownership cards and both CatalogKindHeader and EntityKindPicker turns up empty. There may be other places where the catalog expects to handle capitalized Kinds. After all, the default Kinds are capitalized in the entities. * If a non existing kind is specified, CatalogKindHeader works but not the EntityKindPicker. Well, at least as long as the non-existing kind is not capitalized(!) * The CatalogKindHeader supports allowed kinds, but not the EntityKindFilter. So depending on Catalog-setup they will list different kinds. Refactoring: * Refactored out helper functions to a util library. This ensured that similar logic can be shared between the Kind pickers (and in turn their behaviour is similar). Tests: * Relevant tests added Signed-off-by: Gustaf Lundh --- .changeset/brave-bags-sniff.md | 5 + .changeset/neat-lies-know.md | 5 + plugins/catalog-react/api-report.md | 15 +++ .../EntityKindPicker.test.tsx | 57 ++++++++++ .../EntityKindPicker/EntityKindPicker.tsx | 106 +++++------------- plugins/catalog-react/src/index.ts | 2 + plugins/catalog-react/src/utils/index.ts | 1 + .../src/utils/kindFilterUtils.ts | 82 ++++++++++++++ .../CatalogKindHeader/CatalogKindHeader.tsx | 41 ++----- 9 files changed, 207 insertions(+), 107 deletions(-) create mode 100644 .changeset/brave-bags-sniff.md create mode 100644 .changeset/neat-lies-know.md create mode 100644 plugins/catalog-react/src/utils/kindFilterUtils.ts diff --git a/.changeset/brave-bags-sniff.md b/.changeset/brave-bags-sniff.md new file mode 100644 index 0000000000..2608ca7f1e --- /dev/null +++ b/.changeset/brave-bags-sniff.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog': minor +--- + +Fixes in kind selectors (now OwnershipCards work again) diff --git a/.changeset/neat-lies-know.md b/.changeset/neat-lies-know.md new file mode 100644 index 0000000000..cb355c638f --- /dev/null +++ b/.changeset/neat-lies-know.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-react': minor +--- + +Cleanup and small fixes for the kind selector diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index e8f0b0e6ee..ea67702c9b 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -175,6 +175,7 @@ export const EntityKindPicker: ( // @public export interface EntityKindPickerProps { + allowedKinds?: string[]; // (undocumented) hidden?: boolean; // (undocumented) @@ -431,6 +432,13 @@ export type FavoriteEntityProps = ComponentProps & { entity: Entity; }; +// @public +export function filterAndCapitalize( + allKinds: string[], + allowedKinds?: string[], + forcedKinds?: string[], +): Record; + // @public export function getEntityRelations( entity: Entity | undefined, @@ -503,6 +511,13 @@ export type UnregisterEntityDialogProps = { entity: Entity; }; +// @public +export function useAllKinds(): { + loading: boolean; + error?: Error; + allKinds: string[]; +}; + // @public export function useAsyncEntity< TEntity extends Entity = Entity, diff --git a/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx b/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx index 65ad3f227a..a62669caf9 100644 --- a/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx @@ -147,6 +147,43 @@ describe('', () => { }); }); + it('renders unknown kinds provided in query parameters', async () => { + const rendered = await renderWithEffects( + + + + + , + ); + + expect(rendered.getByText('Frob')).toBeInTheDocument(); + }); + + it('limits kinds when allowedKinds is set', async () => { + const rendered = await renderWithEffects( + + + + + , + ); + + const input = rendered.getByTestId('select'); + fireEvent.click(input); + + expect( + rendered.getByRole('option', { name: 'Component' }), + ).toBeInTheDocument(); + expect( + rendered.getByRole('option', { name: 'Domain' }), + ).toBeInTheDocument(); + expect( + rendered.queryByRole('option', { name: 'Template' }), + ).not.toBeInTheDocument(); + }); + it('responds to external queryParameters changes', async () => { const updateFilters = jest.fn(); const rendered = await renderWithEffects( @@ -180,4 +217,24 @@ describe('', () => { kind: new EntityKindFilter('domain'), }); }); + + it('renders kind from the query parameter even when not in allowedKinds', async () => { + const rendered = await renderWithEffects( + + + + + , + ); + + expect(rendered.getByText('Frob')).toBeInTheDocument(); + + const input = rendered.getByTestId('select'); + fireEvent.click(input); + expect( + rendered.getByRole('option', { name: 'Domain' }), + ).toBeInTheDocument(); + }); }); diff --git a/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.tsx b/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.tsx index c626a68d10..89313790f5 100644 --- a/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.tsx +++ b/plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.tsx @@ -17,64 +17,15 @@ import { Select } from '@backstage/core-components'; import { alertApiRef, useApi } from '@backstage/core-plugin-api'; import { Box } from '@material-ui/core'; -import capitalize from 'lodash/capitalize'; -import sortBy from 'lodash/sortBy'; -import React, { useEffect, useMemo, useRef, useState } from 'react'; -import useAsync from 'react-use/lib/useAsync'; -import { catalogApiRef } from '../../api'; +import React, { useEffect, useMemo, useState } from 'react'; import { EntityKindFilter } from '../../filters'; import { useEntityList } from '../../hooks'; - -function useAvailableKinds() { - const catalogApi = useApi(catalogApiRef); - - const [availableKinds, setAvailableKinds] = useState([]); - - const { - error, - loading, - value: facets, - } = useAsync(async () => { - const facet = 'kind'; - const items = await catalogApi - .getEntityFacets({ - facets: [facet], - }) - .then(response => response.facets[facet] || []); - - return items; - }, [catalogApi]); - - const facetsRef = useRef(facets); - useEffect(() => { - const oldFacets = facetsRef.current; - facetsRef.current = facets; - // Delay processing hook until facets load updates have settled to generate list of kinds; - // This prevents resetting the kind filter due to saved kind value from query params not matching the - // empty set of kind values while values are still being loaded; also only run this hook on changes - // to facets - if (loading || oldFacets === facets || !facets) { - return; - } - - const newKinds = [ - ...new Set( - sortBy(facets, f => f.value).map(f => - f.value.toLocaleLowerCase('en-US'), - ), - ), - ]; - - setAvailableKinds(newKinds); - }, [loading, facets, setAvailableKinds]); - - return { loading, error, availableKinds }; -} +import { filterAndCapitalize, useAllKinds } from '../../utils/kindFilterUtils'; function useEntityKindFilter(opts: { initialFilter: string }): { loading: boolean; error?: Error; - availableKinds: string[]; + allKinds: string[]; selectedKind: string; setSelectedKind: (kind: string) => void; } { @@ -84,23 +35,15 @@ function useEntityKindFilter(opts: { initialFilter: string }): { updateFilters, } = useEntityList(); - const flattenedQueryKind = useMemo( + const queryParamKind = useMemo( () => [kindParameter].flat()[0], [kindParameter], ); const [selectedKind, setSelectedKind] = useState( - flattenedQueryKind ?? filters.kind?.value ?? opts.initialFilter, + queryParamKind ?? filters.kind?.value ?? opts.initialFilter, ); - // Set selected kinds on query parameter updates; this happens at initial page load and from - // external updates to the page location. - useEffect(() => { - if (flattenedQueryKind) { - setSelectedKind(flattenedQueryKind); - } - }, [flattenedQueryKind]); - // Set selected kind from filters; this happens when the kind filter is // updated from another component useEffect(() => { @@ -109,18 +52,26 @@ function useEntityKindFilter(opts: { initialFilter: string }): { } }, [filters.kind]); - const { availableKinds, loading, error } = useAvailableKinds(); - useEffect(() => { updateFilters({ kind: selectedKind ? new EntityKindFilter(selectedKind) : undefined, }); }, [selectedKind, updateFilters]); + // Set selected kinds on query parameter updates; this happens at initial page load and from + // external updates to the page location. + useEffect(() => { + if (queryParamKind) { + setSelectedKind(queryParamKind); + } + }, [queryParamKind]); + + const { allKinds, loading, error } = useAllKinds(); + return { loading, error, - availableKinds, + allKinds: allKinds ?? [], selectedKind, setSelectedKind, }; @@ -132,17 +83,22 @@ function useEntityKindFilter(opts: { initialFilter: string }): { * @public */ export interface EntityKindPickerProps { + /** + * Entity kinds to show in the dropdown; by default all kinds are fetched from the catalog and + * displayed. + */ + allowedKinds?: string[]; initialFilter?: string; hidden?: boolean; } /** @public */ export const EntityKindPicker = (props: EntityKindPickerProps) => { - const { hidden, initialFilter = 'component' } = props; + const { allowedKinds, hidden, initialFilter = 'component' } = props; const alertApi = useApi(alertApiRef); - const { error, availableKinds, selectedKind, setSelectedKind } = + const { error, allKinds, selectedKind, setSelectedKind } = useEntityKindFilter({ initialFilter: initialFilter, }); @@ -156,21 +112,21 @@ export const EntityKindPicker = (props: EntityKindPickerProps) => { } }, [error, alertApi]); - if (availableKinds?.length === 0 || error) return null; + if (error) return null; - const items = [ - ...availableKinds.map((kind: string) => ({ - value: kind, - label: capitalize(kind), - })), - ]; + const options = filterAndCapitalize(allKinds, allowedKinds, [selectedKind]); + + const items = Object.keys(options).map(key => ({ + value: key, + label: options[key], + })); return hidden ? null : ( } - value={selectedKind} + input={} + value={selectedKind.toLocaleLowerCase('en-US')} onChange={e => setSelectedKind(e.target.value as string)} classes={classes} >