Fix CatalogPage and e2e tests

Signed-off-by: Tim Hansen <timbonicus@gmail.com>

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

@@ -123,7 +123,11 @@ export const UserListPicker = () => {
     isStarredEntity: isStarredEntity,
   };
 
-  const { filters, updateFilters, backendEntities } = useEntityListProvider();
+  const {
+    filters: { user: userFilter },
+    updateFilters,
+    backendEntities,
+  } = useEntityListProvider();
   function setSelectedFilter({ id }: { id: UserListFilterKind }) {
     updateFilters({ user: new UserListFilter(id) });
   }
@@ -158,7 +162,7 @@ export const UserListPicker = () => {
                   button
                   divider
                   onClick={() => setSelectedFilter(item)}
-                  selected={item.id === filters.user?.value}
+                  selected={item.id === userFilter?.value}
                   className={classes.menuItem}
                 >
                   {item.icon && (
@@ -167,7 +171,11 @@ export const UserListPicker = () => {
                     </ListItemIcon>
                   )}
                   <ListItemText>
-                    <Typography variant="body1" className={classes.menuTitle}>
+                    <Typography
+                      variant="body1"
+                      className={classes.menuTitle}
+                      data-testid={`user-picker-${item.id}`}
+                    >
                       {item.label}
                     </Typography>
                   </ListItemText>

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

@@ -20,6 +20,7 @@ export {
   EntityListProvider,
   useEntityListProvider,
 } from './useEntityListProvider';
+export type { DefaultEntityFilters } from './useEntityListProvider';
 export { useOwnUser } from './useOwnUser';
 export { useRelatedEntities } from './useRelatedEntities';
 export { useStarredEntities } from './useStarredEntities';

plugins/catalog-react/src/hooks/useEntityListProvider.tsx

@@ -70,7 +70,11 @@ export type EntityListContextProps<
    * Update one or more of the registered filters. Optional filters can be set to `undefined` to
    * reset the filter.
    */
-  updateFilters: (filters: Partial<EntityFilters>) => void;
+  updateFilters: (
+    filters:
+      | Partial<EntityFilters>
+      | ((prevFilters: EntityFilters) => Partial<EntityFilters>),
+  ) => void;
 
   loading: boolean;
   error?: Error;
@@ -148,8 +152,17 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>({
   }, [backendEntities, filterEnv, filters]);
 
   const updateFilters = useCallback(
-    (patch: Partial<EntityFilter>) =>
-      setFilters(prevFilters => ({ ...prevFilters, ...patch })),
+    (
+      update:
+        | Partial<EntityFilter>
+        | ((prevFilters: EntityFilters) => Partial<EntityFilters>),
+    ) => {
+      if (typeof update === 'function') {
+        setFilters(prevFilters => ({ ...prevFilters, ...update(prevFilters) }));
+      } else {
+        setFilters(prevFilters => ({ ...prevFilters, ...update }));
+      }
+    },
     [],
   );
 

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

@@ -129,9 +129,9 @@ describe('CatalogPage', () => {
   // related to some theme issues in mui-table
   // https://github.com/mbrn/material-table/issues/1293
   it('should render', async () => {
-    const { findByText, getByText } = renderWrapped(<CatalogPage />);
+    const { findByText, getByTestId } = renderWrapped(<CatalogPage />);
     expect(await findByText(/Owned \(1\)/)).toBeInTheDocument();
-    fireEvent.click(getByText(/All/));
+    fireEvent.click(getByTestId('user-picker-all'));
     expect(await findByText(/All \(2\)/)).toBeInTheDocument();
   });
   it('should set initial filter correctly', async () => {
@@ -143,21 +143,21 @@ describe('CatalogPage', () => {
   // this test is for fixing the bug after favoriting an entity, the matching entities defaulting
   // to "owned" filter and not based on the selected filter
   it('should render the correct entities filtered on the selectedfilter', async () => {
-    const { findByText, findAllByTitle, getByText } = renderWrapped(
+    const { findByText, findAllByTitle, getByTestId } = renderWrapped(
       <CatalogPage />,
     );
     expect(await findByText(/Owned \(1\)/)).toBeInTheDocument();
     expect(await findByText(/Starred/)).toBeInTheDocument();
-    fireEvent.click(getByText(/Starred/));
+    fireEvent.click(getByTestId('user-picker-starred'));
     expect(await findByText(/Starred \(0\)/)).toBeInTheDocument();
-    fireEvent.click(getByText(/All/));
+    fireEvent.click(getByTestId('user-picker-all'));
     expect(await findByText(/All \(2\)/)).toBeInTheDocument();
 
     const starredIcons = await findAllByTitle('Add to favorites');
     fireEvent.click(starredIcons[0]);
     expect(await findByText(/All \(2\)/)).toBeInTheDocument();
 
-    fireEvent.click(getByText(/Starred/));
+    fireEvent.click(getByTestId('user-picker-starred'));
     waitFor(() => expect(findByText(/Starred \(1\)/)).toBeInTheDocument());
   });
 });

plugins/catalog/src/components/EntityTypePicker/EntityTypePicker.tsx

@@ -14,54 +14,69 @@
  * limitations under the License.
  */
 
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { capitalize } from 'lodash';
+import { useAsync } from 'react-use';
 import { Box } from '@material-ui/core';
-import { Select, useApi } from '@backstage/core';
+import { alertApiRef, Select, useApi } from '@backstage/core';
 import {
   catalogApiRef,
+  DefaultEntityFilters,
   EntityTypeFilter,
   useEntityListProvider,
 } from '@backstage/plugin-catalog-react';
-import { Entity } from '@backstage/catalog-model';
 
 export const EntityTypePicker = () => {
   const catalogApi = useApi(catalogApiRef);
-  const { filters, updateFilters } = useEntityListProvider();
+  const alertApi = useApi(alertApiRef);
+
+  const {
+    filters: { kind: kindFilter, type: typeFilter },
+    updateFilters,
+  } = useEntityListProvider();
   const [types, setTypes] = useState<string[]>([]);
 
-  const kindFilter = filters.kind?.value;
+  const kind = useMemo(() => kindFilter?.value, [kindFilter]);
 
   // Load all valid spec.type values straight from the catalogApi - we want the full set for the
   // selected kinds, not an otherwise filtered set.
-  useEffect(() => {
-    async function loadTypesForKinds() {
-      if (kindFilter) {
-        const response = await catalogApi.getEntities({
-          filter: { kind: kindFilter },
+  const { error, value: entities } = useAsync(async () => {
+    if (kind) {
+      const items = await catalogApi
+        .getEntities({
+          filter: { kind },
           fields: ['spec.type'],
-        });
-        const entities: Entity[] = response.items ?? [];
-        const newTypes = [
-          ...new Set(
-            entities.map(e => e.spec?.type).filter(Boolean) as string[],
-          ),
-        ].sort();
-        setTypes(newTypes);
-
-        if (filters.type && !newTypes.includes(filters.type.value)) {
-          updateFilters({ type: undefined });
-        }
-      }
+        })
+        .then(response => response.items);
+      return items;
     }
-    loadTypesForKinds();
-  }, [filters.type, catalogApi, kindFilter, updateFilters]);
+    return [];
+  }, [kind, catalogApi]);
 
-  const onChange = (value: any) => {
-    updateFilters({ type: new EntityTypeFilter(value) });
-  };
+  useEffect(() => {
+    const newTypes = [
+      ...new Set(
+        (entities ?? []).map(e => e.spec?.type).filter(Boolean) as string[],
+      ),
+    ].sort();
+    setTypes(newTypes);
 
-  if (!kindFilter) return null;
+    updateFilters((oldFilters: DefaultEntityFilters) =>
+      oldFilters.type && !newTypes.includes(oldFilters.type.value)
+        ? { type: undefined }
+        : {},
+    );
+  }, [updateFilters, entities]);
+
+  if (!types) return null;
+
+  if (error) {
+    alertApi.post({
+      message: `Failed to load types for ${kind}`,
+      severity: 'error',
+    });
+    return null;
+  }
 
   const items = [
     { value: 'all', label: 'All' },
@@ -71,12 +86,16 @@ export const EntityTypePicker = () => {
     })),
   ];
 
+  const onChange = (value: any) => {
+    updateFilters({ type: new EntityTypeFilter(value) });
+  };
+
   return (
     <Box pb={1} pt={1}>
       <Select
         label="Type"
         items={items}
-        selected={filters.type?.value ?? 'all'}
+        selected={typeFilter?.value ?? 'all'}
         onChange={onChange}
       />
     </Box>