From 5f5595e8dfd92efbd2997c8985e4e9502eeb17b8 Mon Sep 17 00:00:00 2001 From: Hugh A Miles II Date: Sat, 13 Jun 2026 09:54:38 -0700 Subject: [PATCH] feat(charts): apply active_data_mask in dashboard filter context Extend get_dashboard_filter_context with an optional active_data_mask kwarg so callers can resolve each in-scope native filter against live dashboard filter state instead of saved defaults. A non-empty active extraFormData is applied, an empty one clears the filter (no fallback to default), and filters absent from the mask keep their defaults. Backward compatible: omitting the kwarg reproduces initial-load behavior. Adds unit tests covering override, clear, fallback, defaultToFirstItem, and out-of-scope cases. Co-Authored-By: Claude Opus 4.8 --- .../charts/data/dashboard_filter_context.py | 39 +++- .../charts/test_dashboard_filter_context.py | 172 ++++++++++++++++++ 2 files changed, 209 insertions(+), 2 deletions(-) diff --git a/superset/charts/data/dashboard_filter_context.py b/superset/charts/data/dashboard_filter_context.py index cab9bbc45a1..d610c289d17 100644 --- a/superset/charts/data/dashboard_filter_context.py +++ b/superset/charts/data/dashboard_filter_context.py @@ -209,6 +209,31 @@ def _extract_filter_extra_form_data( return None, DashboardFilterStatus.NOT_APPLIED +def _resolve_filter_extra_form_data( + filter_config: dict[str, Any], + active_data_mask: dict[str, Any] | None, +) -> tuple[dict[str, Any] | None, DashboardFilterStatus]: + """ + Resolve a filter's extra_form_data and status, preferring an active value + from ``active_data_mask`` over the filter's saved default. + + When ``active_data_mask`` provides an entry for this filter, its + ``extraFormData`` is authoritative: a non-empty value is APPLIED, while an + empty value means the user explicitly cleared the filter (NOT_APPLIED, with + no fallback to the saved default). When no active entry exists, fall back to + the saved-default behavior in ``_extract_filter_extra_form_data``. + + Returns (extra_form_data, status). + """ + flt_id = filter_config.get("id", "") + if active_data_mask is not None and flt_id in active_data_mask: + active_efd = (active_data_mask[flt_id] or {}).get("extraFormData") or {} + if active_efd: + return active_efd, DashboardFilterStatus.APPLIED + return None, DashboardFilterStatus.NOT_APPLIED + return _extract_filter_extra_form_data(filter_config) + + def _get_filter_target_column(filter_config: dict[str, Any]) -> str | None: """Extract the target column name from a native filter configuration.""" if targets := filter_config.get("targets", []): @@ -254,16 +279,26 @@ def _check_dashboard_access(dashboard: Dashboard) -> None: def get_dashboard_filter_context( dashboard_id: int, chart_id: int, + *, + active_data_mask: dict[str, Any] | None = None, ) -> DashboardFilterContext: """ Build a DashboardFilterContext for a chart on a dashboard. Loads the dashboard's native filter configuration, determines which - filters are in scope for the given chart, extracts default filter values, + filters are in scope for the given chart, resolves each filter's value, and returns the merged extra_form_data along with metadata about each filter. + When ``active_data_mask`` is provided (e.g. the live filter state from a + dashboard view), each in-scope filter present in the mask uses its active + ``extraFormData`` instead of the saved default; an empty active value means + the filter was cleared. Filters absent from the mask fall back to their + saved defaults, so omitting ``active_data_mask`` reproduces the dashboard's + initial-load behavior. + :param dashboard_id: The ID of the dashboard :param chart_id: The ID of the chart + :param active_data_mask: Optional live filter state keyed by native filter id :returns: DashboardFilterContext with merged extra_form_data and filter metadata :raises ValueError: if dashboard not found or chart not on dashboard :raises SupersetSecurityException: if the user cannot access the dashboard @@ -297,7 +332,7 @@ def get_dashboard_filter_context( flt_id = flt.get("id", "") flt_name = flt.get("name", "") target_column = _get_filter_target_column(flt) - extra_form_data, status = _extract_filter_extra_form_data(flt) + extra_form_data, status = _resolve_filter_extra_form_data(flt, active_data_mask) if extra_form_data and status == DashboardFilterStatus.APPLIED: context.extra_form_data = _merge_extra_form_data( diff --git a/tests/unit_tests/charts/test_dashboard_filter_context.py b/tests/unit_tests/charts/test_dashboard_filter_context.py index b91aaaba467..b571ef89a65 100644 --- a/tests/unit_tests/charts/test_dashboard_filter_context.py +++ b/tests/unit_tests/charts/test_dashboard_filter_context.py @@ -601,6 +601,178 @@ def test_get_dashboard_filter_context_out_of_scope_filter_excluded( assert ctx.filters[0].id == "f1" +def _build_dashboard_mock( + mock_db: MagicMock, + filter_config: list[dict[str, Any]], + chart_ids: list[int], +) -> MagicMock: + """Wire a dashboard MagicMock with the given filters and chart ids.""" + metadata = {"native_filter_configuration": filter_config} + dashboard = MagicMock() + dashboard.id = 1 + dashboard.slices = [MagicMock(id=cid) for cid in chart_ids] + dashboard.json_metadata = json.dumps(metadata) + dashboard.position_json = json.dumps(SAMPLE_POSITION_JSON) + ( + mock_db.session.query.return_value.filter_by.return_value.one_or_none.return_value + ) = dashboard + return dashboard + + +@patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") +@patch("superset.charts.data.dashboard_filter_context.db") +def test_active_data_mask_overrides_default( + mock_db: MagicMock, + mock_check_access: MagicMock, +) -> None: + """An active filter value replaces the saved default.""" + filter_config = [ + _make_filter( + flt_id="f1", + name="Region", + scope_root=["ROOT_ID"], + default_value=["US"], + target_column="region", + ), + ] + _build_dashboard_mock(mock_db, filter_config, [10]) + + active_data_mask = { + "f1": { + "extraFormData": { + "filters": [{"col": "region", "op": "IN", "val": ["FR", "DE"]}] + } + } + } + ctx = get_dashboard_filter_context( + dashboard_id=1, chart_id=10, active_data_mask=active_data_mask + ) + + assert ctx.filters[0].status == DashboardFilterStatus.APPLIED + assert ctx.extra_form_data["filters"][0]["val"] == ["FR", "DE"] + + +@patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") +@patch("superset.charts.data.dashboard_filter_context.db") +def test_active_data_mask_empty_clears_default( + mock_db: MagicMock, + mock_check_access: MagicMock, +) -> None: + """An empty active extraFormData clears the filter; the default is NOT used.""" + filter_config = [ + _make_filter( + flt_id="f1", + name="Region", + scope_root=["ROOT_ID"], + default_value=["US"], + target_column="region", + ), + ] + _build_dashboard_mock(mock_db, filter_config, [10]) + + active_data_mask = {"f1": {"extraFormData": {}}} + ctx = get_dashboard_filter_context( + dashboard_id=1, chart_id=10, active_data_mask=active_data_mask + ) + + assert ctx.filters[0].status == DashboardFilterStatus.NOT_APPLIED + assert "filters" not in ctx.extra_form_data + + +@patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") +@patch("superset.charts.data.dashboard_filter_context.db") +def test_active_data_mask_absent_filter_falls_back_to_default( + mock_db: MagicMock, + mock_check_access: MagicMock, +) -> None: + """A filter not present in the mask keeps its saved default.""" + filter_config = [ + _make_filter( + flt_id="f1", + name="Region", + scope_root=["ROOT_ID"], + default_value=["US"], + target_column="region", + ), + ] + _build_dashboard_mock(mock_db, filter_config, [10]) + + # Mask only references some other filter id + active_data_mask = {"f2": {"extraFormData": {"filters": []}}} + ctx = get_dashboard_filter_context( + dashboard_id=1, chart_id=10, active_data_mask=active_data_mask + ) + + assert ctx.filters[0].status == DashboardFilterStatus.APPLIED + assert ctx.extra_form_data["filters"][0]["val"] == ["US"] + + +@patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") +@patch("superset.charts.data.dashboard_filter_context.db") +def test_active_data_mask_applies_despite_default_to_first_item( + mock_db: MagicMock, + mock_check_access: MagicMock, +) -> None: + """ + defaultToFirstItem filters cannot be resolved from saved config, but when the + frontend supplies a concrete active value it is applied. + """ + filter_config = [ + _make_filter( + flt_id="f1", + name="City", + scope_root=["ROOT_ID"], + default_to_first_item=True, + target_column="city", + ), + ] + _build_dashboard_mock(mock_db, filter_config, [10]) + + active_data_mask = { + "f1": { + "extraFormData": {"filters": [{"col": "city", "op": "IN", "val": ["NYC"]}]} + } + } + ctx = get_dashboard_filter_context( + dashboard_id=1, chart_id=10, active_data_mask=active_data_mask + ) + + assert ctx.filters[0].status == DashboardFilterStatus.APPLIED + assert ctx.extra_form_data["filters"][0]["val"] == ["NYC"] + + +@patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") +@patch("superset.charts.data.dashboard_filter_context.db") +def test_active_data_mask_out_of_scope_filter_still_excluded( + mock_db: MagicMock, + mock_check_access: MagicMock, +) -> None: + """An active value for an out-of-scope filter does not leak into the chart.""" + filter_config = [ + _make_filter( + flt_id="f1", + name="Out-of-scope", + scope_root=["TABS-nonexistent"], + target_column="status", + ), + ] + _build_dashboard_mock(mock_db, filter_config, [10]) + + active_data_mask = { + "f1": { + "extraFormData": { + "filters": [{"col": "status", "op": "IN", "val": ["active"]}] + } + } + } + ctx = get_dashboard_filter_context( + dashboard_id=1, chart_id=10, active_data_mask=active_data_mask + ) + + assert ctx.filters == [] + assert ctx.extra_form_data == {} + + @patch("superset.charts.data.dashboard_filter_context._check_dashboard_access") @patch("superset.charts.data.dashboard_filter_context.db") def test_get_dashboard_filter_context_chart_not_in_layout_receives_root_filters(