mikebridge commented on code in PR #45294:
URL: https://github.com/apache/superset/pull/45294#discussion_r4296344128


##########
superset/common/query_context_factory.py:
##########
@@ -434,8 +435,23 @@ def _apply_granularity(  # noqa: C901
                     if filter["col"] != filter_to_remove
                 ]
 
-    def _apply_filters(self, query_object: QueryObject) -> None:
+    def _apply_filters(
+        self, query_object: QueryObject, form_data: dict[str, Any] | None = 
None
+    ) -> None:
+        """Apply the time range, retaining the source of dashboard 
overrides."""
+        extra_form_data: object = (form_data or {}).get("extra_form_data")
+        dashboard_time_override: bool = (
+            isinstance(extra_form_data, dict)
+            and "time_range" in extra_form_data
+            and extra_form_data["time_range"] == query_object.time_range
+        )

Review Comment:
   Thanks, Amin, good catch. I reproduced it: with a saved query context, 
`get_chart_data` merges `extra_form_data.time_range` into the queries but not 
into `form_data`, so the factory never treated the rewritten "No filter" 
predicate as a dashboard filter, and a missing column was refused.
   
   Fixed in 5f3aed2a55 (plus a small guard in 93a200cf17):
   - The marking the server-side dashboard filter context already did is now a 
shared `mark_dashboard_time_range_filters` helper.
   - MCP's `merge_extra_form_data_filters_into_query` calls it when the request 
carries a `time_range`. That covers `get_chart_data` and `get_chart_sql`. The 
form_data paths already passed `extra_form_data` through to the factory.
   - As before, only unbounded saved ranges (none or "No filter") take the 
request's provenance. A saved bounded range on a missing column is still 
refused.
   
   Tests:
   - `test_dashboard_time_override_preserves_foreign_filter_provenance` gains 
an `mcp` source. Its unbounded cases failed before the change (6 failed) and 
pass now.
   - A helper test checks that a bounded range stays unmarked and that a 
malformed saved filter is skipped.
   
   _Change and reply drafted with AI assistance (Claude Code)._



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