catalog-react: EntityOwnerPicker paginate entities

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>

plugins/catalog-react/package.json

@@ -60,6 +60,7 @@
     "@material-ui/core": "^4.12.2",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.61",
+    "@react-hookz/web": "^23.0.0",
     "@types/react": "^16.13.1 || ^17.0.0",
     "classnames": "^2.2.6",
     "jwt-decode": "^3.1.0",

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

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
-import { fireEvent, screen } from '@testing-library/react';
+import { Entity } from '@backstage/catalog-model';
+import { fireEvent, screen, waitFor } from '@testing-library/react';
 import React from 'react';
 import { MockEntityListContextProvider } from '../../testUtils/providers';
 import { EntityOwnerFilter } from '../../filters';
@@ -28,8 +28,9 @@ import {
 } from '@backstage/test-utils';
 import { catalogApiRef, CatalogApi } from '../..';
 import { errorApiRef } from '@backstage/core-plugin-api';
+import { QueryEntitiesCursorRequest } from '@backstage/catalog-client';
 
-const ownerEntities: Entity[] = [
+const ownerEntitiesBatch1: Entity[] = [
   {
     apiVersion: '1',
     kind: 'Group',
@@ -68,64 +69,52 @@ const ownerEntities: Entity[] = [
   },
 ];
 
-const sampleEntities: Entity[] = [
+const ownerEntitiesBatch2: Entity[] = [
   {
     apiVersion: '1',
-    kind: 'Component',
+    kind: 'Group',
     metadata: {
-      name: 'component-1',
+      name: 'some-owner-batch-2',
     },
-    relations: [
-      {
-        type: 'ownedBy',
-        targetRef: 'group:default/some-owner',
-      },
-      {
-        type: 'ownedBy',
-        targetRef: 'group:default/some-owner-2',
-      },
-    ],
   },
   {
     apiVersion: '1',
-    kind: 'Component',
+    kind: 'Group',
     metadata: {
-      name: 'component-2',
+      name: 'some-owner-2-batch-2',
     },
-    relations: [
-      {
-        type: 'ownedBy',
-        targetRef: 'group:default/another-owner',
+    spec: {
+      profile: {
+        displayName: 'Some Owner Batch 2',
       },
-      {
-        type: 'ownedBy',
-        targetRef: 'group:test-namespace/another-owner-2',
-      },
-    ],
+    },
   },
   {
     apiVersion: '1',
-    kind: 'Component',
+    kind: 'Group',
     metadata: {
-      name: 'component-3',
+      name: 'another-owner-batch-2',
+      title: 'Another Owner Batch 2',
+    },
+  },
+  {
+    apiVersion: '1',
+    kind: 'Group',
+    metadata: {
+      namespace: 'test-namespace',
+      name: 'another-owner-2-batch-2',
+      title: 'Another Owner in Another Namespace Batch 2',
     },
-    relations: [
-      {
-        type: 'ownedBy',
-        targetRef: 'group:default/some-owner',
-      },
-    ],
   },
 ];
 
-const getEntitiesByRefs = jest.fn(async ({ entityRefs }) => ({
-  items: entityRefs.map((e: string) =>
-    ownerEntities.find(f => stringifyEntityRef(f) === e),
-  ),
-}));
+const mockedQueryEntities: jest.MockedFn<CatalogApi['queryEntities']> =
+  jest.fn();
+
 const mockCatalogApi: Partial<CatalogApi> = {
-  getEntitiesByRefs,
+  queryEntities: mockedQueryEntities,
 };
+
 const mockErrorApi = new MockErrorApi();
 
 describe('<EntityOwnerPicker/>', () => {
@@ -134,12 +123,33 @@ describe('<EntityOwnerPicker/>', () => {
     [errorApiRef, mockErrorApi],
   );
 
+  beforeEach(() => {
+    jest.resetAllMocks();
+
+    mockedQueryEntities.mockImplementation(async request => {
+      const totalItems =
+        ownerEntitiesBatch1.length + ownerEntitiesBatch2.length;
+      if ((request as QueryEntitiesCursorRequest).cursor) {
+        return {
+          items: ownerEntitiesBatch2,
+          pageInfo: {},
+          totalItems,
+        };
+      }
+      return {
+        items: ownerEntitiesBatch1,
+        pageInfo: {
+          nextCursor: 'nextCursor',
+        },
+        totalItems,
+      };
+    });
+  });
+
   it('renders all owners', async () => {
     await renderWithEffects(
       <ApiProvider apis={mockApis}>
-        <MockEntityListContextProvider
-          value={{ entities: sampleEntities, backendEntities: sampleEntities }}
-        >
+        <MockEntityListContextProvider value={{}}>
           <EntityOwnerPicker />
         </MockEntityListContextProvider>
       </ApiProvider>,
@@ -147,36 +157,36 @@ describe('<EntityOwnerPicker/>', () => {
     expect(screen.getByText('Owner')).toBeInTheDocument();
 
     fireEvent.click(screen.getByTestId('owner-picker-expand'));
+
+    await waitFor(() =>
+      expect(screen.getByText('Another Owner')).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', async () => {
-    await renderWithEffects(
-      <ApiProvider apis={mockApis}>
-        <MockEntityListContextProvider
-          value={{ entities: sampleEntities, backendEntities: sampleEntities }}
-        >
-          <EntityOwnerPicker />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
+    expect(mockedQueryEntities).toHaveBeenCalledTimes(1);
+
+    fireEvent.scroll(screen.getByTestId('owner-picker-listbox'));
+
+    await waitFor(() =>
+      expect(screen.getByText('some-owner-batch-2')).toBeInTheDocument(),
     );
-    expect(screen.getByText('Owner')).toBeInTheDocument();
 
-    fireEvent.click(screen.getByTestId('owner-picker-expand'));
+    [
+      'some-owner-batch-2',
+      'Some Owner Batch 2',
+      'Another Owner in Another Namespace Batch 2',
+    ].forEach(owner => {
+      expect(screen.getByText(owner)).toBeInTheDocument();
+    });
 
-    expect(screen.getAllByRole('option').map(o => o.textContent)).toEqual([
-      'Another Owner',
-      'Another Owner in Another Namespace',
-      'some-owner',
-      'Some Owner 2',
-    ]);
+    expect(mockedQueryEntities).toHaveBeenCalledTimes(2);
   });
 
   it('respects the query parameter filter value', async () => {
@@ -186,8 +196,6 @@ describe('<EntityOwnerPicker/>', () => {
       <ApiProvider apis={mockApis}>
         <MockEntityListContextProvider
           value={{
-            entities: sampleEntities,
-            backendEntities: sampleEntities,
             updateFilters,
             queryParameters,
           }}
@@ -208,8 +216,6 @@ describe('<EntityOwnerPicker/>', () => {
       <ApiProvider apis={mockApis}>
         <MockEntityListContextProvider
           value={{
-            entities: sampleEntities,
-            backendEntities: sampleEntities,
             updateFilters,
           }}
         >
@@ -222,6 +228,8 @@ describe('<EntityOwnerPicker/>', () => {
     });
 
     fireEvent.click(screen.getByTestId('owner-picker-expand'));
+    await waitFor(() => screen.getByText('some-owner'));
+
     fireEvent.click(screen.getByText('some-owner'));
     expect(updateFilters).toHaveBeenLastCalledWith({
       owners: new EntityOwnerFilter(['group:default/some-owner']),
@@ -234,8 +242,6 @@ describe('<EntityOwnerPicker/>', () => {
       <ApiProvider apis={mockApis}>
         <MockEntityListContextProvider
           value={{
-            entities: sampleEntities,
-            backendEntities: sampleEntities,
             updateFilters,
             filters: { owners: new EntityOwnerFilter(['some-owner']) },
           }}
@@ -249,7 +255,9 @@ describe('<EntityOwnerPicker/>', () => {
     });
     fireEvent.click(screen.getByTestId('owner-picker-expand'));
 
-    expect(screen.getByLabelText('some-owner')).toBeChecked();
+    await waitFor(() =>
+      expect(screen.getByLabelText('some-owner')).toBeChecked(),
+    );
 
     fireEvent.click(screen.getByLabelText('some-owner'));
     expect(updateFilters).toHaveBeenLastCalledWith({
@@ -265,7 +273,6 @@ describe('<EntityOwnerPicker/>', () => {
           value={{
             updateFilters,
             queryParameters: { owners: ['team-a'] },
-            backendEntities: sampleEntities,
           }}
         >
           <EntityOwnerPicker />
@@ -281,7 +288,6 @@ describe('<EntityOwnerPicker/>', () => {
           value={{
             updateFilters,
             queryParameters: { owners: ['team-b'] },
-            backendEntities: sampleEntities,
           }}
         >
           <EntityOwnerPicker />
@@ -292,23 +298,4 @@ describe('<EntityOwnerPicker/>', () => {
       owners: new EntityOwnerFilter(['group:default/team-b']),
     });
   });
-  it('removes owners from filters if there are none available', async () => {
-    const updateFilters = jest.fn();
-    await renderWithEffects(
-      <ApiProvider apis={mockApis}>
-        <MockEntityListContextProvider
-          value={{
-            updateFilters,
-            queryParameters: { owners: ['team-a'] },
-            backendEntities: [],
-          }}
-        >
-          <EntityOwnerPicker />
-        </MockEntityListContextProvider>
-      </ApiProvider>,
-    );
-    expect(updateFilters).toHaveBeenLastCalledWith({
-      owners: undefined,
-    });
-  });
 });

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

@@ -14,17 +14,11 @@
  * limitations under the License.
  */
 
-import {
-  Entity,
-  parseEntityRef,
-  RELATION_OWNED_BY,
-  stringifyEntityRef,
-} from '@backstage/catalog-model';
+import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
 import {
   Box,
   Checkbox,
   FormControlLabel,
-  makeStyles,
   TextField,
   Typography,
 } from '@material-ui/core';
@@ -35,38 +29,69 @@ import { Autocomplete } from '@material-ui/lab';
 import React, { useEffect, useMemo, useState } from 'react';
 import { useEntityList } from '../../hooks/useEntityListProvider';
 import { EntityOwnerFilter } from '../../filters';
-import { getEntityRelations } from '../../utils';
-import useAsync from 'react-use/lib/useAsync';
-import { errorApiRef, useApi } from '@backstage/core-plugin-api';
+import { useApi } from '@backstage/core-plugin-api';
 import { catalogApiRef } from '../../api';
-import { humanizeEntity, humanizeEntityRef } from '../EntityRefLink/humanize';
+import useAsyncFn from 'react-use/lib/useAsyncFn';
+import { useDebouncedEffect } from '@react-hookz/web';
+import PersonIcon from '@material-ui/icons/Person';
+import GroupIcon from '@material-ui/icons/Group';
+import { humanizeEntity } from '../EntityRefLink/humanize';
 
 /** @public */
 export type CatalogReactEntityOwnerPickerClassKey = 'input';
 
-const useStyles = makeStyles(
-  {
-    input: {},
-  },
-  {
-    name: 'CatalogReactEntityOwnerPicker',
-  },
-);
-
 const icon = <CheckBoxOutlineBlankIcon fontSize="small" />;
 const checkedIcon = <CheckBoxIcon fontSize="small" />;
 
 /** @public */
 export const EntityOwnerPicker = () => {
-  const classes = useStyles();
   const {
     updateFilters,
-    backendEntities,
     filters,
     queryParameters: { owners: ownersParameter },
   } = useEntityList();
+
   const catalogApi = useApi(catalogApiRef);
-  const errorApi = useApi(errorApiRef);
+  const [text, setText] = useState('');
+
+  const [{ value, loading }, handleFetch] = useAsyncFn(
+    async (request: { text: string } | { cursor: string; prev: Entity[] }) => {
+      const initialRequest = request as { text: string };
+      const cursorRequest = request as { cursor: string; prev: Entity[] };
+      const limit = 20;
+
+      if (cursorRequest.cursor) {
+        const response = await catalogApi.queryEntities({
+          cursor: cursorRequest.cursor,
+          limit,
+        });
+        return {
+          ...response,
+          items: [...cursorRequest.prev, ...response.items],
+        };
+      }
+
+      return catalogApi.queryEntities({
+        fullTextFilter: {
+          term: initialRequest.text || '',
+          fields: [
+            'metadata.name',
+            'kind',
+            'spec.profile.displayname',
+            'metadata.title',
+          ],
+        },
+        filter: { kind: ['User', 'Group'] },
+        orderFields: [{ field: 'metadata.name', order: 'asc' }],
+        limit,
+      });
+    },
+    [text],
+  );
+
+  useDebouncedEffect(() => handleFetch({ text }), [text], 250);
+
+  const availableOwners = value?.items || [];
 
   const queryParamOwners = useMemo(
     () => [ownersParameter].flat().filter(Boolean) as string[],
@@ -74,72 +99,9 @@ export const EntityOwnerPicker = () => {
   );
 
   const [selectedOwners, setSelectedOwners] = useState(
-    queryParamOwners.length
-      ? new EntityOwnerFilter(queryParamOwners).values
-      : filters.owners?.values ?? [],
+    queryParamOwners.length ? queryParamOwners : 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(() => {
@@ -150,17 +112,12 @@ export const EntityOwnerPicker = () => {
   }, [queryParamOwners]);
 
   useEffect(() => {
-    if (!loading && ownerEntities) {
-      updateFilters({
-        owners:
-          selectedOwners.length && ownerEntities.length
-            ? new EntityOwnerFilter(selectedOwners)
-            : undefined,
-      });
-    }
-  }, [selectedOwners, updateFilters, ownerEntities, loading]);
-
-  if (!loading && !ownerEntities?.length) return null;
+    updateFilters({
+      owners: selectedOwners.length
+        ? new EntityOwnerFilter(selectedOwners)
+        : undefined,
+    });
+  }, [selectedOwners, updateFilters]);
 
   return (
     <Box pb={1} pt={1}>
@@ -170,38 +127,80 @@ export const EntityOwnerPicker = () => {
           multiple
           disableCloseOnSelect
           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={
-                <Checkbox
-                  icon={icon}
-                  checkedIcon={checkedIcon}
-                  checked={selected}
-                />
-              }
-              onClick={event => event.preventDefault()}
-              label={option.label}
-            />
-          )}
+          options={availableOwners}
+          value={selectedOwners as unknown as Entity[]}
+          getOptionSelected={(o, v) => {
+            if (typeof v === 'string') {
+              return stringifyEntityRef(o) === v;
+            }
+            return o === v;
+          }}
+          onChange={(_: object, owners) => {
+            setText('');
+            setSelectedOwners(
+              owners.map(e =>
+                typeof e === 'string' ? e : stringifyEntityRef(e),
+              ),
+            );
+          }}
+          filterOptions={x => x}
+          renderOption={(entity, { selected }) => {
+            const isGroup = entity.kind === 'Group';
+
+            return (
+              <FormControlLabel
+                control={
+                  <Checkbox
+                    icon={icon}
+                    checkedIcon={checkedIcon}
+                    checked={selected}
+                  />
+                }
+                onClick={event => event.preventDefault()}
+                label={
+                  <Box display="flex" flexWrap="wrap" alignItems="center">
+                    {isGroup ? (
+                      <GroupIcon fontSize="small" />
+                    ) : (
+                      <PersonIcon fontSize="small" />
+                    )}
+                    &nbsp;
+                    {humanizeEntity(entity, entity.metadata.name)}
+                  </Box>
+                }
+              />
+            );
+          }}
           size="small"
           popupIcon={<ExpandMoreIcon data-testid="owner-picker-expand" />}
           renderInput={params => (
             <TextField
               {...params}
-              className={classes.input}
+              onChange={e => {
+                setText(e.currentTarget.value);
+              }}
               variant="outlined"
             />
           )}
+          ListboxProps={{
+            onScroll: (e: React.MouseEvent) => {
+              const element = e.currentTarget;
+              const hasReachedEnd =
+                Math.abs(
+                  element.scrollHeight -
+                    element.clientHeight -
+                    element.scrollTop,
+                ) < 1;
+
+              if (hasReachedEnd && value?.pageInfo.nextCursor) {
+                handleFetch({
+                  cursor: value.pageInfo.nextCursor,
+                  prev: value.items,
+                });
+              }
+            },
+            'data-testid': 'owner-picker-listbox',
+          }}
         />
       </Typography>
     </Box>

yarn.lock

@@ -5823,6 +5823,7 @@ __metadata:
     "@material-ui/core": ^4.12.2
     "@material-ui/icons": ^4.9.1
     "@material-ui/lab": 4.0.0-alpha.61
+    "@react-hookz/web": ^23.0.0
     "@testing-library/dom": ^8.0.0
     "@testing-library/jest-dom": ^5.10.1
     "@testing-library/react": ^12.1.3
@@ -13863,6 +13864,22 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@react-hookz/web@npm:^23.0.0":
+  version: 23.0.0
+  resolution: "@react-hookz/web@npm:23.0.0"
+  dependencies:
+    "@react-hookz/deep-equal": ^1.0.4
+  peerDependencies:
+    js-cookie: ^3.0.1
+    react: ^16.8 || ^17 || ^18
+    react-dom: ^16.8 || ^17 || ^18
+  peerDependenciesMeta:
+    js-cookie:
+      optional: true
+  checksum: 230bff62291bcafa65e0b9adcc3f0769fd63fabc226c77149e7797d1ea67362e9ad581ceff3cdfe2949698798d2deefb223c05bdfed1e465b5d92298d48cf3e5
+  languageName: node
+  linkType: hard
+
 "@remix-run/router@npm:1.3.2":
   version: 1.3.2
   resolution: "@remix-run/router@npm:1.3.2"