From 53ca676baa063f2ee9be1b8c30d5e01d5b1328f5 Mon Sep 17 00:00:00 2001 From: Mac Date: Wed, 18 Feb 2026 12:34:27 +0530 Subject: [PATCH 1/4] fix(Home): log fetch errors instead of silently swallowing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both fetch calls used empty .catch() — failures were invisible in dev and production logs. Now logs with console.error. Also removes void keyword from fetch calls per Gemini code review feedback (PR #5). --- packages/web/src/pages/Home.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/web/src/pages/Home.tsx b/packages/web/src/pages/Home.tsx index b321e2d..d5bd713 100644 --- a/packages/web/src/pages/Home.tsx +++ b/packages/web/src/pages/Home.tsx @@ -20,20 +20,23 @@ function Home() { const [genres, setGenres] = useState([]); useEffect(() => { - void fetch('/api/movies') + fetch('/api/movies') .then(res => res.json()) .then((data: Movie[]) => { setMovies(data); setLoading(false); }) - .catch(() => { + .catch((err: unknown) => { + console.error('Failed to fetch movies:', err); setLoading(false); }); - void fetch('/api/movies/meta/genres') + fetch('/api/movies/meta/genres') .then(res => res.json()) .then((data: string[]) => setGenres(data)) - .catch(() => {}); + .catch((err: unknown) => { + console.error('Failed to fetch genres:', err); + }); }, []); const filteredMovies = useMemo(() => { From 1fa5efc85d5df9cbf4e238f15b932d85b38c9bfa Mon Sep 17 00:00:00 2001 From: Mac Date: Wed, 18 Feb 2026 12:35:06 +0530 Subject: [PATCH 2/4] test(Home): assert console.error called on fetch failures Two new tests verify that failed fetch calls log errors via console.error rather than silently swallowing them. --- packages/web/src/pages/Home.test.tsx | 35 +++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/web/src/pages/Home.test.tsx b/packages/web/src/pages/Home.test.tsx index e463c19..6b0764b 100644 --- a/packages/web/src/pages/Home.test.tsx +++ b/packages/web/src/pages/Home.test.tsx @@ -1,7 +1,7 @@ import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { MemoryRouter } from 'react-router-dom' -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import Home from './Home' const mockMovies = [ @@ -20,8 +20,11 @@ function renderHome() { ) } +let consoleErrorSpy: ReturnType + beforeEach(() => { vi.restoreAllMocks() + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) global.fetch = vi.fn().mockImplementation((url: string) => { if (url.includes('/meta/genres')) { return Promise.resolve({ json: () => Promise.resolve(mockGenres) }) @@ -30,6 +33,10 @@ beforeEach(() => { }) }) +afterEach(() => { + consoleErrorSpy.mockRestore() +}) + describe('Home — search and genre filter', () => { it('renders all movies on initial load', async () => { renderHome() @@ -80,4 +87,30 @@ describe('Home — search and genre filter', () => { expect(screen.queryByText('The Dark Knight')).not.toBeInTheDocument() expect(screen.getByText('1 movies found')).toBeInTheDocument() }) + + it('logs console.error when movies fetch fails', async () => { + const fetchError = new Error('Network error') + global.fetch = vi.fn().mockImplementation((url: string) => { + if (url.includes('/meta/genres')) { + return Promise.resolve({ json: () => Promise.resolve(mockGenres) }) + } + return Promise.reject(fetchError) + }) + + renderHome() + await waitFor(() => expect(consoleErrorSpy).toHaveBeenCalledWith('Failed to fetch movies:', fetchError)) + }) + + it('logs console.error when genres fetch fails', async () => { + const fetchError = new Error('Network error') + global.fetch = vi.fn().mockImplementation((url: string) => { + if (url.includes('/meta/genres')) { + return Promise.reject(fetchError) + } + return Promise.resolve({ json: () => Promise.resolve(mockMovies) }) + }) + + renderHome() + await waitFor(() => expect(consoleErrorSpy).toHaveBeenCalledWith('Failed to fetch genres:', fetchError)) + }) }) From 26faad8a443888ac8bd281bc5c7213778b98a691 Mon Sep 17 00:00:00 2001 From: Mac Date: Wed, 18 Feb 2026 12:35:41 +0530 Subject: [PATCH 3/4] =?UTF-8?q?docs:=20add=20Phase=202c+=20to=20both=20pla?= =?UTF-8?q?n=20docs=20=E2=80=94=20Gemini=20fetch=20error=20follow-up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the fetch error logging step (remove void, log catch errors) that was flagged in the Gemini code review on PR #5. --- docs/INCREMENTAL_CHANGE_PLAN.md | 28 ++++++++++++++++++++++++++++ docs/PLAN_OF_ACTION.md | 31 ++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/docs/INCREMENTAL_CHANGE_PLAN.md b/docs/INCREMENTAL_CHANGE_PLAN.md index 987f621..df06d6a 100644 --- a/docs/INCREMENTAL_CHANGE_PLAN.md +++ b/docs/INCREMENTAL_CHANGE_PLAN.md @@ -79,6 +79,7 @@ Each session is self-contained. Start a new Claude Code session, tell it to work | **ci** | `ci/github-actions` | `.github/workflows/ci.yml` (new) | `ci: add GitHub Actions pipeline — lint, test, coverage` | | **2b** | `feat/error-boundary` | `src/ErrorBoundary.tsx` (new), `App.tsx`, `ErrorBoundary.test.tsx` (new) | `feat: add ErrorBoundary around route tree` | | **2c** | `perf/remove-blocking-search` | `Home.tsx`, `Home.test.tsx` | `perf: remove blocking while loop, add useMemo and useCallback` | +| **2c+** | `fix/home-fetch-errors` | `Home.tsx`, `Home.test.tsx` | `fix: log fetch errors instead of silently swallowing them` | | **2d** | `fix/auth-security` | `server/src/routes/auth.ts`, `server/src/seed.ts`, `.env.example`, `AuthContext.tsx`, `auth.test.ts`, `auth.integration.test.ts`, `AuthContext.test.tsx` | `fix: hash passwords, move JWT secret to env, add rate limiting, persist token` | | **2e** | `feat/memoria-design-system` | All `pages/*.tsx`, `App.tsx`, `index.css`, snapshot tests | `feat: apply Memoria dark design system across all pages` | | **2f** | `refactor/code-quality` | `server/src/routes/*.ts`, `server/src/constants.ts` (new), `server/src/seed.ts` | `refactor: typed routes, extract magic strings, add seed guard` | @@ -104,6 +105,7 @@ The session will have everything it needs from those two docs. | ci | CI pipeline — gates all subsequent PRs | 20 min | | 2b | Add ErrorBoundary + unit tests | 20 min | | 2c | Fix performance + unit tests | 20 min | +| 2c+ | Fix silent fetch error swallowing (Gemini follow-up) | 10 min | | 2d | Fix security + unit + integration tests | 60 min | | 2e | Apply Memoria design system + snapshot tests | 2 hrs | | 2f | Code quality cleanup | 30 min | @@ -238,6 +240,32 @@ The blocking `while (Date.now() - start < 2000)` loop in `Home.tsx` freezes the **Tests to write (same PR):** - `Home.test.tsx`: search filter returns correct subset without blocking; genre filter returns correct subset; combined search + genre filter works correctly +### 2c+ — Fetch Error Logging (Gemini review follow-up) + +> Flagged in the Gemini code review on PR #5 ([inline comment on `Home.tsx` line 36](https://github.com/wednesday-solutions/ai-enablement-workshop/pull/5)). + +Both `fetch` calls in `Home.tsx` currently use `.catch(() => {})` — a silent no-op that makes failures invisible during debugging and in production logs. Replace with a logged catch: + +```tsx +// Before +.catch(() => {}) + +// After (movies fetch) +.catch((err: unknown) => { + console.error('Failed to fetch movies:', err); +}) + +// After (genres fetch) +.catch((err: unknown) => { + console.error('Failed to fetch genres:', err); +}) +``` + +Also remove the `void` keyword from the `fetch(...)` call on line 23 — it is non-idiomatic and unnecessary (Gemini comment on line 23). + +**Tests to write (same PR):** +- `Home.test.tsx`: when movies fetch fails, `console.error` is called with the error; when genres fetch fails, `console.error` is called with the error + ### 2d — Security (Priority: High) Work through these one at a time — each is its own commit: diff --git a/docs/PLAN_OF_ACTION.md b/docs/PLAN_OF_ACTION.md index d150f9a..5bd2c6d 100644 --- a/docs/PLAN_OF_ACTION.md +++ b/docs/PLAN_OF_ACTION.md @@ -234,6 +234,35 @@ git commit -m "chore: add ESLint, SonarJS, Prettier, Commitlint, Husky — quali | 2.3 | `Home.tsx` | Wrap filter logic in `useMemo`, wrap handlers in `useCallback` | | 2.2 | `Home.tsx` | Add client-side pagination (show 12 at a time, "Load More") | +### 2c+ — Fetch Error Logging (Gemini review follow-up) + +> Flagged in the Gemini code review on PR #5 — two inline comments on `Home.tsx`. + +| Ref | File | Fix | +|-----|------|-----| +| G1 | `Home.tsx` line 23 | Remove `void` from `fetch(...)` calls — non-idiomatic in TypeScript | +| G2 | `Home.tsx` line 36 | Replace silent `.catch(() => {})` with logged catches — silent no-ops hide errors in prod | + +```tsx +// Before +.catch(() => {}) + +// After (movies fetch) +.catch((err: unknown) => { + console.error('Failed to fetch movies:', err); +}) + +// After (genres fetch) +.catch((err: unknown) => { + console.error('Failed to fetch genres:', err); +}) +``` + +**Tests to write (same PR):** +- `Home.test.tsx`: when movies fetch fails, `console.error` is called with the error; when genres fetch fails, `console.error` is called with the error + +--- + ### Priority 3 — Security | Ref | File | Fix | @@ -475,7 +504,7 @@ Phase 0 Understand Read code, reproduce bugs, review docs/ISSUES_IN_THE Phase 1 Tooling ESLint + SonarJS + Prettier + Commitlint + Husky; lint baseline Phase 2a Fix + Tests Crash fixes; unit tests in the same PR Phase 2.ci CI Pipeline GitHub Actions — gates every subsequent PR automatically -Phase 2b–f Fix + Tests ErrorBoundary → Performance → Security → Memoria → Quality +Phase 2b–f Fix + Tests ErrorBoundary → Performance → Fetch error logging → Security → Memoria → Quality Each PR includes unit + integration tests for its own changes Phase 3 E2E Tests Playwright; full user journeys in a real browser Phase 4 Deploy Docker + docker-compose + deploy job in CI From 976bd1ef5840d65fb0cc0336809e112f008c9391 Mon Sep 17 00:00:00 2001 From: Mac Date: Wed, 18 Feb 2026 12:37:12 +0530 Subject: [PATCH 4/4] docs: replace Memoria with light green design system in Phase 2e References docs/DESIGN_SYSTEM_GUIDE.md instead of the Memoria gist. Updates page-by-page breakdown and checklist to match the light green system (warm neutrals, green-to-teal gradient, Instrument Serif, DM Sans). --- docs/INCREMENTAL_CHANGE_PLAN.md | 24 +++++++++++++++--------- docs/PLAN_OF_ACTION.md | 19 ++++++++++--------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/INCREMENTAL_CHANGE_PLAN.md b/docs/INCREMENTAL_CHANGE_PLAN.md index df06d6a..dd2eded 100644 --- a/docs/INCREMENTAL_CHANGE_PLAN.md +++ b/docs/INCREMENTAL_CHANGE_PLAN.md @@ -282,21 +282,27 @@ Work through these one at a time — each is its own commit: - `auth.integration.test.ts` (integration): full signup → login → access protected route flow against in-memory SQLite; confirms password is hashed in the DB - `AuthContext.test.tsx` (unit): login stores token in localStorage; logout clears localStorage; page reload rehydrates user from localStorage -### 2e — Memoria Design System (Priority: Medium) +### 2e — Light Green Design System (Priority: Medium) -Apply the [Memoria design system](https://gist.github.com/alichherawalla/8234538a50f9d089e0159c3e3634e17c) across the frontend. The core principles: dark monochromatic palette (`bg-neutral-950` base), numbers as heroes (`font-light text-white`), staggered animations via `framer-motion`, no accent colours, no inline styles. +Apply the design system defined in [`docs/DESIGN_SYSTEM_GUIDE.md`](./DESIGN_SYSTEM_GUIDE.md) across the frontend. Core principles: light warm-neutral base (`#FFFFFF` / `#FAFAFA`), green-to-teal brand gradient used sparingly for accents and CTAs, `Instrument Serif` for headlines + `DM Sans` for body, multi-layer shadows for card depth, staggered animations via `framer-motion`. + +Install fonts in `index.html` (Google Fonts import) and `framer-motion`: + +```bash +pnpm --filter @stagepass/web add framer-motion +``` Tackle one page per commit: -1. Global theme + remove legacy CSS -2. Header (extract to its own component) -3. Home page — dark card grid, staggered entry, hero rating -4. Movie Detail — blur-in transition, dark showtime pills -5. Seat Selection — dark grid, white selected state -6. My Bookings — hero amount, dark cards +1. Global theme — fonts, CSS variables from the design system, remove legacy inline styles +2. Header — extract to `components/Header.tsx`, apply nav styling from the guide +3. Home page — white card grid with shadow lifts, brand gradient rating badge, staggered card entry +4. Movie Detail — serif headline, blur-in transition, green pill showtimes +5. Seat Selection — neutral grid, green selected state, brand gradient confirm button +6. My Bookings — stat-display booking amount, white lifted cards 7. Booking Confirmation — spring modal entry animation **Tests to write (same PR):** -- Snapshot tests for each page after styling is applied — one snapshot per page component. These lock in the design system classes and will fail if a future change accidentally removes Memoria styling. +- Snapshot tests for each page after styling is applied — one snapshot per page component. These lock in the design system styles and will fail if a future change accidentally removes them. ### 2f — Code Quality (Priority: Low) diff --git a/docs/PLAN_OF_ACTION.md b/docs/PLAN_OF_ACTION.md index 5bd2c6d..131f178 100644 --- a/docs/PLAN_OF_ACTION.md +++ b/docs/PLAN_OF_ACTION.md @@ -274,18 +274,19 @@ git commit -m "chore: add ESLint, SonarJS, Prettier, Commitlint, Husky — quali | 3.6 | `AuthContext.tsx` | Persist token to `localStorage`, rehydrate on mount | | 4.6 | All | Add `.env` + `dotenv`, add `.env.example`, add `.env` to `.gitignore` | -### Priority 4 — Design System (Memoria) +### Priority 4 — Design System (Light Green) > This is the "Write" phase showpiece. Transform the UI from ugly to polished. -Apply the [Memoria design system](https://gist.github.com/alichherawalla/8234538a50f9d089e0159c3e3634e17c) across all pages: +Apply the design system defined in [`docs/DESIGN_SYSTEM_GUIDE.md`](./DESIGN_SYSTEM_GUIDE.md) across all pages. Core principles: light warm-neutral base (`#FFFFFF` / `#FAFAFA`), green-to-teal brand gradient used sparingly for CTAs and accents, `Instrument Serif` for headlines + `DM Sans` for body, multi-layer card shadows, staggered scroll-driven animations. -- **Global:** Switch to `bg-neutral-950` base, `text-white/neutral-*` hierarchy, no accent colors -- **Home:** Dark card grid with poster, rating as hero number, staggered entry animation -- **MovieDetail:** Split layout with blur-in animation, showtimes as styled pill buttons -- **SeatSelection:** Dark grid, green/red/blue seat states, screen label at top -- **MyBookings:** Timeline-style booking cards, booking amount as hero number -- **All modals/overlays:** `BorderBeam` effect, spring-based 3D entry animation +- **Global:** CSS variables from the design guide, Google Fonts import (`Instrument Serif` + `DM Sans`), remove legacy inline styles +- **Header:** Extract to `components/Header.tsx`, apply nav layout from the guide +- **Home:** White card grid with shadow lift on hover, brand gradient rating badge, staggered card entry animation +- **MovieDetail:** Serif headline, blur-in transition, green pill showtime buttons +- **SeatSelection:** Neutral grid, green selected state, brand gradient confirm button +- **MyBookings:** Stat-display booking amount (large number, small label), lifted white cards +- **BookingConfirmation:** Spring modal entry animation Install animation library: @@ -522,7 +523,7 @@ The codebase is considered "done" when: - [ ] Commitlint + Husky enforce commit format and lint on every commit - [ ] Server test coverage ≥ 70%, web ≥ 60% - [ ] E2E tests cover the full booking flow in a real browser -- [ ] Memoria design system applied to all 5 pages +- [ ] Light green design system (docs/DESIGN_SYSTEM_GUIDE.md) applied to all pages - [ ] Passwords hashed, JWT secret in `.env`, rate limiting on auth - [ ] CI pipeline is in place from Phase 2.ci and passes on every push to `main` - [ ] App runs via `docker compose up`