diff --git a/docs/functional-completion/current-functional-audit.md b/docs/functional-completion/current-functional-audit.md new file mode 100644 index 0000000..60896c7 --- /dev/null +++ b/docs/functional-completion/current-functional-audit.md @@ -0,0 +1,187 @@ +# Current functional audit + +Performed 2026-08-02 against source `feat/mobilityops-functional-completion` (branched from +`design/mobilityops-premium-ui` @ `54dc952`) and the live Unraid deployment at +`http://192.168.10.150:1236` (same revision — see `server-baseline.md`). Findings below are +either reproduced directly (curl against the live server, or reading the exact source lines) +or are structural gaps confirmed against the task's own explicit requirements. Items already +on record as accepted PoC tradeoffs (`docs/deferred.md`, `docs/01-scope-and-non-goals.md`, +`PROJECT_STATE.md` known-limitations) are excluded — this file only lists items that are +genuinely open. + +## Method + +- Full read of `backend/app/api/routers/*.py`, `backend/app/api/deps.py`, + `backend/app/services/returns.py`, `backend/app/models/*.py`, + `frontend/src/pages/*.tsx`, `frontend/src/components/Layout.tsx`, + `frontend/src/context/AuthContext.tsx`, `frontend/src/App.tsx`. +- Live curl verification against `http://192.168.10.150:1236` for the auth/role findings. +- Cross-checked every finding against `docs/deferred.md`, + `artifacts/final-acceptance/summary.md`, and the latest `PROJECT_STATE.md` sections to + avoid re-flagging already-accepted tradeoffs. + +## Findings + +### F1 — Vehicles page renders the unfiltered array (client-side search is inert) + +- Severity: high. Role: both. Route: `/vehicles`. Component: + `frontend/src/pages/Vehicles.tsx`. +- Repro: type any text into the "Search" box on the Vehicles page. The row count label + (`{filtered.length} vehicles`) updates and the empty state correctly appears when nothing + matches, but the `
` mapped over the raw `vehicles` array, not the computed + `filtered` array — every original row stayed visible regardless of the search text. +- Expected: only rows matching the search (combined with the status/attention filters) + render. +- Cause: `vehicles.map(...)` at the table body instead of `filtered.map(...)` (line 78 as + originally read). +- Reproduces locally: yes (read). Reproduces on Unraid: yes (identical bundled source, + `source-revision` matches). +- Fix: **applied** — render body now maps `filtered`. Regression test: + `frontend/e2e/*.spec.ts` search-changes-rows case (added in Batch 1). + +### F2 — Bookings page renders the unfiltered, unpaginated array + +- Severity: high. Role: both. Route: `/bookings`. Component: + `frontend/src/pages/Bookings.tsx`. +- Repro: same class of bug — `visible` (filtered + sliced to 25/page) was computed and used + for the meta line and pagination controls, but `` mapped over the raw `bookings` + array. All bookings rendered on every page regardless of filter or page number. +- Cause: `bookings.map(...)` instead of `visible.map(...)`. +- Fix: **applied** — render body now maps `visible`. Added a page-clamp effect so `page` + cannot point past the last valid page when the filtered set shrinks (e.g. after a status + filter reload returns fewer results than the current page implies). +- Reproduces locally/Unraid: yes/yes. + +### F3 — Session state is `sessionStorage`-authoritative, not server-verified + +- Severity: high. Role: both. Component: `frontend/src/context/AuthContext.tsx`. +- The logged-in `user` object is read from and written to `sessionStorage` + (`mobilityops.demo-user`) directly; there is no call on app start to verify the HttpOnly + session cookie is still valid, and `logout()` only clears local state — it never calls the + server to invalidate the cookie. A stale/edited `sessionStorage` entry (or a cookie that + expired server-side) will keep protected pages rendering as if authenticated until the + first API call 401s, and even then nothing centrally redirects to `/login` (`isSessionExpired` + helper exists in the same file but is never imported/called anywhere). +- Live confirmation: no `GET /api/v1/demo/session` or `POST /api/v1/demo/logout` endpoint + exists yet (`curl` returns 404 for both against the live server). +- Fix: Batch 1 — add both endpoints server-side, make `AuthProvider` verify against + `GET /demo/session` on load, call `POST /demo/logout` on sign-out, and centrally react to + 401s from the `api` client. + +### F4 — Data-quality workbench has no role gate at all (list, detail, defer, reject) + +- Severity: high. Role: Rental Employee. Routes: `/data-quality`, `/data-quality/:ref`. + Endpoints: `GET /api/v1/data-quality/issues`, `GET /api/v1/data-quality/issues/{ref}`, + `POST .../defer`, `POST .../reject`. +- Live confirmation: logged in as `rental_employee` on the live server, `GET + /api/v1/data-quality/issues` and `GET /api/v1/audit` both returned `200` (curl evidence + above). Only `merge-customers` and `scan` are gated to Operations Manager; `defer`/`reject` + are not, and the whole workbench is reachable and actionable by Rental Employee both via + direct API call and via the UI (`DataQuality.tsx` has no role check at all; `Layout.tsx` + shows the "Data quality" and "Audit trail" nav items unconditionally to both roles). +- The task's role matrix (this brief, section 4) puts data-quality and audit entirely under + Operations Manager — Rental Employee's list is dashboard/fleet/vehicle + detail/bookings/booking detail/return/knowledge only. Decision recorded: tighten + `list_issues`, `get_issue`, `defer`, `reject`, and `GET /api/v1/audit` to + `require_operations_manager`; hide the nav items and show the same restricted-message + pattern already used by `Automation.tsx` for direct URL access by Rental Employee. +- Fix: Batch 1. + +### F5 — Return preview does not exist; the review step (if any) cannot be authoritative + +- Severity: high. Section 5 requirement. No `POST + /api/v1/bookings/{public_ref}/return-preview` (or equivalent) endpoint exists anywhere in + `backend/app/api/routers/bookings.py`. The frontend return flow can therefore only ever + show a client-guessed preview, or skip a real preview step entirely. +- Fix: Batch 2 — one authoritative evaluation function shared by preview (no writes) and + commit. + +### F6 — Audit API never exposes `before_json`/`after_json` + +- Severity: medium. `AuditEvent` (`backend/app/models/audit.py`) stores `before_json` and + `after_json`, populated by `record_audit_event` call sites (e.g. `return_registered`, + `vehicle_status_changed`), but `AuditEventOut` (`backend/app/schemas.py`) and the router + (`backend/app/api/routers/audit.py`) only ever return `metadata`, never before/after. The + UI (`Audit.tsx`) therefore cannot show what changed, only that something happened. +- Fix: Batch 2 — add `before`/`after` to `AuditEventOut`, resolve a safe entity link where + possible, render human-readable before/after in the UI behind progressive disclosure. + +### F7 — Data-quality issue evidence is a raw JSON dump for 4 of 5 rule types + +- Severity: medium. `DataQualityIssueDetail.tsx`: for every rule type except + `possible_duplicate_customer`, the only resolution surface is `{JSON.stringify(issue.evidence,
+ null, 2)}` plus generic Defer/Reject buttons. `missing_required_field`,
+ `odometer_regression`, `booking_overlap`, and `vehicle_status_conflict` have no typed,
+ bounded resolution flow at all.
+- Fix: Batch 3.
+
+### F8 — Related-snapshot typing is inferred from rule type, not explicit
+
+- Severity: low. `backend/app/api/routers/data_quality.py::get_issue` infers
+ `related_entity_type = "customer" if issue.rule_type == "possible_duplicate_customer" else
+ "vehicle"` — a `booking_overlap` issue's related entity is actually a booking, not a
+ vehicle, so its snapshot lookup silently returns `None` today. Confirmed by reading
+ `_snapshot()`, which only knows how to look up `customer` or `vehicle` rows.
+- Fix: Batch 3 — typed snapshots for customer/vehicle/booking/inspection.
+
+### F9 — Global search is a blind client-side regex guesser
+
+- Severity: medium. `frontend/src/components/Layout.tsx::handleSearch` pattern-matches
+ `MO-…`/`BK-…`/`DQ-…` and navigates without checking the entity exists, or fuzzy-matches a
+ hardcoded `SEARCH_DESTINATIONS` term list. No backend search endpoint exists. No results
+ panel, no keyboard navigation within results, no role filtering, no debounced live query —
+ it is a single-shot form submit.
+- Fix: Batch 4 — `GET /api/v1/search`.
+
+### F10 — n8n integration status is derived from the single most recent outbox event
+
+- Severity: medium. `Automation.tsx` line: `