fix(dashboard): stop list-valued order_by_cols from 500-ing the datasets API

superset/connectors/sqla/models.py

@@ -329,6 +329,42 @@ def _physical_display_column(reference: dict[str, Any]) -> str | None:
     return None
 
 
+def _column_param_name(column_: Any) -> str | None:
+    """Return the column name a ``COLUMN_FORM_DATA_PARAMS`` entry refers to.
+
+    Entries are not uniformly shaped. Most params hold plain column names or
+    adhoc-column dicts, but ``order_by_cols`` holds ``[column, ascending]``
+    pairs -- canonically JSON *strings* (``'["col", false]'``), though older
+    writers persisted them as real lists. A raw list reaching ``set.update()``
+    raises ``TypeError: unhashable type: 'list'`` and 500s
+    ``GET /api/v1/dashboard/<id>/datasets``, which leaves the whole dashboard
+    without datasource metadata. Mirror the tolerance that
+    ``superset.common.form_data_query_context`` already applies: accept the
+    pair in either encoding and take the column half, which may itself be an
+    adhoc column. A string that is not valid JSON is a plain column name
+    (bracketed names such as ``[Revenue]`` are legal). Any other shape yields
+    ``None`` and is skipped rather than crashing the request.
+    """
+    if utils.is_adhoc_column(column_):
+        return utils.get_column_name(column_)
+    if isinstance(column_, str):
+        # A JSON-encoded ``[column, ascending]`` pair, or a plain column name.
+        if column_.startswith("["):
+            try:
+                parsed = json.loads(column_)
+            except (TypeError, ValueError):
+                return column_
+            return (
+                _column_param_name(parsed[0])
+                if parsed and isinstance(parsed, list)
+                else None
+            )
+        return column_
+    if isinstance(column_, (list, tuple)):
+        return _column_param_name(column_[0]) if column_ else None
+    return None
+
+
 class DatasourceKind(StrEnum):
     VIRTUAL = "virtual"
     PHYSICAL = "physical"
@@ -933,13 +969,10 @@ def data_for_slices(  # pylint: disable=too-many-locals  # noqa: C901
                 column_names.update(query_context_column_names)
             else:
                 _columns = [
-                    (
-                        utils.get_column_name(column_)
-                        if utils.is_adhoc_column(column_)
-                        else column_
-                    )
+                    name
                     for column_param in COLUMN_FORM_DATA_PARAMS
                     for column_ in utils.as_list(form_data.get(column_param) or [])
+                    if (name := _column_param_name(column_)) is not None
                 ]
                 column_names.update(_columns)
 

tests/unit_tests/connectors/sqla/models_test.py

@@ -1970,6 +1970,64 @@ def test_data_for_slices_falls_back_for_invalid_query_context(
     mock_slice.get_query_context.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    "order_by_cols,expected",
+    [
+        # Canonical encoding: a list of JSON strings.
+        (['["price_per_sqm", false]'], ["price_per_sqm", "state"]),
+        # Persisted by older writers as a real list -- used to raise
+        # ``TypeError: unhashable type: 'list'`` and 500 the dashboard
+        # datasets endpoint, leaving every widget without metadata.
+        ([["price_per_sqm", False]], ["price_per_sqm", "state"]),
+        # Non-ASCII column names must survive both encodings.
+        ([["Δ медиана %", True]], ["state", "Δ медиана %"]),
+        # The column half may itself be an adhoc column.
+        (
+            [[{"label": "price_per_sqm", "sqlExpression": "price_per_sqm"}, True]],
+            ["price_per_sqm", "state"],
+        ),
+        # Junk entries are skipped rather than crashing the request.
+        ([[123, False]], ["state"]),
+        ([[]], ["state"]),
+        (['["unclosed'], ["state"]),
+    ],
+)
+def test_data_for_slices_handles_order_by_cols_encodings(
+    mocker: MockerFixture,
+    order_by_cols: list[object],
+    expected: list[str],
+) -> None:
+    """Test ``order_by_cols`` pairs are accepted as JSON strings or raw lists.
+
+    ``order_by_cols`` holds ``[column, ascending]`` pairs. They are canonically
+    stored as JSON strings, but real deployments contain raw lists. A raw list
+    reaching ``set.update()`` raises ``TypeError: unhashable type: 'list'``.
+    """
+    database = mocker.MagicMock()
+    database.id = 1
+
+    table = SqlaTable(
+        table_name="test_table",
+        database=database,
+        columns=[
+            TableColumn(column_name="state"),
+            TableColumn(column_name="price_per_sqm"),
+            TableColumn(column_name="Δ медиана %"),
+        ],
+        metrics=[],
+    )
+
+    mock_slice = mocker.MagicMock()
+    mock_slice.form_data = {"groupby": ["state"], "order_by_cols": order_by_cols}
+    mock_slice.query_context = None
+
+    result = table.data_for_slices([mock_slice])
+
+    assert sorted(column["column_name"] for column in result["columns"]) == sorted(
+        expected
+    )
+
+
 def test_data_for_slices_skips_invalid_query_context_entries(
     mocker: MockerFixture,
 ) -> None: