Skip to content

Commit d19ff25

Browse files
sadpandajoeclaude
andcommitted
fix(dataset): add pagination props to DatasetUsageTab Table component
Fixes pagination display issue where navigating to page 2+ would show incorrect page counts (e.g., "1 page" instead of "3 pages"). **Root cause**: Table component was missing `current`, `total`, and `pageSize` props in the pagination config, causing it to calculate total pages from `data.length` instead of `totalCount`. **Changes**: - Add `current`, `total`, and `pageSize` to pagination config - Update test assertions from `aria-current` to `ant-pagination-item-active` - Update test queries from `getByRole('link')` to `getByRole('listitem')` (Ant Design 5 renders pagination items as <li>, not <a>) - Add TypeScript type guard for pagination prop access - Fix type compatibility for pagination.onChange (undefined → null) **Test results**: All 16 tests passing (pagination navigation, state persistence, edge cases, and async cleanup tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ecce53e commit d19ff25

File tree

2 files changed

+36
-24
lines changed

2 files changed

+36
-24
lines changed

superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/DatasetUsageTab.test.tsx

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ import {
2525
} from 'spec/helpers/testing-library';
2626
import userEvent from '@testing-library/user-event';
2727
import fetchMock from 'fetch-mock';
28-
import { TableProps } from '@superset-ui/core/components/Table';
28+
import {
29+
TableProps,
30+
OnChangeFunction,
31+
} from '@superset-ui/core/components/Table';
2932
import DatasetUsageTab from '.';
3033

3134
jest.mock('@superset-ui/core/components/Table', () => {
@@ -38,19 +41,19 @@ jest.mock('@superset-ui/core/components/Table', () => {
3841
let latestPaginationPageSize: number | undefined;
3942
let latestPaginationCurrent: number | undefined;
4043
let latestPaginationTotal: number | undefined;
41-
let latestOnChangeHandler:
42-
| ((pagination: { current: number; pageSize?: number }) => void)
43-
| null = null;
44+
let latestOnChangeHandler: OnChangeFunction<any> | null = null;
4445

4546
const MockTable = <RecordType extends object>({
4647
pagination,
4748
onChange,
4849
...rest
4950
}: TableProps<RecordType>) => {
50-
latestPaginationPageSize = pagination?.pageSize;
51-
latestPaginationCurrent = pagination?.current;
52-
latestPaginationTotal = pagination?.total;
53-
latestPaginationHandler = pagination?.onChange;
51+
if (pagination && typeof pagination !== 'boolean') {
52+
latestPaginationPageSize = pagination.pageSize;
53+
latestPaginationCurrent = pagination.current;
54+
latestPaginationTotal = pagination.total;
55+
latestPaginationHandler = pagination.onChange || null;
56+
}
5457
latestOnChangeHandler = onChange;
5558

5659
return (
@@ -496,10 +499,10 @@ test('displays correct pagination and navigates between pages', async () => {
496499
totalCount: 30,
497500
});
498501

499-
// Page 1: Verify UI shows page 1 is active (using aria-current for robustness)
502+
// Page 1: Verify UI shows page 1 is active (using Ant Design active class)
500503
await waitFor(() => {
501504
const page1Item = screen.getByRole('listitem', { name: '1' });
502-
expect(page1Item).toHaveAttribute('aria-current', 'page');
505+
expect(page1Item).toHaveClass('ant-pagination-item-active');
503506
});
504507

505508
// Soft assertion: verify component passes correct props to Table
@@ -511,8 +514,8 @@ test('displays correct pagination and navigates between pages', async () => {
511514
expect(tableMock.__getLatestPaginationPageSize()).toBe(25);
512515

513516
// User clicks page 2 (using userEvent for realistic interaction)
514-
const page2Link = screen.getByRole('link', { name: '2' });
515-
await userEvent.click(page2Link);
517+
const page2Item = screen.getByRole('listitem', { name: '2' });
518+
await userEvent.click(page2Item);
516519

517520
// Data fetch called for page 2
518521
await waitFor(() =>
@@ -524,10 +527,10 @@ test('displays correct pagination and navigates between pages', async () => {
524527
),
525528
);
526529

527-
// Verify UI reflects page 2 is now active (aria-current is more resilient than class names)
530+
// Verify UI reflects page 2 is now active (using Ant Design active class)
528531
await waitFor(() => {
529532
const page2Item = screen.getByRole('listitem', { name: '2' });
530-
expect(page2Item).toHaveAttribute('aria-current', 'page');
533+
expect(page2Item).toHaveClass('ant-pagination-item-active');
531534
});
532535

533536
// Soft assertion: Table still receives correct total (catches the bug!)
@@ -555,18 +558,18 @@ test('maintains pagination state across multiple page changes', async () => {
555558
totalCount: 60,
556559
});
557560

558-
// Verify initial state (page 1) using aria-current
561+
// Verify initial state (page 1) using Ant Design active class
559562
await waitFor(() => {
560563
const page1Item = screen.getByRole('listitem', { name: '1' });
561-
expect(page1Item).toHaveAttribute('aria-current', 'page');
564+
expect(page1Item).toHaveClass('ant-pagination-item-active');
562565
});
563566

564567
// Soft assertion: initial props are correct
565568
// eslint-disable-next-line no-underscore-dangle
566569
expect(tableMock.__getLatestPaginationTotal()).toBe(60);
567570

568571
// User navigates to page 2
569-
await userEvent.click(screen.getByRole('link', { name: '2' }));
572+
await userEvent.click(screen.getByRole('listitem', { name: '2' }));
570573

571574
await waitFor(() => {
572575
expect(mockOnFetchCharts).toHaveBeenCalledWith(
@@ -576,7 +579,7 @@ test('maintains pagination state across multiple page changes', async () => {
576579
expect.any(String),
577580
);
578581
const page2Item = screen.getByRole('listitem', { name: '2' });
579-
expect(page2Item).toHaveAttribute('aria-current', 'page');
582+
expect(page2Item).toHaveClass('ant-pagination-item-active');
580583
});
581584

582585
// Soft assertion: props update correctly after navigation
@@ -586,7 +589,7 @@ test('maintains pagination state across multiple page changes', async () => {
586589
expect(tableMock.__getLatestPaginationTotal()).toBe(60);
587590

588591
// User navigates to page 3
589-
await userEvent.click(screen.getByRole('link', { name: '3' }));
592+
await userEvent.click(screen.getByRole('listitem', { name: '3' }));
590593

591594
await waitFor(() => {
592595
expect(mockOnFetchCharts).toHaveBeenCalledWith(
@@ -596,7 +599,7 @@ test('maintains pagination state across multiple page changes', async () => {
596599
expect.any(String),
597600
);
598601
const page3Item = screen.getByRole('listitem', { name: '3' });
599-
expect(page3Item).toHaveAttribute('aria-current', 'page');
602+
expect(page3Item).toHaveClass('ant-pagination-item-active');
600603
});
601604

602605
// Soft assertion: total remains consistent across all navigations
@@ -624,9 +627,11 @@ test('handles total count below one page (no pagination needed)', async () => {
624627
totalCount: 5,
625628
});
626629

627-
// Pagination should not show page 2 link since only 1 page exists
630+
// Pagination should not show page 2 item since only 1 page exists
628631
await waitFor(() => {
629-
expect(screen.queryByRole('link', { name: '2' })).not.toBeInTheDocument();
632+
expect(
633+
screen.queryByRole('listitem', { name: '2' }),
634+
).not.toBeInTheDocument();
630635
});
631636

632637
// Table receives correct props
@@ -654,7 +659,9 @@ test('handles server returning fewer rows than page size', async () => {
654659

655660
// Only page 1 should exist
656661
await waitFor(() => {
657-
expect(screen.queryByRole('link', { name: '2' })).not.toBeInTheDocument();
662+
expect(
663+
screen.queryByRole('listitem', { name: '2' }),
664+
).not.toBeInTheDocument();
658665
});
659666

660667
// Table receives correct total (not data.length!)

superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,12 @@ const DatasetUsageTab = ({
281281
recordCount={totalCount}
282282
usePagination
283283
defaultPageSize={PAGE_SIZE}
284-
pagination={{ hideOnSinglePage: false }}
284+
pagination={{
285+
current: currentPage,
286+
total: totalCount,
287+
pageSize: PAGE_SIZE,
288+
hideOnSinglePage: false,
289+
}}
285290
loading={loading}
286291
size={TableSize.Middle}
287292
rowKey={(record: Chart) =>

0 commit comments

Comments
 (0)