You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/09/25 22:22:48 UTC
(superset) 01/01: test(dashboard): cover native filter dependency cycles and config modal save gating
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch test/native-filter-config-modal
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 39d3a699bab40b376c05d0364166241afbcb69c8
Author: Joe Li <jo...@preset.io>
AuthorDate: Fri Sep 25 15:22:24 2026 -0700
test(dashboard): cover native filter dependency cycles and config modal save gating
Add unit coverage for the native filter config modal:
- hasCircularDependency across acyclic, self, 2-node, 3-node and
diamond-shaped dependency graphs
- dependency picker excludes candidates that would create a cycle and
validation flags only filters in the cycle
- pending filter removal finalizes after its delay and restore cancels it
- save stays blocked while a filter has validation errors and unsaved
state stays true while a removal is pending
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
.../hooks/useFilterOperations.test.ts | 171 ++++++++++++++++++++-
.../hooks/useModalSaveLogic.test.ts | 118 ++++++++++++++
.../nativeFilters/FiltersConfigModal/utils.test.ts | 42 +++++
3 files changed, 329 insertions(+), 2 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.test.ts
index 42311ccf3c7..87a6d475341 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.test.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.test.ts
@@ -16,8 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { Behavior } from '@superset-ui/core';
-import { filterSupportsDependencies } from './useFilterOperations';
+import { useState } from 'react';
+import { renderHook, act } from '@testing-library/react';
+import { Behavior, Filter, Divider } from '@superset-ui/core';
+import type { FormInstance } from '@superset-ui/core/components';
+import {
+ filterSupportsDependencies,
+ useFilterOperations,
+} from './useFilterOperations';
+import { useItemStateManager } from './useItemStateManager';
+import { NativeFiltersForm } from '../types';
const mockItems: Record<string, { value: Record<string, unknown> }> = {
filter_select: {
@@ -89,3 +97,162 @@ test('filterSupportsDependencies returns false for an unknown or missing filterT
expect(filterSupportsDependencies('unknown_type')).toBe(false);
expect(filterSupportsDependencies(undefined)).toBe(false);
});
+
+const buildCascadeConfigMap = (): Record<string, Filter | Divider> =>
+ ({
+ f1: { id: 'f1', filterType: 'filter_select', cascadeParentIds: [] },
+ f2: { id: 'f2', filterType: 'filter_select', cascadeParentIds: [] },
+ f3: { id: 'f3', filterType: 'filter_select', cascadeParentIds: [] },
+ }) as unknown as Record<string, Filter | Divider>;
+
+const buildDependencyForm = (
+ dependenciesByFilter: Record<string, string[]>,
+): FormInstance<NativeFiltersForm> => {
+ const filters = Object.fromEntries(
+ Object.keys(dependenciesByFilter).map(id => [
+ id,
+ { filterType: 'filter_select', dependencies: dependenciesByFilter[id] },
+ ]),
+ );
+ return {
+ getFieldValue: (field: string) =>
+ field === 'filters' ? filters : undefined,
+ setFields: jest.fn(),
+ } as unknown as FormInstance<NativeFiltersForm>;
+};
+
+test('getAvailableFilters excludes a candidate that would create a circular dependency', () => {
+ // f2 already depends on f1, so offering f2 as a dependency of f1 would cycle
+ const form = buildDependencyForm({ f1: [], f2: ['f1'], f3: [] });
+ const { result } = renderHook(() =>
+ useFilterOperations({
+ form,
+ filterState: {
+ removedItems: {},
+ } as unknown as ReturnType<typeof useItemStateManager>,
+ filterIds: ['f1', 'f2', 'f3'],
+ filterConfigMap: buildCascadeConfigMap(),
+ handleModifyItem: jest.fn(),
+ setActiveItem: jest.fn(),
+ setSaveAlertVisible: jest.fn(),
+ }),
+ );
+
+ const availableIds = result.current
+ .getAvailableFilters('f1', id => id)
+ .map(option => option.value);
+
+ expect(availableIds).toEqual(['f3']);
+});
+
+test('validateDependencies flags only the filters that participate in a cycle', () => {
+ // f1 <-> f2 form a cycle; f3 has no dependencies
+ const form = buildDependencyForm({ f1: ['f2'], f2: ['f1'], f3: [] });
+ const { result } = renderHook(() =>
+ useFilterOperations({
+ form,
+ filterState: {
+ removedItems: {},
+ } as unknown as ReturnType<typeof useItemStateManager>,
+ filterIds: ['f1', 'f2', 'f3'],
+ filterConfigMap: buildCascadeConfigMap(),
+ handleModifyItem: jest.fn(),
+ setActiveItem: jest.fn(),
+ setSaveAlertVisible: jest.fn(),
+ }),
+ );
+
+ act(() => {
+ result.current.validateDependencies();
+ });
+
+ const setFieldsMock = form.setFields as jest.Mock;
+ const errorsById: Record<string, unknown[]> = {};
+ setFieldsMock.mock.calls.forEach(([[field]]) => {
+ errorsById[field.name[1]] = field.errors;
+ });
+
+ expect(errorsById.f1.length).toBeGreaterThan(0);
+ expect(errorsById.f2.length).toBeGreaterThan(0);
+ expect(errorsById.f3).toEqual([]);
+});
+
+// Minimal state harness (rather than the full useItemStateManager) so these
+// tests isolate handleRemoveFilter/restoreFilter's own timer handling,
+// without useItemStateManager's independent removedItems cleanup effect
+// masking a broken guard in the hook under test.
+function useFilterOperationsHarness(
+ filterIds: string[],
+ form: FormInstance<NativeFiltersForm>,
+) {
+ const [removedItems, setRemovedItems] = useState<
+ ReturnType<typeof useItemStateManager>['removedItems']
+ >({});
+ const [changes, setChanges] = useState<
+ ReturnType<typeof useItemStateManager>['changes']
+ >({ modified: [], deleted: [], reordered: [] });
+ const filterState = {
+ removedItems,
+ setRemovedItems,
+ changes,
+ setChanges,
+ } as unknown as ReturnType<typeof useItemStateManager>;
+ const filterOperations = useFilterOperations({
+ form,
+ filterState,
+ filterIds,
+ filterConfigMap: {},
+ handleModifyItem: jest.fn(),
+ setActiveItem: jest.fn(),
+ setSaveAlertVisible: jest.fn(),
+ });
+ return { filterState, filterOperations };
+}
+
+test('handleRemoveFilter finalizes the removal once the pending delay elapses', () => {
+ jest.useFakeTimers();
+ const form = {
+ getFieldValue: () => undefined,
+ } as unknown as FormInstance<NativeFiltersForm>;
+ const { result } = renderHook(() => useFilterOperationsHarness(['f1'], form));
+
+ act(() => {
+ result.current.filterOperations.handleRemoveFilter('f1');
+ });
+ expect(result.current.filterState.removedItems.f1).toEqual(
+ expect.objectContaining({ isPending: true }),
+ );
+
+ act(() => {
+ jest.advanceTimersByTime(5000);
+ });
+ expect(result.current.filterState.removedItems.f1).toEqual({
+ isPending: false,
+ });
+
+ jest.useRealTimers();
+});
+
+test('restoreFilter cancels the pending removal before the delay elapses', () => {
+ jest.useFakeTimers();
+ const form = {
+ getFieldValue: () => undefined,
+ } as unknown as FormInstance<NativeFiltersForm>;
+ const { result } = renderHook(() => useFilterOperationsHarness(['f1'], form));
+
+ act(() => {
+ result.current.filterOperations.handleRemoveFilter('f1');
+ });
+ act(() => {
+ result.current.filterOperations.restoreFilter('f1');
+ });
+ expect(result.current.filterState.removedItems.f1).toBeNull();
+
+ act(() => {
+ jest.advanceTimersByTime(5000);
+ });
+ // the timer was cancelled by the restore, so it never finalizes the removal
+ expect(result.current.filterState.removedItems.f1).toBeNull();
+
+ jest.useRealTimers();
+});
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useModalSaveLogic.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useModalSaveLogic.test.ts
new file mode 100644
index 00000000000..f3fdc5be770
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useModalSaveLogic.test.ts
@@ -0,0 +1,118 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { renderHook } from '@testing-library/react';
+import type { FormInstance } from '@superset-ui/core/components';
+import { NativeFiltersForm } from '../types';
+import { useModalSaveLogic, ModalSaveLogicParams } from './useModalSaveLogic';
+import { ItemStateManager } from './useItemStateManager';
+
+jest.mock('react-redux', () => ({
+ useDispatch: () => jest.fn(),
+}));
+
+const buildItemState = (
+ overrides: Partial<ItemStateManager> = {},
+): ItemStateManager => ({
+ changes: { modified: [], deleted: [], reordered: [] },
+ newIds: [],
+ removedItems: {},
+ erroredIds: [],
+ orderedIds: [],
+ renderedIds: [],
+ setChanges: jest.fn(),
+ setNewIds: jest.fn(),
+ setRemovedItems: jest.fn(),
+ setErroredIds: jest.fn(),
+ setOrderedIds: jest.fn(),
+ setRenderedIds: jest.fn(),
+ resetState: jest.fn(),
+ addToRendered: jest.fn(),
+ ...overrides,
+});
+
+const buildParams = (
+ overrides: Partial<ModalSaveLogicParams> = {},
+): ModalSaveLogicParams => ({
+ form: {
+ getFieldValue: jest.fn(),
+ isFieldsTouched: jest.fn(() => false),
+ getFieldsError: jest.fn(() => []),
+ } as unknown as FormInstance<NativeFiltersForm>,
+ configFormRef: { current: null },
+ filterState: buildItemState(),
+ customizationState: buildItemState(),
+ filterIds: [],
+ chartCustomizationIds: [],
+ filterConfigMap: {},
+ chartCustomizationConfigMap: {},
+ initialFilterOrder: [],
+ initialCustomizationOrder: [],
+ unsavedFiltersIds: [],
+ currentItemId: '',
+ setActiveItem: jest.fn(),
+ onSave: jest.fn(),
+ canBeUsedAsDependency: jest.fn(() => true),
+ resetForm: jest.fn(),
+ ...overrides,
+});
+
+test('canSave is false while a filter has validation errors, even with pending changes', () => {
+ const params = buildParams({
+ filterState: buildItemState({
+ erroredIds: ['f1'],
+ changes: { modified: ['f1'], deleted: [], reordered: [] },
+ }),
+ });
+
+ const { result } = renderHook(() => useModalSaveLogic(params));
+
+ expect(result.current.canSave).toBe(false);
+});
+
+test('canSave is true once a change exists and no filter has validation errors', () => {
+ const params = buildParams({
+ filterState: buildItemState({
+ changes: { modified: ['f1'], deleted: [], reordered: [] },
+ }),
+ });
+
+ const { result } = renderHook(() => useModalSaveLogic(params));
+
+ expect(result.current.canSave).toBe(true);
+});
+
+test('hasUnsavedChanges is true while a filter removal is still pending', () => {
+ const params = buildParams({
+ filterState: buildItemState({
+ removedItems: { f1: { isPending: true, timerId: 1 } },
+ }),
+ });
+
+ const { result } = renderHook(() => useModalSaveLogic(params));
+
+ expect(result.current.hasUnsavedChanges).toBe(true);
+});
+
+test('hasUnsavedChanges is false when nothing changed and no removal is pending', () => {
+ const params = buildParams();
+
+ const { result } = renderHook(() => useModalSaveLogic(params));
+
+ expect(result.current.hasUnsavedChanges).toBe(false);
+});
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.test.ts
index 97c7defca59..2e7f63d95ea 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.test.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.test.ts
@@ -23,6 +23,7 @@ import {
isChartCustomizationDivider,
getItemType,
getItemTypeInfo,
+ hasCircularDependency,
NATIVE_FILTER_DIVIDER_PREFIX,
CHART_CUSTOMIZATION_DIVIDER_PREFIX,
NATIVE_FILTER_PREFIX,
@@ -137,3 +138,44 @@ test('getItemTypeInfo returns correct info for customization type', () => {
expect(info.dividerType).toBe(ChartCustomizationType.Divider);
expect(info.itemTypeName).toBe('customization');
});
+
+test('hasCircularDependency returns false for a linear chain with no cycle', () => {
+ const dependencyMap = new Map<string, string[]>([
+ ['a', []],
+ ['b', ['a']],
+ ['c', ['b']],
+ ]);
+ expect(hasCircularDependency(dependencyMap, 'c')).toBe(false);
+});
+
+test('hasCircularDependency returns true for a filter that depends on itself', () => {
+ const dependencyMap = new Map<string, string[]>([['a', ['a']]]);
+ expect(hasCircularDependency(dependencyMap, 'a')).toBe(true);
+});
+
+test('hasCircularDependency returns true for a 2-node cycle', () => {
+ const dependencyMap = new Map<string, string[]>([
+ ['a', ['b']],
+ ['b', ['a']],
+ ]);
+ expect(hasCircularDependency(dependencyMap, 'a')).toBe(true);
+});
+
+test('hasCircularDependency returns true for a longer 3-node cycle', () => {
+ const dependencyMap = new Map<string, string[]>([
+ ['a', ['b']],
+ ['b', ['c']],
+ ['c', ['a']],
+ ]);
+ expect(hasCircularDependency(dependencyMap, 'a')).toBe(true);
+});
+
+test('hasCircularDependency returns false for a diamond with a shared ancestor', () => {
+ const dependencyMap = new Map<string, string[]>([
+ ['d', ['b', 'c']],
+ ['b', ['a']],
+ ['c', ['a']],
+ ['a', []],
+ ]);
+ expect(hasCircularDependency(dependencyMap, 'd')).toBe(false);
+});