fix(dashboard): enable cross-filtering across tabs#38762
fix(dashboard): enable cross-filtering across tabs#38762Mayankaggarwal8055 wants to merge 6 commits intoapache:masterfrom
Conversation
Code Review Agent Run #8a3f89Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
bot
added
change:frontend
codeant-ai-for-open-source
bot
added
the
size:S
Sequence DiagramThis PR changes cross filter targeting from tab scoped behavior to dashboard wide behavior. When a user selects a cross filter in one chart, the system now applies it to all other charts across all tabs. sequenceDiagram
participant User
participant Dashboard
participant CrossFilterConfig
participant RelatedCharts
participant Charts
User->>Dashboard: Select cross filter on source chart
Dashboard->>CrossFilterConfig: Build charts in scope for source chart
CrossFilterConfig-->>Dashboard: Return all chart ids except source
Dashboard->>RelatedCharts: Resolve related charts for update
RelatedCharts-->>Dashboard: Return all slices except source
Dashboard->>Charts: Apply filter to target charts across tabs
Charts-->>User: Updated visualizations in all tabs
Generated by CodeAnt AI |
Mayankaggarwal8055
force-pushed
the
fix/cross-filter-across-tabs
branch
from
e954778 to
634e97b
Compare
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| }} | ||
| title={t('Dashboard properties')} | ||
| isEditMode | ||
| resizable |
There was a problem hiding this comment.
Suggestion: Enabling the modal resizable mode here unintentionally disables the modal mask in the shared Modal component (mask is turned off when resizable is true), which allows click-through interactions with the dashboard while the properties dialog is open. That can cause users to mutate dashboard state behind an active edit form and create inconsistent/accidental edits. Keep this modal non-resizable (or only enable resizing after adding a masked-resizable behavior in the base modal). [logic error]
Severity Level: Major ⚠️
- ⚠️ Dashboard tabs can switch behind properties modal.
- ⚠️ Background dashboard state mutates during property editing.
- ❌ Modal isolation breaks on a common edit flow.| resizable | |
| resizable={false} |
Steps of Reproduction ✅
1. Open a dashboard page and trigger "Edit properties" from the header actions menu;
`useHeaderActionsDropdownMenu.tsx:24-26` calls `showPropertiesModal()`, and
`Header/index.tsx:512-514` sets `showingPropertiesModal` true.
2. The modal renders `PropertiesModal` (`Header/index.tsx` JSX block in the 820-929
section), and this component passes `resizable` to `StandardModal` at
`dashboard/components/PropertiesModal/index.tsx:631`.
3. `StandardModal` forwards `resizable` into core `Modal`
(`src/components/Modal/StandardModal.tsx:135-136`), where `shouldShowMask = !(resizable ||
draggable)` (`packages/superset-ui-core/src/components/Modal/Modal.tsx:280`) and
`mask={shouldShowMask}` is applied (`Modal.tsx:360`), disabling backdrop mask when
resizable is true.
4. While properties dialog is open, click a dashboard tab behind it; tab clicks execute
`handleClickTab` in `dashboard/components/gridComponents/Tabs/Tabs.tsx:230-249`
(`setSelectedTabIndex` / `setActiveKey`), proving background dashboard state can mutate
behind an active edit modal.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
**Line:** 631:631
**Comment:**
*Logic Error: Enabling the modal `resizable` mode here unintentionally disables the modal mask in the shared `Modal` component (`mask` is turned off when `resizable` is true), which allows click-through interactions with the dashboard while the properties dialog is open. That can cause users to mutate dashboard state behind an active edit form and create inconsistent/accidental edits. Keep this modal non-resizable (or only enable resizing after adding a masked-resizable behavior in the base modal).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
- This PR does not introduce or modify the
resizableprop - The change appears to be from an earlier/outdated diff, but let me know if anything still needs adjustment here
| chartConfiguration[chartId].crossFilters.chartsInScope = | ||
| Object.values(charts) | ||
| .map(chart => chart.id) | ||
| .filter(id => id !== Number(chartId)); |
There was a problem hiding this comment.
Suggestion: The new assignment ignores each chart's configured scope and always targets every chart except itself. This breaks saved custom scoping (including exclusions/layer selections) and also ignores global scope exclusions for globally-scoped charts. Recompute chartsInScope from the effective scope (global pointer resolved to global scope, otherwise per-chart scope) via getChartIdsInFilterScope, then remove the source chart id. [logic error]
Severity Level: Critical 🚨
- ❌ Saved custom scope exclusions ignored after dashboard reload.
- ❌ Excluded charts still receive cross-filter query constraints.
- ⚠️ Global-scope exclusions for global charts are bypassed.| chartConfiguration[chartId].crossFilters.chartsInScope = | |
| Object.values(charts) | |
| .map(chart => chart.id) | |
| .filter(id => id !== Number(chartId)); | |
| const scope = chartConfiguration[chartId].crossFilters.scope; | |
| const effectiveScope: NativeFilterScope = isCrossFilterScopeGlobal(scope) | |
| ? globalChartConfiguration.scope | |
| : scope; | |
| chartConfiguration[chartId].crossFilters.chartsInScope = | |
| getChartIdsInFilterScope( | |
| effectiveScope, | |
| Object.values(charts).map(chart => chart.id), | |
| chartLayoutItems, | |
| ).filter(id => id !== Number(chartId)); |
Steps of Reproduction ✅
1. Configure a custom cross-filter scope in the UI and save it: `ScopingModal.tsx:151-167`
computes scoped `chartsInScope` via `getChartIdsInFilterScope`, and
`ScopingModal.tsx:143-146` persists it using `saveChartConfiguration`.
2. Reload the dashboard: hydration calls `getCrossFiltersConfiguration` from
`hydrate.ts:314-320`; then `crossFilters.ts:101-104` overwrites every interactive chart's
`crossFilters.chartsInScope` to all charts except itself, ignoring saved per-chart scope
and global exclusions.
3. Apply a cross-filter from the source chart: active filters are rebuilt in
`DashboardPage.tsx:95-110`, and `activeAllDashboardFilters.ts:119-121` reads the
overwritten `chart_configuration[filterId].crossFilters.chartsInScope`.
4. Observe excluded charts still filtered/requeried:
`getFormDataWithExtraFilters.ts:53-56` applies filters to any chart id in that scope, so
charts meant to be excluded now receive cross-filter `extra_form_data`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/util/crossFilters.ts
**Line:** 101:104
**Comment:**
*Logic Error: The new assignment ignores each chart's configured scope and always targets every chart except itself. This breaks saved custom scoping (including exclusions/layer selections) and also ignores global scope exclusions for globally-scoped charts. Recompute `chartsInScope` from the effective scope (`global` pointer resolved to global scope, otherwise per-chart scope) via `getChartIdsInFilterScope`, then remove the source chart id.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
- Removed the override that was forcing all charts into scope
- Now using getChartIdsInFilterScope with effective scope resolution (global vs per-chart)
- Ensures existing scoped filtering behavior is preserved
| // Always return all charts except source chart | ||
| return Object.values(slices) | ||
| .map(slice => slice.slice_id) | ||
| .filter(id => id !== Number(filterKey)); |
There was a problem hiding this comment.
Suggestion: The new cross-filter logic ignores the scope argument and always returns every chart except the source chart. This breaks existing scoped cross-filter behavior (including chart-level custom scoping) and will trigger refreshes for charts that should remain out of scope. Filter the result by scope first, then exclude the source chart. [logic error]
Severity Level: Major ⚠️
- ❌ Dashboard.applyFilters refreshes out-of-scope charts unnecessarily.
- ⚠️ Cross-filter scoping modal settings are functionally ignored.
- ⚠️ Focus highlighting can include unrelated charts.| // Always return all charts except source chart | |
| return Object.values(slices) | |
| .map(slice => slice.slice_id) | |
| .filter(id => id !== Number(filterKey)); | |
| const chartsInScopeSet = new Set(scope); | |
| return Object.values(slices) | |
| .map(slice => slice.slice_id) | |
| .filter(id => chartsInScopeSet.has(id) && id !== Number(filterKey)); |
Steps of Reproduction ✅
1. Open cross-filter scoping UI and set a custom scope for one chart;
`ScopingModal.handleScopeUpdate()` computes scoped `chartsInScope` via
`getChartIdsInFilterScope` in
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx:151-166`.
2. Save scoping; it is persisted through `saveChartConfiguration()` in
`superset-frontend/src/dashboard/actions/dashboardInfo.ts:57-94`, then consumed as
`chart_configuration` in Redux metadata.
3. Trigger a cross-filter from that chart; active filters are built in
`getAllActiveFilters()` using `chartConfiguration[filterId].crossFilters.chartsInScope`
(`superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts:115-123`) and passed
into `Dashboard.applyFilters()`
(`superset-frontend/src/dashboard/components/Dashboard.tsx:288-303`).
4. `getRelatedChartsForCrossFilter()` ignores `scope` and returns all charts except source
in `superset-frontend/src/dashboard/util/getRelatedCharts.ts:53-61`, so `triggerQuery()`
refreshes out-of-scope charts in `Dashboard.refreshCharts()` (`Dashboard.tsx:100-103`).Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/util/getRelatedCharts.ts
**Line:** 58:61
**Comment:**
*Logic Error: The new cross-filter logic ignores the `scope` argument and always returns every chart except the source chart. This breaks existing scoped cross-filter behavior (including chart-level custom scoping) and will trigger refreshes for charts that should remain out of scope. Filter the result by `scope` first, then exclude the source chart.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
- Restored scope-aware filtering
- Now only returns charts within the provided scope (excluding the source chart)
Code Review Agent Run #f8f4e9Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Mayankaggarwal8055
force-pushed
the
fix/cross-filter-across-tabs
branch
from
4ba0ea0 to
634e97b
Compare
Code Review Agent Run #663bc6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
There was a problem hiding this comment.
Pull request overview
Enables dashboard cross-filtering to propagate beyond the current tab by adjusting how cross-filter scoping and related-chart resolution are computed.
Changes:
- Simplifies related-chart resolution for cross-filters to rely on the computed
chartsInScopeset (and exclude the source chart). - Updates cross-filter configuration to always resolve
chartsInScopeviagetChartIdsInFilterScope(...), using global scope when the chart points to the global configuration. - Tightens TypeScript typing by making
NativeFilterScopea type-only import.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| superset-frontend/src/dashboard/util/getRelatedCharts.ts | Updates cross-filter related-chart resolution to use the provided scope set and exclude the source chart id. |
| superset-frontend/src/dashboard/util/crossFilters.ts | Changes how chartsInScope is derived (effective scope resolution) to support broader scoping behavior. |
| const scope = chartConfiguration[chartId].crossFilters.scope; | ||
|
|
||
| const effectiveScope: NativeFilterScope = isCrossFilterScopeGlobal(scope) | ||
| ? globalChartConfiguration.scope | ||
| : scope; | ||
|
|
||
| chartConfiguration[chartId].crossFilters.chartsInScope = | ||
| getChartIdsInFilterScope( | ||
| effectiveScope, | ||
| Object.values(charts).map(chart => chart.id), | ||
| chartLayoutItems, | ||
| ).filter(id => id !== Number(chartId)); |
There was a problem hiding this comment.
getCrossFiltersConfiguration recomputes getChartIdsInFilterScope(...) for every interactive chart, including when scope is the global pointer. This is redundant with globalChartConfiguration.chartsInScope and can become expensive on large dashboards (nested loop via layoutItems.find). Consider reusing globalChartConfiguration.chartsInScope for the global case (as before) and/or caching the computed chart ids per unique scope, and also computing Object.values(charts).map(chart => chart.id) once outside the loop.
| const scope = chartConfiguration[chartId].crossFilters.scope; | ||
|
|
||
| const effectiveScope: NativeFilterScope = isCrossFilterScopeGlobal(scope) | ||
| ? globalChartConfiguration.scope | ||
| : scope; | ||
|
|
||
| chartConfiguration[chartId].crossFilters.chartsInScope = | ||
| getChartIdsInFilterScope( | ||
| effectiveScope, | ||
| Object.values(charts).map(chart => chart.id), | ||
| chartLayoutItems, | ||
| ).filter(id => id !== Number(chartId)); |
There was a problem hiding this comment.
This change is intended to make cross-filter scoping work across tabs, but there’s no unit test exercising a tabbed dashboard layout (e.g., charts under different TAB-* parents) to prevent regressions. Consider adding a test case (in the existing crossFilters.test.ts) that builds a layout with multiple tabs and verifies chartsInScope includes charts from inactive tabs when global scoping is used.
Code Review Agent Run #b6683bActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #5d2826Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
codeant-ai-for-open-source
bot
added
size:S
User description
SUMMARY
Cross-filtering in dashboards was limited to charts within the same tab, preventing filters from propagating across tabs.
PROBLEM
SOLUTION
RESULT
TESTING
IMPACT
CodeAnt-AI Description
Cross-filters now update charts across the whole dashboard
What Changed
Impact
✅ Cross-filters apply across tabs✅ Fewer manual re-applied filters✅ More consistent dashboard filtering💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.