You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/13 16:14:32 UTC
(superset) 01/01: fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4f839ef2d182486ea49a5da74fe2f44c1451fa4d
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Feb 9 21:49:33 2024 +0100
fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
---
superset-frontend/package-lock.json | 44 +++++++++++++-----
.../src/components/Chart/DrillBy/DrillByModal.tsx | 11 +++--
.../src/components/Chart/chartAction.js | 52 +++++++++++-----------
.../src/components/Chart/chartActions.test.js | 43 +++++++++++++++++-
superset/security/manager.py | 6 ++-
5 files changed, 115 insertions(+), 41 deletions(-)
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index 6cacdb3a0b..8b371df962 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -5346,6 +5346,11 @@
"csstype": "^2.5.7"
}
},
+ "node_modules/@emotion/serialize/node_modules/csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ },
"node_modules/@emotion/sheet": {
"version": "0.9.4",
"resolved": "https://registry.npmjs.org/@emotion/sheet/-/sheet-0.9.4.tgz",
@@ -19773,6 +19778,11 @@
"@types/react": "*"
}
},
+ "node_modules/@types/react/node_modules/csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ },
"node_modules/@types/redux-localstorage": {
"version": "1.0.8",
"resolved": "https://registry.npmjs.org/@types/redux-localstorage/-/redux-localstorage-1.0.8.tgz",
@@ -28067,11 +28077,6 @@
"integrity": "sha512-b0tGHbfegbhPJpxpiBPU2sCkigAqtM9O121le6bbOlgyV+NyGyCmVfJ6QW9eRjz8CpNfWEOYBIMIGRYkLwsIYg==",
"dev": true
},
- "node_modules/csstype": {
- "version": "2.6.9",
- "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz",
- "integrity": "sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q=="
- },
"node_modules/currencyformatter.js": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/currencyformatter.js/-/currencyformatter.js-1.0.5.tgz",
@@ -52126,6 +52131,11 @@
"@emotion/weak-memoize": "0.2.5"
}
},
+ "node_modules/react-select/node_modules/csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ },
"node_modules/react-select/node_modules/dom-helpers": {
"version": "5.1.4",
"resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.1.4.tgz",
@@ -67639,6 +67649,13 @@
"@emotion/unitless": "0.7.5",
"@emotion/utils": "0.11.3",
"csstype": "^2.5.7"
+ },
+ "dependencies": {
+ "csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ }
}
},
"@emotion/sheet": {
@@ -79607,6 +79624,13 @@
"requires": {
"@types/prop-types": "*",
"csstype": "^2.2.0"
+ },
+ "dependencies": {
+ "csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ }
}
},
"@types/react-dom": {
@@ -86171,11 +86195,6 @@
}
}
},
- "csstype": {
- "version": "2.6.9",
- "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz",
- "integrity": "sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q=="
- },
"currencyformatter.js": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/currencyformatter.js/-/currencyformatter.js-1.0.5.tgz",
@@ -104616,6 +104635,11 @@
"@emotion/weak-memoize": "0.2.5"
}
},
+ "csstype": {
+ "version": "2.6.21",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+ "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+ },
"dom-helpers": {
"version": "5.1.4",
"resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.1.4.tgz",
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index d1e8b39e0c..5053a76711 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -54,11 +54,12 @@ import {
LOG_ACTIONS_DRILL_BY_MODAL_OPENED,
LOG_ACTIONS_FURTHER_DRILL_BY,
} from 'src/logger/LogUtils';
+import { getQuerySettings } from 'src/explore/exploreUtils';
import { Dataset, DrillByType } from '../types';
import DrillByChart from './DrillByChart';
import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
import { useContextMenu } from '../ChartContextMenu/useContextMenu';
-import { getChartDataRequest } from '../chartAction';
+import { getChartDataRequest, handleChartDataResponse } from '../chartAction';
import { useDisplayModeToggle } from './useDisplayModeToggle';
import {
DrillByBreadcrumb,
@@ -390,13 +391,17 @@ export default function DrillByModal({
useEffect(() => {
if (drilledFormData) {
+ const [useLegacyApi] = getQuerySettings(drilledFormData);
setIsChartDataLoading(true);
setChartDataResult(undefined);
getChartDataRequest({
formData: drilledFormData,
})
- .then(({ json }) => {
- setChartDataResult(json.result);
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
+ .then(queriesResponse => {
+ setChartDataResult(queriesResponse);
})
.catch(() => {
addDangerToast(t('Failed to load chart data.'));
diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
index 8cd3785ae5..9e7f961abd 100644
--- a/superset-frontend/src/components/Chart/chartAction.js
+++ b/superset-frontend/src/components/Chart/chartAction.js
@@ -374,6 +374,29 @@ export function addChart(chart, key) {
return { type: ADD_CHART, chart, key };
}
+export function handleChartDataResponse(response, json, useLegacyApi) {
+ if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
+ // deal with getChartDataRequest transforming the response data
+ const result = 'result' in json ? json.result : json;
+ switch (response.status) {
+ case 200:
+ // Query results returned synchronously, meaning query was already cached.
+ return Promise.resolve(result);
+ case 202:
+ // Query is running asynchronously and we must await the results
+ if (useLegacyApi) {
+ return waitForAsyncData(result[0]);
+ }
+ return waitForAsyncData(result);
+ default:
+ throw new Error(
+ `Received unexpected response status (${response.status}) while fetching chart data`,
+ );
+ }
+ }
+ return json.result;
+}
+
export function exploreJSON(
formData,
force = false,
@@ -409,31 +432,11 @@ export function exploreJSON(
dispatch(chartUpdateStarted(controller, formData, key));
+ const [useLegacyApi] = getQuerySettings(formData);
const chartDataRequestCaught = chartDataRequest
- .then(({ response, json }) => {
- if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
- // deal with getChartDataRequest transforming the response data
- const result = 'result' in json ? json.result : json;
- const [useLegacyApi] = getQuerySettings(formData);
- switch (response.status) {
- case 200:
- // Query results returned synchronously, meaning query was already cached.
- return Promise.resolve(result);
- case 202:
- // Query is running asynchronously and we must await the results
- if (useLegacyApi) {
- return waitForAsyncData(result[0]);
- }
- return waitForAsyncData(result);
- default:
- throw new Error(
- `Received unexpected response status (${response.status}) while fetching chart data`,
- );
- }
- }
-
- return json.result;
- })
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
.then(queriesResponse => {
queriesResponse.forEach(resultItem =>
dispatch(
@@ -492,7 +495,6 @@ export function exploreJSON(
});
// only retrieve annotations when calling the legacy API
- const [useLegacyApi] = getQuerySettings(formData);
const annotationLayers = useLegacyApi
? formData.annotation_layers || []
: [];
diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js
index b3a6fed9f5..c2a58e6094 100644
--- a/superset-frontend/src/components/Chart/chartActions.test.js
+++ b/superset-frontend/src/components/Chart/chartActions.test.js
@@ -21,10 +21,12 @@ import fetchMock from 'fetch-mock';
import sinon from 'sinon';
import * as chartlib from '@superset-ui/core';
-import { SupersetClient } from '@superset-ui/core';
+import { FeatureFlag, SupersetClient } from '@superset-ui/core';
import { LOG_EVENT } from 'src/logger/actions';
import * as exploreUtils from 'src/explore/exploreUtils';
import * as actions from 'src/components/Chart/chartAction';
+import * as asyncEvent from 'src/middleware/asyncEvent';
+import { handleChartDataResponse } from 'src/components/Chart/chartAction';
describe('chart actions', () => {
const MOCK_URL = '/mockURL';
@@ -33,6 +35,7 @@ describe('chart actions', () => {
let getChartDataUriStub;
let metadataRegistryStub;
let buildQueryRegistryStub;
+ let waitForAsyncDataStub;
let fakeMetadata;
const setupDefaultFetchMock = () => {
@@ -66,6 +69,9 @@ describe('chart actions', () => {
result_format: 'json',
}),
}));
+ waitForAsyncDataStub = sinon
+ .stub(asyncEvent, 'waitForAsyncData')
+ .callsFake(data => Promise.resolve(data));
});
afterEach(() => {
@@ -74,6 +80,11 @@ describe('chart actions', () => {
fetchMock.resetHistory();
metadataRegistryStub.restore();
buildQueryRegistryStub.restore();
+ waitForAsyncDataStub.restore();
+
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: false,
+ };
});
describe('v1 API', () => {
@@ -114,6 +125,36 @@ describe('chart actions', () => {
expect(fetchMock.calls(mockBigIntUrl)).toHaveLength(1);
expect(json.value.toString()).toEqual(expectedBigNumber);
});
+
+ it('handleChartDataResponse should return result if GlobalAsyncQueries flag is disabled', async () => {
+ const result = await handleChartDataResponse(
+ { status: 200 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
+
+ it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and results are returned synchronously', async () => {
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: true,
+ };
+ const result = await handleChartDataResponse(
+ { status: 200 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
+
+ it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and query is running asynchronously', async () => {
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: true,
+ };
+ const result = await handleChartDataResponse(
+ { status: 202 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
});
describe('legacy API', () => {
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 1cc289f877..e6eb77e645 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -453,7 +453,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
level=ErrorLevel.ERROR,
)
- def get_chart_access_error_object( # pylint: disable=invalid-name
+ def get_chart_access_error_object(
self,
dashboard: "Dashboard", # pylint: disable=unused-argument
) -> SupersetError:
@@ -2088,7 +2088,9 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
raise SupersetSecurityException(self.get_chart_access_error_object(chart))
- def get_user_by_username(self, username: str, session: Session = None) -> Optional[User]:
+ def get_user_by_username(
+ self, username: str, session: Session = None
+ ) -> Optional[User]:
"""
Retrieves a user by it's username case sensitive. Optional session parameter
utility method normally useful for celery tasks where the session