feat: add pickup confirmation workflow for approved loans (v0.4.5)#36
Merged
fabiodalez-dev merged 14 commits intomainfrom Jan 21, 2026
Merged
feat: add pickup confirmation workflow for approved loans (v0.4.5)#36fabiodalez-dev merged 14 commits intomainfrom
fabiodalez-dev merged 14 commits intomainfrom
Conversation
…troller - SearchController: fix locations() query to use posizioni/scaffali/mensole tables instead of non-existent collocazione table - PasswordController: replace custom rate limiting with existing RateLimiter class, removing dependency on rate_limit_log table
Added TinyMCE toolbar with bold, italic and lists to the book edit page. Backend and frontend now reflect and render those HTML values properly. Closes #30
- Install @tailwindcss/typography plugin for prose styling - Add CSS for lists, bold, italic, and links in book descriptions - Ensure TinyMCE formatting displays correctly in both backend and frontend views
Fixes #35 The previous nginx configuration had several issues: - Nested if statements (not supported by nginx) - Document root pointed to project root causing redirect loops - Complex nested location blocks causing 302/404 errors Changes: - Document root now points to /public/ directly (correct approach) - Removed all nested if statements - Simplified location blocks for reliable routing - Added separate server block for installer on port 8889 - Added clear comments for PHP-FPM socket configuration - Improved CORS headers for media streaming
- Added /installer/ location on main port using alias directive - Uses $request_filename for proper path resolution with alias - Matches local development config (pinakes.conf) - Allows installer access without separate port 8889
Introduce a new 'da_ritirare' (ready for pickup) state between loan approval and active loan. When a loan is approved for today, the book now shows as ready for pickup instead of immediately "out", because it's still physically in the library until the user picks it up. Key changes: - New loan state 'da_ritirare' with pickup_deadline column - New setting 'pickup_expiry_days' (default 3 days) for pickup deadline - Admin can confirm pickup via new button in loans list - Copy remains 'disponibile' during da_ritirare (physically in library) - Slot counts as occupied for availability calculations - Email notifications for pickup ready and pickup expired - Works without MaintenanceService (on-demand transitions) Flow: pendente → approvazione → da_ritirare → conferma ritiro → in_corso
|
Caution Review failedThe pull request is closed. WalkthroughIl PR introduce lo stato di prestito Changes
Sequence Diagram(s)sequenceDiagram
participant User as Utente/Admin
participant UI as Interfaccia Prestiti
participant LoanCtrl as LoanApprovalController
participant DB as Database
participant Notif as NotificationService
participant Mailer as Servizio Email
User->>UI: Richiesta approvazione prestito
UI->>LoanCtrl: approveLoan(loanId)
LoanCtrl->>DB: Leggi setting pickup_expiry_days
LoanCtrl->>DB: UPDATE prestito SET stato='da_ritirare', pickup_deadline=...
DB-->>LoanCtrl: OK
LoanCtrl->>Notif: sendPickupReadyNotification(loanId)
Notif->>Mailer: Invia template loan_pickup_ready
Mailer->>User: Email ritiro pronto
LoanCtrl->>UI: Risposta OK (stato Da Ritirare)
sequenceDiagram
participant Admin as Admin
participant UI as Interfaccia Prestiti
participant LoanCtrl as LoanApprovalController
participant DB as Database
Admin->>UI: Clicca "Conferma Ritiro"
UI->>LoanCtrl: confirmPickup(loanId)
LoanCtrl->>DB: LOCK & valida stato (da_ritirare o prenotato con data_prestito<=oggi)
alt Validazione OK
LoanCtrl->>DB: UPDATE prestito SET stato='in_corso', pickup_deadline=NULL
LoanCtrl->>DB: UPDATE copia SET stato='prestato'
LoanCtrl->>DB: Ricalcola disponibilità
DB-->>LoanCtrl: Commit
LoanCtrl->>UI: Successo
else Fallita
DB-->>LoanCtrl: Rollback
LoanCtrl->>UI: Errore
end
sequenceDiagram
participant Cron as Scheduler
participant Maint as MaintenanceService
participant DB as Database
participant ReservationMgr as ReservationManager
participant Notif as NotificationService
Cron->>Maint: runAll()
Maint->>DB: Seleziona prenotati con data_prestito<=oggi
loop per ogni prestito
DB-->>Maint: prestito
Maint->>DB: UPDATE stato='da_ritirare', set pickup_deadline
Maint->>DB: Ricalcola disponibilità
Maint->>Notif: sendPickupReadyNotification
end
Maint->>DB: Seleziona da_ritirare con pickup_deadline scaduta
loop per ogni scaduto
DB-->>Maint: prestito
Maint->>ReservationMgr: tenta riassegnazione
Maint->>DB: UPDATE stato='scaduto', clear pickup_deadline
Maint->>DB: Ricalcola disponibilità
Maint->>Notif: sendPickupExpiredNotification
end
Maint-->>Cron: risultati (conteggi)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Support/IcsGenerator.php (1)
102-131: Evento ICS per “da_ritirare” dovrebbe usarepickup_deadline.La query recupera
pickup_deadlinema l’evento usa sempredata_scadenza, quindi per “da_ritirare” si mostra l’intero periodo di prestito invece della finestra di ritiro. Suggerisco di usarepickup_deadlinecome end quando presente.🔧 Proposta di fix
- 'end' => $row['data_scadenza'], + 'end' => ($row['stato'] === 'da_ritirare' && !empty($row['pickup_deadline'])) + ? $row['pickup_deadline'] + : $row['data_scadenza'],
🤖 Fix all issues with AI agents
In `@app/Controllers/LoanApprovalController.php`:
- Around line 367-460: The confirmPickup flow currently allows confirming
'da_ritirare' (or 'prenotato' when data_prestito <= today) without checking
pickup_deadline; update confirmPickup to SELECT pickup_deadline along with the
loan row (use the existing SELECT in prestiti), then after $loan is fetched
check if $loan['pickup_deadline'] is non-empty and if (new DateTime($today) >
new DateTime($loan['pickup_deadline'])) rollback and return a 400 with a clear
message (do not proceed to UPDATE or change copy status); keep the existing
transaction/lock behavior and use the same variables ($loan, $today, $loanId)
and methods (begin_transaction, rollback, commit) so only the extra deadline
check is added.
In `@app/Controllers/SearchController.php`:
- Around line 458-471: The COALESCE in the SQL prepared into $stmt uses
COALESCE(s.codice, s.nome, '') which treats empty strings as valid values and
yields labels like " - Liv. 1"; change it to COALESCE(NULLIF(s.codice, ''),
s.nome, '') so empty '' becomes NULL and s.nome is used instead; update the SQL
string inside the $stmt = $db->prepare(...) block accordingly and keep the rest
of the query (labels, ORDER BY, LIMIT) unchanged.
In `@app/Support/MaintenanceService.php`:
- Around line 248-255: Protect the UPDATE from races with confirmPickup by
adding an expected-state guard in the WHERE clause (e.g., "WHERE id = ? AND
stato = 'in_transito'") so the query only transitions loans that are still in
the expected state, and after execute() check $updateStmt->affected_rows to
determine if the row was actually updated (handle the no-op case instead of
assuming success); update the bind_param call to include the id and/or state
parameter as needed, and keep using $updateStmt->close() after checking
affected_rows.
- Around line 587-592: Dopo il commit ($this->db->commit()) e la chiamata a
$reassignmentService->flushDeferredNotifications(), invoca il servizio di
notifica esistente per inviare la mail “ritiro scaduto”: ad esempio chiamare il
metodo dedicato del servizio di notifiche (es.
RitiroNotificationService::sendExpiredNotification o
$notificationService->sendRitiroScadutoNotification(...)) passando l'entità/ID
del ritiro aggiornato; assicurati di usare il template esistente e di gestire
eventuali errori/log prima di incrementare $expiredCount.
- Around line 550-557: Aggiorna la query in MaintenanceService.php (la variabile
$updateStmt che prepara l'UPDATE su prestiti) per applicare la modifica dello
stato a 'scaduto' solo se il record è ancora nello stato 'da_ritirare' (aggiungi
una condizione WHERE che includa stato = 'da_ritirare'); inoltre dopo
l'esecuzione controlla rowCount() su $updateStmt per rilevare se l'UPDATE ha
effettivamente colpito una riga e gestire il caso in cui nessuna riga sia stata
aggiornata (conferma/log o salto) per evitare TOCTOU con conferme ritiro
concorrenti.
- Around line 236-238: The code instantiates SettingsRepository in
MaintenanceService without importing its namespace; add a proper import or use
the fully-qualified class name to avoid PHP resolving it to
App\Support\SettingsRepository. Open the MaintenanceService class and either add
"use App\Models\SettingsRepository;" at the top or replace new
SettingsRepository(...) with new \App\Models\SettingsRepository(...) where the
constructor is called to ensure the correct class is referenced.
In `@app/Views/frontend/book-detail.php`:
- Line 1563: The book description currently calls
HtmlHelper::sanitizeHtml($book['descrizione']) which strips newlines and loses
textarea formatting; wrap the raw text with nl2br before sanitization so
newlines become <br> tags and then are safely whitelisted by
HtmlHelper::sanitizeHtml (i.e., call sanitizeHtml(nl2br($book['descrizione']))
where the description is rendered).
In `@app/Views/libri/partials/book_form.php`:
- Around line 3638-3665: The initBookTinyMCE() function currently retries
indefinitely and can double-initialize TinyMCE; add a retry counter and a
MAX_RETRIES constant and increment it each retry invoked via
setTimeout(initBookTinyMCE, 100), and before calling tinymce.init(...) guard
against re-initialization by checking tinymce.get('descrizione') or
tinymce.editors for an existing editor instance; if max retries reached, stop
scheduling further retries and optionally log a warning. Ensure the DOM-ready
wiring remains but uses the same guarded initBookTinyMCE to prevent multiple
starts.
- Around line 2850-2858: Sanitizza data.description prima di inserirla nella
textarea e in TinyMCE: usa DOMPurify.sanitize(data.description) (verifica che
window.DOMPurify esista e fai fallback a una stringa vuota o a data.description
non modificata se non presente) e poi assegnalo a descInput.value; quando chiami
tinymce.get('descrizione').setContent(...) passa la versione sanitizzata invece
di data.description per prevenire XSS.
In `@app/Views/libri/scheda_libro.php`:
- Around line 538-540: Wrap the sanitized description output with nl2br to
preserve newline breaks for plain-text descriptions: call nl2br on the result of
App\Support\HtmlHelper::sanitizeHtml($libro['descrizione']) (keeping existing
escaping/sanitization) so the view for the description reproduces line breaks
like other fields (biografie, note, recensioni).
In `@app/Views/prestiti/index.php`:
- Around line 363-367: Replace the UTC-based today computation that uses
toISOString() with a local-date-only string and compare against
row.data_prestito using that local string; specifically, change how today is
built (the variable named today) so it uses the local Date (e.g., via
getFullYear(), getMonth()+1, getDate() or an equivalent local-date formatter)
and then keep the existing isReadyForPickup logic (variables isReadyForPickup,
row.stato, row.data_prestito) so the "Conferma Ritiro" button decision uses
local dates rather than UTC.
🧹 Nitpick comments (3)
app/Views/libri/scheda_libro.php (1)
1149-1186: Centralizzare gli stili.prosenel CSS globale.Con il plugin typography già introdotto, questi stili inline rischiano di duplicarsi rispetto a
public/assets/main.css. Valuta di spostarli lì per evitare drift e semplificare la vista..nginx.conf.example (1)
162-170: Limita l’esposizione dell’installer su rete per default.
Il blocco dell’installer su8889ascolta su tutte le interfacce; meglio vincolarlo a loopback come default di esempio, lasciando la possibilità di estenderlo se necessario.🔒 Diff suggerita
- listen 8889; - listen [::]:8889; + listen 127.0.0.1:8889; + listen [::1]:8889;installer/database/schema.sql (1)
598-620: Aggiungi l’indice supickup_deadlineanche nello schema iniziale.
La migrazione lo prevede: includerlo inschema.sqlmantiene parità e migliora le query di scadenza nei nuovi installi.♻️ Diff suggerita
KEY `processed_by` (`processed_by`), KEY `idx_prestiti_data_scadenza` (`data_scadenza`), + KEY `idx_prestiti_pickup_deadline` (`pickup_deadline`), KEY `idx_prestiti_status` (`stato`,`data_scadenza`),
Comment on lines
538
to
540
| <div class="prose prose-sm max-w-none text-gray-700"> | ||
| <?php echo nl2br(App\Support\HtmlHelper::e($libro['descrizione'])); ?> | ||
| <?php echo App\Support\HtmlHelper::sanitizeHtml($libro['descrizione']); ?> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the specific file and lines
head -n 550 app/Views/libri/scheda_libro.php | tail -n 20Repository: fabiodalez-dev/Pinakes
Length of output: 842
🏁 Script executed:
# Search for HtmlHelper class and sanitizeHtml method
find . -name "*.php" -type f | xargs grep -l "class HtmlHelper"Repository: fabiodalez-dev/Pinakes
Length of output: 96
🏁 Script executed:
# Search for sanitizeHtml method definition
rg -A 10 "function sanitizeHtml|public static function sanitizeHtml"Repository: fabiodalez-dev/Pinakes
Length of output: 719
🏁 Script executed:
# Look for how descrizione field is handled elsewhere in the codebase
rg "descrizione" app/Models/ -A 2 -B 2Repository: fabiodalez-dev/Pinakes
Length of output: 1672
🏁 Script executed:
cat -n app/Support/HtmlHelper.phpRepository: fabiodalez-dev/Pinakes
Length of output: 11677
🏁 Script executed:
# Also search for any usage of nl2br in the codebase
rg "nl2br" --type phpRepository: fabiodalez-dev/Pinakes
Length of output: 1509
🏁 Script executed:
# Check if there are migrations or schema files that show descrizione field type
find . -path "*/migrations/*" -name "*.php" | xargs grep -l "descrizione" 2>/dev/null | head -5Repository: fabiodalez-dev/Pinakes
Length of output: 48
Aggiungi nl2br() per preservare i ritorni a capo nelle descrizioni in plain text.
La funzione sanitizeHtml() utilizza strip_tags() internamente, che rimuove i newline dal testo semplice. Poiché le descrizioni sono archiviate come testo semplice e il resto della codebase applica nl2br() a campi similari (biografie, note, recensioni), è necessario mantenere questa coerenza:
Suggerimento di fix
- <?php echo App\Support\HtmlHelper::sanitizeHtml($libro['descrizione']); ?>
+ <?php echo App\Support\HtmlHelper::sanitizeHtml(nl2br($libro['descrizione'], false)); ?>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div class="prose prose-sm max-w-none text-gray-700"> | |
| <?php echo nl2br(App\Support\HtmlHelper::e($libro['descrizione'])); ?> | |
| <?php echo App\Support\HtmlHelper::sanitizeHtml($libro['descrizione']); ?> | |
| </div> | |
| <div class="prose prose-sm max-w-none text-gray-700"> | |
| <?php echo App\Support\HtmlHelper::sanitizeHtml(nl2br($libro['descrizione'], false)); ?> | |
| </div> |
🤖 Prompt for AI Agents
In `@app/Views/libri/scheda_libro.php` around lines 538 - 540, Wrap the sanitized
description output with nl2br to preserve newline breaks for plain-text
descriptions: call nl2br on the result of
App\Support\HtmlHelper::sanitizeHtml($libro['descrizione']) (keeping existing
escaping/sanitization) so the view for the description reproduces line breaks
like other fields (biografie, note, recensioni).
Code review fixes: - SearchController: use NULLIF with COALESCE for empty string handling - book-detail.php/scheda_libro.php: add nl2br() before sanitizeHtml() - book_form.php: add DOMPurify sanitization for imported descriptions - book_form.php: add TinyMCE max retry limit and double-init guard - vendor.js: add DOMPurify via npm (no CDN) - modern-enhancements.css: centralize .prose styles - schema.sql: add pickup_deadline index - nginx.conf.example: restrict installer to loopback interface
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.nginx.conf.example:
- Around line 83-113: Remove the public-facing "location ^~ /installer/" block
from the main server block and rely on the existing loopback server configured
for 127.0.0.1:8889 and [::1]:8889 to serve the installer; if you must keep an
installer location in the public server, restrict access by replacing it with an
allow/deny ACL that only permits 127.0.0.1 and ::1 (or return 403 for all
others), ensure fastcgi_param SCRIPT_FILENAME still uses $request_filename, and
verify try_files routing is only present in the loopback server to avoid
accidental public exposure of /installer/.
In `@app/Support/IcsGenerator.php`:
- Around line 102-110: The WHERE clause in the $loanSql query filters only on
p.data_scadenza which causes loans with stato = 'da_ritirare' to be included
past their pickup_deadline; update the filter to use an "effective" end date:
for example replace the single (p.data_scadenza >= ? OR p.stato = 'in_ritardo')
condition with a conditional that uses p.pickup_deadline when p.stato =
'da_ritirare' and p.data_scadenza otherwise (e.g. a CASE/COALESCE expression or
explicit OR clauses: (p.stato = 'da_ritirare' AND p.pickup_deadline >= ?) OR
(p.stato != 'da_ritirare' AND p.data_scadenza >= ?) OR p.stato = 'in_ritardo'),
and apply the same change to the other occurrences noted around the $loanSql
usages at the other locations.
- Add cancelPickupReason translation key for proper database storage (instead of using dialog question text) - Add da_ritirare to $statusLabels in PrestitiController for CSV export - Add logging for anomalous copy state during pickup confirmation - Add X-Frame-Options header to nginx media streaming block - Add English translation for new cancelPickupReason string
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@app/Controllers/FrontendController.php`:
- Around line 1582-1584: The SQL that computes occupied copies omits the
'in_ritardo' status, causing availability to be overestimated; update the WHERE
clause that currently reads "stato IN ('in_corso', 'da_ritirare', 'prenotato')"
(the query against table prestiti grouped by libro_id) to include 'in_ritardo'
so delayed loans are counted as occupied copies for libro_id.
In `@app/Models/DashboardStats.php`:
- Around line 14-27: The SQL string in DashboardStats (variable $sql) directly
interpolates $today; change it to avoid direct interpolation by replacing the
'$today' injection with a bound parameter (e.g., :today) or use the DB-side
CURDATE() function, then prepare the statement and bind the PHP date('Y-m-d') to
that parameter before executing; update the code that builds/executes $sql (the
prepared statement call that uses $sql) to bind ':today' (or remove binding if
using CURDATE()) so the query remains functionally identical but no longer
concatenates PHP variables into the SQL.
In `@app/Views/partials/loan-actions-swal.php`:
- Around line 227-325: The cancel-pickup handler currently calls the backend
endpoint '/admin/loans/reject' which only accepts loans with stato='pendente',
so canceling a da_ritirare/scaduto loan will fail; update the backend to add a
dedicated endpoint (e.g. POST '/admin/loans/cancel-pickup') or extend the
existing rejectLoan logic to accept stato IN ('da_ritirare','scaduto') and
perform the proper state transition plus availability/copy updates, then change
the frontend cancel-pickup button handler (the code that calls
sendRequest('/admin/loans/reject', { loan_id, reason }, csrf)) to call the new
endpoint or pass the necessary flag so the backend treats it as a pickup
cancellation; ensure controller/service method (rejectLoan or new
cancelPickupLoan) updates loan status, adjusts item availability/copy counts,
and returns the same success/error payload shape used by showSuccess/showError.
In `@update-0.4.4.md`:
- Around line 1-8: The release note file update-0.4.4.md conflicts with PR `#36`
which targets v0.4.5 and adds pickup workflow fields (e.g., da_ritirare,
pickup_deadline) from branch "issues"; either rename the file to update-0.4.5.md
and update its header and body to describe the v0.4.5 pickup confirmation
workflow and list the new fields (da_ritirare, pickup_deadline) and related
changes, or remove the file from the PR if it was added by mistake—ensure the
MR/commit message and file name/version in the file body match the PR target
version (v0.4.5) before finalizing.
- Around line 9-98: The changelog/update-0.4.4.md is missing the v0.4.5 release
notes that document the new "conferma ritiro" workflow introduced in PR `#36`; add
a new v0.4.5 section (and a corresponding bullet under "Novità Principali") that
lists the core items: new loan state da_ritirare, admin action "Conferma Ritiro"
button, pickup_deadline and pickup_expiry_days fields/behaviour, new email
templates for "pronto per il ritiro" and "ritiro scaduto", ICS calendar
integration for da_ritirare, and related UI, CSV export and filter updates so
readers can see the complete workflow.
In `@updater.md`:
- Around line 200-207: Aggiorna l'elenco delle migrazioni in updater.md
aggiungendo la voce migrate_0.4.5.sql; questa nuova migration introduce la
colonna pickup_deadline e il nuovo stato 'da_ritirare', quindi assicurati che il
nome migrate_0.4.5.sql compaia nell'esempio insieme alle altre migrate_0.4.x.sql
per riflettere correttamente la versione 0.4.5.
- Around line 450-456: Aggiorna la sezione "Version History" aggiungendo una
riga per la versione 0.4.5 nella tabella: inserisci "| 0.4.5 | Added:
Confirmation workflow for pickups; DB migrations for new pickup flow; Pickup
expiration/timeout handling and related fixes |" sotto la riga esistente per
0.4.4 in modo che la cronologia rifletta le modifiche introdotte dal PR.
🧹 Nitpick comments (6)
update-0.4.4.md (1)
125-125: Formatta correttamente l'URL.L'URL nudo viola le best practice Markdown. Racchiudilo tra parentesi angolari o usa la sintassi link appropriata.
📝 Correzione suggerita
-- GitHub Issues: https://github.com/fabiodalez-dev/pinakes/issues +- GitHub Issues: <https://github.com/fabiodalez-dev/pinakes/issues>Oppure, con testo del link:
-- GitHub Issues: https://github.com/fabiodalez-dev/pinakes/issues +- [GitHub Issues](https://github.com/fabiodalez-dev/pinakes/issues)updater.md (1)
46-47: Correggi i problemi di formattazione markdown (opzionale).Il linter markdown segnala alcuni problemi di formattazione:
- Blocchi di codice senza specificatore di linguaggio (linee 46, 121, 202)
- Tabella non circondata da righe vuote (linea 429)
Aggiungere gli specificatori di linguaggio migliora il syntax highlighting, e rispettare la spaziatura delle tabelle migliora la compatibilità con i parser markdown.
♻️ Correzioni proposte
Linea 46-47: aggiungi specificatore
textoplaintext-``` +```text storage/Linea 121-122: aggiungi specificatore
text-``` +```text storage/tmp/Linea 202-203: aggiungi specificatore
text-``` +```text migrate_0.4.0.sqlLinea 429: aggiungi riga vuota prima della tabella
cd storage/plugins && zip -r ../../installer/plugins/dewey-editor.zip dewey-editor/ && cd ../.. ``` 3. **Current bundled plugins**: + | Plugin | Version | File |Also applies to: 121-122, 202-203, 429-437
frontend/css/modern-enhancements.css (1)
370-421: Considera la sovrapposizione con il plugin typography.Questi stili
.prosepersonalizzati potrebbero sovrapporsi alle classi integrate di@tailwindcss/typography(appena aggiunto intailwind.config.js). Se l'intento è usare un sottoinsieme minimo di stili, va bene mantenerli. Altrimenti, potresti usare le classiprosedel plugin direttamente (es.prose prose-sm dark:prose-invert) che offrono una tipografia più completa e testata.L'implementazione attuale funziona correttamente e i focus states sono accessibili.
app/Support/NotificationService.php (1)
990-1071: Valuta un helper condiviso per le notifiche di prestito.
sendPickupReadyNotificationreplica query/variabili disendLoanApprovedNotification; estrarre un helper riduce duplicazioni e drift.app/Views/admin/pending_loans.php (1)
23-121: Refactor opzionale: calcola$todayuna sola volta.
Evita differenze a cavallo di mezzanotte e riduce chiamate ripetute.♻️ Proposta di refactor
- <?php foreach ($pickupLoans as $loan): ?> - <?php - $isExpired = !empty($loan['pickup_deadline']) && $loan['pickup_deadline'] < date('Y-m-d'); - $isExpiringSoon = !empty($loan['pickup_deadline']) && $loan['pickup_deadline'] === date('Y-m-d'); - ?> + <?php $today = date('Y-m-d'); ?> + <?php foreach ($pickupLoans as $loan): ?> + <?php + $isExpired = !empty($loan['pickup_deadline']) && $loan['pickup_deadline'] < $today; + $isExpiringSoon = !empty($loan['pickup_deadline']) && $loan['pickup_deadline'] === $today; + ?>app/Views/dashboard/index.php (1)
736-742: Suggerimento: colore dedicato perda_ritirarenel calendario.
L’etichetta è presente, ma il colore ricade nel default: un arancione migliora la leggibilità.🎨 Proposta di mapping colore
- 'pendente' => '#F59E0B', // Amber + 'pendente' => '#F59E0B', // Amber + 'da_ritirare' => '#F97316', // Orange default => '#6B7280' // GrayAlso applies to: 840-844
Comment on lines
227
to
325
| // Confirm pickup button handler | ||
| document.querySelectorAll('.confirm-pickup-btn').forEach(btn => { | ||
| if (btn.dataset.bound === '1') { | ||
| return; | ||
| } | ||
| btn.dataset.bound = '1'; | ||
| btn.addEventListener('click', async function() { | ||
| const loanId = parseInt(this.dataset.loanId || '0', 10); | ||
| if (!loanId) return; | ||
|
|
||
| if (hasSwal()) { | ||
| const confirmed = await window.Swal.fire({ | ||
| icon: 'question', | ||
| title: t.confirmPickupTitle, | ||
| text: t.confirmPickupText, | ||
| showCancelButton: true, | ||
| confirmButtonText: t.pickupButton, | ||
| cancelButtonText: t.cancelButton, | ||
| confirmButtonColor: '#16a34a', | ||
| focusCancel: true | ||
| }); | ||
| if (!confirmed.isConfirmed) { | ||
| return; | ||
| } | ||
| } else if (!window.confirm(t.confirmPickupTitle)) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const { response, data } = await sendRequest('/admin/loans/confirm-pickup', { loan_id: loanId }, csrf); | ||
| if (response.ok && data.success) { | ||
| const card = this.closest('[data-pickup-card]'); | ||
| if (card) { | ||
| card.remove(); | ||
| } | ||
| await showSuccess(t.successPickupTitle, data.message || ''); | ||
| if (!document.querySelector('.confirm-pickup-btn') && !document.querySelector('.approve-btn')) { | ||
| window.location.reload(); | ||
| } | ||
| } else { | ||
| await showError(data.message); | ||
| } | ||
| } catch (_) { | ||
| await showError(t.serverError); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| // Cancel expired pickup button handler | ||
| document.querySelectorAll('.cancel-pickup-btn').forEach(btn => { | ||
| if (btn.dataset.bound === '1') { | ||
| return; | ||
| } | ||
| btn.dataset.bound = '1'; | ||
| btn.addEventListener('click', async function() { | ||
| const loanId = parseInt(this.dataset.loanId || '0', 10); | ||
| if (!loanId) return; | ||
|
|
||
| if (hasSwal()) { | ||
| const confirmed = await window.Swal.fire({ | ||
| icon: 'warning', | ||
| title: t.cancelPickupTitle, | ||
| text: t.cancelPickupText, | ||
| showCancelButton: true, | ||
| confirmButtonText: t.cancelPickupButton, | ||
| cancelButtonText: t.cancelButton, | ||
| confirmButtonColor: '#dc2626', | ||
| focusCancel: true | ||
| }); | ||
| if (!confirmed.isConfirmed) { | ||
| return; | ||
| } | ||
| } else if (!window.confirm(t.cancelPickupTitle)) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const { response, data } = await sendRequest('/admin/loans/reject', { | ||
| loan_id: loanId, | ||
| reason: t.cancelPickupReason | ||
| }, csrf); | ||
|
|
||
| if (response.ok && data.success) { | ||
| const card = this.closest('[data-pickup-card]'); | ||
| if (card) { | ||
| card.remove(); | ||
| } | ||
| await showSuccess(t.successCancelPickupTitle, data.message || ''); | ||
| if (!document.querySelector('.confirm-pickup-btn') && !document.querySelector('.approve-btn')) { | ||
| window.location.reload(); | ||
| } | ||
| } else { | ||
| await showError(data.message); | ||
| } | ||
| } catch (_) { | ||
| await showError(t.serverError); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Il cancel-pickup usa un endpoint che accetta solo prestiti pendenti.
/admin/loans/reject in backend filtra stato='pendente', quindi per da_ritirare/scaduto la richiesta fallirà. Serve un endpoint dedicato (es. /admin/loans/cancel-pickup) o un’estensione di rejectLoan per gestire questi stati e aggiornare disponibilità/copia.
🤖 Prompt for AI Agents
In `@app/Views/partials/loan-actions-swal.php` around lines 227 - 325, The
cancel-pickup handler currently calls the backend endpoint '/admin/loans/reject'
which only accepts loans with stato='pendente', so canceling a
da_ritirare/scaduto loan will fail; update the backend to add a dedicated
endpoint (e.g. POST '/admin/loans/cancel-pickup') or extend the existing
rejectLoan logic to accept stato IN ('da_ritirare','scaduto') and perform the
proper state transition plus availability/copy updates, then change the frontend
cancel-pickup button handler (the code that calls
sendRequest('/admin/loans/reject', { loan_id, reason }, csrf)) to call the new
endpoint or pass the necessary flag so the backend treats it as a pickup
cancellation; ensure controller/service method (rejectLoan or new
cancelPickupLoan) updates loan status, adjusts item availability/copy counts,
and returns the same success/error payload shape used by showSuccess/showError.
Comment on lines
+1
to
+8
| # Pinakes v0.4.4 - Note di Rilascio | ||
|
|
||
| **Data rilascio:** Gennaio 2026 | ||
|
|
||
| Questa versione introduce importanti miglioramenti al sistema di prenotazioni, ottimizzazioni delle performance, e rafforza la sicurezza complessiva dell'applicazione. Include inoltre un aggiornamento critico per garantire la compatibilità con tutti i tipi di hosting. | ||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
Discrepanza critica tra versione documentata e PR.
Questo file documenta la v0.4.4, ma il PR #36 introduce funzionalità della v0.4.5 (workflow di conferma ritiro con stato da_ritirare, pickup_deadline, ecc.). Gli obiettivi del PR indicano chiaramente "v0.4.5" e il ramo sorgente è issues.
Verifica se:
- Questo file dovrebbe essere rinominato
update-0.4.5.mde il contenuto aggiornato per documentare le nuove funzionalità di ritiro - Oppure il file è stato aggiunto per errore e non dovrebbe far parte di questo PR
🤖 Prompt for AI Agents
In `@update-0.4.4.md` around lines 1 - 8, The release note file update-0.4.4.md
conflicts with PR `#36` which targets v0.4.5 and adds pickup workflow fields
(e.g., da_ritirare, pickup_deadline) from branch "issues"; either rename the
file to update-0.4.5.md and update its header and body to describe the v0.4.5
pickup confirmation workflow and list the new fields (da_ritirare,
pickup_deadline) and related changes, or remove the file from the PR if it was
added by mistake—ensure the MR/commit message and file name/version in the file
body match the PR target version (v0.4.5) before finalizing.
Comment on lines
+9
to
+98
| ## Novità Principali | ||
|
|
||
| ### Sistema di Prenotazioni Potenziato | ||
| - Gestione avanzata delle code di prenotazione | ||
| - Riassegnazione automatica quando un libro diventa disponibile | ||
| - Calcolo disponibilità in tempo reale | ||
| - Notifiche ottimizzate inviate solo al completamento delle operazioni | ||
|
|
||
| ### Performance Migliorate | ||
| - Sistema di caching completamente rivisto | ||
| - Query ottimizzate per operazioni soft-delete | ||
| - Asset frontend più leggeri e veloci | ||
| - Ridotto il tempo di caricamento delle pagine catalogo | ||
|
|
||
| ### Sicurezza Rafforzata | ||
| - Protezione XSS migliorata nei renderer delle liste | ||
| - Gestione CSRF con pagina di sessione scaduta user-friendly | ||
| - Validazione input più rigorosa | ||
| - Controlli null difensivi per prevenire errori JavaScript | ||
|
|
||
| ### Scraping e ISBN | ||
| - Rilevamento ISBN migliorato con cross-check EAN | ||
| - Normalizzazione automatica caratteri MARC-8 dai record SRU | ||
| - Conversione automatica URL cover da HTTP a HTTPS | ||
| - Deduplicazione intelligente nomi autori | ||
| - Gestione fallback quando i plugin restituiscono dati parziali | ||
|
|
||
| --- | ||
|
|
||
| ## Miglioramenti Tecnici | ||
|
|
||
| ### Filtri e Ricerca | ||
| - Autocomplete per autore, editore, genere e posizione | ||
| - Ordinamento DataTable corretto | ||
| - Rendering robusto anche con dati incompleti | ||
|
|
||
| ### Installer e Build | ||
| - Inclusione frontend per personalizzazioni | ||
| - Script di build verificato con controlli automatici | ||
| - Tracciamento package-lock.json per build riproducibili | ||
|
|
||
| ### Database | ||
| - Migrazioni idempotenti (eseguibili più volte senza errori) | ||
| - Schema ottimizzato con AUTO_INCREMENT reset | ||
| - Logging transazionale migliorato | ||
|
|
||
| --- | ||
|
|
||
| ## Nota per Utenti su Hosting Condiviso | ||
|
|
||
| Gli utenti che aggiornano dalla versione 0.4.1, 0.4.2 o 0.4.3 potrebbero riscontrare problemi con l'aggiornamento automatico su alcuni hosting condivisi. | ||
|
|
||
| **Causa:** Le versioni precedenti utilizzavano la directory temporanea di sistema che su alcuni hosting ha restrizioni. | ||
|
|
||
| **Soluzione:** Abbiamo incluso nel pacchetto di rilascio una cartella `test-updater/` con i file corretti da caricare via FTP prima di eseguire l'aggiornamento. In alternativa, usare lo script `manual-update.php` incluso. | ||
|
|
||
| La versione 0.4.4 risolve definitivamente questo problema utilizzando sempre la directory `storage/tmp` dell'applicazione, con download via cURL e retry automatico. | ||
|
|
||
| --- | ||
|
|
||
| ## Changelog Completo | ||
|
|
||
| ### v0.4.4 | ||
| - Updater robusto: usa storage/tmp, cURL, retry automatico, controllo spazio disco | ||
| - Endpoint log per debug aggiornamenti (/admin/updates/logs) | ||
| - Hardening sicurezza renderer liste | ||
| - Fix ConfigStore e QueryCache | ||
| - Fix installer e consistenza cache | ||
|
|
||
| ### v0.4.3 | ||
| - Ottimizzazione caching e query | ||
| - Ottimizzazione asset frontend | ||
| - Fix filtri autocomplete | ||
| - Rendering difensivo DataTable | ||
|
|
||
| ### v0.4.2 | ||
| - Nuovo servizio riassegnazione prenotazioni | ||
| - Fix disponibilità automatica | ||
| - Notifiche differite dopo commit transazione | ||
| - Fix ordinamento DataTable | ||
|
|
||
| ### v0.4.1 | ||
| - Miglioramenti rilevamento ISBN | ||
| - Normalizzazione MARC-8 completa | ||
| - Auto-upgrade URL cover HTTP->HTTPS | ||
| - Cross-check ISBN/EAN per duplicati | ||
| - Recovery mechanism per aggiornamenti falliti | ||
| - Meccanismo di recupero aggiornamenti | ||
| - Inclusione frontend nel pacchetto release | ||
|
|
There was a problem hiding this comment.
Mancano le funzionalità principali della v0.4.5.
Se questo file deve documentare la release v0.4.5, manca completamente la sezione dedicata al workflow di conferma ritiro, che è il cuore del PR #36:
- Nuovo stato prestito
da_ritirareper libri approvati in giornata - Conferma ritiro da parte dell'admin con pulsante "Conferma Ritiro"
- Gestione scadenza ritiro (
pickup_deadline,pickup_expiry_days) - Nuovi template email per "pronto per il ritiro" e "ritiro scaduto"
- Integrazione calendario ICS per stato
da_ritirare - Aggiornamenti UI, CSV export, e filtri
Aggiungi una sezione dedicata nelle "Novità Principali" per documentare questo workflow completo.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: "ingresso" "entrata" "immissione"
Context: ...one scaduta user-friendly - Validazione input più rigorosa - Controlli null difensivi...
(ST_01_005)
🤖 Prompt for AI Agents
In `@update-0.4.4.md` around lines 9 - 98, The changelog/update-0.4.4.md is
missing the v0.4.5 release notes that document the new "conferma ritiro"
workflow introduced in PR `#36`; add a new v0.4.5 section (and a corresponding
bullet under "Novità Principali") that lists the core items: new loan state
da_ritirare, admin action "Conferma Ritiro" button, pickup_deadline and
pickup_expiry_days fields/behaviour, new email templates for "pronto per il
ritiro" and "ritiro scaduto", ICS calendar integration for da_ritirare, and
related UI, CSV export and filter updates so readers can see the complete
workflow.
Comment on lines
+200
to
+207
| Located in `installer/database/migrations/`: | ||
|
|
||
| ``` | ||
| migrate_0.4.0.sql | ||
| migrate_0.4.1.sql | ||
| migrate_0.4.2.sql | ||
| migrate_0.4.4.sql | ||
| ``` |
There was a problem hiding this comment.
Aggiorna l'elenco dei file di migrazione per includere v0.4.5.
La lista dei file di migrazione si ferma a migrate_0.4.4.sql, ma il PR introduce la versione 0.4.5 che include modifiche al database (nuova colonna pickup_deadline, nuovo stato da_ritirare). Aggiungi migrate_0.4.5.sql all'esempio.
📝 Correzione proposta
migrate_0.4.0.sql
migrate_0.4.1.sql
migrate_0.4.2.sql
migrate_0.4.4.sql
+migrate_0.4.5.sql📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Located in `installer/database/migrations/`: | |
| ``` | |
| migrate_0.4.0.sql | |
| migrate_0.4.1.sql | |
| migrate_0.4.2.sql | |
| migrate_0.4.4.sql | |
| ``` | |
| Located in `installer/database/migrations/`: | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@updater.md` around lines 200 - 207, Aggiorna l'elenco delle migrazioni in
updater.md aggiungendo la voce migrate_0.4.5.sql; questa nuova migration
introduce la colonna pickup_deadline e il nuovo stato 'da_ritirare', quindi
assicurati che il nome migrate_0.4.5.sql compaia nell'esempio insieme alle altre
migrate_0.4.x.sql per riflettere correttamente la versione 0.4.5.
Comment on lines
+450
to
+456
| ## Version History | ||
|
|
||
| | Version | Changes | | ||
| |---------|---------| | ||
| | 0.4.4 | Fixed: Always use storage/tmp, cURL download, retry mechanism, disk space check | | ||
| | 0.4.3 | Added: Log viewer endpoint | | ||
| | 0.4.2 | Added: Verbose logging | |
There was a problem hiding this comment.
Aggiungi la versione 0.4.5 alla cronologia.
La cronologia delle versioni si ferma alla 0.4.4, ma questo PR introduce la v0.4.5 con modifiche significative (workflow di conferma ritiro, nuove migrazioni DB, gestione scadenze pickup). Aggiungi una riga che riassuma le novità di questa release.
📝 Aggiunta proposta
| Version | Changes |
|---------|---------|
+| 0.4.5 | Added: Pickup confirmation workflow, pickup deadline management, new loan state da_ritirare |
| 0.4.4 | Fixed: Always use storage/tmp, cURL download, retry mechanism, disk space check |
| 0.4.3 | Added: Log viewer endpoint |
| 0.4.2 | Added: Verbose logging |
| 0.4.1 | Bug: Uses sys_get_temp_dir() (fails on shared hosting) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Version History | |
| | Version | Changes | | |
| |---------|---------| | |
| | 0.4.4 | Fixed: Always use storage/tmp, cURL download, retry mechanism, disk space check | | |
| | 0.4.3 | Added: Log viewer endpoint | | |
| | 0.4.2 | Added: Verbose logging | | |
| ## Version History | |
| | Version | Changes | | |
| |---------|---------| | |
| | 0.4.5 | Added: Pickup confirmation workflow, pickup deadline management, new loan state da_ritirare | | |
| | 0.4.4 | Fixed: Always use storage/tmp, cURL download, retry mechanism, disk space check | | |
| | 0.4.3 | Added: Log viewer endpoint | | |
| | 0.4.2 | Added: Verbose logging | |
🤖 Prompt for AI Agents
In `@updater.md` around lines 450 - 456, Aggiorna la sezione "Version History"
aggiungendo una riga per la versione 0.4.5 nella tabella: inserisci "| 0.4.5 |
Added: Confirmation workflow for pickups; DB migrations for new pickup flow;
Pickup expiration/timeout handling and related fixes |" sotto la riga esistente
per 0.4.4 in modo che la cronologia rifletta le modifiche introdotte dal PR.
- FrontendController: add 'in_ritardo' to availability SQL query - DashboardStats: fix SQL injection by using CURDATE() instead of $today - LoanApprovalController: add dedicated cancelPickup() endpoint - web.php: add POST /admin/loans/cancel-pickup route - loan-actions-swal.php: use cancel-pickup endpoint instead of reject - pending_loans.php: calculate $today once before loop - NotificationService: add sendPickupCancelledNotification() - SettingsMailTemplates: add loan_pickup_cancelled template - dashboard/index.php: add orange color for da_ritirare in calendar - migrate_0.4.5.sql: add email templates for pickup workflow - data_it_IT.sql: add email templates for new installations - en_US.json: add English translations for new strings - public/.htaccess: installer compatibility improvements - public/installer/: add Apache-compatible installer entry point
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@app/Controllers/LoanApprovalController.php`:
- Around line 585-589: The code unconditionally sets a copy back to
'disponibile' which overwrites statuses like 'perso', 'danneggiato', or
'manutenzione'; fetch the current status of the copy via the CopyRepository (use
the same lookup used in confirmPickup), and only call updateStatus($copiaId,
'disponibile') if the current status is a valid transition (i.e., not in
['perso','danneggiato','manutenzione'] — or match the exact allowed-status check
implemented in confirmPickup); otherwise skip the update and leave the status
unchanged.
In `@app/Routes/web.php`:
- Around line 1775-1776: The query adds the stato 'da_ritirare' but the UI logic
only treats 'prenotato'/'pendente' as reserved via the hasReserved flag, causing
'da_ritirare' rows to be colored as "in corso"; update the reservation flag
calculation (the place that computes hasReserved) to include 'da_ritirare'
alongside 'prenotato' and 'pendente' so those records render as reserved
(yellow/orange) instead of overdue/red—locate the code that inspects the
row.stato or constructs hasReserved and add 'da_ritirare' to the set of reserved
statuses.
In `@app/Views/admin/pending_loans.php`:
- Around line 124-139: The header block is only shown when $pickupLoans is
non-empty, so change the conditional around the header (the PHP if that
currently checks !empty($pickupLoans)) to check if either $pickupLoans or
$pendingLoans is non-empty (e.g., if (!empty($pickupLoans) ||
!empty($pendingLoans))) so the "Richieste in Attesa" header and count
(count($pendingLoans)) render when there are pending loans even if $pickupLoans
is empty.
In `@app/Views/dashboard/index.php`:
- Around line 736-739: The calendar legend is missing the new 'da_ritirare'
status even though match($event['status']) sets its color via $startColor (uses
'#F97316'); update the legend rendering to add a dedicated entry for
'da_ritirare' (label e.g. "Da ritirare") with the same orange color '#F97316',
and ensure any localization/translation keys used by the legend are updated
accordingly so the legend matches the status handling in the match(...) block
and the $isReservation logic.
In `@public/.htaccess`:
- Around line 1-3: Aggiungi la direttiva di Options necessaria nel file
.htaccess per permettere mod_rewrite: abilita almeno FollowSymLinks (Options
+FollowSymLinks) oppure preferibilmente SymLinksIfOwnerMatch (Options
+SymLinksIfOwnerMatch) in sostituzione, e verifica che la configurazione del
server consenta Options per questa directory tramite AllowOverride; se possibile
valuta alternative più sicure (Alias/ScriptAlias o mount point) e documenta la
scelta nel commento del file .htaccess.
Comment on lines
1
to
3
| <IfModule mod_rewrite.c> | ||
| Options +FollowSymLinks | ||
| RewriteEngine On |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Apache mod_rewrite FollowSymLinks requirement
💡 Result:
Short answer: When using mod_rewrite in per-directory context (htaccess or ) Apache requires that either Options FollowSymLinks or Options SymLinksIfOwnerMatch be enabled; if both are off RewriteRule is forbidden and will produce an AH00670 403/error. See Apache mod_rewrite docs and examples. [1][2]
Sources:
- Apache mod_rewrite (per-directory rules require Options FollowSymLinks). [1]
- Common error and guidance (AH00670: both FollowSymLinks and SymLinksIfOwnerMatch are off — enable one). [2]
🌐 Web query:
Apache Options FollowSymLinks vs SymLinksIfOwnerMatch security
💡 Result:
Short summary
-
What they do:
- FollowSymLinks — Apache will follow any filesystem symlink in that directory. (httpd.apache.org) [1]
- SymLinksIfOwnerMatch — Apache follows a symlink only if the link and its target have the same owning user id. (httpd.apache.org) [2]
-
Security implications:
- SymLinksIfOwnerMatch is stricter but not a full fix — Apache itself warns the option “should not be considered a security restriction” because symlink checks are subject to race conditions and can be circumvented. (httpd.apache.org) [1][4]
- Allowing either option increases risk: an attacker or malicious insider who can create symlinks (or exploit a race) may expose files outside DocumentRoot. CIS/Tenable recommends disabling symlink-following where possible. (tenable.com) [3][5]
-
Practical guidance:
- Prefer disabling FollowSymLinks (use Options -FollowSymLinks) where not needed. If you must map outside DocumentRoot, prefer explicit Alias/ScriptAlias or controlled filesystem mounts. (svn.apache.org) [6]
- If you need symlinks for multi-user hosting, SymLinksIfOwnerMatch is safer than FollowSymLinks but still imperfect — mitigate with filesystem/kernel controls (e.g., grsecurity/SELinux, careful permissions), restrict who can create links, and audit configs. (blog.hboeck.de) [4][3]
Sources
[1] Apache mod_core Options docs. (httpd.apache.org)
[2] Apache Options description (same docs). (httpd.apache.org)
[3] CIS/Tenable guidance on minimizing Options. (tenable.com)
[4] Analysis of race-condition risks and kernel-level mitigations. (blog.hboeck.de)
[5] Apache URL-to-filesystem mapping / Alias recommendation. (svn.apache.org)
If you want, I can show example Apache config lines (disable FollowSymLinks, use Alias) and explain how to audit your vhosts.
Citations:
- 1: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 2: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 3: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 4: https://www.tenable.com/audits/items/CIS_Apache_HTTP_Server_2.4_Benchmark_v1.5.0_Level_2.audit%3A855613ff20b4b40853114fe346240583?utm_source=openai
- 5: https://svn.apache.org/repos/asf/httpd/httpd/branches/authz-dev/docs/manual/urlmapping.html.en?utm_source=openai
- 6: https://blog.hboeck.de/archives/873-The-tricky-security-issue-with-FollowSymLinks-and-Apache.html?utm_source=openai
- 7: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 8: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 9: https://www.tenable.com/audits/items/CIS_Apache_HTTP_Server_2.4_Benchmark_v1.5.0_Level_2.audit%3A855613ff20b4b40853114fe346240583?utm_source=openai
- 10: https://blog.hboeck.de/archives/873-The-tricky-security-issue-with-FollowSymLinks-and-Apache.html?utm_source=openai
- 11: https://svn.apache.org/repos/asf/httpd/httpd/branches/authz-dev/docs/manual/urlmapping.html.en?utm_source=openai
🌐 Web query:
Apache AllowOverride Options .htaccess compatibility issues
💡 Result:
Key compatibility/behavior issues when using AllowOverride Options with .htaccess (concise):
- AllowOverride applies only in (not , ); if directory disallows it, .htaccess Options will be ignored. (pmcourt.org)
- AllowOverride Options lets .htaccess use the Options directive, but you cannot force a specific inherited Option to remain set — a permitted Options directive in .htaccess can disable inherited options via non‑relative syntax. (Security implication.) (httpd.apache.org)
- Syntax sensitivity: mixing relative (+ / -) and non‑relative Options, or invalid Options syntax, can cause errors or be rejected. Ensure server/global Options are compatible with what .htaccess will set. (stackoverflow.com)
- Version/feature differences: modern 2.4+ docs add AllowOverrideList and more granular override classes; consult the Override Class Index to know exactly which directives are allowed under Options (and related groups). Behaviour and available compatibility modules (e.g., mod_access_compat for legacy auth directives) vary by Apache version. (httpd.apache.org)
- Best practices / pitfalls: prefer putting Options in main server config (for performance/security) and use AllowOverride only where necessary; if you get “Options not allowed here” or internal errors, check the AllowOverride value for that directory and adjust (or move the directive to server config). (stackoverflow.com)
If you want, tell me your Apache version and the .htaccess/Directory config and I’ll point to the exact directives/changes required.
Citations:
- 1: https://www.pmcourt.org/manual/mod/core.html?utm_source=openai
- 2: https://httpd.apache.org/docs/2.2/en/mod/core.html?utm_source=openai
- 3: https://stackoverflow.com/questions/5301776/directory-index-forbidden-by-options-directive?utm_source=openai
- 4: https://httpd.apache.org/docs/current/mod/overrides.html?utm_source=openai
- 5: https://stackoverflow.com/questions/7097004/htaccess-options-not-allowed-here?utm_source=openai
Options +FollowSymLinks è obbligatorio per il corretto funzionamento di mod_rewrite.
Apache richiede che sia FollowSymLinks che SymLinksIfOwnerMatch siano abilitati in contesto per-directory (.htaccess), altrimenti mod_rewrite genera l'errore AH00670 (403 Forbidden). Questa modifica è quindi necessaria.
Considerazioni sulla sicurezza:
-
+SymLinksIfOwnerMatchè preferibile a+FollowSymLinksin quanto segue i symlink solo se il proprietario corrisponde, ma Apache avverte che non deve essere considerata una restrizione di sicurezza reale — è vulnerabile a race condition e può essere bypassata. Entrambe le opzioni aumentano il rischio di esposizione di file al di fuori di DocumentRoot. -
Alternativa consigliata: Se possibile, evitare symlink e utilizzare
AliasoScriptAliasper il mapping esplicito di directory, oppure montaggi di filesystem controllati. Ciò elimina il rischio completamente. -
Compatibilità: Verificare che
AllowOverridenella configurazione del server permettaOptionsper questa directory, altrimenti la direttiva sarà ignorata o causerà errori.
Valutare se le funzionalità di symlink sono effettivamente necessarie o se è possibile ricorrere a soluzioni alternative più sicure.
🤖 Prompt for AI Agents
In `@public/.htaccess` around lines 1 - 3, Aggiungi la direttiva di Options
necessaria nel file .htaccess per permettere mod_rewrite: abilita almeno
FollowSymLinks (Options +FollowSymLinks) oppure preferibilmente
SymLinksIfOwnerMatch (Options +SymLinksIfOwnerMatch) in sostituzione, e verifica
che la configurazione del server consenta Options per questa directory tramite
AllowOverride; se possibile valuta alternative più sicure (Alias/ScriptAlias o
mount point) e documenta la scelta nel commento del file .htaccess.
…ull copia_id - cancelPickup(): Add ReservationReassignmentService to promote next user in queue when pickup is cancelled (critical for systems without MaintenanceService cron job) - confirmPickup(): Block operation if copia_id is NULL to prevent data integrity issues with legacy/migrated loans - Add English translation for new error message Addresses review feedback about queue advancement and data integrity.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/Controllers/LoanApprovalController.php`:
- Around line 487-499: The code reads copie.stato then unconditionally calls
CopyRepository->updateStatus($copiaId, 'prestato'), which risks a TOCTOU race;
fix by performing the check-and-set inside a transaction using a row lock or by
making updateStatus conditional: start a DB transaction, SELECT stato FROM copie
WHERE id = ? FOR UPDATE (or change updateStatus to run UPDATE copie SET
stato='prestato' WHERE id=? AND stato NOT IN
('perso','danneggiato','manutenzione')), then verify affected_rows > 0 before
committing and returning success; ensure CopyRepository::updateStatus and the
surrounding logic use the same transaction/connection and handle rollback on
failure.
- Around line 602-616: Aprire una transazione e bloccare la riga della copia
prima di leggere e aggiornare: usare $db->begin_transaction(), eseguire il
SELECT stato FROM copie WHERE id = ? FOR UPDATE (usando $copyCheckStmt come nel
diff) per leggere lo stato in modo atomico, verificare contro $invalidStates,
chiamare \App\Models\CopyRepository->updateStatus($copiaId, 'disponibile')
mentre la transazione è ancora attiva, poi fare $db->commit(); infine chiamare
$reassignmentService->reassignOnReturn($copiaId) dopo il commit; in caso di
errore fare rollback e chiudere gli statement.
- Document new da_ritirare loan state and pickup workflow - Add loan state flow diagrams - Document calendar integration for pickup-pending loans - Add reservation queue improvements section - Move v0.4.4 to Previous Releases (collapsible) - Add v0.4.5 migration notes to Upgrading section - Update Handy Paths with new controller paths fix: TOCTOU race conditions in LoanApprovalController Add FOR UPDATE to SELECT statements to prevent time-of-check-time-of-use race conditions when checking copy state before updating: - confirmPickup(): Lock copy row before state validation - cancelPickup(): Lock copy row before setting back to disponibile
- Add English email templates for pickup workflow (16-18) - Update bundled plugins max_app_version to 0.4.5 - Set consistent requires_app to 0.4.0 for all plugins
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
da_ritirare: Quando un prestito viene approvato per oggi, il libro passa a "pronto per il ritiro" invece di "in corso"pickup_expiry_days(default 3 giorni) per la deadline di ritiroFlusso Prestito
File Modificati
schema.sql,migrate_0.4.5.sql,data_*.sqlLoanApprovalController.php(+confirmPickup)web.php(POST /admin/loans/confirm-pickup)DataIntegrity,MaintenanceService,NotificationService,ReservationManagerprestiti/index.php(badge + pulsante ritiro)SettingsMailTemplates.php(2 nuovi template)IcsGenerator.php(supporto da_ritirare)en_US.json(+19 traduzioni)Test plan
da_ritirare(nonin_corso)da_ritirarein_corso, copia →prestatoprenotatocon data raggiunta mostra pulsante ritiroscadutoda_ritirareda_ritirareSummary by CodeRabbit
Nuove Funzionalità
Miglioramenti
Migliorie front-end
✏️ Tip: You can customize this high-level summary in your review settings.