From e4fca89217fc52a31053470f1b4c91a56ed3f4e9 Mon Sep 17 00:00:00 2001
From: Diego Medina <diegomedina24@gmail.com>
Date: Wed, 27 Apr 2022 22:37:12 -0400
Subject: [PATCH] fix: Cannot re-order metrics by drag and drop (#19876)

* fix: cannot-re-order-metrics-by-drag-and-drop

* add tests
---
 .../src/components/MetricOption.tsx           | 36 +++++++++++-------
 .../DndMetricSelect.test.tsx                  | 37 ++++++++++++++++++-
 .../DndColumnSelectControl/DndSelectLabel.tsx |  6 ++-
 .../controls/OptionControls/index.tsx         |  8 +++-
 4 files changed, 71 insertions(+), 16 deletions(-)

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx
index cc9bd493a87b..9ee391aba5c1 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx
@@ -47,6 +47,7 @@ export interface MetricOptionProps {
   showType?: boolean;
   url?: string;
   labelRef?: React.RefObject<any>;
+  shouldShowTooltip?: boolean;
 }
 
 export function MetricOption({
@@ -55,6 +56,7 @@ export function MetricOption({
   openInNewWindow = false,
   showFormula = true,
   showType = false,
+  shouldShowTooltip = true,
   url = '',
 }: MetricOptionProps) {
   const verbose = metric.verbose_name || metric.metric_name || metric.label;
@@ -66,6 +68,20 @@ export function MetricOption({
     verbose
   );
 
+  const label = (
+    <span
+      className="option-label metric-option-label"
+      css={(theme: SupersetTheme) =>
+        css`
+          margin-right: ${theme.gridUnit}px;
+        `
+      }
+      ref={labelRef}
+    >
+      {link}
+    </span>
+  );
+
   const warningMarkdown = metric.warning_markdown || metric.warning_text;
 
   const [tooltipText, setTooltipText] = useState<ReactNode>(metric.metric_name);
@@ -77,19 +93,13 @@ export function MetricOption({
   return (
     <FlexRowContainer className="metric-option">
       {showType && <ColumnTypeLabel type="expression" />}
-      <Tooltip id="metric-name-tooltip" title={tooltipText}>
-        <span
-          className="option-label metric-option-label"
-          css={(theme: SupersetTheme) =>
-            css`
-              margin-right: ${theme.gridUnit}px;
-            `
-          }
-          ref={labelRef}
-        >
-          {link}
-        </span>
-      </Tooltip>
+      {shouldShowTooltip ? (
+        <Tooltip id="metric-name-tooltip" title={tooltipText}>
+          {label}
+        </Tooltip>
+      ) : (
+        label
+      )}
       {showFormula && metric.expression && (
         <SQLPopover sqlExpression={metric.expression} />
       )}
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 ae6a8b31f23f..d59f483bc67f 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
@@ -17,7 +17,12 @@
  * under the License.
  */
 import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+  render,
+  screen,
+  within,
+  fireEvent,
+} from 'spec/helpers/testing-library';
 import { DndMetricSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndMetricSelect';
 import { AGGREGATES } from 'src/explore/constants';
 import { EXPRESSION_TYPES } from '../MetricControl/AdhocMetric';
@@ -27,6 +32,7 @@ const defaultProps = {
     {
       metric_name: 'metric_a',
       expression: 'expression_a',
+      verbose_name: 'metric_a',
     },
     {
       metric_name: 'metric_b',
@@ -282,3 +288,32 @@ test('update adhoc metric name when column label in dataset changes', () => {
   expect(screen.getByText('SUM(new col A name)')).toBeVisible();
   expect(screen.getByText('SUM(new col B name)')).toBeVisible();
 });
+
+test('can drag metrics', async () => {
+  const metricValues = ['metric_a', 'metric_b', adhocMetricB];
+  render(<DndMetricSelect {...defaultProps} value={metricValues} multi />, {
+    useDnd: true,
+  });
+
+  expect(screen.getByText('metric_a')).toBeVisible();
+  expect(screen.getByText('Metric B')).toBeVisible();
+
+  const container = screen.getByTestId('dnd-labels-container');
+  expect(container.childElementCount).toBe(4);
+
+  const firstMetric = container.children[0] as HTMLElement;
+  const lastMetric = container.children[2] as HTMLElement;
+  expect(within(firstMetric).getByText('metric_a')).toBeVisible();
+  expect(within(lastMetric).getByText('SUM(Column B)')).toBeVisible();
+
+  fireEvent.mouseOver(within(firstMetric).getByText('metric_a'));
+  expect(await screen.findByText('Metric name')).toBeInTheDocument();
+
+  fireEvent.dragStart(firstMetric);
+  fireEvent.dragEnter(lastMetric);
+  fireEvent.dragOver(lastMetric);
+  fireEvent.drop(lastMetric);
+
+  expect(within(firstMetric).getByText('SUM(Column B)')).toBeVisible();
+  expect(within(lastMetric).getByText('metric_a')).toBeVisible();
+});
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
index 019d9d3b99d0..3947fb3bef98 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
@@ -87,7 +87,11 @@ export default function DndSelectLabel({
       <HeaderContainer>
         <ControlHeader {...props} />
       </HeaderContainer>
-      <DndLabelsContainer canDrop={canDrop} isOver={isOver}>
+      <DndLabelsContainer
+        data-test="dnd-labels-container"
+        canDrop={canDrop}
+        isOver={isOver}
+      >
         {props.valuesRenderer()}
         {displayGhostButton && renderGhostButton()}
       </DndLabelsContainer>
diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
index b97d197cdc9a..74ec85dc143b 100644
--- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
+++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx
@@ -279,7 +279,13 @@ export const OptionControlLabel = ({
         labelRef.current.scrollWidth > labelRef.current.clientWidth);
 
     if (savedMetric && hasMetricName) {
-      return <StyledMetricOption metric={savedMetric} labelRef={labelRef} />;
+      return (
+        <StyledMetricOption
+          metric={savedMetric}
+          labelRef={labelRef}
+          shouldShowTooltip={!isDragging}
+        />
+      );
     }
     if (!shouldShowTooltip) {
       return <LabelText ref={labelRef}>{label}</LabelText>;