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 <gustaf.lundh@axis.com>

.changeset/brave-bags-sniff.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog': minor
+---
+
+Fixes in kind selectors (now OwnershipCards work again)

.changeset/neat-lies-know.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-react': minor
+---
+
+Cleanup and small fixes for the kind selector

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<typeof IconButton> & {
   entity: Entity;
 };
 
+// @public
+export function filterAndCapitalize(
+  allKinds: string[],
+  allowedKinds?: string[],
+  forcedKinds?: string[],
+): Record<string, string>;
+
 // @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,

plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx

@@ -147,6 +147,43 @@ describe('<EntityKindPicker/>', () => {
     });
   });
 
+  it('renders unknown kinds provided in query parameters', async () => {
+    const rendered = await renderWithEffects(
+      <ApiProvider apis={apis}>
+        <MockEntityListContextProvider
+          value={{ queryParameters: { kind: 'frob' } }}
+        >
+          <EntityKindPicker />
+        </MockEntityListContextProvider>
+      </ApiProvider>,
+    );
+
+    expect(rendered.getByText('Frob')).toBeInTheDocument();
+  });
+
+  it('limits kinds when allowedKinds is set', async () => {
+    const rendered = await renderWithEffects(
+      <ApiProvider apis={apis}>
+        <MockEntityListContextProvider>
+          <EntityKindPicker allowedKinds={['component', 'domain']} />
+        </MockEntityListContextProvider>
+      </ApiProvider>,
+    );
+
+    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('<EntityKindPicker/>', () => {
       kind: new EntityKindFilter('domain'),
     });
   });
