batchCooking/README.md
kyuno053 de63be7ba4
Centralize error handling + code quality pass (comments, SCSS theming) (#7)
* Centralize error handling (shared codes + API/client services), code quality pass

## Error handling

Requested: a centralized error-handling service on the API, custom error
codes shared across apps, and a client-side error service for i18n labels.

- packages/shared/src/errors/error-codes.ts — ErrorCode enum + ApiErrorResponse
  contract. Single source of truth: neither side hardcodes a raw error string
  the other has to guess at.
- apps/api: HttpError now carries an ErrorCode (not just a message).
  ErrorHandlerService (new) centralizes every "how do we turn a thrown error
  into an HTTP response" decision — app.ts's error middleware is now a thin
  adapter calling into it. API messages reverted to English/dev-facing (they
  were French from an earlier pass) since user-facing text is now generated
  client-side from the code.
- apps/web: ApiClient (class, singleton instance) throws ApiError carrying
  the code. ErrorMessageService (new) maps every ErrorCode to a localized
  label, structured with a Locale type from the start (only "fr" exists, but
  adding a language later is "add a locale to the map", not "hunt down every
  hardcoded string"). LoginPage/SignupPage now display
  errorMessageService.getLabel(err.code), never err.message directly.
- Tests strengthened to assert on `code`, not just HTTP status (Mocha +
  Cucumber, new "the response error code should be" step). Cypress mocks
  updated to the new {code, message} response shape.

## Code quality pass

Per explicit feedback: heavy JSDoc on every interface/type/class/function/
method/member touched in this PR, explicit public/private visibility on
every class member (ApiClient, ErrorMessageService, ErrorHandlerService,
HttpError), no HTML/logic mixing (styling extracted out of components
entirely, never inline).

ApiClient/ErrorMessageService were initially written as static-only classes;
switched to instance-based singletons (matching ErrorHandlerService's
existing pattern) after Biome's noStaticOnlyClass rule flagged the
static-only shape as an anti-pattern — same "class with visibility
modifiers" outcome, without fighting the linter.

## SCSS + theming

- apps/web/src/styles/_theme.scss — design tokens as CSS custom properties
  on :root (colors, spacing, typography), not plain Sass variables — makes
  them available at runtime, not just compile time, so a future theme
  switch (e.g. dark mode) is "redefine these variables" rather than
  rebuilding stylesheets.
- apps/web/src/styles/global.scss replaces the old single index.css:
  reset + theme import only, loaded once from main.tsx.
- Per-page/component styles colocated (HomePage.tsx + HomePage.scss);
  styles shared by multiple pages within one feature live in that feature's
  folder (features/auth/auth-form.scss, used by both Login/SignupPage) —
  not duplicated per page, not dumped in the global stylesheet either.
- Component-level .scss files intentionally don't `@use` the theme
  partial: they only consume CSS custom properties (global at runtime via
  global.scss), not Sass-level symbols, so importing it would do nothing —
  documented inline rather than left as a silently-redundant import.
- vite.config.ts opts into Sass's modern compiler API to silence a
  legacy-js-api deprecation warning on every build.

## specs/ updates

- New specs/error-handling.md — the ErrorCode/ApiErrorResponse contract,
  both services, with a flow diagram.
- New specs/frontend-architecture.md — apps/web folder structure, routing/
  auth-guard flow, SCSS/theming conventions.
- specs/batch-cooking-architecture.md links to both (original doc content
  otherwise untouched — it's the user's own hand-authored source doc).

## Verification

Full lint/mocha/cucumber/build green. Manually re-verified the whole auth
flow in a real browser against native dev servers (not just the automated
suites): signup, the EMAIL_ALREADY_IN_USE → "Cet email est déjà utilisé"
translation end-to-end (confirmed the raw API response carries the English
dev message + code, and the UI shows the French label), wrong-password
INVALID_CREDENTIALS → its label, and confirmed the theme tokens actually
apply (computed button background-color matches --color-primary, card
max-width matches the token value) rather than trusting the build succeeding.

* Address review: no .d.ts, express-tools package, faker fixtures, numeric codes, real i18n lib

Five explicit review points, addressed on this same PR branch (not a new
PR) per updated preference.

## No .d.ts files in the codebase

- apps/web: vite-env.d.ts removed — its /// <reference types="vite/client" />
  is replaced by "types": ["vite/client"] in tsconfig.app.json, same effect.
- apps/api: src/types/express.d.ts renamed to express-request.augment.ts —
  `declare global` module augmentation works identically in a plain .ts
  file as long as it has a top-level import (making it a module); the
  .d.ts extension wasn't doing anything for us here.

## packages/express-tools — separate package for Express tooling

Moved HttpError and ErrorHandlerService out of apps/api into a new
workspace package, plus a new createErrorMiddleware() factory (the actual
Express 4-arg error-handling middleware, previously inlined in app.ts).
apps/api now just consumes @batch-cooking/express-tools. Has a real build
(tsc -> dist/, same pattern as packages/shared) — required for the same
reason shared needed one: apps/api's Docker image runs plain `node
dist/server.js`, no tsx. apps/api/Dockerfile updated to COPY the new
package's dist alongside shared's.

## faker.js for test fixtures

apps/api/test/auth.test.ts: replaced the hardcoded "Nicolas
Lefevre"/nicolas@example.com fixture (looked like real user data) with
@faker-js/faker, generated fresh per test via buildSignupPayload().
features/step-definitions/auth.steps.ts: fakerized the filler
firstName/lastName/password used for background state the scenarios
don't actually read.

Deliberately did NOT fakerize the literal example values inside
auth.feature itself (alice@example.com etc.) — those are the readable,
illustrative Gherkin examples that are the whole point of BDD scenarios,
not real PII, and randomizing them would make the scenarios harder to
read for no real gain. Flagged this reasoning in the README in case that
call should go the other way.

Caught a real bug while wiring this up: faker.internet.email() sometimes
capitalizes parts of the address, but signupSchema/loginSchema normalize
emails to lowercase — the test fixture needs to match what's actually
stored, so buildSignupPayload() lowercases the generated email too.
Found by actually running the suite repeatedly, not just once.

## ErrorCode: numeric enum, zero hardcoded values

packages/shared/src/errors/error-codes.ts: ErrorCode is now a numeric
enum (4000 VALIDATION_ERROR, 4001 EMAIL_ALREADY_IN_USE, 4010
INVALID_CREDENTIALS, 4011 NOT_AUTHENTICATED, 4040 NOT_FOUND, 5000
INTERNAL_ERROR — grouped by family like HTTP status codes).

Audited and fixed every place that hardcoded a raw code value instead of
referencing the enum: ApiClient's fallback (`"INTERNAL_ERROR" as
ErrorCode` — would no longer even type-check once the enum went numeric,
which is exactly the point), and the Cypress mock bodies (now import
ErrorCode from @batch-cooking/shared instead of typing the string).

Cucumber's "the response error code should be {string}" step still takes
the *name* in the .feature file (readable: "EMAIL_ALREADY_IN_USE") and
resolves it to the real numeric value via ErrorCode[name] — TypeScript's
reverse enum mapping — before comparing, so the Gherkin stays readable
without the step hardcoding a number either.

## Real i18n library (i18next), not a hand-rolled label map

apps/web: added i18next + react-i18next. New locales/fr/translation.json
holds every user-facing string — not just error labels (errors.*), but
the login/signup/home pages' labels, buttons and headings too
(auth.login.*, auth.signup.*, home.*) — via useTranslation()/t() in each
page. ErrorMessageService no longer owns its own label map; it converts
the numeric ErrorCode to its enum member name and delegates the actual
lookup to i18next (errors.<MEMBER_NAME>). Adding a language is now
"add a locale file", not a code change anywhere.

## specs/ and README updated

specs/error-handling.md and specs/frontend-architecture.md rewritten for
the new package, numeric codes, and i18next. New "i18n" and "no .d.ts"
sections. README covers the same, plus a note on the faker.js scope
decision (feature-file literals excluded, on purpose).

## Verification

Full lint/mocha (x3 runs)/cucumber/build green. Re-verified
express-tools' extraction against a real risk (not just tsc passing):
ran `node dist/server.js` standalone (mirrors the Docker runtime, no
tsx) and hit /health, a 404 (confirmed numeric code 4040 over the wire),
and a real signup + duplicate-email 409 (confirmed numeric 4001). Then
re-verified the full pipeline in a real browser against native dev
servers: signup, EMAIL_ALREADY_IN_USE -> i18next -> "Cet email est déjà
utilisé" end-to-end, home page i18next interpolation
({{firstName}}/{{lastName}}) rendering correctly.

* Address second review round: interface comments, res.locals, ExpressServer, assertIsNever

Five more explicit review points, on the same PR branch.

## Every interface key commented

Audited all 6 interfaces in the codebase. Two had partially-commented
members (violates the "every key gets /** */" rule): AuthResult
(apps/api/auth.service.ts) and SafeUserProfile (packages/shared) — both
now fully commented. The other four (AuthTokenPayload,
AuthContextValue, ErrorHandlingResult, ApiErrorResponse) were already
compliant.

## Removed the Express namespace augmentation

apps/api/src/types/express.d.ts (renamed to express-request.augment.ts
in the last round) is gone entirely. requireAuth now attaches the
authenticated profile to `res.locals.userProfile` — Express's own
built-in per-request mechanism for exactly this — typed via a new
AuthLocals interface and `Response<unknown, AuthLocals>`, instead of a
project-wide `declare global` silently changing every Request's type
whether or not it went through the middleware.

## ErrorHandlerService confirmed framework-agnostic

It already had zero Express import. Documented this explicitly (in the
package's index.ts and the new backend-architecture.md spec) as a
deliberate split: ErrorHandlerService is framework-agnostic (would work
behind Fastify too), ExpressServer/createErrorMiddleware are the actual
Express integration layer.

## packages/express-tools: server init + route/middleware utilities

New ExpressServer class, modeled on the pattern shared as a reference
(adapted, not copied 1:1 — deliberately left out the reference's custom
runtime param-type-validation system, since zod already does that job
in this codebase and running two parallel validation mechanisms would
be redundant, not "propre"):
- setupCore() — the common cors/json/cookie-parser stack
- addRoute() — registers a route, warns+skips instead of silently
  double-registering the same method+path
- addMiddleware() / mountRouter() / setErrorHandler()
- listen()
- .instance — the raw Express app, for supertest

Also added wrapAsyncHandler() — forwards a thrown/rejected error from an
async handler to next(err) automatically, removing the manual
try/catch/next(err) every route needed.

apps/api/src/app.ts now builds via ExpressServer (createServer(),
consumed by both server.ts's .listen() and createApp()'s .instance for
tests). auth.routes.ts's signup/login handlers use wrapAsyncHandler
instead of manual try/catch. cookie-parser/cors moved out of apps/api's
own dependencies entirely — they're express-tools' concern now.

## assertIsNever (packages/shared/src/tools/)

Exhaustiveness-check helper for switch/if-chains over a union: takes a
`never`-typed value and throws, so a forgotten case in a later-added
union member becomes a compile error instead of a silent runtime
fallthrough. Verified for real (not just written and assumed correct):
wrote a throwaway switch missing a case and confirmed `tsc` rejects it
with the exact expected error, then deleted the scratch file. No
existing switch/if-chain over a union in the codebase yet to retrofit
it into — noted as ready for when one appears (e.g. the not-yet-built
batch-cooking calculation module or recipe-import pipeline).

## specs/ updated

New specs/backend-architecture.md — ExpressServer, wrapAsyncHandler,
the res.locals decision (with the "why not declare global" reasoning
spelled out), assertIsNever. error-handling.md and
frontend-architecture.md cross-link to it instead of duplicating.
README covers the same, briefly.

## Verification

Full lint/mocha/cucumber/build green. Re-ran `node dist/server.js`
standalone (mirrors Docker, no tsx) after the ExpressServer refactor:
/health, a 404 (numeric 4040), and a real signup + GET /me round trip
confirming res.locals-based auth actually works at runtime, not just
that tsc accepts the types.

* refactor: move ErrorHandlerService/HttpError out of express-tools

ErrorHandlerService has zero dependency on Express — it's a plain
"map an error to {status, body}" service that works identically
behind any HTTP framework. It had no business living in a package
named express-tools.

Extracted HttpError, ErrorHandlerService, and ErrorHandlingResult
into a new packages/error-tools package (same tsc-build-to-dist
pattern as shared/express-tools). express-tools now only keeps the
actual Express-specific layer: ExpressServer, wrapAsyncHandler, and
createErrorMiddleware (which adapts ErrorHandlerService, imported
from error-tools, onto Express).

- packages/error-tools: new package, depends on shared + zod
- packages/express-tools: drops zod dependency, adds error-tools
  dependency for error-middleware.ts's type import
- apps/api: adds error-tools dependency; app.ts, auth.service.ts,
  require-auth.ts now import HttpError/errorHandlerService from
  error-tools instead of express-tools
- apps/api/Dockerfile: adds COPY for packages/error-tools in the
  runtime stage
- specs/error-handling.md, specs/backend-architecture.md, README.md
  updated to reflect the new package split

Verified: pnpm lint, pnpm build (all packages, correct dependency
order), pnpm test (9/9 Mocha), pnpm test:bdd (5/5 Cucumber), full
Docker rebuild + compose up (no crash-loop), curl + browser checks
of /health, unknown-route 404, signup (201), duplicate-email 409
(code 4001 EMAIL_ALREADY_IN_USE) — all going through the moved
ErrorHandlerService/HttpError correctly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-16 18:40:53 +02:00

12 KiB

batchCooking

Structure

Monorepo pnpm workspaces :

  • apps/api — backend Express/TypeScript (squelette générique : healthcheck, config env, Prisma non modélisé, tests Mocha + Cucumber/BDD)
  • apps/web — frontend React/Vite/TypeScript, prêt à être embarqué par Capacitor plus tard. Page de connexion/inscription en place ; le reste est encore un squelette générique.
  • packages/shared — code partagé entre api et web : schémas zod (signupSchema, loginSchema), types (SafeUserProfile), et le contrat d'erreurs (ErrorCode numérique, ApiErrorResponse, voir specs/error-handling.md) — même règles des deux côtés, pas de risque de dérive entre front et back.
  • packages/error-tools — gestion des erreurs, indépendante de tout framework HTTP (n'importe pas express) : HttpError, ErrorHandlerService. Séparé d'express-tools précisément parce que rien ici ne dépend d'Express. Détail : specs/error-handling.md.
  • packages/express-tools — outillage Express générique et réutilisable : ExpressServer (init serveur, routes, middlewares), wrapAsyncHandler, createErrorMiddleware (adapte ErrorHandlerService de error-tools à Express) — séparé d'apps/api, pas de logique métier. Détail : specs/backend-architecture.md.

packages/shared, packages/error-tools et packages/express-tools ont un vrai build (tscdist/, voir leur package.json) : consommés en JS compilé, pas en TS brut — nécessaire pour un runtime Node pur (Docker, pas de transpilation à la volée), voir la note dans specs/frontend-architecture.md.

Prérequis

  • Node.js 22 (voir .nvmrc)
  • pnpm 10 (corepack enable puis corepack use pnpm@10.12.4, ou installation manuelle)
  • Docker (pour Postgres en local)

Installation

pnpm install
cp .env.example .env
cp apps/api/.env.example apps/api/.env
cp apps/web/.env.example apps/web/.env

Puis édite ces deux .env pour renseigner de vrais POSTGRES_USER/POSTGRES_PASSWORD (et la DATABASE_URL correspondante dans apps/api/.env) : les fichiers .env.example ne contiennent volontairement aucun identifiant réel (juste changeme), et docker-compose.yml refuse de démarrer tant que POSTGRES_USER/PASSWORD/DB ne sont pas définis dans .env — pas de valeur par défaut en dur dans les fichiers commités. Même règle pour apps/api/.env : JWT_SECRET est requis, sans défaut (génère le tien, voir le commentaire dans apps/api/.env.example).

Cypress : téléchargement du binaire

pnpm install installe le package cypress mais pas forcément son binaire (le téléchargement du .exe/binaire natif peut être ignoré selon l'environnement où pnpm install a été lancé — ex. un environnement sandboxé/CI dont le cache ne correspond pas à celui de ta machine). Si pnpm --filter web e2e échoue avec une erreur du type :

No version of Cypress is installed in: ...\AppData\Local\Cypress\Cache\...
Please reinstall Cypress by running: cypress install

lance simplement, depuis ta machine :

pnpm --filter web exec cypress install

(à faire une seule fois par machine ; le binaire est mis en cache localement, hors du repo).

Développement

# Base de données Postgres locale
docker compose up -d

# Applique le schéma (première fois / après un changement de prisma/schema.prisma)
pnpm --filter api exec prisma migrate dev

# Backend (http://localhost:3000)
pnpm dev:api

# Frontend (http://localhost:5173)
pnpm dev:web

Conflit de port possible sur 5432 : si tu as déjà un Postgres natif installé sur ta machine (service Windows, Homebrew, etc.), il peut occuper le port 5432 et intercepter les connexions à la place du conteneur Docker (symptôme : Prisma renvoie P1000: Authentication failed alors que les identifiants sont corrects). Dans ce cas, mets POSTGRES_PORT=5433 (ou autre) dans ton .env et adapte le port dans la DATABASE_URL de apps/api/.env.

Qualité / Tests

pnpm lint                  # Biome (lint + format check)
pnpm lint:fix               # Biome --write
pnpm test                   # tests unitaires/intégration (Mocha, apps/api)
pnpm --filter api test:bdd   # tests d'intégration BDD (Cucumber/Gherkin, apps/api)
pnpm --filter web e2e         # tests e2e (Cypress, démarre le serveur dev automatiquement)
pnpm build                     # build de tous les workspaces

La CI GitHub Actions (.github/workflows/ci.yml) exécute lint + tests + build sur chaque push/PR vers main, puis les tests e2e Cypress.

Cucumber (apps/api)

Tests d'intégration lisibles en Gherkin, en complément de Mocha (qui reste pour les tests unitaires purs) :

  • apps/api/features/*.feature — scénarios en Given/When/Then (health.feature sert d'exemple)
  • apps/api/features/step-definitions/*.steps.ts — implémentation des steps
  • apps/api/features/support/world.ts — contexte partagé entre les steps d'un scénario (instancie l'app Express in-process via createApp(), comme le fait déjà supertest côté Mocha — pas besoin de lancer un vrai serveur)
  • apps/api/cucumber.cjs — config (extension .cjs volontaire, voir la remarque TypeScript/ESM ci-dessous)

Pour ajouter un scénario : écrire le .feature, lancer pnpm --filter api test:bdd, implémenter les steps manquants (Cucumber affiche des snippets tout prêts pour ceux qui n'existent pas encore).

Piège TypeScript/ESM à connaître (déjà rencontré avec cypress.config.ts) : les fichiers de config d'outils tiers qui font du chargement dynamique de TS (cucumber.cjs, cypress.config.ts…) sont sensibles au "type": "module" du package.json. cucumber.cjs évite le problème pour sa propre config en étant explicitement CommonJS ; les steps/world restent en .ts ESM classique et sont chargés via tsx (NODE_OPTIONS=--import=tsx, voir le script test:bdd).

Auth (apps/api)

Inscription (création de profil + foyer) et connexion, JWT dans un cookie httpOnly.

  • POST /auth/signup{ firstName, lastName, email, password } → crée le foyer (house) et le profil (user_profiles) en une transaction, pose le cookie de session, renvoie le profil (201)
  • POST /auth/login{ email, password } → pose le cookie de session, renvoie le profil (200) ; message d'erreur volontairement générique (401) que ce soit l'email ou le mot de passe qui soit incorrect
  • POST /auth/logout — efface le cookie (204)
  • GET /auth/me — profil courant, nécessite le cookie de session (401 sinon)

Mots de passe hachés avec argon2. Le hash est indépendant du foyer : un profil crée toujours son propre foyer à l'inscription (rejoindre un foyer existant n'est pas encore implémenté).

argon2 : version pinnée à 0.31.2, pas de ^. La version 0.45.1 (dernière au moment de l'écriture) segfault au runtime sur au moins une configuration Windows — reproduit de façon stable (bash sandboxé, bash non-sandboxé, PowerShell), alors que 0.31.2 fonctionne parfaitement avec la même API. Si tu montes la version, revérifie concrètement (argon2.hash(...) dans un node -e) avant de merger, un pnpm build qui passe ne suffit pas à détecter un crash runtime.

Les tests (Mocha + Cucumber) tournent avec un coût argon2 réduit (NODE_ENV=test, voir auth.service.ts) — le coût par défaut est volontairement élevé (sécurité), ce qui rendrait la suite de tests lente/instable sinon. La CI provisionne un vrai Postgres de service (.github/workflows/ci.yml) et exécute prisma migrate deploy avant les tests.

Les tests automatisés et pnpm dev:api partagent la même base Postgres locale. Lancer pnpm test/test:bdd vide user_profiles/house (TRUNCATE ... CASCADE, voir test-support/reset-db.ts) — si tu es en train de tester manuellement à la main (via le navigateur ou curl) contre le serveur de dev, un run de tests en parallèle efface tes données de test sans prévenir. Pas un bug, juste à savoir.

Page de connexion / inscription (apps/web)

  • src/api/client.tsApiClient (classe, instance unique exportée apiClient) : enveloppe fetch vers l'API (credentials: "include", requis pour que le cookie de session httpOnly parte/revienne — l'API et le front sont sur des origines différentes). URL configurable via VITE_API_URL (voir .env.example).
  • src/features/auth/AuthContext.tsx — état d'auth global ; appelle GET /auth/me au chargement pour restaurer la session depuis le cookie.
  • src/features/auth/RequireAuth.tsx / RedirectIfAuthenticated.tsx — gardes de route (react-router-dom) : / exige d'être connecté, /login et /signup redirigent vers / si on l'est déjà.
  • src/pages/{Login,Signup,Home}Page.tsx — validation client instantanée via les schémas zod partagés (packages/shared), erreurs API traduites via ErrorMessageService (voir ci-dessous).

Détail de l'organisation complète (dossiers, routing, SCSS/theming) : specs/frontend-architecture.md.

Tests Cypress (apps/web/cypress/e2e/) : smoke.cy.ts + auth.cy.ts mockent l'API via cy.intercept plutôt que de dépendre d'un vrai backend — le job e2e de la CI ne provisionne pas de Postgres/API, seulement le serveur de dev Vite. Le comportement réel de l'API est couvert par les suites Mocha/Cucumber d'apps/api (contre une vraie base).

Gestion des erreurs (API ↔ web)

Contrat d'erreurs partagé via packages/shared (ErrorCode, énumération numérique groupée par famille — 4000 validation, 401x auth, 404x not found, 500x interne — et ApiErrorResponse) : l'API renvoie toujours { code, message, details? } (message en anglais, dev-facing — jamais affiché tel quel), et le client traduit code en libellé français via i18next (ErrorMessageService, apps/web/src/services/error-message.service.tsapps/web/src/locales/fr/translation.json). Côté API, ErrorHandlerService (packages/error-tools) et createErrorMiddleware (packages/express-tools) centralisent la transformation de toute erreur levée en réponse HTTP conforme — aucune valeur ErrorCode codée en dur nulle part (toujours ErrorCode.XXX, y compris dans les mocks Cypress).

Détail complet (schéma, exemples, comment ajouter un nouveau code d'erreur) : specs/error-handling.md.

Le profil authentifié (requireAuth) passe par res.locals.userProfile (typé via AuthLocals), pas par une augmentation du namespace global Express — voir specs/backend-architecture.md pour le détail et le pourquoi.

packages/shared fournit aussi assertIsNever (vérification d'exhaustivité de switch/if-chain sur une union, erreur de compilation si un cas est oublié) — voir specs/backend-architecture.md.

i18n

i18next + react-i18next — tout le texte affiché (formulaires, boutons, erreurs) vient de fichiers de locale JSON (apps/web/src/locales/<lng>/translation.json), jamais codé en dur dans un composant. Une seule langue existe aujourd'hui (fr) ; en ajouter une est une question de fichier de locale, pas de code. Détail : specs/frontend-architecture.md.

Données de test (faker.js)

apps/api utilise @faker-js/faker pour toutes les données de test dans test/auth.test.ts (Mocha) et le "bruit" (prénom/nom de remplissage) des steps Cucumber — jamais de nom/email qui ressemble à une vraie personne en dur dans un fixture. Les valeurs littérales des scénarios .feature eux-mêmes (ex. alice@example.com) restent volontairement statiques : c'est le point des scénarios Gherkin lisibles (exemples illustratifs conventionnels en BDD, pas des données réelles) — seules les données de remplissage hors du texte lisible du scénario sont générées.