fix(explore): edit datasource does not update control states (#10284)

superset-frontend/cypress-base/cypress/integration/explore/control.test.js → superset-frontend/cypress-base/cypress/integration/explore/control.test.ts

@@ -21,11 +21,60 @@
 // ***********************************************
 import { FORM_DATA_DEFAULTS, NUM_METRIC } from './visualizations/shared.helper';
 
-describe('Groupby', () => {
-  it('Set groupby', () => {
+describe('Datasource control', () => {
+  const newMetricName = `abc${Date.now()}`;
+
+  before(() => {
     cy.server();
     cy.login();
+    cy.route('GET', '/superset/explore_json/**').as('getJson');
+    cy.route('POST', '/superset/explore_json/**').as('postJson');
+  });
 
+  it('should allow edit datasource', () => {
+    cy.visitChartByName('Num Births Trend');
+    cy.verifySliceSuccess({ waitAlias: '@postJson' });
+    cy.get('#datasource_menu').click();
+    cy.get('a').contains('Edit Datasource').click();
+    // create new metric
+    cy.get('button').contains('Add Item').click();
+    cy.get('input[value="<new metric>"]').click();
+    cy.get('input[value="<new metric>"]')
+      .focus()
+      .clear()
+      .type(`${newMetricName}{enter}`);
+    cy.get('.modal-footer button').contains('Save').click();
+    cy.get('.modal-footer button').contains('OK').click();
+    // select new metric
+    cy.get('.metrics-select:eq(0)').click();
+    cy.get('.metrics-select:eq(0) input[type="text"]')
+      .focus()
+      .type(newMetricName);
+    cy.get('.metrics-select:eq(0) .Select__menu .Select__option')
+      .contains(newMetricName)
+      .click();
+    cy.get('.metrics-select:eq(0) .Select__multi-value__label')
+      .contains(newMetricName)
+      .click();
+    // delete metric
+    cy.get('#datasource_menu').click();
+    cy.get('a').contains('Edit Datasource').click();
+    cy.get(`input[value="${newMetricName}"]`)
+      .closest('tr')
+      .find('.fa-close')
+      .click();
+    cy.get('.modal-footer button').contains('Save').click();
+    cy.get('.modal-footer button').contains('OK').click();
+    cy.get('.Select__multi-value__label')
+      .contains(newMetricName)
+      .should('not.exist');
+  });
+});
+
+describe('Groupby control', () => {
+  it('Set groupby', () => {
+    cy.server();
+    cy.login();
     cy.route('GET', '/superset/explore_json/**').as('getJson');
     cy.route('POST', '/superset/explore_json/**').as('postJson');
     cy.visitChartByName('Num Births Trend');
@@ -71,5 +120,7 @@ describe('Time range filter', () => {
         });
       });
     });
+    cy.get('#filter-popover button').contains('Ok').click();
+    cy.get('#filter-popover').should('not.exist');
   });
 });

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

@@ -41,6 +41,9 @@ const defaultProps = {
       name: 'main',
     },
   },
+  actions: {
+    setDatasource: sinon.spy(),
+  },
   onChange: sinon.spy(),
 };
 
@@ -71,15 +74,15 @@ describe('DatasourceControl', () => {
     let wrapper = setup();
     expect(wrapper.find('#datasource_menu')).toHaveLength(1);
     expect(wrapper.find('#datasource_menu').dive().find(MenuItem)).toHaveLength(
-      2,
+      3,
     );
 
     wrapper = setup({
-      onDatasourceSave: () => {},
+      isEditable: false,
     });
     expect(wrapper.find('#datasource_menu')).toHaveLength(1);
     expect(wrapper.find('#datasource_menu').dive().find(MenuItem)).toHaveLength(
-      3,
+      2,
     );
   });
 });

superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx

