feat(F-138): completed feature

This commit is contained in:
chattie
2026-08-22 10:18:37 +02:00
parent 5098723284
commit fb015932b2
23 changed files with 502 additions and 102 deletions

View File

@@ -0,0 +1,29 @@
# F-138 — Architect note (design)
## What
F-138 — close the `GET /pricing/variants/:id` 404 race by seeding a default `pricing_variant_prices` row immediately after `CreateProductVariant` persists a variant. Pricing owns its table (F-153 precedent for cross-module table read; for cross-module **writes** the repo uses service injection — same pattern `cart` already uses on `pricing` in `build-app.ts`).
## Why
`variants.create` only inserts `catalog_product_variants`; it never inserts `pricing_variant_prices`. Three call sites create variants (product autovariant, lazy GET, explicit POST) — all 404 on the pricing GET until an admin prices them. Customer/cart flows that read a price right after variant creation break.
## Approach
1. `PricingRepository` + `PricingService` gain `seedVariantPrice(variantId): Promise<void>`.
2. `PgPricingRepository.seedVariantPrice`: `INSERT pricing_variant_prices(variant_id, net_unit_amount_cents, offer_cents, cost_cents, vat_rate) VALUES ($1, 0, NULL, NULL, 'general') ON CONFLICT (variant_id) DO NOTHING` — same column set as `setVariantPrice` (`currency`/`created_at`/`updated_at` come from DDL defaults). Idempotent + no injection (`variantId` is a UUID produced by `variants.create`).
3. `CreateProductVariant` ctor += `pricing: PricingService` (type-only import of pricing **public index** — R1 legal); after `variants.create` returns, best-effort `await this.pricing.seedVariantPrice(variant.id)` (try/catch: variant already persisted; seed failure must not fail creation).
4. `catalog.routes.ts`: `CatalogRoutesDeps += pricing`; wire `new CreateProductVariant(repository, variants, pricing)`. Single constructor → all 3 call sites seeded.
5. `build-app.ts`: hoist `const pricing = createPricingService(deps.pool)` above the `registerCatalogRoutes` block so it can be passed in; pricing routes + cart keep using it (unchanged). `createPricingService` already imported.
## Alternatives considered
- (a) Raw SQL insert from `CreateProductVariant` via `deps.pool` — REJECTED: catalog app layer shouldn't own pricing table writes (layering; reviewer would flag). Service injection keeps ownership clean.
- (b) Seed in `PgProductVariantRepository.create` — REJECTED: couples catalog infra to pricing schema; breaks single-responsibility.
- (c) `PricingService` injection (CHOSEN) — matches the established `cart``pricing` cross-module write pattern; testable with a fake.
## Risks
- build-app ordering: hoisting `pricing` const is a pure construction reorder — safe (`deps.pool` available, createPricingService has no side effects).
- Seed failure: made best-effort so variant creation never regresses (AC3).
- tsc `strict` + `noUncheckedIndexedAccess`: not relevant here (no array indexing in new code).
## Tests
- `catalog/tests/variant-use-cases.test.ts` (unit, runnable): fakes for ProductRepository, ProductVariantRepository, PricingService — asserts seed-called, no-seed-when-product-missing, seed-failure-doesn't-break-creation.
- `catalog.itest.ts` AC itest (skipIf !hasDb): POST variant → GET `/pricing/variants/:id` 200 + defaults.
- No migration (variant_id UNIQUE/PK already).

View File

@@ -0,0 +1,21 @@
# F-138 — Documentation (API contract)
## Resumen del cambio de comportamiento
**No hay nuevas rutas ni parámetros públicos.** F-138 corrige una brecha de integridad de datos: al crear una variante de producto, **ahora se inserta inmediatamente una fila en `pricing_variant_prices` con valores por defecto** (`net_unit_amount_cents=0`, `vat_rate='general'`, `currency='EUR'` por defecto de DDL).
## Impacto en la API
- `POST /products` → autovariante `SKU-MV-{id}`: la variante ya creada pasa a tener fila de precio (antes 404 hasta asociar precio).
- `GET /products/:id/variants` (lazy, admin): si crea la variante default, también se sembrará su precio.
- `POST /products/:id/variants`: la variante creada pasa a tener fila de precio.
- `GET /pricing/variants/:variantId`: deja de devolver 404 inmediatamente después de crear una variante (antes devolvía 404 hasta que un admin asociara un precio).
## Idempotencia
El sembrado usa `INSERT ... ON CONFLICT (variant_id) DO NOTHING`, por lo que es re-sembrable sin efecto ni errores (AC2). Mantiene la invariante: **toda variante creada tiene al menos una fila de precio**.
## Best-effort
El sembrado es best-effort dentro de `CreateProductVariant.execute`: si falla (p.ej. error transitorio de BD), la creación de la variante primaria **no** se revierte ni falla. El `variant_id` es UNIQUE/PK por definición de la tabla, por lo que el seed no introduce duplicados.
## Seguridad
- SQL parameterizado (`$1` con el UUID de la variante).
- Valores por defecto son constantes de código, no input de usuario.
- No hay nueva ruta ni query parameter.

