mirror of
https://github.com/apache/superset.git
synced 2026-05-21 15:55:10 +00:00
fix(mcp): JSON-serialize order_by_cols and support sort direction (#39952)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
This commit is contained in:
@@ -37,6 +37,7 @@ from superset.mcp_service.chart.schemas import (
|
||||
MixedTimeseriesChartConfig,
|
||||
PieChartConfig,
|
||||
PivotTableChartConfig,
|
||||
SortByConfig,
|
||||
TableChartConfig,
|
||||
XYChartConfig,
|
||||
)
|
||||
@@ -466,7 +467,14 @@ def map_table_config(config: TableChartConfig) -> Dict[str, Any]:
|
||||
_add_adhoc_filters(form_data, config.filters)
|
||||
|
||||
if config.sort_by:
|
||||
form_data["order_by_cols"] = config.sort_by
|
||||
form_data["order_by_cols"] = [
|
||||
json.dumps(
|
||||
[entry.column, entry.ascending]
|
||||
if isinstance(entry, SortByConfig)
|
||||
else [entry, False]
|
||||
)
|
||||
for entry in config.sort_by
|
||||
]
|
||||
|
||||
form_data["row_limit"] = config.row_limit
|
||||
|
||||
|
||||
@@ -798,6 +798,30 @@ class FilterConfig(BaseModel):
|
||||
return self
|
||||
|
||||
|
||||
class SortByConfig(BaseModel):
|
||||
"""Sort specification with explicit direction.
|
||||
|
||||
Accepts either this object or a bare column-name string in `sort_by`
|
||||
lists. Bare strings default to descending, which matches the
|
||||
sort-by-metric "top N" pattern most commonly used for tables.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(populate_by_name=True)
|
||||
|
||||
column: str = Field(
|
||||
...,
|
||||
min_length=1,
|
||||
max_length=255,
|
||||
validation_alias=AliasChoices("column", "col"),
|
||||
description="Column to sort by",
|
||||
)
|
||||
ascending: bool = Field(
|
||||
False,
|
||||
description="Sort ascending. Defaults to False (descending) to match "
|
||||
"the typical sort-by-metric top-N use case.",
|
||||
)
|
||||
|
||||
|
||||
# Actual chart types
|
||||
class PieChartConfig(UnknownFieldCheckMixin):
|
||||
model_config = ConfigDict(extra="ignore", populate_by_name=True)
|
||||
@@ -1184,8 +1208,12 @@ class TableChartConfig(UnknownFieldCheckMixin):
|
||||
description="Structured filters (column/op/value). "
|
||||
"Do NOT use adhoc_filters or raw SQL expressions.",
|
||||
)
|
||||
sort_by: List[str] | None = Field(
|
||||
sort_by: List[str | SortByConfig] | None = Field(
|
||||
None,
|
||||
description="Columns to sort by. Each entry is either a column-name "
|
||||
"string (defaults to descending) or a SortByConfig object with "
|
||||
"explicit `ascending`. Descending matches the sort-by-metric "
|
||||
"top-N pattern most common for tables.",
|
||||
validation_alias=AliasChoices("sort_by", "order_by_cols", "order_by"),
|
||||
)
|
||||
row_limit: int = Field(1000, description="Max rows returned", ge=1, le=50000)
|
||||
|
||||
@@ -44,6 +44,7 @@ from superset.mcp_service.chart.schemas import (
|
||||
ColumnRef,
|
||||
FilterConfig,
|
||||
LegendConfig,
|
||||
SortByConfig,
|
||||
TableChartConfig,
|
||||
XYChartConfig,
|
||||
)
|
||||
@@ -295,7 +296,7 @@ class TestMapTableConfig:
|
||||
assert result["adhoc_filters"][2]["comparator"] == ["Sports", "Racing"]
|
||||
|
||||
def test_map_table_config_with_sort(self) -> None:
|
||||
"""Test table config mapping with sort"""
|
||||
"""Bare strings default to descending."""
|
||||
config = TableChartConfig(
|
||||
chart_type="table",
|
||||
columns=[ColumnRef(name="product")],
|
||||
@@ -303,7 +304,26 @@ class TestMapTableConfig:
|
||||
)
|
||||
|
||||
result = map_table_config(config)
|
||||
assert result["order_by_cols"] == ["product", "revenue"]
|
||||
assert result["order_by_cols"] == ['["product", false]', '["revenue", false]']
|
||||
|
||||
def test_map_table_config_with_sort_direction(self) -> None:
|
||||
"""SortByConfig entries honor the explicit ascending flag."""
|
||||
config = TableChartConfig(
|
||||
chart_type="table",
|
||||
columns=[ColumnRef(name="product")],
|
||||
sort_by=[
|
||||
SortByConfig(column="product", ascending=True),
|
||||
SortByConfig(column="revenue", ascending=False),
|
||||
"category",
|
||||
],
|
||||
)
|
||||
|
||||
result = map_table_config(config)
|
||||
assert result["order_by_cols"] == [
|
||||
'["product", true]',
|
||||
'["revenue", false]',
|
||||
'["category", false]',
|
||||
]
|
||||
|
||||
def test_map_table_config_ag_grid_table(self) -> None:
|
||||
"""Test table config mapping with AG Grid Interactive Table viz_type"""
|
||||
|
||||
Reference in New Issue
Block a user