[superset] branch master updated: fix(explore): Replace url search params only if current page is Explore (#20972)

[kg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kgabryje 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 9350bbafee fix(explore): Replace url search params only if current page is Explore (#20972)
9350bbafee is described below

commit 9350bbafeeef0f4f3fa43a2068e0ea87afa55fca
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Aug 5 16:59:52 2022 +0200

    fix(explore): Replace url search params only if current page is Explore (#20972)
    
    * fix(explore): Replace url search params only if current page is Explore
    
    * Add unit test
---
 .../src/explore/components/ExploreChartPanel.jsx   |  2 +-
 .../ExploreViewContainer.test.tsx                  | 35 +++++++++++++++++++---
 .../components/ExploreViewContainer/index.jsx      | 21 +++++++------
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index db97db2452..5e97cb4f4f 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -153,7 +153,7 @@ const ExploreChartPanel = ({
   const [showDatasetModal, setShowDatasetModal] = useState(false);
 
   const metaDataRegistry = getChartMetadataRegistry();
-  const { useLegacyApi } = metaDataRegistry.get(vizType);
+  const { useLegacyApi } = metaDataRegistry.get(vizType) ?? {};
   const vizTypeNeedsDataset =
     useLegacyApi && datasource.type !== DatasourceType.Table;
   // added boolean column to below show boolean so that the errors aren't overlapping
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
index 09a217f102..4fc78594a2 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
@@ -84,9 +84,21 @@ fetchMock.put('glob:*/api/v1/explore/form_data*', { key });
 fetchMock.get('glob:*/api/v1/explore/form_data*', {});
 fetchMock.get('glob:*/favstar/slice*', { count: 0 });
 
-const renderWithRouter = (withKey?: boolean) => {
-  const path = '/explore/';
+const defaultPath = '/explore/';
+const renderWithRouter = ({
+  withKey,
+  overridePathname,
+}: {
+  withKey?: boolean;
+  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 };
+    },
+  });
   return render(
     <MemoryRouter initialEntries={[`${path}${search}`]}>
       <Route path={path}>
@@ -123,7 +135,7 @@ 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(true));
+  await waitFor(() => renderWithRouter({ withKey: true }));
   expect(replaceState).not.toHaveBeenLastCalledWith(
     0,
     expect.anything(),
@@ -144,7 +156,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());
+  await waitFor(() => renderWithRouter({ withKey: true }));
   expect(replaceState.mock.calls.length).toBe(1);
   userEvent.click(screen.getByText('Update chart'));
   await waitFor(() => expect(pushState.mock.calls.length).toBe(1));
@@ -153,3 +165,18 @@ test('reuses the same form_data param when updating', async () => {
   pushState.mockRestore();
   getChartControlPanelRegistry().remove('table');
 });
+
+test('doesnt call replaceState when pathname is not /explore', async () => {
+  getChartMetadataRegistry().registerValue(
+    'table',
+    new ChartMetadata({
+      name: 'fake table',
+      thumbnail: '.png',
+      useLegacyApi: false,
+    }),
+  );
+  const replaceState = jest.spyOn(window.history, 'replaceState');
+  await waitFor(() => renderWithRouter({ overridePathname: '/dashboard' }));
+  expect(replaceState).not.toHaveBeenCalled();
+  replaceState.mockRestore();
+});
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index a2c0e1d6ee..7f6a870039 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -206,15 +206,18 @@ const updateHistory = debounce(
         );
         stateModifier = 'pushState';
       }
-      const url = mountExploreUrl(
-        standalone ? URL_PARAMS.standalone.name : null,
-        {
-          [URL_PARAMS.formDataKey.name]: key,
-          ...additionalParam,
-        },
-        force,
-      );
-      window.history[stateModifier](payload, title, url);
+      // avoid race condition in case user changes route before explore updates the url
+      if (window.location.pathname.startsWith('/explore')) {
+        const url = mountExploreUrl(
+          standalone ? URL_PARAMS.standalone.name : null,
+          {
+            [URL_PARAMS.formDataKey.name]: key,
+            ...additionalParam,
+          },
+          force,
+        );
+        window.history[stateModifier](payload, title, url);
+      }
     } catch (e) {
       logging.warn('Failed at altering browser history', e);
     }