You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2019/07/25 17:37:56 UTC
[incubator-superset] branch master updated: [dashboard] fix chart
showing loading icon when filter updated immuned fields (#7895)
This is an automated email from the ASF dual-hosted git repository.
graceguo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new df9efa8 [dashboard] fix chart showing loading icon when filter updated immuned fields (#7895)
df9efa8 is described below
commit df9efa8ed9f6ee263f52b791086b286761e9aab6
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Thu Jul 25 10:37:44 2019 -0700
[dashboard] fix chart showing loading icon when filter updated immuned fields (#7895)
---
.../dashboard/components/Dashboard_spec.jsx | 44 ++++++++++++++++++++++
.../dashboard/fixtures/mockDashboardInfo.js | 5 ++-
.../dashboard/fixtures/mockDashboardState.js | 2 +-
superset/assets/src/chart/Chart.jsx | 7 +---
.../assets/src/dashboard/components/Dashboard.jsx | 28 ++++++++++++--
.../src/dashboard/reducers/getInitialState.js | 4 +-
6 files changed, 78 insertions(+), 12 deletions(-)
diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
index 05d3675..1f6fa6d 100644
--- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
@@ -66,6 +66,24 @@ describe('Dashboard', () => {
});
describe('refreshExcept', () => {
+ const overrideDashboardState = {
+ ...dashboardState,
+ filters: {
+ 1: { region: [] },
+ 2: { country_name: ['USA'] },
+ 3: { region: [], country_name: ['USA'] },
+ },
+ refresh: true,
+ };
+
+ const overrideDashboardInfo = {
+ ...dashboardInfo,
+ metadata: {
+ ...dashboardInfo.metadata,
+ filter_immune_slice_fields: { [chartQueries[chartId].id]: ['region'] },
+ },
+ };
+
const overrideCharts = {
...chartQueries,
1001: {
@@ -108,6 +126,32 @@ describe('Dashboard', () => {
spy.restore();
expect(spy.callCount).toBe(0);
});
+
+ it('should not call triggerQuery for filter_immune_slice_fields', () => {
+ const wrapper = setup({
+ dashboardState: overrideDashboardState,
+ dashboardInfo: overrideDashboardInfo,
+ });
+ const spy = sinon.spy(props.actions, 'triggerQuery');
+ wrapper.instance().refreshExcept('1');
+ expect(spy.callCount).toBe(0);
+ spy.restore();
+ });
+
+ it('should call triggerQuery if filter has more filter-able fields', () => {
+ const wrapper = setup({
+ dashboardState: overrideDashboardState,
+ dashboardInfo: overrideDashboardInfo,
+ });
+ const spy = sinon.spy(props.actions, 'triggerQuery');
+
+ // if filter have additional fields besides immune ones,
+ // should apply filter.
+ wrapper.instance().refreshExcept('3');
+ expect(spy.callCount).toBe(1);
+
+ spy.restore();
+ });
});
describe('componentWillReceiveProps', () => {
diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardInfo.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardInfo.js
index 228c724..46d1281 100644
--- a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardInfo.js
+++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardInfo.js
@@ -19,7 +19,10 @@
export default {
id: 1234,
slug: 'dashboardSlug',
- metadata: {},
+ metadata: {
+ filter_immune_slices: [],
+ filter_immune_slice_fields: {},
+ },
userId: 'mock_user_id',
dash_edit_perm: true,
dash_save_perm: true,
diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js
index 3763ef4..d388711 100644
--- a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js
+++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { id as sliceId } from './mockChartQueries';
+import { sliceId } from './mockChartQueries';
import { BUILDER_PANE_TYPE } from '../../../../src/dashboard/util/constants';
export default {
diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index 08d0663..48ffb75 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -22,7 +22,6 @@ 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';
@@ -82,10 +81,8 @@ class Chart extends React.PureComponent {
}
}
- componentDidUpdate(prevProps) {
- if (this.props.triggerQuery &&
- safeStringify(prevProps.formData) !== safeStringify(this.props.formData)
- ) {
+ componentDidUpdate() {
+ if (this.props.triggerQuery) {
this.runQuery();
}
}
diff --git a/superset/assets/src/dashboard/components/Dashboard.jsx b/superset/assets/src/dashboard/components/Dashboard.jsx
index b584559..13222c6 100644
--- a/superset/assets/src/dashboard/components/Dashboard.jsx
+++ b/superset/assets/src/dashboard/components/Dashboard.jsx
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+/* eslint-disable camelcase */
import React from 'react';
import PropTypes from 'prop-types';
import { t } from '@superset-ui/translation';
@@ -143,11 +144,32 @@ class Dashboard extends React.PureComponent {
}
refreshExcept(filterKey) {
- const immune = this.props.dashboardInfo.metadata.filter_immune_slices || [];
+ const { filters } = this.props.dashboardState || {};
+ const currentFilteredNames =
+ filterKey && filters[filterKey] ? Object.keys(filters[filterKey]) : [];
+ const filter_immune_slices = this.props.dashboardInfo.metadata
+ .filter_immune_slices;
+ const filter_immune_slice_fields = this.props.dashboardInfo.metadata
+ .filter_immune_slice_fields;
this.getAllCharts().forEach(chart => {
- // filterKey is a string, immune array contains numbers
- if (String(chart.id) !== filterKey && immune.indexOf(chart.id) === -1) {
+ // filterKey is a string, filter_immune_slices array contains numbers
+ if (
+ String(chart.id) === filterKey ||
+ filter_immune_slices.includes(chart.id)
+ ) {
+ return;
+ }
+
+ const filter_immune_slice_fields_names =
+ filter_immune_slice_fields[chart.id] || [];
+ // has filter-able field names
+ if (
+ currentFilteredNames.length === 0 ||
+ currentFilteredNames.some(
+ name => !filter_immune_slice_fields_names.includes(name),
+ )
+ ) {
this.props.actions.triggerQuery(true, chart.id);
}
});
diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js
index 14b7937..9120a1f 100644
--- a/superset/assets/src/dashboard/reducers/getInitialState.js
+++ b/superset/assets/src/dashboard/reducers/getInitialState.js
@@ -186,8 +186,8 @@ export default function(bootstrapData) {
slug: dashboard.slug,
metadata: {
filter_immune_slice_fields:
- dashboard.metadata.filter_immune_slice_fields,
- filter_immune_slices: dashboard.metadata.filter_immune_slices,
+ dashboard.metadata.filter_immune_slice_fields || {},
+ filter_immune_slices: dashboard.metadata.filter_immune_slices || [],
timed_refresh_immune_slices:
dashboard.metadata.timed_refresh_immune_slices,
},