From 410f98e2925a998b9868766bd565daf5e267c8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 25 Jun 2020 17:10:41 +0200 Subject: [PATCH] Clean up and finalize --- .../CatalogFilter/CatalogFilter.test.tsx | 221 ++++++---- .../CatalogFilter/CatalogFilter.tsx | 127 ++++-- .../CatalogPage/CatalogPage.test.tsx | 77 ++-- .../components/CatalogPage/CatalogPage.tsx | 118 ++--- .../components/CatalogPage/CatalogTabs.tsx | 55 +++ .../components/CatalogPage/WelcomeBanner.tsx | 55 +++ .../components/EntityPage/EntityPage.test.tsx | 4 +- plugins/catalog/src/data/filters.ts | 19 +- .../src/filter/EntityFilterGroupsProvider.tsx | 205 +++++++++ plugins/catalog/src/filter/context.ts | 38 ++ plugins/catalog/src/filter/index.ts | 28 ++ plugins/catalog/src/filter/types.ts | 53 +++ .../useEntityFilterGroup.test.tsx | 42 +- .../src/filter/useEntityFilterGroup.ts | 69 +++ .../catalog/src/filter/useFilteredEntities.ts | 34 ++ plugins/catalog/src/hooks/useEntities.tsx | 404 ------------------ .../src/hooks/useEntityFilterGroup.tsx | 273 ------------ 17 files changed, 870 insertions(+), 952 deletions(-) create mode 100644 plugins/catalog/src/components/CatalogPage/CatalogTabs.tsx create mode 100644 plugins/catalog/src/components/CatalogPage/WelcomeBanner.tsx create mode 100644 plugins/catalog/src/filter/EntityFilterGroupsProvider.tsx create mode 100644 plugins/catalog/src/filter/context.ts create mode 100644 plugins/catalog/src/filter/index.ts create mode 100644 plugins/catalog/src/filter/types.ts rename plugins/catalog/src/{hooks => filter}/useEntityFilterGroup.test.tsx (74%) create mode 100644 plugins/catalog/src/filter/useEntityFilterGroup.ts create mode 100644 plugins/catalog/src/filter/useFilteredEntities.ts delete mode 100644 plugins/catalog/src/hooks/useEntities.tsx delete mode 100644 plugins/catalog/src/hooks/useEntityFilterGroup.tsx diff --git a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx index cc4cc62fdb..9dd0caa3b4 100644 --- a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx +++ b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx @@ -14,63 +14,78 @@ * limitations under the License. */ +import { Entity } from '@backstage/catalog-model'; +import { + ApiProvider, + ApiRegistry, + IdentityApi, + identityApiRef, + storageApiRef, +} from '@backstage/core'; +import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; +import { fireEvent, render, waitFor } from '@testing-library/react'; import React from 'react'; -import { render, fireEvent } from '@testing-library/react'; -import { wrapInTestApp } from '@backstage/test-utils'; -import { CatalogFilter, CatalogFilterGroup } from './CatalogFilter'; +import { CatalogApi, catalogApiRef } from '../../api/types'; import { EntityGroup } from '../../data/filters'; +import { EntityFilterGroupsProvider } from '../../filter'; +import { CatalogFilter, CatalogFilterGroup } from './CatalogFilter'; describe('Catalog Filter', () => { - const comp1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - name: 'my-component-1', - }, - spec: { - owner: 'team', - }, + const catalogApi: Partial = { + getEntities: () => + Promise.resolve([ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'Entity1', + }, + spec: { + owner: 'tools@example.com', + type: 'service', + }, + }, + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'Entity2', + }, + spec: { + owner: 'not-tools@example.com', + type: 'service', + }, + }, + ] as Entity[]), }; - const comp2 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - name: 'my-component-2', - }, - spec: { - owner: 'team', - }, - }; - const comp3 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - name: 'my-component-3', - }, - spec: { - owner: '', - }, - }; - const defaultFilterProps = { - selectedFilter: EntityGroup.ALL, - onFilterChange: (type: EntityGroup) => type, - entitiesByFilter: { - [EntityGroup.ALL]: [comp1, comp2, comp3], - [EntityGroup.STARRED]: [comp1], - [EntityGroup.OWNED]: [comp1], - }, + + const indentityApi: Partial = { + getUserId: () => 'tools@example.com', }; + + const renderWrapped = (children: React.ReactNode) => + render( + wrapInTestApp( + + {children}, + , + ), + ); + it('should render the different groups', async () => { const mockGroups: CatalogFilterGroup[] = [ { name: 'Test Group 1', items: [] }, { name: 'Test Group 2', items: [] }, ]; - const { findByText } = render( - wrapInTestApp( - , - ), + const { findByText } = renderWrapped( + , ); - for (const group of mockGroups) { expect(await findByText(group.name)).toBeInTheDocument(); } @@ -93,19 +108,16 @@ describe('Catalog Filter', () => { }, ]; - const { findByText } = render( - wrapInTestApp( - , - ), + const { findByText } = renderWrapped( + , ); - const [group] = mockGroups; - for (const item of group.items) { + for (const item of mockGroups[0].items) { expect(await findByText(item.label)).toBeInTheDocument(); } }); - it('should render the count in each item', async () => { + it('selects the first item if no desired initial one is set', async () => { const mockGroups: CatalogFilterGroup[] = [ { name: 'Test Group 1', @@ -113,33 +125,30 @@ describe('Catalog Filter', () => { { id: EntityGroup.ALL, label: 'First Label', - count: 3, }, { id: EntityGroup.STARRED, label: 'Second Label', - count: 1, }, ], }, ]; - const { getAllByText } = render( - wrapInTestApp( - , - ), + const onChange = jest.fn(); + + renderWrapped( + , ); - for (const key of Object.keys(defaultFilterProps.entitiesByFilter)) { - const matcher = new RegExp( - `(${defaultFilterProps.entitiesByFilter[key as EntityGroup].length})`, - ); - const items = await getAllByText(matcher); - items.forEach(el => expect(el).toBeInTheDocument()); - } + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: EntityGroup.ALL, + label: 'First Label', + }); + }); }); - it('should fire the callback when an item is clicked', async () => { + it('selects the initial item', async () => { const mockGroups: CatalogFilterGroup[] = [ { name: 'Test Group 1', @@ -147,39 +156,34 @@ describe('Catalog Filter', () => { { id: EntityGroup.ALL, label: 'First Label', - count: 100, }, { id: EntityGroup.STARRED, label: 'Second Label', - count: 400, }, ], }, ]; - const onSelectedChangeHandler = jest.fn(); + const onChange = jest.fn(); - const { findByText } = render( - wrapInTestApp( - , - ), + renderWrapped( + , ); - const item = mockGroups[0].items[0]; - - const element = await findByText(item.label); - - fireEvent.click(element); - - expect(onSelectedChangeHandler).toHaveBeenCalledWith(item.id); + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: EntityGroup.STARRED, + label: 'Second Label', + }); + }); }); - it('should render a component when a function is passed to the count component', async () => { + it('can change the selected item', async () => { const mockGroups: CatalogFilterGroup[] = [ { name: 'Test Group 1', @@ -187,22 +191,55 @@ describe('Catalog Filter', () => { { id: EntityGroup.ALL, label: 'First Label', - count: () => BACKSTAGE!, }, { id: EntityGroup.STARRED, label: 'Second Label', - count: 400, }, ], }, ]; - const { findByText } = render( - wrapInTestApp( - , - ), + + const onChange = jest.fn(); + + const { findByText } = renderWrapped( + , ); - expect(await findByText('Test Group 1')).toBeInTheDocument(); + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: EntityGroup.ALL, + label: 'First Label', + }); + }); + + fireEvent.click(await findByText('Second Label')); + + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: EntityGroup.STARRED, + label: 'Second Label', + }); + }); + }); + + it('displays match counts properly', async () => { + const mockGroups: CatalogFilterGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: EntityGroup.OWNED, + label: 'First Label', + }, + ], + }, + ]; + + const { findByText } = renderWrapped( + , + ); + + expect(await findByText('1')).toBeInTheDocument(); }); }); diff --git a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx index e40f4806ed..71956e2e22 100644 --- a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx +++ b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx @@ -14,21 +14,26 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import { IconComponent, identityApiRef, useApi } from '@backstage/core'; import { Card, List, ListItemIcon, - ListItemText, ListItemSecondaryAction, - MenuItem, - Typography, - Theme, + ListItemText, makeStyles, + MenuItem, + Theme, + Typography, } from '@material-ui/core'; -import type { IconComponent } from '@backstage/core'; -import { EntityGroup } from '../../data/filters'; -import { EntitiesByFilter } from '../../hooks/useEntities'; +import React, { FC, useCallback, useMemo, useState, useEffect } from 'react'; +import { + EntityFilterOptions, + entityFilters, + EntityGroup, +} from '../../data/filters'; +import { FilterGroup, useEntityFilterGroup } from '../../filter'; +import { useStarredEntities } from '../../hooks/useStarredEntites'; export type CatalogFilterItem = { id: EntityGroup; @@ -68,21 +73,43 @@ const useStyles = makeStyles(theme => ({ }, })); -export const CatalogFilter: FC<{ - selectedFilter: EntityGroup; - onFilterChange: (type: EntityGroup) => void; - entitiesByFilter: EntitiesByFilter; - groups: CatalogFilterGroup[]; -}> = ({ - selectedFilter: selectedId, - onFilterChange: setSelectedFilter, - entitiesByFilter, - groups, -}) => { +type Props = { + filterGroups: CatalogFilterGroup[]; + onChange?: (filterItem: CatalogFilterItem) => void; + initiallySelected?: EntityGroup; +}; + +export const CatalogFilter = ({ + filterGroups, + onChange, + initiallySelected, +}: Props) => { const classes = useStyles(); + const { currentFilter, setCurrentFilter, getFilterCount } = useFilter(); + + const setCurrent = useCallback( + (item: CatalogFilterItem) => { + setCurrentFilter(item.id); + onChange?.(item); + }, + [onChange, setCurrentFilter], + ); + + // Make one initial onChange to inform the surroundings about the selected + // item + useEffect(() => { + const items = filterGroups.flatMap(g => g.items); + const item = items.find(i => i.id === initiallySelected) || items[0]; + if (item) { + onChange?.(item); + } + // intentionally only happens on startup + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + return ( - {groups.map(group => ( + {filterGroups.map(group => ( {group.name} @@ -94,10 +121,8 @@ export const CatalogFilter: FC<{ key={item.id} button divider - onClick={() => { - setSelectedFilter(item.id); - }} - selected={item.id === selectedId} + onClick={() => setCurrent(item)} + selected={item.id === currentFilter} className={classes.menuItem} > {item.icon && ( @@ -111,7 +136,7 @@ export const CatalogFilter: FC<{ - {entitiesByFilter[item.id]?.length ?? '-'} + {getFilterCount(item.id) ?? '-'} ))} @@ -122,3 +147,55 @@ export const CatalogFilter: FC<{ ); }; + +function useFilter(): { + currentFilter: string; + setCurrentFilter: (filterId: string) => void; + getFilterCount: (filterId: string) => number | undefined; +} { + const [currentFilter, setCurrentFilter] = useState('OWNED'); + const { isStarredEntity } = useStarredEntities(); + const userId = useApi(identityApiRef).getUserId(); + + const filterGroup = useMemo(() => { + const result: FilterGroup = { filters: {} }; + const options: EntityFilterOptions = { + userId, + isStarred: isStarredEntity, + }; + for (const [filterId, filterFn] of Object.entries(entityFilters)) { + result.filters[filterId] = entity => filterFn(entity, options); + } + return result; + }, [isStarredEntity, userId]); + + const { setSelectedFilters, state } = useEntityFilterGroup( + 'primary-sidebar', + filterGroup, + ['OWNED'], + ); + + const setCurrent = useCallback( + (filterId: string) => { + setCurrentFilter(filterId); + setSelectedFilters([filterId]); + }, + [setCurrentFilter, setSelectedFilters], + ); + + const getFilterCount = useCallback( + (filterId: string) => { + if (state.type !== 'ready') { + return undefined; + } + return state.state.filters[filterId].matchCount; + }, + [state], + ); + + return { + currentFilter, + setCurrentFilter: setCurrent, + getFilterCount, + }; +} diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx index 7ba1c39bb7..b4dcc2cb20 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx @@ -14,45 +14,43 @@ * limitations under the License. */ +import { Entity } from '@backstage/catalog-model'; import { ApiProvider, ApiRegistry, - errorApiRef, - storageApiRef, - WebStorage, IdentityApi, identityApiRef, + storageApiRef, } from '@backstage/core'; -import { MockErrorApi, wrapInTestApp } from '@backstage/test-utils'; -import { render, fireEvent } from '@testing-library/react'; +import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; +import { fireEvent, render } from '@testing-library/react'; import React from 'react'; import { catalogApiRef } from '../..'; import { CatalogApi } from '../../api/types'; +import { EntityFilterGroupsProvider } from '../../filter'; import { CatalogPage } from './CatalogPage'; -import { Entity } from '@backstage/catalog-model'; describe('CatalogPage', () => { - const mockErrorApi = new MockErrorApi(); const catalogApi: Partial = { getEntities: () => Promise.resolve([ { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', metadata: { name: 'Entity1', }, - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', spec: { owner: 'tools@example.com', type: 'service', }, }, { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', metadata: { name: 'Entity2', }, - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', spec: { owner: 'not-tools@example.com', type: 'service', @@ -62,49 +60,32 @@ describe('CatalogPage', () => { getLocationByEntity: () => Promise.resolve({ id: 'id', type: 'github', target: 'url' }), }; - const mockIndentityApi: Partial = { + const indentityApi: Partial = { getUserId: () => 'tools@example.com', }; + const renderWrapped = (children: React.ReactNode) => + render( + wrapInTestApp( + + {children}, + , + ), + ); + // this test right now causes some red lines in the log output when running tests // related to some theme issues in mui-table // https://github.com/mbrn/material-table/issues/1293 it('should render', async () => { - const { findByText } = render( - wrapInTestApp( - - - , - ), - ); - - const items = await findByText(/All Services \(2\)/); - expect(items).toBeInTheDocument(); - }); - it('should filter by owner', async () => { - const { findByText, getByText } = render( - wrapInTestApp( - - - , - ), - ); - fireEvent.click(getByText(/Owned/)); - const items = await findByText(/Owned \(1\)/); - expect(items).toBeInTheDocument(); + const { findByText, getByText } = renderWrapped(); + expect(await findByText(/Owned \(1\)/)).toBeInTheDocument(); + fireEvent.click(getByText(/All/)); + expect(await findByText(/All \(2\)/)).toBeInTheDocument(); }); }); diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx index 9329eb9317..be357dfed1 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx @@ -15,38 +15,31 @@ */ import { Entity, LocationSpec } from '@backstage/catalog-model'; -import { - Content, - ContentHeader, - DismissableBanner, - HeaderTabs, - SupportButton, -} from '@backstage/core'; -import CatalogLayout from './CatalogLayout'; +import { Content, ContentHeader, SupportButton } from '@backstage/core'; import { rootRoute as scaffolderRootRoute } from '@backstage/plugin-scaffolder'; -import { - Button, - Link, - makeStyles, - Typography, - withStyles, -} from '@material-ui/core'; +import { Button, makeStyles, withStyles } from '@material-ui/core'; import Edit from '@material-ui/icons/Edit'; import GitHub from '@material-ui/icons/GitHub'; import Star from '@material-ui/icons/Star'; import StarOutline from '@material-ui/icons/StarBorder'; -import React, { FC } from 'react'; +import React, { useCallback, useState } from 'react'; import { Link as RouterLink } from 'react-router-dom'; -import { CatalogFilter } from '../CatalogFilter/CatalogFilter'; -import { CatalogTable } from '../CatalogTable/CatalogTable'; -import { useEntities } from '../../hooks/useEntities'; -import { findLocationForEntityMeta } from '../../data/utils'; import { - getCatalogFilterItemByType, EntityGroup, filterGroups, - labeledEntityTypes, + LabeledEntityType, } from '../../data/filters'; +import { findLocationForEntityMeta } from '../../data/utils'; +import { EntityFilterGroupsProvider, useFilteredEntities } from '../../filter'; +import { useStarredEntities } from '../../hooks/useStarredEntites'; +import { + CatalogFilter, + CatalogFilterItem, +} from '../CatalogFilter/CatalogFilter'; +import { CatalogTable } from '../CatalogTable/CatalogTable'; +import CatalogLayout from './CatalogLayout'; +import { CatalogTabs } from './CatalogTabs'; +import { WelcomeBanner } from './WelcomeBanner'; const useStyles = makeStyles(theme => ({ contentWrapper: { @@ -55,27 +48,14 @@ const useStyles = makeStyles(theme => ({ gridTemplateColumns: '250px 1fr', gridColumnGap: theme.spacing(2), }, - emoji: { - fontSize: '125%', - marginRight: theme.spacing(2), - }, })); -export const CatalogPage: FC<{}> = () => { - const { - entitiesByFilter, - error, - loading, - selectedFilter, - setSelectedFilter, - toggleStarredEntity, - isStarredEntity, - selectTypeFilter, - } = useEntities(); - - const filteredEntities = entitiesByFilter[selectedFilter ?? EntityGroup.ALL]; - +const CatalogPageContents = () => { const styles = useStyles(); + const { isStarredEntity, toggleStarredEntity } = useStarredEntities(); + const { loading, error, matchingEntities } = useFilteredEntities(); + const [selectedTab, setSelectedTab] = useState(); + const [selectedSidebarItem, setSelectedSidebarItem] = useState(); const YellowStar = withStyles({ root: { @@ -105,9 +85,7 @@ export const CatalogPage: FC<{}> = () => { return location.target; } }; - const location = findLocationForEntityMeta(rowData.metadata); - return { icon: Edit, tooltip: 'Edit', @@ -129,33 +107,19 @@ export const CatalogPage: FC<{}> = () => { }, ]; + const onTabChanged = useCallback((type: LabeledEntityType) => { + setSelectedTab(type.label); + }, []); + const onSidebarChanged = useCallback((filterItem: CatalogFilterItem) => { + setSelectedSidebarItem(filterItem.label); + }, []); + return ( - { - selectTypeFilter(labeledEntityTypes[index as number].id); - }} - /> + - - - 👋🏼 - - Welcome to Backstage, we are happy to have you. Start by checking - out our{' '} - - getting started - {' '} - page. - - } - id="catalog_page_welcome_banner" - /> - + +