feat(dynamic plugins): migrate dynamic plugins to React#39586
Conversation
* add superset api for dynamic plugins * del view dynamic plugins * fix general plugins url * added new route * refactor adding/editing * fix translations * fix key on key_id in schema * fix key * fix id in schemas plugins * refactor key_id * reorder id column --------- Co-authored-by: aspectie <kovleshenko99@mail.ru>
|
@supersetbot orglabel |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #39586 +/- ##
==========================================
- Coverage 64.58% 64.52% -0.07%
==========================================
Files 2564 2566 +2
Lines 133576 133754 +178
Branches 31033 31070 +37
==========================================
+ Hits 86271 86300 +29
- Misses 45813 45962 +149
Partials 1492 1492
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:
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #717754
Actionable Suggestions - 2
-
superset-frontend/src/features/dynamicPlugins/DynamicPluginModal.tsx - 1
- Type mismatch in callback · Line 112-116
-
superset-frontend/src/pages/DynamicPlugins/index.tsx - 1
- Missing bulk delete functionality · Line 269-288
Additional Suggestions - 1
-
superset-frontend/src/pages/DynamicPlugins/index.tsx - 1
-
Potential incorrect field in key column · Line 123-123The 'Key' column cell displays original.key_id, but the accessor is 'key' and filters use 'key'. Verify if key_id or key should be displayed; if key is correct, update the cell to show original.key for consistency.
-
Review Details
-
Files reviewed - 7 · Commit Range:
1770cbc..1770cbc- superset-frontend/src/components/DynamicPlugins/index.tsx
- superset-frontend/src/features/dynamicPlugins/DynamicPluginModal.tsx
- superset-frontend/src/pages/DynamicPlugins/index.tsx
- superset-frontend/src/views/routes.tsx
- superset/initialization/__init__.py
- superset/security/manager.py
- superset/views/dynamic_plugins.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ 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 evan@preset.io.
Documentation & Help
AI Code Review powered by
| if (!response) return; | ||
| addSuccessToast(t('Dynamic plugin created')); | ||
| if (onPluginAdd) onPluginAdd(response); | ||
| hide(); | ||
| }); |
There was a problem hiding this comment.
The onPluginAdd callback expects a DynamicPluginObject, but response is a number (the ID). Passing the wrong type can cause runtime errors or incorrect behavior in the parent component.
Code suggestion
Check the AI-generated fix before applying
| if (!response) return; | |
| addSuccessToast(t('Dynamic plugin created')); | |
| if (onPluginAdd) onPluginAdd(response); | |
| hide(); | |
| }); | |
| if (!response) return; | |
| addSuccessToast(t('Dynamic plugin created')); | |
| if (onPluginAdd) onPluginAdd({ id: response, ...rest, key: key_id }); | |
| hide(); | |
| }); |
Code Review Run #717754
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| {confirmDelete => ( | ||
| <ListView<DynamicPluginObject> | ||
| className="dynamic-plugins-list-view" | ||
| columns={columns} | ||
| count={pluginsCount} | ||
| data={plugins} | ||
| fetchData={fetchData} | ||
| filters={filters} | ||
| initialSort={initialSort} | ||
| loading={loading} | ||
| pageSize={PAGE_SIZE} | ||
| bulkSelectEnabled={bulkSelectEnabled} | ||
| disableBulkSelect={toggleBulkSelect} | ||
| addDangerToast={addDangerToast} | ||
| addSuccessToast={addSuccessToast} | ||
| emptyState={emptyState} | ||
| refreshData={refreshData} | ||
| /> | ||
| )} | ||
| </ConfirmStatusChange> |
There was a problem hiding this comment.
The ConfirmStatusChange component provides a confirmDelete function to its children, but the ListView does not include bulkActions to utilize it for bulk delete operations. This prevents bulk deletion from working. Add bulkActions similar to other list views like ChartList, with an onSelect handler that calls confirmDelete.
Code Review Run #717754
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
…to feat/migrate-dynamic-plugins
Code Review Agent Run #96c230Actionable 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 |
|
Hey |
|
I think this feature has been largely deprecated in favor of the Extensions effort, but... I suppose this couldn't hurt, if anyone's willing to test/review it. CC @michael-s-molina @villebro in case you know of anyone actively using this. |
|
Hey, our team and our clients use this functionality very actively, so it would be very useful to us. At the moment, Extensions do not allow you to connect additional visualizations. Accordingly, we cannot switch to them. Please take this into consideration when reviewing this PR 😊 |
3 participants
SUMMARY
At the moment, when adding the plugin, it is not displayed in the list of available charts. This PR transfers the presentation of dynamic plugins from server-side rendering (SSR) to fully client-side rendering using React
The new implementation improves maintainability, performance, and consistency with the modern Superset interface stack.
Changes Included 🔧
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before ---
After ---
TESTING INSTRUCTIONS
DYNAMIC_PLUGINSin feature flags.superset initDynamic PluginsADDITIONAL INFORMATION