Add ruff linter/formatter for Python
- Add ruff as dev dependency - Configure ruff in pyproject.toml with strict 88-char line limit - Ignore B008 (FastAPI Depends pattern is standard) - Allow longer lines in tests for readability - Fix all lint issues in source files - Add Makefile targets: lint-backend, format-backend, fix-backend
This commit is contained in:
parent
69bc8413e0
commit
6c218130e9
31 changed files with 1234 additions and 876 deletions
|
|
@ -1,24 +1,24 @@
|
|||
"""Booking routes for users to book appointments."""
|
||||
from datetime import date, datetime, time, timedelta, timezone
|
||||
|
||||
from datetime import UTC, date, datetime, time, timedelta
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
||||
from sqlalchemy import select, and_, func
|
||||
from sqlalchemy import and_, func, select
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy.orm import joinedload
|
||||
|
||||
from auth import require_permission
|
||||
from database import get_db
|
||||
from models import User, Availability, Appointment, AppointmentStatus, Permission
|
||||
from models import Appointment, AppointmentStatus, Availability, Permission, User
|
||||
from schemas import (
|
||||
BookableSlot,
|
||||
AvailableSlotsResponse,
|
||||
BookingRequest,
|
||||
AppointmentResponse,
|
||||
AvailableSlotsResponse,
|
||||
BookableSlot,
|
||||
BookingRequest,
|
||||
PaginatedAppointments,
|
||||
)
|
||||
from shared_constants import SLOT_DURATION_MINUTES, MIN_ADVANCE_DAYS, MAX_ADVANCE_DAYS
|
||||
|
||||
from shared_constants import MAX_ADVANCE_DAYS, MIN_ADVANCE_DAYS, SLOT_DURATION_MINUTES
|
||||
|
||||
router = APIRouter(prefix="/api/booking", tags=["booking"])
|
||||
|
||||
|
|
@ -28,7 +28,7 @@ def _to_appointment_response(
|
|||
user_email: str | None = None,
|
||||
) -> AppointmentResponse:
|
||||
"""Convert an Appointment model to AppointmentResponse schema.
|
||||
|
||||
|
||||
Args:
|
||||
appointment: The appointment model instance
|
||||
user_email: Optional user email. If not provided, uses appointment.user.email
|
||||
|
|
@ -49,7 +49,7 @@ def _to_appointment_response(
|
|||
|
||||
def _get_valid_minute_boundaries() -> tuple[int, ...]:
|
||||
"""Get valid minute boundaries based on SLOT_DURATION_MINUTES.
|
||||
|
||||
|
||||
Assumes SLOT_DURATION_MINUTES divides 60 evenly (e.g., 15 minutes = 0, 15, 30, 45).
|
||||
"""
|
||||
boundaries: list[int] = []
|
||||
|
|
@ -74,12 +74,14 @@ def _validate_booking_date(d: date) -> None:
|
|||
if d < min_date:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot book for today or past dates. Earliest bookable date: {min_date}",
|
||||
detail=f"Cannot book for today or past dates. "
|
||||
f"Earliest bookable: {min_date}",
|
||||
)
|
||||
if d > max_date:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot book more than {MAX_ADVANCE_DAYS} days ahead. Latest bookable: {max_date}",
|
||||
detail=f"Cannot book more than {MAX_ADVANCE_DAYS} days ahead. "
|
||||
f"Latest bookable: {max_date}",
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -89,18 +91,18 @@ def _expand_availability_to_slots(
|
|||
) -> list[BookableSlot]:
|
||||
"""Expand availability time ranges into 15-minute bookable slots."""
|
||||
result: list[BookableSlot] = []
|
||||
|
||||
|
||||
for avail in availability_slots:
|
||||
# Create datetime objects for start and end
|
||||
current = datetime.combine(target_date, avail.start_time, tzinfo=timezone.utc)
|
||||
end = datetime.combine(target_date, avail.end_time, tzinfo=timezone.utc)
|
||||
|
||||
current = datetime.combine(target_date, avail.start_time, tzinfo=UTC)
|
||||
end = datetime.combine(target_date, avail.end_time, tzinfo=UTC)
|
||||
|
||||
# Generate 15-minute slots
|
||||
while current + timedelta(minutes=SLOT_DURATION_MINUTES) <= end:
|
||||
slot_end = current + timedelta(minutes=SLOT_DURATION_MINUTES)
|
||||
result.append(BookableSlot(start_time=current, end_time=slot_end))
|
||||
current = slot_end
|
||||
|
||||
|
||||
return result
|
||||
|
||||
|
||||
|
|
@ -112,7 +114,7 @@ async def get_available_slots(
|
|||
) -> AvailableSlotsResponse:
|
||||
"""Get available booking slots for a specific date."""
|
||||
_validate_booking_date(target_date)
|
||||
|
||||
|
||||
# Get availability for this date
|
||||
result = await db.execute(
|
||||
select(Availability)
|
||||
|
|
@ -120,20 +122,19 @@ async def get_available_slots(
|
|||
.order_by(Availability.start_time)
|
||||
)
|
||||
availability_slots = result.scalars().all()
|
||||
|
||||
|
||||
if not availability_slots:
|
||||
return AvailableSlotsResponse(date=target_date, slots=[])
|
||||
|
||||
|
||||
# Expand to 15-minute slots
|
||||
all_slots = _expand_availability_to_slots(availability_slots, target_date)
|
||||
|
||||
|
||||
# Get existing booked appointments for this date
|
||||
day_start = datetime.combine(target_date, time.min, tzinfo=timezone.utc)
|
||||
day_end = datetime.combine(target_date, time.max, tzinfo=timezone.utc)
|
||||
|
||||
day_start = datetime.combine(target_date, time.min, tzinfo=UTC)
|
||||
day_end = datetime.combine(target_date, time.max, tzinfo=UTC)
|
||||
|
||||
result = await db.execute(
|
||||
select(Appointment.slot_start)
|
||||
.where(
|
||||
select(Appointment.slot_start).where(
|
||||
and_(
|
||||
Appointment.slot_start >= day_start,
|
||||
Appointment.slot_start <= day_end,
|
||||
|
|
@ -142,13 +143,12 @@ async def get_available_slots(
|
|||
)
|
||||
)
|
||||
booked_starts = {row[0] for row in result.fetchall()}
|
||||
|
||||
|
||||
# Filter out already booked slots
|
||||
available_slots = [
|
||||
slot for slot in all_slots
|
||||
if slot.start_time not in booked_starts
|
||||
slot for slot in all_slots if slot.start_time not in booked_starts
|
||||
]
|
||||
|
||||
|
||||
return AvailableSlotsResponse(date=target_date, slots=available_slots)
|
||||
|
||||
|
||||
|
|
@ -161,27 +161,28 @@ async def create_booking(
|
|||
"""Book an appointment slot."""
|
||||
slot_date = request.slot_start.date()
|
||||
_validate_booking_date(slot_date)
|
||||
|
||||
# Validate slot is on the correct minute boundary (derived from SLOT_DURATION_MINUTES)
|
||||
|
||||
# Validate slot is on the correct minute boundary
|
||||
valid_minutes = _get_valid_minute_boundaries()
|
||||
if request.slot_start.minute not in valid_minutes:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Slot start time must be on {SLOT_DURATION_MINUTES}-minute boundary (valid minutes: {valid_minutes})",
|
||||
detail=f"Slot must be on {SLOT_DURATION_MINUTES}-minute boundary "
|
||||
f"(valid minutes: {valid_minutes})",
|
||||
)
|
||||
if request.slot_start.second != 0 or request.slot_start.microsecond != 0:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Slot start time must not have seconds or microseconds",
|
||||
)
|
||||
|
||||
|
||||
# Verify slot falls within availability
|
||||
slot_start_time = request.slot_start.time()
|
||||
slot_end_time = (request.slot_start + timedelta(minutes=SLOT_DURATION_MINUTES)).time()
|
||||
|
||||
slot_end_dt = request.slot_start + timedelta(minutes=SLOT_DURATION_MINUTES)
|
||||
slot_end_time = slot_end_dt.time()
|
||||
|
||||
result = await db.execute(
|
||||
select(Availability)
|
||||
.where(
|
||||
select(Availability).where(
|
||||
and_(
|
||||
Availability.date == slot_date,
|
||||
Availability.start_time <= slot_start_time,
|
||||
|
|
@ -190,13 +191,15 @@ async def create_booking(
|
|||
)
|
||||
)
|
||||
matching_availability = result.scalar_one_or_none()
|
||||
|
||||
|
||||
if not matching_availability:
|
||||
slot_str = request.slot_start.strftime("%Y-%m-%d %H:%M")
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Selected slot at {request.slot_start.strftime('%Y-%m-%d %H:%M')} UTC is not within any available time ranges for {slot_date}. Please select a different time slot.",
|
||||
detail=f"Selected slot at {slot_str} UTC is not within "
|
||||
f"any available time ranges for {slot_date}",
|
||||
)
|
||||
|
||||
|
||||
# Create the appointment
|
||||
slot_end = request.slot_start + timedelta(minutes=SLOT_DURATION_MINUTES)
|
||||
appointment = Appointment(
|
||||
|
|
@ -206,9 +209,9 @@ async def create_booking(
|
|||
note=request.note,
|
||||
status=AppointmentStatus.BOOKED,
|
||||
)
|
||||
|
||||
|
||||
db.add(appointment)
|
||||
|
||||
|
||||
try:
|
||||
await db.commit()
|
||||
await db.refresh(appointment)
|
||||
|
|
@ -216,9 +219,9 @@ async def create_booking(
|
|||
await db.rollback()
|
||||
raise HTTPException(
|
||||
status_code=409,
|
||||
detail="This slot has already been booked. Please select another slot.",
|
||||
)
|
||||
|
||||
detail="This slot has already been booked. Select another slot.",
|
||||
) from None
|
||||
|
||||
return _to_appointment_response(appointment, current_user.email)
|
||||
|
||||
|
||||
|
|
@ -241,60 +244,63 @@ async def get_my_appointments(
|
|||
.order_by(Appointment.slot_start.desc())
|
||||
)
|
||||
appointments = result.scalars().all()
|
||||
|
||||
return [
|
||||
_to_appointment_response(apt, current_user.email)
|
||||
for apt in appointments
|
||||
]
|
||||
|
||||
return [_to_appointment_response(apt, current_user.email) for apt in appointments]
|
||||
|
||||
|
||||
@appointments_router.post("/{appointment_id}/cancel", response_model=AppointmentResponse)
|
||||
@appointments_router.post(
|
||||
"/{appointment_id}/cancel", response_model=AppointmentResponse
|
||||
)
|
||||
async def cancel_my_appointment(
|
||||
appointment_id: int,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: User = Depends(require_permission(Permission.CANCEL_OWN_APPOINTMENT)),
|
||||
) -> AppointmentResponse:
|
||||
"""Cancel one of the current user's appointments."""
|
||||
# Get the appointment with explicit eager loading of user relationship
|
||||
# Get the appointment with eager loading of user relationship
|
||||
result = await db.execute(
|
||||
select(Appointment)
|
||||
.options(joinedload(Appointment.user))
|
||||
.where(Appointment.id == appointment_id)
|
||||
)
|
||||
appointment = result.scalar_one_or_none()
|
||||
|
||||
|
||||
if not appointment:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Appointment with ID {appointment_id} not found. It may have been deleted or the ID is invalid.",
|
||||
detail=f"Appointment {appointment_id} not found",
|
||||
)
|
||||
|
||||
|
||||
# Verify ownership
|
||||
if appointment.user_id != current_user.id:
|
||||
raise HTTPException(status_code=403, detail="Cannot cancel another user's appointment")
|
||||
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="Cannot cancel another user's appointment",
|
||||
)
|
||||
|
||||
# Check if already cancelled
|
||||
if appointment.status != AppointmentStatus.BOOKED:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot cancel appointment with status '{appointment.status.value}'"
|
||||
detail=f"Cannot cancel: status is '{appointment.status.value}'",
|
||||
)
|
||||
|
||||
|
||||
# Check if appointment is in the past
|
||||
if appointment.slot_start <= datetime.now(timezone.utc):
|
||||
appointment_time = appointment.slot_start.strftime('%Y-%m-%d %H:%M') + " UTC"
|
||||
if appointment.slot_start <= datetime.now(UTC):
|
||||
apt_time = appointment.slot_start.strftime("%Y-%m-%d %H:%M")
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot cancel appointment scheduled for {appointment_time} as it is in the past or has already started."
|
||||
detail=f"Cannot cancel appointment at {apt_time} UTC: "
|
||||
"already started or in the past",
|
||||
)
|
||||
|
||||
|
||||
# Cancel the appointment
|
||||
appointment.status = AppointmentStatus.CANCELLED_BY_USER
|
||||
appointment.cancelled_at = datetime.now(timezone.utc)
|
||||
|
||||
appointment.cancelled_at = datetime.now(UTC)
|
||||
|
||||
await db.commit()
|
||||
await db.refresh(appointment)
|
||||
|
||||
|
||||
return _to_appointment_response(appointment, current_user.email)
|
||||
|
||||
|
||||
|
|
@ -302,7 +308,9 @@ async def cancel_my_appointment(
|
|||
# Admin Appointments Endpoints
|
||||
# =============================================================================
|
||||
|
||||
admin_appointments_router = APIRouter(prefix="/api/admin/appointments", tags=["admin-appointments"])
|
||||
admin_appointments_router = APIRouter(
|
||||
prefix="/api/admin/appointments", tags=["admin-appointments"]
|
||||
)
|
||||
|
||||
|
||||
@admin_appointments_router.get("", response_model=PaginatedAppointments)
|
||||
|
|
@ -317,7 +325,7 @@ async def get_all_appointments(
|
|||
count_result = await db.execute(select(func.count(Appointment.id)))
|
||||
total = count_result.scalar() or 0
|
||||
total_pages = (total + per_page - 1) // per_page if total > 0 else 1
|
||||
|
||||
|
||||
# Get paginated appointments with explicit eager loading of user relationship
|
||||
offset = (page - 1) * per_page
|
||||
result = await db.execute(
|
||||
|
|
@ -328,13 +336,13 @@ async def get_all_appointments(
|
|||
.limit(per_page)
|
||||
)
|
||||
appointments = result.scalars().all()
|
||||
|
||||
|
||||
# Build responses using the eager-loaded user relationship
|
||||
records = [
|
||||
_to_appointment_response(apt) # Uses eager-loaded relationship
|
||||
for apt in appointments
|
||||
]
|
||||
|
||||
|
||||
return PaginatedAppointments(
|
||||
records=records,
|
||||
total=total,
|
||||
|
|
@ -344,47 +352,52 @@ async def get_all_appointments(
|
|||
)
|
||||
|
||||
|
||||
@admin_appointments_router.post("/{appointment_id}/cancel", response_model=AppointmentResponse)
|
||||
@admin_appointments_router.post(
|
||||
"/{appointment_id}/cancel", response_model=AppointmentResponse
|
||||
)
|
||||
async def admin_cancel_appointment(
|
||||
appointment_id: int,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
_current_user: User = Depends(require_permission(Permission.CANCEL_ANY_APPOINTMENT)),
|
||||
_current_user: User = Depends(
|
||||
require_permission(Permission.CANCEL_ANY_APPOINTMENT)
|
||||
),
|
||||
) -> AppointmentResponse:
|
||||
"""Cancel any appointment (admin only)."""
|
||||
# Get the appointment with explicit eager loading of user relationship
|
||||
# Get the appointment with eager loading of user relationship
|
||||
result = await db.execute(
|
||||
select(Appointment)
|
||||
.options(joinedload(Appointment.user))
|
||||
.where(Appointment.id == appointment_id)
|
||||
)
|
||||
appointment = result.scalar_one_or_none()
|
||||
|
||||
|
||||
if not appointment:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Appointment with ID {appointment_id} not found. It may have been deleted or the ID is invalid.",
|
||||
detail=f"Appointment {appointment_id} not found",
|
||||
)
|
||||
|
||||
|
||||
# Check if already cancelled
|
||||
if appointment.status != AppointmentStatus.BOOKED:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot cancel appointment with status '{appointment.status.value}'"
|
||||
detail=f"Cannot cancel: status is '{appointment.status.value}'",
|
||||
)
|
||||
|
||||
|
||||
# Check if appointment is in the past
|
||||
if appointment.slot_start <= datetime.now(timezone.utc):
|
||||
appointment_time = appointment.slot_start.strftime('%Y-%m-%d %H:%M') + " UTC"
|
||||
if appointment.slot_start <= datetime.now(UTC):
|
||||
apt_time = appointment.slot_start.strftime("%Y-%m-%d %H:%M")
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Cannot cancel appointment scheduled for {appointment_time} as it is in the past or has already started."
|
||||
detail=f"Cannot cancel appointment at {apt_time} UTC: "
|
||||
"already started or in the past",
|
||||
)
|
||||
|
||||
|
||||
# Cancel the appointment
|
||||
appointment.status = AppointmentStatus.CANCELLED_BY_ADMIN
|
||||
appointment.cancelled_at = datetime.now(timezone.utc)
|
||||
|
||||
appointment.cancelled_at = datetime.now(UTC)
|
||||
|
||||
await db.commit()
|
||||
await db.refresh(appointment)
|
||||
|
||||
|
||||
return _to_appointment_response(appointment) # Uses eager-loaded relationship
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue