Skip to content
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 @@ -473,13 +473,15 @@ const FiltersConfigForm = (
const hasFilledDataset =
!hasDataset || (datasetId && (formFilter?.column || !hasColumn));

const hasAdditionalFilters = FILTERS_WITH_ADHOC_FILTERS.includes(
formFilter?.filterType,
);

const canDependOnOtherFilters = filterSupportsDependencies(
formFilter?.filterType,
);
// Use itemTypeField, not formFilter?.filterType directly: the latter can
// be undefined on the first render before the antd Form hydrates (see
// itemTypeField's own fallback chain above), which would otherwise hide
// this section and the cascade-dependency section for a filter type that
// does support them.
const hasAdditionalFilters =
FILTERS_WITH_ADHOC_FILTERS.includes(itemTypeField);

const canDependOnOtherFilters = filterSupportsDependencies(itemTypeField);
Comment thread
rusackas marked this conversation as resolved.

const isDataDirty = formFilter?.isDataDirty ?? true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ const FILTER_SETTINGS_REGEX = /^filter settings$/i;
const DEFAULT_VALUE_REGEX = /^filter has default value$/i;
const MULTIPLE_REGEX = /^can select multiple values$/i;
const FILTER_REQUIRED_REGEX = /^filter value is required/i;
const DEPENDENCIES_REGEX = /^values are dependent on other filters$/i;
// No trailing `$`: like the other tooltip-bearing checkboxes below, the
// accessible name includes the trailing info icon (e.g. "... other filters
// info-circle"), so an exact-end anchor would never match.
const DEPENDENCIES_REGEX = /^values are dependent on other filters/i;
const FIRST_VALUE_REGEX = /^select first filter value by default/i;
const INVERSE_SELECTION_REGEX = /^inverse selection/i;
const SEARCH_ALL_REGEX = /^dynamically search all filter values/i;
Expand Down Expand Up @@ -532,6 +535,43 @@ test('deletes a filter including dependencies', async () => {
);
}, 30000);

test('shows the dependency control on first render for a saved cascade filter', () => {
const nativeFilterConfig = [
buildNativeFilter('NATIVE_FILTER-1', 'state', ['NATIVE_FILTER-2']),
buildNativeFilter('NATIVE_FILTER-2', 'country', []),
];
const state = {
...defaultState(),
dashboardInfo: {
metadata: {
native_filter_configuration: nativeFilterConfig,
},
},
dashboardLayout,
};
defaultRender(state, { ...props, createNewOnOpen: false });

// No interaction: the dependency control must be checked as soon as the
// modal opens on a filter that already has a cascade parent, without
// waiting for a rerender.
expect(getCheckbox(DEPENDENCIES_REGEX)).toBeChecked();
Comment thread
rusackas marked this conversation as resolved.

// The saved parent ("country") must render as the actual selected
// dependency, not a "(deleted or invalid type)" placeholder. antd Select
// renders the active selection as a span whose title attribute is the
// picked option's label.
expect(
document.querySelector(
'.ant-select-content-has-value[title="country"], .ant-select-selection-item[title="country"]',
),
).toBeInTheDocument();

// hasAdditionalFilters has the same first-render read as
// canDependOnOtherFilters above: the pre-filter control must also be
// present (not merely unchecked) on the very first paint.
expect(getCheckbox(PRE_FILTER_REGEX)).not.toBeChecked();
});

const SORTABLE_ITEM_HEIGHT = 40;
const SORTABLE_ITEM_WIDTH = 200;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type { FormInstance } from '@superset-ui/core/components';
import {
filterSupportsDependencies,
useFilterOperations,
FilterOperationsParams,
} from './useFilterOperations';
import { useItemStateManager } from './useItemStateManager';
import { NativeFiltersForm } from '../types';
Expand Down Expand Up @@ -256,3 +257,64 @@ test('restoreFilter cancels the pending removal before the delay elapses', () =>

jest.useRealTimers();
});

function renderFilterOperations(
filters: Record<string, { filterType: string; dependencies?: string[] }>,
removedItems: Record<string, unknown> = {},
) {
const params: FilterOperationsParams = {
form: {
getFieldValue: () => filters,
} as unknown as FilterOperationsParams['form'],
filterState: {
removedItems,
} as unknown as FilterOperationsParams['filterState'],
filterIds: Object.keys(filters),
filterConfigMap: {},
handleModifyItem: jest.fn(),
setActiveItem: jest.fn(),
setSaveAlertVisible: jest.fn(),
};
return renderHook(() => useFilterOperations(params)).result;
}

test('buildDependencyMap drops a parent id whose filter type no longer supports dependencies', () => {
Comment thread
rusackas marked this conversation as resolved.
// "parent" was a Select filter when "child" was configured to depend on
// it, then the user changed "parent" to Time grain within the same
// editing session (before saving) - the stale edge should not linger.
const result = renderFilterOperations({
parent: { filterType: 'filter_timegrain' },
child: { filterType: 'filter_select', dependencies: ['parent'] },
});

const dependencyMap = result.current.buildDependencyMap();

expect(dependencyMap.get('child')).toEqual([]);
});

test('buildDependencyMap keeps a parent id whose filter type still supports dependencies', () => {
const result = renderFilterOperations({
parent: { filterType: 'filter_select' },
child: { filterType: 'filter_select', dependencies: ['parent'] },
});

const dependencyMap = result.current.buildDependencyMap();

expect(dependencyMap.get('child')).toEqual(['parent']);
});

test('buildDependencyMap drops a parent id that is pending removal', () => {
// "parent" is queued for removal but the form still lists it as
// "child"'s dependency until the pending delete is confirmed or undone.
const result = renderFilterOperations(
{
parent: { filterType: 'filter_select' },
child: { filterType: 'filter_select', dependencies: ['parent'] },
},
{ parent: { isPending: true } },
);

const dependencyMap = result.current.buildDependencyMap();

expect(dependencyMap.get('child')).toEqual([]);
});
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,17 @@ export function useFilterOperations({
} else if (configItem?.cascadeParentIds) {
array = [...configItem.cascadeParentIds];
}
dependencyMap.set(key, array);
// Drop parent ids that no longer qualify (removed, or its filter
// type changed to one that doesn't support cascade dependencies)
// as soon as the map is rebuilt, instead of only at save time.
dependencyMap.set(
key,
array.filter(parentId => canBeUsedAsDependency(parentId)),
);
});
}
return dependencyMap;
}, [filterConfigMap, form]);
}, [canBeUsedAsDependency, filterConfigMap, form]);

const getAvailableFilters = useCallback(
(filterId: string, getItemTitle: (id: string) => string) => {
Expand Down
Loading