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 cascading without instant filtering #14966

Merged
merged 1 commit into from
Jun 3, 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
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import Icon from 'src/components/Icon';
import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl';
import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import { DataMaskStateWithId } from 'src/dataMask/types';

export interface CascadeFilterControlProps {
dataMaskSelected?: DataMaskStateWithId;
filter: CascadeFilter;
directPathToChild?: string[];
onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void;
Expand All @@ -45,6 +47,7 @@ const StyledCaretIcon = styled(Icon)`
`;

const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
dataMaskSelected,
filter,
directPathToChild,
onFilterSelectionChange,
Expand All @@ -53,6 +56,7 @@ const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
<StyledFilterControlBox>
<StyledCaretIcon name="caret-down" />
<FilterControl
dataMaskSelected={dataMaskSelected}
filter={filter}
directPathToChild={directPathToChild}
onFilterSelectionChange={onFilterSelectionChange}
Expand All @@ -63,6 +67,7 @@ const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
{filter.cascadeChildren?.map(childFilter => (
<li key={childFilter.id}>
<CascadeFilterControl
dataMaskSelected={dataMaskSelected}
filter={childFilter}
directPathToChild={directPathToChild}
onFilterSelectionChange={onFilterSelectionChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@ import { styled, t, DataMask } from '@superset-ui/core';
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 { getInitialDataMask } from 'src/dataMask/reducer';
import { DataMaskWithId } from 'src/dataMask/types';
import { DataMaskStateWithId } from 'src/dataMask/types';
import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl';
import CascadeFilterControl from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl';
import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import { RootState } from 'src/dashboard/types';

interface CascadePopoverProps {
dataMaskSelected: DataMaskStateWithId;
filter: CascadeFilter;
visible: boolean;
directPathToChild?: string[];
Expand Down Expand Up @@ -76,16 +74,15 @@ const StyledPill = styled(Pill)`
`;

const CascadePopover: React.FC<CascadePopoverProps> = ({
dataMaskSelected,
filter,
visible,
onVisibleChange,
onFilterSelectionChange,
directPathToChild,
}) => {
const [currentPathToChild, setCurrentPathToChild] = useState<string[]>();
const dataMask = useSelector<RootState, DataMaskWithId>(
state => state.dataMask[filter.id] ?? getInitialDataMask(filter.id),
);
const dataMask = dataMaskSelected[filter.id];

useEffect(() => {
setCurrentPathToChild(directPathToChild);
Expand All @@ -98,7 +95,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
const getActiveChildren = useCallback(
(filter: CascadeFilter): CascadeFilter[] | null => {
const children = filter.cascadeChildren || [];
const currentValue = dataMask.filterState?.value;
const currentValue = dataMask?.filterState?.value;

const activeChildren = children.flatMap(
childFilter => getActiveChildren(childFilter) || [],
Expand Down Expand Up @@ -147,6 +144,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
if (!filter.cascadeChildren?.length) {
return (
<FilterControl
dataMaskSelected={dataMaskSelected}
filter={filter}
directPathToChild={directPathToChild}
onFilterSelectionChange={onFilterSelectionChange}
Expand All @@ -166,6 +164,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({

const content = (
<CascadeFilterControl
dataMaskSelected={dataMaskSelected}
data-test="cascade-filters-control"
key={filter.id}
filter={filter}
Expand All @@ -188,6 +187,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
<div>
{activeFilters.map(activeFilter => (
<FilterControl
dataMaskSelected={dataMaskSelected}
key={activeFilter.id}
filter={activeFilter}
onFilterSelectionChange={onFilterSelectionChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const StyledFilterControlContainer = styled.div`
`;

const FilterControl: React.FC<FilterProps> = ({
dataMaskSelected,
filter,
icon,
onFilterSelectionChange,
Expand All @@ -57,6 +58,7 @@ const FilterControl: React.FC<FilterProps> = ({
<div data-test="filter-icon">{icon}</div>
</StyledFilterControlTitleBox>
<FilterValue
dataMaskSelected={dataMaskSelected}
filter={filter}
directPathToChild={directPathToChild}
onFilterSelectionChange={onFilterSelectionChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { DataMask, styled, t } from '@superset-ui/core';
import { css } from '@emotion/react';
import { useSelector } from 'react-redux';
import * as portals from 'react-reverse-portal';
import { DataMaskState } from 'src/dataMask/types';
import { DataMaskStateWithId } from 'src/dataMask/types';
import { Collapse } from 'src/common/components';
import { TAB_TYPE } from 'src/dashboard/util/componentTypes';
import { RootState } from 'src/dashboard/types';
Expand All @@ -41,7 +41,7 @@ const Wrapper = styled.div`

type FilterControlsProps = {
directPathToChild?: string[];
dataMaskSelected: DataMaskState;
dataMaskSelected: DataMaskStateWithId;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bycatch: incorrect datatype

onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void;
};

Expand Down Expand Up @@ -101,6 +101,7 @@ const FilterControls: FC<FilterControlsProps> = ({
<CascadePopover
data-test="cascade-filters-control"
key={cascadeFilters[index].id}
dataMaskSelected={dataMaskSelected}
visible={visiblePopoverId === cascadeFilters[index].id}
onVisibleChange={visible =>
setVisiblePopoverId(visible ? cascadeFilters[index].id : null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ const FilterItem = styled.div`
`;

const FilterValue: React.FC<FilterProps> = ({
dataMaskSelected,
filter,
directPathToChild,
onFilterSelectionChange,
}) => {
const { id, targets, filterType, adhoc_filters, time_range } = filter;
const metadata = getChartMetadataRegistry().get(filterType);
const cascadingFilters = useCascadingFilters(id);
const cascadingFilters = useCascadingFilters(id, dataMaskSelected);
const [state, setState] = useState<ChartDataResponseResult[]>([]);
const [error, setError] = useState<string>('');
const [formData, setFormData] = useState<Partial<QueryFormData>>({});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,27 @@
*/
import { useSelector } from 'react-redux';
import { NativeFiltersState } from 'src/dashboard/reducers/types';
import { DataMaskStateWithId } from 'src/dataMask/types';
import { ExtraFormData } from '@superset-ui/core';
import { mergeExtraFormData } from '../../utils';
import { useNativeFiltersDataMask } from '../state';

// eslint-disable-next-line import/prefer-default-export
export function useCascadingFilters(id: string) {
export function useCascadingFilters(
id: string,
dataMaskSelected?: DataMaskStateWithId,
): ExtraFormData {
const { filters } = useSelector<any, NativeFiltersState>(
state => state.nativeFilters,
);
const filter = filters[id];
const cascadeParentIds: string[] = filter?.cascadeParentIds ?? [];
let cascadedFilters = {};
const nativeFiltersDataMask = useNativeFiltersDataMask();
cascadeParentIds.forEach(parentId => {
const parentState = nativeFiltersDataMask[parentId] || {};
const { extraFormData: parentExtra = {} } = parentState;
cascadedFilters = mergeExtraFormData(cascadedFilters, parentExtra);
const parentState = dataMaskSelected?.[parentId];
cascadedFilters = mergeExtraFormData(
cascadedFilters,
parentState?.extraFormData,
);
});
return cascadedFilters;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
*/
import React from 'react';
import { DataMask } from '@superset-ui/core';
import { DataMaskStateWithId } from 'src/dataMask/types';
import { Filter } from '../../types';

export interface FilterProps {
dataMaskSelected?: DataMaskStateWithId;
filter: Filter & {
dataMask?: DataMask;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

/* eslint-disable no-param-reassign */
import { HandlerFunction, styled, t } from '@superset-ui/core';
import React, { useEffect, useMemo, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { useDispatch } from 'react-redux';
import cx from 'classnames';
import Icon from 'src/components/Icon';
Expand All @@ -37,11 +37,7 @@ import { testWithId } from 'src/utils/testUtils';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import Loading from 'src/components/Loading';
import { getInitialDataMask } from 'src/dataMask/reducer';
import {
getOnlyExtraFormData,
mapParentFiltersToChildren,
TabIds,
} from './utils';
import { getOnlyExtraFormData, TabIds } from './utils';
import FilterSets from './FilterSets';
import {
useNativeFiltersDataMask,
Expand Down Expand Up @@ -175,10 +171,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
const filterValues = Object.values<Filter>(filters);
const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask();
const [isFilterSetChanged, setIsFilterSetChanged] = useState(false);
const cascadeChildren = useMemo(
() => mapParentFiltersToChildren(filterValues),
[filterValues],
);

useEffect(() => {
setDataMaskSelected(() => dataMaskApplied);
Expand All @@ -190,15 +182,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
) => {
setIsFilterSetChanged(tab !== TabIds.AllFilters);
setDataMaskSelected(draft => {
const children = cascadeChildren[filter.id] || [];
// force instant updating on initialization or for parent filters when dataMaskSelected has filter
if (
dataMaskSelected[filter.id] &&
(filter.isInstant || children.length > 0)
) {
dispatch(updateDataMask(filter.id, dataMask));
}

draft[filter.id] = {
...(getInitialDataMask(filter.id) as DataMaskWithId),
...dataMask,
Expand Down