From d67e404200d6edb311a84724b0bd06ec186db776 Mon Sep 17 00:00:00 2001 From: Grace Date: Mon, 11 Nov 2019 02:03:19 -0800 Subject: [PATCH 1/7] merge filter scope settings into dashboard redux state --- .../src/dashboard/actions/dashboardFilters.js | 16 +++ .../src/dashboard/actions/dashboardLayout.js | 40 +++++- .../src/dashboard/components/Dashboard.jsx | 82 +++++------ .../components/FilterIndicatorsContainer.jsx | 30 ++-- .../src/dashboard/components/Header.jsx | 15 +- .../components/HeaderActionsDropdown.jsx | 12 +- .../src/dashboard/components/SaveModal.jsx | 9 +- .../src/dashboard/components/SliceHeader.jsx | 2 - .../components/SliceHeaderControls.jsx | 5 +- .../filterscope/FilterScopeSelector.jsx | 60 ++++---- .../components/gridComponents/Chart.jsx | 16 ++- .../assets/src/dashboard/containers/Chart.jsx | 11 +- .../src/dashboard/containers/Dashboard.jsx | 10 +- .../dashboard/containers/DashboardHeader.jsx | 12 +- .../dashboard/containers/FilterIndicators.jsx | 6 +- .../src/dashboard/containers/FilterScope.jsx | 11 +- .../dashboard/reducers/dashboardFilters.js | 50 ++++++- .../src/dashboard/reducers/dashboardLayout.js | 1 + .../src/dashboard/reducers/getInitialState.js | 51 ++++++- .../dashboard/util/activeDashboardFilters.js | 132 ++++++++++++++---- .../util/charts/getEffectiveExtraFilters.js | 43 +----- .../charts/getFormDataWithExtraFilters.js | 13 +- .../dashboard/util/getCurrentScopeChartIds.js | 62 -------- .../src/dashboard/util/getDashboardUrl.js | 9 +- .../util/getFilterValuesByFilterId.js | 38 +++++ .../util/isInDifferentFilterScopes.js | 33 +++++ .../src/dashboard/util/newEntitiesFromDrop.js | 4 + .../assets/src/dashboard/util/propShapes.jsx | 26 ++-- .../util/serializeActiveFilterValues.js | 36 +++++ .../dashboard/util/serializeFilterScopes.js | 34 +++++ superset/views/core.py | 14 +- 31 files changed, 567 insertions(+), 316 deletions(-) delete mode 100644 superset/assets/src/dashboard/util/getCurrentScopeChartIds.js create mode 100644 superset/assets/src/dashboard/util/getFilterValuesByFilterId.js create mode 100644 superset/assets/src/dashboard/util/isInDifferentFilterScopes.js create mode 100644 superset/assets/src/dashboard/util/serializeActiveFilterValues.js create mode 100644 superset/assets/src/dashboard/util/serializeFilterScopes.js 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/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..bfc56586eaf9 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,20 +73,22 @@ 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) - ) { + if (currentChartId !== chartId) { Object.keys(columns).forEach(name => { + const chartIdsInFilterScope = getChartIdsInFilterScope({ + filterScope: scopes[name], + }); + + if (!chartIdsInFilterScope.includes(currentChartId)) { + return; + } const colorMapKey = getDashboardFilterKey({ chartId, column: name, @@ -101,7 +100,6 @@ export default class FilterIndicatorsContainer extends React.PureComponent { colorCode: dashboardFiltersColorMap[colorMapKey], componentId, directPathToFilter: directPathToLabel, - scope, isDateFilter, isInstantFilter, name, @@ -116,14 +114,6 @@ export default class FilterIndicatorsContainer extends React.PureComponent { 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 { diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index 569dac97ec92..c740c8ebe6b2 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -53,7 +53,8 @@ const propTypes = { dashboardTitle: PropTypes.string.isRequired, charts: PropTypes.objectOf(chartPropShape).isRequired, layout: PropTypes.object.isRequired, - filters: PropTypes.object.isRequired, + serializedFilters: PropTypes.object.isRequired, + serializedFilterScopes: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, css: PropTypes.string.isRequired, colorNamespace: PropTypes.string, @@ -217,7 +218,8 @@ class Header extends React.PureComponent { css, colorNamespace, colorScheme, - filters, + serializedFilters, + serializedFilterScopes, dashboardInfo, refreshFrequency, } = this.props; @@ -235,7 +237,8 @@ class Header extends React.PureComponent { color_scheme: colorScheme, label_colors: labelColors, dashboard_title: dashboardTitle, - default_filters: safeStringify(filters), + default_filters: safeStringify(serializedFilters), + filter_scopes: safeStringify(serializedFilterScopes), refresh_frequency: refreshFrequency, }; @@ -263,7 +266,8 @@ class Header extends React.PureComponent { const { dashboardTitle, layout, - filters, + serializedFilters, + serializedFilterScopes, expandedSlices, css, colorNamespace, @@ -415,7 +419,8 @@ class Header extends React.PureComponent { dashboardId={dashboardInfo.id} dashboardTitle={dashboardTitle} layout={layout} - filters={filters} + serializedFilters={serializedFilters} + serializedFilterScopes={serializedFilterScopes} 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..40a82eb15cc3 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,8 @@ const propTypes = { userCanSave: PropTypes.bool.isRequired, isLoading: PropTypes.bool.isRequired, layout: PropTypes.object.isRequired, - filters: PropTypes.object.isRequired, + serializedFilters: PropTypes.object.isRequired, + serializedFilterScopes: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, onSave: PropTypes.func.isRequired, }; @@ -120,7 +122,8 @@ class HeaderActionsDropdown extends React.PureComponent { colorScheme, hasUnsavedChanges, layout, - filters, + serializedFilters, + serializedFilterScopes, expandedSlices, onSave, userCanEdit, @@ -148,7 +151,8 @@ class HeaderActionsDropdown extends React.PureComponent { dashboardTitle={dashboardTitle} saveType={SAVE_TYPE_NEWDASHBOARD} layout={layout} - filters={filters} + serializedFilters={serializedFilters} + serializedFilterScopes={serializedFilterScopes} expandedSlices={expandedSlices} refreshFrequency={refreshFrequency} css={css} @@ -200,7 +204,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 { 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,27 @@ 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); + // save does not close modal } filterTree() { @@ -503,7 +507,7 @@ export default class FilterScopeSelector extends React.PureComponent { )}
- + {showSelector && (