You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2022/10/19 13:29:44 UTC
[superset] branch master updated: feat(explore): Don't discard controls with custom sql when changing datasource (#20934)
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new cddc361adc feat(explore): Don't discard controls with custom sql when changing datasource (#20934)
cddc361adc is described below
commit cddc361adc483ed605857a2eb39c5efffa089076
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Oct 19 15:29:38 2022 +0200
feat(explore): Don't discard controls with custom sql when changing datasource (#20934)
---
.../superset-ui-core/src/query/types/Column.ts | 1 +
.../superset-ui-core/src/utils/isDefined.ts | 2 +-
.../explore/components/ControlPanelsContainer.tsx | 41 +++++-
.../components/ExploreViewContainer/index.jsx | 1 +
.../DndAdhocFilterOption.tsx | 6 +
.../DndColumnSelectControl/DndColumnSelect.tsx | 45 ++-----
.../DndMetricSelect.test.tsx | 19 +--
.../DndColumnSelectControl/DndMetricSelect.tsx | 141 ++++++++-------------
.../controls/DndColumnSelectControl/Option.tsx | 10 +-
.../DndColumnSelectControl/OptionWrapper.tsx | 2 +
.../controls/DndColumnSelectControl/types.ts | 1 +
.../FilterControl/AdhocFilter/AdhocFilter.test.js | 1 +
.../controls/FilterControl/AdhocFilter/index.js | 1 +
.../controls/MetricControl/AdhocMetric.js | 1 +
.../controls/MetricControl/AdhocMetric.test.js | 1 +
.../controls/MetricControl/AdhocMetricOption.jsx | 3 +
.../MetricControl/MetricDefinitionValue.jsx | 3 +
.../components/controls/OptionControls/index.tsx | 13 +-
...etControlValuesCompatibleWithDatasource.test.ts | 2 +
.../getControlValuesCompatibleWithDatasource.ts | 7 +-
.../src/explore/reducers/exploreReducer.js | 47 ++++---
superset-frontend/src/reduxUtils.ts | 22 +++-
22 files changed, 190 insertions(+), 180 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts
index 15a5f29de6..a009515363 100644
--- a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts
+++ b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts
@@ -29,6 +29,7 @@ export interface AdhocColumn {
expressionType: 'SQL';
columnType?: 'BASE_AXIS' | 'SERIES';
timeGrain?: string;
+ datasourceWarning?: boolean;
}
/**
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/isDefined.ts b/superset-frontend/packages/superset-ui-core/src/utils/isDefined.ts
index 097115e11c..0cdba14eb6 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/isDefined.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/isDefined.ts
@@ -17,6 +17,6 @@
* under the License.
*/
-export default function isDefined(x: unknown) {
+export default function isDefined<T>(x: T): x is NonNullable<T> {
return x !== null && x !== undefined;
}
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index bbf59a60be..7ed11a964b 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -36,6 +36,7 @@ import {
css,
SupersetTheme,
useTheme,
+ isDefined,
} from '@superset-ui/core';
import {
ControlPanelSectionConfig,
@@ -45,6 +46,9 @@ import {
ExpandedControlItem,
sections,
} from '@superset-ui/chart-controls';
+import { useSelector } from 'react-redux';
+import { rgba } from 'emotion-rgba';
+import { kebabCase } from 'lodash';
import Collapse from 'src/components/Collapse';
import Tabs from 'src/components/Tabs';
@@ -57,9 +61,6 @@ import { ExploreActions } from 'src/explore/actions/exploreActions';
import { ChartState, ExplorePageState } from 'src/explore/types';
import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';
-
-import { rgba } from 'emotion-rgba';
-import { kebabCase } from 'lodash';
import ControlRow from './ControlRow';
import Control from './Control';
import { ExploreAlert } from './ExploreAlert';
@@ -269,6 +270,36 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
const containerRef = useRef<HTMLDivElement>(null);
+ const controlsTransferred = useSelector<
+ ExplorePageState,
+ string[] | undefined
+ >(state => state.explore.controlsTransferred);
+
+ useEffect(() => {
+ if (props.chart.chartStatus === 'success') {
+ controlsTransferred?.forEach(controlName => {
+ const alteredControls = ensureIsArray(
+ props.controls[controlName].value,
+ ).map(value => {
+ if (
+ typeof value === 'object' &&
+ isDefined(value) &&
+ 'datasourceWarning' in value
+ ) {
+ return { ...value, datasourceWarning: false };
+ }
+ return value;
+ });
+ props.actions.setControlValue(controlName, alteredControls);
+ });
+ }
+ }, [
+ controlsTransferred,
+ props.actions,
+ props.chart.chartStatus,
+ props.controls,
+ ]);
+
useEffect(() => {
if (
prevDatasource &&
@@ -455,7 +486,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
>
<Icons.InfoCircleOutlined
css={css`
- ${iconStyles}
+ ${iconStyles};
color: ${errorColor};
`}
/>
@@ -591,7 +622,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
>
<Icons.ExclamationCircleOutlined
css={css`
- ${iconStyles}
+ ${iconStyles};
color: ${errorColor};
`}
/>
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index 8b99decbad..ef7bd01d8f 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -458,6 +458,7 @@ function ExploreViewContainer(props) {
!areObjectsEqual(
props.controls[key].value,
lastQueriedControls[key].value,
+ { ignoreFields: ['datasourceWarning'] },
),
);
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndAdhocFilterOption.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndAdhocFilterOption.tsx
index 714855d43f..bc65ef5e8a 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndAdhocFilterOption.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndAdhocFilterOption.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
+import { t } from '@superset-ui/core';
import { DndItemType } from 'src/explore/components/DndItemType';
import AdhocFilterPopoverTrigger from 'src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger';
import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter';
@@ -63,6 +64,11 @@ export default function DndAdhocFilterOption({
type={DndItemType.FilterOption}
withCaret
isExtra={adhocFilter.isExtra}
+ datasourceWarningMessage={
+ adhocFilter.datasourceWarning
+ ? t('This filter might be incompatible with current dataset')
+ : undefined
+ }
/>
</AdhocFilterPopoverTrigger>
);
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
index f254eece75..071fdbd424 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
@@ -23,6 +23,8 @@ import {
isFeatureEnabled,
tn,
QueryFormColumn,
+ t,
+ isAdhocColumn,
} from '@superset-ui/core';
import {
ColumnMeta,
@@ -35,7 +37,6 @@ import OptionWrapper from 'src/explore/components/controls/DndColumnSelectContro
import { OptionSelector } from 'src/explore/components/controls/DndColumnSelectControl/utils';
import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
import { DndItemType } from 'src/explore/components/DndItemType';
-import { useComponentDidUpdate } from 'src/hooks/useComponentDidUpdate';
import ColumnSelectPopoverTrigger from './ColumnSelectPopoverTrigger';
import { DndControlProps } from './types';
import SelectControl from '../SelectControl';
@@ -68,34 +69,6 @@ function DndColumnSelect(props: DndColumnSelectProps) {
return new OptionSelector(optionsMap, multi, value);
}, [multi, options, value]);
- // synchronize values in case of dataset changes
- const handleOptionsChange = useCallback(() => {
- const optionSelectorValues = optionSelector.getValues();
- if (typeof value !== typeof optionSelectorValues) {
- onChange(optionSelectorValues);
- }
- if (
- typeof value === 'string' &&
- typeof optionSelectorValues === 'string' &&
- value !== optionSelectorValues
- ) {
- onChange(optionSelectorValues);
- }
- if (
- Array.isArray(optionSelectorValues) &&
- Array.isArray(value) &&
- (optionSelectorValues.length !== value.length ||
- optionSelectorValues.every((val, index) => val === value[index]))
- ) {
- onChange(optionSelectorValues);
- }
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [JSON.stringify(value), JSON.stringify(optionSelector.getValues())]);
-
- // useComponentDidUpdate to avoid running this for the first render, to avoid
- // calling onChange when the initial value is not valid for the dataset
- useComponentDidUpdate(handleOptionsChange);
-
const onDrop = useCallback(
(item: DatasourcePanelDndItem) => {
const column = item.value as ColumnMeta;
@@ -142,8 +115,12 @@ function DndColumnSelect(props: DndColumnSelectProps) {
const valuesRenderer = useCallback(
() =>
- optionSelector.values.map((column, idx) =>
- clickEnabled ? (
+ optionSelector.values.map((column, idx) => {
+ const datasourceWarningMessage =
+ isAdhocColumn(column) && column.datasourceWarning
+ ? t('This column might be incompatible with current dataset')
+ : undefined;
+ return clickEnabled ? (
<ColumnSelectPopoverTrigger
key={idx}
columns={options}
@@ -166,6 +143,7 @@ function DndColumnSelect(props: DndColumnSelectProps) {
type={`${DndItemType.ColumnOption}_${name}_${label}`}
canDelete={canDelete}
column={column}
+ datasourceWarningMessage={datasourceWarningMessage}
withCaret
/>
</ColumnSelectPopoverTrigger>
@@ -178,9 +156,10 @@ function DndColumnSelect(props: DndColumnSelectProps) {
type={`${DndItemType.ColumnOption}_${name}_${label}`}
canDelete={canDelete}
column={column}
+ datasourceWarningMessage={datasourceWarningMessage}
/>
- ),
- ),
+ );
+ }),
[
canDelete,
clickEnabled,
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 69727d17fe..80337344a2 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
@@ -133,6 +133,7 @@ test('remove selected custom metric when metric gets removed from dataset', () =
);
expect(screen.getByText('metric_a')).toBeVisible();
expect(screen.queryByText('Metric B')).not.toBeInTheDocument();
+ expect(screen.queryByText('metric_b')).not.toBeInTheDocument();
expect(screen.getByText('SUM(column_a)')).toBeVisible();
expect(screen.getByText('SUM(Column B)')).toBeVisible();
});
@@ -171,15 +172,6 @@ test('remove selected custom metric when metric gets removed from dataset for si
],
};
- // rerender twice - first to update columns, second to update value
- rerender(
- <DndMetricSelect
- {...newPropsWithRemovedMetric}
- value={metricValue}
- onChange={onChange}
- multi={false}
- />,
- );
rerender(
<DndMetricSelect
{...newPropsWithRemovedMetric}
@@ -220,15 +212,6 @@ test('remove selected adhoc metric when column gets removed from dataset', async
],
};
- // rerender twice - first to update columns, second to update value
- rerender(
- <DndMetricSelect
- {...newPropsWithRemovedColumn}
- value={metricValues}
- onChange={onChange}
- multi
- />,
- );
rerender(
<DndMetricSelect
{...newPropsWithRemovedColumn}
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
index 501e814b9a..d3ab394db9 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
@@ -22,14 +22,15 @@ import {
ensureIsArray,
FeatureFlag,
GenericDataType,
+ isAdhocMetricSimple,
isFeatureEnabled,
+ isSavedMetric,
Metric,
QueryFormMetric,
+ t,
tn,
} from '@superset-ui/core';
import { ColumnMeta, withDndFallback } from '@superset-ui/chart-controls';
-import { isEqual } from 'lodash';
-import { usePrevious } from 'src/hooks/usePrevious';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocMetricPopoverTrigger from 'src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger';
import MetricDefinitionValue from 'src/explore/components/controls/MetricControl/MetricDefinitionValue';
@@ -46,24 +47,49 @@ import MetricsControl from '../MetricControl/MetricsControl';
const EMPTY_OBJECT = {};
const DND_ACCEPTED_TYPES = [DndItemType.Column, DndItemType.Metric];
-const isDictionaryForAdhocMetric = (value: any) =>
- value && !(value instanceof AdhocMetric) && value.expressionType;
+const isDictionaryForAdhocMetric = (value: QueryFormMetric) =>
+ value &&
+ !(value instanceof AdhocMetric) &&
+ typeof value !== 'string' &&
+ value.expressionType;
-const coerceAdhocMetrics = (value: any) => {
- if (!value) {
+const coerceMetrics = (
+ addedMetrics: QueryFormMetric | QueryFormMetric[] | undefined | null,
+ savedMetrics: Metric[],
+ columns: ColumnMeta[],
+) => {
+ if (!addedMetrics) {
return [];
}
- if (!Array.isArray(value)) {
- if (isDictionaryForAdhocMetric(value)) {
- return [new AdhocMetric(value)];
+ const metricsCompatibleWithDataset = ensureIsArray(addedMetrics).filter(
+ metric => {
+ if (isSavedMetric(metric)) {
+ return savedMetrics.some(
+ savedMetric => savedMetric.metric_name === metric,
+ );
+ }
+ if (isAdhocMetricSimple(metric)) {
+ return columns.some(
+ column => column.column_name === metric.column.column_name,
+ );
+ }
+ return true;
+ },
+ );
+
+ return metricsCompatibleWithDataset.map(metric => {
+ if (!isDictionaryForAdhocMetric(metric)) {
+ return metric;
}
- return [value];
- }
- return value.map(val => {
- if (isDictionaryForAdhocMetric(val)) {
- return new AdhocMetric(val);
+ if (isAdhocMetricSimple(metric)) {
+ const column = columns.find(
+ col => col.column_name === metric.column.column_name,
+ );
+ if (column) {
+ return new AdhocMetric({ ...metric, column });
+ }
}
- return val;
+ return new AdhocMetric(metric);
});
};
@@ -81,53 +107,8 @@ const getOptionsForSavedMetrics = (
type ValueType = Metric | AdhocMetric | QueryFormMetric;
-// TODO: use typeguards to distinguish saved metrics from adhoc metrics
-const getMetricsMatchingCurrentDataset = (
- values: ValueType[],
- columns: ColumnMeta[],
- savedMetrics: (savedMetricType | Metric)[],
- prevColumns: ColumnMeta[],
- prevSavedMetrics: (savedMetricType | Metric)[],
-): ValueType[] => {
- const areSavedMetricsEqual =
- !prevSavedMetrics || isEqual(prevSavedMetrics, savedMetrics);
- const areColsEqual = !prevColumns || isEqual(prevColumns, columns);
-
- if (areColsEqual && areSavedMetricsEqual) {
- return values;
- }
- return values.reduce((acc: ValueType[], metric) => {
- if (typeof metric === 'string' || (metric as Metric).metric_name) {
- if (
- areSavedMetricsEqual ||
- savedMetrics?.some(
- savedMetric =>
- savedMetric.metric_name === metric ||
- savedMetric.metric_name === (metric as Metric).metric_name,
- )
- ) {
- acc.push(metric);
- }
- return acc;
- }
-
- if (!areColsEqual) {
- const newCol = columns?.find(
- column =>
- (metric as AdhocMetric).column?.column_name === column.column_name,
- );
- if (newCol) {
- acc.push({ ...(metric as AdhocMetric), column: newCol });
- }
- } else {
- acc.push(metric);
- }
- return acc;
- }, []);
-};
-
const DndMetricSelect = (props: any) => {
- const { onChange, multi, columns, savedMetrics } = props;
+ const { onChange, multi } = props;
const handleChange = useCallback(
opts => {
@@ -153,39 +134,20 @@ const DndMetricSelect = (props: any) => {
);
const [value, setValue] = useState<ValueType[]>(
- coerceAdhocMetrics(props.value),
+ coerceMetrics(props.value, props.savedMetrics, props.columns),
);
const [droppedItem, setDroppedItem] = useState<
DatasourcePanelDndItem | typeof EMPTY_OBJECT
>({});
const [newMetricPopoverVisible, setNewMetricPopoverVisible] = useState(false);
- const prevColumns = usePrevious(columns);
- const prevSavedMetrics = usePrevious(savedMetrics);
useEffect(() => {
- setValue(coerceAdhocMetrics(props.value));
- }, [JSON.stringify(props.value)]);
-
- useEffect(() => {
- // Remove selected custom metrics that do not exist in the dataset anymore
- // Remove selected adhoc metrics that use columns which do not exist in the dataset anymore
- // Sync adhoc metrics with dataset columns when they are modified by the user
- if (!props.value) {
- return;
- }
- const propsValues = ensureIsArray(props.value);
- const matchingMetrics = getMetricsMatchingCurrentDataset(
- propsValues,
- columns,
- savedMetrics,
- prevColumns,
- prevSavedMetrics,
- );
-
- if (!isEqual(propsValues, matchingMetrics)) {
- handleChange(matchingMetrics);
- }
- }, [columns, savedMetrics, handleChange]);
+ setValue(coerceMetrics(props.value, props.savedMetrics, props.columns));
+ }, [
+ JSON.stringify(props.value),
+ JSON.stringify(props.savedMetrics),
+ JSON.stringify(props.columns),
+ ]);
const canDrop = useCallback(
(item: DatasourcePanelDndItem) => {
@@ -291,6 +253,11 @@ const DndMetricSelect = (props: any) => {
onDropLabel={handleDropLabel}
type={`${DndItemType.AdhocMetricOption}_${props.name}_${props.label}`}
multi={multi}
+ datasourceWarningMessage={
+ option instanceof AdhocMetric && option.datasourceWarning
+ ? t('This metric might be incompatible with current dataset')
+ : undefined
+ }
/>
),
[
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx
index 3c7ee3c7c1..3de910c137 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx
@@ -38,6 +38,7 @@ export default function Option({
clickClose,
withCaret,
isExtra,
+ datasourceWarningMessage,
canDelete = true,
}: OptionProps) {
const theme = useTheme();
@@ -60,15 +61,18 @@ export default function Option({
</CloseContainer>
)}
<Label data-test="control-label">{children}</Label>
- {isExtra && (
+ {(!!datasourceWarningMessage || isExtra) && (
<StyledInfoTooltipWithTrigger
icon="exclamation-triangle"
placement="top"
bsStyle="warning"
- tooltip={t(`
+ tooltip={
+ datasourceWarningMessage ||
+ t(`
This filter was inherited from the dashboard's context.
It won't be saved when saving the chart.
- `)}
+ `)
+ }
/>
)}
{withCaret && (
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
index 72e9e3fb6e..e227228832 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
@@ -57,6 +57,7 @@ export default function OptionWrapper(
clickClose,
withCaret,
isExtra,
+ datasourceWarningMessage,
canDelete = true,
...rest
} = props;
@@ -176,6 +177,7 @@ export default function OptionWrapper(
clickClose={clickClose}
withCaret={withCaret}
isExtra={isExtra}
+ datasourceWarningMessage={datasourceWarningMessage}
canDelete={canDelete}
>
<Label />
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
index b324689c67..f1658b5ae3 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
@@ -30,6 +30,7 @@ export interface OptionProps {
clickClose: (index: number) => void;
withCaret?: boolean;
isExtra?: boolean;
+ datasourceWarningMessage?: string;
canDelete?: boolean;
}
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js
index 2aa5fd1604..f3e08d7a28 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js
@@ -36,6 +36,7 @@ describe('AdhocFilter', () => {
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'value',
operator: '>',
+ datasourceWarning: false,
comparator: '10',
clause: CLAUSES.WHERE,
filterOptionName: adhocFilter.filterOptionName,
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js
index 5b6278bde2..29486709b1 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js
@@ -118,6 +118,7 @@ export default class AdhocFilter {
}
this.isExtra = !!adhocFilter.isExtra;
this.isNew = !!adhocFilter.isNew;
+ this.datasourceWarning = !!adhocFilter.datasourceWarning;
this.filterOptionName =
adhocFilter.filterOptionName ||
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
index 5b29d7418c..2cf220be9d 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
@@ -76,6 +76,7 @@ export default class AdhocMetric {
this.aggregate = null;
}
this.isNew = !!adhocMetric.isNew;
+ this.datasourceWarning = !!adhocMetric.datasourceWarning;
this.hasCustomLabel = !!(adhocMetric.hasCustomLabel && adhocMetric.label);
this.label = this.hasCustomLabel
? adhocMetric.label
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js
index 336b194e00..5186b1f97d 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js
@@ -34,6 +34,7 @@ describe('AdhocMetric', () => {
expressionType: EXPRESSION_TYPES.SIMPLE,
column: valueColumn,
aggregate: AGGREGATES.SUM,
+ datasourceWarning: false,
label: 'SUM(value)',
hasCustomLabel: false,
optionName: adhocMetric.optionName,
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
index 352480f222..80cf879f7f 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
@@ -38,6 +38,7 @@ const propTypes = {
index: PropTypes.number,
type: PropTypes.string,
multi: PropTypes.bool,
+ datasourceWarningMessage: PropTypes.string,
};
class AdhocMetricOption extends React.PureComponent {
@@ -64,6 +65,7 @@ class AdhocMetricOption extends React.PureComponent {
index,
type,
multi,
+ datasourceWarningMessage,
} = this.props;
return (
@@ -87,6 +89,7 @@ class AdhocMetricOption extends React.PureComponent {
withCaret
isFunction
multi={multi}
+ datasourceWarningMessage={datasourceWarningMessage}
/>
</AdhocMetricPopoverTrigger>
);
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
index 0b5f5de0ac..da5743ec76 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
@@ -35,6 +35,7 @@ const propTypes = {
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
multi: PropTypes.bool,
datasource: PropTypes.object,
+ datasourceWarningMessage: PropTypes.string,
};
export default function MetricDefinitionValue({
@@ -50,6 +51,7 @@ export default function MetricDefinitionValue({
index,
type,
multi,
+ datasourceWarningMessage,
}) {
const getSavedMetricByName = metricName =>
savedMetrics.find(metric => metric.metric_name === metricName);
@@ -78,6 +80,7 @@ export default function MetricDefinitionValue({
savedMetric: savedMetric ?? {},
type,
multi,
+ datasourceWarningMessage,
};
return <AdhocMetricOption {...metricOptionProps} />;
diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
index 74ec85dc14..90ae73c637 100644
--- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
+++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
@@ -179,6 +179,7 @@ export const OptionControlLabel = ({
type,
index,
isExtra,
+ datasourceWarningMessage,
tooltipTitle,
multi = true,
...props
@@ -195,7 +196,8 @@ export const OptionControlLabel = ({
type: string;
index: number;
isExtra?: boolean;
- tooltipTitle: string;
+ datasourceWarningMessage?: string;
+ tooltipTitle?: string;
multi?: boolean;
}) => {
const theme = useTheme();
@@ -314,15 +316,18 @@ export const OptionControlLabel = ({
{isFunction && <Icons.FieldDerived />}
{getLabelContent()}
</Label>
- {isExtra && (
+ {(!!datasourceWarningMessage || isExtra) && (
<StyledInfoTooltipWithTrigger
icon="exclamation-triangle"
placement="top"
bsStyle="warning"
- tooltip={t(`
+ tooltip={
+ datasourceWarningMessage ||
+ t(`
This filter was inherited from the dashboard's context.
It won't be saved when saving the chart.
- `)}
+ `)
+ }
/>
)}
{withCaret && (
diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
index 7da50b0ce4..ad7ccc4801 100644
--- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
+++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
@@ -210,6 +210,7 @@ test('SQL ad-hoc metric values', () => {
},
}),
).toEqual({
+ datasourceWarning: true,
expressionType: 'SQL',
sqlExpression: 'select * from sample_column_1;',
});
@@ -279,6 +280,7 @@ test('SQL ad-hoc filter values', () => {
},
}),
).toEqual({
+ datasourceWarning: true,
expressionType: 'SQL',
sqlExpression: 'select * from sample_column_1;',
});
diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts
index 98590e63ee..7013b59764 100644
--- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts
+++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts
@@ -21,9 +21,9 @@ import { ControlState, Dataset, Metric } from '@superset-ui/chart-controls';
import {
Column,
isAdhocMetricSimple,
+ isAdhocMetricSQL,
isSavedMetric,
isSimpleAdhocFilter,
- isFreeFormAdhocFilter,
JsonValue,
SimpleAdhocFilter,
} from '@superset-ui/core';
@@ -72,7 +72,10 @@ const isControlValueCompatibleWithDatasource = (
column.column_name === (value as SimpleAdhocFilter).subject,
);
}
- if (isFreeFormAdhocFilter(value)) return true;
+ if (isAdhocMetricSQL(value)) {
+ Object.assign(value, { datasourceWarning: true });
+ return true;
+ }
return false;
};
diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js
index 81dd86fbf9..e282d88f04 100644
--- a/superset-frontend/src/explore/reducers/exploreReducer.js
+++ b/superset-frontend/src/explore/reducers/exploreReducer.js
@@ -54,42 +54,39 @@ export default function exploreReducer(state = {}, action) {
const { prevDatasource, newDatasource } = action;
const controls = { ...state.controls };
const controlsTransferred = [];
+
if (
prevDatasource.id !== newDatasource.id ||
prevDatasource.type !== newDatasource.type
) {
// reset time range filter to default
newFormData.time_range = DEFAULT_TIME_RANGE;
-
newFormData.datasource = newDatasource.uid;
+ }
- // reset control values for column/metric related controls
- Object.entries(controls).forEach(([controlName, controlState]) => {
+ // reset control values for column/metric related controls
+ Object.entries(controls).forEach(([controlName, controlState]) => {
+ if (
+ // for direct column select controls
+ controlState.valueKey === 'column_name' ||
+ // for all other controls
+ 'savedMetrics' in controlState ||
+ 'columns' in controlState ||
+ ('options' in controlState && !Array.isArray(controlState.options))
+ ) {
+ newFormData[controlName] = getControlValuesCompatibleWithDatasource(
+ newDatasource,
+ controlState,
+ controlState.value,
+ );
if (
- // for direct column select controls
- controlState.valueKey === 'column_name' ||
- // for all other controls
- 'savedMetrics' in controlState ||
- 'columns' in controlState ||
- ('options' in controlState && !Array.isArray(controlState.options))
+ ensureIsArray(newFormData[controlName]).length > 0 &&
+ newFormData[controlName] !== controls[controlName].default
) {
- controls[controlName] = {
- ...controlState,
- };
- newFormData[controlName] = getControlValuesCompatibleWithDatasource(
- newDatasource,
- controlState,
- controlState.value,
- );
- if (
- ensureIsArray(newFormData[controlName]).length > 0 &&
- newFormData[controlName] !== controls[controlName].default
- ) {
- controlsTransferred.push(controlName);
- }
+ controlsTransferred.push(controlName);
}
- });
- }
+ }
+ });
const newState = {
...state,
diff --git a/superset-frontend/src/reduxUtils.ts b/superset-frontend/src/reduxUtils.ts
index 2cf43fa712..8e744b48dc 100644
--- a/superset-frontend/src/reduxUtils.ts
+++ b/superset-frontend/src/reduxUtils.ts
@@ -19,7 +19,15 @@
import shortid from 'shortid';
import { compose } from 'redux';
import persistState, { StorageAdapter } from 'redux-localstorage';
-import { isEqual, omitBy, isUndefined, isNull } from 'lodash';
+import {
+ isEqual,
+ omitBy,
+ omit,
+ isUndefined,
+ isNull,
+ isEqualWith,
+} from 'lodash';
+import { ensureIsArray } from '@superset-ui/core';
export function addToObject(
state: Record<string, any>,
@@ -181,7 +189,8 @@ export function areObjectsEqual(
opts: {
ignoreUndefined?: boolean;
ignoreNull?: boolean;
- } = { ignoreUndefined: false, ignoreNull: false },
+ ignoreFields?: string[];
+ } = { ignoreUndefined: false, ignoreNull: false, ignoreFields: [] },
) {
let comp1 = obj1;
let comp2 = obj2;
@@ -193,5 +202,14 @@ export function areObjectsEqual(
comp1 = omitBy(comp1, isNull);
comp2 = omitBy(comp2, isNull);
}
+ if (opts.ignoreFields?.length) {
+ const ignoreFields = ensureIsArray(opts.ignoreFields);
+ return isEqualWith(comp1, comp2, (val1, val2) =>
+ isEqual(
+ ensureIsArray(val1).map(value => omit(value, ignoreFields)),
+ ensureIsArray(val2).map(value => omit(value, ignoreFields)),
+ ),
+ );
+ }
return isEqual(comp1, comp2);
}