From 67590600645a97dd45fa5768ea52e366297a5269 Mon Sep 17 00:00:00 2001 From: Oscar Hernandez Date: Wed, 10 Feb 2021 10:29:17 -0600 Subject: [PATCH] Scaffolder: search/filter cleanup --- .../ResultsFilter/ResultsFilter.tsx | 18 +- .../ScaffolderFilter.test.tsx | 339 +++++++++--------- .../ScaffolderPage/ScaffolderPage.tsx | 22 +- .../scaffolder/src/components/useOwnUser.ts | 41 --- 4 files changed, 192 insertions(+), 228 deletions(-) delete mode 100644 plugins/scaffolder/src/components/useOwnUser.ts diff --git a/plugins/scaffolder/src/components/ResultsFilter/ResultsFilter.tsx b/plugins/scaffolder/src/components/ResultsFilter/ResultsFilter.tsx index 5df70bd203..19c88a87f7 100644 --- a/plugins/scaffolder/src/components/ResultsFilter/ResultsFilter.tsx +++ b/plugins/scaffolder/src/components/ResultsFilter/ResultsFilter.tsx @@ -87,25 +87,27 @@ export const ResultsFilter = ({ availableCategories }: Props) => { Categories - {availableCategories.map(t => { - const labelId = `checkbox-list-label-${t}`; + {availableCategories.map(category => { + const labelId = `checkbox-list-label-${category}`; return ( updateSelectedCategories( - selectedCategories.includes(t) - ? selectedCategories.filter(s => s !== t) - : [...selectedCategories, t], + selectedCategories.includes(category) + ? selectedCategories.filter( + selectedCategory => selectedCategory !== category, + ) + : [...selectedCategories, category], ) } > { /> ); diff --git a/plugins/scaffolder/src/components/ScaffolderFilter/ScaffolderFilter.test.tsx b/plugins/scaffolder/src/components/ScaffolderFilter/ScaffolderFilter.test.tsx index 1c9a89c517..b5ec45d8bb 100644 --- a/plugins/scaffolder/src/components/ScaffolderFilter/ScaffolderFilter.test.tsx +++ b/plugins/scaffolder/src/components/ScaffolderFilter/ScaffolderFilter.test.tsx @@ -14,8 +14,9 @@ * limitations under the License. */ -import { CatalogApi } from '@backstage/catalog-client'; -import { Entity } from '@backstage/catalog-model'; +import React from 'react'; +import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; +import { fireEvent, render, waitFor } from '@testing-library/react'; import { ApiProvider, ApiRegistry, @@ -23,10 +24,9 @@ import { identityApiRef, storageApiRef, } from '@backstage/core'; +import { CatalogApi } from '@backstage/catalog-client'; +import { Entity } from '@backstage/catalog-model'; import { catalogApiRef } from '@backstage/plugin-catalog-react'; -import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; -import { fireEvent, render, waitFor } from '@testing-library/react'; -import React from 'react'; import { EntityFilterGroupsProvider } from '../../filter'; import { ButtonGroup, ScaffolderFilter } from './ScaffolderFilter'; @@ -80,185 +80,192 @@ describe('Catalog Filter', () => { ), ); - it('should render the different groups', async () => { - const mockGroups: ButtonGroup[] = [ - { name: 'Test Group 1', items: [] }, - { name: 'Test Group 2', items: [] }, - ]; - const { findByText } = renderWrapped( - , - ); - for (const group of mockGroups) { - expect(await findByText(group.name)).toBeInTheDocument(); - } - }); - - it('should render the different items and their names', async () => { - const mockGroups: ButtonGroup[] = [ - { - name: 'Test Group 1', - items: [ - { - id: 'all', - label: 'First Label', - filterFn: () => true, - }, - { - id: 'starred', - label: 'Second Label', - filterFn: () => false, - }, - ], - }, - ]; - - const { findByText } = renderWrapped( - , - ); - - for (const item of mockGroups[0].items) { - expect(await findByText(item.label)).toBeInTheDocument(); - } - }); - - it('selects the first item if no desired initial one is set', async () => { - const mockGroups: ButtonGroup[] = [ - { - name: 'Test Group 1', - items: [ - { - id: 'all', - label: 'First Label', - filterFn: () => true, - }, - { - id: 'starred', - label: 'Second Label', - filterFn: () => false, - }, - ], - }, - ]; - - const onChange = jest.fn(); - - renderWrapped( - , - ); - - await waitFor(() => { - expect(onChange).toHaveBeenLastCalledWith({ - id: 'all', - label: 'First Label', - }); + describe('filter groups', () => { + it('should render the different groups', async () => { + const mockGroups: ButtonGroup[] = [ + { name: 'Test Group 1', items: [] }, + { name: 'Test Group 2', items: [] }, + ]; + const { findByText } = renderWrapped( + , + ); + for (const group of mockGroups) { + expect(await findByText(group.name)).toBeInTheDocument(); + } }); }); - it('selects the initial item', async () => { - const mockGroups: ButtonGroup[] = [ - { - name: 'Test Group 1', - items: [ - { - id: 'all', - label: 'First Label', - filterFn: () => true, - }, - { - id: 'starred', - label: 'Second Label', - filterFn: () => false, - }, - ], - }, - ]; + describe('filter items', () => { + it('should render the different items and their names', async () => { + const mockGroups: ButtonGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'all', + label: 'First Label', + filterFn: () => true, + }, + { + id: 'starred', + label: 'Second Label', + filterFn: () => false, + }, + ], + }, + ]; - const onChange = jest.fn(); + const { findByText } = renderWrapped( + , + ); - renderWrapped( - , - ); - - await waitFor(() => { - expect(onChange).toHaveBeenLastCalledWith({ - id: 'starred', - label: 'Second Label', - }); + for (const item of mockGroups[0].items) { + expect(await findByText(item.label)).toBeInTheDocument(); + } }); - }); - it('can change the selected item', async () => { - const mockGroups: ButtonGroup[] = [ - { - name: 'Test Group 1', - items: [ - { - id: 'all', - label: 'First Label', - filterFn: () => true, - }, - { - id: 'starred', - label: 'Second Label', - filterFn: () => false, - }, - ], - }, - ]; + it('selects the first item if no desired initial one is set', async () => { + const mockGroups: ButtonGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'all', + label: 'First Label', + filterFn: () => true, + }, + { + id: 'starred', + label: 'Second Label', + filterFn: () => false, + }, + ], + }, + ]; - const onChange = jest.fn(); + const onChange = jest.fn(); - const { findByText } = renderWrapped( - , - ); + renderWrapped( + , + ); - await waitFor(() => { - expect(onChange).toHaveBeenLastCalledWith({ - id: 'all', - label: 'First Label', + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: 'all', + label: 'First Label', + }); }); }); - fireEvent.click(await findByText('Second Label')); + it('selects the initial item', async () => { + const mockGroups: ButtonGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'all', + label: 'First Label', + filterFn: () => true, + }, + { + id: 'starred', + label: 'Second Label', + filterFn: () => false, + }, + ], + }, + ]; - await waitFor(() => { - expect(onChange).toHaveBeenLastCalledWith({ - id: 'starred', - label: 'Second Label', + const onChange = jest.fn(); + + renderWrapped( + , + ); + + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: 'starred', + label: 'Second Label', + }); }); }); - }); - it('displays match counts properly', async () => { - const mockGroups: ButtonGroup[] = [ - { - name: 'Test Group 1', - items: [ - { - id: 'owned', - label: 'First Label', - filterFn: entity => entity.spec?.owner === 'tools@example.com', - }, - ], - }, - ]; + it('can change the selected item', async () => { + const mockGroups: ButtonGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'all', + label: 'First Label', + filterFn: () => true, + }, + { + id: 'starred', + label: 'Second Label', + filterFn: () => false, + }, + ], + }, + ]; - const { findByText } = renderWrapped( - , - ); + const onChange = jest.fn(); - expect(await findByText('1')).toBeInTheDocument(); + const { findByText } = renderWrapped( + , + ); + + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: 'all', + label: 'First Label', + }); + }); + + fireEvent.click(await findByText('Second Label')); + + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + id: 'starred', + label: 'Second Label', + }); + }); + }); + + it('displays match counts properly', async () => { + const mockGroups: ButtonGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'owned', + label: 'First Label', + filterFn: entity => entity.spec?.owner === 'tools@example.com', + }, + ], + }, + ]; + + const { findByText } = renderWrapped( + , + ); + + expect(await findByText('1')).toBeInTheDocument(); + }); }); }); diff --git a/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx b/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx index eba8a1bd48..f3ccd1d440 100644 --- a/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx +++ b/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx @@ -14,7 +14,8 @@ * limitations under the License. */ -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import React, { useEffect, useMemo, useState } from 'react'; +import { EntityMeta, TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { configApiRef, Content, @@ -28,7 +29,6 @@ import { WarningPanel, } from '@backstage/core'; import { Button, Grid, Link, makeStyles, Typography } from '@material-ui/core'; -import React, { useEffect, useMemo, useState } from 'react'; import { Link as RouterLink } from 'react-router-dom'; import { EntityFilterGroupsProvider, useFilteredEntities } from '../../filter'; import { TemplateCard, TemplateCardProps } from '../TemplateCard'; @@ -36,7 +36,6 @@ import { ResultsFilter } from '../ResultsFilter/ResultsFilter'; import { ScaffolderFilter } from '../ScaffolderFilter'; import { ButtonGroup } from '../ScaffolderFilter/ScaffolderFilter'; import StarIcon from '@material-ui/icons/Star'; -import { useOwnUser } from '../useOwnUser'; import { useStarredEntities } from '../../hooks/useStarredEntities'; import SearchToolbar from '../SearchToolbar/SearchToolbar'; @@ -104,21 +103,18 @@ export const ScaffolderPageContents = () => { [] as TemplateEntityV1alpha1[], ); - // Match templates by search input value + const matchesQuery = (metadata: EntityMeta, query: string) => + `${metadata.title}`.toUpperCase().indexOf(query) !== -1 || + metadata.tags?.join('').toUpperCase().indexOf(query) !== -1; + useEffect(() => { - const searchUppercase = search.toUpperCase(); if (search.length === 0) { return setMatchingEntities(filteredEntities); } - // Match search by title|tags return setMatchingEntities( - filteredEntities.filter(template => { - const { title, tags } = template.metadata; - return ( - `${title}`.toUpperCase().indexOf(searchUppercase) !== -1 || - tags?.join('').toUpperCase().indexOf(searchUppercase) !== -1 - ); - }), + filteredEntities.filter(template => + matchesQuery(template.metadata, search.toUpperCase()), + ), ); }, [search, filteredEntities]); diff --git a/plugins/scaffolder/src/components/useOwnUser.ts b/plugins/scaffolder/src/components/useOwnUser.ts deleted file mode 100644 index 29d8a0d11f..0000000000 --- a/plugins/scaffolder/src/components/useOwnUser.ts +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { UserEntity } from '@backstage/catalog-model'; -import { identityApiRef, useApi } from '@backstage/core'; -import { catalogApiRef } from '@backstage/plugin-catalog-react'; -import { useAsync } from 'react-use'; -import { AsyncState } from 'react-use/lib/useAsync'; - -/** - * Get the catalog User entity (if any) that matches the logged-in user. - */ -export function useOwnUser(): AsyncState { - const catalogApi = useApi(catalogApiRef); - const identityApi = useApi(identityApiRef); - - // TODO: get the full entity (or at least the full entity name) from the - // identityApi - return useAsync( - () => - catalogApi.getEntityByName({ - kind: 'User', - namespace: 'default', - name: identityApi.getUserId(), - }) as Promise, - [catalogApi, identityApi], - ); -}