fix(explore): Allow only saved metrics and columns (#27539)

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) => (
+    <div data-test="react-ace">{props.value}</div>
+  ),
+}));
+
+const defaultProps: Omit<DndFilterSelectProps, 'datasource'> = {
   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 (
-    <Provider store={store}>
-      <DndFilterSelect
-        {...defaultProps}
-        value={ensureIsArray(value)}
-        formData={formData}
-        columns={columns}
-      />
-    </Provider>
+    <DndFilterSelect
+      {...defaultProps}
+      datasource={datasource}
+      value={ensureIsArray(value)}
+      formData={formData}
+      columns={columns}
+    />
   );
 }
 
 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(
+    <>
+      <DatasourcePanelDragOption
+        value={{ column_name: 'order_date' }}
+        type={DndItemType.Column}
+      />
+      <DatasourcePanelDragOption
+        value={{ column_name: 'address_line1' }}
+        type={DndItemType.Column}
+      />
+      <DatasourcePanelDragOption
+        value={{ metric_name: 'metric_a', expression: 'AGG(metric_a)' }}
+        type={DndItemType.Metric}
+      />
+      {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(
+      <>
+        <DatasourcePanelDragOption
+          value={{ column_name: 'column_b' }}
+          type={DndItemType.Column}
+        />
+        {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(
+      <>
+        <DatasourcePanelDragOption
+          value={{ column_name: 'column_c' }}
+          type={DndItemType.Column}
+        />
+        <DatasourcePanelDragOption
+          value={{ metric_name: 'metric_a' }}
+          type={DndItemType.Metric}
+        />
+        <DatasourcePanelDragOption
+          value={{ metric_name: 'avg__num' }}
+          type={DndItemType.AdhocMetricOption}
+        />
+        {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();
+  });
+});

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(

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(
+    <>
+      <DatasourcePanelDragOption
+        value={{ metric_name: 'metric_a' }}
+        type={DndItemType.Metric}
+      />
+      <DndMetricSelect {...defaultProps} value={metricValues} multi />
+    </>,
+    {
+      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(
+    <>
+      <DatasourcePanelDragOption
+        value={{ metric_name: 'metric_a' }}
+        type={DndItemType.Metric}
+      />
+      <DndMetricSelect
+        {...defaultProps}
+        value={metricValues}
+        multi
+        datasource={{ extra: '{ "disallow_adhoc_metrics": true }' }}
+      />
+    </>,
+    {
+      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(
+    <>
+      <DatasourcePanelDragOption
+        value={{ metric_name: 'metric_a' }}
+        type={DndItemType.Metric}
+      />
+      <DatasourcePanelDragOption
+        value={{ metric_name: 'metric_c' }}
+        type={DndItemType.Metric}
+      />
+      <DatasourcePanelDragOption
+        value={{ column_name: 'column_1' }}
+        type={DndItemType.Column}
+      />
+      <DndMetricSelect
+        {...defaultProps}
+        value={metricValues}
+        multi
+        datasource={{ extra: '{ "disallow_adhoc_metrics": true }' }}
+      />
+    </>,
+    {
+      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[]) => {

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(

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 {