From 7536cca10666e4a9bc30f8c56ddd82f1eaf9c302 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Fri, 26 Jun 2026 18:18:41 +0200 Subject: [PATCH] fix(loans): promote the waitlist head when reservations fully subscribe the copies (#157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reservation-maintenance cron failed to convert any reservation into a pending loan when a fully-subscribed waitlist queued for a title — e.g. 1 copy + 2 active reservations promoted 0 instead of 1. Root cause: the promotion gate (ReservationManager::processBookAvailability → isDateRangeAvailable → CapacityService::hasFreeCapacity) counted EVERY active reservation overlapping the period as occupying a capacity unit, excluding only the one being promoted. With 1 copy and 2 queued reservations, promoting the head (queue position 1) still counted the trailing entry (position 2) as occupying the only copy, so occ == total and the head was never promoted. Both runs converted 0; F.21b (#157) saw 0 pending instead of 1. Fix: the promotion path now passes the promoted reservation's queue_position so the capacity gate ignores active reservations with a known queue_position strictly greater than it — lower-priority waitlist entries promoted in later runs as copies free up. Reservations AHEAD in the queue (lower position) still count, and a legacy NULL queue_position still counts (conservative — never overbook). The new parameter defaults to null, so the admin-creation gate and overbooked auditor sharing CapacityService are unchanged. Verified: PHPStan level 5 clean; cron repro now promotes exactly one head (run 1 → 1 pending, run 2 → still 1); E2E loan-reservation-complete 26/26 (F.21b passes), loan-overlap 35/35 (overbook prevention intact), loan-state-model 4/4. --- app/Controllers/ReservationManager.php | 16 +++++++++++++--- app/Services/CapacityService.php | 22 +++++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/app/Controllers/ReservationManager.php b/app/Controllers/ReservationManager.php index a274818e..a2dfb7a9 100644 --- a/app/Controllers/ReservationManager.php +++ b/app/Controllers/ReservationManager.php @@ -187,7 +187,16 @@ public function processBookAvailability($bookId) // $reservation['data_fine_richiesta'] for the loan period). $nextReservation['data_fine_richiesta'] = $endDate; - if ($this->isDateRangeAvailable($bookId, $startDate, $endDate, (int) $nextReservation['id'])) { + // #157: pass the promoted reservation's queue_position so the + // capacity gate ignores waitlist entries BEHIND it (they would + // otherwise block the head when the waitlist fully subscribes the + // copies — e.g. 1 copy + 2 queued reservations promoted 0). + // isset() is false for both an absent key and a NULL value, so it + // covers a legacy NULL queue_position without a redundant null check. + $headQueuePos = isset($nextReservation['queue_position']) + ? (int) $nextReservation['queue_position'] + : null; + if ($this->isDateRangeAvailable($bookId, $startDate, $endDate, (int) $nextReservation['id'], $headQueuePos)) { // Create the loan - check return value to handle race conditions // Note: createLoanFromReservation() handles its own transaction internally // when called standalone, but here we're already in a transaction @@ -270,7 +279,7 @@ public function processBookAvailability($bookId) * @param string|null $endDate End date (Y-m-d format) * @return bool True if at least one copy is available */ - private function isDateRangeAvailable($bookId, $startDate, $endDate, ?int $excludeReservationId = null) + private function isDateRangeAvailable($bookId, $startDate, $endDate, ?int $excludeReservationId = null, ?int $excludeReservationsAfterQueuePos = null) { if (!$startDate || !$endDate) { return false; @@ -287,7 +296,8 @@ private function isDateRangeAvailable($bookId, $startDate, $endDate, ?int $exclu (int) $bookId, (string) $startDate, (string) $endDate, - excludeReservationId: $excludeReservationId + excludeReservationId: $excludeReservationId, + excludeReservationsAfterQueuePos: $excludeReservationsAfterQueuePos ); } diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index bed0ab73..a7e7495c 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -93,11 +93,12 @@ public function occupiedCount( string $end, ?int $excludePrestitoId = null, ?int $excludeReservationId = null, - ?int $excludeUserId = null + ?int $excludeUserId = null, + ?int $excludeReservationsAfterQueuePos = null ): int { $intervals = array_merge( $this->holdingLoanIntervals($libroId, $start, $end, $excludePrestitoId, $excludeUserId), - $this->activeReservationIntervals($libroId, $start, $end, $excludeReservationId, $excludeUserId) + $this->activeReservationIntervals($libroId, $start, $end, $excludeReservationId, $excludeUserId, $excludeReservationsAfterQueuePos) ); return $this->sweepPeak($intervals); } @@ -109,13 +110,14 @@ public function hasFreeCapacity( string $end, ?int $excludePrestitoId = null, ?int $excludeReservationId = null, - ?int $excludeUserId = null + ?int $excludeUserId = null, + ?int $excludeReservationsAfterQueuePos = null ): bool { $total = $this->totalCopies($libroId); if ($total <= 0) { return false; } - $occ = $this->occupiedCount($libroId, $start, $end, $excludePrestitoId, $excludeReservationId, $excludeUserId); + $occ = $this->occupiedCount($libroId, $start, $end, $excludePrestitoId, $excludeReservationId, $excludeUserId, $excludeReservationsAfterQueuePos); return $occ < $total; } @@ -150,7 +152,7 @@ private function holdingLoanIntervals(int $libroId, string $start, string $end, * Active reservation intervals overlapping [$start,$end], clamped to the window. * @return list */ - private function activeReservationIntervals(int $libroId, string $start, string $end, ?int $excludeReservationId, ?int $excludeUserId): array + private function activeReservationIntervals(int $libroId, string $start, string $end, ?int $excludeReservationId, ?int $excludeUserId, ?int $excludeReservationsAfterQueuePos = null): array { // Canonical 3-step coalesce chain for the reservation end (no 2-step variants). $rEnd = 'COALESCE(r.data_fine_richiesta, DATE(r.data_scadenza_prenotazione), r.data_inizio_richiesta)'; @@ -173,6 +175,16 @@ private function activeReservationIntervals(int $libroId, string $start, string $types .= 'i'; $params[] = $excludeUserId; } + // Promotion gate (#157): when promoting the queue head, the waitlist + // entries BEHIND it must not occupy capacity — they are lower-priority + // and are promoted in later runs as copies free up. Exclude reservations + // with a known queue_position strictly greater than the promoted one. + // NULL queue_position rows still count (conservative — never overbook). + if ($excludeReservationsAfterQueuePos !== null) { + $sql .= ' AND NOT (r.queue_position IS NOT NULL AND r.queue_position > ?)'; + $types .= 'i'; + $params[] = $excludeReservationsAfterQueuePos; + } return $this->fetchIntervals($sql, $types, $params); }