From 0bfa3cd1795b91a68431bee6c7eebbe4eec46637 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 29 Jul 2026 18:13:04 +0200 Subject: [PATCH 1/3] fix: auto-approve loans and correct catalog counts --- app/Controllers/AutoriApiController.php | 10 ++- app/Controllers/FrontendController.php | 3 +- app/Controllers/LoanApprovalController.php | 7 +- app/Controllers/SettingsController.php | 7 +- app/Controllers/UserActionsController.php | 56 +++++++++++++- app/Models/SettingsRepository.php | 10 +++ app/Views/frontend/book-detail.php | 4 +- app/Views/settings/loans-tab.php | 39 ++++++++++ installer/database/data_da_DK.sql | 4 + installer/database/data_de_DE.sql | 4 + installer/database/data_en_US.sql | 4 + installer/database/data_fr_FR.sql | 4 + installer/database/data_it_IT.sql | 4 + .../database/migrations/migrate_0.7.46.sql | 7 ++ locale/da_DK.json | 8 +- locale/de_DE.json | 8 +- locale/en_US.json | 8 +- locale/fr_FR.json | 8 +- locale/it_IT.json | 8 +- .../src/Controllers/ActionsController.php | 16 +++- .../src/Controllers/HealthController.php | 2 + .../src/Controllers/OpenApiController.php | 23 +++++- tests/issues-301-306.unit.php | 76 +++++++++++++++++++ 23 files changed, 301 insertions(+), 19 deletions(-) create mode 100644 installer/database/migrations/migrate_0.7.46.sql create mode 100644 tests/issues-301-306.unit.php diff --git a/app/Controllers/AutoriApiController.php b/app/Controllers/AutoriApiController.php index 45b0a49f3..a7bcb1986 100644 --- a/app/Controllers/AutoriApiController.php +++ b/app/Controllers/AutoriApiController.php @@ -172,7 +172,10 @@ public function list(Request $request, Response $response, mysqli $db): Response $selectNaz = $colNaz !== null ? "a.`$colNaz` AS nazionalita" : "'' AS nazionalita"; $sql_prepared = "SELECT a.id, a.nome, a.pseudonimo, a.data_nascita, a.data_morte, a.biografia, a.sito_web, $selectNaz, - (SELECT COUNT(*) FROM libri_autori la WHERE la.autore_id = a.id) AS libri_count + (SELECT COUNT(*) + FROM libri_autori la + JOIN libri l ON l.id = la.libro_id AND l.deleted_at IS NULL + WHERE la.autore_id = a.id) AS libri_count FROM autori a $where_prepared ORDER BY $orderColumn $orderDir LIMIT ?, ?"; $stmt = $db->prepare($sql_prepared); @@ -377,7 +380,10 @@ public function bulkExport(Request $request, Response $response, mysqli $db): Re $sql = "SELECT a.id, a.nome, a.pseudonimo, a.data_nascita, a.data_morte, a.sito_web, $selectNaz, - (SELECT COUNT(*) FROM libri_autori la WHERE la.autore_id = a.id) AS libri_count + (SELECT COUNT(*) + FROM libri_autori la + JOIN libri l ON l.id = la.libro_id AND l.deleted_at IS NULL + WHERE la.autore_id = a.id) AS libri_count FROM autori a WHERE a.id IN ($placeholders) ORDER BY a.nome ASC"; diff --git a/app/Controllers/FrontendController.php b/app/Controllers/FrontendController.php index 6cdf1995f..f4fab4a7a 100644 --- a/app/Controllers/FrontendController.php +++ b/app/Controllers/FrontendController.php @@ -1054,7 +1054,8 @@ private function getFilterOptions(mysqli $db, array $filters = []): array LEFT JOIN generi gfp ON gf.parent_id = gfp.id LEFT JOIN generi gfpp ON gfp.parent_id = gfpp.id LEFT JOIN generi sg ON l.sottogenere_id = sg.id - WHERE ( + WHERE l.deleted_at IS NULL + AND ( l.genere_id = g.id OR l.sottogenere_id = g.id OR l.genere_id IN (SELECT id FROM generi WHERE parent_id = g.id) diff --git a/app/Controllers/LoanApprovalController.php b/app/Controllers/LoanApprovalController.php index 054d9c3c0..ffd7f8e72 100644 --- a/app/Controllers/LoanApprovalController.php +++ b/app/Controllers/LoanApprovalController.php @@ -526,10 +526,13 @@ public function approveLoan(Request $request, Response $response, mysqli $db): R $db->commit(); - // Send appropriate notification to user + // Send appropriate notification to user. Issue #301 explicitly keeps + // the approval email in the auto-approval flow; manual immediate + // approvals retain the more specific pickup-ready notification. try { $notificationService = new \App\Support\NotificationService($db); - if ($isFutureLoan) { + $automaticApproval = (bool) $request->getAttribute('automatic_loan_approval', false); + if ($isFutureLoan || $automaticApproval) { // Future loan: send general approval notification $notificationService->sendLoanApprovedNotification($loanId); } else { diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index 29083f1b5..487064a4c 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -1278,7 +1278,7 @@ public function updateEventSettings(Request $request, Response $response, mysqli } /** - * @return array{loan_duration_days: int, pickup_expiry_days: int, max_renewals: int, max_active_loans_per_user: int, max_loan_duration_days: int} + * @return array{loan_duration_days: int, pickup_expiry_days: int, max_renewals: int, max_active_loans_per_user: int, max_loan_duration_days: int, auto_approve_requests: bool} */ private function resolveLoansSettings(SettingsRepository $repository): array { @@ -1288,6 +1288,7 @@ private function resolveLoansSettings(SettingsRepository $repository): array 'max_renewals' => (int) ($repository->get('loans', 'max_renewals', '3') ?? 3), 'max_active_loans_per_user' => (int) ($repository->get('loans', 'max_active_loans_per_user', '0') ?? 0), 'max_loan_duration_days' => (int) ($repository->get('loans', 'max_loan_duration_days', '90') ?? 90), + 'auto_approve_requests' => $repository->autoApproveLoanRequests(), ]; } @@ -1311,12 +1312,16 @@ public function updateLoansSettings(Request $request, Response $response, mysqli $maxRenewals = min(100, max(0, (int) ($data['max_renewals'] ?? 3))); // 0 … 100 $maxActiveLoans = min(1000, max(0, (int) ($data['max_active_loans_per_user'] ?? 0))); // 0 (unlimited) … 1000 $maxLoanDuration = min(3650, max(1, (int) ($data['max_loan_duration_days'] ?? 90))); // 1 day … 10 years (reservation-window cap enforced by ReservationsController) + $autoApprove = isset($data['auto_approve_requests']) + && is_scalar($data['auto_approve_requests']) + && (string) $data['auto_approve_requests'] === '1'; $repository->set('loans', 'loan_duration_days', (string) $loanDurationDays); $repository->set('loans', 'pickup_expiry_days', (string) $pickupExpiryDays); $repository->set('loans', 'max_renewals', (string) $maxRenewals); $repository->set('loans', 'max_active_loans_per_user', (string) $maxActiveLoans); $repository->set('loans', 'max_loan_duration_days', (string) $maxLoanDuration); + $repository->set('loans', 'auto_approve_requests', $autoApprove ? '1' : '0'); $_SESSION['success_message'] = __('Impostazioni prestiti aggiornate correttamente.'); return $response->withHeader('Location', url('/admin/settings?tab=loans'))->withStatus(302); diff --git a/app/Controllers/UserActionsController.php b/app/Controllers/UserActionsController.php index 136aba362..daa149c5a 100644 --- a/app/Controllers/UserActionsController.php +++ b/app/Controllers/UserActionsController.php @@ -9,6 +9,7 @@ use App\Controllers\ReservationManager; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Slim\Psr7\Response as SlimResponse; class UserActionsController { @@ -585,7 +586,9 @@ public function loan(Request $request, Response $response, mysqli $db): Response } } - // Insert as 'pendente' - requires admin approval + // Always create the canonical pending request first. When automatic + // approval is enabled, the same race-safe approval controller used by + // admins promotes it immediately after request creation. $stmt = $db->prepare("INSERT INTO prestiti (libro_id, utente_id, data_prestito, data_scadenza, stato, attivo) VALUES (?, ?, ?, ?, 'pendente', 0)"); $stmt->bind_param('iiss', $libroId, $utenteId, $data_prestito, $data_scadenza); @@ -599,6 +602,11 @@ public function loan(Request $request, Response $response, mysqli $db): Response $stmt->close(); $db->commit(); + // Promote immediately before performing slower email I/O, minimizing + // the post-commit window in which another request could claim the + // same copy. The canonical approval path re-checks every constraint. + $autoApproved = $this->autoApproveLoanRequest($request, $db, $newLoanId); + // Notify admins about new loan request (outside transaction) try { $notificationService = new \App\Support\NotificationService($db); @@ -607,7 +615,11 @@ public function loan(Request $request, Response $response, mysqli $db): Response SecureLogger::warning(__('Notifica richiesta prestito fallita'), ['error' => $e->getMessage()]); } - return $this->back($response, ['loan_request_success' => 1]); + return $this->back($response, [ + 'loan_request_success' => 1, + 'loan_id' => $newLoanId, + 'auto_approved' => $autoApproved ? 1 : 0, + ]); } catch (\Throwable $e) { $db->rollback(); @@ -763,6 +775,46 @@ private function back(Response $response, array $params): Response return $response->withHeader('Location', $referer . $sep . $qs)->withStatus(302); } + /** + * Promote a newly-created request through the canonical approval pipeline. + * A failure deliberately leaves the request pending, so an administrator can + * still process it instead of losing an otherwise valid request. + */ + private function autoApproveLoanRequest(Request $request, mysqli $db, int $loanId): bool + { + $settings = new \App\Models\SettingsRepository($db); + if (!$settings->autoApproveLoanRequests()) { + return false; + } + + try { + $approvalRequest = $request + ->withParsedBody(['loan_id' => $loanId]) + ->withAttribute('automatic_loan_approval', true); + $result = (new LoanApprovalController())->approveLoan( + $approvalRequest, + new SlimResponse(), + $db + ); + + if ($result->getStatusCode() >= 200 && $result->getStatusCode() < 300) { + return true; + } + + SecureLogger::warning('Automatic loan approval left request pending', [ + 'loan_id' => $loanId, + 'status' => $result->getStatusCode(), + ]); + } catch (\Throwable $e) { + SecureLogger::warning('Automatic loan approval failed; request left pending', [ + 'loan_id' => $loanId, + 'error' => $e->getMessage(), + ]); + } + + return false; + } + /** * Validate referer URL to prevent open redirect attacks */ diff --git a/app/Models/SettingsRepository.php b/app/Models/SettingsRepository.php index a4a852457..209a3fb77 100644 --- a/app/Models/SettingsRepository.php +++ b/app/Models/SettingsRepository.php @@ -82,6 +82,16 @@ public function daysBeforeExpiryWarning(): int return max(0, (int)($this->get('advanced', 'days_before_expiry_warning', '3') ?? 3)); } + /** + * Whether user loan requests should skip the manual approval queue. + * Kept here so the web flow and the Mobile API share the same default and + * interpretation of the persisted setting. + */ + public function autoApproveLoanRequests(): bool + { + return ($this->get('loans', 'auto_approve_requests', '0') ?? '0') === '1'; + } + /** * @return array */ diff --git a/app/Views/frontend/book-detail.php b/app/Views/frontend/book-detail.php index 3ab5a857e..bc31f8de1 100644 --- a/app/Views/frontend/book-detail.php +++ b/app/Views/frontend/book-detail.php @@ -1857,7 +1857,9 @@ class="book-cover-large img-fluid"
diff --git a/app/Views/settings/loans-tab.php b/app/Views/settings/loans-tab.php index 0d8d613bd..8b1e4c3c3 100644 --- a/app/Views/settings/loans-tab.php +++ b/app/Views/settings/loans-tab.php @@ -7,6 +7,45 @@
+ +
+
+

+ + +

+

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

+
+ +
+
+
+
diff --git a/installer/database/data_da_DK.sql b/installer/database/data_da_DK.sql index b1efca32f..e5ec2a39d 100644 --- a/installer/database/data_da_DK.sql +++ b/installer/database/data_da_DK.sql @@ -31,6 +31,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_loan_duration_days', '90', 'Maksimal lånevarighed, der kan anmodes om, i dage') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'auto_approve_requests', '0', 'Godkend låneanmodninger automatisk') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `generi` VALUES (1,'Prosa',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); INSERT INTO `generi` VALUES (2,'Poesi',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); INSERT INTO `generi` VALUES (3,'Teater',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); diff --git a/installer/database/data_de_DE.sql b/installer/database/data_de_DE.sql index 605090928..428a34874 100644 --- a/installer/database/data_de_DE.sql +++ b/installer/database/data_de_DE.sql @@ -214,6 +214,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_loan_duration_days', '90', 'Maximal anforderbare Ausleihdauer in Tagen') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'auto_approve_requests', '0', 'Ausleihanfragen automatisch genehmigen') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + -- ============================================================================ -- System Settings - Complete default configuration -- These replace the old storage/settings.json file diff --git a/installer/database/data_en_US.sql b/installer/database/data_en_US.sql index 0de1132e6..8d0a9b0fe 100644 --- a/installer/database/data_en_US.sql +++ b/installer/database/data_en_US.sql @@ -214,6 +214,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_loan_duration_days', '90', 'Maximum requestable loan duration in days') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'auto_approve_requests', '0', 'Automatically approve loan requests') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + -- ============================================================================ -- System Settings - Complete default configuration -- These replace the old storage/settings.json file diff --git a/installer/database/data_fr_FR.sql b/installer/database/data_fr_FR.sql index 03e60c900..d919bac9a 100644 --- a/installer/database/data_fr_FR.sql +++ b/installer/database/data_fr_FR.sql @@ -214,6 +214,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_loan_duration_days', '90', 'Durée maximale demandable pour un emprunt en jours') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'auto_approve_requests', '0', 'Approuver automatiquement les demandes de prêt') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + -- ============================================================================ -- System Settings - Complete default configuration -- These replace the old storage/settings.json file diff --git a/installer/database/data_it_IT.sql b/installer/database/data_it_IT.sql index 9e3f37957..7e30eeaf8 100644 --- a/installer/database/data_it_IT.sql +++ b/installer/database/data_it_IT.sql @@ -31,6 +31,10 @@ INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `desc ('loans', 'max_loan_duration_days', '90', 'Durata massima richiedibile per un prestito in giorni') ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) VALUES +('loans', 'auto_approve_requests', '0', 'Approva automaticamente le richieste di prestito') +ON DUPLICATE KEY UPDATE description = VALUES(description), updated_at = NOW(); + INSERT INTO `generi` VALUES (1,'Prosa',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); INSERT INTO `generi` VALUES (2,'Poesia',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); INSERT INTO `generi` VALUES (3,'Teatro',NULL,'2025-10-20 16:20:00','2025-10-20 16:20:00',NULL); diff --git a/installer/database/migrations/migrate_0.7.46.sql b/installer/database/migrations/migrate_0.7.46.sql new file mode 100644 index 000000000..3d4e811a1 --- /dev/null +++ b/installer/database/migrations/migrate_0.7.46.sql @@ -0,0 +1,7 @@ +-- Migration 0.7.46 +-- Issue #301: optional automatic approval of immediate loan requests. +-- Safe and idempotent: existing administrators keep the manual workflow. + +INSERT INTO `system_settings` (`category`, `setting_key`, `setting_value`, `description`) +VALUES ('loans', 'auto_approve_requests', '0', 'Approva automaticamente le richieste di prestito') +ON DUPLICATE KEY UPDATE `setting_value` = `setting_value`; diff --git a/locale/da_DK.json b/locale/da_DK.json index 1dcf4036f..7981e2d1e 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -6652,5 +6652,11 @@ "Titolo della recensione": "Anmeldelsens titel", "Email dell'utente": "Brugerens e-mail", "Nome completo dell'utente": "Brugerens fulde navn", - "Link alla lista dei desideri": "Link til ønskelisten" + "Link alla lista dei desideri": "Link til ønskelisten", + "Approvazione automatica": "Automatisk godkendelse", + "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.": "Godkend låneanmodninger automatisk, og flyt dem direkte til afventer afhentning.", + "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.": "E-mails om den nye anmodning til administratorer og om godkendelsen til låneren sendes fortsat.", + "Auto-approva le richieste": "Godkend anmodninger automatisk", + "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.": "Spring manuel godkendelse over for nye lån, som brugerne anmoder om.", + "Prestito approvato. Il libro è in attesa di ritiro.": "Lånet er godkendt. Bogen afventer afhentning." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 229631a61..42c859391 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6652,5 +6652,11 @@ "Titolo della recensione": "Titel der Rezension", "Email dell'utente": "E-Mail des Benutzers", "Nome completo dell'utente": "Vollständiger Name des Benutzers", - "Link alla lista dei desideri": "Link zur Wunschliste" + "Link alla lista dei desideri": "Link zur Wunschliste", + "Approvazione automatica": "Automatische Genehmigung", + "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.": "Genehmigt Ausleihanfragen automatisch und setzt sie direkt auf Abholung ausstehend.", + "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.": "Die E-Mails zur neuen Anfrage an die Verwaltung und zur Genehmigung an die ausleihende Person werden weiterhin versendet.", + "Auto-approva le richieste": "Anfragen automatisch genehmigen", + "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.": "Überspringt die manuelle Genehmigung neuer Ausleihanfragen.", + "Prestito approvato. Il libro è in attesa di ritiro.": "Ausleihe genehmigt. Das Buch wartet auf Abholung." } diff --git a/locale/en_US.json b/locale/en_US.json index b3b969279..f8f53154e 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6652,5 +6652,11 @@ "Titolo della recensione": "Review title", "Email dell'utente": "User's email", "Nome completo dell'utente": "User's full name", - "Link alla lista dei desideri": "Link to the wishlist" + "Link alla lista dei desideri": "Link to the wishlist", + "Approvazione automatica": "Automatic approval", + "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.": "Automatically approve loan requests and move them directly to awaiting pickup.", + "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.": "The new request email to administrators and the approval email to the patron will still be sent.", + "Auto-approva le richieste": "Auto-approve requests", + "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.": "Skip manual approval for new loans requested by patrons.", + "Prestito approvato. Il libro è in attesa di ritiro.": "Loan approved. The book is awaiting pickup." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 97045bd6a..9ae09d151 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6652,5 +6652,11 @@ "Titolo della recensione": "Titre de l'avis", "Email dell'utente": "E-mail de l'utilisateur", "Nome completo dell'utente": "Nom complet de l'utilisateur", - "Link alla lista dei desideri": "Lien vers la liste de souhaits" + "Link alla lista dei desideri": "Lien vers la liste de souhaits", + "Approvazione automatica": "Approbation automatique", + "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.": "Approuve automatiquement les demandes de prêt et les place directement en attente de retrait.", + "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.": "Les e-mails de nouvelle demande aux administrateurs et d'approbation à l'usager continueront d'être envoyés.", + "Auto-approva le richieste": "Approuver automatiquement", + "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.": "Désactive l'approbation manuelle des nouveaux prêts demandés par les usagers.", + "Prestito approvato. Il libro è in attesa di ritiro.": "Prêt approuvé. Le livre est en attente de retrait." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 911ccc222..fdc131aa3 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6652,5 +6652,11 @@ "Titolo della recensione": "Titolo della recensione", "Email dell'utente": "Email dell'utente", "Nome completo dell'utente": "Nome completo dell'utente", - "Link alla lista dei desideri": "Link alla lista dei desideri" + "Link alla lista dei desideri": "Link alla lista dei desideri", + "Approvazione automatica": "Approvazione automatica", + "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.": "Approva automaticamente le richieste di prestito e passa subito allo stato in attesa di ritiro.", + "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.": "Le email per la nuova richiesta agli amministratori e per l'approvazione all'utente continueranno a essere inviate.", + "Auto-approva le richieste": "Auto-approva le richieste", + "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.": "Disattiva la fase di approvazione manuale per i nuovi prestiti richiesti dagli utenti.", + "Prestito approvato. Il libro è in attesa di ritiro.": "Prestito approvato. Il libro è in attesa di ritiro." } diff --git a/storage/plugins/mobile-api/src/Controllers/ActionsController.php b/storage/plugins/mobile-api/src/Controllers/ActionsController.php index 7c570c9b8..a3cc10c3b 100644 --- a/storage/plugins/mobile-api/src/Controllers/ActionsController.php +++ b/storage/plugins/mobile-api/src/Controllers/ActionsController.php @@ -177,7 +177,8 @@ public function myReservations(Request $request, ResponseInterface $response): R * rules. The body is `{ book_id, desired_date? }`. * * - With NO desired_date AND a copy currently loanable → immediate loan - * request (UserActionsController::loan), pending admin approval. + * request (UserActionsController::loan), either pending admin approval + * or immediately ready for pickup when auto-approval is enabled. * - Otherwise (a future desired_date, or no copy free now) → reservation in * the queue (UserActionsController::reserve). * @@ -231,10 +232,19 @@ public function requestReservation(Request $request, ResponseInterface $response $outcome = $this->redirectOutcome($result); if (isset($outcome['loan_request_success'])) { + $autoApproved = ($outcome['auto_approved'] ?? '0') === '1'; return ResponseEnvelope::success( $response, - ['type' => 'loan', 'book_id' => $bookId], - ['message' => __('Richiesta di prestito inviata. In attesa di approvazione.')], + [ + 'type' => 'loan', + 'book_id' => $bookId, + 'loan_id' => isset($outcome['loan_id']) ? (int) $outcome['loan_id'] : null, + 'auto_approved' => $autoApproved, + 'status' => $autoApproved ? 'da_ritirare' : 'pendente', + ], + ['message' => $autoApproved + ? __('Prestito approvato. Il libro è in attesa di ritiro.') + : __('Richiesta di prestito inviata. In attesa di approvazione.')], 201 ); } diff --git a/storage/plugins/mobile-api/src/Controllers/HealthController.php b/storage/plugins/mobile-api/src/Controllers/HealthController.php index c6cff3c47..35ab40446 100644 --- a/storage/plugins/mobile-api/src/Controllers/HealthController.php +++ b/storage/plugins/mobile-api/src/Controllers/HealthController.php @@ -69,6 +69,7 @@ public function index( // hides loans, reservations and wishlist everywhere. The app must do // the same, so we gate those feature flags off and expose the mode. $catalogueMode = (bool) ConfigStore::isCatalogueMode(); + $loanApprovalRequired = !(new \App\Models\SettingsRepository($this->db))->autoApproveLoanRequests(); $data = [ 'name' => $name, @@ -88,6 +89,7 @@ public function index( 'reviews' => !$catalogueMode, ], 'catalogue_mode' => $catalogueMode, + 'loan_approval_required' => $loanApprovalRequired, 'app_access_enabled' => $appAccessEnabled, 'registration_enabled' => $registrationEnabled, // Lightweight registration summary (a convenience mirror). The diff --git a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php index 602be9135..65e0d2230 100644 --- a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php +++ b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php @@ -622,7 +622,7 @@ private function paths(): array 'post' => [ 'tags' => ['loans'], 'summary' => 'Request a reservation / loan', - 'description' => 'Honors existing overlap, availability, and max-active-loans rules (same as the web form). Returns error codes for overlap, unavailable, or queue position.', + 'description' => 'Honors existing overlap, availability, max-active-loans, and automatic-approval settings (same as the web form). For immediate loans, data.auto_approved and data.status tell the client whether the request is pending or ready for pickup.', 'operationId' => 'postReservation', 'security' => [['bearerAuth' => []]], 'requestBody' => [ @@ -630,7 +630,22 @@ private function paths(): array 'content' => ['application/json' => ['schema' => ['$ref' => '#/components/schemas/ReservationRequest']]], ], 'responses' => [ - '201' => ['description' => 'Reservation created.', 'content' => $json], + '201' => [ + 'description' => 'Reservation or loan request created.', + 'content' => ['application/json' => ['schema' => [ + 'allOf' => [['$ref' => '#/components/schemas/Envelope']], + 'properties' => ['data' => [ + 'type' => 'object', + 'properties' => [ + 'type' => ['type' => 'string', 'enum' => ['loan', 'reservation']], + 'book_id' => ['type' => 'integer'], + 'loan_id' => ['type' => 'integer', 'nullable' => true], + 'auto_approved' => ['type' => 'boolean', 'nullable' => true], + 'status' => ['type' => 'string', 'nullable' => true, 'enum' => ['pendente', 'da_ritirare']], + ], + ]], + ]]], + ], '400' => ['$ref' => '#/components/responses/UnprocessableEntity'], '401' => ['$ref' => '#/components/responses/Unauthorized'], '409' => ['description' => 'Overlap or book unavailable.', 'content' => $json], @@ -1486,6 +1501,10 @@ private function healthPayloadSchema(): array 'type' => 'boolean', 'description' => 'True when the instance is in catalogue-only mode (loans, reservations and wishlist disabled).', ], + 'loan_approval_required' => [ + 'type' => 'boolean', + 'description' => 'False when immediate loan requests are automatically approved and move directly to da_ritirare.', + ], 'app_access_enabled' => ['type' => 'boolean'], 'registration_enabled' => ['type' => 'boolean'], 'registration' => ['$ref' => '#/components/schemas/RegistrationConfig'], diff --git a/tests/issues-301-306.unit.php b/tests/issues-301-306.unit.php new file mode 100644 index 000000000..66d1775d7 --- /dev/null +++ b/tests/issues-301-306.unit.php @@ -0,0 +1,76 @@ += 3, + 'publisher list/filter/export counts exclude soft-deleted books' +); + +$check(str_contains($settingsRepo, 'function autoApproveLoanRequests()'), 'loan setting has a shared typed accessor'); +$check( + str_contains($settingsController, "set('loans', 'auto_approve_requests'") + && str_contains($settingsView, 'name="auto_approve_requests"') + && str_contains($settingsView, 'class="toggle-checkbox sr-only"'), + 'loan settings persist an accessible toggle using the existing switch component' +); +$check( + strpos($actions, 'autoApproveLoanRequest($request, $db, $newLoanId)') < strpos($actions, 'notifyLoanRequest($newLoanId)'), + 'automatic approval runs before slow email I/O while preserving the administrator notification' +); +$check( + str_contains($approval, "getAttribute('automatic_loan_approval'") + && str_contains($approval, 'sendLoanApprovedNotification($loanId)'), + 'automatic approval sends the patron approval email' +); +$check( + str_contains($mobileActions, "'auto_approved' => \$autoApproved") + && str_contains($mobileActions, "'status' => \$autoApproved ? 'da_ritirare' : 'pendente'") + && str_contains($mobileHealth, "'loan_approval_required' => \$loanApprovalRequired") + && str_contains($mobileDocs, 'data.auto_approved'), + 'Mobile API advertises the setting and returns the actual request outcome' +); +$check( + str_contains($migration, "'auto_approve_requests', '0'") + && str_contains($migration, 'ON DUPLICATE KEY UPDATE `setting_value` = `setting_value`'), + 'upgrade migration is idempotent and preserves manual approval by default' +); + +foreach (['it_IT', 'en_US', 'fr_FR', 'de_DE', 'da_DK'] as $locale) { + $catalog = json_decode((string) file_get_contents($root . "/locale/{$locale}.json"), true, 512, JSON_THROW_ON_ERROR); + $check(isset($catalog['Approvazione automatica']), "{$locale} includes auto-approval translations"); +} + +echo PHP_EOL . "Passed: {$passed} Failed: {$failed}" . PHP_EOL; +exit($failed === 0 ? 0 : 1); From 750632d893329d1a546aca611e25a0d4218a73bc Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 29 Jul 2026 18:32:10 +0200 Subject: [PATCH 2/3] test(#301): behavioural auto-approval coverage + bump version to 0.7.46 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit version.json was still 0.7.45 while this branch ships migrate_0.7.46.sql — version_compare('0.7.46','0.7.45','<=') is FALSE, so the updater would silently skip the auto-approve setting migration on upgrade (and the release preflight would reject it). Bumped to 0.7.46 so the migration runs. Adds tests/loan-auto-approval-301.unit.php: 12 behavioural checks that seed real libri/copie/utenti/prestiti and drive the real UserActionsController::autoApprove- LoanRequest() (which delegates to the canonical approveLoan pipeline), asserting state transitions rather than string-matching the source: - setting accessor: absent/'0'/non-'1' → off, '1' → on; - OFF leaves the request in the pendente queue; - ON + available copy → 'da_ritirare', pickup_deadline set, a copy assigned, loan activated; - ON + no available copy → left pending, never wrongly auto-approved. The run restores the global auto_approve_requests setting and cleans its fixtures. --- tests/loan-auto-approval-301.unit.php | 222 ++++++++++++++++++++++++++ version.json | 2 +- 2 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 tests/loan-auto-approval-301.unit.php diff --git a/tests/loan-auto-approval-301.unit.php b/tests/loan-auto-approval-301.unit.php new file mode 100644 index 000000000..2e45cacdb --- /dev/null +++ b/tests/loan-auto-approval-301.unit.php @@ -0,0 +1,222 @@ +set_charset('utf8mb4'); +} catch (Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — mandatory for this test: {$e->getMessage()}\n"); + exit(1); +} + +// Keep email out of the way — approveLoan sends the patron notification; the +// 'mail' driver never blocks, but we also don't want a stray SMTP config to hang. +$db->query("UPDATE system_settings SET setting_value='mail' WHERE category='email' AND setting_key IN ('driver_mode','type')"); + +$run = substr(hash('sha256', uniqid((string) getmypid(), true)), 0, 10); +$titlePrefix = 'ZZAUTO301_' . $run; +$emailDomain = '@auto301.test.local'; + +// Preserve the global auto-approve setting so the run doesn't leave it toggled. +$origAuto = null; +if ($r = $db->query("SELECT setting_value FROM system_settings WHERE category='loans' AND setting_key='auto_approve_requests'")) { + if ($row = $r->fetch_assoc()) { + $origAuto = $row['setting_value']; + } +} + +$cleanup = static function () use ($db, $titlePrefix, $emailDomain, $origAuto): void { + $titleLike = $titlePrefix . '%'; + $emailLike = '%' . $emailDomain; + foreach ([ + 'DELETE p FROM prestiti p JOIN libri l ON l.id = p.libro_id WHERE l.titolo LIKE ?', + 'DELETE r FROM prenotazioni r JOIN libri l ON l.id = r.libro_id WHERE l.titolo LIKE ?', + 'DELETE c FROM copie c JOIN libri l ON l.id = c.libro_id WHERE l.titolo LIKE ?', + 'DELETE FROM libri WHERE titolo LIKE ?', + ] as $sql) { + $stmt = $db->prepare($sql); + $stmt->bind_param('s', $titleLike); + $stmt->execute(); + $stmt->close(); + } + $stmt = $db->prepare('DELETE FROM utenti WHERE email LIKE ?'); + $stmt->bind_param('s', $emailLike); + $stmt->execute(); + $stmt->close(); + // Restore the auto-approve setting to its pre-test value (or remove it). + if ($origAuto === null) { + $db->query("DELETE FROM system_settings WHERE category='loans' AND setting_key='auto_approve_requests'"); + } else { + $stmt = $db->prepare("INSERT INTO system_settings (category, setting_key, setting_value) VALUES ('loans','auto_approve_requests',?) ON DUPLICATE KEY UPDATE setting_value = VALUES(setting_value)"); + $stmt->bind_param('s', $origAuto); + $stmt->execute(); + $stmt->close(); + } +}; + +$cleanup(); +set_exception_handler(static function (Throwable $e) use ($cleanup, $db): void { + try { $cleanup(); } catch (Throwable) {} + fwrite(STDERR, "FAIL: {$e->getMessage()}\n"); + $db->close(); + exit(1); +}); + +$pass = 0; +$check = static function (bool $ok, string $label) use (&$pass): void { + if (!$ok) { + throw new RuntimeException($label); + } + $pass++; + echo " OK {$label}\n"; +}; + +// ── Fixture helpers ───────────────────────────────────────────────────────── +$bookSeq = 0; +$makeBook = static function (int $copies) use ($db, $titlePrefix, $run, &$bookSeq): int { + $bookSeq++; + $title = $titlePrefix . '_' . $bookSeq; + $stmt = $db->prepare("INSERT INTO libri (titolo, stato, copie_totali, copie_disponibili) VALUES (?, 'disponibile', ?, ?)"); + $stmt->bind_param('sii', $title, $copies, $copies); + $stmt->execute(); + $bookId = (int) $db->insert_id; + $stmt->close(); + $copyRepo = new CopyRepository($db); + for ($i = 1; $i <= $copies; $i++) { + $copyRepo->create($bookId, 'ZZA301-' . $run . '-' . $bookSeq . '-' . $i, 'disponibile'); + } + return $bookId; +}; + +$userSeq = 0; +$makeUser = static function () use ($db, $run, $emailDomain, &$userSeq): int { + $userSeq++; + $card = 'ZZA301' . strtoupper($run) . $userSeq; + $email = $run . '-' . $userSeq . $emailDomain; + $password = password_hash('test', PASSWORD_BCRYPT); + $stmt = $db->prepare("INSERT INTO utenti (codice_tessera, nome, cognome, email, password, tipo_utente) VALUES (?, 'Auto', 'Test', ?, ?, 'standard')"); + $stmt->bind_param('sss', $card, $email, $password); + $stmt->execute(); + $id = (int) $db->insert_id; + $stmt->close(); + return $id; +}; + +$today = DateHelper::today(); +$due = (new DateTimeImmutable($today))->modify('+14 days')->format('Y-m-d'); + +// A fresh pending request: attivo=0, stato='pendente', no copy assigned yet, +// data_prestito = today so approval yields the immediate 'da_ritirare' state. +$makePendingLoan = static function (int $bookId, int $userId, ?string $state = 'pendente') use ($db, $today, $due): int { + $attivo = $state === 'pendente' ? 0 : 1; + $stmt = $db->prepare("INSERT INTO prestiti (libro_id, utente_id, data_prestito, data_scadenza, stato, origine, attivo) VALUES (?, ?, ?, ?, ?, 'diretto', ?)"); + $stmt->bind_param('iisssi', $bookId, $userId, $today, $due, $state, $attivo); + $stmt->execute(); + $id = (int) $db->insert_id; + $stmt->close(); + return $id; +}; + +$setAuto = static function (?string $value) use ($db): void { + if ($value === null) { + $db->query("DELETE FROM system_settings WHERE category='loans' AND setting_key='auto_approve_requests'"); + return; + } + (new SettingsRepository($db))->set('loans', 'auto_approve_requests', $value); +}; + +$accessor = static fn (): bool => (new SettingsRepository($db))->autoApproveLoanRequests(); + +// autoApproveLoanRequest() is private — invoke the real method by reflection so +// the test drives the exact code path the loan() request handler triggers. +$controller = new UserActionsController(); +$autoMethod = new ReflectionMethod($controller, 'autoApproveLoanRequest'); +$autoMethod->setAccessible(true); +$autoApprove = static function (int $loanId) use ($controller, $autoMethod, $db): bool { + $request = (new ServerRequestFactory())->createServerRequest('POST', '/prestito'); + return (bool) $autoMethod->invoke($controller, $request, $db, $loanId); +}; + +$loanField = static function (int $loanId, string $col) use ($db) { + $stmt = $db->prepare("SELECT {$col} AS v FROM prestiti WHERE id = ?"); + $stmt->bind_param('i', $loanId); + $stmt->execute(); + $v = $stmt->get_result()->fetch_assoc()['v'] ?? null; + $stmt->close(); + return $v; +}; + +// ── A. Setting accessor: the default must be safe for existing installs ────── +echo "A. SettingsRepository::autoApproveLoanRequests() accessor\n"; +$setAuto(null); +$check($accessor() === false, '01 absent setting → off (existing installs keep the manual queue)'); +$setAuto('0'); +$check($accessor() === false, "02 setting '0' → off"); +$setAuto('1'); +$check($accessor() === true, "03 setting '1' → on"); +$setAuto('yes'); +$check($accessor() === false, "04 any non-'1' value → off (strict opt-in)"); + +// ── B. Auto-approval behaviour through the canonical pipeline ──────────────── +echo "B. autoApproveLoanRequest() behaviour\n"; + +// 05 — OFF: the request must remain in the manual pendente queue. +$setAuto('0'); +$loanOff = $makePendingLoan($makeBook(1), $makeUser()); +$check($autoApprove($loanOff) === false, '05 setting off → returns false'); +$check($loanField($loanOff, 'stato') === 'pendente', '05b setting off → loan stays pendente (manual workflow preserved)'); + +// 06-08 — ON with an available copy: promoted to da_ritirare via the real pipeline. +$setAuto('1'); +$loanOn = $makePendingLoan($makeBook(1), $makeUser()); +$check($autoApprove($loanOn) === true, '06 setting on + available copy → returns true'); +$check($loanField($loanOn, 'stato') === 'da_ritirare', "07 auto-approved loan → stato 'da_ritirare' (waiting for pickup)"); +$check($loanField($loanOn, 'pickup_deadline') !== null, '08 auto-approved immediate loan → pickup_deadline is set'); + +// 09 — reuses the pipeline: a copy is locked/assigned and the loan is activated. +$check((int) $loanField($loanOn, 'attivo') === 1 && $loanField($loanOn, 'copia_id') !== null, + '09 auto-approval assigns a physical copy and activates the loan (canonical pipeline)'); + +// 10 — ON but no available copy: must not force-approve; request stays pending. +$setAuto('1'); +$loanNoCopy = $makePendingLoan($makeBook(0), $makeUser()); +$check($autoApprove($loanNoCopy) === false, '10 setting on + no available copy → returns false (graceful)'); +$check($loanField($loanNoCopy, 'stato') === 'pendente', '10b no-copy request is left pending, never wrongly auto-approved'); + +$cleanup(); +$db->close(); +echo "\n{$pass} checks passed\n"; +exit(0); diff --git a/version.json b/version.json index 4c7237815..a00a79077 100644 --- a/version.json +++ b/version.json @@ -1,5 +1,5 @@ { "name": "Pinakes", - "version": "0.7.45", + "version": "0.7.46", "description": "Library Management System - Sistema di Gestione Bibliotecaria" } From d676f42ee2710ac68ea6c879773385f72daa4e05 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 29 Jul 2026 18:56:12 +0200 Subject: [PATCH 3/3] test(#301): behavioural test for migrate_0.7.46.sql MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Code Quality meta-guard (issue-237-regression.unit.php) requires the release-version migration to have a matching tests/migration-.unit.php. migrate_0.7.46.sql shipped without one, failing CI. Adds tests/migration-0.7.46.unit.php: runs the real migration against a sandbox system_settings table and asserts the loans/auto_approve_requests setting is created with the safe default '0', a re-run preserves an admin's '1' (does not reset it — ON DUPLICATE KEY UPDATE keeps the stored value), and no duplicate row is created. --- tests/migration-0.7.46.unit.php | 125 ++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 tests/migration-0.7.46.unit.php diff --git a/tests/migration-0.7.46.unit.php b/tests/migration-0.7.46.unit.php new file mode 100644 index 000000000..32e37beee --- /dev/null +++ b/tests/migration-0.7.46.unit.php @@ -0,0 +1,125 @@ +set_charset('utf8mb4'); +} catch (\Throwable $e) { + // A migration test that silently skips its DB section is a false green in CI. + fwrite(STDERR, "FAIL: database unreachable — the migration section is mandatory: {$e->getMessage()}\n"); + exit(1); +} + +$SB = 'zz_mig_settings_0746'; +$migration = (string) file_get_contents($root . '/installer/database/migrations/migrate_0.7.46.sql'); +// Retarget the REAL migration at a sandbox table so the test never touches the +// live system_settings. +$sandbox = static fn (string $sql): string => str_replace('`system_settings`', "`{$SB}`", $sql); + +$runMigration = static function () use ($db, $migration, $sandbox): void { + // array_filter drops the empty tail after the final ';', so every remaining + // statement is non-empty. + foreach (array_filter(array_map('trim', preg_split('/;\s*\n/', $sandbox(preg_replace('/^--.*$/m', '', $migration) ?? $migration)))) as $statement) { + $db->query($statement); + } +}; + +$readSetting = static function () use ($db, $SB): ?string { + $row = $db->query("SELECT setting_value FROM {$SB} WHERE category='loans' AND setting_key='auto_approve_requests'")->fetch_assoc(); + return $row === null ? null : (string) $row['setting_value']; +}; + +$cleanup = static function () use ($db, $SB): void { + $db->query("DROP TABLE IF EXISTS {$SB}"); +}; + +try { + $cleanup(); + + $db->query("CREATE TABLE {$SB} ( + id int NOT NULL AUTO_INCREMENT, + category varchar(50) NOT NULL, + setting_key varchar(100) NOT NULL, + setting_value text, + description text, + created_at timestamp NULL DEFAULT CURRENT_TIMESTAMP, + updated_at timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (id), + UNIQUE KEY unique_setting (category, setting_key) + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci"); + + // Pre-migration: setting absent. + $check($readSetting() === null, 'auto_approve_requests absent before migration'); + + $runMigration(); + + // Fresh install / upgrade: the setting is created with the safe default '0'. + $check($readSetting() === '0', "migration creates loans/auto_approve_requests = '0' (manual approval stays the default)"); + $count = (int) $db->query("SELECT COUNT(*) FROM {$SB} WHERE category='loans' AND setting_key='auto_approve_requests'")->fetch_row()[0]; + $check($count === 1, 'exactly one settings row created'); + + // An administrator turns auto-approval ON. + $db->query("UPDATE {$SB} SET setting_value='1' WHERE category='loans' AND setting_key='auto_approve_requests'"); + + // Re-running the migration must NOT reset that choice (ON DUPLICATE KEY UPDATE + // keeps the stored value) and must not duplicate the row. + $runMigration(); + $check($readSetting() === '1', "second run preserves the admin's '1' (does not reset to '0')"); + $count = (int) $db->query("SELECT COUNT(*) FROM {$SB} WHERE category='loans' AND setting_key='auto_approve_requests'")->fetch_row()[0]; + $check($count === 1, 'second run does not duplicate the row (idempotent upsert)'); + + $cleanup(); +} catch (\Throwable $e) { + $fail++; + echo " FAIL exception: {$e->getMessage()}\n"; + $cleanup(); +} + +echo "\n{$pass} PASS, {$fail} FAIL\n"; +exit($fail === 0 ? 0 : 1);