diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx index 50e8bd98f922..a4ad59438743 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx +++ b/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(); +}); diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index a2843b7ca65c..8b99decbad5f 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/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 { diff --git a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts index 5039d3421fb1..58a10c21c151 100644 --- a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts +++ b/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', ); }); diff --git a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts index f09a36385802..0340dded75b2 100644 --- a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts +++ b/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, }; }, {}); }; diff --git a/superset/models/slice.py b/superset/models/slice.py index d644e7b7472e..32b347266d2b 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -334,8 +334,7 @@ def icons(self) -> str: @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: