Merge commit 'a892ae1c38cf308a9652370e0c7c2e22a6d9283c'

2026-04-10 06:18:11 +00:00 · 2026-04-10 06:18:11 +00:00 · 86d0a949c5
commit 86d0a949c5
parent f151b96f88 a892ae1c38
5 changed files with 516 additions and 2 deletions
--- a/.planning/phases/03-dashboard-intake-ui/03-02-SUMMARY.md
+++ b/.planning/phases/03-dashboard-intake-ui/03-02-SUMMARY.md
@ -0,0 +1,121 @@
+---
+phase: 03-dashboard-intake-ui
+plan: "02"
+subsystem: api, netbox
+tags: [go, inventory, rest-api, interface, tdd, chi]
+dependency_graph:
+  requires:
+    - internal/netbox.Client (ListDevices + GetDevice)
+  provides:
+    - internal/api/handlers.InventoryHandler
+    - internal/api/handlers.NewInventoryHandler
+    - internal/api/handlers.InventoryNetBoxClient (interface)
+    - internal/api/handlers.InventoryItemResponse
+    - GET /api/inventory route
+    - GET /api/inventory/{id} route
+  affects:
+    - internal/api/router.go (NewRouter signature extended)
+    - cmd/hwlab/main.go (InventoryHandler constructed and wired)
+tech_stack:
+  added: []
+  patterns:
+    - interface-for-testability (InventoryNetBoxClient mirrors IntakeNetBoxClient pattern)
+    - nil-safe-slice (nil PhotoURLs coerced to [] before JSON encoding)
+    - tdd-red-green (7 tests written failing, then handler implemented to pass)
+key_files:
+  created:
+    - internal/api/handlers/inventory.go
+    - internal/api/handlers/inventory_test.go
+  modified:
+    - internal/api/router.go
+    - cmd/hwlab/main.go
+decisions:
+  - id: INV-01
+    summary: "InventoryNetBoxClient interface wraps only ListDevices+GetDevice — keeps the handler narrow and fully testable without a live NetBox connection"
+  - id: INV-02
+    summary: "nil PhotoURLs from netbox.CustomFields coerced to []string{} in deviceToResponse to guarantee JSON encodes as [] not null — prevents frontend null-handling bugs"
+  - id: INV-03
+    summary: "Hard-coded limit=200 in ListDevices call (T-03-06) — caller cannot request unbounded result sets; pagination deferred to a future plan"
+metrics:
+  duration: "~10 minutes"
+  completed: "2026-04-10T00:00:00Z"
+  tasks_completed: 2
+  files_created: 2
+  files_modified: 2
+---
+
+# Phase 3 Plan 02: GET /api/inventory + GET /api/inventory/:id Summary
+
+**One-liner:** Inventory list and detail REST endpoints behind a narrow InventoryNetBoxClient interface, with 7 TDD unit tests covering all HTTP status paths and routes wired into the chi router.
+
+## What Was Built
+
+### `internal/api/handlers/inventory.go`
+
+- `InventoryNetBoxClient` interface with `ListDevices` and `GetDevice` — `*netbox.Client` satisfies it without any changes
+- `InventoryHandler` struct and `NewInventoryHandler(nb InventoryNetBoxClient) *InventoryHandler` constructor
+- `InventoryItemResponse` JSON struct: `id`, `name`, `asset_tag`, `hw_id`, `catalog_status`, `product_url`, `firmware_version`, `test_date`, `test_data`, `ai_notes`, `photo_urls`
+- `deviceToResponse` helper: maps `netbox.Device` → `InventoryItemResponse`, coerces nil `PhotoURLs` to `[]string{}`
+- `ListInventory`: `GET /api/inventory` — calls `ListDevices(ctx, 200)`, returns 200 JSON array or 502 on error
+- `GetInventoryItem`: `GET /api/inventory/{id}` — validates ID with `strconv.Atoi` (400 on failure), calls `GetDevice`, returns 200/404/502
+
+### `internal/api/handlers/inventory_test.go`
+
+Seven TDD unit tests using `mockInventoryNetBoxClient`:
+
+| Test | Scenario | Expected |
+|------|----------|----------|
+| TestListInventoryEmpty | mock returns [] | 200, empty JSON array |
+| TestListInventoryItems | mock returns 2 devices | 200, 2-item array with hw_id/name/catalog_status/asset_tag/photo_urls |
+| TestListInventoryNetBoxError | mock returns error | 502, {"error": "..."} |
+| TestGetInventoryItemFound | mock returns device id=42 | 200, device JSON with correct fields |
+| TestGetInventoryItemNotFound | mock returns "404 not found" error | 404, {"error": "item not found"} |
+| TestGetInventoryItemInvalidID | path param = "abc" | 400, {"error": "id must be an integer"} |
+| TestGetInventoryItemNetBoxError | mock returns server error | 502, {"error": "..."} |
+
+### `internal/api/router.go`
+
+- `NewRouter` signature extended: `NewRouter(staticFiles fs.FS, intakeHandler http.Handler, inventoryHandler *handlers.InventoryHandler) http.Handler`
+- Two new routes in `/api` group: `GET /inventory` and `GET /inventory/{id}`
+
+### `cmd/hwlab/main.go`
+
+- `inventoryHandler := handlers.NewInventoryHandler(nbClient)` constructed after NetBox client
+- Passed as third argument to `api.NewRouter`
+
+## Deviations from Plan
+
+None — plan executed exactly as written.
+
+## Known Stubs
+
+None — both endpoints call real `netbox.Client` methods (`ListDevices`, `GetDevice`). No mock or placeholder data flows to responses in production.
+
+## Threat Surface Coverage
+
+All four threats from the plan's threat register are addressed:
+
+| Threat | Mitigation | Where |
+|--------|------------|-------|
+| T-03-04: URL param tampering | `strconv.Atoi` rejects non-integer IDs → 400 | inventory.go:GetInventoryItem |
+| T-03-05: Error message info disclosure | Errors describe NetBox connectivity only; no credentials or paths exposed | inventory.go error paths |
+| T-03-06: DoS via unbounded list | Hard-coded `limit=200` in ListDevices call | inventory.go:ListInventory |
+| T-03-07: Unauthenticated endpoints | Accepted for Phase 3 homelab single-operator; Phase 4+ can add auth middleware | router.go |
+
+## Self-Check
+
+Files created:
+- internal/api/handlers/inventory.go: FOUND
+- internal/api/handlers/inventory_test.go: FOUND
+
+Commits:
+- b0b6153: feat(03-02): InventoryHandler with list+detail endpoints and 7 unit tests
+- 743611f: feat(03-02): wire GET /api/inventory and GET /api/inventory/{id} routes
+
+`go build ./...`: PASS
+`go test ./internal/api/handlers/... -run "TestListInventory|TestGetInventory" -v`: 7/7 PASS
+`go test ./...`: all packages PASS
+`grep "inventory" internal/api/router.go`: both GET routes PRESENT
+`grep "NewInventoryHandler" cmd/hwlab/main.go`: PRESENT
+
+## Self-Check: PASSED
--- a/cmd/hwlab/main.go
+++ b/cmd/hwlab/main.go
@ -77,7 +77,8 @@ func main() {
 		cfg.AI.QuickAddThreshold,
 	)

