fix(dashboard): native filter refresh when dataset changes by sadpandajoe · Pull Request #35964 · apache/superset

sadpandajoe · 2025-11-03T22:07:38Z

SUMMARY

Fixes #35674 - Native filters now properly refresh when the dataset is changed in the filter configuration modal.

Root Cause:
The refresh condition in FiltersConfigForm.tsx:707 had an unnecessary hasDefaultValue gate that prevented refreshing even when data was dirty:

  // Before
  if (hasDefaultValue && isDataDirty) { refresh(); }

  // After
  if (isDataDirty) { refresh(); }

The hasDefaultValue gate blocked refreshes when filters didn't have default values, even though the dataset had. changed. The isDataDirty flag already serves as the natural transition guard for when data needs refreshing.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: Fixes [6.0.0rc2] When editing a filter and selecting a dataset, frontend filter does not take effect #35674
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai · 2025-11-03T22:07:43Z

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

Copilot

Pull Request Overview

This PR enhances the native filters configuration modal by fixing race conditions in async data fetching, improving test coverage, and re-enabling previously skipped tests. The changes address issues where filter data wasn't properly refreshing when datasets changed and where stale async responses could overwrite newer data.

Fixed race condition bugs in async filter data fetching with request tracking
Added comprehensive unit tests for FiltersConfigForm component
Re-enabled and improved existing tests in FiltersConfigModal

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx	Enhanced test helpers, added mock dataset endpoints, re-enabled skipped tests with proper async handling and form stability checks
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx	Added request tracking refs (isMountedRef, latestRequestIdRef) to prevent stale async responses and memory leaks; removed hasDefaultValue dependency to allow refresh on dataset changes
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.test.tsx	New comprehensive test suite covering synchronous/async query handling, dataset/column changes, error scenarios, cleanup behavior, and race conditions

...ard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.test.tsx

...ashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

...ashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

...ard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.test.tsx

Add comprehensive unit test coverage for the FiltersConfigForm component's refresh mechanism, validating the fix for issue #35674 where filters didn't refresh when changing datasets if default values were enabled. Test Coverage (10 passing tests): - Dataset/column change triggers with exact API payload validation - Sync and async query response handling (200/202 status codes) - Error handling for both sync and async paths - isDataDirty state transition verification - Rapid dataset changes (debouncing) - Component cleanup on unmount Component Improvements (discovered during testing): - Fix memory leak: Add isMountedRef to prevent state updates after unmount - Fix race condition: Add latestRequestIdRef with monotonic counter to ignore stale async responses Related to #35674 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Fixes #35674 where native filters wouldn't refresh when changing datasets if no default value was set. **Root Cause:** The refresh condition had an unnecessary `hasDefaultValue` gate that blocked refreshes even when data was dirty (e.g., dataset changed). This was introduced in commit d3f6145 (Sept 2021) and created an unintended restriction. **Fix:** - Removed `hasDefaultValue` from refresh condition (FiltersConfigForm.tsx:707) - `isDataDirty` already provides the natural transition guard (false→true→false) - Dataset changes now trigger refresh regardless of default value preference **Additional Improvements:** - Fixed memory leak: Added `isMountedRef` to prevent state updates after unmount - Fixed race condition: Added `latestRequestIdRef` with monotonic counter to ignore stale responses - Removed unused `hasDefaultValue` from useEffect dependency array **Testing:** - Added comprehensive unit test suite (10 tests, FiltersConfigForm.test.tsx) - Tests cover: dataset/column changes, sync/async responses, error handling, cleanup - All tests follow "avoid nesting" pattern (top-level test() blocks) - Properly typed mocks using concrete types (no `any`) - Integration test baseline unchanged (10 failed, 8 passed - pre-existing failures) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

… test mocks The datasetResult() mock function was missing the database.database_name field that FiltersConfigForm expects when rendering dataset labels. This caused 10 test failures after the refresh mechanism changes enabled earlier dataset access. Added the database object with database_name property to match the real API response structure and ensure proper test coverage of dataset label rendering. Test Results: - Before: 10 failures (all database_name undefined errors) - After: 4 failures (pre-existing unrelated issues) - Fix eliminated all 10 database_name errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

- Remove isMountedRef and latestRequestIdRef from useCallback dependency array (refs are stable and don't need to be dependencies) - Fix Apache license header typo in FiltersConfigForm.test.tsx ("AS IS" was missing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

…orm test - Fix asyncResolve type from MockChartDataResponse to unknown[] to match waitForAsyncData return type - Fix asyncPromise type from Promise<MockChartDataResponse> to Promise<unknown[]> - Replace createMockChartResponse() call with correct result array structure waitForAsyncData returns Promise<ChartDataResponseResult[]> (array of results), not the full response wrapper object. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2025-11-07T22:08:34Z

🎪 Showtime is building environment on GHA for 3cdc0b7

github-actions · 2025-11-07T22:17:51Z

🎪 Showtime deployed environment on GHA for 3cdc0b7

• Environment: http://18.237.248.193:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

sadpandajoe · 2025-11-10T18:29:49Z

Closing in favor of #35488

pull-request-size bot added the size/XXL label Nov 3, 2025

sadpandajoe added the review:draft label Nov 3, 2025

sadpandajoe force-pushed the fix-filter-frontend-refresh branch 3 times, most recently from 15b734f to a002b54 Compare November 6, 2025 22:19

sadpandajoe changed the title ~~fix(native-filters): frontend filters not refreshing after switching datasets~~ fix(dashboard): native filter refresh when dataset changes Nov 6, 2025

sadpandajoe requested a review from Copilot November 6, 2025 22:30

Copilot AI reviewed Nov 6, 2025

View reviewed changes

...ard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.test.tsx Outdated Show resolved Hide resolved

...ashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx Outdated Show resolved Hide resolved

sadpandajoe requested a review from Copilot November 7, 2025 01:18

Copilot AI reviewed Nov 7, 2025

View reviewed changes

...ashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx Outdated Show resolved Hide resolved

...ard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.test.tsx Outdated Show resolved Hide resolved

sadpandajoe and others added 5 commits November 7, 2025 09:43

sadpandajoe force-pushed the fix-filter-frontend-refresh branch from 7ee78ff to 3cdc0b7 Compare November 7, 2025 19:18

sadpandajoe added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Nov 7, 2025

github-actions bot added 🎪 3cdc0b7 🚦 running Environment 3cdc0b7 status: running 🎪 3cdc0b7 🌐 18.237.248.193:8080 Environment 3cdc0b7 URL: http://18.237.248.193:8080 (click to visit) and removed 🎪 🎯 3cdc0b7 Active environment pointer - 3cdc0b7 is receiving traffic labels Nov 7, 2025

sadpandajoe closed this Nov 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dashboard): native filter refresh when dataset changes#35964

fix(dashboard): native filter refresh when dataset changes#35964
sadpandajoe wants to merge 5 commits intomasterfrom
fix-filter-frontend-refresh

sadpandajoe commented Nov 3, 2025 •

edited

Loading

Uh oh!

korbit-ai bot commented Nov 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2025

Uh oh!

github-actions bot commented Nov 7, 2025

Uh oh!

sadpandajoe commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sadpandajoe commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Nov 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2025

Uh oh!

github-actions bot commented Nov 7, 2025

Uh oh!

sadpandajoe commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sadpandajoe commented Nov 3, 2025 •

edited

Loading