Skip to content

Conversation

zhravan
Copy link
Collaborator

@zhravan zhravan commented Sep 2, 2025

Summary by CodeRabbit

  • New Features

    • Server-side pagination, sorting (name/status), and debounced search for containers; API returns containers plus pagination metadata.
    • Toggle between table and card views with saved preference and UI controls (search, page size, pagination).
  • Enhancements

    • Table supports clickable sort headers, optional per-row actions, and Play/Pause toggle with improved running-state detection.
    • Improved loading/refresh indicators, total-count display, and stable UI when parameters change.
  • Chores

    • Updated release metadata.

Copy link
Contributor

coderabbitai bot commented Sep 2, 2025

Walkthrough

Backend 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

Cohort / File(s) Summary
API Versioning
api/api/versions.json
Updated v1 release_date timestamp.
Backend: Container List Controller
api/internal/features/container/controller/list_containers.go
New pipeline: parse params, build Docker filters, call Docker ListContainers, summarize rows, apply search/sort/paginate, then append detailed inspect info for current page; response now includes pagination/filter metadata; per-container inspect errors are logged and skipped; method arg renamed to fuegoCtx.
Backend: Container Types (listing)
api/internal/features/container/types/container_types.go
Added exported types ContainerListParams and ContainerListRow for list requests and lightweight row summaries.
Backend: Docker Service
api/internal/features/deploy/docker/init.go
Added ListContainers(opts container.ListOptions) ([]container.Summary, error) to DockerRepository and implemented on DockerService; ListAllContainers changed to return errors instead of panicking.
Frontend: Redux Service (Containers API)
view/redux/services/container/containerApi.ts
Added ContainerListParams; getContainers now accepts params and returns { containers, total_count, page, page_size }; GET passes query params; transformResponse updated for nested response shape.
Frontend: Containers Hook
view/app/containers/hooks/use-container-list.ts
Reworked to use server-side pagination/sorting and debounced search; exposes page/pageSize/totalCount/totalPages/searchInput/sortBy/sortOrder/handleSort/isFetching/initialized and caches lastData to avoid UI flashes.
Frontend: Containers Page
view/app/containers/page.tsx
Adds persistent table/card view toggle (localStorage), search bar, page size select, pagination controls, wiring to hook state; passes sorting to table and shows totals; feature-flag guarded.
Frontend: Table and Actions
view/app/containers/components/table.tsx, view/app/containers/components/actions.tsx
Table adds sortable headers (name/status), new props sortBy/sortOrder/onSort/onAction, conditional Actions column; Actions swaps StopCircle for Pause/Play icon toggle and improves running-state check.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Possibly related PRs

Suggested labels

nixopus-docker

Suggested reviewers

  • raghavyuva

Poem

I twitch my whiskers at pages new,
Hop through rows and filters too.
Pause, play, sort — a tidy spree,
Cards or tables — pick by me.
I fetch the list and nibble logs 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@zhravan
Copy link
Collaborator Author

zhravan commented Sep 2, 2025

image

@zhravan
Copy link
Collaborator Author

zhravan commented Sep 2, 2025

image

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.tsx

If 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 buttons

Improve 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” claims

Implementation 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 params

Clarify 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 shapes

Container.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 label

Use 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 toggle

Aids 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 string

Prefer 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5912ce5 and 65d0442.

📒 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_date

The 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>

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 optional title) 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, and containers.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.

📥 Commits

Reviewing files that changed from the base of the PR and between 65d0442 and 62daffa.

📒 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) alongside ListAllContainers 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 and isFetching 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.

@raghavyuva raghavyuva merged commit 7400fda into raghavyuva:master Sep 3, 2025
4 checks passed
@zhravan zhravan linked an issue Sep 8, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Containers Listing - Paginate, Search, Sort

2 participants