-
-
Notifications
You must be signed in to change notification settings - Fork 49
feat: container listing with pagination, search, and sort #367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: container listing with pagination, search, and sort #367
Conversation
WalkthroughBackend container listing was refactored to parse query params, build Docker filters, summarize then paginate/search/sort, and append per-page details. Frontend and RTK updates add server-side pagination, sorting, debounced search, table actions, and a table/card page view with pagination controls. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Containers Page (Table/Card)
participant Hook as useContainerList
participant RTK as Redux getContainers
participant API as HTTP /containers
participant Ctl as ContainerController
participant Dock as DockerService
UI->>Hook: user sets page/search/sort
Hook->>RTK: getContainers({page,page_size,search,sort_by,sort_order})
RTK->>API: GET /containers?...
API->>Ctl: handle request
rect rgba(220,240,255,0.35)
Ctl->>Ctl: parseContainerListParams
Ctl->>Ctl: buildDockerFilters
Ctl->>Dock: ListContainers({All:true,Filters})
Dock-->>Ctl: []container.Summary
Ctl->>Ctl: summarizeContainers -> []ContainerListRow
Ctl->>Ctl: applySearchSortPaginate -> pageRows, totalCount
alt pageRows need details
loop per row in pageRows
Ctl->>Ctl: appendContainerInfo (inspect)
end
end
Ctl-->>API: { containers, total_count, page, page_size }
end
API-->>RTK: response
RTK-->>Hook: data
Hook-->>UI: containers, totals, paging, sort state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
view/app/containers/components/actions.tsx (1)
8-8
: Circular dependency risk: actions.tsx imports Action from card.tsxIf card.tsx imports ContainerActions from actions.tsx (it uses it), this creates a module cycle that can break SSR/HMR. Move Action to a shared types file and import from there.
Apply this diff and add a shared types module:
- import { Action } from './card'; + import { Action } from './types';Create view/app/containers/components/types.ts:
export enum Action { START = 'start', STOP = 'stop', REMOVE = 'remove', }Update card.tsx to import { Action } from './types' (not shown here).
🧹 Nitpick comments (24)
view/app/containers/components/actions.tsx (1)
74-82
: Add accessible labels to action buttonsImprove a11y and tooltips.
- <Button + <Button variant={ghostVariant} size={iconSize} disabled={isProtected} - onClick={(e) => onClickHandler(e, Action.STOP)} + aria-label="Pause container" + title="Pause container" + onClick={(e) => onClickHandler(e, Action.STOP)} > <Pause className={iconStyle} /> </Button>- <Button + <Button variant={ghostVariant} size={iconSize} disabled={isProtected} - onClick={(e) => onClickHandler(e, Action.START)} + aria-label="Start container" + title="Start container" + onClick={(e) => onClickHandler(e, Action.START)} > <Play className={iconStyle} /> </Button>Also applies to: 85-93
api/internal/features/deploy/docker/init.go (1)
99-103
: Fix docstring: remove “sorted by ID” and “panics” claimsImplementation neither sorts nor panics. Align comments or add sorting explicitly.
-// ListAllContainers returns a list of all containers running on the host, along with their -// IDs, names, and statuses. The returned list is sorted by container ID in ascending order. -// -// If an error occurs while listing the containers, it panics with the error. +// ListAllContainers returns all containers (Docker default ordering). +// Errors are returned to the caller.api/internal/features/container/types/container_types.go (2)
112-121
: Document and validate paging/sorting paramsClarify 1-based vs 0-based page, default/max page_size, allowed sort_by fields, and sort_order values ("asc"/"desc"). Consider input validation upstream and/or enum types.
+// ContainerListParams defines server-side paging, search, and sorting. +// Page is 1-based; PageSize defaults to 20 (max 100); SortOrder: "asc"|"desc". type ContainerListParams struct {
123-131
: created type inconsistency (string vs int64) across container shapesContainer.Created is string (Line 9), ContainerListRow.Created is int64. This mismatches client expectations and sorting. Either align both to int64 (Unix seconds/ms) or both to RFC3339 strings; document the contract.
view/app/containers/page.tsx (3)
117-137
: Localize UI strings and use correct key for search labelUse i18n keys instead of hardcoded “Page size”, and a containers-specific search label.
- <SearchBar + <SearchBar searchTerm={searchInput} handleSearchChange={(e) => setSearchInput(e.target.value)} - label={t('common.searchFiles')} + label={t('containers.search')} /> ... - <TypographyMuted>Page size</TypographyMuted> + <TypographyMuted>{t('common.page_size')}</TypographyMuted>
149-158
: Add accessible label to view-mode toggleAids screen readers and clarifies intent.
- <Button + <Button variant="outline" size="icon" + aria-label={viewMode === 'table' ? 'Switch to card view' : 'Switch to table view'} + title={viewMode === 'table' ? 'Card view' : 'Table view'} onClick={() => {
195-200
: Localize container count stringPrefer i18n/plurals over hardcoded English.
- <TypographyMuted>{totalCount} containers</TypographyMuted> + <TypographyMuted>{t('containers.count', { count: totalCount })}</TypographyMuted>view/redux/services/container/containerApi.ts (3)
26-32
: Type: expose all supported filters for parity with backend (optional).Backend supports status/name/image filters; exposing them here keeps types aligned and prevents ad-hoc any casts downstream.
export type ContainerListParams = { page: number; page_size: number; search?: string; sort_by?: 'name' | 'status'; sort_order?: 'asc' | 'desc'; + status?: string; + name?: string; + image?: string; };
43-47
: Avoid sending undefined/empty query params.Build params conditionally to keep URLs clean and prevent backend from treating empty strings as filters.
- query: ({ page, page_size, search, sort_by, sort_order }) => ({ + query: ({ page, page_size, search, sort_by, sort_order, status, name, image }) => { + const params: Record<string, string | number> = { page, page_size }; + if (search) params.search = search; + if (sort_by) params.sort_by = sort_by; + if (sort_order) params.sort_order = sort_order; + if (status) params.status = status; + if (name) params.name = name; + if (image) params.image = image; + return { url: CONTAINERURLS.GET_CONTAINERS, - method: 'GET', - params: { page, page_size, search, sort_by, sort_order } - }), + method: 'GET', + params + }; + }),
49-56
: Response typing: include echoed metadata (optional).Backend returns sort/search echo fields; widen typing so consumers can use them without casting.
- transformResponse: (response: { - data: { - containers: Container[]; - total_count: number; - page: number; - page_size: number; - }; - }) => { + transformResponse: (response: { + data: { + containers: Container[]; + total_count: number; + page: number; + page_size: number; + sort_by?: 'name' | 'status' | 'created'; + sort_order?: 'asc' | 'desc'; + search?: string; + status?: string; + name?: string; + image?: string; + }; + }) => { return response.data; }api/internal/features/container/controller/list_containers.go (2)
206-216
: Avoid O(n*m) scan for ports; index summaries by ID.Precompute a map to reduce per-page enrichment from O(n*m) to O(n).
-f or _, s := range summaries { - if s.ID == r.ID { - for _, p := range s.Ports { - cd.Ports = append(cd.Ports, containertypes.Port{ - PrivatePort: int(p.PrivatePort), - PublicPort: int(p.PublicPort), - Type: p.Type, - }) - } - break - } -} +// build once per request (move above loop if acceptable) +summaryByID := make(map[string]container.Summary, len(summaries)) +for _, s := range summaries { + summaryByID[s.ID] = s +} +if s, ok := summaryByID[r.ID]; ok { + for _, p := range s.Ports { + cd.Ports = append(cd.Ports, containertypes.Port{ + PrivatePort: int(p.PrivatePort), + PublicPort: int(p.PublicPort), + Type: p.Type, + }) + } +}
70-75
: Validate sort_by against allowed fields.Normalize invalid values to a default to avoid surprising behavior.
if sortBy == "" { sortBy = "name" } + if sortBy != "name" && sortBy != "status" && sortBy != "created" { + sortBy = "name" + }view/app/containers/hooks/use-container-list.ts (3)
74-78
: Use server page_size for totalPages.If backend clamps page_size, UI stays consistent by deriving from effectiveData.page_size.
- const totalCount = effectiveData?.total_count ?? 0; - const totalPages = Math.max(1, Math.ceil(totalCount / pageSize)); + const totalCount = effectiveData?.total_count ?? 0; + const serverPageSize = effectiveData?.page_size ?? pageSize; + const totalPages = Math.max(1, Math.ceil(totalCount / serverPageSize));
79-86
: Reset to first page on sort change (UX).Sorting while on deep pages can yield empty screens; go back to page 1.
const handleSort = (field: 'name' | 'status') => { if (sortBy === field) { setSortOrder((prev) => (prev === 'asc' ? 'desc' : 'asc')); } else { setSortBy(field); setSortOrder('asc'); } + setPage(1); };
67-73
: Optionally sync page with server echo.If backend adjusts page (e.g., page > totalPages), reflect it to avoid UI desync.
useEffect(() => { if (data) { setLastData(data); if (!initialized) setInitialized(true); } }, [data]); + +useEffect(() => { + if (effectiveData?.page && effectiveData.page !== page) { + setPage(effectiveData.page); + } +}, [effectiveData?.page]);view/app/containers/components/table.tsx (9)
18-21
: Use a type-only import for Action (and disambiguate the name).Prevents bundling a value and avoids confusion with RBAC’s Action type. Alias locally to clarify intent.
-import { Action } from './card'; +import type { Action as ContainerAction } from './card'; @@ - onAction?: (id: string, action: Action) => void; + onAction?: (id: string, action: ContainerAction) => void;Also applies to: 35-35
24-36
: sortOrder prop is unused in the UI.Either reflect it (icon/aria) or drop it to avoid dead props.
49-55
: Make the sortable Name header keyboard-accessible and expose sort state.Move click to a button and set aria-sort; leverage sortOrder.
-<TableHead onClick={() => onSort && onSort('name')} className={`select-none ${onSort ? 'cursor-pointer' : ''}`}> - <div className="flex items-center gap-1"> +<TableHead + aria-sort={sortBy === 'name' ? (sortOrder === 'asc' ? 'ascending' : 'descending') : 'none'} + className="select-none" +> + <button + type="button" + onClick={() => onSort?.('name')} + disabled={!onSort} + className={`flex items-center gap-1 ${onSort ? 'cursor-pointer' : 'cursor-default'}`} + > <TypographySmall>{t('dashboard.containers.table.headers.name')}</TypographySmall> <ArrowUpDown className={`h-3 w-3 ${sortBy === 'name' ? 'opacity-100' : 'opacity-40'}`} /> - <span className="sr-only">Sort</span> -</div> + <span className="sr-only">Sort</span> + </button> </TableHead>
59-64
: Same accessibility and sort-state treatment for Status header.Replicate the button/aria-sort pattern.
-<TableHead onClick={() => onSort && onSort('status')} className={`select-none ${onSort ? 'cursor-pointer' : ''}`}> - <div className="flex items-center gap-1"> +<TableHead + aria-sort={sortBy === 'status' ? (sortOrder === 'asc' ? 'ascending' : 'descending') : 'none'} + className="select-none" +> + <button + type="button" + onClick={() => onSort?.('status')} + disabled={!onSort} + className={`flex items-center gap-1 ${onSort ? 'cursor-pointer' : 'cursor-default'}`} + > <TypographySmall>{t('dashboard.containers.table.headers.status')}</TypographySmall> <ArrowUpDown className={`h-3 w-3 ${sortBy === 'status' ? 'opacity-100' : 'opacity-40'}`} /> - <span className="sr-only">Sort</span> - </div> + <span className="sr-only">Sort</span> + </button> </TableHead>
72-76
: Localize the “Actions” header.Keep consistency with other headers.
-<TypographySmall>Actions</TypographySmall> +<TypographySmall>{t('dashboard.containers.table.headers.actions')}</TypographySmall>
134-138
: Redundant stopPropagation.ContainerActions already stops propagation; the TableCell handler is unnecessary.
-{onAction && ( - <TableCell onClick={(e) => e.stopPropagation()}> - <ContainerActions container={container} onAction={onAction} /> - </TableCell> -)} +{onAction && ( + <TableCell> + <ContainerActions container={container} onAction={onAction} /> + </TableCell> +)}
143-147
: Empty-state colSpan should match column count.Avoids semantic/table layout mismatch when Actions column is hidden.
-<TableCell colSpan={7} className="text-center py-6"> +<TableCell colSpan={onAction ? 7 : 6} className="text-center py-6">
86-89
: Safer timestamp parse.Number(...) avoids radix quirks; Date will handle integers correctly.
- new Date(parseInt(container.created) * 1000) + new Date(Number(container.created) * 1000)
112-117
: Prefer stable keys over index in ports map.Avoids re-render issues on list reorder.
-{container.ports.slice(0, 2).map((port, index) => ( - <TypographySmall key={index}> +{container.ports.slice(0, 2).map((port) => ( + <TypographySmall key={`${port.private_port}-${port.public_port ?? ''}`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
api/api/versions.json
(1 hunks)api/internal/features/container/controller/list_containers.go
(1 hunks)api/internal/features/container/types/container_types.go
(1 hunks)api/internal/features/deploy/docker/init.go
(2 hunks)view/app/containers/components/actions.tsx
(3 hunks)view/app/containers/components/table.tsx
(3 hunks)view/app/containers/hooks/use-container-list.ts
(4 hunks)view/app/containers/page.tsx
(5 hunks)view/redux/services/container/containerApi.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
api/internal/features/container/types/container_types.go (1)
api/internal/features/container/types/image.go (1)
Image
(3-12)
view/redux/services/container/containerApi.ts (1)
view/redux/base-query.ts (1)
baseQueryWithReauth
(57-90)
api/internal/features/container/controller/list_containers.go (3)
api/internal/features/container/controller/init.go (1)
ContainerController
(15-21)api/internal/types/response.go (1)
Response
(5-20)api/internal/features/container/types/container_types.go (7)
ContainerListParams
(112-121)ContainerListRow
(123-131)Container
(3-17)HostConfig
(40-44)Port
(19-23)Mount
(25-30)Network
(32-38)
view/app/containers/hooks/use-container-list.ts (1)
view/redux/services/container/containerApi.ts (1)
Container
(5-24)
view/app/containers/page.tsx (3)
view/components/ui/search-bar.tsx (1)
SearchBar
(11-26)view/components/ui/typography.tsx (2)
TypographyMuted
(73-77)TypographyH2
(9-15)view/app/containers/components/card.tsx (1)
ContainerCard
(22-58)
view/app/containers/components/table.tsx (3)
view/lib/rbac.ts (1)
Action
(22-22)view/components/ui/typography.tsx (1)
TypographySmall
(61-71)view/app/containers/components/actions.tsx (1)
ContainerActions
(26-68)
🔇 Additional comments (6)
api/api/versions.json (1)
6-6
: Confirm timezone/format expectations for release_dateThe value now includes a +05:30 offset while end_of_life uses Z (UTC). If consumers assume UTC or uniform formatting, standardize to Z or document the offset.
api/internal/features/deploy/docker/init.go (1)
25-29
: No action required: DockerRepository change won’t break anything
There are no mocks or fakes of DockerRepository in the codebase, and DockerService already implements ListContainers, so this interface expansion introduces no breakage.Likely an incorrect or invalid review comment.
api/internal/features/container/controller/list_containers.go (2)
39-54
: Response envelope looks good.Structured data plus echoed params will help the UI remain consistent.
21-26
: Ignore: DockerService.ListContainers correctly uses container.ListOptions and []container.Summary from the Docker SDK for ContainerList; no changes needed.Likely an incorrect or invalid review comment.
view/app/containers/hooks/use-container-list.ts (1)
27-30
: Refetch behavior looks good.Passing refetchOnMountOrArgChange keeps the list fresh across param changes.
view/app/containers/components/table.tsx (1)
105-107
: Status/state mismatch: verify intended source.Color is derived from container.status but label shows container.state. Align both to the same field.
Possible fix (if status is canonical):
-<Badge className={getStatusColor(container.status)}> - {container.state || 'Unknown'} -</Badge> +<Badge className={getStatusColor(container.status)}> + {container.status || 'Unknown'} +</Badge>
…se stableSlice / remove muted titles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (4)
api/internal/features/deploy/docker/init.go (1)
99-112
: Resolved prior panic-on-error feedback.
ContainerList
errors are now propagated instead of panicking. Thanks for closing this out.api/internal/features/container/controller/list_containers.go (3)
137-187
: Comparator fix is correct and stable.Using
SliceStable
and directional comparisons avoids the previous “!less” pitfall.
57-96
: Cap page_size to prevent heavy per-request inspect loops.Unbounded
page_size
can cause O(page_size) Docker inspect calls. Clamp to a sane max (e.g., 100).pageSize, _ := strconv.Atoi(pageSizeStr) if pageSize < 1 { pageSize = 10 } + if pageSize > 100 { + pageSize = 100 + }I can also add an integration test ensuring the cap is honored; say the API returns at most 100 items when
page_size
> 100.
189-248
: Fix nil-pointer panics on Inspect results (HostConfig/NetworkSettings may be nil).Several dereferences can panic depending on Docker daemon/config. Guard them and improve lookup performance by indexing summaries by ID.
-func (c *ContainerController) appendContainerInfo(pageRows []containertypes.ContainerListRow, summaries []container.Summary) []containertypes.Container { - result := make([]containertypes.Container, 0, len(pageRows)) - for _, r := range pageRows { +func (c *ContainerController) appendContainerInfo(pageRows []containertypes.ContainerListRow, summaries []container.Summary) []containertypes.Container { + result := make([]containertypes.Container, 0, len(pageRows)) + // Index summaries by ID for O(1) lookup + sumIndex := make(map[string]container.Summary, len(summaries)) + for _, s := range summaries { + sumIndex[s.ID] = s + } + for _, r := range pageRows { info, err := c.dockerService.GetContainerById(r.ID) if err != nil { - c.logger.Log(logger.Error, "Error inspecting container", r.ID) + c.logger.Log(logger.Error, "Error inspecting container", r.ID+": "+err.Error()) continue } - cd := containertypes.Container{ + ip := "" + if info.NetworkSettings != nil { + ip = info.NetworkSettings.IPAddress + } + hostCfg := containertypes.HostConfig{} + if info.HostConfig != nil { + hostCfg = containertypes.HostConfig{ + Memory: info.HostConfig.Memory, + MemorySwap: info.HostConfig.MemorySwap, + CPUShares: info.HostConfig.CPUShares, + } + } + cd := containertypes.Container{ ID: r.ID, Name: r.Name, Image: r.Image, Status: r.Status, State: r.State, Created: info.Created, Labels: r.Labels, Command: "", - IPAddress: info.NetworkSettings.IPAddress, - HostConfig: containertypes.HostConfig{ - Memory: info.HostConfig.Memory, - MemorySwap: info.HostConfig.MemorySwap, - CPUShares: info.HostConfig.CPUShares, - }, + IPAddress: ip, + HostConfig: hostCfg, } if info.Config != nil && info.Config.Cmd != nil && len(info.Config.Cmd) > 0 { cd.Command = info.Config.Cmd[0] } - for _, s := range summaries { - if s.ID == r.ID { - for _, p := range s.Ports { - cd.Ports = append(cd.Ports, containertypes.Port{ - PrivatePort: int(p.PrivatePort), - PublicPort: int(p.PublicPort), - Type: p.Type, - }) - } - break - } - } + if s, ok := sumIndex[r.ID]; ok { + for _, p := range s.Ports { + cd.Ports = append(cd.Ports, containertypes.Port{ + PrivatePort: int(p.PrivatePort), + PublicPort: int(p.PublicPort), + Type: p.Type, + }) + } + } for _, m := range info.Mounts { cd.Mounts = append(cd.Mounts, containertypes.Mount{ Type: string(m.Type), Source: m.Source, Destination: m.Destination, Mode: m.Mode, }) } - for name, network := range info.NetworkSettings.Networks { - cd.Networks = append(cd.Networks, containertypes.Network{ - Name: name, - IPAddress: network.IPAddress, - Gateway: network.Gateway, - MacAddress: network.MacAddress, - Aliases: network.Aliases, - }) - } + if info.NetworkSettings != nil && info.NetworkSettings.Networks != nil { + for name, network := range info.NetworkSettings.Networks { + cd.Networks = append(cd.Networks, containertypes.Network{ + Name: name, + IPAddress: network.IPAddress, + Gateway: network.Gateway, + MacAddress: network.MacAddress, + Aliases: network.Aliases, + }) + } + } result = append(result, cd) } return result }
🧹 Nitpick comments (6)
api/internal/features/deploy/docker/init.go (1)
99-112
: Nice: stopped panicking on list errors; but doc says “sorted by container ID” while code does not sort.Either implement the stated sort or correct the comment. If you prefer to fix the comment, apply:
-// ListAllContainers returns a list of all containers running on the host, along with their -// IDs, names, and statuses. The returned list is sorted by container ID in ascending order. +// ListAllContainers returns all containers running on the host, along with their +// IDs, names, and statuses. Order is determined by Docker’s API (no additional sorting).view/app/containers/page.tsx (3)
21-28
: Validate persisted viewMode from localStorage to avoid invalid values.Guard against unexpected stored values to keep UI predictable.
- const [viewMode, setViewMode] = React.useState<'table' | 'card'>(() => { - if (typeof window !== 'undefined') { - const existing = window.localStorage.getItem('containers_view'); - return (existing as 'table' | 'card') || 'table'; - } - return 'table'; - }); + const [viewMode, setViewMode] = React.useState<'table' | 'card'>(() => { + if (typeof window !== 'undefined') { + const existing = window.localStorage.getItem('containers_view'); + return existing === 'card' || existing === 'table' ? (existing as 'table' | 'card') : 'table'; + } + return 'table'; + });
117-159
: Polish: fix search label key and add accessibility label for the view toggle.
- The label
t('common.searchFiles')
looks unrelated to containers; prefer a containers-specific key.- Add
aria-label
(and optionaltitle
) for the toggle icon-only button.- <SearchBar - searchTerm={searchInput} - handleSearchChange={(e) => setSearchInput(e.target.value)} - label={t('common.searchFiles')} - /> + <SearchBar + searchTerm={searchInput} + handleSearchChange={(e) => setSearchInput(e.target.value)} + label={t('containers.search')} + /> @@ - <Button + <Button variant="outline" size="icon" onClick={() => { const next = viewMode === 'table' ? 'card' : 'table'; setViewMode(next); if (typeof window !== 'undefined') window.localStorage.setItem('containers_view', next); }} + aria-label={viewMode === 'table' ? t('containers.switch_to_card_view') : t('containers.switch_to_table_view')} + title={viewMode === 'table' ? t('containers.switch_to_card_view') : t('containers.switch_to_table_view')} >If
containers.search
,containers.switch_to_card_view
, andcontainers.switch_to_table_view
don’t exist, I can draft the i18n entries.
193-198
: Internationalize the count string.Use i18n with pluralization support.
- <TypographyMuted>{totalCount} containers</TypographyMuted> + <TypographyMuted> + {t('containers.count', { count: totalCount })} + </TypographyMuted>Let me know if you want me to add a default
containers.count
key with plural rules.api/internal/features/container/controller/list_containers.go (2)
57-96
: Optional: sanitize sort_by to an allowed set.Avoid surprising defaults by normalizing unknown values.
sortBy := strings.ToLower(strings.TrimSpace(q.Get("sort_by"))) @@ - if sortBy == "" { - sortBy = "name" - } + switch sortBy { + case "name", "status", "created": + // ok + case "": + sortBy = "name" + default: + sortBy = "name" + }
189-248
: Optional: parallelize per-container inspect with a small worker pool.For large pages this will reduce tail latency. Keep a cap (e.g., 4–8 workers) to avoid overwhelming the daemon. I can draft this if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
api/api/versions.json
(1 hunks)api/internal/features/container/controller/list_containers.go
(1 hunks)api/internal/features/deploy/docker/init.go
(2 hunks)view/app/containers/page.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/api/versions.json
🧰 Additional context used
🧬 Code graph analysis (2)
api/internal/features/container/controller/list_containers.go (4)
api/internal/features/container/controller/init.go (1)
ContainerController
(15-21)api/internal/types/response.go (1)
Response
(5-20)api/internal/features/logger/init.go (1)
Error
(22-22)api/internal/features/container/types/container_types.go (7)
ContainerListParams
(112-121)ContainerListRow
(123-131)Container
(3-17)HostConfig
(40-44)Port
(19-23)Mount
(25-30)Network
(32-38)
view/app/containers/page.tsx (3)
view/components/ui/search-bar.tsx (1)
SearchBar
(11-26)view/components/ui/typography.tsx (2)
TypographyH2
(9-15)TypographyMuted
(73-77)view/app/containers/components/card.tsx (1)
ContainerCard
(22-58)
🔇 Additional comments (5)
api/internal/features/deploy/docker/init.go (2)
26-29
: Good interface surface: error-first, filter-capable listing added.Adding
ListContainers(opts container.ListOptions) ([]container.Summary, error)
alongsideListAllContainers
is the right abstraction and aligns with the controller’s native Docker filters. No concerns.
114-122
: LGTM: thin wrapper that cleanly propagates errors.
ListContainers
directly delegates to the Docker client and returns(nil, err)
on failure.view/app/containers/page.tsx (2)
89-96
: Refresh button state handling is correct.Disabling on both
isRefreshing
andisFetching
prevents duplicate traffic and flicker.
167-191
: Two-mode rendering (card/table) is cleanly implemented.Props are correctly passed to both views; routing and actions are wired.
api/internal/features/container/controller/list_containers.go (1)
17-55
: Handler flow (parse → filter → summarize → search/sort/paginate → enrich) looks solid.Error propagation via HTTP 500 and structured response metadata is consistent.
Summary by CodeRabbit
New Features
Enhancements
Chores