Fix: Add pagination to admin appointments endpoint
- Added pagination with page/per_page query params - Fixed N+1 query by using eager-loaded user relationship - Removed unused _get_user_email helper function - Updated frontend to handle paginated response - Regenerated API types
This commit is contained in:
parent
1cd60b4bbc
commit
77e7f98e1e
4 changed files with 70 additions and 28 deletions
|
|
@ -2,7 +2,7 @@
|
||||||
from datetime import date, datetime, time, timedelta, timezone
|
from datetime import date, datetime, time, timedelta, timezone
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
from fastapi import APIRouter, Depends, HTTPException, Query
|
||||||
from sqlalchemy import select, and_
|
from sqlalchemy import select, and_, func
|
||||||
from sqlalchemy.exc import IntegrityError
|
from sqlalchemy.exc import IntegrityError
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
|
@ -14,6 +14,7 @@ from schemas import (
|
||||||
AvailableSlotsResponse,
|
AvailableSlotsResponse,
|
||||||
BookingRequest,
|
BookingRequest,
|
||||||
AppointmentResponse,
|
AppointmentResponse,
|
||||||
|
PaginatedAppointments,
|
||||||
)
|
)
|
||||||
from shared_constants import SLOT_DURATION_MINUTES, MIN_ADVANCE_DAYS, MAX_ADVANCE_DAYS
|
from shared_constants import SLOT_DURATION_MINUTES, MIN_ADVANCE_DAYS, MAX_ADVANCE_DAYS
|
||||||
|
|
||||||
|
|
@ -199,13 +200,6 @@ async def create_booking(
|
||||||
appointments_router = APIRouter(prefix="/api/appointments", tags=["appointments"])
|
appointments_router = APIRouter(prefix="/api/appointments", tags=["appointments"])
|
||||||
|
|
||||||
|
|
||||||
async def _get_user_email(db: AsyncSession, user_id: int) -> str:
|
|
||||||
"""Get user email by ID."""
|
|
||||||
result = await db.execute(select(User.email).where(User.id == user_id))
|
|
||||||
email = result.scalar_one_or_none()
|
|
||||||
return email or "unknown"
|
|
||||||
|
|
||||||
|
|
||||||
@appointments_router.get("", response_model=list[AppointmentResponse])
|
@appointments_router.get("", response_model=list[AppointmentResponse])
|
||||||
async def get_my_appointments(
|
async def get_my_appointments(
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
|
|
@ -296,34 +290,52 @@ async def cancel_my_appointment(
|
||||||
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=list[AppointmentResponse])
|
@admin_appointments_router.get("", response_model=PaginatedAppointments)
|
||||||
async def get_all_appointments(
|
async def get_all_appointments(
|
||||||
|
page: int = Query(1, ge=1),
|
||||||
|
per_page: int = Query(10, ge=1, le=100),
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
_current_user: User = Depends(require_permission(Permission.VIEW_ALL_APPOINTMENTS)),
|
_current_user: User = Depends(require_permission(Permission.VIEW_ALL_APPOINTMENTS)),
|
||||||
) -> list[AppointmentResponse]:
|
) -> PaginatedAppointments:
|
||||||
"""Get all appointments (admin only), sorted by date descending."""
|
"""Get all appointments (admin only), sorted by date descending with pagination."""
|
||||||
|
# Get total count
|
||||||
|
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 (user relationship is eager-loaded via lazy="joined")
|
||||||
|
offset = (page - 1) * per_page
|
||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
select(Appointment)
|
select(Appointment)
|
||||||
.order_by(Appointment.slot_start.desc())
|
.order_by(Appointment.slot_start.desc())
|
||||||
|
.offset(offset)
|
||||||
|
.limit(per_page)
|
||||||
)
|
)
|
||||||
appointments = result.scalars().all()
|
appointments = result.scalars().all()
|
||||||
|
|
||||||
responses = []
|
# Build responses using the eager-loaded user relationship
|
||||||
for apt in appointments:
|
records = [
|
||||||
user_email = await _get_user_email(db, apt.user_id)
|
AppointmentResponse(
|
||||||
responses.append(AppointmentResponse(
|
|
||||||
id=apt.id,
|
id=apt.id,
|
||||||
user_id=apt.user_id,
|
user_id=apt.user_id,
|
||||||
user_email=user_email,
|
user_email=apt.user.email, # Uses eager-loaded relationship
|
||||||
slot_start=apt.slot_start,
|
slot_start=apt.slot_start,
|
||||||
slot_end=apt.slot_end,
|
slot_end=apt.slot_end,
|
||||||
note=apt.note,
|
note=apt.note,
|
||||||
status=apt.status.value,
|
status=apt.status.value,
|
||||||
created_at=apt.created_at,
|
created_at=apt.created_at,
|
||||||
cancelled_at=apt.cancelled_at,
|
cancelled_at=apt.cancelled_at,
|
||||||
))
|
)
|
||||||
|
for apt in appointments
|
||||||
|
]
|
||||||
|
|
||||||
return responses
|
return PaginatedAppointments(
|
||||||
|
records=records,
|
||||||
|
total=total,
|
||||||
|
page=page,
|
||||||
|
per_page=per_page,
|
||||||
|
total_pages=total_pages,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@admin_appointments_router.post("/{appointment_id}/cancel", response_model=AppointmentResponse)
|
@admin_appointments_router.post("/{appointment_id}/cancel", response_model=AppointmentResponse)
|
||||||
|
|
@ -363,12 +375,10 @@ async def admin_cancel_appointment(
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(appointment)
|
await db.refresh(appointment)
|
||||||
|
|
||||||
user_email = await _get_user_email(db, appointment.user_id)
|
|
||||||
|
|
||||||
return AppointmentResponse(
|
return AppointmentResponse(
|
||||||
id=appointment.id,
|
id=appointment.id,
|
||||||
user_id=appointment.user_id,
|
user_id=appointment.user_id,
|
||||||
user_email=user_email,
|
user_email=appointment.user.email, # Uses eager-loaded relationship
|
||||||
slot_start=appointment.slot_start,
|
slot_start=appointment.slot_start,
|
||||||
slot_end=appointment.slot_end,
|
slot_end=appointment.slot_end,
|
||||||
note=appointment.note,
|
note=appointment.note,
|
||||||
|
|
|
||||||
|
|
@ -716,8 +716,13 @@ class TestAdminViewAppointments:
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
assert len(data) >= 1
|
# Paginated response
|
||||||
assert any(apt["note"] == "Test" for apt in data)
|
assert "records" in data
|
||||||
|
assert "total" in data
|
||||||
|
assert "page" in data
|
||||||
|
assert "per_page" in data
|
||||||
|
assert len(data["records"]) >= 1
|
||||||
|
assert any(apt["note"] == "Test" for apt in data["records"])
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_regular_user_cannot_view_all_appointments(self, client_factory, regular_user):
|
async def test_regular_user_cannot_view_all_appointments(self, client_factory, regular_user):
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ import { useRequireAuth } from "../../hooks/useRequireAuth";
|
||||||
import { components } from "../../generated/api";
|
import { components } from "../../generated/api";
|
||||||
|
|
||||||
type AppointmentResponse = components["schemas"]["AppointmentResponse"];
|
type AppointmentResponse = components["schemas"]["AppointmentResponse"];
|
||||||
|
type PaginatedAppointments = components["schemas"]["PaginatedResponse_AppointmentResponse_"];
|
||||||
|
|
||||||
// Helper to format datetime
|
// Helper to format datetime
|
||||||
function formatDateTime(isoString: string): string {
|
function formatDateTime(isoString: string): string {
|
||||||
|
|
@ -200,8 +201,9 @@ export default function AdminAppointmentsPage() {
|
||||||
|
|
||||||
const fetchAppointments = useCallback(async () => {
|
const fetchAppointments = useCallback(async () => {
|
||||||
try {
|
try {
|
||||||
const data = await api.get<AppointmentResponse[]>("/api/admin/appointments");
|
// Fetch with large per_page to get all appointments for now
|
||||||
setAppointments(data);
|
const data = await api.get<PaginatedAppointments>("/api/admin/appointments?per_page=100");
|
||||||
|
setAppointments(data.records);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error("Failed to fetch appointments:", err);
|
console.error("Failed to fetch appointments:", err);
|
||||||
setError("Failed to load appointments");
|
setError("Failed to load appointments");
|
||||||
|
|
|
||||||
|
|
@ -445,7 +445,7 @@ export interface paths {
|
||||||
};
|
};
|
||||||
/**
|
/**
|
||||||
* Get All Appointments
|
* Get All Appointments
|
||||||
* @description Get all appointments (admin only), sorted by date descending.
|
* @description Get all appointments (admin only), sorted by date descending with pagination.
|
||||||
*/
|
*/
|
||||||
get: operations["get_all_appointments_api_admin_appointments_get"];
|
get: operations["get_all_appointments_api_admin_appointments_get"];
|
||||||
put?: never;
|
put?: never;
|
||||||
|
|
@ -704,6 +704,19 @@ export interface components {
|
||||||
/** Revoked At */
|
/** Revoked At */
|
||||||
revoked_at: string | null;
|
revoked_at: string | null;
|
||||||
};
|
};
|
||||||
|
/** PaginatedResponse[AppointmentResponse] */
|
||||||
|
PaginatedResponse_AppointmentResponse_: {
|
||||||
|
/** Records */
|
||||||
|
records: components["schemas"]["AppointmentResponse"][];
|
||||||
|
/** Total */
|
||||||
|
total: number;
|
||||||
|
/** Page */
|
||||||
|
page: number;
|
||||||
|
/** Per Page */
|
||||||
|
per_page: number;
|
||||||
|
/** Total Pages */
|
||||||
|
total_pages: number;
|
||||||
|
};
|
||||||
/** PaginatedResponse[CounterRecordResponse] */
|
/** PaginatedResponse[CounterRecordResponse] */
|
||||||
PaginatedResponse_CounterRecordResponse_: {
|
PaginatedResponse_CounterRecordResponse_: {
|
||||||
/** Records */
|
/** Records */
|
||||||
|
|
@ -1617,7 +1630,10 @@ export interface operations {
|
||||||
};
|
};
|
||||||
get_all_appointments_api_admin_appointments_get: {
|
get_all_appointments_api_admin_appointments_get: {
|
||||||
parameters: {
|
parameters: {
|
||||||
query?: never;
|
query?: {
|
||||||
|
page?: number;
|
||||||
|
per_page?: number;
|
||||||
|
};
|
||||||
header?: never;
|
header?: never;
|
||||||
path?: never;
|
path?: never;
|
||||||
cookie?: never;
|
cookie?: never;
|
||||||
|
|
@ -1630,7 +1646,16 @@ export interface operations {
|
||||||
[name: string]: unknown;
|
[name: string]: unknown;
|
||||||
};
|
};
|
||||||
content: {
|
content: {
|
||||||
"application/json": components["schemas"]["AppointmentResponse"][];
|
"application/json": components["schemas"]["PaginatedResponse_AppointmentResponse_"];
|
||||||
|
};
|
||||||
|
};
|
||||||
|
/** @description Validation Error */
|
||||||
|
422: {
|
||||||
|
headers: {
|
||||||
|
[name: string]: unknown;
|
||||||
|
};
|
||||||
|
content: {
|
||||||
|
"application/json": components["schemas"]["HTTPValidationError"];
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue