--- convoy: fix-layout-default-user brief_number: 1 depends_on: [] files: - components/Layout.js - test/components/Layout.test.js - package.json - package-lock.json cross_brief_commitments: - brief: 2 description: | Brief 2 changes the seven pages that today either omit the `user` prop or pass a hardcoded maintainer-email object. Brief 2 assumes Brief 1's new logged-out branch (default `user = null`, "Sign in" CTA in `UserProfileDropdown`) is in place — without it, those pages would render "Sign in" before their own auth state resolved, but Layout's default would silently rewrite that back to the maintainer email. Ship Brief 1 first in the diff. --- # Brief 1: Default Layout's `user` to null + render a logged-out branch + lock the contract with a vitest test ## Goal (1 sentence) Change `components/Layout.js`'s `user` default from `{ email: 'me@randallstillwell.com', role: 'user' }` to `null`, replace `UserProfileDropdown`'s avatar+email+menu with a "Sign in" link to `/login` when `user === null`, and add a vitest test under `test/components/Layout.test.js` that locks in the contract by asserting the rendered tree never contains `me@randallstillwell.com` for the no-user / null-user branches. ## Files in scope (do not edit anything else) - `components/Layout.js` — modified (default-prop fix + logged-out `UserProfileDropdown` branch). - `test/components/Layout.test.js` — new (regression test). - `package.json` — modified (add `jsdom` and `@testing-library/react` to `devDependencies`). - `package-lock.json` — regenerated by `npm install`. ## Conventions to follow - **`.cursor/rules/ui-and-theming.mdc` § Component conventions.** Already documents the intent: "Avoid hardcoded default values for `user` props. … New components must default to `null` and render a logged-out state." This brief is the first concrete application of that rule. - **`.cursor/rules/auth-and-permissions.mdc` § Authentication state on the client.** `useAuth()` returns `{ user, loading, … }` where `user === null` means logged out. Layout's null-user rendering must be safe for that case; do **not** add new logic that throws on `user === null`. - **`.cursor/rules/no-go-zones.mdc`.** Do not edit `components/Layout.js.backup`. Do not edit anything under `lib/**` or `.github/**`. Do not touch `.cursor/rules/**`. - **Brief size discipline.** This brief is one component change, one new test, two devDeps. **Do NOT**: - Split the Layout god-component (`god-component-split` convoy owns that). - Migrate Layout off `lib/auth-context.js` / `lib/admin-auth.js` (Layout doesn't import either today; both legacy providers are queued for `single-auth-provider`). - Rename "Deck Hearth" or "DH" to "TCG Vault" (`pick-a-name` convoy). - Remove the dead `user` prop on `MobileNavigation` (deferred follow-up, see convoy file § "Anything flagged but not acted on"). - Touch `components/MobileNavigation.js` at all. - **Vitest test patterns.** Match `test/lib/permission-middleware.test.js`'s shape: `describe` block per behavior cluster, `vi.mock(...)` for module dependencies, plain `expect()` matchers (no jest-dom required). The new test uses a per-file `// @vitest-environment jsdom` directive at the top (vitest v3 supports this) so `vitest.config.js`'s global `environment: 'node'` does not need to change. ## Acceptance criteria ### `components/Layout.js` - [ ] **Change the default-prop on line 562.** Before: ```js export default function Layout({ children, user = { email: 'me@randallstillwell.com', role: 'user' }, showSearch = false }) { ``` After: ```js export default function Layout({ children, user = null, showSearch = false }) { ``` No other change to that line. - [ ] **Add a logged-out branch to `UserProfileDropdown`.** The branch MUST be placed **after** the existing `useState(false)` call (rules of hooks: hooks must be called in the same order every render — moving the early return above `useState` would throw "Rendered more hooks than during the previous render" the moment `user` flips from `null` to an object on a subsequent render). Verbatim shape — the implementer MAY adjust class names to match neighboring sidebar items, but every prop / behavior must be present: ```js function UserProfileDropdown({ user, onMobileMenuClose }) { // Hook order is fixed for both branches; do not move this below the // null-user early return — see rules-of-hooks (AGENTS.md Gotcha #11.5). const [isDropdownOpen, setIsDropdownOpen] = useState(false); // Logged-out: replace avatar + email + dropdown with a Sign-in CTA. if (!user) { return (
Sign in
); } // Existing dropdown body (profileMenuItems, getProfileIcon, JSX) — // unchanged below this line. } ``` - The `useState` stays where it is today (currently line 9); the early return is inserted **between** `useState` and the existing `profileMenuItems` declaration. `setIsDropdownOpen` is unused on the null branch; that's fine — React does not warn on unused state setters, and the variable is declared because the hook MUST run. - The `` is non-negotiable — the test in `test/components/Layout.test.js` queries `getByRole('link', { name: /sign in/i })` and asserts every match has `href="/login"`. - `onClick={onMobileMenuClose}` keeps the mobile drawer behavior consistent with the existing items (the dropdown's existing items also call this on click — see line 73-74 of the current file). - The SVG is a "log-in" / "arrow-into-box" glyph (mirror of the existing logout SVG at line 39-41). Implementer MAY substitute another inline SVG so long as it is wrapped in `aria-hidden="true"` and the visible label is exactly "Sign in" (the test does a case-insensitive `/sign in/i` match — "Sign In" / "Sign in" / "SIGN IN" all work, but "Login" / "Log in" would fail the assertion). - **Lint check.** Run `npm run lint -- components/Layout.js` after the change — confirm zero new `react-hooks/rules-of-hooks` violations. Pre-existing lint baseline issues (per `AGENTS.md` Gotcha #11.5) may be present elsewhere in the file but `react-hooks/rules-of-hooks` should not regress in `UserProfileDropdown`. - [ ] **Do not modify** `NavigationContent` (lines 127-560). Its `authenticatedNavigation`, `myCollectionNavigation`, `adminNavigation` already gate on `user` correctly; the public + community sections render unconditionally and are correct for both logged-out and logged-in states. - [ ] **Do not modify** the Layout body (lines 568-793) — the desktop sidebar, mobile drawer, mobile overlay, and main-content wrapper are all unchanged. They pass `user` (now potentially `null`) to `MobileNavigation`, `NavigationContent`, and `UserProfileDropdown`; each of those handles `null` correctly after this brief. - [ ] **Do not touch** the `'me@randallstillwell.com'` literal anywhere except the line 562 default — there are no other references in `components/Layout.js` (verified by `rg "me@randallstillwell" components/Layout.js` returning a single hit before this brief). - [ ] **Branding.** Lines 593-596 ("DH" / "Deck Hearth" mobile drawer header) and 686-689 (desktop sidebar header) stay unchanged. `pick-a-name` owns branding. ### `test/components/Layout.test.js` (new) - [ ] **Create the directory** `test/components/` if it does not exist (it does not as of this brief). The vitest glob `include: ['test/**/*.test.js']` (see `vitest.config.js` line 8) automatically picks the new file up. - [ ] **Set the per-file environment** with `// @vitest-environment jsdom` as the first line. Do NOT modify `vitest.config.js`'s global `environment: 'node'` — other tests (auth, permission-middleware) run in node and changing the default would force every test through jsdom unnecessarily. - [ ] **Verbatim shape** — the implementer MAY tighten queries, but every assertion must be present and the file must run green: ```js // @vitest-environment jsdom import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, cleanup, screen } from '@testing-library/react'; // Mock next/router so useRouter() does not crash without a RouterContext. // Layout reads `router.pathname` only; the rest of the surface (`prefetch`, // `events`, `push`) is for next/link's internals — provide stubs so prefetch // does not throw when mounts. vi.mock('next/router', () => ({ useRouter: () => ({ pathname: '/', asPath: '/', query: {}, push: vi.fn(), replace: vi.fn(), prefetch: vi.fn().mockResolvedValue(undefined), events: { on: vi.fn(), off: vi.fn(), emit: vi.fn() }, }), })); // Mock next/link to a plain . The real next/link triggers prefetch on // mount via the router; bypassing it removes a class of jsdom flake without // changing the rendered DOM that the assertions inspect. vi.mock('next/link', () => ({ __esModule: true, default: ({ href, children, ...rest }) => { // children may be a single element (e.g. a
) or text; wrap in . return ( {children} ); }, })); // Mock the theme context so useTheme() does not require a ThemeProvider. vi.mock('../../lib/theme-context', () => ({ useTheme: () => ({ theme: 'light', toggleTheme: vi.fn() }), })); import Layout from '../../components/Layout'; describe('Layout — logged-out rendering (regression: P0 #7)', () => { afterEach(() => cleanup()); it('does NOT render the maintainer email when no user prop is passed', () => { const { container } = render(page body); expect(container.textContent).not.toContain('me@randallstillwell.com'); }); it('does NOT render the maintainer email when user is null', () => { const { container } = render(page body); expect(container.textContent).not.toContain('me@randallstillwell.com'); }); it('renders a Sign-in link to /login when user is null', () => { render(page body); const links = screen.getAllByRole('link', { name: /sign in/i }); expect(links.length).toBeGreaterThanOrEqual(1); // Both desktop sidebar + mobile drawer render UserProfileDropdown, // so we expect TWO Sign-in links (one per copy). for (const link of links) { expect(link.getAttribute('href')).toBe('/login'); } }); it('renders the supplied user email when user is an object', () => { const { container } = render( page body ); expect(container.textContent).toContain('foo@bar.com'); expect(container.textContent).not.toContain('me@randallstillwell.com'); }); it('does NOT render a "Guest" placeholder when logged out', () => { // Decision A says the logged-out copy is "Sign in", not "Guest". // This test prevents a future revert that ships "Guest" as the default // (which would still hide the maintainer email but skip the CTA). const { container } = render(page body); expect(container.textContent).not.toContain('Guest'); }); }); ``` - [ ] **All five tests pass** under `npm run test:run`. - [ ] **The pre-existing 24 tests** (16 in `permission-middleware.test.js`, 3 in `auth-secret.test.js`, 5 in `auth-utils.test.js`) **remain green**. This brief does not touch any file they cover, so the only failure mode is a CI environment regression — investigate and fix before merging. ### `package.json` + `package-lock.json` - [ ] **Add to `devDependencies`** — single command: `npm install --save-dev jsdom @testing-library/react`. Resolved versions at architect time (2026-05-23): `jsdom@29.1.1`, `@testing-library/react@16.3.2`. Caret ranges are fine (matches the existing devDep style — `vitest@^3.2.4`). - [ ] **Verify `@testing-library/dom` is present.** `@testing-library/react@16` declares `@testing-library/dom@^10.0.0` as a **peer dependency** (verified by `npm view @testing-library/react peerDependencies`). npm 7+ auto-installs peers, so a single `npm install --save-dev jsdom @testing-library/react` should resolve it transitively. After running install, run `npm ls @testing-library/dom` and confirm a single `^10.x` entry appears. If it does NOT (npm version too old or peer-install was disabled), explicitly add it: `npm install --save-dev @testing-library/dom@^10.0.0`. - [ ] **Do NOT add `@testing-library/jest-dom`.** The test uses plain `expect().toContain()` / `toBeGreaterThanOrEqual()` matchers — no jest-dom matchers needed. Adding it would expand the dep surface for no acceptance-criteria benefit. - [ ] **Do NOT add `@types/react`.** The repo is JavaScript-only (`AGENTS.md` Gotcha #9); the React 18/19 type peer on `@testing-library/react` is irrelevant when there is no `tsconfig.json`. npm will warn-and-skip the type peers, which is the documented behavior — leave the warning alone. - [ ] **Do NOT bump any other dep** while you have `package.json` open. No `next` bump, no `vitest` bump, no react bump. If npm hoists a transitive minor that flips a lockfile entry, that's fine; if it tries to bump a top-level dep, stop and ask. - [ ] **Verify** `package-lock.json` was regenerated and committed (the `npm install` run produces the lockfile diff; commit it as part of the same change). ### `vitest.config.js` - [ ] **No change.** The new test file uses `// @vitest-environment jsdom` per-file. The global `environment: 'node'` (line 5) stays so the existing 24 tests do not slow down by going through jsdom unnecessarily. - [ ] (Sanity check, not a code change.) Confirm the existing `include: ['test/**/*.test.js']` glob (line 8) catches `test/components/Layout.test.js`. It does — the test file matches the recursive glob. ### Smoke (manual, in addition to the vitest run) Run these in order; paste the relevant output / screenshots into the PR description: - [ ] **Logged-out smoke.** With no `auth_token` in `localStorage` (DevTools → Application → Local Storage → clear `auth_token`), `npm run dev` and visit `http://localhost:3000/dashboard` (or any page that renders Layout). Expect: - The desktop sidebar bottom shows "Sign in" with a small icon, in place of the avatar + email + dropdown chevron. - Clicking "Sign in" routes to `/login`. - **The string `me@randallstillwell.com` does NOT appear anywhere on the page.** (Open DevTools → Console → run `document.body.innerText.includes('me@randallstillwell.com')` → expect `false`.) - The "My Collection", admin, and authenticated-only nav items are hidden. - Public + community nav items (Cards, Scanner, Deck Builder placeholder, Community section) are still visible. - [ ] **Logged-in smoke.** Log in as a non-admin (`admin@tcgvault.com` works since the seed admin has `role='admin'` — use a non-admin signup or temporarily flip the row). Visit `/dashboard`. Expect: - The desktop sidebar bottom shows the real user's email + role. - Clicking the avatar opens the dropdown with Profile / Settings / Logout (no Admin Panel for non-admin). - [ ] **Mobile drawer smoke.** Resize Chrome DevTools to a phone preset (iPhone 13). Open the mobile drawer ("More" tap). Expect: the same Sign-in / authenticated states render in the drawer's bottom section as in the desktop sidebar. - [ ] **Lint.** `npm run lint` exits 0 (or matches the existing pre-PR baseline — pre-existing lint errors are fine; do not introduce new ones). - [ ] **Tests.** `npm run test:run` is green: 5 new + 24 pre-existing = 29 tests pass. ### Out of scope (do not do these) - [ ] No edits to any of the 17 pages — that's Brief 2. - [ ] No edits to `components/MobileNavigation.js` — its dead `user` prop is a deferred cleanup. - [ ] No edits to `lib/use-auth.js`, `lib/auth-context.js`, `lib/admin-auth.js`, `lib/permission-middleware.js`, or `lib/auth-secret.js`. The auth surface is downstream of this brief. - [ ] No edits to `vitest.config.js` (the per-file `// @vitest-environment jsdom` directive is the entire mechanism). - [ ] No new vitest tests beyond `test/components/Layout.test.js`. Brief 2 does not add tests either (Decision D = D2 covers the contract at the component boundary). - [ ] No `.cursor/rules/*.mdc` updates. The rule (`ui-and-theming.mdc` § Component conventions) already documents the intent. - [ ] No `AGENTS.md` Gotcha #8 update — that's the post-convoy doc-writer pass. - [ ] No `README.md` update. - [ ] No `CHANGELOG.md` (none exists yet — `adopt-keep-a-changelog` is a separate convoy). ## Rationale (≤3 sentences) The Layout default is the actual P0 #7 bug; this brief fixes it at the source and locks in the contract with a regression test that asserts the maintainer email never reappears for the null-user branches. Splitting the page audit into Brief 2 keeps the conceptually interesting change (Layout + logged-out branch + test harness) reviewable on its own; the page edits are mechanical and benefit from being grouped separately. Adding `jsdom` + `@testing-library/react` as devDeps is the smallest harness that lets vitest exercise React rendering — the tools are common, the surface is two packages, and the value (preventing a recurrence of a default-prop email leak) is high.