This is an automated email from the ASF dual-hosted git repository. amitmiran pushed a commit to branch 1.2 in repository https://gitbox.apache.org/repos/asf/superset.git
commit f2236fcb8ff5bee075dbdadf82aaf61f863f7c78 Author: Ville Brofeldt <[email protected]> AuthorDate: Fri May 7 21:07:44 2021 +0300 fix(chart-data): handle url_params in csv export and native filters (#14526) (cherry picked from commit 66a4c94a1ed542e69fe6399bab4c01d4540486cf) --- superset-frontend/src/dashboard/actions/hydrate.js | 31 ++----------- .../dashboard/components/nativeFilters/utils.ts | 3 +- .../src/dashboard/util/extractUrlParams.test.ts | 53 ++++++++++++++++++++++ .../src/dashboard/util/extractUrlParams.ts | 49 ++++++++++++++++++++ superset/views/utils.py | 10 +++- tests/utils_tests.py | 20 ++++++++ 6 files changed, 137 insertions(+), 29 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 9e4e2b0..64eaeca 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -24,7 +24,6 @@ import { CategoricalColorNamespace, getChartMetadataRegistry, } from '@superset-ui/core'; -import querystring from 'query-string'; import { chart } from 'src/chart/chartReducer'; import { initSliceEntities } from 'src/dashboard/reducers/sliceEntities'; @@ -55,27 +54,7 @@ import getLocationHash from 'src/dashboard/util/getLocationHash'; import newComponentFactory from 'src/dashboard/util/newComponentFactory'; import { TIME_RANGE } from 'src/visualizations/FilterBox/FilterBox'; import { FeatureFlag, isFeatureEnabled } from '../../featureFlags'; - -const reservedQueryParams = new Set(['standalone', 'edit']); - -/** - * Returns the url params that are used to customize queries - * in datasets built using sql lab. - * We may want to extract this to some kind of util in the future. - */ -const extractUrlParams = queryParams => - Object.entries(queryParams).reduce((acc, [key, value]) => { - if (reservedQueryParams.has(key)) return acc; - // if multiple url params share the same key (?foo=bar&foo=baz), they will appear as an array. - // Only one value can be used for a given query param, so we just take the first one. - if (Array.isArray(value)) { - return { - ...acc, - [key]: value[0], - }; - } - return { ...acc, [key]: value }; - }, {}); +import extractUrlParams from '../util/extractUrlParams'; export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD'; @@ -85,9 +64,9 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => ( ) => { const { user, common } = getState(); let { metadata } = dashboardData; - const queryParams = querystring.parse(window.location.search); - const urlParams = extractUrlParams(queryParams); - const editMode = queryParams.edit === 'true'; + const regularUrlParams = extractUrlParams('regular'); + const reservedUrlParams = extractUrlParams('reserved'); + const editMode = reservedUrlParams.edit === 'true'; let preselectFilters = {}; @@ -154,7 +133,7 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => ( ...slice.form_data, url_params: { ...slice.form_data.url_params, - ...urlParams, + ...regularUrlParams, }, }; chartQueries[key] = { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index a42540a..a674a27 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -28,6 +28,7 @@ import { import { Charts } from 'src/dashboard/types'; import { RefObject } from 'react'; import { DataMaskStateWithId } from 'src/dataMask/types'; +import extractUrlParams from 'src/dashboard/util/extractUrlParams'; import { Filter } from './types'; export const getFormData = ({ @@ -76,7 +77,7 @@ export const getFormData = ({ defaultValue: defaultDataMask?.filterState?.value, time_range, time_range_endpoints: ['inclusive', 'exclusive'], - url_params: {}, + url_params: extractUrlParams('regular'), viz_type: filterType, inputRef, }; diff --git a/superset-frontend/src/dashboard/util/extractUrlParams.test.ts b/superset-frontend/src/dashboard/util/extractUrlParams.test.ts new file mode 100644 index 0000000..dd81c10 --- /dev/null +++ b/superset-frontend/src/dashboard/util/extractUrlParams.test.ts @@ -0,0 +1,53 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import extractUrlParams from './extractUrlParams'; + +const originalWindowLocation = window.location; + +describe('extractUrlParams', () => { + beforeAll(() => { + // @ts-ignore + delete window.location; + // @ts-ignore + window.location = { search: '?edit=true&abc=123' }; + }); + + afterAll(() => { + window.location = originalWindowLocation; + }); + + it('returns all urlParams', () => { + expect(extractUrlParams('all')).toEqual({ + edit: 'true', + abc: '123', + }); + }); + + it('returns reserved urlParams', () => { + expect(extractUrlParams('reserved')).toEqual({ + edit: 'true', + }); + }); + + it('returns regular urlParams', () => { + expect(extractUrlParams('regular')).toEqual({ + abc: '123', + }); + }); +}); diff --git a/superset-frontend/src/dashboard/util/extractUrlParams.ts b/superset-frontend/src/dashboard/util/extractUrlParams.ts new file mode 100644 index 0000000..7ef7dd9 --- /dev/null +++ b/superset-frontend/src/dashboard/util/extractUrlParams.ts @@ -0,0 +1,49 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import querystring from 'query-string'; +import { JsonObject } from '@superset-ui/core'; + +const reservedQueryParams = new Set(['standalone', 'edit']); + +export type UrlParamType = 'reserved' | 'regular' | 'all'; + +/** + * Returns the url params that are used to customize queries + */ +export default function extractUrlParams( + urlParamType: UrlParamType, +): JsonObject { + const queryParams = querystring.parse(window.location.search); + return Object.entries(queryParams).reduce((acc, [key, value]) => { + if ( + (urlParamType === 'regular' && reservedQueryParams.has(key)) || + (urlParamType === 'reserved' && !reservedQueryParams.has(key)) + ) + return acc; + // if multiple url params share the same key (?foo=bar&foo=baz), they will appear as an array. + // Only one value can be used for a given query param, so we just take the first one. + if (Array.isArray(value)) { + return { + ...acc, + [key]: value[0], + }; + } + return { ...acc, [key]: value }; + }, {}); +} diff --git a/superset/views/utils.py b/superset/views/utils.py index f45aa04..3c9021c 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -129,7 +129,7 @@ def loads_request_json(request_json_data: str) -> Dict[Any, Any]: return {} -def get_form_data( +def get_form_data( # pylint: disable=too-many-locals slice_id: Optional[int] = None, use_slice_data: bool = False ) -> Tuple[Dict[str, Any], Optional[Slice]]: form_data = {} @@ -144,7 +144,13 @@ def get_form_data( if request_json_data: form_data.update(request_json_data) if request_form_data: - form_data.update(loads_request_json(request_form_data)) + parsed_form_data = loads_request_json(request_form_data) + # some chart data api requests are form_data + queries = parsed_form_data.get("queries") + if isinstance(queries, list): + form_data.update(queries[0]) + else: + form_data.update(parsed_form_data) # request params can overwrite the body if request_args_data: form_data.update(loads_request_json(request_args_data)) diff --git a/tests/utils_tests.py b/tests/utils_tests.py index c054624..5781965 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -1015,6 +1015,26 @@ class TestUtils(SupersetTestCase): self.assertEqual(slc, None) + def test_get_form_data_request_form_with_queries(self) -> None: + # the CSV export uses for requests, even when sending requests to + # /api/v1/chart/data + with app.test_request_context( + data={ + "form_data": json.dumps({"queries": [{"url_params": {"foo": "bar"}}]}) + } + ): + form_data, slc = get_form_data() + + self.assertEqual( + form_data, + { + "url_params": {"foo": "bar"}, + "time_range_endpoints": get_time_range_endpoints(form_data={}), + }, + ) + + self.assertEqual(slc, None) + def test_get_form_data_request_args_and_form(self) -> None: with app.test_request_context( data={"form_data": json.dumps({"foo": "bar"})},
