From cf0d361710dd64641c2c391c896a39e6c5d8301e Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Wed, 12 May 2021 11:43:58 -0600 Subject: [PATCH] Fix CatalogPage and e2e tests Signed-off-by: Tim Hansen --- .../UserListPicker/UserListPicker.tsx | 14 +++- plugins/catalog-react/src/hooks/index.ts | 1 + .../src/hooks/useEntityListProvider.tsx | 19 ++++- .../CatalogPage/CatalogPage.test.tsx | 12 +-- .../EntityTypePicker/EntityTypePicker.tsx | 79 ++++++++++++------- 5 files changed, 83 insertions(+), 42 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index 16df12c2a7..6d1e77ca43 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/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 = () => { )} - + {item.label} diff --git a/plugins/catalog-react/src/hooks/index.ts b/plugins/catalog-react/src/hooks/index.ts index 46be6e9604..77026d1245 100644 --- a/plugins/catalog-react/src/hooks/index.ts +++ b/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'; diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index a9bac64158..7117b9b932 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/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) => void; + updateFilters: ( + filters: + | Partial + | ((prevFilters: EntityFilters) => Partial), + ) => void; loading: boolean; error?: Error; @@ -148,8 +152,17 @@ export const EntityListProvider = ({ }, [backendEntities, filterEnv, filters]); const updateFilters = useCallback( - (patch: Partial) => - setFilters(prevFilters => ({ ...prevFilters, ...patch })), + ( + update: + | Partial + | ((prevFilters: EntityFilters) => Partial), + ) => { + if (typeof update === 'function') { + setFilters(prevFilters => ({ ...prevFilters, ...update(prevFilters) })); + } else { + setFilters(prevFilters => ({ ...prevFilters, ...update })); + } + }, [], ); diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx index 03f211a7bb..2f08fb15c1 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx +++ b/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(); + const { findByText, getByTestId } = renderWrapped(); 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( , ); 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()); }); }); diff --git a/plugins/catalog/src/components/EntityTypePicker/EntityTypePicker.tsx b/plugins/catalog/src/components/EntityTypePicker/EntityTypePicker.tsx index e8ef1de8c6..94ad2323c9 100644 --- a/plugins/catalog/src/components/EntityTypePicker/EntityTypePicker.tsx +++ b/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([]); - 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 (