From 6ad9e386ad425453311d307b734b18ac5bea5dd8 Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Thu, 6 Jul 2023 11:50:55 +0200 Subject: [PATCH] DS Picker: Filter available DS based on component props (#70613) * Apply filters consistently to every list in the picker * Display all built-in DS when editing a panel * Add `uploadFile` prop to toggle the CSV file DS --- .../src/selectors/components.ts | 6 +- .../components/FileDropzone/FileDropzone.tsx | 2 +- .../TopBar/TopSearchBarSection.test.tsx | 8 +- .../AppChrome/TopBar/TopSearchBarSection.tsx | 2 +- .../picker/BuiltInDataSourceList.tsx | 47 +++++++- .../picker/DataSourceDropdown.test.tsx | 27 ++++- .../components/picker/DataSourceDropdown.tsx | 37 ++++-- .../picker/DataSourceModal.test.tsx | 114 +++++++++++++----- .../components/picker/DataSourceModal.tsx | 65 +++++++--- .../datasources/components/picker/utils.ts | 4 +- .../features/query/components/QueryGroup.tsx | 7 +- 11 files changed, 240 insertions(+), 79 deletions(-) diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index a223ac0460c..05d789ae69e 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/packages/grafana-e2e-selectors/src/selectors/components.ts @@ -2,8 +2,8 @@ // however there are many cases where your component may not need an aria-label // (a )} - {onClickAddCSV && config.featureToggles.editPanelCSVDragAndDrop && ( + {uploadFile && config.featureToggles.editPanelCSVDragAndDrop && ( diff --git a/public/app/features/datasources/components/picker/DataSourceModal.test.tsx b/public/app/features/datasources/components/picker/DataSourceModal.test.tsx index 96daf836584..7d95388e3d7 100644 --- a/public/app/features/datasources/components/picker/DataSourceModal.test.tsx +++ b/public/app/features/datasources/components/picker/DataSourceModal.test.tsx @@ -1,11 +1,11 @@ -import { findByText, queryByText, render, screen } from '@testing-library/react'; +import { queryByTestId, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { DataSourceInstanceSettings, DataSourcePluginMeta, PluginMetaInfo, PluginType } from '@grafana/data'; -import { config, GetDataSourceListFilters } from '@grafana/runtime'; +import { config } from '@grafana/runtime'; -import { DataSourceModal } from './DataSourceModal'; +import { DataSourceModal, DataSourceModalProps } from './DataSourceModal'; const pluginMetaInfo: PluginMetaInfo = { author: { name: '' }, @@ -40,10 +40,17 @@ const mockDSBuiltIn = createDS('mock.datasource.builtin', 3, true); const mockDSList = [mockDS1, mockDS2, mockDSBuiltIn]; -const setup = (onChange = () => {}, onDismiss = () => {}) => { - const props = { onChange, onDismiss, current: mockDS1.name }; +const setup = (partialProps: Partial = {}) => { window.HTMLElement.prototype.scrollIntoView = function () {}; - return render(); + + const props: DataSourceModalProps = { + ...partialProps, + onChange: partialProps.onChange || jest.fn(), + onDismiss: partialProps.onDismiss || jest.fn(), + current: partialProps.current || mockDS1, + }; + + return render(); }; jest.mock('@grafana/runtime', () => { @@ -61,17 +68,19 @@ jest.mock('@grafana/runtime', () => { jest.mock('@grafana/runtime/src/services/dataSourceSrv', () => { return { getDataSourceSrv: () => ({ - getList: (filters: GetDataSourceListFilters) => { - if (filters.filter) { - return mockDSList.filter(filters.filter); - } - return mockDSList; - }, - getInstanceSettings: () => mockDS1, + getList: getListMock, + getInstanceSettings: getInstanceSettingsMock, }), }; }); +const getListMock = jest.fn(); +const getInstanceSettingsMock = jest.fn(); +beforeEach(() => { + getListMock.mockReturnValue(mockDSList); + getInstanceSettingsMock.mockReturnValue(mockDS1); +}); + describe('DataSourceDropdown', () => { it('should render', () => { expect(() => setup()).not.toThrow(); @@ -90,24 +99,74 @@ describe('DataSourceDropdown', () => { }); it('only displays the file drop area when the the ff is enabled', async () => { + const defaultValue = config.featureToggles.editPanelCSVDragAndDrop; config.featureToggles.editPanelCSVDragAndDrop = true; - setup(); - expect(await screen.findByText('Drop file here or click to upload')).toBeInTheDocument(); - config.featureToggles.editPanelCSVDragAndDrop = false; + setup({ uploadFile: true }); + + expect(await screen.queryByTestId('file-drop-zone-default-children')).toBeInTheDocument(); + config.featureToggles.editPanelCSVDragAndDrop = defaultValue; }); it('does not show the file drop area when the ff is disabled', async () => { - setup(); - expect(screen.queryByText('Drop file here or click to upload')).toBeNull(); + const defaultValue = config.featureToggles.editPanelCSVDragAndDrop; + config.featureToggles.editPanelCSVDragAndDrop = false; + + setup({ uploadFile: true }); + expect(await screen.queryByTestId('file-drop-zone-default-children')).toBeNull(); + + config.featureToggles.editPanelCSVDragAndDrop = defaultValue; }); - it('should only display built in datasources in the right column', async () => { - setup(); - const dsList = await screen.findByTestId('data-sources-list'); - const builtInDSList = (await screen.findAllByTestId('built-in-data-sources-list'))[1]; //The second element needs to be selected as the first element is the one on the left, under the regular data sources. + it('should not display the drop zone by default', async () => { + const defaultValue = config.featureToggles.editPanelCSVDragAndDrop; + config.featureToggles.editPanelCSVDragAndDrop = true; - expect(queryByText(dsList, mockDSBuiltIn.name)).toBeNull(); - expect(await findByText(builtInDSList, mockDSBuiltIn.name, { selector: 'span' })).toBeInTheDocument(); + const component = setup(); + + expect(queryByTestId(component.container, 'file-drop-zone-default-children')).toBeNull(); + config.featureToggles.editPanelCSVDragAndDrop = defaultValue; + }); + + it('should display the drop zone when uploadFile is enabled', async () => { + const defaultValue = config.featureToggles.editPanelCSVDragAndDrop; + config.featureToggles.editPanelCSVDragAndDrop = true; + setup({ uploadFile: true }); + + expect(await screen.queryByTestId('file-drop-zone-default-children')).toBeInTheDocument(); + config.featureToggles.editPanelCSVDragAndDrop = defaultValue; + }); + + it('should fetch the DS applying the correct filters consistently across lists', async () => { + const filters = { + mixed: true, + tracing: true, + dashboard: true, + metrics: true, + type: 'foo', + annotations: true, + variables: true, + alerting: true, + pluginId: 'pluginid', + logs: true, + }; + + const props = { + onChange: () => {}, + onDismiss: () => {}, + current: mockDS1.name, + ...filters, + }; + + getListMock.mockClear(); + render(); + + // Every call to the service must contain same filters + expect(getListMock).toHaveBeenCalled(); + getListMock.mock.calls.forEach((call) => + expect(call[0]).toMatchObject({ + ...filters, + }) + ); }); }); @@ -134,9 +193,10 @@ describe('DataSourceDropdown', () => { const user = userEvent.setup(); config.featureToggles.editPanelCSVDragAndDrop = true; const onChange = jest.fn(); - setup(onChange); + setup({ onChange, uploadFile: true }); + const fileInput = ( - await screen.findByText('Drop file here or click to upload') + await screen.queryByTestId('file-drop-zone-default-children')! ).parentElement!.parentElement!.querySelector('input'); const file = new File([''], 'test.csv', { type: 'text/plain' }); expect(fileInput).toBeInTheDocument(); @@ -154,7 +214,7 @@ describe('DataSourceDropdown', () => { it('should call the onChange handler with the correct datasource', async () => { const user = userEvent.setup(); const onChange = jest.fn(); - setup(onChange); + setup({ onChange }); await user.click(await screen.findByText(mockDS2.name, { selector: 'span' })); expect(onChange.mock.lastCall[0].name).toEqual(mockDS2.name); }); diff --git a/public/app/features/datasources/components/picker/DataSourceModal.tsx b/public/app/features/datasources/components/picker/DataSourceModal.tsx index 013d3b45a47..b1caf35835d 100644 --- a/public/app/features/datasources/components/picker/DataSourceModal.tsx +++ b/public/app/features/datasources/components/picker/DataSourceModal.tsx @@ -35,7 +35,7 @@ const INTERACTION_ITEM = { DISMISS: 'dismiss', }; -interface DataSourceModalProps { +export interface DataSourceModalProps { onChange: (ds: DataSourceInstanceSettings, defaultQueries?: DataQuery[] | GrafanaQuery[]) => void; current: DataSourceRef | string | null | undefined; onDismiss: () => void; @@ -43,6 +43,7 @@ interface DataSourceModalProps { reportedInteractionFrom?: string; // DS filters + filter?: (ds: DataSourceInstanceSettings) => boolean; tracing?: boolean; mixed?: boolean; dashboard?: boolean; @@ -53,11 +54,22 @@ interface DataSourceModalProps { alerting?: boolean; pluginId?: string; logs?: boolean; + uploadFile?: boolean; } export function DataSourceModal({ + tracing, dashboard, mixed, + metrics, + type, + annotations, + variables, + alerting, + pluginId, + logs, + uploadFile, + filter, onChange, current, onDismiss, @@ -106,6 +118,29 @@ export function DataSourceModal({ } }); + // Built-in data sources used twice because of mobile layout adjustments + // In movile the list is appended to the bottom of the DS list + const BuiltInList = ({ className }: { className?: string }) => { + return ( + + ); + }; + return ( matchDataSourceWithSearch(ds, search) && !ds.meta.builtIn} onChange={onChangeDataSource} current={current} onClickEmptyStateCTA={() => @@ -144,27 +175,27 @@ export function DataSourceModal({ src: analyticsInteractionSrc, }) } - /> - (filter ? filter?.(ds) : true) && matchDataSourceWithSearch(ds, search) && !ds.meta.builtIn} + variables={variables} + tracing={tracing} + metrics={metrics} + type={type} + annotations={annotations} + alerting={alerting} + pluginId={pluginId} + logs={logs} dashboard={dashboard} mixed={mixed} - className={styles.appendBuiltInDataSourcesList} - onChange={onChangeDataSource} - current={current} /> +
- + - {config.featureToggles.editPanelCSVDragAndDrop && ( + {uploadFile && config.featureToggles.editPanelCSVDragAndDrop && ( undefined} diff --git a/public/app/features/datasources/components/picker/utils.ts b/public/app/features/datasources/components/picker/utils.ts index 104c60b9a6d..6770c10cc6d 100644 --- a/public/app/features/datasources/components/picker/utils.ts +++ b/public/app/features/datasources/components/picker/utils.ts @@ -20,7 +20,7 @@ export function dataSourceLabel( dataSource: DataSourceInstanceSettings | string | DataSourceRef | null | undefined ) { if (!dataSource) { - return 'Select a data source'; + return undefined; } if (typeof dataSource === 'string') { @@ -35,7 +35,7 @@ export function dataSourceLabel( return `${dataSource.uid} - not found`; } - return 'Select a data source'; + return undefined; } export function getDataSourceCompareFn( diff --git a/public/app/features/query/components/QueryGroup.tsx b/public/app/features/query/components/QueryGroup.tsx index d83ec4eefdf..552cbc30dcf 100644 --- a/public/app/features/query/components/QueryGroup.tsx +++ b/public/app/features/query/components/QueryGroup.tsx @@ -272,7 +272,12 @@ export class QueryGroup extends PureComponent { const { isDataSourceModalOpen } = this.state; const commonProps = { + metrics: true, + mixed: true, + dashboard: true, + variables: true, current: this.props.options.dataSource, + uploadFile: true, onChange: async (ds: DataSourceInstanceSettings, defaultQueries?: DataQuery[] | GrafanaQuery[]) => { await this.onChangeDataSource(ds, defaultQueries); this.onCloseDataSourceModal(); @@ -284,7 +289,7 @@ export class QueryGroup extends PureComponent { {isDataSourceModalOpen && config.featureToggles.advancedDataSourcePicker && ( )} - + ); };