gabotorresruiz commented on code in PR #45294:
URL: https://github.com/apache/superset/pull/45294#discussion_r4316830890
##########
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:
Not a blocker, and possibly a shape no client can send: the stamp needs
`extra_form_data["time_range"]` to equal `query_object.time_range`, so when the
two disagree a saved `No filter` on a column the view lacks refuses rather than
taking the dashboard pass (`200` on `0ce9b6ca46`, `400` here). The `mismatched`
params pin that as intentional. Is there a real payload where a dashboard's
`extra_form_data` time range is not the one the query ends up with? If not I am
happy to leave it.
##########
superset/common/query_context_factory.py:
##########
@@ -434,8 +434,19 @@ 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
+ )
if query_object.time_range:
for filter_object in query_object.filter:
if filter_object["op"] == "TEMPORAL_RANGE":
filter_object["val"] = query_object.time_range
+ if dashboard_time_override:
+ filter_object["isExtra"] = True
Review Comment:
Confirmed closed, thanks. On `1ec256c2` all three rows refuse again:
`gone_col` `TEMPORAL_RANGE` `2024-01-01 : 2024-06-01` is `400` with and without
a dashboard range, and `total_amount` in the same shape is `400` with one, so
the refusal `master` already had is back. Reverting the `was_unbounded` check
turns `test_time_override_stamps_only_unbounded_saved_predicates` and the
bounded rows of
`test_dashboard_time_override_preserves_foreign_filter_provenance` red, and the
real range case you added there is what catches it.
##########
superset/charts/data/dashboard_filter_context.py:
##########
@@ -382,5 +383,11 @@ def apply_dashboard_filter_context( # noqa: C901
for src_key, target_key in
EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items():
if src_key in extra_form_data:
query[target_key] = extra_form_data[src_key]
+ if src_key == "time_range" and extra_form_data[src_key]:
+ # The query factory rewrites temporal predicates with this
range.
+ filter_: QueryObjectFilterClause
+ for filter_ in query.get("filters") or []:
+ if filter_.get("op") == "TEMPORAL_RANGE":
+ filter_["isExtra"] = True
Review Comment:
Closed as well. The `server` rows only go green because the same value check
landed here, and the `workbook.py:223` caller inherits it: with a dashboard
range applied through `apply_dashboard_filter_context`, a saved `2024-01-01 :
2024-06-01` on a missing column stays unmarked and refuses, while `None` and
`No filter` are marked and render. Reverting this guard on its own turns four
params of `test_dashboard_time_override_preserves_foreign_filter_provenance`
and the new MCP helper test red.
--
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]
