bito-code-review[bot] commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r4004772870
##########
superset-frontend/scripts/gen-qc-registry.mjs:
##########
@@ -0,0 +1,187 @@
+// Deterministic, re-runnable codegen: maps every plugin `buildQuery` module
to the
+// viz_type key(s) it is registered under, and emits registry.generated.ts
consumed
+// by entry.ts. Join: buildQuery module <- (index.ts that imports it) ->
plugin class
+// name -> MainPreset `.configure({ key: VizType.X })` -> VizType enum string.
+// A single builder legitimately maps to several keys (e.g. echarts_timeseries
+ _line/_bar/...).
+import { readFileSync, writeFileSync, readdirSync, statSync, existsSync } from
'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const FE = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+const PLUGINS = path.join(FE, 'plugins');
+const OUT = path.join(FE, 'src', 'backend-querycontext',
'registry.generated.ts');
+
+// --- 1. VizType enum: EnumName -> 'string_value' ---
+const vizTypeSrc = readFileSync(
+ path.join(FE, 'packages/superset-ui-core/src/chart/types/VizType.ts'),
+ 'utf8',
+);
+const VIZ_ENUM = {};
+for (const m of vizTypeSrc.matchAll(/(\w+)\s*=\s*['"]([\w-]+)['"]/g))
VIZ_ENUM[m[1]] = m[2];
+
+// --- 2. MainPreset: ClassName -> viz string ---
+const mainPreset = readFileSync(
+ path.join(FE, 'src/visualizations/presets/MainPreset.js'),
+ 'utf8',
+);
+// MainPreset renames on import (e.g. `import { PivotTableChartPlugin as
+// PivotTableChartPluginV2 } from '...'`), then `new
PivotTableChartPluginV2()`. Map
+// each local `new X()` name back to the ORIGINAL package export name the
codegen sees.
+const importOrig = {}; // localName -> package-export name
+for (const im of
mainPreset.matchAll(/import\s*\{([^}]*)\}\s*from\s*['"][^'"]+['"]/g)) {
+ for (let spec of im[1].split(',')) {
+ spec = spec.trim();
+ if (!spec) continue;
+ const as = spec.match(/^(\w+)\s+as\s+(\w+)$/);
+ if (as) importOrig[as[2]] = as[1];
+ else if (/^\w+$/.test(spec)) importOrig[spec] = spec;
+ }
+}
+const CLASS_TO_VIZ = {};
+for (const m of mainPreset.matchAll(
+ /new\s+(\w+)\s*\(\s*\)\s*\.configure\(\s*\{\s*key:\s*VizType\.(\w+)/g,
+)) {
Review Comment:
<!-- Bito Reply -->
The suggestion provided by the reviewer was intended to improve the regex to
account for constructor arguments, which was causing certain plugins to be
skipped during registry generation. Since you have already addressed this by
implementing an explicit override in the generator and narrowing the scope of
the matcher, the original suggestion is no longer necessary to apply. You may
choose to ignore or resolve the comment as addressed.
##########
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 {}
Review Comment:
<!-- Bito Reply -->
The suggestion to use `json.loads(chart.params)` is correct and necessary to
avoid a `TypeError` when `params` is a JSON string. Applying this pattern
ensures the code handles both dictionary and string inputs consistently,
matching the established pattern in other parts of the codebase.
**superset/commands/chart/importers/v1/utils.py**
```
try:
params = config.get("params")
if isinstance(params, str):
params = json.loads(params)
params = params or {}
```
--
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]