@@ -27,7 +27,6 @@ import {
   getFormDataFromControls,
   applyMapStateToPropsToControl,
   getAllControlsState,
-  getControlsState,
 } from 'src/explore/controlUtils';
 
 describe('controlUtils', () => {

superset-frontend/src/components/TooltipWrapper.jsx

@@ -26,6 +26,10 @@ const propTypes = {
   tooltip: PropTypes.node.isRequired,
   children: PropTypes.node.isRequired,
   placement: PropTypes.string,
+  trigger: PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.arrayOf(PropTypes.string),
+  ]),
 };
 
 const defaultProps = {
@@ -37,11 +41,13 @@ export default function TooltipWrapper({
   tooltip,
   children,
   placement,
+  trigger,
 }) {
   return (
     <OverlayTrigger
       placement={placement}
       overlay={<Tooltip id={`${kebabCase(label)}-tooltip`}>{tooltip}</Tooltip>}
+      trigger={trigger}
     >
       {children}
     </OverlayTrigger>

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

@@ -389,6 +389,7 @@ function mapStateToProps(state) {
   const form_data = getFormDataFromControls(explore.controls);
   const chartKey = Object.keys(charts)[0];
   const chart = charts[chartKey];
+
   return {
     isDatasourceMetaLoading: explore.isDatasourceMetaLoading,
     datasource: explore.datasource,

superset-frontend/src/explore/components/controls/DatasourceControl.jsx

@@ -38,9 +38,11 @@ import TooltipWrapper from '../../../components/TooltipWrapper';
 import './DatasourceControl.less';
 
 const propTypes = {
+  actions: PropTypes.object.isRequired,
   onChange: PropTypes.func,
   value: PropTypes.string,
   datasource: PropTypes.object.isRequired,
+  isEditable: PropTypes.bool,
   onDatasourceSave: PropTypes.func,
 };
 
@@ -48,6 +50,7 @@ const defaultProps = {
   onChange: () => {},
   onDatasourceSave: null,
   value: null,
+  isEditable: true,
 };
 
 class DatasourceControl extends React.PureComponent {
@@ -58,6 +61,7 @@ class DatasourceControl extends React.PureComponent {
       showChangeDatasourceModal: false,
       menuExpanded: false,
     };
+    this.onDatasourceSave = this.onDatasourceSave.bind(this);
     this.toggleChangeDatasourceModal = this.toggleChangeDatasourceModal.bind(
       this,
     );
@@ -66,6 +70,13 @@ class DatasourceControl extends React.PureComponent {
     this.renderDatasource = this.renderDatasource.bind(this);
   }
 
+  onDatasourceSave(datasource) {
+    this.props.actions.setDatasource(datasource);
+    if (this.props.onDatasourceSave) {
+      this.props.onDatasourceSave(datasource);
+    }
+  }
+
   toggleShowDatasource() {
     this.setState(({ showDatasource }) => ({
       showDatasource: !showDatasource,
@@ -120,7 +131,7 @@ class DatasourceControl extends React.PureComponent {
 
   render() {
     const { showChangeDatasourceModal, showEditDatasourceModal } = this.state;
-    const { datasource, onChange, onDatasourceSave, value } = this.props;
+    const { datasource, onChange, value } = this.props;
     return (
       <div>
         <ControlHeader {...this.props} />
@@ -128,6 +139,7 @@ class DatasourceControl extends React.PureComponent {
           <TooltipWrapper
             label="change-datasource"
             tooltip={t('Click to change the datasource')}
+            trigger={['hover']}
           >
             <DropdownButton
               title={datasource.name}
@@ -148,7 +160,7 @@ class DatasourceControl extends React.PureComponent {
                   {t('Explore in SQL Lab')}
                 </MenuItem>
               )}
-              {!!this.props.onDatasourceSave && (
+              {this.props.isEditable && (
                 <MenuItem eventKey="3" onClick={this.toggleEditDatasourceModal}>
                   {t('Edit Datasource')}
                 </MenuItem>
@@ -179,11 +191,11 @@ class DatasourceControl extends React.PureComponent {
         <DatasourceModal
           datasource={datasource}
           show={showEditDatasourceModal}
-          onDatasourceSave={onDatasourceSave}
+          onDatasourceSave={this.onDatasourceSave}
           onHide={this.toggleEditDatasourceModal}
         />
         <ChangeDatasourceModal
-          onDatasourceSave={onDatasourceSave}
+          onDatasourceSave={this.onDatasourceSave}
           onHide={this.toggleChangeDatasourceModal}
           show={showChangeDatasourceModal}
           onChange={onChange}

superset-frontend/src/explore/controlUtils.js

@@ -20,7 +20,6 @@ import memoizeOne from 'memoize-one';
 import { getChartControlPanelRegistry } from '@superset-ui/chart';
 import { expandControlConfig } from '@superset-ui/chart-controls';
 import { controls as SHARED_CONTROLS } from './controls';
-import * as exploreActions from './actions/exploreActions';
 import * as SECTIONS from './controlPanels/sections';
 
 export function getFormDataFromControls(controlsState) {
@@ -94,7 +93,7 @@ export function applyMapStateToPropsToControl(controlState, controlPanelState) {
   if (mapStateToProps && controlPanelState) {
     return {
       ...controlState,
-      ...mapStateToProps(controlPanelState, controlState, exploreActions),
+      ...mapStateToProps(controlPanelState, controlState),
     };
   }
   return controlState;

superset-frontend/src/explore/controls.jsx

@@ -204,9 +204,9 @@ export const controls = {
     label: t('Datasource'),
     default: null,
     description: null,
-    mapStateToProps: (state, control, actions) => ({
-      datasource: state.datasource,
-      onDatasourceSave: actions ? actions.setDatasource : () => {},
+    mapStateToProps: ({ datasource }) => ({
+      datasource,
+      isEditable: !!datasource,
     }),
   },