aminghadersohi commented on code in PR #42490:
URL: https://github.com/apache/superset/pull/42490#discussion_r3675395035


##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -755,6 +751,71 @@ def _ensure_temporal_adhoc_filter(form_data: Dict[str, 
Any], column: str) -> Non
     form_data["adhoc_filters"] = existing
 
 
+def _bind_dashboard_time_range_filter(
+    form_data: Dict[str, Any],
+    config: ChartConfig,
+    dataset_id: int | str | None,
+) -> None:
+    """Bind charts without time configuration to a temporal filter subject."""
+    if form_data.get("granularity_sqla") or any(
+        filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+        for filter_ in form_data.get("adhoc_filters", [])
+    ):
+        return

Review Comment:
   Fixed in abfb554720. Dashboard binding no longer exits merely because a 
different TEMPORAL_RANGE filter exists; the explicit temporal column is added 
unless that same subject is already bound. Added regression coverage for 
preserving the different chart-specific filter alongside the requested binding.



##########
superset/mcp_service/chart/tool/update_chart_preview.py:
##########
@@ -102,6 +102,59 @@ def _get_previous_form_data(form_data_key: str) -> 
dict[str, Any] | None:
     return None
 
 
+def _preserve_previous_adhoc_filters(
+    new_form_data: dict[str, Any], previous_form_data: dict[str, Any]
+) -> None:
+    """Preserve cached filters without dropping mapper-generated bindings."""
+    previous_filters = previous_form_data.get("adhoc_filters")
+    if not isinstance(previous_filters, list) or not previous_filters:
+        return
+
+    generated_filters = new_form_data.get("adhoc_filters", [])
+    generated_temporal_subjects = {
+        filter_.get("subject")
+        for filter_ in generated_filters
+        if isinstance(filter_, dict) and filter_.get("operator") == 
"TEMPORAL_RANGE"
+    }
+    previous_temporal_subjects = {
+        filter_.get("subject")
+        for filter_ in previous_filters
+        if isinstance(filter_, dict) and filter_.get("operator") == 
"TEMPORAL_RANGE"
+    }
+    replace_temporal_binding = (
+        bool(generated_temporal_subjects)
+        and generated_temporal_subjects != previous_temporal_subjects
+    )

Review Comment:
   Fixed in abfb554720. Cached temporal filters are removed whenever the new 
mapper output has a different temporal-subject set, including when the new 
mapping has no valid temporal binding. Unrelated cached adhoc filters remain 
preserved, with regression coverage.



-- 
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]

Reply via email to