diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx index f47c3a0101..6be82472d9 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx @@ -18,7 +18,6 @@ */ import React from 'react'; import thunk from 'redux-thunk'; -import { Provider } from 'react-redux'; import configureStore from 'redux-mock-store'; import { @@ -29,7 +28,13 @@ import { import { ColumnMeta } from '@superset-ui/chart-controls'; import { TimeseriesDefaultFormData } from '@superset-ui/plugin-chart-echarts'; -import { render, screen } from 'spec/helpers/testing-library'; +import { + fireEvent, + render, + screen, + within, +} from 'spec/helpers/testing-library'; +import type { AsyncAceEditorProps } from 'src/components/AsyncAceEditor'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter'; import { @@ -38,13 +43,21 @@ import { } from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect'; import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants'; import { ExpressionTypes } from '../FilterControl/types'; +import { Datasource } from '../../../types'; +import { DndItemType } from '../../DndItemType'; +import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption'; -const defaultProps: DndFilterSelectProps = { +jest.mock('src/components/AsyncAceEditor', () => ({ + SQLEditor: (props: AsyncAceEditorProps) => ( +
{props.value}
+ ), +})); + +const defaultProps: Omit = { type: 'DndFilterSelect', name: 'Filter', value: [], columns: [], - datasource: PLACEHOLDER_DATASOURCE, formData: null, savedMetrics: [], selectedMetrics: [], @@ -64,25 +77,26 @@ function setup({ value = undefined, formData = baseFormData, columns = [], + datasource = PLACEHOLDER_DATASOURCE, }: { value?: AdhocFilter; formData?: QueryFormData; columns?: ColumnMeta[]; + datasource?: Datasource; } = {}) { return ( - - - + ); } test('renders with default props', async () => { - render(setup(), { useDnd: true }); + render(setup(), { useDnd: true, store }); expect( await screen.findByText('Drop columns/metrics here or click'), ).toBeInTheDocument(); @@ -95,6 +109,7 @@ test('renders with value', async () => { }); render(setup({ value }), { useDnd: true, + store, }); expect(await screen.findByText('COUNT(*)')).toBeInTheDocument(); }); @@ -110,6 +125,7 @@ test('renders options with saved metric', async () => { }), { useDnd: true, + store, }, ); expect( @@ -131,6 +147,7 @@ test('renders options with column', async () => { }), { useDnd: true, + store, }, ); expect( @@ -153,9 +170,187 @@ test('renders options with adhoc metric', async () => { }), { useDnd: true, + store, }, ); expect( await screen.findByText('Drop columns/metrics here or click'), ).toBeInTheDocument(); }); + +test('cannot drop a column that is not part of the simple column selection', () => { + const adhocMetric = new AdhocMetric({ + expression: 'AVG(birth_names.num)', + metric_name: 'avg__num', + }); + const { getByTestId, getAllByTestId } = render( + <> + + + + {setup({ + formData: { + ...baseFormData, + ...TimeseriesDefaultFormData, + metrics: [adhocMetric], + }, + columns: [{ column_name: 'order_date' }], + })} + , + { + useDnd: true, + store, + }, + ); + + const selections = getAllByTestId('DatasourcePanelDragOption'); + const acceptableColumn = selections[0]; + const unacceptableColumn = selections[1]; + const metricType = selections[2]; + const currentMetric = getByTestId('dnd-labels-container'); + + fireEvent.dragStart(unacceptableColumn); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument(); + + fireEvent.dragStart(acceptableColumn); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + const filterConfigPopup = screen.getByTestId('filter-edit-popover'); + expect(within(filterConfigPopup).getByText('order_date')).toBeInTheDocument(); + + fireEvent.keyDown(filterConfigPopup, { + key: 'Escape', + code: 'Escape', + keyCode: 27, + charCode: 27, + }); + expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument(); + + fireEvent.dragStart(metricType); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect( + within(screen.getByTestId('filter-edit-popover')).getByTestId('react-ace'), + ).toHaveTextContent('AGG(metric_a)'); +}); + +describe('when disallow_adhoc_metrics is set', () => { + test('can drop a column type from the simple column selection', () => { + const adhocMetric = new AdhocMetric({ + expression: 'AVG(birth_names.num)', + metric_name: 'avg__num', + }); + const { getByTestId } = render( + <> + + {setup({ + formData: { + ...baseFormData, + ...TimeseriesDefaultFormData, + metrics: [adhocMetric], + }, + datasource: { + ...PLACEHOLDER_DATASOURCE, + extra: '{ "disallow_adhoc_metrics": true }', + }, + columns: [{ column_name: 'column_a' }, { column_name: 'column_b' }], + })} + , + { + useDnd: true, + store, + }, + ); + + const acceptableColumn = getByTestId('DatasourcePanelDragOption'); + const currentMetric = getByTestId('dnd-labels-container'); + + fireEvent.dragStart(acceptableColumn); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + const filterConfigPopup = screen.getByTestId('filter-edit-popover'); + expect(within(filterConfigPopup).getByText('column_b')).toBeInTheDocument(); + }); + + test('cannot drop any other types of selections apart from simple column selection', () => { + const adhocMetric = new AdhocMetric({ + expression: 'AVG(birth_names.num)', + metric_name: 'avg__num', + }); + const { getByTestId, getAllByTestId } = render( + <> + + + + {setup({ + formData: { + ...baseFormData, + ...TimeseriesDefaultFormData, + metrics: [adhocMetric], + }, + datasource: { + ...PLACEHOLDER_DATASOURCE, + extra: '{ "disallow_adhoc_metrics": true }', + }, + columns: [{ column_name: 'column_a' }, { column_name: 'column_c' }], + })} + , + { + useDnd: true, + store, + }, + ); + + const selections = getAllByTestId('DatasourcePanelDragOption'); + const acceptableColumn = selections[0]; + const unacceptableMetric = selections[1]; + const unacceptableType = selections[2]; + const currentMetric = getByTestId('dnd-labels-container'); + + fireEvent.dragStart(unacceptableMetric); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument(); + + fireEvent.dragStart(unacceptableType); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument(); + + fireEvent.dragStart(acceptableColumn); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + const filterConfigPopup = screen.getByTestId('filter-edit-popover'); + expect(within(filterConfigPopup).getByText('column_c')).toBeInTheDocument(); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx index bf8b7b6e81..955c480724 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx @@ -85,6 +85,16 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { canDelete, } = props; + const extra = useMemo<{ disallow_adhoc_metrics?: boolean }>(() => { + let extra = {}; + if (datasource?.extra) { + try { + extra = JSON.parse(datasource.extra); + } catch {} // eslint-disable-line no-empty + } + return extra; + }, [datasource?.extra]); + const propsValues = Array.from(props.value ?? []); const [values, setValues] = useState( propsValues.map((filter: OptionValueType) => @@ -149,6 +159,17 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { optionsForSelect(props.columns, props.formData), ); + const availableColumnSet = useMemo( + () => + new Set( + options.map( + ({ column_name, filterOptionName }) => + column_name ?? filterOptionName, + ), + ), + [options], + ); + useEffect(() => { if (datasource && datasource.type === 'table') { const dbId = datasource.database?.id; @@ -384,14 +405,21 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { const canDrop = useCallback( (item: DatasourcePanelDndItem) => { + if ( + extra.disallow_adhoc_metrics && + (item.type !== DndItemType.Column || + !availableColumnSet.has((item.value as ColumnMeta).column_name)) + ) { + return false; + } + if (item.type === DndItemType.Column) { - return props.columns.some( - col => col.column_name === (item.value as ColumnMeta).column_name, - ); + const columnName = (item.value as ColumnMeta).column_name; + return availableColumnSet.has(columnName); } return true; }, - [props.columns], + [availableColumnSet, extra], ); const handleDrop = useCallback( diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx index 30be2cebb0..9d6a7423f0 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx @@ -28,6 +28,8 @@ import { import { DndMetricSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndMetricSelect'; import { AGGREGATES } from 'src/explore/constants'; import { EXPRESSION_TYPES } from '../MetricControl/AdhocMetric'; +import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption'; +import { DndItemType } from '../../DndItemType'; const defaultProps = { savedMetrics: [ @@ -307,6 +309,125 @@ test('can drag metrics', async () => { expect(within(lastMetric).getByText('metric_a')).toBeVisible(); }); +test('cannot drop a duplicated item', () => { + const metricValues = ['metric_a']; + const { getByTestId } = render( + <> + + + , + { + useDnd: true, + }, + ); + + const acceptableMetric = getByTestId('DatasourcePanelDragOption'); + const currentMetric = getByTestId('dnd-labels-container'); + + const currentMetricSelection = currentMetric.children.length; + + fireEvent.dragStart(acceptableMetric); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(currentMetric.children).toHaveLength(currentMetricSelection); + expect(currentMetric).toHaveTextContent('metric_a'); +}); + +test('can drop a saved metric when disallow_adhoc_metrics', () => { + const metricValues = ['metric_b']; + const { getByTestId } = render( + <> + + + , + { + useDnd: true, + }, + ); + + const acceptableMetric = getByTestId('DatasourcePanelDragOption'); + const currentMetric = getByTestId('dnd-labels-container'); + + const currentMetricSelection = currentMetric.children.length; + + fireEvent.dragStart(acceptableMetric); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(currentMetric.children).toHaveLength(currentMetricSelection + 1); + expect(currentMetric.children[1]).toHaveTextContent('metric_a'); +}); + +test('cannot drop non-saved metrics when disallow_adhoc_metrics', () => { + const metricValues = ['metric_b']; + const { getByTestId, getAllByTestId } = render( + <> + + + + + , + { + useDnd: true, + }, + ); + + const selections = getAllByTestId('DatasourcePanelDragOption'); + const acceptableMetric = selections[0]; + const unacceptableMetric = selections[1]; + const unacceptableType = selections[2]; + const currentMetric = getByTestId('dnd-labels-container'); + + const currentMetricSelection = currentMetric.children.length; + + fireEvent.dragStart(unacceptableMetric); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(currentMetric.children).toHaveLength(currentMetricSelection); + expect(currentMetric).not.toHaveTextContent('metric_c'); + + fireEvent.dragStart(unacceptableType); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(currentMetric.children).toHaveLength(currentMetricSelection); + expect(currentMetric).not.toHaveTextContent('column_1'); + + fireEvent.dragStart(acceptableMetric); + fireEvent.dragOver(currentMetric); + fireEvent.drop(currentMetric); + + expect(currentMetric.children).toHaveLength(currentMetricSelection + 1); + expect(currentMetric).toHaveTextContent('metric_a'); +}); + test('title changes on custom SQL text change', async () => { let metricValues = [adhocMetricA, 'metric_b']; const onChange = (val: any[]) => { diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx index 8f489773e8..2c98ea4c48 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx @@ -105,7 +105,27 @@ const getOptionsForSavedMetrics = ( type ValueType = Metric | AdhocMetric | QueryFormMetric; const DndMetricSelect = (props: any) => { - const { onChange, multi } = props; + const { onChange, multi, datasource, savedMetrics } = props; + + const extra = useMemo<{ disallow_adhoc_metrics?: boolean }>(() => { + let extra = {}; + if (datasource?.extra) { + try { + extra = JSON.parse(datasource.extra); + } catch {} // eslint-disable-line no-empty + } + return extra; + }, [datasource?.extra]); + + const savedMetricSet = useMemo( + () => + new Set( + (savedMetrics as savedMetricType[]).map( + ({ metric_name }) => metric_name, + ), + ), + [savedMetrics], + ); const handleChange = useCallback( opts => { @@ -148,11 +168,19 @@ const DndMetricSelect = (props: any) => { const canDrop = useCallback( (item: DatasourcePanelDndItem) => { + if ( + extra.disallow_adhoc_metrics && + (item.type !== DndItemType.Metric || + !savedMetricSet.has(item.value.metric_name)) + ) { + return false; + } + const isMetricAlreadyInValues = item.type === 'metric' ? value.includes(item.value.metric_name) : false; return !isMetricAlreadyInValues; }, - [value], + [value, extra, savedMetricSet], ); const onNewMetric = useCallback( diff --git a/superset-frontend/src/explore/types.ts b/superset-frontend/src/explore/types.ts index 85240c919d..ee249e0fc3 100644 --- a/superset-frontend/src/explore/types.ts +++ b/superset-frontend/src/explore/types.ts @@ -68,6 +68,7 @@ export type Datasource = Dataset & { datasource?: string; schema?: string; is_sqllab_view?: boolean; + extra?: string; }; export interface ExplorePageInitialData {