feat(F-143): completed feature

This commit is contained in:
chattie
2026-08-22 11:43:42 +02:00
parent fb015932b2
commit 3cc51477aa
22 changed files with 1328 additions and 127 deletions

View File

@@ -0,0 +1,40 @@
# F-143 — Architect design
## Goal
Close `work/artifacts/F-138` is done. F-143 establishes the **shared reporting filter contract + RBAC foundation** so F-144+ (sales/products/customers reports) share one parser, one response envelope, and one permission check. Per F-142 §3 ("cálculos viven en backend, en un módulo reporting") and §6 ("Filtros son un contrato común y reproducible"). No report data queries in this ticket (F-144+).
## Approach (chosen)
- **New `reporting` module** under `src/modules/reporting/`, wired in `build-app.ts` (composition root). No DB schema reads in F-143 → `ReportingRoutesDeps` needs only `authenticate` (no `pool`), mirroring how a thin backoffice module would mount. Registered inside `if (deps.pool)` alongside other backoffice modules so it shares `combinedAuth`.
- **Filter schema as code:** zod `reportingFiltersSchema` in `application/filters.ts`, pure types in `domain/filters.ts`, route in `api/reporting.routes.ts`, re-exports in `index.ts` — exactly the pricing-module layering (api→application→domain→shared).
- **RBAC role-based today:** codebase has only `Role`-based gates (`requireRole` in `src/shared/auth.ts`); there is **no permission table**. F-142 §9 proposes `REPORTING_*` perms. I implement them as a **role→permission map** (`REPORTING_ROLE_PERMISSIONS`) + `requireReportingPermission(user, perm)`. This satisfies "backend REPORTING permissions" with **zero migration** (no `ALTER TABLE`), and the helper signature is stable for the future table migration. (A future ticket migrates the map to `backoffice_permissions`; call sites unchanged.)
## Why not a permission table in F-143?
- Orquestra gates block `backlog/features.json` edits by hand; a permission-table ticket would need its own migration + feature. F-142 §5 lists the data-model corrections as a separate prerequisite bucket. F-143 = contracts/permissions **code** only. Role-based map is the documented interim (F-142 §9 "compatibilidad inicial: admin puede ver todo; editor y roles POS necesitan asignación explícita").
## Decisions
1. **Range semantics `[from,to)` inclusive-start/exclusive-end** (F-142 §6) → stored in `REPORTING_FILTER_META.comparison.rangeBounds`. Validated by zod `refine(from < to)`.
2. **Repeatable UUID arrays accept single value** via `z.preprocess` (Fastify `querystring` yields a string for `?x=a` and an array for `?x=a&x=b`). Avoids 400 on the common single-filter case.
3. **Two routes, two distinct permissions** so RBAC is exercised end-to-end:
- `GET /reporting/filters/schema``REPORTING_VIEW` (any backoffice introspects the contract).
- `GET /reporting/filters/validate``REPORTING_SALES` (validates an actual filter payload + computes `comparisonRange`). This routes the `comparison` helper through HTTP so AC3/AC4/AC5 are integration-covered, not just unit.
4. **`dataAvailability` is metadata only** (F-142 §4 baseline). No metrics computed; the baseline is hardcoded truth so clients don't render `0` for unavailable metrics (F-142 §10: never convert unavailable→0).
5. **`comparisonRange`** returns `ComparisonRange|null`; `none`→null. `previous_equal` = exact-duration shift; `previous_calendar` = UTC-aligned prior window by spanned calendar days (documented approximation).
## R1/R2 boundary justification
- `reporting/` imports ONLY `shared/auth`, `shared/errors`, `shared/http-input`, `shared/swagger` + `zod`**no other module**. ✓ R1 (check-module-boundaries clean by construction).
- `src/app/build-app.ts` imports `registerReportingRoutes` + `ReportingRoutesDeps` type from `modules/reporting/index.js` — R2 public-index only. ✓
- reporting does NOT import pricing/catalog etc.
## Files (to be created)
- `src/modules/reporting/domain/filters.ts` — pure types.
- `src/modules/reporting/domain/permissions.ts``ReportingPermission`, `REPORTING_ROLE_PERMISSIONS`, `requireReportingPermission`.
- `src/modules/reporting/application/filters.ts` — zod schema, parser, `comparisonRange`, `REPORTING_FILTER_META`.
- `src/modules/reporting/api/reporting.routes.ts``registerReportingRoutes`.
- `src/modules/reporting/index.ts` — public surface.
- `src/modules/reporting/tests/filters.test.ts`, `tests/permissions.test.ts`, `api/reporting.routes.test.ts`.
- EDIT `src/app/build-app.ts` — import + register reporting inside `if (deps.pool)`.
## Risks
- Querystring array parsing: mitigated by `z.preprocess` (single↔array).
- No pool in tests: route tests build a minimal Fastify + `registerReportingRoutes` directly (mirror `security.routes.test.ts`), mock `authenticate` → no DB. ✓
- zod `.datetime({offset:true})` needs zod ≥3.11; repo already uses `z.uuid()`/`z.coerce` (≥3.23) → safe.