View File

@@ -0,0 +1,31 @@
# F-138 — Implementer evidence
## What
Closed the `GET /pricing/variants/:id` 404 race: `PricingService.seedVariantPrice` (pricing owns its table) is injected into `CreateProductVariant` and called best-effort right after variant creation. The single constructor covers all 3 call sites (product autovariant, lazy variant GET, explicit POST variant).
## Design recap (architect-approved)
- `PricingRepository` + `PricingService` gain `seedVariantPrice(variantId): Promise<void>`.
- `PgPricingRepository.seedVariantPrice`: `INSERT pricing_variant_prices(variant_id, net_unit_amount_cents, offer_cents, cost_cents, vat_rate) VALUES ($1, 0, NULL, NULL, 'general') ON CONFLICT (variant_id) DO NOTHING` — same column set as `setVariantPrice` (`currency`/`created_at`/`updated_at` via DDL defaults). Idempotent + parameterized.
- `CreateProductVariant` ctor += `pricing: PricingServicePort` (type-only import of pricing **public index** → R1-clean); after `variants.create`, best-effort `await this.pricing.seedVariantPrice(variant.id)` (try/catch: variant already persisted; seed failure must not fail/roll back variant creation).
- `catalog.routes.ts`: `CatalogRoutesDeps += pricing`; `new CreateProductVariant(repository, variants, deps.pricing)`.
- `build-app.ts`: hoist `const pricing = createPricingService(deps.pool)` above `registerCatalogRoutes` so it can be passed in; pricing routes + cart keep using it (unchanged).
## Files changed (source)
- `pricing/domain/ports.ts` — +`seedVariantPrice` on `PricingService` + `PricingRepository`.
- `pricing/application/pricing-service.ts` — +`seedVariantPrice` delegate.
- `pricing/infrastructure/pg-pricing-repository.ts` — +`seedVariantPrice` impl.
- `catalog/application/variant-use-cases.ts` — import `PricingServicePort`; ctor + execute seed (best-effort).
- `catalog/api/catalog.routes.ts``CatalogRoutesDeps += pricing: PricingServicePort`; ctor passes `deps.pricing`.
- `app/build-app.ts` — hoist `pricing` const; pass to `registerCatalogRoutes` deps.
## Tests
- **NEW** `catalog/tests/variant-use-cases.test.ts` (3 tests, runnable no DB): FakeProductRepository + FakeProductVariantRepository + FakePricingService — asserts seed-called-with-variant.id, no-seed-when-product-missing, seed-failure-doesn't-break-creation.
- `catalog.itest.ts` AC itest (skipIf !hasDb): POST `/products/:id/variants``pricing_variant_prices` row exists with `net=0/vat=general/currency=EUR`.
- Collateral (fake fakes updated to satisfy new port method): `pricing/tests/pricing-service.test.ts` repo() factory, `cart/tests/cart-service.test.ts` + `checkout/tests/checkout-service.test.ts` PricingServicePort fakes — added `seedVariantPrice: async () => undefined` no-op.
## Verification (fresh run)
- `npx tsc --noEmit`**0 errors** (strict, noUncheckedIndexedAccess).
- `npx vitest run` (targeted: variant/pricing/cart/checkout/product/build-app) → **16/16 pass**.
- `npm test`**209 passed | 57 skipped** (was 206 → +3 new; no regression in cart/checkout/pricing/build-app).
- `scripts/check-module-boundaries.mjs src`**0 NEW R1 violations**. (Pre-existing R1 on `security.routes.ts:14``log-broadcaster` is out of F-138 scope; git diff proves untouched.)
- `./scripts/verify.sh`**VERIFY_OK** (pre-close).

View File

@@ -0,0 +1,14 @@
{
"feature_id": "F-138",
"agent": "leader",
"stage": "close",
"verdict": "APPROVED",
"summary": "F-138 completed: variant creation now auto-seeds a default pricing_variant_prices row (idempotent via ON CONFLICT DO NOTHING, best-effort so variant creation never regresses). All gates (reviewer/security/qa) APPROVED with evidence. Build green: tsc 0 errors, 209 tests passed (57 DB itests skipped without TEST_DATABASE_URL), verify.sh OK.",
"checks": [
{"item": "reviewer APPROVED", "ok": true, "evidence": "work/artifacts/F-138/reviewer.json"},
{"item": "security APPROVED", "ok": true, "evidence": "work/artifacts/F-138/security.json"},
{"item": "qa APPROVED", "ok": true, "evidence": "work/artifacts/F-138/qa.json"},
{"item": "verify.sh green (pre-close)", "ok": true, "evidence": "VERIFY_OK"}
],
"issues": []
}

View File

