Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(native-filters): Update filter saving #14370

Merged
merged 9 commits into from
Apr 27, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ export const createHandleTabEdit = (
}
};

export const generateFilterId = () => `NATIVE_FILTER-${shortid.generate()}`;
export const NATIVE_FILTER_PREFIX = 'NATIVE_FILTER-';
export const generateFilterId = () =>
`${NATIVE_FILTER_PREFIX}${shortid.generate()}`;

export const getFilterIds = (config: FilterConfiguration) =>
config.map(filter => filter.id);
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { bindActionCreators } from 'redux';
import { bindActionCreators, Dispatch } from 'redux';
import { connect } from 'react-redux';

import Dashboard from '../components/Dashboard';
Expand All @@ -28,9 +28,13 @@ import {
import { triggerQuery } from '../../chart/chartAction';
import { logEvent } from '../../logger/actions';
import { getActiveFilters } from '../util/activeDashboardFilters';
import { getAllActiveFilters } from '../util/activeAllDashboardFilters';
import {
getAllActiveFilters,
getRelevantDataMask,
} from '../util/activeAllDashboardFilters';
import { RootState } from '../types';

function mapStateToProps(state) {
function mapStateToProps(state: RootState) {
const {
datasources,
sliceEntities,
Expand Down Expand Up @@ -62,21 +66,18 @@ function mapStateToProps(state) {
// eslint-disable-next-line camelcase
chartConfiguration: dashboardInfo.metadata?.chart_configuration,
nativeFilters: nativeFilters.filters,
dataMask,
dataMask: getRelevantDataMask(dataMask, 'isApplied'),
layout: dashboardLayout.present,
}),
},
ownDataCharts: Object.values(dataMask).reduce(
(prev, next) => ({ ...prev, [next.id]: next.ownState }),
{},
),
ownDataCharts: getRelevantDataMask(dataMask, 'ownState', 'ownState'),
slices: sliceEntities.slices,
layout: dashboardLayout.present,
impressionId,
};
}

function mapDispatchToProps(dispatch) {
function mapDispatchToProps(dispatch: Dispatch) {
return {
actions: bindActionCreators(
{
Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/dashboard/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ChartProps } from '@superset-ui/core';
import { ChartProps, JsonObject } from '@superset-ui/core';
import { chart } from 'src/chart/chartReducer';
import componentTypes from 'src/dashboard/util/componentTypes';
import { DataMaskStateWithId } from '../dataMask/types';
import { NativeFiltersState } from './reducers/types';

export type ChartReducerInitialState = typeof chart;

Expand All @@ -46,11 +47,16 @@ export type DashboardState = { editMode: boolean; directPathToChild: string[] };

/** Root state of redux */
export type RootState = {
datasources: JsonObject;
sliceEntities: JsonObject;
charts: { [key: string]: Chart };
dashboardLayout: DashboardLayoutState;
dashboardFilters: {};
dashboardState: DashboardState;
dataMask: DataMaskStateWithId;
dashboardInfo: JsonObject;
impressionId: string;
nativeFilters: NativeFiltersState;
};

/** State of dashboardLayout in redux */
Expand Down
17 changes: 15 additions & 2 deletions superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { DataMaskStateWithId } from 'src/dataMask/types';
import { JsonObject } from '@superset-ui/core';
import { CHART_TYPE } from './componentTypes';
import { Scope } from '../components/nativeFilters/types';
import { ActiveFilters, LayoutItem } from '../types';
import { ChartConfiguration, Filters } from '../reducers/types';
import { DASHBOARD_ROOT_ID } from './constants';
import { DataMaskStateWithId } from '../../dataMask/types';

// Looking for affected chart scopes and values
export const findAffectedCharts = ({
Expand Down Expand Up @@ -71,6 +72,18 @@ export const findAffectedCharts = ({
);
};

export const getRelevantDataMask = (
dataMask: DataMaskStateWithId,
filterBy: string,
prop?: string,
): JsonObject | DataMaskStateWithId =>
Object.values(dataMask)
.filter(item => item[filterBy])
.reduce(
(prev, next) => ({ ...prev, [next.id]: prop ? next[prop] : next }),
{},
);

export const getAllActiveFilters = ({
chartConfiguration,
nativeFilters,
Expand All @@ -92,7 +105,7 @@ export const getAllActiveFilters = ({
excluded: [filterId],
};
// Iterate over all roots to find all affected charts
scope.rootPath.forEach(layoutItemId => {
scope.rootPath.forEach((layoutItemId: string | number) => {
layout[layoutItemId].children.forEach((child: string) => {
// Need exclude from affected charts, charts that located in scope `excluded`
findAffectedCharts({
Expand Down
14 changes: 12 additions & 2 deletions superset-frontend/src/dataMask/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ import {
SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE,
UPDATE_DATA_MASK,
} from './actions';
import { NATIVE_FILTER_PREFIX } from '../dashboard/components/nativeFilters/FiltersConfigModal/utils';
import { Filter } from '../dashboard/components/nativeFilters/types';

export function getInitialDataMask(id: string): DataMaskWithId {
return {
id,
extraFormData: {},
filterState: {},
ownState: {},
isApplied: false,
};
}

Expand All @@ -42,17 +45,24 @@ const dataMaskReducer = produce(
switch (action.type) {
case UPDATE_DATA_MASK:
draft[action.filterId] = {
...getInitialDataMask(action.filterId),
...draft[action.filterId],
...action.dataMask,
id: action.filterId,
isApplied: true,
};
return draft;

case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE:
(action.filterConfig ?? []).forEach(filter => {
(action.filterConfig ?? []).forEach((filter: Filter) => {
cleanState[filter.id] =
draft[filter.id] ?? getInitialDataMask(filter.id);
});
// Get back all other non-native filters
Object.values(draft).forEach(filter => {
if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) {
cleanState[filter?.id] = filter;
}
});
return cleanState;

default:
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dataMask/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ export enum DataMaskType {

export type DataMaskState = { [id: string]: DataMask };

export type DataMaskWithId = DataMask & { id: string };
export type DataMaskWithId = { id: string; isApplied?: boolean } & DataMask;
export type DataMaskStateWithId = { [filterId: string]: DataMaskWithId };