codeant-ai-for-open-source[bot] commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r3817805869
##########
superset/commands/chart/importers/v1/utils.py:
##########
@@ -70,6 +77,64 @@ def import_chart(
filter_chart_annotations(config)
+ # Synthesize a query_context for imported charts that arrive without one,
so
+ # the first `GET /api/v1/chart/{pk}/data/` returns data instead of HTTP 400
+ # "Chart has no query context saved" (issue #33615, ADR-013). Guarded on an
+ # ABSENT context so an existing/remapped one is never overwritten (FR-006).
+ #
+ # Two-tier derivation:
+ # 1. AUTHORITATIVE — run the chart's real frontend `buildQuery` in V8
+ # (QueryContextGenerator) for byte-faithful parity with the UI.
+ # 2. FALLBACK — a pure-Python generic derivation
+ # (`build_query_context_config`) when the V8 bundle / py_mini_racer is
+ # unavailable or the viz type is not (yet) covered by the bundle.
+ # Either way the datasource is taken from the importer-resolved id/type
ONLY,
+ # never a value carried in params (ADR-014 authz/RLS). A per-chart
derivation
+ # error must never abort the bundle (RISK-T03 / FR-004).
+ if not config.get("query_context"):
+ try:
+ params = config.get("params") or {}
+ viz_type = config["viz_type"]
+ datasource_id = config.get("datasource_id")
+ datasource_type = config.get("datasource_type", "table")
+
+ query_context_config = None
+ if datasource_id:
+ # form_data for the JS builder: the datasource is the
+ # importer-resolved id/type only (overwrite any incoming
+ # params.datasource — never trust it; ADR-014).
+ js_params = {
+ **params,
+ "datasource": f"{datasource_id}__{datasource_type}",
+ }
+ query_context_config = get_query_context_generator().generate(
+ viz_type, js_params
+ )
+ if query_context_config is None:
+ query_context_config = build_query_context_config(
+ params, viz_type, datasource_id, datasource_type
Review Comment:
Yes—this is a valid mismatch. Synthesis currently runs before
`migrate_chart()`, so legacy charts such as `dual_line` can persist a context
based on the pre-migration viz type and single-series form data, while the
migrated chart expects `mixed_timeseries` and multiple queries.
Synthesis should run **after migration**, using the migrated config:
```python
# Filter annotations first.
filter_chart_annotations(config)
# Serialize params as expected by migration.
config["params"] = json.dumps(config["params"])
# Apply legacy viz/parameter migrations.
config = migrate_chart(config)
# Synthesize from the final viz type and params, preserving an existing
context.
_synthesize_query_context_if_absent(config)
```
The helper should also normalize `config["params"]` back to a dictionary
before deriving, since migration leaves it serialized:
```python
params = config.get("params") or {}
if isinstance(params, str):
params = json.loads(params)
```
This preserves the non-destructive behavior for imported contexts while
ensuring newly synthesized contexts reflect the migrated chart, including all
query objects required by `mixed_timeseries`. A regression test for a
`dual_line` import should assert that the persisted context has the migrated
viz/query shape and both series.
--
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]
