From ad91868f817062c36db2bf9cd904a5c25c4fef0b Mon Sep 17 00:00:00 2001 From: Cliff Hill Date: Sat, 6 Sep 2025 13:08:06 -0400 Subject: [PATCH] Making the booking form better. Signed-off-by: Cliff Hill --- backend/migrations/env.py | 20 +- .../0002_add_booking_exclusion_constraint.py | 2 +- .../versions/0006_make_bookings_tz_aware.py | 2 +- backend/src/backend/routers/bookings.py | 18 +- backend/src/backend/services/bookings.py | 8 +- compose.dev.yml | 10 +- frontend/src/components/BookingForm.tsx | 230 +++++++++++++++--- 7 files changed, 217 insertions(+), 73 deletions(-) diff --git a/backend/migrations/env.py b/backend/migrations/env.py index cadb4bfe..118b911e 100644 --- a/backend/migrations/env.py +++ b/backend/migrations/env.py @@ -15,17 +15,19 @@ from sqlalchemy import Connection from sqlalchemy import pool from sqlalchemy.ext.asyncio import async_engine_from_config -from backend.db import DATABASE_URL -from backend.models import Base +# Attempt to find the first .env file in current or parent directories +env_path = Path(__file__).resolve().parent +while (env_path / ".env").exists() is False and env_path != env_path.parent: + env_path = env_path.parent +if (env_path / ".env").exists(): + load_dotenv(dotenv_path=env_path / ".env") +else: + raise FileNotFoundError("No .env file found in current or parent directories.") + +from backend.db import DATABASE_URL # noqa: E402 +from backend.models import Base # noqa: E402 -# Try to load .env from local directory first, then parent directory if not found -local_env = Path(__file__).resolve().parent.parent / ".env" -parent_env = Path(__file__).resolve().parent.parent.parent / ".env" -if local_env.exists(): - load_dotenv(dotenv_path=local_env) -elif parent_env.exists(): - load_dotenv(dotenv_path=parent_env) # this is the Alembic Config object, which provides # access to the values within the .ini file in use. diff --git a/backend/migrations/versions/0002_add_booking_exclusion_constraint.py b/backend/migrations/versions/0002_add_booking_exclusion_constraint.py index 0eacb992..8a56fc3d 100644 --- a/backend/migrations/versions/0002_add_booking_exclusion_constraint.py +++ b/backend/migrations/versions/0002_add_booking_exclusion_constraint.py @@ -25,7 +25,7 @@ def upgrade(): ADD CONSTRAINT bookings_no_overlap EXCLUDE USING gist ( room_id WITH =, - tstzrange(start_time, end_time) WITH && + tstzrange(start_time, end_time, '[)') WITH && ); """ ) diff --git a/backend/migrations/versions/0006_make_bookings_tz_aware.py b/backend/migrations/versions/0006_make_bookings_tz_aware.py index 9ac01999..2b127372 100644 --- a/backend/migrations/versions/0006_make_bookings_tz_aware.py +++ b/backend/migrations/versions/0006_make_bookings_tz_aware.py @@ -53,7 +53,7 @@ def upgrade() -> None: ADD CONSTRAINT bookings_no_overlap EXCLUDE USING gist ( room_id WITH =, - tstzrange(start_time, end_time) WITH && + tstzrange(start_time, end_time, '[)') WITH && ); """ ) diff --git a/backend/src/backend/routers/bookings.py b/backend/src/backend/routers/bookings.py index 07a90de5..5106ef80 100644 --- a/backend/src/backend/routers/bookings.py +++ b/backend/src/backend/routers/bookings.py @@ -162,26 +162,10 @@ async def create_booking(booking: BookingCreate, session: DBSession) -> BookingR HTTPException: If a database error or unexpected error occurs. """ logger.debug("Received request to create a new booking") - from datetime import datetime - from datetime import timedelta - - def round_to_strictly_future_quarter(dt: datetime) -> datetime: - minute = dt.minute - add_minutes = (15 - (minute % 15)) % 15 - rounded = dt.replace(second=0, microsecond=0) + timedelta(minutes=add_minutes) - if rounded <= dt: - rounded += timedelta(minutes=15) - return rounded try: booking_data = booking.model_dump() - now = datetime.now(tz=booking_data["start_time"].tzinfo) - # Use the later of now or requested start time - effective_start = max(now, booking_data["start_time"]) - booking_data["start_time"] = round_to_strictly_future_quarter(effective_start) - # For end_time, keep the original duration - duration = booking_data["end_time"] - booking_data["start_time"] - booking_data["end_time"] = booking_data["start_time"] + duration + # No rounding: use start_time and end_time as provided db_booking = Booking(**booking_data) created_booking = await new_booking( session, db_booking, publish_room_availability_event diff --git a/backend/src/backend/services/bookings.py b/backend/src/backend/services/bookings.py index 634afe6a..98e66040 100644 --- a/backend/src/backend/services/bookings.py +++ b/backend/src/backend/services/bookings.py @@ -164,8 +164,8 @@ async def _validate_new_booking_no_overlap( ) -> None: overlap_stmt = select(Booking).where( Booking.room_id == booking.room_id, - func.tstzrange(Booking.start_time, Booking.end_time, "[]").op("&&")( - func.tstzrange(booking.start_time, booking.end_time, "[]") + func.tstzrange(Booking.start_time, Booking.end_time, "[)").op("&&")( + func.tstzrange(booking.start_time, booking.end_time, "[)") ), ) result = await session.scalars(overlap_stmt) @@ -295,8 +295,8 @@ async def _validate_no_overlap( overlap_stmt = select(Booking).where( Booking.room_id == room_id, Booking.id != booking_id, - func.tstzrange(Booking.start_time, Booking.end_time, "[]").op("&&")( - func.tstzrange(new_start, new_end, "[]") + func.tstzrange(Booking.start_time, Booking.end_time, "[)").op("&&")( + func.tstzrange(new_start, new_end, "[)") ), ) overlapping = (await session.scalars(overlap_stmt)).first() diff --git a/compose.dev.yml b/compose.dev.yml index 63307c8f..5a95dc9b 100644 --- a/compose.dev.yml +++ b/compose.dev.yml @@ -20,13 +20,9 @@ services: - "${BACKEND_PORT}:${BACKEND_PORT}" command: [ - "uvicorn", - "backend.main:app", - "--host", - "0.0.0.0", - "--port", - "${BACKEND_PORT}", - "--reload", + "sh", + "-c", + "alembic upgrade head && uvicorn backend.main:app --host 0.0.0.0 --port ${BACKEND_PORT} --reload", ] restart: always diff --git a/frontend/src/components/BookingForm.tsx b/frontend/src/components/BookingForm.tsx index cd11f101..a07f08f6 100644 --- a/frontend/src/components/BookingForm.tsx +++ b/frontend/src/components/BookingForm.tsx @@ -88,8 +88,6 @@ function getEditBookingRoomId(editBooking: any): string { return id !== undefined && id !== null ? String(id) : ""; } -// ...existing code... - const BookingForm: React.FC = ({ open, onClose, @@ -129,6 +127,26 @@ const BookingForm: React.FC = ({ // Re-validate form on any relevant change to keep errors and button state in sync // (moved below state declarations) + // Track interval between start and end + const [intervalMinutes, setIntervalMinutes] = useState(() => { + if (isEdit && editBooking?.start && editBooking?.end) { + const startDate = new Date(editBooking.start); + const endDate = new Date(editBooking.end); + return Math.max( + 1, + Math.round((endDate.getTime() - startDate.getTime()) / 60000) + ); + } + if (slotInfo?.start && slotInfo?.end) { + const startDate = new Date(slotInfo.start); + const endDate = new Date(slotInfo.end); + return Math.max( + 1, + Math.round((endDate.getTime() - startDate.getTime()) / 60000) + ); + } + return 30; + }); useEffect(() => { if (editBooking && editBooking.id) { const currentId = String(editBooking.id); @@ -216,13 +234,6 @@ const BookingForm: React.FC = ({ } }, [room_id, rooms]); - // (removed duplicate declaration) - - // Keep room_id in sync with editBooking or rooms - // Removed duplicate effect that would overwrite user changes to room_id - - // ...existing code... - // Place these hooks after all state and helper declarations // (Removed duplicate debug log) useEffect(() => { @@ -240,21 +251,116 @@ const BookingForm: React.FC = ({ const [title, setTitle] = useState(isEdit ? editBooking.title : ""); // Compute initial start/end for new bookings // For new bookings, use slotInfo.start/end if provided, otherwise next quarter hour from now - const [start, setStart] = useState(() => { - if (isEdit && editBooking?.start) return editBooking.start; - if (slotInfo?.start) return slotInfo.start; + // Helper: Find next available 30-min slot for a room, checking every quarter hour + async function getNextAvailableSlot( + roomId: string + ): Promise<{ start: Date; end: Date } | null> { const now = new Date(); - const roundedStart = roundToStrictlyFutureQuarter(now); - return roundedStart.toISOString(); - }); - const [end, setEnd] = useState(() => { - if (isEdit && editBooking?.end) return editBooking.end; - if (slotInfo?.end) return slotInfo.end; + const dateStr = now.toISOString().slice(0, 10); + try { + const res = await axios.get(`/bookings/room/${roomId}`, { + params: { date: dateStr }, + }); + const bookings = res.data; + // Find the earliest available quarter-hour after now or after the last booking ends + let earliest = now; + if (bookings.length > 0) { + // Find the latest end_time among bookings that end after now + const futureBookings = bookings.filter( + (b: { end_time: string }) => new Date(b.end_time) > now + ); + if (futureBookings.length > 0) { + const latestEnd = futureBookings.reduce( + (max: Date, b: { end_time: string }) => { + const end = new Date(b.end_time); + return end > max ? end : max; + }, + now + ); + if (latestEnd > earliest) earliest = latestEnd; + } + } + // Only round up for initial candidate + let candidate = new Date(earliest.getTime()); + candidate.setSeconds(0, 0); + if (candidate.getMinutes() % 15 !== 0) { + candidate = roundToStrictlyFutureQuarter(candidate); + } + for (let i = 0; i < 96; i++) { + // 96 quarter-hours in 24 hours + const candidateEnd = new Date(candidate.getTime() + 30 * 60000); + // Find the first overlapping booking + const conflict = bookings.find( + (b: { start_time: string; end_time: string }) => { + return ( + candidate.toISOString() < b.end_time && + candidateEnd.toISOString() > b.start_time + ); + } + ); + if (!conflict) { + return { start: candidate, end: candidateEnd }; + } + // Move candidate to the end of the overlapped booking + candidate = new Date(conflict.end_time); + // After moving, round up to the next quarter hour if needed + if (candidate.getMinutes() % 15 !== 0) { + candidate = roundToStrictlyFutureQuarter(candidate); + } + } + } catch (e) { + // fallback: just use next quarter hour + const candidate = roundToStrictlyFutureQuarter(now); + return { + start: candidate, + end: new Date(candidate.getTime() + 30 * 60000), + }; + } + return null; + } + + // Initial start time logic + const [start, setStart] = useState(""); + const [end, setEnd] = useState(""); + const userChangedStart = useRef(false); + + // On mount or room change, find next available slot for new bookings + useEffect(() => { + if (userChangedStart.current) return; + if (isEdit) { + setStart(editBooking?.start || ""); + // Always set end to 30 minutes after start for initial booking + if (editBooking?.start) { + const startDate = new Date(editBooking.start); + setEnd(new Date(startDate.getTime() + 30 * 60000).toISOString()); + } else { + setEnd(editBooking?.end || ""); + } + return; + } const now = new Date(); - const roundedStart = roundToStrictlyFutureQuarter(now); - const roundedEnd = new Date(roundedStart.getTime() + 30 * 60000); - return roundedEnd.toISOString(); - }); + if (slotInfo?.start) { + const slotStart = new Date(slotInfo.start); + if (slotStart > now) { + setStart(slotInfo.start); + // Always set end to 30 minutes after start + setEnd(new Date(slotStart.getTime() + 30 * 60000).toISOString()); + return; + } + } + if (room_id) { + getNextAvailableSlot(room_id).then((slot) => { + if (slot) { + setStart(slot.start.toISOString()); + setEnd(new Date(slot.start.getTime() + 30 * 60000).toISOString()); + } else { + const candidate = roundToStrictlyFutureQuarter(now); + setStart(candidate.toISOString()); + setEnd(new Date(candidate.getTime() + 30 * 60000).toISOString()); + } + }); + } + }, [room_id, isEdit, slotInfo]); const [invitees, setInvitees] = useState([]); const [availableInvitees, setAvailableInvitees] = useState(allInvitees); @@ -267,12 +373,6 @@ const BookingForm: React.FC = ({ // Track which rooms are unavailable due to conflicts const [conflictingRoomIds, setConflictingRoomIds] = useState([]); - // Re-validate form on any relevant change to keep errors and button state in sync - useEffect(() => { - validate(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [room_id, invitees, start, end]); - // Fetch conflicting rooms for the selected time useEffect(() => { if (!start || !end) { @@ -384,24 +484,63 @@ const BookingForm: React.FC = ({ if (invitees.length > roomCapacity) { newErrors.invitees = `The number of invitees exceeds the room's capacity (${roomCapacity}).`; } + let hasInviteeError = false; invitees.forEach((email, idx) => { if (!/^[^@\s]+@[^@\s]+\.[^@\s]+$/.test(email)) { newErrors[`invitee_${idx}`] = `Invalid email: ${email}`; + hasInviteeError = true; } }); + if (hasInviteeError) { + newErrors.invitees = "One or more invitees have invalid email addresses."; + } + + // Overlap validation for start/end time (selected room) + if (room_id && start && end && roomBookings.length > 0) { + const startTime = new Date(start).toISOString(); + const endTime = new Date(end).toISOString(); + const hasConflict = roomBookings.some((b: any) => { + // Exclude current booking if editing + if (isEdit && b.id === editBooking?.id) return false; + return startTime < b.end_time && endTime > b.start_time; + }); + if (hasConflict) { + newErrors.room_id = "Room is unavailable for the selected time."; + newErrors.start = "Start time overlaps with another booking."; + newErrors.end = "End time overlaps with another booking."; + } + } + if (start && new Date(start) < new Date()) { + newErrors.start = "Start time cannot be in the past."; + } + setErrors(newErrors); if (Object.keys(newErrors).length > 0) { logger.warn("[BookingForm] Validation failed", newErrors); - logger.warn("[BookingForm] Validation failed", newErrors); } return Object.keys(newErrors).length === 0; } - // Re-validate form on any relevant change to keep errors and button state in sync + const [roomBookings, setRoomBookings] = useState([]); + + // Fetch bookings for selected room and date + useEffect(() => { + if (!room_id || !start) { + setRoomBookings([]); + return; + } + const dateStr = new Date(start).toISOString().slice(0, 10); + axios + .get(`/bookings/room/${room_id}`, { params: { date: dateStr } }) + .then((res) => setRoomBookings(res.data)) + .catch(() => setRoomBookings([])); + }, [room_id, start]); + + // Move this effect after roomBookings is defined useEffect(() => { validate(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [room_id, invitees, start, end]); + }, [room_id, invitees, start, end, roomBookings]); const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); @@ -450,6 +589,8 @@ const BookingForm: React.FC = ({ end_time: end, title: title, }; + // DEBUG: Log payload before submission + console.log("Booking payload submitted:", payload); let res; let triedRoomFirst = false; try { @@ -713,17 +854,22 @@ const BookingForm: React.FC = ({ type="datetime-local" value={start ? formatLocalDateTimeInput(new Date(start)) : ""} onChange={(e) => { + userChangedStart.current = true; const value = e.target.value; - // value is in 'YYYY-MM-DDTHH:mm' local time if (value) { - // Convert local time to ISO string const [datePart, timePart] = value.split("T"); const [year, month, day] = datePart.split("-").map(Number); const [hour, minute] = timePart.split(":").map(Number); const localDate = new Date(year, month - 1, day, hour, minute); + // Calculate new end time using current interval setStart(localDate.toISOString()); + const newEnd = new Date( + localDate.getTime() + intervalMinutes * 60000 + ); + setEnd(newEnd.toISOString()); } else { setStart(""); + setEnd(""); } }} error={!!errors.start} @@ -741,9 +887,19 @@ const BookingForm: React.FC = ({ type="datetime-local" value={end ? formatLocalDateTimeInput(new Date(end)) : ""} onChange={(e) => { - const date = new Date(e.target.value); + const value = e.target.value; + const date = new Date(value); if (!isNaN(date.getTime())) { setEnd(date.toISOString()); + // Update interval to match new end time + if (start) { + const startDate = new Date(start); + const newInterval = Math.max( + 1, + Math.round((date.getTime() - startDate.getTime()) / 60000) + ); + setIntervalMinutes(newInterval); + } } else { setEnd(""); } @@ -845,6 +1001,12 @@ const BookingForm: React.FC = ({ }) )} + {/* Show invitee field error if any invitee error exists */} + {errors.invitees && ( + + {errors.invitees} + + )} {invitees.map( (email, idx) => errors[`invitee_${idx}`] && (