From 6280b34483ca526e2e819aa46ae738b215a46efd Mon Sep 17 00:00:00 2001
From: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Date: Fri, 22 Jan 2021 00:51:59 +0100
Subject: [PATCH] fix(explore): don't allow selecting duplicated saved metric
 (#12657)

---
 .../MetricControl/AdhocMetricEditPopover.jsx  | 16 ++++++++--------
 .../MetricControl/AdhocMetricOption.jsx       |  6 +++---
 .../AdhocMetricPopoverTrigger.tsx             |  4 ++--
 .../MetricControl/MetricDefinitionValue.jsx   |  4 +++-
 .../controls/MetricControl/MetricsControl.jsx | 19 ++++++++++++++++++-
 5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
index ae39d183b2..f27f776c15 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
@@ -43,7 +43,7 @@ const propTypes = {
   onResize: PropTypes.func.isRequired,
   getCurrentTab: PropTypes.func,
   columns: PropTypes.arrayOf(columnType),
-  savedMetrics: PropTypes.arrayOf(savedMetricType),
+  savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
   savedMetric: savedMetricType,
   datasourceType: PropTypes.string,
   title: PropTypes.shape({
@@ -76,8 +76,8 @@ export default class AdhocMetricEditPopover extends React.Component {
   // "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.savedMetrics) &&
-    this.props.savedMetrics.length > 0
+    Array.isArray(this.props.savedMetricsOptions) &&
+    this.props.savedMetricsOptions.length > 0
       ? SAVED_TAB_KEY
       : this.props.adhocMetric.expressionType;
 
@@ -173,7 +173,7 @@ export default class AdhocMetricEditPopover extends React.Component {
   }
 
   onSavedMetricChange(savedMetricId) {
-    const savedMetric = this.props.savedMetrics.find(
+    const savedMetric = this.props.savedMetricsOptions.find(
       metric => metric.id === savedMetricId,
     );
     this.setState(prevState => ({
@@ -259,7 +259,7 @@ export default class AdhocMetricEditPopover extends React.Component {
       adhocMetric: propsAdhocMetric,
       savedMetric: propsSavedMetric,
       columns,
-      savedMetrics,
+      savedMetricsOptions,
       onChange,
       onClose,
       onResize,
@@ -303,7 +303,7 @@ export default class AdhocMetricEditPopover extends React.Component {
     };
 
     const savedSelectProps = {
-      placeholder: t('%s saved metric(s)', savedMetrics?.length ?? 0),
+      placeholder: t('%s saved metric(s)', savedMetricsOptions?.length ?? 0),
       value: savedMetric?.verbose_name || savedMetric?.metric_name,
       onChange: this.onSavedMetricChange,
       allowClear: true,
@@ -349,8 +349,8 @@ export default class AdhocMetricEditPopover extends React.Component {
                 <strong>{t('Saved metric')}</strong>
               </FormLabel>
               <Select name="select-saved" {...savedSelectProps}>
-                {Array.isArray(savedMetrics) &&
-                  savedMetrics.map(savedMetric => (
+                {Array.isArray(savedMetricsOptions) &&
+                  savedMetricsOptions.map(savedMetric => (
                     <Select.Option
                       value={savedMetric.id}
                       filterBy={
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
index a43d93f20b..ad063fb74f 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
@@ -31,7 +31,7 @@ const propTypes = {
   onMetricEdit: PropTypes.func.isRequired,
   onRemoveMetric: PropTypes.func,
   columns: PropTypes.arrayOf(columnType),
-  savedMetrics: PropTypes.arrayOf(savedMetricType),
+  savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
   savedMetric: savedMetricType,
   datasourceType: PropTypes.string,
   onMoveLabel: PropTypes.func,
@@ -55,7 +55,7 @@ class AdhocMetricOption extends React.PureComponent {
       adhocMetric,
       onMetricEdit,
       columns,
-      savedMetrics,
+      savedMetricsOptions,
       savedMetric,
       datasourceType,
       onMoveLabel,
@@ -67,7 +67,7 @@ class AdhocMetricOption extends React.PureComponent {
         adhocMetric={adhocMetric}
         onMetricEdit={onMetricEdit}
         columns={columns}
-        savedMetrics={savedMetrics}
+        savedMetricsOptions={savedMetricsOptions}
         savedMetric={savedMetric}
         datasourceType={datasourceType}
       >
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
index c5d0e92f06..8080b2bff3 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
@@ -29,7 +29,7 @@ export type AdhocMetricPopoverTriggerProps = {
   adhocMetric: AdhocMetric;
   onMetricEdit: () => void;
   columns: { column_name: string; type: string }[];
-  savedMetrics: savedMetricType[];
+  savedMetricsOptions: savedMetricType[];
   savedMetric: savedMetricType;
   datasourceType: string;
   children: ReactNode;
@@ -105,7 +105,7 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
         adhocMetric={adhocMetric}
         title={title}
         columns={this.props.columns}
-        savedMetrics={this.props.savedMetrics}
+        savedMetricsOptions={this.props.savedMetricsOptions}
         savedMetric={this.props.savedMetric}
         datasourceType={this.props.datasourceType}
         onResize={this.onPopoverResize}
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
index 30acfdaa54..704c2c8b0d 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 = {
   onDropLabel: PropTypes.func,
   columns: PropTypes.arrayOf(columnType),
   savedMetrics: PropTypes.arrayOf(savedMetricType),
+  savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
   multi: PropTypes.bool,
   datasourceType: PropTypes.string,
 };
@@ -45,6 +46,7 @@ export default function MetricDefinitionValue({
   onRemoveMetric,
   columns,
   savedMetrics,
+  savedMetricsOptions,
   datasourceType,
   onMoveLabel,
   onDropLabel,
@@ -68,7 +70,7 @@ export default function MetricDefinitionValue({
       onMetricEdit,
       onRemoveMetric,
       columns,
-      savedMetrics,
+      savedMetricsOptions,
       datasourceType,
       adhocMetric,
       onMoveLabel,
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
index 7b7233fe49..a4916e95dc 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
@@ -64,6 +64,16 @@ const defaultProps = {
   columns: [],
 };
 
+function getOptionsForSavedMetrics(savedMetrics, currentMetricValues) {
+  return (
+    savedMetrics?.filter(savedMetric =>
+      Array.isArray(currentMetricValues)
+        ? !currentMetricValues.includes(savedMetric.metric_name)
+        : savedMetric,
+    ) ?? []
+  );
+}
+
 function isDictionaryForAdhocMetric(value) {
   return value && !(value instanceof AdhocMetric) && value.expressionType;
 }
@@ -131,6 +141,10 @@ class MetricsControl extends React.PureComponent {
         onRemoveMetric={() => this.onRemoveMetric(index)}
         columns={this.props.columns}
         savedMetrics={this.props.savedMetrics}
+        savedMetricsOptions={getOptionsForSavedMetrics(
+          this.props.savedMetrics,
+          this.props.value,
+        )}
         datasourceType={this.props.datasourceType}
         onMoveLabel={this.moveLabel}
         onDropLabel={() => this.props.onChange(this.state.value)}
@@ -268,7 +282,10 @@ class MetricsControl extends React.PureComponent {
         adhocMetric={new AdhocMetric({ isNew: true })}
         onMetricEdit={this.onNewMetric}
         columns={this.props.columns}
-        savedMetrics={this.props.savedMetrics}
+        savedMetricsOptions={getOptionsForSavedMetrics(
+          this.props.savedMetrics,
+          this.props.value,
+        )}
         savedMetric={{}}
         datasourceType={this.props.datasourceType}
         createNew