Repository navigation
perf(datasource): add pagination to datasource editor tables to prevent browser freeze - #37555
Conversation
There was a problem hiding this comment.
Code Review Agent Run #59bf11
Actionable Suggestions - 4
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx - 1
- Search breaks pagination · Line 297-325
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/DatasetUsageTab.test.tsx - 1
- Inadequate Test Assertion · Line 656-682
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx - 1
- Removed dataset settings UI · Line 2026-2026
-
superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts - 1
- Logic Error: Incorrect Extra Metrics Extraction · Line 29-44
Additional Suggestions - 2
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx - 1
-
Unsafe null handling in filter · Line 2040-2046Replace `col.column_name?.toLowerCase().includes(...)` with `( (col.column_name || '').toLowerCase() ).includes(...)` to safely handle null or undefined column_name.
-
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx - 1
-
Incomplete assertions in case-insensitive test · Line 656-658The test checks that 'sum__num' is found with 'SUM' input, but doesn't verify that non-matching metrics like 'avg__num' are hidden. This leaves the test incomplete in asserting the full filtering logic.
-
Review Details
-
Files reviewed - 10 · Commit Range:
937be9b..937be9b- superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts
- superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts
- superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
- superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/DatasetUsageTab.test.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
- superset-frontend/src/components/Datasource/types.ts
- superset-frontend/src/explore/components/controls/TextAreaControl.tsx
-
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 evan@preset.io.
Documentation & Help
937be9b to
c3a6d49
Compare
Code Review Agent Run #611ebfActionable 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 |
c3a6d49 to
2d4e485
Compare
There was a problem hiding this comment.
Code Review Agent Run #1f6ca0
Actionable Suggestions - 1
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx - 1
- Runtime Error in Column Search Filter · Line 2437-2438
Additional Suggestions - 2
-
superset-frontend/src/components/Datasource/DatasourceModal/index.tsx - 1
-
Modal behavior change · Line 366-367Adding `resizable` and `draggable` props disables the modal mask, allowing background interaction and changing the modal from blocking to non-blocking behavior. This may not be intended for dataset editing, where users should focus on the modal.
-
-
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx - 1
-
Inconsistent search filtering logic · Line 2393-2399The search for columns and calculated columns only filters on column_name, but metrics search includes both metric_name and verbose_name. Since columns have verbose_name (used in display), users may expect to search by label too. This creates inconsistent behavior where searching by verbose_name works for metrics but not for columns/calculated columns.
Code suggestion
@@ -2392,8 +2392,12 @@ - ? this.state.databaseColumns.filter(col => - col.column_name - ?.toLowerCase() - .includes( - this.state.columnSearchTerm.toLowerCase(), - ), - ) + ? this.state.databaseColumns.filter(col => + col.column_name + ?.toLowerCase() + .includes( + this.state.columnSearchTerm.toLowerCase(), + ) || + col.verbose_name + ?.toLowerCase() + .includes( + this.state.columnSearchTerm.toLowerCase(), + ), - ) + ) @@ -2381,1 +2381,1 @@ - placeholder={t('Search columns by name')} + placeholder={t('Search columns by key or label')} @@ -2436,8 +2436,12 @@ - ? this.state.calculatedColumns.filter(col => - col.column_name - ?.toLowerCase() - .includes( - this.state.calculatedColumnSearchTerm.toLowerCase(), - ), - ) + ? this.state.calculatedColumns.filter(col => + col.column_name + ?.toLowerCase() + .includes( + this.state.calculatedColumnSearchTerm.toLowerCase(), + ) || + col.verbose_name + ?.toLowerCase() + .includes( + this.state.calculatedColumnSearchTerm.toLowerCase(), + ), - ) + ) @@ -2423,1 +2423,1 @@ - placeholder={t('Search calculated columns by name')} + placeholder={t('Search calculated columns by key or label')}
-
Review Details
-
Files reviewed - 7 · Commit Range:
46250ff..2d4e485- superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
- superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/DatasetUsageTab.test.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
- superset-frontend/src/components/Datasource/types.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 evan@preset.io.
Documentation & Help
1. DatasetUsageTab/index.tsx — Search breaks pagination
Bot: Client-side search filters current page but pagination still fetches unfiltered server data.
Fix: Disabled pagination entirely when searchTerm is active (pagination={false}). When searching, all matches from the current page are shown without pagination controls.
---
2. DatasetUsageTab/DatasetUsageTab.test.tsx — Inadequate test assertion
Bot: Test compared 1/1 in both filtered and unfiltered states — identical with only 2 mock charts.
Fix: Rewrote test to verify pagination is hidden when searching and restored when cleared, and that only matching chart appears during search.
---
3. CollectionTable/index.tsx — currentPage can exceed available pages
Bot: When collection shrinks, currentPage can exceed maxPage, showing an empty page.
Fix: Added clamping logic — currentPage = Math.min(this.state.currentPage, maxPage) before passing to the table's pagination config.
---
4. DatasourceEditor.tsx — renderDefaultColumnSettings() accidentally removed
Bot: The default datetime column and currency code column settings UI was missing from both the Columns and Calculated Columns tabs.
Fix: Restored {this.renderDefaultColumnSettings()} at the top of both tab content sections.
---
5. DatasourceEditor.tsx + CollectionTable + types.ts — Filtered 5. DatasourceEditor.tsx + CollectionTable + types.ts — Filtered 5.rics/columns to CollectionTable meant edits while searching would drop all non-matching items.
Fix:
- Added filterTerm and filterFields props to CRUDCollectionProps and ColumnCollectionTableProps
- CollectionTable now applies filtering for display only ( - CollectionTable now applies filtering for display only ( - Collection
- All three tables (metrics, columns, calculated columns) now pass their full data to collection and use filterTerm/filterFields for visual filtering only
There was a problem hiding this comment.
Code Review Agent Run #d6e220
Actionable Suggestions - 1
-
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx - 1
- Incorrect pagination clamping with filtering · Line 453-464
Review Details
-
Files reviewed - 7 · Commit Range:
2d4e485..4e630a8- superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
- superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/DatasetUsageTab.test.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
- superset-frontend/src/components/Datasource/types.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 evan@preset.io.
Documentation & Help
…ted column search is case-insensitive) to use getByDisplayValue/queryByDisplayValue instead of getByText/queryByText, since the column_name field in the calculated columns tab is rendered as a TextControl input (not plain text) because editableColumnName=true is passed to that table.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…thout an accessible name, so getByRole('list', { name: 'Pagination' }) fails. Replaced with getByTitle('1') to check for the page-1
listitem (presence = pagination visible) and queryByTitle('Next Page') to check pagination is hidden while searching.
Code Review Agent Run #ffa242Actionable 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 |
…ayData.length so clamping is based on filtered results - Added total: totalItems to paginationConfig so Ant Design Table knows the correct total for the filtered view
Code Review Agent Run #00d4c9Actionable 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 |
villebro
left a comment
There was a problem hiding this comment.
LGTM, thanks for this amazing improvement! ❤️
…nt browser freeze (apache#37555) Co-authored-by: madhushree agarwal <madhushree_agarwal@apple.com>
SUMMARY
This update significantly enhances the user experience within the datasource editor, particularly when managing a large number of metrics.
Previously, accessing metrics caused the editor window to become unresponsive for over six minutes when dealing with 100 or more metrics. To resolve this critical performance issue, we've implemented pagination, dramatically reducing response times to under two seconds, even with the same extensive datasets (displaying 100 metrics per page).
Furthermore, this improvement includes enhanced usability features such as sorting, along with resizable windows and expression fields, providing a more fluid and efficient editing environment.
TLDR;
CollectionTablecomponent used in the dataset editor, with configurable page size (default 25, options: 10/25/50/100)BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before -
When opening the metrics tab for datasets with 100+ metrics:
Screen.Recording.-.Pagination.before.mov
After -
Screen.Recording.-.Pagination.after.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION