From 547f70007fff046cff534d72ad8af38d583047af Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:22:48 +0000 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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 08/10] =?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 09/10] 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 10/10] =?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)' );