You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jo...@apache.org on 2022/10/12 22:52:51 UTC
[superset] branch master updated: fix: Preserve unknown URL params (#21785)
This is an automated email from the ASF dual-hosted git repository.
johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 11d7d6e078 fix: Preserve unknown URL params (#21785)
11d7d6e078 is described below
commit 11d7d6e078b75079c432d8d8028dac45678b2c37
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Wed Oct 12 19:52:36 2022 -0300
fix: Preserve unknown URL params (#21785)
---
.../ExploreViewContainer.test.tsx | 32 ++++++++++++++++------
.../components/ExploreViewContainer/index.jsx | 4 ++-
.../exploreUtils/getParsedExploreURLParams.test.ts | 2 +-
.../exploreUtils/getParsedExploreURLParams.ts | 5 ++--
superset/models/slice.py | 3 +-
5 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
index 50e8bd98f9..a4ad594387 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 a2843b7ca6..8b99decbad 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 5039d3421f..58a10c21c1 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 f09a363858..0340dded75 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 d644e7b747..32b347266d 100644
--- a/superset/models/slice.py
+++ b/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: