From 3a33f2ba8fe2cef05f41139082598e9e78cfabe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 09:58:03 -0400 Subject: [PATCH 1/6] feat(@helm/web): add multi-product API methods to client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listProducts() → GET /api/products getProductBySlug(slug) → GET /api/products/:slug listItemsForProduct(slug) → GET /api/products/:slug/items Legacy methods (getProduct, listItems, getItem) kept for backward compat until cleanup PR. 6 new tests covering happy paths, URL encoding, 404, network error, and empty response. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/lib/api.test.ts | 55 ++++++++++++++++++++++++++++++++++++ apps/web/src/lib/api.ts | 10 +++++++ 2 files changed, 65 insertions(+) diff --git a/apps/web/src/lib/api.test.ts b/apps/web/src/lib/api.test.ts index c509e2f..5e41810 100644 --- a/apps/web/src/lib/api.test.ts +++ b/apps/web/src/lib/api.test.ts @@ -78,3 +78,58 @@ describe('api.getItem', () => { expect(result.ok).toBe(false); }); }); + +describe('api.listProducts', () => { + it('returns array of products', async () => { + const products = [ + { product: { slug: 'helm', name: 'Helm' } }, + { product: { slug: 'helm-playground', name: 'Helm Playground' } }, + ]; + mockFetch.mockResolvedValueOnce(mockOk(products)); + const result = await api.listProducts(); + expect(result.ok).toBe(true); + if (result.ok) expect(result.data).toHaveLength(2); + }); + + it('returns network error on fetch throw', async () => { + mockFetch.mockRejectedValueOnce(new Error('Network failure')); + const result = await api.listProducts(); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error.type).toBe('network'); + }); +}); + +describe('api.getProductBySlug', () => { + it('URL-encodes the slug', async () => { + mockFetch.mockResolvedValueOnce(mockOk({ product: { slug: 'helm' } })); + await api.getProductBySlug('helm'); + const calledUrl = mockFetch.mock.calls[0]?.[0] as string; + expect(calledUrl).toContain('/api/products/helm'); + }); + + it('returns http error on 404', async () => { + mockFetch.mockResolvedValueOnce(mockErr(404, { error: 'Product not found: ghost' })); + const result = await api.getProductBySlug('ghost'); + expect(result.ok).toBe(false); + if (!result.ok) expect((result.error as { status: number }).status).toBe(404); + }); +}); + +describe('api.listItemsForProduct', () => { + it('fetches from the product-scoped items endpoint', async () => { + const items = [{ externalId: 'issue_1', productSlug: 'helm-playground' }]; + mockFetch.mockResolvedValueOnce(mockOk(items)); + const result = await api.listItemsForProduct('helm-playground'); + const calledUrl = mockFetch.mock.calls[0]?.[0] as string; + expect(calledUrl).toContain('/api/products/helm-playground/items'); + expect(result.ok).toBe(true); + if (result.ok) expect(result.data).toHaveLength(1); + }); + + it('returns empty array when product has no items', async () => { + mockFetch.mockResolvedValueOnce(mockOk([])); + const result = await api.listItemsForProduct('new-product'); + expect(result.ok).toBe(true); + if (result.ok) expect(result.data).toHaveLength(0); + }); +}); diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index 9eeddf2..8bd0292 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -68,6 +68,16 @@ async function fetchJson(path: string): Promise> { // ── API client ──────────────────────────────────────────────────────────────── export const api = { + // ── Multi-product routes (Session 8+) ────────────────────────────────────── + listProducts: (): Promise> => fetchJson('/api/products'), + + getProductBySlug: (slug: string): Promise> => + fetchJson(`/api/products/${encodeURIComponent(slug)}`), + + listItemsForProduct: (slug: string): Promise> => + fetchJson(`/api/products/${encodeURIComponent(slug)}/items`), + + // ── Legacy single-product routes — kept for backward compat, remove after cleanup PR ── getProduct: (): Promise> => fetchJson('/api/product'), listItems: (): Promise> => fetchJson('/api/items'), getItem: (id: string): Promise> => From 066fa9545d8e6a33232f2cf40074b8ed9c4084ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 09:59:03 -0400 Subject: [PATCH 2/6] feat(@helm/web): add ProductTabs component with 5s polling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renders horizontal tabs from GET /api/products, marks active tab via useParams slug, links to /products/:slug. Polls every 5s to surface newly-registered products without reload. Loading: blank bar (no layout shift). Error: inline ⚠ message. Empty registry: renders null. 6 tests covering: render, active marking, hrefs, error, empty, re-poll. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/components/ProductTabs.test.tsx | 117 +++++++++++++++++++ apps/web/src/components/ProductTabs.tsx | 54 +++++++++ 2 files changed, 171 insertions(+) create mode 100644 apps/web/src/components/ProductTabs.test.tsx create mode 100644 apps/web/src/components/ProductTabs.tsx diff --git a/apps/web/src/components/ProductTabs.test.tsx b/apps/web/src/components/ProductTabs.test.tsx new file mode 100644 index 0000000..b0f046c --- /dev/null +++ b/apps/web/src/components/ProductTabs.test.tsx @@ -0,0 +1,117 @@ +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ProductTabs } from './ProductTabs.js'; + +// ── Mock api ────────────────────────────────────────────────────────────────── + +const { mockListProducts } = vi.hoisted(() => ({ mockListProducts: vi.fn() })); + +vi.mock('../lib/api.js', async (importOriginal) => { + const real = await importOriginal(); + return { ...real, api: { ...real.api, listProducts: mockListProducts } }; +}); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +const PRODUCTS = [ + { product: { slug: 'helm', name: 'Helm' }, workflow: { stages_enabled: [] } }, + { + product: { slug: 'helm-playground', name: 'Helm Playground' }, + workflow: { stages_enabled: [] }, + }, +]; + +function ok(data: T) { + return { ok: true as const, data }; +} +function err(message: string) { + return { ok: false as const, error: { type: 'network' as const, message } }; +} + +function renderTabs(initialPath = '/products/helm') { + return render( + + + } /> + } /> + + , + ); +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('ProductTabs', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('renders a tab for each product', async () => { + mockListProducts.mockResolvedValue(ok(PRODUCTS)); + renderTabs(); + + await vi.waitFor(() => { + expect(screen.getByText('Helm')).toBeInTheDocument(); + expect(screen.getByText('Helm Playground')).toBeInTheDocument(); + }); + }); + + it('marks the active tab based on the current slug', async () => { + mockListProducts.mockResolvedValue(ok(PRODUCTS)); + renderTabs('/products/helm-playground'); + + await vi.waitFor(() => { + const activeLink = screen.getByRole('link', { name: 'Helm Playground' }); + expect(activeLink).toHaveAttribute('aria-current', 'page'); + const inactiveLink = screen.getByRole('link', { name: 'Helm' }); + expect(inactiveLink).not.toHaveAttribute('aria-current'); + }); + }); + + it('each tab links to /products/:slug', async () => { + mockListProducts.mockResolvedValue(ok(PRODUCTS)); + renderTabs(); + + await vi.waitFor(() => { + expect(screen.getByRole('link', { name: 'Helm' })).toHaveAttribute('href', '/products/helm'); + expect(screen.getByRole('link', { name: 'Helm Playground' })).toHaveAttribute( + 'href', + '/products/helm-playground', + ); + }); + }); + + it('shows error state when listProducts fails', async () => { + mockListProducts.mockResolvedValue(err('Network error')); + renderTabs(); + + await vi.waitFor(() => { + expect(screen.getByText(/Failed to load products/)).toBeInTheDocument(); + }); + }); + + it('renders nothing when product list is empty', async () => { + mockListProducts.mockResolvedValue(ok([])); + const { container } = renderTabs(); + + await vi.waitFor(() => { + expect(container.firstChild).toBeNull(); + }); + }); + + it('re-fetches products after polling interval', async () => { + mockListProducts.mockResolvedValue(ok(PRODUCTS)); + renderTabs(); + + await vi.waitFor(() => expect(mockListProducts).toHaveBeenCalledTimes(1)); + + await vi.runOnlyPendingTimersAsync(); + await vi.waitFor(() => expect(mockListProducts).toHaveBeenCalledTimes(2)); + }); +}); diff --git a/apps/web/src/components/ProductTabs.tsx b/apps/web/src/components/ProductTabs.tsx new file mode 100644 index 0000000..4cd60e7 --- /dev/null +++ b/apps/web/src/components/ProductTabs.tsx @@ -0,0 +1,54 @@ +import { useCallback } from 'react'; +import { Link, useParams } from 'react-router-dom'; +import { api } from '../lib/api.js'; +import { usePolling } from '../hooks/usePolling.js'; + +/** + * Horizontal product tabs rendered above every kanban/detail view. + * Polls every 5s so newly-registered products appear without a page reload. + */ +export function ProductTabs() { + const { slug } = useParams<{ slug?: string }>(); + const fetchProducts = useCallback(() => api.listProducts(), []); + const { data: products, error, loading } = usePolling(fetchProducts, 5_000); + + // Blank bar while loading — avoids layout shift. + if (loading) { + return
; + } + + if (error) { + return ( +
+

⚠ Failed to load products

+
+ ); + } + + if (!products?.length) return null; + + return ( + + ); +} From 6fff7bae4feea9dbf1e21cc3124ce9dc970db821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 10:00:21 -0400 Subject: [PATCH 3/6] feat(@helm/web): add URL-based routing and refactor views for multi-product MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit App.tsx: /products/:slug → Kanban, /products/:slug/items/:externalId → ItemDetail. ProductsRedirect resolves first product slug and navigates; shows empty state if registry is empty. LegacyItemRedirect maps old /items/:id to the new path for backward compat. Kanban: reads slug from useParams, calls getProductBySlug + listItemsForProduct, renders ProductTabs above the board. Product not found → inline error with back link. ItemCard links updated to /products/:slug/items/:externalId. ItemDetail: reads slug + externalId from useParams, back link resolves to /products/:slug. Error and loading states unchanged. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/App.tsx | 71 +++++++++++++++++++++++++++++-- apps/web/src/views/ItemDetail.tsx | 17 +++++--- apps/web/src/views/Kanban.tsx | 65 +++++++++++++++++++++++----- 3 files changed, 133 insertions(+), 20 deletions(-) diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index b249c67..5ee61ed 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -1,13 +1,78 @@ -import { BrowserRouter, Route, Routes } from 'react-router-dom'; +import { useCallback } from 'react'; +import { BrowserRouter, Navigate, Route, Routes, useParams } from 'react-router-dom'; import { Kanban } from './views/Kanban.js'; import { ItemDetail } from './views/ItemDetail.js'; +import { api } from './lib/api.js'; +import { usePolling } from './hooks/usePolling.js'; + +// ── Redirect helpers ────────────────────────────────────────────────────────── + +/** + * Resolves the first registered product slug and redirects to /products/:slug. + * Shows an empty state if the registry is empty so the app never crashes. + */ +function ProductsRedirect() { + const fetchProducts = useCallback(() => api.listProducts(), []); + const { data: products, loading } = usePolling(fetchProducts, null); + + if (loading) { + return ( +
+

