From 7a8ec7106dbd12426d03e1b40e103107e1b76996 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Fri, 21 Aug 2026 01:58:53 +0200 Subject: [PATCH 01/56] feat(loans): allow multiple physical copies per borrower --- app/Controllers/LoanApprovalController.php | 39 +- app/Controllers/PrestitiController.php | 108 ++- app/Controllers/SettingsController.php | 7 +- app/Models/SettingsRepository.php | 10 + .../ReservationReassignmentService.php | 38 +- app/Support/LoanMultiplicityPolicy.php | 117 +++ app/Support/MaintenanceService.php | 38 +- app/Views/prestiti/crea_prestito.php | 19 +- app/Views/settings/loans-tab.php | 39 + docs/settings.MD | 4 + installer/database/data_da_DK.sql | 4 + installer/database/data_de_DE.sql | 4 + installer/database/data_en_US.sql | 4 + installer/database/data_fr_FR.sql | 4 + installer/database/data_it_IT.sql | 4 + locale/da_DK.json | 8 +- locale/de_DE.json | 8 +- locale/en_US.json | 8 +- locale/fr_FR.json | 8 +- locale/it_IT.json | 8 +- tests/multiple-copy-loans-238.unit.php | 812 ++++++++++++++++++ 21 files changed, 1218 insertions(+), 73 deletions(-) create mode 100644 app/Support/LoanMultiplicityPolicy.php create mode 100644 tests/multiple-copy-loans-238.unit.php diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 0479fe004..a8765709c 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -267,20 +267,11 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R return $response->withHeader('Content-Type', 'application/json')->withStatus(403); } - $dupStmt = $db->prepare(" - SELECT id FROM prestiti - WHERE libro_id = ? AND utente_id = ? AND id != ? - AND ( - (attivo = 1 AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) - OR (attivo = 0 AND stato = 'pendente') - ) - LIMIT 1 - "); - $dupStmt->bind_param('iii', $libroId, $utenteId, $loanId); - $dupStmt->execute(); - $hasActiveDuplicate = $dupStmt->get_result()->num_rows > 0; - $dupStmt->close(); - if ($hasActiveDuplicate) { + // Approval atomically assigns a locked physical copy below, so the + // opt-in multiplicity policy may coexist with other copy-bound loans. + // Sibling pending/copyless rows remain blocking in every mode. + $multiplicityPolicy = new \App\Support\LoanMultiplicityPolicy($db); + if ($multiplicityPolicy->hasBlockingLoan($libroId, $utenteId, true, $loanId)) { $db->rollback(); $response->getBody()->write(json_encode([ 'success' => false, @@ -288,6 +279,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R ])); return $response->withHeader('Content-Type', 'application/json')->withStatus(409); } + $borrowerCommittedCopyIds = $multiplicityPolicy->committedCopyIds($libroId, $utenteId, $loanId); $dupReservationStmt = $db->prepare(" SELECT id @@ -364,7 +356,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // If loan already has a valid assigned copy, we can skip global slot counting $selectedCopy = null; - if ($existingCopiaId !== null) { + if ($existingCopiaId !== null && !in_array($existingCopiaId, $borrowerCommittedCopyIds, true)) { $existingCopyStmt = $db->prepare(" SELECT c.id FROM copie c WHERE c.id = ? @@ -409,6 +401,17 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R SELECT c.id FROM copie c WHERE c.libro_id = ? AND c.stato IN ('disponibile', 'prenotato') + AND NOT EXISTS ( + SELECT 1 FROM prestiti own + WHERE own.copia_id = c.id + AND own.libro_id = ? + AND own.utente_id = ? + AND own.id != ? + AND ( + (own.attivo = 0 AND own.stato = 'pendente') + OR (own.attivo = 1 AND own.stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) + ) + ) AND NOT EXISTS ( SELECT 1 FROM prestiti p WHERE p.copia_id = c.id @@ -421,7 +424,7 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R ) LIMIT 1 "); - $overlapStmt->bind_param('iss', $libroId, $dataScadenza, $dataPrestito); + $overlapStmt->bind_param('iiiiss', $libroId, $libroId, $utenteId, $loanId, $dataScadenza, $dataPrestito); $overlapStmt->execute(); $overlapResult = $overlapStmt->get_result(); $selectedCopy = $overlapResult ? $overlapResult->fetch_assoc() : null; @@ -433,6 +436,10 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R // Fallback: try date-aware method to find available copy for the requested period $copyRepo = new \App\Models\CopyRepository($db); $availableCopies = $copyRepo->getAvailableByBookIdForDateRange($libroId, $dataPrestito, $dataScadenza); + $availableCopies = array_values(array_filter( + $availableCopies, + static fn (array $copy): bool => !in_array((int) $copy['id'], $borrowerCommittedCopyIds, true) + )); if (empty($availableCopies)) { $db->rollback(); diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index fa2079490..36cab5137 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -172,7 +172,9 @@ public function createForm(Request $request, Response $response, mysqli $db): Re // il vecchio date('Y-m-d') (TZ processo, spesso UTC) mostrava "ieri" dopo // mezzanotte, e il '+1 month' della view divergeva dal default server (30gg). $defaultDataPrestito = \App\Support\DateHelper::today(); - $defaultLoanDays = (new \App\Models\SettingsRepository($db))->loanDurationDays(); + $loanSettings = new \App\Models\SettingsRepository($db); + $defaultLoanDays = $loanSettings->loanDurationDays(); + $allowMultipleLoansSameBook = $loanSettings->allowsMultipleLoansSameBook(); $defaultDataScadenza = date('Y-m-d', strtotime($defaultDataPrestito . " +{$defaultLoanDays} days")); ob_start(); @@ -291,24 +293,16 @@ public function store(Request $request, Response $response, mysqli $db): Respons return $response->withHeader('Location', url('/admin/loans/create') . '?error=book_not_found')->withStatus(302); } - // Case 8: Prevent multiple active reservations/loans for the same book by the same user - // Note: 'pendente' has attivo=0, other active states have attivo=1 - $dupStmt = $db->prepare(" - SELECT id FROM prestiti - WHERE libro_id = ? AND utente_id = ? AND ( - (attivo = 0 AND stato = 'pendente') - OR (attivo = 1 AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) - ) - FOR UPDATE - "); - $dupStmt->bind_param('ii', $libro_id, $utente_id); - $dupStmt->execute(); - if ($dupStmt->get_result()->num_rows > 0) { - $dupStmt->close(); + // By default the historical borrower/title uniqueness rule remains. + // The opt-in policy relaxes it only for this staff flow, which always + // resolves and locks a physical copy before INSERT. Pending/copyless + // commitments still block, as do active queue reservations below. + $multiplicityPolicy = new \App\Support\LoanMultiplicityPolicy($db); + if ($multiplicityPolicy->hasBlockingLoan($libro_id, $utente_id, true)) { $db->rollback(); return $response->withHeader('Location', url('/admin/loans/create') . '?error=duplicate_reservation')->withStatus(302); } - $dupStmt->close(); + $borrowerCommittedCopyIds = $multiplicityPolicy->committedCopyIds($libro_id, $utente_id); $dupReservationStmt = $db->prepare(" SELECT id @@ -385,10 +379,10 @@ public function store(Request $request, Response $response, mysqli $db): Respons // auto-assign paths below (fully backward compatible). $copyCode = trim($oldInput['copy_code'] ?? ''); if ($copyCode !== '') { - // Lock ONLY the copie row here — the requested book is already - // locked above (line ~155), so JOINing+locking libri would lock a - // (potentially different) book row second and invert lock order → - // deadlock risk when a scanned code belongs to another book. + // Resolve without locking first: borrower/title commitments must + // be locked before the selected copy to preserve the canonical + // order. The final copy lock + overlap re-check below is the + // authority immediately before INSERT. // Soft-delete stays covered: the copy_wrong_book check below // requires this copy to belong to $libro_id, which was resolved // under `deleted_at IS NULL`. @@ -397,7 +391,6 @@ public function store(Request $request, Response $response, mysqli $db): Respons FROM copie c WHERE c.numero_inventario = ? LIMIT 1 - FOR UPDATE "); $codeStmt->bind_param('s', $copyCode); $codeStmt->execute(); @@ -415,6 +408,14 @@ public function store(Request $request, Response $response, mysqli $db): Respons $forcedCopyId = (int) $codeRow['id']; + // Multiple open rows for one borrower/title must represent + // different physical items even when their date windows do not + // overlap. The book lock makes this early snapshot race-safe. + if (in_array($forcedCopyId, $borrowerCommittedCopyIds, true)) { + $db->rollback(); + return $response->withHeader('Location', url('/admin/loans/create') . '?error=copy_not_available')->withStatus(302); + } + // Must be in a lendable state and have no overlapping hold for the period. $lendable = !in_array($codeRow['stato'], ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true); if ($lendable) { @@ -473,6 +474,16 @@ public function store(Request $request, Response $response, mysqli $db): Respons SELECT c.id FROM copie c WHERE c.libro_id = ? AND c.stato NOT IN ('perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento') + AND NOT EXISTS ( + SELECT 1 FROM prestiti own + WHERE own.copia_id = c.id + AND own.libro_id = ? + AND own.utente_id = ? + AND ( + (own.attivo = 0 AND own.stato = 'pendente') + OR (own.attivo = 1 AND own.stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) + ) + ) AND NOT EXISTS ( SELECT 1 FROM prestiti p WHERE p.copia_id = c.id @@ -485,7 +496,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons ) LIMIT 1 "); - $overlapStmt->bind_param('iss', $libro_id, $data_scadenza, $data_prestito); + $overlapStmt->bind_param('iiiss', $libro_id, $libro_id, $utente_id, $data_scadenza, $data_prestito); $overlapStmt->execute(); $overlapResult = $overlapStmt->get_result(); $selectedCopy = $overlapResult ? $overlapResult->fetch_assoc() : null; @@ -498,10 +509,17 @@ public function store(Request $request, Response $response, mysqli $db): Respons } // Lock selected copy and re-check overlap to prevent race conditions - $lockCopyStmt = $db->prepare("SELECT id FROM copie WHERE id = ? FOR UPDATE"); + $lockCopyStmt = $db->prepare("SELECT id, stato, libro_id FROM copie WHERE id = ? FOR UPDATE"); $lockCopyStmt->bind_param('i', $selectedCopy['id']); $lockCopyStmt->execute(); + $lockedCopy = $lockCopyStmt->get_result()->fetch_assoc(); $lockCopyStmt->close(); + if (!$lockedCopy + || (int) $lockedCopy['libro_id'] !== $libro_id + || in_array($lockedCopy['stato'], ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true)) { + $db->rollback(); + return $response->withHeader('Location', url('/admin/loans/create') . '?error=copy_not_available')->withStatus(302); + } // Race condition check to prevent double-booking $overlapCopyStmt = $db->prepare(" @@ -673,13 +691,16 @@ public function store(Request $request, Response $response, mysqli $db): Respons SecureLogger::warning(__('Notifica prestito fallita'), ['error' => $e->getMessage()]); } - // "Salva e registra un'altra copia": keep borrower, dates and note, - // but clear both book and copy. Active loans are unique per user/book, - // so retaining the previous book would make the next submit fail the - // duplicate guard. Scanning the next inventory code fills its book. + // "Salva e registra un'altra copia": always clear the physical copy. + // With the opt-in mode enabled, retain the book too so the operator + // can scan several copies of the same title without repeating search. + // Strict mode preserves the historical title-reset behaviour. if (($oldInput['save_and_new'] ?? '') === '1') { $retain = $oldInput; - unset($retain['libro_id'], $retain['copy_code'], $retain['save_and_new']); + unset($retain['copy_code'], $retain['save_and_new']); + if (!$multiplicityPolicy->isEnabled()) { + unset($retain['libro_id']); + } $_SESSION['loan_form_old'] = $retain; $redirectUrl = url('/admin/loans/create') . '?created=1'; $scaricaPdf = ($oldInput['scarica_pdf'] ?? '') === '1'; @@ -882,21 +903,22 @@ public function update(Request $request, Response $response, mysqli $db, int $id return $response->withHeader('Location', url('/admin/loans') . '?error=' . $eligibilityError)->withStatus(302); } - // Dup-check (libro, nuovo utente) sugli stati attivi, escludendo il - // prestito in modifica — stesso predicato di store(). - $dupStmt = $db->prepare(" - SELECT id FROM prestiti - WHERE libro_id = ? AND utente_id = ? AND id <> ? AND ( - (attivo = 0 AND stato = 'pendente') - OR (attivo = 1 AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) - ) - FOR UPDATE - "); - $dupStmt->bind_param('iii', $libroId, $newUserId, $id); - $dupStmt->execute(); - $hasDup = $dupStmt->get_result()->num_rows > 0; - $dupStmt->close(); - if ($hasDup) { + // Reassignment may use the relaxed rule only when this existing + // loan is already tied to a physical copy. Legacy copyless rows + // deliberately retain strict title-level uniqueness. + $multiplicityPolicy = new \App\Support\LoanMultiplicityPolicy($db); + if ($multiplicityPolicy->hasBlockingLoan( + $libroId, + $newUserId, + $locked['copia_id'] !== null, + $id + )) { + $db->rollback(); + return $response->withHeader('Location', url('/admin/loans') . '?error=duplicate_reservation')->withStatus(302); + } + $committedCopyIds = $multiplicityPolicy->committedCopyIds($libroId, $newUserId, $id); + if ($locked['copia_id'] !== null + && in_array((int) $locked['copia_id'], $committedCopyIds, true)) { $db->rollback(); return $response->withHeader('Location', url('/admin/loans') . '?error=duplicate_reservation')->withStatus(302); } diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index f97226854..4a711a726 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -1310,7 +1310,7 @@ public function updateEventSettings(Request $request, Response $response, mysqli } /** - * @return array{loan_duration_days: int, pickup_expiry_days: int, max_renewals: int, max_active_loans_per_user: int, max_loan_duration_days: int, auto_approve_requests: bool, recall_auto_enabled: bool, recall_interval_days: int, recall_max_count: int, app_timezone: string} + * @return array{loan_duration_days: int, pickup_expiry_days: int, max_renewals: int, max_active_loans_per_user: int, max_loan_duration_days: int, auto_approve_requests: bool, allow_multiple_loans_same_book: bool, recall_auto_enabled: bool, recall_interval_days: int, recall_max_count: int, app_timezone: string} */ private function resolveLoansSettings(SettingsRepository $repository): array { @@ -1321,6 +1321,7 @@ private function resolveLoansSettings(SettingsRepository $repository): array 'max_active_loans_per_user' => (int) ($repository->get('loans', 'max_active_loans_per_user', '0') ?? 0), 'max_loan_duration_days' => (int) ($repository->get('loans', 'max_loan_duration_days', '90') ?? 90), 'auto_approve_requests' => $repository->autoApproveLoanRequests(), + 'allow_multiple_loans_same_book' => $repository->allowsMultipleLoansSameBook(), // #360: automatic recall (sollecito) schedule for overdue loans. 'recall_auto_enabled' => (string) ($repository->get('loans', 'recall_auto_enabled', '0') ?? '0') === '1', 'recall_interval_days' => (int) ($repository->get('loans', 'recall_interval_days', '7') ?? 7), @@ -1353,6 +1354,9 @@ public function updateLoansSettings(Request $request, Response $response, mysqli $autoApprove = isset($data['auto_approve_requests']) && is_scalar($data['auto_approve_requests']) && (string) $data['auto_approve_requests'] === '1'; + $allowMultipleLoansSameBook = isset($data['allow_multiple_loans_same_book']) + && is_scalar($data['allow_multiple_loans_same_book']) + && (string) $data['allow_multiple_loans_same_book'] === '1'; // #360: automatic recall schedule. The same clamps are re-applied at // read time by NotificationService::sendLoanRecalls(). $recallEnabled = isset($data['recall_auto_enabled']) @@ -1367,6 +1371,7 @@ public function updateLoansSettings(Request $request, Response $response, mysqli $repository->set('loans', 'max_active_loans_per_user', (string) $maxActiveLoans); $repository->set('loans', 'max_loan_duration_days', (string) $maxLoanDuration); $repository->set('loans', 'auto_approve_requests', $autoApprove ? '1' : '0'); + $repository->set('loans', 'allow_multiple_loans_same_book', $allowMultipleLoansSameBook ? '1' : '0'); $repository->set('loans', 'recall_auto_enabled', $recallEnabled ? '1' : '0'); $repository->set('loans', 'recall_interval_days', (string) $recallInterval); $repository->set('loans', 'recall_max_count', (string) $recallMaxCount); diff --git a/app/Models/SettingsRepository.php b/app/Models/SettingsRepository.php index cc43db959..ad3e65471 100644 --- a/app/Models/SettingsRepository.php +++ b/app/Models/SettingsRepository.php @@ -97,6 +97,16 @@ public function autoApproveLoanRequests(): bool return ($this->get('loans', 'auto_approve_requests', '0') ?? '0') === '1'; } + /** + * Whether staff workflows may register more than one open loan for the same + * borrower and title when every loan is tied to a distinct physical copy. + * Missing or malformed values deliberately preserve the strict default. + */ + public function allowsMultipleLoansSameBook(): bool + { + return ($this->get('loans', 'allow_multiple_loans_same_book', '0') ?? '0') === '1'; + } + /** * Configured default loan duration. Invalid or missing values retain the * historical 30-day fallback in every loan creation/update path. diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index c9dc21b6a..9db643a96 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -187,6 +187,16 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): void } $reservation = $lockedReservation; + // Existing same-title duplicates must keep distinct physical copies + // even when their windows do not overlap (and even if the setting was + // subsequently disabled). Lock sibling commitments before the copy. + $committedCopyIds = (new \App\Support\LoanMultiplicityPolicy($this->db)) + ->committedCopyIds($libroId, (int) $reservation['utente_id'], (int) $reservation['id']); + if (in_array($newCopiaId, $committedCopyIds, true)) { + $this->rollbackIfOwned($ownTransaction); + return; + } + // Verifica che la nuova copia sia effettivamente disponibile (lock) $stmt = $this->db->prepare("SELECT id, stato FROM copie WHERE id = ? FOR UPDATE"); $stmt->bind_param('i', $newCopiaId); @@ -318,6 +328,7 @@ public function reassignOnCopyLost(int $copiaId): void $libroId, $excludedCopies, $reservationId, + (int) $reservation['utente_id'], $resStart, $resEnd ); @@ -338,7 +349,7 @@ public function reassignOnCopyLost(int $copiaId): void $lockBook->close(); $lockReservation = $this->db->prepare(" - SELECT id, copia_id, data_prestito, data_scadenza + SELECT id, copia_id, utente_id, data_prestito, data_scadenza FROM prestiti WHERE id = ? AND libro_id = ? AND copia_id = ? AND ( (attivo = 1 AND stato IN ('prenotato','da_ritirare')) @@ -356,6 +367,14 @@ public function reassignOnCopyLost(int $copiaId): void $resStart = (string) $currentReservation['data_prestito']; $resEnd = (string) $currentReservation['data_scadenza']; + $committedCopyIds = (new \App\Support\LoanMultiplicityPolicy($this->db)) + ->committedCopyIds($libroId, (int) $currentReservation['utente_id'], $reservationId); + if (in_array($nextCopyId, $committedCopyIds, true)) { + $this->rollbackIfOwned($ownTransaction); + $excludedCopies[] = $nextCopyId; + continue; + } + // Lock della nuova copia e verifica stato (race condition protection) $stmt = $this->db->prepare("SELECT id, stato FROM copie WHERE id = ? FOR UPDATE"); $stmt->bind_param('i', $nextCopyId); @@ -540,6 +559,7 @@ private function findAvailableCopyExcluding( int $libroId, array $excludeCopiaIds, int $reservationId, + int $userId, string $startDate, string $endDate ): ?int @@ -549,6 +569,18 @@ private function findAvailableCopyExcluding( FROM copie c WHERE c.libro_id = ? AND c.stato NOT IN ('perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento') + AND NOT EXISTS ( + SELECT 1 + FROM prestiti own + WHERE own.copia_id = c.id + AND own.libro_id = ? + AND own.utente_id = ? + AND own.id <> ? + AND ( + (own.attivo = 0 AND own.stato = 'pendente') + OR (own.attivo = 1 AND own.stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) + ) + ) AND NOT EXISTS ( SELECT 1 FROM prestiti p @@ -562,8 +594,8 @@ private function findAvailableCopyExcluding( ) ) "; - $params = [$libroId, $reservationId, $endDate, $startDate]; - $types = 'iiss'; + $params = [$libroId, $libroId, $userId, $reservationId, $reservationId, $endDate, $startDate]; + $types = 'iiiiiss'; if (!empty($excludeCopiaIds)) { $placeholders = implode(',', array_fill(0, count($excludeCopiaIds), '?')); diff --git a/app/Support/LoanMultiplicityPolicy.php b/app/Support/LoanMultiplicityPolicy.php new file mode 100644 index 000000000..5008a4d14 --- /dev/null +++ b/app/Support/LoanMultiplicityPolicy.php @@ -0,0 +1,117 @@ +settings = $settings ?? new SettingsRepository($db); + } + + public function isEnabled(): bool + { + return $this->settings->allowsMultipleLoansSameBook(); + } + + /** + * Whether another open loan blocks the proposed borrower/title assignment. + * + * The relaxed rule is available only when the caller guarantees that the + * operation will finish with a physical copy. Otherwise the historical, + * title-level uniqueness rule is retained. + */ + public function hasBlockingLoan( + int $bookId, + int $userId, + bool $operationWillBindCopy, + ?int $excludeLoanId = null + ): bool { + $strict = !$operationWillBindCopy || !$this->isEnabled(); + $excludeSql = $excludeLoanId !== null ? ' AND id <> ?' : ''; + $activeCopyPredicate = $strict ? '' : ' AND copia_id IS NULL'; + + $stmt = $this->db->prepare(" + SELECT id + FROM prestiti + WHERE libro_id = ? AND utente_id = ?{$excludeSql} AND ( + (attivo = 0 AND stato = 'pendente') + OR ( + attivo = 1{$activeCopyPredicate} + AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo') + ) + ) + LIMIT 1 + FOR UPDATE + "); + + if ($excludeLoanId !== null) { + $stmt->bind_param('iii', $bookId, $userId, $excludeLoanId); + } else { + $stmt->bind_param('ii', $bookId, $userId); + } + $stmt->execute(); + $hasBlockingLoan = $stmt->get_result()->num_rows > 0; + $stmt->close(); + + return $hasBlockingLoan; + } + + /** + * Physical copies already committed by this borrower for the same title. + * + * The list ignores date overlap on purpose: two open same-borrower/title rows + * represent two physical items even when their scheduled windows do not + * overlap. It remains authoritative after the setting is switched off so + * lifecycle jobs cannot corrupt duplicates that were validly created while + * enabled. Callers use it to reject or skip an already committed copy. + * + * @return list + */ + public function committedCopyIds( + int $bookId, + int $userId, + ?int $excludeLoanId = null + ): array { + $excludeSql = $excludeLoanId !== null ? ' AND id <> ?' : ''; + $stmt = $this->db->prepare(" + SELECT copia_id + FROM prestiti + WHERE libro_id = ? AND utente_id = ?{$excludeSql} + AND copia_id IS NOT NULL + AND ( + (attivo = 0 AND stato = 'pendente') + OR (attivo = 1 AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo')) + ) + FOR UPDATE + "); + + if ($excludeLoanId !== null) { + $stmt->bind_param('iii', $bookId, $userId, $excludeLoanId); + } else { + $stmt->bind_param('ii', $bookId, $userId); + } + $stmt->execute(); + $result = $stmt->get_result(); + $copyIds = []; + while ($row = $result->fetch_assoc()) { + $copyIds[(int) $row['copia_id']] = true; + } + $stmt->close(); + + return array_keys($copyIds); + } +} diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index ded5dca0f..793e08b68 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -525,7 +525,7 @@ public function activateScheduledLoans(): int // guard (BUG8/D13): never promote a reservation whose whole window is already // past into 'da_ritirare' — its expiry cron culls it instead. $stmt = $this->db->prepare(" - SELECT id, copia_id, libro_id, data_scadenza FROM prestiti + SELECT id, copia_id, libro_id, utente_id, data_scadenza FROM prestiti WHERE stato = 'prenotato' AND data_prestito <= ? AND data_scadenza >= ? @@ -573,7 +573,7 @@ public function activateScheduledLoans(): int } $lockLoan = $this->db->prepare(" - SELECT id, copia_id, libro_id, data_prestito, data_scadenza + SELECT id, copia_id, libro_id, utente_id, data_prestito, data_scadenza FROM prestiti WHERE id = ? AND stato = 'prenotato' AND attivo = 1 AND data_prestito <= ? AND data_scadenza >= ? @@ -588,6 +588,11 @@ public function activateScheduledLoans(): int continue; } $loan = $lockedLoan; + $committedCopyIds = (new LoanMultiplicityPolicy($this->db))->committedCopyIds( + $bookId, + (int) $loan['utente_id'], + $loanId + ); // #366 guard: a reservation may only become 'da_ritirare' (and get // the pickup-ready email) when a physical copy is genuinely free @@ -632,6 +637,12 @@ 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; + 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 + // distinct physical item, regardless of date overlap. + $copiaId = 0; + } if ($copiaId > 0) { $copyStmt = $this->db->prepare('SELECT stato FROM copie WHERE id = ? AND libro_id = ? FOR UPDATE'); $copyStmt->bind_param('ii', $copiaId, $bookId); @@ -674,6 +685,18 @@ public function activateScheduledLoans(): int FROM copie c WHERE c.libro_id = ? AND c.stato IN ('disponibile', 'prenotato') + AND NOT EXISTS ( + SELECT 1 + FROM prestiti own + WHERE own.copia_id = c.id + AND own.libro_id = ? + AND own.utente_id = ? + AND own.id <> ? + AND ( + (own.attivo = 0 AND own.stato = 'pendente') + OR (own.attivo = 1 AND own.stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) + ) + ) AND NOT EXISTS ( SELECT 1 FROM prestiti p @@ -687,7 +710,16 @@ public function activateScheduledLoans(): int LIMIT 1 FOR UPDATE "); - $freeCopy->bind_param('iiss', $bookId, $loanId, $loan['data_scadenza'], $loan['data_prestito']); + $freeCopy->bind_param( + 'iiiiiss', + $bookId, + $bookId, + $loan['utente_id'], + $loanId, + $loanId, + $loan['data_scadenza'], + $loan['data_prestito'] + ); $freeCopy->execute(); $freeCopyRow = $freeCopy->get_result()->fetch_assoc(); $freeCopy->close(); diff --git a/app/Views/prestiti/crea_prestito.php b/app/Views/prestiti/crea_prestito.php index 18f99308f..b89314625 100644 --- a/app/Views/prestiti/crea_prestito.php +++ b/app/Views/prestiti/crea_prestito.php @@ -16,6 +16,7 @@ $meUserId = (int) ($meUserId ?? 0); $meUserName = (string) ($meUserName ?? ''); $defaultLoanDays = max(1, (int) ($defaultLoanDays ?? 30)); +$allowMultipleLoansSameBook = (bool) ($allowMultipleLoansSameBook ?? false); $csrf = Csrf::ensureToken(); // Get locale from session (same as frontend/layout.php) @@ -117,7 +118,9 @@ class="ml-auto inline-flex items-center px-3 py-2 bg-red-600 hover:bg-red-700 te -
+ @@ -176,6 +179,11 @@ class="inline-flex items-center gap-2 whitespace-nowrap px-4 py-2 bg-gray-100 te

+ +

+ +

+ @@ -249,7 +257,7 @@ class="mt-0.5 h-4 w-4 rounded border-gray-300 text-gray-800 focus:ring-gray-500" + dati condivisi mantenuti; il codice copia viene sempre azzerato. --> @@ -957,6 +965,13 @@ function resolve() { }); } setupCopyCodeResolver(); + + // In the batch workflow, return the operator directly to the scanner field + // after a successful save. The selected title is retained server-side. + if (loanForm && loanCreatedAlert && loanForm.dataset.multipleCopyMode === '1') { + const nextCopyCode = document.getElementById('copy_code'); + if (nextCopyCode) nextCopyCode.focus(); + } }); diff --git a/app/Views/settings/loans-tab.php b/app/Views/settings/loans-tab.php index 9c31f9f0d..f30db36cd 100644 --- a/app/Views/settings/loans-tab.php +++ b/app/Views/settings/loans-tab.php @@ -252,6 +252,45 @@ class="block w-32 rounded-xl border-gray-300 focus:border-gray-500 focus:ring-gr + +
+
+

