gabotorresruiz commented on code in PR #44022:
URL: https://github.com/apache/superset/pull/44022#discussion_r3973337350
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -334,6 +361,9 @@ async def get_chart_info(
if error is not None:
return error
+ if request.extra_form_data:
Review Comment:
Good catch, fixed in f613dcb383. The form_data_key only branch now calls
`_attach_active_filters(result, request.extra_form_data)` after redaction,
mirroring the identifier path, so a forwarded gender filter plus "Last year"
surfaces under `active_filters`/`active_time_range` instead of reading as
`[]`/`null`. Pinned by the new `test_unsaved_chart_surfaces_active_filters`
(drives the form_data_key only path and asserts both are populated).
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -265,6 +265,7 @@ def generate(self) -> ASCIIPreview | ChartError:
query_context = build_query_context_from_form_data(
form_data,
chart=self.chart,
+ extra_form_data=self.request.extra_form_data,
Review Comment:
Fixed in f613dcb383. `test_extra_form_data_filters_reach_preview_query` is
now parametrized over `table`, `ascii`, and `vega_lite`. Each strategy builds
its own query context, so the assertion is on the captured query context
(populated before any format specific rendering), which now fails if the
forwarding is removed from any of the three strategies rather than just the
table one.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -3506,6 +3524,27 @@ class ChartFiltersInfo(BaseModel):
"chart."
),
)
+ active_filters: List[Dict[str, Any]] = Field(
+ default_factory=list,
+ description=(
+ "Dashboard native filters the user currently has ACTIVE on this
chart "
+ "(live selections forwarded from the dashboard via
extra_form_data), "
+ "distinct from the chart's own saved filters and from
dashboard_filters "
+ "(the dashboard's default/configured state). A non-empty list
means the "
+ "chart is being viewed filtered; report these as the active
filters. "
+ "Column-based filters only; an active time-range filter is
reported "
+ "separately under active_time_range."
Review Comment:
Applied in f613dcb383. `active_filters` now reads "Column-based and adhoc
filters exactly as forwarded, in their original shapes; an active time-range
filter is reported separately under active_time_range." I also aligned the
`_attach_active_filters` docstring, which had the same "Column-based filters"
wording.
--
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]
