From e58a3aba545fd03f2af33b0075c4cacf09f776a3 Mon Sep 17 00:00:00 2001
From: "Hugh A. Miles II" <hughmil3s@gmail.com>
Date: Mon, 16 Oct 2023 14:00:09 -0400
Subject: [PATCH] fix: permalink save/overwrites in explore (#25112)

Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
---
 .../src/explore/components/SaveModal.test.jsx | 35 +++++++++++++++++--
 .../src/explore/components/SaveModal.tsx      | 23 +++++++-----
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/superset-frontend/src/explore/components/SaveModal.test.jsx b/superset-frontend/src/explore/components/SaveModal.test.jsx
index bdb93e542935d..29bb278269e19 100644
--- a/superset-frontend/src/explore/components/SaveModal.test.jsx
+++ b/superset-frontend/src/explore/components/SaveModal.test.jsx
@@ -27,7 +27,10 @@ import Button from 'src/components/Button';
 import fetchMock from 'fetch-mock';
 
 import * as saveModalActions from 'src/explore/actions/saveModalActions';
-import SaveModal, { StyledModal } from 'src/explore/components/SaveModal';
+import SaveModal, {
+  PureSaveModal,
+  StyledModal,
+} from 'src/explore/components/SaveModal';
 import { BrowserRouter } from 'react-router-dom';
 
 const middlewares = [thunk];
@@ -100,8 +103,12 @@ const queryDefaultProps = {
 };
 
 const fetchDashboardsEndpoint = `glob:*/dashboardasync/api/read?_flt_0_owners=${1}`;
+const fetchChartEndpoint = `glob:*/api/v1/chart/${1}*`;
 
-beforeAll(() => fetchMock.get(fetchDashboardsEndpoint, mockDashboardData));
+beforeAll(() => {
+  fetchMock.get(fetchDashboardsEndpoint, mockDashboardData);
+  fetchMock.get(fetchChartEndpoint, { id: 1, dashboards: [1] });
+});
 
 afterAll(() => fetchMock.restore());
 
@@ -226,3 +233,27 @@ test('set dataset name when chart source is query', () => {
   expect(wrapper.find('[data-test="new-dataset-name"]')).toExist();
   expect(wrapper.state().datasetName).toBe('test');
 });
+
+test('make sure slice_id in the URLSearchParams before the redirect', () => {
+  const myProps = {
+    ...defaultProps,
+    slice: { slice_id: 1, slice_name: 'title', owners: [1] },
+    actions: {
+      setFormData: jest.fn(),
+      updateSlice: jest.fn(() => Promise.resolve({ id: 1 })),
+      getSliceDashboards: jest.fn(),
+    },
+    user: { userId: 1 },
+    history: {
+      replace: jest.fn(),
+    },
+    dispatch: jest.fn(),
+  };
+
+  const saveModal = new PureSaveModal(myProps);
+  const result = saveModal.handleRedirect(
+    'https://example.com/?name=John&age=30',
+    { id: 1 },
+  );
+  expect(result.get('slice_id')).toEqual('1');
+});
diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx
index d390e4638894c..ed5b244cd70cf 100644
--- a/superset-frontend/src/explore/components/SaveModal.tsx
+++ b/superset-frontend/src/explore/components/SaveModal.tsx
@@ -164,6 +164,17 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
     this.props.dispatch(setSaveChartModalVisibility(false));
   }
 
+  handleRedirect = (windowLocationSearch: string, chart: any) => {
+    const searchParams = new URLSearchParams(windowLocationSearch);
+    searchParams.set('save_action', this.state.action);
+    if (this.state.action !== 'overwrite') {
+      searchParams.delete('form_data_key');
+    }
+
+    searchParams.set('slice_id', chart.id.toString());
+    return searchParams;
+  };
+
   async saveOrOverwrite(gotodash: boolean) {
     this.setState({ isLoading: true });
 
@@ -270,14 +281,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
         return;
       }
 
-      const searchParams = new URLSearchParams(window.location.search);
-      searchParams.set('save_action', this.state.action);
-      if (this.state.action !== 'overwrite') {
-        searchParams.delete('form_data_key');
-      }
-      if (this.state.action === 'saveas') {
-        searchParams.set('slice_id', value.id.toString());
-      }
+      const searchParams = this.handleRedirect(window.location.search, value);
       this.props.history.replace(`/explore/?${searchParams.toString()}`);
 
       this.setState({ isLoading: false });
@@ -527,3 +531,6 @@ function mapStateToProps({
 }
 
 export default withRouter(connect(mapStateToProps)(SaveModal));
+
+// User for testing purposes need to revisit once we convert this to functional component
+export { SaveModal as PureSaveModal };