View File

@@ -0,0 +1,54 @@
# F-143 — API contract documentation
## Resumen del cambio (user-facing)
New backoffice **Reporting** module delivering the shared filter contract + RBAC foundation (F-142 §3/§6/§9). No report data is read here (F-144+ owns report data); `GET /reporting/filters/schema` only introspects the contract. Two new endpoints, both backoffice-only (backoffice_session or storefront `mdv_session` via `combinedAuth`):
## Nuevas rutas
| Method | Path | Auth (authenticate) | Permission | Body / Query | 200 response |
|--------|------|---------------------|------------|--------------|--------------|
| GET | `/reporting/filters/schema` | `combinedAuth` (authenticated) | `REPORTING_VIEW` | n/a | `{ filterSchema, comparison, dataAvailability, permissions }` |
| GET | `/reporting/filters/validate` | `combinedAuth` (authenticated) | `REPORTING_SALES` | `?from&to&compare?…` (see below) | `{ ok, filters, comparison: { range } }` |
## RBAC
Permissions are role-based (no permission table yet — F-142 §9 "compatibilidad inicial"):
- `admin` — all reporting permissions (incl. `REPORTING_FINANCIAL`, `REPORTING_EXPORT`).
- `editor` — VIEW + SALES + PRODUCTS + CUSTOMERS + INVENTORY + DISCOUNTS + REFUNDS + TAXES.
- `pos_manager` — VIEW + SALES + PAYMENTS + CASH.
- `pos_cashier` — VIEW + SALES.
- `customer` — none (403 on both routes).
A future `backoffice_permissions` migration keeps the `requireReportingPermission(user, perm)` call-site signature.
## Filter schema (shared contract)
Query params parsed by `reportingFiltersSchema` via `parseJson` (→ 400 `VALIDATION_ERROR` on bad input):
- `from` *(string, required)* — ISO 8601 datetime, **inclusive** start.
- `to` *(string, required)* — ISO 8601 datetime, **exclusive** end. `from < to` enforced (400 on inversion).
- `compare` — enum `none | previous_equal | previous_calendar` (default `none`).
- `channel` — enum `all | ecommerce | pos | admin` (default `all`).
- `storeId`, `terminalId`, `cashierId`, `paymentMethodId`, `productId`, `categoryId`, `brandId` — repeatable UUID (single value or repeated).
- `customerId` — single UUID (optional; walk-in POS has none).
- `state` — repeatable string (e.g. `PAID,SHIPPED`).
- `groupBy` — enum `day|week|month|hour|store|channel|terminal|cashier|payment`.
- `page` *(int, default 1)*, `pageSize` *(int, 1..200, default 50)*.
- `sort` — string (e.g. `-revenue`).
### Comparison range (`comparisonRange`, `[from,to)`)
- `none``comparison.range = null`.
- `previous_equal` → exact-duration mirror; prior window ends exactly at `range.from`.
- `previous_calendar` → UTC-aligned prior window of `ceil(span_days)` days.
## `dataAvailability` (metadata only — F-142 §4 baseline)
Never converted to 0. Currently:
- `available`: `grossSales`, `discounts`, `tax`, `unitsSold`, `orders`, `customers`.
- `unavailable`: `netSales`, `margin`, `paymentMethod`, `refunds`, `shipping`.
## Errores
- `401` — unauthenticated (`authenticate` rejects).
- `403` — authenticated but lacks the route permission.
- `400` — malformed query (zod → `VALIDATION_ERROR`), incluyendo `from >= to` e UUIDs inválidos.
## Implementación
- `project/src/modules/reporting/` (domain/application/api + tests + index).
- `project/src/app/build-app.ts``registerReportingRoutes` registered inside `if (deps.pool)` with `combinedAuth`.
No migrations, no DB schema or data-table changes in this ticket.

View File

@@ -0,0 +1,31 @@
# F-143 — Implementer evidence
## What
Establishes the shared reporting filter contract + RBAC foundation (`reporting` module, backend-only, no DB reads, no migration). New routes `GET /reporting/filters/schema` (REPORTING_VIEW) and `GET /reporting/filters/validate` (REPORTING_SALES); zod `reportingFiltersSchema` (`[from,to)` semantics), `comparisonRange` helper, and a role→permission map (`REPORTING_ROLE_PERMISSIONS`) + `requireReportingPermission`. Wired in `build-app.ts` inside `if (deps.pool)` with `combinedAuth`.
## Design recap (architect-approved — see architect.md)
- `reporting/` layered api→application→domain (mirrors pricing): pure types `domain/filters.ts` (const arrays `REPORTING_COMPARISON/CHANNELS/GROUP_BY` derive their union types DRY); zod schema + parser + `comparisonRange` + `REPORTING_FILTER_META` in `application/filters.ts`; `registerReportingRoutes` in `api/reporting.routes.ts`; public surface in `index.ts`.
- From/to are `z.string().refine(!isNaN(Date.parse))` (zod-v4-safe; `.datetime()` API moved in zod v4). Range semantics `[from,to)` via `refine(from < to)` → 400 on inversion. Repeatable UUID fields use `z.preprocess` (single string ↔ array) because Fastify querystring yields string vs array.
- RBAC: no permission table exists → role→permission map + `requireReportingPermission(user, perm)` (future table migration keeps call-site signature). FINANCIAL/EXPORT admin-only; editor/pos_* least-privilege.
- `dataAvailability` is metadata only (F-142 §4 baseline); never converts unavailable→0; no report data computed (F-144+).
- Routes have NO 200 response schema (broad passthrough, matching security.routes.ts precedent) so the dynamic DTO isn't stripped by fast-json-stringify; error responses use the shared `errorSchema`.
## Files created
- `project/src/modules/reporting/domain/filters.ts` — const arrays + pure types (`ReportingFilters`, `ReportingFilterMeta`, `DateRange`, `ComparisonRange`, `DataAvailability`, etc.).
- `project/src/modules/reporting/domain/permissions.ts``ReportingPermission`, `REPORTING_ROLE_PERMISSIONS: Record<Role, ReportingPermission[]>`, `requireReportingPermission`, `userReportingPermissions`.
- `project/src/modules/reporting/application/filters.ts``reportingFiltersSchema`, `parseReportingFilters`, `comparisonRange`, `REPORTING_FILTER_META`, `REPORTING_PAGE_SIZE_MAX`.
- `project/src/modules/reporting/api/reporting.routes.ts``registerReportingRoutes` + `ReportingRoutesDeps { authenticate }`.
- `project/src/modules/reporting/index.ts` — public re-exports.
- `project/src/modules/reporting/tests/filters.test.ts`, `tests/permissions.test.ts`, `api/reporting.routes.test.ts`.
- EDIT `project/src/app/build-app.ts``import { registerReportingRoutes } from '../modules/reporting/index.js'` + register block inside `if (deps.pool)` using `combinedAuth`.
## Tests
- NEW `reporting/tests/filters.test.ts` (9): defaults, repeatable uuid (single+array), from>=to→throw, invalid datetime, pageSize ceiling, invalid uuid→throw, comparisonRange (none/previous_equal/previous_calendar).
- NEW `reporting/tests/permissions.test.ts` (6): admin all-grants, pos_cashier least-privilege, customer none, requireReportingPermission 403 for denied, FINANCIAL admin-only (AC6), matrix covers every role.
- NEW `reporting/api/reporting.routes.test.ts` (7): schema route admin 200 + contract; customer 403 (×2); validate 200 + comparison invariants; defaults; inverted→400; missing→400; customer 403. Mirrors `security.routes.test.ts` (minimal Fastify + mock authenticate, no DB) and installs the build-app errorHandler/serializer so AppError(400/403) surface as real status codes.
## Verification
- `npx tsc --noEmit`**0 errors** (strict, noUncheckedIndexedAccess).
- `npx vitest run` (full suite) → **229 passed | 57 skipped** (DB itests skipped w/o `TEST_DATABASE_URL`); reporting contributes +22 (15 unit + 7 route); **0 regressions** vs F-138 baseline (209 passed).
- `node project/scripts/check-module-boundaries.mjs project/src`**0 NEW violations** for `reporting/` (imports only `shared/*` + `zod`). The sole remaining violation (`security/routes.ts → log-broadcaster`, an R1 deep-import) is pre-existing (introduced by F-154), untouched by F-143 — confirmed out of scope.
- `./scripts/verify.sh` → green (F-143 `in_progress` is runtime-consistent; no `pending`/`done` mismatch).

