Compare commits

...

10 Commits

Author SHA1 Message Date
Evan Rusackas
6e185a32b1 fix(test): correct Action category parity in pagination test
The test fixture assigned 'Action' to odd-numbered IDs but the test
expected 'Item 2' (an even ID) on the first filtered page. Flip the
modulo so even IDs are 'Action', matching the test's "even-ids"
comment and assertion.
2026-04-24 23:06:20 -07:00
Evan Rusackas
562b63235e address review: move no-results test off page 1 too
Mirrors the pagination-reset test: navigate to page 3 before typing
the non-matching search term so this case also exercises the
pageIndex > pageCount - 1 branch (where the filtered result set
shrinks to zero rows from a later page).
2026-04-23 07:56:30 -07:00
Evan Rusackas
5c080137d4 address review: drop false-positive clear-filter assertion
The 'returns to page 1' check never left page 1 before clearing the
filter, so it would have passed even with pagination reset broken.
Pagination-reset behavior is covered end-to-end by the dedicated
'resets pagination when filter reduces data below current page' test.
2026-04-23 02:33:43 -07:00
Evan Rusackas
3c2add0c2e address review: exercise pagination reset from non-first page
Navigate to page 3 before filtering so the test actually hits the
pageIndex > pageCount - 1 branch in TableView that this PR introduces.
2026-04-23 02:32:30 -07:00
Evan Rusackas
9072faf389 fix(test): correct GenericDataType import path
The test file imported GenericDataType from '@apache-superset/core/api/core',
which does not exist. Sibling files in the same module (types.ts,
DataTableControls.tsx, SamplesPane.tsx) all import it from
'@apache-superset/core/common', which is where the enum is actually
exported from superset-core/src/common/index.ts. This was causing
TS2307 in lint-frontend and a module-not-found error in
sharded-jest-tests (5).

Updated: superset-frontend/src/explore/components/DataTablesPane/test/SingleQueryResultPane.test.tsx
2026-04-22 11:50:45 -07:00
Evan Rusackas
166ec498f3 chore: trigger CI retry 2026-04-02 18:30:05 +00:00
Evan Rusackas
3ca831bc30 fix(TableView): reset pagination when data reduces below current page
When filtering a paginated table, if the current page exceeds the new
page count, the pagination state becomes invalid. This adds automatic
reset-to-first-page behavior to TableView, matching what ListView
already does.

This is a proper fix that handles the issue at the component level
rather than requiring consumers to use React key patterns for remounting.

Fixes #31403

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-02 18:30:05 +00:00
Evan Rusackas
5448f06f76 docs: expand comment explaining key prop trade-off
Addresses CodeAnt AI review by documenting why key={filterText} is used
despite the per-keystroke remount, and why key={filterText ? 'filtered' : 'unfiltered'}
wouldn't solve the problem.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-02 18:30:05 +00:00
Evan Rusackas
779e0426dd fix: proper TypeScript types in test file
- Use GenericDataType enum instead of numeric literals
- Cast data through unknown to satisfy strict TypeScript checking

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-02 18:30:05 +00:00
Evan Rusackas
ddfcfa18c4 fix(explore): reset table pagination when filtering in View as Table
When using "View as Table" on a chart, navigating to the last page and
then searching for text that reduces the result set would cause an error:
"getPaginationModel(): currentPage shouldn't be greater than totalPages"

This fix uses React's key prop pattern to reset TableView's internal state
when the filter text changes. When the key changes, React treats it as a
new component instance and resets all internal state including pagination.

Changes:
- Add key={filterText} to TableView to reset pagination on filter change
- Add initialPageIndex={0} as explicit default
- Add explanatory comment in component
- Add tests following flat test() pattern per project guidelines

Fixes #31403

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-02 18:30:05 +00:00
3 changed files with 197 additions and 0 deletions

View File

@@ -370,3 +370,37 @@ test('should handle large datasets with pagination', () => {
expect(screen.getByRole('list')).toBeInTheDocument();
expect(screen.getByText('1-10 of 100')).toBeInTheDocument();
});
test('should reset to first page when data reduces below current page', async () => {
// Start with 30 items, 10 per page = 3 pages
const initialData = Array.from({ length: 30 }, (_, i) => ({
id: i,
age: 20 + i,
name: `Person ${i}`,
}));
const props = {
...mockedProps,
data: initialData,
pageSize: 10,
};
const { rerender } = render(<TableView {...props} />);
// Navigate to page 3 (last page)
const page3 = screen.getByRole('listitem', { name: '3' });
await userEvent.click(page3);
await waitFor(() => {
expect(screen.getByText('21-30 of 30')).toBeInTheDocument();
});
// Reduce data to only 5 items (fewer than current page would show)
const reducedData = initialData.slice(0, 5);
rerender(<TableView {...props} data={reducedData} />);
// Should reset to page 1 since page 3 no longer exists
await waitFor(() => {
expect(screen.getByText('1-5 of 5')).toBeInTheDocument();
});
});

