fix(dashboard): 500 error caused by data_for_slices API (apache#16053)

superset/connectors/base/models.py

@@ -282,10 +282,9 @@ def data_for_slices(self, slices: List[Slice]) -> Dict[str, Any]:
         column_names = set()
         for slc in slices:
             form_data = slc.form_data
-
             # pull out all required metrics from the form_data
-            for param in METRIC_FORM_DATA_PARAMS:
-                for metric in utils.get_iterable(form_data.get(param) or []):
+            for metric_param in METRIC_FORM_DATA_PARAMS:
+                for metric in utils.get_iterable(form_data.get(metric_param) or []):
                     metric_names.add(utils.get_metric_name(metric))
                     if utils.is_adhoc_metric(metric):
                         column_names.add(
@@ -308,8 +307,8 @@ def data_for_slices(self, slices: List[Slice]) -> Dict[str, Any]:
 
             column_names.update(
                 column
-                for column in utils.get_iterable(form_data.get(param) or [])
-                for param in COLUMN_FORM_DATA_PARAMS
+                for column_param in COLUMN_FORM_DATA_PARAMS
+                for column in utils.get_iterable(form_data.get(column_param) or [])
             )
 
         filtered_metrics = [

superset/utils/core.py

@@ -1217,10 +1217,10 @@ def get_metric_name(metric: Metric) -> str:
 
 
 def get_metric_names(metrics: Sequence[Metric]) -> List[str]:
-    return [get_metric_name(metric) for metric in metrics]
+    return [metric for metric in map(get_metric_name, metrics) if metric]
 
 
-def get_main_metric_name(metrics: Sequence[Metric]) -> Optional[str]:
+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
 
@@ -1427,7 +1427,6 @@ def get_iterable(x: Any) -> List[Any]:
     :param x: The object
     :returns: An iterable representation
     """
-
     return x if isinstance(x, list) else [x]
 
 
@@ -1464,12 +1463,7 @@ def get_column_names_from_metrics(metrics: List[Metric]) -> List[str]:
     :param metrics: Ad-hoc metric
     :return: column name if simple metric, otherwise None
     """
-    columns: List[str] = []
-    for metric in metrics:
-        column_name = get_column_name_from_metric(metric)
-        if column_name:
-            columns.append(column_name)
-    return columns
+    return [col for col in map(get_column_name_from_metric, metrics) if col]
 
 
 def extract_dataframe_dtypes(df: pd.DataFrame) -> List[GenericDataType]:

superset/viz.py

@@ -1230,7 +1230,7 @@ def query_obj(self) -> QueryObjectDict:
         d = super().query_obj()
         sort_by = self.form_data.get(
             "timeseries_limit_metric"
-        ) or utils.get_main_metric_name(d.get("metrics") or [])
+        ) or utils.get_first_metric_name(d.get("metrics") or [])
         is_asc = not self.form_data.get("order_desc")
         if sort_by:
             sort_by_label = utils.get_metric_name(sort_by)

tests/integration_tests/model_tests.py

@@ -497,13 +497,15 @@ def test_data_for_slices(self):
         slc = (
             metadata_db.session.query(Slice)
             .filter_by(
-                datasource_id=tbl.id,
-                datasource_type=tbl.type,
-                slice_name="Participants",
+                datasource_id=tbl.id, datasource_type=tbl.type, slice_name="Genders",
             )
             .first()
         )
         data_for_slices = tbl.data_for_slices([slc])
-        self.assertEqual(len(data_for_slices["columns"]), 0)
-        self.assertEqual(len(data_for_slices["metrics"]), 1)
-        self.assertEqual(len(data_for_slices["verbose_map"].keys()), 2)
+        assert len(data_for_slices["metrics"]) == 1
+        assert len(data_for_slices["columns"]) == 1
+        assert data_for_slices["metrics"][0]["metric_name"] == "sum__num"
+        assert data_for_slices["columns"][0]["column_name"] == "gender"
+        assert set(data_for_slices["verbose_map"].keys()) == set(
+            ["__timestamp", "sum__num", "gender",]
+        )