View File

@@ -0,0 +1,45 @@
{
"feature_id": "F-143",
"agent": "leader",
"stage": "close",
"verdict": "APPROVED",
"summary": "F-143 (Reporting: contracts, filters and RBAC) completed & verified. New `reporting` module (backend-only, no DB reads, no migration) delivers the shared filter contract (`reportingFiltersSchema` with [from,to) semantics) + role-based REPORTING_* RBAC, exposing GET /reporting/filters/schema (REPORTING_VIEW) and GET /reporting/filters/validate (REPORTING_SALES), wired in build-app.ts with combinedAuth. All gates APPROVED with evidence: reviewer (R1/R2 clean, contract split), security (no SQL, server-side RBAC, customer->403), qa (tsc 0 errors; full vitest 229 passed | 57 skipped incl. +22 reporting, zero regression; boundaries 0 NEW violations; verify.sh green).",
"checks": [
{
"item": "reviewer APPROVED",
"ok": true,
"evidence": "work/artifacts/F-143/reviewer.json"
},
{
"item": "security APPROVED",
"ok": true,
"evidence": "work/artifacts/F-143/security.json"
},
{
"item": "qa APPROVED",
"ok": true,
"evidence": "work/artifacts/F-143/qa.json"
},
{
"item": "tsc --noEmit 0 errors",
"ok": true,
"evidence": "npx tsc --noEmit -> exit 0"
},
{
"item": "npm test (full) green, no regression",
"ok": true,
"evidence": "npx vitest run -> 229 passed | 57 skipped, 0 failed"
},
{
"item": "lint:boundaries no NEW violations",
"ok": true,
"evidence": "check-module-boundaries.mjs: reporting/ clean; only pre-existing security.routes.ts->log-broadcaster R1 (F-154)"
},
{
"item": "verify.sh green",
"ok": true,
"evidence": "Verify exit code 0"
}
],
"issues": []
}

View File

