From 3b0aeb6b997203a7ff1767cf1027363081c18814 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 15:46:58 +0100 Subject: [PATCH 1/9] Apply closure dates throughout the reservation lifecycle --- app/Controllers/CopyController.php | 20 +- app/Controllers/LoanApprovalController.php | 150 +++---- app/Controllers/PrestitiController.php | 17 +- app/Controllers/ReservationsController.php | 76 +--- app/Controllers/UserActionsController.php | 6 +- app/Services/CapacityService.php | 48 +- .../ReservationReassignmentService.php | 109 +++-- app/Services/StocktakeService.php | 27 +- app/Support/IcsGenerator.php | 31 +- app/Support/LibraryCalendar.php | 50 ++- app/Support/MaintenanceService.php | 95 ++-- app/Support/NotificationService.php | 8 +- .../src/Support/StaffLoanReader.php | 4 +- .../src/Support/UserLoanCalendar.php | 65 +-- .../plugins/ncip-server/NcipServerPlugin.php | 27 +- tests/reservation-closures-100.unit.php | 421 ++++++++++++++++++ 16 files changed, 787 insertions(+), 367 deletions(-) create mode 100644 tests/reservation-closures-100.unit.php diff --git a/app/Controllers/CopyController.php b/app/Controllers/CopyController.php index 1d068c01f..172b521b1 100644 --- a/app/Controllers/CopyController.php +++ b/app/Controllers/CopyController.php @@ -289,25 +289,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int $reservationManager = null; if (in_array($stato, ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true)) { - // One copy may host several non-overlapping future holds. - for ($guard = 0; $guard < 1000; $guard++) { - $held = $db->prepare(" - SELECT 1 FROM prestiti - WHERE copia_id = ? AND ( - (attivo = 1 AND stato IN ('prenotato','da_ritirare')) - OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') - ) - LIMIT 1 - "); - $held->bind_param('i', $copyId); - $held->execute(); - $stillHeld = (bool) $held->get_result()->fetch_row(); - $held->close(); - if (!$stillHeld) { - break; - } - $reassignmentService->reassignOnCopyLost($copyId); - } + $reassignmentService->reassignAllOnCopyLost($copyId); } elseif ($stato === 'disponibile') { $reassignmentService->reassignOnReturn($copyId); $reservationManager = new \App\Controllers\ReservationManager($db); diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 3ea1fe2f8..93e45107a 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -264,7 +264,11 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // guardia nascerebbe un 'da_ritirare' con deadline già passata (mai // ritirabile) e, con :fine nel passato, i predicati di overlap sotto // diventerebbero ciechi ai prestiti correnti. - if ($dataScadenza !== null && $dataScadenza !== '' && $dataScadenza < $today) { + $calendar = \App\Support\LibraryCalendar::forDb($db); + $capacity = new \App\Services\CapacityService($db, lockingReads: true); + $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::reservationBranch($db, $libroId, $existingCopiaId) : null; + $usableDue = $dataScadenza !== null ? $calendar->nextOpenDay($dataScadenza, $branch) : null; + if ($usableDue !== null && $usableDue !== '' && $usableDue < $today) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -360,29 +364,6 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R } } - // Determine state: 'prenotato' if future loan, 'da_ritirare' if immediate - // User must confirm pickup before loan becomes 'in_corso' - $isFutureLoan = ($dataPrestito > $today); - $newState = $isFutureLoan ? 'prenotato' : 'da_ritirare'; - - // Calculate pickup deadline for immediate loans (da_ritirare state) - $pickupDeadline = null; - if (!$isFutureLoan) { - $settingsRepo = new \App\Models\SettingsRepository($db); - $pickupDays = (int) ($settingsRepo->get('loans', 'pickup_expiry_days', '3') ?? 3); - // Base su $today (timezone applicativo) per coerenza con la - // classificazione immediato/futuro a cavallo della mezzanotte. - // Open days only (closures calendar); identical to +N days without closures. - $pickupDeadline = \App\Support\LibraryCalendar::pickupDeadline($db, $today, $pickupDays); - // Cap alla fine della finestra del prestito (L1): una deadline oltre - // data_scadenza permetterebbe di confermare il ritiro di un prestito - // già scaduto e terrebbe la copia impegnata oltre la finestra. - if ($dataScadenza !== null && $dataScadenza !== '' && $pickupDeadline > $dataScadenza) { - $pickupDeadline = $dataScadenza; - } - } - - // Step 1: Try to use pre-assigned copy first (avoids false rejection when slots are at capacity) // If loan already has a valid assigned copy, we can skip global slot counting $selectedCopy = null; @@ -416,6 +397,12 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R $existingCopyResult = $existingCopyStmt->get_result(); $selectedCopy = $existingCopyResult ? $existingCopyResult->fetch_assoc() : null; $existingCopyStmt->close(); + if ($selectedCopy !== null) { + $window = $this->approvalWindow($db, $calendar, $capacity, $libroId, $loanId, (int) $selectedCopy['id'], (string) $dataPrestito, (string) $dataScadenza, $today); + if ($window === null) { + $selectedCopy = null; + } + } } // Step 2: If no valid pre-assigned copy, check global availability and find a new copy @@ -425,8 +412,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // end, not only the requested window: a fully occupied earlier day // means this loan would depend on a preceding loan/reservation being // collected and returned on time, which is the bypass fixed by #384. - $capacity = new \App\Services\CapacityService($db); - if (!$capacity->hasFreeCapacity($libroId, $today, $dataScadenza, excludePrestitoId: $loanId)) { + if (!$capacity->hasFreeCapacity($libroId, $today, $usableDue, excludePrestitoId: $loanId)) { $db->rollback(); // Name the longest loan still possible from today when a // later commitment is what blocks the approval, so staff @@ -436,7 +422,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // return), but the loan itself starts on data_prestito: // the figures count from there, and a conflict before it // cannot be fixed by shortening, so no figures then. - $blocked = array_filter([$capacity->firstUnavailableDateInRange($libroId, $today, $dataScadenza, null, $loanId)]); + $blocked = array_filter([$capacity->firstUnavailableDateInRange($libroId, $today, $usableDue, null, $loanId)]); $cap = ($blocked !== [] && min($blocked) > $dataPrestito) ? \App\Support\LoanWindow::cap($dataPrestito, $blocked) : null; @@ -508,20 +494,26 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R ) ), '9999-12-31') DESC, c.id ASC - LIMIT 1 "); $overlapStmt->bind_param('iiiisssiss', $libroId, $libroId, $utenteId, $loanId, $dataScadenza, $today, $dataPrestito, $loanId, $dataScadenza, $dataScadenza); $overlapStmt->execute(); $overlapResult = $overlapStmt->get_result(); - $selectedCopy = $overlapResult ? $overlapResult->fetch_assoc() : null; + $candidates = $overlapResult ? $overlapResult->fetch_all(MYSQLI_ASSOC) : []; $overlapStmt->close(); + foreach ($candidates as $copy) { + $window = $this->approvalWindow($db, $calendar, $capacity, $libroId, $loanId, (int) $copy['id'], (string) $dataPrestito, (string) $dataScadenza, $today); + if ($window !== null) { + $selectedCopy = $copy; + break; + } + } } // Step 3: Step 2 is the authoritative allocator. The historical // CopyRepository fallback only repeated a looser date-overlap query; // after #384 it could reintroduce the exact preceding-return candidate // deliberately rejected above. - if (!$selectedCopy) { + if (!$selectedCopy || !isset($window)) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -536,36 +528,9 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R $lockCopyStmt->execute(); $lockCopyStmt->close(); - // The copy is known only now: a due date closed at its branch moves - // to the branch's next open day when the longer window still fits - // (otherwise the requested date stays, as before the calendar). - if ($dataScadenza !== null && $dataScadenza !== '') { - $branchDue = \App\Services\CirculationPolicy::dueDateFor((string) $dataScadenza, 0, $db, (int) $selectedCopy['id']); - if ($branchDue !== $dataScadenza - && (new \App\Services\CapacityService($db))->hasFreeCapacity($libroId, (string) $dataPrestito, $branchDue, excludePrestitoId: $loanId)) { - // The selected copy must be free on the added days too - // (same occupancy predicate as the recheck below). - $extraStart = \App\Support\LibraryCalendar::shift((string) $dataScadenza, 1); - $extraStmt = $db->prepare(" - SELECT 1 FROM prestiti - WHERE copia_id = ? AND id != ? - AND data_prestito <= ? - AND (stato = 'in_ritardo' OR (stato = 'in_corso' AND data_scadenza < ?) OR data_scadenza >= ?) - AND ( - (attivo = 1 AND stato IN ('in_corso','prenotato','da_ritirare','in_ritardo')) - OR (stato = 'pendente' AND copia_id IS NOT NULL) - ) - LIMIT 1 - "); - $extraStmt->bind_param('iisss', $selectedCopy['id'], $loanId, $branchDue, $today, $extraStart); - $extraStmt->execute(); - $extraBusy = (bool) $extraStmt->get_result()->fetch_row(); - $extraStmt->close(); - if (!$extraBusy) { - $dataScadenza = $branchDue; - } - } - } + [$dataPrestito, $dataScadenza] = $window; + $isFutureLoan = $dataPrestito > $today; + $newState = $isFutureLoan ? 'prenotato' : 'da_ritirare'; // Occupazione canonica #157: la copia è tenuta da un prestito attivo // OPPURE da un pending di conversione prenotazione che ha già una @@ -632,30 +597,18 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R throw new \RuntimeException(__('Impossibile aggiornare lo stato della copia')); } - // The pickup days count the open days of the chosen copy's branch. - if ($pickupDeadline !== null) { - $pickupDeadline = \App\Support\LibraryCalendar::pickupDeadline($db, $today, $pickupDays, null, (int) $selectedCopy['id']); - if ($dataScadenza !== null && $dataScadenza !== '' && $pickupDeadline > $dataScadenza) { - $pickupDeadline = $dataScadenza; - } - } - - // Assegna la copia al prestito con lo stato corretto e pickup_deadline se applicabile - if ($pickupDeadline !== null) { - $stmt = $db->prepare(" - UPDATE prestiti - SET stato = ?, attivo = 1, copia_id = ?, pickup_deadline = ?, data_scadenza = ?{$pickupNotificationResetSql} - WHERE id = ? AND stato = 'pendente' - "); - $stmt->bind_param('sissi', $newState, $selectedCopy['id'], $pickupDeadline, $dataScadenza, $loanId); - } else { - $stmt = $db->prepare(" - UPDATE prestiti - SET stato = ?, attivo = 1, copia_id = ?, pickup_deadline = NULL, data_scadenza = ?{$pickupNotificationResetSql} - WHERE id = ? AND stato = 'pendente' - "); - $stmt->bind_param('sisi', $newState, $selectedCopy['id'], $dataScadenza, $loanId); + // A scheduled approval never announces a pickup on a closed day. + $pickupDeadline = null; + if ($newState === 'da_ritirare') { + $pickupDays = (int) ((new \App\Models\SettingsRepository($db))->get('loans', 'pickup_expiry_days', '3') ?? 3); + $pickupDeadline = \App\Support\LibraryCalendar::pickupDeadline($db, $today, $pickupDays, $dataScadenza, (int) $selectedCopy['id']); } + $stmt = $db->prepare(" + UPDATE prestiti + SET stato = ?, attivo = 1, copia_id = ?, pickup_deadline = ?, data_prestito = ?, data_scadenza = ?{$pickupNotificationResetSql} + WHERE id = ? AND stato = 'pendente' + "); + $stmt->bind_param('sisssi', $newState, $selectedCopy['id'], $pickupDeadline, $dataPrestito, $dataScadenza, $loanId); $stmt->execute(); $stmt->close(); @@ -727,6 +680,24 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R } } + /** @return array{string,string}|null */ + private function approvalWindow(mysqli $db, \App\Support\LibraryCalendar $calendar, \App\Services\CapacityService $capacity, int $bookId, int $loanId, int $copyId, string $start, string $due, string $today): ?array + { + if (!$calendar->hasRules()) { + return [$start, $due]; + } + $branch = \App\Support\LibraryCalendar::branchOfCopy($db, $copyId); + $start = $calendar->nextOpenDay($start, $branch); + if ($start <= $today && !$calendar->isOpen($today, $branch)) { + $start = $calendar->nextOpenDay($today, $branch); + } + if (!$calendar->isOpen($start, $branch)) { + return null; + } + $usableDue = $capacity->reservationDueDateForCopy($bookId, $copyId, max($start, $today), $due, $loanId); + return $usableDue === null ? null : [$start, $usableDue]; + } + public function rejectLoan(Request $request, Response $response, mysqli $db): Response { $data = $request->getParsedBody(); @@ -1054,8 +1025,11 @@ public function confirmPickup(Request $request, Response $response, mysqli $db): return $response->withHeader('Content-Type', 'application/json')->withStatus(400); } - // Check if pickup deadline has passed - if (!empty($loan['pickup_deadline']) && $today > $loan['pickup_deadline']) { + // A closure may have been imported before the maintenance pass + // had a chance to persist the new pickup deadline. + $copiaId = (int) $loan['copia_id']; + $pickupDeadline = \App\Support\LibraryCalendar::effectivePickupDeadline($db, $loan['pickup_deadline'], $loan['data_scadenza'], $copiaId); + if ($pickupDeadline !== null && $today > $pickupDeadline) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -1076,8 +1050,6 @@ public function confirmPickup(Request $request, Response $response, mysqli $db): return $response->withHeader('Content-Type', 'application/json')->withStatus(400); } - $copiaId = $loan['copia_id'] ? (int) $loan['copia_id'] : null; - // Update loan state to 'in_corso' $updateStmt = $db->prepare(" UPDATE prestiti @@ -1253,7 +1225,7 @@ public function cancelPickup(Request $request, Response $response, mysqli $db): // Poi lock + ri-verifica del prestito con le guardie di stato. // Accept 'da_ritirare' OR 'prenotato' if data_prestito <= today $stmt = $db->prepare(" - SELECT id, libro_id, copia_id, utente_id, stato, pickup_deadline + SELECT id, libro_id, copia_id, utente_id, stato, pickup_deadline, data_scadenza FROM prestiti WHERE id = ? AND attivo = 1 AND (stato = 'da_ritirare' OR (stato = 'prenotato' AND data_prestito <= ?)) @@ -1297,7 +1269,7 @@ public function cancelPickup(Request $request, Response $response, mysqli $db): // reporting semantics (annullato = cancelled, scaduto = expired). // Legacy rows without a deadline are necessarily treated as a manual // cancellation because there is no expiry fact to assert. - $pickupDeadline = !empty($loan['pickup_deadline']) ? (string) $loan['pickup_deadline'] : null; + $pickupDeadline = \App\Support\LibraryCalendar::effectivePickupDeadline($db, $loan['pickup_deadline'], $loan['data_scadenza'], $copiaId); $terminalState = $pickupDeadline !== null && $pickupDeadline < $today ? 'scaduto' : 'annullato'; diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 8ced56956..d178176c3 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -1684,22 +1684,7 @@ public function processReturn(Request $request, Response $response, mysqli $db, // come fa CopyController::updateCopy per lo stesso evento. $reassignmentService = new \App\Services\ReservationReassignmentService($db); $reassignmentService->setExternalTransaction(true); - // A lost copy can hold MORE THAN ONE non-overlapping future reservation - // (period-based pre-booking, see the comment above). reassignOnCopyLost() - // handles a single hold, so loop until none is left pointing at the dead - // copy — each call either reassigns the hold to another copy or nulls its - // copia_id, so it always makes progress (bounded guard for safety). - for ($reassignGuard = 0; $reassignGuard < 1000; $reassignGuard++) { - $stillHeld = $db->query( - "SELECT 1 FROM prestiti WHERE copia_id = " . (int) $copia_id - . " AND ((attivo = 1 AND stato IN ('prenotato','da_ritirare'))" - . " OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione')) LIMIT 1" - ); - if (!$stillHeld || $stillHeld->num_rows === 0) { - break; - } - $reassignmentService->reassignOnCopyLost((int) $copia_id); - } + $reassignmentService->reassignAllOnCopyLost((int) $copia_id); } // Ricalcola le copie disponibili DOPO l'eventuale riassegnazione/conversione diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index 7b5d779c2..bf7c8cf45 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -45,7 +45,7 @@ public function __construct(?mysqli $db = null) // suo payload esponeva gli utente_id dei prenotanti (le rotte reali passano // da getBookAvailabilityData(), che filtra i campi sensibili). - private function calculateAvailability($currentLoans, $existingReservations, int $totalCopies, ?string $startDate = null, int $days = 730, ?int $excludeUserId = null, ?array $capacityReservationIntervals = null) + private function calculateAvailability(array $currentLoans, array $reservationIntervals, int $totalCopies, ?string $startDate, int $days, bool $hasActiveReservation, array $closedDates, string $startUntil) { // Default start = "today" in the APP timezone (DateHelper), not the PHP // process TZ (usually UTC): a bare `new DateTime()` made the mobile @@ -112,47 +112,7 @@ private function calculateAvailability($currentLoans, $existingReservations, int $loanIntervals[] = [$startDateLoan, $endDateLoan]; } - // F040: does the excluded user already hold an active reservation on this - // book? Same predicate as the date-less duplicate guard in - // createReservation (prenotazioni WHERE libro_id AND utente_id AND - // stato='attiva'). Surfaced so the picker can warn instead of showing an - // all-green calendar that the guard would reject for every date. - $hasActiveReservation = false; - - $reservationIntervals = []; - foreach ($existingReservations as $reservation) { - // Skip reservation if it belongs to the excluded user (e.g. the user making the request) - if ($excludeUserId !== null && isset($reservation['utente_id']) && (int) $reservation['utente_id'] === $excludeUserId) { - $hasActiveReservation = true; - continue; - } - if ($capacityReservationIntervals !== null) { - continue; - } - - // Occupancy start mirrors CapacityService's COALESCE fallback: a legacy - // 'attiva' row whose data_inizio_richiesta is NULL still occupies its - // copy, using the reservation deadline (data_scadenza_prenotazione) as - // the start. It must NOT be skipped — otherwise the calendars would - // free a slot the canonical capacity gate still counts. Only drop the - // row if neither a start nor a deadline is known. - $resStart = $reservation['data_inizio_richiesta'] ?? null; - if (!$resStart && !empty($reservation['data_scadenza_prenotazione'])) { - $resStart = substr((string) $reservation['data_scadenza_prenotazione'], 0, 10); - } - if (!$resStart) { - continue; - } - $resEnd = $reservation['data_fine_richiesta'] ?? null; - if (!$resEnd && !empty($reservation['data_scadenza_prenotazione'])) { - $resEnd = substr((string) $reservation['data_scadenza_prenotazione'], 0, 10); - } - if (!$resEnd || $resEnd < $resStart) { - $resEnd = $resStart; - } - $reservationIntervals[] = [$resStart, $resEnd]; - } - $reservationIntervals = $capacityReservationIntervals ?? $reservationIntervals; + $closedDays = array_fill_keys($closedDates, true); $unavailableDates = []; $daysData = []; @@ -184,10 +144,8 @@ private function calculateAvailability($currentLoans, $existingReservations, int if ($available <= 0) { $state = $loaned > 0 ? 'borrowed' : 'reserved'; $unavailableDates[] = $d; - } else { - if ($earliestAvailable === null) { - $earliestAvailable = $d; - } + } elseif ($earliestAvailable === null && $d <= $startUntil && !isset($closedDays[$d])) { + $earliestAvailable = $d; } $daysData[] = [ @@ -826,17 +784,16 @@ public function getBookAvailabilityData($bookId, ?string $startDate = null, int $loansResult = $stmt->get_result(); $currentLoans = $loansResult->fetch_all(MYSQLI_ASSOC); - // Get existing reservations - $stmt = $this->db->prepare(" - SELECT data_inizio_richiesta, data_fine_richiesta, data_scadenza_prenotazione, stato, queue_position, utente_id - FROM prenotazioni - WHERE libro_id = ? AND stato = 'attiva' - ORDER BY queue_position ASC - "); - $stmt->bind_param('i', $bookId); - $stmt->execute(); - $reservationsResult = $stmt->get_result(); - $existingReservations = $reservationsResult->fetch_all(MYSQLI_ASSOC); + // Surface the same duplicate guard used by createReservation even + // when the requesting patron's commitment is excluded from capacity. + $hasActiveReservation = false; + if ($excludeUserId !== null) { + $stmt = $this->db->prepare("SELECT 1 FROM prenotazioni WHERE libro_id = ? AND utente_id = ? AND stato = 'attiva' LIMIT 1"); + $stmt->bind_param('ii', $bookId, $excludeUserId); + $stmt->execute(); + $hasActiveReservation = $stmt->get_result()->num_rows > 0; + $stmt->close(); + } // The durations every calendar proposes with (public book page, admin // loan and reservation forms, the app): the configured default, capped @@ -863,12 +820,13 @@ public function getBookAvailabilityData($bookId, ?string $startDate = null, int ->closedDatesBetween( $from, \App\Support\LibraryCalendar::shift($from, $closedHorizon), - \App\Support\LibraryCalendar::requestBranch($this->db, (int) $bookId) + \App\Support\LibraryCalendar::requestBranch($this->db, (int) $bookId), + $closedHorizon + 1 ); $reservationIntervals = (new \App\Services\CapacityService($this->db))->reservationIntervalsInRange( (int) $bookId, $from, \App\Support\LibraryCalendar::shift($from, max(0, $days - 1)), $excludeUserId ); - return $this->calculateAvailability($currentLoans, $existingReservations, $totalCopies, $startDate, $days, $excludeUserId, $reservationIntervals) + [ + return $this->calculateAvailability($currentLoans, $reservationIntervals, $totalCopies, $startDate, $days, $hasActiveReservation, $closedDates, $startUntil) + [ 'loan_duration_days' => $policy->requestDurationDays(), 'max_loan_duration_days' => $policy->maxRequestDays, 'staff_loan_duration_days' => $policy->durationDays, diff --git a/app/Controllers/UserActionsController.php b/app/Controllers/UserActionsController.php index 58b7da599..5bf6537e7 100644 --- a/app/Controllers/UserActionsController.php +++ b/app/Controllers/UserActionsController.php @@ -195,7 +195,7 @@ public function cancelLoan(Request $request, Response $response, mysqli $db): Re // bloccante, quindi un'approvazione/annullamento concorrente può // averne cambiato lo stato (o, TOCTOU, il libro) nel frattempo. $stmt = $db->prepare(" - SELECT id, copia_id, stato, libro_id, pickup_deadline + SELECT id, copia_id, stato, libro_id, pickup_deadline, data_scadenza FROM prestiti WHERE id = ? AND utente_id = ? AND ( (attivo = 0 AND stato = 'pendente') @@ -224,7 +224,9 @@ public function cancelLoan(Request $request, Response $response, mysqli $db): Re // ciò che il cron avrebbe chiuso come 'scaduto'. Solo 'da_ritirare' // porta una deadline attiva; prenotato/pendente restano 'annullato'. $todayApp = \App\Support\DateHelper::today(); - $pickupDeadline = !empty($loan['pickup_deadline']) ? (string) $loan['pickup_deadline'] : null; + $pickupDeadline = \App\Support\LibraryCalendar::effectivePickupDeadline( + $db, $loan['pickup_deadline'], $loan['data_scadenza'], $loan['copia_id'] !== null ? (int) $loan['copia_id'] : null + ); $pickupExpired = (string) $loan['stato'] === 'da_ritirare' && $pickupDeadline !== null && $pickupDeadline < $todayApp; diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index 909d96e7b..a63d276fd 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -158,7 +158,7 @@ public function hasFreeCapacity( } /** - * Earliest day on or after $from with at least one free capacity unit. + * Earliest open day on or after $from with at least one free capacity unit. * * Uses the same canonical HOLDING/reservation intervals as every write * gate, then sweeps only their boundary events. Open-ended overdue loans @@ -180,6 +180,8 @@ public function firstAvailableDate(int $libroId, string $from): ?string if ($total <= 0) { return null; } + $calendar = \App\Support\LibraryCalendar::forDb($this->db); + $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::requestBranch($this->db, $libroId) : null; // Keep one day available for finite end+1 events. Any interval clamped // to this horizon is effectively open-ended for application purposes. @@ -188,10 +190,6 @@ public function firstAvailableDate(int $libroId, string $from): ?string $this->holdingLoanIntervals($libroId, $from, $horizon, null, null), $this->activeReservationIntervals($libroId, $from, $horizon, null, null) ); - if ($intervals === []) { - return $from; - } - /** @var array $events */ $events = []; foreach ($intervals as [$start, $end]) { @@ -206,21 +204,20 @@ public function firstAvailableDate(int $libroId, string $from): ?string $occupied = 0; $cursor = $from; foreach ($events as $date => $delta) { - // No boundary changes between cursor and date: if capacity was - // already free, cursor is the earliest possible answer. + // Search only this free segment. If it is entirely closed, the + // first open day could already belong to the next borrower. if ($date > $cursor && $occupied < $total) { - return $cursor; + $open = $calendar->firstOpenDayBetween($cursor, \App\Support\LibraryCalendar::shift($date, -1), $branch); + if ($open !== null) { + return $open; + } } $occupied += $delta; - if ($date >= $from && $occupied < $total) { - return $date; - } $cursor = $date; } - // Remaining occupancy is made exclusively of horizon-clamped, - // open-ended commitments; their physical return date is unknown. - return null; + // Remaining occupants have no known release date. + return $occupied < $total ? $calendar->firstOpenDayBetween($cursor, '9999-12-31', $branch) : null; } /** @@ -441,9 +438,28 @@ private function holdingLoanIntervals(int $libroId, string $start, string $end, } /** Whether this physical copy has a commitment in the inclusive window. */ - public function copyHasHoldingLoan(int $libroId, int $copyId, string $start, string $end): bool + public function copyHasHoldingLoan(int $libroId, int $copyId, string $start, string $end, ?int $excludeLoanId = null): bool + { + return $this->holdingLoanIntervals($libroId, $start, $end, $excludeLoanId, null, $copyId) !== []; + } + + /** Effective due day when both this copy and the title can honor the window. */ + public function reservationDueDateForCopy(int $bookId, int $copyId, string $start, string $storedDue, ?int $excludeLoanId = null): ?string { - return $this->holdingLoanIntervals($libroId, $start, $end, null, null, $copyId) !== []; + $calendar = \App\Support\LibraryCalendar::forDb($this->db); + if (!$calendar->hasRules()) { + return $storedDue; + } + $branch = \App\Support\LibraryCalendar::branchOfCopy($this->db, $copyId); + $due = $calendar->nextOpenDay($storedDue, $branch); + if ($due < $start || !$calendar->isOpen($due, $branch) + || $this->copyHasHoldingLoan($bookId, $copyId, $start, $due, $excludeLoanId)) { + return null; + } + if ($due > $storedDue && !$this->hasFreeCapacity($bookId, \App\Support\LibraryCalendar::shift($storedDue, 1), $due, excludePrestitoId: $excludeLoanId)) { + return null; + } + return $due; } /** diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index 6f0c1bde1..a1fbfa616 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -151,6 +151,8 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool // ricevere la copia rientrata prima di candidati validi — la // terrebbe impegnata solo fino alla scadenza del pickup. $today = \App\Support\DateHelper::today(); + $calendar = \App\Support\LibraryCalendar::forDb($this->db); + $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; $prefilterStmt = $this->db->prepare(" SELECT p.id FROM prestiti p @@ -163,7 +165,7 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool AND p.data_scadenza >= ? ORDER BY p.created_at ASC, p.id ASC "); - $prefilterStmt->bind_param('is', $libroId, $today); + $prefilterStmt->bind_param('is', $libroId, $candidateFloor); $prefilterStmt->execute(); $prefilterResult = $prefilterStmt->get_result(); $candidateIds = []; @@ -189,7 +191,7 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool ORDER BY created_at ASC, id ASC FOR UPDATE "); - $candidateParams = array_merge([$libroId], $candidateIds, [$today]); + $candidateParams = array_merge([$libroId], $candidateIds, [$candidateFloor]); $candidateTypes = 'i' . str_repeat('i', count($candidateIds)) . 's'; $candidateLockStmt->bind_param($candidateTypes, ...$candidateParams); $candidateLockStmt->execute(); @@ -248,6 +250,8 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool $nonLendableStates = ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento']; $reservation = null; + $dueDate = null; + $capacity = new CapacityService($this->db, lockingReads: true); foreach ($candidates as $candidate) { // Authoritative blocked-state re-check from the locked copy batch. // A row whose original copy became usable is no longer a candidate. @@ -259,7 +263,7 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool } } - if (!\App\Support\LoanMultiplicityPolicy::candidateConflictsWithOpenCommitments( + if (\App\Support\LoanMultiplicityPolicy::candidateConflictsWithOpenCommitments( (int) $candidate['id'], (int) $candidate['utente_id'], (string) $candidate['data_prestito'], @@ -267,9 +271,20 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool $targetCommitments, $today )) { - $reservation = $candidate; - break; + continue; + } + $originalBranch = $calendar->hasRules() + ? \App\Support\LibraryCalendar::reservationBranch($this->db, $libroId, $currentCopyId) : null; + $promisedEnd = $calendar->nextOpenDay((string) $candidate['data_scadenza'], $originalBranch); + if ($promisedEnd < $today) { + continue; + } + $dueDate = $capacity->reservationDueDateForCopy($libroId, $newCopiaId, (string) $candidate['data_prestito'], $promisedEnd, (int) $candidate['id']); + if ($dueDate === null) { + continue; } + $reservation = $candidate; + break; } if ($reservation === null) { $this->rollbackIfOwned($ownTransaction); @@ -279,10 +294,10 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool // Aggiorna il prestito/prenotazione con la nuova copia $stmt = $this->db->prepare(" UPDATE prestiti - SET copia_id = ? + SET copia_id = ?, data_scadenza = ? WHERE id = ? "); - $stmt->bind_param('ii', $newCopiaId, $reservation['id']); + $stmt->bind_param('isi', $newCopiaId, $dueDate, $reservation['id']); $stmt->execute(); $stmt->close(); @@ -336,7 +351,7 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): bool * Gestisce la perdita di una copia (es. segnata come persa/danneggiata). * Cerca di riassegnare la prenotazione a un'altra copia se possibile. */ - public function reassignOnCopyLost(int $copiaId): void + public function reassignOnCopyLost(int $copiaId): bool { // Trova un impegno HOLDING "futuro" su questa copia da riassegnare. Include // 'da_ritirare' (ritiro in attesa) oltre a 'prenotato' (BUG7b/D12): perdere @@ -345,6 +360,9 @@ public function reassignOnCopyLost(int $copiaId): void // un hold già scaduto ne riattiverebbe di fatto la finestra su una nuova // copia — quello va lasciato alla scadenza/pulizia, non alla riassegnazione. $today = \App\Support\DateHelper::today(); + $calendar = \App\Support\LibraryCalendar::forDb($this->db); + $originalBranch = $calendar->hasRules() ? \App\Support\LibraryCalendar::branchOfCopy($this->db, $copiaId) : null; + $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; $stmt = $this->db->prepare(" SELECT id, libro_id, utente_id, data_prestito, data_scadenza FROM prestiti @@ -353,21 +371,27 @@ public function reassignOnCopyLost(int $copiaId): void AND ( (attivo = 1 AND stato IN ('prenotato', 'da_ritirare')) OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) ORDER BY data_prestito ASC, id ASC - LIMIT 1 "); - $stmt->bind_param('is', $copiaId, $today); + $stmt->bind_param('is', $copiaId, $candidateFloor); $stmt->execute(); - $reservation = $stmt->get_result()->fetch_assoc(); + $rows = $stmt->get_result(); + $reservation = null; + while ($candidate = $rows->fetch_assoc()) { + if ($calendar->nextOpenDay((string) $candidate['data_scadenza'], $originalBranch) >= $today) { + $reservation = $candidate; + break; + } + } $stmt->close(); - if (!$reservation) { - return; + if ($reservation === null) { + return false; } $libroId = (int) $reservation['libro_id']; $reservationId = (int) $reservation['id']; $resStart = (string) $reservation['data_prestito']; - $resEnd = (string) $reservation['data_scadenza']; + $resEnd = $calendar->nextOpenDay((string) $reservation['data_scadenza'], $originalBranch); $excludedCopies = [$copiaId]; // Copie da escludere dalla ricerca // The allocator pre-filters overlaps, so retries are only needed when a // concurrent transaction claims a candidate between lookup and lock. @@ -389,8 +413,7 @@ public function reassignOnCopyLost(int $copiaId): void if (!$nextCopyId) { // Nessuna copia disponibile - $this->handleNoCopyAvailable($reservationId); - return; + return $this->handleNoCopyAvailable($reservationId, $copiaId, $resEnd); } // Riassegna @@ -411,16 +434,20 @@ public function reassignOnCopyLost(int $copiaId): void OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) FOR UPDATE "); - $lockReservation->bind_param('iiis', $reservationId, $libroId, $copiaId, $today); + $lockReservation->bind_param('iiis', $reservationId, $libroId, $copiaId, $candidateFloor); $lockReservation->execute(); $currentReservation = $lockReservation->get_result()->fetch_assoc(); $lockReservation->close(); if (!$currentReservation) { $this->rollbackIfOwned($ownTransaction); - return; + return false; } $resStart = (string) $currentReservation['data_prestito']; - $resEnd = (string) $currentReservation['data_scadenza']; + $resEnd = $calendar->nextOpenDay((string) $currentReservation['data_scadenza'], $originalBranch); + if ($resEnd < $today) { + $this->rollbackIfOwned($ownTransaction); + return false; + } $committedCopyIds = (new \App\Support\LoanMultiplicityPolicy($this->db)) ->committedCopyIds($libroId, (int) $currentReservation['utente_id'], $reservationId); @@ -469,10 +496,17 @@ public function reassignOnCopyLost(int $copiaId): void $excludedCopies[] = $nextCopyId; continue; } + $dueDate = (new CapacityService($this->db, lockingReads: true)) + ->reservationDueDateForCopy($libroId, $nextCopyId, $resStart, $resEnd, $reservationId); + if ($dueDate === null) { + $this->rollbackIfOwned($ownTransaction); + $excludedCopies[] = $nextCopyId; + continue; + } // Aggiorna prenotazione - $stmt = $this->db->prepare("UPDATE prestiti SET copia_id = ? WHERE id = ?"); - $stmt->bind_param('ii', $nextCopyId, $reservationId); + $stmt = $this->db->prepare("UPDATE prestiti SET copia_id = ?, data_scadenza = ? WHERE id = ?"); + $stmt->bind_param('isi', $nextCopyId, $dueDate, $reservationId); $stmt->execute(); $stmt->close(); @@ -485,8 +519,7 @@ public function reassignOnCopyLost(int $copiaId): void $this->commitIfOwned($ownTransaction); - // Riassegnazione completata con successo - return; + return true; } catch (\Throwable $e) { $this->rollbackIfOwned($ownTransaction); @@ -514,13 +547,24 @@ public function reassignOnCopyLost(int $copiaId): void 'reservation_id' => $reservationId, 'attempts' => $maxRetries ]); - $this->handleNoCopyAvailable($reservationId); + return $this->handleNoCopyAvailable($reservationId, $copiaId, $resEnd); + } + + /** Reassign all still-usable holds without spinning on already expired rows. */ + public function reassignAllOnCopyLost(int $copyId): void + { + for ($guard = 0; $guard < 1000; $guard++) { + if (!$this->reassignOnCopyLost($copyId)) { + return; + } + } + throw new \RuntimeException('Holds still on copy ' . $copyId . ' after reassignment'); } /** * Gestisce il caso in cui non ci sono copie disponibili per una prenotazione. */ - private function handleNoCopyAvailable(int $reservationId): void + private function handleNoCopyAvailable(int $reservationId, int $lostCopyId, string $promisedEnd): bool { // Meglio impostare copia_id a NULL per indicare "in coda senza copia" o "in attesa" // E notificare l'utente che è tornato in lista d'attesa @@ -530,7 +574,7 @@ private function handleNoCopyAvailable(int $reservationId): void $row = $lookup->get_result()->fetch_assoc(); $lookup->close(); if (!$row) { - return; + return false; } $libroId = (int) $row['libro_id']; @@ -543,18 +587,21 @@ private function handleNoCopyAvailable(int $reservationId): void $lockBook->close(); $stmt = $this->db->prepare(" - UPDATE prestiti SET copia_id = NULL - WHERE id = ? AND libro_id = ? + UPDATE prestiti + SET copia_id = NULL, data_scadenza = GREATEST(data_scadenza, ?), + stato = IF(stato = 'da_ritirare', 'prenotato', stato), + pickup_deadline = NULL + WHERE id = ? AND libro_id = ? AND copia_id = ? AND ( (attivo = 1 AND stato IN ('prenotato','da_ritirare')) OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) "); - $stmt->bind_param('ii', $reservationId, $libroId); + $stmt->bind_param('siii', $promisedEnd, $reservationId, $libroId, $lostCopyId); $stmt->execute(); $updated = $stmt->affected_rows; $stmt->close(); if ($updated < 1) { $this->rollbackIfOwned($ownTransaction); - return; + return false; } $this->commitIfOwned($ownTransaction); @@ -570,6 +617,7 @@ private function handleNoCopyAvailable(int $reservationId): void } else { $this->notifyUserCopyUnavailable($reservationId, 'lost_copy'); } + return true; } catch (\Throwable $e) { $this->rollbackIfOwned($ownTransaction); @@ -582,6 +630,7 @@ private function handleNoCopyAvailable(int $reservationId): void 'reservation_id' => $reservationId, 'error' => $e->getMessage() ]); + return false; } } diff --git a/app/Services/StocktakeService.php b/app/Services/StocktakeService.php index 101dd0990..4706992c8 100644 --- a/app/Services/StocktakeService.php +++ b/app/Services/StocktakeService.php @@ -1954,32 +1954,7 @@ private function markBookCopiesLost(int $bookId, array $copies, int $sessionId, $stmt->execute(); $stmt->close(); - // A copy may carry several future holds: move every one of them elsewhere. - // Same predicate as reassignOnCopyLost(), window still valid included: - // an expired hold is left to the expiry cleanup, and counting it here - // would spin the guard without the reassignment ever taking it. - // A hold still there after the guard rolls the whole book back: - // committing would leave a reader's hold on a lost copy. - $today = DateHelper::today(); - for ($guard = 0; ; $guard++) { - $held = $this->db->prepare( - "SELECT 1 FROM prestiti WHERE copia_id = ? AND data_scadenza >= ? AND ((attivo = 1 AND stato IN ('prenotato','da_ritirare')) OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione')) LIMIT 1" - ); - if ($held === false) { - throw new \RuntimeException($this->db->error); - } - $held->bind_param('is', $copyId, $today); - $held->execute(); - $stillHeld = $held->get_result()->fetch_row() !== null; - $held->close(); - if (!$stillHeld) { - break; - } - if ($guard >= 1000) { - throw new \RuntimeException('Holds still on copy ' . $copyId . ' after reassignment'); - } - $reassignment->reassignOnCopyLost($copyId); - } + $reassignment->reassignAllOnCopyLost($copyId); $changed[] = ['id' => $copyId] + $current; } diff --git a/app/Support/IcsGenerator.php b/app/Support/IcsGenerator.php index 468b8b7a1..a1ae2a051 100644 --- a/app/Support/IcsGenerator.php +++ b/app/Support/IcsGenerator.php @@ -195,12 +195,14 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD { $events = []; $today = DateHelper::today(); + $calendar = LibraryCalendar::forDb($this->db); + $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; $visible = $catalogueOnly ? ' AND ' . \App\Support\BookVisibility::catalogue($this->db, 'l') : ''; // Fetch active/scheduled loans - $loanSql = "SELECT p.id, p.stato, p.copia_id, p.data_prestito, p.data_scadenza, p.pickup_deadline, + $loanSql = "SELECT p.id, p.libro_id, p.stato, p.copia_id, p.data_prestito, p.data_scadenza, p.pickup_deadline, l.titolo, CONCAT(u.nome, ' ', u.cognome) AS utente_nome, u.email, p.updated_at FROM prestiti p @@ -221,7 +223,7 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD if ($stmt === false) { return $events; // Return empty array on prepare failure } - if (!$stmt->bind_param('s', $today) || !$stmt->execute()) { + if (!$stmt->bind_param('s', $candidateFloor) || !$stmt->execute()) { $stmt->close(); return $events; } @@ -230,18 +232,17 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD LibraryCalendar::primeCopyBranches($this->db, array_map(static fn (array $r): int => (int) ($r['copia_id'] ?? 0), $loans)); foreach ($loans as $row) { $dates = LibraryCalendar::loanDates($this->db, $row, $today); - // A booked loan ends on its usable day, which a closure can put after the stored one. - if ($row['stato'] === 'prenotato' && (string) ($dates['effective_due_at'] ?? $row['data_scadenza']) < $today) { - continue; - } if (in_array($row['stato'], ['in_corso', 'in_ritardo'], true)) { $row['stato'] = $dates['is_overdue'] ? 'in_ritardo' : 'in_corso'; } - // For da_ritirare state, use pickup_deadline as end date (shows pickup window) - // For other states, use data_scadenza (shows full loan period) - $endDate = ($row['stato'] === 'da_ritirare' && !empty($row['pickup_deadline'])) - ? $row['pickup_deadline'] - : ($dates['effective_due_at'] ?? $row['data_scadenza']); + $copyId = $row['copia_id'] !== null ? (int) $row['copia_id'] : null; + $endDate = $row['stato'] === 'da_ritirare' + ? LibraryCalendar::effectivePickupDeadline($this->db, $row['pickup_deadline'], $row['data_scadenza'], $copyId) + : null; + $endDate ??= $dates['effective_due_at'] ?? $row['data_scadenza']; + if (in_array($row['stato'], ['prenotato', 'da_ritirare'], true) && $endDate < $today) { + continue; + } if ($row['stato'] === 'in_ritardo') { // The copy remains physically occupied through today; ending the // event on the missed due date falsely shows it as available. @@ -262,7 +263,7 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD $stmt->close(); // Fetch active reservations - $resSql = "SELECT r.id, r.stato, r.data_prenotazione, r.data_scadenza_prenotazione, + $resSql = "SELECT r.id, r.libro_id, r.stato, r.data_prenotazione, r.data_scadenza_prenotazione, r.data_inizio_richiesta, r.data_fine_richiesta, l.titolo, CONCAT(u.nome, ' ', u.cognome) AS utente_nome, u.email, r.updated_at @@ -276,7 +277,7 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD if ($stmt === false) { return $events; } - if (!$stmt->bind_param('s', $today) || !$stmt->execute()) { + if (!$stmt->bind_param('s', $candidateFloor) || !$stmt->execute()) { $stmt->close(); return $events; } @@ -288,6 +289,10 @@ private function fetchEvents(bool $catalogueOnly = false, bool $includePersonalD $endDate = $row['data_fine_richiesta'] ?? ($row['data_scadenza_prenotazione'] ? substr((string) $row['data_scadenza_prenotazione'], 0, 10) : null) ?? $startDate; + [$startDate, $endDate] = LibraryCalendar::reservationCalendarDates($this->db, (int) $row['libro_id'], $startDate, $endDate); + if ($endDate < $today) { + continue; + } $events[] = [ 'uid' => 'reservation-' . $row['id'] . '@pinakes', 'title' => '📅 ' . __('Prenotazione') . ': ' . $row['titolo'], diff --git a/app/Support/LibraryCalendar.php b/app/Support/LibraryCalendar.php index ecb05119f..ab261ea63 100644 --- a/app/Support/LibraryCalendar.php +++ b/app/Support/LibraryCalendar.php @@ -252,7 +252,16 @@ public static function loanDates(\mysqli $db, array $loan, ?string $today = null // A scheduled loan whose move was refused also ends on the next open day; // it cannot be late or due yet, so only the date follows the calendar. $scheduled = (int) ($loan['attivo'] ?? 1) === 1 && in_array($loan['stato'] ?? '', ['prenotato', 'da_ritirare'], true); - $effectiveDue = ($holding || $scheduled) && $due !== null ? self::dueDate($db, $due, $copyId) : $due; + $effectiveDue = $due; + if (($holding || $scheduled) && $due !== null) { + $calendar = self::forDb($db); + if ($calendar->hasRules()) { + $branch = $scheduled && $copyId === null && isset($loan['libro_id']) + ? self::requestBranch($db, (int) $loan['libro_id']) + : self::branchOfCopy($db, $copyId); + $effectiveDue = $calendar->nextOpenDay($due, $branch); + } + } return [ 'effective_due_at' => $effectiveDue, 'is_overdue' => $holding && $due !== null && self::isPastDue($db, $due, $copyId, $today), @@ -732,6 +741,24 @@ public static function requestDueDate(\mysqli $db, string $startYmd, int $days, return $calendar->nextOpenDay($due, self::requestBranch($db, $bookId)); } + /** An unassigned hold follows the book's request branch until a copy is pinned. */ + public static function reservationBranch(\mysqli $db, int $bookId, ?int $copyId): ?int + { + return $copyId === null ? self::requestBranch($db, $bookId) : self::branchOfCopy($db, $copyId); + } + + /** @return array{string,string} Effective start and inclusive end for an active queue event. */ + public static function reservationCalendarDates(\mysqli $db, int $bookId, string $start, string $end): array + { + $calendar = self::forDb($db); + if (!$calendar->hasRules()) { + return [$start, $end]; + } + $branch = self::requestBranch($db, $bookId); + $end = $calendar->nextOpenDay($end, $branch); + return [min($calendar->nextOpenDay($start, $branch), $end), $end]; + } + /** A closed deadline gives the reader the whole first open day to use it. */ public static function reservationExpired(\mysqli $db, string $deadline, int $bookId, string $now): bool { @@ -927,6 +954,18 @@ public function pickupDeadlineOnReopening(string $deadline, ?string $dueDate, ?i return DateHelper::isISODateFormat($due) ? max($deadline, min($next, $due)) : $next; } + /** The collection deadline shown to readers, even before reconciliation. */ + public static function effectivePickupDeadline(\mysqli $db, ?string $deadline, ?string $due, ?int $copyId): ?string + { + if ($deadline === null || $deadline === '') { + return null; + } + $calendar = self::forDb($db); + return $calendar->hasRules() + ? $calendar->pickupDeadlineOnReopening(substr($deadline, 0, 10), $due, self::branchOfCopy($db, $copyId)) + : substr($deadline, 0, 10); + } + // ── Queries ───────────────────────────────────────────────────────────── /** True when weekly closures, closure rows or selected holidays apply. */ @@ -1074,20 +1113,19 @@ public function addOpenDays(string $from, int $n, ?int $sedeId = null): string /** * Closed days in [$from, $to], both included, for the request calendars. - * Listed day by day, so the list stops MAX_SCAN_DAYS days after $from: - * the callers ask for a month or a 400-day horizon, never a span that - * long, and a ten-year closure is not expanded into ten years of dates. + * The default protects callers from expanding arbitrary long intervals. + * A request picker with an already bounded horizon can opt into all its days. * * @return list */ - public function closedDatesBetween(string $from, string $to, ?int $sedeId = null): array + public function closedDatesBetween(string $from, string $to, ?int $sedeId = null, int $maxDays = self::MAX_SCAN_DAYS): array { if (!$this->hasRules() || !DateHelper::isISODateFormat($from) || !DateHelper::isISODateFormat($to) || $to < $from) { return []; } $closed = []; $day = $from; - for ($i = 0; $i < self::MAX_SCAN_DAYS && $day <= $to; $i++) { + for ($i = 0; $i < $maxDays && $day <= $to; $i++) { if (!$this->isOpen($day, $sedeId)) { $closed[] = $day; } diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index eb8ddd44b..cb0bfee66 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -864,6 +864,13 @@ public function activateScheduledLoans(): int // dipende dalla session timezone del client DB, che differiva tra cron // (UTC forzato) e web (nessuna impostazione). $today = DateHelper::today(); + $calendar = LibraryCalendar::forDb($this->db); + if (!$calendar->isOpen($today)) { + return 0; + } + // A stored end before today can still reach reopening. The scan + // accepts those candidates only when the locked calendar check agrees. + $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; // Find all scheduled loans that should be activated. data_scadenza >= today // guard (BUG8/D13): never promote a reservation whose whole window is already @@ -880,7 +887,7 @@ public function activateScheduledLoans(): int throw new \RuntimeException('Failed to prepare scheduled loans query'); } - $stmt->bind_param('ss', $today, $today); + $stmt->bind_param('ss', $today, $candidateFloor); $stmt->execute(); $result = $stmt->get_result(); $scheduledLoans = $result ? $result->fetch_all(MYSQLI_ASSOC) : []; @@ -923,7 +930,7 @@ public function activateScheduledLoans(): int AND data_prestito <= ? AND data_scadenza >= ? FOR UPDATE "); - $lockLoan->bind_param('iss', $loanId, $today, $today); + $lockLoan->bind_param('iss', $loanId, $today, $candidateFloor); $lockLoan->execute(); $lockedLoan = $lockLoan->get_result()->fetch_assoc(); $lockLoan->close(); @@ -932,6 +939,13 @@ public function activateScheduledLoans(): int continue; } $loan = $lockedLoan; + $oldDue = (string) $loan['data_scadenza']; + $pinnedCopy = $loan['copia_id'] !== null ? (int) $loan['copia_id'] : null; + $branch = $calendar->hasRules() ? LibraryCalendar::reservationBranch($this->db, $bookId, $pinnedCopy) : null; + if (!$calendar->isOpen($today, $branch) || $calendar->nextOpenDay($oldDue, $branch) < $today) { + $this->db->rollback(); + continue; + } // A scheduled commitment can become collectible days or weeks // after approval. Recheck mutable account/card eligibility at @@ -1077,7 +1091,6 @@ public function activateScheduledLoans(): int AND p.data_prestito <= ? AND p.data_scadenza >= ?) ) ) ORDER BY c.id - LIMIT 1 FOR UPDATE "); $freeCopy->bind_param( @@ -1091,9 +1104,15 @@ public function activateScheduledLoans(): int $loan['data_prestito'] ); $freeCopy->execute(); - $freeCopyRow = $freeCopy->get_result()->fetch_assoc(); + $freeCopyRows = $freeCopy->get_result()->fetch_all(MYSQLI_ASSOC); $freeCopy->close(); - $copiaId = (int) ($freeCopyRow['id'] ?? 0); + foreach ($freeCopyRows as $freeCopyRow) { + $candidateId = (int) $freeCopyRow['id']; + if ($this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $oldDue, $today) !== null) { + $copiaId = $candidateId; + break; + } + } if ($copiaId <= 0) { $this->db->rollback(); SecureLogger::info(__('Attivazione prestito rinviata: nessuna copia fisica assegnabile'), [ @@ -1112,19 +1131,10 @@ public function activateScheduledLoans(): int // on a day that branch is closed moves to its next open day now, // before the pickup deadline is capped at it and the reader is // told both (the pickup-ready e-mail below). - $dueDate = (string) $loan['data_scadenza']; - if ($dueDate !== '') { - $oldDue = substr($dueDate, 0, 10); - $dueDate = LibraryCalendar::dueDate($this->db, $oldDue, $copiaId); - // Only when the added days are free, as extendDueDates() - // decides: no other commitment of this copy reaches them - // (the overlap trigger would refuse the whole activation, - // on every pass) and the title has room for them. Otherwise - // the loan is activated with its date, which the late-day - // count already treats as the first open day after it. - if ($dueDate > $oldDue && !$this->addedDaysFree((int) $loan['id'], $bookId, $copiaId, $oldDue, $dueDate)) { - $dueDate = $oldDue; - } + $dueDate = $this->activationDueDate($calendar, $loanId, $bookId, $copiaId, (string) $loan['data_prestito'], $oldDue, $today); + if ($dueDate === null) { + $this->db->rollback(); + continue; } // Open days only (closures calendar); identical to +N days without closures. $pickupDeadline = LibraryCalendar::pickupDeadline($this->db, $today, $pickupDays, null, $copiaId); @@ -1142,8 +1152,7 @@ public function activateScheduledLoans(): int data_scadenza = COALESCE(?, data_scadenza){$pickupColumnReset} WHERE id = ? AND stato = 'prenotato' AND attivo = 1 AND data_scadenza >= ? "); - $newDue = $dueDate !== '' ? $dueDate : null; - $updateStmt->bind_param('sisis', $pickupDeadline, $copiaId, $newDue, $loan['id'], $today); + $updateStmt->bind_param('sisis', $pickupDeadline, $copiaId, $dueDate, $loan['id'], $candidateFloor); $updateStmt->execute(); $affectedRows = $updateStmt->affected_rows; $updateStmt->close(); @@ -1204,31 +1213,18 @@ public function activateScheduledLoans(): int return $activatedCount; } - /** - * Whether a loan's due date can grow from $oldDue to $newDue at activation: - * no other active commitment of $copiaId overlaps the added days (the - * per-copy overlap trigger would refuse the UPDATE), and the title has a - * free copy for them besides this loan (copyless bookings, CapacityService). - */ - private function addedDaysFree(int $loanId, int $bookId, int $copiaId, string $oldDue, string $newDue): bool + /** A closed or already promised copy cannot be announced as ready. */ + private function activationDueDate(LibraryCalendar $calendar, int $loanId, int $bookId, int $copyId, string $start, string $storedDue, string $today): ?string { - $from = LibraryCalendar::shift($oldDue, 1); - $stmt = $this->db->prepare(" - SELECT 1 FROM prestiti - WHERE copia_id = ? AND id <> ? - AND ((attivo = 1 AND stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) - OR (attivo = 0 AND stato = 'pendente')) - AND data_prestito <= ? AND data_scadenza >= ? - LIMIT 1 - "); - $stmt->bind_param('iiss', $copiaId, $loanId, $newDue, $from); - $stmt->execute(); - $taken = $stmt->get_result()->num_rows > 0; - $stmt->close(); - if ($taken) { - return false; + if (!$calendar->hasRules()) { + return $storedDue; + } + $branch = LibraryCalendar::branchOfCopy($this->db, $copyId); + if (!$calendar->isOpen($today, $branch)) { + return null; } - return (new \App\Services\CapacityService($this->db))->hasFreeCapacity($bookId, $from, $newDue, excludePrestitoId: $loanId); + $capacity = new \App\Services\CapacityService($this->db, lockingReads: true); + return $capacity->reservationDueDateForCopy($bookId, $copyId, max($start, $today), $storedDue, $loanId); } /** @@ -1361,7 +1357,8 @@ public function checkExpiredReservations(): int { $today = DateHelper::today(); // Closures calendar: nothing expires on a day the library is closed. - if (!LibraryCalendar::forDb($this->db)->isOpen($today)) { + $calendar = LibraryCalendar::forDb($this->db); + if (!$calendar->isOpen($today)) { return 0; } @@ -1423,7 +1420,7 @@ public function checkExpiredReservations(): int } $lockLoan = $this->db->prepare(" - SELECT id, libro_id, copia_id, utente_id + SELECT id, libro_id, copia_id, utente_id, data_scadenza FROM prestiti WHERE id = ? AND ( @@ -1442,9 +1439,11 @@ public function checkExpiredReservations(): int continue; } $copiaId = $lockedReservation['copia_id'] ? (int) $lockedReservation['copia_id'] : null; - // A closure of the assigned copy's branch counts too: the - // reader could not come in there today either. - if ($copiaId !== null && !LibraryCalendar::forDb($this->db)->isOpen($today, LibraryCalendar::branchOfCopy($this->db, $copiaId))) { + // Reopening is still usable even if the stored date could + // not be moved, for both pinned and unassigned commitments. + $branch = $calendar->hasRules() ? LibraryCalendar::reservationBranch($this->db, $libroId, $copiaId) : null; + if (!$calendar->isOpen($today, $branch) + || $calendar->nextOpenDay((string) $lockedReservation['data_scadenza'], $branch) >= $today) { $this->db->rollback(); continue; } diff --git a/app/Support/NotificationService.php b/app/Support/NotificationService.php index 23fda00a0..fb3e312f2 100644 --- a/app/Support/NotificationService.php +++ b/app/Support/NotificationService.php @@ -2330,18 +2330,20 @@ public function sendPickupReadyNotification(int $loanId): bool { $days = $endDate->diff($startDate)->days; $recipientLocale = $this->resolveRecipientLocale((string) $loan['utente_email']); + $copyId = $loan['copia_id'] !== null ? (int) $loan['copia_id'] : null; + $pickupDeadline = LibraryCalendar::effectivePickupDeadline($this->db, $loan['pickup_deadline'], $loan['data_scadenza'], $copyId); $variables = [ 'utente_nome' => $loan['utente_nome'], 'libro_titolo' => $loan['libro_titolo'], 'data_inizio' => $this->formatEmailDate($loan['data_prestito'], false, $recipientLocale), 'data_fine' => $this->formatEmailDate($this->usableDueDate($loan), false, $recipientLocale), 'giorni_prestito' => $days, - 'giorni_ritiro' => $this->effectivePickupDays((string) $loan['data_prestito'], $loan['pickup_deadline'] ? (string) $loan['pickup_deadline'] : null, !empty($loan['copia_id']) ? (int) $loan['copia_id'] : null), - 'scadenza_ritiro' => $loan['pickup_deadline'] ? $this->formatEmailDate($loan['pickup_deadline'], false, $recipientLocale) : '', + 'giorni_ritiro' => $this->effectivePickupDays((string) $loan['data_prestito'], $pickupDeadline, $copyId), + 'scadenza_ritiro' => $pickupDeadline !== null ? $this->formatEmailDate($pickupDeadline, false, $recipientLocale) : '', // #304: alias under the DB column name so a customised template using // {{pickup_deadline}} (the natural name a user copies from the schema) // resolves as well as the canonical {{scadenza_ritiro}}. - 'pickup_deadline' => $loan['pickup_deadline'] ? $this->formatEmailDate($loan['pickup_deadline'], false, $recipientLocale) : '', + 'pickup_deadline' => $pickupDeadline !== null ? $this->formatEmailDate($pickupDeadline, false, $recipientLocale) : '', 'pickup_instructions' => $this->translateInLocale('Recati in biblioteca durante gli orari di apertura per ritirare il libro.', $recipientLocale), 'sezione_calendario' => $this->buildCalendarSection((int)$loan['id'], (string)$loan['libro_titolo'], (string)$loan['data_prestito'], $this->usableDueDate($loan), $recipientLocale) ]; diff --git a/storage/plugins/mobile-api/src/Support/StaffLoanReader.php b/storage/plugins/mobile-api/src/Support/StaffLoanReader.php index 8d9a5506f..b855fc349 100644 --- a/storage/plugins/mobile-api/src/Support/StaffLoanReader.php +++ b/storage/plugins/mobile-api/src/Support/StaffLoanReader.php @@ -568,7 +568,9 @@ public function mapLoan(array $r): array 'due_at' => $due, 'effective_due_at' => $dates['effective_due_at'], 'is_overdue' => $isOverdue, - 'pickup_deadline' => self::date($r['pickup_deadline'] ?? null), + 'pickup_deadline' => $status === 'da_ritirare' + ? \App\Support\LibraryCalendar::effectivePickupDeadline($this->db, self::date($r['pickup_deadline'] ?? null), $due, $copyId) + : self::date($r['pickup_deadline'] ?? null), 'returned_at' => self::date($r['data_restituzione'] ?? null), 'returned_late' => (int) ($r['restituito_in_ritardo'] ?? 0) === 1, 'days_overdue' => $daysOverdue, diff --git a/storage/plugins/mobile-api/src/Support/UserLoanCalendar.php b/storage/plugins/mobile-api/src/Support/UserLoanCalendar.php index 6ad980376..ca0926e3c 100644 --- a/storage/plugins/mobile-api/src/Support/UserLoanCalendar.php +++ b/storage/plugins/mobile-api/src/Support/UserLoanCalendar.php @@ -73,7 +73,7 @@ private function loans(int $userId, string $today): array { $in = "'" . implode("','", self::STATES) . "'"; $stmt = $this->db->prepare( - "SELECT p.id, p.stato, p.copia_id, p.data_prestito, p.data_scadenza, p.pickup_deadline, + "SELECT p.id, p.libro_id, p.stato, p.copia_id, p.data_prestito, p.data_scadenza, p.pickup_deadline, DATE_FORMAT(p.updated_at, '%Y-%m-%d %H:%i:%s') AS updated_at, UNIX_TIMESTAMP(p.updated_at) AS seq, l.titolo FROM prestiti p @@ -89,7 +89,8 @@ private function loans(int $userId, string $today): array if ($stmt === false) { throw new \RuntimeException('loan calendar prepare failed: ' . $this->db->error); } - $stmt->bind_param('is', $userId, $today); + $candidateFloor = \App\Support\LibraryCalendar::forDb($this->db)->hasRules() ? '1000-01-01' : $today; + $stmt->bind_param('is', $userId, $candidateFloor); $stmt->execute(); $result = $stmt->get_result(); $rows = []; @@ -100,9 +101,20 @@ private function loans(int $userId, string $today): array \App\Support\LibraryCalendar::primeCopyBranches($this->db, array_map(static fn (array $r): int => (int) ($r['copia_id'] ?? 0), $rows)); - // A booked loan ends on its usable day, which a closure can put after the stored one. - return array_values(array_filter($rows, fn (array $r): bool => $r['stato'] !== 'prenotato' - || \App\Support\LibraryCalendar::dueDate($this->db, substr((string) $r['data_scadenza'], 0, 10), $r['copia_id'] !== null ? (int) $r['copia_id'] : null) >= $today)); + $active = []; + foreach ($rows as $row) { + $dates = \App\Support\LibraryCalendar::loanDates($this->db, $row, $today); + $copyId = $row['copia_id'] !== null ? (int) $row['copia_id'] : null; + $end = $row['stato'] === 'da_ritirare' + ? \App\Support\LibraryCalendar::effectivePickupDeadline($this->db, $row['pickup_deadline'], $row['data_scadenza'], $copyId) + : null; + $row['calendar_end'] = $end ?? $dates['effective_due_at'] ?? $row['data_scadenza']; + $row['calendar_overdue'] = $dates['is_overdue']; + if (!in_array($row['stato'], ['prenotato', 'da_ritirare'], true) || $row['calendar_end'] >= $today) { + $active[] = $row; + } + } + return $active; } /** @@ -111,7 +123,7 @@ private function loans(int $userId, string $today): array private function reservations(int $userId, string $today): array { $stmt = $this->db->prepare( - "SELECT r.id, r.data_prenotazione, r.data_scadenza_prenotazione, r.data_inizio_richiesta, + "SELECT r.id, r.libro_id, r.data_prenotazione, r.data_scadenza_prenotazione, r.data_inizio_richiesta, r.data_fine_richiesta, DATE_FORMAT(r.updated_at, '%Y-%m-%d %H:%i:%s') AS updated_at, UNIX_TIMESTAMP(r.updated_at) AS seq, l.titolo FROM prenotazioni r @@ -123,12 +135,22 @@ private function reservations(int $userId, string $today): array if ($stmt === false) { throw new \RuntimeException('reservation calendar prepare failed: ' . $this->db->error); } - $stmt->bind_param('is', $userId, $today); + $candidateFloor = \App\Support\LibraryCalendar::forDb($this->db)->hasRules() ? '1000-01-01' : $today; + $stmt->bind_param('is', $userId, $candidateFloor); $stmt->execute(); $result = $stmt->get_result(); $rows = []; while ($result !== false && ($r = $result->fetch_assoc()) !== null) { - $rows[] = $r; + $start = $r['data_inizio_richiesta'] + ?? (!empty($r['data_prenotazione']) ? substr((string) $r['data_prenotazione'], 0, 10) : null) + ?? substr((string) $r['data_scadenza_prenotazione'], 0, 10); + $end = $r['data_fine_richiesta'] + ?? (!empty($r['data_scadenza_prenotazione']) ? substr((string) $r['data_scadenza_prenotazione'], 0, 10) : null) + ?? $start; + [$r['calendar_start'], $r['calendar_end']] = \App\Support\LibraryCalendar::reservationCalendarDates($this->db, (int) $r['libro_id'], $start, $end); + if ($r['calendar_end'] >= $today) { + $rows[] = $r; + } } $stmt->close(); @@ -141,13 +163,10 @@ private function reservations(int $userId, string $today): array private function loanEvent(array $loan, string $today, \DateTimeZone $tz, bool $withAlarm): string { $state = (string) $loan['stato']; - $dates = \App\Support\LibraryCalendar::loanDates($this->db, $loan, $today); if (in_array($state, ['in_corso', 'in_ritardo'], true)) { - $state = $dates['is_overdue'] ? 'in_ritardo' : 'in_corso'; + $state = $loan['calendar_overdue'] ? 'in_ritardo' : 'in_corso'; } - $end = ($state === 'da_ritirare' && !empty($loan['pickup_deadline'])) - ? (string) $loan['pickup_deadline'] - : (string) ($dates['effective_due_at'] ?? $loan['data_scadenza']); + $end = (string) $loan['calendar_end']; if ($state === 'in_ritardo') { // Still physically out today: never end the event on the missed date. $end = max($end, $today); @@ -157,9 +176,10 @@ private function loanEvent(array $loan, string $today, \DateTimeZone $tz, bool $ // with it, one step per day past the stored date, so a client that // replaces an event by SEQUENCE takes the new end. $revisionShift = 0; - $storedDue = substr((string) ($loan['data_scadenza'] ?? ''), 0, 10); - if (in_array($state, ['in_corso', 'in_ritardo', 'prenotato'], true) && $storedDue !== '' && $end > $storedDue) { - $revisionShift = (int) (new \DateTimeImmutable($storedDue))->diff(new \DateTimeImmutable($end))->days; + $storedEnd = substr((string) ($state === 'da_ritirare' && $loan['pickup_deadline'] !== null + ? $loan['pickup_deadline'] : $loan['data_scadenza']), 0, 10); + if ($storedEnd !== '' && $end > $storedEnd) { + $revisionShift = (int) (new \DateTimeImmutable($storedEnd))->diff(new \DateTimeImmutable($end))->days; } $updatedAt = (string) ($loan['updated_at'] ?? ''); // Removing a closure lowers the shift: the calendar's revision rose by @@ -209,22 +229,17 @@ private function loanEvent(array $loan, string $today, \DateTimeZone $tz, bool $ */ private function reservationEvent(array $r, \DateTimeZone $tz): string { - $start = $r['data_inizio_richiesta'] - ?? (!empty($r['data_prenotazione']) ? substr((string) $r['data_prenotazione'], 0, 10) : null) - ?? substr((string) $r['data_scadenza_prenotazione'], 0, 10); - $end = $r['data_fine_richiesta'] - ?? (!empty($r['data_scadenza_prenotazione']) ? substr((string) $r['data_scadenza_prenotazione'], 0, 10) : null) - ?? $start; + $this->calendarRevision ??= \App\Support\LibraryCalendar::revision($this->db); $title = (string) $r['titolo']; return $this->event( 'reservation-' . (int) $r['id'] . '@pinakes', - (string) $start, - (string) $end, + (string) $r['calendar_start'], + (string) $r['calendar_end'], '📅 ' . __('Prenotazione') . ': ' . $title, __('Libro') . ': ' . $title . "\n" . __('Tipo') . ': ' . __('Prenotazione'), (string) ($r['updated_at'] ?? ''), - (int) ($r['seq'] ?? 0), + (int) ($r['seq'] ?? 0) + $this->calendarRevision, $tz, '' ); diff --git a/storage/plugins/ncip-server/NcipServerPlugin.php b/storage/plugins/ncip-server/NcipServerPlugin.php index e5b22c945..98df1df69 100644 --- a/storage/plugins/ncip-server/NcipServerPlugin.php +++ b/storage/plugins/ncip-server/NcipServerPlugin.php @@ -1166,16 +1166,11 @@ private function handleRequestItem( } $requestId = (string) ($message?->RequestId->RequestIdentifierValue ?? ''); - $today = \App\Support\DateHelper::today(); - // A request is capped like the website and app ones: the loan length, - // but never past the longest request window of the patron type. - $loanDays = (new \App\Services\CirculationPolicy($this->db))->forLoan($userId, $itemId)->requestDurationDays(); - $dueDate = \App\Services\CirculationPolicy::dueDateFor($today, $loanDays, $this->db); $failureReason = 'db_error'; $requestOutcome = $this->createRequestItemNcip( $itemId, $userId, - $dueDate, + null, $requestId !== '' ? $requestId : null, $failureReason ); @@ -1196,7 +1191,7 @@ private function handleRequestItem( ); } - return $this->xmlResponse($response, $this->buildRequestItemResponse($itemId, $userId, $dueDate)); + return $this->xmlResponse($response, $this->buildRequestItemResponse($itemId, $userId, $requestOutcome['due_date'])); } /** @@ -2109,19 +2104,19 @@ private function createLoanAtomic(int $bookId, int $userId, string &$dueDate, in /** * @param-out string $failureReason Stable rejection reason for RequestItem. - * @return array{loan_id:?int,reservation_id:?int}|null + * @return array{loan_id:?int,reservation_id:?int,due_date:string}|null */ private function createRequestItemNcip( int $bookId, int $userId, - string $dueDate, + ?string $dueDate, ?string $requestId, ?string &$failureReason = null ): ?array { $failureReason = 'db_error'; $today = \App\Support\DateHelper::today(); - if (!\App\Support\DateHelper::isISODateFormat($dueDate) || $dueDate < $today) { + if ($dueDate !== null && (!\App\Support\DateHelper::isISODateFormat($dueDate) || $dueDate < $today)) { $failureReason = 'invalid_due'; return null; } @@ -2191,10 +2186,14 @@ private function createRequestItemNcip( return null; } + $startDate = \App\Support\LibraryCalendar::requestStartDay($this->db, $today, $bookId); + $dueDate = $dueDate === null + ? \App\Support\LibraryCalendar::requestDueDate($this->db, $startDate, $requestPolicy->requestDurationDays(), $bookId) + : \App\Support\LibraryCalendar::requestDueDate($this->db, max($startDate, $dueDate), 0, $bookId); $routing = (new \App\Services\LoanRequestGate($this->db))->route( $bookId, $userId, - $today, + $startDate, $dueDate, inCallerTransaction: true ); @@ -2214,7 +2213,7 @@ private function createRequestItemNcip( throw new \RuntimeException('Failed to audit NCIP reservation request.'); } $this->db->commit(); - return ['loan_id' => null, 'reservation_id' => $reservationId]; + return ['loan_id' => null, 'reservation_id' => $reservationId, 'due_date' => $dueDate]; } // A waitlist reservation does not consume an active-loan slot. @@ -2241,7 +2240,7 @@ private function createRequestItemNcip( VALUES (?, ?, ?, ?, 'pendente', 'ncip', ?, 0, NOW(), NOW())" ); if ($stmt === false) { $this->db->rollback(); return null; } - $stmt->bind_param('iisss', $bookId, $userId, $today, $dueDate, $requestId); + $stmt->bind_param('iisss', $bookId, $userId, $startDate, $dueDate, $requestId); if (!$stmt->execute()) { $stmt->close(); $this->db->rollback(); return null; } $id = (int) $stmt->insert_id; $stmt->close(); @@ -2261,7 +2260,7 @@ private function createRequestItemNcip( ]); } } - return $id > 0 ? ['loan_id' => $id, 'reservation_id' => null] : null; + return $id > 0 ? ['loan_id' => $id, 'reservation_id' => null, 'due_date' => $dueDate] : null; } catch (\Throwable $e) { $this->db->rollback(); SecureLogger::error('[NcipServer] createRequestItemNcip failed: ' . $e->getMessage()); diff --git a/tests/reservation-closures-100.unit.php b/tests/reservation-closures-100.unit.php new file mode 100644 index 000000000..57583ec32 --- /dev/null +++ b/tests/reservation-closures-100.unit.php @@ -0,0 +1,421 @@ +db->prepare("INSERT INTO calendario_chiusure (data_inizio,data_fine,descrizione,ricorrente_annuale,sede_id,attiva) VALUES (?,?,'Reservation matrix',?,?,?)"); + $repeats = (int) $yearly; + $enabled = (int) $active; + $stmt->bind_param('ssiii', $start, $end, $repeats, $branch, $enabled); + $stmt->execute(); + $stmt->close(); + LibraryCalendar::reset(); +} + +function matrixHoliday(SettingsReview25 $t, string $start, int $days, ?int $branch = null, bool $active = true): void +{ + $stmt = $t->db->prepare("INSERT INTO festivita_personalizzate (nome,tipo,date_specifiche,durata_giorni,sede_id,attiva) VALUES ('Reservation matrix holiday','dates',?,?,?,?)"); + $enabled = (int) $active; + $stmt->bind_param('siii', $start, $days, $branch, $enabled); + $stmt->execute(); + $stmt->close(); + LibraryCalendar::reset(); +} + +function matrixReserve(SettingsReview25 $t, array $book, int $reader, ?string $from, string $end, int $position = 1): int +{ + $stmt = $t->db->prepare("INSERT INTO prenotazioni (libro_id,utente_id,data_inizio_richiesta,data_fine_richiesta,data_scadenza_prenotazione,stato,queue_position) + VALUES (?,?,?,?,CONCAT(?,' 23:59:59'),'attiva',?)"); + $stmt->bind_param('iisssi', $book['id'], $reader, $from, $end, $end, $position); + $stmt->execute(); + $id = (int) $stmt->insert_id; + $stmt->close(); + return $id; +} + +function matrixLoan(SettingsReview25 $t, array $book, ?int $copy, string $start, string $end, string $state = 'prenotato', string $origin = 'richiesta', ?string $pickup = null): int +{ + $stmt = $t->db->prepare("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo,pickup_deadline) + VALUES (?,?,?,?,?,?,?,?,?)"); + $active = (int) ($state !== 'pendente'); + $stmt->bind_param('iiissssis', $book['id'], $copy, $t->reader, $start, $end, $state, $origin, $active, $pickup); + $stmt->execute(); + $id = (int) $stmt->insert_id; + $stmt->close(); + return $id; +} + +function matrixCopy(SettingsReview25 $t, array $book, int $branch): int +{ + $t->db->query("INSERT INTO copie (libro_id,numero_inventario,stato,sede_id) VALUES ({$book['id']},'Matrix-{$book['id']}-{$branch}','disponibile',{$branch})"); + return (int) $t->db->insert_id; +} + +function matrixState(SettingsReview25 $t, int $loan): string +{ + return (string) $t->scalar("SELECT stato FROM prestiti WHERE id={$loan}"); +} + +function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void +{ + SettingsReview25::check( + preg_match('/^UID:' . preg_quote($uid, '/') . '\r?\n(?:(?!END:VEVENT).)*?^DTEND;VALUE=DATE:' + . str_replace('-', '', $exclusiveEnd) . '\r?$/ms', $ics) === 1, + "{$uid} ends on the expected exclusive date {$exclusiveEnd}" + ); +} + +$calendars = [ + 'library closure, one day' => [1, static fn (SettingsReview25 $t, string $d) => matrixClosure($t, $d, $d)], + 'library closure, three days' => [3, static fn (SettingsReview25 $t, string $d) => matrixClosure($t, $d, $t->day(2, $d))], + 'observed New Year across the year boundary' => [2, static function (SettingsReview25 $t, string $d): void { + $newYear = $t->day(1, $d); + HolidayPresets::apply($t->db, 'US', '', ['US:new_year'], $newYear); + LibraryCalendar::reset(); + }, static function (SettingsReview25 $t): string { + // If January 1 is Saturday, the Friday before is observed as well. + for ($year = (int) substr($t->today, 0, 4) + 1; ; $year++) { + $newYear = new DateTimeImmutable($year . '-01-01', new DateTimeZone('UTC')); + if ($newYear->format('N') === '6') { + return $newYear->modify('-1 day')->format('Y-m-d'); + } + } + }], + 'another branch closes' => [0, static fn (SettingsReview25 $t, string $d) => matrixClosure($t, $d, $t->day(2, $d), $t->branchB)], + 'weekly closure' => [1, static fn (SettingsReview25 $t, string $d) => $t->set('loans', 'closed_weekdays', (new DateTimeImmutable($d))->format('w'))], + 'yearly closure projected from last year' => [2, static function (SettingsReview25 $t, string $d): void { + $year = (int) substr($d, 0, 4) - 1; + matrixClosure($t, $year . '-01-05', $year . '-01-06', yearly: true); + }, static fn (SettingsReview25 $t): string => ((int) substr($t->today, 0, 4) + 1) . '-01-05'], + 'custom holiday in the library' => [2, static fn (SettingsReview25 $t, string $d) => matrixHoliday($t, $d, 2)], + 'custom holiday at the lending branch' => [3, static fn (SettingsReview25 $t, string $d) => matrixHoliday($t, $d, 3, $t->branchA)], + 'inactive holiday' => [0, static fn (SettingsReview25 $t, string $d) => matrixHoliday($t, $d, 3, $t->branchA, false)], + 'dated closure followed by a custom holiday' => [2, static function (SettingsReview25 $t, string $d): void { + matrixClosure($t, $d, $d); + matrixHoliday($t, $t->day(1, $d), 1); + }], +]; + +/** @var array $cases */ +$cases = []; +foreach ($calendars as $name => $calendarSpec) { + [$closedDays, $apply] = $calendarSpec; + $chooseDate = $calendarSpec[2] ?? static fn (SettingsReview25 $t): string => $t->day(12); + $fixture = static function (SettingsReview25 $t) use ($closedDays, $apply, $chooseDate): array { + $book = $t->book($t->branchA); + $date = $chooseDate($t); + $apply($t, $date); + return [$book, $date, $t->day($closedDays, $date)]; + }; + + $cases["public date picker skips {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $view = (new ReservationsController($t->db))->getBookAvailabilityData($b['id'], $date, 5); + SettingsReview25::eq($view['earliest_available'], $open, 'Earliest selectable start'); + SettingsReview25::eq(in_array($date, $view['closed_dates'], true), $date !== $open, 'Closed dates and selectable starts agree'); + }; + $cases["copy-available estimate skips {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $date), $open, 'First open day with physical capacity'); + }; + $cases["staff creates a same-day hold during {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + (new ReservationsAdminController())->store($t->request('/admin/reservations/create', [ + 'libro_id' => $b['id'], 'utente_id' => $t->reader, 'data_prenotazione' => $date, 'data_scadenza' => $date, + ]), new Response(), $t->db); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $open], 'Stored staff hold covers the usable date'); + }; + $cases["staff edits a live hold into {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $id = matrixReserve($t, $b, $t->reader, $t->day(6), $t->day(7)); + (new ReservationsAdminController())->update($t->request('/admin/reservations/edit', [ + 'stato' => 'attiva', 'data_prenotazione' => $date, 'data_scadenza_prenotazione' => $date, + 'data_inizio_richiesta' => $date, 'data_fine_richiesta' => $date, + ]), new Response(), $t->db, $id); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE id={$id}")->fetch_row(); + SettingsReview25::eq($row, [$open, $open], 'Edited hold follows the same calendar as creation'); + }; + $cases["web request normalizes its start through {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $_SESSION['user']['id'] = $t->reader; + $_SESSION['user']['tipo_utente'] = 'standard'; + $end = $t->day(4, $date); + $response = (new ReservationsController($t->db))->createReservation($t->request('/api/reservations', [ + 'start_date' => $date, 'end_date' => $end, + ]), new Response(), ['id' => $b['id']]); + SettingsReview25::eq($response->getStatusCode(), 200, 'Web request is accepted'); + $row = $t->db->query("SELECT data_prestito,data_scadenza FROM prestiti WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $end], 'Loan starts on the usable day and keeps the open end'); + }; + $cases["explicit reader queue follows {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $_SESSION['user']['id'] = $t->reader; + $_SESSION['user']['tipo_utente'] = 'standard'; + $end = $t->day(4, $date); + (new UserActionsController())->reserve($t->request('/user/reserve', [ + 'libro_id' => $b['id'], 'desired_date' => $date, 'end_date' => $end, + ]), new Response(), $t->db); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $end], 'Reader hold is persisted on the usable window'); + }; + $cases["an existing queue still holds capacity through {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $id = matrixReserve($t, $b, $t->reader, $t->day(3), $date); + $capacity = new CapacityService($t->db); + SettingsReview25::check(!$capacity->hasFreeCapacity($b['id'], $open, $open), 'Last effective day belongs to the existing reservation'); + SettingsReview25::check($capacity->hasFreeCapacity($b['id'], $t->day(1, $open), $t->day(1, $open)), 'The following day is released'); + $calendar = (new UserLoanCalendar($t->db))->generate($t->reader, 'Test', includeReservations: true); + matrixCalendarEnd($calendar, 'reservation-' . $id . '@pinakes', $t->day(1, $open)); + }; + $cases["expiry uses the effective deadline after {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $deadline = $date . ' 18:00:00'; + $last = $open . ($date === $open ? ' 18:00:00' : ' 23:59:59'); + $after = $date === $open ? $open . ' 18:00:01' : $t->day(1, $open) . ' 00:00:00'; + SettingsReview25::eq(LibraryCalendar::reservationDeadline($t->db, $deadline, $b['id']), $last, 'Reader-visible queue deadline'); + SettingsReview25::check(!LibraryCalendar::reservationExpired($t->db, $deadline, $b['id'], $last), 'The final instant is still usable'); + SettingsReview25::check(LibraryCalendar::reservationExpired($t->db, $deadline, $b['id'], $after), 'The queue expires after the usable window'); + }; +} + +$cases['a free but closed segment does not mask an occupied reopening day'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + matrixClosure($t, $t->day(2), $t->day(3)); + matrixReserve($t, $b, $t->reader, $t->day(4), $t->day(5)); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $t->day(2)), $t->day(6), 'The first open and unoccupied day is after the later hold'); +}; +$cases['a picker cannot suggest an open date outside the selectable start horizon'] = static function (SettingsReview25 $t): void { + $b = $t->book(); + matrixClosure($t, $t->day(2), $t->day(4)); + $view = (new ReservationsController($t->db))->getBookAvailabilityData($b['id'], $t->day(2), 3, coverLoanWindow: true); + SettingsReview25::eq($view['earliest_available'], null, 'Extra return-date data must not extend selectable starts'); + SettingsReview25::check(count($view['days']) > 3, 'Return-date data still extends beyond the start horizon'); +}; +$cases['reopening cannot give a date to an overdue copy with no known return'] = static function (SettingsReview25 $t): void { + $b = $t->book(); + $t->loan($b, $t->day(-1), ['status' => 'in_ritardo']); + matrixHoliday($t, $t->today, 1); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $t->today), null, 'Physical return remains unknown'); +}; +$cases['a long but bounded picker includes holidays beyond the former scan cap'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $holiday = $t->day(1200); + matrixHoliday($t, $holiday, 1); + $view = (new ReservationsController($t->db))->getBookAvailabilityData($b['id'], $t->today, 180, coverLoanWindow: true); + SettingsReview25::check(isset($view['by_date'][$holiday]), 'The return date is inside the advertised window'); + SettingsReview25::check(in_array($holiday, $view['closed_dates'], true), 'The return date is also listed as closed'); +}; + +foreach ([ + 'scheduled, pinned' => ['prenotato', 'richiesta', true], + 'web pending, copyless' => ['pendente', 'richiesta', false], + 'queue conversion, pinned' => ['pendente', 'prenotazione', true], + 'NCIP pending, copyless' => ['pendente', 'ncip', false], +] as $kind => [$state, $origin, $pinned]) { + $cases["reopening protects {$kind} from the expiry sweep"] = static function (SettingsReview25 $t) use ($state, $origin, $pinned): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $pinned ? $b['copies'][0] : null, $t->day(-5), $t->day(-1), $state, $origin); + matrixHoliday($t, $t->day(-1), 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->checkExpiredReservations(), 0, 'Expiry cannot consume the usable reopening day'); + SettingsReview25::eq(matrixState($t, $loan), $state, 'The original state is retained'); + }; +} +$cases['copyless requests do not expire on their only branch holiday'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, null, $t->day(-5), $t->day(-1), 'pendente'); + matrixHoliday($t, $t->today, 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->checkExpiredReservations(), 0, 'Only branch is closed'); + SettingsReview25::eq(matrixState($t, $loan), 'pendente', 'The request remains pending'); +}; +$cases['a holiday at a different branch does not postpone a genuinely expired request'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(-1)); + matrixHoliday($t, $t->today, 1, $t->branchB); + SettingsReview25::eq((new MaintenanceService($t->db))->checkExpiredReservations(), 1, 'Unrelated holiday does not protect a past window'); + SettingsReview25::eq(matrixState($t, $loan), 'scaduto', 'The expired booking is closed'); +}; + +$cases['the library never announces scheduled pickups on a day it is closed'] = static function (SettingsReview25 $t): void { + $b = $t->book(); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(5)); + matrixClosure($t, $t->today, $t->today); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 0, 'No activation during a library closure'); + SettingsReview25::eq(matrixState($t, $loan), 'prenotato', 'The loan remains scheduled'); +}; +$cases['a pinned scheduled pickup waits for its own branch to reopen'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + matrixCopy($t, $b, $t->branchB); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(5)); + matrixHoliday($t, $t->today, 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 0, 'The announced branch is still closed'); + SettingsReview25::eq(matrixState($t, $loan), 'prenotato', 'An open sibling does not silently move an assigned pickup'); +}; +$cases['an unassigned scheduled pickup chooses an open branch'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $openCopy = matrixCopy($t, $b, $t->branchB); + $loan = matrixLoan($t, $b, null, $t->today, $t->day(5)); + matrixHoliday($t, $t->today, 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 1, 'An open copy can serve the booking'); + SettingsReview25::eq((int) $t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), $openCopy, 'The closed branch was skipped'); +}; +$cases['a scheduled loan can activate on the reopening day of a past stored end'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-4), $t->day(-1)); + matrixHoliday($t, $t->day(-1), 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 1, 'The scheduled loan can still be collected'); + $row = $t->db->query("SELECT stato,data_scadenza,pickup_deadline FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq($row, ['da_ritirare', $t->today, $t->today], 'Both deadlines are capped to the reopening day'); +}; +$cases['staff can confirm an unreconciled pickup on the effective deadline'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(4), 'da_ritirare', pickup: $t->day(-1)); + matrixHoliday($t, $t->day(-1), 1, $t->branchA); + matrixCalendarEnd((new IcsGenerator($t->db))->generate(), 'loan-' . $loan . '@pinakes', $t->day(1)); + matrixCalendarEnd((new UserLoanCalendar($t->db))->generate($t->reader, 'Test'), 'loan-' . $loan . '@pinakes', $t->day(1)); + $response = (new LoanApprovalController())->confirmPickup($t->request('/admin/loans/pickup', ['loan_id' => $loan]), new Response(), $t->db); + SettingsReview25::eq($response->getStatusCode(), 200, 'Reopening is still a valid pickup day before the sweep'); + SettingsReview25::eq(matrixState($t, $loan), 'in_corso', 'Pickup starts the loan'); +}; +$cases['a closed pickup cannot outlive an unchanged open return deadline'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(-2), 'da_ritirare', pickup: $t->day(-3)); + matrixHoliday($t, $t->day(-3), 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->checkExpiredPickups(), 1, 'A genuinely elapsed pickup is not revived'); + SettingsReview25::eq(matrixState($t, $loan), 'scaduto', 'The expired pickup frees the copy'); +}; +$cases['approval during a branch holiday schedules rather than announces immediate pickup'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, null, $t->today, $t->day(5), 'pendente'); + matrixHoliday($t, $t->today, 1, $t->branchA); + $response = (new LoanApprovalController())->approveLoan($t->request('/admin/loans/approve', ['loan_id' => $loan]), new Response(), $t->db); + SettingsReview25::eq($response->getStatusCode(), 200, 'Approval can reserve a future open pickup'); + $row = $t->db->query("SELECT stato,data_prestito,pickup_deadline FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq($row, ['prenotato', $t->day(1), null], 'The loan will activate only on reopening'); +}; +$cases['approval skips a copy whose effective return day is already promised'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $openCopy = matrixCopy($t, $b, $t->branchB); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(2), 'pendente'); + $t->loan($b, $t->day(7), ['copy' => $b['copies'][0], 'reader' => $t->secondReader, 'start' => $t->day(3), 'status' => 'prenotato']); + matrixHoliday($t, $t->day(2), 1, $t->branchA); + $response = (new LoanApprovalController())->approveLoan($t->request('/admin/loans/approve', ['loan_id' => $loan]), new Response(), $t->db); + SettingsReview25::eq($response->getStatusCode(), 200, 'A compatible copy can be approved'); + SettingsReview25::eq((int) $t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), $openCopy, 'The first copy cannot promise the reopening day twice'); +}; +$cases['a returned copy cannot take a hold that conflicts on its reopening day'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $copy = $b['copies'][0]; + $loan = matrixLoan($t, $b, null, $t->day(2), $t->day(4)); + $t->loan($b, $t->day(8), ['copy' => $copy, 'reader' => $t->secondReader, 'start' => $t->day(5), 'status' => 'prenotato']); + matrixHoliday($t, $t->day(4), 1, $t->branchA); + SettingsReview25::check(!(new ReservationReassignmentService($t->db))->reassignOnReturn($copy), 'The returned copy is not usable for both promises'); + SettingsReview25::eq($t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), null, 'The first hold remains unassigned'); +}; +$cases['a replacement copy in another branch stores its usable return day'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchB); + $replacement = matrixCopy($t, $b, $t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(2), $t->day(4)); + $t->db->query("UPDATE copie SET stato='perso' WHERE id={$b['copies'][0]}"); + matrixHoliday($t, $t->day(4), 1, $t->branchA); + (new ReservationReassignmentService($t->db))->reassignOnCopyLost($b['copies'][0]); + $row = $t->db->query("SELECT copia_id,data_scadenza FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq([(int) $row[0], $row[1]], [$replacement, $t->day(5)], 'Replacement commitment uses the destination calendar'); +}; + +// The inactive-rule controls already exercise the web picker and creation. +// Use the remaining two slots for both cancellation entry points. +unset($cases['copy-available estimate skips inactive holiday'], $cases['staff edits a live hold into inactive holiday']); +foreach (['staff', 'reader'] as $actor) { + $cases["{$actor} cancellation on reopening is voluntary, not an expiry"] = static function (SettingsReview25 $t) use ($actor): void { + $b = $t->book($t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(4), 'da_ritirare', pickup: $t->day(-1)); + matrixHoliday($t, $t->day(-1), 1, $t->branchA); + $request = $t->request('/cancel', ['loan_id' => $loan]); + if ($actor === 'staff') { + (new LoanApprovalController())->cancelPickup($request, new Response(), $t->db); + } else { + $_SESSION['user']['id'] = $t->reader; + $_SESSION['user']['tipo_utente'] = 'standard'; + (new UserActionsController())->cancelLoan($request, new Response(), $t->db); + } + SettingsReview25::eq(matrixState($t, $loan), 'annullato', 'The collection deadline is still usable'); + }; +} + +unset($cases['staff creates a same-day hold during inactive holiday']); +$cases['NCIP requests start on reopening and return the persisted effective due date'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + matrixHoliday($t, $t->today, 1, $t->branchA); + matrixHoliday($t, $t->day(15), 1, $t->branchA); + $plugin = new NcipServerPlugin($t->db, new App\Support\HookManager($t->db)); + $plugin->onInstall(); + $reason = ''; + $outcome = (new ReflectionMethod($plugin, 'createRequestItemNcip'))->invokeArgs($plugin, [$b['id'], $t->reader, null, 'matrix-ncip', &$reason]); + SettingsReview25::check(is_array($outcome), "NCIP can place the request (reason: {$reason})"); + $id = (int) $outcome['loan_id']; + $row = $t->db->query("SELECT data_prestito,data_scadenza FROM prestiti WHERE id={$id}")->fetch_row(); + SettingsReview25::eq($row, [$t->day(1), $t->day(16)], 'Duration starts at reopening and skips the return holiday'); + SettingsReview25::eq($outcome['due_date'], $row[1], 'NCIP returns the same due date as the stored request'); +}; + +$passed = $failed = 0; +$suite = null; +$savedSession = $_SESSION ?? []; +try { + SettingsReview25::eq(count($cases), 100, 'There must be exactly 100 distinct acceptance cases'); + $selected = array_filter(explode(',', getenv('RESERVATION_CASE') ?: '')); + SettingsReview25::check(array_diff($selected, range(1, 100)) === [], 'RESERVATION_CASE must contain existing case numbers'); + $suite = new SettingsReview25(); + ConfigStore::usingConnection($suite->db, static function () use ($suite, $cases, $selected, &$passed, &$failed): void { + $number = 0; + foreach ($cases as $name => $case) { + $number++; + if ($selected !== [] && !in_array((string) $number, $selected, true)) { + continue; + } + try { + $suite->reset(); + $case($suite); + $passed++; + echo "PASS {$number} {$name}\n"; + } catch (Throwable $error) { + $failed++; + echo "FAIL {$number} {$name}: {$error->getMessage()}\n"; + } + } + }); +} catch (Throwable $error) { + $failed++; + fwrite(STDERR, 'FAIL setup: ' . $error->getMessage() . PHP_EOL); +} finally { + $_SESSION = $savedSession; + LibraryCalendar::reset(); + $suite?->close(); +} +echo "Reservation closures: {$passed} passed, {$failed} failed\n"; +exit($failed === 0 ? 0 : 1); From 87a948e757163ee264599e08f4fa0b641f45a972 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 15:56:39 +0100 Subject: [PATCH 2/9] Respect overlapping closures across lending branches --- app/Controllers/ReservationsController.php | 2 +- app/Services/CapacityService.php | 4 +- app/Support/DataIntegrity.php | 2 +- app/Support/LibraryCalendar.php | 81 ++++++++++++----- .../closures-calendar-existing-loans.spec.js | 11 ++- tests/closures-calendar-loans.spec.js | 5 +- tests/reservation-closures-100.unit.php | 89 +++++++++++-------- 7 files changed, 126 insertions(+), 68 deletions(-) diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index bf7c8cf45..0beb11811 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -820,7 +820,7 @@ public function getBookAvailabilityData($bookId, ?string $startDate = null, int ->closedDatesBetween( $from, \App\Support\LibraryCalendar::shift($from, $closedHorizon), - \App\Support\LibraryCalendar::requestBranch($this->db, (int) $bookId), + \App\Support\LibraryCalendar::requestScope($this->db, (int) $bookId), $closedHorizon + 1 ); $reservationIntervals = (new \App\Services\CapacityService($this->db))->reservationIntervalsInRange( diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index a63d276fd..297c00e21 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -181,7 +181,7 @@ public function firstAvailableDate(int $libroId, string $from): ?string return null; } $calendar = \App\Support\LibraryCalendar::forDb($this->db); - $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::requestBranch($this->db, $libroId) : null; + $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::requestScope($this->db, $libroId) : null; // Keep one day available for finite end+1 events. Any interval clamped // to this horizon is effectively open-ended for application purposes. @@ -621,7 +621,7 @@ private function fetchIntervals(string $sql, string $types, array $params, ?stri $out = []; $calendar = \App\Support\LibraryCalendar::forDb($this->db); $branch = $reservationBookId !== null && $calendar->hasRules() - ? \App\Support\LibraryCalendar::requestBranch($this->db, $reservationBookId, $this->lockingReads) + ? \App\Support\LibraryCalendar::requestScope($this->db, $reservationBookId, $this->lockingReads) : null; while ($row = $res->fetch_assoc()) { $s = (string) $row['s']; diff --git a/app/Support/DataIntegrity.php b/app/Support/DataIntegrity.php index 4563f5be3..6596131b3 100644 --- a/app/Support/DataIntegrity.php +++ b/app/Support/DataIntegrity.php @@ -243,7 +243,7 @@ private function reservationEndTodaySql(string $today, ?int $bookId = null): str while ($row = $rows->fetch_assoc()) { $id = (int) $row['libro_id']; if (!array_key_exists($id, $branches)) { - $branches[$id] = LibraryCalendar::requestBranch($this->db, $id, lockingRead: true); + $branches[$id] = LibraryCalendar::requestScope($this->db, $id, lockingRead: true); } if ($calendar->nextOpenDay((string) $row['due'], $branches[$id]) >= $today) { $extended[] = (int) $row['id']; diff --git a/app/Support/LibraryCalendar.php b/app/Support/LibraryCalendar.php index ab261ea63..db48c1426 100644 --- a/app/Support/LibraryCalendar.php +++ b/app/Support/LibraryCalendar.php @@ -257,7 +257,7 @@ public static function loanDates(\mysqli $db, array $loan, ?string $today = null $calendar = self::forDb($db); if ($calendar->hasRules()) { $branch = $scheduled && $copyId === null && isset($loan['libro_id']) - ? self::requestBranch($db, (int) $loan['libro_id']) + ? self::requestScope($db, (int) $loan['libro_id']) : self::branchOfCopy($db, $copyId); $effectiveDue = $calendar->nextOpenDay($due, $branch); } @@ -713,9 +713,7 @@ public static function holdingEnd(\mysqli $db, string $storedDue, ?int $copiaId) } /** - * The next open day a request for this book can start on: its copies' - * branch when they all sit in one, otherwise the library as a whole, since - * no copy is bound yet. + * The next day at least one branch with a lendable copy can serve a request. */ public static function requestStartDay(\mysqli $db, string $ymd, int $bookId): string { @@ -723,7 +721,7 @@ public static function requestStartDay(\mysqli $db, string $ymd, int $bookId): s if (!$calendar->hasRules()) { return $ymd; } - return $calendar->nextOpenDay($ymd, self::requestBranch($db, $bookId)); + return $calendar->nextOpenDay($ymd, self::requestScope($db, $bookId)); } /** @@ -738,13 +736,13 @@ public static function requestDueDate(\mysqli $db, string $startYmd, int $days, if (!$calendar->hasRules()) { return $due; } - return $calendar->nextOpenDay($due, self::requestBranch($db, $bookId)); + return $calendar->nextOpenDay($due, self::requestScope($db, $bookId)); } - /** An unassigned hold follows the book's request branch until a copy is pinned. */ - public static function reservationBranch(\mysqli $db, int $bookId, ?int $copyId): ?int + /** @return int|list|null Scope before and after a hold is pinned to a copy. */ + public static function reservationBranch(\mysqli $db, int $bookId, ?int $copyId): int|array|null { - return $copyId === null ? self::requestBranch($db, $bookId) : self::branchOfCopy($db, $copyId); + return $copyId === null ? self::requestScope($db, $bookId) : self::branchOfCopy($db, $copyId); } /** @return array{string,string} Effective start and inclusive end for an active queue event. */ @@ -754,7 +752,7 @@ public static function reservationCalendarDates(\mysqli $db, int $bookId, string if (!$calendar->hasRules()) { return [$start, $end]; } - $branch = self::requestBranch($db, $bookId); + $branch = self::requestScope($db, $bookId); $end = $calendar->nextOpenDay($end, $branch); return [min($calendar->nextOpenDay($start, $branch), $end), $end]; } @@ -770,7 +768,7 @@ public static function reservationExpired(\mysqli $db, string $deadline, int $bo if (!$calendar->hasRules()) { return $deadline < $now; } - $branch = self::requestBranch($db, $bookId); + $branch = self::requestScope($db, $bookId); if (!$calendar->isOpen(substr($now, 0, 10), $branch)) { return false; } @@ -783,14 +781,14 @@ public static function reservationExpired(\mysqli $db, string $deadline, int $bo * request branch; an open day (or no calendar) keeps the stored value. * Used by expiry and by the expiry e-mail, so both quote the same day. */ - public static function reservationDeadline(\mysqli $db, string $deadline, int $bookId, ?int $branch = null): string + public static function reservationDeadline(\mysqli $db, string $deadline, int $bookId, int|array|null $branch = null): string { $day = substr($deadline, 0, 10); $calendar = self::forDb($db); if (!DateHelper::isISODateFormat($day) || !$calendar->hasRules()) { return $deadline; } - $effective = $calendar->nextOpenDay($day, $branch ?? self::requestBranch($db, $bookId)); + $effective = $calendar->nextOpenDay($day, $branch ?? self::requestScope($db, $bookId)); return $effective > $day ? $effective . ' 23:59:59' : $deadline; } @@ -801,12 +799,24 @@ public static function legacyReservationDeadlineFloor(\mysqli $db, string $today if (!$calendar->hasRules()) { return $today; } - $previousOpen = $calendar->previousOpenDay(self::shift($today, -1), self::requestBranch($db, $bookId)); + $previousOpen = $calendar->previousOpenDay(self::shift($today, -1), self::requestScope($db, $bookId)); return self::shift($previousOpen, 1); } /** The branch of the book's lendable copies when they all sit in one, otherwise null. */ public static function requestBranch(\mysqli $db, int $bookId, bool $lockingRead = false): ?int + { + $scope = self::requestScope($db, $bookId, $lockingRead); + return is_int($scope) ? $scope : null; + } + + /** + * Open at any branch with a lendable copy. An unlocated copy follows the + * library calendar, which is also the fallback for titles with no copies. + * + * @return int|list|null + */ + public static function requestScope(\mysqli $db, int $bookId, bool $lockingRead = false): int|array|null { $lendable = (new \App\Services\CirculationPolicy($db))->lendableCopySql(''); $distinct = $lockingRead ? '' : 'DISTINCT '; @@ -819,7 +829,10 @@ public static function requestBranch(\mysqli $db, int $bookId, bool $lockingRead $stmt->execute(); $branches = array_values(array_unique(array_column($stmt->get_result()->fetch_all(MYSQLI_NUM), 0), SORT_REGULAR)); $stmt->close(); - return count($branches) === 1 && $branches[0] !== null ? (int) $branches[0] : null; + if ($branches === [] || in_array(null, $branches, true)) { + return null; + } + return count($branches) === 1 ? (int) $branches[0] : array_map('intval', $branches); } /** @@ -979,8 +992,16 @@ public function hasRules(): bool * only when that branch is asked for; a closure without a branch counts * for every branch and for the library as a whole ($sedeId = null). */ - public function isOpen(string $ymd, ?int $sedeId = null): bool + public function isOpen(string $ymd, int|array|null $sedeId = null): bool { + if (is_array($sedeId)) { + foreach ($sedeId as $branch) { + if ($this->isOpen($ymd, $branch)) { + return true; + } + } + return false; + } if (!DateHelper::isISODateFormat($ymd)) { return true; } @@ -1039,14 +1060,24 @@ private function isHoliday(string $ymd, ?int $sedeId = null): bool * inside a dated or yearly closure jumps to the end of that occurrence. * If no open day is found within the scan bound, keep the original day. */ - public function nextOpenDay(string $ymd, ?int $sedeId = null): string + public function nextOpenDay(string $ymd, int|array|null $sedeId = null): string { return $this->firstOpenDayBetween($ymd, '9999-12-31', $sedeId) ?? $ymd; } /** First open day in the inclusive window, or null if it has none. */ - public function firstOpenDayBetween(string $ymd, string $lastDay, ?int $sedeId = null): ?string + public function firstOpenDayBetween(string $ymd, string $lastDay, int|array|null $sedeId = null): ?string { + if (is_array($sedeId)) { + $first = null; + foreach ($sedeId as $branch) { + $candidate = $this->firstOpenDayBetween($ymd, $lastDay, $branch); + if ($candidate !== null && ($first === null || $candidate < $first)) { + $first = $candidate; + } + } + return $first; + } if (!DateHelper::isISODateFormat($ymd)) { return null; } @@ -1073,8 +1104,18 @@ public function shiftForward(string $ymd, ?int $sedeId = null): string * $ymd itself when open, otherwise the last open day before it: the * mirror of nextOpenDay(), jumping to the day before a one-off closure. */ - public function previousOpenDay(string $ymd, ?int $sedeId = null): string + public function previousOpenDay(string $ymd, int|array|null $sedeId = null): string { + if (is_array($sedeId)) { + $last = null; + foreach ($sedeId as $branch) { + $candidate = $this->previousOpenDay($ymd, $branch); + if ($this->isOpen($candidate, $branch) && ($last === null || $candidate > $last)) { + $last = $candidate; + } + } + return $last ?? $ymd; + } if (!DateHelper::isISODateFormat($ymd)) { return $ymd; } @@ -1118,7 +1159,7 @@ public function addOpenDays(string $from, int $n, ?int $sedeId = null): string * * @return list */ - public function closedDatesBetween(string $from, string $to, ?int $sedeId = null, int $maxDays = self::MAX_SCAN_DAYS): array + public function closedDatesBetween(string $from, string $to, int|array|null $sedeId = null, int $maxDays = self::MAX_SCAN_DAYS): array { if (!$this->hasRules() || !DateHelper::isISODateFormat($from) || !DateHelper::isISODateFormat($to) || $to < $from) { return []; diff --git a/tests/closures-calendar-existing-loans.spec.js b/tests/closures-calendar-existing-loans.spec.js index d0c680dba..d5993168d 100644 --- a/tests/closures-calendar-existing-loans.spec.js +++ b/tests/closures-calendar-existing-loans.spec.js @@ -339,7 +339,7 @@ test.describe.serial('Closures added after the loan', () => { } }); - test('Move 11: when the next booking of that copy starts the day after, the activation still happens and keeps the date', async () => { + test('Move 11: activation waits if another booking holds the copy on reopening', async () => { const book = title('activate-booked'); const copy = book.copyIds[0]; const due = addDays(today, 12); @@ -354,9 +354,12 @@ test.describe.serial('Closures added after the loan', () => { db(`INSERT INTO prestiti (libro_id, copia_id, utente_id, data_prestito, data_scadenza, stato, origine, attivo) VALUES (${book.id}, NULL, ${patron.id}, '${today}', '${due}', 'prenotato', 'prenotazione', 1)`); const loanId = F.dbInt(`SELECT id FROM prestiti WHERE libro_id=${book.id} AND utente_id=${patron.id} ORDER BY id DESC LIMIT 1`); runMaintenance(); - // Moving the date would overlap the next booking (the trigger would - // refuse the whole activation, on every pass): activated, date kept. - expect(loanRow(loanId).split('|').slice(0, 2)).toEqual(['da_ritirare', due]); + // A closed stored date cannot make an overlap on the usable day safe. + expect(loanRow(loanId).split('|').slice(0, 2)).toEqual(['prenotato', due]); + expect(F.dbInt(`SELECT COALESCE(copia_id,0) FROM prestiti WHERE id=${loanId}`)).toBe(0); + db(`DELETE FROM prestiti WHERE libro_id=${book.id} AND utente_id=${other.id}`); + runMaintenance(); + expect(loanRow(loanId).split('|').slice(0, 2)).toEqual(['da_ritirare', addDays(due, 1)]); expect(F.dbInt(`SELECT copia_id FROM prestiti WHERE id=${loanId}`)).toBe(copy); } finally { db(`DELETE FROM prestiti WHERE libro_id=${book.id}`); diff --git a/tests/closures-calendar-loans.spec.js b/tests/closures-calendar-loans.spec.js index 401dc14a9..776de5818 100644 --- a/tests/closures-calendar-loans.spec.js +++ b/tests/closures-calendar-loans.spec.js @@ -347,7 +347,7 @@ test.describe('Closures calendar — circulation', () => { } }); - test('Admin 9: the approval keeps the requested due date when the copy is taken right after it', async ({ page }) => { + test('Admin 9: approval chooses another copy if the pinned one is promised on reopening', async ({ page }) => { db(`INSERT INTO sedi (nome) VALUES ('${TAG} sede piena')`); const sede = parseInt(db(`SELECT id FROM sedi WHERE nome='${TAG} sede piena'`), 10); const book = books.approvebusy; @@ -356,6 +356,7 @@ test.describe('Closures calendar — circulation', () => { const copyA = parseInt(db(`SELECT id FROM copie WHERE libro_id=${book.id} LIMIT 1`), 10); // A second copy keeps the title-level capacity free on the extra day. db(`INSERT INTO copie (libro_id, stato, numero_inventario, created_at) VALUES (${book.id}, 'disponibile', 'CAL-approvebusy2-${RUN}', NOW())`); + const copyB = parseInt(db(`SELECT id FROM copie WHERE numero_inventario='CAL-approvebusy2-${RUN}'`), 10); db(`UPDATE libri SET copie_totali=2, copie_disponibili=2 WHERE id=${book.id}`); db(`UPDATE copie SET sede_id=${sede} WHERE id=${copyA}`); const due = addDays(today, 10); @@ -377,7 +378,7 @@ test.describe('Closures calendar — circulation', () => { await page.locator('.swal2-confirm').click(); const body = await (await reply).json(); expect(body.success, JSON.stringify(body)).toBe(true); - expect(db(`SELECT CONCAT(stato,'|',copia_id,'|',data_scadenza) FROM prestiti WHERE id=${loanId}`)).toBe(`da_ritirare|${copyA}|${due}`); + expect(db(`SELECT CONCAT(stato,'|',copia_id,'|',data_scadenza) FROM prestiti WHERE id=${loanId}`)).toBe(`da_ritirare|${copyB}|${due}`); } finally { db(`DELETE FROM calendario_chiusure WHERE sede_id=${sede}`); db(`UPDATE copie SET sede_id=NULL WHERE libro_id=${book.id}`); diff --git a/tests/reservation-closures-100.unit.php b/tests/reservation-closures-100.unit.php index 57583ec32..b65b089a0 100644 --- a/tests/reservation-closures-100.unit.php +++ b/tests/reservation-closures-100.unit.php @@ -138,40 +138,43 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::eq($view['earliest_available'], $open, 'Earliest selectable start'); SettingsReview25::eq(in_array($date, $view['closed_dates'], true), $date !== $open, 'Closed dates and selectable starts agree'); }; - $cases["copy-available estimate skips {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { - [$b, $date, $open] = $fixture($t); - SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $date), $open, 'First open day with physical capacity'); - }; - $cases["staff creates a same-day hold during {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { - [$b, $date, $open] = $fixture($t); - (new ReservationsAdminController())->store($t->request('/admin/reservations/create', [ - 'libro_id' => $b['id'], 'utente_id' => $t->reader, 'data_prenotazione' => $date, 'data_scadenza' => $date, - ]), new Response(), $t->db); - $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); - SettingsReview25::eq($row, [$open, $open], 'Stored staff hold covers the usable date'); - }; - $cases["staff edits a live hold into {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { - [$b, $date, $open] = $fixture($t); - $id = matrixReserve($t, $b, $t->reader, $t->day(6), $t->day(7)); - (new ReservationsAdminController())->update($t->request('/admin/reservations/edit', [ - 'stato' => 'attiva', 'data_prenotazione' => $date, 'data_scadenza_prenotazione' => $date, - 'data_inizio_richiesta' => $date, 'data_fine_richiesta' => $date, - ]), new Response(), $t->db, $id); - $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE id={$id}")->fetch_row(); - SettingsReview25::eq($row, [$open, $open], 'Edited hold follows the same calendar as creation'); - }; - $cases["web request normalizes its start through {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { - [$b, $date, $open] = $fixture($t); - $_SESSION['user']['id'] = $t->reader; - $_SESSION['user']['tipo_utente'] = 'standard'; - $end = $t->day(4, $date); - $response = (new ReservationsController($t->db))->createReservation($t->request('/api/reservations', [ - 'start_date' => $date, 'end_date' => $end, - ]), new Response(), ['id' => $b['id']]); - SettingsReview25::eq($response->getStatusCode(), 200, 'Web request is accepted'); - $row = $t->db->query("SELECT data_prestito,data_scadenza FROM prestiti WHERE libro_id={$b['id']}")->fetch_row(); - SettingsReview25::eq($row, [$open, $end], 'Loan starts on the usable day and keeps the open end'); - }; + // The inactive-rule control needs no duplicate checks of the other writers. + if ($name !== 'inactive holiday') { + $cases["copy-available estimate skips {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $date), $open, 'First open day with physical capacity'); + }; + $cases["staff creates a same-day hold during {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + (new ReservationsAdminController())->store($t->request('/admin/reservations/create', [ + 'libro_id' => $b['id'], 'utente_id' => $t->reader, 'data_prenotazione' => $date, 'data_scadenza' => $date, + ]), new Response(), $t->db); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $open], 'Stored staff hold covers the usable date'); + }; + $cases["staff edits a live hold into {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $id = matrixReserve($t, $b, $t->reader, $t->day(6), $t->day(7)); + (new ReservationsAdminController())->update($t->request('/admin/reservations/edit', [ + 'stato' => 'attiva', 'data_prenotazione' => $date, 'data_scadenza_prenotazione' => $date, + 'data_inizio_richiesta' => $date, 'data_fine_richiesta' => $date, + ]), new Response(), $t->db, $id); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE id={$id}")->fetch_row(); + SettingsReview25::eq($row, [$open, $open], 'Edited hold follows the same calendar as creation'); + }; + $cases["web request normalizes its start through {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $_SESSION['user']['id'] = $t->reader; + $_SESSION['user']['tipo_utente'] = 'standard'; + $end = $t->day(4, $date); + $response = (new ReservationsController($t->db))->createReservation($t->request('/api/reservations', [ + 'start_date' => $date, 'end_date' => $end, + ]), new Response(), ['id' => $b['id']]); + SettingsReview25::eq($response->getStatusCode(), 200, 'Web request is accepted'); + $row = $t->db->query("SELECT data_prestito,data_scadenza FROM prestiti WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $end], 'Loan starts on the usable day and keeps the open end'); + }; + } $cases["explicit reader queue follows {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { [$b, $date, $open] = $fixture($t); $_SESSION['user']['id'] = $t->reader; @@ -347,9 +350,6 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::eq([(int) $row[0], $row[1]], [$replacement, $t->day(5)], 'Replacement commitment uses the destination calendar'); }; -// The inactive-rule controls already exercise the web picker and creation. -// Use the remaining two slots for both cancellation entry points. -unset($cases['copy-available estimate skips inactive holiday'], $cases['staff edits a live hold into inactive holiday']); foreach (['staff', 'reader'] as $actor) { $cases["{$actor} cancellation on reopening is voluntary, not an expiry"] = static function (SettingsReview25 $t) use ($actor): void { $b = $t->book($t->branchA); @@ -367,7 +367,6 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void }; } -unset($cases['staff creates a same-day hold during inactive holiday']); $cases['NCIP requests start on reopening and return the persisted effective due date'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); matrixHoliday($t, $t->today, 1, $t->branchA); @@ -383,6 +382,20 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::eq($outcome['due_date'], $row[1], 'NCIP returns the same due date as the stored request'); }; +$cases['a mixed-branch title waits until at least one lending branch reopens'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + matrixCopy($t, $b, $t->branchB); + $day = $t->day(2); + matrixClosure($t, $day, $t->day(4), $t->branchA); + matrixHoliday($t, $day, 1, $t->branchB); + $firstOpen = $t->day(3); + $availability = (new ReservationsController($t->db))->getBookAvailabilityData($b['id'], $day, 4); + SettingsReview25::eq($availability['earliest_available'], $firstOpen, 'No site can serve the request on the first day'); + SettingsReview25::check(in_array($day, $availability['closed_dates'], true), 'Overlapping branch closures are presented as closed'); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $day), $firstOpen, 'The copy-available estimate follows the same branch scope'); + SettingsReview25::eq(LibraryCalendar::reservationDeadline($t->db, $day . ' 18:00:00', $b['id']), $firstOpen . ' 23:59:59', 'A deadline extends until one branch can accept it'); +}; + $passed = $failed = 0; $suite = null; $savedSession = $_SESSION ?? []; From a8e662ea9af3046d6e56cd969031a3533abc780c Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 16:06:44 +0100 Subject: [PATCH 3/9] Roll back copy loss when a valid hold cannot be moved --- .../ReservationReassignmentService.php | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index a1fbfa616..fae8de1e1 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -363,26 +363,7 @@ public function reassignOnCopyLost(int $copiaId): bool $calendar = \App\Support\LibraryCalendar::forDb($this->db); $originalBranch = $calendar->hasRules() ? \App\Support\LibraryCalendar::branchOfCopy($this->db, $copiaId) : null; $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; - $stmt = $this->db->prepare(" - SELECT id, libro_id, utente_id, data_prestito, data_scadenza - FROM prestiti - WHERE copia_id = ? - AND data_scadenza >= ? - AND ( (attivo = 1 AND stato IN ('prenotato', 'da_ritirare')) - OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) - ORDER BY data_prestito ASC, id ASC - "); - $stmt->bind_param('is', $copiaId, $candidateFloor); - $stmt->execute(); - $rows = $stmt->get_result(); - $reservation = null; - while ($candidate = $rows->fetch_assoc()) { - if ($calendar->nextOpenDay((string) $candidate['data_scadenza'], $originalBranch) >= $today) { - $reservation = $candidate; - break; - } - } - $stmt->close(); + $reservation = $this->usableHoldOnCopy($copiaId, $today); if ($reservation === null) { return false; @@ -550,11 +531,47 @@ public function reassignOnCopyLost(int $copiaId): bool return $this->handleNoCopyAvailable($reservationId, $copiaId, $resEnd); } + /** @return array|null */ + private function usableHoldOnCopy(int $copiaId, string $today, bool $lockingRead = false): ?array + { + $calendar = \App\Support\LibraryCalendar::forDb($this->db); + $originalBranch = $calendar->hasRules() ? \App\Support\LibraryCalendar::branchOfCopy($this->db, $copiaId) : null; + $candidateFloor = $calendar->hasRules() ? '1000-01-01' : $today; + $lock = $lockingRead ? ' FOR UPDATE' : ''; + $stmt = $this->db->prepare(" + SELECT id, libro_id, utente_id, data_prestito, data_scadenza + FROM prestiti + WHERE copia_id = ? + AND data_scadenza >= ? + AND ( (attivo = 1 AND stato IN ('prenotato', 'da_ritirare')) + OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) + ORDER BY data_prestito ASC, id ASC{$lock} + "); + $stmt->bind_param('is', $copiaId, $candidateFloor); + $stmt->execute(); + $rows = $stmt->get_result(); + $reservation = null; + while ($candidate = $rows->fetch_assoc()) { + if ($calendar->nextOpenDay((string) $candidate['data_scadenza'], $originalBranch) >= $today) { + $reservation = $candidate; + break; + } + } + $stmt->close(); + + return $reservation; + } + /** Reassign all still-usable holds without spinning on already expired rows. */ public function reassignAllOnCopyLost(int $copyId): void { for ($guard = 0; $guard < 1000; $guard++) { if (!$this->reassignOnCopyLost($copyId)) { + // A refused update (for example a trigger restoring the + // assignment) must not look like the end of the queue. + if ($this->usableHoldOnCopy($copyId, \App\Support\DateHelper::today(), lockingRead: true) !== null) { + throw new \RuntimeException('Hold could not be moved from copy ' . $copyId); + } return; } } From 141fb77047e78db3b52b4b5bf09bd4dbf49b3bf9 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 16:37:24 +0100 Subject: [PATCH 4/9] Honor both pickup deadlines and try another copy at activation --- app/Controllers/LoanApprovalController.php | 17 +++++--- app/Support/LibraryCalendar.php | 14 +++++-- app/Support/MaintenanceService.php | 13 ++++++- tests/reservation-closures-100.unit.php | 45 ++++++++++++++-------- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 93e45107a..132d29bce 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -1038,10 +1038,15 @@ public function confirmPickup(Request $request, Response $response, mysqli $db): return $response->withHeader('Content-Type', 'application/json')->withStatus(400); } - // Guardia L1: pickup_deadline (per prestiti storici, pre-cap) può superare - // data_scadenza. Non avviare un 'in_corso' la cui finestra è già interamente - // trascorsa: nascerebbe scaduto. - if (!empty($loan['data_scadenza']) && $loan['data_scadenza'] < $today) { + // On reopening, a still-unreconciled return date can move under + // this same book lock. Do not start a loan if the extended + // window is already promised to another reader. + $dueDate = $loan['data_scadenza']; + if ($dueDate !== null && $dueDate < $today) { + $capacity = new \App\Services\CapacityService($db, lockingReads: true); + $dueDate = $capacity->reservationDueDateForCopy($libroId, $copiaId, $today, $dueDate, $loanId); + } + if (!empty($loan['data_scadenza']) && ($dueDate === null || $dueDate < $today)) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -1053,10 +1058,10 @@ public function confirmPickup(Request $request, Response $response, mysqli $db): // Update loan state to 'in_corso' $updateStmt = $db->prepare(" UPDATE prestiti - SET stato = 'in_corso', pickup_deadline = NULL + SET stato = 'in_corso', pickup_deadline = NULL, data_scadenza = ? WHERE id = ? "); - $updateStmt->bind_param('i', $loanId); + $updateStmt->bind_param('si', $dueDate, $loanId); $updateStmt->execute(); $updateStmt->close(); diff --git a/app/Support/LibraryCalendar.php b/app/Support/LibraryCalendar.php index db48c1426..3b69f6db7 100644 --- a/app/Support/LibraryCalendar.php +++ b/app/Support/LibraryCalendar.php @@ -920,7 +920,7 @@ public static function extendPickupDeadlines(\mysqli $db): int $moved = 0; foreach ($rows as $row) { $deadline = substr((string) $row['pickup_deadline'], 0, 10); - $next = $calendar->pickupDeadlineOnReopening($deadline, $row['data_scadenza'], $row['sede_id'] !== null ? (int) $row['sede_id'] : null); + $next = $calendar->pickupDeadlineOnReopening($deadline, $row['data_scadenza'], $row['sede_id'] !== null ? (int) $row['sede_id'] : null, useEffectiveDue: false); if ($next <= $deadline || $next < $today) { continue; } @@ -942,7 +942,7 @@ public static function extendPickupDeadlines(\mysqli $db): int return; } $deadline = substr((string) $current['pickup_deadline'], 0, 10); - $next = $calendar->pickupDeadlineOnReopening($deadline, $current['data_scadenza'], $current['sede_id'] !== null ? (int) $current['sede_id'] : null); + $next = $calendar->pickupDeadlineOnReopening($deadline, $current['data_scadenza'], $current['sede_id'] !== null ? (int) $current['sede_id'] : null, useEffectiveDue: false); if ($next <= $deadline || $next < $today) { return; } @@ -960,11 +960,17 @@ public static function extendPickupDeadlines(\mysqli $db): int } /** A closed pickup day can move forward, within the stored loan period. */ - public function pickupDeadlineOnReopening(string $deadline, ?string $dueDate, ?int $branch): string + public function pickupDeadlineOnReopening(string $deadline, ?string $dueDate, ?int $branch, bool $useEffectiveDue = true): string { $next = $this->nextOpenDay($deadline, $branch); $due = substr($dueDate ?? '', 0, 10); - return DateHelper::isISODateFormat($due) ? max($deadline, min($next, $due)) : $next; + if (!DateHelper::isISODateFormat($due)) { + return $next; + } + // Reads can show the reopening day before maintenance reconciles the + // loan. Writes must keep the pickup deadline inside the stored period. + $cap = $useEffectiveDue ? $this->nextOpenDay($due, $branch) : $due; + return max($deadline, min($next, $cap)); } /** The collection deadline shown to readers, even before reconciliation. */ diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index cb0bfee66..e5480199a 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -1015,6 +1015,7 @@ public function activateScheduledLoans(): int // promoting them with copia_id=NULL only moves the failure to // confirmPickup(), which correctly refuses a copy-less loan. $copiaId = !empty($loan['copia_id']) ? (int) $loan['copia_id'] : 0; + $dueDate = null; if ($copiaId > 0 && in_array($copiaId, $committedCopyIds, true)) { // Repair legacy/reassigned rows that share a copy with another // open loan for this borrower/title: activation must choose a @@ -1060,6 +1061,13 @@ public function activateScheduledLoans(): int 'copia_impegnata' => $copyHeld, ]); $copiaId = 0; + } else { + $dueDate = $this->activationDueDate($calendar, $loanId, $bookId, $copiaId, (string) $loan['data_prestito'], $oldDue, $today); + if ($dueDate === null) { + // The pinned copy cannot honor its effective return + // window. Try an open sibling before deferring. + $copiaId = 0; + } } } if ($copiaId <= 0) { @@ -1108,8 +1116,10 @@ public function activateScheduledLoans(): int $freeCopy->close(); foreach ($freeCopyRows as $freeCopyRow) { $candidateId = (int) $freeCopyRow['id']; - if ($this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $oldDue, $today) !== null) { + $candidateDue = $this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $oldDue, $today); + if ($candidateDue !== null) { $copiaId = $candidateId; + $dueDate = $candidateDue; break; } } @@ -1131,7 +1141,6 @@ public function activateScheduledLoans(): int // on a day that branch is closed moves to its next open day now, // before the pickup deadline is capped at it and the reader is // told both (the pickup-ready e-mail below). - $dueDate = $this->activationDueDate($calendar, $loanId, $bookId, $copiaId, (string) $loan['data_prestito'], $oldDue, $today); if ($dueDate === null) { $this->db->rollback(); continue; diff --git a/tests/reservation-closures-100.unit.php b/tests/reservation-closures-100.unit.php index b65b089a0..f7c83151c 100644 --- a/tests/reservation-closures-100.unit.php +++ b/tests/reservation-closures-100.unit.php @@ -175,17 +175,19 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::eq($row, [$open, $end], 'Loan starts on the usable day and keeps the open end'); }; } - $cases["explicit reader queue follows {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { - [$b, $date, $open] = $fixture($t); - $_SESSION['user']['id'] = $t->reader; - $_SESSION['user']['tipo_utente'] = 'standard'; - $end = $t->day(4, $date); - (new UserActionsController())->reserve($t->request('/user/reserve', [ - 'libro_id' => $b['id'], 'desired_date' => $date, 'end_date' => $end, - ]), new Response(), $t->db); - $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); - SettingsReview25::eq($row, [$open, $end], 'Reader hold is persisted on the usable window'); - }; + if ($name !== 'inactive holiday') { + $cases["explicit reader queue follows {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { + [$b, $date, $open] = $fixture($t); + $_SESSION['user']['id'] = $t->reader; + $_SESSION['user']['tipo_utente'] = 'standard'; + $end = $t->day(4, $date); + (new UserActionsController())->reserve($t->request('/user/reserve', [ + 'libro_id' => $b['id'], 'desired_date' => $date, 'end_date' => $end, + ]), new Response(), $t->db); + $row = $t->db->query("SELECT data_inizio_richiesta,data_fine_richiesta FROM prenotazioni WHERE libro_id={$b['id']}")->fetch_row(); + SettingsReview25::eq($row, [$open, $end], 'Reader hold is persisted on the usable window'); + }; + } $cases["an existing queue still holds capacity through {$name}"] = static function (SettingsReview25 $t) use ($fixture): void { [$b, $date, $open] = $fixture($t); $id = matrixReserve($t, $b, $t->reader, $t->day(3), $date); @@ -294,15 +296,16 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void $row = $t->db->query("SELECT stato,data_scadenza,pickup_deadline FROM prestiti WHERE id={$loan}")->fetch_row(); SettingsReview25::eq($row, ['da_ritirare', $t->today, $t->today], 'Both deadlines are capped to the reopening day'); }; -$cases['staff can confirm an unreconciled pickup on the effective deadline'] = static function (SettingsReview25 $t): void { +$cases['staff can confirm when both unreconciled deadlines fall in the same closure'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); - $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(4), 'da_ritirare', pickup: $t->day(-1)); - matrixHoliday($t, $t->day(-1), 1, $t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(-1), 'da_ritirare', pickup: $t->day(-2)); + matrixHoliday($t, $t->day(-2), 2, $t->branchA); matrixCalendarEnd((new IcsGenerator($t->db))->generate(), 'loan-' . $loan . '@pinakes', $t->day(1)); matrixCalendarEnd((new UserLoanCalendar($t->db))->generate($t->reader, 'Test'), 'loan-' . $loan . '@pinakes', $t->day(1)); $response = (new LoanApprovalController())->confirmPickup($t->request('/admin/loans/pickup', ['loan_id' => $loan]), new Response(), $t->db); SettingsReview25::eq($response->getStatusCode(), 200, 'Reopening is still a valid pickup day before the sweep'); SettingsReview25::eq(matrixState($t, $loan), 'in_corso', 'Pickup starts the loan'); + SettingsReview25::eq($t->due($loan), $t->today, 'The confirmed loan stores the safe return day'); }; $cases['a closed pickup cannot outlive an unchanged open return deadline'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); @@ -353,8 +356,8 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void foreach (['staff', 'reader'] as $actor) { $cases["{$actor} cancellation on reopening is voluntary, not an expiry"] = static function (SettingsReview25 $t) use ($actor): void { $b = $t->book($t->branchA); - $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(4), 'da_ritirare', pickup: $t->day(-1)); - matrixHoliday($t, $t->day(-1), 1, $t->branchA); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-5), $t->day(-1), 'da_ritirare', pickup: $t->day(-2)); + matrixHoliday($t, $t->day(-2), 2, $t->branchA); $request = $t->request('/cancel', ['loan_id' => $loan]); if ($actor === 'staff') { (new LoanApprovalController())->cancelPickup($request, new Response(), $t->db); @@ -396,6 +399,16 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::eq(LibraryCalendar::reservationDeadline($t->db, $day . ' 18:00:00', $b['id']), $firstOpen . ' 23:59:59', 'A deadline extends until one branch can accept it'); }; +$cases['a pinned booking tries a sibling when the original copy is promised on reopening'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $sibling = matrixCopy($t, $b, $t->branchB); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(4)); + $t->loan($b, $t->day(8), ['copy' => $b['copies'][0], 'reader' => $t->secondReader, 'start' => $t->day(5), 'status' => 'prenotato']); + matrixHoliday($t, $t->day(4), 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 1, 'A compatible sibling keeps the pickup valid'); + SettingsReview25::eq((int) $t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), $sibling, 'The previously pinned copy is protected for the next reader'); +}; + $passed = $failed = 0; $suite = null; $savedSession = $_SESSION ?? []; From 2727507afb97f71ea1afda92c1857349859ab78a Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 16:47:55 +0100 Subject: [PATCH 5/9] Reject manual pickup while its branch is closed --- app/Controllers/LoanApprovalController.php | 12 +++++++++++- app/Services/CapacityService.php | 4 ++-- app/Support/LibraryCalendar.php | 2 +- locale/da_DK.json | 1 + locale/de_DE.json | 1 + locale/en_US.json | 1 + locale/fr_FR.json | 1 + locale/it_IT.json | 1 + tests/reservation-closures-100.unit.php | 2 ++ 9 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 132d29bce..c1c66a647 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -1025,9 +1025,19 @@ public function confirmPickup(Request $request, Response $response, mysqli $db): return $response->withHeader('Content-Type', 'application/json')->withStatus(400); } + $copiaId = (int) $loan['copia_id']; + $calendar = \App\Support\LibraryCalendar::forDb($db); + if ($calendar->hasRules() && !$calendar->isOpen($today, \App\Support\LibraryCalendar::branchOfCopy($db, $copiaId))) { + $db->rollback(); + $response->getBody()->write(json_encode([ + 'success' => false, + 'message' => __('La sede è chiusa oggi. Riprova il prossimo giorno di apertura.'), + ])); + return $response->withHeader('Content-Type', 'application/json')->withStatus(400); + } + // A closure may have been imported before the maintenance pass // had a chance to persist the new pickup deadline. - $copiaId = (int) $loan['copia_id']; $pickupDeadline = \App\Support\LibraryCalendar::effectivePickupDeadline($db, $loan['pickup_deadline'], $loan['data_scadenza'], $copiaId); if ($pickupDeadline !== null && $today > $pickupDeadline) { $db->rollback(); diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index 5d2562d6f..55281ac51 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -484,8 +484,8 @@ private function activeReservationIntervals(int $libroId, string $start, string // Start falls back to the reservation deadline for legacy 'attiva' rows whose // data_inizio_richiesta is NULL (nullable column). For normal rows the COALESCE // returns data_inizio_richiesta unchanged, so this is behaviour-preserving; it - // only stops such legacy holds from silently vanishing from the occupancy peak - // (they never promote either, so they'd otherwise allow overbooking their copy). + // stops such legacy holds from vanishing from the occupancy peak while + // promotion uses the same fallback day. $rStart = 'COALESCE(r.data_inizio_richiesta, DATE(r.data_scadenza_prenotazione))'; // A reservation belonging to a currently ineligible patron is paused, // not allowed to consume scarce capacity and starve eligible readers. diff --git a/app/Support/LibraryCalendar.php b/app/Support/LibraryCalendar.php index 3b69f6db7..a70c94941 100644 --- a/app/Support/LibraryCalendar.php +++ b/app/Support/LibraryCalendar.php @@ -959,7 +959,7 @@ public static function extendPickupDeadlines(\mysqli $db): int return $moved; } - /** A closed pickup day can move forward, within the stored loan period. */ + /** A closed pickup day can move forward, up to the loan's return day. */ public function pickupDeadlineOnReopening(string $deadline, ?string $dueDate, ?int $branch, bool $useEffectiveDue = true): string { $next = $this->nextOpenDay($deadline, $branch); diff --git a/locale/da_DK.json b/locale/da_DK.json index 7b67798ce..c98943742 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -2352,6 +2352,7 @@ "Il sito web deve essere un URL valido (es. https://www.esempio.com).": "Webstedet skal være en gyldig URL (f.eks. [https://www.eksempel.dk](https://www.eksempel.dk)).", "Il sito web deve essere un URL valido.": "Hjemmesiden skal være en gyldig URL.", "Il termine per il ritiro è scaduto": "Afhentningsfristen er udløbet", + "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.": "Filialen er lukket i dag. Prøv igen næste åbningsdag.", "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?": "Afhentningsfristen er udløbet. Vil du annullere dette udlån?", "Il titolo del libro è obbligatorio.": "Bogens titel er påkrævet.", "Il titolo principale della pagina": "Sidens hovedtitel", diff --git a/locale/de_DE.json b/locale/de_DE.json index b16dbd007..4bf28adc1 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -2352,6 +2352,7 @@ "Il sito web deve essere un URL valido (es. https://www.esempio.com).": "Die Website muss eine gültige URL sein (z.B. https://www.beispiel.de).", "Il sito web deve essere un URL valido.": "Die Website muss eine gültige URL sein.", "Il termine per il ritiro è scaduto": "Die Abholfrist ist abgelaufen", + "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.": "Die Zweigstelle ist heute geschlossen. Versuchen Sie es am nächsten Öffnungstag erneut.", "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?": "Die Abholfrist ist abgelaufen. Möchten Sie diese Ausleihe stornieren?", "Il titolo del libro è obbligatorio.": "Der Buchtitel ist erforderlich.", "Il titolo principale della pagina": "Der Haupttitel der Seite", diff --git a/locale/en_US.json b/locale/en_US.json index 9f7b5801e..febcd8b7f 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -2352,6 +2352,7 @@ "Il sito web deve essere un URL valido (es. https://www.esempio.com).": "Website must be a valid URL (e.g. https://www.example.com).", "Il sito web deve essere un URL valido.": "Website must be a valid URL.", "Il termine per il ritiro è scaduto": "The pickup deadline has passed", + "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.": "The branch is closed today. Try again on the next open day.", "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?": "The pickup deadline has passed. Do you want to cancel this loan?", "Il titolo del libro è obbligatorio.": "The book title is required.", "Il titolo principale della pagina": "The main page title", diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 04c9828a5..2f5c06daf 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -2352,6 +2352,7 @@ "Il sito web deve essere un URL valido (es. https://www.esempio.com).": "Le site web doit être une URL valide (ex. : https://www.exemple.com).", "Il sito web deve essere un URL valido.": "Le site web doit être une URL valide.", "Il termine per il ritiro è scaduto": "Le délai de récupération est dépassé", + "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.": "Le site est fermé aujourd’hui. Réessayez le prochain jour d’ouverture.", "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?": "Le délai de récupération est dépassé. Voulez-vous annuler cet emprunt ?", "Il titolo del libro è obbligatorio.": "Le titre du livre est obligatoire.", "Il titolo principale della pagina": "Le titre principal de la page", diff --git a/locale/it_IT.json b/locale/it_IT.json index ad4b3e647..52721bc43 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -2352,6 +2352,7 @@ "Il sito web deve essere un URL valido (es. https://www.esempio.com).": "Il sito web deve essere un URL valido (es. https://www.esempio.com).", "Il sito web deve essere un URL valido.": "Il sito web deve essere un URL valido.", "Il termine per il ritiro è scaduto": "Il termine per il ritiro è scaduto", + "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.": "La sede è chiusa oggi. Riprova il prossimo giorno di apertura.", "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?": "Il termine per il ritiro è scaduto. Vuoi annullare questo prestito?", "Il titolo del libro è obbligatorio.": "Il titolo del libro è obbligatorio.", "Il titolo principale della pagina": "Il titolo principale della pagina", diff --git a/tests/reservation-closures-100.unit.php b/tests/reservation-closures-100.unit.php index f7c83151c..96d51aef9 100644 --- a/tests/reservation-closures-100.unit.php +++ b/tests/reservation-closures-100.unit.php @@ -278,6 +278,8 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(5)); matrixHoliday($t, $t->today, 1, $t->branchA); SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 0, 'The announced branch is still closed'); + $response = (new LoanApprovalController())->confirmPickup($t->request('/admin/loans/pickup', ['loan_id' => $loan]), new Response(), $t->db); + SettingsReview25::eq($response->getStatusCode(), 400, 'Direct staff confirmation also respects the assigned branch closure'); SettingsReview25::eq(matrixState($t, $loan), 'prenotato', 'An open sibling does not silently move an assigned pickup'); }; $cases['an unassigned scheduled pickup chooses an open branch'] = static function (SettingsReview25 $t): void { From 9dd45b1ca9be6fb4287ec51a0ec2a3f772fa91d0 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 7 Nov 2026 17:05:01 +0100 Subject: [PATCH 6/9] Address review of reservation and holiday edge cases --- .../Admin/LibraryCalendarController.php | 18 +- app/Controllers/LoanApprovalController.php | 14 +- app/Controllers/ReservationManager.php | 65 ++++--- app/Services/CapacityService.php | 93 ++++++++-- .../ReservationReassignmentService.php | 20 +++ app/Support/DataIntegrity.php | 164 ++++++++---------- app/Support/IcsGenerator.php | 2 +- app/Support/LibraryCalendar.php | 47 ++++- tests/mvcc-lockfirst-circulation.unit.php | 2 +- tests/reservation-calendar-edgecases.unit.php | 73 +++++++- tests/reservation-closures-100.unit.php | 31 +++- 11 files changed, 380 insertions(+), 149 deletions(-) diff --git a/app/Controllers/Admin/LibraryCalendarController.php b/app/Controllers/Admin/LibraryCalendarController.php index 863a94bea..62496c612 100644 --- a/app/Controllers/Admin/LibraryCalendarController.php +++ b/app/Controllers/Admin/LibraryCalendarController.php @@ -684,7 +684,7 @@ private function lockName(\mysqli $db): string * * @param array{start: string, end: string, description: string, yearly: bool, sede: ?int} $row * @param list|null $closedWeekdays - * @param array{rows: list>, weekdays: list, holidays: list>, years: list, open: array}|null $state + * @param array{rows: list>, calendar: LibraryCalendar, holidays: list>, years: list, open: array}|null $state */ private function yearCoverageError(\mysqli $db, array $row, ?array $closedWeekdays = null, ?array &$state = null): ?string { @@ -697,8 +697,10 @@ private function yearCoverageError(\mysqli $db, array $row, ?array $closedWeekda // exists from the 0.9.0 migration on. $hasActive = $db->query("SHOW COLUMNS FROM calendario_chiusure LIKE 'attiva'"); $activeOnly = ($hasActive instanceof \mysqli_result && $hasActive->num_rows > 0) ? ' AND attiva = 1' : ''; - $result = $db->query('SELECT data_inizio, data_fine, ricorrente_annuale, sede_id FROM calendario_chiusure WHERE ricorrente_annuale = 1' . $activeOnly); - $holidays = array_merge(HolidayPresets::calendarRules($db) ?? [], CustomHolidays::calendarRules($db)); + $presets = HolidayPresets::calendarRules($db); + $manualOnly = $presets !== null && HolidayPresets::schemaReady($db) ? ' AND regola_festivita IS NULL' : ''; + $result = $db->query('SELECT data_inizio, data_fine, ricorrente_annuale, sede_id FROM calendario_chiusure WHERE ricorrente_annuale = 1' . $activeOnly . $manualOnly); + $holidays = array_merge($presets ?? [], CustomHolidays::calendarRules($db)); $currentYear = (int) substr(DateHelper::today(), 0, 4); $firstYear = min(1900, $currentYear - 1); $years = array_fill_keys(range($firstYear, min(9999, max($firstYear + 399, $currentYear + 27))), true); @@ -714,9 +716,11 @@ private function yearCoverageError(\mysqli $db, array $row, ?array $closedWeekda } } ksort($years); + $rows = $result instanceof \mysqli_result ? $result->fetch_all(MYSQLI_ASSOC) : []; + $weekdays = $closedWeekdays ?? LibraryCalendar::parseWeekdays((new SettingsRepository($db))->get('loans', 'closed_weekdays', '')); $next = [ - 'rows' => $result instanceof \mysqli_result ? $result->fetch_all(MYSQLI_ASSOC) : [], - 'weekdays' => $closedWeekdays ?? LibraryCalendar::parseWeekdays((new SettingsRepository($db))->get('loans', 'closed_weekdays', '')), + 'rows' => $rows, + 'calendar' => new LibraryCalendar($weekdays, $rows, $holidays), 'holidays' => $holidays, 'years' => array_keys($years), 'open' => [], @@ -725,10 +729,11 @@ private function yearCoverageError(\mysqli $db, array $row, ?array $closedWeekda $next = $state; $stored = ['data_inizio' => $row['start'], 'data_fine' => $row['end'], 'ricorrente_annuale' => 1, 'sede_id' => $row['sede']]; $next['rows'][] = $stored; + $next['calendar'] = $state['calendar']->withAdditionalClosure($stored); // The new row alone: does it close a day found open before? $added = new LibraryCalendar([], [$stored]); } - $calendar = null; + $calendar = $next['calendar']; $branches = [$row['sede']]; if ($row['sede'] === null) { foreach ($next['rows'] as $stored) { @@ -752,7 +757,6 @@ private function yearCoverageError(\mysqli $db, array $row, ?array $closedWeekda } $day = LibraryCalendar::shift($next['open'][$key], 1); } - $calendar ??= new LibraryCalendar($next['weekdays'], $next['rows'], $next['holidays']); $open = $calendar->firstOpenDayBetween($day, $year . '-12-31', $branch); if ($open === null) { return __('Con questa chiusura annuale la biblioteca resterebbe chiusa tutto l\'anno: almeno un giorno deve restare aperto.'); diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index c1c66a647..044965700 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -266,9 +266,11 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // diventerebbero ciechi ai prestiti correnti. $calendar = \App\Support\LibraryCalendar::forDb($db); $capacity = new \App\Services\CapacityService($db, lockingReads: true); - $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::reservationBranch($db, $libroId, $existingCopiaId) : null; + // The preliminary check uses the earliest return day across the + // book's branches. A pinned copy may fail and have to be replaced. + $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::requestScope($db, $libroId, lockingRead: true) : null; $usableDue = $dataScadenza !== null ? $calendar->nextOpenDay($dataScadenza, $branch) : null; - if ($usableDue !== null && $usableDue !== '' && $usableDue < $today) { + if (!$calendar->hasRules() && $usableDue !== null && $usableDue !== '' && $usableDue < $today) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -412,7 +414,11 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // end, not only the requested window: a fully occupied earlier day // means this loan would depend on a preceding loan/reservation being // collected and returned on time, which is the bypass fixed by #384. - if (!$capacity->hasFreeCapacity($libroId, $today, $usableDue, excludePrestitoId: $loanId)) { + // An open sibling can make the earliest effective due date + // fall in the past while another copy can still be collected + // today. The per-copy check handles its actual due date below. + $minimumDue = max($today, (string) $usableDue); + if (!$capacity->hasFreeCapacity($libroId, $today, $minimumDue, excludePrestitoId: $loanId)) { $db->rollback(); // Name the longest loan still possible from today when a // later commitment is what blocks the approval, so staff @@ -422,7 +428,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // return), but the loan itself starts on data_prestito: // the figures count from there, and a conflict before it // cannot be fixed by shortening, so no figures then. - $blocked = array_filter([$capacity->firstUnavailableDateInRange($libroId, $today, $usableDue, null, $loanId)]); + $blocked = array_filter([$capacity->firstUnavailableDateInRange($libroId, $today, $minimumDue, null, $loanId)]); $cap = ($blocked !== [] && min($blocked) > $dataPrestito) ? \App\Support\LoanWindow::cap($dataPrestito, $blocked) : null; diff --git a/app/Controllers/ReservationManager.php b/app/Controllers/ReservationManager.php index 1d187d84c..ec1166a09 100644 --- a/app/Controllers/ReservationManager.php +++ b/app/Controllers/ReservationManager.php @@ -145,9 +145,8 @@ private function rollbackIfOwned(bool $ownTransaction): void /** * Process reservations when a book becomes available * - * Finds the next date-eligible reservation (where start date <= today) - * and attempts to convert it to a pending loan. Only processes one - * reservation at a time to maintain queue order. + * Converts the first date-eligible reservation whose period can be honored. + * Earlier reservations retain their claim on capacity while waiting. * * Uses row-level locking (SELECT FOR UPDATE) to prevent race conditions * where multiple processes try to convert the same reservation. @@ -163,6 +162,7 @@ public function processBookAvailability($bookId) $circulation = new \App\Services\CirculationPolicy($this->db); $limitSql = $circulation->totalLoanLimitSql('u'); $ownTransaction = $this->beginTransactionIfNeeded(); + $candidateStmt = null; try { // Lock the book row first to serialize all reservation processing for this book @@ -210,7 +210,7 @@ public function processBookAvailability($bookId) $materialRefusedSql = $materialRefused === [] ? '' : ' AND r.utente_id NOT IN (' . implode(',', array_map('intval', $materialRefused)) . ')'; - $stmt = $this->db->prepare(" + $candidateStmt = $this->db->prepare(" SELECT r.*, u.email, u.nome, u.cognome FROM prenotazioni r JOIN utenti u ON r.utente_id = u.id @@ -224,18 +224,27 @@ public function processBookAvailability($bookId) AND cap.attivo = 1 AND cap.stato IN (" . \App\Services\CirculationPolicy::ACTIVE_LOAN_STATES_SQL . ") ) < {$limitSql}) - ORDER BY r.queue_position ASC + AND ((? IS NULL AND r.queue_position IS NOT NULL) + OR r.queue_position > ? OR (r.queue_position <=> ? AND r.id > ?)) + ORDER BY r.queue_position ASC, r.id ASC LIMIT 1 FOR UPDATE "); $legacyFloor = \App\Support\LibraryCalendar::legacyReservationDeadlineFloor($this->db, $today, (int) $bookId); - $stmt->bind_param('isss', $bookId, $today, $legacyFloor, $today); - $stmt->execute(); - $result = $stmt->get_result(); - $nextReservation = $result ? $result->fetch_assoc() : null; - $stmt->close(); - - if ($nextReservation !== null) { + $position = null; + $reservationId = 0; + $candidateStmt->bind_param('isssiiii', $bookId, $today, $legacyFloor, $today, $position, $position, $position, $reservationId); + + while (true) { + $candidateStmt->execute(); + $result = $candidateStmt->get_result(); + $nextReservation = $result->fetch_assoc(); + $result->free(); + if ($nextReservation === null) { + break; + } + $position = isset($nextReservation['queue_position']) ? (int) $nextReservation['queue_position'] : null; + $reservationId = (int) $nextReservation['id']; $promotedUserId = (int) $nextReservation['utente_id']; $userLock = $this->db->prepare('SELECT id FROM utenti WHERE id = ? FOR UPDATE'); $userLock->bind_param('i', $promotedUserId); @@ -252,7 +261,7 @@ public function processBookAvailability($bookId) 'utente_id' => (int) $nextReservation['utente_id'], 'motivo' => $eligibilityError, ]); - $nextReservation = null; + continue; } else { // The SQL predicate avoids head-of-line blocking; this // second check under the user lock closes the race with @@ -262,12 +271,9 @@ public function processBookAvailability($bookId) $promotedUserId ); if ($limitError !== null) { - $nextReservation = null; + continue; } } - } - - if ($nextReservation) { // Check if the desired date range is available. Resolve the canonical // R_END once: a legacy prenotazione may have data_fine_richiesta NULL but // data_scadenza_prenotazione set — passing the raw NULL would make @@ -285,8 +291,7 @@ public function processBookAvailability($bookId) // reopening. Start the resulting loan on that usable day. $startDate = \App\Support\LibraryCalendar::requestStartDay($this->db, $startDate, (int) $bookId); if ($startDate > $today) { - $this->commitIfOwned($ownTransaction); - return false; + continue; } } // Feed both resolved bounds to createLoanFromReservation() too. @@ -309,16 +314,15 @@ public function processBookAvailability($bookId) excludeReservationsAfterQueuePos: $headQueuePos, excludeQueueTailStartingBy: $storedEndDate )) { - // The head stays 'attiva' and blocks the queue until staff - // resolve it (the integrity audit lists it): say why. + // Leave this hold intact and consider later readers whose + // dates fit around it and all other existing commitments. \App\Support\SecureLogger::info('Promozione prenotazione rinviata: la chiusura allunga il periodo su una promessa già fatta', [ 'prenotazione_id' => (int) $nextReservation['id'], 'libro_id' => $bookId, 'fine_richiesta' => $storedEndDate, 'fine_effettiva' => $endDate, ]); - $this->commitIfOwned($ownTransaction); - return false; + continue; } // #157: pass the promoted reservation's queue_position so the @@ -353,9 +357,8 @@ public function processBookAvailability($bookId) // when called standalone, but here we're already in a transaction $loanCreated = $this->createLoanFromReservation($nextReservation, $storedEndDate); - if ($loanCreated === false) { - // Race condition detected - loan creation failed. Restore - // the claim first: with an EXTERNAL transaction we cannot + if ($loanCreated === null || $loanCreated === false) { + // Restore the claim first: with an EXTERNAL transaction we cannot // roll back the owner's work, and leaving the row // 'completata' without its loan would be committed by the // caller (reservation lost). We still hold the row lock, @@ -364,6 +367,10 @@ public function processBookAvailability($bookId) $unclaim->bind_param('i', $nextReservation['id']); $unclaim->execute(); $unclaim->close(); + if ($loanCreated === null) { + // No copy can serve this reader; no loan was written. + continue; + } $this->rollbackIfOwned($ownTransaction); return false; } @@ -441,6 +448,8 @@ public function processBookAvailability($bookId) } \App\Support\SecureLogger::error('processBookAvailability error', ['error' => $e->getMessage()]); return false; + } finally { + $candidateStmt?->close(); } } @@ -486,7 +495,7 @@ private function isDateRangeAvailable($bookId, $startDate, $endDate, ?int $exclu * The loan requires admin confirmation of physical book pickup. * * @param array{id?: int|string, libro_id: int, utente_id: int, data_inizio_richiesta: string, data_fine_richiesta: string, queue_position?: int|string|null} $reservation Reservation data - * @return int|false Loan ID on success, false on failure (no copies available or race condition) + * @return int|false|null Loan ID on success, null if no copy fits, false on write failure. */ private function createLoanFromReservation($reservation, ?string $storedEndDate = null) { @@ -649,7 +658,7 @@ private function createLoanFromReservation($reservation, ?string $storedEndDate } if ($copyId === null) { $this->rollbackIfOwned($ownTransaction); - return false; + return null; } // Create loan with copia_id and origine='prenotazione' diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index 55281ac51..10509b53d 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -184,14 +184,28 @@ public function firstAvailableDate(int $libroId, string $from): ?string } $calendar = \App\Support\LibraryCalendar::forDb($this->db); $branch = $calendar->hasRules() ? \App\Support\LibraryCalendar::requestScope($this->db, $libroId) : null; + $copyBranches = $calendar->hasRules() ? $this->mixedLendingCopyBranches($libroId) : []; + $copyIntervals = []; + $trackCopy = $copyBranches === [] ? null : static function (int $copyId, string $start, string $end) use (&$copyIntervals): void { + $copyIntervals[$copyId][] = [$start, $end]; + }; // Keep one day available for finite end+1 events. Any interval clamped // to this horizon is effectively open-ended for application purposes. $horizon = '9999-12-30'; $intervals = array_merge( - $this->holdingLoanIntervals($libroId, $from, $horizon, null, null), + $this->holdingLoanIntervals($libroId, $from, $horizon, null, null, onCopyInterval: $trackCopy), $this->activeReservationIntervals($libroId, $from, $horizon, null, null) ); + foreach ($copyIntervals as &$held) { + sort($held); + } + unset($held); + $firstOpen = static function (string $start, string $end) use ($calendar, $branch, $copyBranches, $copyIntervals): ?string { + return $copyBranches === [] + ? $calendar->firstOpenDayBetween($start, $end, $branch) + : self::firstFreeOpenCopyDay($calendar, $start, $end, $copyBranches, $copyIntervals); + }; /** @var array $events */ $events = []; foreach ($intervals as [$start, $end]) { @@ -209,7 +223,7 @@ public function firstAvailableDate(int $libroId, string $from): ?string // Search only this free segment. If it is entirely closed, the // first open day could already belong to the next borrower. if ($date > $cursor && $occupied < $total) { - $open = $calendar->firstOpenDayBetween($cursor, \App\Support\LibraryCalendar::shift($date, -1), $branch); + $open = $firstOpen($cursor, \App\Support\LibraryCalendar::shift($date, -1)); if ($open !== null) { return $open; } @@ -219,7 +233,55 @@ public function firstAvailableDate(int $libroId, string $from): ?string } // Remaining occupants have no known release date. - return $occupied < $total ? $calendar->firstOpenDayBetween($cursor, '9999-12-31', $branch) : null; + return $occupied < $total ? $firstOpen($cursor, $horizon) : null; + } + + /** @return array physical copies in titles with more than one branch calendar */ + private function mixedLendingCopyBranches(int $bookId): array + { + $lendable = ($this->circulation ??= new CirculationPolicy($this->db))->lendableCopySql(''); + $stmt = $this->db->prepare("SELECT id, sede_id FROM copie WHERE libro_id = ? AND {$lendable} + AND stato NOT IN ('perso','danneggiato','manutenzione','in_restauro','in_trasferimento')"); + $stmt->bind_param('i', $bookId); + $stmt->execute(); + $rows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC); + $stmt->close(); + $copies = []; + foreach ($rows as $row) { + $copies[(int) $row['id']] = $row['sede_id'] === null ? null : (int) $row['sede_id']; + } + return count(array_unique($copies, SORT_REGULAR)) > 1 ? $copies : []; + } + + /** + * @param array $branches + * @param array> $intervals + */ + private static function firstFreeOpenCopyDay(\App\Support\LibraryCalendar $calendar, string $start, string $end, array $branches, array $intervals): ?string + { + $first = null; + foreach ($branches as $copyId => $branch) { + $cursor = $start; + $last = $first === null ? $end : \App\Support\LibraryCalendar::shift($first, -1); + foreach ($intervals[$copyId] ?? [] as [$heldFrom, $heldThrough]) { + if ($heldThrough < $cursor) { + continue; + } + if ($heldFrom > $cursor) { + $open = $calendar->firstOpenDayBetween($cursor, min($last, \App\Support\LibraryCalendar::shift($heldFrom, -1)), $branch); + if ($open !== null) { + $first = $open; + continue 2; + } + } + if ($heldThrough >= $last) { + continue 2; + } + $cursor = \App\Support\LibraryCalendar::shift($heldThrough, 1); + } + $first = $calendar->firstOpenDayBetween($cursor, $last, $branch) ?? $first; + } + return $first; } /** @@ -388,9 +450,10 @@ private function enumerateDays(string $start, string $end): array /** * HOLDING loan intervals overlapping [$start,$end], clamped to the window. + * @param (callable(int, string, string): void)|null $onCopyInterval * @return list */ - private function holdingLoanIntervals(int $libroId, string $start, string $end, ?int $excludePrestitoId, ?int $excludeUserId, ?int $copyId = null): array + private function holdingLoanIntervals(int $libroId, string $start, string $end, ?int $excludePrestitoId, ?int $excludeUserId, ?int $copyId = null, ?callable $onCopyInterval = null): array { // An unreturned overdue loan has no known end yet. Its contractual due // date is in the past, but the physical copy remains out of the library: @@ -401,13 +464,17 @@ private function holdingLoanIntervals(int $libroId, string $start, string $end, // same unreturned copy — treat it exactly like 'in_ritardo' (one branch, // one clamp, never both on the same row: the OR selects a single case). $today = \App\Support\DateHelper::today(); - $openEnded = "(p.attivo = 1 AND (p.stato = 'in_ritardo' OR (p.stato = 'in_corso' AND p.data_scadenza < ?)))"; + $hasCalendar = \App\Support\LibraryCalendar::forDb($this->db)->hasRules(); + $useRawDue = $hasCalendar ? '0' : '1'; + $openEnded = "(p.attivo = 1 AND (p.stato = 'in_ritardo' OR ({$useRawDue} = 1 AND p.stato = 'in_corso' AND p.data_scadenza < ?)))"; $sql = "SELECT GREATEST(p.data_prestito, ?) AS s, LEAST(CASE WHEN {$openEnded} THEN ? ELSE p.data_scadenza END, ?) AS e, IF(p.attivo = 1, p.data_scadenza, NULL) AS stored_due, + p.stato AS loan_state, + p.libro_id AS loan_book, p.copia_id AS stored_copy FROM prestiti p WHERE p.libro_id = ? @@ -419,7 +486,6 @@ private function holdingLoanIntervals(int $libroId, string $start, string $end, // A loan whose stored day is closed ends on the next open day, which the // reader is told, even when the move was refused and the row kept its // date. With a calendar, a stored day before the window can still reach it. - $hasCalendar = \App\Support\LibraryCalendar::forDb($this->db)->hasRules(); $params = [$start, $today, $end, $end, $libroId, $end, $today, $hasCalendar ? '1000-01-01' : $start]; if ($excludePrestitoId !== null) { $sql .= ' AND p.id <> ?'; @@ -436,7 +502,7 @@ private function holdingLoanIntervals(int $libroId, string $start, string $end, $types .= 'i'; $params[] = $copyId; } - return $this->fetchIntervals($sql, $types, $params, $end); + return $this->fetchIntervals($sql, $types, $params, $end, today: $today, onCopyInterval: $onCopyInterval); } /** Whether this physical copy has a commitment in the inclusive window. */ @@ -613,9 +679,10 @@ private function reachesLoanLimit(int $userId, int $limit, string $start, string /** * @param string $types * @param list $params + * @param (callable(int, string, string): void)|null $onCopyInterval * @return list */ - private function fetchIntervals(string $sql, string $types, array $params, ?string $windowEnd = null, ?int $reservationBookId = null, ?string $today = null): array + private function fetchIntervals(string $sql, string $types, array $params, ?string $windowEnd = null, ?int $reservationBookId = null, ?string $today = null, ?callable $onCopyInterval = null): array { if ($this->lockingReads) { $sql .= ' FOR UPDATE'; @@ -637,9 +704,12 @@ private function fetchIntervals(string $sql, string $types, array $params, ?stri $s = (string) $row['s']; $e = (string) $row['e']; $storedDue = (string) ($row['stored_due'] ?? ''); - if ($windowEnd !== null && $storedDue !== '' && $e === $storedDue) { + if ($windowEnd !== null && $storedDue !== '' && $calendar->hasRules() && $row['loan_state'] !== 'in_ritardo') { $copyId = $row['stored_copy'] !== null ? (int) $row['stored_copy'] : null; - $e = min(\App\Support\LibraryCalendar::holdingEnd($this->db, $storedDue, $copyId), $windowEnd); + $scheduledBookId = $row['loan_state'] === 'prenotato' ? (int) $row['loan_book'] : null; + $e = $row['loan_state'] === 'in_corso' && $today !== null && \App\Support\LibraryCalendar::isPastDue($this->db, $storedDue, $copyId, $today) + ? $windowEnd + : min(\App\Support\LibraryCalendar::holdingEnd($this->db, $storedDue, $copyId, $scheduledBookId, $this->lockingReads), $windowEnd); } if ($windowEnd !== null && isset($row['reservation_due'])) { $effectiveEnd = $calendar->nextOpenDay((string) $row['reservation_due'], $branch); @@ -656,6 +726,9 @@ private function fetchIntervals(string $sql, string $types, array $params, ?stri } if ($s !== '' && $e !== '' && $s <= $e) { $out[] = [$s, $e]; + if ($onCopyInterval !== null && isset($row['stored_copy'])) { + $onCopyInterval((int) $row['stored_copy'], $s, $e); + } } } $stmt->close(); diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index fae8de1e1..3cb871597 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -603,6 +603,26 @@ private function handleNoCopyAvailable(int $reservationId, int $lostCopyId, stri $lockBook->execute(); $lockBook->close(); + $current = $this->db->prepare('SELECT data_scadenza FROM prestiti WHERE id = ? AND libro_id = ? AND copia_id = ? FOR UPDATE'); + $current->bind_param('iii', $reservationId, $libroId, $lostCopyId); + $current->execute(); + $storedEnd = $current->get_result()->fetch_row()[0] ?? null; + $current->close(); + if ($storedEnd === null) { + $this->rollbackIfOwned($ownTransaction); + return false; + } + if ($promisedEnd > $storedEnd) { + $capacity = new CapacityService($this->db, lockingReads: true); + // On a same-day handover the reopening day was never occupied + // by this hold. Storing the adjusted date as an inclusive raw + // end would reserve that day even though no copy was assigned. + if ((new \App\Models\SettingsRepository($this->db))->allowsSameDayHandover() + || !$capacity->hasFreeCapacity($libroId, \App\Support\LibraryCalendar::shift((string) $storedEnd, 1), $promisedEnd, excludePrestitoId: $reservationId)) { + $promisedEnd = (string) $storedEnd; + } + } + $stmt = $this->db->prepare(" UPDATE prestiti SET copia_id = NULL, data_scadenza = GREATEST(data_scadenza, ?), diff --git a/app/Support/DataIntegrity.php b/app/Support/DataIntegrity.php index 6596131b3..14fb22ea8 100644 --- a/app/Support/DataIntegrity.php +++ b/app/Support/DataIntegrity.php @@ -72,7 +72,8 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): $stmt->execute(); $stmt->close(); - $reservationStillHolds = $this->reservationEndTodaySql($today); + $scheduledStillHolds = $this->scheduledLoanEndTodaySql($today); + $unassignedHoldCount = $this->unassignedHoldCountTodaySql($today, $scheduledStillHolds); // Ricalcola copie_disponibili e stato per tutti i libri non soft-deleted // Conta le copie fisicamente disponibili OGGI: // - Include copie 'disponibile' e 'prenotato' (fisicamente in biblioteca) @@ -93,7 +94,7 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): AND ( ( p.attivo = 1 AND ( p.stato IN ('in_corso', 'in_ritardo', 'da_ritirare') - OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND p.data_scadenza >= '{$today}') + OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND {$scheduledStillHolds}) ) ) OR ( p.attivo = 0 AND p.stato = 'pendente' AND p.copia_id IS NOT NULL ) ) @@ -101,15 +102,7 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): AND c.stato IN ('disponibile', 'prenotato') AND {$lendable} AND p.id IS NULL - ) - ( - SELECT COUNT(*) - FROM prenotazioni pr - WHERE pr.libro_id = l.id - AND pr.stato = 'attiva' - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) IS NOT NULL - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) <= '{$today}' - AND {$reservationStillHolds} - ), + ) - {$unassignedHoldCount}, 0 ), copie_totali = ( @@ -127,7 +120,7 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): AND ( ( p.attivo = 1 AND ( p.stato IN ('in_corso', 'in_ritardo', 'da_ritirare') - OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND p.data_scadenza >= '{$today}') + OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND {$scheduledStillHolds}) ) ) OR ( p.attivo = 0 AND p.stato = 'pendente' AND p.copia_id IS NOT NULL ) ) @@ -135,15 +128,7 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): AND c.stato IN ('disponibile', 'prenotato') AND {$lendable} AND p.id IS NULL - ) - ( - SELECT COUNT(*) - FROM prenotazioni pr - WHERE pr.libro_id = l.id - AND pr.stato = 'attiva' - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) IS NOT NULL - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) <= '{$today}' - AND {$reservationStillHolds} - ), + ) - {$unassignedHoldCount}, 0 ) > 0 THEN 'disponibile' WHEN (SELECT COUNT(*) FROM copie c WHERE c.libro_id = l.id AND c.stato = 'prestato') > 0 THEN 'prestato' @@ -215,42 +200,72 @@ public function recalculateAllBookAvailability(bool $insideTransaction = false): return $results; } - /** Extend the counter's SQL predicate only for reservations reaching today after a closure. */ - private function reservationEndTodaySql(string $today, ?int $bookId = null): string + /** A copyless scheduled loan takes a title slot until it receives a copy. */ + private function unassignedHoldCountTodaySql(string $today, string $scheduledStillHolds, ?int $bookId = null): string + { + $reservationCount = $this->reservationCountTodaySql($today, $bookId); + return "({$reservationCount} + (SELECT COUNT(*) FROM prestiti p WHERE p.libro_id = l.id AND p.copia_id IS NULL + AND p.attivo = 1 AND p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND {$scheduledStillHolds}))"; + } + + /** The counter uses the same effective intervals and pauses as the booking gates. */ + private function reservationCountTodaySql(string $today, ?int $bookId = null): string { - $end = 'COALESCE(pr.data_fine_richiesta, DATE(pr.data_scadenza_prenotazione), pr.data_inizio_richiesta)'; - $raw = "{$end} >= '{$today}'"; - $calendar = LibraryCalendar::forDb($this->db); - if (!$calendar->hasRules()) { + $start = 'COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione))'; + if (!LibraryCalendar::forDb($this->db)->hasRules()) { + return "(SELECT COUNT(*) FROM prenotazioni pr WHERE pr.libro_id = l.id AND pr.stato = 'attiva' + AND {$start} <= '{$today}' + AND COALESCE(pr.data_fine_richiesta, DATE(pr.data_scadenza_prenotazione), pr.data_inizio_richiesta) >= '{$today}')"; + } + $capacity = new \App\Services\CapacityService($this->db, lockingReads: true); + if ($bookId !== null) { + return (string) count($capacity->reservationIntervalsInRange($bookId, $today, $today)); + } + + $stmt = $this->db->prepare("SELECT pr.libro_id FROM prenotazioni pr WHERE pr.stato = 'attiva' AND {$start} <= ? FOR UPDATE"); + $stmt->bind_param('s', $today); + $stmt->execute(); + $bookIds = array_unique(array_column($stmt->get_result()->fetch_all(MYSQLI_NUM), 0)); + $stmt->close(); + $counts = []; + foreach ($bookIds as $id) { + $id = (int) $id; + $count = count($capacity->reservationIntervalsInRange($id, $today, $today)); + if ($count > 0) { + $counts[] = "WHEN {$id} THEN {$count}"; + } + } + return $counts === [] ? '0' : '(CASE l.id ' . implode(' ', $counts) . ' ELSE 0 END)'; + } + + /** Scheduled holds still occupied today after their stored due day. */ + private function scheduledLoanEndTodaySql(string $today, ?int $bookId = null): string + { + $raw = "p.data_scadenza >= '{$today}'"; + if (!LibraryCalendar::forDb($this->db)->hasRules()) { return $raw; } - $bookFilter = $bookId === null ? '' : ' AND pr.libro_id = ?'; - $stmt = $this->db->prepare( - "SELECT pr.id, pr.libro_id, {$end} AS due - FROM prenotazioni pr WHERE pr.stato = 'attiva' - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) <= ? - AND {$end} < ?{$bookFilter} FOR UPDATE" - ); + $bookFilter = $bookId === null ? '' : ' AND libro_id = ?'; + $stmt = $this->db->prepare("SELECT id, libro_id, copia_id, data_scadenza FROM prestiti + WHERE attivo = 1 AND stato = 'prenotato' + AND data_prestito <= ? AND data_scadenza < ?{$bookFilter} FOR UPDATE"); if ($bookId === null) { $stmt->bind_param('ss', $today, $today); } else { $stmt->bind_param('ssi', $today, $today, $bookId); } $stmt->execute(); - $rows = $stmt->get_result(); - $branches = []; + $rows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC); + $stmt->close(); + LibraryCalendar::primeCopyBranches($this->db, array_map(static fn (array $r): int => (int) $r['copia_id'], $rows)); $extended = []; - while ($row = $rows->fetch_assoc()) { - $id = (int) $row['libro_id']; - if (!array_key_exists($id, $branches)) { - $branches[$id] = LibraryCalendar::requestScope($this->db, $id, lockingRead: true); - } - if ($calendar->nextOpenDay((string) $row['due'], $branches[$id]) >= $today) { + foreach ($rows as $row) { + $copyId = isset($row['copia_id']) ? (int) $row['copia_id'] : null; + if (LibraryCalendar::holdingEnd($this->db, (string) $row['data_scadenza'], $copyId, (int) $row['libro_id'], lockingRead: true) >= $today) { $extended[] = (int) $row['id']; } } - $stmt->close(); - return $extended === [] ? $raw : "({$raw} OR pr.id IN (" . implode(',', $extended) . '))'; + return $extended === [] ? $raw : "({$raw} OR p.id IN (" . implode(',', $extended) . '))'; } /** @@ -407,7 +422,8 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction $stmt->execute(); $stmt->close(); - $reservationStillHolds = $this->reservationEndTodaySql($today, $bookId); + $scheduledStillHolds = $this->scheduledLoanEndTodaySql($today, $bookId); + $unassignedHoldCount = $this->unassignedHoldCountTodaySql($today, $scheduledStillHolds, $bookId); // Aggiorna copie_disponibili e stato del libro dalla tabella copie // Conta le copie fisicamente disponibili OGGI: // - Include copie 'disponibile' e 'prenotato' (fisicamente in biblioteca) @@ -428,7 +444,7 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction AND ( ( p.attivo = 1 AND ( p.stato IN ('in_corso', 'in_ritardo', 'da_ritirare') - OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND p.data_scadenza >= '{$today}') + OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND {$scheduledStillHolds}) ) ) OR ( p.attivo = 0 AND p.stato = 'pendente' AND p.copia_id IS NOT NULL ) ) @@ -436,15 +452,7 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction AND c.stato IN ('disponibile', 'prenotato') AND {$lendable} AND p.id IS NULL - ) - ( - SELECT COUNT(*) - FROM prenotazioni pr - WHERE pr.libro_id = ? - AND pr.stato = 'attiva' - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) IS NOT NULL - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) <= '{$today}' - AND {$reservationStillHolds} - ), + ) - {$unassignedHoldCount}, 0 ), copie_totali = ( @@ -462,7 +470,7 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction AND ( ( p.attivo = 1 AND ( p.stato IN ('in_corso', 'in_ritardo', 'da_ritirare') - OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND p.data_scadenza >= '{$today}') + OR (p.stato = 'prenotato' AND p.data_prestito <= '{$today}' AND {$scheduledStillHolds}) ) ) OR ( p.attivo = 0 AND p.stato = 'pendente' AND p.copia_id IS NOT NULL ) ) @@ -470,15 +478,7 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction AND c.stato IN ('disponibile', 'prenotato') AND {$lendable} AND p.id IS NULL - ) - ( - SELECT COUNT(*) - FROM prenotazioni pr - WHERE pr.libro_id = ? - AND pr.stato = 'attiva' - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) IS NOT NULL - AND COALESCE(pr.data_inizio_richiesta, DATE(pr.data_scadenza_prenotazione)) <= '{$today}' - AND {$reservationStillHolds} - ), + ) - {$unassignedHoldCount}, 0 ) > 0 THEN 'disponibile' WHEN (SELECT COUNT(*) FROM copie c WHERE c.libro_id = ? AND c.stato = 'prestato') > 0 THEN 'prestato' @@ -494,7 +494,7 @@ public function recalculateBookAvailability(int $bookId, bool $insideTransaction END WHERE id = ? "); - $stmt->bind_param('iiiiiiiiii', $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId); + $stmt->bind_param('iiiiiiii', $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId, $bookId); $result = $stmt->execute(); $stmt->close(); @@ -1071,22 +1071,9 @@ private function findOverbookedCirculationPeriods(): array { $eventsByBook = []; $applicationToday = DateHelper::today(); $stmt = $this->db->prepare(" - SELECT libro_id, 'prestito' AS source_type, id AS source_id, copia_id, attivo, + SELECT libro_id, copia_id, attivo, stato, DATE(data_prestito) AS start_date, - DATE(CASE - -- Overdue loans occupy the copy open-endedly. The sentinel is - -- 9999-12-30 (NOT 9999-12-31) so the +1-day endExclusive below - -- stays a 4-digit 'YYYY-MM-DD' string: '10000-01-01' would sort - -- BEFORE every real date under ksort's lexicographic order - -- ('1' < '2'), zeroing the overdue occupancy in the peak scan. - -- A date-overdue `in_corso` row is the same unreturned - -- physical copy before maintenance flips its state (#366). - WHEN attivo = 1 AND ( - stato = 'in_ritardo' - OR (stato = 'in_corso' AND data_scadenza < ?) - ) THEN '9999-12-30' - ELSE data_scadenza - END) AS end_date + DATE(data_scadenza) AS end_date FROM prestiti WHERE ( (attivo = 1 AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) @@ -1095,7 +1082,6 @@ private function findOverbookedCirculationPeriods(): array { AND data_prestito IS NOT NULL AND data_scadenza IS NOT NULL "); - $stmt->bind_param('s', $applicationToday); $stmt->execute(); $intervalsResult = $stmt->get_result(); @@ -1110,11 +1096,15 @@ private function findOverbookedCirculationPeriods(): array { if (!preg_match('/^\d{4}-\d{2}-\d{2}$/', $start) || !preg_match('/^\d{4}-\d{2}-\d{2}$/', $end)) { continue; } - // A loan due on a closed day keeps its book until the day the - // reader is told, as in CapacityService, even when the stored date - // could not move. Overdue loans keep their open-ended sentinel. - if ((int) $row['attivo'] === 1 && $end !== '9999-12-30') { - $end = LibraryCalendar::holdingEnd($this->db, $end, $row['copia_id'] !== null ? (int) $row['copia_id'] : null); + if ((int) $row['attivo'] === 1) { + $copyId = $row['copia_id'] !== null ? (int) $row['copia_id'] : null; + $due = LibraryCalendar::dueDate($this->db, $end, $copyId); + $end = LibraryCalendar::holdingEnd($this->db, $end, $copyId, $row['stato'] === 'prenotato' ? $bookId : null); + if ($row['stato'] === 'in_ritardo' || ($row['stato'] === 'in_corso' && $due < $applicationToday)) { + // A genuinely overdue loan holds indefinitely. Leave one + // day for the exclusive end to stay within a four-digit year. + $end = '9999-12-30'; + } } if ($end < $start) { $end = $start; diff --git a/app/Support/IcsGenerator.php b/app/Support/IcsGenerator.php index a1ae2a051..5e02cc5fc 100644 --- a/app/Support/IcsGenerator.php +++ b/app/Support/IcsGenerator.php @@ -80,7 +80,7 @@ public function generateForLoan(int $loanId): ?string // loan on a record must keep being able to add it to their calendar, // even in the legacy case of a flagged title that still carries one. - $stmt = $this->db->prepare("SELECT p.id, p.stato, p.attivo, p.copia_id, p.data_prestito, p.data_scadenza, p.updated_at, + $stmt = $this->db->prepare("SELECT p.id, p.libro_id, p.stato, p.attivo, p.copia_id, p.data_prestito, p.data_scadenza, p.updated_at, l.titolo, CONCAT(u.nome, ' ', u.cognome) AS utente_nome FROM prestiti p JOIN libri l ON p.libro_id = l.id AND l.deleted_at IS NULL diff --git a/app/Support/LibraryCalendar.php b/app/Support/LibraryCalendar.php index a70c94941..61219e5d0 100644 --- a/app/Support/LibraryCalendar.php +++ b/app/Support/LibraryCalendar.php @@ -60,6 +60,9 @@ final class LibraryCalendar /** @var array>> per year: date => branches ('*' = every branch) */ private array $holidayDates = []; + /** @var array>> adjacent source years used to build holidayDates */ + private array $holidaySourceDates = []; + /** @var array per-connection cache, keyed by spl_object_id */ private static array $instances = []; @@ -113,6 +116,17 @@ public function __construct(array $closedWeekdays = [], array $closures = [], ar } } + /** + * Add a manual closure without discarding the computed holiday dates. + * @param array{data_inizio:string, data_fine:string, ricorrente_annuale:int, sede_id:?int} $row + */ + public function withAdditionalClosure(array $row): self + { + $next = clone $this; + $next->closures = array_merge($this->closures, (new self([], [$row]))->closures); + return $next; + } + /** The calendar stored in the database (settings + closures table). */ public static function fromDatabase(\mysqli $db, bool $configuration = false): self { @@ -703,9 +717,12 @@ public static function revision(\mysqli $db): int * return day the reader is told, or the day before it when the library hands * the book over on that day itself. Both capacity and the request calendar read it. */ - public static function holdingEnd(\mysqli $db, string $storedDue, ?int $copiaId): string + public static function holdingEnd(\mysqli $db, string $storedDue, ?int $copiaId, ?int $scheduledBookId = null, bool $lockingRead = false): string { - $usable = self::dueDate($db, $storedDue, $copiaId); + $calendar = self::forDb($db); + $usable = $copiaId === null && $scheduledBookId !== null && $calendar->hasRules() + ? $calendar->nextOpenDay($storedDue, self::requestScope($db, $scheduledBookId, $lockingRead)) + : self::dueDate($db, $storedDue, $copiaId); if ($usable > $storedDue && (new \App\Models\SettingsRepository($db))->allowsSameDayHandover()) { return (new \DateTimeImmutable($usable))->modify('-1 day')->format('Y-m-d'); } @@ -1043,24 +1060,38 @@ private function isHoliday(string $ymd, ?int $sedeId = null): bool $year = (int) substr($ymd, 0, 4); if (!isset($this->holidayDates[$year])) { $dates = []; + $sources = []; // An observed New Year's Day can land on 31 December of the // preceding year. Include adjacent source years before filtering. for ($sourceYear = max(1, $year - 1); $sourceYear <= min(9999, $year + 1); $sourceYear++) { - foreach ($this->holidayRules as $rule) { - $branch = isset($rule['sede']) && is_int($rule['sede']) ? $rule['sede'] : '*'; - foreach (Holidays\HolidayCalculator::datesOf($rule, $sourceYear) as $date) { - if ((int) substr($date, 0, 4) === $year) { - $dates[$date][$branch] = true; - } + $sourceDates = $this->holidaySourceDates[$sourceYear] ?? $this->holidaysForSourceYear($sourceYear); + $sources[$sourceYear] = $sourceDates; + foreach ($sourceDates as $date => $branches) { + if ((int) substr($date, 0, 4) === $year) { + $dates[$date] = ($dates[$date] ?? []) + $branches; } } } + $this->holidaySourceDates = $sources; $this->holidayDates[$year] = $dates; } $branches = $this->holidayDates[$year][$ymd] ?? null; return $branches !== null && (isset($branches['*']) || ($sedeId !== null && isset($branches[$sedeId]))); } + /** @return array> */ + private function holidaysForSourceYear(int $year): array + { + $dates = []; + foreach ($this->holidayRules as $rule) { + $branch = isset($rule['sede']) && is_int($rule['sede']) ? $rule['sede'] : '*'; + foreach (Holidays\HolidayCalculator::datesOf($rule, $year) as $date) { + $dates[$date][$branch] = true; + } + } + return $dates; + } + /** * $ymd itself when open, otherwise the first open day after it. A day * inside a dated or yearly closure jumps to the end of that occurrence. diff --git a/tests/mvcc-lockfirst-circulation.unit.php b/tests/mvcc-lockfirst-circulation.unit.php index f8a75c217..12de48c24 100644 --- a/tests/mvcc-lockfirst-circulation.unit.php +++ b/tests/mvcc-lockfirst-circulation.unit.php @@ -353,7 +353,7 @@ function check(bool $cond, string $desc): void $rmSource = $readSource('/app/Controllers/ReservationManager.php'); $pba = $extractMethod($rmSource, 'function processBookAvailability('); $fifoLockingRead = (bool) preg_match( - '/FROM prenotazioni r\s+JOIN utenti u ON r\.utente_id = u\.id.*?ORDER BY r\.queue_position ASC\s+LIMIT 1\s+FOR UPDATE/s', + '/FROM prenotazioni r\s+JOIN utenti u ON r\.utente_id = u\.id.*?ORDER BY r\.queue_position ASC(?:, r\.id ASC)?\s+LIMIT 1\s+FOR UPDATE/s', $pba ); $queueReadUsesOffset = (bool) preg_match( diff --git a/tests/reservation-calendar-edgecases.unit.php b/tests/reservation-calendar-edgecases.unit.php index d1f64e28b..27c27ee74 100644 --- a/tests/reservation-calendar-edgecases.unit.php +++ b/tests/reservation-calendar-edgecases.unit.php @@ -86,9 +86,10 @@ DateHelper::synchronizeDatabaseSession($db); $db->query("INSERT INTO utenti (nome,cognome,email,password,stato,tipo_utente,codice_tessera,email_verificata) VALUES ('Calendar','One','calendar-one@example.invalid','x','attivo','standard','CAL1',1), - ('Calendar','Two','calendar-two@example.invalid','x','attivo','standard','CAL2',1)"); + ('Calendar','Two','calendar-two@example.invalid','x','attivo','standard','CAL2',1), + ('Calendar','Three','calendar-three@example.invalid','x','attivo','standard','CAL3',1)"); $users = array_map('intval', array_column($db->query('SELECT id FROM utenti ORDER BY id')->fetch_all(MYSQLI_ASSOC), 'id')); - [$user, $other] = $users; + [$user, $other, $third] = $users; $reset = static function () use ($db): void { foreach (['prenotazioni', 'prestiti', 'copie', 'libri', 'calendario_chiusure', 'festivita_personalizzate', 'system_settings'] as $table) { $db->query("DELETE FROM {$table}"); @@ -166,6 +167,22 @@ $issues = (new DataIntegrity($db))->verifyDataConsistency(); $check(in_array('overbooked_circulation_period', array_column($issues, 'type'), true), 'integrity audit exposes conflicts caused by a later closure'); + $reset(); + [$b, $copy] = $book(); + $db->query("INSERT INTO copie (libro_id,numero_inventario,stato) VALUES ({$b},'CAL-SIBLING-{$b}','disponibile')"); + $head = $reserve($b, $user, $today, $d(2)); + $tail = $reserve($b, $other, $today, $d(1), 2); + $later = $reserve($b, $third, $d(4), $d(7), 3); + $db->query("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo) + VALUES ({$b},{$copy},{$user},'{$d(4)}','{$d(7)}','prenotato','richiesta',1)"); + $close($d(2), $d(3)); + $db->begin_transaction(); + $manager = new ReservationManager($db); + $manager->setExternalTransaction(true); + $check($manager->processBookAvailability($b) && $state($tail) === 'completata' + && $state($head) === 'attiva' && $state($later) === 'attiva', 'a head blocked only on reopening does not stall a shorter eligible hold on a sibling'); + $db->rollback(); + $reset(); [$b] = $book(); $head = $reserve($b, $user, $today, $d(2)); @@ -290,6 +307,9 @@ $waitingLoan = $pickupLoan($waitingBook, $waitingCopy, $user); $close($d(-1), $d(-1)); $check((new MaintenanceService($db))->checkExpiredPickups() === 0, 'pickup expiry protects the reopening day before deadlines are reconciled'); + [$expiredBook, $expiredCopy] = $book(); + $expiredLoan = $pickupLoan($expiredBook, $expiredCopy, $other); + $db->query("UPDATE prestiti SET pickup_deadline='{$d(-2)}' WHERE id={$expiredLoan}"); $holder = new mysqli($args[0], $args[1], $args[2], $sandbox, $args[4], $args[5]); $holder->begin_transaction(); $holder->query("SELECT id FROM libri WHERE id={$blockedBook} FOR UPDATE"); @@ -304,9 +324,11 @@ $pickupRow = static fn (int $id): array => $db->query("SELECT stato, pickup_deadline FROM prestiti WHERE id={$id}")->fetch_assoc(); $check((bool) array_filter($run['errors'], static fn ($e): bool => str_starts_with((string) $e, 'extendPickupDeadlines')), 'maintenance reports the failed pickup reconciliation'); $check($pickupRow($waitingLoan)['stato'] === 'da_ritirare', 'a failed reconciliation defers pickup expiry instead of cancelling the pickup'); + $check($pickupRow($expiredLoan)['stato'] === 'da_ritirare', 'a failed reconciliation skips even the independently expired pickup'); $run = (new MaintenanceService($db))->runAll(); $check($pickupRow($waitingLoan)['stato'] === 'da_ritirare' && $pickupRow($waitingLoan)['pickup_deadline'] === $today && $pickupRow($blockedLoan)['pickup_deadline'] === $today, 'the next maintenance pass moves both pickup deadlines to the reopening day'); + $check($pickupRow($expiredLoan)['stato'] === 'scaduto', 'a successful reconciliation resumes the deferred expiry sweep'); $reset(); $calendarAdmin->store($post(['data_inizio' => '2026-01-01', 'data_fine' => '2026-12-29', 'ricorrente_annuale' => '1', 'descrizione' => 'Annual closure']), new Response(), $db); @@ -330,6 +352,14 @@ $calendarAdmin->storeCustom($post(['nome' => 'Cross-year closure', 'tipo' => 'dates', 'date_specifiche' => '2400-12-31', 'durata_giorni' => '2', 'attiva' => '1']), new Response(), $db); $check((int) $db->query('SELECT COUNT(*) FROM festivita_personalizzate')->fetch_row()[0] === 0, 'coverage checks the following year when an explicit holiday crosses New Year'); + $reset(); + \App\Support\Holidays\HolidayPresets::apply($db, 'IT', '', [], $today); + $db->query("INSERT INTO calendario_chiusure (data_inizio,data_fine,descrizione,ricorrente_annuale,regola_festivita) + VALUES ('2026-12-31','2026-12-31','Obsolete preset row',1,'IT:christmas')"); + unset($_SESSION['error_message']); + $calendarAdmin->store($post(['data_inizio' => '2026-01-01', 'data_fine' => '2026-12-30', 'ricorrente_annuale' => '1', 'descrizione' => 'Only December 31 open']), new Response(), $db); + $check(!isset($_SESSION['error_message']) && LibraryCalendar::forDb($db)->isOpen('2027-12-31'), 'the coverage guard ignores obsolete preset rows just like the effective calendar'); + $reset(); [$b] = $book(); $reserve($b, $user, $d(10), $d(12)); @@ -368,6 +398,45 @@ $issues = (new DataIntegrity($db))->verifyDataConsistency(); $check(in_array('overbooked_circulation_period', array_column($issues, 'type'), true), 'integrity audit counts a loan until the reopening day after its closed due date'); + $reset(); + [$b, $copy] = $book(); + $close($d(-1), $d(-1)); + $db->query("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo) + VALUES ({$b},{$copy},{$user},'{$d(-3)}','{$d(-1)}','in_corso','richiesta',1)"); + $legacy = $reserve($b, $other, $d(1), $d(1)); + $db->query("UPDATE prenotazioni SET data_inizio_richiesta=NULL, data_fine_richiesta=NULL WHERE id={$legacy}"); + $integrity = new DataIntegrity($db); + $overbooked = static fn (): bool => (bool) array_filter($integrity->verifyDataConsistency(), static fn (array $issue): bool => + $issue['type'] === 'overbooked_circulation_period' && str_contains($issue['message'], "(ID: {$b})")); + $check(!$overbooked(), 'a loan due on reopening today is not treated as indefinitely overdue tomorrow'); + $db->query("UPDATE prenotazioni SET data_scadenza_prenotazione='{$today} 23:59:59' WHERE id={$legacy}"); + $check($overbooked(), 'the audit counts a legacy NULL-start booking against an overlapping loan'); + + $reset(); + [$b, $copy] = $book(); + $reserve($b, $user, $d(-2), $d(-1)); + $close($d(-1), $d(-1)); + $db->query("UPDATE utenti SET stato='sospeso' WHERE id={$user}"); + $available = static fn (): int => (int) $db->query("SELECT copie_disponibili FROM libri WHERE id={$b}")->fetch_row()[0]; + $integrity->recalculateBookAvailability($b); + $check($available() === 1, 'a paused queue entry extended by a closure does not consume the catalogue counter'); + $db->query("UPDATE utenti SET stato='attivo' WHERE id={$user}"); + $integrity->recalculateAllBookAvailability(); + $check($available() === 0, 'the global counter includes the restored queue entry through reopening'); + $db->query("DELETE FROM prenotazioni WHERE libro_id={$b}"); + $db->query("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo) + VALUES ({$b},{$copy},{$user},'{$d(-3)}','{$d(-1)}','prenotato','richiesta',1)"); + $db->query("UPDATE libri SET copie_disponibili=1 WHERE id={$b}"); + $integrity->recalculateBookAvailability($b); + $check($available() === 0, 'the single-title counter keeps a scheduled copy through its effective return day'); + $db->query("UPDATE libri SET copie_disponibili=1 WHERE id={$b}"); + $integrity->recalculateAllBookAvailability(); + $check($available() === 0, 'the global counter also keeps a scheduled copy through its effective return day'); + $db->query("UPDATE prestiti SET copia_id=NULL WHERE libro_id={$b}"); + $db->query("UPDATE libri SET copie_disponibili=1 WHERE id={$b}"); + $integrity->recalculateBookAvailability($b); + $check($available() === 0, 'a scheduled hold without an assigned copy still uses one title slot on reopening'); + // nextOpenDay() now steps over a whole yearly closure: same answers // as the day-by-day walk, including the New Year wrap and 29 February. $yearly = new LibraryCalendar([], [ diff --git a/tests/reservation-closures-100.unit.php b/tests/reservation-closures-100.unit.php index 96d51aef9..9fdcf54be 100644 --- a/tests/reservation-closures-100.unit.php +++ b/tests/reservation-closures-100.unit.php @@ -330,10 +330,22 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void $openCopy = matrixCopy($t, $b, $t->branchB); $loan = matrixLoan($t, $b, $b['copies'][0], $t->today, $t->day(2), 'pendente'); $t->loan($b, $t->day(7), ['copy' => $b['copies'][0], 'reader' => $t->secondReader, 'start' => $t->day(3), 'status' => 'prenotato']); + matrixReserve($t, $b, $t->operator, $t->day(3), $t->day(7)); matrixHoliday($t, $t->day(2), 1, $t->branchA); $response = (new LoanApprovalController())->approveLoan($t->request('/admin/loans/approve', ['loan_id' => $loan]), new Response(), $t->db); SettingsReview25::eq($response->getStatusCode(), 200, 'A compatible copy can be approved'); SettingsReview25::eq((int) $t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), $openCopy, 'The first copy cannot promise the reopening day twice'); + + matrixHoliday($t, $t->day(-1), 1, $t->branchA); + foreach ([$t->branchA, $t->branchB] as $pinnedBranch) { + $otherBranch = $pinnedBranch === $t->branchA ? $t->branchB : $t->branchA; + $b = $t->book($pinnedBranch); + $sibling = matrixCopy($t, $b, $otherBranch); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(-4), $t->day(-1), 'pendente'); + $response = (new LoanApprovalController())->approveLoan($t->request('/admin/loans/approve', ['loan_id' => $loan]), new Response(), $t->db); + SettingsReview25::eq($response->getStatusCode(), 200, 'An elapsed return window at one branch does not discard a valid reopening at another'); + SettingsReview25::eq((int) $t->scalar("SELECT copia_id FROM prestiti WHERE id={$loan}"), $pinnedBranch === $t->branchA ? $b['copies'][0] : $sibling, 'The chosen copy must still have a usable return date'); + } }; $cases['a returned copy cannot take a hold that conflicts on its reopening day'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); @@ -353,6 +365,16 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void (new ReservationReassignmentService($t->db))->reassignOnCopyLost($b['copies'][0]); $row = $t->db->query("SELECT copia_id,data_scadenza FROM prestiti WHERE id={$loan}")->fetch_row(); SettingsReview25::eq([(int) $row[0], $row[1]], [$replacement, $t->day(5)], 'Replacement commitment uses the destination calendar'); + + $b = $t->book($t->branchA); + $sibling = matrixCopy($t, $b, $t->branchB); + $loan = matrixLoan($t, $b, $b['copies'][0], $t->day(2), $t->day(4)); + $t->loan($b, $t->day(7), ['copy' => $sibling, 'reader' => $t->secondReader, 'start' => $t->day(5), 'status' => 'prenotato']); + $t->set('loans', 'same_day_handover_after_closure', '1'); + $t->db->query("UPDATE copie SET stato='perso' WHERE id={$b['copies'][0]}"); + SettingsReview25::check((new ReservationReassignmentService($t->db))->reassignOnCopyLost($b['copies'][0]), 'Copy loss can release a hold with no replacement'); + $row = $t->db->query("SELECT copia_id,data_scadenza FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq($row, [null, $t->day(4)], 'An unassigned hold does not take the reopening day from the next reader'); }; foreach (['staff', 'reader'] as $actor) { @@ -389,7 +411,7 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void $cases['a mixed-branch title waits until at least one lending branch reopens'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); - matrixCopy($t, $b, $t->branchB); + $copyB = matrixCopy($t, $b, $t->branchB); $day = $t->day(2); matrixClosure($t, $day, $t->day(4), $t->branchA); matrixHoliday($t, $day, 1, $t->branchB); @@ -399,6 +421,13 @@ function matrixCalendarEnd(string $ics, string $uid, string $exclusiveEnd): void SettingsReview25::check(in_array($day, $availability['closed_dates'], true), 'Overlapping branch closures are presented as closed'); SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $day), $firstOpen, 'The copy-available estimate follows the same branch scope'); SettingsReview25::eq(LibraryCalendar::reservationDeadline($t->db, $day . ' 18:00:00', $b['id']), $firstOpen . ' 23:59:59', 'A deadline extends until one branch can accept it'); + + $copyless = matrixLoan($t, $b, null, $t->today, $day); + matrixCalendarEnd((string) (new IcsGenerator($t->db))->generateForLoan($copyless), 'loan-' . $copyless . '@pinakes', $t->day(4)); + SettingsReview25::eq((new CapacityService($t->db))->occupiedCount($b['id'], $firstOpen, $firstOpen), 1, 'A copyless booking also occupies title capacity until an actual lending branch reopens'); + $t->db->query("DELETE FROM prestiti WHERE id={$copyless}"); + $t->loan($b, $t->day(6), ['copy' => $copyB, 'start' => $firstOpen, 'status' => 'prenotato']); + SettingsReview25::eq((new CapacityService($t->db))->firstAvailableDate($b['id'], $day), $t->day(5), 'The open branch must also have an unoccupied copy'); }; $cases['a pinned booking tries a sibling when the original copy is promised on reopening'] = static function (SettingsReview25 $t): void { From 38cddb035880969531db98f8b28d8cb548f263d7 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 8 Nov 2026 02:40:19 +0100 Subject: [PATCH 7/9] Closures review: a sibling copy keeps the promised return day, inverted windows are refused without rules, copyless bookings end where capacity does - MaintenanceService activation asks a sibling copy to cover the return day the reader was promised (the effective due of the pinned copy's branch, or of the title's branches), as reassignment already does. - CapacityService::reservationDueDateForCopy() refuses a stored return day before the start also when the calendar has no rules. - The request calendar passes the title to holdingEnd() for copyless prenotato rows, as capacity and the integrity audit do. - New tests/reservation-closures-review.unit.php: three cases, all failing on the previous commit. --- app/Controllers/ReservationsController.php | 7 +- app/Services/CapacityService.php | 4 +- app/Support/MaintenanceService.php | 8 +- tests/reservation-closures-review.unit.php | 99 ++++++++++++++++++++++ 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 tests/reservation-closures-review.unit.php diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index 0beb11811..67dc2cab6 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -736,7 +736,10 @@ private function autoApproveLoanRequest($request, int $loanId, ?bool $knownEnabl private function holdingEnd(array $loan): string { $copyId = !empty($loan['copia_id']) ? (int) $loan['copia_id'] : null; - return \App\Support\LibraryCalendar::holdingEnd($this->db, substr((string) $loan['data_scadenza'], 0, 10), $copyId); + // A scheduled hold without a copy reads the title's branches, as + // CapacityService and DataIntegrity do, so both see the same end. + $scheduledBookId = ($loan['stato'] ?? '') === 'prenotato' && isset($loan['libro_id']) ? (int) $loan['libro_id'] : null; + return \App\Support\LibraryCalendar::holdingEnd($this->db, substr((string) $loan['data_scadenza'], 0, 10), $copyId, $scheduledBookId); } /** @@ -771,7 +774,7 @@ public function getBookAvailabilityData($bookId, ?string $startDate = null, int // copia_id) also holds its copy until pickup confirmation (#157, model // A-refined). Bare 'pendente' requests with no copy are excluded. $stmt = $this->db->prepare(" - SELECT data_prestito, data_scadenza, data_restituzione, pickup_deadline, stato, copia_id + SELECT libro_id, data_prestito, data_scadenza, data_restituzione, pickup_deadline, stato, copia_id FROM prestiti WHERE libro_id = ? AND ( (attivo = 1 AND stato IN ('in_corso', 'in_ritardo', 'da_ritirare', 'prenotato')) diff --git a/app/Services/CapacityService.php b/app/Services/CapacityService.php index 10509b53d..bf55e80a1 100644 --- a/app/Services/CapacityService.php +++ b/app/Services/CapacityService.php @@ -516,7 +516,9 @@ public function reservationDueDateForCopy(int $bookId, int $copyId, string $star { $calendar = \App\Support\LibraryCalendar::forDb($this->db); if (!$calendar->hasRules()) { - return $storedDue; + // Without rules nextOpenDay() is the identity: the same inverted + // window the calendar branch refuses below is refused here too. + return $storedDue < $start ? null : $storedDue; } $branch = \App\Support\LibraryCalendar::branchOfCopy($this->db, $copyId); $due = $calendar->nextOpenDay($storedDue, $branch); diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index b36e57fbd..a53eba551 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -1012,6 +1012,12 @@ public function activateScheduledLoans(): int // confirmPickup(), which correctly refuses a copy-less loan. $copiaId = !empty($loan['copia_id']) ? (int) $loan['copia_id'] : 0; $dueDate = null; + // The return day the reader was promised: the effective due of + // the pinned copy's branch (or of the title's branches when + // unpinned). A sibling copy elsewhere must still cover it. + $promisedDue = $calendar->hasRules() + ? $calendar->nextOpenDay($oldDue, LibraryCalendar::reservationBranch($this->db, $bookId, $copiaId > 0 ? $copiaId : null)) + : $oldDue; if ($copiaId > 0 && in_array($copiaId, $committedCopyIds, true)) { // Repair legacy/reassigned rows that share a copy with another // open loan for this borrower/title: activation must choose a @@ -1112,7 +1118,7 @@ public function activateScheduledLoans(): int $freeCopy->close(); foreach ($freeCopyRows as $freeCopyRow) { $candidateId = (int) $freeCopyRow['id']; - $candidateDue = $this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $oldDue, $today); + $candidateDue = $this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $promisedDue, $today); if ($candidateDue !== null) { $copiaId = $candidateId; $dueDate = $candidateDue; diff --git a/tests/reservation-closures-review.unit.php b/tests/reservation-closures-review.unit.php new file mode 100644 index 000000000..81d0f022e --- /dev/null +++ b/tests/reservation-closures-review.unit.php @@ -0,0 +1,99 @@ +db->prepare("INSERT INTO festivita_personalizzate (nome,tipo,date_specifiche,durata_giorni,sede_id,attiva) VALUES ('Review holiday','dates',?,?,?,1)"); + $stmt->bind_param('sii', $start, $days, $branch); + $stmt->execute(); + $stmt->close(); + LibraryCalendar::reset(); +} + +function reviewLoan(SettingsReview25 $t, array $book, ?int $copy, string $start, string $end): int +{ + $stmt = $t->db->prepare("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo) VALUES (?,?,?,?,?,'prenotato','richiesta',1)"); + $stmt->bind_param('iiiss', $book['id'], $copy, $t->reader, $start, $end); + $stmt->execute(); + $id = (int) $stmt->insert_id; + $stmt->close(); + return $id; +} + +function reviewCopy(SettingsReview25 $t, array $book, int $branch): int +{ + $t->db->query("INSERT INTO copie (libro_id,numero_inventario,stato,sede_id) VALUES ({$book['id']},'Review-{$book['id']}-{$branch}','disponibile',{$branch})"); + return (int) $t->db->insert_id; +} + +$cases = []; +$cases['a sibling copy in another branch keeps the promised return day'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $sibling = reviewCopy($t, $b, $t->branchB); + $loan = reviewLoan($t, $b, $b['copies'][0], $t->today, $t->day(4)); + $t->loan($b, $t->day(8), ['copy' => $b['copies'][0], 'reader' => $t->secondReader, 'start' => $t->day(5), 'status' => 'prenotato']); + reviewHoliday($t, $t->day(4), 1, $t->branchA); + SettingsReview25::eq(LibraryCalendar::dueDate($t->db, $t->day(4), $b['copies'][0]), $t->day(5), 'The reader was promised the reopening day'); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 1, 'The sibling activates the booking'); + $row = $t->db->query("SELECT copia_id, data_scadenza FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq([(int) $row[0], $row[1]], [$sibling, $t->day(5)], 'The sibling stores the promised return day, not its own earlier one'); +}; +$cases['an inverted window is refused also without calendar rules'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $capacity = new CapacityService($t->db); + SettingsReview25::check(!LibraryCalendar::forDb($t->db)->hasRules(), 'The fixture has no calendar rules'); + SettingsReview25::eq($capacity->reservationDueDateForCopy($b['id'], $b['copies'][0], $t->day(3), $t->day(1)), null, 'A return day before the start is unusable'); + SettingsReview25::eq($capacity->reservationDueDateForCopy($b['id'], $b['copies'][0], $t->day(1), $t->day(3)), $t->day(3), 'A valid window keeps its stored return day'); +}; +$cases['the request calendar ends a copyless booking where capacity does'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + reviewLoan($t, $b, null, $t->day(1), $t->day(3)); + reviewHoliday($t, $t->day(3), 1, $t->branchA); + $capacityHeld = (new CapacityService($t->db))->occupiedCount($b['id'], $t->day(4), $t->day(4)); + SettingsReview25::eq($capacityHeld, 1, 'Capacity holds the only copy until the lending branch reopens'); + $view = (new ReservationsController($t->db))->getBookAvailabilityData($b['id'], $t->today, 8); + SettingsReview25::eq($view['by_date'][$t->day(4)]['available'], 0, 'The calendar does not offer the day capacity refuses'); +}; + +$passed = $failed = 0; +$suite = null; +$savedSession = $_SESSION ?? []; +try { + $suite = new SettingsReview25(); + ConfigStore::usingConnection($suite->db, static function () use ($suite, $cases, &$passed, &$failed): void { + foreach ($cases as $name => $case) { + try { + $suite->reset(); + $case($suite); + $passed++; + echo "PASS {$name}\n"; + } catch (Throwable $error) { + $failed++; + echo "FAIL {$name}: {$error->getMessage()}\n"; + } + } + }); +} catch (Throwable $error) { + $failed++; + fwrite(STDERR, 'FAIL setup: ' . $error->getMessage() . PHP_EOL); +} finally { + $_SESSION = $savedSession; + LibraryCalendar::reset(); + $suite?->close(); +} +echo "Reservation closures review: {$passed} passed, {$failed} failed\n"; +exit($failed === 0 ? 0 : 1); From 4146c8f9d788d1858b0f926fb129bddf65873139 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 8 Nov 2026 02:54:16 +0100 Subject: [PATCH 8/9] Activation: a sibling copy keeps the promised return day only if the title still has room for the days it adds When the promised day is past the stored due, the added days were held through the pinned copy; on a sibling they now go through the title capacity check (excluding this loan), and activation is deferred when another booking already fills them. New case in tests/reservation-closures-review.unit.php, failing without the check. --- app/Support/MaintenanceService.php | 8 ++++++++ tests/reservation-closures-review.unit.php | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index a53eba551..c3b0b8f8c 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -1116,6 +1116,14 @@ public function activateScheduledLoans(): int $freeCopy->execute(); $freeCopyRows = $freeCopy->get_result()->fetch_all(MYSQLI_ASSOC); $freeCopy->close(); + // The days the promise adds past the stored due were held + // through the pinned copy; on a sibling they must still fit + // the title, or two readers would share the reopening day. + if ($promisedDue > $oldDue + && !(new \App\Services\CapacityService($this->db, lockingReads: true)) + ->hasFreeCapacity($bookId, LibraryCalendar::shift($oldDue, 1), $promisedDue, excludePrestitoId: $loanId)) { + $freeCopyRows = []; + } foreach ($freeCopyRows as $freeCopyRow) { $candidateId = (int) $freeCopyRow['id']; $candidateDue = $this->activationDueDate($calendar, $loanId, $bookId, $candidateId, (string) $loan['data_prestito'], $promisedDue, $today); diff --git a/tests/reservation-closures-review.unit.php b/tests/reservation-closures-review.unit.php index 81d0f022e..cfae937eb 100644 --- a/tests/reservation-closures-review.unit.php +++ b/tests/reservation-closures-review.unit.php @@ -52,6 +52,20 @@ function reviewCopy(SettingsReview25 $t, array $book, int $branch): int $row = $t->db->query("SELECT copia_id, data_scadenza FROM prestiti WHERE id={$loan}")->fetch_row(); SettingsReview25::eq([(int) $row[0], $row[1]], [$sibling, $t->day(5)], 'The sibling stores the promised return day, not its own earlier one'); }; +$cases['a sibling copy does not take a promised day the title has already given away'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $sibling = reviewCopy($t, $b, $t->branchB); + $loan = reviewLoan($t, $b, $b['copies'][0], $t->today, $t->day(4)); + $t->loan($b, $t->day(8), ['copy' => $b['copies'][0], 'reader' => $t->secondReader, 'start' => $t->day(5), 'status' => 'prenotato']); + // A copyless booking of a third reader starts on the reopening day: with + // two copies, the second reader and this one already fill it. + $t->db->query("INSERT INTO prestiti (libro_id,copia_id,utente_id,data_prestito,data_scadenza,stato,origine,attivo) VALUES ({$b['id']},NULL,{$t->operator},'{$t->day(5)}','{$t->day(7)}','prenotato','richiesta',1)"); + reviewHoliday($t, $t->day(4), 1, $t->branchA); + SettingsReview25::eq((new MaintenanceService($t->db))->activateScheduledLoans(), 0, 'Activation is deferred rather than overbooking the reopening day'); + $row = $t->db->query("SELECT copia_id, stato FROM prestiti WHERE id={$loan}")->fetch_row(); + SettingsReview25::eq([(int) $row[0], $row[1]], [$b['copies'][0], 'prenotato'], 'The booking keeps its pinned copy and waits'); + SettingsReview25::check($sibling > 0, 'The sibling exists but cannot keep the promise'); +}; $cases['an inverted window is refused also without calendar rules'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); $capacity = new CapacityService($t->db); From 6a85c63b90e7c8ea92993b97c139b63b93eb98dd Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 8 Nov 2026 04:26:20 +0100 Subject: [PATCH 9/9] Approval: refuse a return day before the start also without calendar rules The no-rules branch of approvalWindow() returned the stored pair unchecked, while the calendar branch refuses an inverted window. New case in tests/reservation-closures-review.unit.php. --- app/Controllers/LoanApprovalController.php | 3 ++- tests/reservation-closures-review.unit.php | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 044965700..51bde9505 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -690,7 +690,8 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R private function approvalWindow(mysqli $db, \App\Support\LibraryCalendar $calendar, \App\Services\CapacityService $capacity, int $bookId, int $loanId, int $copyId, string $start, string $due, string $today): ?array { if (!$calendar->hasRules()) { - return [$start, $due]; + // The calendar branch refuses a return day before the start; so does this one. + return $due < $start ? null : [$start, $due]; } $branch = \App\Support\LibraryCalendar::branchOfCopy($db, $copyId); $start = $calendar->nextOpenDay($start, $branch); diff --git a/tests/reservation-closures-review.unit.php b/tests/reservation-closures-review.unit.php index cfae937eb..3cbd7bf22 100644 --- a/tests/reservation-closures-review.unit.php +++ b/tests/reservation-closures-review.unit.php @@ -73,6 +73,16 @@ function reviewCopy(SettingsReview25 $t, array $book, int $branch): int SettingsReview25::eq($capacity->reservationDueDateForCopy($b['id'], $b['copies'][0], $t->day(3), $t->day(1)), null, 'A return day before the start is unusable'); SettingsReview25::eq($capacity->reservationDueDateForCopy($b['id'], $b['copies'][0], $t->day(1), $t->day(3)), $t->day(3), 'A valid window keeps its stored return day'); }; +$cases['approval refuses an inverted window also without calendar rules'] = static function (SettingsReview25 $t): void { + $b = $t->book($t->branchA); + $window = new ReflectionMethod(\App\Controllers\LoanApprovalController::class, 'approvalWindow'); + $controller = (new ReflectionClass(\App\Controllers\LoanApprovalController::class))->newInstanceWithoutConstructor(); + $calendar = LibraryCalendar::forDb($t->db); + SettingsReview25::check(!$calendar->hasRules(), 'The fixture has no calendar rules'); + $capacity = new CapacityService($t->db); + SettingsReview25::eq($window->invoke($controller, $t->db, $calendar, $capacity, $b['id'], 0, $b['copies'][0], $t->day(3), $t->day(1), $t->today), null, 'A return day before the start is not approved'); + SettingsReview25::eq($window->invoke($controller, $t->db, $calendar, $capacity, $b['id'], 0, $b['copies'][0], $t->day(1), $t->day(3), $t->today), [$t->day(1), $t->day(3)], 'A valid window is approved as stored'); +}; $cases['the request calendar ends a copyless booking where capacity does'] = static function (SettingsReview25 $t): void { $b = $t->book($t->branchA); reviewLoan($t, $b, null, $t->day(1), $t->day(3));