diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 5fb59a7d1..460cfcb0e 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -383,8 +383,12 @@ class DashboardRestApi(BaseSupersetModelRestApi): self.dashboard_dataset_schema.dump(dataset) for dataset in datasets ] return self.response(200, result=result) - except TypeError: - return self.response_400(message=gettext("Dataset schema is invalid.")) + except (TypeError, ValueError) as err: + return self.response_400( + message=gettext( + "Dataset schema is invalid, caused by: %(error)s", error=str(err) + ) + ) except DashboardAccessDeniedError: return self.response_403() except DashboardNotFoundError: diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index e7f61964e..4449b3dfa 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -61,7 +61,9 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -class Query(Model, ExtraJSONMixin, ExploreMixin): # pylint: disable=abstract-method +class Query( + Model, ExtraJSONMixin, ExploreMixin +): # pylint: disable=abstract-method,too-many-public-methods """ORM model for SQL query Now that SQL Lab support multi-statement execution, an entry in this diff --git a/superset/utils/core.py b/superset/utils/core.py index ba7237b77..6ce5a8a83 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -1294,7 +1294,7 @@ def get_metric_name( sql_expression = metric.get("sqlExpression") if sql_expression: return sql_expression - elif expression_type == "SIMPLE": + if expression_type == "SIMPLE": column: AdhocMetricColumn = metric.get("column") or {} column_name = column.get("column_name") aggregate = metric.get("aggregate") @@ -1302,10 +1302,12 @@ def get_metric_name( return f"{aggregate}({column_name})" if column_name: return column_name - raise ValueError(__("Invalid metric object")) - verbose_map = verbose_map or {} - return verbose_map.get(metric, metric) # type: ignore + if isinstance(metric, str): + verbose_map = verbose_map or {} + return verbose_map.get(metric, metric) + + raise ValueError(__("Invalid metric object: %(metric)s", metric=str(metric))) def get_column_names( diff --git a/tests/integration_tests/fixtures/deck_geojson_form_data.json b/tests/integration_tests/fixtures/deck_geojson_form_data.json index 422197a28..e8258c7d4 100644 --- a/tests/integration_tests/fixtures/deck_geojson_form_data.json +++ b/tests/integration_tests/fixtures/deck_geojson_form_data.json @@ -43,5 +43,5 @@ "granularity_sqla": null, "autozoom": true, "url_params": {}, - "size": 100 + "size": "100" } diff --git a/tests/integration_tests/fixtures/deck_path_form_data.json b/tests/integration_tests/fixtures/deck_path_form_data.json index 39cc2007f..ac2e404d8 100644 --- a/tests/integration_tests/fixtures/deck_path_form_data.json +++ b/tests/integration_tests/fixtures/deck_path_form_data.json @@ -45,5 +45,5 @@ "granularity_sqla": null, "autozoom": true, "url_params": {}, - "size": 100 + "size": "100" } diff --git a/tests/integration_tests/viz_tests.py b/tests/integration_tests/viz_tests.py index 6a8bda3df..137e2a474 100644 --- a/tests/integration_tests/viz_tests.py +++ b/tests/integration_tests/viz_tests.py @@ -716,7 +716,7 @@ class TestPairedTTest(SupersetTestCase): self.assertEqual(data, expected) def test_get_data_empty_null_keys(self): - form_data = {"groupby": [], "metrics": ["", None]} + form_data = {"groupby": [], "metrics": [""]} datasource = self.get_datasource_mock() # Test data raw = {} @@ -739,19 +739,13 @@ class TestPairedTTest(SupersetTestCase): "group": "All", } ], - "NULL": [ - { - "values": [ - {"x": 100, "y": 10}, - {"x": 200, "y": 20}, - {"x": 300, "y": 30}, - ], - "group": "All", - } - ], } self.assertEqual(data, expected) + form_data = {"groupby": [], "metrics": [None]} + with self.assertRaises(ValueError): + viz.viz_types["paired_ttest"](datasource, form_data) + class TestPartitionViz(SupersetTestCase): @patch("superset.viz.BaseViz.query_obj") diff --git a/tests/unit_tests/core_tests.py b/tests/unit_tests/core_tests.py index f7a004715..bd151011a 100644 --- a/tests/unit_tests/core_tests.py +++ b/tests/unit_tests/core_tests.py @@ -105,6 +105,18 @@ def test_get_metric_name_invalid_metric(): with pytest.raises(ValueError): get_metric_name(metric) + metric = deepcopy(SQL_ADHOC_METRIC) + del metric["expressionType"] + with pytest.raises(ValueError): + get_metric_name(metric) + + with pytest.raises(ValueError): + get_metric_name(None) + with pytest.raises(ValueError): + get_metric_name(0) + with pytest.raises(ValueError): + get_metric_name({}) + def test_get_metric_names(): assert get_metric_names(