diff --git a/superset/assets/javascripts/chart/Chart.jsx b/superset/assets/javascripts/chart/Chart.jsx index 111c48ee24d00..7bb71aca471c9 100644 --- a/superset/assets/javascripts/chart/Chart.jsx +++ b/superset/assets/javascripts/chart/Chart.jsx @@ -207,7 +207,7 @@ class Chart extends React.PureComponent { /> } - {!this.props.chartAlert && + {!isLoading && !this.props.chartAlert && ); diff --git a/superset/assets/javascripts/dashboard/components/Dashboard.jsx b/superset/assets/javascripts/dashboard/components/Dashboard.jsx index c99c293604ad9..bcc13778d2fe1 100644 --- a/superset/assets/javascripts/dashboard/components/Dashboard.jsx +++ b/superset/assets/javascripts/dashboard/components/Dashboard.jsx @@ -93,12 +93,21 @@ class Dashboard extends React.PureComponent { } componentDidUpdate(prevProps) { - if (!areObjectsEqual(prevProps.filters, this.props.filters) && this.props.refresh) { - const currentFilterKeys = Object.keys(this.props.filters); - if (currentFilterKeys.length) { - currentFilterKeys.forEach(key => (this.refreshExcept(key))); - } else { - this.refreshExcept(); + if (this.props.refresh) { + let changedFilterKey; + const prevFiltersKeySet = new Set(Object.keys(prevProps.filters)); + Object.keys(this.props.filters).some((key) => { + prevFiltersKeySet.delete(key); + if (prevProps.filters[key] === undefined || + !areObjectsEqual(prevProps.filters[key], this.props.filters[key])) { + changedFilterKey = key; + return true; + } + return false; + }); + // has changed filter or removed a filter? + if (!!changedFilterKey || prevFiltersKeySet.size) { + this.refreshExcept(changedFilterKey); } } } diff --git a/superset/assets/spec/javascripts/chart/Chart_spec.jsx b/superset/assets/spec/javascripts/chart/Chart_spec.jsx index f4754f477ca7d..9d45e85c905a0 100644 --- a/superset/assets/spec/javascripts/chart/Chart_spec.jsx +++ b/superset/assets/spec/javascripts/chart/Chart_spec.jsx @@ -6,6 +6,8 @@ import sinon from 'sinon'; import { chart as initChart } from '../../../javascripts/chart/chartReducer'; import Chart from '../../../javascripts/chart/Chart'; +import ChartBody from '../../../javascripts/chart/ChartBody'; +import Loading from '../../../javascripts/components/Loading'; describe('Chart', () => { const chart = { @@ -30,15 +32,20 @@ describe('Chart', () => { }, }; + let wrapper; + beforeEach(() => { + wrapper = shallow( + , + ); + }); describe('renderViz', () => { - let wrapper; let stub; beforeEach(() => { - wrapper = shallow( - , - ); stub = sinon.stub(wrapper.instance(), 'renderViz'); }); + afterEach(() => { + stub.restore(); + }); it('should not call when loading', () => { const prevProp = wrapper.props(); @@ -68,4 +75,11 @@ describe('Chart', () => { expect(stub.callCount).to.equals(1); }); }); + + describe('render', () => { + it('should render ChartBody after loading is completed', () => { + expect(wrapper.find(Loading)).to.have.length(1); + expect(wrapper.find(ChartBody)).to.have.length(0); + }); + }); }); diff --git a/superset/assets/spec/javascripts/dashboard/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/Dashboard_spec.jsx index 95f013f115080..1ac495992a630 100644 --- a/superset/assets/spec/javascripts/dashboard/Dashboard_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/Dashboard_spec.jsx @@ -26,7 +26,7 @@ describe('Dashboard', () => { it('should render', () => { const wrapper = shallow(); expect(wrapper.find('#dashboard-container')).to.have.length(1); - expect(wrapper.instance().getAllSlices()).to.have.length(2); + expect(wrapper.instance().getAllSlices()).to.have.length(3); }); it('should handle metadata default_filters', () => { @@ -51,10 +51,8 @@ describe('Dashboard', () => { it('should carry updated filter', () => { wrapper.setProps({ filters: { - 256: { - region: [], - country_name: ['France'], - }, + 256: { region: [] }, + 257: { country_name: ['France'] }, }, }); const extraFilters = wrapper.instance().getFormDataExtra(selectedSlice).extra_filters; @@ -74,7 +72,7 @@ describe('Dashboard', () => { }); it('should not refresh filter slice', () => { - const filterKey = Object.keys(defaultFilters)[0]; + const filterKey = Object.keys(defaultFilters)[1]; wrapper.instance().refreshExcept(filterKey); expect(spy.callCount).to.equal(1); expect(spy.getCall(0).args[0].length).to.equal(1); @@ -83,7 +81,102 @@ describe('Dashboard', () => { it('should refresh all slices', () => { wrapper.instance().refreshExcept(); expect(spy.callCount).to.equal(1); - expect(spy.getCall(0).args[0].length).to.equal(2); + expect(spy.getCall(0).args[0].length).to.equal(3); + }); + }); + + describe('componentDidUpdate', () => { + let wrapper; + let refreshExceptSpy; + let fetchSlicesStub; + let prevProp; + beforeEach(() => { + wrapper = shallow(); + prevProp = wrapper.instance().props; + refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept'); + fetchSlicesStub = sinon.stub(wrapper.instance(), 'fetchSlices'); + }); + afterEach(() => { + fetchSlicesStub.restore(); + refreshExceptSpy.restore(); + }); + + describe('should check if filter has change', () => { + beforeEach(() => { + refreshExceptSpy.reset(); + }); + it('no change', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: [] }, + 257: { country_name: ['United States'] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + expect(refreshExceptSpy.callCount).to.equal(0); + }); + + it('remove filter', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: [] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + expect(refreshExceptSpy.callCount).to.equal(1); + }); + + it('change filter', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: [] }, + 257: { country_name: ['Canada'] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + expect(refreshExceptSpy.callCount).to.equal(1); + }); + + it('add filter', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: [] }, + 257: { country_name: ['Canada'] }, + 258: { another_filter: ['new'] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + expect(refreshExceptSpy.callCount).to.equal(1); + }); + }); + + it('should refresh if refresh flag is true', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: ['Asian'] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + const fetchArgs = fetchSlicesStub.lastCall.args[0]; + expect(fetchArgs).to.have.length(2); + }); + + it('should not refresh filter_immune_slices', () => { + wrapper.setProps({ + refresh: true, + filters: { + 256: { region: [] }, + 257: { country_name: ['Canada'] }, + }, + }); + wrapper.instance().componentDidUpdate(prevProp); + const fetchArgs = fetchSlicesStub.lastCall.args[0]; + expect(fetchArgs).to.have.length(1); }); }); }); diff --git a/superset/assets/spec/javascripts/dashboard/fixtures.jsx b/superset/assets/spec/javascripts/dashboard/fixtures.jsx index 7b95e7c65533c..1b9cf21b62c1b 100644 --- a/superset/assets/spec/javascripts/dashboard/fixtures.jsx +++ b/superset/assets/spec/javascripts/dashboard/fixtures.jsx @@ -1,10 +1,8 @@ import { getInitialState } from '../../../javascripts/dashboard/reducers'; export const defaultFilters = { - 256: { - region: [], - country_name: ['United States'], - }, + 256: { region: [] }, + 257: { country_name: ['United States'] }, }; export const regionFilter = { datasource: null, @@ -39,6 +37,35 @@ export const regionFilter = { slice_name: 'Region Filters', slice_url: '/superset/explore/table/2/?form_data=%7B%22slice_id%22%3A%20256%7D', }; +export const countryFilter = { + datasource: null, + description: null, + description_markeddown: '', + edit_url: '/slicemodelview/edit/257', + form_data: { + datasource: '2__table', + date_filter: false, + filters: [], + granularity_sqla: null, + groupby: ['country_name'], + having: '', + instant_filtering: true, + metric: 'sum__SP_POP_TOTL', + show_druid_time_granularity: false, + show_druid_time_origin: false, + show_sqla_time_column: false, + show_sqla_time_granularity: false, + since: '100 years ago', + slice_id: 257, + time_grain_sqla: null, + until: 'now', + viz_type: 'filter_box', + where: '', + }, + slice_id: 257, + slice_name: 'Country Filters', + slice_url: '/superset/explore/table/2/?form_data=%7B%22slice_id%22%3A%20257%7D', +}; export const slice = { datasource: null, description: null, @@ -100,7 +127,7 @@ const mockDashboardData = { id: 2, metadata: { default_filters: JSON.stringify(defaultFilters), - filter_immune_slices: [], + filter_immune_slices: [256], timed_refresh_immune_slices: [], filter_immune_slice_fields: {}, expanded_slices: {}, @@ -122,7 +149,7 @@ const mockDashboardData = { }, ], slug: 'births', - slices: [regionFilter, slice], + slices: [regionFilter, slice, countryFilter], standalone_mode: false, }; export const { dashboard, charts } = getInitialState({ diff --git a/superset/assets/spec/javascripts/dashboard/reducers_spec.js b/superset/assets/spec/javascripts/dashboard/reducers_spec.js index 17b8f75df54c3..8022928ddf4b5 100644 --- a/superset/assets/spec/javascripts/dashboard/reducers_spec.js +++ b/superset/assets/spec/javascripts/dashboard/reducers_spec.js @@ -11,8 +11,10 @@ describe('Dashboard reducers', () => { type: actions.REMOVE_SLICE, slice: initState.dashboard.slices[1], }; + expect(initState.dashboard.slices).to.have.length(3); + const { dashboard, filters, refresh } = reducers(initState, action); - expect(dashboard.slices).to.have.length(1); + expect(dashboard.slices).to.have.length(2); expect(filters).to.deep.equal(defaultFilters); expect(refresh).to.equal(false); }); @@ -22,9 +24,12 @@ describe('Dashboard reducers', () => { type: actions.REMOVE_SLICE, slice: initState.dashboard.slices[0], }; + const initFilters = Object.keys(initState.filters); + expect(initFilters).to.have.length(2); + const { dashboard, filters, refresh } = reducers(initState, action); - expect(dashboard.slices).to.have.length(1); - expect(filters).to.deep.equal({}); + expect(dashboard.slices).to.have.length(2); + expect(Object.keys(filters)).to.have.length(1); expect(refresh).to.equal(true); }); }); diff --git a/superset/assets/stylesheets/superset.less b/superset/assets/stylesheets/superset.less index c5a8ea7d4576f..488adebd5ed62 100644 --- a/superset/assets/stylesheets/superset.less +++ b/superset/assets/stylesheets/superset.less @@ -200,6 +200,7 @@ div.widget { .stack-trace-container, .slice_container { opacity: 0.5; + position: relative; } } }