feat(ui): add pagination to applications list - #1587
Conversation
2265016 to
07fe3d8
Compare
There was a problem hiding this comment.
Pull request overview
Adds frontend pagination for the Applications list to align with backend GET /apps pagination support and match the existing Packages list pagination behavior, using totalCount from the API response.
Changes:
- Extend
API.getApplications()to accept optional query params and build a query string. - Update
ApplicationsStoreto track applications pagination state (page,perPage) andapplicationsTotalCount, and refetch on create/delete with page adjustments. - Integrate MUI
TablePaginationinto the Applications list UI and wire it to store pagination state.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/stores/ApplicationsStore.ts | Adds applications pagination/query state + totalCount handling; refetches list on pagination/create/delete. |
| frontend/src/components/Applications/ApplicationList.tsx | Adds TablePagination controls and connects page changes to the store. |
| frontend/src/api/API.ts | Adds optional query params support for GET /apps requests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
frontend/src/components/Applications/ApplicationList.tsx:182
TablePaginationis rendered even when there are no applications (countis 0), which results in a confusing “0–0 of 0” footer. Other lists avoid showing pagination when the total is 0 (e.g.,frontend/src/components/Packages/List.tsxshows an empty state instead). Consider only rendering pagination whenapplicationsTotalCount > 0.
<TablePagination
rowsPerPageOptions={[]}
component="div"
count={props.applicationsTotalCount || 0}
rowsPerPage={props.applicationsQueryParams?.perPage || 10}
page={props.applicationsQueryParams?.page || 0}
backIconButtonProps={{
'aria-label': t('frequent|previous_page'),
}}
nextIconButtonProps={{
'aria-label': t('frequent|next_page'),
}}
onPageChange={props.handleChangePage || (() => { })}
/>
frontend/src/stores/ApplicationsStore.ts:50
setApplicationsQueryParamsupdates the store’sapplicationsQueryParamsbut doesn’t emit a change until the network request completes, soTablePaginationremains controlled by the oldpagevalue and won’t visually update immediately after a click (especially noticeable on slow networks). Emit a change right after updating the params so the UI can reflect the new page selection immediately.
setApplicationsQueryParams(params: ApplicationsQueryParams) {
this.applicationsQueryParams = params;
this.getApplications();
}
07fe3d8 to
0d470e2
Compare
0d470e2 to
c81a673
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (5)
frontend/src/stores/ApplicationsStore.ts:190
deleteApplicationdecrementsthis.applicationsQueryParams.pageby mutating the existing object. Prefer an immutable update to avoid shared references and keep the pagination state update explicit.
return API.deleteApplication(applicationID).then(() => {
const isLastItemOnPage = this.applications.length === 1;
if (isLastItemOnPage && this.applicationsQueryParams.page > 0) {
this.applicationsQueryParams.page -= 1;
}
frontend/src/stores/ApplicationsStore.ts:134
createApplicationresets pagination by mutatingthis.applicationsQueryParams.pagein-place. Prefer replacing the query params object to avoid shared-reference mutations and keep pagination state updates explicit.
) {
await API.createApplication(data, clonedApplication);
this.applicationsQueryParams.page = 0;
frontend/src/stores/ApplicationsStore.ts:40
getApplicationsQueryParams()returns the store's internalapplicationsQueryParamsobject by reference. That allows callers to mutate store state without going through a setter (and without any clear change notification), which can lead to subtle bugs.
This issue also appears in the following locations of the same file:
- line 132
- line 186
getApplicationsQueryParams() {
return this.applicationsQueryParams;
}
frontend/src/components/Applications/ApplicationList.tsx:39
- Pagination wiring is new here (updating store query params on
onPageChange), but there are no component-level tests validating it (e.g., that clicking next/prev callssetApplicationsQueryParamsand thatcount/pageprops are derived from store state). The repo already uses Vitest + RTL for similar list components (seefrontend/src/__tests__/Common/PackagesList.spec.tsx).
function handleChangePage(
_event: React.MouseEvent<HTMLButtonElement, MouseEvent> | null,
newPage: number
) {
applicationsStore().setApplicationsQueryParams({ ...applicationsQueryParams, page: newPage });
}
frontend/src/api/API.ts:47
getApplicationsaccepts an untypedqueryOptionsindex signature (any), which makes it easy to accidentally send unexpected params/values and hides type issues when buildingURLSearchParams. Other API methods in this file use explicit option types (e.g.getChannelFloorsuses{ page?: number; perpage?: number }around API.ts:216). Consider typing this to the supported pagination params and stringify values when appending.
static getApplications(queryOptions?: {
[key: string]: any;
}): Promise<WithCount<{ applications: Application[] }>> {
const params = new URLSearchParams();
if (queryOptions) {
GET /apps already supports page/perpage params, but the frontend fetched all applications unpaginated silently truncating results past the backend's default page size with no indication to the user. Mirrors the existing Packages list pagination pattern (TablePagination, page-indexed store state). Fixes flatcar#548 Signed-off-by: Mostafa Abdelglel <14712022100624@stud.cu.edu.eg>
c81a673 to
754647b
Compare
Fixes #548
Add pagination to applications list
Summary
GET /appssupports pagination (page, perpage), but the frontend ignored both, fetching applications with no params silently truncating results past the backend's default page size. This adds pagination to the Applications list, mirroring the existing Packages list (GET /apps/<appID>/packages) for UI and code consistency.Changes
API.getApplications()now accepts page/perpage query paramsApplicationsStore: addedapplicationsQueryParamsandapplicationsTotalCountstate;createApplicationresets to page 0 on create;deleteApplicationsteps back a page when the last item on a non-first page is removedApplicationList: integrated MUITablePagination, wired to the storeTesting done
npm run lint,npx tsc -bnpm run test79/79 passing (no existing tests covered this area)?page=x&perpage=y, pagination controls render/update correctly, deleting the last item on a non-first page steps back to the previous page