From a78a85edccaaaa249791089332e261873cb52b18 Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <michael.s.molina@gmail.com>
Date: Tue, 1 Aug 2023 16:10:58 -0300
Subject: [PATCH] fix: Explore misleading save action

---
 .../src/explore/actions/saveModalActions.js   |   5 -
 .../src/explore/components/SaveModal.tsx      | 205 +++++++++---------
 .../src/explore/reducers/saveModalReducer.js  |   3 -
 3 files changed, 107 insertions(+), 106 deletions(-)

diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
index de4f7af84c37e..1a9970ab7582e 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.js
+++ b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -49,11 +49,6 @@ export function saveSliceSuccess(data) {
   return { type: SAVE_SLICE_SUCCESS, data };
 }
 
-export const REMOVE_SAVE_MODAL_ALERT = 'REMOVE_SAVE_MODAL_ALERT';
-export function removeSaveModalAlert() {
-  return { type: REMOVE_SAVE_MODAL_ALERT };
-}
-
 const extractAddHocFiltersFromFormData = formDataToHandle =>
   Object.entries(formDataToHandle).reduce(
     (acc, [key, value]) =>
diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx
index 592bab2f9f964..3142d6b75892a 100644
--- a/superset-frontend/src/explore/components/SaveModal.tsx
+++ b/superset-frontend/src/explore/components/SaveModal.tsx
@@ -67,7 +67,6 @@ interface SaveModalProps extends RouteComponentProps {
 type SaveModalState = {
   newSliceName?: string;
   datasetName: string;
-  alert: string | null;
   action: SaveActionType;
   isLoading: boolean;
   saveStatus?: string | null;
@@ -92,7 +91,6 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
     this.state = {
       newSliceName: props.sliceName,
       datasetName: props.datasource?.name,
-      alert: null,
       action: this.canOverwriteSlice() ? 'overwrite' : 'saveas',
       isLoading: false,
       vizType: props.form_data?.viz_type,
@@ -103,7 +101,6 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
     this.changeAction = this.changeAction.bind(this);
     this.saveOrOverwrite = this.saveOrOverwrite.bind(this);
     this.isNewDashboard = this.isNewDashboard.bind(this);
-    this.removeAlert = this.removeAlert.bind(this);
     this.onHide = this.onHide.bind(this);
   }
 
@@ -163,8 +160,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
   }
 
   async saveOrOverwrite(gotodash: boolean) {
-    this.setState({ alert: null, isLoading: true });
-    this.props.actions.removeSaveModalAlert();
+    this.setState({ isLoading: true });
 
     //  Create or retrieve dashboard
     type DashboardGetResponse = {
@@ -324,89 +320,115 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
     };
   };
 
-  renderSaveChartModal = () => (
-    <Form data-test="save-modal-body" layout="vertical">
-      {(this.state.alert || this.props.alert) && (
-        <Alert
-          type="warning"
-          message={this.state.alert || this.props.alert}
-          onClose={this.removeAlert}
-        />
-      )}
-      <FormItem data-test="radio-group">
-        <Radio
-          id="overwrite-radio"
-          disabled={!this.canOverwriteSlice()}
-          checked={this.state.action === 'overwrite'}
-          onChange={() => this.changeAction('overwrite')}
-          data-test="save-overwrite-radio"
-        >
-          {t('Save (Overwrite)')}
-        </Radio>
-        <Radio
-          id="saveas-radio"
-          data-test="saveas-radio"
-          checked={this.state.action === 'saveas'}
-          onChange={() => this.changeAction('saveas')}
-        >
-          {t('Save as...')}
-        </Radio>
-      </FormItem>
-      <hr />
-      <FormItem label={t('Chart name')} required>
-        <Input
-          name="new_slice_name"
-          type="text"
-          placeholder="Name"
-          value={this.state.newSliceName}
-          onChange={this.onSliceNameChange}
-          data-test="new-chart-name"
-        />
-      </FormItem>
-      {this.props.datasource?.type === 'query' && (
-        <FormItem label={t('Dataset Name')} required>
-          <InfoTooltipWithTrigger
-            tooltip={t('A reusable dataset will be saved with your chart.')}
-            placement="right"
-          />
+  renderSaveChartModal = () => {
+    const info = this.info();
+    return (
+      <Form data-test="save-modal-body" layout="vertical">
+        <FormItem data-test="radio-group">
+          <Radio
+            id="overwrite-radio"
+            disabled={!this.canOverwriteSlice()}
+            checked={this.state.action === 'overwrite'}
+            onChange={() => this.changeAction('overwrite')}
+            data-test="save-overwrite-radio"
+          >
+            {t('Save (Overwrite)')}
+          </Radio>
+          <Radio
+            id="saveas-radio"
+            data-test="saveas-radio"
+            checked={this.state.action === 'saveas'}
+            onChange={() => this.changeAction('saveas')}
+          >
+            {t('Save as...')}
+          </Radio>
+        </FormItem>
+        <hr />
+        <FormItem label={t('Chart name')} required>
           <Input
-            name="dataset_name"
+            name="new_slice_name"
             type="text"
-            placeholder="Dataset Name"
-            value={this.state.datasetName}
-            onChange={this.handleDatasetNameChange}
-            data-test="new-dataset-name"
+            placeholder="Name"
+            value={this.state.newSliceName}
+            onChange={this.onSliceNameChange}
+            data-test="new-chart-name"
           />
         </FormItem>
-      )}
-      {!(
-        isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
-        this.state.vizType === 'filter_box'
-      ) && (
-        <FormItem
-          label={t('Add to dashboard')}
-          data-test="save-chart-modal-select-dashboard-form"
-        >
-          <AsyncSelect
-            allowClear
-            allowNewOptions
-            ariaLabel={t('Select a dashboard')}
-            options={this.loadDashboards}
-            onChange={this.onDashboardChange}
-            value={this.state.dashboard}
-            placeholder={
-              <div>
-                <b>{t('Select')}</b>
-                {t(' a dashboard OR ')}
-                <b>{t('create')}</b>
-                {t(' a new one')}
-              </div>
-            }
+        {this.props.datasource?.type === 'query' && (
+          <FormItem label={t('Dataset Name')} required>
+            <InfoTooltipWithTrigger
+              tooltip={t('A reusable dataset will be saved with your chart.')}
+              placement="right"
+            />
+            <Input
+              name="dataset_name"
+              type="text"
+              placeholder="Dataset Name"
+              value={this.state.datasetName}
+              onChange={this.handleDatasetNameChange}
+              data-test="new-dataset-name"
+            />
+          </FormItem>
+        )}
+        {!(
+          isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
+          this.state.vizType === 'filter_box'
+        ) && (
+          <FormItem
+            label={t('Add to dashboard')}
+            data-test="save-chart-modal-select-dashboard-form"
+          >
+            <AsyncSelect
+              allowClear
+              allowNewOptions
+              ariaLabel={t('Select a dashboard')}
+              options={this.loadDashboards}
+              onChange={this.onDashboardChange}
+              value={this.state.dashboard}
+              placeholder={
+                <div>
+                  <b>{t('Select')}</b>
+                  {t(' a dashboard OR ')}
+                  <b>{t('create')}</b>
+                  {t(' a new one')}
+                </div>
+              }
+            />
+          </FormItem>
+        )}
+        {info && <Alert type="info" message={info} closable={false} />}
+        {this.props.alert && (
+          <Alert
+            css={{ marginTop: info ? 16 : undefined }}
+            type="warning"
+            message={this.props.alert}
+            closable={false}
           />
-        </FormItem>
-      )}
-    </Form>
-  );
+        )}
+      </Form>
+    );
+  };
+
+  info = () => {
+    const isNewDashboard = this.isNewDashboard();
+    let chartWillBeCreated = false;
+    if (
+      this.props.slice &&
+      (this.state.action !== 'overwrite' || !this.canOverwriteSlice())
+    ) {
+      chartWillBeCreated = true;
+    }
+    if (chartWillBeCreated && isNewDashboard) {
+      return t('A new chart and dashboard will be created.');
+    }
+    if (chartWillBeCreated) {
+      return t('A new chart will be created.');
+    }
+    if (isNewDashboard) {
+      return t('A new dashboard will be created.');
+    }
+    return null;
+  };
 
   renderFooter = () => (
     <div data-test="save-modal-footer">
@@ -426,9 +448,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
         }
         onClick={() => this.saveOrOverwrite(true)}
       >
-        {this.isNewDashboard()
-          ? t('Save & go to new dashboard')
-          : t('Save & go to dashboard')}
+        {t('Save & go to dashboard')}
       </Button>
       <Button
         id="btn_modal_save"
@@ -443,22 +463,11 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
         }
         data-test="btn-modal-save"
       >
-        {!this.canOverwriteSlice() && this.props.slice
-          ? t('Save as new chart')
-          : this.isNewDashboard()
-          ? t('Save to new dashboard')
-          : t('Save')}
+        {t('Save')}
       </Button>
     </div>
   );
 
-  removeAlert() {
-    if (this.props.alert) {
-      this.props.actions.removeSaveModalAlert();
-    }
-    this.setState({ alert: null });
-  }
-
   render() {
     return (
       <StyledModal
diff --git a/superset-frontend/src/explore/reducers/saveModalReducer.js b/superset-frontend/src/explore/reducers/saveModalReducer.js
index 85983e1ef1ca4..572cabde39d0d 100644
--- a/superset-frontend/src/explore/reducers/saveModalReducer.js
+++ b/superset-frontend/src/explore/reducers/saveModalReducer.js
@@ -40,9 +40,6 @@ export default function saveModalReducer(state = {}, action) {
     [actions.SAVE_SLICE_SUCCESS](data) {
       return { ...state, data };
     },
-    [actions.REMOVE_SAVE_MODAL_ALERT]() {
-      return { ...state, saveModalAlert: null };
-    },
     [HYDRATE_EXPLORE]() {
       return { ...action.data.saveModal };
     },