[explore view] inline edit slice name should not overwrite (#9817)

superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx

@@ -23,12 +23,12 @@ import { ExploreChartHeader } from 'src/explore/components/ExploreChartHeader';
 import ExploreActionButtons from 'src/explore/components/ExploreActionButtons';
 import EditableTitle from 'src/components/EditableTitle';
 
-const stub = jest.fn(() => ({
-  then: () => {},
-}));
+const saveSliceStub = jest.fn();
+const updateChartTitleStub = jest.fn();
 const mockProps = {
   actions: {
-    saveSlice: stub,
+    saveSlice: saveSliceStub,
+    updateChartTitle: updateChartTitleStub,
   },
   can_overwrite: true,
   can_download: true,
@@ -65,24 +65,9 @@ describe('ExploreChartHeader', () => {
     expect(wrapper.find(ExploreActionButtons)).toHaveLength(1);
   });
 
-  it('should updateChartTitleOrSaveSlice for existed slice', () => {
+  it('should update title but not save', () => {
     const newTitle = 'New Chart Title';
-    wrapper.instance().updateChartTitleOrSaveSlice(newTitle);
-    expect(stub.call).toHaveLength(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).toHaveLength(1);
-    expect(stub).toHaveBeenCalledWith(mockProps.form_data, {
-      action: 'saveas',
-      slice_name: newTitle,
-    });
+    const editableTitle = wrapper.find(EditableTitle);
+    expect(editableTitle.props().onSaveTitle).toBe(updateChartTitleStub);
   });
 });

superset-frontend/src/explore/actions/exploreActions.js

@@ -124,8 +124,8 @@ export function removeControlPanelAlert() {
 }
 
 export const UPDATE_CHART_TITLE = 'UPDATE_CHART_TITLE';
-export function updateChartTitle(slice_name) {
-  return { type: UPDATE_CHART_TITLE, slice_name };
+export function updateChartTitle(sliceName) {
+  return { type: UPDATE_CHART_TITLE, sliceName };
 }
 
 export const CREATE_NEW_SLICE = 'CREATE_NEW_SLICE';

superset-frontend/src/explore/components/ExploreChartHeader.jsx

@@ -47,6 +47,7 @@ const propTypes = {
   can_download: PropTypes.bool.isRequired,
   isStarred: PropTypes.bool.isRequired,
   slice: PropTypes.object,
+  sliceName: PropTypes.string,
   table_name: PropTypes.string,
   form_data: PropTypes.object,
   timeout: PropTypes.number,
@@ -63,6 +64,10 @@ export class ExploreChartHeader extends React.PureComponent {
     this.closePropertiesModal = this.closePropertiesModal.bind(this);
   }
 
+  getSliceName() {
+    return this.props.sliceName || t('%s - untitled', this.props.table_name);
+  }
+
   postChartFormData() {
     this.props.actions.postChartFormData(
       this.props.form_data,
@@ -72,40 +77,6 @@ export 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.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);
-        this.props.actions.createNewSlice(
-          data.can_add,
-          data.can_download,
-          data.can_overwrite,
-          data.slice,
-          data.form_data,
-        );
-        this.props.addHistory({
-          isReplace: true,
-          title: `[chart] ${data.slice.slice_name}`,
-        });
-      } else {
-        this.props.actions.updateChartTitle(newTitle);
-      }
-    });
-  }
-
   openProperiesModal() {
     this.setState({
       isPropertiesModalOpen: true,
@@ -118,16 +89,6 @@ export class ExploreChartHeader extends React.PureComponent {
     });
   }
 
-  renderChartTitle() {
-    let title;
-    if (this.props.slice) {
-      title = this.props.slice.slice_name;
-    } else {
-      title = t('%s - untitled', this.props.table_name);
-    }
-    return title;
-  }
-
   render() {
     const formData = this.props.form_data;
     const {
@@ -143,9 +104,9 @@ export class ExploreChartHeader extends React.PureComponent {
     return (
       <div id="slice-header" className="clearfix panel-title-large">
         <EditableTitle
-          title={this.renderChartTitle()}
+          title={this.getSliceName()}
           canEdit={!this.props.slice || this.props.can_overwrite}
-          onSaveTitle={this.updateChartTitleOrSaveSlice.bind(this)}
+          onSaveTitle={this.props.actions.updateChartTitle}
         />
 
         {this.props.slice && (

superset-frontend/src/explore/components/ExploreChartPanel.jsx

@@ -37,6 +37,7 @@ const propTypes = {
   width: PropTypes.string.isRequired,
   isStarred: PropTypes.bool.isRequired,
   slice: PropTypes.object,
+  sliceName: PropTypes.string,
   table_name: PropTypes.string,
   vizType: PropTypes.string.isRequired,
   form_data: PropTypes.object,
@@ -103,6 +104,7 @@ class ExploreChartPanel extends React.PureComponent {
         can_download={this.props.can_download}
         isStarred={this.props.isStarred}
         slice={this.props.slice}
+        sliceName={this.props.sliceName}
         table_name={this.props.table_name}
         form_data={this.props.form_data}
         timeout={this.props.timeout}

superset-frontend/src/explore/components/ExploreViewContainer.jsx

@@ -61,6 +61,7 @@ const propTypes = {
   isDatasourceMetaLoading: PropTypes.bool.isRequired,
   chart: chartPropShape.isRequired,
   slice: PropTypes.object,
+  sliceName: PropTypes.string,
   controls: PropTypes.object.isRequired,
   forcedHeight: PropTypes.string,
   form_data: PropTypes.object.isRequired,
@@ -335,6 +336,7 @@ class ExploreViewContainer extends React.Component {
             onHide={this.toggleModal}
             actions={this.props.actions}
             form_data={this.props.form_data}
+            sliceName={this.props.sliceName}
           />
         )}
         <div className="row">
@@ -403,6 +405,7 @@ function mapStateToProps(state) {
       : 'slice-container',
     isStarred: explore.isStarred,
     slice: explore.slice,
+    sliceName: explore.sliceName,
     triggerRender: explore.triggerRender,
     form_data,
     table_name: form_data.datasource_name,

superset-frontend/src/explore/components/SaveModal.jsx

@@ -45,7 +45,7 @@ class SaveModal extends React.Component {
     this.state = {
       saveToDashboardId: null,
       newDashboardName: '',
-      newSliceName: '',
+      newSliceName: props.sliceName,
       dashboards: [],
       alert: null,
       action: props.can_overwrite ? 'overwrite' : 'saveas',
@@ -100,21 +100,17 @@ class SaveModal extends React.Component {
     this.props.actions.removeSaveModalAlert();
     const sliceParams = {};
 
-    let sliceName = null;
-    sliceParams.action = this.state.action;
     if (this.props.slice && this.props.slice.slice_id) {
       sliceParams.slice_id = this.props.slice.slice_id;
     }
     if (sliceParams.action === 'saveas') {
-      sliceName = this.state.newSliceName;
-      if (sliceName === '') {
+      if (this.state.newSliceName === '') {
         this.setState({ alert: t('Please enter a chart name') });
         return;
       }
-      sliceParams.slice_name = sliceName;
-    } else {
-      sliceParams.slice_name = this.props.slice.slice_name;
     }
+    sliceParams.action = this.state.action;
+    sliceParams.slice_name = this.state.newSliceName;
 
     const addToDash = this.state.addToDash;
     sliceParams.add_to_dash = addToDash;
@@ -208,7 +204,7 @@ class SaveModal extends React.Component {
           </Radio>
           <input
             name="new_slice_name"
-            placeholder={t('[chart name]')}
+            placeholder={this.state.newSliceName || t('[chart name]')}
             onChange={this.onChange.bind(this, 'newSliceName')}
             onFocus={this.changeAction.bind(this, 'saveas')}
           />

superset-frontend/src/explore/reducers/exploreReducer.js

@@ -131,10 +131,9 @@ export default function exploreReducer(state = {}, action) {
       };
     },
     [actions.UPDATE_CHART_TITLE]() {
-      const updatedSlice = { ...state.slice, slice_name: action.slice_name };
       return {
         ...state,
-        slice: updatedSlice,
+        sliceName: action.sliceName,
       };
     },
     [actions.RESET_FIELDS]() {

superset-frontend/src/explore/reducers/getInitialState.js

@@ -41,6 +41,7 @@ export default function getInitialState(bootstrapData) {
   };
 
   const slice = bootstrappedState.slice;
+  const sliceName = slice ? slice.slice_name : null;
 
   const sliceFormData = slice
     ? getFormDataFromControls(getControlsState(bootstrapData, slice.form_data))
@@ -68,7 +69,10 @@ export default function getInitialState(bootstrapData) {
       dashboards: [],
       saveModalAlert: null,
     },
-    explore: bootstrappedState,
+    explore: {
+      ...bootstrappedState,
+      sliceName,
+    },
     impressionId: shortid.generate(),
     messageToasts: getToastsFromPyFlashMessages(
       (bootstrapData.common || {}).flash_messages || [],