fix(chart-data): handle url_params in csv export and native filters (#14526)

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] = {

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,
   };

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',
+    });
+  });
+});

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 };
+  }, {});
+}

superset/views/utils.py

@@ -124,7 +124,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 = {}
@@ -139,7 +139,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))

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"})},