feat(audit): expose structured before/after evidence
audit_events already stored before_json/after_json, but the API and UI only ever surfaced metadata -- the audit trail could say something happened but never show what changed. Add before/after to AuditEventOut, resolve a safe entity_ref/entity_link for vehicle/booking/data-quality-issue entities (customer stays label-only; no customer detail route exists in this PoC), and render a human-readable change summary in the UI with the raw before/after/metadata JSON kept behind a <details> disclosure rather than shown by default.
This commit is contained in:
@@ -1,15 +1,55 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
from collections.abc import Sequence
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, Query
|
from fastapi import APIRouter, Depends, Query
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app.api.deps import get_db, require_operations_manager
|
from app.api.deps import get_db, require_operations_manager
|
||||||
from app.models.audit import AuditEvent
|
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
|
from app.schemas import AuditEventOut, CurrentUser
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/v1/audit", tags=["audit"])
|
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])
|
@router.get("", response_model=list[AuditEventOut])
|
||||||
def list_audit_events(
|
def list_audit_events(
|
||||||
@@ -31,17 +71,27 @@ def list_audit_events(
|
|||||||
if correlation_id:
|
if correlation_id:
|
||||||
stmt = stmt.where(AuditEvent.correlation_id == correlation_id)
|
stmt = stmt.where(AuditEvent.correlation_id == correlation_id)
|
||||||
events = db.scalars(stmt).all()
|
events = db.scalars(stmt).all()
|
||||||
return [
|
entity_refs = _resolve_entity_refs(db, events)
|
||||||
AuditEventOut(
|
|
||||||
id=str(e.id),
|
out = []
|
||||||
actor_type=e.actor_type,
|
for e in events:
|
||||||
actor_label=e.actor_label,
|
ref = entity_refs.get(e.entity_id) if e.entity_id else None
|
||||||
action=e.action,
|
route = _ROUTE_TEMPLATES.get(e.entity_type)
|
||||||
entity_type=e.entity_type,
|
out.append(
|
||||||
entity_id=str(e.entity_id) if e.entity_id else None,
|
AuditEventOut(
|
||||||
correlation_id=str(e.correlation_id),
|
id=str(e.id),
|
||||||
occurred_at=e.occurred_at,
|
actor_type=e.actor_type,
|
||||||
metadata=e.metadata_json,
|
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
|
||||||
]
|
|
||||||
|
|||||||
@@ -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):
|
def test_demo_login_is_audited(ops_client):
|
||||||
response = ops_client.get("/api/v1/audit", params={"action": "demo_login"})
|
response = ops_client.get("/api/v1/audit", params={"action": "demo_login"})
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -14,3 +21,54 @@ def test_audit_requires_authentication(client):
|
|||||||
def test_audit_requires_operations_manager(employee_client):
|
def test_audit_requires_operations_manager(employee_client):
|
||||||
response = employee_client.get("/api/v1/audit")
|
response = employee_client.get("/api/v1/audit")
|
||||||
assert response.status_code == 403
|
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"]
|
||||||
|
|||||||
@@ -113,6 +113,24 @@ test("bookings page: pagination renders at most 25 rows and page 2 differs from
|
|||||||
await expect(prevButton).toBeEnabled();
|
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 }) => {
|
test("data quality page: status and rule-type filters work", async ({ page }) => {
|
||||||
await page.goto("/data-quality");
|
await page.goto("/data-quality");
|
||||||
await expect(page.locator(".data-table")).toBeVisible();
|
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();
|
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 }) => {
|
test("knowledge page: form submits and clears input", async ({ page }) => {
|
||||||
await page.goto("/knowledge");
|
await page.goto("/knowledge");
|
||||||
const input = page.getByPlaceholder(/What must I do when a vehicle returns with damage/);
|
const input = page.getByPlaceholder(/What must I do when a vehicle returns with damage/);
|
||||||
|
|||||||
@@ -1,9 +1,25 @@
|
|||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
|
import { Link } from "react-router-dom";
|
||||||
import { api } from "../api/client";
|
import { api } from "../api/client";
|
||||||
import type { AuditEvent } from "../api/types";
|
import type { AuditEvent } from "../api/types";
|
||||||
import { useAuth } from "../context/AuthContext";
|
import { useAuth } from "../context/AuthContext";
|
||||||
import { EmptyState, ErrorState, LoadingState, PageHeader } from "../components/PageChrome";
|
import { EmptyState, ErrorState, LoadingState, PageHeader } from "../components/PageChrome";
|
||||||
|
|
||||||
|
function describeChanges(before: Record<string, unknown> | null, after: Record<string, unknown> | 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() {
|
export function Audit() {
|
||||||
const { user } = useAuth();
|
const { user } = useAuth();
|
||||||
const [events, setEvents] = useState<AuditEvent[] | null>(null);
|
const [events, setEvents] = useState<AuditEvent[] | null>(null);
|
||||||
@@ -60,7 +76,8 @@ export function Audit() {
|
|||||||
<th scope="col">Actor</th>
|
<th scope="col">Actor</th>
|
||||||
<th scope="col">Action</th>
|
<th scope="col">Action</th>
|
||||||
<th scope="col">Entity</th>
|
<th scope="col">Entity</th>
|
||||||
<th scope="col">Correlation</th>
|
<th scope="col">Change</th>
|
||||||
|
<th scope="col">Details</th>
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
<tbody>
|
<tbody>
|
||||||
@@ -73,8 +90,20 @@ export function Audit() {
|
|||||||
</td>
|
</td>
|
||||||
<td data-label="Actor"><strong>{e.actor_label}</strong><small className="table-subtext">{e.actor_type}</small></td>
|
<td data-label="Actor"><strong>{e.actor_label}</strong><small className="table-subtext">{e.actor_type}</small></td>
|
||||||
<td data-label="Action">{e.action.replace(/_/g, " ")}</td>
|
<td data-label="Action">{e.action.replace(/_/g, " ")}</td>
|
||||||
<td data-label="Entity">{e.entity_type}</td>
|
<td data-label="Entity">
|
||||||
<td className="mono" data-label="Correlation"><details><summary>{e.correlation_id.slice(0, 8)}</summary><pre>{JSON.stringify(e.metadata ?? {}, null, 2)}</pre></details></td>
|
{e.entity_link ? (
|
||||||
|
<Link to={e.entity_link}>{e.entity_ref ?? e.entity_type}</Link>
|
||||||
|
) : (
|
||||||
|
e.entity_ref ?? e.entity_type
|
||||||
|
)}
|
||||||
|
</td>
|
||||||
|
<td data-label="Change">{describeChanges(e.before, e.after)}</td>
|
||||||
|
<td className="mono" data-label="Details">
|
||||||
|
<details>
|
||||||
|
<summary>{e.correlation_id.slice(0, 8)}</summary>
|
||||||
|
<pre>{JSON.stringify({ before: e.before, after: e.after, metadata: e.metadata }, null, 2)}</pre>
|
||||||
|
</details>
|
||||||
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
))}
|
))}
|
||||||
</tbody>
|
</tbody>
|
||||||
|
|||||||
Reference in New Issue
Block a user