rusackas commented on code in PR #41714:
URL: https://github.com/apache/superset/pull/41714#discussion_r3694017855
##########
superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx:
##########
@@ -336,35 +363,114 @@ const DeckMulti = (props: DeckMultiProps) => {
},
} as any as JsonObject & { slice_id: number };
- const url = getExploreLongUrl(subsliceCopy.form_data, 'json');
-
- if (url) {
- SupersetClient.get({ endpoint: url })
- .then(({ json }) => {
- const layer = createLayerFromData(subsliceCopy, json);
+ const vizType = subsliceCopy.form_data.viz_type as string;
+ Promise.all([
+ getChartBuildQueryRegistry().get(vizType),
+ getChartTransformPropsRegistry().get(vizType),
+ ])
+ .then(([layerBuildQuery, layerTransformProps]) => {
+ if (
+ typeof layerBuildQuery !== 'function' ||
+ typeof layerTransformProps !== 'function'
+ ) {
+ throw new Error(`Unknown deck.gl layer type: ${vizType}`);
+ }
+ const queryContext = layerBuildQuery(
+ subsliceCopy.form_data as QueryFormData,
+ );
+ return SupersetClient.post({
+ endpoint: '/api/v1/chart/data',
+ jsonPayload: {
+ ...queryContext,
+ result_format: 'json',
+ result_type: 'full',
+ },
+ }).then(({ json }) => {
+ // A newer loadLayers call (deck_slices or the visibility filter
+ // changed again) has already reset state; this response belongs
+ // to an abandoned generation and must not write into it.
+ if (loadGenerationRef.current !== generation) {
+ return;
+ }
+ const chartProps = new ChartProps({
+ width: props.width,
+ height: props.height,
+ datasource: props.datasource as unknown as JsonObject,
+ formData: subsliceCopy.form_data,
+ queriesData: (json as JsonObject).result,
+ theme,
+ hooks: {},
+ initialValues: {},
+ });
+ const layerProps = layerTransformProps(chartProps) as JsonObject;
+ const layer = createLayerFromData(subsliceCopy,
layerProps.payload);
setSubSlicesLayers(subSlicesLayers => ({
...subSlicesLayers,
[subsliceCopy.slice_id]: layer,
}));
- })
- .catch(error => {
- throw new Error(
- `Error loading layer for slice ${subsliceCopy.slice_id}:
${error}`,
- );
+
+ // Refit the viewport to the data now that this layer's features
are
+ // known (unless autozoom is off). The initial getAdjustedViewport
+ // could not do this because the v1 payload carries no features.
+ const layerFeatures = (layerProps.payload as JsonObject |
undefined)
+ ?.data?.features;
+ if (formData.autozoom !== false && Array.isArray(layerFeatures)) {
+ layerFeaturesRef.current = {
+ ...layerFeaturesRef.current,
+ [vizType]: layerFeatures,
Review Comment:
You're right, that's a real bug. I keyed the accumulator by slice_id now
(still bucketing by viz_type internally for collectPoints, which needs the
buckets), so two same-type layers get concatenated instead of the second
clobbering the first. Pushed the fix.
##########
superset-frontend/packages/superset-ui-core/src/chart/clients/ChartClient.ts:
##########
@@ -104,24 +104,15 @@ export default class ChartClient {
const buildQueryRegistry = getChartBuildQueryRegistry();
if (metaDataRegistry.has(visType)) {
- const { useLegacyApi } = metaDataRegistry.get(visType)!;
const buildQuery =
(await buildQueryRegistry.get(visType)) ?? (() => formData);
- const requestConfig: RequestConfig = useLegacyApi
- ? {
- endpoint: '/explore_json/',
- postPayload: {
- form_data: buildQuery(formData),
- },
- ...options,
- }
- : {
- endpoint: '/api/v1/chart/data',
- jsonPayload: {
- query_context: buildQuery(formData),
- },
- ...options,
- };
+ const requestConfig: RequestConfig = {
+ endpoint: '/api/v1/chart/data',
+ jsonPayload: {
+ query_context: buildQuery(formData),
Review Comment:
Confirmed, this was already broken for any non-legacy viz_type before this
PR (that branch of loadQueryData was gated by useLegacyApi and never touched),
and this PR just widens the blast radius to deck_multi too. Pushed the fix: it
posts buildQuery(formData) directly as the payload now instead of nesting it
under query_context, and unwraps response.json.result instead of assuming a
bare array. Updated the existing ChartClient test too since its fetchMock
response did not match the real {result: [...]} shape.
##########
superset/migrations/shared/migrate_viz/base.py:
##########
@@ -167,7 +167,13 @@ def upgrade_slice(cls, slc: Slice) -> None:
if "form_data" in query_context:
query_context["form_data"] = clz.data
- queries_bak = copy.deepcopy(query_context["queries"])
+ # A stored query_context is expected to carry "queries", but
+ # an atypical/malformed one (e.g. hand-edited via the API)
+ # missing it must not raise here: viz_type was already
+ # flipped above, so an uncaught exception at this point
+ # would leave the slice half-migrated (new viz_type, but
+ # stale params/query_context in the old shape).
+ queries_bak = copy.deepcopy(query_context.get("queries"))
Review Comment:
Good catch. Fixed by backing up the whole original query_context (wrapped in
a sentinel key) when it lacks "queries", instead of just backing up None --
downgrade_slice now restores it verbatim in that case rather than nulling it
out. Added a test asserting a full upgrade/downgrade round-trip on a
query_context missing "queries".
--
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]
