diff --git a/backend/app/api/routers/audit.py b/backend/app/api/routers/audit.py index b5a5edf..de17abb 100644 --- a/backend/app/api/routers/audit.py +++ b/backend/app/api/routers/audit.py @@ -1,15 +1,55 @@ from __future__ import annotations +import uuid +from collections.abc import Sequence +from typing import Any + from fastapi import APIRouter, Depends, Query from sqlalchemy import select from sqlalchemy.orm import Session from app.api.deps import get_db, require_operations_manager from app.models.audit import AuditEvent +from app.models.booking import Booking +from app.models.customer import Customer +from app.models.data_quality import DataQualityIssue +from app.models.vehicle import Vehicle from app.schemas import AuditEventOut, CurrentUser router = APIRouter(prefix="/api/v1/audit", tags=["audit"]) +# Only entity types with a stable public reference and (optionally) a real frontend route +# are resolved here. Types like "system", "knowledge" or "mcp_tool" carry no linkable +# entity_id and are left as plain labels. +_ENTITY_MODELS: dict[str, Any] = { + "vehicle": Vehicle, + "booking": Booking, + "customer": Customer, + "data_quality_issue": DataQualityIssue, +} +_ROUTE_TEMPLATES: dict[str, str] = { + "vehicle": "/vehicles/{ref}", + "booking": "/bookings/{ref}", + "data_quality_issue": "/data-quality/{ref}", + # No customer detail route exists in this proof of concept; still resolve the + # reference for display, just without a link. +} + + +def _resolve_entity_refs(db: Session, events: Sequence[AuditEvent]) -> dict[uuid.UUID, str]: + ids_by_type: dict[str, set[uuid.UUID]] = {} + for event in events: + if event.entity_id is not None and event.entity_type in _ENTITY_MODELS: + ids_by_type.setdefault(event.entity_type, set()).add(event.entity_id) + + refs: dict[uuid.UUID, str] = {} + for entity_type, ids in ids_by_type.items(): + model = _ENTITY_MODELS[entity_type] + rows: Sequence[Any] = db.scalars(select(model).where(model.id.in_(ids))).all() + for row in rows: + refs[row.id] = row.public_ref + return refs + @router.get("", response_model=list[AuditEventOut]) def list_audit_events( @@ -31,17 +71,27 @@ def list_audit_events( if correlation_id: stmt = stmt.where(AuditEvent.correlation_id == correlation_id) events = db.scalars(stmt).all() - return [ - AuditEventOut( - id=str(e.id), - actor_type=e.actor_type, - actor_label=e.actor_label, - action=e.action, - entity_type=e.entity_type, - entity_id=str(e.entity_id) if e.entity_id else None, - correlation_id=str(e.correlation_id), - occurred_at=e.occurred_at, - metadata=e.metadata_json, + entity_refs = _resolve_entity_refs(db, events) + + out = [] + for e in events: + ref = entity_refs.get(e.entity_id) if e.entity_id else None + route = _ROUTE_TEMPLATES.get(e.entity_type) + out.append( + AuditEventOut( + id=str(e.id), + actor_type=e.actor_type, + actor_label=e.actor_label, + action=e.action, + entity_type=e.entity_type, + entity_id=str(e.entity_id) if e.entity_id else None, + entity_ref=ref, + entity_link=route.format(ref=ref) if route and ref else None, + correlation_id=str(e.correlation_id), + occurred_at=e.occurred_at, + before=e.before_json, + after=e.after_json, + metadata=e.metadata_json, + ) ) - for e in events - ] + return out diff --git a/backend/tests/test_audit.py b/backend/tests/test_audit.py index ecda136..78a9257 100644 --- a/backend/tests/test_audit.py +++ b/backend/tests/test_audit.py @@ -1,3 +1,10 @@ +from sqlalchemy import select + +from app.core.db import SessionLocal +from app.models.booking import Booking +from app.models.vehicle import Vehicle + + def test_demo_login_is_audited(ops_client): response = ops_client.get("/api/v1/audit", params={"action": "demo_login"}) assert response.status_code == 200 @@ -14,3 +21,54 @@ def test_audit_requires_authentication(client): def test_audit_requires_operations_manager(employee_client): response = employee_client.get("/api/v1/audit") assert response.status_code == 403 + + +def _activate_booking(vehicle_ref: str, start_odometer_km: int) -> str: + db = SessionLocal() + try: + vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref)) + booking = db.scalar( + select(Booking).where(Booking.vehicle_id == vehicle.id, Booking.status == "returned") + ) + booking.status = "active" + booking.start_odometer_km = start_odometer_km + booking.end_odometer_km = None + db.commit() + return booking.public_ref + finally: + db.close() + + +def test_return_registered_audit_event_exposes_before_after_and_link(ops_client): + booking_ref = _activate_booking("MO-015", start_odometer_km=17000) + vehicle_before = ops_client.get("/api/v1/vehicles/MO-015").json() + ops_client.post( + f"/api/v1/bookings/{booking_ref}/return", + json={ + "end_odometer_km": vehicle_before["odometer_km"] + 10, + "fuel_level_percent": 50, + "cleanliness_ok": True, + "damage_reported": False, + "technical_warning": False, + }, + headers={"Idempotency-Key": "test-audit-before-after-001"}, + ) + + key = "test-audit-before-after-001" + events = ops_client.get( + "/api/v1/audit", params={"action": "return_registered"} + ).json() + event = next(e for e in events if e["metadata"]["idempotency_key"] == key) + assert event["before"] == {"status": "active"} + assert event["after"]["status"] == "returned" + assert event["entity_ref"] == booking_ref + assert event["entity_link"] == f"/bookings/{booking_ref}" + + vehicle_events = ops_client.get( + "/api/v1/audit", + params={"action": "vehicle_status_changed", "correlation_id": event["correlation_id"]}, + ).json() + assert len(vehicle_events) == 1 + assert vehicle_events[0]["entity_ref"] == "MO-015" + assert vehicle_events[0]["entity_link"] == "/vehicles/MO-015" + assert vehicle_events[0]["before"]["odometer_km"] == vehicle_before["odometer_km"] diff --git a/frontend/e2e/interactive-elements.spec.ts b/frontend/e2e/interactive-elements.spec.ts index 24bfc66..9d9f7f3 100644 --- a/frontend/e2e/interactive-elements.spec.ts +++ b/frontend/e2e/interactive-elements.spec.ts @@ -113,6 +113,24 @@ test("bookings page: pagination renders at most 25 rows and page 2 differs from await expect(prevButton).toBeEnabled(); }); +test("return preview correctly reports blocked (not maintenance) for damage reported", async ({ + page, + request, +}) => { + await resetDemoData(request); + await page.goto("/bookings/BK-DEMO-RETURN"); + await page.getByLabel("End odometer (km)").fill("55000"); + await page.getByLabel("Fuel level (%)").fill("40"); + await page.getByRole("checkbox", { name: "Damage reported" }).check(); + await page.getByRole("button", { name: "Review return" }).click(); + + // The preview is the server's authoritative evaluation: damage always routes to + // "blocked", never "maintenance" -- this used to be guessed client-side and wrong. + await expect(page.getByText("Damage was reported on return.")).toBeVisible(); + const statusRegion = page.locator(".impact-preview"); + await expect(statusRegion.getByText("blocked", { exact: true })).toBeVisible(); +}); + test("data quality page: status and rule-type filters work", async ({ page }) => { await page.goto("/data-quality"); await expect(page.locator(".data-table")).toBeVisible(); @@ -174,6 +192,38 @@ test("audit page: action filter works", async ({ page }) => { expect(actions.every((a) => a.includes("demo login"))).toBeTruthy(); }); +test("audit page: shows human-readable before/after and a safe entity link", async ({ + page, + request, +}) => { + await resetDemoData(request); + // demo/reset deletes the acting session's own cookie, so submit the return through + // page.request instead -- it shares the browser context's still-valid OM session from + // beforeEach rather than the now-logged-out standalone `request` fixture. + const submitted = await page.request.post("/api/v1/bookings/BK-DEMO-RETURN/return", { + data: { + end_odometer_km: 60000, + fuel_level_percent: 55, + cleanliness_ok: true, + damage_reported: false, + technical_warning: false, + }, + headers: { "Idempotency-Key": "e2e-audit-before-after-check" }, + }); + expect(submitted.ok()).toBeTruthy(); + + await page.goto("/audit"); + await page.getByLabel("Action").fill("return_registered"); + await expect(page.locator(".data-table tbody tr").first()).toBeVisible(); + + const changeCell = page.locator(".data-table tbody tr").first().locator("td").nth(4); + await expect(changeCell).toContainText("status"); + await expect(changeCell).toContainText("returned"); + + const entityCell = page.locator(".data-table tbody tr").first().locator("td").nth(3); + await expect(entityCell.locator("a")).toHaveAttribute("href", /\/bookings\/BK-/); +}); + test("knowledge page: form submits and clears input", async ({ page }) => { await page.goto("/knowledge"); const input = page.getByPlaceholder(/What must I do when a vehicle returns with damage/); diff --git a/frontend/src/pages/Audit.tsx b/frontend/src/pages/Audit.tsx index 6bb65d2..01e522c 100644 --- a/frontend/src/pages/Audit.tsx +++ b/frontend/src/pages/Audit.tsx @@ -1,9 +1,25 @@ import { useEffect, useState } from "react"; +import { Link } from "react-router-dom"; import { api } from "../api/client"; import type { AuditEvent } from "../api/types"; import { useAuth } from "../context/AuthContext"; import { EmptyState, ErrorState, LoadingState, PageHeader } from "../components/PageChrome"; +function describeChanges(before: Record | null, after: Record | null): string { + if (!before && !after) return "No recorded change detail."; + const keys = new Set([...Object.keys(before ?? {}), ...Object.keys(after ?? {})]); + const lines: string[] = []; + for (const key of keys) { + const b = before?.[key]; + const a = after?.[key]; + if (JSON.stringify(b) === JSON.stringify(a)) continue; + if (b === undefined) lines.push(`${key}: set to ${JSON.stringify(a)}`); + else if (a === undefined) lines.push(`${key}: was ${JSON.stringify(b)}`); + else lines.push(`${key}: ${JSON.stringify(b)} → ${JSON.stringify(a)}`); + } + return lines.length > 0 ? lines.join("; ") : "No field-level change detected."; +} + export function Audit() { const { user } = useAuth(); const [events, setEvents] = useState(null); @@ -60,7 +76,8 @@ export function Audit() { Actor Action Entity - Correlation + Change + Details @@ -73,8 +90,20 @@ export function Audit() { {e.actor_label}{e.actor_type} {e.action.replace(/_/g, " ")} - {e.entity_type} -
{e.correlation_id.slice(0, 8)}
{JSON.stringify(e.metadata ?? {}, null, 2)}
+ + {e.entity_link ? ( + {e.entity_ref ?? e.entity_type} + ) : ( + e.entity_ref ?? e.entity_type + )} + + {describeChanges(e.before, e.after)} + +
+ {e.correlation_id.slice(0, 8)} +
{JSON.stringify({ before: e.before, after: e.after, metadata: e.metadata }, null, 2)}
+
+ ))}