feat: Add Saved Metrics tab to metrics popover (#12123)

* Implement saved metrics * Fix bug in sql editor * Fix unit tests * Fix outlines in popovers * Add types for saved metrics * Add translations * Move savedMetricType to a separate file
2026-04-19 16:14:52 +00:00 · 2020-12-18 20:11:49 +01:00
parent 8e625e0a64
commit 2a23744223
15 changed files with 239 additions and 152 deletions
--- a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
@@ -49,6 +49,8 @@ function setup(overrides) {
  const onClose = sinon.spy();
  const props = {
    adhocMetric: sumValueAdhocMetric,
+    savedMetric: {},
+    savedMetrics: [],
    onChange,
    onClose,
    onResize: () => {},
@@ -62,7 +64,7 @@ function setup(overrides) {
 describe('AdhocMetricEditPopover', () => {
  it('renders a popover with edit metric form contents', () => {
    const { wrapper } = setup();
-    expect(wrapper.find(FormGroup)).toHaveLength(3);
+    expect(wrapper.find(FormGroup)).toHaveLength(4);
    expect(wrapper.find(Button)).toHaveLength(2);
  });

--- a/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx
@@ -41,11 +41,15 @@ function setup(overrides) {
  const onMetricEdit = sinon.spy();
  const props = {
    adhocMetric: sumValueAdhocMetric,
+    savedMetric: {},
+    savedMetrics: [],
    onMetricEdit,
    columns,
    ...overrides,
  };
-  const wrapper = shallow(<AdhocMetricOption {...props} />);
+  const wrapper = shallow(<AdhocMetricOption {...props} />)
+    .find('AdhocMetricPopoverTrigger')
+    .shallow();
  return { wrapper, onMetricEdit };
 }

@@ -56,13 +60,6 @@ describe('AdhocMetricOption', () => {
    expect(wrapper.find('OptionControlLabel')).toExist();
  });

-  it('overlay should open if metric is new', () => {
-    const { wrapper } = setup({
-      adhocMetric: sumValueAdhocMetric.duplicateWith({ isNew: true }),
-    });
-    expect(wrapper.find(Popover).props().defaultVisible).toBe(true);
-  });
-
  it('overwrites the adhocMetric in state with onLabelChange', () => {
    const { wrapper } = setup();
    wrapper.instance().onLabelChange({ target: { value: 'new label' } });
--- a/superset-frontend/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx
@@ -35,7 +35,7 @@ describe('MetricDefinitionValue', () => {
    const wrapper = shallow(
      <MetricDefinitionValue option={{ metric_name: 'a_saved_metric' }} />,
    );
-    expect(wrapper.find('OptionControlLabel')).toExist();
+    expect(wrapper.find('AdhocMetricOption')).toExist();
  });

  it('renders an AdhocMetricOption given an adhoc metric', () => {
--- a/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
@@ -169,7 +169,7 @@ describe('MetricsControl', () => {
      const editedMetric = sumValueAdhocMetric.duplicateWith({
        aggregate: AGGREGATES.AVG,
      });
-      component.instance().onMetricEdit(editedMetric);
+      component.instance().onMetricEdit(editedMetric, sumValueAdhocMetric);

      expect(onChange.lastCall.args).toEqual([[editedMetric]]);
    });
--- a/superset-frontend/src/common/components/Tabs/Tabs.tsx
+++ b/superset-frontend/src/common/components/Tabs/Tabs.tsx
@@ -23,15 +23,18 @@ import Icon from 'src/components/Icon';

 interface TabsProps {
  fullWidth?: boolean;
+  allowOverflow?: boolean;
 }

-const notForwardedProps = ['fullWidth'];
+const notForwardedProps = ['fullWidth', 'allowOverflow'];

 const StyledTabs = styled(AntdTabs, {
  shouldForwardProp: prop => !notForwardedProps.includes(prop),
 })<TabsProps>`
+  overflow: ${({ allowOverflow }) => (allowOverflow ? 'visible' : 'hidden')};
+
  .ant-tabs-content-holder {
-    overflow: auto;
+    overflow: ${({ allowOverflow }) => (allowOverflow ? 'visible' : 'auto')};
  }

  .ant-tabs-tab {
--- a/superset-frontend/src/explore/AdhocMetric.js
+++ b/superset-frontend/src/explore/AdhocMetric.js
@@ -92,9 +92,12 @@ export default class AdhocMetric {

  translateToSql() {
    if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
-      return `${this.aggregate || ''}(${
-        (this.column && this.column.column_name) || ''
-      })`;
+      const aggregate = this.aggregate || '';
+      // eslint-disable-next-line camelcase
+      const column = this.column?.column_name
+        ? `(${this.column.column_name})`
+        : '';
+      return aggregate + column;
    }
    if (this.expressionType === EXPRESSION_TYPES.SQL) {
      return this.sqlExpression;
--- a/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx
+++ b/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx
@@ -150,11 +150,12 @@ export default class AdhocFilterEditPopover extends React.Component {
          className="adhoc-filter-edit-tabs"
          data-test="adhoc-filter-edit-tabs"
          style={{ height: this.state.height, width: this.state.width }}
+          allowOverflow
        >
          <Tabs.TabPane
            className="adhoc-filter-edit-tab"
            key={EXPRESSION_TYPES.SIMPLE}
-            tab="Simple"
+            tab={t('Simple')}
          >
            <AdhocFilterEditPopoverSimpleTabContent
              adhocFilter={this.state.adhocFilter}
@@ -169,7 +170,7 @@ export default class AdhocFilterEditPopover extends React.Component {
          <Tabs.TabPane
            className="adhoc-filter-edit-tab"
            key={EXPRESSION_TYPES.SQL}
-            tab="Custom SQL"
+            tab={t('Custom SQL')}
          >
            {!this.props.datasource ||
            this.props.datasource.type !== 'druid' ? (
--- a/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx
+++ b/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+/* eslint-disable camelcase */
 import React from 'react';
 import PropTypes from 'prop-types';
 import { FormGroup } from 'react-bootstrap';
@@ -31,6 +32,7 @@ import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';

 import { AGGREGATES_OPTIONS } from '../constants';
 import columnType from '../propTypes/columnType';
+import savedMetricType from '../propTypes/savedMetricType';
 import AdhocMetric, { EXPRESSION_TYPES } from '../AdhocMetric';

 const propTypes = {
@@ -39,6 +41,8 @@ const propTypes = {
  onClose: PropTypes.func.isRequired,
  onResize: PropTypes.func.isRequired,
  columns: PropTypes.arrayOf(columnType),
+  savedMetrics: PropTypes.arrayOf(savedMetricType),
+  savedMetric: savedMetricType,
  datasourceType: PropTypes.string,
  title: PropTypes.shape({
    label: PropTypes.string,
@@ -54,6 +58,8 @@ const ResizeIcon = styled.i`
  margin-left: ${({ theme }) => theme.gridUnit * 2}px;
 `;

+const SAVED_TAB_KEY = 'SAVED';
+
 const startingWidth = 320;
 const startingHeight = 240;

@@ -63,6 +69,7 @@ export default class AdhocMetricEditPopover extends React.Component {
    this.onSave = this.onSave.bind(this);
    this.onColumnChange = this.onColumnChange.bind(this);
    this.onAggregateChange = this.onAggregateChange.bind(this);
+    this.onSavedMetricChange = this.onSavedMetricChange.bind(this);
    this.onSqlExpressionChange = this.onSqlExpressionChange.bind(this);
    this.onDragDown = this.onDragDown.bind(this);
    this.onMouseMove = this.onMouseMove.bind(this);
@@ -72,6 +79,7 @@ export default class AdhocMetricEditPopover extends React.Component {

    this.state = {
      adhocMetric: this.props.adhocMetric,
+      savedMetric: this.props.savedMetric,
      width: startingWidth,
      height: startingHeight,
    };
@@ -88,16 +96,24 @@ export default class AdhocMetricEditPopover extends React.Component {
    const { title } = this.props;
    const { hasCustomLabel } = title;
    let { label } = title;
-    const { adhocMetric } = this.state;
+    const { adhocMetric, savedMetric } = this.state;
    const metricLabel = adhocMetric.label;
    if (!hasCustomLabel) {
      label = metricLabel;
    }
-    this.props.onChange({
-      ...adhocMetric,
-      label,
-      hasCustomLabel,
-    });
+
+    const metric = savedMetric?.metric_name ? savedMetric : adhocMetric;
+    const oldMetric = this.props.savedMetric?.metric_name
+      ? this.props.savedMetric
+      : this.props.adhocMetric;
+    this.props.onChange(
+      {
+        ...metric,
+        label,
+        hasCustomLabel,
+      },
+      oldMetric,
+    );
    this.props.onClose();
  }

@@ -108,6 +124,7 @@ export default class AdhocMetricEditPopover extends React.Component {
        column,
        expressionType: EXPRESSION_TYPES.SIMPLE,
      }),
+      savedMetric: undefined,
    }));
  }

@@ -118,6 +135,22 @@ export default class AdhocMetricEditPopover extends React.Component {
        aggregate,
        expressionType: EXPRESSION_TYPES.SIMPLE,
      }),
+      savedMetric: undefined,
+    }));
+  }
+
+  onSavedMetricChange(savedMetricId) {
+    const savedMetric = this.props.savedMetrics.find(
+      metric => metric.id === savedMetricId,
+    );
+    this.setState(prevState => ({
+      savedMetric,
+      adhocMetric: prevState.adhocMetric.duplicateWith({
+        column: undefined,
+        aggregate: undefined,
+        sqlExpression: undefined,
+        expressionType: EXPRESSION_TYPES.SIMPLE,
+      }),
    }));
  }

@@ -127,6 +160,7 @@ export default class AdhocMetricEditPopover extends React.Component {
        sqlExpression,
        expressionType: EXPRESSION_TYPES.SQL,
      }),
+      savedMetric: undefined,
    }));
  }

@@ -171,13 +205,19 @@ export default class AdhocMetricEditPopover extends React.Component {
  }

  renderColumnOption(option) {
-    return <ColumnOption column={option} showType />;
+    const column = { ...option };
+    if (column.metric_name && !column.verbose_name) {
+      column.verbose_name = column.metric_name;
+    }
+    return <ColumnOption column={column} showType />;
  }

  render() {
    const {
      adhocMetric: propsAdhocMetric,
+      savedMetric: propsSavedMetric,
      columns,
+      savedMetrics,
      onChange,
      onClose,
      onResize,
@@ -185,7 +225,7 @@ export default class AdhocMetricEditPopover extends React.Component {
      ...popoverProps
    } = this.props;

-    const { adhocMetric } = this.state;
+    const { adhocMetric, savedMetric } = this.state;
    const keywords = sqlKeywords.concat(
      columns.map(column => ({
        name: column.column_name,
@@ -220,14 +260,32 @@ export default class AdhocMetricEditPopover extends React.Component {
      showSearch: true,
    };

+    const savedSelectProps = {
+      placeholder: t('%s saved metric(s)', savedMetrics?.length ?? 0),
+      value: savedMetric?.verbose_name || savedMetric?.metric_name,
+      onChange: this.onSavedMetricChange,
+      allowClear: true,
+      showSearch: true,
+      autoFocus: true,
+      filterOption: (input, option) =>
+        option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+    };
+
    if (this.props.datasourceType === 'druid') {
      aggregateSelectProps.options = aggregateSelectProps.options.filter(
        aggregate => aggregate !== 'AVG',
      );
    }

-    const stateIsValid = adhocMetric.isValid();
-    const hasUnsavedChanges = !adhocMetric.equals(propsAdhocMetric);
+    const stateIsValid = adhocMetric.isValid() || savedMetric?.metric_name;
+    const hasUnsavedChanges =
+      !adhocMetric.equals(propsAdhocMetric) ||
+      (!(
+        typeof savedMetric?.metric_name === 'undefined' &&
+        typeof propsSavedMetric?.metric_name === 'undefined'
+      ) &&
+        savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+
    return (
      <div
        id="metrics-edit-popover"
@@ -237,19 +295,20 @@ export default class AdhocMetricEditPopover extends React.Component {
        <Tabs
          id="adhoc-metric-edit-tabs"
          data-test="adhoc-metric-edit-tabs"
-          defaultActiveKey={adhocMetric.expressionType}
+          defaultActiveKey={
+            propsSavedMetric.metric_name
+              ? SAVED_TAB_KEY
+              : adhocMetric.expressionType
+          }
          className="adhoc-metric-edit-tabs"
          style={{ height: this.state.height, width: this.state.width }}
          onChange={this.refreshAceEditor}
+          allowOverflow
        >
-          <Tabs.TabPane
-            className="adhoc-metric-edit-tab"
-            key={EXPRESSION_TYPES.SIMPLE}
-            tab="Simple"
-          >
+          <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
            <FormGroup>
              <FormLabel>
-                <strong>column</strong>
+                <strong>{t('column')}</strong>
              </FormLabel>
              <Select name="select-column" {...columnSelectProps}>
                {columns.map(column => (
@@ -265,7 +324,7 @@ export default class AdhocMetricEditPopover extends React.Component {
            </FormGroup>
            <FormGroup>
              <FormLabel>
-                <strong>aggregate</strong>
+                <strong>{t('aggregate')}</strong>
              </FormLabel>
              <Select name="select-aggregate" {...aggregateSelectProps}>
                {AGGREGATES_OPTIONS.map(option => (
@@ -276,10 +335,30 @@ export default class AdhocMetricEditPopover extends React.Component {
              </Select>
            </FormGroup>
          </Tabs.TabPane>
+          <Tabs.TabPane key={SAVED_TAB_KEY} tab={t('Saved')}>
+            <FormGroup>
+              <FormLabel>
+                <strong>{t('Saved metric')}</strong>
+              </FormLabel>
+              <Select name="select-saved" {...savedSelectProps}>
+                {Array.isArray(savedMetrics) &&
+                  savedMetrics.map(savedMetric => (
+                    <Select.Option
+                      value={savedMetric.id}
+                      filterBy={
+                        savedMetric.verbose_name || savedMetric.metric_name
+                      }
+                      key={savedMetric.id}
+                    >
+                      {this.renderColumnOption(savedMetric)}
+                    </Select.Option>
+                  ))}
+              </Select>
+            </FormGroup>
+          </Tabs.TabPane>
          <Tabs.TabPane
-            className="adhoc-metric-edit-tab"
            key={EXPRESSION_TYPES.SQL}
-            tab="Custom SQL"
+            tab={t('Custom SQL')}
            data-test="adhoc-metric-edit-tab#custom"
          >
            {this.props.datasourceType !== 'druid' ? (
@@ -315,7 +394,7 @@ export default class AdhocMetricEditPopover extends React.Component {
            data-test="AdhocMetricEdit#cancel"
            cta
          >
-            Close
+            {t('Close')}
          </Button>
          <Button
            disabled={!stateIsValid}
@@ -327,7 +406,7 @@ export default class AdhocMetricEditPopover extends React.Component {
            onClick={this.onSave}
            cta
          >
-            Save
+            {t('Save')}
          </Button>
          <ResizeIcon
            role="button"
--- a/superset-frontend/src/explore/components/AdhocMetricOption.jsx
+++ b/superset-frontend/src/explore/components/AdhocMetricOption.jsx
@@ -18,53 +18,26 @@
 */
 import React from 'react';
 import PropTypes from 'prop-types';
-import Popover from 'src/common/components/Popover';
-import AdhocMetricEditPopoverTitle from 'src/explore/components/AdhocMetricEditPopoverTitle';
-import AdhocMetricEditPopover from './AdhocMetricEditPopover';
 import AdhocMetric from '../AdhocMetric';
 import columnType from '../propTypes/columnType';
+import savedMetricType from '../propTypes/savedMetricType';
 import { OptionControlLabel } from './OptionControls';
+import AdhocMetricPopoverTrigger from './AdhocMetricPopoverTrigger';

 const propTypes = {
  adhocMetric: PropTypes.instanceOf(AdhocMetric),
  onMetricEdit: PropTypes.func.isRequired,
  onRemoveMetric: PropTypes.func,
  columns: PropTypes.arrayOf(columnType),
+  savedMetrics: PropTypes.arrayOf(savedMetricType),
+  savedMetric: savedMetricType,
  datasourceType: PropTypes.string,
 };

 class AdhocMetricOption extends React.PureComponent {
  constructor(props) {
    super(props);
-    this.onPopoverResize = this.onPopoverResize.bind(this);
-    this.onLabelChange = this.onLabelChange.bind(this);
    this.onRemoveMetric = this.onRemoveMetric.bind(this);
-    this.closePopover = this.closePopover.bind(this);
-    this.togglePopover = this.togglePopover.bind(this);
-    this.state = {
-      popoverVisible: undefined,
-      title: {
-        label: props.adhocMetric.label,
-        hasCustomLabel: props.adhocMetric.hasCustomLabel,
-      },
-    };
-  }
-
-  componentWillUnmount() {
-    // isNew is used to auto-open the popup. Once popup is viewed, it's not
-    // considered new anymore. We mutate the prop directly because we don't
-    // want excessive rerenderings.
-    this.props.adhocMetric.isNew = false;
-  }
-
-  onLabelChange(e) {
-    const label = e.target.value;
-    this.setState({
-      title: {
-        label: label || this.props.adhocMetric.label,
-        hasCustomLabel: !!label,
-      },
-    });
  }

  onRemoveMetric(e) {
@@ -72,64 +45,32 @@ class AdhocMetricOption extends React.PureComponent {
    this.props.onRemoveMetric();
  }

-  onPopoverResize() {
-    this.forceUpdate();
-  }
-
-  closePopover() {
-    this.togglePopover(false);
-  }
-
-  togglePopover(visible) {
-    this.setState(({ popoverVisible }) => {
-      this.props.adhocMetric.isNew = false;
-      return {
-        popoverVisible: visible === undefined ? !popoverVisible : visible,
-      };
-    });
-  }
-
  render() {
-    const { adhocMetric } = this.props;
-
-    const overlayContent = (
-      <AdhocMetricEditPopover
-        adhocMetric={adhocMetric}
-        title={this.state.title}
-        columns={this.props.columns}
-        datasourceType={this.props.datasourceType}
-        onResize={this.onPopoverResize}
-        onClose={this.closePopover}
-        onChange={this.props.onMetricEdit}
-      />
-    );
-
-    const popoverTitle = (
-      <AdhocMetricEditPopoverTitle
-        title={this.state.title}
-        defaultLabel={adhocMetric.label}
-        onChange={this.onLabelChange}
-      />
-    );
-
+    const {
+      adhocMetric,
+      onMetricEdit,
+      columns,
+      savedMetrics,
+      savedMetric,
+      datasourceType,
+    } = this.props;
    return (
-      <Popover
-        placement="right"
-        trigger="click"
-        disabled
-        content={overlayContent}
-        defaultVisible={this.state.popoverVisible || adhocMetric.isNew}
-        visible={this.state.popoverVisible}
-        onVisibleChange={this.togglePopover}
-        title={popoverTitle}
+      <AdhocMetricPopoverTrigger
+        adhocMetric={adhocMetric}
+        onMetricEdit={onMetricEdit}
+        columns={columns}
+        savedMetrics={savedMetrics}
+        savedMetric={savedMetric}
+        datasourceType={datasourceType}
      >
        <OptionControlLabel
+          savedMetric={savedMetric}
          label={adhocMetric.label}
          onRemove={this.onRemoveMetric}
          isAdhoc
          isFunction
        />
-      </Popover>
+      </AdhocMetricPopoverTrigger>
    );
  }
 }
--- a/superset-frontend/src/explore/components/AdhocMetricPopoverTrigger.tsx
+++ b/superset-frontend/src/explore/components/AdhocMetricPopoverTrigger.tsx
@@ -21,12 +21,14 @@ import Popover from 'src/common/components/Popover';
 import AdhocMetricEditPopoverTitle from 'src/explore/components/AdhocMetricEditPopoverTitle';
 import AdhocMetricEditPopover from './AdhocMetricEditPopover';
 import AdhocMetric from '../AdhocMetric';
-import columnType from '../propTypes/columnType';
+import { savedMetricType } from '../types';

 export type AdhocMetricPopoverTriggerProps = {
  adhocMetric: AdhocMetric;
  onMetricEdit: () => void;
-  columns: typeof columnType[];
+  columns: { column_name: string; type: string }[];
+  savedMetrics: savedMetricType[];
+  savedMetric: savedMetricType;
  datasourceType: string;
  children: ReactNode;
  createNew?: boolean;
@@ -88,6 +90,8 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
        adhocMetric={adhocMetric}
        title={this.state.title}
        columns={this.props.columns}
+        savedMetrics={this.props.savedMetrics}
+        savedMetric={this.props.savedMetric}
        datasourceType={this.props.datasourceType}
        onResize={this.onPopoverResize}
        onClose={this.closePopover}
--- a/superset-frontend/src/explore/components/MetricDefinitionValue.jsx
+++ b/superset-frontend/src/explore/components/MetricDefinitionValue.jsx
@@ -18,7 +18,6 @@
 */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { MetricOption } from '@superset-ui/chart-controls';

 import AdhocMetricOption from './AdhocMetricOption';
 import AdhocMetric from '../AdhocMetric';
@@ -32,6 +31,7 @@ const propTypes = {
  onMetricEdit: PropTypes.func,
  onRemoveMetric: PropTypes.func,
  columns: PropTypes.arrayOf(columnType),
+  savedMetrics: PropTypes.arrayOf(savedMetricType),
  multi: PropTypes.bool,
  datasourceType: PropTypes.string,
 };
@@ -41,27 +41,34 @@ export default function MetricDefinitionValue({
  onMetricEdit,
  onRemoveMetric,
  columns,
+  savedMetrics,
  datasourceType,
 }) {
+  const getSavedMetricByName = metricName =>
+    savedMetrics.find(metric => metric.metric_name === metricName);
+
+  let savedMetric;
  if (option.metric_name) {
-    return (
-      <OptionControlLabel
-        label={<MetricOption metric={option} />}
-        onRemove={onRemoveMetric}
-        isFunction
-      />
-    );
+    savedMetric = option;
+  } else if (typeof option === 'string') {
+    savedMetric = getSavedMetricByName(option);
  }
-  if (option instanceof AdhocMetric) {
-    return (
-      <AdhocMetricOption
-        adhocMetric={option}
-        onMetricEdit={onMetricEdit}
-        onRemoveMetric={onRemoveMetric}
-        columns={columns}
-        datasourceType={datasourceType}
-      />
-    );
+
+  if (option instanceof AdhocMetric || savedMetric) {
+    const adhocMetric =
+      option instanceof AdhocMetric ? option : new AdhocMetric({});
+
+    const metricOptionProps = {
+      onMetricEdit,
+      onRemoveMetric,
+      columns,
+      savedMetrics,
+      datasourceType,
+      adhocMetric,
+      savedMetric: savedMetric ?? {},
+    };
+
+    return <AdhocMetricOption {...metricOptionProps} />;
  }
  if (typeof option === 'string') {
    return (
--- a/superset-frontend/src/explore/components/OptionControls.tsx
+++ b/superset-frontend/src/explore/components/OptionControls.tsx
@@ -18,7 +18,9 @@
 */
 import React from 'react';
 import { styled, useTheme } from '@superset-ui/core';
+import { ColumnOption } from '@superset-ui/chart-controls';
 import Icon from '../../components/Icon';
+import { savedMetricType } from '../types';

 const OptionControlContainer = styled.div<{ isAdhoc?: boolean }>`
  display: flex;
@@ -109,17 +111,30 @@ export const AddIconButton = styled.button`

 export const OptionControlLabel = ({
  label,
+  savedMetric,
  onRemove,
  isAdhoc,
  isFunction,
  ...props
 }: {
  label: string | React.ReactNode;
+  savedMetric?: savedMetricType;
  onRemove: () => void;
  isAdhoc?: boolean;
  isFunction?: boolean;
 }) => {
  const theme = useTheme();
+  const getLabelContent = () => {
+    if (savedMetric?.metric_name) {
+      // add column_name to fix typescript error
+      const column = { ...savedMetric, column_name: '' };
+      if (!column.verbose_name) {
+        column.verbose_name = column.metric_name;
+      }
+      return <ColumnOption column={column} />;
+    }
+    return label;
+  };
  return (
    <OptionControlContainer
      isAdhoc={isAdhoc}
@@ -135,7 +150,7 @@ export const OptionControlLabel = ({
      </CloseContainer>
      <Label data-test="control-label">
        {isFunction && <Icon name="function" viewBox="0 0 16 11" />}
-        {label}
+        {getLabelContent()}
      </Label>
      {isAdhoc && (
        <CaretContainer>
--- a/superset-frontend/src/explore/components/controls/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricsControl.jsx
@@ -128,6 +128,7 @@ class MetricsControl extends React.PureComponent {
        onMetricEdit={this.onMetricEdit}
        onRemoveMetric={() => this.onRemoveMetric(index)}
        columns={this.props.columns}
+        savedMetrics={this.props.savedMetrics}
        datasourceType={this.props.datasourceType}
      />
    );
@@ -178,17 +179,25 @@ class MetricsControl extends React.PureComponent {
    );
  }

-  onMetricEdit(changedMetric) {
-    let newValue = this.state.value.map(value => {
-      if (value.optionName === changedMetric.optionName) {
-        return changedMetric;
-      }
-      return value;
-    });
-    if (!this.props.multi) {
-      newValue = newValue[0];
-    }
-    this.props.onChange(newValue);
+  onMetricEdit(changedMetric, oldMetric) {
+    this.setState(
+      prevState => ({
+        value: prevState.value.map(value => {
+          if (
+            // compare saved metrics
+            value === oldMetric.metric_name ||
+            // compare adhoc metrics
+            value.optionName === oldMetric.optionName
+          ) {
+            return changedMetric;
+          }
+          return value;
+        }),
+      }),
+      () => {
+        this.onChange(this.state.value);
+      },
+    );
  }

  onRemoveMetric(index) {
@@ -242,6 +251,8 @@ class MetricsControl extends React.PureComponent {
        adhocMetric={new AdhocMetric({})}
        onMetricEdit={this.onNewMetric}
        columns={this.props.columns}
+        savedMetrics={this.props.savedMetrics}
+        savedMetric={{}}
        datasourceType={this.props.datasourceType}
        createNew
      >
--- a/superset-frontend/src/explore/propTypes/savedMetricType.js
+++ b/superset-frontend/src/explore/propTypes/savedMetricType.js
@@ -20,5 +20,6 @@ import PropTypes from 'prop-types';

 export default PropTypes.shape({
  metric_name: PropTypes.string.isRequired,
+  verbose_name: PropTypes.string,
  expression: PropTypes.string.isRequired,
 });
--- a/superset-frontend/src/explore/types.ts
+++ b/superset-frontend/src/explore/types.ts
@@ -0,0 +1,23 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+export type savedMetricType = {
+  metric_name: string;
+  verbose_name: string;
+  expression: string;
+};