+
+  it('renders kind from the query parameter even when not in allowedKinds', async () => {
+    const rendered = await renderWithEffects(
+      <ApiProvider apis={apis}>
+        <MockEntityListContextProvider
+          value={{ queryParameters: { kind: 'Frob' } }}
+        >
+          <EntityKindPicker allowedKinds={['domain']} />
+        </MockEntityListContextProvider>
+      </ApiProvider>,
+    );
+
+    expect(rendered.getByText('Frob')).toBeInTheDocument();
+
+    const input = rendered.getByTestId('select');
+    fireEvent.click(input);
+    expect(
+      rendered.getByRole('option', { name: 'Domain' }),
+    ).toBeInTheDocument();
+  });
 });

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<string[]>([]);
-
-  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 : (
     <Box pb={1} pt={1}>
       <Select
         label="Kind"
         items={items}
-        selected={selectedKind}
+        selected={selectedKind.toLocaleLowerCase('en-US')}
         onChange={value => setSelectedKind(String(value))}
       />
     </Box>

plugins/catalog-react/src/index.ts

@@ -35,5 +35,7 @@ export {
   getEntityRelations,
   getEntitySourceLocation,
   isOwnerOf,
+  useAllKinds,
+  filterAndCapitalize,
 } from './utils';
 export type { EntitySourceLocation } from './utils';

plugins/catalog-react/src/utils/index.ts

@@ -18,3 +18,4 @@ export { getEntityRelations } from './getEntityRelations';
 export { getEntitySourceLocation } from './getEntitySourceLocation';
 export type { EntitySourceLocation } from './getEntitySourceLocation';
 export { isOwnerOf } from './isOwnerOf';
+export { useAllKinds, filterAndCapitalize } from './kindFilterUtils';

plugins/catalog-react/src/utils/kindFilterUtils.ts

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2022 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { useApi } from '@backstage/core-plugin-api';
+import { capitalize } from '@material-ui/core';
+import useAsync from 'react-use/lib/useAsync';
+import { catalogApiRef } from '../api';
+
+/**
+ * Fetch and return all availible kinds.
+ *
+ * @public
+ */
+export function useAllKinds(): {
+  loading: boolean;
+  error?: Error;
+  allKinds: string[];
+} {
+  const catalogApi = useApi(catalogApiRef);
+
+  const {
+    error,
+    loading,
+    value: allKinds,
+  } = useAsync(async () => {
+    const items = await catalogApi
+      .getEntityFacets({ facets: ['kind'] })
+      .then(response => response.facets.kind?.map(f => f.value).sort() || []);
+    return items;
+  }, [catalogApi]);
+
+  return { loading, error, allKinds: allKinds ?? [] };
+}
+
+/**
+ * Filter and capitalize accessible kinds.
+ *
+ * @public
+ */
+export function filterAndCapitalize(
+  allKinds: string[],
+  allowedKinds?: string[],
+  forcedKinds?: string[],
+): Record<string, string> {
+  // Before allKinds is loaded, or when a kind is entered manually in the URL, selectedKind may not
+  // be present in allKinds. It should still be shown in the dropdown, but may not have the nice
+  // enforced casing from the catalog-backend. This makes a key/value record for the Select options,
+  // including selectedKind if it's unknown - but allows the selectedKind to get clobbered by the
+  // more proper catalog kind if it exists.
+  const availableKinds = allKinds
+    .concat(forcedKinds ?? [])
+    .filter(k =>
+      allowedKinds
+        ? allowedKinds.some(
+            a => a.toLocaleLowerCase('en-US') === k.toLocaleLowerCase('en-US'),
+          ) ||
+          forcedKinds?.some(
+            f => f.toLocaleLowerCase('en-US') === k.toLocaleLowerCase('en-US'),
+          )
+        : true,
+    );
+
+  const capitalizedKinds = availableKinds.sort().reduce((acc, kind) => {
+    acc[kind.toLocaleLowerCase('en-US')] = capitalize(kind);
+    return acc;
+  }, {} as Record<string, string>);
+
+  return capitalizedKinds;
+}

plugins/catalog/src/components/CatalogKindHeader/CatalogKindHeader.tsx

@@ -16,7 +16,6 @@
 
 import React, { useEffect, useState, useMemo } from 'react';
 import {
-  capitalize,
   createStyles,
   InputBase,
   makeStyles,
@@ -25,12 +24,11 @@ import {
   Theme,
 } from '@material-ui/core';
 import {
-  catalogApiRef,
   EntityKindFilter,
+  filterAndCapitalize,
+  useAllKinds,
   useEntityList,
 } from '@backstage/plugin-catalog-react';
-import useAsync from 'react-use/lib/useAsync';
-import { useApi } from '@backstage/core-plugin-api';
 
 const useStyles = makeStyles((theme: Theme) =>
   createStyles({
@@ -62,12 +60,7 @@ export interface CatalogKindHeaderProps {
 export function CatalogKindHeader(props: CatalogKindHeaderProps) {
   const { initialFilter = 'component', allowedKinds } = props;
   const classes = useStyles();
-  const catalogApi = useApi(catalogApiRef);
-  const { value: allKinds } = useAsync(async () => {
-    return await catalogApi
-      .getEntityFacets({ facets: ['kind'] })
-      .then(response => response.facets.kind?.map(f => f.value).sort() || []);
-  });
+  const { allKinds } = useAllKinds();
   const {
     filters,
     updateFilters,
@@ -75,11 +68,12 @@ export function CatalogKindHeader(props: CatalogKindHeaderProps) {
   } = useEntityList();
 
   const queryParamKind = useMemo(
-    () => [kindParameter].flat()[0]?.toLocaleLowerCase('en-US'),
+    () => [kindParameter].flat()[0],
     [kindParameter],
   );
+
   const [selectedKind, setSelectedKind] = useState(
-    queryParamKind ?? initialFilter,
+    queryParamKind ?? filters.kind?.value ?? initialFilter,
   );
 
   // Set selected kind from filters; this happens when the kind filter is
@@ -104,29 +98,12 @@ export function CatalogKindHeader(props: CatalogKindHeaderProps) {
     }
   }, [queryParamKind]);
 
-  // Before allKinds is loaded, or when a kind is entered manually in the URL, selectedKind may not
-  // be present in allKinds. It should still be shown in the dropdown, but may not have the nice
-  // enforced casing from the catalog-backend. This makes a key/value record for the Select options,
-  // including selectedKind if it's unknown - but allows the selectedKind to get clobbered by the
-  // more proper catalog kind if it exists.
-  const availableKinds = [capitalize(selectedKind)].concat(
-    allKinds?.filter(k =>
-      allowedKinds
-        ? allowedKinds.some(
-            a => a.toLocaleLowerCase('en-US') === k.toLocaleLowerCase('en-US'),
-          )
-        : true,
-    ) ?? [],
-  );
-  const options = availableKinds.sort().reduce((acc, kind) => {
-    acc[kind.toLocaleLowerCase('en-US')] = kind;
-    return acc;
-  }, {} as Record<string, string>);
+  const options = filterAndCapitalize(allKinds, allowedKinds, [selectedKind]);
 
   return (
     <Select
-      input={<InputBase value={selectedKind} />}
-      value={selectedKind}
+      input={<InputBase />}
+      value={selectedKind.toLocaleLowerCase('en-US')}
       onChange={e => setSelectedKind(e.target.value as string)}
       classes={classes}
     >