feat(returns): add authoritative return preview

The return-review step predicted operational consequences independently in
the frontend, and got it wrong: damage or a technical warning was described
as routing to "maintenance" when the actual domain rule (returns.py) routes
it to "blocked", and the no-contradiction case was described as becoming
"available" when the vehicle actually always goes to "cleaning" first
(only reaching "maintenance" if the service threshold was crossed).

Extract the evaluation returns.py already performed inline into a pure
evaluate_return() function with no writes -- resulting status (with an
explanation), odometer regression, would-create-quality-issue,
next-booking-risk -- and share it between a new non-mutating
POST /bookings/{ref}/return-preview endpoint and the existing commit path,
so preview and commit can never drift apart again. The result screen also
now distinguishes local commit success from n8n delivery (still queued/
unconfirmed) instead of implying both succeeded, and links to any created
quality issue for Operations Manager.
This commit is contained in:
NuklearRabbit
2026-08-02 05:33:12 +02:00
parent 62ac9f825c
commit f5212959b4
8 changed files with 435 additions and 77 deletions
+35 -2
View File
@@ -8,8 +8,14 @@ from app.api.deps import get_current_user, get_db
from app.models.booking import Booking
from app.models.customer import Customer
from app.models.vehicle import Vehicle
from app.schemas import BookingOut, CurrentUser, RegisterReturnRequest
from app.services.returns import register_vehicle_return
from app.schemas import (
BookingOut,
CurrentUser,
NextBookingRisk,
RegisterReturnRequest,
ReturnPreviewResult,
)
from app.services.returns import preview_vehicle_return, register_vehicle_return
router = APIRouter(prefix="/api/v1/bookings", tags=["bookings"])
@@ -66,6 +72,33 @@ def get_booking(
return _to_out(booking, customer, vehicle)
@router.post("/{public_ref}/return-preview", response_model=ReturnPreviewResult)
def preview_return(
public_ref: str,
body: RegisterReturnRequest,
db: Session = Depends(get_db),
_user: CurrentUser = Depends(get_current_user),
) -> ReturnPreviewResult:
booking, vehicle, evaluation = preview_vehicle_return(db, public_ref, body)
return ReturnPreviewResult(
booking_ref=booking.public_ref,
vehicle_ref=vehicle.public_ref,
canonical_odometer_km=evaluation.canonical_odometer_km,
submitted_odometer_km=evaluation.submitted_odometer_km,
odometer_regression=evaluation.odometer_regression,
resulting_odometer_km=evaluation.resulting_odometer_km,
resulting_vehicle_status=evaluation.resulting_vehicle_status,
status_reason=evaluation.status_reason,
would_create_quality_issue=evaluation.would_create_quality_issue,
attention_reasons=evaluation.attention_reasons,
next_booking_risk=(
NextBookingRisk(**evaluation.next_booking_risk)
if evaluation.next_booking_risk is not None
else None
),
)
@router.post("/{public_ref}/return")
def register_return(
public_ref: str,
+18
View File
@@ -74,6 +74,20 @@ class RegisterReturnResult(BaseModel):
next_booking_risk: NextBookingRisk | None
class ReturnPreviewResult(BaseModel):
booking_ref: str
vehicle_ref: str
canonical_odometer_km: int
submitted_odometer_km: int
odometer_regression: bool
resulting_odometer_km: int
resulting_vehicle_status: str
status_reason: str
would_create_quality_issue: bool
attention_reasons: list[str]
next_booking_risk: NextBookingRisk | None
class InspectionOut(BaseModel):
public_ref: str
booking_ref: str
@@ -216,6 +230,10 @@ class AuditEventOut(BaseModel):
action: str
entity_type: str
entity_id: str | None
entity_ref: str | None = None
entity_link: str | None = None
correlation_id: str
occurred_at: datetime
before: dict[str, Any] | None = None
after: dict[str, Any] | None = None
metadata: dict[str, Any] | None = None
+123 -50
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import uuid
from dataclasses import dataclass
from datetime import UTC, datetime
from sqlalchemy import select
@@ -25,12 +26,120 @@ def _next_public_ref(db: Session) -> str:
return f"{REF_PREFIX}-{len(existing) + 1:04d}"
def _derive_vehicle_status(body: RegisterReturnRequest, vehicle: Vehicle, new_odometer: int) -> str:
if body.damage_reported or body.technical_warning:
return "blocked"
def _derive_vehicle_status_with_reason(
body: RegisterReturnRequest, vehicle: Vehicle, new_odometer: int
) -> tuple[str, str]:
if body.damage_reported and body.technical_warning:
return "blocked", "Damage and a technical warning were both reported on return."
if body.damage_reported:
return "blocked", "Damage was reported on return."
if body.technical_warning:
return "blocked", "A technical warning was reported on return."
if new_odometer >= vehicle.next_service_km:
return "maintenance"
return "cleaning"
return (
"maintenance",
f"Odometer reached the {vehicle.next_service_km:,} km service threshold.",
)
return "cleaning", "No damage, technical warning or service threshold; routed to cleaning."
@dataclass
class ReturnEvaluation:
canonical_odometer_km: int
submitted_odometer_km: int
odometer_regression: bool
resulting_odometer_km: int
resulting_vehicle_status: str
status_reason: str
would_create_quality_issue: bool
attention_reasons: list[str]
next_booking_risk: dict | None
def evaluate_return(
db: Session, booking: Booking, vehicle: Vehicle, body: RegisterReturnRequest, *, now: datetime
) -> ReturnEvaluation:
"""Pure evaluation of what a return would do. No writes; safe to call from a
non-mutating preview endpoint. `register_vehicle_return` uses the same function so
preview and commit can never drift apart."""
odometer_regression = body.end_odometer_km < vehicle.odometer_km
resulting_odometer_km = vehicle.odometer_km if odometer_regression else body.end_odometer_km
resulting_status, status_reason = _derive_vehicle_status_with_reason(
body, vehicle, resulting_odometer_km
)
attention_reasons = []
if body.damage_reported:
attention_reasons.append("damage_reported")
if body.technical_warning:
attention_reasons.append("technical_warning")
if odometer_regression:
attention_reasons.append("odometer_regression")
next_booking = db.scalar(
select(Booking)
.where(
Booking.vehicle_id == vehicle.id,
Booking.status == "reserved",
Booking.starts_at > now,
)
.order_by(Booking.starts_at.asc())
)
next_booking_risk = None
if next_booking is not None:
hours_until = (next_booking.starts_at - now).total_seconds() / 3600
next_booking_risk = {
"booking_ref": next_booking.public_ref,
"starts_at": next_booking.starts_at.isoformat(),
"at_risk": resulting_status != "cleaning" or hours_until < 4,
}
return ReturnEvaluation(
canonical_odometer_km=vehicle.odometer_km,
submitted_odometer_km=body.end_odometer_km,
odometer_regression=odometer_regression,
resulting_odometer_km=resulting_odometer_km,
resulting_vehicle_status=resulting_status,
status_reason=status_reason,
would_create_quality_issue=odometer_regression,
attention_reasons=attention_reasons,
next_booking_risk=next_booking_risk,
)
def _load_active_booking_and_vehicle(
db: Session, booking_ref: str, *, lock: bool
) -> tuple[Booking, Vehicle]:
stmt = select(Booking).where(Booking.public_ref == booking_ref)
if lock:
stmt = stmt.with_for_update()
booking = db.scalar(stmt)
if booking is None:
raise AppError("BOOKING_NOT_FOUND", "Booking not found.", status_code=404)
vehicle_stmt = select(Vehicle).where(Vehicle.id == booking.vehicle_id)
if lock:
vehicle_stmt = vehicle_stmt.with_for_update()
vehicle = db.scalar(vehicle_stmt)
if vehicle is None:
raise AppError(
"VEHICLE_NOT_FOUND", "The vehicle for this booking could not be found.", status_code=404
)
return booking, vehicle
def preview_vehicle_return(
db: Session, booking_ref: str, body: RegisterReturnRequest
) -> tuple[Booking, Vehicle, ReturnEvaluation]:
booking, vehicle = _load_active_booking_and_vehicle(db, booking_ref, lock=False)
if booking.status != "active":
raise AppError(
"INVALID_BOOKING_STATE",
f"Booking is '{booking.status}', not 'active'; it cannot be returned.",
status_code=409,
)
evaluation = evaluate_return(db, booking, vehicle, body, now=datetime.now(UTC))
return booking, vehicle, evaluation
def register_vehicle_return(
@@ -53,14 +162,7 @@ def register_vehicle_return(
)
return existing.response_status, existing.response_body
booking = db.scalar(select(Booking).where(Booking.public_ref == booking_ref).with_for_update())
if booking is None:
raise AppError("BOOKING_NOT_FOUND", "Booking not found.", status_code=404)
vehicle = db.scalar(select(Vehicle).where(Vehicle.id == booking.vehicle_id).with_for_update())
if vehicle is None:
raise AppError(
"VEHICLE_NOT_FOUND", "The vehicle for this booking could not be found.", status_code=404
)
booking, vehicle = _load_active_booking_and_vehicle(db, booking_ref, lock=True)
# Re-check after acquiring the row lock: a concurrent identical-key request may have
# just committed while we were waiting.
@@ -79,6 +181,7 @@ def register_vehicle_return(
now = datetime.now(UTC)
correlation_id = uuid.uuid4()
evaluation = evaluate_return(db, booking, vehicle, body, now=now)
inspection = Inspection(
public_ref=_next_public_ref(db),
@@ -104,13 +207,8 @@ def register_vehicle_return(
booking.status = "returned"
booking.end_odometer_km = body.end_odometer_km
odometer_regression = body.end_odometer_km < vehicle.odometer_km
quality_issue_ref: str | None = None
canonical_odometer = vehicle.odometer_km
if not odometer_regression:
canonical_odometer = body.end_odometer_km
vehicle.odometer_km = canonical_odometer
else:
if evaluation.odometer_regression:
issue = DataQualityIssue(
public_ref=f"DQ-RET-{str(inspection.public_ref).split('-')[-1]}",
rule_type="odometer_regression",
@@ -121,7 +219,7 @@ def register_vehicle_return(
evidence_json={
"summary": (
f"Return submitted {body.end_odometer_km} km, below canonical "
f"{vehicle.odometer_km} km."
f"{evaluation.canonical_odometer_km} km."
),
"entity_ref": vehicle.public_ref,
"related_refs": [booking.public_ref, inspection.public_ref],
@@ -133,7 +231,8 @@ def register_vehicle_return(
db.flush()
quality_issue_ref = issue.public_ref
resulting_status = _derive_vehicle_status(body, vehicle, canonical_odometer)
resulting_status = evaluation.resulting_vehicle_status
vehicle.odometer_km = evaluation.resulting_odometer_km
vehicle.operational_status = resulting_status
vehicle.version += 1
@@ -164,14 +263,6 @@ def register_vehicle_return(
},
)
attention_reasons = []
if body.damage_reported:
attention_reasons.append("damage_reported")
if body.technical_warning:
attention_reasons.append("technical_warning")
if odometer_regression:
attention_reasons.append("odometer_regression")
event = OutboxEvent(
event_id=uuid.uuid4(),
event_type="vehicle.returned.v1",
@@ -189,7 +280,7 @@ def register_vehicle_return(
"vehicle_ref": vehicle.public_ref,
"inspection_ref": inspection.public_ref,
"resulting_vehicle_status": resulting_status,
"attention_reasons": attention_reasons,
"attention_reasons": evaluation.attention_reasons,
},
"aggregate_ref": booking.public_ref,
},
@@ -199,33 +290,15 @@ def register_vehicle_return(
)
db.add(event)
next_booking = db.scalar(
select(Booking)
.where(
Booking.vehicle_id == vehicle.id,
Booking.status == "reserved",
Booking.starts_at > now,
)
.order_by(Booking.starts_at.asc())
)
next_booking_risk = None
if next_booking is not None:
hours_until = (next_booking.starts_at - now).total_seconds() / 3600
next_booking_risk = {
"booking_ref": next_booking.public_ref,
"starts_at": next_booking.starts_at.isoformat(),
"at_risk": resulting_status != "cleaning" or hours_until < 4,
}
response_body = {
"booking_ref": booking.public_ref,
"vehicle_ref": vehicle.public_ref,
"inspection_ref": inspection.public_ref,
"resulting_vehicle_status": resulting_status,
"odometer_regression": odometer_regression,
"odometer_regression": evaluation.odometer_regression,
"quality_issue_ref": quality_issue_ref,
"workflow_event_id": str(event.event_id),
"next_booking_risk": next_booking_risk,
"next_booking_risk": evaluation.next_booking_risk,
}
db.add(
+147
View File
@@ -1,11 +1,16 @@
import threading
import uuid
from datetime import UTC, datetime, timedelta
from fastapi.testclient import TestClient
from sqlalchemy import select
from app.core.db import SessionLocal
from app.main import app
from app.models.audit import AuditEvent
from app.models.booking import Booking
from app.models.customer import Customer
from app.models.outbox import OutboxEvent
from app.models.vehicle import Vehicle
@@ -39,6 +44,148 @@ def _activate_booking(vehicle_ref: str, start_odometer_km: int) -> str:
db.close()
def _set_next_service_km(vehicle_ref: str, threshold: int) -> None:
db = SessionLocal()
try:
vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref))
vehicle.next_service_km = threshold
db.commit()
finally:
db.close()
def _add_reserved_booking(vehicle_ref: str, *, hours_from_now: float) -> str:
db = SessionLocal()
try:
vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref))
customer = db.scalar(select(Customer))
starts_at = datetime.now(UTC) + timedelta(hours=hours_from_now)
public_ref = f"BK-TEST-{uuid.uuid4().hex[:8].upper()}"
booking = Booking(
public_ref=public_ref,
customer_id=customer.id,
vehicle_id=vehicle.id,
starts_at=starts_at,
ends_at=starts_at + timedelta(days=2),
status="reserved",
requirements_complete=True,
)
db.add(booking)
db.commit()
return public_ref
finally:
db.close()
def _counts() -> tuple[int, int]:
db = SessionLocal()
try:
return (
len(db.scalars(select(AuditEvent)).all()),
len(db.scalars(select(OutboxEvent)).all()),
)
finally:
db.close()
def test_preview_performs_no_writes_and_matches_commit(ops_client):
booking_ref = _activate_booking("MO-006", start_odometer_km=30000)
vehicle_before = ops_client.get("/api/v1/vehicles/MO-006").json()
new_reading = vehicle_before["odometer_km"] + 25
body = _return_body(end_odometer_km=new_reading)
audit_before, outbox_before = _counts()
preview = ops_client.post(f"/api/v1/bookings/{booking_ref}/return-preview", json=body)
assert preview.status_code == 200
preview_body = preview.json()
audit_after, outbox_after = _counts()
assert (audit_after, outbox_after) == (audit_before, outbox_before)
booking_mid = ops_client.get(f"/api/v1/bookings/{booking_ref}").json()
assert booking_mid["status"] == "active" # preview did not mutate the booking
vehicle_mid = ops_client.get("/api/v1/vehicles/MO-006").json()
assert vehicle_mid["odometer_km"] == vehicle_before["odometer_km"]
assert preview_body["odometer_regression"] is False
assert preview_body["resulting_odometer_km"] == new_reading
assert preview_body["canonical_odometer_km"] == vehicle_before["odometer_km"]
commit = ops_client.post(
f"/api/v1/bookings/{booking_ref}/return",
json=body,
headers={"Idempotency-Key": "test-preview-matches-commit-001"},
)
assert commit.status_code == 201
commit_body = commit.json()
assert commit_body["resulting_vehicle_status"] == preview_body["resulting_vehicle_status"]
assert commit_body["odometer_regression"] == preview_body["odometer_regression"]
assert commit_body["next_booking_risk"] == preview_body["next_booking_risk"]
def test_preview_detects_odometer_regression(ops_client):
booking_ref = _activate_booking("MO-007", start_odometer_km=15000)
vehicle_before = ops_client.get("/api/v1/vehicles/MO-007").json()
low_reading = vehicle_before["odometer_km"] - 100
preview = ops_client.post(
f"/api/v1/bookings/{booking_ref}/return-preview",
json=_return_body(end_odometer_km=low_reading),
)
assert preview.status_code == 200
body = preview.json()
assert body["odometer_regression"] is True
assert body["would_create_quality_issue"] is True
assert "odometer_regression" in body["attention_reasons"]
assert body["resulting_odometer_km"] == vehicle_before["odometer_km"]
vehicle_after = ops_client.get("/api/v1/vehicles/MO-007").json()
assert vehicle_after["odometer_km"] == vehicle_before["odometer_km"]
def test_preview_detects_service_due(ops_client):
booking_ref = _activate_booking("MO-009", start_odometer_km=18000)
vehicle_before = ops_client.get("/api/v1/vehicles/MO-009").json()
_set_next_service_km("MO-009", vehicle_before["odometer_km"] + 50)
preview = ops_client.post(
f"/api/v1/bookings/{booking_ref}/return-preview",
json=_return_body(end_odometer_km=vehicle_before["odometer_km"] + 100),
)
assert preview.status_code == 200
body = preview.json()
assert body["resulting_vehicle_status"] == "maintenance"
assert "service threshold" in body["status_reason"]
def test_preview_detects_next_booking_risk(ops_client):
booking_ref = _activate_booking("MO-011", start_odometer_km=19000)
_add_reserved_booking("MO-011", hours_from_now=2)
preview = ops_client.post(
f"/api/v1/bookings/{booking_ref}/return-preview",
json=_return_body(end_odometer_km=19500),
)
assert preview.status_code == 200
risk = preview.json()["next_booking_risk"]
assert risk is not None
assert risk["at_risk"] is True # less than 4 hours away
def test_preview_requires_active_booking(ops_client):
booking_ref = _activate_booking("MO-014", start_odometer_km=21000)
ops_client.post(
f"/api/v1/bookings/{booking_ref}/return",
json=_return_body(end_odometer_km=21500),
headers={"Idempotency-Key": "test-preview-requires-active-001"},
)
preview = ops_client.post(
f"/api/v1/bookings/{booking_ref}/return-preview",
json=_return_body(end_odometer_km=22000),
)
assert preview.status_code == 409
assert preview.json()["error"]["code"] == "INVALID_BOOKING_STATE"
def test_register_return_success_updates_canonical_odometer(ops_client):
booking_ref = _activate_booking("MO-003", start_odometer_km=20000)
vehicle_before = ops_client.get("/api/v1/vehicles/MO-003").json()