New Feature Pagination #4704
New Feature Pagination #4704
Conversation
vinodkiran
requested review from
Copilot,
0xi4o and
HenryHengZJ
and removed request for
0xi4o
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
2) ensure page limits are passed on load 3) co-pilot review comments (n+1 query) 4)
2) ensure page limits are passed on load 3) co-pilot review comments (n+1 query) 4) refresh lists after insert/delete.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… API key services - Added check for empty entities in DocumentStoreDTO.fromEntities to return an empty array. - Updated condition in getAllDocumentStores to handle total count correctly, allowing for zero total. - Refined logic in getAllApiKeys to check for empty keys and ensure correct API key retrieval. - Adjusted UI components to safely handle potential undefined apiKeys array.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
Adds pageable list views across the app by wiring up front-end state, components, and API calls with pagination parameters and updating back-end services/controllers to accept and enforce page/limit queries.
- Integrates a reusable
TablePaginationcomponent into list views (variables, tools, evaluators, evaluations, docstores, datasets, dataset items, chatflows, agentflows, API keys, agent executions) - Updates all REST client methods to accept
{ params: { page, limit } }and back-end controllers/services to implement skip/take logic and return{ data, total } - Adds
getPageAndLimitParamshelper and updates pagination utilities to centralize query parsing
Reviewed Changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 5 comments.
Show a summary per file
| File/Pattern | Description |
|---|---|
| packages/ui/src/views/*/index.jsx | Inject pagination state, call refresh/applyFilters, render TablePagination |
| packages/ui/src/views/datasets/DatasetItems.jsx | Same as above for dataset rows |
| packages/ui/src/ui-component/pagination/TablePagination.jsx | New component driving pagination UI and events |
| packages/ui/src/api/*.js | Updated every getAllX to pass { params } |
| packages/server/src/utils/pagination.ts | New helper to extract/validate page & limit |
| packages/server/src/services/*/index.ts | Extended services to accept page/limit, use skip/take, return count |
| packages/server/src/controllers/*/index.ts | Wire controllers to getPageAndLimitParams and return paged responses |
Comments suppressed due to low confidence (1)
packages/ui/src/views/evaluators/index.jsx:17
- The
useErrorimport is unused in this file; consider removing it to reduce dead code.
import TablePagination, { DEFAULT_ITEMS_PER_PAGE } from '@/ui-component/pagination/TablePagination'
| {variables.filter(filterVariables).map((variable, index) => ( | ||
| <StyledTableRow | ||
| key={index} |
There was a problem hiding this comment.
[nitpick] Avoid using the array index as the React key; use a unique identifier (e.g., variable.id) to prevent rendering issues when list items change order.
Copilot uses AI. Check for mistakes.
| import { useTheme } from '@mui/material/styles' | ||
|
|
||
| // project imports | ||
| import ErrorBoundary from '@/ErrorBoundary' | ||
| import { useError } from '@/store/context/ErrorContext' |
There was a problem hiding this comment.
The useError hook is imported but never used; please remove unused imports.
| import { useError } from '@/store/context/ErrorContext' |
Copilot uses AI. Check for mistakes.
| @@ -26,9 +26,11 @@ import ConfirmDialog from '@/ui-component/dialog/ConfirmDialog' | |||
| import AddEditDatasetRowDialog from './AddEditDatasetRowDialog' | |||
| import UploadCSVFileDialog from '@/views/datasets/UploadCSVFileDialog' | |||
| import ErrorBoundary from '@/ErrorBoundary' | |||
| import { useError } from '@/store/context/ErrorContext' | |||
There was a problem hiding this comment.
Unused useError import detected; remove it to clean up the code.
| import { useError } from '@/store/context/ErrorContext' |
Copilot uses AI. Check for mistakes.
| </FormControl> | ||
| </Box> | ||
| {totalItems > 0 && ( | ||
| <Typography variant='body'> |
There was a problem hiding this comment.
Material-UI doesn’t define a body typography variant; switch to body1 or body2 to avoid warnings.
| <Typography variant='body'> | |
| <Typography variant='body2'> |
Copilot uses AI. Check for mistakes.
| <MenuItem value={12}>12</MenuItem> | ||
| <MenuItem value={24}>24</MenuItem> | ||
| <MenuItem value={48}>48</MenuItem> | ||
| <MenuItem value={100}>100</MenuItem> |
There was a problem hiding this comment.
[nitpick] Hard-coded page-size options limit flexibility; consider making the options configurable via props or a shared constant.
| <MenuItem value={12}>12</MenuItem> | |
| <MenuItem value={24}>24</MenuItem> | |
| <MenuItem value={48}>48</MenuItem> | |
| <MenuItem value={100}>100</MenuItem> | |
| {pageSizeOptions.map((option) => ( | |
| <MenuItem key={option} value={option}> | |
| {option} | |
| </MenuItem> | |
| ))} |
Copilot uses AI. Check for mistakes.
Adding Pagination Support (Table view and Card view) for the following
The following will NOT be taken up in this PR and will be handled separately shortly.