This is an automated email from the ASF dual-hosted git repository. aminghadersohi pushed a commit to branch mcp-rls-plugins-99978 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2ebb3bcd9f750fe5734dda8af22258abd8a4a30c Author: Mehmet Salih Yavuz <[email protected]> AuthorDate: Thu May 21 17:48:21 2026 +0300 feat(mcp): include applied dashboard filters in get_chart_info (#39620) Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> --- superset/mcp_service/chart/chart_helpers.py | 120 ++++++++++ superset/mcp_service/chart/schemas.py | 50 +++++ superset/mcp_service/chart/tool/get_chart_info.py | 105 +++++++-- .../mcp_service/chart/tool/test_get_chart_info.py | 243 ++++++++++++++++++++- 4 files changed, 498 insertions(+), 20 deletions(-) diff --git a/superset/mcp_service/chart/chart_helpers.py b/superset/mcp_service/chart/chart_helpers.py index fca0f1087a2..f64e851408c 100644 --- a/superset/mcp_service/chart/chart_helpers.py +++ b/superset/mcp_service/chart/chart_helpers.py @@ -32,6 +32,7 @@ from urllib.parse import parse_qs, urlparse from superset.constants import EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS if TYPE_CHECKING: + from superset.mcp_service.chart.schemas import AppliedDashboardFilter from superset.models.slice import Slice logger = logging.getLogger(__name__) @@ -44,6 +45,10 @@ QUERY_CONTEXT_EXTRA_FORM_DATA_OVERRIDE_KEYS = { } +class ChartNotOnDashboardError(ValueError): + """Raised when a chart is not part of the given dashboard's slices.""" + + def find_chart_by_identifier( identifier: int | str, query_options: list[Any] | None = None, @@ -537,3 +542,118 @@ def extract_form_data_key_from_url(url: str | None) -> str | None: parsed = urlparse(url) values = parse_qs(parsed.query).get("form_data_key", []) return values[0] if values else None + + +def _match_adhoc_by_subject( + adhoc_filters: Any, column: str | None +) -> tuple[str | None, Any] | None: + if not column or not isinstance(adhoc_filters, list): + return None + for af in adhoc_filters: + if isinstance(af, dict) and af.get("subject") == column: + return af.get("operator"), af.get("comparator") + return None + + +def _match_legacy_by_col( + legacy_filters: Any, column: str | None +) -> tuple[str | None, Any] | None: + if not column or not isinstance(legacy_filters, list): + return None + for f in legacy_filters: + if isinstance(f, dict) and f.get("col") == column: + return f.get("op"), f.get("val") + return None + + +def _resolve_filter_operator_and_value( + extra_form_data: dict[str, Any] | None, + column: str | None, +) -> tuple[str | None, Any]: + """Pull operator and value for a dashboard filter from its + default extra_form_data, matching on target column where applicable.""" + if not extra_form_data: + return None, None + + if match := _match_adhoc_by_subject(extra_form_data.get("adhoc_filters"), column): + return match + if match := _match_legacy_by_col(extra_form_data.get("filters"), column): + return match + # Temporal filters contribute time_range with no target column + if time_range := extra_form_data.get("time_range"): + return "TIME_RANGE", time_range + return None, None + + +def build_applied_dashboard_filters( + dashboard_id: int, chart_id: int +) -> list[AppliedDashboardFilter]: + """Resolve dashboard-level native filters in scope for a chart. + + Validates that the dashboard exists, the caller has access, and the chart + is on the dashboard. Returns one AppliedDashboardFilter per non-DIVIDER + native filter whose scope includes the chart, populated with the filter's + default operator and value. + + Raises DashboardNotFoundError if the dashboard is missing, + ChartNotOnDashboardError if the chart is not on it, and + SupersetSecurityException if the caller cannot access the dashboard. + """ + # Local imports avoid circular deps at module load + from superset import db, security_manager + from superset.charts.data.dashboard_filter_context import ( + _extract_filter_extra_form_data, + _get_filter_target_column, + _is_filter_in_scope_for_chart, + ) + from superset.commands.dashboard.exceptions import DashboardNotFoundError + from superset.mcp_service.chart.schemas import AppliedDashboardFilter + from superset.models.dashboard import Dashboard + from superset.utils import json + + dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one_or_none() + if not dashboard: + raise DashboardNotFoundError(dashboard_id=str(dashboard_id)) + + security_manager.raise_for_access(dashboard=dashboard) + + slice_ids = {slc.id for slc in dashboard.slices} + if chart_id not in slice_ids: + raise ChartNotOnDashboardError( + f"Chart {chart_id} is not on dashboard {dashboard_id}" + ) + + metadata = json.loads(dashboard.json_metadata or "{}") + native_filter_config = metadata.get("native_filter_configuration", []) + if not isinstance(native_filter_config, list): + return [] + position_json = json.loads(dashboard.position_json or "{}") + if not isinstance(position_json, dict): + position_json = {} + + applied: list[AppliedDashboardFilter] = [] + for flt in native_filter_config: + if not isinstance(flt, dict): + continue + if flt.get("type", "") == "DIVIDER": + continue + if not _is_filter_in_scope_for_chart(flt, chart_id, position_json): + continue + + extra_form_data, status = _extract_filter_extra_form_data(flt) + column = _get_filter_target_column(flt) + operator, value = _resolve_filter_operator_and_value(extra_form_data, column) + + applied.append( + AppliedDashboardFilter( + id=flt.get("id"), + name=flt.get("name"), + filter_type=flt.get("filterType"), + column=column, + operator=operator, + value=value, + status=status.value, + ) + ) + + return applied diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py index 81ddd56a540..6e9e67d8722 100644 --- a/superset/mcp_service/chart/schemas.py +++ b/superset/mcp_service/chart/schemas.py @@ -279,6 +279,15 @@ class GetChartInfoRequest(BaseModel): "Can be used alone (without identifier) for unsaved charts." ), ) + dashboard_id: int | None = Field( + default=None, + description=( + "When provided, resolves dashboard-level native filters that are in " + "scope for this chart on the given dashboard and returns them under " + "filters.dashboard_filters. Requires the chart to be on the dashboard " + "and the caller to have dashboard access." + ), + ) @model_validator(mode="after") def validate_identifier_or_form_data_key(self) -> "GetChartInfoRequest": @@ -2192,6 +2201,38 @@ class AdhocFilter(BaseModel): model_config = ConfigDict(extra="ignore") +class AppliedDashboardFilter(BaseModel): + """A dashboard-level native filter resolved against a specific chart. + + Returned when get_chart_info is called with a dashboard_id. Values come + from the filter's default state on the saved dashboard (not a permalink). + """ + + id: str | None = Field(None, description="Native filter ID") + name: str | None = Field(None, description="Filter display name") + filter_type: str | None = Field( + None, description="Native filter type (e.g. filter_select, filter_range)" + ) + column: str | None = Field(None, description="Target column the filter applies to") + operator: str | None = Field( + None, + description=( + "Filter operator as stored in extra_form_data (e.g. 'IN', '==', 'LIKE', " + "or 'TIME_RANGE' for temporal filters with no target column)" + ), + ) + value: Any | None = Field( + None, description="Filter value(s) from the default data mask" + ) + status: str = Field( + ..., + description=( + "Whether the filter contributes to the chart query: 'applied', " + "'not_applied', or 'not_applied_uses_default_to_first_item_prequery'" + ), + ) + + class ChartFiltersInfo(BaseModel): """Structured representation of all filters applied to a chart.""" @@ -2233,6 +2274,15 @@ class ChartFiltersInfo(BaseModel): None, description="Custom HAVING clause applied to the chart query", ) + dashboard_filters: List[AppliedDashboardFilter] = Field( + default_factory=list, + description=( + "Dashboard-level native filters in scope for this chart on the " + "dashboard passed via get_chart_info's dashboard_id argument. Empty " + "when no dashboard_id was provided or no native filter targets this " + "chart." + ), + ) # Rebuild ChartInfo so Pydantic can resolve the ChartFiltersInfo forward reference. diff --git a/superset/mcp_service/chart/tool/get_chart_info.py b/superset/mcp_service/chart/tool/get_chart_info.py index 79ae03da41c..6dcbae7d7c0 100644 --- a/superset/mcp_service/chart/tool/get_chart_info.py +++ b/superset/mcp_service/chart/tool/get_chart_info.py @@ -25,12 +25,19 @@ from fastmcp import Context from sqlalchemy.orm import subqueryload from superset_core.mcp.decorators import tool, ToolAnnotations +from superset.commands.dashboard.exceptions import DashboardNotFoundError +from superset.exceptions import SupersetSecurityException from superset.extensions import event_logger -from superset.mcp_service.chart.chart_helpers import get_cached_form_data +from superset.mcp_service.chart.chart_helpers import ( + build_applied_dashboard_filters, + ChartNotOnDashboardError, + get_cached_form_data, +) from superset.mcp_service.chart.chart_utils import validate_chart_dataset from superset.mcp_service.chart.schemas import ( CHART_FORM_DATA_EXCLUDED_FIELD_NAMES, ChartError, + ChartFiltersInfo, ChartInfo, extract_filters_from_form_data, GetChartInfoRequest, @@ -89,6 +96,66 @@ FORM_DATA_OVERRIDE_EXCLUDED_FIELD_NAMES = ( ) +async def _validate_chart_dataset_access( + result: ChartInfo, ctx: Context +) -> ChartError | None: + """Validate that the chart's dataset is accessible to the current user. + + Returns a ChartError if the dataset is not accessible, otherwise None. + Logs any non-fatal warnings (e.g., virtual dataset warnings) via ctx. + """ + from superset.daos.chart import ChartDAO + + if not result.id: + return None + chart = ChartDAO.find_by_id(result.id) + if not chart: + return None + validation_result = validate_chart_dataset(chart, check_access=True) + if not validation_result.is_valid: + await ctx.warning( + "Chart found but dataset is not accessible: %s" % (validation_result.error,) + ) + return ChartError( + error=validation_result.error or "Chart's dataset is not accessible", + error_type="DatasetNotAccessible", + ) + for warning in validation_result.warnings: + await ctx.warning("Dataset warning: %s" % (warning,)) + return None + + +async def _attach_dashboard_filters( + result: ChartInfo, dashboard_id: int, ctx: Context +) -> ChartError | None: + """Resolve dashboard-scoped native filters and attach them to result.filters. + + Returns a ChartError to surface to the caller on validation / access + failures, or None on success (including the no-filters case). + """ + if not result.id: + return None + with event_logger.log_context(action="mcp.get_chart_info.dashboard_filters"): + try: + dashboard_filters = build_applied_dashboard_filters(dashboard_id, result.id) + except DashboardNotFoundError as exc: + await ctx.warning("Dashboard not found: %s" % (str(exc),)) + return ChartError(error=str(exc), error_type="DashboardNotFound") + except ChartNotOnDashboardError as exc: + await ctx.warning("Chart not on dashboard: %s" % (str(exc),)) + return ChartError(error=str(exc), error_type="ChartNotOnDashboard") + except SupersetSecurityException as exc: + await ctx.warning("Dashboard not accessible: %s" % (str(exc),)) + return ChartError(error=str(exc), error_type="DashboardNotAccessible") + + if dashboard_filters: + if result.filters is None: + result.filters = ChartFiltersInfo(dashboard_filters=dashboard_filters) + else: + result.filters.dashboard_filters = dashboard_filters + return None + + def _apply_unsaved_state_override(result: ChartInfo, form_data_key: str) -> None: """Override a ChartInfo's form_data with cached unsaved state.""" from superset.utils import json as utils_json @@ -178,6 +245,17 @@ async def get_chart_info( } ``` + With dashboard context to resolve applied dashboard-level filters: + ```json + { + "identifier": 123, + "dashboard_id": 45 + } + ``` + When dashboard_id is provided, the response's filters.dashboard_filters + lists native filters (with column, operator, and value) that are in scope + for this chart on that dashboard. + Returns chart details including name, type, and URL. """ from superset.daos.chart import ChartDAO @@ -247,23 +325,14 @@ async def get_chart_info( ) # Validate the chart's dataset is accessible - if result.id: - chart = ChartDAO.find_by_id(result.id) - if chart: - validation_result = validate_chart_dataset(chart, check_access=True) - if not validation_result.is_valid: - await ctx.warning( - "Chart found but dataset is not accessible: %s" - % (validation_result.error,) - ) - return ChartError( - error=validation_result.error - or "Chart's dataset is not accessible", - error_type="DatasetNotAccessible", - ) - # Log any warnings (e.g., virtual dataset warnings) - for warning in validation_result.warnings: - await ctx.warning("Dataset warning: %s" % (warning,)) + dataset_error = await _validate_chart_dataset_access(result, ctx) + if dataset_error is not None: + return dataset_error + + if request.dashboard_id: + error = await _attach_dashboard_filters(result, request.dashboard_id, ctx) + if error is not None: + return error else: await ctx.warning("Chart retrieval failed: error=%s" % (str(result),)) diff --git a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py index 3518cbb9ea7..7459ab46bb0 100644 --- a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py +++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py @@ -16,18 +16,25 @@ # under the License. """ -Unit tests for get_chart_info MCP tool privacy behavior. +Unit tests for get_chart_info MCP tool: dashboard-filter resolution and +privacy behavior. """ import importlib from contextlib import nullcontext from types import SimpleNamespace -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from fastmcp import Client +from superset.commands.dashboard.exceptions import DashboardNotFoundError from superset.mcp_service.app import mcp +from superset.mcp_service.chart.chart_helpers import ( + _resolve_filter_operator_and_value, + build_applied_dashboard_filters, + ChartNotOnDashboardError, +) from superset.mcp_service.chart.schemas import ( ChartInfo, extract_filters_from_form_data, @@ -84,6 +91,238 @@ def _make_chart_info() -> ChartInfo: ) +class TestGetChartInfoRequestSchema: + def test_dashboard_id_optional(self): + request = GetChartInfoRequest(identifier=1) + assert request.dashboard_id is None + + def test_dashboard_id_accepted(self): + request = GetChartInfoRequest(identifier=1, dashboard_id=42) + assert request.dashboard_id == 42 + + +class TestResolveFilterOperatorAndValue: + def test_matches_adhoc_filter_by_subject(self): + efd = { + "adhoc_filters": [ + { + "subject": "country", + "operator": "IN", + "comparator": ["US", "CA"], + } + ] + } + assert _resolve_filter_operator_and_value(efd, "country") == ( + "IN", + ["US", "CA"], + ) + + def test_matches_legacy_filter_by_col(self): + efd = {"filters": [{"col": "state", "op": "==", "val": "NY"}]} + assert _resolve_filter_operator_and_value(efd, "state") == ("==", "NY") + + def test_time_range_when_no_column(self): + efd = {"time_range": "Last 7 days"} + assert _resolve_filter_operator_and_value(efd, None) == ( + "TIME_RANGE", + "Last 7 days", + ) + + def test_column_not_in_extra_form_data(self): + efd = { + "adhoc_filters": [{"subject": "other", "operator": "==", "comparator": 1}] + } + assert _resolve_filter_operator_and_value(efd, "country") == (None, None) + + def test_none_extra_form_data(self): + assert _resolve_filter_operator_and_value(None, "country") == (None, None) + + def test_ignores_non_dict_entries(self): + efd = { + "adhoc_filters": ["not-a-dict", None], + "filters": [42, "foo"], + } + assert _resolve_filter_operator_and_value(efd, "country") == (None, None) + + +class TestBuildAppliedDashboardFilters: + """The helper validates access, checks chart-on-dashboard, iterates + native filters, resolves scope, and maps each to AppliedDashboardFilter.""" + + def _make_dashboard(self, json_metadata=None, position_json=None, slice_ids=None): + dashboard = MagicMock() + dashboard.json_metadata = json_metadata or "{}" + dashboard.position_json = position_json or "{}" + dashboard.slices = [MagicMock(id=sid) for sid in (slice_ids or [])] + return dashboard + + def test_chart_not_on_dashboard_raises(self): + dashboard = self._make_dashboard(slice_ids=[2, 3]) + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + with pytest.raises(ChartNotOnDashboardError, match="not on dashboard"): + build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + def test_dashboard_not_found_raises(self): + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = None # noqa: E501 + with pytest.raises(DashboardNotFoundError): + build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + def test_in_scope_filter_with_static_default(self): + native_filter = { + "id": "NATIVE_FILTER-1", + "name": "Country", + "type": "NATIVE_FILTER", + "filterType": "filter_select", + "chartsInScope": [1], + "targets": [{"column": {"name": "country"}, "datasetId": 7}], + "defaultDataMask": { + "filterState": {"value": ["US"]}, + "extraFormData": { + "adhoc_filters": [ + { + "subject": "country", + "operator": "IN", + "comparator": ["US"], + } + ] + }, + }, + } + dashboard = self._make_dashboard( + json_metadata='{"native_filter_configuration": %s}' + % _json(native_filter_list=[native_filter]), + slice_ids=[1], + ) + + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + result = build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + assert len(result) == 1 + flt = result[0] + assert flt.id == "NATIVE_FILTER-1" + assert flt.name == "Country" + assert flt.filter_type == "filter_select" + assert flt.column == "country" + assert flt.operator == "IN" + assert flt.value == ["US"] + assert flt.status == "applied" + + def test_excluded_chart_filter_skipped(self): + native_filter = { + "id": "NATIVE_FILTER-1", + "name": "Region", + "type": "NATIVE_FILTER", + "filterType": "filter_select", + "chartsInScope": [2, 3], # chart 1 excluded + "targets": [{"column": {"name": "region"}, "datasetId": 7}], + "defaultDataMask": { + "filterState": {"value": ["NA"]}, + "extraFormData": { + "filters": [{"col": "region", "op": "==", "val": "NA"}] + }, + }, + } + dashboard = self._make_dashboard( + json_metadata='{"native_filter_configuration": %s}' + % _json(native_filter_list=[native_filter]), + slice_ids=[1], + ) + + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + result = build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + assert result == [] + + def test_default_to_first_item_marks_prequery(self): + native_filter = { + "id": "NATIVE_FILTER-1", + "name": "Region", + "type": "NATIVE_FILTER", + "filterType": "filter_select", + "chartsInScope": [1], + "targets": [{"column": {"name": "region"}, "datasetId": 7}], + "controlValues": {"defaultToFirstItem": True}, + "defaultDataMask": {}, + } + dashboard = self._make_dashboard( + json_metadata='{"native_filter_configuration": %s}' + % _json(native_filter_list=[native_filter]), + slice_ids=[1], + ) + + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + result = build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + assert len(result) == 1 + assert result[0].status == "not_applied_uses_default_to_first_item_prequery" + assert result[0].operator is None + assert result[0].value is None + + def test_divider_entry_skipped(self): + divider = { + "id": "DIVIDER-1", + "name": "Section header", + "type": "DIVIDER", + } + dashboard = self._make_dashboard( + json_metadata='{"native_filter_configuration": %s}' + % _json(native_filter_list=[divider]), + slice_ids=[1], + ) + + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + result = build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + assert result == [] + + def test_no_native_filters_returns_empty_list(self): + dashboard = self._make_dashboard( + json_metadata="{}", + slice_ids=[1], + ) + + with ( + patch("superset.db") as mock_db, + patch("superset.security_manager"), + ): + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value = dashboard # noqa: E501 + result = build_applied_dashboard_filters(dashboard_id=10, chart_id=1) + + assert result == [] + + +def _json(native_filter_list): + """Serialize a native_filter list as JSON string for embedding in + json_metadata fixtures without escaping issues.""" + from superset.utils import json + + return json.dumps(native_filter_list) + + class TestGetChartInfoPrivacy: @pytest.mark.asyncio async def test_restricted_user_redacts_saved_chart_data_model_fields(