@@ -0,0 +1,18 @@
{
"feature_id": "F-138",
"agent": "qa",
"stage": "qa_gate",
"verdict": "APPROVED",
"summary": "Build is green and behavior is verified. tsc --noEmit returns 0 errors; the full vitest suite passes 209/209 (57 DB itests skipped without TEST_DATABASE_URL) with no regressions; the 3 new CreateProductVariant unit tests directly assert the AC. The AC integration test in catalog.itest.ts is present and skipped only due to no TEST_DATABASE_URL (repo convention).",
"checks": [
{"item": "AC1 — seed called with variant.id right after creation", "ok": true, "evidence": "variant-use-cases.test.ts 'seeds a default price row immediately after creating the variant' → pricing.seedCalls == ['variant-1']"},
{"item": "AC1 — no seed when product not found", "ok": true, "evidence": "variant-use-cases.test.ts 'does not seed a price row when the product does not exist' → pricing.seedCalls == []"},
{"item": "AC3 — best-effort: creation succeeds even if seed throws", "ok": true, "evidence": "variant-use-cases.test.ts 'returns the created variant even if seeding the price row fails' → result === variant, seedCalls recorded"},
{"item": "AC2/AC4 — idempotency + 3 call sites covered", "ok": true, "evidence": "PgPricingRepository.seedVariantPrice uses ON CONFLICT DO NOTHING; single constructor shared by autovariant, lazy, and explicit POST"},
{"item": "tsc strict (noUncheckedIndexedAccess) — 0 errors", "ok": true, "evidence": "npx tsc --noEmit → exit 0"},
{"item": "no full-suite regression", "ok": true, "evidence": "npm test → 209 passed | 57 skipped (206 baseline + 3 new)"},
{"item": "lint:boundaries — no NEW violations", "ok": true, "evidence": "check-module-boundaries.mjs: only the pre-existing security.routes.ts→log-broadcaster R1 remains"},
{"item": "AC integration test present", "ok": true, "evidence": "catalog.itest.ts 'seeds a default price row immediately after variant creation (F-138 AC1, AC2, AC4)' (skipIf !hasDb)"}
],
"issues": []
}

View File

@@ -0,0 +1,15 @@
{
"feature_id": "F-138",
"agent": "reviewer",
"stage": "review_gate",
"verdict": "APPROVED",
"summary": "Design review approved. PricingService.seedVariantPrice (pricing owns pricing_variant_prices) is injected into CreateProductVariant via the pricing public index (R1-clean, identical to the cart→pricing cross-module write pattern). A single constructor covers all 3 variant-creation call sites. build-app hoists the pricing const above catalog registration — a pure construction reorder with no side effects. Seed is best-effort so variant creation never regresses.",
"checks": [
{"item": "catalog→pricing cross-module import goes through public index (R1)", "ok": true, "evidence": "import type { PricingServicePort } from '../../pricing/index.js' in variant-use-cases.ts and catalog.routes.ts"},
{"item": "single CreateProductVariant constructor covers all call sites", "ok": true, "evidence": "new CreateProductVariant(repository, variants, deps.pricing) — used by POST /products (autovariant), GET /products/:id/variants (lazy), POST /products/:id/variants"},
{"item": "build-app pricing hoist does not break pricing/cart routes", "ok": true, "evidence": "createPricingService(deps.pool) moved above registerCatalogRoutes; registerPricingRoutes and registerCartRoutes unchanged and still reference the same pricing instance"},
{"item": "no new R1 / boundary violations", "ok": true, "evidence": "check-module-boundaries.mjs reports 0 NEW violations for F-138"},
{"item": "no migration required", "ok": true, "evidence": "pricing_variant_prices.variant_id already UNIQUE/PK (setVariantPrice already uses ON CONFLICT (variant_id))"}
],
"issues": []
}

View File

@@ -0,0 +1,14 @@
{
"feature_id": "F-138",
"agent": "security",
"stage": "security_gate",
"verdict": "APPROVED",
"summary": "Seed SQL is fully parameterized: variantId passed via $1 placeholder (it is a UUID produced by variants.create, never raw user input). The default values (net=0, offer=NULL, cost=NULL, vat_rate='general') are code constants, not user input — no SQL injection surface. ON CONFLICT DO NOTHING guarantees idempotency and prevents duplicate-key errors. The seed is wrapped in try/catch inside the use-case so a seed failure cannot roll back or fail the primary variant creation.",
"checks": [
{"item": "SQL parameterization", "ok": true, "evidence": "PgPricingRepository.seedVariantPrice uses $1 placeholder with params tuple [variantId]"},
{"item": "no string concatenation of user input into SQL", "ok": true, "evidence": "net_unit_amount_cents=0, offer_cents=NULL, cost_cents=NULL, vat_rate='general' are literal constants"},
{"item": "no new user-facing SQL path introduced", "ok": true, "evidence": "seedVariantPrice is an internal post-create step; no new public route or query parameter"},
{"item": "pre-existing R1 (security.routes.ts:14 → log-broadcaster) not introduced by F-138", "ok": true, "evidence": "git diff: security.routes.ts line 14 (log-broadcaster import) is untouched by F-138"}
],
"issues": []
}