-	router := api.NewRouter(staticFS, intakeHandler)
+	inventoryHandler := handlers.NewInventoryHandler(nbClient)
+	router := api.NewRouter(staticFS, intakeHandler, inventoryHandler)
 	addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port)
 	log.Printf("HWLab starting on %s", addr)

--- a/internal/api/handlers/inventory.go
+++ b/internal/api/handlers/inventory.go
@ -0,0 +1,110 @@
+package handlers
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+	"strings"
+
+	"github.com/go-chi/chi/v5"
+
+	"git.georgsen.dk/hwlab/internal/netbox"
+)
+
+// InventoryNetBoxClient is the narrow interface the inventory handler needs.
+// *netbox.Client satisfies this interface.
+type InventoryNetBoxClient interface {
+	ListDevices(ctx context.Context, limit int) ([]netbox.Device, error)
+	GetDevice(ctx context.Context, id int) (*netbox.Device, error)
+}
+
+// InventoryHandler handles GET /api/inventory and GET /api/inventory/{id}.
+type InventoryHandler struct {
+	nb InventoryNetBoxClient
+}
+
+// NewInventoryHandler creates an InventoryHandler.
+func NewInventoryHandler(nb InventoryNetBoxClient) *InventoryHandler {
+	return &InventoryHandler{nb: nb}
+}
+
+// InventoryItemResponse is the JSON shape the frontend consumes.
+type InventoryItemResponse struct {
+	ID            int      `json:"id"`
+	Name          string   `json:"name"`
+	AssetTag      string   `json:"asset_tag"`
+	HWID          string   `json:"hw_id"`
+	CatalogStatus string   `json:"catalog_status"`
+	ProductURL    string   `json:"product_url,omitempty"`
+	Firmware      string   `json:"firmware_version,omitempty"`
+	TestDate      string   `json:"test_date,omitempty"`
+	TestData      string   `json:"test_data,omitempty"`
+	AINotes       string   `json:"ai_notes,omitempty"`
+	PhotoURLs     []string `json:"photo_urls"`
+}
+
+func deviceToResponse(d netbox.Device) InventoryItemResponse {
+	urls := d.CustomFields.PhotoURLs
+	if urls == nil {
+		urls = []string{}
+	}
+	return InventoryItemResponse{
+		ID:            d.ID,
+		Name:          d.Name,
+		AssetTag:      d.AssetTag,
+		HWID:          d.CustomFields.HWID,
+		CatalogStatus: d.CustomFields.CatalogStatus,
+		ProductURL:    d.CustomFields.ProductURL,
+		Firmware:      d.CustomFields.FirmwareVersion,
+		TestDate:      d.CustomFields.TestDate,
+		TestData:      d.CustomFields.TestData,
+		AINotes:       d.CustomFields.AINotes,
+		PhotoURLs:     urls,
+	}
+}
+
+// ListInventory handles GET /api/inventory — returns up to 200 items.
+func (h *InventoryHandler) ListInventory(w http.ResponseWriter, r *http.Request) {
+	devices, err := h.nb.ListDevices(r.Context(), 200)
+	if err != nil {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusBadGateway)
+		_ = json.NewEncoder(w).Encode(map[string]string{"error": fmt.Sprintf("netbox unavailable: %s", err)})
+		return
+	}
+	items := make([]InventoryItemResponse, 0, len(devices))
+	for _, d := range devices {
+		items = append(items, deviceToResponse(d))
+	}
+	w.Header().Set("Content-Type", "application/json")
+	_ = json.NewEncoder(w).Encode(items)
+}
+
+// GetInventoryItem handles GET /api/inventory/{id}.
+func (h *InventoryHandler) GetInventoryItem(w http.ResponseWriter, r *http.Request) {
+	rawID := chi.URLParam(r, "id")
+	id, err := strconv.Atoi(rawID)
+	if err != nil {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusBadRequest)
+		_ = json.NewEncoder(w).Encode(map[string]string{"error": "id must be an integer"})
+		return
+	}
+
+	device, err := h.nb.GetDevice(r.Context(), id)
+	if err != nil {
+		w.Header().Set("Content-Type", "application/json")
+		if strings.Contains(err.Error(), "404") || strings.Contains(strings.ToLower(err.Error()), "not found") {
+			w.WriteHeader(http.StatusNotFound)
+			_ = json.NewEncoder(w).Encode(map[string]string{"error": "item not found"})
+			return
+		}
+		w.WriteHeader(http.StatusBadGateway)
+		_ = json.NewEncoder(w).Encode(map[string]string{"error": fmt.Sprintf("netbox unavailable: %s", err)})
+		return
+	}
+	w.Header().Set("Content-Type", "application/json")
+	_ = json.NewEncoder(w).Encode(deviceToResponse(*device))
+}
--- a/internal/api/handlers/inventory_test.go
+++ b/internal/api/handlers/inventory_test.go
@ -0,0 +1,279 @@
+package handlers
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-chi/chi/v5"
+
+	"git.georgsen.dk/hwlab/internal/netbox"
+)
+
+// mockInventoryNetBoxClient implements InventoryNetBoxClient for unit tests.
+type mockInventoryNetBoxClient struct {
+	listDevices func(ctx context.Context, limit int) ([]netbox.Device, error)
+	getDevice   func(ctx context.Context, id int) (*netbox.Device, error)
+}
+
+func (m *mockInventoryNetBoxClient) ListDevices(ctx context.Context, limit int) ([]netbox.Device, error) {
+	return m.listDevices(ctx, limit)
+}
+
+func (m *mockInventoryNetBoxClient) GetDevice(ctx context.Context, id int) (*netbox.Device, error) {
+	return m.getDevice(ctx, id)
+}
+
+// newInventoryRouter wraps the handler in a chi router with the {id} param registered,
+// so chi.URLParam works in tests.
+func newInventoryRouter(h *InventoryHandler) http.Handler {
+	r := chi.NewRouter()
+	r.Get("/api/inventory", h.ListInventory)
+	r.Get("/api/inventory/{id}", h.GetInventoryItem)
+	return r
+}
+
+// TestListInventoryEmpty: mock returns [], handler returns 200 with JSON `[]`.
+func TestListInventoryEmpty(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		listDevices: func(_ context.Context, _ int) ([]netbox.Device, error) {
+			return []netbox.Device{}, nil
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", w.Code)
+	}
+	var items []InventoryItemResponse
+	if err := json.NewDecoder(w.Body).Decode(&items); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if len(items) != 0 {
+		t.Errorf("expected empty array, got %d items", len(items))
+	}
+}
+
+// TestListInventoryItems: mock returns 2 devices, handler returns 200 with JSON array of 2 items.
+func TestListInventoryItems(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		listDevices: func(_ context.Context, _ int) ([]netbox.Device, error) {
+			return []netbox.Device{
+				{
+					ID:       1,
+					Name:     "Raspberry Pi 4",
+					AssetTag: "HW-00001",
+					CustomFields: netbox.CustomFields{
+						HWID:          "HW-00001",
+						CatalogStatus: "indexed",
+						PhotoURLs:     []string{"http://example.com/photo1.jpg"},
+					},
+				},
+				{
+					ID:       2,
+					Name:     "Cisco Switch",
+					AssetTag: "HW-00002",
+					CustomFields: netbox.CustomFields{
+						HWID:          "HW-00002",
+						CatalogStatus: "needs_research",
+						PhotoURLs:     nil,
+					},
+				},
+			}, nil
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", w.Code)
+	}
+	var items []InventoryItemResponse
+	if err := json.NewDecoder(w.Body).Decode(&items); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if len(items) != 2 {
+		t.Fatalf("expected 2 items, got %d", len(items))
+	}
+
+	// Check first item fields
+	if items[0].HWID != "HW-00001" {
+		t.Errorf("item[0] hw_id: expected HW-00001, got %s", items[0].HWID)
+	}
+	if items[0].Name != "Raspberry Pi 4" {
+		t.Errorf("item[0] name: expected 'Raspberry Pi 4', got %s", items[0].Name)
+	}
+	if items[0].CatalogStatus != "indexed" {
+		t.Errorf("item[0] catalog_status: expected indexed, got %s", items[0].CatalogStatus)
+	}
+	if items[0].AssetTag != "HW-00001" {
+		t.Errorf("item[0] asset_tag: expected HW-00001, got %s", items[0].AssetTag)
+	}
+	if len(items[0].PhotoURLs) != 1 {
+		t.Errorf("item[0] photo_urls: expected 1, got %d", len(items[0].PhotoURLs))
+	}
+
+	// Second item — nil PhotoURLs should become empty slice in JSON
+	if items[1].PhotoURLs == nil {
+		t.Errorf("item[1] photo_urls: expected non-nil empty slice, got nil")
+	}
+}
+
+// TestListInventoryNetBoxError: mock returns error, handler returns 502.
+func TestListInventoryNetBoxError(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		listDevices: func(_ context.Context, _ int) ([]netbox.Device, error) {
+			return nil, errors.New("connection refused")
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusBadGateway {
+		t.Fatalf("expected 502, got %d", w.Code)
+	}
+	var body map[string]string
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if body["error"] == "" {
+		t.Error("expected non-empty error field")
+	}
+}
+
+// TestGetInventoryItemFound: mock returns device with id=42, GET /api/inventory/42 returns 200.
+func TestGetInventoryItemFound(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		getDevice: func(_ context.Context, id int) (*netbox.Device, error) {
+			if id != 42 {
+				t.Errorf("expected id=42, got %d", id)
+			}
+			return &netbox.Device{
+				ID:       42,
+				Name:     "Test Device",
+				AssetTag: "HW-00042",
+				CustomFields: netbox.CustomFields{
+					HWID:          "HW-00042",
+					CatalogStatus: "indexed",
+					AINotes:       "some notes",
+					PhotoURLs:     []string{},
+				},
+			}, nil
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory/42", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", w.Code)
+	}
+	var item InventoryItemResponse
+	if err := json.NewDecoder(w.Body).Decode(&item); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if item.ID != 42 {
+		t.Errorf("expected id=42, got %d", item.ID)
+	}
+	if item.HWID != "HW-00042" {
+		t.Errorf("expected hw_id=HW-00042, got %s", item.HWID)
+	}
+}
+
+// TestGetInventoryItemNotFound: mock returns error containing "404", handler returns 404.
+func TestGetInventoryItemNotFound(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		getDevice: func(_ context.Context, _ int) (*netbox.Device, error) {
+			return nil, errors.New("get device 99: 404 not found")
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory/99", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("expected 404, got %d", w.Code)
+	}
+	var body map[string]string
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if body["error"] == "" {
+		t.Error("expected non-empty error field")
+	}
+}
+
+// TestGetInventoryItemInvalidID: GET /api/inventory/abc returns 400.
+func TestGetInventoryItemInvalidID(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		getDevice: func(_ context.Context, _ int) (*netbox.Device, error) {
+			t.Error("GetDevice should not be called for invalid ID")
+			return nil, nil
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory/abc", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("expected 400, got %d", w.Code)
+	}
+	var body map[string]string
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if body["error"] == "" {
+		t.Error("expected non-empty error field")
+	}
+}
+
+// TestGetInventoryItemNetBoxError: mock returns server error, handler returns 502.
+func TestGetInventoryItemNetBoxError(t *testing.T) {
+	mock := &mockInventoryNetBoxClient{
+		getDevice: func(_ context.Context, _ int) (*netbox.Device, error) {
+			return nil, errors.New("internal server error")
+		},
+	}
+	h := NewInventoryHandler(mock)
+	router := newInventoryRouter(h)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/inventory/5", nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusBadGateway {
+		t.Fatalf("expected 502, got %d", w.Code)
+	}
+	var body map[string]string
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("decode error: %v", err)
+	}
+	if body["error"] == "" {
+		t.Error("expected non-empty error field")
+	}
+}
--- a/internal/api/router.go
+++ b/internal/api/router.go
@ -33,7 +33,8 @@ func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 // NewRouter creates the chi router. staticFiles is the fs.FS rooted at web/dist,
 // passed from main.go where the go:embed directive lives.
 // intakeHandler handles POST /api/intake (multipart photo upload).
-func NewRouter(staticFiles fs.FS, intakeHandler http.Handler) http.Handler {
+// inventoryHandler handles GET /api/inventory and GET /api/inventory/{id}.
+func NewRouter(staticFiles fs.FS, intakeHandler http.Handler, inventoryHandler *handlers.InventoryHandler) http.Handler {
 	r := chi.NewRouter()
 	r.Use(middleware.Logger)
 	r.Use(middleware.Recoverer)
@ -42,6 +43,8 @@ func NewRouter(staticFiles fs.FS, intakeHandler http.Handler) http.Handler {
 	r.Route("/api", func(r chi.Router) {
 		r.Get("/health", handlers.Health)
 		r.Post("/intake", intakeHandler.ServeHTTP)
+		r.Get("/inventory", inventoryHandler.ListInventory)
+		r.Get("/inventory/{id}", inventoryHandler.GetInventoryItem)
 	})

 	// SPA fallback — serve static files; unknown paths fall back to index.html.