feat: add list accounts with pagination #24
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR implements a paginated accounts listing feature restricted to users with GAME_MASTER or ADMIN permissions. The feature allows authorized users to view and navigate through account records via the /admin/accounts/list route.
Key Changes:
- Added backend API endpoint with permission checks for listing accounts with pagination
- Implemented frontend UI component with table display and pagination controls
- Added role-based navigation menu item visible only to authorized users
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/sections/list_acconts/index.tsx | New component implementing the accounts list UI with table and pagination |
| apps/web/src/routes/_auth/list-accounts/index.lazy.tsx | Route configuration for incorrect path /list-accounts |
| apps/web/src/routes/_auth/admin/accounts/list/index.lazy.tsx | Route configuration for correct path /admin/accounts/list |
| apps/web/src/routeTree.gen.ts | Generated route tree updates for both new routes |
| apps/web/src/layout/Navigation/index.tsx | Conditional rendering of admin menu based on user role |
| apps/web/src/components/Menu/index.tsx | Refactored to accept dynamic menu items as props |
| apps/web/src/components/Menu/Item/index.tsx | Accessibility improvement changing div to button, icon imports moved |
| apps/web/.env.example | Environment configuration template added |
| apps/api/src/shared/schemas/ListAccounts.ts | Schema definition for account list response |
| apps/api/src/presentation/v1/routes/index.ts | Router registration for admin routes |
| apps/api/src/presentation/v1/routes/admin/index.ts | Admin router namespace setup |
| apps/api/src/presentation/v1/routes/admin/accounts/list/index.ts | List accounts endpoint with GAME_MASTER permission requirement |
| apps/api/src/presentation/v1/routes/admin/accounts/index.ts | Admin accounts router setup |
| apps/api/src/infra/di/tokens.ts | Dependency injection token for ListAccountsUseCase |
| apps/api/src/infra/di/containers/usecases.ts | UseCase registration in DI container |
| apps/api/src/domain/repositories/account/index.ts | Repository method for fetching paginated accounts |
| apps/api/src/application/usecases/session/info/contract.ts | Session contract updated to include account type |
| apps/api/src/application/usecases/factory.ts | UseCase factory updated to resolve ListAccountsUseCase |
| apps/api/src/application/usecases/account/listAccounts/index.ts | UseCase implementation for listing accounts |
| apps/api/src/application/usecases/account/listAccounts/contract.ts | Contract schema for list accounts use case |
| apps/api/src/application/usecases/account/index.ts | Export for listAccounts use case |
| apps/api/src/application/services/session/index.ts | Session service updated to return account type |
| apps/api/src/application/services/accounts/index.ts | Accounts service method for listing accounts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| to="/" | ||
| className={"font-bold text-blue-900 underline"} | ||
| > | ||
| Detailis |
There was a problem hiding this comment.
Corrected spelling of 'Detailis' to 'Details'.
| Detailis | |
| Details |
| <Container | ||
| title="Accounts" | ||
| actions={ | ||
| <Tooltip content="Refresh History"> |
There was a problem hiding this comment.
The tooltip content 'Refresh History' is misleading. This button refreshes the accounts list, not history. Consider changing to 'Refresh Accounts' or 'Refresh List'.
| > | ||
| <td className="border border-septenary p-1 text-center"> | ||
| <span className="font-bold text-secondary text-sm"> | ||
| {index + 1}. |
There was a problem hiding this comment.
The row number calculation is incorrect for paginated data. It shows 1-10 on every page instead of the global position. Use (pagination.page - 1) * pagination.size + index + 1 to display correct row numbers across pages.
| {index + 1}. | |
| {(pagination.page - 1) * pagination.size + index + 1}. |
| import { createLazyFileRoute } from "@tanstack/react-router"; | ||
| import { ListAccounts } from "@/sections/list_acconts"; | ||
|
|
||
| export const Route = createLazyFileRoute("/_auth/list-accounts/")({ | ||
| component: RouteComponent, | ||
| }); | ||
|
|
||
| function RouteComponent() { | ||
| return <ListAccounts />; | ||
| } |
There was a problem hiding this comment.
This route file creates an unnecessary duplicate path /list-accounts that serves the same component as /admin/accounts/list. According to the PR description, only /admin/accounts/list should be accessible. This file should be removed to avoid confusion and maintain a single source of truth.
| import { createLazyFileRoute } from "@tanstack/react-router"; | |
| import { ListAccounts } from "@/sections/list_acconts"; | |
| export const Route = createLazyFileRoute("/_auth/list-accounts/")({ | |
| component: RouteComponent, | |
| }); | |
| function RouteComponent() { | |
| return <ListAccounts />; | |
| } |
| menus={[{ label: "Updates", to: "/", hot: true }]} | ||
| /> | ||
| {items.map((item) => ( | ||
| <MenuItem label={item.label} icon={item.icon} menus={item.menus} /> |
There was a problem hiding this comment.
Missing key prop in mapped MenuItem component. Add key={item.label} or a unique identifier to avoid React reconciliation issues.
| <MenuItem label={item.label} icon={item.icon} menus={item.menus} /> | |
| <MenuItem key={item.label} label={item.label} icon={item.icon} menus={item.menus} /> |
| const page = opts?.pagination.page ?? 1; | ||
| const size = opts?.pagination.size ?? 10; | ||
|
|
||
| const [storeHistory, total] = await Promise.all([ |
There was a problem hiding this comment.
The variable name storeHistory is misleading and inconsistent with the function's purpose. This method lists accounts, not store history. Rename to accounts for clarity.
| ]); | ||
|
|
||
| return { | ||
| storeHistory, |
There was a problem hiding this comment.
The returned property name storeHistory is misleading and inconsistent with the function's purpose. This method lists accounts, not store history. Rename to accounts for clarity.
1 participant
Description
Implements the accounts listing feature with pagination on both backend and frontend, restricted to users with elevated permissions (GAME_MASTER / ADMIN).
Behaviour
Actual
Expected
Users with GAME_MASTER or ADMIN permission can:
Users without permission cannot see the admin menu nor access the route.
Fixes #issuenumber
Type of change
Checklist