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

feat(native-filters): apply scoping of native filters to dashboard #12716

Merged
merged 9 commits into from
Jan 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions superset-frontend/spec/fixtures/mockNativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,117 @@ export const nativeFilters: NativeFiltersState = {
},
},
};

export const extraFormData = {
append_form_data: {
filters: [
{
col: 'ethnic_minority',
op: 'IN',
val: 'No, not an ethnic minority',
},
],
},
};

export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA';

export const singleNativeFiltersState = {
filters: {
[NATIVE_FILTER_ID]: {
id: [NATIVE_FILTER_ID],
name: 'eth',
type: 'text',
targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
defaultValue: null,
cascadeParentIds: [],
scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] },
inverseSelection: false,
isInstant: true,
allowsMultipleValues: false,
isRequired: false,
},
},
filtersState: {
[NATIVE_FILTER_ID]: {
id: NATIVE_FILTER_ID,
extraFormData,
},
},
};

export const layoutForSingleNativeFilter = {
'CHART-ZHVS7YasaQ': {
children: [],
id: 'CHART-ZHVS7YasaQ',
meta: {
chartId: 230,
height: 50,
sliceName: 'Pie Chart',
uuid: '05ef6145-3950-4f59-891f-160852613eca',
width: 12,
},
parents: ['ROOT_ID', 'GRID_ID', 'ROW-NweUz7oC0'],
type: 'CHART',
},
'CHART-gsGu8NIKQT': {
children: [],
id: 'CHART-gsGu8NIKQT',
meta: {
chartId: 227,
height: 50,
sliceName: 'Another Chart',
uuid: 'ddb78f6c-7876-47fc-ae98-70183b05ba90',
width: 4,
},
parents: ['ROOT_ID', 'GRID_ID', 'ROW-QkiTjeZGs'],
type: 'CHART',
},
'CHART-hgYjD8axJX': {
children: [],
id: 'CHART-hgYjD8axJX',
meta: {
chartId: 229,
height: 47,
sliceName: 'Bar Chart',
uuid: 'e1501e54-d632-4fdc-ae16-07cafee31093',
width: 12,
},
parents: ['ROOT_ID', 'GRID_ID', 'ROW-mcdVZi0rL'],
type: 'CHART',
},
DASHBOARD_VERSION_KEY: 'v2',
GRID_ID: {
children: ['ROW-mcdVZi0rL', 'ROW-NweUz7oC0', 'ROW-QkiTjeZGs'],
id: 'GRID_ID',
parents: ['ROOT_ID'],
type: 'GRID',
},
HEADER_ID: {
id: 'HEADER_ID',
type: 'HEADER',
meta: { text: 'My Native Filter Dashboard' },
},
ROOT_ID: { children: ['GRID_ID'], id: 'ROOT_ID', type: 'ROOT' },
'ROW-NweUz7oC0': {
children: ['CHART-ZHVS7YasaQ'],
id: 'ROW-NweUz7oC0',
meta: { background: 'BACKGROUND_TRANSPARENT' },
parents: ['ROOT_ID', 'GRID_ID'],
type: 'ROW',
},
'ROW-QkiTjeZGs': {
children: ['CHART-gsGu8NIKQT'],
id: 'ROW-QkiTjeZGs',
meta: { background: 'BACKGROUND_TRANSPARENT' },
parents: ['ROOT_ID', 'GRID_ID'],
type: 'ROW',
},
'ROW-mcdVZi0rL': {
children: ['CHART-hgYjD8axJX'],
id: 'ROW-mcdVZi0rL',
meta: { '0': 'ROOT_ID', background: 'BACKGROUND_TRANSPARENT' },
parents: ['ROOT_ID', 'GRID_ID'],
type: 'ROW',
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,17 @@ import newComponentFactory from 'src/dashboard/util/newComponentFactory';
// mock data
import chartQueries from 'spec/fixtures/mockChartQueries';
import datasources from 'spec/fixtures/mockDatasource';
import {
extraFormData,
NATIVE_FILTER_ID,
layoutForSingleNativeFilter,
singleNativeFiltersState,
} from 'spec/fixtures/mockNativeFilters';
import dashboardInfo from 'spec/fixtures/mockDashboardInfo';
import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout';
import dashboardState from 'spec/fixtures/mockDashboardState';
import { sliceEntitiesForChart as sliceEntities } from 'spec/fixtures/mockSliceEntities';
import { getActiveNativeFilters } from 'src/dashboard/util/activeDashboardNativeFilters';

describe('Dashboard', () => {
const props = {
Expand Down Expand Up @@ -141,6 +148,27 @@ describe('Dashboard', () => {
expect(wrapper.instance().appliedFilters).toBe(OVERRIDE_FILTERS);
});

it('should call refresh when native filters changed', () => {
wrapper.setProps({
activeFilters: {
...OVERRIDE_FILTERS,
...getActiveNativeFilters({
nativeFilters: singleNativeFiltersState,
layout: layoutForSingleNativeFilter,
}),
},
});
wrapper.instance().componentDidUpdate(prevProps);
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual({
...OVERRIDE_FILTERS,
[NATIVE_FILTER_ID]: {
scope: [230],
values: [extraFormData],
},
});
});

it('should call refresh if a filter is added', () => {
const newFilter = {
gender: { values: ['boy', 'girl'], scope: [1] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
import getFormDataWithExtraFilters, {
GetFormDataWithExtraFiltersArguments,
} from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import { LayoutItem } from 'src/dashboard/types';
import { dashboardLayout } from '../../../fixtures/mockDashboardLayout';
import { sliceId as chartId } from '../../../fixtures/mockChartQueries';

describe('getFormDataWithExtraFilters', () => {
const chartId = 8675309;
const mockArgs = {
const filterId = 'native-filter-1';
const mockArgs: GetFormDataWithExtraFiltersArguments = {
chart: {
id: chartId,
formData: {
Expand All @@ -37,11 +44,27 @@ describe('getFormDataWithExtraFilters', () => {
region: ['Spain'],
color: ['pink', 'purple'],
},
sliceId: chartId,
nativeFilters: {
filters: {},
filtersState: {},
filters: {
[filterId]: ({
id: filterId,
scope: {
rootPath: [DASHBOARD_ROOT_ID],
excluded: [],
},
} as unknown) as Filter,
},
filtersState: {
[filterId]: {
id: filterId,
extraFormData: {},
},
},
},
layout: (dashboardLayout.present as unknown) as {
[key: string]: LayoutItem;
},
sliceId: chartId,
};

it('should include filters from the passed filters', () => {
Expand Down
8 changes: 2 additions & 6 deletions superset-frontend/src/dashboard/components/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,11 @@ class Dashboard extends React.PureComponent {
}
}

componentDidUpdate(prevProps) {
componentDidUpdate() {
const { hasUnsavedChanges, editMode } = this.props.dashboardState;

const { appliedFilters } = this;
const { activeFilters, nativeFilters } = this.props;
// do not apply filter when dashboard in edit mode
if (!areObjectsEqual(prevProps.nativeFilters, nativeFilters)) {
this.refreshCharts(this.getAllCharts().map(chart => chart.id));
}
const { activeFilters } = this.props;
if (!editMode && !areObjectsEqual(appliedFilters, activeFilters)) {
this.applyFilters();
}
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/dashboard/containers/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ function mapStateToProps(
charts: chartQueries,
dashboardInfo,
dashboardState,
dashboardLayout,
datasources,
sliceEntities,
nativeFilters,
Expand All @@ -57,6 +58,7 @@ function mapStateToProps(

// note: this method caches filters if possible to prevent render cascades
const formData = getFormDataWithExtraFilters({
layout: dashboardLayout.present,
chart,
filters: getAppliedFilterValues(id),
colorScheme,
Expand Down
10 changes: 8 additions & 2 deletions superset-frontend/src/dashboard/containers/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
import { triggerQuery } from '../../chart/chartAction';
import { logEvent } from '../../logger/actions';
import { getActiveFilters } from '../util/activeDashboardFilters';
import { getActiveNativeFilters } from '../util/activeDashboardNativeFilters';

function mapStateToProps(state) {
const {
Expand All @@ -54,10 +55,15 @@ function mapStateToProps(state) {
// When dashboard is first loaded into browser,
// its value is from preselect_filters that dashboard owner saved in dashboard's meta data
// When user start interacting with dashboard, it will be user picked values from all filter_box
activeFilters: getActiveFilters(),
activeFilters: {
...getActiveFilters(),
...getActiveNativeFilters({
nativeFilters,
layout: dashboardLayout.present,
}),
},
slices: sliceEntities.slices,
layout: dashboardLayout.present,
nativeFilters,
impressionId,
};
}
Expand Down
5 changes: 3 additions & 2 deletions superset-frontend/src/dashboard/reducers/nativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ export function getInitialFilterState(id: string): FilterState {

export function getInitialState(
filterConfig: FilterConfiguration,
prevFiltersState: { [filterId: string]: FilterState },
): NativeFiltersState {
const filters = {};
const filtersState = {};
const state = { filters, filtersState };
filterConfig.forEach(filter => {
const { id } = filter;
filters[id] = filter;
filtersState[id] = getInitialFilterState(id);
filtersState[id] = prevFiltersState?.[id] || getInitialFilterState(id);
});
return state;
}
Expand All @@ -67,7 +68,7 @@ export default function nativeFilterReducer(
};

case SET_FILTER_CONFIG_COMPLETE:
return getInitialState(action.filterConfig);
return getInitialState(action.filterConfig, filtersState);

// TODO handle SET_FILTER_CONFIG_FAIL action
default:
Expand Down
10 changes: 10 additions & 0 deletions superset-frontend/src/dashboard/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type ChartReducerInitialState = typeof chart;
// chart query built from initialState
// Ref: https://github.com/apache/superset/blob/dcac860f3e5528ecbc39e58f045c7388adb5c3d0/superset-frontend/src/dashboard/reducers/getInitialState.js#L120
export interface ChartQueryPayload extends Partial<ChartReducerInitialState> {
id: number;
formData: ChartProps['formData'];
form_data?: ChartProps['rawFormData'];
[key: string]: unknown;
Expand Down Expand Up @@ -69,3 +70,12 @@ export type LayoutItem = {
width: number;
};
};

type ActiveFilter = {
scope: number[];
values: any[];
};

export type ActiveFilters = {
[key: string]: ActiveFilter;
};
Loading