610 lines
40 KiB
Markdown
610 lines
40 KiB
Markdown
# Architecture
|
||
|
||
The reference document for how this system is put together and, more importantly, which rules
|
||
must not be broken. Decisions and their trade-offs live in [`docs/adr/`](./adr/README.md).
|
||
|
||
---
|
||
|
||
## 1. System topology
|
||
|
||
```
|
||
┌─────────────┐
|
||
Customer ───────────────▶│ │
|
||
│ Cloudflare │ TLS, WAF, CDN, DDoS
|
||
Admin ──────────────────▶│ │
|
||
└──────┬──────┘
|
||
│
|
||
┌──────▼──────┐
|
||
│ Nginx │ routing, gzip, rate ceiling,
|
||
└──┬───┬───┬──┘ immutable asset caching
|
||
┌────────────────┘ │ └────────────────┐
|
||
│ │ │
|
||
┌───────▼───────┐ ┌───────▼───────┐ ┌───────▼───────┐
|
||
│ Storefront │ │ Admin │ │ API │
|
||
│ Next.js │ │ Next.js │ │ NestJS │
|
||
│ :3000 │ │ :3001 │ │ :4000 │
|
||
└───────┬───────┘ └───────┬───────┘ └───────┬───────┘
|
||
│ │ │
|
||
└────── REST ────────┴──── REST ──────────┤
|
||
│
|
||
┌───────────────┬───────────────┼───────────────┐
|
||
│ │ │ │
|
||
┌──────▼─────┐ ┌──────▼─────┐ ┌──────▼─────┐ ┌──────▼─────┐
|
||
│ PostgreSQL │ │ Redis │ │ R2 / S3 │ │ Providers │
|
||
│ (record) │ │ (cache) │ │ (media) │ │ (future) │
|
||
└────────────┘ └────────────┘ └────────────┘ └────────────┘
|
||
```
|
||
|
||
**The load-bearing rule:** only the API touches PostgreSQL, Redis or object storage. Both
|
||
frontends reach data exclusively through the REST API. See [ADR-0004](./adr/0004-the-admin-dashboard-has-no-database-access.md).
|
||
|
||
Server-side rendering in both Next apps calls the API over the internal Docker network
|
||
(`API_INTERNAL_URL`), skipping the public hostname and TLS entirely.
|
||
|
||
---
|
||
|
||
## 2. Responsibilities
|
||
|
||
| Component | Owns | Explicitly does not |
|
||
| ------------------ | ---------------------------------------------------------------- | --------------------------------------------- |
|
||
| `apps/storefront` | Customer UX, SEO, rendering strategy, client cart state | Business rules, pricing maths, DB access |
|
||
| `apps/admin` | Back-office UX, bulk editing, operational views | Business rules, DB access, its own auth model |
|
||
| `apps/api` | **All** business logic, persistence, authorization, integrations | Rendering, presentation concerns |
|
||
| `packages/*` | Contracts and reusable primitives | Anything app-specific or stateful |
|
||
| `infrastructure/*` | Runtime topology, container builds, local dev | Application behaviour |
|
||
|
||
Pricing is the clarifying example. The storefront may _format_ `{ amount: 250000, currency: 'VND' }`
|
||
as `₫250.000`. It may never _compute_ a discount, a subtotal or a shipping cost. If a number
|
||
appears on a receipt, the API produced it.
|
||
|
||
---
|
||
|
||
## 3. Dependency rules
|
||
|
||
```
|
||
apps/storefront ──┐
|
||
├──▶ @sport/api-client ──▶ @sport/types
|
||
apps/admin ───────┤ ▲
|
||
├──▶ @sport/ui ────────────────┤
|
||
└──▶ @sport/validation ────────┤
|
||
│
|
||
apps/api ────────────▶ @sport/validation ────────┤
|
||
└───────────▶ @sport/types ─────────────┘
|
||
```
|
||
|
||
Allowed:
|
||
|
||
- Any app → any package.
|
||
- `@sport/validation`, `@sport/api-client` → `@sport/types`.
|
||
- `@sport/ui` → nothing but React and styling utilities.
|
||
|
||
Forbidden, and enforced rather than merely documented:
|
||
|
||
| Rule | Enforced by |
|
||
| ---------------------------------------- | ------------------------------------------------- |
|
||
| App → app | No workspace dependency exists |
|
||
| Package → app | No workspace dependency exists |
|
||
| Frontend → `@prisma/client` or `ioredis` | ESLint `no-restricted-imports` |
|
||
| `@sport/types` → any framework | Zero dependencies in its `package.json` |
|
||
| Cross-module deep imports in the API | ESLint `no-restricted-imports` on `@/modules/*/*` |
|
||
| Circular imports | ESLint `import-x/no-cycle` |
|
||
|
||
`@sport/types` has no dependencies at all, and that is a deliberate constraint: it is imported
|
||
by a NestJS server, two React apps and eventually a React Native app. The moment it depends on
|
||
a framework, one of those consumers breaks.
|
||
|
||
---
|
||
|
||
## 4. What may and may not be shared
|
||
|
||
**Share** — things that are true everywhere:
|
||
|
||
- Domain types and API contracts (`@sport/types`)
|
||
- Input shape/format rules (`@sport/validation`)
|
||
- API access (`@sport/api-client`)
|
||
- UI infrastructure: Button, Input, Dialog, Sheet, DropdownMenu, Tabs, Badge,
|
||
Skeleton (`@sport/ui`, shadcn/ui owned as source — see
|
||
[ADR-0017](./adr/0017-shadcn-for-infrastructure-hand-built-for-brand.md))
|
||
- Build configuration (`@sport/config`, `@sport/eslint-config`)
|
||
|
||
**Do not share** — things that only look shareable:
|
||
|
||
- **Domain components.** `<ProductCard>` knows about sale badges, price ranges and colour
|
||
swatches. It belongs to the storefront. The admin's product row needs status, stock and
|
||
margin. Merging them produces a component with fourteen props and two consumers who both
|
||
fight it. These live in `apps/storefront/src/components/commerce/` — hero, mega menu, site
|
||
header, product card, product gallery, product detail, filter sheet — and are built by hand
|
||
because they _are_ the brand. The dividing line: infrastructure comes from the registry,
|
||
identity is written here.
|
||
- **Business logic.** It lives in the API. A discount calculated in a shared package is a
|
||
discount that can disagree with the invoice.
|
||
- **App state.** Cart, auth session and filter state are app-specific. Shared stores create
|
||
invisible coupling between two applications that must be free to diverge.
|
||
- **Feature-to-feature imports.** If two storefront features need the same thing, it moves up
|
||
to `@/components` or `@/lib` — it does not get imported sideways.
|
||
|
||
The test for `@sport/ui`: _would this component be meaningful in the admin dashboard?_ If not,
|
||
it is not a design-system primitive.
|
||
|
||
---
|
||
|
||
## 5. Backend module boundaries
|
||
|
||
Twenty-one modules, each owning its tables exclusively — `checkout` is the exception and owns
|
||
none, existing purely to coordinate cart, catalog and inventory. Full anatomy in
|
||
[`apps/api/src/modules/README.md`](../apps/api/src/modules/README.md).
|
||
|
||
| Group | Modules |
|
||
| ------------------- | ---------------------------------------------------------------------------------------- |
|
||
| Cross-cutting | `auth`, `health` |
|
||
| Identity | `users`, `customers` |
|
||
| Catalog | `products`, `product-variants`, `categories`, `collections`, `brands`, `media`, `search` |
|
||
| Commerce | `inventory`, `carts`, `checkout`, `orders`, `payments` |
|
||
| Marketing & content | `promotions`, `coupons`, `wishlist`, `reviews`, `cms` |
|
||
|
||
Communication:
|
||
|
||
| Need | Mechanism |
|
||
| --------------------------------------- | ------------------------------------------------ |
|
||
| An answer now, to continue this request | Call the other module's `public/` service |
|
||
| To react to something that happened | Subscribe to its domain event |
|
||
| To change another module's data | Call its public service — never write its tables |
|
||
|
||
`inventory`, `orders`, `payments` and `search` carry an additional constraint: no shared
|
||
transactions with the rest of the monolith, so they remain extractable.
|
||
|
||
---
|
||
|
||
## 6. Request lifecycle
|
||
|
||
```
|
||
Request
|
||
│
|
||
├─▶ RequestIdMiddleware assign/propagate x-request-id
|
||
├─▶ ThrottlerGuard per-IP rate limit
|
||
├─▶ AccessTokenGuard verify JWT, check audience, attach actor ← opt-out via @Public()
|
||
├─▶ PermissionsGuard evaluate @RequirePermissions
|
||
├─▶ ZodValidationPipe parse + coerce body/query
|
||
│
|
||
├─▶ Controller ─▶ Service ─▶ Repository ─▶ Prisma
|
||
│
|
||
├─▶ ResponseEnvelopeInterceptor wrap in { success: true, data, meta }
|
||
└─▶ AllExceptionsFilter any throw → { success: false, error, meta }
|
||
```
|
||
|
||
Authentication is **on by default**. `AccessTokenGuard` is registered globally and a route
|
||
becomes public only by explicitly declaring `@Public()`. Forgetting a decorator therefore fails
|
||
closed — a new endpoint is never accidentally exposed.
|
||
|
||
---
|
||
|
||
## 7. API conventions
|
||
|
||
### Response envelope
|
||
|
||
Every response uses one of exactly two shapes, including 500s. Clients branch on `success`,
|
||
never on the status code.
|
||
|
||
```jsonc
|
||
// success
|
||
{ "success": true, "data": { }, "meta": { "requestId": "019f…", "timestamp": "2026-08-11T…" } }
|
||
|
||
// failure
|
||
{
|
||
"success": false,
|
||
"error": { "code": "INSUFFICIENT_STOCK", "message": "Only 2 left in size M.",
|
||
"fields": { "items.0.quantity": ["Only 2 available"] } },
|
||
"meta": { "requestId": "019f…", "timestamp": "2026-08-11T…" }
|
||
}
|
||
```
|
||
|
||
`error.code` is a stable machine identifier from `API_ERROR_CODES`. **A code is never renamed or
|
||
repurposed once shipped** — mobile apps and partner integrations branch on those strings. Adding
|
||
a code is always safe; changing one is a breaking API change.
|
||
|
||
`error.message` is safe to display to an end user. Internal detail (SQL, Prisma metadata, stack
|
||
traces) never reaches the client in production; it goes to the log, correlated by `requestId`.
|
||
|
||
### Status codes
|
||
|
||
| Code | Meaning |
|
||
| --------------- | -------------------------------------------------- |
|
||
| 200 / 201 / 204 | Success |
|
||
| 400 | Malformed request |
|
||
| 401 | Missing, invalid or expired credentials |
|
||
| 403 | Authenticated but not permitted |
|
||
| 404 | Not found, or hidden from this actor |
|
||
| 409 | Conflict (duplicate, invalid state transition) |
|
||
| 422 | Validation failed — carries `error.fields` |
|
||
| 429 | Rate limited |
|
||
| 500 | Unexpected — always generic message, always logged |
|
||
|
||
### Pagination
|
||
|
||
Offset (`?page=&perPage=`) for admin tables, where "page 7 of 42" is a real requirement. Cursor
|
||
(`?cursor=&limit=`) for storefront listings, where correctness under concurrent writes matters
|
||
more than random access. Chosen per endpoint, never mixed.
|
||
|
||
---
|
||
|
||
## 8. Logging
|
||
|
||
Structured JSON via pino. One line per request: method, path, status, duration, `requestId`,
|
||
`actorId`. Domain logs carry the same `requestId`, so a full trace — Cloudflare → Nginx → API —
|
||
is one grep.
|
||
|
||
Levels: `error` for 5xx and unexpected failures; `warn` for 4xx and degraded dependencies;
|
||
`info` for lifecycle and significant domain events; `debug` for development only.
|
||
|
||
Redaction happens **at the logger**, not at each call site: `authorization`, `cookie`,
|
||
`set-cookie`, and password/token/card fields are censored centrally. Relying on developers to
|
||
remember is how credentials end up in log storage.
|
||
|
||
Application code uses Nest's standard `Logger`, which `main.ts` routes into pino. Nothing
|
||
injects `PinoLogger` directly — it is transient-scoped, and injecting it would silently make the
|
||
consumer transient too, which for `PrismaService` would mean a second connection pool.
|
||
|
||
---
|
||
|
||
## 9. Authentication and sessions
|
||
|
||
See [ADR-0008](./adr/0008-short-access-tokens-rotating-refresh-tokens-separate-audiences.md) for
|
||
the decision and [ADR-0015](./adr/0015-frontends-reach-the-api-through-their-own-origin.md) for
|
||
the transport.
|
||
|
||
### The two tokens
|
||
|
||
| | Access token | Refresh token |
|
||
| ------------ | ---------------------------------------------- | --------------------------------------- |
|
||
| Form | JWT | Opaque random bytes |
|
||
| Lifetime | 15 minutes | 30 days |
|
||
| Stored where | Client memory only | httpOnly cookie + SHA-256 in `sessions` |
|
||
| Revocable | No (stateless by design) | Yes |
|
||
| Carries | userId, audience, type, permissions, sessionId | Nothing — it is a lookup key |
|
||
|
||
The refresh token is **not** a JWT. There is nothing for a client to read in it, and because it
|
||
is checked against a row it can be revoked — which a stateless token fundamentally cannot be.
|
||
Only its hash is stored, so a database leak yields no usable credential.
|
||
|
||
### Rotation and reuse detection
|
||
|
||
```
|
||
login → session A (new family)
|
||
refresh(A) → session B, A.replacedBy=B (same family)
|
||
refresh(A) → REUSE: revoke whole family
|
||
```
|
||
|
||
A rotated token that shows up again means either it leaked or the client is broken. Both justify
|
||
killing the family, which signs out the thief _and_ the legitimate device — deliberately, because
|
||
the alternative is letting a known-compromised session continue.
|
||
|
||
### Audience separation
|
||
|
||
Every token carries an audience (`storefront` / `admin`) and each has its own login endpoint and
|
||
its own cookie name. It is enforced three times: at issuance (the user's type must match), on
|
||
every request (`AccessTokenGuard`, before permissions are read), and per controller via
|
||
`@RequireAudience`.
|
||
|
||
### Why login is deliberately uninformative
|
||
|
||
Unknown email, wrong password, suspended account and wrong audience all return the same message
|
||
and status, and the unknown-email path burns equivalent CPU so it is not measurably faster.
|
||
Anything else turns the login form into a user-enumeration oracle.
|
||
|
||
Failures are counted twice in Redis: per account (stops guessing one password from many IPs) and
|
||
per IP (stops spraying one password across many accounts). Only failures count, so a busy
|
||
legitimate user is never locked out.
|
||
|
||
### Known limitation
|
||
|
||
A permission change takes effect within one access-token lifetime, not instantly — that is the
|
||
price of stateless guards with no database read on the hot path. `CACHE_KEYS.revokedSession`
|
||
exists for an immediate denylist and is deliberately not wired up; it reintroduces exactly the
|
||
lookup the design removes.
|
||
|
||
---
|
||
|
||
## 10. Internationalisation
|
||
|
||
Two languages, **two mechanisms**, split by who owns the string. Conflating them is the usual
|
||
way an i18n project ends up half-finished. See
|
||
[ADR-0013](./adr/0013-content-translations-in-typed-tables-ui-strings-in-message-catalogs.md).
|
||
|
||
| String | Owner | Lives in | Changes via |
|
||
| -------------------------------------- | ----------------- | ----------------------------------- | ---------------- |
|
||
| "Add to bag", "Filters" | Designers/devs | `src/messages/{vi,en}.json` | Deploy |
|
||
| Sport names, sort labels, availability | Devs (code enums) | Message catalogs | Deploy |
|
||
| Product name, description, spec rows | Merchandisers | `*_translations` tables | Admin, no deploy |
|
||
| Category / collection / brand names | Merchandisers | `*_translations` tables | Admin, no deploy |
|
||
| Colour labels ("Đen" / "Black") | Merchandisers | `product_option_value_translations` | Admin, no deploy |
|
||
|
||
### Resolution rules
|
||
|
||
1. **Locale in, resolved out.** The API takes `?locale=` (falling back to `Accept-Language`,
|
||
then `vi`) and returns already-resolved strings. The frontend never sees a translation table
|
||
and never writes fallback logic.
|
||
2. **Field-level fallback.** A translation row wins per column; any column that is null or blank
|
||
falls back to the base row. A product with a translated name but no translated description
|
||
renders the translated name and the original description — never a blank.
|
||
3. **Missing translations are legitimate.** Sizes (`S`, `M`, `L`) carry no translation rows at
|
||
all, because they are identical in both languages. The fallback covers it.
|
||
4. **Every cache key is namespaced by locale.** Forgetting this serves the first visitor's
|
||
language to everyone — the classic i18n caching bug. `CACHE_KEYS` builds every key in one file
|
||
precisely so this cannot be forgotten locally.
|
||
|
||
### URLs
|
||
|
||
Storefront uses `localePrefix: 'as-needed'`: Vietnamese from clean paths, English under `/en`.
|
||
Product slugs are themselves translated, so `/products/ao-chay-bo-aero` and
|
||
`/en/products/aero-run-tee` are the same product. Consequently:
|
||
|
||
- Every product carries `alternateSlugs`, which feeds `hreflang` alternates and the language
|
||
switcher.
|
||
- Product lookup matches a slug in _any_ locale, then redirects to the canonical URL for the
|
||
requested locale. One redirect keeps every cross-locale link alive without duplicate content.
|
||
|
||
The admin resolves locale from a cookie with no URL segment — it is `noindex` everywhere, so
|
||
locale-in-path would add a proxy hop and double the route tree for nothing.
|
||
|
||
---
|
||
|
||
## 11. Naming conventions
|
||
|
||
| Thing | Convention | Example |
|
||
| --------------------- | ------------------------- | ------------------------------ |
|
||
| Files | kebab-case | `product-variant.service.ts` |
|
||
| React components | PascalCase file + export | `ProductCard.tsx` |
|
||
| Classes | PascalCase | `ProductVariantService` |
|
||
| Variables / functions | camelCase | `calculateSubtotal` |
|
||
| Constants | SCREAMING_SNAKE | `PAGINATION_DEFAULTS` |
|
||
| Types / interfaces | PascalCase, no `I` prefix | `ProductVariant` |
|
||
| Database tables | snake_case plural | `product_variants` |
|
||
| Database columns | snake_case | `sale_price_amount` |
|
||
| Prisma models | PascalCase singular | `ProductVariant` |
|
||
| API routes | kebab-case plural | `/api/v1/product-variants` |
|
||
| Permissions | `resource.action` | `product.update` |
|
||
| Domain events | `resource.past_tense` | `order.placed` |
|
||
| Redis keys | `domain:entity:id` | `catalog:product:slug:air-tee` |
|
||
| Env vars | SCREAMING_SNAKE | `JWT_ACCESS_SECRET` |
|
||
| Branches | `type/short-description` | `feat/variant-matrix-editor` |
|
||
|
||
Booleans read as assertions: `isActive`, `hasVariants`, `canRefund`. Money fields end in
|
||
`Amount` and are always integers.
|
||
|
||
---
|
||
|
||
## 12. Configuration and environment
|
||
|
||
Four `.env` files, each with a committed `.env.example`:
|
||
|
||
| File | Consumed by | Contains |
|
||
| ---------------------- | ------------------- | ----------------------------------------------------------- |
|
||
| `/.env` | docker-compose only | Ports, container credentials |
|
||
| `apps/api/.env` | API | `DATABASE_URL`, JWT secrets, storage credentials |
|
||
| `apps/storefront/.env` | Storefront | `NEXT_PUBLIC_*`, `API_INTERNAL_URL` |
|
||
| `apps/admin/.env` | Admin | `NEXT_PUBLIC_*`, `API_INTERNAL_URL` — **no `DATABASE_URL`** |
|
||
|
||
Rules:
|
||
|
||
1. **The API validates its entire environment at boot** with Zod and refuses to start on any
|
||
problem, listing all of them at once. A missing JWT secret is discovered at deploy time, not
|
||
at 2am by a customer.
|
||
2. **`process.env` is read in exactly one place per app.** Everything else injects a typed
|
||
config object.
|
||
3. **`NEXT_PUBLIC_*` is public.** It is inlined into the client bundle. No secret ever carries
|
||
that prefix.
|
||
4. **`NEXT_PUBLIC_*` is baked at build time**, not container start — which is why the
|
||
Dockerfiles take them as build args.
|
||
5. Secrets are never committed. `pnpm setup` generates real JWT secrets locally so that not even
|
||
a laptop runs on a value present in the repository.
|
||
|
||
---
|
||
|
||
## 13. Where premature abstraction must be avoided
|
||
|
||
Places where the instinct to generalise should be resisted until a second real case appears:
|
||
|
||
- **Payment providers.** Build VNPay concretely first. One implementation does not reveal the
|
||
right interface; two do. Guessing produces an abstraction shaped like VNPay with a misleading
|
||
name.
|
||
- **A generic repository layer.** `BaseRepository<T>` with generic CRUD sounds appealing and
|
||
ends as a layer that obstructs every non-trivial query. Prisma is already the abstraction.
|
||
- **CQRS / event sourcing.** The inventory ledger is append-only because inventory genuinely
|
||
needs an audit trail — that is not a mandate to apply the pattern everywhere.
|
||
- **A shared `<DataTable>` before three tables exist.** Two tables with different needs produce
|
||
a component with thirty props.
|
||
- **A plugin architecture for the CMS.** Build the homepage blocks that are needed. A page
|
||
builder is a product, not a feature.
|
||
- **Micro-optimising the cache.** Add caching when a query is measurably slow, keyed and
|
||
invalidated deliberately. Cache invalidation bugs are worse than slow pages.
|
||
- **Extracting a service.** The boundaries exist so extraction _stays possible_, not so it
|
||
happens. Extract when there is a real scaling or team-boundary problem.
|
||
|
||
---
|
||
|
||
## 14. Architectural risks to prevent from day one
|
||
|
||
| Risk | Why it is fatal later | Prevention in place |
|
||
| ---------------------------------------------- | --------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||
| Size/colour as product columns | Cannot express per-combination stock or price; requires re-modelling after orders exist | Variant model ([ADR-0003](./adr/0003-product-and-productvariant-as-separate-entities.md)) |
|
||
| Float money | Silent discrepancies, unfixable retroactively | Integer minor units ([ADR-0011](./adr/0011-money-as-integer-minor-units.md)) |
|
||
| Role checks scattered in code | Authorization becomes unauditable; new roles need deploys | RBAC + global guard ([ADR-0007](./adr/0007-rbac-permissions-instead-of-role-checks.md)) |
|
||
| Admin querying the DB directly | A second write path where authorization is forgotten | No DB driver in admin ([ADR-0004](./adr/0004-the-admin-dashboard-has-no-database-access.md)) |
|
||
| Module boundary erosion | The monolith becomes unsplittable and untestable | ESLint boundary rules + `public/` barrels |
|
||
| Order lines joined to live catalog | Historical invoices change when prices do | Snapshot fields on order lines (milestone 5) |
|
||
| Overselling under concurrency | Real money, real customers, real refunds | `reserved` column + transactional reservation |
|
||
| Unversioned API | Cannot ship a breaking change once a mobile app exists | URI versioning from request one ([ADR-0005](./adr/0005-uri-based-api-versioning.md)) |
|
||
| Binaries in PostgreSQL | Backups and replication degrade permanently | Object storage ([ADR-0009](./adr/0009-media-in-s3-compatible-storage-metadata-in-postgresql.md)) |
|
||
| Single-warehouse inventory | Adding a location later means migrating live stock history | `(variant, location)` keys from the start |
|
||
| Secrets in the repository | One leak compromises production | Validated env, generated dev secrets, `.env` gitignored |
|
||
| No request correlation | Production incidents become guesswork | `x-request-id` end to end |
|
||
| Browser-only failures passing every check | Server rendering and curl both succeed while every click fails | Client paths must be exercised in a real browser before a milestone closes |
|
||
| `onUnauthorized` retrying the refresh call | Unbounded refresh loop hammering the API from the browser | `skipAuthRetry` on all auth endpoints plus a single-flight refresh |
|
||
| Concurrent 401s each rotating the token | The second rotation reads as token reuse and revokes the family | One shared in-flight refresh promise |
|
||
| Editing options wiping variant pricing | A merchandiser's price and stock work vanishes on a cosmetic edit | Order-independent combination signatures; `planVariantMatrix` is pure and tested ([ADR-0016](./adr/0016-option-values-are-retained-when-variants-reference-them.md)) |
|
||
| Deleting an option value referenced by history | Breaks or cascades into past orders | `Restrict` FK plus retain-if-referenced ([ADR-0016](./adr/0016-option-values-are-retained-when-variants-reference-them.md)) |
|
||
| A catalog write not invalidating the cache | Edits appear not to work, so operators repeat them | Every write path ends in `afterWrite` → `deleteByPrefix('catalog:')` |
|
||
| Stock edited outside the ledger | "Why is this number wrong?" becomes unanswerable | Stock is not settable on the variant endpoint; it moves only through inventory |
|
||
|
||
---
|
||
|
||
## 15. Testing that actually catches things
|
||
|
||
The catalog and auth milestones both passed lint, typecheck, unit tests, a full build and
|
||
curl-based API checks — while the admin login button did nothing at all in a browser. Three
|
||
defects hid in that gap, and all three shared one property: **they only exist in a browser.**
|
||
|
||
- `globalThis.fetch` stored on an object and called as a method throws `Illegal invocation`
|
||
in a browser and works fine in Node. Server rendering and curl could not have caught it.
|
||
- `onUnauthorized` refreshing through an endpoint that was itself retryable recursed without
|
||
bound — visible only as a flood of requests in a real network panel.
|
||
- Next 16 refuses upstream images on private IPs, and reports it with the _same_ message it
|
||
uses for an unmatched remote pattern. Only the rendered page revealed it.
|
||
|
||
The rule this earns: **a milestone touching client behaviour is not done until its
|
||
interactions have been clicked in a real browser**, with the console and network panel open.
|
||
Regression tests now cover the first two (`packages/api-client/src/http-client.spec.ts`); the
|
||
third belongs in an end-to-end test when one exists.
|
||
|
||
Authoring the M3 editor added a second rule. Two defects survived a green pipeline and a
|
||
successful-looking click-through, and both needed the _second_ step of a flow to appear:
|
||
|
||
- The variant grid saved correctly, showed "saved", and left every row marked dirty. It compared
|
||
its draft against a `product` prop the editor shell never refreshed, so a save could not be seen
|
||
by the thing measuring whether one was needed. One save looked fine; it was saving _twice_ that
|
||
exposed it. The shell now owns the product and every tab hands its result back.
|
||
- The inventory screen listed `StockLevel`, which is a projection of the ledger. A variant that has
|
||
never moved has no row — so freshly created variants were invisible there, and since that screen
|
||
is the only route to an adjustment, they could never be given stock at all. Every seeded product
|
||
had stock already, so the whole catalog looked healthy. Only a product created from scratch
|
||
showed it.
|
||
|
||
M4's listing controls added a third variant of the same lesson. Sorting a listing _after_
|
||
loading more products showed the same product twice — once from the freshly sorted first page
|
||
and once left over from the products appended under the previous order. `LoadMore` keeps its
|
||
position in the React tree across a soft navigation, so its `useState` survived a query change
|
||
the server had already acted on. Neither action alone revealed anything; only the pair did. It
|
||
is keyed on the listing identity now.
|
||
|
||
The same pass caught a link built with a raw `<a href>` instead of the locale-aware `Link`, which
|
||
sent an English shopper following "load more" to the Vietnamese listing — invisible in the
|
||
default locale, which is exactly why it survived.
|
||
|
||
So: **exercise a flow on data you just created, not only on seeded data, perform each action
|
||
twice, and do it in a non-default locale.**
|
||
|
||
The sweep before M5 added one more, and it is the sharpest of the set. Two of the worst defects
|
||
in the catalog write path were not logic errors at all — they were **library behaviour that had
|
||
drifted out from under the intent written in the comments**:
|
||
|
||
- `z.record(z.enum(LOCALES), …)` is exhaustive in Zod 4. The helper was named
|
||
`requiredTranslations`, its comment said "at least the default locale", and its `.refine`
|
||
checked exactly that — and was unreachable. Every single-language product was rejected.
|
||
- `.partial()` makes a field optional but keeps its `.default([])`. So an omitted
|
||
`genderTargets` arrived as `[]`, and `[]` is truthy. Renaming a product cleared its targeting,
|
||
dropped every collection link and deleted all of its attributes.
|
||
|
||
Both typechecked. Both read correctly. Neither had a test, because the behaviour they depended on
|
||
was assumed rather than asserted. `packages/validation/src/catalog-admin.spec.ts` now pins both.
|
||
|
||
So: **when a schema encodes an intent, assert the intent — not the shape.** "Absent means leave
|
||
alone" and "one language is enough" are claims about behaviour, and a type signature cannot make
|
||
either of them true.
|
||
|
||
The review before M6 found the sharpest one yet, and no amount of reading would have caught it.
|
||
Checkout reserved stock by reading `on_hand - reserved`, checking it, then writing
|
||
`reserved + n` — all inside a transaction, which _looks_ safe and is not. Two concurrent
|
||
checkouts for a single unit both read `reserved = 0`, both wrote `1`, and **both orders were
|
||
created**: one unit sold twice, with the reservation count showing one. A transaction gives
|
||
atomicity, not isolation from a concurrent read-modify-write; under Postgres's default READ
|
||
COMMITTED the second writer simply overwrites.
|
||
|
||
The fix is to let the database do the arithmetic and carry its own guard:
|
||
|
||
```sql
|
||
UPDATE stock_levels SET reserved = reserved + $n
|
||
WHERE variant_id = $v AND location_id = $l AND on_hand - reserved >= $n
|
||
```
|
||
|
||
Zero rows affected means someone got there first. The same shape now applies to releasing and
|
||
committing reservations, and order status transitions use a compare-and-set on the status they
|
||
were validated against.
|
||
|
||
So: **any state that two requests can contend for must be changed in one statement that re-checks
|
||
its own precondition.** Reading, deciding in JavaScript and then writing is a lost update wearing
|
||
a transaction as a disguise — and the test for it is not a code review, it is two requests fired
|
||
at once. Seed data has been through every code path already; new data has been through none. The
|
||
inventory case is pinned in `apps/api/src/modules/inventory/inventory-list.spec.ts`.
|
||
|
||
M6 added a variant that is worth naming separately, because the symptom pointed at the wrong
|
||
layer entirely. Search returned nothing for typos, SKUs or Vietnamese without diacritics — but
|
||
matched exact names perfectly. Two separate causes, and the first masked the second:
|
||
|
||
1. `listByIds` passed the whole filter through to the catalog, which re-applied `q` as a plain
|
||
`name CONTAINS q`. Everything search found by brand, SKU, typo or diacritic was then
|
||
intersected away by a substring match on the name.
|
||
2. A SQL comment inside a tagged template literal contained a backtick. That terminated the
|
||
template, produced invalid JavaScript, and the API **crashed on startup** — while an older
|
||
process kept serving port 4000. Every test I ran was answered by the previous build.
|
||
|
||
The second is the one to remember. `pkill -f "node dist/main.js"` had not matched the running
|
||
process, `/health` returned 200 the whole time, and the build itself reported success. Restarts
|
||
are now done by killing whatever holds the port and then confirming the new process actually
|
||
logged a successful start — checking that the port answers proves nothing about _which_ build is
|
||
answering.
|
||
|
||
---
|
||
|
||
## 16. Deliberate limitations
|
||
|
||
Stated plainly so they are choices rather than oversights.
|
||
|
||
**Shipped (M0–M6)**
|
||
|
||
- Catalog reads: products, variants, options, categories, collections, brands, navigation —
|
||
localised, cached, filtered and faceted.
|
||
- Storefront browsing and PDP in Vietnamese and English, with per-locale slugs and `hreflang`.
|
||
- Auth: login, refresh rotation with reuse detection, audience separation, login throttling.
|
||
- RBAC enforcement end to end, plus user administration and a role viewer in the admin.
|
||
- Admin catalog authoring: product create/edit with per-locale content tabs, an option builder that
|
||
regenerates the variant matrix, per-variant SKU and pricing, imagery assigned per colourway,
|
||
media uploaded straight to storage, and stock received through the append-only ledger.
|
||
- Storefront listing controls: sort, load-more pagination that stays crawlable, and a mobile
|
||
filter sheet.
|
||
- Search: PostgreSQL full-text with weighted documents, diacritic folding and trigram typo
|
||
tolerance, behind a `SearchProvider` seam (ADR-0012). The index is a projection owned by
|
||
SearchModule and rebuilt from a domain event, so nothing else knows it exists.
|
||
- Commerce: a Redis-backed guest bag priced from the live catalog on every read, guest checkout,
|
||
orders that snapshot everything they display (ADR-0018), stock reserved at placement and either
|
||
released on cancel or shipped on fulfilment, and an admin order lifecycle with an explicit
|
||
transition table.
|
||
|
||
**Not built yet, and why**
|
||
|
||
- **No customer-facing auth UI.** The storefront login/register endpoints exist and are tested;
|
||
the screens land with the account milestone (M8) where they belong.
|
||
- **The role editor is read-only.** Viewing which role grants what is the question operators
|
||
actually ask; editing grants is destructive and wants a confirmation flow and an audit entry.
|
||
- **A password reset does not revoke existing sessions.** Doing it properly means revoking every
|
||
family for the user and belongs with the session-management screen, not bolted onto the
|
||
endpoint. Noted in the code.
|
||
- **No email.** Password reset, order confirmations and back-in-stock alerts all need it; Mailpit
|
||
is already running locally for when it lands.
|
||
- **No payment tables.** M9. An order lands `PENDING` / `UNPAID`; nothing pretends money moved.
|
||
- **No shipping cost.** M9. `shippingAmount` is a stored zero rather than an absent field, so a
|
||
total is always the sum of parts someone can name.
|
||
- **Carts are not promoted to PostgreSQL.** A guest bag lives in Redis and is the acceptable loss
|
||
named in ADR-0010; attaching one to a customer account arrives with M8.
|
||
- **`best_selling` still falls back to newest.** Orders exist now, but ranking by them needs enough
|
||
of them to mean something. `relevance` is real as of M6. Falling back is honest; a fake ranking
|
||
is not.
|
||
- **No sport/gender facet counts.** Navigated by route, not refined in-page, so a count would
|
||
render nowhere.
|
||
- **Facet counts are computed per request.** Fine at this catalog size; the fix when it is not is
|
||
a search index (ADR-0012), not a bigger query.
|
||
- **The event bus is in-process and lossy.** Anything that must not be lost stays in the same
|
||
transaction as its cause.
|
||
- **No observability beyond logs.** Traces and metrics are worth adding once there is production
|
||
traffic to explain.
|
||
- **No CDN, TLS or WAF config.** That belongs to the deployment repository.
|