fix: permalink save/overwrites in explore (apache#25112)

hughhhh · eschutho · web-flow · commit e58a3aba545f · 2023-10-16T14:00:09.000-04:00
Co-authored-by: Elizabeth Thompson &lt;eschutho@gmail.com&gt;
diff --git 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
@@ -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 };
