[dashboard] After update filter, trigger new queries when charts are visible

superset/assets/cypress/integration/dashboard/dashboard.helper.js

@@ -16,7 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export const WORLD_HEALTH_DASHBOARD = '/superset/dashboard/world_health';
+export const WORLD_HEALTH_DASHBOARD = '/superset/dashboard/world_health/';
+export const TABBED_DASHBOARD = '/superset/dashboard/tabbed_dash/';
 
 export const CHECK_DASHBOARD_FAVORITE_ENDPOINT = '/superset/favstar/Dashboard/*/count';
 

superset/assets/cypress/integration/dashboard/index.test.js

@@ -22,6 +22,7 @@ import DashboardFavStarTest from './fav_star';
 import DashboardFilterTest from './filter';
 import DashboardLoadTest from './load';
 import DashboardSaveTest from './save';
+import DashboardTabsTest from './tabs';
 
 describe('Dashboard', () => {
   DashboardControlsTest();
@@ -30,4 +31,5 @@ describe('Dashboard', () => {
   DashboardFilterTest();
   DashboardLoadTest();
   DashboardSaveTest();
+  DashboardTabsTest();
 });

superset/assets/cypress/integration/dashboard/tabs.js

@@ -0,0 +1,157 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { TABBED_DASHBOARD } from './dashboard.helper';
+
+export default () => describe('tabs', () => {
+  let filterId;
+  let treemapId;
+  let linechartId;
+  let boxplotId;
+
+  // cypress can not handle window.scrollTo
+  // https://github.com/cypress-io/cypress/issues/2761
+  // add this exception handler to pass test
+  const handleException = () => {
+    // return false to prevent the error from
+    // failing this test
+    cy.on('uncaught:exception', () => false);
+  };
+
+  beforeEach(() => {
+    cy.server();
+    cy.login();
+
+    cy.visit(TABBED_DASHBOARD);
+
+    cy.get('#app').then((data) => {
+      const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
+      const dashboard = bootstrapData.dashboard_data;
+      filterId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'filter_box')).slice_id;
+      boxplotId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'box_plot')).slice_id;
+      treemapId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'treemap')).slice_id;
+      linechartId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'line')).slice_id;
+
+      const filterFormdata = {
+        slice_id: filterId,
+      };
+      const filterRequest = `/superset/explore_json/?form_data=${JSON.stringify(filterFormdata)}`;
+      cy.route('POST', filterRequest).as('filterRequest');
+
+      const treemapFormdata = {
+        slice_id: treemapId,
+      };
+      const treemapRequest = `/superset/explore_json/?form_data=${JSON.stringify(treemapFormdata)}`;
+      cy.route('POST', treemapRequest).as('treemapRequest');
+
+      const linechartFormdata = {
+        slice_id: linechartId,
+      };
+      const linechartRequest = `/superset/explore_json/?form_data=${JSON.stringify(linechartFormdata)}`;
+      cy.route('POST', linechartRequest).as('linechartRequest');
+
+      const boxplotFormdata = {
+        slice_id: boxplotId,
+      };
+      const boxplotRequest = `/superset/explore_json/?form_data=${JSON.stringify(boxplotFormdata)}`;
+      cy.route('POST', boxplotRequest).as('boxplotRequest');
+    });
+  });
+
+  it('should load charts when tab is visible', () => {
+    // landing in first tab, should see 2 charts
+    cy.wait('@filterRequest');
+    cy.get('.grid-container .filter_box').should('be.exist');
+    cy.wait('@treemapRequest');
+    cy.get('.grid-container .treemap').should('be.exist');
+    cy.get('.grid-container .box_plot').should('not.be.exist');
+    cy.get('.grid-container .line').should('not.be.exist');
+
+    // click row level tab, see 1 more chart
+    cy.get('.tab-content ul.nav.nav-tabs li')
+      .last()
+      .find('.editable-title input')
+      .click();
+    cy.wait('@linechartRequest');
+    cy.get('.grid-container .line').should('be.exist');
+
+    // click top level tab, see 1 more chart
+    handleException();
+    cy.get('.dashboard-component-tabs')
+      .first()
+      .find('ul.nav.nav-tabs li')
+      .last()
+      .find('.editable-title input')
+      .click();
+    cy.wait('@boxplotRequest');
+    cy.get('.grid-container .box_plot').should('be.exist');
+  });
+
+  it('should send new queries when tab becomes visible', () => {
+    // landing in first tab
+    cy.wait('@filterRequest');
+    cy.wait('@treemapRequest');
+
+    // creating route and stubbing filtered route
+    cy.route('POST', '/superset/explore_json/*').as('updatedChartRequest');
+
+    // apply filter
+    cy.get('.Select-control')
+      .first()
+      .find('input')
+      .first()
+      .type('South Asia{enter}', { force: true });
+
+    // send new query from same tab
+    cy.wait('@updatedChartRequest')
+      .then((xhr) => {
+        const requestFormData = xhr.request.body;
+        const requestParams = JSON.parse(requestFormData.get('form_data'));
+        expect(requestParams.extra_filters[0])
+          .deep.eq({ col: 'region', op: 'in', val: ['South Asia'] });
+      });
+
+    // click row level tab, send 1 more query
+    cy.get('.tab-content ul.nav.nav-tabs li')
+      .last()
+      .click();
+    cy.wait('@updatedChartRequest')
+      .then((xhr) => {
+        const requestFormData = xhr.request.body;
+        const requestParams = JSON.parse(requestFormData.get('form_data'));
+        expect(requestParams.extra_filters[0])
+          .deep.eq({ col: 'region', op: 'in', val: ['South Asia'] });
+      });
+
+    // click top level tab, send 1 more query
+    handleException();
+    cy.get('.dashboard-component-tabs')
+      .first()
+      .find('ul.nav.nav-tabs li')
+      .last()
+      .find('.editable-title input')
+      .click();
+    cy.wait('@updatedChartRequest')
+      .then((xhr) => {
+        const requestFormData = xhr.request.body;
+        const requestParams = JSON.parse(requestFormData.get('form_data'));
+        expect(requestParams.extra_filters[0])
+          .deep.eq({ col: 'region', op: 'in', val: ['South Asia'] });
+      });
+  });
+});

superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx

@@ -39,7 +39,7 @@ describe('Dashboard', () => {
     actions: {
       addSliceToDashboard() {},
       removeSliceFromDashboard() {},
-      postChartFormData() {},
+      triggerQuery() {},
       logEvent() {},
     },
     initMessages: [],
@@ -82,15 +82,15 @@ describe('Dashboard', () => {
       },
     };
 
-    it('should call postChartFormData for all non-exempt slices', () => {
+    it('should call triggerQuery for all non-exempt slices', () => {
       const wrapper = setup({ charts: overrideCharts, slices: overrideSlices });
-      const spy = sinon.spy(props.actions, 'postChartFormData');
+      const spy = sinon.spy(props.actions, 'triggerQuery');
       wrapper.instance().refreshExcept('1001');
       spy.restore();
       expect(spy.callCount).toBe(Object.keys(overrideCharts).length - 1);
     });
 
-    it('should not call postChartFormData for filter_immune_slices', () => {
+    it('should not call triggerQuery for filter_immune_slices', () => {
       const wrapper = setup({
         charts: overrideCharts,
         dashboardInfo: {
@@ -103,7 +103,7 @@ describe('Dashboard', () => {
           },
         },
       });
-      const spy = sinon.spy(props.actions, 'postChartFormData');
+      const spy = sinon.spy(props.actions, 'triggerQuery');
       wrapper.instance().refreshExcept();
       spy.restore();
       expect(spy.callCount).toBe(0);

superset/assets/spec/javascripts/explore/components/ExploreViewContainer_spec.jsx

@@ -19,6 +19,7 @@
 import React from 'react';
 import configureStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
+import sinon from 'sinon';
 import { shallow } from 'enzyme';
 
 import getInitialState from 'src/explore/reducers/getInitialState';
@@ -58,7 +59,7 @@ describe('ExploreViewContainer', () => {
     wrapper = shallow(<ExploreViewContainer />, {
       context: { store },
       disableLifecycleMethods: true,
-    });
+    }).dive();
   });
 
   it('renders', () => {
@@ -68,14 +69,37 @@ describe('ExploreViewContainer', () => {
   });
 
   it('renders QueryAndSaveButtons', () => {
-    expect(wrapper.dive().find(QueryAndSaveBtns)).toHaveLength(1);
+    expect(wrapper.find(QueryAndSaveBtns)).toHaveLength(1);
   });
 
   it('renders ControlPanelsContainer', () => {
-    expect(wrapper.dive().find(ControlPanelsContainer)).toHaveLength(1);
+    expect(wrapper.find(ControlPanelsContainer)).toHaveLength(1);
   });
 
   it('renders ChartContainer', () => {
-    expect(wrapper.dive().find(ChartContainer)).toHaveLength(1);
+    expect(wrapper.find(ChartContainer)).toHaveLength(1);
+  });
+
+  describe('componentWillReceiveProps()', () => {
+    it('when controls change, should call resetControls', () => {
+      expect(wrapper.instance().props.controls.viz_type.value).toBe('table');
+      const resetControls = sinon.stub(wrapper.instance().props.actions, 'resetControls');
+      const triggerQuery = sinon.stub(wrapper.instance().props.actions, 'triggerQuery');
+
+      // triggers componentWillReceiveProps
+      wrapper.setProps({
+        controls: {
+          viz_type: {
+            value: 'bar',
+          },
+        },
+      });
+      expect(resetControls.callCount).toBe(1);
+      // exploreview container should not force chart run query
+      // it should be controlled by redux state.
+      expect(triggerQuery.callCount).toBe(0);
+      resetControls.reset();
+      triggerQuery.reset();
+    });
   });
 });

superset/assets/src/chart/Chart.jsx

@@ -22,6 +22,7 @@ import { Alert } from 'react-bootstrap';
 
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
 import { Logger, LOG_ACTIONS_RENDER_CHART_CONTAINER } from '../logger/LogUtils';
+import { safeStringify } from '../utils/safeStringify';
 import Loading from '../components/Loading';
 import RefreshChartOverlay from '../components/RefreshChartOverlay';
 import StackTraceMessage from '../components/StackTraceMessage';
@@ -69,25 +70,38 @@ class Chart extends React.PureComponent {
     super(props);
     this.handleRenderContainerFailure = this.handleRenderContainerFailure.bind(this);
   }
+
   componentDidMount() {
     if (this.props.triggerQuery) {
-      if (this.props.chartId > 0 && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)) {
-        // Load saved chart with a GET request
-        this.props.actions.getSavedChart(
-          this.props.formData,
-          false,
-          this.props.timeout,
-          this.props.chartId,
-        );
-      } else {
-        // Create chart with POST request
-        this.props.actions.postChartFormData(
-          this.props.formData,
-          false,
-          this.props.timeout,
-          this.props.chartId,
-        );
-      }
+      this.runQuery();
+    }
+  }
+
+  componentDidUpdate(prevProps) {
+    if (this.props.triggerQuery &&
+      safeStringify(prevProps.formData) !== safeStringify(this.props.formData)
+    ) {
+      this.runQuery();
+    }
+  }
+
+  runQuery() {
+    if (this.props.chartId > 0 && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)) {
+      // Load saved chart with a GET request
+      this.props.actions.getSavedChart(
+        this.props.formData,
+        false,
+        this.props.timeout,
+        this.props.chartId,
+      );
+    } else {
+      // Create chart with POST request
+      this.props.actions.postChartFormData(
+        this.props.formData,
+        false,
+        this.props.timeout,
+        this.props.chartId,
+      );
     }
   }
 

superset/assets/src/dashboard/components/Dashboard.jsx

@@ -30,7 +30,6 @@ import {
   loadStatsPropShape,
 } from '../util/propShapes';
 import { areObjectsEqual } from '../../reduxUtils';
-import getFormDataWithExtraFilters from '../util/charts/getFormDataWithExtraFilters';
 import { LOG_ACTIONS_MOUNT_DASHBOARD } from '../../logger/LogUtils';
 import OmniContainer from '../../components/OmniContainer';
 
@@ -40,7 +39,7 @@ const propTypes = {
   actions: PropTypes.shape({
     addSliceToDashboard: PropTypes.func.isRequired,
     removeSliceFromDashboard: PropTypes.func.isRequired,
-    postChartFormData: PropTypes.func.isRequired,
+    triggerQuery: PropTypes.func.isRequired,
     logEvent: PropTypes.func.isRequired,
   }).isRequired,
   dashboardInfo: dashboardInfoPropShape.isRequired,
@@ -149,21 +148,7 @@ class Dashboard extends React.PureComponent {
     this.getAllCharts().forEach(chart => {
       // filterKey is a string, immune array contains numbers
       if (String(chart.id) !== filterKey && immune.indexOf(chart.id) === -1) {
-        const updatedFormData = getFormDataWithExtraFilters({
-          chart,
-          dashboardMetadata: this.props.dashboardInfo.metadata,
-          filters: this.props.dashboardState.filters,
-          colorScheme: this.props.dashboardState.colorScheme,
-          colorNamespace: this.props.dashboardState.colorNamespace,
-          sliceId: chart.id,
-        });
-
-        this.props.actions.postChartFormData(
-          updatedFormData,
-          false,
-          this.props.timeout,
-          chart.id,
-        );
+        this.props.actions.triggerQuery(true, chart.id);
       }
     });
   }

superset/assets/src/dashboard/components/DashboardBuilder.jsx

@@ -221,6 +221,7 @@ class DashboardBuilder extends React.Component {
                           // see isValidChild for why tabs do not increment the depth of their children
                           depth={DASHBOARD_ROOT_DEPTH + 1} // (topLevelTabs ? 0 : 1)}
                           width={width}
+                          isComponentVisible={index === tabIndex}
                         />
                       </TabPane>
                     ))}