fix: improve loan approval logic and add security hardening

[fabiodalez-dev]

Summary

• LoanApprovalController: Fix false rejection when approving loans with pre-assigned copies at full capacity
• ReservationsAdminController: Add date format validation in store method
• Updater: Improve path protection for files in subdirectories

Changes

LoanApprovalController

The global slot counting (checking total copies vs overlapping loans/reservations) was happening before validating if the loan already had a valid pre-assigned copy. This caused false rejections when:

• All slots were occupied
• But the loan being approved already had a specific copy assigned

Fix: Reordered the logic to validate pre-assigned copy first, skip global counting if valid.

ReservationsAdminController

Added missing date format validation (/^\d{4}-\d{2}-\d{2}$/) for dataPrenotazione and dataScadenza in the store() method (already present in update()).

Updater.php

Enhanced path protection to also block deletion of files with protected basename in any subdirectory (e.g., subdir/.env).

Test plan

• Approve a loan that already has an assigned copy when all book copies are occupied by other loans
• Create a reservation with invalid date formats - should reset to defaults
• Verify Updater cleanup cannot delete .env files in subdirectories

Summary by CodeRabbit

Note di Rilascio

• Bug Fixes

  • Corretta la validazione del formato data nelle prenotazioni per evitare valori malformati.
  • Evitata la riattivazione di copie in stati non ripristinabili durante la gestione dei ritiri scaduti.

• Refactor

  • Ottimizzato il flusso di approvazione dei prestiti con controlli di disponibilità più accurati e protezione contro condizioni di gara.
  • Rafforzata la protezione dei file critici durante le operazioni di aggiornamento.

• Comportamento

  • Le prenotazioni non marcano più una copia come "prestato" fino alla conferma del ritiro.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Rifattorizzato il flusso di approvazione prestiti per preferire copie già assegnate, introdotte verifiche multi-step di disponibilità e lock post-selezione; aggiunta validazione formato date su prenotazioni; estesa protezione basename in cleanup post-install; modificata logica di ricalcolo disponibilità e guardia su ripristino copie scadute.

Changes

Cohort / File(s)	Riepilogo modifiche
Logica approvazione prestiti app/Controllers/LoanApprovalController.php	Riprogettazione di approveLoan: pre-check per copia già assegnata, conteggi data-aware (copie/prestiti/prenotazioni), fallback con CopyRepository, lock + re-check per race-condition, aggiustamenti su transizioni di stato e pickupDeadline.
Validazione date prenotazioni app/Controllers/ReservationsAdminController.php	Store: validazione esplicita del formato di dataPrenotazione e dataScadenza; valori non validi resettati a stringa vuota prima della logica di default.
Protezione file durante update app/Support/Updater.php	cleanupFile: aggiunto controllo sul basename del percorso per bloccare la cancellazione di nomi protetti (es. .env) anche se presenti in sottocartelle, oltre ai controlli path/prefix esistenti.
Ricalcolo disponibilità copie app/Support/DataIntegrity.php	Rework delle query di ricalcolo: solo prestiti con stato in_corso o in_ritardo rendono la copia prestato; rimosso il comportamento che considerava prenotato + data_prestito <= CURDATE() come prestito attivo.
Manutenzione e pickup scaduti app/Support/MaintenanceService.php	checkExpiredPickups: introdotta lista nonRestorableStates (perso, danneggiato, manutenzione) e condizione che evita il ripristino in disponibile per quegli stati.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Admin as Admin/Client
participant Controller as LoanApprovalController
participant Repo as CopyRepository
participant DB as Database
participant Lock as LockService
participant Notif as NotificationService

rect rgba(200,200,255,0.5)
Admin->>Controller: richiesta approveLoan(loanId)
end

rect rgba(200,255,200,0.5)
Controller->>DB: verifica copia già assegnata (loan.copia_id)
alt copia assegnata valida
    Controller->>Lock: lock(copia_id)
    Lock->>DB: verifica overlapp e stato
    DB-->>Controller: risultato verifica
else nessuna copia assegnata
    Controller->>DB: conteggi disponibilità e overlapping (multi-step)
    DB-->>Controller: conteggi e candidate
    Controller->>Repo: findAvailableCopy(period)
    Repo-->>Controller: copia selezionata o null
    Controller->>Lock: lock(copia_id)
    Lock->>DB: re-check overlaps
    DB-->>Controller: risultato
end

rect rgba(255,200,200,0.5)
Controller->>DB: assign copia, update stato prestito/limiti
Controller->>Notif: invia notifiche rilevanti
Controller->>Admin: risposta JSON (success/error)
end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: improve reservations system consistency and data integrity #37 — Modifica parallela su LoanApprovalController per riutilizzo di copia_id già assegnata e controlli TOCTOU.
• fix: improve loan approval logic and add security hardening #38 — Cambiamenti simili sul flusso di selezione/locking delle copie e check multi-step di disponibilità.
• feat: Reservation pickup confirmation + ICS calendar export #14 — PR correlata che estende gestione origine e recupero prestiti pendenti, toccando gli stessi metodi di approvazione.

Poem

🐰 Saltello lieve, copio e controllo,
Se la copia è già lì, la tengo ben stretto,
Date pulite, file al sicuro nel buco,
Lock e re-check, niente più capriccio,
Con un battito di zampa approvo quel prestito! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Il titolo è direttamente correlato ai cambiamenti principali del PR: migliora la logica di approvazione dei prestiti e aggiunge protezione di sicurezza nell'Updater.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/loan-approval-and-updater-improvements

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}