mirror of
https://github.com/apache/superset.git
synced 2026-04-19 16:14:52 +00:00
fix(mcp): replace uuid with url and changed_on_humanized in default list columns (#38566)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -68,7 +68,8 @@ class TestListChartsRequestSchema:
|
||||
"""Test creating request with all defaults.
|
||||
|
||||
Note: select_columns defaults to empty list, which triggers
|
||||
minimal default columns (id, slice_name, viz_type, uuid) in the tool.
|
||||
minimal default columns (id, slice_name, viz_type, url,
|
||||
changed_on_humanized) in the tool.
|
||||
"""
|
||||
request = ListChartsRequest()
|
||||
|
||||
@@ -183,7 +184,13 @@ class TestChartDefaultColumnFiltering:
|
||||
from superset.mcp_service.common.schema_discovery import CHART_DEFAULT_COLUMNS
|
||||
|
||||
# Required minimal columns must be present
|
||||
required_columns = {"id", "slice_name", "viz_type", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"slice_name",
|
||||
"viz_type",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(CHART_DEFAULT_COLUMNS))
|
||||
|
||||
# Heavy columns should NOT be in defaults
|
||||
@@ -191,6 +198,7 @@ class TestChartDefaultColumnFiltering:
|
||||
assert "query_context" not in CHART_DEFAULT_COLUMNS
|
||||
assert "description" not in CHART_DEFAULT_COLUMNS
|
||||
assert "datasource_name" not in CHART_DEFAULT_COLUMNS
|
||||
assert "uuid" not in CHART_DEFAULT_COLUMNS
|
||||
|
||||
def test_empty_select_columns_default(self):
|
||||
"""Test that select_columns defaults to empty list which triggers
|
||||
|
||||
@@ -108,14 +108,14 @@ async def test_list_dashboards_basic(mock_list, mcp_server):
|
||||
dashboards = data["dashboards"]
|
||||
assert len(dashboards) == 1
|
||||
assert dashboards[0]["dashboard_title"] == "Test Dashboard"
|
||||
assert dashboards[0]["uuid"] == "test-dashboard-uuid-1"
|
||||
assert dashboards[0]["slug"] == "test-dashboard"
|
||||
# Note: published is not in minimal default columns (id, dashboard_title,
|
||||
# slug, uuid) - use select_columns to include it if needed
|
||||
# slug, url, changed_on_humanized) - use select_columns to include it
|
||||
|
||||
assert "uuid" in data["columns_requested"]
|
||||
assert "url" in data["columns_requested"]
|
||||
assert "slug" in data["columns_requested"]
|
||||
assert "uuid" in data["columns_loaded"]
|
||||
assert "changed_on_humanized" in data["columns_requested"]
|
||||
assert "url" in data["columns_loaded"]
|
||||
assert "slug" in data["columns_loaded"]
|
||||
|
||||
|
||||
@@ -529,13 +529,14 @@ class TestDashboardDefaultColumnFiltering:
|
||||
DASHBOARD_DEFAULT_COLUMNS,
|
||||
)
|
||||
|
||||
# Should have exactly 4 minimal columns
|
||||
assert len(DASHBOARD_DEFAULT_COLUMNS) == 4
|
||||
# Should have exactly 5 minimal columns
|
||||
assert len(DASHBOARD_DEFAULT_COLUMNS) == 5
|
||||
assert set(DASHBOARD_DEFAULT_COLUMNS) == {
|
||||
"id",
|
||||
"dashboard_title",
|
||||
"slug",
|
||||
"uuid",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
|
||||
# Heavy columns should NOT be in defaults
|
||||
@@ -543,6 +544,7 @@ class TestDashboardDefaultColumnFiltering:
|
||||
assert "published" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "json_metadata" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "position_json" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "uuid" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
|
||||
def test_empty_select_columns_default(self):
|
||||
"""Test that select_columns defaults to empty list which triggers
|
||||
@@ -604,7 +606,8 @@ class TestDashboardDefaultColumnFiltering:
|
||||
assert "id" in data["columns_requested"]
|
||||
assert "dashboard_title" in data["columns_requested"]
|
||||
assert "slug" in data["columns_requested"]
|
||||
assert "uuid" in data["columns_requested"]
|
||||
assert "url" in data["columns_requested"]
|
||||
assert "changed_on_humanized" in data["columns_requested"]
|
||||
|
||||
# Verify heavy columns are NOT in columns_loaded by default
|
||||
assert "json_metadata" not in data["columns_loaded"]
|
||||
|
||||
@@ -194,13 +194,13 @@ async def test_list_datasets_basic(mock_list, mcp_server):
|
||||
assert len(data["datasets"]) == 1
|
||||
assert data["datasets"][0]["id"] == 1
|
||||
assert data["datasets"][0]["table_name"] == "Test DatasetInfo"
|
||||
assert data["datasets"][0]["uuid"] == "test-dataset-uuid-1"
|
||||
# Note: columns and metrics are not in minimal default columns
|
||||
# (id, table_name, schema, uuid). Use select_columns to include them.
|
||||
# (id, table_name, schema, changed_on_humanized). Use select_columns
|
||||
# to include them.
|
||||
|
||||
# Verify UUID is in default columns (datasets don't have slugs)
|
||||
assert "uuid" in data["columns_requested"]
|
||||
assert "uuid" in data["columns_loaded"]
|
||||
# Verify changed_on_humanized is in default columns
|
||||
assert "changed_on_humanized" in data["columns_requested"]
|
||||
assert "changed_on_humanized" in data["columns_loaded"]
|
||||
|
||||
|
||||
@patch("superset.daos.dataset.DatasetDAO.list")
|
||||
@@ -1069,7 +1069,8 @@ async def test_list_datasets_includes_columns_and_metrics(mock_list, mcp_server)
|
||||
"""Test that columns and metrics are included when explicitly requested.
|
||||
|
||||
Note: columns and metrics are not in minimal default columns
|
||||
(id, table_name, schema, uuid). Use select_columns to include them.
|
||||
(id, table_name, schema, changed_on_humanized). Use select_columns to
|
||||
include them.
|
||||
"""
|
||||
dataset = MagicMock()
|
||||
dataset.id = 11
|
||||
@@ -1198,7 +1199,13 @@ class TestDatasetDefaultColumnFiltering:
|
||||
|
||||
# Should have exactly 4 minimal columns
|
||||
assert len(DATASET_DEFAULT_COLUMNS) == 4
|
||||
assert set(DATASET_DEFAULT_COLUMNS) == {"id", "table_name", "schema", "uuid"}
|
||||
assert set(DATASET_DEFAULT_COLUMNS) == {
|
||||
"id",
|
||||
"table_name",
|
||||
"schema",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert "uuid" not in DATASET_DEFAULT_COLUMNS
|
||||
|
||||
# Heavy columns should NOT be in defaults
|
||||
assert "columns" not in DATASET_DEFAULT_COLUMNS
|
||||
@@ -1229,7 +1236,7 @@ class TestDatasetDefaultColumnFiltering:
|
||||
"id",
|
||||
"table_name",
|
||||
"schema",
|
||||
"uuid",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
|
||||
# Verify heavy columns are NOT in columns_loaded
|
||||
@@ -1312,7 +1319,7 @@ class TestDatasetDefaultColumnFiltering:
|
||||
dataset_item = data["datasets"][0]
|
||||
|
||||
# Verify ONLY default columns are present in the response item
|
||||
expected_keys = {"id", "table_name", "schema_name", "uuid"}
|
||||
expected_keys = {"id", "table_name", "schema_name", "changed_on_humanized"}
|
||||
actual_keys = set(dataset_item.keys())
|
||||
|
||||
# The response should only contain the default columns, NOT all columns
|
||||
|
||||
@@ -116,34 +116,55 @@ class TestModelSchemaInfo:
|
||||
assert info.model_type == "chart"
|
||||
assert len(info.select_columns) > 0
|
||||
# Check default columns include required minimal set
|
||||
required_columns = {"id", "slice_name", "viz_type", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"slice_name",
|
||||
"viz_type",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(info.default_select))
|
||||
assert "slice_name" in info.filter_columns
|
||||
assert info.default_sort == "changed_on"
|
||||
|
||||
def test_dataset_default_columns(self):
|
||||
"""Test dataset default columns include required minimal set."""
|
||||
required_columns = {"id", "table_name", "schema", "uuid"}
|
||||
required_columns = {"id", "table_name", "schema", "changed_on_humanized"}
|
||||
assert required_columns.issubset(set(DATASET_DEFAULT_COLUMNS))
|
||||
# These should NOT be in defaults
|
||||
assert "columns" not in DATASET_DEFAULT_COLUMNS
|
||||
assert "metrics" not in DATASET_DEFAULT_COLUMNS
|
||||
assert "uuid" not in DATASET_DEFAULT_COLUMNS
|
||||
|
||||
def test_dashboard_default_columns(self):
|
||||
"""Test dashboard default columns include required minimal set."""
|
||||
required_columns = {"id", "dashboard_title", "slug", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"dashboard_title",
|
||||
"slug",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(DASHBOARD_DEFAULT_COLUMNS))
|
||||
# These should NOT be in defaults
|
||||
assert "published" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "charts" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "uuid" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
|
||||
def test_chart_default_columns(self):
|
||||
"""Test chart default columns include required minimal set."""
|
||||
required_columns = {"id", "slice_name", "viz_type", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"slice_name",
|
||||
"viz_type",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(CHART_DEFAULT_COLUMNS))
|
||||
# These should NOT be in defaults
|
||||
assert "description" not in CHART_DEFAULT_COLUMNS
|
||||
assert "form_data" not in CHART_DEFAULT_COLUMNS
|
||||
assert "uuid" not in CHART_DEFAULT_COLUMNS
|
||||
|
||||
|
||||
class TestGetSchemaToolViaClient:
|
||||
@@ -177,7 +198,13 @@ class TestGetSchemaToolViaClient:
|
||||
assert "search_columns" in info
|
||||
|
||||
# Check default columns include required minimal set
|
||||
required_columns = {"id", "slice_name", "viz_type", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"slice_name",
|
||||
"viz_type",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(info["default_select"]))
|
||||
|
||||
@patch("superset.daos.dataset.DatasetDAO.get_filterable_columns_and_operators")
|
||||
@@ -201,7 +228,7 @@ class TestGetSchemaToolViaClient:
|
||||
assert info["model_type"] == "dataset"
|
||||
|
||||
# Check default columns include required minimal set
|
||||
required_columns = {"id", "table_name", "schema", "uuid"}
|
||||
required_columns = {"id", "table_name", "schema", "changed_on_humanized"}
|
||||
assert required_columns.issubset(set(info["default_select"]))
|
||||
|
||||
# Check search columns
|
||||
@@ -229,7 +256,13 @@ class TestGetSchemaToolViaClient:
|
||||
assert info["model_type"] == "dashboard"
|
||||
|
||||
# Check default columns include required minimal set
|
||||
required_columns = {"id", "dashboard_title", "slug", "uuid"}
|
||||
required_columns = {
|
||||
"id",
|
||||
"dashboard_title",
|
||||
"slug",
|
||||
"url",
|
||||
"changed_on_humanized",
|
||||
}
|
||||
assert required_columns.issubset(set(info["default_select"]))
|
||||
|
||||
# Check sortable columns include expected values
|
||||
|
||||
Reference in New Issue
Block a user