Skip to content

Commit

Permalink
refactor: updates usage of ownFilters to ownState
Browse files Browse the repository at this point in the history
  • Loading branch information
simcha90 committed Apr 6, 2021
1 parent 4d4b371 commit 58b9199
Show file tree
Hide file tree
Showing 20 changed files with 61 additions and 53 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/spec/fixtures/mockNativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const nativeFilters: NativeFiltersState = {

export const dataMaskWith2Filters: DataMaskStateWithId = {
crossFilters: {},
ownFilters: {},
ownState: {},
nativeFilters: {
'NATIVE_FILTER-e7Q8zKixx': {
id: 'NATIVE_FILTER-e7Q8zKixx',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const nativeFiltersInfo: NativeFiltersState = {

export const mockDataMaskInfo: DataMaskStateWithId = {
[DataMaskType.CrossFilters]: {},
[DataMaskType.OwnFilters]: {},
[DataMaskType.OwnState]: {},
[DataMaskType.NativeFilters]: {
DefaultsID: {
id: 'DefaultId',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('getFormDataWithExtraFilters', () => {
},
dataMask: {
crossFilters: {},
ownFilters: {},
ownState: {},
nativeFilters: {
[filterId]: {
id: filterId,
Expand Down
8 changes: 4 additions & 4 deletions superset-frontend/src/chart/ChartRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const propTypes = {
setDataMask: PropTypes.func,
onFilterMenuOpen: PropTypes.func,
onFilterMenuClose: PropTypes.func,
ownCurrentState: PropTypes.object,
ownState: PropTypes.object,
};

const BLANK = {};
Expand Down Expand Up @@ -94,7 +94,7 @@ class ChartRenderer extends React.Component {
return (
this.hasQueryResponseChange ||
nextProps.annotationData !== this.props.annotationData ||
nextProps.ownCurrentState !== this.props.ownCurrentState ||
nextProps.ownState !== this.props.ownState ||
nextProps.height !== this.props.height ||
nextProps.width !== this.props.width ||
nextProps.triggerRender ||
Expand Down Expand Up @@ -184,7 +184,7 @@ class ChartRenderer extends React.Component {
annotationData,
datasource,
initialValues,
ownCurrentState,
ownState,
formData,
queriesResponse,
} = this.props;
Expand Down Expand Up @@ -224,7 +224,7 @@ class ChartRenderer extends React.Component {
datasource={datasource}
initialValues={initialValues}
formData={formData}
ownCurrentState={ownCurrentState}
ownState={ownState}
hooks={this.hooks}
behaviors={[Behavior.CROSS_FILTER]}
queriesData={queriesResponse}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const propTypes = {
sliceCanEdit: PropTypes.bool.isRequired,
addSuccessToast: PropTypes.func.isRequired,
addDangerToast: PropTypes.func.isRequired,
ownCurrentState: PropTypes.object,
ownState: PropTypes.object,
};

const defaultProps = {
Expand Down Expand Up @@ -260,7 +260,7 @@ export default class Chart extends React.Component {
sliceCanEdit,
addSuccessToast,
addDangerToast,
ownCurrentState,
ownState,
handleToggleFullSize,
isFullSize,
} = this.props;
Expand Down Expand Up @@ -368,7 +368,7 @@ export default class Chart extends React.Component {
dashboardId={dashboardId}
initialValues={initialValues}
formData={formData}
ownCurrentState={ownCurrentState}
ownState={ownState}
queriesResponse={chart.queriesResponse}
timeout={timeout}
triggerQuery={chart.triggerQuery}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import Popover from 'src/components/Popover';
import Icon from 'src/components/Icon';
import { Pill } from 'src/dashboard/components/FiltersBadge/Styles';
import { useSelector } from 'react-redux';
import { getInitialMask } from 'src/dataMask/reducer';
import { getInitialFiltersMask } from 'src/dataMask/reducer';
import { MaskWithId } from 'src/dataMask/types';
import FilterControl from '../FilterControls/FilterControl';
import CascadeFilterControl from './CascadeFilterControl';
Expand Down Expand Up @@ -84,7 +84,8 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
const [currentPathToChild, setCurrentPathToChild] = useState<string[]>();
const dataMask = useSelector<any, MaskWithId>(
state =>
state.dataMask.nativeFilters[filter.id] ?? getInitialMask(filter.id),
state.dataMask.nativeFilters[filter.id] ??
getInitialFiltersMask(filter.id),
);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import React, { FC } from 'react';
import Icon from 'src/components/Icon';
import Button from 'src/components/Button';
import { useSelector } from 'react-redux';
import { getInitialMask } from 'src/dataMask/reducer';
import { getInitialFiltersMask } from 'src/dataMask/reducer';
import { DataMaskUnit, DataMaskUnitWithId } from 'src/dataMask/types';
import FilterConfigurationLink from './FilterConfigurationLink';
import { useFilters } from './state';
Expand Down Expand Up @@ -81,7 +81,7 @@ const Header: FC<HeaderProps> = ({
const handleClearAll = () => {
filterValues.forEach(filter => {
setDataMaskSelected(draft => {
draft[filter.id] = getInitialMask(filter.id);
draft[filter.id] = getInitialFiltersMask(filter.id);
});
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const getFormData = ({

export function mergeExtraFormData(
originalExtra: ExtraFormData = {},
newExtra: ExtraFormData,
newExtra: ExtraFormData = {},
): ExtraFormData {
const {
override_form_data: originalOverride = {},
Expand All @@ -82,6 +82,7 @@ export function mergeExtraFormData(
override_form_data: newOverride = {},
append_form_data: newAppend = {},
custom_form_data: newCustom = {},
own_state: newOwnState = {},
} = newExtra;

const appendKeys = new Set([
Expand All @@ -100,6 +101,7 @@ export function mergeExtraFormData(

return {
custom_form_data: newCustom,
own_state: newOwnState,
override_form_data: {
...originalOverride,
...newOverride,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/containers/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function mapStateToProps(
supersetCanExplore: !!dashboardInfo.superset_can_explore,
supersetCanCSV: !!dashboardInfo.superset_can_csv,
sliceCanEdit: !!dashboardInfo.slice_can_edit,
ownCurrentState: dataMask.ownFilters?.[id]?.currentState,
ownState: dataMask.ownState?.[id],
crossFilterCurrentState: dataMask.crossFilters?.[id]?.currentState,
};
}
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/containers/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function mapStateToProps(state) {
layout: dashboardLayout.present,
}),
},
ownDataCharts: dataMask.ownFilters ?? {},
ownDataCharts: dataMask.ownState ?? {},
slices: sliceEntities.slices,
layout: dashboardLayout.present,
impressionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,9 @@ export default function getFormDataWithExtraFilters({
};
}

const { extraFormData: newExtra = {} } =
dataMask?.ownFilters?.[chart.id] ?? {};
extraData.extra_form_data = mergeExtraFormData(
extraData?.extra_form_data,
newExtra,
);
extraData.extra_form_data = mergeExtraFormData(extraData?.extra_form_data, {
own_state: dataMask?.ownState?.[chart.id],
});

const formData = {
...chart.formData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,12 @@ export const getAffectedOwnDataCharts = (
const chartIds = Object.keys(ownDataCharts);
const appliedChartIds = Object.keys(appliedOwnDataCharts);
const affectedIds: string[] = arrayDiff(chartIds, appliedChartIds).filter(
id =>
ownDataCharts[id]?.extraFormData ||
appliedOwnDataCharts[id]?.extraFormData,
id => ownDataCharts[id] || appliedOwnDataCharts[id],
);
const checkForUpdateIds = new Set<string>([...chartIds, ...appliedChartIds]);
checkForUpdateIds.forEach(chartId => {
if (
!areObjectsEqual(
ownDataCharts[chartId]?.extraFormData,
appliedOwnDataCharts[chartId]?.extraFormData,
)
!areObjectsEqual(ownDataCharts[chartId], appliedOwnDataCharts[chartId])
) {
affectedIds.push(chartId);
}
Expand Down
11 changes: 6 additions & 5 deletions superset-frontend/src/dataMask/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@
import { DataMaskType, MaskWithId } from './types';
import { FilterConfiguration } from '../dashboard/components/nativeFilters/types';
import { FeatureFlag, isFeatureEnabled } from '../featureFlags';
import { JsonObject } from '../../cypress-base/cypress/utils';

export const UPDATE_DATA_MASK = 'UPDATE_DATA_MASK';
export interface UpdateDataMask {
type: typeof UPDATE_DATA_MASK;
filterId: string;
[DataMaskType.NativeFilters]?: Omit<MaskWithId, 'id'>;
[DataMaskType.CrossFilters]?: Omit<MaskWithId, 'id'>;
[DataMaskType.OwnFilters]?: Omit<MaskWithId, 'id'>;
[DataMaskType.OwnState]?: JsonObject;
}

export const SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE =
Expand All @@ -48,15 +49,15 @@ export function updateDataMask(
dataMask: {
nativeFilters?: Omit<MaskWithId, 'id'>;
crossFilters?: Omit<MaskWithId, 'id'>;
ownFilters?: Omit<MaskWithId, 'id'>;
ownState?: JsonObject;
},
): UpdateDataMask {
const { nativeFilters, crossFilters, ownFilters } = dataMask;
const { nativeFilters, crossFilters, ownState } = dataMask;
const filteredDataMask: {
nativeFilters?: Omit<MaskWithId, 'id'>;
crossFilters?: Omit<MaskWithId, 'id'>;
ownFilters?: Omit<MaskWithId, 'id'>;
} = { ownFilters };
ownState?: JsonObject;
} = { ownState };
if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && nativeFilters) {
filteredDataMask.nativeFilters = nativeFilters;
}
Expand Down
15 changes: 10 additions & 5 deletions superset-frontend/src/dataMask/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
/* eslint-disable no-param-reassign */
// <- When we work with Immer, we need reassign, so disabling lint
import produce from 'immer';
import { MaskWithId, DataMaskType, DataMaskStateWithId, Mask } from './types';
import { DataMaskStateWithId, DataMaskType, Mask, MaskWithId } from './types';
import {
AnyDataMaskAction,
SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE,
UPDATE_DATA_MASK,
UpdateDataMask,
} from './actions';

export function getInitialMask(id: string): MaskWithId {
export function getInitialFiltersMask(id: string): MaskWithId {
return {
id,
extraFormData: {},
Expand All @@ -42,9 +42,13 @@ const setUnitDataMask = (
dataMaskState: DataMaskStateWithId,
) => {
if (action[unitName]) {
const otherProps: Partial<MaskWithId> = {};
if (unitName !== DataMaskType.OwnState) {
otherProps.id = action.filterId;
}
dataMaskState[unitName][action.filterId] = {
...(action[unitName] as Mask),
id: action.filterId,
...otherProps,
};
}
};
Expand All @@ -63,7 +67,8 @@ const dataMaskReducer = produce(
draft[action.unitName] = {};
(action.filterConfig ?? []).forEach(filter => {
draft[action.unitName][filter.id] =
oldData[action.unitName][filter.id] ?? getInitialMask(filter.id);
oldData[action.unitName][filter.id] ??
getInitialFiltersMask(filter.id);
});
break;

Expand All @@ -73,7 +78,7 @@ const dataMaskReducer = produce(
{
[DataMaskType.NativeFilters]: {},
[DataMaskType.CrossFilters]: {},
[DataMaskType.OwnFilters]: {},
[DataMaskType.OwnState]: {},
},
);

Expand Down
7 changes: 4 additions & 3 deletions superset-frontend/src/dataMask/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/
import { ExtraFormData, DataMaskCurrentState } from '@superset-ui/core';
import { JsonObject } from '../../cypress-base/cypress/utils';

export enum DataMaskType {
NativeFilters = 'nativeFilters',
CrossFilters = 'crossFilters',
OwnFilters = 'ownFilters',
OwnState = 'ownState',
}

export type Mask = {
Expand All @@ -32,13 +33,13 @@ export type DataMaskUnit = { [filterId: string]: Mask };
export type DataMaskState = {
[DataMaskType.NativeFilters]: Mask;
[DataMaskType.CrossFilters]: Mask;
[DataMaskType.OwnFilters]: Mask;
[DataMaskType.OwnState]: JsonObject;
};

export type MaskWithId = Mask & { id: string };
export type DataMaskUnitWithId = { [filterId: string]: MaskWithId };
export type DataMaskStateWithId = {
[DataMaskType.NativeFilters]: DataMaskUnitWithId;
[DataMaskType.CrossFilters]: DataMaskUnitWithId;
[DataMaskType.OwnFilters]: DataMaskUnitWithId;
[DataMaskType.OwnState]: JsonObject;
};
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const propTypes = {
table_name: PropTypes.string,
vizType: PropTypes.string.isRequired,
form_data: PropTypes.object,
ownCurrentState: PropTypes.object,
ownState: PropTypes.object,
standalone: PropTypes.number,
timeout: PropTypes.number,
refreshOverlayVisible: PropTypes.bool,
Expand Down Expand Up @@ -190,7 +190,7 @@ const ExploreChartPanel = props => {
<ChartContainer
width={Math.floor(chartWidth)}
height={newHeight}
ownCurrentState={props.ownCurrentState}
ownState={props.ownState}
annotationData={chart.annotationData}
chartAlert={chart.chartAlert}
chartStackTrace={chart.chartStackTrace}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ function ExploreViewContainer(props) {
reRenderChart();
}
}
}, [props.controls, props.ownCurrentState]);
}, [props.controls, props.ownState]);

const chartIsStale = useMemo(() => {
if (previousControls) {
Expand All @@ -356,11 +356,11 @@ function ExploreViewContainer(props) {
}, [previousControls, props.controls]);

useEffect(() => {
if (props.ownCurrentState !== undefined) {
if (props.ownState !== undefined) {
onQuery();
reRenderChart();
}
}, [props.ownCurrentState]);
}, [props.ownState]);

if (chartIsStale) {
props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS);
Expand Down Expand Up @@ -557,7 +557,9 @@ function mapStateToProps(state) {
form_data.extra_form_data = mergeExtraFormData(
{ ...form_data.extra_form_data },
{
...dataMask?.ownFilters?.[form_data.slice_id]?.extraFormData,
own_state: {
...dataMask?.ownState?.[form_data.slice_id],
},
},
);
const chartKey = Object.keys(charts)[0];
Expand Down Expand Up @@ -589,7 +591,7 @@ function mapStateToProps(state) {
forcedHeight: explore.forced_height,
chart,
timeout: explore.common.conf.SUPERSET_WEBSERVER_TIMEOUT,
ownCurrentState: dataMask?.ownFilters?.[form_data.slice_id]?.currentState,
ownState: dataMask?.ownState?.[form_data.slice_id],
impressionId,
};
}
Expand Down
Loading

0 comments on commit 58b9199

Please sign in to comment.