From c22c31ed13175ca4813b5d1f0556d6532c0cda1d Mon Sep 17 00:00:00 2001 From: Levente Balogh Date: Tue, 18 Apr 2023 08:15:35 +0200 Subject: [PATCH] Plugins: Fix plugin catalog filtering (#66663) * fix: make the filters work together * tests: add more tests --- .../plugins/admin/components/SearchField.tsx | 2 + .../plugins/admin/pages/Browse.test.tsx | 37 ++++++++++ .../features/plugins/admin/pages/Browse.tsx | 2 +- .../plugins/admin/state/selectors.test.ts | 69 +++++++++++++++++++ .../features/plugins/admin/state/selectors.ts | 54 +++++++-------- 5 files changed, 136 insertions(+), 28 deletions(-) create mode 100644 public/app/features/plugins/admin/state/selectors.test.ts diff --git a/public/app/features/plugins/admin/components/SearchField.tsx b/public/app/features/plugins/admin/components/SearchField.tsx index c251f34b1cc..64866968616 100644 --- a/public/app/features/plugins/admin/components/SearchField.tsx +++ b/public/app/features/plugins/admin/components/SearchField.tsx @@ -20,6 +20,7 @@ const useDebounceWithoutFirstRender = (callBack: () => any, delay = 0, deps: Rea isFirstRender.current = false; return; } + console.log('--------- DEBUOUNCE'); return callBack(); }, delay, @@ -42,6 +43,7 @@ export const SearchField = ({ value, onSearch }: Props) => { }} placeholder="Search Grafana plugins" onChange={(value) => { + console.log('--------- ONCHANGE', value); setQuery(value); }} width={46} diff --git a/public/app/features/plugins/admin/pages/Browse.test.tsx b/public/app/features/plugins/admin/pages/Browse.test.tsx index b3da8c31164..554556330d8 100644 --- a/public/app/features/plugins/admin/pages/Browse.test.tsx +++ b/public/app/features/plugins/admin/pages/Browse.test.tsx @@ -207,6 +207,43 @@ describe('Browse list of plugins', () => { expect(queryByText('Plugin 2')).not.toBeInTheDocument(); expect(queryByText('Plugin 3')).not.toBeInTheDocument(); }); + + it('should be possible to filter plugins by type', async () => { + const { queryByText } = renderBrowse('/plugins?filterByType=datasource&filterBy=all', [ + getCatalogPluginMock({ id: 'plugin-1', name: 'Plugin 1', type: PluginType.app }), + getCatalogPluginMock({ id: 'plugin-2', name: 'Plugin 2', type: PluginType.app }), + getCatalogPluginMock({ id: 'plugin-3', name: 'Plugin 3', type: PluginType.datasource }), + ]); + await waitFor(() => expect(queryByText('Plugin 3')).toBeInTheDocument()); + // Other plugin types shouldn't be shown + expect(queryByText('Plugin 1')).not.toBeInTheDocument(); + expect(queryByText('Plugin 2')).not.toBeInTheDocument(); + }); + + it('should be possible to filter plugins both by type and a keyword', async () => { + const { queryByText } = renderBrowse('/plugins?filterByType=datasource&filterBy=all&q=Foo', [ + getCatalogPluginMock({ id: 'plugin-1', name: 'Plugin 1', type: PluginType.app }), + getCatalogPluginMock({ id: 'plugin-2', name: 'Plugin 2', type: PluginType.datasource }), + getCatalogPluginMock({ id: 'plugin-3', name: 'Foo plugin', type: PluginType.datasource }), + ]); + await waitFor(() => expect(queryByText('Foo plugin')).toBeInTheDocument()); + // Other plugin types shouldn't be shown + expect(queryByText('Plugin 1')).not.toBeInTheDocument(); + expect(queryByText('Plugin 2')).not.toBeInTheDocument(); + }); + + it('should list all available plugins if the keyword is empty', async () => { + const { queryByText } = renderBrowse('/plugins?filterBy=all&q=', [ + getCatalogPluginMock({ id: 'plugin-1', name: 'Plugin 1', type: PluginType.app }), + getCatalogPluginMock({ id: 'plugin-2', name: 'Plugin 2', type: PluginType.panel }), + getCatalogPluginMock({ id: 'plugin-3', name: 'Plugin 3', type: PluginType.datasource }), + ]); + + // We did not filter for any specific plugin type, so all plugins should be shown + await waitFor(() => expect(queryByText('Plugin 1')).toBeInTheDocument()); + expect(queryByText('Plugin 2')).toBeInTheDocument(); + expect(queryByText('Plugin 3')).toBeInTheDocument(); + }); }); describe('when sorting', () => { diff --git a/public/app/features/plugins/admin/pages/Browse.tsx b/public/app/features/plugins/admin/pages/Browse.tsx index 62cb83ba6dc..61cb863e675 100644 --- a/public/app/features/plugins/admin/pages/Browse.tsx +++ b/public/app/features/plugins/admin/pages/Browse.tsx @@ -55,7 +55,7 @@ export default function Browse({ route }: GrafanaRouteComponentProps): ReactElem }; const onSearch = (q: string) => { - history.push({ query: { filterBy: 'all', filterByType: 'all', q } }); + history.push({ query: { filterBy, filterByType, q } }); }; // How should we handle errors? diff --git a/public/app/features/plugins/admin/state/selectors.test.ts b/public/app/features/plugins/admin/state/selectors.test.ts new file mode 100644 index 00000000000..c038a4930f2 --- /dev/null +++ b/public/app/features/plugins/admin/state/selectors.test.ts @@ -0,0 +1,69 @@ +import { PluginType } from '@grafana/data'; +import { configureStore } from 'app/store/configureStore'; + +import { getCatalogPluginMock, getPluginsStateMock } from '../__mocks__'; + +import { find } from './selectors'; + +describe('Plugins Selectors', () => { + describe('find()', () => { + const store = configureStore({ + plugins: getPluginsStateMock([ + getCatalogPluginMock({ id: 'plugin-1', name: 'Plugin 1', isInstalled: true, type: PluginType.datasource }), + getCatalogPluginMock({ id: 'plugin-2', name: 'Plugin 2', isInstalled: true, type: PluginType.datasource }), + getCatalogPluginMock({ id: 'plugin-3', name: 'Plugin 3', isInstalled: true, type: PluginType.panel }), + getCatalogPluginMock({ id: 'plugin-4', name: 'Plugin 4', isInstalled: false, type: PluginType.panel }), + getCatalogPluginMock({ id: 'plugin-5', name: 'Plugin 5', isInstalled: true, type: PluginType.app }), + ]), + }); + + it('should return all plugins if there are no filters', () => { + const query = ''; + const filterBy = 'all'; + const filterByType = 'all'; + const results = find(query, filterBy, filterByType)(store.getState()); + + expect(results).toHaveLength(5); + }); + + it('should be possible to search only by the "query"', () => { + const query = 'Plugin 3'; + const filterBy = 'all'; + const filterByType = 'all'; + const results = find(query, filterBy, filterByType)(store.getState()); + + expect(results).toHaveLength(1); + expect(results[0].name).toBe('Plugin 3'); + }); + + it('should be possible to search by plugin type', () => { + const query = ''; + const filterBy = 'all'; + const filterByType = PluginType.panel; + const results = find(query, filterBy, filterByType)(store.getState()); + + expect(results).toHaveLength(2); + expect(results.map(({ name }) => name)).toEqual(['Plugin 3', 'Plugin 4']); + }); + + it('should be possible to search by plugin state (installed / all)', () => { + const query = ''; + const filterBy = 'installed'; + const filterByType = 'all'; + const results = find(query, filterBy, filterByType)(store.getState()); + + expect(results).toHaveLength(4); + expect(results.map(({ name }) => name)).toEqual(['Plugin 1', 'Plugin 2', 'Plugin 3', 'Plugin 5']); + }); + + it('should be possible to search by multiple filters', () => { + const query = '2'; + const filterBy = 'all'; + const filterByType = PluginType.datasource; + const results = find(query, filterBy, filterByType)(store.getState()); + + expect(results).toHaveLength(1); + expect(results[0].name).toBe('Plugin 2'); + }); + }); +}); diff --git a/public/app/features/plugins/admin/state/selectors.ts b/public/app/features/plugins/admin/state/selectors.ts index 1b84ff6cd60..f6068facdd8 100644 --- a/public/app/features/plugins/admin/state/selectors.ts +++ b/public/app/features/plugins/admin/state/selectors.ts @@ -2,7 +2,7 @@ import { createSelector } from '@reduxjs/toolkit'; import { PluginError, PluginErrorCode, unEscapeStringFromRegex } from '@grafana/data'; -import { RequestStatus, PluginCatalogStoreState } from '../types'; +import { RequestStatus, PluginCatalogStoreState, CatalogPlugin } from '../types'; import { pluginsAdapter } from './reducer'; @@ -14,43 +14,43 @@ export const selectDisplayMode = createSelector(selectRoot, ({ settings }) => se export const { selectAll, selectById } = pluginsAdapter.getSelectors(selectItems); -const selectInstalled = (filterBy: string) => +const findByState = (state: string) => createSelector(selectAll, (plugins) => - plugins.filter((plugin) => (filterBy === 'installed' ? plugin.isInstalled : !plugin.isCore)) + plugins.filter((plugin) => (state === 'installed' ? plugin.isInstalled : !plugin.isCore)) ); -const findByInstallAndType = (filterBy: string, filterByType: string) => - createSelector(selectInstalled(filterBy), (plugins) => - plugins.filter((plugin) => filterByType === 'all' || plugin.type === filterByType) +type PluginFilters = { + state: string; + type: string; +}; + +const findPluginsByFilters = (filters: PluginFilters) => + createSelector(findByState(filters.state), (plugins) => + plugins.filter((plugin) => filters.type === 'all' || plugin.type === filters.type) ); -const findByKeyword = (searchBy: string) => - createSelector(selectAll, (plugins) => { - if (searchBy === '') { - return []; +const findByKeyword = (plugins: CatalogPlugin[], query: string) => { + if (query === '') { + return plugins; + } + + return plugins.filter((plugin) => { + const fields: String[] = []; + if (plugin.name) { + fields.push(plugin.name.toLowerCase()); } - return plugins.filter((plugin) => { - const fields: String[] = []; - if (plugin.name) { - fields.push(plugin.name.toLowerCase()); - } + if (plugin.orgName) { + fields.push(plugin.orgName.toLowerCase()); + } - if (plugin.orgName) { - fields.push(plugin.orgName.toLowerCase()); - } - - return fields.some((f) => f.includes(unEscapeStringFromRegex(searchBy).toLowerCase())); - }); + return fields.some((f) => f.includes(unEscapeStringFromRegex(query).toLowerCase())); }); +}; export const find = (searchBy: string, filterBy: string, filterByType: string) => - createSelector( - findByInstallAndType(filterBy, filterByType), - findByKeyword(searchBy), - (filteredPlugins, searchedPlugins) => { - return searchBy === '' ? filteredPlugins : searchedPlugins; - } + createSelector(findPluginsByFilters({ state: filterBy, type: filterByType }), (filteredPlugins) => + findByKeyword(filteredPlugins, searchBy) ); export const selectPluginErrors = createSelector(selectAll, (plugins) =>