You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/19 13:09:56 UTC
(superset) 03/06: fix(explore): Allow only saved metrics and columns (#27539)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d4314a92d8985c9cbdd1f7a146834b1893cbce6d
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Mar 18 09:51:02 2024 -0700
fix(explore): Allow only saved metrics and columns (#27539)
---
.../DndFilterSelect.test.tsx | 221 +++++++++++++++++++--
.../DndColumnSelectControl/DndFilterSelect.tsx | 41 +++-
.../DndMetricSelect.test.tsx | 121 +++++++++++
.../DndColumnSelectControl/DndMetricSelect.tsx | 32 ++-
superset-frontend/src/explore/types.ts | 1 +
5 files changed, 400 insertions(+), 16 deletions(-)
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';
+
+jest.mock('src/components/AsyncAceEditor', () => ({
+ SQLEditor: (props: AsyncAceEditorProps) => (
+ <div data-test="react-ace">{props.value}</div>
+ ),
+}));
-const defaultProps: DndFilterSelectProps = {
+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();
+ });
+});
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
index 5295bd6dae..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;
@@ -382,7 +403,25 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
return new AdhocFilter(config);
}, [droppedItem]);
- const canDrop = useCallback(() => true, []);
+ 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) {
+ const columnName = (item.value as ColumnMeta).column_name;
+ return availableColumnSet.has(columnName);
+ }
+ return true;
+ },
+ [availableColumnSet, extra],
+ );
+
const handleDrop = useCallback(
(item: DatasourcePanelDndItem) => {
setDroppedItem(item.value);
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(
+ <>
+ <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[]) => {
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 {