View File

@@ -250,6 +250,29 @@ const RawTableView = ({
}
}, [initialState.sortBy, onServerPagination, serverPagination, sortBy]);
// Reset to first page when current page exceeds available pages
// (e.g., when filtering reduces the data below the current page)
const pageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
const pageCount = Math.ceil(data.length / pageSize);
useEffect(() => {
if (
withPagination &&
!serverPagination &&
!loading &&
pageIndex > pageCount - 1 &&
pageCount > 0
) {
gotoPage(0);
}
}, [
withPagination,
serverPagination,
loading,
pageIndex,
pageCount,
gotoPage,
]);
return (
<TableViewStyles {...props} ref={tableRef}>
<TableCollection

View File

@@ -0,0 +1,140 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import {
render,
screen,
waitFor,
userEvent,
} from 'spec/helpers/testing-library';
import { GenericDataType } from '@apache-superset/core/common';
import { SingleQueryResultPane } from '../components/SingleQueryResultPane';
// Mock data matching what useFilteredTableData expects: Record<string, any>[]
const mockData: Record<string, unknown>[] = Array.from(
{ length: 30 },
(_, i) => ({
id: i + 1,
name: `Item ${i + 1}`,
category: (i + 1) % 2 === 0 ? 'Action' : 'Horror',
}),
);
const defaultProps = {
// Note: The SingleQueryResultPaneProp type expects Record<string, any>[][]
// but useFilteredTableData and useTableColumns actually work with Record<string, any>[]
// This type mismatch exists in the codebase - cast through unknown to satisfy TypeScript
data: mockData as unknown as Record<string, unknown>[][],
colnames: ['id', 'name', 'category'],
coltypes: [
GenericDataType.Numeric,
GenericDataType.String,
GenericDataType.String,
],
rowcount: 30,
datasourceId: '1__table',
dataSize: 10, // 10 items per page, so 3 pages total
isVisible: true,
canDownload: true,
};
test('SingleQueryResultPane renders table and filters data', async () => {
render(<SingleQueryResultPane {...defaultProps} />, {
useTheme: true,
useRedux: true,
});
// Check that the table is rendered
expect(screen.getByText('Item 1')).toBeInTheDocument();
// Search for 'Action'
const searchInput = screen.getByPlaceholderText('Search');
await userEvent.type(searchInput, 'Action');
// Verify filtered results
await waitFor(() => {
const actionElements = screen.getAllByText('Action');
expect(actionElements.length).toBeGreaterThan(0);
expect(screen.queryByText('Horror')).not.toBeInTheDocument();
});
});
test('SingleQueryResultPane renders TableView correctly', () => {
const { container } = render(<SingleQueryResultPane {...defaultProps} />, {
useTheme: true,
useRedux: true,
});
// Verify TableView is rendered
const tableView = container.querySelector('.table-condensed');
expect(tableView).toBeInTheDocument();
});
test('SingleQueryResultPane resets pagination when filter reduces data below current page', async () => {
render(<SingleQueryResultPane {...defaultProps} />, {
useTheme: true,
useRedux: true,
});
// With 30 rows and dataSize=10, there are 3 pages. Navigate to the last page.
const page3 = screen.getByRole('listitem', { name: '3' });
await userEvent.click(page3);
// Confirm we actually moved off page 1 before filtering.
await waitFor(() => {
expect(screen.getByText('Item 21')).toBeInTheDocument();
expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
});
// Filter to a non-empty subset that collapses the dataset below page 3.
// 'Action' matches the 15 even-ids (2 pages at dataSize=10).
const searchInput = screen.getByPlaceholderText('Search');
await userEvent.type(searchInput, 'Action');
// Pagination should have reset to page 1 of the filtered set.
await waitFor(() => {
expect(screen.getByText('Item 2')).toBeInTheDocument();
expect(screen.queryByText('Item 21')).not.toBeInTheDocument();
expect(screen.queryByText('Horror')).not.toBeInTheDocument();
});
});
test('SingleQueryResultPane shows no results message when filter matches nothing', async () => {
render(<SingleQueryResultPane {...defaultProps} />, {
useTheme: true,
useRedux: true,
});
// Navigate off page 1 first so the empty-filter case exercises the
// pageIndex > pageCount - 1 branch this PR fixes.
const page3 = screen.getByRole('listitem', { name: '3' });
await userEvent.click(page3);
await waitFor(() => {
expect(screen.getByText('Item 21')).toBeInTheDocument();
});
// Search for text that doesn't exist
const searchInput = screen.getByPlaceholderText('Search');
await userEvent.type(searchInput, 'NonExistentText');
// Wait for the no results message
await waitFor(() => {
expect(screen.getByText('No results')).toBeInTheDocument();
});
});