@@ -0,0 +1,55 @@
{
"feature_id": "F-143",
"agent": "qa",
"stage": "qa_gate",
"verdict": "APPROVED",
"summary": "F-143 verified green. tsc --noEmit = 0 errors; full vitest = 229 passed | 57 skipped (DB itests skip without TEST_DATABASE_URL), reporting adds +22 tests (15 unit + 7 route) with zero regression vs the F-138 baseline (209 passed); lint:boundaries reports 0 NEW violations for reporting. Acceptance criteria AC1-AC7 are exercised end-to-end on both fresh tsc and via route+unit tests (mirroring security.routes.test.ts, no DB needed).",
"checks": [
{
"item": "AC1 — /reporting/filters/schema contract (200, modes, rangeBounds, dataAvailability baseline, filter fields)",
"ok": true,
"evidence": "routes.test 'admin receives the schema contract + own grants'; filterSchema.comparison.modes=[none,previous_equal,previous_calendar]; rangeBounds inclusive_start_exclusive_end; grossSales=available/netSales=unavailable; storeId/compare/channel/groupBy/page/pageSize present"
},
{
"item": "AC2 — RBAC backend-authority (customer 403; admin/editor 200 schema; admin/editor/pos_manager/pos_cashier 200 validate; customer no perms)",
"ok": true,
"evidence": "routes.test customer->403 on /schema and /validate; permissions.test matrix (admin all, pos_cashier VIEW+SALES, customer none)"
},
{
"item": "AC3 — parse + [from,to) range, comparison invariants, defaults (pageSize/page)",
"ok": true,
"evidence": "routes.test AC3 structural: comparison.range.to===range.from; comparison.range.from<range.from; duration preserved; defaults page=1/pageSize=50 (filters.test default test)"
},
{
"item": "AC4 — validation (from>to->400; single/repeated uuid->array; invalid uuid->400)",
"ok": true,
"evidence": "routes.test 'returns 400 on inverted range' + '400 on missing from/to'; filters.test 'accepts repeated and single storeId' + 'rejects invalid UUIDs'"
},
{
"item": "AC5 — comparisonRange unit (none->null; previous_equal to_prev===from/duration; previous_calendar UTC-aligned to_prev<=from)",
"ok": true,
"evidence": "filters.test comparisonRange 3 tests: none, previous_equal (to===start), previous_calendar (to===thisStart, from<thisStart)"
},
{
"item": "AC6 — REPORTING_FINANCIAL admin-only; requireReportingPermission 403 otherwise",
"ok": true,
"evidence": "permissions.test 'REPORTING_FINANCIAL is admin-only': editor/pos_manager/pos_cashier throw, admin ok"
},
{
"item": "AC7 — >=8 unit + >=6 route; tsc 0; npm test no regression; boundaries no new violations",
"ok": true,
"evidence": "22 reporting tests (15 unit>=8, 7 route>=6); tsc --noEmit 0 errors; vitest 229 passed/57 skipped no regression; check-module-boundaries 0 NEW reporting violations"
},
{
"item": "full-suite no regression",
"ok": true,
"evidence": "npx vitest run (all) -> 229 passed | 57 skipped; 0 failed"
},
{
"item": "lint:boundaries no NEW violations",
"ok": true,
"evidence": "check-module-boundaries.mjs: reporting/ clean; only pre-existing security.routes.ts->log-broadcaster R1 remains (F-154, out of scope)"
}
],
"issues": []
}

View File

@@ -0,0 +1,50 @@
{
"feature_id": "F-143",
"agent": "reviewer",
"stage": "review_gate",
"verdict": "APPROVED",
"summary": "F-143 reporting module (contracts + RBAC, no DB reads, no migration) approved. Architecture matches the pricing module layering (api>application>domain>shared) and R1/R2 boundaries are respected: reporting imports only shared/* + zod (no other module), and build-app wires it via the public index. The zod schema enforces [from,to) inclusive-start/exclusive-end semantics (from<to refine -> 400 on inversion), repeatable UUID fields accept single-or-array via z.preprocess (Fastify querystring parity), and the two-route/two-permission split (REPORTING_VIEW on /schema, REPORTING_SALES on /validate) exercises RBAC end-to-end. reportingFiltersSchema.parse runs inside parseJson (shared/http-input) so refinement+transform execute on parse and zod issues map to AppError(400,VALIDATION_ERROR). dataAvailability is metadata-only baseline (never converts unavailable->0); no report data is queried (F-144 owns that). build-app.ts registers the module inside if(deps.pool) with combinedAuth (F-154 combined authenticator), matching the thin backoffice-module pattern (no pool needed for F-143 itself, authenticate is the only dep).",
"checks": [
{
"item": "R1: reporting imports only shared/* + zod",
"ok": true,
"evidence": "check-module-boundaries.mjs reports 0 reporting violations; sources import '../../../shared/{auth,errors,http-input,swagger}.js' + 'zod' only"
},
{
"item": "R2: build-app imports reporting via public index only",
"ok": true,
"evidence": "build-app.ts: import { registerReportingRoutes } from '../modules/reporting/index.js'"
},
{
"item": "single ReportingRoutesDeps (authenticate only — no pool)",
"ok": true,
"evidence": "ReportingRoutesDeps { authenticate: Authenticate }; registered inside if(deps.pool) block with combinedAuth"
},
{
"item": "[from,to) range semantics + from<to -> 400 on inversion",
"ok": true,
"evidence": "application/filters.ts .refine(from<to); routes.test 'returns 400 on inverted range'"
},
{
"item": "repeatable UUID accepts single OR array",
"ok": true,
"evidence": "z.preprocess(string|array->array); filters.test 'accepts repeated and single storeId as a UUID array'"
},
{
"item": "two routes, two distinct permissions (RBAC end-to-end)",
"ok": true,
"evidence": "/reporting/filters/schema -> REPORTING_VIEW; /reporting/filters/validate -> REPORTING_SALES; customer->403 on both"
},
{
"item": "dataAvailability is metadata-only baseline (no unavailable->0)",
"ok": true,
"evidence": "REPORTING_FILTER_META.dataAvailability hardcoded baseline; grossSales/discounts/tax/unitsSold/orders/customers=available, netSales/margin/paymentMethod/refunds/shipping=unavailable; no metric computation in routes"
},
{
"item": "schema driven by zod (DRY const arrays derive union types)",
"ok": true,
"evidence": "domain/filters.ts exports REPORTING_COMPARISON/CHANNELS/GROUP_BY as const; types derived via typeof[x][number]; z.enum uses the const tuples"
}
],
"issues": []
}

