codeant-ai-for-open-source[bot] commented on code in PR #42136:
URL: https://github.com/apache/superset/pull/42136#discussion_r3603362830
##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -258,6 +258,7 @@ export default typedMemo(function DataTable<D extends
object>({
sortTypes,
autoResetGlobalFilter: !isEqual(columnNames, previousColumnNames),
autoResetSortBy: !isEqual(columnNames, previousColumnNames),
+ autoResetPage: false,
Review Comment:
**Suggestion:** Setting page auto-reset to an unconditional false keeps
stale page indexes after real dataset changes (for example when filters/search
reduce total rows), so the table can remain on an out-of-range page and render
an empty body even though matching rows exist. Make page reset conditional
(aligned with the existing column-change reset logic) or explicitly clamp/reset
the current page when row count/page count shrinks. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ TableChart search shows empty page despite matching rows.
- ⚠️ Client-side pagination highlights no active page after filtering.
- ⚠️ Users must manually reset to first page after filters.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Table chart in Superset with client-side pagination and
search enabled
(TableChart component at
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:166-203,
where
`includeSearch`, `pageSize`, and `serverPagination=false` are taken from
props; see lines
182-185).
2. Render the chart so that it has multiple pages of data (e.g., `rowCount`
and `data`
large enough) and confirm that `DataTable` is used for rendering with
`pageSize > 0` and
`serverPagination` wired through the props (TableChart.tsx:113-151 shows the
`<DataTable>`
invocation, passing `pageSize`, `rowCount`, `data`, and `serverPagination`).
3. In the running UI, navigate to a later page (e.g., page 3) using the
pagination
control, which updates `pageIndex` inside `useTable` (DataTable.tsx:228-247
shows `state:
{ pageIndex, pageSize, ... }`) and passes that into the pagination component
via
`resultCurrentPage = pageIndex` (DataTable.tsx:194-197) and
`currentPage={resultCurrentPage}` (SimplePagination usage at
DataTable.tsx:120-127).
4. While still on that later page, type a search term in the Search input
(SearchInput
component in TableChart.tsx:47-60, wired into `GlobalFilter` in
DataTable.tsx:97-109),
which for client-side mode calls `setGlobalFilter(query)`
(DataTable.tsx:33-41) and
reduces the number of `rows` before pagination (`rows` comes from `useTable`
at
DataTable.tsx:228-235, with filtering applied by `defaultGlobalFilter` at
DataTable.tsx:215-224).
5. After filtering, the total number of matching `rows` shrinks so that
there is now only
one page of results (so `pageCount` from `useTable` at DataTable.tsx:235-237
becomes 1),
but because `autoResetPage` is hardcoded to `false` in the `useTable` options
(DataTable.tsx:251-260, specifically `autoResetPage: false`), the internal
`state.pageIndex` remains at the previous value (e.g., 2 for page 3).
6. With `pageIndex` still pointing past the last page, `usePagination` in
react-table
computes an empty `page` array (no rows in the slice for that index), so
DataTable’s
render logic falls into the “no results” branch (`page && page.length > 0 ?
... :` at
DataTable.tsx:140-159) and renders the “No matching records
found”/`noResults` message
even though filtered rows exist (`rowsRef` and `onFilteredDataChange` still
see those rows
at DataTable.tsx:19-31).
7. At the same time, the `SimplePagination` control is driven by `pageCount`
and
`currentPage=pageIndex` (DataTable.tsx:194-197 and 120-127) and its
implementation
(Pagination.tsx:21-90) uses `pageCount` to generate items; when `pageCount`
shrinks (e.g.,
total=1) and `currentPage` remains out-of-range, it renders a single page
button (0) with
no “active” class (`className={currentPage === item ? 'active' : undefined}`
at
Pagination.tsx:95-100), leaving the UI in an inconsistent state: the table
body shows a
no-results row while the pagination still indicates there is data.
8. There is no compensating logic that clamps or resets `pageIndex` when
`resultsSize`/`pageCount` shrink (DataTable.tsx:179-214 only adjusts
`pageSize`, and
TableChart.tsx:45-52 explicitly resets `currentPage` only for
serverPagination searches),
so once a user filters or searches while on a later page in client-side
mode, the page
index can stay stale and the UI remains stuck on an effectively “invalid”
page until the
user manually clicks back to page 1.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=41d93aaf4d9d442c84fe4479d8c847c3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=41d93aaf4d9d442c84fe4479d8c847c3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
**Line:** 261:261
**Comment:**
*Incorrect Condition Logic: Setting page auto-reset to an unconditional
false keeps stale page indexes after real dataset changes (for example when
filters/search reduce total rows), so the table can remain on an out-of-range
page and render an empty body even though matching rows exist. Make page reset
conditional (aligned with the existing column-change reset logic) or explicitly
clamp/reset the current page when row count/page count shrinks.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42136&comment_hash=193214c3cfc572a40da758d9d3003bb140e726a2f966d47a2283446c8c84dad0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42136&comment_hash=193214c3cfc572a40da758d9d3003bb140e726a2f966d47a2283446c8c84dad0&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
