From 1c12167d8d12b6a092356c008286f6f07f1c9954 Mon Sep 17 00:00:00 2001
From: John Bodley <4567245+john-bodley@users.noreply.github.com>
Date: Wed, 3 Nov 2021 13:59:38 -0700
Subject: [PATCH] fix: Revert default series sort-by metric and enforce non-xor
 with series limit (#17236)

Co-authored-by: John Bodley <john.bodley@airbnb.com>
---
 superset-frontend/src/explore/controls.jsx | 26 ++++++++++++++++------
 superset/utils/core.py                     |  5 -----
 superset/viz.py                            |  4 +---
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/superset-frontend/src/explore/controls.jsx b/superset-frontend/src/explore/controls.jsx
index 640e0ad165fe..e2f82c91613f 100644
--- a/superset-frontend/src/explore/controls.jsx
+++ b/superset-frontend/src/explore/controls.jsx
@@ -92,7 +92,7 @@ export const D3_FORMAT_OPTIONS = [
 
 const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];
 
-const SERIES_LIMITS = [0, 5, 10, 25, 50, 100, 500];
+const SERIES_LIMITS = [5, 10, 25, 50, 100, 500];
 
 export const D3_FORMAT_DOCS =
   'D3 format syntax: https://github.com/d3/d3-format';
@@ -123,7 +123,10 @@ const groupByControl = {
   label: t('Group by'),
   default: [],
   includeTime: false,
-  description: t('One or many controls to group by'),
+  description: t(
+    'One or many columns to group by. High cardinality groupings should include a sort by metric ' +
+      'and series limit to limit the number of fetched and rendered series.',
+  ),
   optionRenderer: c => <StyledColumnOption column={c} showType />,
   valueKey: 'column_name',
   filterOption: ({ data: opt }, text) =>
@@ -358,6 +361,10 @@ export const controls = {
     validators: [legacyValidateInteger],
     default: 10000,
     choices: formatSelectOptions(ROW_LIMIT_OPTIONS),
+    description: t(
+      'Limits the number of rows that get displayed. Should be used in conjunction with a sort ' +
+        'by metric.',
+    ),
   },
 
   limit: {
@@ -366,11 +373,13 @@ export const controls = {
     label: t('Series limit'),
     validators: [legacyValidateInteger],
     choices: formatSelectOptions(SERIES_LIMITS),
+    clearable: true,
     description: t(
-      'Limits the number of time series that get displayed. A sub query ' +
-        '(or an extra phase where sub queries are not supported) is applied to limit ' +
-        'the number of time series that get fetched and displayed. This feature is useful ' +
-        'when grouping by high cardinality dimension(s).',
+      'Limits the number of series that get displayed. Should be used in conjunction with a sort ' +
+        'by metric. A joined subquery (or an extra phase where subqueries are not supported) is ' +
+        'applied to limit the number of series that get fetched and rendered. This feature is ' +
+        'useful when grouping by high cardinality column(s) though does increase the query ' +
+        'complexity and cost.',
     ),
   },
 
@@ -379,7 +388,10 @@ export const controls = {
     label: t('Sort by'),
     default: null,
     clearable: true,
-    description: t('Metric used to define the top series'),
+    description: t(
+      'Metric used to define the top series. Should be used in conjunction with the series or ' +
+        'row limit',
+    ),
     mapStateToProps: state => ({
       columns: state.datasource ? state.datasource.columns : [],
       savedMetrics: state.datasource ? state.datasource.metrics : [],
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 83191b2303d0..10b8afdaeb80 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -1317,11 +1317,6 @@ def get_metric_names(metrics: Sequence[Metric]) -> List[str]:
     return [metric for metric in map(get_metric_name, metrics) if metric]
 
 
-def get_first_metric_name(metrics: Sequence[Metric]) -> Optional[str]:
-    metric_labels = get_metric_names(metrics)
-    return metric_labels[0] if metric_labels else None
-
-
 def ensure_path_exists(path: str) -> None:
     try:
         os.makedirs(path)
diff --git a/superset/viz.py b/superset/viz.py
index ecf4f63c6a20..f1415b2fc06d 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -1251,9 +1251,7 @@ class NVD3TimeSeriesViz(NVD3Viz):
 
     def query_obj(self) -> QueryObjectDict:
         query_obj = super().query_obj()
-        sort_by = self.form_data.get(
-            "timeseries_limit_metric"
-        ) or utils.get_first_metric_name(query_obj.get("metrics") or [])
+        sort_by = self.form_data.get("timeseries_limit_metric")
         is_asc = not self.form_data.get("order_desc")
         if sort_by:
             sort_by_label = utils.get_metric_name(sort_by)