From cbaba76345d28e639ac31424ca18864367e4df55 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Sun, 7 Jun 2026 22:28:57 -0700 Subject: [PATCH] =?UTF-8?q?fix(=C2=A722):=20catalog=20scopes=20to=20the=20?= =?UTF-8?q?named=20collection=20in=20the=20URL=20(E2E-caught)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useCollectionId read useParams().collectionId, but the Catalog renders at /p/:projectId/* — outside the c/:collectionId route — so it always fell back to the default collection. The faceted filter (SLICE-3) and bulk action bar (SLICE-5) therefore never scoped to a named (fields-bearing) collection in the deployed app; the Catalog unit test had mocked useCollectionId, hiding it. Resolve the /c// segment from the pathname when the route param isn't in scope. Caught by the new Playwright metadata suite. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/lib/entryPaths.js | 14 ++++++-- frontend/src/lib/useCollectionId.test.jsx | 42 +++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 frontend/src/lib/useCollectionId.test.jsx diff --git a/frontend/src/lib/entryPaths.js b/frontend/src/lib/entryPaths.js index b780f5a..a4ce000 100644 --- a/frontend/src/lib/entryPaths.js +++ b/frontend/src/lib/entryPaths.js @@ -4,7 +4,7 @@ // project-scoped, so the builders emit the default collection segment; the // collection-aware link layer (named collections) lands in S2. Components build // links via these helpers so that flip happens in one place. -import { useParams } from 'react-router-dom' +import { useParams, useLocation } from 'react-router-dom' import { useProject } from '../components/ProjectLayout.jsx' import { useDeployment } from '../context/DeploymentProvider' @@ -41,7 +41,17 @@ export function useProjectId() { // §22 S2 — the collection id a component should scope to: the `/c/:collectionId/` // route segment when present, else the project's default collection. +// +// The route param is only in scope for components rendered *under* the +// `c/:collectionId` route (e.g. RFCView). The Catalog renders one level up at +// `/p/:projectId/*` — a sibling of that route — so `useParams().collectionId` +// is undefined there and it would wrongly fall back to the default collection +// (the faceted/bulk catalog UI would then never scope to a named collection). +// Fall back to parsing the `/c//` segment from the pathname so the Catalog +// scopes correctly regardless of its position in the route tree. export function useCollectionId() { const { collectionId } = useParams() - return collectionId || DEFAULT_COLLECTION + const { pathname } = useLocation() + const fromPath = pathname.match(/\/c\/([^/]+)/) + return collectionId || (fromPath && fromPath[1]) || DEFAULT_COLLECTION } diff --git a/frontend/src/lib/useCollectionId.test.jsx b/frontend/src/lib/useCollectionId.test.jsx new file mode 100644 index 0000000..8b31a5a --- /dev/null +++ b/frontend/src/lib/useCollectionId.test.jsx @@ -0,0 +1,42 @@ +// Regression: useCollectionId must resolve the /c// segment even when the +// component renders ABOVE the `c/:collectionId` route (the Catalog's position +// at `/p/:projectId/*`), where useParams() does not expose collectionId. A bug +// here makes the faceted/bulk catalog silently scope to the default collection. +import { describe, it, expect } from 'vitest' +import { renderHook } from '@testing-library/react' +import { MemoryRouter, Routes, Route } from 'react-router-dom' +import { useCollectionId } from './entryPaths.js' + +function wrapperFor(initialPath, routePath) { + return ({ children }) => ( + + + + + + ) +} + +describe('useCollectionId', () => { + it('reads the /c// segment from the path at the Catalog level (no param in scope)', () => { + // The Catalog matches `/p/:projectId/*` — collectionId is NOT a param here. + const { result } = renderHook(() => useCollectionId(), { + wrapper: wrapperFor('/p/ohm/c/bdd/e/checkout', '/p/:projectId/*'), + }) + expect(result.current).toBe('bdd') + }) + + it('honours the route param when in scope', () => { + const { result } = renderHook(() => useCollectionId(), { + wrapper: wrapperFor('/p/ohm/c/features/e/login', '/p/:projectId/c/:collectionId/*'), + }) + expect(result.current).toBe('features') + }) + + it('falls back to the default collection when there is no /c/ segment', () => { + const { result } = renderHook(() => useCollectionId(), { + wrapper: wrapperFor('/p/ohm', '/p/:projectId/*'), + }) + expect(result.current).toBe('default') + }) +})