Loading…

+
+ ); + } + + if (!products?.length) { + return ( +
+
+

No products registered.

+

+ Add a product to .helm/products.yaml and restart the server. +

+
+
+ ); + } + + return ; +} + +/** + * Legacy /items/:id → /products/:firstSlug/items/:id so old bookmarks still work. + */ +function LegacyItemRedirect() { + const { id } = useParams<{ id: string }>(); + const fetchProducts = useCallback(() => api.listProducts(), []); + const { data: products, loading } = usePolling(fetchProducts, null); + + if (loading) { + return ( +
+

Loading…

+
+ ); + } + + if (!products?.length || !id) return ; + return ; +} + +// ── App ─────────────────────────────────────────────────────────────────────── export default function App() { return ( - } /> - } /> + {/* Multi-product routes */} + } /> + } /> + } /> + + {/* Backward-compat redirects */} + } /> + } /> ); diff --git a/apps/web/src/views/ItemDetail.tsx b/apps/web/src/views/ItemDetail.tsx index 50cef69..eed376d 100644 --- a/apps/web/src/views/ItemDetail.tsx +++ b/apps/web/src/views/ItemDetail.tsx @@ -41,18 +41,21 @@ function HistoryRow({ event, index }: { event: WorkflowEvent; index: number }) { } export function ItemDetail() { - const { id } = useParams<{ id: string }>(); + const { slug, externalId } = useParams<{ slug: string; externalId: string }>(); const fetcher = useCallback((): ReturnType => { - if (!id) { + if (!externalId) { return Promise.resolve({ ok: false, error: { type: 'network', message: 'Missing item id in route.' }, }); } - return api.getItem(id); - }, [id]); - const { data: item, error, loading } = usePolling(fetcher, id ? 5_000 : null); + return api.getItem(externalId); + }, [externalId]); + + const { data: item, error, loading } = usePolling(fetcher, externalId ? 5_000 : null); + + const backLink = slug ? `/products/${slug}` : '/products'; if (loading) { return ( @@ -69,7 +72,7 @@ export function ItemDetail() {

