From 93198eda286bce8dde7bccf84ba429d2a558aa69 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Mon, 10 Aug 2026 13:50:14 +0200 Subject: [PATCH 01/69] =?UTF-8?q?fix:=20loan/reservation=20coherence=20aud?= =?UTF-8?q?it=20=E2=80=94=20clocks,=20availability,=20soft-delete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-domain coherence review of the loan/reservation system. Eleven fixes, each verified first-hand before changing anything: Soft-delete & data exposure - getBookAvailabilityData() now guards `deleted_at IS NULL` and returns null for missing/soft-deleted books; every caller (calendar route, localized availability route, disponibilita endpoint, mobile API) 404s cleanly. The localized public route used to serve real per-day occupancy for soft-deleted books to anonymous clients. One clock everywhere (app timezone via DateHelper) - calculateAvailability() default start was `new DateTime()` (process TZ): the mobile calendar began on "yesterday" between midnight and 2am app time. - DashboardStats mixed three clocks (SQL server date, process date(), DateHelper) for the same `data_prestito <= today` predicate. - DataIntegrity expired prenotazioni on server NOW() while ReservationManager used the app clock for the same rows. - User dashboard and pending-loans views computed overdue with time() (process TZ), flagging loans overdue on the afternoon of the due day — the cron only does so from the following day. - crea_prestito prefills now use the app today + the configured loan_duration_days (was process today + '+1 month'). Availability payload coherence - next_due on /api/books/{id}/availability filtered only `attivo = 1`: it could return the past due date of an overdue loan or the far-future date of a scheduled one. Now holding-states only and never in the past. - occupied_ranges on /api/libri/{id}/disponibilita omitted the pendente-with-copy arm and the active reservation queue, contradicting first_available computed per-day in the same response. - Web calendar routes now exclude the requesting user's own reservations (query param the mobile app and the server write gate already honoured), so the picker no longer paints red days the server would accept. Guards & lifecycle symmetry - PrestitiController::store() strict-validates both dates as ISO like update() does: the old strtotime guard was defeated by int<=false coercion on unparseable input, and '12/03/2026' parsed American-style. - LoanRepository::update() fallback honoured a hardcoded +14 days — half the seeded loan_duration_days default; now reads the setting. - rejectLoan now promotes the waitlist after freeing capacity (it was the only release path that left a freed copy idle until the next maintenance run) and flushes deferred notifications after commit. New regression guard: tests/loan-coherence-audit.unit.php (10 checks). Verified: PHPStan level 5 clean; 99 standalone unit tests pass; E2E loan-reservation-complete 26/26, loan-state-model + loan-overlap 43/43, full-test 137 passed / 8 install-skipped; soft-delete guard exercised live (200 → soft-delete 404 → restore 200). --- app/Controllers/LoanApprovalController.php | 20 +++ app/Controllers/PrestitiController.php | 23 +++- app/Controllers/ReservationsController.php | 23 +++- app/Models/DashboardStats.php | 12 +- app/Models/LoanRepository.php | 13 +- app/Routes/web.php | 73 +++++++++-- app/Support/DataIntegrity.php | 18 ++- app/Views/admin/pending_loans.php | 3 +- app/Views/prestiti/crea_prestito.php | 4 +- app/Views/prestiti/modifica_prestito.php | 2 +- app/Views/user_dashboard/index.php | 5 +- app/Views/user_dashboard/prenotazioni.php | 9 +- .../src/Controllers/CatalogController.php | 4 + tests/loan-coherence-audit.unit.php | 119 ++++++++++++++++++ 14 files changed, 301 insertions(+), 27 deletions(-) create mode 100644 tests/loan-coherence-audit.unit.php diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index ffd7f8e72..5cbbcfbea 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -679,8 +679,28 @@ public function rejectLoan(Request $request, Response $response, mysqli $db): Re throw new \RuntimeException('Failed to recalculate book availability'); } + // Promote the waitlist: a rejected reservation-conversion 'pendente' + // held a copy, and every other release path (return, cancel, expiry) + // immediately converts the next queued reservation — rejectLoan was + // the only one that left the freed capacity idle until the next + // maintenance run. processBookAvailability() is a no-op for bare + // pendings (nothing was occupied) and for soft-deleted books. + $reservationManager = new \App\Controllers\ReservationManager($db); + $reservationManager->setExternalTransaction(true); + for ($promoGuard = 0; $promoGuard < 1000 && $reservationManager->processBookAvailability($bookId); $promoGuard++) { + // keep promoting while freed capacity converts the next queued reservation + } + $db->commit(); + // Notifiche accodate durante la transazione esterna (P2): inviale ora + // che il commit è avvenuto, come fa MaintenanceService. + try { + $reservationManager->flushDeferredNotifications(); + } catch (\Throwable $flushError) { + \App\Support\SecureLogger::warning("[rejectLoan] Deferred notification flush failed: " . $flushError->getMessage()); + } + // Send notification AFTER successful commit (outside transaction) // Use pre-fetched data since loan is deleted try { diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 087b8d14f..368ea89e8 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -168,6 +168,16 @@ public function createForm(Request $request, Response $response, mysqli $db): Re } } + // Prefill delle date nel timezone APPLICATIVO e con la durata configurata: + // 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 = (int) ((new \App\Models\SettingsRepository($db))->get('loans', 'loan_duration_days', '30') ?? 30); + if ($defaultLoanDays < 1) { + $defaultLoanDays = 30; + } + $defaultDataScadenza = date('Y-m-d', strtotime($defaultDataPrestito . " +{$defaultLoanDays} days")); + ob_start(); require __DIR__ . '/../Views/prestiti/crea_prestito.php'; $content = ob_get_clean(); @@ -237,8 +247,19 @@ public function store(Request $request, Response $response, mysqli $db): Respons return $response->withHeader('Location', url('/admin/loans/create') . '?error=missing_fields')->withStatus(302); } + // Validazione ISO stretta di ENTRAMBE le date (stessa regola di update()): + // il vecchio guard `strtotime($a) <= strtotime($b)` con una data non + // parsabile confrontava int con false in modo booleano e PASSAVA, e + // l'ambiguità '12/03/2026' veniva letta all'americana (3 dicembre). + // L'input arriva libero (il campo è data-no-flatpickr), quindi qui è + // l'unico punto di difesa prima dell'INSERT. + if (!\App\Support\DateHelper::isISODateFormat($data_prestito) || !\App\Support\DateHelper::isISODateFormat($data_scadenza)) { + return $response->withHeader('Location', url('/admin/loans/create') . '?error=invalid_dates')->withStatus(302); + } + // Verifica che la data di scadenza sia successiva alla data di prestito - if (strtotime($data_scadenza) <= strtotime($data_prestito)) { + // (confronto lessicografico sicuro: entrambe validate Y-m-d qui sopra) + if ($data_scadenza <= $data_prestito) { return $response->withHeader('Location', url('/admin/loans/create') . '?error=invalid_dates')->withStatus(302); } diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index 7d3214ce7..322a1c394 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -104,7 +104,11 @@ public function getBookAvailability($request, $response, $args) private function calculateAvailability($currentLoans, $existingReservations, int $totalCopies, ?string $startDate = null, int $days = 730, ?int $excludeUserId = null) { - $start = $startDate ? new DateTime($startDate) : new DateTime(); // today by default + // Default start = "today" in the APP timezone (DateHelper), not the PHP + // process TZ (usually UTC): a bare `new DateTime()` made the mobile + // calendar (the only null-start caller) begin on "yesterday" between + // midnight and 2am Rome time, diverging from every web surface. + $start = new DateTime($startDate ?: \App\Support\DateHelper::today()); $start->setTime(0, 0, 0); // Normalize intervals (#157, model A-refined): @@ -501,8 +505,23 @@ public function createReservation($request, $response, $args) } } - public function getBookAvailabilityData($bookId, ?string $startDate = null, int $days = 730, ?int $excludeUserId = null) + /** + * Per-day availability payload for a book, or NULL when the book does not + * exist or is soft-deleted. Every caller must 404 on null: without this + * guard the method served real per-day occupancy for soft-deleted books + * (libri queries MUST honour deleted_at IS NULL). + */ + public function getBookAvailabilityData($bookId, ?string $startDate = null, int $days = 730, ?int $excludeUserId = null): ?array { + $bookStmt = $this->db->prepare("SELECT id FROM libri WHERE id = ? AND deleted_at IS NULL"); + $bookStmt->bind_param('i', $bookId); + $bookStmt->execute(); + $bookExists = $bookStmt->get_result()->fetch_assoc() !== null; + $bookStmt->close(); + if (!$bookExists) { + return null; + } + $totalCopies = $this->getBookTotalCopies($bookId); // Get current and future loans for this book. Approved states always diff --git a/app/Models/DashboardStats.php b/app/Models/DashboardStats.php index 4133595c2..44e96ecf8 100644 --- a/app/Models/DashboardStats.php +++ b/app/Models/DashboardStats.php @@ -14,6 +14,12 @@ public function counts(): array { $db = $this->db; return QueryCache::remember('dashboard_counts', function () use ($db): array { + // "Oggi" nel timezone APPLICATIVO (DateHelper), non la funzione data + // del server MySQL: il cron attiva i prestiti con DateHelper::today() e questa + // stessa card deve contare gli stessi ritiri pronti — con due orologi + // diversi, a cavallo della mezzanotte i due conteggi divergevano. + // Y-m-d validato da DateHelper: interpolazione sicura tra apici. + $today = \App\Support\DateHelper::today(); $sql = "SELECT (SELECT COUNT(*) FROM libri WHERE deleted_at IS NULL) AS libri, (SELECT COUNT(*) FROM utenti) AS utenti, @@ -22,7 +28,7 @@ public function counts(): array (SELECT COUNT(*) FROM prestiti WHERE stato = 'pendente') AS prestiti_pendenti, (SELECT COUNT(*) FROM prestiti WHERE stato = 'pendente' AND origine = 'prenotazione') AS ritiri_da_confermare, (SELECT COUNT(*) FROM prestiti WHERE stato = 'pendente' AND (origine = 'richiesta' OR origine IS NULL)) AS richieste_manuali, - (SELECT COUNT(*) FROM prestiti WHERE stato = 'da_ritirare' OR (stato = 'prenotato' AND data_prestito <= CURDATE())) AS pickup_pronti"; + (SELECT COUNT(*) FROM prestiti WHERE stato = 'da_ritirare' OR (stato = 'prenotato' AND data_prestito <= '{$today}')) AS pickup_pronti"; $result = $db->query($sql); if ($result && $row = $result->fetch_assoc()) { @@ -127,7 +133,7 @@ public function pendingLoans(int $limit = 4): array public function pickupReadyLoans(int $limit = 6): array { $rows = []; - $today = date('Y-m-d'); + $today = \App\Support\DateHelper::today(); $sql = "SELECT p.id, p.libro_id, p.utente_id, p.stato, p.data_prestito, p.data_scadenza, p.pickup_deadline, p.created_at, l.titolo, l.copertina_url, @@ -155,7 +161,7 @@ public function pickupReadyLoans(int $limit = 6): array public function scheduledLoans(int $limit = 6): array { $rows = []; - $today = date('Y-m-d'); + $today = \App\Support\DateHelper::today(); $sql = "SELECT p.id, p.libro_id, p.utente_id, p.stato, p.data_prestito, p.data_scadenza, p.created_at, l.titolo, l.copertina_url, diff --git a/app/Models/LoanRepository.php b/app/Models/LoanRepository.php index 1446edecf..e2cf93be0 100644 --- a/app/Models/LoanRepository.php +++ b/app/Models/LoanRepository.php @@ -93,7 +93,18 @@ public function update(int $id, array $data): bool $utente_id = (int) ($data['utente_id'] ?? 0); // "Oggi" nel timezone applicativo (M9): mai date() (TZ processo, spesso UTC). $data_prestito = $data['data_prestito'] ?? DateHelper::today(); - $data_scadenza = $data['data_scadenza'] ?? date('Y-m-d', strtotime(DateHelper::today() . ' +14 days')); + if (isset($data['data_scadenza'])) { + $data_scadenza = $data['data_scadenza']; + } else { + // Fallback dalla setting di durata prestito: il vecchio +14gg + // hardcoded era metà del default seminato (30) e ignorava la + // configurazione dell'admin — stesso fix M5b già applicato a renew(). + $loanDays = (int) ((new SettingsRepository($this->db))->get('loans', 'loan_duration_days', '30') ?? 30); + if ($loanDays < 1) { + $loanDays = 30; + } + $data_scadenza = date('Y-m-d', strtotime($data_prestito . " +{$loanDays} days")); + } $processed_by = $data['processed_by'] ?? null; $stmt->bind_param('issii', $utente_id, $data_prestito, $data_scadenza, $processed_by, $id); return $stmt->execute(); diff --git a/app/Routes/web.php b/app/Routes/web.php index df83d7ba3..10b674800 100644 --- a/app/Routes/web.php +++ b/app/Routes/web.php @@ -1962,10 +1962,18 @@ return $response->withStatus(404)->withHeader('Content-Type', 'application/json'); } $data['available'] = ($data['copies_available'] > 0); - // Next due date among active loans + // Next due date among HOLDING loans, never in the past. The bare + // `attivo = 1` version could return the past data_scadenza of an + // in_ritardo loan ("available again on ") or the + // far-future date of a scheduled prenotato — align the predicate with + // NotificationService::getNextAvailabilityDate. if (!$data['available']) { - $stmt = $db->prepare("SELECT MIN(data_scadenza) AS next_due FROM prestiti WHERE libro_id = ? AND attivo = 1"); - $stmt->bind_param('i', $bookId); + $today = \App\Support\DateHelper::today(); + $stmt = $db->prepare("SELECT MIN(data_scadenza) AS next_due FROM prestiti + WHERE libro_id = ? AND attivo = 1 + AND stato IN ('in_corso','in_ritardo','da_ritirare','prenotato') + AND data_scadenza >= ?"); + $stmt->bind_param('is', $bookId, $today); $stmt->execute(); $res = $stmt->get_result(); $row = $res->fetch_assoc(); @@ -1993,7 +2001,16 @@ if ($days > 180) $days = 180; $controller = new \App\Controllers\ReservationsController($db); - $availability = $controller->getBookAvailabilityData($bookId, \App\Support\DateHelper::today(), $days); + // Exclude the requesting user's own reservations, like the mobile + // calendar and the server-side write gate already do: without it the + // picker painted the user's own reserved days red while the server + // would have accepted the same dates. + $sessionUserId = isset($_SESSION['user']['id']) ? (int) $_SESSION['user']['id'] : null; + $availability = $controller->getBookAvailabilityData($bookId, \App\Support\DateHelper::today(), $days, $sessionUserId); + if ($availability === null) { + $response->getBody()->write(json_encode(['success' => false, 'message' => __('Libro non trovato')])); + return $response->withStatus(404)->withHeader('Content-Type', 'application/json'); + } $response->getBody()->write(json_encode([ 'total_copies' => $availability['total_copies'] ?? 0, @@ -2162,11 +2179,19 @@ return $response->withStatus(404)->withHeader('Content-Type', 'application/json'); } - // Intervalli occupati (per la visualizzazione) dai prestiti attivi + // Intervalli occupati (per la visualizzazione). Predicato HOLDING completo + // (#157): oltre agli stati attivi, anche il 'pendente' da conversione + // prenotazione che detiene già una copia. Senza questi due archi il + // payload si contraddiceva: occupied_ranges diceva "libero" mentre + // first_available/is_available_now (calcolati per-giorno qui sotto) + // contavano anche pendenti-con-copia e coda prenotazioni. $stmt = $db->prepare(" SELECT data_prestito, data_scadenza, stato FROM prestiti - WHERE libro_id = ? AND attivo = 1 AND stato IN ('in_corso', 'da_ritirare', 'prenotato', 'in_ritardo') + WHERE libro_id = ? AND ( + (attivo = 1 AND stato IN ('in_corso', 'da_ritirare', 'prenotato', 'in_ritardo')) + OR (attivo = 0 AND stato = 'pendente' AND copia_id IS NOT NULL) + ) ORDER BY data_prestito "); $stmt->bind_param('i', $libroId); @@ -2182,12 +2207,35 @@ } $stmt->close(); + // Anche la coda prenotazioni occupa il suo periodo promesso (stessa + // regola di CapacityService): catena COALESCE canonica per i bound. + $resStmt = $db->prepare(" + SELECT COALESCE(data_inizio_richiesta, DATE(data_scadenza_prenotazione)) AS r_start, + COALESCE(data_fine_richiesta, DATE(data_scadenza_prenotazione), data_inizio_richiesta) AS r_end + FROM prenotazioni + WHERE libro_id = ? AND stato = 'attiva' + ORDER BY queue_position ASC + "); + $resStmt->bind_param('i', $libroId); + $resStmt->execute(); + $resResult = $resStmt->get_result(); + while ($row = $resResult->fetch_assoc()) { + if (!empty($row['r_start'])) { + $occupiedRanges[] = [ + 'from' => $row['r_start'], + 'to' => $row['r_end'] ?? $row['r_start'], + 'stato' => 'prenotazione' + ]; + } + } + $resStmt->close(); + // first_available / is_available_now: delega al calcolo per-giorno e per-copia // (AVAIL-001). Il vecchio "giorno dopo la scadenza più lontana" ignorava le // copie multiple, restituendo una data troppo conservativa. $today = \App\Support\DateHelper::today(); $reservations = new \App\Controllers\ReservationsController($db); - $availability = $reservations->getBookAvailabilityData($libroId, $today, 180); + $availability = $reservations->getBookAvailabilityData($libroId, $today, 180) ?? []; $todayData = $availability['by_date'][$today] ?? null; $isAvailableNow = $todayData !== null && (int) ($todayData['available'] ?? 0) > 0; @@ -2465,12 +2513,19 @@ $db = $app->getContainer()->get('db'); $bookId = (int)$args['id']; $controller = new \App\Controllers\ReservationsController($db); - $availability = $controller->getBookAvailabilityData($bookId, \App\Support\DateHelper::today(), 180); + // Public endpoint (the book-page picker) — when a session exists, + // exclude the user's own reservations like the write gate does. + $sessionUserId = isset($_SESSION['user']['id']) ? (int) $_SESSION['user']['id'] : null; + $availability = $controller->getBookAvailabilityData($bookId, \App\Support\DateHelper::today(), 180, $sessionUserId); + if ($availability === null) { + $response->getBody()->write(json_encode(['success' => false, 'message' => __('Libro non trovato')])); + return $response->withStatus(404)->withHeader('Content-Type', 'application/json'); + } $data = [ 'success' => true, 'availability' => [ 'unavailable_dates' => $availability['unavailable_dates'] ?? [], - 'earliest_available' => $availability['earliest_available'] ?? date('Y-m-d'), + 'earliest_available' => $availability['earliest_available'] ?? \App\Support\DateHelper::today(), 'days' => $availability['days'] ?? [] ] ]; diff --git a/app/Support/DataIntegrity.php b/app/Support/DataIntegrity.php index 8f796ba47..9fbd6ba8b 100644 --- a/app/Support/DataIntegrity.php +++ b/app/Support/DataIntegrity.php @@ -554,14 +554,19 @@ public function verifyDataConsistency(): array { } $stmt->close(); - // 8. Verifica prenotazioni scadute ancora attive + // 8. Verifica prenotazioni scadute ancora attive. Orologio APPLICATIVO + // (DateHelper), non NOW() del server MySQL: ReservationManager:: + // cancelExpiredReservations valuta la stessa scadenza con DateHelper — + // due orologi diversi sulla stessa riga segnalavano/curavano in disaccordo. $stmt = $this->db->prepare(" SELECT id, libro_id, utente_id, data_scadenza_prenotazione FROM prenotazioni WHERE stato = 'attiva' AND data_scadenza_prenotazione IS NOT NULL - AND data_scadenza_prenotazione < NOW() + AND data_scadenza_prenotazione < ? "); + $appNow = \App\Support\DateHelper::now(); + $stmt->bind_param('s', $appNow); $stmt->execute(); $result = $stmt->get_result(); if ($result && $result->num_rows > 0) { @@ -1056,14 +1061,19 @@ public function fixDataInconsistencies(): array { $results['fixed'] += $this->db->affected_rows; $stmt->close(); - // 5. Annulla prenotazioni scadute (data_scadenza_prenotazione < NOW()) + // 5. Annulla prenotazioni scadute — orologio APPLICATIVO (DateHelper), + // stesso clock di ReservationManager::cancelExpiredReservations e del + // check n.8 sopra: con NOW() del server la stessa riga poteva risultare + // scaduta per un percorso e valida per l'altro a cavallo di mezzanotte. $stmt = $this->db->prepare(" UPDATE prenotazioni SET stato = 'annullata' WHERE stato = 'attiva' AND data_scadenza_prenotazione IS NOT NULL - AND data_scadenza_prenotazione < NOW() + AND data_scadenza_prenotazione < ? "); + $appNow = \App\Support\DateHelper::now(); + $stmt->bind_param('s', $appNow); $stmt->execute(); $results['fixed'] += $this->db->affected_rows; $stmt->close(); diff --git a/app/Views/admin/pending_loans.php b/app/Views/admin/pending_loans.php index 92b0b1bdb..e18b059f3 100644 --- a/app/Views/admin/pending_loans.php +++ b/app/Views/admin/pending_loans.php @@ -56,7 +56,8 @@
- + diff --git a/app/Views/prestiti/crea_prestito.php b/app/Views/prestiti/crea_prestito.php index bdcccd3cc..5b10364f2 100644 --- a/app/Views/prestiti/crea_prestito.php +++ b/app/Views/prestiti/crea_prestito.php @@ -174,14 +174,14 @@ class="inline-flex items-center gap-2 whitespace-nowrap px-4 py-2 bg-gray-100 te
- +
- +
diff --git a/app/Views/prestiti/modifica_prestito.php b/app/Views/prestiti/modifica_prestito.php index 68b1cc83a..86fb21ace 100644 --- a/app/Views/prestiti/modifica_prestito.php +++ b/app/Views/prestiti/modifica_prestito.php @@ -88,7 +88,7 @@ class="rounded-lg border border-gray-300 bg-white px-4 py-2 text-gray-900 focus: diff --git a/app/Views/user_dashboard/index.php b/app/Views/user_dashboard/index.php index f75e486f9..f6edde0a2 100644 --- a/app/Views/user_dashboard/index.php +++ b/app/Views/user_dashboard/index.php @@ -444,7 +444,10 @@ 'autore' => '' ]); $scadenza = strtotime($prestito['data_scadenza'] ?? ''); - $oggi = time(); + // Mezzanotte di "oggi" nel timezone applicativo: con time() (TZ + // processo) il conteggio dei giorni cambiava a mezzanotte UTC, + // in disaccordo col cron (updateOverdueLoans usa DateHelper). + $oggi = strtotime(\App\Support\DateHelper::today()); if ($scadenza === false || $scadenza === 0) { $giorni_rimanenti = 0; $scaduto = false; diff --git a/app/Views/user_dashboard/prenotazioni.php b/app/Views/user_dashboard/prenotazioni.php index a3bbae2a7..9c6c55a93 100644 --- a/app/Views/user_dashboard/prenotazioni.php +++ b/app/Views/user_dashboard/prenotazioni.php @@ -538,10 +538,15 @@ function accountLineIcon(string $name): string { diff --git a/storage/plugins/mobile-api/src/Controllers/CatalogController.php b/storage/plugins/mobile-api/src/Controllers/CatalogController.php index 8772dcd21..a2147ed0f 100644 --- a/storage/plugins/mobile-api/src/Controllers/CatalogController.php +++ b/storage/plugins/mobile-api/src/Controllers/CatalogController.php @@ -732,6 +732,10 @@ public function bookAvailability( $avail = (new \App\Controllers\ReservationsController($this->db)) ->getBookAvailabilityData($bookId, null, 180, $userId > 0 ? $userId : null); + if ($avail === null) { + // Soft-deleted between the fetchBookCore() guard above and here. + return ResponseEnvelope::error($response, 'not_found', __('Libro non trovato.'), 404); + } return ResponseEnvelope::success($response, [ 'total_copies' => (int) ($avail['total_copies'] ?? 0), diff --git a/tests/loan-coherence-audit.unit.php b/tests/loan-coherence-audit.unit.php new file mode 100644 index 000000000..9546819ee --- /dev/null +++ b/tests/loan-coherence-audit.unit.php @@ -0,0 +1,119 @@ += today (it could return the past due date of an + * in_ritardo loan). + * 4. DashboardStats must use the app clock, never CURDATE()/date('Y-m-d') + * (three different "todays" on the same dashboard). + * 5. LoanRepository::update() fallback must honour loan_duration_days + * (hardcoded '+14 days' halved the seeded 30-day default). + * 6. rejectLoan must promote the waitlist after freeing capacity, like every + * other release path, and flush deferred notifications after commit. + * 7. DataIntegrity must expire prenotazioni on the app clock, not NOW() + * (ReservationManager uses DateHelper for the same rows). + * 8. PrestitiController::store() must strict-validate both dates as ISO + * (the old strtotime guard passed unparseable input via int<=false). + * 9. The occupied_ranges payload must include the pendente-with-copy arm + * and the active reservation queue (it contradicted first_available). + * 10. User dashboard overdue displays must use the app clock, matching the + * cron's `data_scadenza < today` semantics. + */ + +$root = dirname(__DIR__); + +$reservations = (string) file_get_contents($root . '/app/Controllers/ReservationsController.php'); +$web = (string) file_get_contents($root . '/app/Routes/web.php'); +$dashboard = (string) file_get_contents($root . '/app/Models/DashboardStats.php'); +$loanRepo = (string) file_get_contents($root . '/app/Models/LoanRepository.php'); +$approval = (string) file_get_contents($root . '/app/Controllers/LoanApprovalController.php'); +$integrity = (string) file_get_contents($root . '/app/Support/DataIntegrity.php'); +$prestiti = (string) file_get_contents($root . '/app/Controllers/PrestitiController.php'); +$prenotView = (string) file_get_contents($root . '/app/Views/user_dashboard/prenotazioni.php'); +$indexView = (string) file_get_contents($root . '/app/Views/user_dashboard/index.php'); +$pendingView = (string) file_get_contents($root . '/app/Views/admin/pending_loans.php'); + +$checks = []; + +// 1. Soft-delete guard in the availability data provider (+ nullable contract). +$checks['getBookAvailabilityData guards deleted_at and returns ?array'] = + str_contains($reservations, 'SELECT id FROM libri WHERE id = ? AND deleted_at IS NULL') + && preg_match('/function getBookAvailabilityData\([^)]*\):\s*\?array/', $reservations) === 1; + +// 2. App-timezone default start in calculateAvailability. +$checks['calculateAvailability defaults start to DateHelper::today()'] = + str_contains($reservations, 'new DateTime($startDate ?: \App\Support\DateHelper::today())') + && !preg_match('/\$start = \$startDate \? new DateTime\(\$startDate\) : new DateTime\(\);/', $reservations); + +// 3. next_due: holding states only, never in the past. +$nextDuePos = strpos($web, 'AS next_due FROM prestiti'); +$checks['next_due filters holding states and >= today'] = + $nextDuePos !== false + && str_contains(substr($web, $nextDuePos, 400), "stato IN ('in_corso','in_ritardo','da_ritirare','prenotato')") + && str_contains(substr($web, $nextDuePos, 400), 'data_scadenza >= ?'); + +// 4. DashboardStats: one clock (DateHelper), no CURDATE()/process date(). +$checks['DashboardStats uses only the app clock'] = + !str_contains($dashboard, 'CURDATE()') + && !str_contains($dashboard, "date('Y-m-d')") + && substr_count($dashboard, 'DateHelper::today()') >= 3; + +// 5. LoanRepository::update() fallback honours the configured duration. +$checks['LoanRepository::update fallback reads loan_duration_days'] = + !str_contains($loanRepo, "+14 days") + && str_contains($loanRepo, "get('loans', 'loan_duration_days', '30')"); + +// 6. rejectLoan promotes the waitlist + flushes deferred notifications. +$rejectPos = strpos($approval, 'function rejectLoan'); +$rejectBody = $rejectPos !== false ? substr($approval, $rejectPos, 10000) : ''; +$checks['rejectLoan promotes waitlist and flushes notifications'] = + str_contains($rejectBody, 'processBookAvailability($bookId)') + && str_contains($rejectBody, 'flushDeferredNotifications()'); + +// 7. DataIntegrity expires prenotazioni on the app clock. +$checks['DataIntegrity uses app clock for reservation expiry'] = + !preg_match('/data_scadenza_prenotazione < NOW\(\)/', $integrity) + && substr_count($integrity, 'data_scadenza_prenotazione < ?') >= 2; + +// 8. store() strict-validates both dates (same rule as update()). +$storePos = strpos($prestiti, 'public function store('); +$storeBody = $storePos !== false ? substr($prestiti, $storePos, 4000) : ''; +$checks['store() ISO-validates data_prestito and data_scadenza'] = + substr_count($storeBody, 'DateHelper::isISODateFormat') >= 2 + && !str_contains($storeBody, 'strtotime($data_scadenza) <= strtotime($data_prestito)'); + +// 9. occupied_ranges: pendente-with-copy arm + reservation queue included. +$rangesPos = strpos($web, 'Intervalli occupati'); +$rangesBody = $rangesPos !== false ? substr($web, $rangesPos, 2500) : ''; +$checks['occupied_ranges includes pendente-with-copy and prenotazioni'] = + str_contains($rangesBody, "stato = 'pendente' AND copia_id IS NOT NULL") + && str_contains($rangesBody, "FROM prenotazioni"); + +// 10. Views: overdue/day-count displays on the app clock. +$checks['user dashboard views use the app clock for overdue'] = + !preg_match('/strtotime\(\$dueAt\) < time\(\)/', $prenotView) + && !preg_match('/strtotime\(\$scadenza\) < time\(\)/', $prenotView) + && str_contains($indexView, 'strtotime(\App\Support\DateHelper::today())') + && str_contains($pendingView, 'DateHelper::today()'); + +$failed = 0; +foreach ($checks as $label => $ok) { + echo ($ok ? '[PASS] ' : '[FAIL] ') . $label . "\n"; + if (!$ok) { + $failed++; + } +} +echo $failed === 0 ? "\nOK\n" : "\n{$failed} FAILED\n"; +exit($failed === 0 ? 0 : 1); From 620636c175cf3f0aa18e1f146962346f1c7affab Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Mon, 10 Aug 2026 20:26:48 +0200 Subject: [PATCH 02/69] =?UTF-8?q?fix:=20loan=20lifecycle=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20#301=20modal=20path,=20real=20timezone=20setting,?= =?UTF-8?q?=20reject=20audit,=20renewal=20email?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups agreed after the coherence audit, plus the second report on #301. Every fix was verified behaviourally and the whole branch diff went through an adversarial multi-agent review whose three confirmed findings are fixed here too. Auto-approval on the real entry point (#301) - The book-detail modal posts to ReservationsController::createReservation, which never consulted `auto_approve_requests` — the option only worked on the other (form) entry point, so real users' requests always landed in the approval queue. The modal path now promotes through the same race-safe canonical approval pipeline; with the option on, an available-copy request lands directly in "waiting for pickup" (pickup confirmation deliberately stays). The modal SweetAlert now branches on auto_approved instead of telling the user to wait for an approval that already happened. - New behavioural test drives the real endpoint with the option off and on (10 assertions: off → pending preserved; on → approved, copy assigned, pickup deadline set). app.timezone becomes a real setting - ConfigStore default + per-locale installer seeds (it→Rome, de→Berlin, fr→Paris, da→Copenhagen, en_US→UTC) + a validated select in the loans settings tab (DateTimeZone::listIdentifiers whitelist; invalid input is ignored). The adversarial review caught that loadDatabaseSettings()'s app-category allowlist never mapped the row back — without that mapping the whole feature was inert. Verified end-to-end over HTTP: save Copenhagen → the select reads back Copenhagen; existing installs without the row keep the previous Europe/Rome behaviour. Reject with audit instead of DELETE - rejectLoan was the only terminal transition that destroyed its row. It now marks the request annullato with processed_by + reason note, preserving history and statistics; the affected_rows race guard and the duplicate-check semantics (user can re-request) are unchanged. Renewal confirmation email - New loan_renewed template (base + the 4 locale overrides) and sendLoanRenewedNotification, fired post-commit by renew(): the borrower now learns the new due date even when the librarian renews at the desk. Verified live via Mailpit (renewal moved the due date and delivered the email with the new date and remaining renewals). Availability badge copy - "Non Disponibile" → "Non disponibile oggi" on both the hero badge and the sidebar state (the review caught the sidebar leftover): the aggregate badge is a today snapshot and no longer reads as contradicting the calendar's free future days. i18n: 8 new keys in all 5 locales (parity 6774 everywhere); the mail-template count check now derives from the base catalogue instead of hardcoding 22. Verified: PHPStan level 5 clean; 100 standalone unit tests pass (incl. the new reservation-path #301 test and 21 coherence guards); E2E loan suites 69/69 and full-test 137 passed / 8 install-skipped; timezone save/read-back and renewal email exercised live over HTTP. --- app/Controllers/LoanApprovalController.php | 21 +- app/Controllers/PrestitiController.php | 9 + app/Controllers/ReservationsController.php | 76 ++++++- app/Controllers/SettingsController.php | 15 +- app/Support/ConfigStore.php | 14 ++ app/Support/NotificationService.php | 42 ++++ app/Support/SettingsMailTemplates.php | 18 ++ app/Support/mail_templates/da_DK.php | 13 ++ app/Support/mail_templates/de_DE.php | 13 ++ app/Support/mail_templates/en_US.php | 13 ++ app/Support/mail_templates/fr_FR.php | 13 ++ app/Views/frontend/book-detail.php | 14 +- app/Views/settings/loans-tab.php | 29 +++ installer/database/data_da_DK.sql | 1 + installer/database/data_de_DE.sql | 1 + installer/database/data_en_US.sql | 1 + installer/database/data_fr_FR.sql | 1 + installer/database/data_it_IT.sql | 1 + locale/da_DK.json | 10 +- locale/de_DE.json | 10 +- locale/en_US.json | 10 +- locale/fr_FR.json | 10 +- locale/it_IT.json | 10 +- ...uto-approval-301-reservation-path.unit.php | 198 ++++++++++++++++++ tests/loan-coherence-audit.unit.php | 92 ++++++++ tests/settings-orphans-hardening.unit.php | 7 +- 26 files changed, 618 insertions(+), 24 deletions(-) create mode 100644 tests/loan-auto-approval-301-reservation-path.unit.php diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 5cbbcfbea..07d32ff04 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -658,9 +658,24 @@ public function rejectLoan(Request $request, Response $response, mysqli $db): Re $userName = $loan['utente_nome']; $bookTitle = $loan['libro_titolo']; - // Delete the loan - $stmt = $db->prepare("DELETE FROM prestiti WHERE id = ? AND stato = 'pendente'"); - $stmt->bind_param('i', $loanId); + // Mark as annullato instead of deleting: the rejection was the only + // terminal transition that destroyed its row, leaving no audit of + // who rejected what and blinding the statistics. Same shape as the + // user cancel path (stato='annullato', attivo=0, processed_by, note); + // the duplicate-request checks ignore 'annullato', so the user can + // request the same book again. + $rejectedBy = isset($_SESSION['user']['id']) ? (int) $_SESSION['user']['id'] : null; + $rejectNote = "\n[Admin] " . __('Richiesta rifiutata'); + if (is_scalar($reason) && (string) $reason !== '') { + $rejectNote .= ': ' . (string) $reason; + } + $stmt = $db->prepare(" + UPDATE prestiti + SET stato = 'annullato', attivo = 0, processed_by = ?, + note = CONCAT(COALESCE(note, ''), ?), updated_at = NOW() + WHERE id = ? AND stato = 'pendente' + "); + $stmt->bind_param('isi', $rejectedBy, $rejectNote, $loanId); $stmt->execute(); if ($db->affected_rows === 0) { diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 368ea89e8..6743403da 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -1855,6 +1855,15 @@ public function renew(Request $request, Response $response, mysqli $db, int $id) $db->commit(); $_SESSION['success_message'] = __('Prestito rinnovato correttamente. Nuova scadenza: %s', format_date($newDueDate, false, '/')); + // Conferma al lettore con la NUOVA scadenza, DOPO il commit: prima + // il rinnovo era l'unica transizione benefica senza email — se lo + // faceva il bibliotecario al banco, l'utente non lo sapeva proprio. + try { + (new \App\Support\NotificationService($db))->sendLoanRenewedNotification($id, $maxRenewals); + } catch (\Throwable $notifError) { + SecureLogger::warning(__('Notifica rinnovo prestito fallita'), ['loan_id' => $id, 'error' => $notifError->getMessage()]); + } + $successUrl = $redirectTo ?? url('/admin/loans'); $separator = strpos($successUrl, '?') === false ? '?' : '&'; return $response->withHeader('Location', url($successUrl . $separator . 'renewed=1'))->withStatus(302); diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index 322a1c394..3e93d0e9a 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -471,23 +471,38 @@ public function createReservation($request, $response, $args) $stmt->bind_param('iiss', $bookId, $userId, $startDate, $endDate); if ($stmt->execute()) { - $loanRequestId = $this->db->insert_id; + $loanRequestId = (int) $this->db->insert_id; $this->db->commit(); - // Send notification to admins - try { - $notificationService = new NotificationService($this->db); - $notificationService->notifyLoanRequest($loanRequestId); - } catch (\Throwable $notifError) { - \App\Support\SecureLogger::error('Error sending notification for loan request', ['error' => $notifError->getMessage()]); - // Don't fail the loan request creation if notification fails + // #301: honour the automatic-approval setting on THIS entry point + // too. The book-detail modal posts here, but the auto-approve + // lived only in UserActionsController::loan() — so real users' + // requests always landed in the admin approval queue even with + // the option enabled. Same race-safe canonical pipeline: a + // failure deliberately leaves the request pending for an admin. + $autoApproved = $this->autoApproveLoanRequest($request, $loanRequestId); + + if (!$autoApproved) { + // Send notification to admins (an auto-approved request no + // longer needs admin action — the old "new request" email + // would carry a stale approval link). + try { + $notificationService = new NotificationService($this->db); + $notificationService->notifyLoanRequest($loanRequestId); + } catch (\Throwable $notifError) { + \App\Support\SecureLogger::error('Error sending notification for loan request', ['error' => $notifError->getMessage()]); + // Don't fail the loan request creation if notification fails + } } $response->getBody()->write(json_encode([ 'success' => true, - 'message' => __('Richiesta di prestito inviata con successo'), + 'message' => $autoApproved + ? __('Prestito approvato - in attesa di ritiro') + : __('Richiesta di prestito inviata con successo'), 'loan_request_id' => $loanRequestId, - 'status' => 'pending_approval' + 'auto_approved' => $autoApproved, + 'status' => $autoApproved ? 'approved' : 'pending_approval' ])); return $response->withHeader('Content-Type', 'application/json'); } else { @@ -505,6 +520,47 @@ public function createReservation($request, $response, $args) } } + /** + * Promote a newly-created request through the canonical approval pipeline + * when the automatic-approval setting is on (#301). Mirrors + * UserActionsController::autoApproveLoanRequest — a failure deliberately + * leaves the request pending so an administrator can still process it. + */ + private function autoApproveLoanRequest($request, int $loanId): bool + { + $settings = new \App\Models\SettingsRepository($this->db); + if (!$settings->autoApproveLoanRequests()) { + return false; + } + + try { + $approvalRequest = $request + ->withParsedBody(['loan_id' => $loanId]) + ->withAttribute('automatic_loan_approval', true); + $result = (new \App\Controllers\LoanApprovalController())->approveLoan( + $approvalRequest, + new \Slim\Psr7\Response(), + $this->db + ); + + if ($result->getStatusCode() >= 200 && $result->getStatusCode() < 300) { + return true; + } + + \App\Support\SecureLogger::warning('Automatic loan approval left request pending (createReservation)', [ + 'loan_id' => $loanId, + 'status' => $result->getStatusCode(), + ]); + } catch (\Throwable $e) { + \App\Support\SecureLogger::warning('Automatic loan approval failed; request left pending (createReservation)', [ + 'loan_id' => $loanId, + 'error' => $e->getMessage(), + ]); + } + + return false; + } + /** * Per-day availability payload for a book, or NULL when the book does not * exist or is soft-deleted. Every caller must 404 on null: without this diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index b7d9f4399..4cc0d72e1 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -1283,7 +1283,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} + * @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, app_timezone: string} */ private function resolveLoansSettings(SettingsRepository $repository): array { @@ -1294,6 +1294,8 @@ 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(), + // App-wide clock for due dates and automatisms (DateHelper reads it). + 'app_timezone' => (string) \App\Support\ConfigStore::get('app.timezone', 'Europe/Rome'), ]; } @@ -1328,6 +1330,17 @@ public function updateLoansSettings(Request $request, Response $response, mysqli $repository->set('loans', 'max_loan_duration_days', (string) $maxLoanDuration); $repository->set('loans', 'auto_approve_requests', $autoApprove ? '1' : '0'); + // App timezone: DateHelper computes the loan clock ("today"/"now") from + // this. Validate against the canonical identifier list — an invalid or + // missing value leaves the stored setting untouched (the DateHelper + // fallback ladder keeps working either way). + $timezone = isset($data['app_timezone']) && is_scalar($data['app_timezone']) + ? trim((string) $data['app_timezone']) + : ''; + if ($timezone !== '' && in_array($timezone, \DateTimeZone::listIdentifiers(), true)) { + \App\Support\ConfigStore::set('app.timezone', $timezone); + } + $_SESSION['success_message'] = __('Impostazioni prestiti aggiornate correttamente.'); return $response->withHeader('Location', url('/admin/settings?tab=loans'))->withStatus(302); } diff --git a/app/Support/ConfigStore.php b/app/Support/ConfigStore.php index 212e14acb..b2161e531 100644 --- a/app/Support/ConfigStore.php +++ b/app/Support/ConfigStore.php @@ -36,6 +36,12 @@ public static function all(): array 'logo' => '', 'footer_description' => 'Il tuo sistema Pinakes per catalogare, gestire e condividere la tua collezione libraria.', 'locale' => 'it_IT', + // App-wide timezone: DateHelper::today()/now() compute the loan + // clock from this. Was a phantom key (read but defined nowhere, + // always resolving to the hardcoded fallback) — now a real + // default, seeded per-locale by the installer and editable from + // the loans settings tab. + 'timezone' => 'Europe/Rome', 'social_facebook' => '', 'social_twitter' => '', 'social_instagram' => '', @@ -419,6 +425,14 @@ private static function loadDatabaseSettings(): array if (isset($raw['app']['locale'])) { self::$dbSettingsCache['app']['locale'] = (string) $raw['app']['locale']; } + // Load timezone (loan clock — DateHelper reads app.timezone). + // This mapping is what makes the setting REAL: without it the + // installer seed and the loans-tab save wrote a row that was + // never read back, and get('app.timezone') always returned the + // hardcoded default (caught by the adversarial review). + if (isset($raw['app']['timezone']) && $raw['app']['timezone'] !== '') { + self::$dbSettingsCache['app']['timezone'] = (string) $raw['app']['timezone']; + } // Load social links $socialKeys = ['social_facebook', 'social_twitter', 'social_instagram', 'social_linkedin', 'social_bluesky', 'social_telegram']; foreach ($socialKeys as $socialKey) { diff --git a/app/Support/NotificationService.php b/app/Support/NotificationService.php index c20c13028..d742e84cb 100644 --- a/app/Support/NotificationService.php +++ b/app/Support/NotificationService.php @@ -916,6 +916,48 @@ private function sendToAdmins(string $templateName, array $variables): bool { /** * Invia email di approvazione prestito all'utente */ + /** + * Renewal confirmation with the NEW due date. Without it the borrower — + * especially when the librarian renews at the desk — had no way to know + * the deadline moved. + */ + public function sendLoanRenewedNotification(int $loanId, int $maxRenewals): bool { + try { + $stmt = $this->db->prepare(" + SELECT p.*, l.titolo as libro_titolo, + CONCAT(u.nome, ' ', u.cognome) as utente_nome, u.email as utente_email + FROM prestiti p + JOIN libri l ON p.libro_id = l.id AND l.deleted_at IS NULL + JOIN utenti u ON p.utente_id = u.id + WHERE p.id = ? + "); + $stmt->bind_param('i', $loanId); + $stmt->execute(); + $result = $stmt->get_result(); + + if (!$loan = $result->fetch_assoc()) { + $stmt->close(); + return false; + } + $stmt->close(); + + $remaining = max(0, $maxRenewals - (int) ($loan['renewals'] ?? 0)); + + $variables = [ + 'utente_nome' => $loan['utente_nome'], + 'libro_titolo' => $loan['libro_titolo'], + 'data_fine' => $this->formatEmailDate($loan['data_scadenza']), + 'rinnovi_rimanenti' => (string) $remaining, + ]; + + return $this->sendWithRetry($loan['utente_email'], 'loan_renewed', $variables); + + } catch (\Throwable $e) { + SecureLogger::error("Failed to send loan renewed notification: " . $e->getMessage()); + return false; + } + } + public function sendLoanApprovedNotification(int $loanId): bool { try { $stmt = $this->db->prepare(" diff --git a/app/Support/SettingsMailTemplates.php b/app/Support/SettingsMailTemplates.php index 9780dd5f1..144c267ae 100644 --- a/app/Support/SettingsMailTemplates.php +++ b/app/Support/SettingsMailTemplates.php @@ -209,6 +209,24 @@ public static function all(?string $locale = null): array