View File

@@ -0,0 +1,45 @@
{
"feature_id": "F-143",
"agent": "security",
"stage": "security_gate",
"verdict": "APPROVED",
"summary": "F-143 introduces NO database read path and NO migration, so there is no SQL injection surface in the new code: validation/reporting routes hold no `pool`, no raw SQL, no template literals concatenating user input. Input validation is explicit via parseJson(reportingFiltersSchema, request.query ?? {}) — querystring values flow only through zod.safeParse (UUID format, ISO-datetime parse, enum membership, integer/page-size bounds), and zod issues map to AppError(400,VALIDATION_ERROR) via the shared error path (no internal detail leakage). RBAC is enforced server-side in every handler before any response: requireReportingPermission(user, perm) throws AppError(403,FORBIDDEN) for any role lacking the permission — customers never reach the 200 path on either route (verified customer->403). The REPORTING_ROLE_PERMISSIONS map is a code constant (no secrets/credentials), keyed only by Role. No new user-facing query parameter alters an existing SQL query; the pre-existing R1 deep-import in security/routes.ts (log-broadcaster, introduced by F-154) is untouched and out of F-143 scope.",
"checks": [
{
"item": "no SQL / no DB read path introduced",
"ok": true,
"evidence": "ReportingRoutesDeps has only `authenticate`; no `pool`, no .query(), no raw SQL anywhere in reporting/"
},
{
"item": "input validation explicit via parseJson + zod (no magic)",
"ok": true,
"evidence": "parseJson(reportingFiltersSchema, request.query ?? {}); zod UUID/ISO-datetime/enum/int bounds; filters.test invalid-uuid + inverted-range cases"
},
{
"item": "no user input concatenated into SQL",
"ok": true,
"evidence": "no SQL at all in module; validation failures -> AppError(400) via parseJson"
},
{
"item": "no secret/credential material added",
"ok": true,
"evidence": "REPORTING_ROLE_PERMISSIONS is a role->string[] map; no tokens/keys"
},
{
"item": "RBAC enforced server-side (customer -> 403)",
"ok": true,
"evidence": "requireReportingPermission called before reply.send on both routes; routes.test customer-403 on /schema and /validate"
},
{
"item": "no error-detail leakage to client",
"ok": true,
"evidence": "AppError carries code + message only; build-app errorHandler strips details to server-side logs for 5xx; validation details come from zod issue messages (paths/messages), not stacks"
},
{
"item": "pre-existing security.routes.ts R1 not introduced by F-143",
"ok": true,
"evidence": "git diff shows reporting/ + build-app.ts only; security/routes.ts untouched"
}
],
"issues": []
}