Skip to content

Commit

Permalink
fix: permalink save/overwrites in explore (apache#25112)
Browse files Browse the repository at this point in the history
Co-authored-by: Elizabeth Thompson <[email protected]>
  • Loading branch information
hughhhh and eschutho authored Oct 16, 2023
1 parent 3c89c7e commit 506415b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 10 deletions.
35 changes: 33 additions & 2 deletions superset-frontend/src/explore/components/SaveModal.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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');
});
23 changes: 15 additions & 8 deletions superset-frontend/src/explore/components/SaveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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 };

0 comments on commit 506415b

Please sign in to comment.