diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
index d3ab394db92ea..32a9c3756bb18 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
@@ -328,7 +328,7 @@ const DndMetricSelect = (props: any) => {
}
return new AdhocMetric(config);
}
- return new AdhocMetric({ isNew: true });
+ return new AdhocMetric({});
}, [droppedItem]);
const ghostButtonText = isFeatureEnabled(FeatureFlag.ENABLE_DND_WITH_CLICK_UX)
@@ -370,6 +370,7 @@ const DndMetricSelect = (props: any) => {
visible={newMetricPopoverVisible}
togglePopover={togglePopover}
closePopover={closePopover}
+ isNew
>
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
index 8688796593e1e..752fc457b948c 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js
@@ -75,7 +75,6 @@ export default class AdhocMetric {
this.column = null;
this.aggregate = null;
}
- this.isNew = !!adhocMetric.isNew;
this.datasourceWarning = !!adhocMetric.datasourceWarning;
this.hasCustomLabel = !!(adhocMetric.hasCustomLabel && adhocMetric.label);
this.label = this.hasCustomLabel
@@ -125,9 +124,6 @@ export default class AdhocMetric {
duplicateWith(nextFields) {
return new AdhocMetric({
...this,
- // all duplicate metrics are not considered new by default
- isNew: false,
- // but still overridable by nextFields
...nextFields,
});
}
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 5186b1f97dfad..4edc21572f8ee 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.test.js
@@ -39,7 +39,6 @@ describe('AdhocMetric', () => {
hasCustomLabel: false,
optionName: adhocMetric.optionName,
sqlExpression: null,
- isNew: false,
});
});
@@ -47,7 +46,6 @@ describe('AdhocMetric', () => {
const adhocMetric1 = new AdhocMetric({
column: valueColumn,
aggregate: AGGREGATES.SUM,
- isNew: true,
});
const adhocMetric2 = adhocMetric1.duplicateWith({
aggregate: AGGREGATES.AVG,
@@ -58,10 +56,6 @@ describe('AdhocMetric', () => {
expect(adhocMetric1.aggregate).toBe(AGGREGATES.SUM);
expect(adhocMetric2.aggregate).toBe(AGGREGATES.AVG);
-
- // duplicated clone should not be new
- expect(adhocMetric1.isNew).toBe(true);
- expect(adhocMetric2.isNew).toStrictEqual(false);
});
it('can verify equality', () => {
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx
index 8829d4e283d9f..3614e7222ce2b 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx
@@ -45,7 +45,7 @@ const createProps = () => ({
expression: 'sum(num)',
},
],
- adhocMetric: new AdhocMetric({ isNew: true }),
+ adhocMetric: new AdhocMetric({}),
datasource: {
extra: '{}',
type: 'table',
@@ -152,6 +152,16 @@ test('Clicking on "Save" should not call onChange and onClose', () => {
expect(props.onClose).toBeCalledTimes(0);
});
+test('Clicking on "Save" should call onChange and onClose for new metric', () => {
+ const props = createProps();
+ render();
+ expect(props.onChange).toBeCalledTimes(0);
+ expect(props.onClose).toBeCalledTimes(0);
+ userEvent.click(screen.getByRole('button', { name: 'Save' }));
+ expect(props.onChange).toBeCalledTimes(1);
+ expect(props.onClose).toBeCalledTimes(1);
+});
+
test('Should switch to tab:Simple', () => {
const props = createProps();
props.getCurrentTab.mockImplementation(tab => {
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
index f49eb5b4f750c..c824d1e929790 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
@@ -19,7 +19,13 @@
/* eslint-disable camelcase */
import React from 'react';
import PropTypes from 'prop-types';
-import { t, styled, ensureIsArray, DatasourceType } from '@superset-ui/core';
+import {
+ isDefined,
+ t,
+ styled,
+ ensureIsArray,
+ DatasourceType,
+} from '@superset-ui/core';
import Tabs from 'src/components/Tabs';
import Button from 'src/components/Button';
import { Select } from 'src/components';
@@ -55,11 +61,13 @@ const propTypes = {
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
datasource: PropTypes.object,
+ isNewMetric: PropTypes.bool,
};
const defaultProps = {
columns: [],
getCurrentTab: noOp,
+ isNewMetric: false,
};
const StyledSelect = styled(Select)`
@@ -78,12 +86,7 @@ export const SAVED_TAB_KEY = 'SAVED';
export default class AdhocMetricEditPopover extends React.PureComponent {
// "Saved" is a default tab unless there are no saved metrics for dataset
- defaultActiveTabKey =
- (this.props.savedMetric.metric_name || this.props.adhocMetric.isNew) &&
- Array.isArray(this.props.savedMetricsOptions) &&
- this.props.savedMetricsOptions.length > 0
- ? SAVED_TAB_KEY
- : this.props.adhocMetric.expressionType;
+ defaultActiveTabKey = this.getDefaultTab();
constructor(props) {
super(props);
@@ -99,6 +102,7 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
this.onTabChange = this.onTabChange.bind(this);
this.handleAceEditorRef = this.handleAceEditorRef.bind(this);
this.refreshAceEditor = this.refreshAceEditor.bind(this);
+ this.getDefaultTab = this.getDefaultTab.bind(this);
this.state = {
adhocMetric: this.props.adhocMetric,
@@ -106,7 +110,6 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
width: POPOVER_INITIAL_WIDTH,
height: POPOVER_INITIAL_HEIGHT,
};
-
document.addEventListener('mouseup', this.onMouseUp);
}
@@ -137,6 +140,22 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
document.removeEventListener('mousemove', this.onMouseMove);
}
+ getDefaultTab() {
+ const { adhocMetric, savedMetric, savedMetricsOptions, isNewMetric } =
+ this.props;
+ if (isDefined(adhocMetric.column) || isDefined(adhocMetric.sqlExpression)) {
+ return adhocMetric.expressionType;
+ }
+ if (
+ (isNewMetric || savedMetric.metric_name) &&
+ Array.isArray(savedMetricsOptions) &&
+ savedMetricsOptions.length > 0
+ ) {
+ return SAVED_TAB_KEY;
+ }
+ return adhocMetric.expressionType;
+ }
+
onSave() {
const { adhocMetric, savedMetric } = this.state;
@@ -279,6 +298,7 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
onClose,
onResize,
datasource,
+ isNewMetric,
...popoverProps
} = this.props;
const { adhocMetric, savedMetric } = this.state;
@@ -325,6 +345,7 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
const stateIsValid = adhocMetric.isValid() || savedMetric?.metric_name;
const hasUnsavedChanges =
+ isNewMetric ||
!adhocMetric.equals(propsAdhocMetric) ||
(!(
typeof savedMetric?.metric_name === 'undefined' &&
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
index 0136f0a8aa7d2..c593cc2ee878c 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
@@ -44,6 +44,7 @@ export type AdhocMetricPopoverTriggerProps = {
visible?: boolean;
togglePopover?: (visible: boolean) => void;
closePopover?: () => void;
+ isNew?: boolean;
};
export type AdhocMetricPopoverTriggerState = {
@@ -223,6 +224,7 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
onChange={this.onChange}
getCurrentTab={this.getCurrentTab}
getCurrentLabel={this.getCurrentLabel}
+ isNewMetric={this.props.isNew}
/>
);
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
index da5743ec766f6..67d9d6e4d231a 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
@@ -65,7 +65,7 @@ export default function MetricDefinitionValue({
if (option instanceof AdhocMetric || savedMetric) {
const adhocMetric =
- option instanceof AdhocMetric ? option : new AdhocMetric({ isNew: true });
+ option instanceof AdhocMetric ? option : new AdhocMetric({});
const metricOptionProps = {
onMetricEdit,
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
index 853ca90294b18..617be7112bdf3 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
@@ -217,10 +217,7 @@ const MetricsControl = ({
[propsValue, savedMetrics],
);
- const newAdhocMetric = useMemo(
- () => new AdhocMetric({ isNew: true }),
- [value],
- );
+ const newAdhocMetric = useMemo(() => new AdhocMetric({}), [value]);
const addNewMetricPopoverTrigger = useCallback(
trigger => {
if (isAddNewMetricDisabled()) {
@@ -234,6 +231,7 @@ const MetricsControl = ({
savedMetricsOptions={savedMetricOptions}
savedMetric={emptySavedMetric}
datasource={datasource}
+ isNew
>
{trigger}
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.test.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.test.jsx
index c255e6a62b53a..12a19c5cdf796 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.test.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.test.jsx
@@ -100,7 +100,6 @@ describe.skip('MetricsControl', () => {
hasCustomLabel: false,
optionName: 'blahblahblah',
sqlExpression: null,
- isNew: false,
},
]);
});