Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions docs/INCREMENTAL_CHANGE_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand All @@ -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 |
Expand Down Expand Up @@ -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:
Expand All @@ -254,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 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`.

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.
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)

Expand Down
50 changes: 40 additions & 10 deletions docs/PLAN_OF_ACTION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -245,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:

Expand Down Expand Up @@ -475,7 +505,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
Expand All @@ -493,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`
35 changes: 34 additions & 1 deletion packages/web/src/pages/Home.test.tsx
Original file line number Diff line number Diff line change
@@ -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 = [
Expand All @@ -20,8 +20,11 @@ function renderHome() {
)
}

let consoleErrorSpy: ReturnType<typeof vi.spyOn>

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) })
Expand All @@ -30,6 +33,10 @@ beforeEach(() => {
})
})

afterEach(() => {
consoleErrorSpy.mockRestore()
})

describe('Home — search and genre filter', () => {
it('renders all movies on initial load', async () => {
renderHome()
Expand Down Expand Up @@ -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))
})
})
11 changes: 7 additions & 4 deletions packages/web/src/pages/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,23 @@ function Home() {
const [genres, setGenres] = useState<string[]>([]);

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);
});
}, []);
Comment on lines 22 to 40
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The useEffect hook initiates asynchronous operations (fetches) but does not provide a cleanup function. This can lead to attempts to update state on an unmounted component if the component unmounts before the fetches complete, which will cause a memory leak and a warning from React.

It's a best practice to clean up effects that involve subscriptions or async requests. You can use an AbortController to cancel the fetch requests when the component unmounts. This will also require checking for AbortError in your catch blocks to avoid logging canceled requests as errors.

  useEffect(() => {
    const controller = new AbortController();
    const { signal } = controller;

    fetch('/api/movies', { signal })
      .then(res => res.json())
      .then((data: Movie[]) => {
        setMovies(data);
        setLoading(false);
      })
      .catch((err: any) => {
        if (err.name !== 'AbortError') {
          console.error('Failed to fetch movies:', err);
          setLoading(false);
        }
      });

    fetch('/api/movies/meta/genres', { signal })
      .then(res => res.json())
      .then((data: string[]) => setGenres(data))
      .catch((err: any) => {
        if (err.name !== 'AbortError') {
          console.error('Failed to fetch genres:', err);
        }
      });

    return () => {
      controller.abort();
    };
  }, []);


const filteredMovies = useMemo(() => {
Expand Down
Loading