Fixed review comments

* Now we don't react on external changes to the query kindParameter
* Casing is always correct (APIs instead of Apis etc)

Signed-off-by: Gustaf Lundh <gustaf.lundh@axis.com>

.changeset/brave-bags-sniff.md

@@ -3,3 +3,4 @@
 ---
 
 Fixes in kind selectors (now OwnershipCards work again)
+EntityKindPicker now accepts an optional allowedKinds prop, just like CatalogKindHeader.

.changeset/neat-lies-know.md

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

plugins/catalog-react/api-report.md

@@ -432,6 +432,9 @@ export type FavoriteEntityProps = ComponentProps<typeof IconButton> & {
   entity: Entity;
 };
 
+// Warning: (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
+// Warning: (ae-missing-release-tag) "filterAndCapitalize" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
 // @public
 export function filterAndCapitalize(
   allKinds: string[],
@@ -511,6 +514,9 @@ export type UnregisterEntityDialogProps = {
   entity: Entity;
 };
 
+// Warning: (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
+// Warning: (ae-missing-release-tag) "useAllKinds" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
 // @public
 export function useAllKinds(): {
   loading: boolean;

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

@@ -148,21 +148,21 @@ describe('<EntityKindPicker/>', () => {
   });
 
   it('renders unknown kinds provided in query parameters', async () => {
-    const rendered = await renderWithEffects(
+    await renderWithEffects(
       <ApiProvider apis={apis}>
         <MockEntityListContextProvider
-          value={{ queryParameters: { kind: 'frob' } }}
+          value={{ queryParameters: { kind: 'FROb' } }}
         >
           <EntityKindPicker />
         </MockEntityListContextProvider>
       </ApiProvider>,
     );
 
-    expect(rendered.getByText('Frob')).toBeInTheDocument();
+    expect(screen.getByText('FROb')).toBeInTheDocument();
   });
 
   it('limits kinds when allowedKinds is set', async () => {
-    const rendered = await renderWithEffects(
+    await renderWithEffects(
       <ApiProvider apis={apis}>
         <MockEntityListContextProvider>
           <EntityKindPicker allowedKinds={['component', 'domain']} />
@@ -170,56 +170,20 @@ describe('<EntityKindPicker/>', () => {
       </ApiProvider>,
     );
 
-    const input = rendered.getByTestId('select');
+    const input = screen.getByTestId('select');
     fireEvent.click(input);
 
     expect(
-      rendered.getByRole('option', { name: 'Component' }),
+      screen.getByRole('option', { name: 'Component' }),
     ).toBeInTheDocument();
+    expect(screen.getByRole('option', { name: 'Domain' })).toBeInTheDocument();
     expect(
-      rendered.getByRole('option', { name: 'Domain' }),
-    ).toBeInTheDocument();
-    expect(
-      rendered.queryByRole('option', { name: 'Template' }),
+      screen.queryByRole('option', { name: 'Template' }),
     ).not.toBeInTheDocument();
   });
 
-  it('responds to external queryParameters changes', async () => {
-    const updateFilters = jest.fn();
-    const rendered = await renderWithEffects(
-      <ApiProvider apis={apis}>
-        <MockEntityListContextProvider
-          value={{
-            updateFilters,
-            queryParameters: { kind: 'component' },
-          }}
-        >
-          <EntityKindPicker />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
-    );
-    expect(updateFilters).toHaveBeenLastCalledWith({
-      kind: new EntityKindFilter('component'),
-    });
-    rendered.rerender(
-      <ApiProvider apis={apis}>
-        <MockEntityListContextProvider
-          value={{
-            updateFilters,
-            queryParameters: { kind: 'domain' },
-          }}
-        >
-          <EntityKindPicker />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
-    );
-    expect(updateFilters).toHaveBeenLastCalledWith({
-      kind: new EntityKindFilter('domain'),
-    });
-  });
-
   it('renders kind from the query parameter even when not in allowedKinds', async () => {
-    const rendered = await renderWithEffects(
+    await renderWithEffects(
       <ApiProvider apis={apis}>
         <MockEntityListContextProvider
           value={{ queryParameters: { kind: 'Frob' } }}
@@ -229,12 +193,10 @@ describe('<EntityKindPicker/>', () => {
       </ApiProvider>,
     );
 
-    expect(rendered.getByText('Frob')).toBeInTheDocument();
+    expect(screen.getByText('Frob')).toBeInTheDocument();
 
-    const input = rendered.getByTestId('select');
+    const input = screen.getByTestId('select');
     fireEvent.click(input);
-    expect(
-      rendered.getByRole('option', { name: 'Domain' }),
-    ).toBeInTheDocument();
+    expect(screen.getByRole('option', { name: 'Domain' })).toBeInTheDocument();
   });
 });

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

@@ -20,7 +20,7 @@ import { Box } from '@material-ui/core';
 import React, { useEffect, useMemo, useState } from 'react';
 import { EntityKindFilter } from '../../filters';
 import { useEntityList } from '../../hooks';
-import { filterAndCapitalize, useAllKinds } from '../../utils/kindFilterUtils';
+import { filterKinds, useAllKinds } from '../../utils/kindFilterUtils';
 
 function useEntityKindFilter(opts: { initialFilter: string }): {
   loading: boolean;
@@ -58,14 +58,6 @@ function useEntityKindFilter(opts: { initialFilter: string }): {
     });
   }, [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 {
@@ -114,7 +106,7 @@ export const EntityKindPicker = (props: EntityKindPickerProps) => {
 
   if (error) return null;
 
-  const options = filterAndCapitalize(allKinds, allowedKinds, [selectedKind]);
+  const options = filterKinds(allKinds, allowedKinds, selectedKind);
 
   const items = Object.keys(options).map(key => ({
     value: key,

plugins/catalog-react/src/index.ts

@@ -36,6 +36,6 @@ export {
   getEntitySourceLocation,
   isOwnerOf,
   useAllKinds,
-  filterAndCapitalize,
+  filterKinds,
 } from './utils';
 export type { EntitySourceLocation } from './utils';

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

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

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

@@ -15,14 +15,12 @@
  */
 
 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
+ * @internal
  */
 export function useAllKinds(): {
   loading: boolean;
@@ -48,35 +46,40 @@ export function useAllKinds(): {
 /**
  * Filter and capitalize accessible kinds.
  *
- * @public
+ * @internal
  */
-export function filterAndCapitalize(
+export function filterKinds(
   allKinds: string[],
   allowedKinds?: string[],
-  forcedKinds?: 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,
+  let availableKinds = allKinds;
+  if (allowedKinds) {
+    availableKinds = availableKinds.filter(k =>
+      allowedKinds.some(
+        a => a.toLocaleLowerCase('en-US') === k.toLocaleLowerCase('en-US'),
+      ),
     );
+  }
+  if (
+    forcedKinds &&
+    !allKinds.some(
+      a =>
+        a.toLocaleLowerCase('en-US') === forcedKinds.toLocaleLowerCase('en-US'),
+    )
+  ) {
+    availableKinds = availableKinds.concat([forcedKinds]);
+  }
 
-  const capitalizedKinds = availableKinds.sort().reduce((acc, kind) => {
-    acc[kind.toLocaleLowerCase('en-US')] = capitalize(kind);
+  const kindsMap = availableKinds.sort().reduce((acc, kind) => {
+    acc[kind.toLocaleLowerCase('en-US')] = kind;
     return acc;
   }, {} as Record<string, string>);
 
-  return capitalizedKinds;
+  return kindsMap;
 }

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

@@ -105,14 +105,14 @@ describe('<CatalogKindHeader />', () => {
     await renderWithEffects(
       <ApiProvider apis={apis}>
         <MockEntityListContextProvider
-          value={{ queryParameters: { kind: 'frob' } }}
+          value={{ queryParameters: { kind: 'FROb' } }}
         >
           <CatalogKindHeader />
         </MockEntityListContextProvider>
       </ApiProvider>,
     );
 
-    expect(screen.getByText('Frobs')).toBeInTheDocument();
+    expect(screen.getByText('FRObs')).toBeInTheDocument();
   });
 
   it('updates the kind filter', async () => {
@@ -136,40 +136,6 @@ describe('<CatalogKindHeader />', () => {
     });
   });
 
-  it('responds to external queryParameters changes', async () => {
-    const updateFilters = jest.fn();
-    const rendered = await renderWithEffects(
-      <ApiProvider apis={apis}>
-        <MockEntityListContextProvider
-          value={{
-            updateFilters,
-            queryParameters: { kind: ['components'] },
-          }}
-        >
-          <CatalogKindHeader />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
-    );
-    expect(updateFilters).toHaveBeenLastCalledWith({
-      kind: new EntityKindFilter('components'),
-    });
-    rendered.rerender(
-      <ApiProvider apis={apis}>
-        <MockEntityListContextProvider
-          value={{
-            updateFilters,
-            queryParameters: { kind: ['template'] },
-          }}
-        >
-          <CatalogKindHeader />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
-    );
-    expect(updateFilters).toHaveBeenLastCalledWith({
-      kind: new EntityKindFilter('template'),
-    });
-  });
-
   it('limits kinds when allowedKinds is set', async () => {
     await renderWithEffects(
       <ApiProvider apis={apis}>

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

@@ -25,7 +25,7 @@ import {
 } from '@material-ui/core';
 import {
   EntityKindFilter,
-  filterAndCapitalize,
+  filterKinds,
   useAllKinds,
   useEntityList,
 } from '@backstage/plugin-catalog-react';
@@ -90,15 +90,7 @@ export function CatalogKindHeader(props: CatalogKindHeaderProps) {
     });
   }, [selectedKind, updateFilters]);
 
-  // Set selected Kind 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 options = filterAndCapitalize(allKinds, allowedKinds, [selectedKind]);
+  const options = filterKinds(allKinds, allowedKinds, selectedKind);
 
   return (
     <Select