diff --git a/superset/assets/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx b/superset/assets/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx index ddc107ab5ecfc..ec7278ffd8e4e 100644 --- a/superset/assets/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx @@ -23,14 +23,25 @@ import ExploreChartHeader from '../../../../src/explore/components/ExploreChartH import ExploreActionButtons from '../../../../src/explore/components/ExploreActionButtons'; import EditableTitle from '../../../../src/components/EditableTitle'; +const stub = jest.fn(() => ({ + then: () => {}, +})); const mockProps = { - actions: {}, + actions: { + saveSlice: stub, + }, can_overwrite: true, can_download: true, isStarred: true, - slice: {}, + slice: { + form_data: { + viz_type: 'line', + }, + }, table_name: 'foo', - form_data: {}, + form_data: { + viz_type: 'table', + }, timeout: 1000, chart: { queryResponse: {}, @@ -53,4 +64,25 @@ describe('ExploreChartHeader', () => { expect(wrapper.find(EditableTitle)).toHaveLength(1); expect(wrapper.find(ExploreActionButtons)).toHaveLength(1); }); + + it('should updateChartTitleOrSaveSlice for existed slice', () => { + const newTitle = 'New Chart Title'; + wrapper.instance().updateChartTitleOrSaveSlice(newTitle); + expect(stub.call.length).toEqual(1); + expect(stub).toHaveBeenCalledWith(mockProps.slice.form_data, { + action: 'overwrite', + slice_name: newTitle, + }); + }); + + it('should updateChartTitleOrSaveSlice for new slice', () => { + const newTitle = 'New Chart Title'; + wrapper.setProps({ slice: undefined }); + wrapper.instance().updateChartTitleOrSaveSlice(newTitle); + expect(stub.call.length).toEqual(1); + expect(stub).toHaveBeenCalledWith(mockProps.form_data, { + action: 'saveas', + slice_name: newTitle, + }); + }); }); diff --git a/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx b/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx index 1353e0e938efb..316c76e0ef690 100644 --- a/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx @@ -146,7 +146,10 @@ describe('SaveModal', () => { sinon.stub(defaultProps.actions, 'saveSlice').callsFake(() => Promise.resolve({ - data: { dashboard: '/mock/', slice: { slice_url: '/mock/' } }, + data: { + dashboard: '/mock_dashboard/', + slice: { slice_url: '/mock_slice/' }, + }, }), ); }); @@ -190,6 +193,52 @@ describe('SaveModal', () => { const args = defaultProps.actions.saveSlice.getCall(0).args; expect(args[1].new_dashboard_name).toBe(newDashboardName); }); + + describe('should always reload or redirect', () => { + let wrapper; + beforeEach(() => { + wrapper = getWrapper(); + sinon.stub(window.location, 'assign'); + }); + afterEach(() => { + window.location.assign.restore(); + }); + + it('Save & go to dashboard', done => { + wrapper.instance().saveOrOverwrite(true); + defaultProps.actions.saveSlice().then(() => { + expect(window.location.assign.callCount).toEqual(1); + expect(window.location.assign.getCall(0).args[0]).toEqual( + 'http://localhost/mock_dashboard/', + ); + done(); + }); + }); + + it('saveas new slice', done => { + wrapper.setState({ action: 'saveas', newSliceName: 'new slice name' }); + wrapper.instance().saveOrOverwrite(false); + defaultProps.actions.saveSlice().then(() => { + expect(window.location.assign.callCount).toEqual(1); + expect(window.location.assign.getCall(0).args[0]).toEqual( + '/mock_slice/', + ); + done(); + }); + }); + + it('overwrite original slice', done => { + wrapper.setState({ action: 'overwrite' }); + wrapper.instance().saveOrOverwrite(false); + defaultProps.actions.saveSlice().then(() => { + expect(window.location.assign.callCount).toEqual(1); + expect(window.location.assign.getCall(0).args[0]).toEqual( + '/mock_slice/', + ); + done(); + }); + }); + }); }); describe('fetchDashboards', () => { diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index d4d5967c0e6b0..11989c6e397f5 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -61,11 +61,18 @@ class ExploreChartHeader extends React.PureComponent { updateChartTitleOrSaveSlice(newTitle) { const isNewSlice = !this.props.slice; + const currentFormData = isNewSlice + ? this.props.form_data + : this.props.slice.form_data; + const params = { slice_name: newTitle, action: isNewSlice ? 'saveas' : 'overwrite', }; - this.props.actions.saveSlice(this.props.form_data, params).then(json => { + // this.props.slice hold the original slice params stored in slices table + // when chart is saved or overwritten, the explore view will reload page + // to make sure sync with updated query params + this.props.actions.saveSlice(currentFormData, params).then(json => { const { data } = json; if (isNewSlice) { this.props.actions.updateChartId(data.slice.slice_id, 0); diff --git a/superset/assets/src/explore/components/SaveModal.jsx b/superset/assets/src/explore/components/SaveModal.jsx index 609b448e4110e..0cd8bad5e8f22 100644 --- a/superset/assets/src/explore/components/SaveModal.jsx +++ b/superset/assets/src/explore/components/SaveModal.jsx @@ -154,9 +154,9 @@ class SaveModal extends React.Component { } // Go to new slice url or dashboard url if (gotodash) { - window.location = supersetURL(data.dashboard); + window.location.assign(supersetURL(data.dashboard)); } else { - window.location = data.slice.slice_url; + window.location.assign(data.slice.slice_url); } }); this.props.onHide();