diff --git a/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js b/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js index 0dfca9cdbfa3..1da05689d5e5 100644 --- a/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js +++ b/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js @@ -39,6 +39,7 @@ import { } from '../../../../src/dashboard/actions/dashboardLayout'; import { setUnsavedChanges } from '../../../../src/dashboard/actions/dashboardState'; +import * as dashboardFilters from '../../../../src/dashboard/actions/dashboardFilters'; import { addWarningToast, ADD_TOAST, @@ -80,6 +81,12 @@ describe('dashboardLayout actions', () => { return { getState, dispatch, state }; } + beforeEach(() => { + sinon.spy(dashboardFilters, 'updateLayoutComponents'); + }); + afterEach(() => { + dashboardFilters.updateLayoutComponents.restore(); + }); describe('updateComponents', () => { it('should dispatch an updateLayout action', () => { @@ -92,6 +99,9 @@ describe('dashboardLayout actions', () => { type: UPDATE_COMPONENTS, payload: { nextComponents }, }); + + // update component should not trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(0); }); it('should dispatch a setUnsavedChanges action if hasUnsavedChanges=false', () => { @@ -101,8 +111,10 @@ describe('dashboardLayout actions', () => { const nextComponents = { 1: {} }; const thunk = updateComponents(nextComponents); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(2); expect(dispatch.getCall(1).args[0]).toEqual(setUnsavedChanges(true)); + + // update component should not trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(0); }); }); @@ -111,11 +123,13 @@ describe('dashboardLayout actions', () => { const { getState, dispatch } = setup(); const thunk = deleteComponent('id', 'parentId'); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(1); expect(dispatch.getCall(0).args[0]).toEqual({ type: DELETE_COMPONENT, payload: { id: 'id', parentId: 'parentId' }, }); + + // delete components should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should dispatch a setUnsavedChanges action if hasUnsavedChanges=false', () => { @@ -124,8 +138,10 @@ describe('dashboardLayout actions', () => { }); const thunk = deleteComponent('id', 'parentId'); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(2); - expect(dispatch.getCall(1).args[0]).toEqual(setUnsavedChanges(true)); + expect(dispatch.getCall(2).args[0]).toEqual(setUnsavedChanges(true)); + + // delete components should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); }); @@ -149,7 +165,8 @@ describe('dashboardLayout actions', () => { }, }); - expect(dispatch.callCount).toBe(2); + // update dashboard title should not trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(0); }); }); @@ -159,11 +176,13 @@ describe('dashboardLayout actions', () => { const dropResult = {}; const thunk = createTopLevelTabs(dropResult); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(1); expect(dispatch.getCall(0).args[0]).toEqual({ type: CREATE_TOP_LEVEL_TABS, payload: { dropResult }, }); + + // create top level tabs should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should dispatch a setUnsavedChanges action if hasUnsavedChanges=false', () => { @@ -173,8 +192,10 @@ describe('dashboardLayout actions', () => { const dropResult = {}; const thunk = createTopLevelTabs(dropResult); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(2); - expect(dispatch.getCall(1).args[0]).toEqual(setUnsavedChanges(true)); + expect(dispatch.getCall(2).args[0]).toEqual(setUnsavedChanges(true)); + + // create top level tabs should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); }); @@ -184,11 +205,13 @@ describe('dashboardLayout actions', () => { const dropResult = {}; const thunk = deleteTopLevelTabs(dropResult); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(1); expect(dispatch.getCall(0).args[0]).toEqual({ type: DELETE_TOP_LEVEL_TABS, payload: {}, }); + + // delete top level tabs should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should dispatch a setUnsavedChanges action if hasUnsavedChanges=false', () => { @@ -198,8 +221,10 @@ describe('dashboardLayout actions', () => { const dropResult = {}; const thunk = deleteTopLevelTabs(dropResult); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(2); - expect(dispatch.getCall(1).args[0]).toEqual(setUnsavedChanges(true)); + expect(dispatch.getCall(2).args[0]).toEqual(setUnsavedChanges(true)); + + // delete top level tabs should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); }); @@ -261,6 +286,9 @@ describe('dashboardLayout actions', () => { thunk2(dispatch, getState); expect(dispatch.callCount).toBe(3); + + // resize components should not trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(0); }); }); @@ -286,7 +314,8 @@ describe('dashboardLayout actions', () => { }, }); - expect(dispatch.callCount).toBe(2); + // create components should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should move a component if the component is not new', () => { @@ -315,7 +344,8 @@ describe('dashboardLayout actions', () => { }, }); - expect(dispatch.callCount).toBe(2); + // create components should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should dispatch a toast if the drop overflows the destination', () => { @@ -379,9 +409,6 @@ describe('dashboardLayout actions', () => { parentId: 'parentId', }, }); - - // move thunk, delete thunk, delete result actions - expect(dispatch.callCount).toBe(3); }); it('should create top-level tabs if dropped on root', () => { @@ -404,8 +431,6 @@ describe('dashboardLayout actions', () => { dropResult, }, }); - - expect(dispatch.callCount).toBe(2); }); it('should dispatch a toast if drop top-level tab into nested tab', () => { @@ -497,8 +522,10 @@ describe('dashboardLayout actions', () => { const thunk = redoLayoutAction(); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(1); expect(dispatch.getCall(0).args[0]).toEqual(UndoActionCreators.redo()); + + // redo/undo should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); it('should dispatch a setUnsavedChanges(true) action if hasUnsavedChanges=false', () => { @@ -507,9 +534,10 @@ describe('dashboardLayout actions', () => { }); const thunk = redoLayoutAction(); thunk(dispatch, getState); + expect(dispatch.getCall(2).args[0]).toEqual(setUnsavedChanges(true)); - expect(dispatch.callCount).toBe(2); - expect(dispatch.getCall(1).args[0]).toEqual(setUnsavedChanges(true)); + // redo/undo should trigger action for dashboardFilters + expect(dashboardFilters.updateLayoutComponents.callCount).toEqual(1); }); }); }); diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx index 02e71e44c9e5..fbf44fa1884e 100644 --- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx @@ -24,7 +24,7 @@ import Dashboard from '../../../../src/dashboard/components/Dashboard'; import DashboardBuilder from '../../../../src/dashboard/containers/DashboardBuilder'; // mock data -import chartQueries, { sliceId as chartId } from '../fixtures/mockChartQueries'; +import chartQueries from '../fixtures/mockChartQueries'; import datasources from '../../../fixtures/mockDatasource'; import dashboardInfo from '../fixtures/mockDashboardInfo'; import { dashboardLayout } from '../fixtures/mockDashboardLayout'; @@ -46,7 +46,7 @@ describe('Dashboard', () => { dashboardState, dashboardInfo, charts: chartQueries, - filters: {}, + activeFilters: {}, slices: sliceEntities.slices, datasources, layout: dashboardLayout.present, @@ -61,10 +61,12 @@ describe('Dashboard', () => { return wrapper; } + // activeFilters map use id_column) as key const OVERRIDE_FILTERS = { - 1: { region: [] }, - 2: { country_name: ['USA'] }, - 3: { region: [], country_name: ['USA'] }, + '1_region': [], + '2_country_name': ['USA'], + '3_region': [], + '3_country_name': ['USA'], }; it('should render a DashboardBuilder', () => { @@ -72,85 +74,6 @@ describe('Dashboard', () => { expect(wrapper.find(DashboardBuilder)).toHaveLength(1); }); - describe('refreshExcept', () => { - const overrideDashboardInfo = { - ...dashboardInfo, - metadata: { - ...dashboardInfo.metadata, - filterImmuneSliceFields: { [chartQueries[chartId].id]: ['region'] }, - }, - }; - - const overrideCharts = { - ...chartQueries, - 1001: { - ...chartQueries[chartId], - id: 1001, - }, - }; - - const overrideSlices = { - ...props.slices, - 1001: { - ...props.slices[chartId], - slice_id: 1001, - }, - }; - - it('should call triggerQuery for all non-exempt slices', () => { - const wrapper = setup({ charts: overrideCharts, slices: overrideSlices }); - 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 triggerQuery for filterImmuneSlices', () => { - const wrapper = setup({ - charts: overrideCharts, - dashboardInfo: { - ...dashboardInfo, - metadata: { - ...dashboardInfo.metadata, - filterImmuneSlices: Object.keys(overrideCharts).map(id => - Number(id), - ), - }, - }, - }); - const spy = sinon.spy(props.actions, 'triggerQuery'); - wrapper.instance().refreshExcept(); - spy.restore(); - expect(spy.callCount).toBe(0); - }); - - it('should not call triggerQuery for filterImmuneSliceFields', () => { - const wrapper = setup({ - filters: OVERRIDE_FILTERS, - 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({ - filters: OVERRIDE_FILTERS, - 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', () => { const layoutWithExtraChart = { ...props.layout, @@ -186,17 +109,17 @@ describe('Dashboard', () => { describe('componentDidUpdate', () => { let wrapper; let prevProps; - let refreshExceptSpy; + let refreshSpy; beforeEach(() => { - wrapper = setup({ filters: OVERRIDE_FILTERS }); + wrapper = setup({ activeFilters: OVERRIDE_FILTERS }); wrapper.instance().appliedFilters = OVERRIDE_FILTERS; prevProps = wrapper.instance().props; - refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept'); + refreshSpy = sinon.spy(wrapper.instance(), 'refreshCharts'); }); afterEach(() => { - refreshExceptSpy.restore(); + refreshSpy.restore(); }); it('should not call refresh when is editMode', () => { @@ -207,15 +130,15 @@ describe('Dashboard', () => { }, }); wrapper.instance().componentDidUpdate(prevProps); - expect(refreshExceptSpy.callCount).toBe(0); + expect(refreshSpy.callCount).toBe(0); }); it('should not call refresh when there is no change', () => { wrapper.setProps({ - filters: OVERRIDE_FILTERS, + activeFilters: OVERRIDE_FILTERS, }); wrapper.instance().componentDidUpdate(prevProps); - expect(refreshExceptSpy.callCount).toBe(0); + expect(refreshSpy.callCount).toBe(0); expect(wrapper.instance().appliedFilters).toBe(OVERRIDE_FILTERS); }); @@ -224,12 +147,12 @@ describe('Dashboard', () => { gender: ['boy', 'girl'], }; wrapper.setProps({ - filters: { + activeFilters: { ...OVERRIDE_FILTERS, ...newFilter, }, }); - expect(refreshExceptSpy.callCount).toBe(1); + expect(refreshSpy.callCount).toBe(1); expect(wrapper.instance().appliedFilters).toEqual({ ...OVERRIDE_FILTERS, ...newFilter, @@ -238,23 +161,23 @@ describe('Dashboard', () => { it('should call refresh if a filter is removed', () => { wrapper.setProps({ - filters: {}, + activeFilters: {}, }); - expect(refreshExceptSpy.callCount).toBe(1); + expect(refreshSpy.callCount).toBe(1); expect(wrapper.instance().appliedFilters).toEqual({}); }); it('should call refresh if a filter is changed', () => { wrapper.setProps({ - filters: { + activeFilters: { ...OVERRIDE_FILTERS, - region: ['Canada'], + '1_region': ['Canada'], }, }); - expect(refreshExceptSpy.callCount).toBe(1); + expect(refreshSpy.callCount).toBe(1); expect(wrapper.instance().appliedFilters).toEqual({ ...OVERRIDE_FILTERS, - region: ['Canada'], + '1_region': ['Canada'], }); }); }); diff --git a/superset/assets/spec/javascripts/dashboard/components/FilterIndicatorsContainer_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/FilterIndicatorsContainer_spec.jsx index 0e5392fca4ef..089f18540902 100644 --- a/superset/assets/spec/javascripts/dashboard/components/FilterIndicatorsContainer_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/FilterIndicatorsContainer_spec.jsx @@ -20,28 +20,34 @@ import React from 'react'; import { shallow } from 'enzyme'; import { dashboardFilters } from '../fixtures/mockDashboardFilters'; +import { sliceId as chartId } from '../fixtures/mockChartQueries'; import { filterId, column } from '../fixtures/mockSliceEntities'; import FilterIndicatorsContainer from '../../../../src/dashboard/components/FilterIndicatorsContainer'; import FilterIndicator from '../../../../src/dashboard/components/FilterIndicator'; import * as colorMap from '../../../../src/dashboard/util/dashboardFiltersColorMap'; +import { buildActiveFilters } from '../../../../src/dashboard/util/activeDashboardFilters'; +import { getDashboardFilterKey } from '../../../../src/dashboard/util/getDashboardFilterKey'; +import { DASHBOARD_ROOT_ID } from '../../../../src/dashboard/util/constants'; +import { dashboardWithFilter } from '../fixtures/mockDashboardLayout'; describe('FilterIndicatorsContainer', () => { - const chartId = 1; const mockedProps = { dashboardFilters, chartId, chartStatus: 'success', - filterImmuneSlices: [], - filterImmuneSliceFields: {}, setDirectPathToChild: () => {}, filterFieldOnFocus: {}, }; - colorMap.getFilterColorKey = jest.fn(() => 'id_column'); colorMap.getFilterColorMap = jest.fn(() => ({ - id_column: 'badge-1', + [getDashboardFilterKey({ chartId, column })]: 'badge-1', })); + buildActiveFilters({ + dashboardFilters, + components: dashboardWithFilter, + }); + function setup(overrideProps) { return shallow( , @@ -58,18 +64,25 @@ describe('FilterIndicatorsContainer', () => { expect(wrapper.find(FilterIndicator)).toHaveLength(0); }); - it('should not show indicator when chart is immune', () => { - const wrapper = setup({ filterImmuneSlices: [chartId] }); - expect(wrapper.find(FilterIndicator)).toHaveLength(0); - }); - - it('should not show indicator when chart field is immune', () => { - const wrapper = setup({ filterImmuneSliceFields: { [chartId]: [column] } }); - expect(wrapper.find(FilterIndicator)).toHaveLength(0); - }); - it('should show indicator', () => { const wrapper = setup(); expect(wrapper.find(FilterIndicator)).toHaveLength(1); }); + + it('should not show indicator when chart is immune', () => { + const overwriteDashboardFilters = { + ...dashboardFilters, + [filterId]: { + ...dashboardFilters[filterId], + scopes: { + region: { + scope: [DASHBOARD_ROOT_ID], + immune: [chartId], + }, + }, + }, + }; + const wrapper = setup({ dashboardFilters: overwriteDashboardFilters }); + expect(wrapper.find(FilterIndicator)).toHaveLength(0); + }); }); diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardFilters.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardFilters.js index 7f13db721500..c5640c512402 100644 --- a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardFilters.js +++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardFilters.js @@ -36,6 +36,7 @@ export const dashboardFilters = { ], scopes: { region: DASHBOARD_FILTER_SCOPE_GLOBAL, + gender: DASHBOARD_FILTER_SCOPE_GLOBAL, }, isDateFilter: false, isInstantFilter: true, diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardLayout.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardLayout.js index 3267c5c7ecc5..ee1dc4fe345f 100644 --- a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardLayout.js +++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardLayout.js @@ -207,3 +207,54 @@ export const filterComponent = { sliceName: 'Filter', }, }; + +export const dashboardWithFilter = { + [DASHBOARD_ROOT_ID]: { + type: DASHBOARD_ROOT_TYPE, + id: DASHBOARD_ROOT_ID, + children: [DASHBOARD_GRID_ID], + }, + + [DASHBOARD_GRID_ID]: { + type: DASHBOARD_GRID_TYPE, + id: DASHBOARD_GRID_ID, + children: ['ROW_ID'], + meta: {}, + }, + + [DASHBOARD_HEADER_ID]: { + type: DASHBOARD_HEADER_TYPE, + id: DASHBOARD_HEADER_ID, + meta: { + text: 'New dashboard', + }, + }, + + ROW_ID: { + ...newComponentFactory(ROW_TYPE), + id: 'ROW_ID', + children: ['CHART_ID', 'FILTER_ID'], + }, + + FILTER_ID: { + ...newComponentFactory(CHART_TYPE), + id: 'FILTER_ID', + meta: { + chartId: filterId, + width: 3, + height: 10, + chartName: 'filter name', + }, + }, + + CHART_ID: { + ...newComponentFactory(CHART_TYPE), + id: 'CHART_ID', + meta: { + chartId, + width: 3, + height: 10, + chartName: 'Mock chart name', + }, + }, +}; diff --git a/superset/assets/spec/javascripts/dashboard/reducers/dashboardFilters_spec.js b/superset/assets/spec/javascripts/dashboard/reducers/dashboardFilters_spec.js index e234248ce3bc..b759610e4290 100644 --- a/superset/assets/spec/javascripts/dashboard/reducers/dashboardFilters_spec.js +++ b/superset/assets/spec/javascripts/dashboard/reducers/dashboardFilters_spec.js @@ -21,6 +21,7 @@ import { ADD_FILTER, REMOVE_FILTER, CHANGE_FILTER, + UPDATE_DASHBOARD_FILTERS_SCOPE, } from '../../../../src/dashboard/actions/dashboardFilters'; import dashboardFiltersReducer, { DASHBOARD_FILTER_SCOPE_GLOBAL, @@ -35,6 +36,7 @@ import { column, } from '../fixtures/mockSliceEntities'; import { filterComponent } from '../fixtures/mockDashboardLayout'; +import * as activeDashboardFilters from '../../../../src/dashboard/util/activeDashboardFilters'; describe('dashboardFilters reducer', () => { const form_data = sliceEntitiesForDashboard.slices[filterId].form_data; @@ -98,6 +100,7 @@ describe('dashboardFilters reducer', () => { }, scopes: { [column]: DASHBOARD_FILTER_SCOPE_GLOBAL, + gender: DASHBOARD_FILTER_SCOPE_GLOBAL, }, }, }); @@ -129,7 +132,8 @@ describe('dashboardFilters reducer', () => { [column]: column, }, scopes: { - [column]: DASHBOARD_FILTER_SCOPE_GLOBAL, + region: DASHBOARD_FILTER_SCOPE_GLOBAL, + gender: DASHBOARD_FILTER_SCOPE_GLOBAL, }, }, }); @@ -143,4 +147,33 @@ describe('dashboardFilters reducer', () => { }), ).toEqual({}); }); + + it('should buildActiveFilters on UPDATE_DASHBOARD_FILTERS_SCOPE', () => { + const regionScope = { + scope: ['TAB-1'], + immune: [], + }; + const genderScope = { + scope: ['ROOT_ID'], + immune: [1], + }; + const scopes = { + [`${filterId}_region`]: regionScope, + [`${filterId}_gender`]: genderScope, + }; + activeDashboardFilters.buildActiveFilters = jest.fn(); + expect( + dashboardFiltersReducer(dashboardFilters, { + type: UPDATE_DASHBOARD_FILTERS_SCOPE, + scopes, + })[filterId].scopes, + ).toEqual({ + region: regionScope, + gender: genderScope, + }); + + // when UPDATE_DASHBOARD_FILTERS_SCOPE is changed, applicable filters to a chart + // might be changed. + expect(activeDashboardFilters.buildActiveFilters).toBeCalled(); + }); }); diff --git a/superset/assets/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset/assets/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index 76a2da666f6c..8b6b7fc97098 100644 --- a/superset/assets/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/superset/assets/spec/javascripts/dashboard/util/getDashboardUrl_spec.js @@ -17,10 +17,16 @@ * under the License. */ import getDashboardUrl from '../../../../src/dashboard/util/getDashboardUrl'; +import { DASHBOARD_FILTER_SCOPE_GLOBAL } from '../../../../src/dashboard/reducers/dashboardFilters'; describe('getChartIdsFromLayout', () => { it('should encode filters', () => { - const filters = { 35: { key: ['value'] } }; + const filters = { + '35_key': { + values: ['value'], + scope: DASHBOARD_FILTER_SCOPE_GLOBAL, + }, + }; const url = getDashboardUrl('path', filters); expect(url).toBe( 'path?preselect_filters=%7B%2235%22%3A%7B%22key%22%3A%5B%22value%22%5D%7D%7D', diff --git a/superset/assets/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.js b/superset/assets/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.js index 544b2d484f5e..95cb1419d992 100644 --- a/superset/assets/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.js +++ b/superset/assets/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.js @@ -33,15 +33,9 @@ describe('getFormDataWithExtraFilters', () => { ], }, }, - dashboardMetadata: { - filterImmuneSlices: [], - filterImmuneSliceFields: {}, - }, filters: { - filterId: { - region: ['Spain'], - color: ['pink', 'purple'], - }, + region: ['Spain'], + color: ['pink', 'purple'], }, sliceId: chartId, }; @@ -60,26 +54,4 @@ describe('getFormDataWithExtraFilters', () => { val: ['pink', 'purple'], }); }); - - it('should not add additional filters if the slice is immune to them', () => { - const result = getFormDataWithExtraFilters({ - ...mockArgs, - dashboardMetadata: { - filterImmuneSlices: [chartId], - }, - }); - expect(result.extra_filters).toHaveLength(0); - }); - - it('should not add additional filters for fields to which the slice is immune', () => { - const result = getFormDataWithExtraFilters({ - ...mockArgs, - dashboardMetadata: { - filterImmuneSliceFields: { - [chartId]: ['region'], - }, - }, - }); - expect(result.extra_filters).toHaveLength(1); - }); }); diff --git a/superset/assets/src/dashboard/actions/dashboardFilters.js b/superset/assets/src/dashboard/actions/dashboardFilters.js index c2b8b9c1d99a..b8f92b8df05e 100644 --- a/superset/assets/src/dashboard/actions/dashboardFilters.js +++ b/superset/assets/src/dashboard/actions/dashboardFilters.js @@ -46,11 +46,13 @@ export const CHANGE_FILTER = 'CHANGE_FILTER'; export function changeFilter(chartId, newSelectedValues, merge) { return (dispatch, getState) => { if (isValidFilter(getState, chartId)) { + const components = getState().dashboardLayout.present; return dispatch({ type: CHANGE_FILTER, chartId, newSelectedValues, merge, + components, }); } return getState().dashboardFilters; @@ -66,3 +68,17 @@ export function updateDirectPathToFilter(chartId, path) { return getState().dashboardFilters; }; } + +export const UPDATE_LAYOUT_COMPONENTS = 'UPDATE_LAYOUT_COMPONENTS'; +export function updateLayoutComponents(components) { + return dispatch => { + dispatch({ type: UPDATE_LAYOUT_COMPONENTS, components }); + }; +} + +export const UPDATE_DASHBOARD_FILTERS_SCOPE = 'UPDATE_DASHBOARD_FILTERS_SCOPE'; +export function updateDashboardFiltersScope(scopes) { + return dispatch => { + dispatch({ type: UPDATE_DASHBOARD_FILTERS_SCOPE, scopes }); + }; +} diff --git a/superset/assets/src/dashboard/actions/dashboardLayout.js b/superset/assets/src/dashboard/actions/dashboardLayout.js index b276e374bb90..1122918d2f00 100644 --- a/superset/assets/src/dashboard/actions/dashboardLayout.js +++ b/superset/assets/src/dashboard/actions/dashboardLayout.js @@ -20,6 +20,7 @@ import { ActionCreators as UndoActionCreators } from 'redux-undo'; import { t } from '@superset-ui/translation'; import { addWarningToast } from '../../messageToasts/actions'; +import { updateLayoutComponents } from './dashboardFilters'; import { setUnsavedChanges } from './dashboardState'; import { TABS_TYPE, ROW_TYPE } from '../util/componentTypes'; import { @@ -29,6 +30,10 @@ import { } from '../util/constants'; import dropOverflowsParent from '../util/dropOverflowsParent'; import findParentId from '../util/findParentId'; +import isInDifferentFilterScopes from '../util/isInDifferentFilterScopes'; + +// Component CRUD ------------------------------------------------------------- +export const UPDATE_COMPONENTS = 'UPDATE_COMPONENTS'; // this is a helper that takes an action as input and dispatches // an additional setUnsavedChanges(true) action after the dispatch in the case @@ -42,15 +47,22 @@ function setUnsavedChangesAfterAction(action) { dispatch(result); } + const isComponentLevelEvent = + result.type === UPDATE_COMPONENTS && + result.payload && + result.payload.nextComponents; + // trigger dashboardFilters state update if dashboard layout is changed. + if (!isComponentLevelEvent) { + const components = getState().dashboardLayout.present; + dispatch(updateLayoutComponents(components)); + } + if (!getState().dashboardState.hasUnsavedChanges) { dispatch(setUnsavedChanges(true)); } }; } -// Component CRUD ------------------------------------------------------------- -export const UPDATE_COMPONENTS = 'UPDATE_COMPONENTS'; - export const updateComponents = setUnsavedChangesAfterAction( nextComponents => ({ type: UPDATE_COMPONENTS, @@ -199,12 +211,13 @@ export function handleComponentDrop(dropResult) { // call getState() again down here in case redux state is stale after // previous dispatch(es) - const { dashboardLayout: undoableLayout } = getState(); + const { dashboardFilters, dashboardLayout: undoableLayout } = getState(); // if we moved a child from a Tab or Row parent and it was the only child, delete the parent. if (!isNewComponent) { const { present: layout } = undoableLayout; - const sourceComponent = layout[source.id]; + const sourceComponent = layout[source.id] || {}; + const destinationComponent = layout[destination.id] || {}; if ( (sourceComponent.type === TABS_TYPE || sourceComponent.type === ROW_TYPE) && @@ -217,6 +230,23 @@ export function handleComponentDrop(dropResult) { }); dispatch(deleteComponent(source.id, parentId)); } + + // show warning if item has been moved between different scope + if ( + isInDifferentFilterScopes({ + dashboardFilters, + source: (sourceComponent.parents || []).concat(source.id), + destination: (destinationComponent.parents || []).concat( + destination.id, + ), + }) + ) { + dispatch( + addWarningToast( + t('This chart has been moved to a different filter scope.'), + ), + ); + } } return null; diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js index b1d850fb89d1..cea6e25564eb 100644 --- a/superset/assets/src/dashboard/actions/dashboardState.js +++ b/superset/assets/src/dashboard/actions/dashboardState.js @@ -38,6 +38,10 @@ import { addDangerToast, } from '../../messageToasts/actions'; import { UPDATE_COMPONENTS_PARENTS_LIST } from '../actions/dashboardLayout'; +import serializeActiveFilterValues from '../util/serializeActiveFilterValues'; +import serializeFilterScopes from '../util/serializeFilterScopes'; +import { getActiveFilters } from '../util/activeDashboardFilters'; +import { safeStringify } from '../../utils/safeStringify'; export const SET_UNSAVED_CHANGES = 'SET_UNSAVED_CHANGES'; export function setUnsavedChanges(hasUnsavedChanges) { @@ -178,10 +182,19 @@ export function saveDashboardRequest(data, id, saveType) { directPathToFilter.push(componentId); dispatch(updateDirectPathToFilter(chartId, directPathToFilter)); }); - + // serialize selected values for each filter field, grouped by filter id + const serializedFilters = serializeActiveFilterValues(getActiveFilters()); + // serialize filter scope for each filter field, grouped by filter id + const serializedFilterScopes = serializeFilterScopes(dashboardFilters); return SupersetClient.post({ endpoint: `/superset/${path}/${id}/`, - postPayload: { data }, + postPayload: { + data: { + ...data, + default_filters: safeStringify(serializedFilters), + filter_scopes: safeStringify(serializedFilterScopes), + }, + }, }) .then(response => { dispatch(saveDashboardRequestSuccess()); diff --git a/superset/assets/src/dashboard/components/Dashboard.jsx b/superset/assets/src/dashboard/components/Dashboard.jsx index 334704e4c697..441f80798b60 100644 --- a/superset/assets/src/dashboard/components/Dashboard.jsx +++ b/superset/assets/src/dashboard/components/Dashboard.jsx @@ -28,9 +28,7 @@ import { slicePropShape, dashboardInfoPropShape, dashboardStatePropShape, - loadStatsPropShape, } from '../util/propShapes'; -import { areObjectsEqual } from '../../reduxUtils'; import { LOG_ACTIONS_MOUNT_DASHBOARD } from '../../logger/LogUtils'; import OmniContainer from '../../components/OmniContainer'; import { safeStringify } from '../../utils/safeStringify'; @@ -48,9 +46,8 @@ const propTypes = { dashboardState: dashboardStatePropShape.isRequired, charts: PropTypes.objectOf(chartPropShape).isRequired, slices: PropTypes.objectOf(slicePropShape).isRequired, - filters: PropTypes.object.isRequired, + activeFilters: PropTypes.object.isRequired, datasources: PropTypes.object.isRequired, - loadStats: loadStatsPropShape.isRequired, layout: PropTypes.object.isRequired, impressionId: PropTypes.string.isRequired, initMessages: PropTypes.array, @@ -118,31 +115,42 @@ class Dashboard extends React.PureComponent { const { hasUnsavedChanges, editMode } = this.props.dashboardState; const appliedFilters = this.appliedFilters; - const { filters } = this.props; + const { activeFilters } = this.props; // do not apply filter when dashboard in edit mode - if (!editMode && safeStringify(appliedFilters) !== safeStringify(filters)) { + if ( + !editMode && + safeStringify(appliedFilters) !== safeStringify(activeFilters) + ) { // refresh charts if a filter was removed, added, or changed - let changedFilterKey = null; - const currFilterKeys = Object.keys(filters); + const currFilterKeys = Object.keys(activeFilters); const appliedFilterKeys = Object.keys(appliedFilters); - currFilterKeys.forEach(key => { - if ( - // filter was added or changed - typeof appliedFilters[key] === 'undefined' || - !areObjectsEqual(appliedFilters[key], filters[key]) + const allKeys = new Set(currFilterKeys.concat(appliedFilterKeys)); + const affectedChartIds = []; + [...allKeys].forEach(filterKey => { + if (!currFilterKeys.includes(filterKey)) { + // removed filter? + [].push.apply(affectedChartIds, appliedFilters[filterKey].scope); + } else if (!appliedFilterKeys.includes(filterKey)) { + // added filter? + [].push.apply(affectedChartIds, activeFilters[filterKey].scope); + } else if ( + safeStringify(activeFilters[filterKey].values) !== + safeStringify(appliedFilters[filterKey].values) || + safeStringify(activeFilters[filterKey].scope) !== + safeStringify(appliedFilters[filterKey].scope) ) { - changedFilterKey = key; + // changed filter field value? + const affectedScope = activeFilters[filterKey].scope.concat( + appliedFilters[filterKey].scope, + ); + [].push.apply(affectedChartIds, affectedScope); } }); - if ( - !!changedFilterKey || - currFilterKeys.length !== appliedFilterKeys.length // remove 1 or more filters - ) { - this.refreshExcept(changedFilterKey); - this.appliedFilters = filters; - } + const idSet = new Set(affectedChartIds); + this.refreshCharts([...idSet]); + this.appliedFilters = activeFilters; } if (hasUnsavedChanges) { @@ -157,35 +165,9 @@ class Dashboard extends React.PureComponent { return Object.values(this.props.charts); } - refreshExcept(filterKey) { - const { filters } = this.props; - const currentFilteredNames = - filterKey && filters[filterKey] ? Object.keys(filters[filterKey]) : []; - const filterImmuneSlices = this.props.dashboardInfo.metadata - .filterImmuneSlices; - const filterImmuneSliceFields = this.props.dashboardInfo.metadata - .filterImmuneSliceFields; - - this.getAllCharts().forEach(chart => { - // filterKey is a string, filter_immune_slices array contains numbers - if ( - String(chart.id) === filterKey || - filterImmuneSlices.includes(chart.id) - ) { - return; - } - - const filterImmuneSliceFieldsNames = - filterImmuneSliceFields[chart.id] || []; - // has filter-able field names - if ( - currentFilteredNames.length === 0 || - currentFilteredNames.some( - name => !filterImmuneSliceFieldsNames.includes(name), - ) - ) { - this.props.actions.triggerQuery(true, chart.id); - } + refreshCharts(ids) { + ids.forEach(id => { + this.props.actions.triggerQuery(true, id); }); } diff --git a/superset/assets/src/dashboard/components/FilterIndicatorsContainer.jsx b/superset/assets/src/dashboard/components/FilterIndicatorsContainer.jsx index f2871085ec86..795c645b4cce 100644 --- a/superset/assets/src/dashboard/components/FilterIndicatorsContainer.jsx +++ b/superset/assets/src/dashboard/components/FilterIndicatorsContainer.jsx @@ -23,6 +23,7 @@ import { isEmpty } from 'lodash'; import FilterIndicator from './FilterIndicator'; import FilterIndicatorGroup from './FilterIndicatorGroup'; import { FILTER_INDICATORS_DISPLAY_LENGTH } from '../util/constants'; +import { getChartIdsInFilterScope } from '../util/activeDashboardFilters'; import { getDashboardFilterKey } from '../util/getDashboardFilterKey'; import { getFilterColorMap } from '../util/dashboardFiltersColorMap'; @@ -33,8 +34,6 @@ const propTypes = { chartStatus: PropTypes.string, // from redux - filterImmuneSlices: PropTypes.arrayOf(PropTypes.number).isRequired, - filterImmuneSliceFields: PropTypes.object.isRequired, setDirectPathToChild: PropTypes.func.isRequired, filterFieldOnFocus: PropTypes.object.isRequired, }; @@ -59,8 +58,6 @@ export default class FilterIndicatorsContainer extends React.PureComponent { const { dashboardFilters, chartId: currentChartId, - filterImmuneSlices, - filterImmuneSliceFields, filterFieldOnFocus, } = this.props; @@ -76,60 +73,50 @@ export default class FilterIndicatorsContainer extends React.PureComponent { chartId, componentId, directPathToFilter, - scope, isDateFilter, isInstantFilter, columns, labels, + scopes, } = dashboardFilter; - // do not apply filter on filter_box itself - // do not apply filter on filterImmuneSlices list - if ( - currentChartId !== chartId && - !filterImmuneSlices.includes(currentChartId) - ) { - Object.keys(columns).forEach(name => { - const colorMapKey = getDashboardFilterKey({ - chartId, - column: name, + if (currentChartId !== chartId) { + Object.keys(columns) + .filter(name => + getChartIdsInFilterScope({ filterScope: scopes[name] }).includes( + currentChartId, + ), + ) + .forEach(name => { + const colorMapKey = getDashboardFilterKey({ + chartId, + column: name, + }); + const indicator = { + chartId, + colorCode: dashboardFiltersColorMap[colorMapKey], + componentId, + directPathToFilter: directPathToFilter.concat(`LABEL-${name}`), + isDateFilter, + isInstantFilter, + name, + label: labels[name] || name, + values: + isEmpty(columns[name]) || + (isDateFilter && columns[name] === 'No filter') + ? [] + : [].concat(columns[name]), + isFilterFieldActive: + chartId === filterFieldOnFocus.chartId && + name === filterFieldOnFocus.column, + }; + + if (isEmpty(indicator.values)) { + indicators[1].push(indicator); + } else { + indicators[0].push(indicator); + } }); - const directPathToLabel = directPathToFilter.slice(); - directPathToLabel.push(`LABEL-${name}`); - const indicator = { - chartId, - colorCode: dashboardFiltersColorMap[colorMapKey], - componentId, - directPathToFilter: directPathToLabel, - scope, - isDateFilter, - isInstantFilter, - name, - label: labels[name] || name, - values: - isEmpty(columns[name]) || - (isDateFilter && columns[name] === 'No filter') - ? [] - : [].concat(columns[name]), - isFilterFieldActive: - chartId === filterFieldOnFocus.chartId && - name === filterFieldOnFocus.column, - }; - - // do not apply filter on fields in the filterImmuneSliceFields map - if ( - filterImmuneSliceFields[currentChartId] && - filterImmuneSliceFields[currentChartId].includes(name) - ) { - return; - } - - if (isEmpty(indicator.values)) { - indicators[1].push(indicator); - } else { - indicators[0].push(indicator); - } - }); } return indicators; diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index 569dac97ec92..843eeb5c8dda 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -53,7 +53,6 @@ const propTypes = { dashboardTitle: PropTypes.string.isRequired, charts: PropTypes.objectOf(chartPropShape).isRequired, layout: PropTypes.object.isRequired, - filters: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, css: PropTypes.string.isRequired, colorNamespace: PropTypes.string, @@ -217,7 +216,6 @@ class Header extends React.PureComponent { css, colorNamespace, colorScheme, - filters, dashboardInfo, refreshFrequency, } = this.props; @@ -235,7 +233,6 @@ class Header extends React.PureComponent { color_scheme: colorScheme, label_colors: labelColors, dashboard_title: dashboardTitle, - default_filters: safeStringify(filters), refresh_frequency: refreshFrequency, }; @@ -263,7 +260,6 @@ class Header extends React.PureComponent { const { dashboardTitle, layout, - filters, expandedSlices, css, colorNamespace, @@ -415,7 +411,6 @@ class Header extends React.PureComponent { dashboardId={dashboardInfo.id} dashboardTitle={dashboardTitle} layout={layout} - filters={filters} expandedSlices={expandedSlices} css={css} colorNamespace={colorNamespace} diff --git a/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx b/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx index 6fc0182551e5..cf89e5eca289 100644 --- a/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx +++ b/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx @@ -29,6 +29,7 @@ import injectCustomCss from '../util/injectCustomCss'; import { SAVE_TYPE_NEWDASHBOARD } from '../util/constants'; import URLShortLinkModal from '../../components/URLShortLinkModal'; import getDashboardUrl from '../util/getDashboardUrl'; +import { getActiveFilters } from '../util/activeDashboardFilters'; const propTypes = { addSuccessToast: PropTypes.func.isRequired, @@ -50,7 +51,6 @@ const propTypes = { userCanSave: PropTypes.bool.isRequired, isLoading: PropTypes.bool.isRequired, layout: PropTypes.object.isRequired, - filters: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, onSave: PropTypes.func.isRequired, }; @@ -120,7 +120,6 @@ class HeaderActionsDropdown extends React.PureComponent { colorScheme, hasUnsavedChanges, layout, - filters, expandedSlices, onSave, userCanEdit, @@ -148,7 +147,6 @@ class HeaderActionsDropdown extends React.PureComponent { dashboardTitle={dashboardTitle} saveType={SAVE_TYPE_NEWDASHBOARD} layout={layout} - filters={filters} expandedSlices={expandedSlices} refreshFrequency={refreshFrequency} css={css} @@ -200,7 +198,7 @@ class HeaderActionsDropdown extends React.PureComponent { )} diff --git a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx index 65565f07cb29..779609d850c2 100644 --- a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx +++ b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx @@ -23,11 +23,11 @@ import { Dropdown, MenuItem } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; import URLShortLinkModal from '../../components/URLShortLinkModal'; import getDashboardUrl from '../util/getDashboardUrl'; +import { getActiveFilters } from '../util/activeDashboardFilters'; const propTypes = { slice: PropTypes.object.isRequired, componentId: PropTypes.string.isRequired, - filters: PropTypes.object.isRequired, addDangerToast: PropTypes.func.isRequired, isCached: PropTypes.bool, isExpanded: PropTypes.bool, @@ -107,7 +107,6 @@ class SliceHeaderControls extends React.PureComponent { isCached, cachedDttm, updatedDttm, - filters, componentId, addDangerToast, } = this.props; @@ -162,7 +161,7 @@ class SliceHeaderControls extends React.PureComponent { , - uncheck: , - halfCheck: , - expandClose: , - expandOpen: , - expandAll: ( - {t('Expand all')} - ), - collapseAll: ( - {t('Collapse all')} - ), - parentClose: , - parentOpen: , - leaf: , -}; - export default function FilterFieldTree({ activeKey, nodes = [], @@ -85,7 +62,7 @@ export default function FilterFieldTree({ onClick={onClick} onCheck={onCheck} onExpand={onExpand} - icons={FILTER_FIELD_CHECKBOX_TREE_ICONS} + icons={treeIcons} /> ); } diff --git a/superset/assets/src/dashboard/components/filterscope/FilterScopeModal.jsx b/superset/assets/src/dashboard/components/filterscope/FilterScopeModal.jsx index b2a8be8f0fca..fc4b76620f8e 100644 --- a/superset/assets/src/dashboard/components/filterscope/FilterScopeModal.jsx +++ b/superset/assets/src/dashboard/components/filterscope/FilterScopeModal.jsx @@ -35,7 +35,7 @@ export default class FilterScopeModal extends React.PureComponent { } handleCloseModal() { - if (this.modal) { + if (this.modal.current) { this.modal.current.close(); } } diff --git a/superset/assets/src/dashboard/components/filterscope/FilterScopeSelector.jsx b/superset/assets/src/dashboard/components/filterscope/FilterScopeSelector.jsx index ae42a57c835d..9435d7bc897e 100644 --- a/superset/assets/src/dashboard/components/filterscope/FilterScopeSelector.jsx +++ b/superset/assets/src/dashboard/components/filterscope/FilterScopeSelector.jsx @@ -26,24 +26,26 @@ import buildFilterScopeTreeEntry from '../../util/buildFilterScopeTreeEntry'; import getFilterScopeNodesTree from '../../util/getFilterScopeNodesTree'; import getFilterFieldNodesTree from '../../util/getFilterFieldNodesTree'; import getFilterScopeParentNodes from '../../util/getFilterScopeParentNodes'; -import getCurrentScopeChartIds from '../../util/getCurrentScopeChartIds'; import getKeyForFilterScopeTree from '../../util/getKeyForFilterScopeTree'; import getSelectedChartIdForFilterScopeTree from '../../util/getSelectedChartIdForFilterScopeTree'; +import getFilterScopeFromNodesTree from '../../util/getFilterScopeFromNodesTree'; import getRevertedFilterScope from '../../util/getRevertedFilterScope'; import FilterScopeTree from './FilterScopeTree'; import FilterFieldTree from './FilterFieldTree'; +import { getChartIdsInFilterScope } from '../../util/activeDashboardFilters'; import { getChartIdAndColumnFromFilterKey, getDashboardFilterKey, } from '../../util/getDashboardFilterKey'; import { ALL_FILTERS_ROOT } from '../../util/constants'; +import { dashboardFilterPropShape } from '../../util/propShapes'; const propTypes = { - dashboardFilters: PropTypes.object.isRequired, + dashboardFilters: PropTypes.objectOf(dashboardFilterPropShape).isRequired, layout: PropTypes.object.isRequired, - filterImmuneSlices: PropTypes.arrayOf(PropTypes.number).isRequired, - filterImmuneSliceFields: PropTypes.object.isRequired, - setDirectPathToChild: PropTypes.func.isRequired, + + updateDashboardFiltersScope: PropTypes.func.isRequired, + setUnsavedChanges: PropTypes.func.isRequired, onCloseModal: PropTypes.func.isRequired, }; @@ -51,12 +53,7 @@ export default class FilterScopeSelector extends React.PureComponent { constructor(props) { super(props); - const { - dashboardFilters, - filterImmuneSlices, - filterImmuneSliceFields, - layout, - } = props; + const { dashboardFilters, layout } = props; if (Object.keys(dashboardFilters).length > 0) { // display filter fields in tree structure @@ -88,14 +85,12 @@ export default class FilterScopeSelector extends React.PureComponent { filterFields: [filterKey], selectedChartId: filterId, }); - const checked = getCurrentScopeChartIds({ - scopeComponentIds: ['ROOT_ID'], // dashboardFilters[chartId].scopes[columnName], - filterField: columnName, - filterImmuneSlices, - filterImmuneSliceFields, - components: layout, - }); const expanded = getFilterScopeParentNodes(nodes, 1); + // display filter_box chart as checked, but do not show checkbox + const chartIdsInFilterScope = getChartIdsInFilterScope({ + filterScope: dashboardFilters[filterId].scopes[columnName], + }); + return { ...mapByChartId, [filterKey]: { @@ -103,7 +98,7 @@ export default class FilterScopeSelector extends React.PureComponent { nodes, // filtered nodes in display if searchText is not empty nodesFiltered: [...nodes], - checked, + checked: chartIdsInFilterScope, expanded, }, }; @@ -304,18 +299,28 @@ export default class FilterScopeSelector extends React.PureComponent { onSave() { const { filterScopeMap } = this.state; - console.log( - 'i am current state', - this.allfilterFields.reduce( - (map, key) => ({ + const allFilterFieldScopes = this.allfilterFields.reduce( + (map, filterKey) => { + const nodes = filterScopeMap[filterKey].nodes; + const checkedChartIds = filterScopeMap[filterKey].checked; + + return { ...map, - [key]: filterScopeMap[key].checked, - }), - {}, - ), + [filterKey]: getFilterScopeFromNodesTree({ + filterKey, + nodes, + checkedChartIds, + }), + }; + }, + {}, ); - // save does not close modal + this.props.updateDashboardFiltersScope(allFilterFieldScopes); + this.props.setUnsavedChanges(true); + + // click Save button will do save and close modal + this.props.onCloseModal(); } filterTree() { @@ -486,7 +491,7 @@ export default class FilterScopeSelector extends React.PureComponent {

{t('Configure filter scopes')}

- {this.renderEditingFiltersName()} + {showSelector && this.renderEditingFiltersName()}
{!showSelector ? ( @@ -503,7 +508,7 @@ export default class FilterScopeSelector extends React.PureComponent { )}
- + {showSelector && (