{error}

- + ← Back to board
@@ -84,7 +87,7 @@ export function ItemDetail() { {/* Header */}
← Back to board diff --git a/apps/web/src/views/Kanban.tsx b/apps/web/src/views/Kanban.tsx index bfba294..5bc236d 100644 --- a/apps/web/src/views/Kanban.tsx +++ b/apps/web/src/views/Kanban.tsx @@ -1,9 +1,10 @@ import { useCallback, useMemo } from 'react'; -import { Link } from 'react-router-dom'; +import { Link, useParams } from 'react-router-dom'; import { api } from '../lib/api.js'; import type { ItemState } from '../lib/api.js'; import type { WorkflowStage } from '@helm/workflow'; import { usePolling } from '../hooks/usePolling.js'; +import { ProductTabs } from '../components/ProductTabs.js'; // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -22,10 +23,10 @@ function relativeTime(iso: string): string { // ── Sub-components ──────────────────────────────────────────────────────────── -function ItemCard({ item }: { item: ItemState }) { +function ItemCard({ item, slug }: { item: ItemState; slug: string }) { return (

{item.externalId}

@@ -35,7 +36,15 @@ function ItemCard({ item }: { item: ItemState }) { ); } -function KanbanColumn({ stage, items }: { stage: WorkflowStage; items: ItemState[] }) { +function KanbanColumn({ + stage, + items, + slug, +}: { + stage: WorkflowStage; + items: ItemState[]; + slug: string; +}) { return (
@@ -46,7 +55,7 @@ function KanbanColumn({ stage, items }: { stage: WorkflowStage; items: ItemState
{items.map((item) => ( - + ))} {items.length === 0 &&

—

}
@@ -57,8 +66,25 @@ function KanbanColumn({ stage, items }: { stage: WorkflowStage; items: ItemState // ── Main view ───────────────────────────────────────────────────────────────── export function Kanban() { - const fetchProduct = useCallback(() => api.getProduct(), []); - const fetchItems = useCallback(() => api.listItems(), []); + const { slug } = useParams<{ slug: string }>(); + + const fetchProduct = useCallback( + () => + slug + ? api.getProductBySlug(slug) + : Promise.resolve({ + ok: false as const, + error: { type: 'network' as const, message: 'Missing product slug in route.' }, + }), + [slug], + ); + const fetchItems = useCallback( + () => + slug + ? api.listItemsForProduct(slug) + : Promise.resolve({ ok: true as const, data: [] as ItemState[] }), + [slug], + ); const { data: product, error: productError, loading } = usePolling(fetchProduct, null); const { data: items, error: itemsError } = usePolling(fetchItems, 5_000); @@ -73,8 +99,19 @@ export function Kanban() { if (productError) { return ( -
-

{productError}

+
+ +
+
+

{productError}

+ + ← Back to products + +
+
); } @@ -109,10 +146,18 @@ export function Kanban() {
+ {/* Product tabs */} + + {/* Board */}
{stages.map((stage) => ( - + ))}
From 98f37bc7e1ea2d812b0b87216a6d7c94e8a526d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 10:04:04 -0400 Subject: [PATCH 4/6] test(@helm/web): add Kanban + App routing tests, fix useMemo hook order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useMemo was called after early returns in Kanban — violates Rules of Hooks. Moved before loading/error guards so it's always called unconditionally. Tests added: - Kanban.test.tsx: 5 cases — columns, item card, correct href, error+back link, empty items. Uses MemoryRouter with /products/:slug route. - App.test.tsx: 2 cases — empty registry shows empty state, redirect to /products/:firstSlug when products exist. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/App.test.tsx | 68 +++++++++++++++ apps/web/src/views/Kanban.test.tsx | 133 +++++++++++++++++++++++++++++ apps/web/src/views/Kanban.tsx | 26 +++--- 3 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 apps/web/src/App.test.tsx create mode 100644 apps/web/src/views/Kanban.test.tsx diff --git a/apps/web/src/App.test.tsx b/apps/web/src/App.test.tsx new file mode 100644 index 0000000..5aa57f2 --- /dev/null +++ b/apps/web/src/App.test.tsx @@ -0,0 +1,68 @@ +import { render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import App from './App.js'; + +// ── Mocks ───────────────────────────────────────────────────────────────────── + +const { mockListProducts, mockGetProductBySlug, mockListItemsForProduct } = vi.hoisted(() => ({ + mockListProducts: vi.fn(), + mockGetProductBySlug: vi.fn(), + mockListItemsForProduct: vi.fn(), +})); + +vi.mock('./lib/api.js', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + api: { + ...real.api, + listProducts: mockListProducts, + getProductBySlug: mockGetProductBySlug, + listItemsForProduct: mockListItemsForProduct, + }, + }; +}); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function ok(data: T) { + return { ok: true as const, data }; +} + +const PRODUCTS = [ + { product: { slug: 'helm', name: 'Helm' }, workflow: { stages_enabled: ['discovery'] as const } }, +]; + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('App routing', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('shows empty state when registry returns no products', async () => { + mockListProducts.mockResolvedValue(ok([])); + render(); + + await vi.waitFor(() => { + expect(screen.getByText('No products registered.')).toBeInTheDocument(); + }); + }); + + it('redirects / to /products/:firstSlug when products exist', async () => { + mockListProducts.mockResolvedValue(ok(PRODUCTS)); + mockGetProductBySlug.mockResolvedValue(ok(PRODUCTS[0])); + mockListItemsForProduct.mockResolvedValue(ok([])); + render(); + + await vi.waitFor(() => { + // After redirect, Kanban header renders "Helm / Helm" breadcrumb + expect(screen.getByRole('heading', { name: /Helm/ })).toBeInTheDocument(); + }); + }); +}); diff --git a/apps/web/src/views/Kanban.test.tsx b/apps/web/src/views/Kanban.test.tsx new file mode 100644 index 0000000..e24d738 --- /dev/null +++ b/apps/web/src/views/Kanban.test.tsx @@ -0,0 +1,133 @@ +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Kanban } from './Kanban.js'; + +// ── Mocks ───────────────────────────────────────────────────────────────────── + +const { mockGetProductBySlug, mockListItemsForProduct, mockListProducts } = vi.hoisted(() => ({ + mockGetProductBySlug: vi.fn(), + mockListItemsForProduct: vi.fn(), + mockListProducts: vi.fn(), +})); + +vi.mock('../lib/api.js', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + api: { + ...real.api, + getProductBySlug: mockGetProductBySlug, + listItemsForProduct: mockListItemsForProduct, + listProducts: mockListProducts, + }, + }; +}); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function ok(data: T) { + return { ok: true as const, data }; +} +function err(message: string) { + return { ok: false as const, error: { type: 'network' as const, message } }; +} + +const PRODUCT = { + product: { slug: 'helm-playground', name: 'Helm Playground' }, + workflow: { stages_enabled: ['discovery', 'spec-ready'] as const }, +}; + +const ITEMS = [ + { + externalId: 'issue_1', + productSlug: 'helm-playground', + currentStage: 'discovery', + history: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, +]; + +function renderKanban(slug = 'helm-playground') { + return render( + + + } /> + + , + ); +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('Kanban', () => { + beforeEach(() => { + vi.useFakeTimers(); + mockListProducts.mockResolvedValue(ok([])); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('renders columns from product stages_enabled', async () => { + mockGetProductBySlug.mockResolvedValue(ok(PRODUCT)); + mockListItemsForProduct.mockResolvedValue(ok(ITEMS)); + renderKanban(); + + // Stage names appear lowercase in DOM; CSS `uppercase` is a visual-only transform. + // getAllByText is used because stage name also appears in the item card subtitle. + await vi.waitFor(() => { + expect(screen.getAllByText('discovery').length).toBeGreaterThan(0); + expect(screen.getAllByText('spec-ready').length).toBeGreaterThan(0); + }); + }); + + it('shows item card in the correct column', async () => { + mockGetProductBySlug.mockResolvedValue(ok(PRODUCT)); + mockListItemsForProduct.mockResolvedValue(ok(ITEMS)); + renderKanban(); + + await vi.waitFor(() => { + expect(screen.getByText('issue_1')).toBeInTheDocument(); + }); + }); + + it('item card links to /products/:slug/items/:externalId', async () => { + mockGetProductBySlug.mockResolvedValue(ok(PRODUCT)); + mockListItemsForProduct.mockResolvedValue(ok(ITEMS)); + renderKanban(); + + await vi.waitFor(() => { + const link = screen.getByRole('link', { name: /issue_1/ }); + expect(link).toHaveAttribute('href', '/products/helm-playground/items/issue_1'); + }); + }); + + it('shows error with back link when product fetch fails', async () => { + mockGetProductBySlug.mockResolvedValue(err('Product not found: ghost')); + mockListItemsForProduct.mockResolvedValue(ok([])); + renderKanban('ghost'); + + await vi.waitFor(() => { + expect(screen.getByText('Product not found: ghost')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: /back to products/i })).toHaveAttribute( + 'href', + '/products', + ); + }); + }); + + it('renders empty columns (no crash) when items list is empty', async () => { + mockGetProductBySlug.mockResolvedValue(ok(PRODUCT)); + mockListItemsForProduct.mockResolvedValue(ok([])); + renderKanban(); + + await vi.waitFor(() => { + expect(screen.getByText('discovery')).toBeInTheDocument(); + expect(screen.getAllByText('—').length).toBeGreaterThan(0); + }); + }); +}); diff --git a/apps/web/src/views/Kanban.tsx b/apps/web/src/views/Kanban.tsx index 5bc236d..da5091a 100644 --- a/apps/web/src/views/Kanban.tsx +++ b/apps/web/src/views/Kanban.tsx @@ -89,6 +89,19 @@ export function Kanban() { const { data: product, error: productError, loading } = usePolling(fetchProduct, null); const { data: items, error: itemsError } = usePolling(fetchItems, 5_000); + // useMemo must be called unconditionally — before any early returns. + const stages = product?.workflow.stages_enabled ?? []; + const allItems = items ?? []; + const itemsByStage = useMemo(() => { + const grouped = new Map(); + for (const item of allItems) { + const list = grouped.get(item.currentStage) ?? []; + list.push(item); + grouped.set(item.currentStage, list); + } + return grouped; + }, [allItems]); + if (loading) { return (
@@ -116,19 +129,6 @@ export function Kanban() { ); } - const stages = product?.workflow.stages_enabled ?? []; - const allItems = items ?? []; - - const itemsByStage = useMemo(() => { - const grouped = new Map(); - for (const item of allItems) { - const list = grouped.get(item.currentStage) ?? []; - list.push(item); - grouped.set(item.currentStage, list); - } - return grouped; - }, [allItems]); - return (
{/* Header */} From 55d3501fff0f4d59cd0cf7a378a353af99a2bdac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 10:27:26 -0400 Subject: [PATCH 5/6] fix(@helm/web): address 7 CodeRabbit findings on PR #13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. App.test.tsx: assert window.location.pathname === '/products/helm' to verify the redirect contract directly (not just rendered content) 2. App.tsx ProductsRedirect: expose error from usePolling so registry failures show "Failed to load products" instead of "no products" empty state. LegacyItemRedirect: redirect to / on error. 3+4. App.tsx + ProductTabs.tsx: encodeURIComponent on slug in Navigate and tab Link hrefs 5. api.test.ts: getProductBySlug URL-encoding test uses 'helm/playground with space' to actually exercise encodeURIComponent 6. ItemDetail.tsx: validate item.productSlug === slug — mismatch shows "Item does not belong to this product." error instead of rendering wrong item silently 7. Kanban.tsx: encodeURIComponent on slug + externalId in ItemCard href Addresses CodeRabbit review comments on PR #13. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/App.test.tsx | 2 +- apps/web/src/App.tsx | 22 ++++++++++++++++++---- apps/web/src/components/ProductTabs.tsx | 2 +- apps/web/src/lib/api.test.ts | 9 +++++---- apps/web/src/views/ItemDetail.tsx | 8 ++++++-- apps/web/src/views/Kanban.tsx | 2 +- 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/apps/web/src/App.test.tsx b/apps/web/src/App.test.tsx index 5aa57f2..4718e03 100644 --- a/apps/web/src/App.test.tsx +++ b/apps/web/src/App.test.tsx @@ -61,7 +61,7 @@ describe('App routing', () => { render(); await vi.waitFor(() => { - // After redirect, Kanban header renders "Helm / Helm" breadcrumb + expect(window.location.pathname).toBe('/products/helm'); expect(screen.getByRole('heading', { name: /Helm/ })).toBeInTheDocument(); }); }); diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 5ee61ed..453f782 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -13,7 +13,7 @@ import { usePolling } from './hooks/usePolling.js'; */ function ProductsRedirect() { const fetchProducts = useCallback(() => api.listProducts(), []); - const { data: products, loading } = usePolling(fetchProducts, null); + const { data: products, error, loading } = usePolling(fetchProducts, null); if (loading) { return ( @@ -23,6 +23,14 @@ function ProductsRedirect() { ); } + if (error) { + return ( +
+

Failed to load products: {error}

+
+ ); + } + if (!products?.length) { return (
@@ -36,7 +44,7 @@ function ProductsRedirect() { ); } - return ; + return ; } /** @@ -45,7 +53,7 @@ function ProductsRedirect() { function LegacyItemRedirect() { const { id } = useParams<{ id: string }>(); const fetchProducts = useCallback(() => api.listProducts(), []); - const { data: products, loading } = usePolling(fetchProducts, null); + const { data: products, error, loading } = usePolling(fetchProducts, null); if (loading) { return ( @@ -55,8 +63,14 @@ function LegacyItemRedirect() { ); } + if (error) return ; if (!products?.length || !id) return ; - return ; + return ( + + ); } // ── App ─────────────────────────────────────────────────────────────────────── diff --git a/apps/web/src/components/ProductTabs.tsx b/apps/web/src/components/ProductTabs.tsx index 4cd60e7..1ca5f1a 100644 --- a/apps/web/src/components/ProductTabs.tsx +++ b/apps/web/src/components/ProductTabs.tsx @@ -35,7 +35,7 @@ export function ProductTabs() { return ( { }); describe('api.getProductBySlug', () => { - it('URL-encodes the slug', async () => { - mockFetch.mockResolvedValueOnce(mockOk({ product: { slug: 'helm' } })); - await api.getProductBySlug('helm'); + it('URL-encodes special characters in the slug', async () => { + const slug = 'helm/playground with space'; + mockFetch.mockResolvedValueOnce(mockOk({ product: { slug } })); + await api.getProductBySlug(slug); const calledUrl = mockFetch.mock.calls[0]?.[0] as string; - expect(calledUrl).toContain('/api/products/helm'); + expect(calledUrl).toContain(`/api/products/${encodeURIComponent(slug)}`); }); it('returns http error on 404', async () => { diff --git a/apps/web/src/views/ItemDetail.tsx b/apps/web/src/views/ItemDetail.tsx index eed376d..ce9ff2f 100644 --- a/apps/web/src/views/ItemDetail.tsx +++ b/apps/web/src/views/ItemDetail.tsx @@ -56,6 +56,8 @@ export function ItemDetail() { const { data: item, error, loading } = usePolling(fetcher, externalId ? 5_000 : null); const backLink = slug ? `/products/${slug}` : '/products'; + // Guard against navigating to /products/A/items/X where X belongs to product B. + const slugMismatch = Boolean(item && slug && item.productSlug !== slug); if (loading) { return ( @@ -67,11 +69,13 @@ export function ItemDetail() { // Full-page error only when there's no prior data — transient poll failures // show an inline warning so the last-known content stays visible. - if (error && !item) { + if ((error && !item) || slugMismatch) { return (
-

{error}

+

+ {slugMismatch ? 'Item does not belong to this product.' : error} +

← Back to board diff --git a/apps/web/src/views/Kanban.tsx b/apps/web/src/views/Kanban.tsx index da5091a..a5b4a33 100644 --- a/apps/web/src/views/Kanban.tsx +++ b/apps/web/src/views/Kanban.tsx @@ -26,7 +26,7 @@ function relativeTime(iso: string): string { function ItemCard({ item, slug }: { item: ItemState; slug: string }) { return (

{item.externalId}

From 2589741a2b0f310f966482a1c0bbbb0e20a17326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Herna=CC=81n=20Pau=CC=81l=20Ossando=CC=81n?= Date: Tue, 19 May 2026 10:49:01 -0400 Subject: [PATCH 6/6] fix(@helm/web): address 3 more CodeRabbit findings on PR #13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. ItemDetail.tsx: encode slug in backLink (missed in previous round — inconsistent with App.tsx and Kanban.tsx encoding) 2. App.test.tsx: shallow-copy data in ok() helper so mutable references don't leak between tests 3. api.test.ts: listItemsForProduct URL encoding test uses slug with reserved chars ('helm/playground with space') to actually exercise encodeURIComponent Addresses CodeRabbit review comments on PR #13. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/App.test.tsx | 8 +++++++- apps/web/src/lib/api.test.ts | 9 +++++---- apps/web/src/views/ItemDetail.tsx | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/apps/web/src/App.test.tsx b/apps/web/src/App.test.tsx index 4718e03..ee87809 100644 --- a/apps/web/src/App.test.tsx +++ b/apps/web/src/App.test.tsx @@ -26,7 +26,13 @@ vi.mock('./lib/api.js', async (importOriginal) => { // ── Helpers ─────────────────────────────────────────────────────────────────── function ok(data: T) { - return { ok: true as const, data }; + const safeData = + data && typeof data === 'object' + ? Array.isArray(data) + ? ([...data] as T) + : ({ ...data } as T) + : data; + return { ok: true as const, data: safeData }; } const PRODUCTS = [ diff --git a/apps/web/src/lib/api.test.ts b/apps/web/src/lib/api.test.ts index f7a1917..d58803f 100644 --- a/apps/web/src/lib/api.test.ts +++ b/apps/web/src/lib/api.test.ts @@ -117,12 +117,13 @@ describe('api.getProductBySlug', () => { }); describe('api.listItemsForProduct', () => { - it('fetches from the product-scoped items endpoint', async () => { - const items = [{ externalId: 'issue_1', productSlug: 'helm-playground' }]; + it('fetches from the product-scoped items endpoint and URL-encodes the slug', async () => { + const slug = 'helm/playground with space'; + const items = [{ externalId: 'issue_1', productSlug: slug }]; mockFetch.mockResolvedValueOnce(mockOk(items)); - const result = await api.listItemsForProduct('helm-playground'); + const result = await api.listItemsForProduct(slug); const calledUrl = mockFetch.mock.calls[0]?.[0] as string; - expect(calledUrl).toContain('/api/products/helm-playground/items'); + expect(calledUrl).toContain(`/api/products/${encodeURIComponent(slug)}/items`); expect(result.ok).toBe(true); if (result.ok) expect(result.data).toHaveLength(1); }); diff --git a/apps/web/src/views/ItemDetail.tsx b/apps/web/src/views/ItemDetail.tsx index ce9ff2f..6ba96ad 100644 --- a/apps/web/src/views/ItemDetail.tsx +++ b/apps/web/src/views/ItemDetail.tsx @@ -55,7 +55,7 @@ export function ItemDetail() { const { data: item, error, loading } = usePolling(fetcher, externalId ? 5_000 : null); - const backLink = slug ? `/products/${slug}` : '/products'; + const backLink = slug ? `/products/${encodeURIComponent(slug)}` : '/products'; // Guard against navigating to /products/A/items/X where X belongs to product B. const slugMismatch = Boolean(item && slug && item.productSlug !== slug);