Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 111 additions & 12 deletions app/Controllers/PrestitiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,12 @@ public function update(Request $request, Response $response, mysqli $db, int $id
// giornata singola) è lecita: createReservation accetta end == start,
// quindi un rifiuto strettamente esclusivo renderebbe immodificabili
// i prestiti a giornata nati dal calendario utente.
if (strtotime($newScadenza) === false || strtotime($newPrestito) === false
|| strtotime($newScadenza) < strtotime($newPrestito)) {
// Formato STRETTO Y-m-d (niente strtotime): valori ambigui o date di
// calendario inesistenti (2026-02-30) vanno rifiutati, non normalizzati
// — i confronti lessicografici a valle e il match con i valori salvati
// presuppongono stringhe canoniche (CodeRabbit, PR #337).
if (!self::isStrictIsoDate($newPrestito) || !self::isStrictIsoDate($newScadenza)
|| $newScadenza < $newPrestito) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return $response->withHeader('Location', url('/admin/loans') . '?error=invalid_dates')->withStatus(302);
}

Expand All @@ -774,7 +778,11 @@ public function update(Request $request, Response $response, mysqli $db, int $id

// Lock del prestito e ri-verifica sotto lock: stato aperto invariato e
// libro_id non cambiato (TOCTOU sulla lettura non bloccante iniziale).
$lockLoan = $db->prepare('SELECT attivo, data_restituzione, libro_id, utente_id FROM prestiti WHERE id=? FOR UPDATE');
// Rilegge anche le DATE correnti: un update concorrente tra la lettura
// iniziale e questo lock le può aver cambiate, e la finestra "vecchia"
// del check di capacità qui sotto deve basarsi sui valori realmente
// salvati, non su quelli pre-transazione (CodeRabbit, PR #337).
$lockLoan = $db->prepare('SELECT attivo, data_restituzione, libro_id, copia_id, utente_id, data_prestito, data_scadenza FROM prestiti WHERE id=? FOR UPDATE');
$lockLoan->bind_param('i', $id);
$lockLoan->execute();
$locked = $lockLoan->get_result()->fetch_assoc();
Expand All @@ -788,6 +796,19 @@ public function update(Request $request, Response $response, mysqli $db, int $id
return $response->withHeader('Location', url('/admin/loans') . '?error=loan_update_failed')->withStatus(302);
}

// Ricostruisci i valori effettivi dai dati LOCKATI: i campi non inviati
// dal form devono completarsi con lo stato corrente reale della riga.
// Ri-valida il range con gli stessi criteri del pre-check (che resta
// come fast-fail senza aprire la transazione).
$newUserId = isset($updateData['utente_id']) ? (int) $updateData['utente_id'] : (int) $locked['utente_id'];
$newPrestito = (string) ($updateData['data_prestito'] ?? $locked['data_prestito']);
$newScadenza = (string) ($updateData['data_scadenza'] ?? $locked['data_scadenza']);
if (!self::isStrictIsoDate($newPrestito) || !self::isStrictIsoDate($newScadenza)
|| $newScadenza < $newPrestito) {
$db->rollback();
return $response->withHeader('Location', url('/admin/loans') . '?error=invalid_dates')->withStatus(302);
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Se l'utente cambia, ri-esegui i controlli di store() (M6b): il campo
// arriva da hidden field e senza ricontrolli permetterebbe di aggirare
// idoneità e dup-check assegnando il prestito a un altro utente.
Expand Down Expand Up @@ -858,15 +879,68 @@ public function update(Request $request, Response $response, mysqli $db, int $id
}
}

// #11: if the loan is being RESCHEDULED, re-check the new window against
// #11: if the loan is being RESCHEDULED, re-check the new dates against
// overlapping loans + queue reservations vs capacity (renew() does this — update()
// used to accept any new dates and only recalc counters, silently extending a loan
// over a queued reservation). Only when the dates actually change.
if ($newPrestito !== (string) $current['data_prestito'] || $newScadenza !== (string) $current['data_scadenza']) {
// #336: check ONLY the newly-claimed segments — the EXACT set difference
// new window ∖ old window. Checking the WHOLE new window re-counted
// commitments that already coexist with the current period (e.g. a
// queued reservation overlapping the loan), so on a 1-copy book ANY
// date edit — even shortening the loan — bounced with
// no_copies_available. Days inside the old window (boundary days
// included: they are already held by this loan) need no re-check;
// only genuinely added days need free capacity.
// Old window from the LOCKED row, not the pre-transaction read.
$oldPrestito = (string) $locked['data_prestito'];
$oldScadenza = (string) $locked['data_scadenza'];
if ($newPrestito !== $oldPrestito || $newScadenza !== $oldScadenza) {
// Y-m-d strings compare correctly lexicographically (validated
// strict above); ±1 day via DateTimeImmutable, no TZ ambiguity.
$dayBefore = static fn (string $ymd): string => (new \DateTimeImmutable($ymd))->modify('-1 day')->format('Y-m-d');
$dayAfter = static fn (string $ymd): string => (new \DateTimeImmutable($ymd))->modify('+1 day')->format('Y-m-d');
$claimedWindows = [];
if ($newPrestito < $oldPrestito) {
$claimedWindows[] = [$newPrestito, min($dayBefore($oldPrestito), $newScadenza)];
}
if ($newScadenza > $oldScadenza) {
$claimedWindows[] = [max($dayAfter($oldScadenza), $newPrestito), $newScadenza];
}
$capacity = new \App\Services\CapacityService($db);
if (!$capacity->hasFreeCapacity($libroId, $newPrestito, $newScadenza, excludePrestitoId: $id)) {
$db->rollback();
return $response->withHeader('Location', url('/admin/loans') . '?error=no_copies_available')->withStatus(302);
foreach ($claimedWindows as [$claimStart, $claimEnd]) {
if (!$capacity->hasFreeCapacity($libroId, $claimStart, $claimEnd, excludePrestitoId: $id)) {
$db->rollback();
return $response->withHeader('Location', url('/admin/loans') . '?error=no_copies_available')->withStatus(302);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// CapacityService decides at BOOK level. With multiple copies it
// can report spare capacity even when the physical copy assigned
// to this loan has another future loan on the added days. The DB
// trigger would reject the UPDATE later, but only as the generic
// loan_update_failed. Mirror renew()/bulkExtend() here so the
// conflict is detected before the write and reported truthfully.
$copyId = $locked['copia_id'] !== null ? (int) $locked['copia_id'] : null;
if ($copyId !== null && $claimedWindows !== []) {
$copyOverlap = $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"
);
foreach ($claimedWindows as [$claimStart, $claimEnd]) {
$copyOverlap->bind_param('iiss', $copyId, $id, $claimEnd, $claimStart);
$copyOverlap->execute();
if ((bool) $copyOverlap->get_result()->fetch_row()) {
$copyOverlap->close();
$db->rollback();
return $response->withHeader('Location', url('/admin/loans') . '?error=loan_copy_conflict')->withStatus(302);
}
}
$copyOverlap->close();
}
}

Expand Down Expand Up @@ -900,7 +974,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id
// emails. A data_prestito-only edit does not affect the overdue clock,
// so it is intentionally excluded from this guard (do not reuse the
// combined data_prestito||data_scadenza condition above).
if ($newScadenza !== (string) $current['data_scadenza']) {
if ($newScadenza !== (string) $locked['data_scadenza']) {
$today = \App\Support\DateHelper::today();
$recalcStato = $db->prepare(
"UPDATE prestiti
Expand Down Expand Up @@ -1571,17 +1645,22 @@ private function applyBulkLoanExtension(
$todayDate = \DateTimeImmutable::createFromFormat('!Y-m-d', $today);
$base = ($todayDate !== false && $todayDate > $dueDate) ? $todayDate : $dueDate;
$newDueDate = $base->modify('+' . $days . ' days')->format('Y-m-d');
$loanStart = (string) $loan['data_prestito'];

// Apply each accepted extension immediately inside the transaction, so the
// next capacity check sees all earlier proposed extensions too.
if (!$capacity->hasFreeCapacity($bookId, $loanStart, $newDueDate, excludePrestitoId: $loanId)) {
// #336: BOTH gates check the same interval — only the days the extension
// actually adds (day after the current due date → new due date). The due
// date itself is already held by this loan, and the copy-overlap check
// previously scanned the whole loan window while capacity scanned the
// extension window: two different intervals for one decision (CodeRabbit).
$extensionStart = $dueDate->modify('+1 day')->format('Y-m-d');
if (!$capacity->hasFreeCapacity($bookId, $extensionStart, $newDueDate, excludePrestitoId: $loanId)) {
return null;
}

$copyId = $loan['copia_id'] !== null ? (int) $loan['copia_id'] : null;
if ($copyId !== null) {
$copyOverlap->bind_param('iiss', $copyId, $loanId, $newDueDate, $loanStart);
$copyOverlap->bind_param('iiss', $copyId, $loanId, $newDueDate, $extensionStart);
$copyOverlap->execute();
if ((bool) $copyOverlap->get_result()->fetch_row()) {
return null;
Expand Down Expand Up @@ -2025,6 +2104,26 @@ public function exportCsv(Request $request, Response $response, mysqli $db): Res
->withHeader('Pragma', 'no-cache');
}

/**
* Data in formato STRETTAMENTE Y-m-d E valida sul calendario reale.
* A differenza di strtotime(), rifiuta formati ambigui ('2026-2-5') e date
* inesistenti ('2026-02-30' — che strtotime normalizzerebbe al 2 marzo):
* il round-trip con createFromFormat garantisce input canonico, così i
* confronti lessicografici tra stringhe Y-m-d restano corretti.
*/
private static function isStrictIsoDate(string $value): bool
{
// Shape-check PRIMA di toccare DateTime: createFromFormat() lancia
// ValueError su input con byte NUL ('2026-01-01%00'), che fuori da un
// try diventerebbe un 500 invece di invalid_dates (CodeRabbit, #337).
// /D àncora la fine reale della stringa (niente newline finale tollerato).
if (!preg_match('/^\d{4}-\d{2}-\d{2}$/D', $value)) {
return false;
}
$dt = \DateTime::createFromFormat('Y-m-d', $value);
return $dt !== false && $dt->format('Y-m-d') === $value;
}

private function guardStaffAccess(Response $response): ?Response
{
$role = $_SESSION['user']['tipo_utente'] ?? '';
Expand Down
9 changes: 6 additions & 3 deletions app/Controllers/UserActionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ public function reservationsPage(Request $request, Response $response, mysqli $d
}
$stmt->close();

// Storico prestiti (ultimi 20) - solo prestiti conclusi
// Storico prestiti (ultimi 20) - tutti i prestiti conclusi, inclusi
// annullati e scaduti (prima sparivano dallo storico). Questi non hanno
// data_restituzione: ordina sul momento di chiusura (updated_at) così
// un annullamento recente non finisce in fondo alla lista.
$sql = "SELECT pr.id, pr.libro_id, pr.data_prestito, pr.data_restituzione, pr.stato,
l.titolo, l.copertina_url,
EXISTS(SELECT 1 FROM recensioni r WHERE r.libro_id = pr.libro_id AND r.utente_id = ?) as has_review
FROM prestiti pr
JOIN libri l ON l.id = pr.libro_id AND l.deleted_at IS NULL
WHERE pr.utente_id = ? AND pr.attivo = 0 AND pr.stato IN ('restituito','perso','danneggiato')
ORDER BY pr.data_restituzione DESC, pr.data_prestito DESC
WHERE pr.utente_id = ? AND pr.attivo = 0 AND pr.stato IN ('restituito','perso','danneggiato','annullato','scaduto')
ORDER BY COALESCE(pr.data_restituzione, pr.updated_at) DESC, pr.data_prestito DESC
LIMIT 20";
$stmt = $db->prepare($sql);
$stmt->bind_param('ii', $uid, $uid);
Expand Down
13 changes: 8 additions & 5 deletions app/Controllers/UserDashboardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ public function index(Request $request, Response $response, mysqli $db): Respons
$stats['preferiti'] = (int)($res->fetch_assoc()['c'] ?? 0);
$stmt->close();

// Count user loan history (exclude soft-deleted books)
$stmt = $db->prepare("SELECT COUNT(*) AS c FROM prestiti p JOIN libri l ON p.libro_id = l.id WHERE p.utente_id = ? AND p.attivo = 0 AND p.stato IN ('restituito','perso','danneggiato') AND l.deleted_at IS NULL");
// Count user loan history (exclude soft-deleted books) — includes
// cancelled/expired loans, same predicate as the history list below.
$stmt = $db->prepare("SELECT COUNT(*) AS c FROM prestiti p JOIN libri l ON p.libro_id = l.id WHERE p.utente_id = ? AND p.attivo = 0 AND p.stato IN ('restituito','perso','danneggiato','annullato','scaduto') AND l.deleted_at IS NULL");
$stmt->bind_param('i', $userId);
$stmt->execute();
$res = $stmt->get_result();
Expand Down Expand Up @@ -181,15 +182,17 @@ public function prenotazioni(Request $request, Response $response, mysqli $db, m
}
$stmt->close();

// Past loans (completed)
// Past loans (completed) — includes cancelled/expired loans, which have
// no data_restituzione: order on the closing moment (updated_at) so a
// recent cancellation doesn't sink to the bottom of the list.
$stmt = $db->prepare("
SELECT pr.id, pr.libro_id, pr.data_prestito, pr.data_restituzione, pr.stato,
l.titolo, l.copertina_url,
EXISTS(SELECT 1 FROM recensioni r WHERE r.libro_id = pr.libro_id AND r.utente_id = ?) AS has_review
FROM prestiti pr
JOIN libri l ON l.id = pr.libro_id
WHERE pr.utente_id = ? AND pr.attivo = 0 AND pr.stato IN ('restituito','perso','danneggiato') AND l.deleted_at IS NULL
ORDER BY pr.data_restituzione DESC, pr.data_prestito DESC
WHERE pr.utente_id = ? AND pr.attivo = 0 AND pr.stato IN ('restituito','perso','danneggiato','annullato','scaduto') AND l.deleted_at IS NULL
ORDER BY COALESCE(pr.data_restituzione, pr.updated_at) DESC, pr.data_prestito DESC
LIMIT 50
");
$stmt->bind_param('ii', $userId, $userId);
Expand Down
20 changes: 11 additions & 9 deletions app/Support/IcsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ private function fetchEvents(): array
*/
private function getLoanTitle(string $status, string $bookTitle): string
{
// Divergenza voluta da translate_loan_status(): questi sono TITOLI di
// eventi calendario ("Prestito Scaduto", "Prestito Programmato"), non
// etichette di stato — le etichette passano da translateStatus() sotto.
$prefix = match($status) {
'in_corso' => '📖 ' . __('Prestito'),
'da_ritirare' => '📦 ' . __('Da Ritirare'),
Expand Down Expand Up @@ -272,15 +275,14 @@ private function getReservationDescription(array $row): string
*/
private function translateStatus(string $status): string
{
return match($status) {
'in_corso' => __('In corso'),
'da_ritirare' => __('Da Ritirare'),
'prenotato' => __('Programmato'),
'in_ritardo' => __('Scaduto'),
'pendente' => __('In attesa'),
'attiva' => __('Attiva'),
default => $status
};
// 'attiva' è lo stato delle prenotazioni (tabella prenotazioni), fuori
// dall'enum prestiti; tutti gli stati prestito passano dall'helper
// canonico translate_loan_status() (#333) — il vecchio alias "Scaduto"
// per in_ritardo collideva con lo stato 'scaduto' vero e proprio.
if ($status === 'attiva') {
return __('Attiva');
}
return translate_loan_status($status);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions app/Views/admin/integrity_report.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
?>
<!-- Report Integrità Dati -->
<div class="flex-1 overflow-x-hidden">
<!-- Page Header -->
<div class="bg-white/50 backdrop-blur-sm border-b border-gray-200/80 dark:bg-gray-900/50 dark:border-gray-800/80 sticky top-0 z-30">
<!-- Page Header — non sticky: con sticky top-0 z-30 (stesso z-index dell'header
del layout, ma successivo nel DOM) copriva l'header dell'app e il dropdown
delle notifiche durante lo scroll (#334). -->
<div class="bg-white/50 backdrop-blur-sm border-b border-gray-200/80 dark:bg-gray-900/50 dark:border-gray-800/80">
<div class="px-6 py-4">
<div class="flex items-center justify-between">
<div class="flex items-center space-x-3">
Expand Down
Loading
Loading