pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: Removing parent filter causes incorrect state of child filter (#16876) 

* fix: Removing parent filter causes incorrect state of child filter

* removed to isRemoved

* Shows (deleted) when the parent is removed

* Removes unnecessary code
This commit is contained in:
Michael S. Molina 2021-09-29 13:04:07 -03:00 committed by GitHub
parent 3272d1c086
commit 4da4fe4136
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx
View File

@ -37,6 +37,7 @@ describe('FilterScope', () => {
restoreFilter: jest.fn(),
parentFilters: [],
save,
removedFilters: {},
};
const MockModal = ({ scope }: { scope?: object }) => {

128
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
View File

@ -44,7 +44,7 @@ import React, {
useState,
} from 'react';
import { useSelector } from 'react-redux';
import { isEqual } from 'lodash';
import { isEqual, isEmpty } from 'lodash';
import { FormItem } from 'src/components/Form';
import { Input } from 'src/common/components';
import { Select } from 'src/components';
@ -70,7 +70,7 @@ import {
} from 'src/dashboard/types';
import Loading from 'src/components/Loading';
import { ColumnSelect } from './ColumnSelect';
import { NativeFiltersForm } from '../types';
import { NativeFiltersForm, FilterRemoval } from '../types';
import {
FILTER_SUPPORTED_TYPES,
hasTemporalColumns,
@ -264,7 +264,7 @@ const FilterPanels = {
export interface FiltersConfigFormProps {
filterId: string;
filterToEdit?: Filter;
removed?: boolean;
removedFilters: Record<string, FilterRemoval>;
restoreFilter: (filterId: string) => void;
form: FormInstance<NativeFiltersForm>;
parentFilters: { id: string; title: string }[];
@ -300,21 +300,22 @@ const FiltersConfigForm = (
{
filterId,
filterToEdit,
removed,
removedFilters,
restoreFilter,
form,
parentFilters,
}: FiltersConfigFormProps,
ref: React.RefObject<any>,
) => {
const isRemoved = !!removedFilters[filterId];
const [error, setError] = useState<string>('');
const [metrics, setMetrics] = useState<Metric[]>([]);
const [activeTabKey, setActiveTabKey] = useState<string>(
FilterTabs.configuration.key,
);
const [activeFilterPanelKey, setActiveFilterPanelKey] = useState<
string | string[]
>(FilterPanels.basic.key);
string | string[] | undefined
>();
const [undoFormValues, setUndoFormValues] = useState<Record<
string,
any
@ -325,6 +326,24 @@ const FiltersConfigForm = (
const formValues = form.getFieldValue('filters')?.[filterId];
const formFilter = formValues || undoFormValues || defaultFormFilter;
const parentFilterOptions = useMemo(
() =>
parentFilters.map(filter => ({
value: filter.id,
label: filter.title,
})),
[parentFilters],
);
const parentId =
formFilter?.parentFilter?.value || filterToEdit?.cascadeParentIds?.[0];
const parentFilter = parentFilterOptions.find(
({ value }) => value === parentId,
);
const hasParentFilter = !!parentFilter;
const nativeFilterItems = getChartMetadataRegistry().items;
const nativeFilterVizTypes = Object.entries(nativeFilterItems)
// @ts-ignore
@ -374,7 +393,7 @@ const FiltersConfigForm = (
filterType: formFilter?.filterType,
filterToEdit,
formFilter,
removed,
removed: isRemoved,
})
: {};
const hasColumn = !!mainControlItems.groupby;
@ -506,19 +525,6 @@ const FiltersConfigForm = (
[filterId, form, formChanged],
);
const parentFilterOptions = parentFilters.map(filter => ({
value: filter.id,
label: filter.title,
}));
const parentFilter = parentFilterOptions.find(
({ value }) =>
value === formFilter?.parentFilter?.value ||
value === filterToEdit?.cascadeParentIds?.[0],
);
const hasParentFilter = !!parentFilter;
const hasPreFilter =
!!formFilter?.adhoc_filters ||
!!formFilter?.time_range ||
@ -583,28 +589,32 @@ const FiltersConfigForm = (
return Promise.reject(new Error(t('Pre-filter is required')));
};
let hasCheckedAdvancedControl = hasParentFilter || hasPreFilter || hasSorting;
if (!hasCheckedAdvancedControl) {
hasCheckedAdvancedControl = Object.keys(controlItems)
.filter(key => !BASIC_CONTROL_ITEMS.includes(key))
.some(key => controlItems[key].checked);
}
const ParentSelect = ({
value,
...rest
}: {
value?: { value: string | number };
}) => (
<Select
ariaLabel={t('Parent filter')}
placeholder={t('None')}
options={parentFilterOptions}
allowClear
value={value?.value}
{...rest}
/>
);
}) => {
const parentId = value?.value;
const isParentRemoved = parentId && removedFilters[parentId];
let options = parentFilterOptions;
if (isParentRemoved) {
options = [
{ label: t('(deleted)'), value: parentId as string },
...parentFilterOptions,
];
}
return (
<Select
ariaLabel={t('Parent filter')}
placeholder={t('None')}
options={options}
allowClear
value={parentId}
{...rest}
/>
);
};
useEffect(() => {
if (datasetId) {
@ -647,12 +657,28 @@ const FiltersConfigForm = (
]);
useEffect(() => {
const activeFilterPanelKey = [FilterPanels.basic.key];
if (hasCheckedAdvancedControl) {
activeFilterPanelKey.push(FilterPanels.advanced.key);
// Run only once when the control items are available
if (!activeFilterPanelKey && !isEmpty(controlItems)) {
const hasCheckedAdvancedControl =
hasParentFilter ||
hasPreFilter ||
hasSorting ||
Object.keys(controlItems)
.filter(key => !BASIC_CONTROL_ITEMS.includes(key))
.some(key => controlItems[key].checked);
setActiveFilterPanelKey(
hasCheckedAdvancedControl
? [FilterPanels.basic.key, FilterPanels.advanced.key]
: FilterPanels.basic.key,
);
}
setActiveFilterPanelKey(activeFilterPanelKey);
}, [hasCheckedAdvancedControl]);
}, [
activeFilterPanelKey,
hasParentFilter,
hasPreFilter,
hasSorting,
controlItems,
]);
const initiallyExcludedCharts = useMemo(() => {
const excluded: number[] = [];
@ -678,20 +704,20 @@ const FiltersConfigForm = (
useEffect(() => {
// just removed, saving current form items for eventual undo
if (removed) {
if (isRemoved) {
setUndoFormValues(formValues);
}
}, [removed]);
}, [isRemoved]);
useEffect(() => {
// the filter was just restored after undo
if (undoFormValues && !removed) {
if (undoFormValues && !isRemoved) {
setNativeFilterFieldValues(form, filterId, undoFormValues);
setUndoFormValues(null);
}
}, [formValues, filterId, form, removed, undoFormValues]);
}, [formValues, filterId, form, isRemoved, undoFormValues]);
if (removed) {
if (isRemoved) {
return <RemovedFilter onClick={() => restoreFilter(filterId)} />;
}
@ -718,13 +744,13 @@ const FiltersConfigForm = (
name={['filters', filterId, 'name']}
label={<StyledLabel>{t('Filter name')}</StyledLabel>}
initialValue={filterToEdit?.name}
rules={[{ required: !removed, message: t('Name is required') }]}
rules={[{ required: !isRemoved, message: t('Name is required') }]}
>
<Input {...getFiltersConfigModalTestId('name-input')} />
</StyledFormItem>
<StyledFormItem
name={['filters', filterId, 'filterType']}
rules={[{ required: !removed, message: t('Name is required') }]}
rules={[{ required: !isRemoved, message: t('Name is required') }]}
initialValue={filterToEdit?.filterType || 'filter_select'}
label={<StyledLabel>{t('Filter Type')}</StyledLabel>}
{...getFiltersConfigModalTestId('filter-type')}
@ -782,7 +808,7 @@ const FiltersConfigForm = (
: undefined
}
rules={[
{ required: !removed, message: t('Dataset is required') },
{ required: !isRemoved, message: t('Dataset is required') },
]}
{...getFiltersConfigModalTestId('datasource-input')}
>
@ -837,7 +863,7 @@ const FiltersConfigForm = (
formChanged();
}}
>
{!removed && (
{!isRemoved && (
<StyledRowSubFormItem
name={['filters', filterId, 'defaultDataMask']}
initialValue={initialDefaultValue}

24
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
View File

@ -196,6 +196,27 @@ export function FiltersConfigModal({
title: getFilterTitle(id),
}));
const cleanDeletedParents = (values: NativeFiltersForm | null) => {
Object.keys(filterConfigMap).forEach(key => {
const filter = filterConfigMap[key];
const parentId = filter.cascadeParentIds?.[0];
if (parentId && removedFilters[parentId]) {
filter.cascadeParentIds = [];
}
});
const filters = values?.filters;
if (filters) {
Object.keys(filters).forEach(key => {
const filter = filters[key];
const parentId = filter.parentFilter?.value;
if (parentId && removedFilters[parentId]) {
filter.parentFilter = undefined;
}
});
}
};
const handleSave = async () => {
const values: NativeFiltersForm | null = await validateForm(
form,
@ -207,6 +228,7 @@ export function FiltersConfigModal({
);
if (values) {
cleanDeletedParents(values);
createHandleSave(
filterConfigMap,
filterIds,
@ -295,7 +317,7 @@ export function FiltersConfigModal({
form={form}
filterId={id}
filterToEdit={filterConfigMap[id]}
removed={!!removedFilters[id]}
removedFilters={removedFilters}
restoreFilter={restoreFilter}
parentFilters={getParentFilters(id)}
/>

2
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts
View File

@ -36,7 +36,7 @@ export interface NativeFiltersFormItem {
};
defaultValue: any;
defaultDataMask: DataMask;
parentFilter: {
parentFilter?: {
value: string;
label: string;
};

6
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
View File

@ -65,8 +65,8 @@ export const validateForm = async (
'Cannot create cyclic hierarchy',
);
}
const parentId = formValues.filters[filterId]
? formValues.filters[filterId].parentFilter?.value
const parentId = formValues.filters?.[filterId]
? formValues.filters[filterId]?.parentFilter?.value
: filterConfigMap[filterId]?.cascadeParentIds?.[0];
if (parentId) {
validateCycles(parentId, [...trace, filterId]);
@ -111,7 +111,7 @@ export const createHandleSave = (
.filter(id => !removedFilters[id])
.map(id => {
// create a filter config object from the form inputs
const formInputs = values.filters[id];
const formInputs = values.filters?.[id];
// if user didn't open a filter, return the original config
if (!formInputs) return filterConfigMap[id];
const target: Partial<Target> = {};