Importante: Ricorda di restituire il libro entro la data di scadenza. Riceverai un promemoria alcuni giorni prima della scadenza.

Buona lettura!

+HTML, + ], + 'loan_renewed' => [ + 'label' => __('Prestito rinnovato'), + 'description' => __("Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza."), + 'subject' => '🔄 Il tuo prestito è stato rinnovato', + 'placeholders' => ['utente_nome', 'libro_titolo', 'data_fine', 'rinnovi_rimanenti'], + 'body' => <<<'HTML' +

Il tuo prestito è stato rinnovato

+

Ciao {{utente_nome}},

+

Il prestito del libro "{{libro_titolo}}" è stato rinnovato.

+

Dettagli del rinnovo:

+
    +
  • Nuova data di scadenza: {{data_fine}}
  • +
  • Rinnovi ancora disponibili: {{rinnovi_rimanenti}}
  • +
+

Importante: Ricorda di restituire il libro entro la nuova data di scadenza. Riceverai un promemoria alcuni giorni prima.

+

Buona lettura!

HTML, ], 'loan_rejected' => [ diff --git a/app/Support/mail_templates/da_DK.php b/app/Support/mail_templates/da_DK.php index fd54ec670..124ee1e0f 100644 --- a/app/Support/mail_templates/da_DK.php +++ b/app/Support/mail_templates/da_DK.php @@ -195,6 +195,19 @@

{{pickup_instructions}}

God fornøjelse med læsningen!

', + ], + 'loan_renewed' => [ + 'subject' => '🔄 Dit lån er blevet fornyet', + 'body' => '

Dit lån er blevet fornyet

+

Hej {{utente_nome}},

+

Lånet af bogen "{{libro_titolo}}" er blevet fornyet.

+

Detaljer om fornyelsen:

+
    +
  • Ny afleveringsdato: {{data_fine}}
  • +
  • Fornyelser stadig til rådighed: {{rinnovi_rimanenti}}
  • +
+

Vigtigt: Husk at aflevere bogen inden den nye afleveringsdato. Du modtager en påmindelse nogle dage før.

+

God læselyst!

', ], 'loan_rejected' => [ 'subject' => '❌ Din udlånsanmodning er ikke blevet godkendt', diff --git a/app/Support/mail_templates/de_DE.php b/app/Support/mail_templates/de_DE.php index cf68623c0..e5998cf50 100644 --- a/app/Support/mail_templates/de_DE.php +++ b/app/Support/mail_templates/de_DE.php @@ -185,6 +185,19 @@

📦 So funktioniert die Abholung

{{pickup_instructions}}

+

Viel Freude beim Lesen!

', + ], + 'loan_renewed' => [ + 'subject' => '🔄 Ihre Ausleihe wurde verlängert', + 'body' => '

Ihre Ausleihe wurde verlängert

+

Hallo {{utente_nome}},

+

Die Ausleihe des Buches "{{libro_titolo}}" wurde verlängert.

+

Details der Verlängerung:

+
    +
  • Neues Fälligkeitsdatum: {{data_fine}}
  • +
  • Noch verfügbare Verlängerungen: {{rinnovi_rimanenti}}
  • +
+

Wichtig: Bitte geben Sie das Buch bis zum neuen Fälligkeitsdatum zurück. Sie erhalten einige Tage vorher eine Erinnerung.

Viel Freude beim Lesen!

', ], 'loan_rejected' => [ diff --git a/app/Support/mail_templates/en_US.php b/app/Support/mail_templates/en_US.php index 2faec95a1..076c68490 100644 --- a/app/Support/mail_templates/en_US.php +++ b/app/Support/mail_templates/en_US.php @@ -185,6 +185,19 @@

📦 How to pick it up

{{pickup_instructions}}

+

Happy reading!

', + ], + 'loan_renewed' => [ + 'subject' => '🔄 Your loan has been renewed', + 'body' => '

Your loan has been renewed

+

Hi {{utente_nome}},

+

Your loan of "{{libro_titolo}}" has been renewed.

+

Renewal details:

+
    +
  • New due date: {{data_fine}}
  • +
  • Renewals still available: {{rinnovi_rimanenti}}
  • +
+

Important: Please return the book by the new due date. You will receive a reminder a few days before.

Happy reading!

', ], 'loan_rejected' => [ diff --git a/app/Support/mail_templates/fr_FR.php b/app/Support/mail_templates/fr_FR.php index 32d3a999d..998900fa1 100644 --- a/app/Support/mail_templates/fr_FR.php +++ b/app/Support/mail_templates/fr_FR.php @@ -185,6 +185,19 @@

📦 Comment récupérer votre livre

{{pickup_instructions}}

+

Bonne lecture !

', + ], + 'loan_renewed' => [ + 'subject' => '🔄 Votre prêt a été renouvelé', + 'body' => '

Votre prêt a été renouvelé

+

Bonjour {{utente_nome}},

+

Le prêt du livre "{{libro_titolo}}" a été renouvelé.

+

Détails du renouvellement :

+
    +
  • Nouvelle date d\'échéance : {{data_fine}}
  • +
  • Renouvellements encore disponibles : {{rinnovi_rimanenti}}
  • +
+

Important : Pensez à rendre le livre avant la nouvelle date d\'échéance. Vous recevrez un rappel quelques jours avant.

Bonne lecture !

', ], 'loan_rejected' => [ diff --git a/app/Views/frontend/book-detail.php b/app/Views/frontend/book-detail.php index 1af099695..2bfb5471b 100644 --- a/app/Views/frontend/book-detail.php +++ b/app/Views/frontend/book-detail.php @@ -1894,7 +1894,7 @@ class="book-cover-large img-fluid" ? ($book['copie_totali'] > 1 ? "{$book['copie_disponibili']}/{$book['copie_totali']} " . __("Disponibili") : __("Disponibile")) - : __("Non Disponibile") ?> + : __("Non disponibile oggi") /* lo snapshot è di OGGI: il calendario può mostrare giorni futuri liberi */ ?> @@ -2435,7 +2435,7 @@ class="status-badge bg-gray-100 text-gray-900 border px-3 py-2 no-underline keyw
- 0) ? __("Disponibile") : __("Non Disponibile") ?> + 0) ? __("Disponibile") : __("Non disponibile oggi") ?>
@@ -2805,6 +2805,11 @@ function setFavUI(isFav) { const successRangeTpl = %1$s al %2$s'), JSON_UNESCAPED_UNICODE | JSON_HEX_TAG); ?>; const successOneMonthTpl = %s per 1 mese'), JSON_UNESCAPED_UNICODE | JSON_HEX_TAG); ?>; const successFootnote = ; + // #301: con l'auto-approvazione attiva il server risponde auto_approved=true + // e il prestito è GIÀ in attesa di ritiro — il copy "appena sarà approvata" + // sarebbe falso e l'utente aspetterebbe un'approvazione già avvenuta. + const approvedTitle = ; + const approvedFootnote = ; async function updateReservationsBadge() { const badge = document.getElementById('nav-res-count'); @@ -3084,10 +3089,11 @@ function setFavUI(isFav) { ? successRangeTpl.replace('%1$s', formatDateIT(formValues.startDate)).replace('%2$s', formatDateIT(formValues.endDate)) : successOneMonthTpl.replace('%s', formatDateIT(formValues.startDate)); + const isAutoApproved = result.auto_approved === true; Swal.fire({ icon: 'success', - title: __('Richiesta Inviata!'), - html: `${successHtml}
${successFootnote}` + title: isAutoApproved ? approvedTitle : __('Richiesta Inviata!'), + html: `${successHtml}
${isAutoApproved ? approvedFootnote : successFootnote}` }); return; } else { diff --git a/app/Views/settings/loans-tab.php b/app/Views/settings/loans-tab.php index 8b1e4c3c3..db25b10f2 100644 --- a/app/Views/settings/loans-tab.php +++ b/app/Views/settings/loans-tab.php @@ -86,6 +86,35 @@ class="block w-32 rounded-xl border-gray-300 focus:border-gray-500 focus:ring-gr + +
+
+

+ + +

+

+
+
+
+ + +

+ + +

+
+
+
+
diff --git a/installer/database/data_da_DK.sql b/installer/database/data_da_DK.sql index e5ec2a39d..1d32f04e7 100644 --- a/installer/database/data_da_DK.sql +++ b/installer/database/data_da_DK.sql @@ -282,6 +282,7 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('app', 'logo_path', '', 'Sti til applikationens logo', NOW()), ('app', 'footer_description', 'Dit Pinakes-system til at katalogisere, administrere og dele din bogsamling.', 'Sidefodsbeskrivelse', NOW()), ('app', 'locale', 'da_DK', 'Default application locale', NOW()), +('app', 'timezone', 'Europe/Copenhagen', 'Application timezone (loan clock)', NOW()), ('app', 'social_facebook', '', 'Facebook-profil-URL', NOW()), ('app', 'social_twitter', '', 'Twitter-profil-URL', NOW()), ('app', 'social_instagram', '', 'Instagram-profil-URL', NOW()), diff --git a/installer/database/data_de_DE.sql b/installer/database/data_de_DE.sql index 428a34874..7b4a9e78c 100644 --- a/installer/database/data_de_DE.sql +++ b/installer/database/data_de_DE.sql @@ -229,6 +229,7 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('app', 'logo_path', '', 'Pfad zum Anwendungslogo', NOW()), ('app', 'footer_description', 'Ihr Pinakes-System zum Katalogisieren, Verwalten und Teilen Ihrer Büchersammlung.', 'Fußzeilenbeschreibung', NOW()), ('app', 'locale', 'de_DE', 'Standard-Spracheinstellung der Anwendung', NOW()), +('app', 'timezone', 'Europe/Berlin', 'Application timezone (loan clock)', NOW()), ('app', 'social_facebook', '', 'Facebook-Profil-URL', NOW()), ('app', 'social_twitter', '', 'Twitter-Profil-URL', NOW()), ('app', 'social_instagram', '', 'Instagram-Profil-URL', NOW()), diff --git a/installer/database/data_en_US.sql b/installer/database/data_en_US.sql index 8d0a9b0fe..1dc9282ed 100644 --- a/installer/database/data_en_US.sql +++ b/installer/database/data_en_US.sql @@ -229,6 +229,7 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('app', 'logo_path', '', 'Path to application logo', NOW()), ('app', 'footer_description', 'Your Pinakes system to catalog, manage, and share your book collection.', 'Footer description', NOW()), ('app', 'locale', 'en_US', 'Default application locale', NOW()), +('app', 'timezone', 'UTC', 'Application timezone (loan clock)', NOW()), ('app', 'social_facebook', '', 'Facebook profile URL', NOW()), ('app', 'social_twitter', '', 'Twitter profile URL', NOW()), ('app', 'social_instagram', '', 'Instagram profile URL', NOW()), diff --git a/installer/database/data_fr_FR.sql b/installer/database/data_fr_FR.sql index d919bac9a..925bcabfd 100644 --- a/installer/database/data_fr_FR.sql +++ b/installer/database/data_fr_FR.sql @@ -229,6 +229,7 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('app', 'logo_path', '', 'Chemin vers le logo de l''application', NOW()), ('app', 'footer_description', 'Votre système Pinakes pour cataloguer, gérer et partager votre collection de livres.', 'Description du pied de page', NOW()), ('app', 'locale', 'fr_FR', 'Paramètre de langue par défaut de l''application', NOW()), +('app', 'timezone', 'Europe/Paris', 'Application timezone (loan clock)', NOW()), ('app', 'social_facebook', '', 'URL du profil Facebook', NOW()), ('app', 'social_twitter', '', 'URL du profil Twitter', NOW()), ('app', 'social_instagram', '', 'URL du profil Instagram', NOW()), diff --git a/installer/database/data_it_IT.sql b/installer/database/data_it_IT.sql index 7e30eeaf8..253d47cb6 100644 --- a/installer/database/data_it_IT.sql +++ b/installer/database/data_it_IT.sql @@ -282,6 +282,7 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('app', 'logo_path', '', 'Path to application logo', NOW()), ('app', 'footer_description', 'Il tuo sistema Pinakes per catalogare, gestire e condividere la tua collezione libraria.', 'Footer description', NOW()), ('app', 'locale', 'it_IT', 'Default application locale', NOW()), +('app', 'timezone', 'Europe/Rome', 'Application timezone (loan clock)', NOW()), ('app', 'social_facebook', '', 'Facebook profile URL', NOW()), ('app', 'social_twitter', '', 'Twitter profile URL', NOW()), ('app', 'social_instagram', '', 'Instagram profile URL', NOW()), diff --git a/locale/da_DK.json b/locale/da_DK.json index 300785f3b..462a6e3f1 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -6764,5 +6764,13 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatisk adfærd: Hvis du indtaster kode i \"Analytisk JavaScript\" eller \"Marketing-JavaScript\", vil de tilhørende til/fra-knapper i Privatlivsindstillinger automatisk blive valgt.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Vigtigt: Du skal manuelt angive de cookies, som disse scripts sporer, på Cookie-siden for at overholde GDPR.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sproget er slettet, men en oversættelsesfil kunne ikke fjernes: den kan dukke op igen, hvis du genopretter sproget med samme kode." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sproget er slettet, men en oversættelsesfil kunne ikke fjernes: den kan dukke op igen, hvis du genopretter sproget med samme kode.", + "Fuso orario": "Tidszone", + "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.": "Tidszone der bruges til at beregne afleveringsfrister, aktiveringer og automatik for lån. Den skal svare til bibliotekets lokale tidszone.", + "Predefinito: Europe/Rome": "Standard: Europe/Rome", + "Prestito rinnovato": "Lån fornyet", + "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.": "Sendes til brugeren, når et lån fornyes, med den nye afleveringsdato.", + "Notifica rinnovo prestito fallita": "Meddelelse om fornyelse af lån mislykkedes", + "Non disponibile oggi": "Ikke tilgængelig i dag", + "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro.": "Din anmodning blev godkendt automatisk: du modtager en e-mail med afhentningsinstruktioner." } diff --git a/locale/de_DE.json b/locale/de_DE.json index e543c5fdb..8a5eec382 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6764,5 +6764,13 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatisches Verhalten: Wenn Sie Code in „Analyse-JavaScript“ oder „Marketing-JavaScript“ eingeben, werden die jeweiligen Schalter in Datenschutzeinstellungen automatisch aktiviert.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Wichtig: Sie müssen die von diesen Skripten erfassten Cookies manuell auf der Cookie-Seite auflisten, um die DSGVO einzuhalten.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sprache gelöscht, aber eine Übersetzungsdatei konnte nicht entfernt werden: Sie könnte wieder auftauchen, wenn Sie die Sprache mit demselben Code neu anlegen." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sprache gelöscht, aber eine Übersetzungsdatei konnte nicht entfernt werden: Sie könnte wieder auftauchen, wenn Sie die Sprache mit demselben Code neu anlegen.", + "Fuso orario": "Zeitzone", + "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.": "Zeitzone für die Berechnung von Fälligkeiten, Aktivierungen und Ausleih-Automatismen. Sie muss der Ortszeit Ihrer Bibliothek entsprechen.", + "Predefinito: Europe/Rome": "Standard: Europe/Rome", + "Prestito rinnovato": "Ausleihe verlängert", + "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.": "Wird an den Benutzer gesendet, wenn eine Ausleihe verlängert wird, mit dem neuen Fälligkeitsdatum.", + "Notifica rinnovo prestito fallita": "Benachrichtigung über die Verlängerung fehlgeschlagen", + "Non disponibile oggi": "Heute nicht verfügbar", + "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro.": "Ihre Anfrage wurde automatisch genehmigt: Sie erhalten eine E-Mail mit den Abholhinweisen." } diff --git a/locale/en_US.json b/locale/en_US.json index f6d73071e..60cb5ce15 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6764,5 +6764,13 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatic behaviour: If you enter code in \"Analytics JavaScript\" or \"Marketing JavaScript\", the respective toggles in Privacy Settings will be selected automatically.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important: You must manually list the cookies tracked by these scripts on the Cookie Page for GDPR compliance.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Language deleted, but a translation file could not be removed: it may reappear if you recreate the language with the same code." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Language deleted, but a translation file could not be removed: it may reappear if you recreate the language with the same code.", + "Fuso orario": "Time zone", + "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.": "Time zone used to compute due dates, activations and loan automatisms. It must match your library's local time zone.", + "Predefinito: Europe/Rome": "Default: Europe/Rome", + "Prestito rinnovato": "Loan renewed", + "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.": "Sent to the user when a loan is renewed, with the new due date.", + "Notifica rinnovo prestito fallita": "Loan renewal notification failed", + "Non disponibile oggi": "Not available today", + "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro.": "Your request was approved automatically: you will receive an email with pickup instructions." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 7751a5eac..bd213dfe4 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6764,5 +6764,13 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Comportement automatique : si vous saisissez du code dans « JavaScript analytique » ou « JavaScript marketing », les interrupteurs correspondants dans Paramètres de confidentialité seront automatiquement activés.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important : vous devez lister manuellement les cookies suivis par ces scripts sur la Page Cookies pour la conformité RGPD.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code.", + "Fuso orario": "Fuseau horaire", + "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.": "Fuseau horaire utilisé pour calculer les échéances, les activations et les automatismes des prêts. Il doit correspondre au fuseau horaire de votre bibliothèque.", + "Predefinito: Europe/Rome": "Par défaut : Europe/Rome", + "Prestito rinnovato": "Prêt renouvelé", + "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.": "Envoyé à l'utilisateur lorsqu'un prêt est renouvelé, avec la nouvelle date d'échéance.", + "Notifica rinnovo prestito fallita": "Échec de la notification de renouvellement du prêt", + "Non disponibile oggi": "Non disponible aujourd'hui", + "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro.": "Votre demande a été approuvée automatiquement : vous recevrez un e-mail avec les instructions de retrait." } diff --git a/locale/it_IT.json b/locale/it_IT.json index fdd0e35f5..51a9a220b 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6764,5 +6764,13 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.", + "Fuso orario": "Fuso orario", + "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.": "Fuso orario usato per calcolare scadenze, attivazioni e automatismi dei prestiti. Deve corrispondere al fuso orario della tua biblioteca.", + "Predefinito: Europe/Rome": "Predefinito: Europe/Rome", + "Prestito rinnovato": "Prestito rinnovato", + "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.": "Inviata all'utente quando un prestito viene rinnovato, con la nuova data di scadenza.", + "Notifica rinnovo prestito fallita": "Notifica rinnovo prestito fallita", + "Non disponibile oggi": "Non disponibile oggi", + "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro.": "La tua richiesta è stata approvata automaticamente: riceverai una email con le istruzioni per il ritiro." } diff --git a/tests/loan-auto-approval-301-reservation-path.unit.php b/tests/loan-auto-approval-301-reservation-path.unit.php new file mode 100644 index 000000000..f0a3a3a88 --- /dev/null +++ b/tests/loan-auto-approval-301-reservation-path.unit.php @@ -0,0 +1,198 @@ +set_charset('utf8mb4'); +} catch (Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — mandatory for this test: {$e->getMessage()}\n"); + exit(1); +} + +// Keep email out of the way (approveLoan sends the patron notification). +$db->query("UPDATE system_settings SET setting_value='mail' WHERE category='email' AND setting_key IN ('driver_mode','type')"); + +$run = substr(hash('sha256', uniqid((string) getmypid(), true)), 0, 10); +$titlePrefix = 'ZZRES301_' . $run; +$emailDomain = '@res301.test.local'; + +// Preserve the global auto-approve setting so the run doesn't leave it toggled. +$origAuto = null; +if ($r = $db->query("SELECT setting_value FROM system_settings WHERE category='loans' AND setting_key='auto_approve_requests'")) { + if ($row = $r->fetch_assoc()) { + $origAuto = $row['setting_value']; + } +} + +$cleanup = static function () use ($db, $titlePrefix, $emailDomain, $origAuto): void { + $titleLike = $titlePrefix . '%'; + $emailLike = '%' . $emailDomain; + foreach ([ + 'DELETE p FROM prestiti p JOIN libri l ON l.id = p.libro_id WHERE l.titolo LIKE ?', + 'DELETE r FROM prenotazioni r JOIN libri l ON l.id = r.libro_id WHERE l.titolo LIKE ?', + 'DELETE c FROM copie c JOIN libri l ON l.id = c.libro_id WHERE l.titolo LIKE ?', + 'DELETE FROM libri WHERE titolo LIKE ?', + ] as $sql) { + $stmt = $db->prepare($sql); + $stmt->bind_param('s', $titleLike); + $stmt->execute(); + $stmt->close(); + } + $stmt = $db->prepare('DELETE FROM utenti WHERE email LIKE ?'); + $stmt->bind_param('s', $emailLike); + $stmt->execute(); + $stmt->close(); + if ($origAuto === null) { + $db->query("DELETE FROM system_settings WHERE category='loans' AND setting_key='auto_approve_requests'"); + } else { + $stmt = $db->prepare("INSERT INTO system_settings (category, setting_key, setting_value) VALUES ('loans','auto_approve_requests',?) ON DUPLICATE KEY UPDATE setting_value = VALUES(setting_value)"); + $stmt->bind_param('s', $origAuto); + $stmt->execute(); + $stmt->close(); + } +}; + +$cleanup(); +set_exception_handler(static function (Throwable $e) use ($cleanup, $db): void { + try { $cleanup(); } catch (Throwable) {} + fwrite(STDERR, "FAIL: {$e->getMessage()}\n"); + $db->close(); + exit(1); +}); + +$pass = 0; +$check = static function (bool $ok, string $label) use (&$pass): void { + if (!$ok) { + throw new RuntimeException($label); + } + $pass++; + echo " OK {$label}\n"; +}; + +// ── Fixture helpers ───────────────────────────────────────────────────────── +$bookSeq = 0; +$makeBook = static function (int $copies) use ($db, $titlePrefix, $run, &$bookSeq): int { + $bookSeq++; + $title = $titlePrefix . '_' . $bookSeq; + $stmt = $db->prepare("INSERT INTO libri (titolo, stato, copie_totali, copie_disponibili) VALUES (?, 'disponibile', ?, ?)"); + $stmt->bind_param('sii', $title, $copies, $copies); + $stmt->execute(); + $bookId = (int) $db->insert_id; + $stmt->close(); + $copyRepo = new CopyRepository($db); + for ($i = 1; $i <= $copies; $i++) { + $copyRepo->create($bookId, 'ZZR301-' . $run . '-' . $bookSeq . '-' . $i, 'disponibile'); + } + return $bookId; +}; + +$userSeq = 0; +$makeUser = static function () use ($db, $run, $emailDomain, &$userSeq): int { + $userSeq++; + $card = 'ZZR301' . strtoupper($run) . $userSeq; + $email = $run . '-' . $userSeq . $emailDomain; + $password = password_hash('test', PASSWORD_BCRYPT); + $stmt = $db->prepare("INSERT INTO utenti (codice_tessera, nome, cognome, email, password, tipo_utente, stato) VALUES (?, 'Res', 'Test', ?, ?, 'standard', 'attivo')"); + $stmt->bind_param('sss', $card, $email, $password); + $stmt->execute(); + $id = (int) $db->insert_id; + $stmt->close(); + return $id; +}; + +$today = DateHelper::today(); +$due = (new DateTimeImmutable($today))->modify('+14 days')->format('Y-m-d'); + +// Drive the REAL createReservation with a JSON request + session user, exactly +// like the book-detail modal does (CSRF is middleware, not in the controller). +$callCreate = static function (int $bookId, int $userId) use ($db, $today, $due): array { + $_SESSION['user'] = ['id' => $userId, 'tipo_utente' => 'standard']; + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', '/api/libro/' . $bookId . '/reservation') + ->withHeader('Content-Type', 'application/x-www-form-urlencoded') + ->withParsedBody(['start_date' => $today, 'end_date' => $due]); + $controller = new ReservationsController($db); + $result = $controller->createReservation($request, new SlimResponse(), ['id' => $bookId]); + $payload = json_decode((string) $result->getBody(), true) ?: []; + unset($_SESSION['user']); + return ['status' => $result->getStatusCode(), 'payload' => $payload]; +}; + +$loanField = static function (int $loanId, string $col) use ($db) { + $stmt = $db->prepare("SELECT {$col} AS v FROM prestiti WHERE id = ?"); + $stmt->bind_param('i', $loanId); + $stmt->execute(); + $v = $stmt->get_result()->fetch_assoc()['v'] ?? null; + $stmt->close(); + return $v; +}; + +$settings = new SettingsRepository($db); + +// ── A. Setting OFF: the manual approval queue is preserved ────────────────── +echo "A. createReservation with auto_approve OFF\n"; +$settings->set('loans', 'auto_approve_requests', '0'); +$resOff = $callCreate($makeBook(1), $makeUser()); +$check($resOff['status'] === 200 && ($resOff['payload']['success'] ?? false) === true, '01 request accepted'); +$check(($resOff['payload']['status'] ?? '') === 'pending_approval', '02 response status = pending_approval'); +$check(($resOff['payload']['auto_approved'] ?? null) === false, '03 response auto_approved = false'); +$loanOffId = (int) ($resOff['payload']['loan_request_id'] ?? 0); +$check($loanOffId > 0 && $loanField($loanOffId, 'stato') === 'pendente', '04 loan stays pendente (manual workflow preserved)'); + +// ── B. Setting ON: the modal path now honours the option (#301) ───────────── +echo "B. createReservation with auto_approve ON\n"; +$settings->set('loans', 'auto_approve_requests', '1'); +$resOn = $callCreate($makeBook(1), $makeUser()); +$check($resOn['status'] === 200 && ($resOn['payload']['success'] ?? false) === true, '05 request accepted'); +$check(($resOn['payload']['auto_approved'] ?? null) === true, '06 response auto_approved = true'); +$check(($resOn['payload']['status'] ?? '') === 'approved', '07 response status = approved'); +$loanOnId = (int) ($resOn['payload']['loan_request_id'] ?? 0); +$check($loanOnId > 0 && $loanField($loanOnId, 'stato') === 'da_ritirare', "08 loan promoted to 'da_ritirare' (approval phase skipped, pickup preserved)"); +$check((int) $loanField($loanOnId, 'attivo') === 1 && $loanField($loanOnId, 'copia_id') !== null, '09 canonical pipeline assigned a copy and activated the loan'); +$check($loanField($loanOnId, 'pickup_deadline') !== null, '10 immediate auto-approved loan has a pickup deadline'); + +$cleanup(); +$db->close(); +echo "\n{$pass} checks passed\n"; +exit(0); diff --git a/tests/loan-coherence-audit.unit.php b/tests/loan-coherence-audit.unit.php index 9546819ee..65cb50042 100644 --- a/tests/loan-coherence-audit.unit.php +++ b/tests/loan-coherence-audit.unit.php @@ -108,6 +108,98 @@ && str_contains($indexView, 'strtotime(\App\Support\DateHelper::today())') && str_contains($pendingView, 'DateHelper::today()'); +// ── Follow-up fixes (agreed design decisions + #301 second report) ────────── + +// 11. #301: the book-detail modal path (createReservation) must honour the +// automatic-approval setting like UserActionsController::loan does. +$checks['createReservation honours auto_approve_requests (#301)'] = + str_contains($reservations, 'autoApproveLoanRequest($request, $loanRequestId)') + && str_contains($reservations, "'automatic_loan_approval'"); + +// 12. rejectLoan must keep an audit row (annullato), never DELETE it. +$checks['rejectLoan cancels with audit instead of deleting'] = + !str_contains($rejectBody, 'DELETE FROM prestiti') + && str_contains($rejectBody, "SET stato = 'annullato'") + && str_contains($rejectBody, 'processed_by'); + +// 13. Renewal confirmation email: template in base + all 4 locale overrides, +// NotificationService sender, and the post-commit call in renew(). +$mailBase = (string) file_get_contents($root . '/app/Support/SettingsMailTemplates.php'); +$notif = (string) file_get_contents($root . '/app/Support/NotificationService.php'); +$renewedEverywhere = str_contains($mailBase, "'loan_renewed'"); +foreach (['da_DK', 'de_DE', 'en_US', 'fr_FR'] as $mailLocale) { + $renewedEverywhere = $renewedEverywhere + && str_contains((string) file_get_contents($root . "/app/Support/mail_templates/{$mailLocale}.php"), "'loan_renewed'"); +} +$checks['loan_renewed template exists in base + 4 locale overrides'] = $renewedEverywhere; +$checks['renew() sends the renewal confirmation post-commit'] = + str_contains($notif, 'function sendLoanRenewedNotification') + && str_contains($prestiti, 'sendLoanRenewedNotification($id, $maxRenewals)'); + +// 14. app.timezone is a real setting: ConfigStore default, per-locale installer +// seed, loans-tab select, validated save. +$configStore = (string) file_get_contents($root . '/app/Support/ConfigStore.php'); +$loansTab = (string) file_get_contents($root . '/app/Views/settings/loans-tab.php'); +$settingsCtrl = (string) file_get_contents($root . '/app/Controllers/SettingsController.php'); +$seededEverywhere = true; +foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $seedLocale) { + $seededEverywhere = $seededEverywhere + && str_contains((string) file_get_contents($root . "/installer/database/data_{$seedLocale}.sql"), "('app', 'timezone'"); +} +$checks['app.timezone has a ConfigStore default and is seeded in all 5 installers'] = + str_contains($configStore, "'timezone' => 'Europe/Rome'") && $seededEverywhere; +$checks['loans settings expose and validate the timezone'] = + str_contains($loansTab, 'name="app_timezone"') + && str_contains($settingsCtrl, 'DateTimeZone::listIdentifiers()') + && str_contains($settingsCtrl, "ConfigStore::set('app.timezone'"); + +// 15. Book badge: the unavailable state is a TODAY snapshot — the copy must say +// so, or it reads as contradicting the calendar's free future days. +$bookDetail = (string) file_get_contents($root . '/app/Views/frontend/book-detail.php'); +$checks['book badge says "Non disponibile oggi" (today snapshot)'] = + str_contains($bookDetail, '__("Non disponibile oggi")'); + +// 16. i18n parity: the 5 locales share the same key count and all carry the +// new keys introduced by these fixes. +$parityOk = true; +$counts = []; +$mustHave = ['Fuso orario', 'Prestito rinnovato', 'Non disponibile oggi']; +foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $l10n) { + $decoded = json_decode((string) file_get_contents($root . "/locale/{$l10n}.json"), true); + if (!is_array($decoded)) { + $parityOk = false; + break; + } + $counts[] = count($decoded); + foreach ($mustHave as $mk) { + $parityOk = $parityOk && array_key_exists($mk, $decoded); + } +} +$checks['i18n parity: 5 locales aligned and carrying the new keys'] = + $parityOk && count(array_unique($counts)) === 1; + +// ── Adversarial-review findings (multi-agent workflow on this branch) ──────── + +// 17. ConfigStore must MAP the timezone row back from the DB: without the +// loadDatabaseSettings() entry the seeds + save wrote a row that was never +// read, and get('app.timezone') always returned the hardcoded default. +$checks['ConfigStore maps the app.timezone DB row into the cache'] = + str_contains($configStore, "raw['app']['timezone']"); + +// 18. Both availability labels on the book page must use the today-snapshot +// copy — the sidebar "Stato" said "Non Disponibile" while the hero badge +// said "Non disponibile oggi" for the same condition. +$checks['book page has no leftover "Non Disponibile" label'] = + !str_contains($bookDetail, '__("Non Disponibile")') + && substr_count($bookDetail, '__("Non disponibile oggi")') >= 2; + +// 19. The reservation modal must branch on the auto-approve response: with the +// option on, the loan is already approved and "waiting for approval" copy +// would be false (the user would wait for an approval that already happened). +$checks['reservation modal branches on result.auto_approved'] = + str_contains($bookDetail, 'result.auto_approved === true') + && str_contains($bookDetail, 'approvedFootnote'); + $failed = 0; foreach ($checks as $label => $ok) { echo ($ok ? '[PASS] ' : '[FAIL] ') . $label . "\n"; diff --git a/tests/settings-orphans-hardening.unit.php b/tests/settings-orphans-hardening.unit.php index 4d3b0599f..3520a9eaa 100644 --- a/tests/settings-orphans-hardening.unit.php +++ b/tests/settings-orphans-hardening.unit.php @@ -16,9 +16,14 @@ $checks['control characters rejected'] = HtmlHelper::sanitizePublicHttpUrl("https://example.org/\ncookies") === ''; $checks['relative URL rejected for admin-provided public link'] = HtmlHelper::sanitizePublicHttpUrl('/cookies') === ''; +// The template count is DERIVED from the base (it_IT) catalogue, never +// hardcoded: a hardcoded number goes stale and turns red on every legitimate +// new template (it broke at 22→23 when loan_renewed was added). +$baseTemplateCount = count(SettingsMailTemplates::all('it_IT')); +$checks['base catalogue is non-empty'] = $baseTemplateCount > 0; foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR'] as $locale) { $allTemplates = SettingsMailTemplates::all($locale); - $checks["all 22 templates exist for {$locale}"] = count($allTemplates) === 22; + $checks["all {$baseTemplateCount} templates exist for {$locale}"] = count($allTemplates) === $baseTemplateCount; $template = SettingsMailTemplates::get('user_registration_verification', $locale); $checks["verification template exists for {$locale}"] = is_array($template) && str_contains((string) ($template['body'] ?? ''), '{{sezione_verifica}}') From 547f70007fff046cff534d72ad8af38d583047af Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:22:48 +0000 Subject: [PATCH 03/69] fix(loans): admin UI fixes for issues #333, #334, #336 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #333 — a loan cancelled by the user (stato='annullato') showed as "Unknown" in every admin view and looked stuck: no badge/label switch handled the enum value. Render it everywhere (loans list SSR + DataTables, details page, user details, book page history), add an Annullato filter button and CSV-export state, and stop showing "not yet returned" for loans closed without a return (annullato/scaduto). #334 — the loans-overview and integrity-report page headers were sticky at top-0 z-30, the same z-index as the layout header but later in the DOM, so they painted over the app header and its notifications dropdown. Drop the sticky positioning. #336 — editing a loan's dates re-checked capacity over the WHOLE new window, so a commitment already coexisting with the current period (e.g. a queued reservation) bounced ANY date edit — even shortening — with no_copies_available. update() now checks only the newly-claimed date segments (renew()'s extension-window convention) and bulk extension checks the extension window only. Also surface clear error banners: /admin/loans explains capacity conflicts and renew() failures, and the admin book page now recognizes the 'extension_conflicts' / 'renewal_failed' keys renew() actually emits (it only knew the never emitted 'renewal_conflict'). New strings translated in all bundled locales. Closes #333, closes #334, closes #336. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Controllers/PrestitiController.php | 36 +++++-- app/Views/admin/integrity_report.php | 6 +- app/Views/admin/pending_loans.php | 6 +- app/Views/libri/scheda_libro.php | 16 +++ app/Views/prestiti/dettagli_prestito.php | 16 ++- app/Views/prestiti/index.php | 32 ++++++ app/Views/utenti/dettagli_utente.php | 6 ++ 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/issues-333-334-336.unit.php | 130 +++++++++++++++++++++++ 13 files changed, 252 insertions(+), 16 deletions(-) create mode 100644 tests/issues-333-334-336.unit.php diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 087b8d14f..3fcd0eccd 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -858,15 +858,35 @@ 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 (before the old start and/or + // after the old due date), like renew() checks just the extension 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 + // are already held by this loan; only the added days need free capacity. + $oldPrestito = (string) $current['data_prestito']; + $oldScadenza = (string) $current['data_scadenza']; + if ($newPrestito !== $oldPrestito || $newScadenza !== $oldScadenza) { + $claimedWindows = []; + if ($newPrestito < $oldPrestito) { + // Y-m-d strings compare correctly lexicographically. The inclusive + // boundary day (old start / old due) mirrors renew()'s convention. + $claimedWindows[] = [$newPrestito, min($oldPrestito, $newScadenza)]; + } + if ($newScadenza > $oldScadenza) { + $claimedWindows[] = [max($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); + } } } @@ -1575,7 +1595,11 @@ private function applyBulkLoanExtension( // 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: capacity is checked on the EXTENSION window only (current due date → + // new due date), the same convention as renew(). Checking the whole loan + // window re-counted commitments already coexisting with the current period, + // rejecting extensions that add no new conflict. + if (!$capacity->hasFreeCapacity($bookId, (string) $loan['data_scadenza'], $newDueDate, excludePrestitoId: $loanId)) { return null; } diff --git a/app/Views/admin/integrity_report.php b/app/Views/admin/integrity_report.php index 05a0930fc..7c9cc56d6 100644 --- a/app/Views/admin/integrity_report.php +++ b/app/Views/admin/integrity_report.php @@ -3,8 +3,10 @@ ?>
- -
+ +
diff --git a/app/Views/admin/pending_loans.php b/app/Views/admin/pending_loans.php index 92b0b1bdb..64f8ac1d8 100644 --- a/app/Views/admin/pending_loans.php +++ b/app/Views/admin/pending_loans.php @@ -1,7 +1,9 @@
- -
+ +
diff --git a/app/Views/libri/scheda_libro.php b/app/Views/libri/scheda_libro.php index e535ddcf5..64d434e84 100644 --- a/app/Views/libri/scheda_libro.php +++ b/app/Views/libri/scheda_libro.php @@ -66,8 +66,14 @@ echo __('Numero massimo di rinnovi raggiunto per questo prestito.'); break; case 'renewal_conflict': + case 'extension_conflicts': + // #336: renew() emette 'extension_conflicts' — il vecchio switch conosceva + // solo 'renewal_conflict' (mai emesso) e mostrava il messaggio generico. echo __('Impossibile rinnovare: un altro prestito o prenotazione occupa il periodo richiesto.'); break; + case 'renewal_failed': + echo __('Rinnovo non riuscito. Riprova.'); + break; default: echo __('Operazione non riuscita. Riprova.'); } @@ -1300,6 +1306,16 @@ class="text-red-600 hover:text-red-900 transition-colors" $statusIcon = 'fa-box'; $statusLabel = __('Da Ritirare'); break; + case 'annullato': + $statusClass = 'bg-gray-200 text-gray-700'; + $statusIcon = 'fa-ban'; + $statusLabel = __('Annullato'); + break; + case 'scaduto': + $statusClass = 'bg-gray-200 text-gray-700'; + $statusIcon = 'fa-calendar-times'; + $statusLabel = __('Scaduto'); + break; } ?> diff --git a/app/Views/prestiti/dettagli_prestito.php b/app/Views/prestiti/dettagli_prestito.php index 71dda48bd..2208c2244 100644 --- a/app/Views/prestiti/dettagli_prestito.php +++ b/app/Views/prestiti/dettagli_prestito.php @@ -12,6 +12,8 @@ function formatLoanStatus($status) { 'restituito' => __('Restituito'), 'perso' => __('Perso'), 'danneggiato' => __('Danneggiato'), + 'annullato' => __('Annullato'), + 'scaduto' => __('Scaduto'), default => __('Sconosciuto'), }; } @@ -91,7 +93,18 @@ function formatLoanStatus($status) {
- +
@@ -110,6 +123,7 @@ function formatLoanStatus($status) { 'in_corso' => 'bg-blue-100 text-blue-800', 'in_ritardo' => 'bg-yellow-100 text-yellow-800', 'perso', 'danneggiato' => 'bg-red-100 text-red-800', + 'annullato', 'scaduto' => 'bg-gray-200 text-gray-700', default => 'bg-gray-100 text-gray-800' }; ?>"> diff --git a/app/Views/prestiti/index.php b/app/Views/prestiti/index.php index 5b7308d70..a2211a71f 100644 --- a/app/Views/prestiti/index.php +++ b/app/Views/prestiti/index.php @@ -24,6 +24,8 @@ function getStatusBadge($status) { return "" . __("Danneggiato") . ""; case 'scaduto': return "" . __("Scaduto") . ""; + case 'annullato': + return "" . __("Annullato") . ""; default: return "" . __("Sconosciuto") . ""; } @@ -110,6 +112,29 @@ function getStatusBadge($status) { case 'loan_not_closable': echo __('Prestito non trovato o non chiudibile.'); break; + case 'no_copies_available': + // #336: dire solo "nessuna copia" era fuorviante — il vero motivo + // è un conflitto con un altro impegno nel periodo richiesto. + echo __('Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.'); + break; + case 'extension_conflicts': + echo __('Impossibile rinnovare: un altro prestito o prenotazione occupa il periodo richiesto.'); + break; + case 'loan_overdue': + echo __('Impossibile rinnovare: il prestito è in ritardo.'); + break; + case 'max_renewals': + echo __('Numero massimo di rinnovi raggiunto per questo prestito.'); + break; + case 'loan_not_active': + echo __('Il prestito non è più attivo.'); + break; + case 'loan_not_picked_up': + echo __('Impossibile rinnovare: il prestito non è ancora stato ritirato.'); + break; + case 'book_not_found': + echo __('Libro non trovato o non più disponibile.'); + break; default: echo __('Errore durante l\'aggiornamento del prestito.'); } @@ -313,6 +338,7 @@ class="inline-flex items-center gap-2 whitespace-nowrap px-4 py-2 bg-gray-100 te +
@@ -552,6 +578,8 @@ className: 'text-center', return ``; case 'scaduto': return ``; + case 'annullato': + return ``; default: return ``; } @@ -892,6 +920,10 @@ function applyChecked() { ${__('Scaduto')} +
`, showCancelButton: true, diff --git a/app/Views/utenti/dettagli_utente.php b/app/Views/utenti/dettagli_utente.php index 2efec0c1f..9c96fe53e 100644 --- a/app/Views/utenti/dettagli_utente.php +++ b/app/Views/utenti/dettagli_utente.php @@ -19,6 +19,12 @@ function getLoanStatusBadge($status) { return "" . __("Perso") . ""; case 'danneggiato': return "" . __("Danneggiato") . ""; + case 'da_ritirare': + return "" . __("Da Ritirare") . ""; + case 'scaduto': + return "" . __("Scaduto") . ""; + case 'annullato': + return "" . __("Annullato") . ""; default: return "" . __("Sconosciuto") . ""; } diff --git a/locale/da_DK.json b/locale/da_DK.json index 300785f3b..c78ef56c2 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -6764,5 +6764,7 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatisk adfærd: Hvis du indtaster kode i \"Analytisk JavaScript\" eller \"Marketing-JavaScript\", vil de tilhørende til/fra-knapper i Privatlivsindstillinger automatisk blive valgt.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Vigtigt: Du skal manuelt angive de cookies, som disse scripts sporer, på Cookie-siden for at overholde GDPR.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sproget er slettet, men en oversættelsesfil kunne ikke fjernes: den kan dukke op igen, hvis du genopretter sproget med samme kode." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sproget er slettet, men en oversættelsesfil kunne ikke fjernes: den kan dukke op igen, hvis du genopretter sproget med samme kode.", + "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Ændringen blev ikke gemt: I den nye periode er alle eksemplarer allerede optaget af andre udlån eller reservationer.", + "Rinnovo non riuscito. Riprova.": "Fornyelsen mislykkedes. Prøv igen." } diff --git a/locale/de_DE.json b/locale/de_DE.json index e543c5fdb..c0033319d 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6764,5 +6764,7 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatisches Verhalten: Wenn Sie Code in „Analyse-JavaScript“ oder „Marketing-JavaScript“ eingeben, werden die jeweiligen Schalter in Datenschutzeinstellungen automatisch aktiviert.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Wichtig: Sie müssen die von diesen Skripten erfassten Cookies manuell auf der Cookie-Seite auflisten, um die DSGVO einzuhalten.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sprache gelöscht, aber eine Übersetzungsdatei konnte nicht entfernt werden: Sie könnte wieder auftauchen, wenn Sie die Sprache mit demselben Code neu anlegen." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sprache gelöscht, aber eine Übersetzungsdatei konnte nicht entfernt werden: Sie könnte wieder auftauchen, wenn Sie die Sprache mit demselben Code neu anlegen.", + "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Änderung nicht gespeichert: Im neuen Zeitraum sind alle Exemplare bereits durch andere Ausleihen oder Vormerkungen belegt.", + "Rinnovo non riuscito. Riprova.": "Verlängerung fehlgeschlagen. Bitte erneut versuchen." } diff --git a/locale/en_US.json b/locale/en_US.json index f6d73071e..fe1dd1f03 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6764,5 +6764,7 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Automatic behaviour: If you enter code in \"Analytics JavaScript\" or \"Marketing JavaScript\", the respective toggles in Privacy Settings will be selected automatically.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important: You must manually list the cookies tracked by these scripts on the Cookie Page for GDPR compliance.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Language deleted, but a translation file could not be removed: it may reappear if you recreate the language with the same code." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Language deleted, but a translation file could not be removed: it may reappear if you recreate the language with the same code.", + "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Change not saved: in the new period every copy is already taken by other loans or reservations.", + "Rinnovo non riuscito. Riprova.": "Renewal failed. Please try again." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 7751a5eac..a6dc2417c 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6764,5 +6764,7 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Comportement automatique : si vous saisissez du code dans « JavaScript analytique » ou « JavaScript marketing », les interrupteurs correspondants dans Paramètres de confidentialité seront automatiquement activés.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important : vous devez lister manuellement les cookies suivis par ces scripts sur la Page Cookies pour la conformité RGPD.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code.", + "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Modification non enregistrée : sur la nouvelle période, tous les exemplaires sont déjà occupés par d'autres prêts ou réservations.", + "Rinnovo non riuscito. Riprova.": "Échec du renouvellement. Veuillez réessayer." } diff --git a/locale/it_IT.json b/locale/it_IT.json index fdd0e35f5..c210a4bf3 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6764,5 +6764,7 @@ "https://...": "https://...", "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.": "⚙️ Comportamento Automatico: Se inserisci codice in \"JavaScript Analitici\" o \"JavaScript Marketing\", i rispettivi toggle in Impostazioni Privacy verranno automaticamente selezionati.", "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.", - "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice." + "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.", + "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.", + "Rinnovo non riuscito. Riprova.": "Rinnovo non riuscito. Riprova." } diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php new file mode 100644 index 000000000..fdb645bc7 --- /dev/null +++ b/tests/issues-333-334-336.unit.php @@ -0,0 +1,130 @@ += 2, + 'loans list renders the annullato badge in both the SSR and DataTables paths' +); +$check( + str_contains($loansIndex, 'data-status="annullato"'), + 'loans list offers an Annullato status filter button' +); +$check( + str_contains($loansIndex, 'value="annullato"'), + 'CSV export dialog includes the annullato state' +); +$check( + str_contains($loanDetails, "'annullato' => __('Annullato')") + && str_contains($loanDetails, "'scaduto' => __('Scaduto')"), + 'loan details page labels annullato/scaduto instead of Sconosciuto' +); +$check( + str_contains($loanDetails, "['annullato', 'scaduto']"), + 'loan details page does not claim "not yet returned" for cancelled/expired loans' +); +$check( + str_contains($userDetails, "case 'annullato':") && str_contains($userDetails, "case 'da_ritirare':"), + 'user details page labels annullato (and da_ritirare) loans' +); +$check( + str_contains($bookPage, "case 'annullato':"), + 'admin book page loan history labels annullato loans' +); + +echo "== #334: page header no longer covers the notifications dropdown ==\n"; +// Match class attributes only — the explanatory comments in those views cite +// the removed utility string verbatim. +$check( + !preg_match('/class="[^"]*sticky top-0 z-30/', $pendingLoans), + 'loans overview page header is not sticky at the layout header z-index' +); +$check( + !preg_match('/class="[^"]*sticky top-0 z-30/', $integrityReport), + 'integrity report page header is not sticky at the layout header z-index' +); + +echo "== #336: date edits check only newly-claimed days; clear error messages ==\n"; +$updateStart = strpos($controller, 'public function update('); +$closeStart = strpos($controller, 'public function close('); +$updateSource = ($updateStart !== false && $closeStart !== false) + ? substr($controller, $updateStart, $closeStart - $updateStart) + : ''; +$check( + str_contains($updateSource, '$claimedWindows') + && str_contains($updateSource, 'excludePrestitoId: $id'), + 'update() checks capacity on the newly-claimed windows through CapacityService' +); +$check( + !str_contains($updateSource, 'hasFreeCapacity($libroId, $newPrestito, $newScadenza'), + 'update() no longer re-checks the whole loan window (which bounced every edit)' +); +$bulkStart = strpos($controller, 'private function applyBulkLoanExtension('); +$renewStart = strpos($controller, 'public function renew('); +$bulkSource = ($bulkStart !== false && $renewStart !== false) + ? substr($controller, $bulkStart, $renewStart - $bulkStart) + : ''; +$check( + str_contains($bulkSource, "hasFreeCapacity(\$bookId, (string) \$loan['data_scadenza'], \$newDueDate"), + 'bulk extension checks the extension window only, like renew()' +); +$check( + str_contains($loansIndex, "case 'no_copies_available':") + && str_contains($loansIndex, "case 'extension_conflicts':"), + 'loans list banner explains capacity conflicts instead of a generic error' +); +$check( + str_contains($bookPage, "case 'extension_conflicts':") + && str_contains($bookPage, "case 'renewal_failed':"), + "book page banner recognizes renew()'s actual error keys" +); + +// The new user-facing strings must be translated in every bundled locale. +$newStrings = [ + 'Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.', + 'Rinnovo non riuscito. Riprova.', +]; +foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $locale) { + $bundle = json_decode((string) file_get_contents($root . '/locale/' . $locale . '.json'), true); + $ok = is_array($bundle); + foreach ($newStrings as $key) { + $ok = $ok && isset($bundle[$key]) && $bundle[$key] !== ''; + } + $check($ok, "locale {$locale} translates the new loan error strings"); +} + +echo PHP_EOL . "Passed: {$passed}, Failed: {$failed}" . PHP_EOL; +exit($failed === 0 ? 0 : 1); From 59b06e23d21e07fc91455a9f9ee9c80c5f64266c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:35:08 +0000 Subject: [PATCH 04/69] refactor(loans): canonical status badge partial; show cancelled loans in user history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #333. Instead of five per-view copies of the stato → badge switch (the reason 'annullato' was missed in the first place), app/Views/partials/loan-status-badge.php is now the single source of truth for color/icon/label of every prestiti.stato value. All admin surfaces consume it: the loans list (both the SSR rows and the DataTables column, via a PHP-generated JS map), the loan details page, the user details page and the book page loan history. Labels come from the existing translate_loan_status() helper, already used by the PDF and CSV export, so text and badges cannot diverge. The partial lives under app/Views (not app/helpers.php) because Tailwind's content globs only scan app/Views/** — moving the class names out of the views would drop them from the compiled CSS. User-facing change: cancelled (annullato) and pickup-expired (scaduto) loans now appear in the user's loan history (profile + account dashboard + history counter) instead of vanishing. They sort by their closing time (COALESCE with updated_at) so a NULL return date doesn't sink them to the bottom, show a dedicated icon/label, and hide the "leave a review" button since the book never went out (the server would reject the review anyway). Regression guards extended accordingly (36 checks). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Controllers/UserActionsController.php | 9 ++- app/Controllers/UserDashboardController.php | 13 ++-- app/Views/libri/scheda_libro.php | 66 ++------------------- app/Views/partials/loan-status-badge.php | 63 ++++++++++++++++++++ app/Views/prestiti/dettagli_prestito.php | 33 ++--------- app/Views/prestiti/index.php | 66 ++++----------------- app/Views/profile/reservations.php | 16 ++++- app/Views/user_dashboard/prenotazioni.php | 14 ++++- app/Views/utenti/dettagli_utente.php | 33 ++--------- tests/issues-333-334-336.unit.php | 66 +++++++++++++++++---- 10 files changed, 181 insertions(+), 198 deletions(-) create mode 100644 app/Views/partials/loan-status-badge.php diff --git a/app/Controllers/UserActionsController.php b/app/Controllers/UserActionsController.php index b5be5d1d9..0951430e2 100644 --- a/app/Controllers/UserActionsController.php +++ b/app/Controllers/UserActionsController.php @@ -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, DATE(pr.updated_at)) DESC, pr.data_prestito DESC LIMIT 20"; $stmt = $db->prepare($sql); $stmt->bind_param('ii', $uid, $uid); diff --git a/app/Controllers/UserDashboardController.php b/app/Controllers/UserDashboardController.php index c26e9e463..f2e5f6346 100644 --- a/app/Controllers/UserDashboardController.php +++ b/app/Controllers/UserDashboardController.php @@ -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(); @@ -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, DATE(pr.updated_at)) DESC, pr.data_prestito DESC LIMIT 50 "); $stmt->bind_param('ii', $userId, $userId); diff --git a/app/Views/libri/scheda_libro.php b/app/Views/libri/scheda_libro.php index 64d434e84..9b53a6346 100644 --- a/app/Views/libri/scheda_libro.php +++ b/app/Views/libri/scheda_libro.php @@ -1,6 +1,9 @@ - - - - - + + 0): ?> diff --git a/app/Views/partials/loan-status-badge.php b/app/Views/partials/loan-status-badge.php new file mode 100644 index 000000000..5b37fdaec --- /dev/null +++ b/app/Views/partials/loan-status-badge.php @@ -0,0 +1,63 @@ + stato => badge HTML pronto da stampare + */ + function loan_status_badge_map(): array + { + $base = 'inline-flex items-center px-3 py-1 rounded-full text-xs font-medium'; + // stato => [classi colore Tailwind, icona FontAwesome] + $defs = [ + 'pendente' => ['bg-orange-100 text-orange-800', 'fa-hourglass-half'], + 'prenotato' => ['bg-purple-100 text-purple-800', 'fa-calendar-check'], + 'da_ritirare' => ['bg-amber-100 text-amber-800', 'fa-box'], + 'in_corso' => ['bg-blue-100 text-blue-800', 'fa-clock'], + 'in_ritardo' => ['bg-yellow-100 text-yellow-800', 'fa-exclamation-triangle'], + 'restituito' => ['bg-green-100 text-green-800', 'fa-check-circle'], + 'perso' => ['bg-red-100 text-red-800', 'fa-times-circle'], + 'danneggiato' => ['bg-red-100 text-red-800', 'fa-times-circle'], + 'scaduto' => ['bg-gray-200 text-gray-700', 'fa-calendar-times'], + 'annullato' => ['bg-gray-200 text-gray-700', 'fa-ban'], + ]; + $map = []; + foreach ($defs as $stato => [$colors, $icon]) { + $map[$stato] = '' + . htmlspecialchars(translate_loan_status($stato), ENT_QUOTES, 'UTF-8') . ''; + } + return $map; + } +} + +if (!function_exists('loan_status_badge')) { + /** + * Badge HTML per un singolo stato; fallback "Sconosciuto" per valori + * fuori enum (non dovrebbe più accadere: la mappa copre tutto l'enum). + */ + function loan_status_badge(?string $stato): string + { + if ($stato !== null && $stato !== '') { + $map = loan_status_badge_map(); + if (isset($map[$stato])) { + return $map[$stato]; + } + } + return '' + . htmlspecialchars(__('Sconosciuto'), ENT_QUOTES, 'UTF-8') . ''; + } +} diff --git a/app/Views/prestiti/dettagli_prestito.php b/app/Views/prestiti/dettagli_prestito.php index 2208c2244..b15ccba21 100644 --- a/app/Views/prestiti/dettagli_prestito.php +++ b/app/Views/prestiti/dettagli_prestito.php @@ -1,22 +1,9 @@ __('In Attesa di Approvazione'), - 'prenotato' => __('Prenotato'), - 'da_ritirare' => __('Da Ritirare'), - 'in_corso' => __('In Corso'), - 'in_ritardo' => __('In Ritardo'), - 'restituito' => __('Restituito'), - 'perso' => __('Perso'), - 'danneggiato' => __('Danneggiato'), - 'annullato' => __('Annullato'), - 'scaduto' => __('Scaduto'), - default => __('Sconosciuto'), - }; -} +// Badge canonico degli stati prestito (#333): colore/icona/etichetta arrivano +// dal partial condiviso, niente mappa locale da tenere allineata. +require_once __DIR__ . '/../partials/loan-status-badge.php'; ?>
@@ -114,19 +101,7 @@ function formatLoanStatus($status) {
- +
diff --git a/app/Views/prestiti/index.php b/app/Views/prestiti/index.php index a2211a71f..151faaefb 100644 --- a/app/Views/prestiti/index.php +++ b/app/Views/prestiti/index.php @@ -1,35 +1,9 @@ " . __("Pendente") . ""; - case 'prenotato': - return "" . __("Prenotato") . ""; - case 'da_ritirare': - return "" . __("Da Ritirare") . ""; - case 'in_corso': - return "" . __("In Corso") . ""; - case 'in_ritardo': - return "" . __("In Ritardo") . ""; - case 'restituito': - return "" . __("Restituito") . ""; - case 'perso': - return "" . __("Perso") . ""; - case 'danneggiato': - return "" . __("Danneggiato") . ""; - case 'scaduto': - return "" . __("Scaduto") . ""; - case 'annullato': - return "" . __("Annullato") . ""; - default: - return "" . __("Sconosciuto") . ""; - } -} +// Badge canonico degli stati prestito (#333): unica mappa colore/icona/etichetta +// condivisa tra rendering SSR e colonne DataTables. +require_once __DIR__ . '/../partials/loan-status-badge.php'; $applicationToday = \App\Support\DateHelper::today(); ?> @@ -397,7 +371,7 @@ class="inline-flex items-center gap-2 whitespace-nowrap px-4 py-2 bg-gray-100 te - + ; + // Mappa canonica stato → badge HTML, generata dal partial loan-status-badge.php + // così la colonna DataTables e il rendering SSR non possono divergere (#333). + const loanStatusBadges = ; + const loanStatusUnknownBadge = ; // Initialize DataTable const table = new DataTable('#prestiti-table', { @@ -558,31 +536,9 @@ className: 'text-center align-middle', data: 'stato', className: 'text-center', render: function(data, type, row) { - const baseClasses = 'inline-flex items-center px-3 py-1 rounded-full text-xs font-medium'; - switch (data) { - case 'pendente': - return ``; - case 'prenotato': - return ``; - case 'da_ritirare': - return ``; - case 'in_corso': - return ``; - case 'in_ritardo': - return ``; - case 'restituito': - return ``; - case 'perso': - return ``; - case 'danneggiato': - return ``; - case 'scaduto': - return ``; - case 'annullato': - return ``; - default: - return ``; - } + // Badge dalla mappa canonica PHP (loan-status-badge.php): + // stessa fonte del rendering SSR, nessuna mappa duplicata in JS. + return loanStatusBadges[data] || loanStatusUnknownBadge; } }, { diff --git a/app/Views/profile/reservations.php b/app/Views/profile/reservations.php index 15020ef1e..93316c869 100644 --- a/app/Views/profile/reservations.php +++ b/app/Views/profile/reservations.php @@ -568,10 +568,20 @@ 'perso' => __('Perso'), 'danneggiato' => __('Danneggiato'), 'prestato' => __('Prestato'), - 'in_corso' => __('In corso') + 'in_corso' => __('In corso'), + 'annullato' => __('Annullato'), + 'scaduto' => __('Scaduto') ]; $statusLabel = $statusLabels[$p['stato']] ?? ucfirst(str_replace('_', ' ', $p['stato'])); + $statusIcon = match ($p['stato']) { + 'annullato' => 'fa-ban', + 'scaduto' => 'fa-calendar-times', + default => 'fa-check-circle', + }; $hasReview = !empty($p['has_review']); + // Un prestito annullato/scaduto non è mai uscito: nessuna recensione + // proponibile (il server la rifiuterebbe: richiede restituito/in corso). + $canReview = !in_array($p['stato'], ['annullato', 'scaduto'], true); ?>
@@ -584,7 +594,7 @@

- +
@@ -599,7 +609,7 @@ - + +
diff --git a/app/Views/utenti/dettagli_utente.php b/app/Views/utenti/dettagli_utente.php index 9c96fe53e..fc3dbf110 100644 --- a/app/Views/utenti/dettagli_utente.php +++ b/app/Views/utenti/dettagli_utente.php @@ -1,34 +1,9 @@ " . __("In attesa") . ""; - case 'prenotato': - return "" . __("Prenotato") . ""; - case 'in_corso': - return "" . __("In Corso") . ""; - case 'in_ritardo': - return "" . __("In Ritardo") . ""; - case 'restituito': - return "" . __("Restituito") . ""; - case 'perso': - return "" . __("Perso") . ""; - case 'danneggiato': - return "" . __("Danneggiato") . ""; - case 'da_ritirare': - return "" . __("Da Ritirare") . ""; - case 'scaduto': - return "" . __("Scaduto") . ""; - case 'annullato': - return "" . __("Annullato") . ""; - default: - return "" . __("Sconosciuto") . ""; - } -} +// Badge canonico degli stati prestito (#333): colore/icona/etichetta arrivano +// dal partial condiviso, niente mappa locale da tenere allineata. +require_once __DIR__ . '/../partials/loan-status-badge.php'; $id = (int)($utente['id'] ?? 0); $name = trim(($utente['nome'] ?? '') . ' ' . ($utente['cognome'] ?? '')); @@ -315,7 +290,7 @@ function getLoanStatusBadge($status) {
- + diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index fdb645bc7..fbd8020cb 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -33,11 +33,42 @@ $pendingLoans = (string) file_get_contents($root . '/app/Views/admin/pending_loans.php'); $integrityReport = (string) file_get_contents($root . '/app/Views/admin/integrity_report.php'); $controller = (string) file_get_contents($root . '/app/Controllers/PrestitiController.php'); +$badgePartial = (string) file_get_contents($root . '/app/Views/partials/loan-status-badge.php'); +$helpers = (string) file_get_contents($root . '/app/helpers.php'); +$profileReservations = (string) file_get_contents($root . '/app/Views/profile/reservations.php'); +$userDashboard = (string) file_get_contents($root . '/app/Views/user_dashboard/prenotazioni.php'); +$userActions = (string) file_get_contents($root . '/app/Controllers/UserActionsController.php'); +$userDashboardCtrl = (string) file_get_contents($root . '/app/Controllers/UserDashboardController.php'); -echo "== #333: stato 'annullato' rendered everywhere ==\n"; +echo "== #333: canonical badge covers the whole stato enum, used by every admin view ==\n"; +// The shared partial is the ONLY badge map: every enum value must be there, +// and the labels must come from the same helper the PDF/CSV already use. +foreach (['pendente', 'prenotato', 'da_ritirare', 'in_corso', 'in_ritardo', 'restituito', 'perso', 'danneggiato', 'scaduto', 'annullato'] as $stato) { + $check( + str_contains($badgePartial, "'{$stato}'"), + "canonical badge map covers '{$stato}'" + ); +} +$check( + str_contains($badgePartial, 'translate_loan_status(') + && str_contains($helpers, "'annullato' => __('Annullato')"), + 'badge labels come from translate_loan_status(), which maps annullato' +); +foreach ([ + 'loans list' => $loansIndex, + 'loan details page' => $loanDetails, + 'user details page' => $userDetails, + 'admin book page' => $bookPage, +] as $surface => $source) { + $check( + str_contains($source, 'loan-status-badge.php') && str_contains($source, 'loan_status_badge('), + "{$surface} renders states through the shared badge partial" + ); +} $check( - substr_count($loansIndex, "case 'annullato':") >= 2, - 'loans list renders the annullato badge in both the SSR and DataTables paths' + str_contains($loansIndex, 'loan_status_badge_map()') + && !str_contains($loansIndex, "case 'in_corso':"), + 'DataTables status column uses the PHP-generated map, no duplicated JS switch' ); $check( str_contains($loansIndex, 'data-status="annullato"'), @@ -47,22 +78,33 @@ str_contains($loansIndex, 'value="annullato"'), 'CSV export dialog includes the annullato state' ); -$check( - str_contains($loanDetails, "'annullato' => __('Annullato')") - && str_contains($loanDetails, "'scaduto' => __('Scaduto')"), - 'loan details page labels annullato/scaduto instead of Sconosciuto' -); $check( str_contains($loanDetails, "['annullato', 'scaduto']"), 'loan details page does not claim "not yet returned" for cancelled/expired loans' ); + +echo "== #333 follow-up: cancelled loans appear in the user-facing history ==\n"; +$check( + substr_count($userActions, "'restituito','perso','danneggiato','annullato','scaduto'") === 1, + 'profile history query includes cancelled/expired loans' +); +$check( + substr_count($userDashboardCtrl, "'restituito','perso','danneggiato','annullato','scaduto'") === 2, + 'user dashboard history query AND its counter include cancelled/expired loans' +); +$check( + str_contains($userActions, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))') + && str_contains($userDashboardCtrl, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))'), + 'history sorts cancelled loans by closing time instead of sinking NULL return dates' +); $check( - str_contains($userDetails, "case 'annullato':") && str_contains($userDetails, "case 'da_ritirare':"), - 'user details page labels annullato (and da_ritirare) loans' + str_contains($profileReservations, "'annullato' => __('Annullato')") + && str_contains($userDashboard, "'annullato' => __('Annullato')"), + 'both user history views label the annullato state' ); $check( - str_contains($bookPage, "case 'annullato':"), - 'admin book page loan history labels annullato loans' + str_contains($profileReservations, '$canReview') && str_contains($userDashboard, '$canReview'), + 'history hides the review button for loans that never went out (annullato/scaduto)' ); echo "== #334: page header no longer covers the notifications dropdown ==\n"; From 627ee2af6458138f81a3bb9782d85574cdd0dbd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:44:03 +0000 Subject: [PATCH 05/69] refactor(loans): route every stato label through the canonical helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the #333 centralization sweep. New loan_status_label_map() in app/helpers.php (enum → label via translate_loan_status) feeds the JS lookups that previously kept hand-maintained copies: the stats 'loans by status' chart and the admin dashboard calendar popup. The ICS feed (IcsGenerator::translateStatus), the book page occupancy calendar, the user history views and the loans-overview state chips now delegate to translate_loan_status() as well; the user dashboard active-loan badges keep their frontend styling but take their label text from the helper. Side effect worth noting: the calendar/ICS alias "Scaduto" for in_ritardo is gone — since 'scaduto' (pickup expired) is a real enum state, that alias had become genuinely ambiguous; everything now says "In Ritardo". Two deliberate exceptions are documented in place: the per-copy row hint on the book page ("In prestito" describes the physical copy, not the loan state) and the ICS event titles (event naming with emoji, not status labels). The return form's outcome select also stays local: it mixes loan outcomes with copy destinations ("Restituito — copia in manutenzione"), which are not stato labels. Regression guards extended to 44 checks, including 'no local stato → label literal map left' probes on the swept views. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Support/IcsGenerator.php | 20 +++++----- app/Views/admin/pending_loans.php | 4 +- app/Views/admin/stats.php | 10 ++--- app/Views/dashboard/index.php | 16 ++++---- app/Views/libri/scheda_libro.php | 16 +++----- app/Views/profile/reservations.php | 14 ++----- app/Views/user_dashboard/prenotazioni.php | 20 ++++------ app/helpers.php | 21 ++++++++++ tests/issues-333-334-336.unit.php | 48 +++++++++++++++++++++-- 9 files changed, 106 insertions(+), 63 deletions(-) diff --git a/app/Support/IcsGenerator.php b/app/Support/IcsGenerator.php index 76217fc7e..bf5582f1c 100644 --- a/app/Support/IcsGenerator.php +++ b/app/Support/IcsGenerator.php @@ -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'), @@ -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); } /** diff --git a/app/Views/admin/pending_loans.php b/app/Views/admin/pending_loans.php index 64f8ac1d8..a15e8dade 100644 --- a/app/Views/admin/pending_loans.php +++ b/app/Views/admin/pending_loans.php @@ -151,7 +151,7 @@ class="w-16 h-22 object-cover rounded-lg shadow-sm"

- +

@@ -354,7 +354,7 @@ class="w-16 h-22 object-cover rounded-lg shadow-sm" - +

diff --git a/app/Views/admin/stats.php b/app/Views/admin/stats.php index 7d6f54f7f..cfc1c8f68 100644 --- a/app/Views/admin/stats.php +++ b/app/Views/admin/stats.php @@ -348,13 +348,9 @@ class="w-10 h-14 object-cover rounded shadow-sm" // Loans By Status Chart const loansByStatusData = ; const statusLabels = loansByStatusData.map(item => { - const labels = { - 'in_corso': , - 'pendente': , - 'in_ritardo': , - 'perso': , - 'danneggiato': - }; + // Etichette canoniche degli stati prestito (loan_status_label_map, #333): + // niente mappa locale da tenere allineata all'enum. + const labels = ; return labels[item.stato] || item.stato; }); const statusValues = loansByStatusData.map(item => parseInt(item.totale)); diff --git a/app/Views/dashboard/index.php b/app/Views/dashboard/index.php index d57a2795c..a80c05b05 100644 --- a/app/Views/dashboard/index.php +++ b/app/Views/dashboard/index.php @@ -945,14 +945,14 @@ function escapeHtml(str) { eventClick: function(info) { const props = info.event.extendedProps; const typeLabel = props.type === 'prenotazione' ? : ; - const statusLabels = { - 'in_corso': , - 'prenotato': , - 'da_ritirare': , - 'in_ritardo': , - 'pendente': , - 'attiva': - }; + // Etichette canoniche degli stati prestito (loan_status_label_map, + // #333) + lo stato 'attiva' delle prenotazioni, che non fa parte + // dell'enum prestiti. Il vecchio alias "Scaduto" per in_ritardo è + // sparito: ora collide con lo stato 'scaduto' vero e proprio. + const statusLabels = __('Attiva')]), + JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT + ) ?>; const statusLabel = statusLabels[props.status] || props.status; // Use originalStart/originalEnd with fallback to event dates diff --git a/app/Views/libri/scheda_libro.php b/app/Views/libri/scheda_libro.php index 9b53a6346..421014f15 100644 --- a/app/Views/libri/scheda_libro.php +++ b/app/Views/libri/scheda_libro.php @@ -1099,7 +1099,9 @@ class="text-red-600 hover:text-red-900 transition-colors" __('In prestito'), 'in_ritardo' => __('In ritardo'), @@ -1411,15 +1413,9 @@ class="text-red-600 hover:text-red-900 transition-colors" default => $copyColor }; - // Status label - $statusLabel = match($stato) { - 'in_corso' => __('In prestito'), - 'prenotato' => __('Prenotato'), - 'in_ritardo' => __('In ritardo'), - 'pendente' => __('In attesa'), - 'da_ritirare' => __('Da Ritirare'), - default => ucfirst($stato) - }; + // Etichetta canonica dello stato (translate_loan_status, #333): stessa + // dicitura dei badge nella tabella prestiti di questa stessa pagina. + $statusLabel = translate_loan_status((string) $stato); // FullCalendar expects end date to be exclusive, so add 1 day $endDateObj = new DateTime($endDate); diff --git a/app/Views/profile/reservations.php b/app/Views/profile/reservations.php index 93316c869..2dafbeb1e 100644 --- a/app/Views/profile/reservations.php +++ b/app/Views/profile/reservations.php @@ -562,17 +562,9 @@ __('Restituito'), - 'in_ritardo' => __('Restituito in ritardo'), - 'perso' => __('Perso'), - 'danneggiato' => __('Danneggiato'), - 'prestato' => __('Prestato'), - 'in_corso' => __('In corso'), - 'annullato' => __('Annullato'), - 'scaduto' => __('Scaduto') - ]; - $statusLabel = $statusLabels[$p['stato']] ?? ucfirst(str_replace('_', ' ', $p['stato'])); + // Etichetta canonica dello stato (translate_loan_status, #333): lo storico + // contiene solo stati chiusi (restituito/perso/danneggiato/annullato/scaduto). + $statusLabel = translate_loan_status((string) $p['stato']); $statusIcon = match ($p['stato']) { 'annullato' => 'fa-ban', 'scaduto' => 'fa-calendar-times', diff --git a/app/Views/user_dashboard/prenotazioni.php b/app/Views/user_dashboard/prenotazioni.php index 6a02826e0..f0e87318e 100644 --- a/app/Views/user_dashboard/prenotazioni.php +++ b/app/Views/user_dashboard/prenotazioni.php @@ -652,9 +652,11 @@ function accountLineIcon(string $name): string {
['icon' => 'fa-box-open', 'label' => __('Da ritirare'), 'style' => 'background: #dbeafe; color: #1e40af; border: 1px solid #93c5fd;'], - 'prenotato' => ['icon' => 'fa-bookmark', 'label' => __('Prenotato'), 'style' => 'background: #ede9fe; color: #5b21b6; border: 1px solid #c4b5fd;'], + 'da_ritirare' => ['icon' => 'fa-box-open', 'label' => translate_loan_status('da_ritirare'), 'style' => 'background: #dbeafe; color: #1e40af; border: 1px solid #93c5fd;'], + 'prenotato' => ['icon' => 'fa-bookmark', 'label' => translate_loan_status('prenotato'), 'style' => 'background: #ede9fe; color: #5b21b6; border: 1px solid #c4b5fd;'], ]; if (isset($statoBadges[$stato])): ?>
@@ -779,17 +781,9 @@ function accountLineIcon(string $name): string {
__('Restituito'), - 'in_ritardo' => __('Restituito in ritardo'), - 'perso' => __('Perso'), - 'danneggiato' => __('Danneggiato'), - 'prestato' => __('Prestato'), - 'in_corso' => __('In corso'), - 'annullato' => __('Annullato'), - 'scaduto' => __('Scaduto'), - ]; - $statusLabel = $statusLabels[$loan['stato']] ?? ucfirst(str_replace('_', ' ', (string)$loan['stato'])); + // Etichetta canonica dello stato (translate_loan_status, #333): lo storico + // contiene solo stati chiusi (restituito/perso/danneggiato/annullato/scaduto). + $statusLabel = translate_loan_status((string) $loan['stato']); $statusIcon = match ($loan['stato']) { 'annullato' => 'fa-ban', 'scaduto' => 'fa-calendar-times', diff --git a/app/helpers.php b/app/helpers.php index 0fa8a448f..8002bbfd1 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -329,6 +329,27 @@ function translate_loan_status(string $status): string } } +if (!function_exists('loan_status_label_map')) { + /** + * Full prestiti.stato enum => localized label, via translate_loan_status() + * (single source of truth for the wording). For contexts that need the whole + * map at once — e.g. json_encode() into a JS lookup for charts/calendars — + * instead of hand-maintained per-view copies (#333). For HTML badges in the + * admin views use loan_status_badge() (app/Views/partials/loan-status-badge.php). + * + * @return array + */ + function loan_status_label_map(): array + { + $states = ['pendente', 'prenotato', 'da_ritirare', 'in_corso', 'in_ritardo', 'restituito', 'perso', 'danneggiato', 'annullato', 'scaduto']; + $map = []; + foreach ($states as $stato) { + $map[$stato] = translate_loan_status($stato); + } + return $map; + } +} + if (!function_exists('full_name')) { /** * Join a member's given name and surname into a display name. diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index fbd8020cb..b26f01d34 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -98,15 +98,57 @@ 'history sorts cancelled loans by closing time instead of sinking NULL return dates' ); $check( - str_contains($profileReservations, "'annullato' => __('Annullato')") - && str_contains($userDashboard, "'annullato' => __('Annullato')"), - 'both user history views label the annullato state' + str_contains($profileReservations, "'annullato' => 'fa-ban'") + && str_contains($userDashboard, "'annullato' => 'fa-ban'"), + 'both user history views give cancelled loans a dedicated icon' ); $check( str_contains($profileReservations, '$canReview') && str_contains($userDashboard, '$canReview'), 'history hides the review button for loans that never went out (annullato/scaduto)' ); +echo "== #333 sweep: every stato-label consumer routes through the canonical helpers ==\n"; +$statsView = (string) file_get_contents($root . '/app/Views/admin/stats.php'); +$dashboardView = (string) file_get_contents($root . '/app/Views/dashboard/index.php'); +$icsGenerator = (string) file_get_contents($root . '/app/Support/IcsGenerator.php'); +$check( + str_contains($helpers, 'function loan_status_label_map()') + && str_contains($helpers, "translate_loan_status(\$stato)"), + 'helpers.php exposes the enum-wide label map built on translate_loan_status()' +); +$check( + str_contains($statsView, 'loan_status_label_map()'), + 'stats chart labels come from the canonical label map' +); +$check( + str_contains($dashboardView, 'loan_status_label_map()'), + 'dashboard calendar labels come from the canonical label map' +); +$check( + str_contains($icsGenerator, 'translate_loan_status($status)'), + 'ICS feed status labels delegate to translate_loan_status()' +); +$check( + str_contains($bookPage, 'translate_loan_status((string) $stato)'), + 'book page occupancy calendar labels delegate to translate_loan_status()' +); +$check( + str_contains($profileReservations, 'translate_loan_status(') + && str_contains($userDashboard, 'translate_loan_status('), + 'user history views delegate labels to translate_loan_status()' +); +// No hand-maintained stato→label literals left outside the two helpers: the +// old maps always spelled a quoted state key next to a __() label call. +foreach ([ + 'admin stats view' => $statsView, + 'admin dashboard view' => $dashboardView, +] as $surface => $source) { + $check( + !preg_match('/[\'"]in_corso[\'"]\s*(=>|:)\s*(<\?=\s*)?(json_encode\()?__\(/', $source), + "{$surface} keeps no local stato→label literal map" + ); +} + echo "== #334: page header no longer covers the notifications dropdown ==\n"; // Match class attributes only — the explanatory comments in those views cite // the removed utility string verbatim. From b4f4fd8aef28d8b9e16ce1995a51f0c1121b7d27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:47:53 +0000 Subject: [PATCH 06/69] feat(mobile-api): align /me/loans history with web; additive status_label (1.4.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mobile history had the same gap just fixed on the web: cancelled (annullato) and pickup-expired (scaduto) loans vanished from /me/loans history. The endpoint now returns them, ordered by closing time (COALESCE with updated_at) so rows without a return date don't sink. 'status' is documented as the raw prestiti.stato value, so the extra states are an additive, spec-compatible change. Every loan payload also gains 'status_label': the server-localized label from the canonical translate_loan_status() helper (#333), so clients no longer need their own stato→label map and future enum values degrade gracefully. Documented in the OpenAPI schema; plugin version bumped to 1.4.3. The public API needs no change: it only ever exposes a book's active loan (attivo=1), which cancelled loans never are. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- storage/plugins/mobile-api/plugin.json | 2 +- .../src/Controllers/ActionsController.php | 14 +++++++++--- .../src/Controllers/OpenApiController.php | 1 + tests/issues-333-334-336.unit.php | 22 +++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/storage/plugins/mobile-api/plugin.json b/storage/plugins/mobile-api/plugin.json index b38f1f136..bcfe7c1f0 100644 --- a/storage/plugins/mobile-api/plugin.json +++ b/storage/plugins/mobile-api/plugin.json @@ -2,7 +2,7 @@ "name": "mobile-api", "display_name": "Mobile API", "description": "API REST/JSON versionata (/api/v1) per l'app companion mobile di Pinakes: discovery, autenticazione a token per dispositivo, ricerca catalogo, prestiti/prenotazioni, wishlist, profilo, messaggi e notifiche push. Disattivata per default finché non viene abilitata.", - "version": "1.4.2", + "version": "1.4.3", "author": "Fabiodalez", "author_url": "", "plugin_url": "", diff --git a/storage/plugins/mobile-api/src/Controllers/ActionsController.php b/storage/plugins/mobile-api/src/Controllers/ActionsController.php index 32a45b710..7f6c00479 100644 --- a/storage/plugins/mobile-api/src/Controllers/ActionsController.php +++ b/storage/plugins/mobile-api/src/Controllers/ActionsController.php @@ -99,13 +99,17 @@ public function myLoans(Request $request, ResponseInterface $response): Response $active[] = $this->mapLoan($r); } - // Concluded history (most recent 30). + // Concluded history (most recent 30) — includes cancelled/expired + // loans like the web history. They have no data_restituzione: order + // on the closing moment (updated_at) so a recent cancellation does + // not sink to the bottom. `status` is documented as the raw + // prestiti.stato value, so the extra states are additive. $sql = "SELECT pr.id, pr.libro_id, pr.data_prestito, pr.data_restituzione, pr.stato, l.titolo, l.copertina_url 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, DATE(pr.updated_at)) DESC, pr.data_prestito DESC LIMIT 30"; foreach ($this->fetchScoped($sql, $userId) as $r) { $history[] = $this->mapLoan($r); @@ -972,6 +976,10 @@ private function mapLoan(array $r): array 'title' => (string) ($r['titolo'] ?? ''), 'cover_url' => absoluteUrl($this->coverPath($r['copertina_url'] ?? null)), 'status' => $status, + // Additive since 1.4.3: server-localized label from the canonical + // translate_loan_status() helper (#333), so clients need no local + // stato→label map and new enum values degrade gracefully. + 'status_label' => translate_loan_status($status), 'loaned_at' => $this->nullableString($r['data_prestito'] ?? null), 'due_at' => $dueAt, // Server-authoritative visibility cue: the Android device may be in a diff --git a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php index b60ac2461..93ff3b40c 100644 --- a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php +++ b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php @@ -1261,6 +1261,7 @@ private function loanItemSchema(): array 'title' => ['type' => 'string'], 'cover_url' => ['type' => 'string', 'format' => 'uri', 'nullable' => true], 'status' => ['type' => 'string', 'description' => 'Raw prestiti.stato value.'], + 'status_label' => ['type' => 'string', 'description' => 'Server-localized label for status (since 1.4.3). Prefer this over client-side status maps.'], 'loaned_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_attention' => [ diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index b26f01d34..a2cc35ecf 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -149,6 +149,28 @@ ); } +echo "== #333 API: mobile /me/loans matches the web history and labels via the helper ==\n"; +$mobileActions = (string) file_get_contents($root . '/storage/plugins/mobile-api/src/Controllers/ActionsController.php'); +$mobileOpenApi = (string) file_get_contents($root . '/storage/plugins/mobile-api/src/Controllers/OpenApiController.php'); +$mobilePlugin = json_decode((string) file_get_contents($root . '/storage/plugins/mobile-api/plugin.json'), true); +$check( + str_contains($mobileActions, "'restituito','perso','danneggiato','annullato','scaduto'") + && str_contains($mobileActions, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))'), + 'mobile loans history includes cancelled/expired loans with closing-time order' +); +$check( + str_contains($mobileActions, "'status_label' => translate_loan_status(\$status)"), + 'mobile loan payload carries a server-localized status_label from the canonical helper' +); +$check( + str_contains($mobileOpenApi, "'status_label'"), + 'OpenAPI schema documents the additive status_label field' +); +$check( + is_array($mobilePlugin) && version_compare((string) ($mobilePlugin['version'] ?? '0'), '1.4.3', '>='), + 'mobile-api plugin version bumped for the additive API change' +); + echo "== #334: page header no longer covers the notifications dropdown ==\n"; // Match class attributes only — the explanatory comments in those views cite // the removed utility string verbatim. From fc5cb4ab95000e771f72399346daf9c931e57865 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 11:23:59 +0000 Subject: [PATCH 07/69] fix(loans): address CodeRabbit review on PR #337 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update(): re-read data_prestito/data_scadenza (and utente_id) in the FOR UPDATE query and rebuild the effective values + range validation from the LOCKED row. The claimed-windows capacity math previously used the pre-transaction read, so a concurrent update between the initial read and the lock could get its dates silently overwritten and the capacity check computed against a stale old window. The pre-transaction validation stays as a fast-fail. - History ordering: drop DATE() from the COALESCE fallback in the three history queries (profile, user dashboard, mobile API) so two loans closed the same day sort by their real closing moment. - /admin/loans banner: handle 'renewal_failed' explicitly. - user dashboard review button: htmlspecialchars(..., ENT_QUOTES) instead of HtmlHelper::e() in the data-book-title attribute (path rule). - Regression test: noisy source reader (missing/empty file → exit 1), ordered-position guards + non-empty assertions on the extracted update()/applyBulkLoanExtension() sections, so the negative checks can no longer pass vacuously. - mobile-api: additive 'requested_at' (DATE of created_at) on every /me/loans payload, documented in OpenAPI — gives clients an honest date for cancelled/expired loans, which never went out (pairs with the Android PR #30 review); status_label description softened to match the fallback-for-unknown-states semantics. Guards now 51 checks, all green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Controllers/PrestitiController.php | 26 +++++- app/Controllers/UserActionsController.php | 2 +- app/Controllers/UserDashboardController.php | 2 +- app/Views/prestiti/index.php | 3 + app/Views/user_dashboard/prenotazioni.php | 2 +- .../src/Controllers/ActionsController.php | 16 ++-- .../src/Controllers/OpenApiController.php | 3 +- tests/issues-333-334-336.unit.php | 79 ++++++++++++------- 8 files changed, 93 insertions(+), 40 deletions(-) diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 3fcd0eccd..d4ef1db83 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -774,7 +774,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, 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(); @@ -788,6 +792,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 (strtotime($newScadenza) === false || strtotime($newPrestito) === false + || strtotime($newScadenza) < strtotime($newPrestito)) { + $db->rollback(); + return $response->withHeader('Location', url('/admin/loans') . '?error=invalid_dates')->withStatus(302); + } + // 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. @@ -869,8 +886,9 @@ public function update(Request $request, Response $response, mysqli $db, int $id // 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 // are already held by this loan; only the added days need free capacity. - $oldPrestito = (string) $current['data_prestito']; - $oldScadenza = (string) $current['data_scadenza']; + // 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) { $claimedWindows = []; if ($newPrestito < $oldPrestito) { @@ -920,7 +938,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 diff --git a/app/Controllers/UserActionsController.php b/app/Controllers/UserActionsController.php index 0951430e2..e5d25f4a0 100644 --- a/app/Controllers/UserActionsController.php +++ b/app/Controllers/UserActionsController.php @@ -81,7 +81,7 @@ public function reservationsPage(Request $request, Response $response, mysqli $d 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','annullato','scaduto') - ORDER BY COALESCE(pr.data_restituzione, DATE(pr.updated_at)) DESC, pr.data_prestito DESC + 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); diff --git a/app/Controllers/UserDashboardController.php b/app/Controllers/UserDashboardController.php index f2e5f6346..b708dab8e 100644 --- a/app/Controllers/UserDashboardController.php +++ b/app/Controllers/UserDashboardController.php @@ -192,7 +192,7 @@ public function prenotazioni(Request $request, Response $response, mysqli $db, m 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','annullato','scaduto') AND l.deleted_at IS NULL - ORDER BY COALESCE(pr.data_restituzione, DATE(pr.updated_at)) DESC, pr.data_prestito DESC + ORDER BY COALESCE(pr.data_restituzione, pr.updated_at) DESC, pr.data_prestito DESC LIMIT 50 "); $stmt->bind_param('ii', $userId, $userId); diff --git a/app/Views/prestiti/index.php b/app/Views/prestiti/index.php index 151faaefb..2e444f0ee 100644 --- a/app/Views/prestiti/index.php +++ b/app/Views/prestiti/index.php @@ -109,6 +109,9 @@ case 'book_not_found': echo __('Libro non trovato o non più disponibile.'); break; + case 'renewal_failed': + echo __('Rinnovo non riuscito. Riprova.'); + break; default: echo __('Errore durante l\'aggiornamento del prestito.'); } diff --git a/app/Views/user_dashboard/prenotazioni.php b/app/Views/user_dashboard/prenotazioni.php index f0e87318e..41ca2c805 100644 --- a/app/Views/user_dashboard/prenotazioni.php +++ b/app/Views/user_dashboard/prenotazioni.php @@ -815,7 +815,7 @@ function accountLineIcon(string $name): string {
- diff --git a/storage/plugins/mobile-api/src/Controllers/ActionsController.php b/storage/plugins/mobile-api/src/Controllers/ActionsController.php index 7f6c00479..9ff0e94c8 100644 --- a/storage/plugins/mobile-api/src/Controllers/ActionsController.php +++ b/storage/plugins/mobile-api/src/Controllers/ActionsController.php @@ -89,7 +89,7 @@ public function myLoans(Request $request, ResponseInterface $response): Response // Active loans (scheduled / to-pickup / in-progress / overdue). $sql = "SELECT pr.id, pr.libro_id, pr.data_prestito, pr.data_scadenza, pr.data_restituzione, - pr.stato, pr.renewals, l.titolo, l.copertina_url + pr.stato, pr.renewals, pr.created_at, l.titolo, l.copertina_url 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 = 1 @@ -105,11 +105,11 @@ public function myLoans(Request $request, ResponseInterface $response): Response // not sink to the bottom. `status` is documented as the raw // prestiti.stato value, so the extra states are additive. $sql = "SELECT pr.id, pr.libro_id, pr.data_prestito, pr.data_restituzione, pr.stato, - l.titolo, l.copertina_url + pr.created_at, l.titolo, l.copertina_url 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','annullato','scaduto') - ORDER BY COALESCE(pr.data_restituzione, DATE(pr.updated_at)) DESC, pr.data_prestito DESC + ORDER BY COALESCE(pr.data_restituzione, pr.updated_at) DESC, pr.data_prestito DESC LIMIT 30"; foreach ($this->fetchScoped($sql, $userId) as $r) { $history[] = $this->mapLoan($r); @@ -977,9 +977,15 @@ private function mapLoan(array $r): array 'cover_url' => absoluteUrl($this->coverPath($r['copertina_url'] ?? null)), 'status' => $status, // Additive since 1.4.3: server-localized label from the canonical - // translate_loan_status() helper (#333), so clients need no local - // stato→label map and new enum values degrade gracefully. + // translate_loan_status() helper (#333). For KNOWN states clients + // keep their own device-localized wording; this is the fallback for + // states a client version doesn't recognize yet. 'status_label' => translate_loan_status($status), + // Additive since 1.4.3: the date the loan request was created + // (DATE part of created_at). For cancelled/expired loans — which + // never went out — this is the only honest date to show; loaned_at + // is the *requested start*, not a borrow date. + 'requested_at' => !empty($r['created_at']) ? substr((string) $r['created_at'], 0, 10) : null, 'loaned_at' => $this->nullableString($r['data_prestito'] ?? null), 'due_at' => $dueAt, // Server-authoritative visibility cue: the Android device may be in a diff --git a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php index 93ff3b40c..eb0fa63be 100644 --- a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php +++ b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php @@ -1261,7 +1261,8 @@ private function loanItemSchema(): array 'title' => ['type' => 'string'], 'cover_url' => ['type' => 'string', 'format' => 'uri', 'nullable' => true], 'status' => ['type' => 'string', 'description' => 'Raw prestiti.stato value.'], - 'status_label' => ['type' => 'string', 'description' => 'Server-localized label for status (since 1.4.3). Prefer this over client-side status maps.'], + 'status_label' => ['type' => 'string', 'description' => 'Server-localized label for status (since 1.4.3). Fallback for clients without a local mapping for a state; known states may keep device-localized wording.'], + 'requested_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true, 'description' => 'Date the loan request was created (since 1.4.3). The honest date for cancelled/expired loans, which never went out.'], 'loaned_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_attention' => [ diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index a2cc35ecf..d6b95f892 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -25,20 +25,36 @@ echo ($condition ? '[PASS] ' : '[FAIL] ') . $label . PHP_EOL; $condition ? $passed++ : $failed++; }; +// Lettura "rumorosa": molti check qui sotto sono NEGATIVI (!str_contains) e +// passerebbero in silenzio su una sorgente vuota. Un file mancante o illeggibile +// deve far fallire subito il test, non farlo diventare verde a vuoto. +$src = static function (string $relPath) use ($root): string { + $path = $root . '/' . $relPath; + if (!is_file($path)) { + fwrite(STDERR, "[FATAL] missing source file: {$relPath}" . PHP_EOL); + exit(1); + } + $content = file_get_contents($path); + if ($content === false || $content === '') { + fwrite(STDERR, "[FATAL] unreadable/empty source file: {$relPath}" . PHP_EOL); + exit(1); + } + return $content; +}; -$loansIndex = (string) file_get_contents($root . '/app/Views/prestiti/index.php'); -$loanDetails = (string) file_get_contents($root . '/app/Views/prestiti/dettagli_prestito.php'); -$userDetails = (string) file_get_contents($root . '/app/Views/utenti/dettagli_utente.php'); -$bookPage = (string) file_get_contents($root . '/app/Views/libri/scheda_libro.php'); -$pendingLoans = (string) file_get_contents($root . '/app/Views/admin/pending_loans.php'); -$integrityReport = (string) file_get_contents($root . '/app/Views/admin/integrity_report.php'); -$controller = (string) file_get_contents($root . '/app/Controllers/PrestitiController.php'); -$badgePartial = (string) file_get_contents($root . '/app/Views/partials/loan-status-badge.php'); -$helpers = (string) file_get_contents($root . '/app/helpers.php'); -$profileReservations = (string) file_get_contents($root . '/app/Views/profile/reservations.php'); -$userDashboard = (string) file_get_contents($root . '/app/Views/user_dashboard/prenotazioni.php'); -$userActions = (string) file_get_contents($root . '/app/Controllers/UserActionsController.php'); -$userDashboardCtrl = (string) file_get_contents($root . '/app/Controllers/UserDashboardController.php'); +$loansIndex = $src('app/Views/prestiti/index.php'); +$loanDetails = $src('app/Views/prestiti/dettagli_prestito.php'); +$userDetails = $src('app/Views/utenti/dettagli_utente.php'); +$bookPage = $src('app/Views/libri/scheda_libro.php'); +$pendingLoans = $src('app/Views/admin/pending_loans.php'); +$integrityReport = $src('app/Views/admin/integrity_report.php'); +$controller = $src('app/Controllers/PrestitiController.php'); +$badgePartial = $src('app/Views/partials/loan-status-badge.php'); +$helpers = $src('app/helpers.php'); +$profileReservations = $src('app/Views/profile/reservations.php'); +$userDashboard = $src('app/Views/user_dashboard/prenotazioni.php'); +$userActions = $src('app/Controllers/UserActionsController.php'); +$userDashboardCtrl = $src('app/Controllers/UserDashboardController.php'); echo "== #333: canonical badge covers the whole stato enum, used by every admin view ==\n"; // The shared partial is the ONLY badge map: every enum value must be there, @@ -93,8 +109,8 @@ 'user dashboard history query AND its counter include cancelled/expired loans' ); $check( - str_contains($userActions, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))') - && str_contains($userDashboardCtrl, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))'), + str_contains($userActions, 'COALESCE(pr.data_restituzione, pr.updated_at)') + && str_contains($userDashboardCtrl, 'COALESCE(pr.data_restituzione, pr.updated_at)'), 'history sorts cancelled loans by closing time instead of sinking NULL return dates' ); $check( @@ -108,9 +124,9 @@ ); echo "== #333 sweep: every stato-label consumer routes through the canonical helpers ==\n"; -$statsView = (string) file_get_contents($root . '/app/Views/admin/stats.php'); -$dashboardView = (string) file_get_contents($root . '/app/Views/dashboard/index.php'); -$icsGenerator = (string) file_get_contents($root . '/app/Support/IcsGenerator.php'); +$statsView = $src('app/Views/admin/stats.php'); +$dashboardView = $src('app/Views/dashboard/index.php'); +$icsGenerator = $src('app/Support/IcsGenerator.php'); $check( str_contains($helpers, 'function loan_status_label_map()') && str_contains($helpers, "translate_loan_status(\$stato)"), @@ -150,12 +166,12 @@ } echo "== #333 API: mobile /me/loans matches the web history and labels via the helper ==\n"; -$mobileActions = (string) file_get_contents($root . '/storage/plugins/mobile-api/src/Controllers/ActionsController.php'); -$mobileOpenApi = (string) file_get_contents($root . '/storage/plugins/mobile-api/src/Controllers/OpenApiController.php'); -$mobilePlugin = json_decode((string) file_get_contents($root . '/storage/plugins/mobile-api/plugin.json'), true); +$mobileActions = $src('storage/plugins/mobile-api/src/Controllers/ActionsController.php'); +$mobileOpenApi = $src('storage/plugins/mobile-api/src/Controllers/OpenApiController.php'); +$mobilePlugin = json_decode($src('storage/plugins/mobile-api/plugin.json'), true); $check( str_contains($mobileActions, "'restituito','perso','danneggiato','annullato','scaduto'") - && str_contains($mobileActions, 'COALESCE(pr.data_restituzione, DATE(pr.updated_at))'), + && str_contains($mobileActions, 'COALESCE(pr.data_restituzione, pr.updated_at)'), 'mobile loans history includes cancelled/expired loans with closing-time order' ); $check( @@ -163,8 +179,13 @@ 'mobile loan payload carries a server-localized status_label from the canonical helper' ); $check( - str_contains($mobileOpenApi, "'status_label'"), - 'OpenAPI schema documents the additive status_label field' + str_contains($mobileOpenApi, "'status_label'") && str_contains($mobileOpenApi, "'requested_at'"), + 'OpenAPI schema documents the additive status_label and requested_at fields' +); +$check( + str_contains($mobileActions, "'requested_at'") + && substr_count($mobileActions, 'pr.created_at') >= 3, + 'every /me/loans payload carries requested_at, selected in all three queries' ); $check( is_array($mobilePlugin) && version_compare((string) ($mobilePlugin['version'] ?? '0'), '1.4.3', '>='), @@ -186,9 +207,12 @@ echo "== #336: date edits check only newly-claimed days; clear error messages ==\n"; $updateStart = strpos($controller, 'public function update('); $closeStart = strpos($controller, 'public function close('); -$updateSource = ($updateStart !== false && $closeStart !== false) +$updateSource = ($updateStart !== false && $closeStart !== false && $closeStart > $updateStart) ? substr($controller, $updateStart, $closeStart - $updateStart) : ''; +// Guardia: il check negativo qui sotto passerebbe a vuoto su una sezione +// vuota — l'estrazione deve aver realmente trovato il corpo di update(). +$check($updateSource !== '', 'update() source section extracted (guards the negative checks below)'); $check( str_contains($updateSource, '$claimedWindows') && str_contains($updateSource, 'excludePrestitoId: $id'), @@ -200,9 +224,10 @@ ); $bulkStart = strpos($controller, 'private function applyBulkLoanExtension('); $renewStart = strpos($controller, 'public function renew('); -$bulkSource = ($bulkStart !== false && $renewStart !== false) +$bulkSource = ($bulkStart !== false && $renewStart !== false && $renewStart > $bulkStart) ? substr($controller, $bulkStart, $renewStart - $bulkStart) : ''; +$check($bulkSource !== '', 'applyBulkLoanExtension() source section extracted'); $check( str_contains($bulkSource, "hasFreeCapacity(\$bookId, (string) \$loan['data_scadenza'], \$newDueDate"), 'bulk extension checks the extension window only, like renew()' @@ -224,7 +249,7 @@ 'Rinnovo non riuscito. Riprova.', ]; foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $locale) { - $bundle = json_decode((string) file_get_contents($root . '/locale/' . $locale . '.json'), true); + $bundle = json_decode($src('locale/' . $locale . '.json'), true); $ok = is_array($bundle); foreach ($newStrings as $key) { $ok = $ok && isset($bundle[$key]) && $bundle[$key] !== ''; From 77a76d6de82846aafebc69ad05d830ec8298e90b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 13:41:09 +0000 Subject: [PATCH 08/69] fix(loans): address second-round CodeRabbit review on PR #337 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update(): strict date validation replaces strtotime(). New isStrictIsoDate() requires exact Y-m-d AND a real calendar date (round-trip via createFromFormat), so ambiguous inputs ('2026-2-5') and impossible dates ('2026-02-30', which strtotime silently normalizes) are rejected instead of flowing — as non-canonical strings — into the lexicographic window math, the due-date-changed comparison and LoanRepository::update(). Applied to both the pre-transaction fast-fail and the under-lock re-validation. - update(): claimed windows are now the EXACT set difference new∖old — the boundary days (old start / old due) are already held by this loan, so a commitment sitting only on a boundary day no longer produces a false conflict. - applyBulkLoanExtension(): one interval for one decision — capacity and the same-copy overlap check both gate on the added days only (day after the current due date → new due date); the copy check previously scanned the whole loan window while capacity scanned the extension window. - mobile-api OpenAPI: requested_at declares nullability the 3.1 way (type ['string','null'], no 'nullable' flag), matching mapLoan()'s explicit null. Regression guards extended to 54 checks (exact-diff boundaries, no strtotime in update(), unified bulk interval, 3.1 nullability), all green; isStrictIsoDate verified against ambiguous/impossible/leap-year inputs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Controllers/PrestitiController.php | 65 ++++++++++++------- .../src/Controllers/OpenApiController.php | 2 +- tests/issues-333-334-336.unit.php | 19 +++++- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index d4ef1db83..39fec3569 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -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) { return $response->withHeader('Location', url('/admin/loans') . '?error=invalid_dates')->withStatus(302); } @@ -799,8 +803,8 @@ public function update(Request $request, Response $response, mysqli $db, int $id $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 (strtotime($newScadenza) === false || strtotime($newPrestito) === false - || strtotime($newScadenza) < strtotime($newPrestito)) { + if (!self::isStrictIsoDate($newPrestito) || !self::isStrictIsoDate($newScadenza) + || $newScadenza < $newPrestito) { $db->rollback(); return $response->withHeader('Location', url('/admin/loans') . '?error=invalid_dates')->withStatus(302); } @@ -879,25 +883,28 @@ public function update(Request $request, Response $response, mysqli $db, int $id // 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. - // #336: check ONLY the newly-claimed segments (before the old start and/or - // after the old due date), like renew() checks just the extension 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 - // are already held by this loan; only the added days need free capacity. + // #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) { - // Y-m-d strings compare correctly lexicographically. The inclusive - // boundary day (old start / old due) mirrors renew()'s convention. - $claimedWindows[] = [$newPrestito, min($oldPrestito, $newScadenza)]; + $claimedWindows[] = [$newPrestito, min($dayBefore($oldPrestito), $newScadenza)]; } if ($newScadenza > $oldScadenza) { - $claimedWindows[] = [max($oldScadenza, $newPrestito), $newScadenza]; + $claimedWindows[] = [max($dayAfter($oldScadenza), $newPrestito), $newScadenza]; } $capacity = new \App\Services\CapacityService($db); foreach ($claimedWindows as [$claimStart, $claimEnd]) { @@ -1609,21 +1616,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. - // #336: capacity is checked on the EXTENSION window only (current due date → - // new due date), the same convention as renew(). Checking the whole loan - // window re-counted commitments already coexisting with the current period, - // rejecting extensions that add no new conflict. - if (!$capacity->hasFreeCapacity($bookId, (string) $loan['data_scadenza'], $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; @@ -2067,6 +2075,19 @@ 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 + { + $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'] ?? ''; diff --git a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php index eb0fa63be..99314a418 100644 --- a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php +++ b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php @@ -1262,7 +1262,7 @@ private function loanItemSchema(): array 'cover_url' => ['type' => 'string', 'format' => 'uri', 'nullable' => true], 'status' => ['type' => 'string', 'description' => 'Raw prestiti.stato value.'], 'status_label' => ['type' => 'string', 'description' => 'Server-localized label for status (since 1.4.3). Fallback for clients without a local mapping for a state; known states may keep device-localized wording.'], - 'requested_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true, 'description' => 'Date the loan request was created (since 1.4.3). The honest date for cancelled/expired loans, which never went out.'], + 'requested_at' => ['type' => ['string', 'null'], 'format' => 'date', 'description' => 'Date the loan request was created (since 1.4.3). The honest date for cancelled/expired loans, which never went out.'], 'loaned_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_at' => ['type' => 'string', 'format' => 'date', 'nullable' => true], 'due_attention' => [ diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index d6b95f892..167b46510 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -182,6 +182,10 @@ str_contains($mobileOpenApi, "'status_label'") && str_contains($mobileOpenApi, "'requested_at'"), 'OpenAPI schema documents the additive status_label and requested_at fields' ); +$check( + str_contains($mobileOpenApi, "'requested_at' => ['type' => ['string', 'null']"), + 'requested_at declares nullability the OpenAPI 3.1 way (type array, no nullable flag)' +); $check( str_contains($mobileActions, "'requested_at'") && substr_count($mobileActions, 'pr.created_at') >= 3, @@ -218,6 +222,16 @@ && str_contains($updateSource, 'excludePrestitoId: $id'), 'update() checks capacity on the newly-claimed windows through CapacityService' ); +$check( + str_contains($updateSource, '$dayBefore($oldPrestito)') + && str_contains($updateSource, '$dayAfter($oldScadenza)'), + 'claimed windows are the exact set difference (boundary days already held are excluded)' +); +$check( + substr_count($updateSource, 'isStrictIsoDate(') >= 4 + && !str_contains($updateSource, 'strtotime('), + 'update() validates dates strictly (exact Y-m-d, real calendar) with no strtotime' +); $check( !str_contains($updateSource, 'hasFreeCapacity($libroId, $newPrestito, $newScadenza'), 'update() no longer re-checks the whole loan window (which bounced every edit)' @@ -229,8 +243,9 @@ : ''; $check($bulkSource !== '', 'applyBulkLoanExtension() source section extracted'); $check( - str_contains($bulkSource, "hasFreeCapacity(\$bookId, (string) \$loan['data_scadenza'], \$newDueDate"), - 'bulk extension checks the extension window only, like renew()' + str_contains($bulkSource, 'hasFreeCapacity($bookId, $extensionStart, $newDueDate') + && str_contains($bulkSource, '$copyOverlap->bind_param(\'iiss\', $copyId, $loanId, $newDueDate, $extensionStart)'), + 'bulk extension gates capacity AND copy overlap on the same added-days interval' ); $check( str_contains($loansIndex, "case 'no_copies_available':") From 78ac15ba1b5fdbe242ecf7d98f659b13a6cd3c5f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 13:45:43 +0000 Subject: [PATCH 09/69] test(loans): align history-predicate consistency guard with PR #337 semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's loan-reservation-consistency guard pins the exact history predicate shared by the three history consumers (user dashboard, profile, mobile API). PR #337 deliberately extended that predicate with the closed no-return states (annullato, scaduto) so cancelled/expired loans stop vanishing from history (#333) — update the pinned literal accordingly. The guard still enforces that all three consumers share one predicate. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- tests/loan-reservation-consistency.unit.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/loan-reservation-consistency.unit.php b/tests/loan-reservation-consistency.unit.php index 0a276484f..490ddec78 100644 --- a/tests/loan-reservation-consistency.unit.php +++ b/tests/loan-reservation-consistency.unit.php @@ -132,7 +132,11 @@ function assertNotContainsText(string $needle, string $haystack, string $message ] as $historyPath) { $history = readFileOrFail($root . '/' . $historyPath); assertNotContainsText("stato != 'prestato'", $history, "{$historyPath} must not show pending/cancelled rows as loan history"); - assertContainsText("stato IN ('restituito','perso','danneggiato')", $history, "{$historyPath} must use terminal physical-loan outcomes for history"); + // Since PR #337 the history predicate includes the CLOSED no-return states + // too (annullato = user-cancelled, scaduto = pickup expired): they are + // terminal outcomes and must not vanish from the user's history (#333). + // The three consumers must keep sharing this exact predicate. + assertContainsText("stato IN ('restituito','perso','danneggiato','annullato','scaduto')", $history, "{$historyPath} must use terminal loan outcomes (incl. cancelled/expired) for history"); } echo "Loan/reservation consistency unit checks passed.\n"; From 2a7a3d7b399498344d4bb6ae8a354912188d57d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 15:41:08 +0000 Subject: [PATCH 10/69] =?UTF-8?q?fix(loans):=20third-round=20CodeRabbit=20?= =?UTF-8?q?review=20=E2=80=94=20NUL-safe=20date=20validation,=20structural?= =?UTF-8?q?=20test=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - isStrictIsoDate(): shape-check via /^\d{4}-\d{2}-\d{2}$/D BEFORE touching DateTime — createFromFormat() throws ValueError on input containing NUL bytes (data_prestito=2026-01-01%00), which outside the try block surfaced as an HTTP 500 instead of invalid_dates. The anchor also rejects trailing newlines and non-ASCII digits. Verified against NUL/newline/Arabic-digit inputs: all return false, nothing throws. - Regression guards made structural: the requested_at check now isolates the schema line and asserts the ABSENCE of the legacy 'nullable' flag alongside the 3.1 type array (the mixed form no longer passes), and the exact-diff check asserts the boundary helpers actually feed the claimedWindows bounds instead of merely appearing in the source. 54 guards green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- app/Controllers/PrestitiController.php | 7 +++++++ tests/issues-333-334-336.unit.php | 22 +++++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index 39fec3569..e095825f3 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -2084,6 +2084,13 @@ public function exportCsv(Request $request, Response $response, mysqli $db): Res */ 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; } diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index 167b46510..8a3c0a2c1 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -182,9 +182,19 @@ str_contains($mobileOpenApi, "'status_label'") && str_contains($mobileOpenApi, "'requested_at'"), 'OpenAPI schema documents the additive status_label and requested_at fields' ); +// Strutturale (CodeRabbit): estrai la RIGA dello schema e verifica insieme la +// forma 3.1 e l'ASSENZA del flag legacy — "type array + nullable=true" non passa. +$requestedAtSchemaLine = ''; +foreach (explode("\n", $mobileOpenApi) as $openApiLine) { + if (str_contains($openApiLine, "'requested_at'")) { + $requestedAtSchemaLine = $openApiLine; + break; + } +} $check( - str_contains($mobileOpenApi, "'requested_at' => ['type' => ['string', 'null']"), - 'requested_at declares nullability the OpenAPI 3.1 way (type array, no nullable flag)' + str_contains($requestedAtSchemaLine, "'type' => ['string', 'null']") + && !str_contains($requestedAtSchemaLine, "'nullable'"), + 'requested_at declares nullability the OpenAPI 3.1 way (type array, no legacy nullable flag)' ); $check( str_contains($mobileActions, "'requested_at'") @@ -222,10 +232,12 @@ && str_contains($updateSource, 'excludePrestitoId: $id'), 'update() checks capacity on the newly-claimed windows through CapacityService' ); +// Strutturale (CodeRabbit): i boundary helper devono ALIMENTARE il calcolo +// delle finestre, non solo comparire nel testo della funzione. $check( - str_contains($updateSource, '$dayBefore($oldPrestito)') - && str_contains($updateSource, '$dayAfter($oldScadenza)'), - 'claimed windows are the exact set difference (boundary days already held are excluded)' + str_contains($updateSource, '$claimedWindows[] = [$newPrestito, min($dayBefore($oldPrestito), $newScadenza)]') + && str_contains($updateSource, '$claimedWindows[] = [max($dayAfter($oldScadenza), $newPrestito), $newScadenza]'), + 'claimed windows are the exact set difference (boundary helpers feed the window bounds)' ); $check( substr_count($updateSource, 'isStrictIsoDate(') >= 4 From 8cb790151fee821972ab491cfe93637b00668d0b Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 11 Aug 2026 19:39:35 +0200 Subject: [PATCH 11/69] fix(loans): guard assigned copy during date edits --- app/Controllers/PrestitiController.php | 31 +++++++++++++- app/Views/prestiti/index.php | 3 ++ locale/da_DK.json | 3 +- locale/de_DE.json | 3 +- locale/en_US.json | 3 +- locale/fr_FR.json | 3 +- locale/it_IT.json | 3 +- tests/issues-333-334-336.unit.php | 9 +++- tests/loan-bulk-extension-capacity.unit.php | 47 +++++++++++++++++++-- 9 files changed, 94 insertions(+), 11 deletions(-) diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index e095825f3..effa3e129 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -782,7 +782,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id // 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, utente_id, data_prestito, data_scadenza FROM prestiti WHERE id=? FOR UPDATE'); + $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(); @@ -913,6 +913,35 @@ public function update(Request $request, Response $response, mysqli $db, int $id return $response->withHeader('Location', url('/admin/loans') . '?error=no_copies_available')->withStatus(302); } } + + // 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(); + } } // update() del repository non tocca MAI i campi lifecycle (vedi il suo diff --git a/app/Views/prestiti/index.php b/app/Views/prestiti/index.php index 2e444f0ee..6909725b1 100644 --- a/app/Views/prestiti/index.php +++ b/app/Views/prestiti/index.php @@ -91,6 +91,9 @@ // è un conflitto con un altro impegno nel periodo richiesto. echo __('Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.'); break; + case 'loan_copy_conflict': + echo __('Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.'); + break; case 'extension_conflicts': echo __('Impossibile rinnovare: un altro prestito o prenotazione occupa il periodo richiesto.'); break; diff --git a/locale/da_DK.json b/locale/da_DK.json index c78ef56c2..8da15f76d 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -6766,5 +6766,6 @@ "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella
Pagina Cookie per conformità GDPR.": "📋 Vigtigt: Du skal manuelt angive de cookies, som disse scripts sporer, på Cookie-siden for at overholde GDPR.", "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sproget er slettet, men en oversættelsesfil kunne ikke fjernes: den kan dukke op igen, hvis du genopretter sproget med samme kode.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Ændringen blev ikke gemt: I den nye periode er alle eksemplarer allerede optaget af andre udlån eller reservationer.", - "Rinnovo non riuscito. Riprova.": "Fornyelsen mislykkedes. Prøv igen." + "Rinnovo non riuscito. Riprova.": "Fornyelsen mislykkedes. Prøv igen.", + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Ændringen blev ikke gemt: Det tildelte eksemplar er allerede optaget af et andet lån i den nye periode." } diff --git a/locale/de_DE.json b/locale/de_DE.json index c0033319d..1a2a2cc92 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6766,5 +6766,6 @@ "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Wichtig: Sie müssen die von diesen Skripten erfassten Cookies manuell auf der Cookie-Seite auflisten, um die DSGVO einzuhalten.", "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Sprache gelöscht, aber eine Übersetzungsdatei konnte nicht entfernt werden: Sie könnte wieder auftauchen, wenn Sie die Sprache mit demselben Code neu anlegen.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Änderung nicht gespeichert: Im neuen Zeitraum sind alle Exemplare bereits durch andere Ausleihen oder Vormerkungen belegt.", - "Rinnovo non riuscito. Riprova.": "Verlängerung fehlgeschlagen. Bitte erneut versuchen." + "Rinnovo non riuscito. Riprova.": "Verlängerung fehlgeschlagen. Bitte erneut versuchen.", + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Änderung nicht gespeichert: Das zugewiesene Exemplar ist im neuen Zeitraum bereits durch eine andere Ausleihe belegt." } diff --git a/locale/en_US.json b/locale/en_US.json index fe1dd1f03..7af4145be 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6766,5 +6766,6 @@ "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important: You must manually list the cookies tracked by these scripts on the Cookie Page for GDPR compliance.", "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Language deleted, but a translation file could not be removed: it may reappear if you recreate the language with the same code.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Change not saved: in the new period every copy is already taken by other loans or reservations.", - "Rinnovo non riuscito. Riprova.": "Renewal failed. Please try again." + "Rinnovo non riuscito. Riprova.": "Renewal failed. Please try again.", + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Change not saved: the assigned copy is already committed to another loan in the new period." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index a6dc2417c..05ef7f26c 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6766,5 +6766,6 @@ "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Important : vous devez lister manuellement les cookies suivis par ces scripts sur la Page Cookies pour la conformité RGPD.", "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Modification non enregistrée : sur la nouvelle période, tous les exemplaires sont déjà occupés par d'autres prêts ou réservations.", - "Rinnovo non riuscito. Riprova.": "Échec du renouvellement. Veuillez réessayer." + "Rinnovo non riuscito. Riprova.": "Échec du renouvellement. Veuillez réessayer.", + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Modification non enregistrée : l’exemplaire attribué est déjà réservé par un autre prêt pendant la nouvelle période." } diff --git a/locale/it_IT.json b/locale/it_IT.json index c210a4bf3..141bf1eee 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6766,5 +6766,6 @@ "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.": "📋 Importante: Devi elencare manualmente i cookie tracciati da questi script nella Pagina Cookie per conformità GDPR.", "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.", - "Rinnovo non riuscito. Riprova.": "Rinnovo non riuscito. Riprova." + "Rinnovo non riuscito. Riprova.": "Rinnovo non riuscito. Riprova.", + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo." } diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index 8a3c0a2c1..4902eaa5f 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -232,6 +232,11 @@ && str_contains($updateSource, 'excludePrestitoId: $id'), 'update() checks capacity on the newly-claimed windows through CapacityService' ); +$check( + str_contains($updateSource, '$copyOverlap->bind_param(\'iiss\', $copyId, $id, $claimEnd, $claimStart)') + && str_contains($updateSource, '?error=loan_copy_conflict'), + 'update() checks the assigned copy on the same newly-claimed windows with a dedicated error' +); // Strutturale (CodeRabbit): i boundary helper devono ALIMENTARE il calcolo // delle finestre, non solo comparire nel testo della funzione. $check( @@ -261,7 +266,8 @@ ); $check( str_contains($loansIndex, "case 'no_copies_available':") - && str_contains($loansIndex, "case 'extension_conflicts':"), + && str_contains($loansIndex, "case 'extension_conflicts':") + && str_contains($loansIndex, "case 'loan_copy_conflict':"), 'loans list banner explains capacity conflicts instead of a generic error' ); $check( @@ -273,6 +279,7 @@ // The new user-facing strings must be translated in every bundled locale. $newStrings = [ 'Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.', + 'Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.', 'Rinnovo non riuscito. Riprova.', ]; foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $locale) { diff --git a/tests/loan-bulk-extension-capacity.unit.php b/tests/loan-bulk-extension-capacity.unit.php index b7d3aa9b5..50a0a5aa5 100644 --- a/tests/loan-bulk-extension-capacity.unit.php +++ b/tests/loan-bulk-extension-capacity.unit.php @@ -2,9 +2,10 @@ declare(strict_types=1); /** - * End-to-end database contract for #281 bulk extension. It invokes the real - * controller and proves book-capacity conflicts, physical-copy conflicts and - * all-or-nothing rollback behavior. + * End-to-end database contract for #281 bulk extension and #336 manual date + * editing. It invokes the real controller and proves book-capacity conflicts, + * physical-copy conflicts, newly-claimed-window semantics and all-or-nothing + * rollback behavior. * * Run: php tests/loan-bulk-extension-capacity.unit.php */ @@ -156,6 +157,21 @@ return (new PrestitiController())->bulkExtend($request, $response, $db); }; +$callUpdate = static function (int $loanId, int $userId, string $start, string $due) use ($db) { + // The controller authorizes from the session, while processed_by must also + // reference a real user because of the FK. Reuse the fixture borrower. + $_SESSION['user'] = ['tipo_utente' => 'admin', 'id' => $userId]; + $request = (new ServerRequestFactory()) + ->createServerRequest('POST', '/admin/loans/edit/' . $loanId) + ->withParsedBody([ + 'utente_id' => $userId, + 'data_prestito' => $start, + 'data_scadenza' => $due, + ]); + $response = (new ResponseFactory())->createResponse(); + return (new PrestitiController())->update($request, $response, $db, $loanId); +}; + $today = new DateTimeImmutable(DateHelper::today()); $start = $today->modify('-5 days')->format('Y-m-d'); $due = $today->modify('+2 days')->format('Y-m-d'); @@ -189,15 +205,38 @@ echo "B. Physical-copy schedule remains exclusive\n"; [$copyBook, [$scheduledCopy, $freeCopy]] = $makeBook(2); -$currentLoan = $makeLoan($copyBook, $scheduledCopy, $makeUser(), $start, $due); +$currentUser = $makeUser(); +$currentLoan = $makeLoan($copyBook, $scheduledCopy, $currentUser, $start, $due); $futureStart = $today->modify('+8 days')->format('Y-m-d'); $futureEnd = $today->modify('+12 days')->format('Y-m-d'); $futureLoan = $makeLoan($copyBook, $scheduledCopy, $makeUser(), $futureStart, $futureEnd, 'prenotato'); $check($futureLoan > 0 && $freeCopy > 0, 'fixture has spare book capacity but a busy assigned copy'); +$updateResponse = $callUpdate($currentLoan, $currentUser, $start, $today->modify('+10 days')->format('Y-m-d')); +$check(str_contains($updateResponse->getHeaderLine('Location'), 'error=loan_copy_conflict'), 'manual date edit reports the dedicated same-copy conflict'); +$check($dueDate($currentLoan) === $due, 'manual copy conflict leaves the original due date unchanged'); $response = $callBulk([$currentLoan], 10); $check(str_contains($response->getHeaderLine('Location'), 'error=bulk_extend_conflict'), 'same-copy future schedule blocks the extension despite spare book capacity'); $check($dueDate($currentLoan) === $due, 'copy conflict leaves the original due date unchanged'); +echo "C. Manual edit checks only newly claimed days (#336)\n"; +[$reservationBook, [$reservationCopy]] = $makeBook(1); +$reservationHolder = $makeUser(); +$reservationLoan = $makeLoan($reservationBook, $reservationCopy, $reservationHolder, $start, $due); +$queuedUser = $makeUser(); +$oldOverlapStart = $today->modify('-1 day')->format('Y-m-d'); +$oldOverlapEnd = $today->modify('+1 day')->format('Y-m-d'); +$stmt = $db->prepare( + "INSERT INTO prenotazioni (libro_id, utente_id, data_inizio_richiesta, data_fine_richiesta, data_scadenza_prenotazione, stato, queue_position) + VALUES (?, ?, ?, ?, ?, 'attiva', 1)" +); +$stmt->bind_param('iisss', $reservationBook, $queuedUser, $oldOverlapStart, $oldOverlapEnd, $oldOverlapEnd); +$stmt->execute(); +$stmt->close(); +$editedDue = $today->modify('+4 days')->format('Y-m-d'); +$updateResponse = $callUpdate($reservationLoan, $reservationHolder, $start, $editedDue); +$check(!str_contains($updateResponse->getHeaderLine('Location'), 'error='), 'existing reservation on the old window does not block a free added-days segment'); +$check($dueDate($reservationLoan) === $editedDue, 'manual edit persists the conflict-free extension'); + // ── Area 4: overdue loans clear "Overdue" when extended (issue #281 gap) ───── echo "D. Extending an overdue loan clears the overdue status\n"; $loanState = static function (int $loanId) use ($db): string { From 7b680b01869fec985e2dea966ab4babe7a6e23cc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 17:47:29 +0000 Subject: [PATCH 12/69] =?UTF-8?q?fix(loans):=20fourth-round=20CodeRabbit?= =?UTF-8?q?=20review=20=E2=80=94=20French=20wording,=20structural=20review?= =?UTF-8?q?-guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - locale/fr_FR.json: 'réservé par un autre prêt' implied the loan reserves the copy; 'affecté à un autre prêt' matches the source meaning (the copy is already assigned to another loan). - Regression guard: assert the exact $canReview exclusion expression in both user history views (each with its own loop variable, $p / $loan) instead of the variable's mere presence. 55 guards green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014KsK1ikTUGbUr5W3xrSJ6w --- locale/fr_FR.json | 2 +- tests/issues-333-334-336.unit.php | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 05ef7f26c..ae737f590 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6767,5 +6767,5 @@ "Lingua eliminata, ma un file di traduzione non è stato rimosso: potrebbe riapparire se ricrei la lingua con lo stesso codice.": "Langue supprimée, mais un fichier de traduction n'a pas pu être supprimé : il pourrait réapparaître si vous recréez la langue avec le même code.", "Modifica non salvata: nel nuovo periodo tutte le copie sono già impegnate da altri prestiti o prenotazioni.": "Modification non enregistrée : sur la nouvelle période, tous les exemplaires sont déjà occupés par d'autres prêts ou réservations.", "Rinnovo non riuscito. Riprova.": "Échec du renouvellement. Veuillez réessayer.", - "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Modification non enregistrée : l’exemplaire attribué est déjà réservé par un autre prêt pendant la nouvelle période." + "Modifica non salvata: la copia assegnata è già impegnata da un altro prestito nel nuovo periodo.": "Modification non enregistrée : l’exemplaire attribué est déjà affecté à un autre prêt pendant la nouvelle période." } diff --git a/tests/issues-333-334-336.unit.php b/tests/issues-333-334-336.unit.php index 4902eaa5f..61f150614 100644 --- a/tests/issues-333-334-336.unit.php +++ b/tests/issues-333-334-336.unit.php @@ -118,8 +118,11 @@ && str_contains($userDashboard, "'annullato' => 'fa-ban'"), 'both user history views give cancelled loans a dedicated icon' ); +// Strutturale (CodeRabbit): asserisci l'ESPRESSIONE di esclusione, non la sola +// presenza della variabile — le due viste iterano con nomi diversi ($p / $loan). $check( - str_contains($profileReservations, '$canReview') && str_contains($userDashboard, '$canReview'), + str_contains($profileReservations, "\$canReview = !in_array(\$p['stato'], ['annullato', 'scaduto'], true)") + && str_contains($userDashboard, "\$canReview = !in_array(\$loan['stato'], ['annullato', 'scaduto'], true)"), 'history hides the review button for loans that never went out (annullato/scaduto)' ); From 585a33f7dcff20f9f18eeda868550f7b059230d0 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 11 Aug 2026 20:14:31 +0200 Subject: [PATCH 13/69] fix(loans): address discussion 238 workflow feedback --- app/Controllers/LibriController.php | 1 + app/Models/CopyRepository.php | 31 +++++++++++++++ app/Views/prestiti/crea_prestito.php | 24 +++++++++++- frontend/js/copy-scanner.js | 53 ++++++++++++++++++++++---- public/assets/copy-scanner.bundle.js | 2 +- tests/discussion-238-followup.unit.php | 35 +++++++++++++++++ tests/full-test.spec.js | 10 +++++ tests/label-pdf-content.spec.js | 13 ++++++- 8 files changed, 158 insertions(+), 11 deletions(-) diff --git a/app/Controllers/LibriController.php b/app/Controllers/LibriController.php index bda9664d3..f551b63e8 100644 --- a/app/Controllers/LibriController.php +++ b/app/Controllers/LibriController.php @@ -2858,6 +2858,7 @@ public function generateCopyLabelsPDF(Request $request, Response $response, mysq $copie[] = $copyRow; } $copiesStmt->close(); + $copie = \App\Models\CopyRepository::sortByInventoryNumber($copie); if (count($copie) === 0) { $_SESSION['error_message'] = __('Nessuna copia disponibile per la stampa delle etichette.'); diff --git a/app/Models/CopyRepository.php b/app/Models/CopyRepository.php index d0d298585..1b6205417 100644 --- a/app/Models/CopyRepository.php +++ b/app/Models/CopyRepository.php @@ -14,6 +14,36 @@ public function __construct(mysqli $db) $this->db = $db; } + /** + * Sort physical-copy rows by inventory code using human/natural ordering. + * VARCHAR ordering puts C10 before C2; labels and the admin table must instead + * follow the sequence staff see on the physical copies (#238). + * + * @param array> $copies + * @return array> + */ + public static function sortByInventoryNumber(array $copies): array + { + usort($copies, static function (array $left, array $right): int { + $leftCode = (string) ($left['numero_inventario'] ?? ''); + $rightCode = (string) ($right['numero_inventario'] ?? ''); + $comparison = strnatcasecmp($leftCode, $rightCode); + if ($comparison !== 0) { + return $comparison; + } + + // Make ties deterministic across collations/case variants and repeated + // calls, even though inventory codes are normally globally unique. + $comparison = strcmp($leftCode, $rightCode); + if ($comparison !== 0) { + return $comparison; + } + return ((int) ($left['id'] ?? 0)) <=> ((int) ($right['id'] ?? 0)); + }); + + return $copies; + } + /** * Ottiene tutte le copie di un libro */ @@ -48,6 +78,7 @@ public function getByBookId(int $bookId): array } $stmt->close(); + $copie = self::sortByInventoryNumber($copie); $copyIndexes = []; foreach ($copie as $index => $copy) { diff --git a/app/Views/prestiti/crea_prestito.php b/app/Views/prestiti/crea_prestito.php index bdcccd3cc..92051c91c 100644 --- a/app/Views/prestiti/crea_prestito.php +++ b/app/Views/prestiti/crea_prestito.php @@ -96,7 +96,7 @@ -