fix: Preserve unknown URL params (#21785)

superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx

@@ -75,7 +75,8 @@ const reduxState = {
   },
 };
 
-const key = 'aWrs7w29sd';
+const KEY = 'aWrs7w29sd';
+const SEARCH = `?form_data_key=${KEY}&dataset_id=1`;
 
 jest.mock('react-resize-detector', () => ({
   __esModule: true,
@@ -87,21 +88,20 @@ jest.mock('lodash/debounce', () => ({
   default: (fuc: Function) => fuc,
 }));
 
-fetchMock.post('glob:*/api/v1/explore/form_data*', { key });
-fetchMock.put('glob:*/api/v1/explore/form_data*', { key });
+fetchMock.post('glob:*/api/v1/explore/form_data*', { key: KEY });
+fetchMock.put('glob:*/api/v1/explore/form_data*', { key: KEY });
 fetchMock.get('glob:*/api/v1/explore/form_data*', {});
 fetchMock.get('glob:*/favstar/slice*', { count: 0 });
 
 const defaultPath = '/explore/';
 const renderWithRouter = ({
-  withKey,
+  search = '',
   overridePathname,
 }: {
-  withKey?: boolean;
+  search?: string;
   overridePathname?: string;
 } = {}) => {
   const path = overridePathname ?? defaultPath;
-  const search = withKey ? `?form_data_key=${key}&dataset_id=1` : '';
   Object.defineProperty(window, 'location', {
     get() {
       return { pathname: path, search };
@@ -143,12 +143,12 @@ test('generates a new form_data param when none is available', async () => {
 
 test('generates a different form_data param when one is provided and is mounting', async () => {
   const replaceState = jest.spyOn(window.history, 'replaceState');
-  await waitFor(() => renderWithRouter({ withKey: true }));
+  await waitFor(() => renderWithRouter({ search: SEARCH }));
   expect(replaceState).not.toHaveBeenLastCalledWith(
     0,
     expect.anything(),
     undefined,
-    expect.stringMatching(key),
+    expect.stringMatching(KEY),
   );
   expect(replaceState).toHaveBeenCalledWith(
     expect.anything(),
@@ -164,7 +164,7 @@ test('reuses the same form_data param when updating', async () => {
   });
   const replaceState = jest.spyOn(window.history, 'replaceState');
   const pushState = jest.spyOn(window.history, 'pushState');
-  await waitFor(() => renderWithRouter({ withKey: true }));
+  await waitFor(() => renderWithRouter({ search: SEARCH }));
   expect(replaceState.mock.calls.length).toBe(1);
   userEvent.click(screen.getByText('Update chart'));
   await waitFor(() => expect(pushState.mock.calls.length).toBe(1));
@@ -188,3 +188,17 @@ test('doesnt call replaceState when pathname is not /explore', async () => {
   expect(replaceState).not.toHaveBeenCalled();
   replaceState.mockRestore();
 });
+
+test('preserves unknown parameters', async () => {
+  const replaceState = jest.spyOn(window.history, 'replaceState');
+  const unknownParam = 'test=123';
+  await waitFor(() =>
+    renderWithRouter({ search: `${SEARCH}&${unknownParam}` }),
+  );
+  expect(replaceState).toHaveBeenCalledWith(
+    expect.anything(),
+    undefined,
+    expect.stringMatching(unknownParam),
+  );
+  replaceState.mockRestore();
+});

superset-frontend/src/explore/components/ExploreViewContainer/index.jsx

@@ -167,7 +167,9 @@ const updateHistory = debounce(
   ) => {
     const payload = { ...formData };
     const chartId = formData.slice_id;
-    const additionalParam = {};
+    const params = new URLSearchParams(window.location.search);
+    const additionalParam = Object.fromEntries(params);
+
     if (chartId) {
       additionalParam[URL_PARAMS.sliceId.name] = chartId;
     } else {

superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts

@@ -31,7 +31,7 @@ test('get form_data_key and slice_id from search params - url when moving from d
     `${EXPLORE_BASE_URL}?form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd&slice_id=56`,
   );
   expect(getParsedExploreURLParams().toString()).toEqual(
-    'slice_id=56&form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd',
+    'form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd&slice_id=56',
   );
 });
 

superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts

@@ -77,7 +77,7 @@ const EXPLORE_URL_PATH_PARAMS = {
 // we need to "flatten" the search params to use them with /v1/explore endpoint
 const getParsedExploreURLSearchParams = (search: string) => {
   const urlSearchParams = new URLSearchParams(search);
-  return Object.keys(EXPLORE_URL_SEARCH_PARAMS).reduce((acc, currentParam) => {
+  return Array.from(urlSearchParams.keys()).reduce((acc, currentParam) => {
     const paramValue = urlSearchParams.get(currentParam);
     if (paramValue === null) {
       return acc;
@@ -93,9 +93,10 @@ const getParsedExploreURLSearchParams = (search: string) => {
     if (typeof parsedParamValue === 'object') {
       return { ...acc, ...parsedParamValue };
     }
+    const key = EXPLORE_URL_SEARCH_PARAMS[currentParam]?.name || currentParam;
     return {
       ...acc,
-      [EXPLORE_URL_SEARCH_PARAMS[currentParam].name]: parsedParamValue,
+      [key]: parsedParamValue,
     };
   }, {});
 };

superset/models/slice.py

@@ -334,8 +334,7 @@ class Slice(  # pylint: disable=too-many-public-methods
 
     @property
     def url(self) -> str:
-        form_data = f"%7B%22slice_id%22%3A%20{self.id}%7D"
-        return f"/explore/?slice_id={self.id}&form_data={form_data}"
+        return f"/explore/?slice_id={self.id}"
 
     def get_query_context_factory(self) -> QueryContextFactory:
         if self.query_context_factory is None: