Skip to content

Commit 8a15ec2

Browse files
committed
fix(gaq): make async opt-in explicit + fix per-dashboard override persistence
Address review feedback on #43429: - async opt-in was applied inside the shared v1ChartDataRequest, so direct callers that read response.json.result (AlertReportModal, ViewQueryModal, fetchTopNValues, useResultsPane) would receive a 202 {task_ids} and break. Gate injection on an explicit getChartDataRequest({enableAsyncMode}) that only the 202-handling callers set (exploreJSON, FilterValue, FiltersConfigForm, DrillByModal, ChartVersionPreview). - the dashboard properties async override was never applied when a dashboard already had json_metadata.async_mode: the editor copy carried it so the '=== undefined' guard skipped the dropdown. Omit async_mode from the Advanced JSON editor copy (dropdown is the sole source of truth, mirroring show_chart_timestamps) and apply the dropdown value unconditionally on save.
1 parent 124264b commit 8a15ec2

6 files changed

Lines changed: 36 additions & 10 deletions

File tree

superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,8 @@ export default function DrillByModal({
405405
const requestDrillData = () =>
406406
getChartDataRequest({
407407
formData: drilledFormData,
408+
// 202 is handled below via handleChartDataResponse.
409+
enableAsyncMode: true,
408410
});
409411
requestDrillData()
410412
.then(({ response, json }) =>
@@ -502,6 +504,8 @@ export default function DrillByModal({
502504
const requestDrillData = () =>
503505
getChartDataRequest({
504506
formData: drilledFormData,
507+
// 202 is handled below via handleChartDataResponse.
508+
enableAsyncMode: true,
505509
});
506510
requestDrillData()
507511
.then(({ response, json }) =>

superset-frontend/src/components/Chart/chartAction.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ export interface RequestParams {
254254
// Per-dashboard async-mode override (from json_metadata.async_mode), used to
255255
// resolve whether this render requests async execution.
256256
async_mode_override?: AsyncModeOverride;
257+
// Whether the caller handles an HTTP 202 async task response (see
258+
// GetChartDataRequestParams.enableAsyncMode).
259+
enableAsyncMode?: boolean;
257260
[key: string]: unknown;
258261
}
259262

@@ -283,6 +286,11 @@ export interface GetChartDataRequestParams {
283286
force?: boolean;
284287
requestParams?: RequestParams;
285288
ownState?: JsonObject;
289+
// Opt into asynchronous execution. Only set by callers that handle an HTTP 202
290+
// task response (via handleChartDataResponse / waitForAsyncData); direct
291+
// consumers that read `response.json.result` must leave this false so they keep
292+
// the synchronous flow.
293+
enableAsyncMode?: boolean;
286294
}
287295

288296
// runAnnotationQuery params interface
@@ -449,9 +457,11 @@ const v1ChartDataRequest = async (
449457
}).toString();
450458

451459
// Opt full JSON chart-data renders into async execution per the resolved policy
452-
// (feature flag + deployment default + optional per-dashboard override). The
453-
// server treats an absent async_mode as synchronous, so this is additive.
460+
// (feature flag + deployment default + optional per-dashboard override). Only
461+
// callers that handle a 202 task response set enableAsyncMode; the server treats
462+
// an absent async_mode as synchronous, so this is additive.
454463
const asyncMode =
464+
requestParams.enableAsyncMode === true &&
455465
resultFormat === 'json' &&
456466
resultType === 'full' &&
457467
resolveAsyncMode(requestParams.async_mode_override);
@@ -480,9 +490,11 @@ export async function getChartDataRequest({
480490
force = false,
481491
requestParams = {},
482492
ownState = {},
493+
enableAsyncMode = false,
483494
}: GetChartDataRequestParams): Promise<ChartDataRequestResponse> {
484495
let querySettings: RequestParams = {
485496
...requestParams,
497+
enableAsyncMode,
486498
};
487499

488500
if (domainShardingEnabled) {
@@ -743,6 +755,8 @@ export function exploreJSON(
743755
force: fromCache ? false : force,
744756
requestParams,
745757
ownState,
758+
// exploreJSON handles 202 via handleChartDataResponse.
759+
enableAsyncMode: true,
746760
});
747761

748762
const chartDataRequest = requestChartData();

superset-frontend/src/dashboard/components/PropertiesModal/index.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ const PropertiesModal = ({
209209
'map_label_colors',
210210
'color_scheme_domain',
211211
'show_chart_timestamps',
212+
// Edited via the async-mode dropdown, not the raw JSON editor, so the
213+
// dropdown is the single source of truth (mirrors show_chart_timestamps).
214+
'async_mode',
212215
]);
213216

214217
setJsonMetadata(metaDataCopy ? jsonStringify(metaDataCopy) : '');
@@ -357,14 +360,13 @@ const PropertiesModal = ({
357360
// refresh") rather than falling through to the dropdown value (#42116).
358361
jsonMetadataObj.refresh_frequency =
359362
jsonMetadataObj.refresh_frequency ?? refreshFrequency;
360-
// Persist the per-dashboard async override (unless set directly in the
361-
// Advanced JSON editor). 'default' clears it so the deployment default applies.
362-
if (jsonMetadataObj.async_mode === undefined) {
363-
if (asyncMode === 'default') {
364-
delete jsonMetadataObj.async_mode;
365-
} else {
366-
jsonMetadataObj.async_mode = asyncMode;
367-
}
363+
// Persist the per-dashboard async override from the dropdown (the sole source
364+
// of truth — async_mode is omitted from the Advanced JSON editor). 'default'
365+
// clears it so the deployment default applies.
366+
if (asyncMode === 'default') {
367+
delete jsonMetadataObj.async_mode;
368+
} else {
369+
jsonMetadataObj.async_mode = asyncMode;
368370
}
369371
jsonMetadataObj.show_chart_timestamps = Boolean(showChartTimestamps);
370372
const customLabelColors = jsonMetadataObj.label_colors || {};

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,8 @@ const FilterValue: FC<FilterValueProps> = ({
283283
formData: newFormData,
284284
force: fromCache ? false : shouldRefresh,
285285
ownState: filterOwnState,
286+
// 202 is handled below via waitForAsyncData.
287+
enableAsyncMode: true,
286288
});
287289
requestFilterData()
288290
.then(({ response, json }) => {

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,8 @@ const FiltersConfigForm = (
530530
getChartDataRequest({
531531
formData,
532532
force: fromCache ? false : force,
533+
// 202 is handled below via waitForAsyncData.
534+
enableAsyncMode: true,
533535
});
534536
requestDefaultValues()
535537
.then(({ response, json }) => {

superset-frontend/src/features/versionHistory/ChartVersionPreview.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ export default function ChartVersionPreview() {
181181
const requestPreviewData = () =>
182182
getChartDataRequest({
183183
formData: previewFormData,
184+
// 202 is handled below via handleChartDataResponse.
185+
enableAsyncMode: true,
184186
});
185187
const { response, json } = await requestPreviewData();
186188
const result = await handleChartDataResponse(

0 commit comments

Comments
 (0)