+ + +

+

+
+
+ +
+ +
+
+
+
+
+
+
+ +

+
+ +
+
+
+
diff --git a/docs/settings.MD b/docs/settings.MD index a90d4df72..866bca9fe 100644 --- a/docs/settings.MD +++ b/docs/settings.MD @@ -234,9 +234,12 @@ Imposti le regole globali dei prestiti (categoria setting `loans`): | `pickup_expiry_days` | Giorni entro cui ritirare un prestito approvato prima della scadenza ritiro | 3 | 1 … 365 | | `max_renewals` | Numero massimo di rinnovi (0 = illimitato) | 3 | 0 … 100 | | `max_active_loans_per_user` | Massimo prestiti attivi per utente (0 = illimitato) | 0 | 0 … 1000 | +| `allow_multiple_loans_same_book` | Consente allo stesso utente più prestiti attivi dello stesso libro su copie fisiche distinte | 0 (disattivato) | Toggle 0 / 1 | > **Importante**: il server **ricalcola e blocca (clamp)** ogni valore entro i limiti indicati a prescindere dagli attributi `min`/`max` del form (che sono solo indicativi). Un POST manipolato non può quindi impostare durate o rinnovi assurdi. +Il toggle **Più copie dello stesso titolo** è disattivato per impostazione predefinita: il normale flusso di biblioteca continua quindi a permettere un solo prestito o una sola richiesta attiva per utente e libro. Attivalo solo quando serve affidare contemporaneamente allo stesso utente più copie fisiche dello stesso titolo, per esempio in una scuola o in un centro didattico. Ogni prestito deve riferirsi a una copia distinta e conta separatamente nel limite `max_active_loans_per_user`; richieste in attesa e prenotazioni restano invece uniche per utente e libro. + --- ### 8. **Impostazioni Avanzate** - Personalizzazioni e funzionalità extra @@ -487,6 +490,7 @@ Categorie effettivamente usate dal `SettingsController` (chiave `category`): ('loans', 'pickup_expiry_days', '3'), ('loans', 'max_renewals', '3'), ('loans', 'max_active_loans_per_user', '0'), -- 0 = illimitato +('loans', 'allow_multiple_loans_same_book', '0'), -- 0 = comportamento standard, 1 = copie distinte ('advanced', 'days_before_expiry_warning', '3'), ('advanced', 'session_lifetime', '180'), -- minuti ('advanced', 'force_https', '0'), diff --git a/installer/database/data_da_DK.sql b/installer/database/data_da_DK.sql index 0811cfc38..cc07f503d 100644 --- a/installer/database/data_da_DK.sql +++ b/installer/database/data_da_DK.sql @@ -23,6 +23,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_active_loans_per_user', '0', 'Maksimalt antal aktive lån pr. bruger (0 = ingen grænse)') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'allow_multiple_loans_same_book', '0', 'Tillad den samme bruger at have flere aktive lån af samme bog på forskellige fysiske eksemplarer') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES ('loans', 'max_renewals', '3', 'Maksimalt antal fornyelser tilladt pr. lån') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); diff --git a/installer/database/data_de_DE.sql b/installer/database/data_de_DE.sql index f2f35735f..a253a52a5 100644 --- a/installer/database/data_de_DE.sql +++ b/installer/database/data_de_DE.sql @@ -206,6 +206,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_active_loans_per_user', '0', 'Maximale Anzahl aktiver Ausleihen pro Benutzer (0 = kein Limit)') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'allow_multiple_loans_same_book', '0', 'Erlaubt demselben Benutzer mehrere aktive Ausleihen desselben Buches mit unterschiedlichen physischen Exemplaren') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES ('loans', 'max_renewals', '3', 'Maximale Anzahl erlaubter Verlängerungen pro Ausleihe') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); diff --git a/installer/database/data_en_US.sql b/installer/database/data_en_US.sql index c6e357e30..3dc15ec2c 100644 --- a/installer/database/data_en_US.sql +++ b/installer/database/data_en_US.sql @@ -206,6 +206,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_active_loans_per_user', '0', 'Maximum active loans per user (0 = no limit)') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'allow_multiple_loans_same_book', '0', 'Allow the same user to have multiple active loans for the same book on distinct physical copies') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES ('loans', 'max_renewals', '3', 'Maximum number of renewals allowed per loan') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); diff --git a/installer/database/data_fr_FR.sql b/installer/database/data_fr_FR.sql index 5b7d6f2e2..64cb261ef 100644 --- a/installer/database/data_fr_FR.sql +++ b/installer/database/data_fr_FR.sql @@ -206,6 +206,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_active_loans_per_user', '0', 'Nombre maximal d\'emprunts actifs par utilisateur (0 = aucune limite)') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'allow_multiple_loans_same_book', '0', 'Autorise un même utilisateur à avoir plusieurs emprunts actifs du même livre sur des exemplaires physiques distincts') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES ('loans', 'max_renewals', '3', 'Nombre maximal de renouvellements autorisés par emprunt') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); diff --git a/installer/database/data_it_IT.sql b/installer/database/data_it_IT.sql index 2286858be..a9a51c6af 100644 --- a/installer/database/data_it_IT.sql +++ b/installer/database/data_it_IT.sql @@ -23,6 +23,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_active_loans_per_user', '0', 'Numero massimo di prestiti attivi per utente (0 = nessun limite)') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'allow_multiple_loans_same_book', '0', 'Consente allo stesso utente di avere più prestiti attivi dello stesso libro su copie fisiche distinte') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES ('loans', 'max_renewals', '3', 'Numero massimo di rinnovi consentiti per prestito') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); diff --git a/locale/da_DK.json b/locale/da_DK.json index 0cf15f9ef..60a025786 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -6875,5 +6875,11 @@ "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).": "%s valgte lån modtager rykkeren (kun de forfaldne).", "Impossibile generare la ricevuta PDF.": "Kvitterings-PDF'en kunne ikke genereres.", "Attivazione prestito rinviata: nessuna copia libera": "Aktivering af lån udskudt: intet eksemplar ledigt", - "Attivazione prestito rinviata: copia assegnata non in sede": "Aktivering af lån udskudt: tildelt eksemplar ikke på biblioteket" + "Attivazione prestito rinviata: copia assegnata non in sede": "Aktivering af lån udskudt: tildelt eksemplar ikke på biblioteket", + "Più copie dello stesso titolo": "Flere eksemplarer af samme titel", + "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.": "Tillad den samme bruger at have flere aktive lån af samme bog, så længe hvert lån er knyttet til et særskilt fysisk eksemplar.", + "Consenti prestiti multipli per copia": "Tillad flere lån af særskilte eksemplarer", + "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.": "Nyttigt for skoler og undervisningscentre, der udlåner flere eksemplarer af den samme bog til en lærer.", + "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.": "Afventende anmodninger og reservationer er fortsat unikke pr. bruger og bog. Hvert eksemplar tæller med i grænsen for aktive lån.", + "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare.": "Tilstanden for flere lån er aktiv: Bogen forbliver valgt; scan det næste eksemplar for at fortsætte." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 716676e5e..85b6b3066 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6875,5 +6875,11 @@ "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).": "%s ausgewählte Ausleihen erhalten die Mahnung (nur die überfälligen).", "Impossibile generare la ricevuta PDF.": "Die Quittungs-PDF konnte nicht erstellt werden.", "Attivazione prestito rinviata: nessuna copia libera": "Ausleihe-Aktivierung verschoben: kein Exemplar frei", - "Attivazione prestito rinviata: copia assegnata non in sede": "Ausleihe-Aktivierung verschoben: zugewiesenes Exemplar nicht in der Bibliothek" + "Attivazione prestito rinviata: copia assegnata non in sede": "Ausleihe-Aktivierung verschoben: zugewiesenes Exemplar nicht in der Bibliothek", + "Più copie dello stesso titolo": "Mehrere Exemplare desselben Titels", + "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.": "Erlaube demselben Benutzer mehrere aktive Ausleihen desselben Buches, sofern jede Ausleihe einem anderen physischen Exemplar zugeordnet ist.", + "Consenti prestiti multipli per copia": "Mehrfachausleihen für verschiedene Exemplare zulassen", + "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.": "Nützlich für Schulen und Bildungseinrichtungen, die einer Lehrkraft mehrere Exemplare desselben Lehrbuchs anvertrauen.", + "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.": "Ausstehende Anfragen und Vormerkungen bleiben pro Benutzer und Buch eindeutig. Jedes Exemplar zählt zum Limit für aktive Ausleihen.", + "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare.": "Mehrfachausleihmodus aktiv: Das Buch bleibt ausgewählt; scannen Sie das nächste Exemplar, um fortzufahren." } diff --git a/locale/en_US.json b/locale/en_US.json index d0d215ae9..873eb96e1 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6875,5 +6875,11 @@ "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).": "%s selected loans will receive the reminder (only the overdue ones).", "Impossibile generare la ricevuta PDF.": "Could not generate the receipt PDF.", "Attivazione prestito rinviata: nessuna copia libera": "Loan activation deferred: no copy free", - "Attivazione prestito rinviata: copia assegnata non in sede": "Loan activation deferred: assigned copy not in library" + "Attivazione prestito rinviata: copia assegnata non in sede": "Loan activation deferred: assigned copy not in library", + "Più copie dello stesso titolo": "Multiple copies of the same title", + "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.": "Allow the same user to have multiple active loans for the same book, provided each loan is associated with a distinct physical copy.", + "Consenti prestiti multipli per copia": "Allow multiple loans for separate copies", + "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.": "Useful for schools and learning centers that assign multiple copies of the same textbook to a teacher.", + "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.": "Pending requests and reservations remain unique per user and book. Each copy counts toward the active loan limit.", + "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare.": "Multiple-loan mode is active: the book remains selected; scan the next copy to continue." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 595bc47c4..74ed3030e 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6875,5 +6875,11 @@ "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).": "%s prêts sélectionnés recevront le rappel (uniquement ceux en retard).", "Impossibile generare la ricevuta PDF.": "Impossible de générer le PDF du reçu.", "Attivazione prestito rinviata: nessuna copia libera": "Activation du prêt reportée : aucun exemplaire libre", - "Attivazione prestito rinviata: copia assegnata non in sede": "Activation du prêt reportée : exemplaire attribué absent de la bibliothèque" + "Attivazione prestito rinviata: copia assegnata non in sede": "Activation du prêt reportée : exemplaire attribué absent de la bibliothèque", + "Più copie dello stesso titolo": "Plusieurs exemplaires du même titre", + "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.": "Autorisez un même utilisateur à avoir plusieurs emprunts actifs du même livre, à condition que chaque emprunt soit associé à un exemplaire physique distinct.", + "Consenti prestiti multipli per copia": "Autoriser plusieurs emprunts sur des exemplaires distincts", + "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.": "Utile pour les écoles et les centres de formation qui confient plusieurs exemplaires du même ouvrage à un enseignant.", + "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.": "Les demandes en attente et les réservations restent uniques par utilisateur et par livre. Chaque exemplaire compte dans la limite des emprunts actifs.", + "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare.": "Mode de prêts multiples actif : le livre reste sélectionné ; scannez l'exemplaire suivant pour continuer." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 0444f8a9d..db2d481ca 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6875,5 +6875,11 @@ "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).": "%s prestiti selezionati riceveranno il sollecito (solo quelli scaduti).", "Impossibile generare la ricevuta PDF.": "Impossibile generare la ricevuta PDF.", "Attivazione prestito rinviata: nessuna copia libera": "Attivazione prestito rinviata: nessuna copia libera", - "Attivazione prestito rinviata: copia assegnata non in sede": "Attivazione prestito rinviata: copia assegnata non in sede" + "Attivazione prestito rinviata: copia assegnata non in sede": "Attivazione prestito rinviata: copia assegnata non in sede", + "Più copie dello stesso titolo": "Più copie dello stesso titolo", + "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.": "Consenti allo stesso utente di avere più prestiti attivi dello stesso libro, purché ogni prestito sia associato a una copia fisica distinta.", + "Consenti prestiti multipli per copia": "Consenti prestiti multipli per copia", + "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.": "Utile per scuole e centri didattici che affidano più copie dello stesso testo a un docente.", + "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.": "Richieste in attesa e prenotazioni restano uniche per utente e libro. Ogni copia conta nel limite dei prestiti attivi.", + "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare.": "Modalità prestiti multipli attiva: il libro resta selezionato; scansiona la copia successiva per continuare." } diff --git a/tests/multiple-copy-loans-238.unit.php b/tests/multiple-copy-loans-238.unit.php new file mode 100644 index 000000000..7208c2e78 --- /dev/null +++ b/tests/multiple-copy-loans-238.unit.php @@ -0,0 +1,812 @@ +set_charset('utf8mb4'); +} catch (Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — mandatory for this test: {$e->getMessage()}\n"); + exit(1); +} + +$testNo = 0; +$failed = 0; +$check = static function (bool $condition, string $description) use (&$testNo, &$failed): void { + $testNo++; + printf("[%02d] %s: %s\n", $testNo, $condition ? 'PASS' : 'FAIL', $description); + if (!$condition) { + $failed++; + } +}; + +$run = bin2hex(random_bytes(6)); +$titlePrefix = "ZZ_238MULTI_{$run}_"; +$emailDomain = '@238multi.test.local'; +$today = DateHelper::today(); +$due = (new DateTimeImmutable($today))->modify('+14 days')->format('Y-m-d'); +$sessionBefore = $_SESSION ?? []; + +/** @return array{exists: bool, value: ?string} */ +$captureSetting = static function (string $key) use ($db): array { + $stmt = $db->prepare("SELECT setting_value FROM system_settings WHERE category = 'loans' AND setting_key = ? LIMIT 1"); + $stmt->bind_param('s', $key); + $stmt->execute(); + $row = $stmt->get_result()->fetch_assoc(); + $stmt->close(); + + return [ + 'exists' => $row !== null, + 'value' => $row !== null && $row['setting_value'] !== null ? (string) $row['setting_value'] : null, + ]; +}; + +$restoreSetting = static function (string $key, array $original) use ($db): void { + if (!$original['exists']) { + $stmt = $db->prepare("DELETE FROM system_settings WHERE category = 'loans' AND setting_key = ?"); + $stmt->bind_param('s', $key); + $stmt->execute(); + $stmt->close(); + ConfigStore::clearCache(); + return; + } + + $value = $original['value']; + $stmt = $db->prepare( + "INSERT INTO system_settings (category, setting_key, setting_value) + VALUES ('loans', ?, ?) + ON DUPLICATE KEY UPDATE setting_value = VALUES(setting_value)" + ); + $stmt->bind_param('ss', $key, $value); + $stmt->execute(); + $stmt->close(); + ConfigStore::clearCache(); +}; + +$originalMultiplicity = $captureSetting('allow_multiple_loans_same_book'); +$originalMaxLoans = $captureSetting('max_active_loans_per_user'); + +$userSequence = 0; +$makeUser = static function (string $role = 'standard') use ($db, $run, $emailDomain, &$userSequence): int { + $userSequence++; + $card = 'Z238' . strtoupper(substr($run, 0, 8)) . $userSequence; + $email = "u{$userSequence}-{$run}{$emailDomain}"; + $surname = "ZZ238Multi {$userSequence}"; + $stmt = $db->prepare( + "INSERT INTO utenti + (codice_tessera, nome, cognome, email, password, stato, tipo_utente, email_verificata) + VALUES (?, 'Test', ?, ?, 'x', 'attivo', ?, 1)" + ); + $stmt->bind_param('ssss', $card, $surname, $email, $role); + $stmt->execute(); + $stmt->close(); + + return (int) $db->insert_id; +}; + +$bookSequence = 0; +/** @return array{0: int, 1: list, 2: list} */ +$makeBook = static function (int $copies) use ($db, $titlePrefix, &$bookSequence): array { + $bookSequence++; + $title = $titlePrefix . $bookSequence; + $stmt = $db->prepare( + 'INSERT INTO libri (titolo, copie_totali, copie_disponibili, created_at, updated_at) + VALUES (?, ?, ?, NOW(), NOW())' + ); + $stmt->bind_param('sii', $title, $copies, $copies); + $stmt->execute(); + $stmt->close(); + $bookId = (int) $db->insert_id; + + $copyIds = []; + $copyCodes = []; + for ($copyNo = 1; $copyNo <= $copies; $copyNo++) { + $code = "ZZ238M-{$bookId}-C{$copyNo}"; + $stmt = $db->prepare( + "INSERT INTO copie (libro_id, numero_inventario, stato) VALUES (?, ?, 'disponibile')" + ); + $stmt->bind_param('is', $bookId, $code); + $stmt->execute(); + $stmt->close(); + $copyIds[] = (int) $db->insert_id; + $copyCodes[] = $code; + } + + return [$bookId, $copyIds, $copyCodes]; +}; + +$makeLoan = static function ( + int $bookId, + ?int $copyId, + int $userId, + string $state, + int $active, + ?string $startDate = null, + ?string $endDate = null +) use ($db, $today, $due): int { + $loanStart = $startDate ?? $today; + $loanEnd = $endDate ?? $due; + $stmt = $db->prepare( + "INSERT INTO prestiti + (libro_id, copia_id, utente_id, data_prestito, data_scadenza, stato, origine, attivo) + VALUES (?, ?, ?, ?, ?, ?, 'diretto', ?)" + ); + $stmt->bind_param('iiisssi', $bookId, $copyId, $userId, $loanStart, $loanEnd, $state, $active); + $stmt->execute(); + $stmt->close(); + + return (int) $db->insert_id; +}; + +$makeActiveReservation = static function (int $bookId, int $userId) use ($db, $today, $due): int { + $stmt = $db->prepare( + "INSERT INTO prenotazioni + (libro_id, utente_id, data_inizio_richiesta, data_fine_richiesta, + data_scadenza_prenotazione, stato, queue_position, notifica_inviata) + VALUES (?, ?, ?, ?, ?, 'attiva', 1, 0)" + ); + $stmt->bind_param('iisss', $bookId, $userId, $today, $due, $due); + $stmt->execute(); + $stmt->close(); + + return (int) $db->insert_id; +}; + +$loanCount = static function (int $bookId, int $userId) use ($db): int { + $stmt = $db->prepare('SELECT COUNT(*) FROM prestiti WHERE libro_id = ? AND utente_id = ?'); + $stmt->bind_param('ii', $bookId, $userId); + $stmt->execute(); + $count = (int) $stmt->get_result()->fetch_row()[0]; + $stmt->close(); + return $count; +}; + +$cleanup = static function () use ($db, $titlePrefix, $run, $emailDomain): void { + $titleLike = $db->real_escape_string($titlePrefix) . '%'; + $db->query("DELETE n FROM admin_notifications n JOIN prestiti p ON p.id = n.related_id JOIN libri l ON l.id = p.libro_id WHERE l.titolo LIKE '{$titleLike}'"); + $db->query("DELETE r FROM prenotazioni r JOIN libri l ON l.id = r.libro_id WHERE l.titolo LIKE '{$titleLike}'"); + $db->query("DELETE p FROM prestiti p JOIN libri l ON l.id = p.libro_id WHERE l.titolo LIKE '{$titleLike}'"); + $db->query("DELETE c FROM copie c JOIN libri l ON l.id = c.libro_id WHERE l.titolo LIKE '{$titleLike}'"); + $db->query("DELETE FROM libri WHERE titolo LIKE '{$titleLike}'"); + $emailLike = $db->real_escape_string("u%-{$run}{$emailDomain}"); + $db->query("DELETE FROM utenti WHERE email LIKE '{$emailLike}'"); +}; + +$callStore = static function ( + int $adminId, + int $userId, + int $bookId, + string $copyCode, + bool $saveAndNew = false, + ?string $startDate = null, + ?string $endDate = null +) use ($db, $today, $due) { + $_SESSION['user'] = ['tipo_utente' => 'admin', 'id' => $adminId]; + $body = [ + 'utente_id' => (string) $userId, + 'libro_id' => (string) $bookId, + 'copy_code' => $copyCode, + 'data_prestito' => $startDate ?? $today, + 'data_scadenza' => $endDate ?? $due, + 'note' => 'Discussion #238 regression', + 'consegna_immediata' => '1', + 'scarica_pdf' => '0', + ]; + if ($saveAndNew) { + $body['save_and_new'] = '1'; + } + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', '/admin/loans/store') + ->withParsedBody($body); + + return (new PrestitiController())->store( + $request, + (new ResponseFactory())->createResponse(), + $db + ); +}; + +$callApproval = static function (int $loanId) use ($db) { + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', '/admin/loans/approve') + ->withParsedBody(['loan_id' => (string) $loanId]); + + return (new LoanApprovalController())->approveLoan( + $request, + (new ResponseFactory())->createResponse(), + $db + ); +}; + +$cleanup(); +$settings = new SettingsRepository($db); + +try { + /* ================= setting accessor: default and strict parsing ====== */ + $db->query( + "DELETE FROM system_settings + WHERE category = 'loans' AND setting_key = 'allow_multiple_loans_same_book'" + ); + ConfigStore::clearCache(); + $check(!$settings->allowsMultipleLoansSameBook(), 'missing setting keeps the safe OFF default'); + + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $check($settings->allowsMultipleLoansSameBook(), "only persisted value '1' enables multiplicity"); + + foreach (['0', 'true', 'yes', '01', ''] as $malformed) { + $settings->set('loans', 'allow_multiple_loans_same_book', $malformed); + $check( + !$settings->allowsMultipleLoansSameBook(), + "value " . var_export($malformed, true) . ' fails closed' + ); + } + + // Keep capacity limits out of these fixtures; the feature must not alter + // that independent rule (covered by the existing loan limit test suites). + $settings->set('loans', 'max_active_loans_per_user', '0'); + + /* ================= central policy: OFF, ON and unbound rows ========= */ + $policyUser = $makeUser(); + [$policyBook, $policyCopies] = $makeBook(3); + $activeBoundLoan = $makeLoan($policyBook, $policyCopies[0], $policyUser, 'in_corso', 1); + + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + $policy = new LoanMultiplicityPolicy($db); + $check( + $policy->hasBlockingLoan($policyBook, $policyUser, true), + 'OFF blocks a second borrower/title loan even when both operations bind copies' + ); + + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $policy = new LoanMultiplicityPolicy($db); + $check( + !$policy->hasBlockingLoan($policyBook, $policyUser, true), + 'ON ignores an existing copy-bound sibling for a new copy-bound operation' + ); + $check( + $policy->hasBlockingLoan($policyBook, $policyUser, false), + 'ON remains strict when the proposed operation will not bind a physical copy' + ); + + $sameCopyRejected = false; + try { + $makeLoan($policyBook, $policyCopies[0], $policyUser, 'in_corso', 1); + } catch (mysqli_sql_exception $e) { + $sameCopyRejected = true; + } + $sameCopyCount = (int) $db->query( + "SELECT COUNT(*) FROM prestiti WHERE copia_id = {$policyCopies[0]} AND attivo = 1" + )->fetch_row()[0]; + $check( + $sameCopyRejected && $sameCopyCount === 1, + 'the database overlap guard still forbids two open loans on the same physical copy' + ); + + $pendingLoan = $makeLoan($policyBook, null, $policyUser, 'pendente', 0); + $check( + $policy->hasBlockingLoan($policyBook, $policyUser, true), + 'a sibling pending/copyless request remains blocking while ON' + ); + $check( + !$policy->hasBlockingLoan($policyBook, $policyUser, true, $pendingLoan), + 'excluding the loan being approved does not make its own pending row self-conflict' + ); + $check( + $policy->hasBlockingLoan($policyBook, $policyUser, false, $pendingLoan), + 'exclude-id does not relax an unbound operation against another open title loan' + ); + + $db->query("UPDATE prestiti SET stato = 'annullato', attivo = 0 WHERE id = {$pendingLoan}"); + $activeCopylessLoan = $makeLoan($policyBook, null, $policyUser, 'in_corso', 1); + $check( + $policy->hasBlockingLoan($policyBook, $policyUser, true), + 'a legacy active copyless loan remains blocking while ON' + ); + $check( + !$policy->hasBlockingLoan($policyBook, $policyUser, true, $activeCopylessLoan), + 'exclude-id correctly removes the legacy row currently being changed' + ); + + /* ================= real staff creation and quick-copy workflow ====== */ + $admin = $makeUser('admin'); + $multiBorrower = $makeUser(); + [$multiBook, $multiCopyIds, $multiCopyCodes] = $makeBook(3); + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + + $response = $callStore($admin, $multiBorrower, $multiBook, $multiCopyCodes[0], true); + $check( + $response->getStatusCode() === 302 && str_contains($response->getHeaderLine('Location'), 'created=1'), + 'real store() creates the first scanned physical-copy loan in quick mode' + ); + $retained = $_SESSION['loan_form_old'] ?? []; + $check( + (int) ($retained['libro_id'] ?? 0) === $multiBook + && (int) ($retained['utente_id'] ?? 0) === $multiBorrower + && ($retained['note'] ?? '') === 'Discussion #238 regression', + 'quick mode retains borrower, title, dates and note while multiplicity is ON' + ); + $check( + !array_key_exists('copy_code', $retained) && !array_key_exists('save_and_new', $retained), + 'quick mode always clears the physical-copy code and one-shot submit flag' + ); + + $response = $callStore($admin, $multiBorrower, $multiBook, $multiCopyCodes[0]); + $check( + str_contains($response->getHeaderLine('Location'), 'error=copy_not_available') + && $loanCount($multiBook, $multiBorrower) === 1, + 'real store() still rejects reusing the same scanned copy while ON' + ); + + $response = $callStore($admin, $multiBorrower, $multiBook, $multiCopyCodes[1]); + $distinctCopies = $db->query( + "SELECT COUNT(DISTINCT copia_id), COUNT(*) + FROM prestiti + WHERE libro_id = {$multiBook} AND utente_id = {$multiBorrower} AND attivo = 1" + )->fetch_row(); + $check( + str_contains($response->getHeaderLine('Location'), 'created=1') + && (int) $distinctCopies[0] === 2 + && (int) $distinctCopies[1] === 2, + 'real store() permits two simultaneous loans of distinct copies of one title while ON' + ); + + // Each physical copy is still a full active loan for the independent + // borrower cap. The opt-in changes title multiplicity, never quota math. + $settings->set('loans', 'max_active_loans_per_user', '2'); + $response = $callStore($admin, $multiBorrower, $multiBook, $multiCopyCodes[2]); + $thirdCopyLoanCount = (int) $db->query( + "SELECT COUNT(*) FROM prestiti + WHERE libro_id = {$multiBook} AND utente_id = {$multiBorrower} AND attivo = 1" + )->fetch_row()[0]; + $thirdCopyAssigned = (int) $db->query( + "SELECT COUNT(*) FROM prestiti WHERE copia_id = {$multiCopyIds[2]}" + )->fetch_row()[0]; + $check( + str_contains($response->getHeaderLine('Location'), 'error=max_loans_reached') + && $thirdCopyLoanCount === 2 + && $thirdCopyAssigned === 0, + 'max_active_loans_per_user counts every same-title copy and blocks the next loan' + ); + $settings->set('loans', 'max_active_loans_per_user', '0'); + + // P2 regression: copy identity is stronger than date overlap. An open + // future commitment to copy A means a second open same-borrower/title row + // must represent copy B, even when the requested windows are disjoint. + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $windowBorrower = $makeUser(); + [$windowBook, $windowCopies, $windowCopyCodes] = $makeBook(2); + $futureStart = (new DateTimeImmutable($today))->modify('+20 days')->format('Y-m-d'); + $futureEnd = (new DateTimeImmutable($futureStart))->modify('+5 days')->format('Y-m-d'); + $scheduledCopyLoan = $makeLoan( + $windowBook, + $windowCopies[0], + $windowBorrower, + 'prenotato', + 1, + $futureStart, + $futureEnd + ); + + $windowPolicy = new LoanMultiplicityPolicy($db); + $committedBefore = $windowPolicy->committedCopyIds($windowBook, $windowBorrower); + $committedExcludingCurrent = $windowPolicy->committedCopyIds( + $windowBook, + $windowBorrower, + $scheduledCopyLoan + ); + $check( + $committedBefore === [$windowCopies[0]] && $committedExcludingCurrent === [], + 'committedCopyIds tracks physical identity and honours excludeLoanId' + ); + + $response = $callStore( + $admin, + $windowBorrower, + $windowBook, + $windowCopyCodes[0] + ); + $check( + str_contains($response->getHeaderLine('Location'), 'error=copy_not_available') + && $loanCount($windowBook, $windowBorrower) === 1, + 'explicit copy reuse is rejected for disjoint same-borrower/title windows while ON' + ); + + $response = $callStore($admin, $windowBorrower, $windowBook, ''); + $automaticCopyRow = $db->query( + "SELECT copia_id FROM prestiti + WHERE libro_id = {$windowBook} AND utente_id = {$windowBorrower} AND id <> {$scheduledCopyLoan} + ORDER BY id DESC LIMIT 1" + )->fetch_assoc() ?: []; + $check( + str_contains($response->getHeaderLine('Location'), 'created=1') + && $loanCount($windowBook, $windowBorrower) === 2 + && (int) ($automaticCopyRow['copia_id'] ?? 0) === $windowCopies[1], + 'automatic assignment skips the committed copy and chooses another physical copy for a disjoint window' + ); + + // Queue reservations are book-level commitments and deliberately remain + // unique even though distinct copy-bound staff loans may coexist. + $reservedBorrower = $makeUser(); + [$reservedBook, , $reservedCopyCodes] = $makeBook(2); + $activeReservation = $makeActiveReservation($reservedBook, $reservedBorrower); + $response = $callStore($admin, $reservedBorrower, $reservedBook, $reservedCopyCodes[0]); + $reservationState = (string) $db->query( + "SELECT stato FROM prenotazioni WHERE id = {$activeReservation}" + )->fetch_row()[0]; + $check( + str_contains($response->getHeaderLine('Location'), 'error=duplicate_reservation') + && $loanCount($reservedBook, $reservedBorrower) === 0 + && $reservationState === 'attiva', + 'an active same-user/title reservation still blocks real store() while ON' + ); + + /* ================= strict mode remains backward compatible ========== */ + $strictBorrower = $makeUser(); + [$strictBook, , $strictCopyCodes] = $makeBook(2); + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + + $response = $callStore($admin, $strictBorrower, $strictBook, $strictCopyCodes[0], true); + $check(str_contains($response->getHeaderLine('Location'), 'created=1'), 'strict mode still creates a normal first loan'); + $strictRetained = $_SESSION['loan_form_old'] ?? []; + $check( + !array_key_exists('libro_id', $strictRetained), + 'strict quick mode preserves the historical behaviour of clearing the title' + ); + + $response = $callStore($admin, $strictBorrower, $strictBook, $strictCopyCodes[1]); + $check( + str_contains($response->getHeaderLine('Location'), 'error=duplicate_reservation') + && $loanCount($strictBook, $strictBorrower) === 1, + 'strict mode rejects a second distinct copy for the same borrower/title' + ); + + /* ================= real pending-loan approval semantics ============= */ + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $approvalUser = $makeUser(); + [$approvalBook, $approvalCopies] = $makeBook(2); + $makeLoan($approvalBook, $approvalCopies[0], $approvalUser, 'in_corso', 1); + $loanToApprove = $makeLoan($approvalBook, null, $approvalUser, 'pendente', 0); + + $response = $callApproval($loanToApprove); + $approvalBody = (array) json_decode((string) $response->getBody(), true); + $approvedRow = $db->query( + "SELECT stato, attivo, copia_id FROM prestiti WHERE id = {$loanToApprove}" + )->fetch_assoc() ?: []; + $check( + $response->getStatusCode() === 200 + && ($approvalBody['success'] ?? null) === true + && ($approvedRow['stato'] ?? '') === 'da_ritirare' + && (int) ($approvedRow['attivo'] ?? 0) === 1 + && (int) ($approvedRow['copia_id'] ?? 0) === $approvalCopies[1], + 'real approveLoan() assigns a distinct copy beside an existing copy-bound loan while ON' + ); + + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + $strictApprovalUser = $makeUser(); + [$strictApprovalBook, $strictApprovalCopies] = $makeBook(2); + $makeLoan($strictApprovalBook, $strictApprovalCopies[0], $strictApprovalUser, 'in_corso', 1); + $strictPendingLoan = $makeLoan($strictApprovalBook, null, $strictApprovalUser, 'pendente', 0); + + $response = $callApproval($strictPendingLoan); + $strictApprovalRow = $db->query( + "SELECT stato, attivo, copia_id FROM prestiti WHERE id = {$strictPendingLoan}" + )->fetch_assoc() ?: []; + $check( + $response->getStatusCode() === 409 + && ($strictApprovalRow['stato'] ?? '') === 'pendente' + && (int) ($strictApprovalRow['attivo'] ?? 1) === 0 + && $strictApprovalRow['copia_id'] === null, + 'real approveLoan() remains title-strict beside an active loan while OFF' + ); + + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $pendingSiblingUser = $makeUser(); + [$pendingSiblingBook] = $makeBook(2); + $pendingCandidate = $makeLoan($pendingSiblingBook, null, $pendingSiblingUser, 'pendente', 0); + $pendingSibling = $makeLoan($pendingSiblingBook, null, $pendingSiblingUser, 'pendente', 0); + + $response = $callApproval($pendingCandidate); + $pendingRows = (int) $db->query( + "SELECT COUNT(*) FROM prestiti + WHERE id IN ({$pendingCandidate}, {$pendingSibling}) AND stato = 'pendente' AND attivo = 0 AND copia_id IS NULL" + )->fetch_row()[0]; + $check( + $response->getStatusCode() === 409 && $pendingRows === 2, + 'real approveLoan() keeps a sibling pending/copyless request blocking while ON' + ); + + /* ================= real reassignment semantics ====================== */ + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $reassignTarget = $makeUser(); + $reassignSource = $makeUser(); + [$reassignBook, $reassignCopies] = $makeBook(2); + $makeLoan($reassignBook, $reassignCopies[0], $reassignTarget, 'in_corso', 1); + $loanToReassign = $makeLoan($reassignBook, $reassignCopies[1], $reassignSource, 'in_corso', 1); + + $_SESSION['user'] = ['tipo_utente' => 'admin', 'id' => $admin]; + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', "/admin/loans/edit/{$loanToReassign}") + ->withParsedBody(['utente_id' => (string) $reassignTarget]); + $response = (new PrestitiController())->update( + $request, + (new ResponseFactory())->createResponse(), + $db, + $loanToReassign + ); + $reassignedUser = (int) $db->query( + "SELECT utente_id FROM prestiti WHERE id = {$loanToReassign}" + )->fetch_row()[0]; + $check( + !str_contains($response->getHeaderLine('Location'), 'error=') && $reassignedUser === $reassignTarget, + 'real update() permits reassignment when both same-title loans have distinct copies' + ); + + $legacyTarget = $makeUser(); + $legacySource = $makeUser(); + [$legacyBook, $legacyCopies] = $makeBook(1); + $makeLoan($legacyBook, $legacyCopies[0], $legacyTarget, 'in_corso', 1); + $legacyCopylessLoan = $makeLoan($legacyBook, null, $legacySource, 'in_corso', 1); + + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', "/admin/loans/edit/{$legacyCopylessLoan}") + ->withParsedBody(['utente_id' => (string) $legacyTarget]); + $response = (new PrestitiController())->update( + $request, + (new ResponseFactory())->createResponse(), + $db, + $legacyCopylessLoan + ); + $legacyUserAfter = (int) $db->query( + "SELECT utente_id FROM prestiti WHERE id = {$legacyCopylessLoan}" + )->fetch_row()[0]; + $check( + str_contains($response->getHeaderLine('Location'), 'error=duplicate_reservation') + && $legacyUserAfter === $legacySource, + 'real update() keeps legacy copyless reassignment strict while ON' + ); + + /* ================= lifecycle allocation after disabling the toggle === */ + // Existing duplicate rows are legitimate historical state. Turning the + // option OFF must prevent new duplicates, not let background lifecycle + // allocators collapse two rows back onto one physical item. + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $newCopyUser = $makeUser(); + [$newCopyBook, $newCopyCopies] = $makeBook(2); + $newCopySiblingStart = (new DateTimeImmutable($today))->modify('+30 days')->format('Y-m-d'); + $newCopySiblingEnd = (new DateTimeImmutable($today))->modify('+35 days')->format('Y-m-d'); + $newCopyTargetStart = (new DateTimeImmutable($today))->modify('+40 days')->format('Y-m-d'); + $newCopyTargetEnd = (new DateTimeImmutable($today))->modify('+45 days')->format('Y-m-d'); + $newCopySibling = $makeLoan( + $newCopyBook, + $newCopyCopies[0], + $newCopyUser, + 'prenotato', + 1, + $newCopySiblingStart, + $newCopySiblingEnd + ); + $newCopyTarget = $makeLoan( + $newCopyBook, + null, + $newCopyUser, + 'prenotato', + 1, + $newCopyTargetStart, + $newCopyTargetEnd + ); + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + + $committedAfterDisable = (new LoanMultiplicityPolicy($db))->committedCopyIds( + $newCopyBook, + $newCopyUser, + $newCopyTarget + ); + $check( + $committedAfterDisable === [$newCopyCopies[0]], + 'disabling the toggle preserves committed-copy knowledge for existing duplicate lifecycle rows' + ); + + $db->begin_transaction(); + try { + (new ReservationReassignmentService($db)) + ->setExternalTransaction(true) + ->reassignOnNewCopy($newCopyBook, $newCopyCopies[0]); + $db->commit(); + } catch (Throwable $e) { + $db->rollback(); + throw $e; + } + $newCopyAfterRejectedReuse = $db->query( + "SELECT copia_id FROM prestiti WHERE id = {$newCopyTarget}" + )->fetch_row()[0]; + $check( + $newCopyAfterRejectedReuse === null, + 'reassignOnNewCopy() does not reuse a sibling committed copy on a disjoint window after toggle OFF' + ); + + $db->begin_transaction(); + try { + (new ReservationReassignmentService($db)) + ->setExternalTransaction(true) + ->reassignOnNewCopy($newCopyBook, $newCopyCopies[1]); + $db->commit(); + } catch (Throwable $e) { + $db->rollback(); + throw $e; + } + $newCopyAssignments = $db->query( + "SELECT id, copia_id FROM prestiti WHERE id IN ({$newCopySibling}, {$newCopyTarget}) ORDER BY id" + )->fetch_all(MYSQLI_ASSOC); + $check( + count($newCopyAssignments) === 2 + && (int) $newCopyAssignments[0]['copia_id'] === $newCopyCopies[0] + && (int) $newCopyAssignments[1]['copia_id'] === $newCopyCopies[1], + 'reassignOnNewCopy() assigns the available alternative without changing the existing sibling' + ); + + // Copy-loss allocator: copy A is already committed by this borrower/title, + // copy B is lost, so the disjoint hold must move to copy C rather than A. + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $copyLostUser = $makeUser(); + [$copyLostBook, $copyLostCopies] = $makeBook(3); + $copyLostSiblingStart = (new DateTimeImmutable($today))->modify('+50 days')->format('Y-m-d'); + $copyLostSiblingEnd = (new DateTimeImmutable($today))->modify('+55 days')->format('Y-m-d'); + $copyLostTargetStart = (new DateTimeImmutable($today))->modify('+60 days')->format('Y-m-d'); + $copyLostTargetEnd = (new DateTimeImmutable($today))->modify('+65 days')->format('Y-m-d'); + $copyLostSibling = $makeLoan( + $copyLostBook, + $copyLostCopies[0], + $copyLostUser, + 'prenotato', + 1, + $copyLostSiblingStart, + $copyLostSiblingEnd + ); + $copyLostTarget = $makeLoan( + $copyLostBook, + $copyLostCopies[1], + $copyLostUser, + 'prenotato', + 1, + $copyLostTargetStart, + $copyLostTargetEnd + ); + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + $db->query("UPDATE copie SET stato = 'danneggiato' WHERE id = {$copyLostCopies[1]}"); + + (new ReservationReassignmentService($db))->reassignOnCopyLost($copyLostCopies[1]); + $copyLostAssignments = $db->query( + "SELECT id, copia_id FROM prestiti WHERE id IN ({$copyLostSibling}, {$copyLostTarget}) ORDER BY id" + )->fetch_all(MYSQLI_ASSOC); + $check( + count($copyLostAssignments) === 2 + && (int) $copyLostAssignments[0]['copia_id'] === $copyLostCopies[0] + && (int) $copyLostAssignments[1]['copia_id'] === $copyLostCopies[2], + 'reassignOnCopyLost() skips the committed sibling copy and chooses the distinct alternative after toggle OFF' + ); + + // Maintenance allocator: a legacy/unpinned loan whose window starts today + // must become ready on copy B; copy A belongs to its same-title future + // sibling even though the two windows are disjoint. + $settings->set('loans', 'allow_multiple_loans_same_book', '1'); + $maintenanceUser = $makeUser(); + [$maintenanceBook, $maintenanceCopies] = $makeBook(2); + $maintenanceSiblingStart = (new DateTimeImmutable($today))->modify('+10 days')->format('Y-m-d'); + $maintenanceSiblingEnd = (new DateTimeImmutable($today))->modify('+15 days')->format('Y-m-d'); + $maintenanceTargetEnd = (new DateTimeImmutable($today))->modify('+5 days')->format('Y-m-d'); + $maintenanceSibling = $makeLoan( + $maintenanceBook, + $maintenanceCopies[0], + $maintenanceUser, + 'prenotato', + 1, + $maintenanceSiblingStart, + $maintenanceSiblingEnd + ); + $maintenanceTarget = $makeLoan( + $maintenanceBook, + null, + $maintenanceUser, + 'prenotato', + 1, + $today, + $maintenanceTargetEnd + ); + $settings->set('loans', 'allow_multiple_loans_same_book', '0'); + + $activated = (new MaintenanceService($db))->activateScheduledLoans(); + $maintenanceAssignments = $db->query( + "SELECT id, stato, copia_id FROM prestiti + WHERE id IN ({$maintenanceSibling}, {$maintenanceTarget}) ORDER BY id" + )->fetch_all(MYSQLI_ASSOC); + $check( + $activated >= 1 + && count($maintenanceAssignments) === 2 + && (int) $maintenanceAssignments[0]['copia_id'] === $maintenanceCopies[0] + && $maintenanceAssignments[0]['stato'] === 'prenotato' + && (int) $maintenanceAssignments[1]['copia_id'] === $maintenanceCopies[1] + && $maintenanceAssignments[1]['stato'] === 'da_ritirare', + 'activateScheduledLoans() assigns an unpinned loan to the distinct alternative after toggle OFF' + ); + + // The variable is intentionally referenced so static analysers also verify + // that our initial policy fixture was actually created and not optimized away. + $check($activeBoundLoan > 0 && count($multiCopyIds) === 3, 'fixtures retain explicit physical-copy identity'); +} catch (Throwable $e) { + $failed++; + fwrite(STDERR, "UNCAUGHT TEST ERROR: {$e->getMessage()}\n{$e->getTraceAsString()}\n"); +} finally { + try { + // Harmless in autocommit mode; essential if a controller threw after + // opening a transaction, so fixture cleanup never inherits row locks. + $db->rollback(); + } catch (Throwable $e) { + // No active transaction is the normal path. + } + + try { + $cleanup(); + } catch (Throwable $e) { + $failed++; + fwrite(STDERR, "CLEANUP ERROR: {$e->getMessage()}\n"); + } + + try { + $restoreSetting('allow_multiple_loans_same_book', $originalMultiplicity); + $restoreSetting('max_active_loans_per_user', $originalMaxLoans); + } catch (Throwable $e) { + $failed++; + fwrite(STDERR, "SETTING RESTORE ERROR: {$e->getMessage()}\n"); + } + + $_SESSION = $sessionBefore; +} + +echo "\nPassed: " . ($testNo - $failed) . " Failed: {$failed}\n"; +exit($failed > 0 ? 1 : 0); From 40662cbfdcdbf5ab78df4db12d067ff564f2e743 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Fri, 21 Aug 2026 02:27:15 +0200 Subject: [PATCH 02/56] fix(loans): address multiplicity review findings --- .../ReservationReassignmentService.php | 210 +++++++++++------- app/Support/LoanMultiplicityPolicy.php | 11 +- app/Views/settings/loans-tab.php | 4 +- docs/settings.MD | 2 +- locale/da_DK.json | 4 +- locale/de_DE.json | 4 +- locale/en_US.json | 4 +- locale/fr_FR.json | 4 +- locale/it_IT.json | 4 +- tests/multiple-copy-loans-238.unit.php | 198 +++++++++++++++-- 10 files changed, 337 insertions(+), 108 deletions(-) diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index 9db643a96..bb24c855c 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -130,35 +130,6 @@ private function rollbackIfOwned(bool $ownTransaction): void */ public function reassignOnNewCopy(int $libroId, int $newCopiaId): void { - // 1. Trova prenotazioni che sono "bloccate" (assegnate a copie non disponibili o senza copia) - // Ordina per data creazione (FIFO) - // Solo righe GENUINAMENTE bloccate: senza copia, oppure con una copia in - // stato NON prestabile. NON 'c.stato != disponibile' — una copia 'prenotato' - // o 'prestato' per un periodo NON sovrapposto è legittimamente assegnata e - // non va strappata (BUG6/D11). - $stmt = $this->db->prepare(" - SELECT p.id, p.copia_id, p.utente_id, p.data_prestito, p.data_scadenza - FROM prestiti p - LEFT JOIN copie c ON p.copia_id = c.id - WHERE p.libro_id = ? - AND ( (p.attivo = 1 AND p.stato IN ('prenotato', 'da_ritirare')) - OR (p.attivo = 0 AND p.stato = 'pendente' AND p.origine = 'prenotazione') ) - AND ( p.copia_id IS NULL - OR c.stato IN ('perso','danneggiato','manutenzione','in_restauro','in_trasferimento') ) - ORDER BY p.created_at ASC - LIMIT 1 - "); - $stmt->bind_param('i', $libroId); - $stmt->execute(); - $result = $stmt->get_result(); - $reservation = $result->fetch_assoc(); - $stmt->close(); - - if (!$reservation) { - return; - } - - // 2. Se abbiamo trovato una prenotazione da sbloccare, proviamo ad assegnarla alla nuova copia $ownTransaction = $this->beginTransactionIfNeeded(); try { // CI-SOFT-DELETE-EXEMPT: an existing hold must be released/reassigned even if its book is deleted. @@ -167,66 +138,155 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): void $lockBook->execute(); $lockBook->close(); - // The initial lookup was intentionally non-locking to preserve book - // first ordering. Revalidate the chosen hold now, under the book lock. - $lockedReservationStmt = $this->db->prepare(" + // Cheap advisory pre-filter after the canonical book lock: only rows + // that are currently copyless or pinned to an operationally unavailable + // copy enter the locking batch. The authoritative state re-check happens + // below after both prestiti and copie rows have been locked. + $prefilterStmt = $this->db->prepare(" + SELECT p.id + FROM prestiti p + LEFT JOIN copie c ON p.copia_id = c.id + WHERE p.libro_id = ? + AND ( (p.attivo = 1 AND p.stato IN ('prenotato', 'da_ritirare')) + OR (p.attivo = 0 AND p.stato = 'pendente' AND p.origine = 'prenotazione') ) + AND (p.copia_id IS NULL + OR c.stato IN ('perso','danneggiato','manutenzione','in_restauro','in_trasferimento')) + ORDER BY p.created_at ASC, p.id ASC + "); + $prefilterStmt->bind_param('i', $libroId); + $prefilterStmt->execute(); + $prefilterResult = $prefilterStmt->get_result(); + $candidateIds = []; + while ($row = $prefilterResult->fetch_assoc()) { + $candidateIds[] = (int) $row['id']; + } + $prefilterStmt->close(); + if ($candidateIds === []) { + $this->rollbackIfOwned($ownTransaction); + return; + } + + // Lock the pre-filtered prestiti as one deterministic FIFO batch and + // re-assert their lifecycle state. No copy row is locked yet. + $candidatePlaceholders = implode(',', array_fill(0, count($candidateIds), '?')); + $candidateLockStmt = $this->db->prepare(" SELECT id, copia_id, utente_id, data_prestito, data_scadenza FROM prestiti - WHERE id = ? AND libro_id = ? - AND ( (attivo = 1 AND stato IN ('prenotato','da_ritirare')) + WHERE libro_id = ? AND id IN ({$candidatePlaceholders}) + AND ( (attivo = 1 AND stato IN ('prenotato', 'da_ritirare')) OR (attivo = 0 AND stato = 'pendente' AND origine = 'prenotazione') ) + ORDER BY created_at ASC, id ASC FOR UPDATE "); - $lockedReservationStmt->bind_param('ii', $reservation['id'], $libroId); - $lockedReservationStmt->execute(); - $lockedReservation = $lockedReservationStmt->get_result()->fetch_assoc(); - $lockedReservationStmt->close(); - if (!$lockedReservation) { - $this->rollbackIfOwned($ownTransaction); - return; - } - $reservation = $lockedReservation; - - // Existing same-title duplicates must keep distinct physical copies - // even when their windows do not overlap (and even if the setting was - // subsequently disabled). Lock sibling commitments before the copy. - $committedCopyIds = (new \App\Support\LoanMultiplicityPolicy($this->db)) - ->committedCopyIds($libroId, (int) $reservation['utente_id'], (int) $reservation['id']); - if (in_array($newCopiaId, $committedCopyIds, true)) { + $candidateParams = array_merge([$libroId], $candidateIds); + $candidateTypes = str_repeat('i', count($candidateParams)); + $candidateLockStmt->bind_param($candidateTypes, ...$candidateParams); + $candidateLockStmt->execute(); + $candidateResult = $candidateLockStmt->get_result(); + $candidates = $candidateResult ? $candidateResult->fetch_all(MYSQLI_ASSOC) : []; + $candidateLockStmt->close(); + if ($candidates === []) { $this->rollbackIfOwned($ownTransaction); return; } - // Verifica che la nuova copia sia effettivamente disponibile (lock) - $stmt = $this->db->prepare("SELECT id, stato FROM copie WHERE id = ? FOR UPDATE"); - $stmt->bind_param('i', $newCopiaId); - $stmt->execute(); - $copyStatus = $stmt->get_result()->fetch_assoc(); - $stmt->close(); + // Lock every open commitment already attached to the target copy in a + // single current read. This one result set answers both questions for + // every candidate: same-borrower physical identity and date overlap. + // It replaces one committedCopyIds() plus one overlap query per row. + $targetCommitmentStmt = $this->db->prepare(" + SELECT id, utente_id, data_prestito, data_scadenza, stato + FROM prestiti + WHERE libro_id = ? AND copia_id = ? + AND ( (attivo = 0 AND stato = 'pendente') + OR (attivo = 1 AND stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) ) + ORDER BY id ASC + FOR UPDATE + "); + $targetCommitmentStmt->bind_param('ii', $libroId, $newCopiaId); + $targetCommitmentStmt->execute(); + $targetCommitmentResult = $targetCommitmentStmt->get_result(); + $targetCommitments = $targetCommitmentResult + ? $targetCommitmentResult->fetch_all(MYSQLI_ASSOC) + : []; + $targetCommitmentStmt->close(); + + // Lock the target and every candidate's currently pinned copy in one + // ascending-ID batch. All prestiti locks above are therefore complete + // before the first copie lock (libri -> prestiti -> copie). + $copyIds = [$newCopiaId => true]; + foreach ($candidates as $candidate) { + if ($candidate['copia_id'] !== null) { + $copyIds[(int) $candidate['copia_id']] = true; + } + } + $copyIds = array_keys($copyIds); + sort($copyIds, SORT_NUMERIC); + $copyPlaceholders = implode(',', array_fill(0, count($copyIds), '?')); + $copyLockStmt = $this->db->prepare(" + SELECT id, stato + FROM copie + WHERE libro_id = ? AND id IN ({$copyPlaceholders}) + ORDER BY id ASC + FOR UPDATE + "); + $copyParams = array_merge([$libroId], $copyIds); + $copyTypes = str_repeat('i', count($copyParams)); + $copyLockStmt->bind_param($copyTypes, ...$copyParams); + $copyLockStmt->execute(); + $copyResult = $copyLockStmt->get_result(); + $copiesById = []; + while ($copy = $copyResult->fetch_assoc()) { + $copiesById[(int) $copy['id']] = (string) $copy['stato']; + } + $copyLockStmt->close(); - if (!$copyStatus || !in_array($copyStatus['stato'], ['disponibile', 'prenotato'], true)) { + $targetCopyState = $copiesById[$newCopiaId] ?? null; + if ($targetCopyState === null || !in_array($targetCopyState, ['disponibile', 'prenotato'], true)) { $this->rollbackIfOwned($ownTransaction); return; } - // Non riassegnare se la copia target ha un impegno HOLDING sovrapposto - // al periodo della prenotazione: eviterebbe un SIGNAL del trigger di - // overlap che avvelenerebbe la transazione (BUG6/D11). Overlap inclusivo. - $ovl = $this->db->prepare(" - SELECT 1 FROM prestiti - WHERE copia_id = ? AND id <> ? - AND data_prestito <= ? AND (stato = 'in_ritardo' OR data_scadenza >= ?) - AND ( (attivo = 1 AND stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) - OR (attivo = 0 AND stato = 'pendente' AND copia_id IS NOT NULL) ) - LIMIT 1 - "); - $ovl->bind_param('iiss', $newCopiaId, $reservation['id'], $reservation['data_scadenza'], $reservation['data_prestito']); - $ovl->execute(); - $hasOverlap = (bool) $ovl->get_result()->fetch_row(); - $ovl->close(); - if ($hasOverlap) { - // La nuova copia non è libera per l'intero periodo: lascia la - // prenotazione bloccata, verrà riassegnata da un'altra copia/ritorno. + $nonLendableStates = ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento']; + $reservation = null; + 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. + $currentCopyId = $candidate['copia_id'] !== null ? (int) $candidate['copia_id'] : null; + if ($currentCopyId !== null) { + $currentCopyState = $copiesById[$currentCopyId] ?? null; + if ($currentCopyState === null || !in_array($currentCopyState, $nonLendableStates, true)) { + continue; + } + } + + $identityConflict = false; + $hasOverlap = false; + foreach ($targetCommitments as $commitment) { + if ((int) $commitment['id'] === (int) $candidate['id']) { + continue; + } + + if ((int) $commitment['utente_id'] === (int) $candidate['utente_id']) { + $identityConflict = true; + break; + } + + $startsBeforeCandidateEnds = (string) $commitment['data_prestito'] + <= (string) $candidate['data_scadenza']; + $endsAfterCandidateStarts = $commitment['stato'] === 'in_ritardo' + || (string) $commitment['data_scadenza'] >= (string) $candidate['data_prestito']; + if ($startsBeforeCandidateEnds && $endsAfterCandidateStarts) { + $hasOverlap = true; + } + } + + if (!$identityConflict && !$hasOverlap) { + $reservation = $candidate; + break; + } + } + if ($reservation === null) { $this->rollbackIfOwned($ownTransaction); return; } diff --git a/app/Support/LoanMultiplicityPolicy.php b/app/Support/LoanMultiplicityPolicy.php index 5008a4d14..9e54b7bd6 100644 --- a/app/Support/LoanMultiplicityPolicy.php +++ b/app/Support/LoanMultiplicityPolicy.php @@ -42,7 +42,14 @@ public function hasBlockingLoan( ): bool { $strict = !$operationWillBindCopy || !$this->isEnabled(); $excludeSql = $excludeLoanId !== null ? ' AND id <> ?' : ''; - $activeCopyPredicate = $strict ? '' : ' AND copia_id IS NULL'; + // A promoted reservation remains a queue commitment until pickup, even + // when it already has a physical copy. The relaxed rule applies only to + // physical loans; it must not create a second queue position for the + // same borrower/title. Once picked up (`in_corso`/`in_ritardo`), the row + // is a copy-bound physical loan and can coexist like a direct checkout. + $activeBlockingPredicate = $strict + ? '' + : " AND (copia_id IS NULL OR (origine = 'prenotazione' AND stato IN ('prenotato', 'da_ritirare')))"; $stmt = $this->db->prepare(" SELECT id @@ -50,7 +57,7 @@ public function hasBlockingLoan( WHERE libro_id = ? AND utente_id = ?{$excludeSql} AND ( (attivo = 0 AND stato = 'pendente') OR ( - attivo = 1{$activeCopyPredicate} + attivo = 1{$activeBlockingPredicate} AND stato IN ('prenotato', 'da_ritirare', 'in_corso', 'in_ritardo') ) ) diff --git a/app/Views/settings/loans-tab.php b/app/Views/settings/loans-tab.php index f30db36cd..67e8371f7 100644 --- a/app/Views/settings/loans-tab.php +++ b/app/Views/settings/loans-tab.php @@ -272,8 +272,8 @@ class="block w-32 rounded-xl border-gray-300 focus:border-gray-500 focus:ring-gr
- -

+ +