-
Notifications
You must be signed in to change notification settings - Fork 16.5k
fix(dynamic-group-by): scope chart refreshes to affected charts only #36955
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #a7fcbbActionable 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 |
Nitpicks 🔍
|
superset-frontend/src/dashboard/actions/chartCustomizationActions.ts
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
Outdated
Show resolved
Hide resolved
|
CodeAnt AI finished reviewing your PR. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #e57967Actionable 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 |
superset-frontend/src/dashboard/actions/chartCustomizationActions.ts
Outdated
Show resolved
Hide resolved
Code Review Agent Run #4ba813Actionable 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 |
|
Can we add some test cases? |
|
🎪 Showtime deployed environment on GHA for c3b83fd • Environment: http://35.85.215.129:8080 (admin/admin) |
|
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 · |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review Agent Run #715f0f
Actionable Suggestions - 1
-
superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/GroupByFilterCard.tsx - 1
- Faulty fetch deduplication logic · Line 441-446
Additional Suggestions - 1
-
superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx - 1
-
Race condition in async fetch · Line 432-437The async fetch logic has a race condition: if the dataset changes while a fetch is in progress, the old fetch may complete later and overwrite the state with stale data. To fix, track the current fetching dataset ID and only apply state updates if they match.
-
Review Details
-
Files reviewed - 7 · Commit Range:
ee76300..9eed291- superset-frontend/src/dashboard/actions/chartCustomizationActions.ts
- superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx
- superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/GroupByFilterCard.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
- superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts
- superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
- superset-frontend/src/dashboard/util/getRelatedCharts.ts
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
...set-frontend/src/dashboard/components/nativeFilters/ChartCustomization/GroupByFilterCard.tsx
Outdated
Show resolved
Hide resolved
|
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 · |
Code Review Agent Run #38263cActionable 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 |
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
Outdated
Show resolved
Hide resolved
| const previouslyAffectedChartIds: number[] = []; | ||
| const originalCustomizations = chartCustomizationItems || []; | ||
|
|
||
| originalCustomizations.forEach(oldItem => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole logic seems fragile to me. It definitely needs some drying up also.
|
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 · |
Code Review Agent Run #6c5b7eActionable Suggestions - 0Additional Suggestions - 1
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 |
Code Review Agent Run #3bc615Actionable 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 |
SUMMARY
This change prevents request storms caused by Dynamic Group By chart customizations by aligning chart refresh behavior with the existing native filters pattern.
Previously, applying or saving chart customizations triggered a forced re-query of all charts on the dashboard, regardless of whether they were affected. This caused unnecessary request fan-out and excessive /api/v1/chart/data traffic on dashboards with many charts.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE:

AFTER;

TESTING INSTRUCTIONS
1)Open a dashboard with multiple charts (ideally 10+).
2)Configure Dynamic Group By / Chart Customization for one or more charts.
3)Open the browser DevTools → Network tab (filter by /api/v1/chart/data).
4)Apply the chart customization.
Verify:
1)Only charts affected by the customization issue /api/v1/chart/data requests.
2)Unaffected charts do not re-query.
3)Each affected chart triggers only one request per apply action.
4)No repeated or cascading requests are observed.
ADDITIONAL INFORMATION