codeant-ai-for-open-source[bot] commented on code in PR #41131:
URL: https://github.com/apache/superset/pull/41131#discussion_r3436348835
##########
superset/charts/data/dashboard_filter_context.py:
##########
@@ -314,3 +314,39 @@ def get_dashboard_filter_context(
)
return context
+
+
+def apply_dashboard_filter_context(
+ query_context: dict[str, Any],
+ extra_form_data: dict[str, Any],
+) -> None:
+ """
+ Apply dashboard filter context.
+
+ Filters are removed from ``extra_form_data`` to avoid duplicated values
when
+ using ``filter_values()`` macro.
+
+ :param query_context: The chart's query context (mutated in place)
+ :param extra_form_data: The dashboard's merged extra_form_data to apply.
It's
+ also mutated in place.
+ """
+ extra_filters = extra_form_data.pop("filters", [])
+ for query in query_context.get("queries", []):
+ if extra_filters:
+ existing_filters = query.get("filters") or []
+ query["filters"] = existing_filters + [
+ {**flt, "isExtra": True} for flt in extra_filters
+ ]
+
+ extras = query.get("extras") or {}
+ for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS:
+ if key in extra_form_data:
+ extras[key] = extra_form_data[key]
+ if extras:
+ query["extras"] = extras
+
+ 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]
+
+ query["extra_form_data"] = extra_form_data
Review Comment:
**Suggestion:** This assignment overwrites any pre-existing
`query["extra_form_data"]` from the saved chart query context, so chart-level
extra form settings can be silently dropped when `filters_dashboard_id` is
used. Merge the dashboard extra form data into the existing query-level value
instead of replacing it, so previously persisted query-specific settings are
preserved. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Chart-specific extra_form_data lost when dashboard filters applied.
- ⚠️ Virtual dataset Jinja macros may miss saved settings.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a chart with a saved query context that includes query-level extra
form data by
following the pattern in
`superset/tests/integration_tests/charts/data/api_tests.py:_setup_chart_with_query_context`
(lines 1810–1838), but add `"extra_form_data": {"existing_flag": true}`
inside the single
entry in `"queries"` before persisting it to `Slice.query_context`.
2. Call the chart data API with a dashboard filter context as in
`TestGetChartDataWithDashboardFilter.test_get_data_with_dashboard_filter_context`
(same
file, lines 41–75), e.g. `GET
/api/v1/chart/<pk>/data/?filters_dashboard_id=1`, so that
`ChartDataRestApi.get_data` in `superset/charts/data/api.py` (around line
193–208) loads
`chart.query_context` into `json_body` and obtains a
`DashboardFilterContext` with
non-empty `extra_form_data`.
3. Observe that `get_data` calls `apply_dashboard_filter_context(json_body,
efd)` at
`superset/charts/data/api.py:208`, and inside
`apply_dashboard_filter_context` (new helper
in `superset/charts/data/dashboard_filter_context.py`, lines 319–352) the
loop assigns
`query["extra_form_data"] = extra_form_data`, overwriting any pre-existing
`query["extra_form_data"]` from the saved query context instead of merging
into it.
4. When the query is later executed, `g.form_data` is set to `json_body` in
`ChartDataRestApi.get_data` (line 214), and `get_form_data` in
`superset/views/utils.py`
(lines 40–45) pulls the first query into the top-level form data; because
`query["extra_form_data"]` now only contains the dashboard-level
`extra_form_data`, any
original chart-specific extra form settings stored there are lost whenever
`filters_dashboard_id` is used, changing behavior compared to running the
same chart
without that parameter.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5f571bf306154498b69a09b79db5d15c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5f571bf306154498b69a09b79db5d15c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/data/dashboard_filter_context.py
**Line:** 352:352
**Comment:**
*Api Mismatch: This assignment overwrites any pre-existing
`query["extra_form_data"]` from the saved chart query context, so chart-level
extra form settings can be silently dropped when `filters_dashboard_id` is
used. Merge the dashboard extra form data into the existing query-level value
instead of replacing it, so previously persisted query-specific settings are
preserved.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41131&comment_hash=f000df116d0427dd206bcd53d907aa31344f6e741109f6f776a5e13579b31eab&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41131&comment_hash=f000df116d0427dd206bcd53d907aa31344f6e741109f6f776a5e13579b31eab&reaction=dislike'>👎</a>
--
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]
