MDeadman commented on code in PR #38638:
URL: https://github.com/apache/superset/pull/38638#discussion_r2932924874
##########
docs/static/resources/openapi.json:
##########
@@ -1739,6 +1739,45 @@
"$ref": "#/components/schemas/ChartDataResponseResult"
},
"type": "array"
+ },
Review Comment:
So I was not sure the best way to generate the updated OpenAPI docs. I found
a command in the Superset CLI, but when I ran that, it added my new changes,
but is also deleted a bunch of un-related stuff which made me nervous, so I
just went ahead and manually updated the openapi.json file here.
Please let me know if there is a better approach!
##########
superset/charts/data/api.py:
##########
@@ -156,6 +180,53 @@ def get_data( # noqa: C901
json_body["result_type"] = request.args.get("type",
ChartDataResultType.FULL)
json_body["force"] = request.args.get("force")
+ # Apply dashboard filter context when filters_dashboard_id is provided
+ dashboard_filter_context: DashboardFilterContext | None = None
+ filters_dashboard_id = request.args.get("filters_dashboard_id",
type=int)
+ if filters_dashboard_id is not None:
+ try:
+ dashboard_filter_context = get_dashboard_filter_context(
+ dashboard_id=filters_dashboard_id,
+ chart_id=pk,
+ )
+ except ValueError as error:
+ return self.response_400(message=str(error))
+ except SupersetSecurityException:
+ return self.response_403()
+
+ if dashboard_filter_context.extra_form_data:
+ efd = dashboard_filter_context.extra_form_data
+ extra_filters = efd.get("filters", [])
+
+ for query in json_body.get("queries", []):
+ if extra_filters:
+ existing = query.get("filters", [])
+ query["filters"] = existing + [
+ {**f, "isExtra": True} for f in extra_filters
+ ]
+
+ extras = query.get("extras", {})
+ for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS:
+ if key in efd:
+ extras[key] = efd[key]
+ if extras:
+ query["extras"] = extras
+
+ for (
+ src_key,
+ target_key,
+ ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items():
+ if src_key in efd:
+ query[target_key] = efd[src_key]
+
+ query["extra_form_data"] = efd
+
+ # We need to apply the form data to the global context as jinga
+ # templating pulls form data from the request globally, so this
+ # fallback ensures it has the filters and extra_form_data
applied
+ # when used in get_sqla_query which constructs the final query.
+ g.form_data = json_body
Review Comment:
This was painful to understand lol.
##########
superset/charts/data/api.py:
##########
@@ -156,6 +180,53 @@ def get_data( # noqa: C901
json_body["result_type"] = request.args.get("type",
ChartDataResultType.FULL)
json_body["force"] = request.args.get("force")
+ # Apply dashboard filter context when filters_dashboard_id is provided
+ dashboard_filter_context: DashboardFilterContext | None = None
+ filters_dashboard_id = request.args.get("filters_dashboard_id",
type=int)
+ if filters_dashboard_id is not None:
+ try:
+ dashboard_filter_context = get_dashboard_filter_context(
+ dashboard_id=filters_dashboard_id,
+ chart_id=pk,
+ )
+ except ValueError as error:
+ return self.response_400(message=str(error))
+ except SupersetSecurityException:
+ return self.response_403()
+
+ if dashboard_filter_context.extra_form_data:
+ efd = dashboard_filter_context.extra_form_data
+ extra_filters = efd.get("filters", [])
+
+ for query in json_body.get("queries", []):
+ if extra_filters:
+ existing = query.get("filters", [])
+ query["filters"] = existing + [
+ {**f, "isExtra": True} for f in extra_filters
+ ]
+
+ extras = query.get("extras", {})
+ for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS:
+ if key in efd:
+ extras[key] = efd[key]
+ if extras:
+ query["extras"] = extras
+
+ for (
+ src_key,
+ target_key,
+ ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items():
+ if src_key in efd:
+ query[target_key] = efd[src_key]
+
+ query["extra_form_data"] = efd
Review Comment:
I noticed there are other places in the codebase where we appear to be doing
similar things. For example I noticed there is a merge_extra_filters() function
utils/core(https://github.com/apache/superset/blob/6e7d6a85b451b5bc569d10f8581c16a92f752ba2/superset/utils/core.py#L1126),
but I was not sure how that was being used, and have not had the time to dig
into whether that would be a viable or better approach.
This is not the best in that we are re-implimenting some logic here that
exists in other forms on the front-end/backend, but from my knowledge of the
codebase right now I believe this is the best approach. Please let me know if
you have thoughts on another approach!
##########
superset/charts/data/dashboard_filter_context.py:
##########
@@ -0,0 +1,288 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import logging
+from dataclasses import dataclass, field
+from enum import Enum
+from typing import Any
+
+from flask_babel import gettext as _
+
+from superset import db, security_manager
+from superset.models.dashboard import Dashboard
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+CHART_TYPE = "CHART"
+
+
+class DashboardFilterStatus(str, Enum):
+ APPLIED = "applied"
+ NOT_APPLIED = "not_applied"
+ NOT_APPLIED_USES_DEFAULT_TO_FIRST_ITEM_PREQUERY = (
+ "not_applied_uses_default_to_first_item_prequery"
+ )
+
+
+@dataclass
+class DashboardFilterInfo:
+ id: str
+ name: str
+ status: DashboardFilterStatus
+ column: str | None = None
+
+
+@dataclass
+class DashboardFilterContext:
+ extra_form_data: dict[str, Any] = field(default_factory=dict)
+ filters: list[DashboardFilterInfo] = field(default_factory=list)
+
+ def to_dict(self) -> dict[str, Any]:
+ return {
+ "filters": [
+ {
+ "id": f.id,
+ "name": f.name,
+ "status": f.status.value,
+ **({"column": f.column} if f.column else {}),
+ }
+ for f in self.filters
+ ],
+ }
+
+
+def _is_filter_in_scope_for_chart(
+ filter_config: dict[str, Any],
+ chart_id: int,
+ position_json: dict[str, Any],
+) -> bool:
+ """
+ Determines whether a native filter applies to a given chart. When
+ chartsInScope is present on the filter config, uses that directly.
+ Otherwise falls back to scope.rootPath and scope.excluded with
+ the dashboard layout.
+ """
+ if charts_in_scope := filter_config.get("chartsInScope"):
+ return chart_id in charts_in_scope
+
+ scope = filter_config.get("scope", {})
+ root_path: list[str] = scope.get("rootPath", [])
+ excluded: list[int] = scope.get("excluded", [])
+
+ if chart_id in excluded:
+ return False
+
+ chart_layout_item = _find_chart_layout_item(chart_id, position_json)
+ if not chart_layout_item:
+ return False
+
+ parents: list[str] = chart_layout_item.get("parents", [])
+ return any(parent in root_path for parent in parents)
+
+
+def _find_chart_layout_item(
+ chart_id: int,
+ position_json: dict[str, Any],
+) -> dict[str, Any] | None:
+ """Find the layout item for a chart in the dashboard position JSON."""
+ for item in position_json.values():
+ if not isinstance(item, dict):
+ continue
+ if (
+ item.get("type") == CHART_TYPE
+ and item.get("meta", {}).get("chartId") == chart_id
+ ):
+ return item
+ return None
+
+
+def _merge_extra_form_data(
+ base: dict[str, Any],
+ new: dict[str, Any],
+) -> dict[str, Any]:
+ """
+ Merge two extra_form_data dicts, appending list-type keys (like filters,
+ adhoc_filters) and overriding scalar keys (like granularity_sqla,
time_range).
+ """
+ append_keys = {"filters", "adhoc_filters", "custom_form_data"}
+ override_keys = {
+ "granularity_sqla",
+ "time_grain_sqla",
+ "time_range",
+ "druid_time_origin",
+ "time_column",
+ "time_grain",
+ }
+
+ merged: dict[str, Any] = {}
+
+ for key in append_keys:
+ base_val = base.get(key, [])
+ new_val = new.get(key, [])
+ combined = list(base_val) + list(new_val)
+ if combined:
+ merged[key] = combined
+
+ for key in override_keys:
+ if key in new:
+ merged[key] = new[key]
+ elif key in base:
+ merged[key] = base[key]
+
+ return merged
+
+
+def _extract_filter_extra_form_data(
+ filter_config: dict[str, Any],
+) -> tuple[dict[str, Any] | None, DashboardFilterStatus]:
+ """
+ Extract extra_form_data from a native filter's defaultDataMask.
+
+ Mirrors frontend dashboard behavior except for defaultToFirstItem
+ filters: filters with a static default contribute their
+ extraFormData to the query. Filters without a default or with
+ defaultToFirstItem set to True are simply not applied.
+
+ Returns (extra_form_data, status).
+ """
+ default_data_mask = filter_config.get("defaultDataMask", {})
+ control_values = filter_config.get("controlValues", {})
+
+ extra_form_data = default_data_mask.get("extraFormData")
+ filter_state = default_data_mask.get("filterState", {})
+ has_static_default = filter_state.get("value") is not None
+
+ if control_values.get("defaultToFirstItem"):
+ return (
+ None,
+
DashboardFilterStatus.NOT_APPLIED_USES_DEFAULT_TO_FIRST_ITEM_PREQUERY,
+ )
+
+ if has_static_default and extra_form_data:
+ return extra_form_data, DashboardFilterStatus.APPLIED
+
+ return None, DashboardFilterStatus.NOT_APPLIED
+
+
+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", []):
+ column = targets[0].get("column", {})
+ if isinstance(column, dict):
+ return column.get("name")
+ if isinstance(column, str):
+ return column
+ return None
Review Comment:
I included this target column info to help end users understand how
dashboard filters were impacting their queries more precisely, but this might
not be essential or super useful depending on the use case for this feature.
Can remove if needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
