feat(dashboard): chart customizations modal and plugins#36062
Conversation
|
Bito Automatic Review Skipped - Large PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #36062 +/- ##
===========================================
+ Coverage 0 68.12% +68.12%
===========================================
Files 0 645 +645
Lines 0 48297 +48297
Branches 0 5237 +5237
===========================================
+ Hits 0 32904 +32904
- Misses 0 14109 +14109
- Partials 0 1284 +1284
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8818f4d to
dc321b6
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds Chart Customizations functionality to the dashboard filters modal, enabling users to configure chart-specific customizations (like Time Grain and Time Column) through a unified interface. The implementation includes a plugin-based architecture for extensibility.
Key Changes:
- Backend API endpoints for chart customization CRUD operations
- Unified filters/customizations modal with collapsible sections
- Plugin system for chart customizations (Time Grain, Time Column, Dynamic GroupBy)
- Redux state refactoring to consolidate filter and customization management
Reviewed Changes
Copilot reviewed 96 out of 99 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit_tests/dashboards/test_chart_customizations_dao.py |
Comprehensive unit tests for chart customization DAO operations |
tests/integration_tests/dashboards/api_tests.py |
API integration tests for customization endpoints |
superset/dashboards/api.py |
New PUT endpoint for chart customizations configuration |
superset/daos/dashboard.py |
DAO method to update chart customizations in dashboard metadata |
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx |
Major refactor to support both filters and customizations |
superset-frontend/src/dataMask/reducer.ts |
Enhanced data mask handling for chart customizations |
Comments suppressed due to low confidence (3)
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/state.ts:1
- Comment line removed contains 'TODO' marker without resolution. If the hack is still present, the comment should remain; if resolved, ensure the code is clean.
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.tsx:400 - Dataset columns are fetched every time the component renders or dependencies change. Consider memoizing the fetch result or lifting this data fetching to a higher level component to avoid redundant API calls when multiple GroupByFilterCard components use the same dataset.
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.tsx:388 - Variable 'datasetSource' is of type date, object or regular expression, but it is compared to an expression of type null.
| if attributes: | ||
| metadata = json.loads(dashboard.json_metadata or "{}") |
There was a problem hiding this comment.
The method doesn't handle the case when attributes is None or empty dict. If attributes is None, the function will return updated_configuration which is undefined, causing an UnboundLocalError. Add validation at the start or initialize updated_configuration = [] before the if attributes: block.
| const [renderedFilters, setRenderedFilters] = useState<string[]>( | ||
| initialCurrentFilterId && isFilterId(initialCurrentFilterId) | ||
| ? [initialCurrentFilterId] | ||
| : [], | ||
| ); |
There was a problem hiding this comment.
[nitpick] The initialization logic for renderedFilters and renderedCustomizations is duplicated. Consider extracting this into a helper function or consolidating the logic to reduce duplication and improve maintainability.
| const behaviors = useMemo( | ||
| () => [ | ||
| isCustomization ? Behavior.ChartCustomization : Behavior.NativeFilter, | ||
| ], | ||
| [isCustomization], | ||
| ); |
There was a problem hiding this comment.
[nitpick] The behaviors array should be defined outside the component or as a constant map since it has only two possible values. Creating a new array on every render (even with useMemo) is unnecessary. Consider: const behaviors = isCustomization ? [Behavior.ChartCustomization] : [Behavior.NativeFilter];
| const customizationFilterId = (item as { id: string }).id; | ||
|
|
||
| // Only set metadata default if not already loaded from backend | ||
| const customization = item.customization as { | ||
| column?: string | string[]; | ||
| defaultDataMask?: { filterState?: { value?: string[] } }; | ||
| }; |
There was a problem hiding this comment.
Multiple type assertions with as suggest the types are not well-defined. Consider defining proper TypeScript interfaces for chart customization items to avoid repeated type casting and improve type safety throughout the codebase.
651d3bb to
a828844
Compare
| type: typeof ChartCustomizationType.ChartCustomization; | ||
| name: string; | ||
| filterType: string; | ||
| targets: [Partial<NativeFilterTarget>]; |
There was a problem hiding this comment.
Is this type correct? Is it really a single item in an array?
| useEffect(() => { | ||
| handleChange(defaultValue ?? []); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [JSON.stringify(defaultValue)]); | ||
|
|
||
| useEffect(() => { | ||
| handleChange(filterState.value ?? []); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [JSON.stringify(filterState.value)]); |
There was a problem hiding this comment.
Can't we just include handleChange in the dependency array.
There was a problem hiding this comment.
It causes infinite re-renders, and I couldn't find a quick solution. This code is from the existing Dynamic Group By feature
9b5e27f to
d57d604
Compare
69572e9 to
ad67edc
Compare
| mask => mask?.extraFormData?.visible_deckgl_layers !== undefined, | ||
| ); | ||
|
|
||
| const visibleDeckLayersFromRedux = |
There was a problem hiding this comment.
Can we optimize this with createSelector?
| if (!visibleDeckLayers) { | ||
| visibleDeckLayers = ( | ||
| formData.extra_form_data as ExtraFormData & { | ||
| visible_deckgl_layers?: number[]; |
There was a problem hiding this comment.
You created a similar type in QueryFormData.ts, can we use that?
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…l layers customization
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
I have validated this ticket. Looks good on my side. |
geido
left a comment
There was a problem hiding this comment.
QA and code review passed. Stamping as a code owner to unblock.
User description
SUMMARY
Requirements for chart customizations:
Replace current structure (separate "Add filter" / "Add customization" / "Add divider" buttons)
With: Single dropdown button
Add collapsible sections (like dataset folders in Explore):
Section 1: Filters
Section 2: Customization
User can reorder items within this section only
Important constraint: Customization controls cannot be moved to Filters section and vice versa
Requirements for Deck.gl Layer Visibility customization:
BY DEFAULT IT’S A MULTISELECT
Design and requirements for reordering layers:

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
CodeAnt-AI Description
Unify configuration: edit filters and chart customizations in one modal and persist changes
What Changed
Impact
✅ Configure filters and chart customizations from one modal✅ Persist chart customization changes when applying filters✅ Hide layers in multi-layer deck.gl charts from dashboard💡 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.