Skip to content

Conversation

@fabiodalez-dev
Copy link
Owner

@fabiodalez-dev fabiodalez-dev commented Dec 3, 2025

Summary

This PR introduces Pinakes v0.3.0 with the Dewey Editor plugin, complete cleanup of legacy Dewey database code, and automatic Dewey extraction from SBN.

New Features

  • Dewey Editor plugin: Visual editor for managing Dewey classification JSON files
    • Add, edit, delete Dewey codes with validation
    • Import/export JSON with automatic backup
    • Merge imports preserving existing data
    • Support for both Italian and English locales
    • Improved decimal prefix error messages (shows base code)
    • Unsaved changes warning before import/restore operations
  • SBN Dewey extraction: Automatically import Dewey classification from SBN (Italian National Library) API
    • Extract Dewey codes and names from SBN full records
    • Auto-populate JSON files with new classifications (language-aware)
    • Book form automatically populates Dewey field when scraping
  • Z39.50/SRU Dewey scraping: Extract Dewey classification from MARC 082 field and DC elements
  • Author/Publisher merge: Bulk merge duplicate authors or publishers
    • Select 2+ entries and merge into one
    • Choose which entry to keep as primary
    • Optionally rename the merged entry
    • All books automatically reassigned to merged entry
  • Author name normalization: Improved duplicate detection
    • Converts "Surname, Name" → "Name Surname" format
    • Case-insensitive matching
    • Fuzzy matching with word order independence
    • Prevents duplicate authors during scraping (e.g., "Levi, Primo" matches "Primo Levi")
  • Plugin encryption key: Added PLUGIN_ENCRYPTION_KEY to environment for secure plugin settings

Breaking Changes ⚠️

Database schema changes require migration for existing installations:

mysql -u USER -p DATABASE < installer/database/migrations/migrate_0.3.0.sql

Or run manually:

ALTER TABLE libri CHANGE COLUMN classificazione_dowey classificazione_dewey VARCHAR(50) DEFAULT NULL;

Removed

  • classificazione table (Dewey data now in JSON files)
  • DeweyController.php and legacy admin page
  • FK columns dowey_category_id and dowey_subcategory_id

Changed

  • Database now has 39 tables (was 40)
  • Fixed typo: doweydewey throughout codebase

Improvements

  • Custom Dewey codes (not in JSON) now display correctly with parent name fallback
  • Book form handles missing codes gracefully
  • Better error handling in dewey-editor plugin
  • Z39ServerPlugin now uses PLUGIN_ENCRYPTION_KEY for decryption (same as PluginManager)
  • Author normalization in AuthorRepository prevents duplicate entries during scraping

Files Changed

  • Removed: DeweyController.php, admin/dewey.php
  • Modified: Multiple PHP files (dowey→dewey rename, SBN integration)
  • Added: CHANGELOG.md, migration script, dewey-editor.zip, DeweyAutoPopulator.php
  • Added: Author/Publisher merge functionality in admin panels

Test Plan

  • Fresh installation creates correct tables
  • Existing installation works after running migration
  • Dewey Editor plugin functions (add/edit/delete/import/export)
  • Book form displays Dewey names correctly
  • Book detail page shows Dewey classification with name
  • SBN scraping returns Dewey classification
  • Dewey field auto-populated in book form after scraping
  • Plugin encryption/decryption works with PLUGIN_ENCRYPTION_KEY
  • Author merge works correctly (select 2+, choose primary, optional rename)
  • Publisher merge works correctly
  • Author normalization prevents duplicates during scraping

Summary by CodeRabbit

  • Nuove Funzionalità

    • Editor Dewey amministrativo completo: albero, import/merge, backup/restore, validazione, ricerca e API di navigazione
    • UI libro: selezione Dewey a chip, inserimento manuale, risoluzione nomi e integrazione import ISBN
    • Merge batch per autori/editori via interfacce pubbliche; auto‑popolamento Dewey da sorgenti esterne; normalizzazione autori
  • Chores / Correzioni

    • Versione aggiornata a 0.3.0 e migrazione campo classificazione_dewey
    • Nuovo bundle per sanitizzazione, ampio aggiornamento CSS/JS e ottimizzazioni dark‑mode
    • Accesso a feed .ics, esempio chiave plugin per cifratura e aggiornamenti traduzioni UI

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

- Add dewey-editor plugin for visual editing of Dewey classification JSON
  - Tree view with expand/collapse for all hierarchy levels
  - Edit names, add decimal codes, delete decimal codes
  - Import/Export JSON with validation
  - Automatic backup before save (max 5 backups)
  - Support for IT/EN locales
  - Integrated with standard app layout (sidebar, header)

- Improve Dewey selector in book form
  - Auto-navigate to all hierarchy levels when editing existing book
  - Calculate and display full path (e.g., 500 → 590 → 599 → 599.9)
  - Pre-select values in all dropdowns based on saved code

- Fix status badges contrast in book detail page
  - Use solid background colors with white text for better readability
  - Disponibile (green), Prestato (red), In ritardo (amber),
    Danneggiato (orange), Perso (gray)

- Add "Open Editor" button in plugins page for dewey-editor

- Update .gitignore for local settings and webpack artifacts

- Rebuild frontend assets
@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

Aggiunge il plugin "Dewey Editor" con editor ad albero, API amministrative e backup/import/merge; migra il campo classificazione_doweyclassificazione_dewey attraverso modello, controller, view e migration; introduce supporto per formato flat dei dati Dewey, auto‑popolazione da SBN/SRU, nuove rotte API (children/search/reseed) e aggiornamenti ad assets, installatore e UI.

Changes

Cohort / File(s) Cambio
Plugin Dewey Editor
storage/plugins/dewey-editor/DeweyEditorPlugin.php, storage/plugins/dewey-editor/classes/DeweyValidator.php, storage/plugins/dewey-editor/plugin.json, storage/plugins/dewey-editor/views/editor.php, storage/plugins/dewey-editor/wrapper.php
Nuovo plugin: lifecycle, rotte admin/API, editor ad albero, operazioni data (get/save/validate/export/import/merge), backup/restore, validazione ricorsiva e helper I/O; API pubbliche esposte per la view.
API Dewey & Routing
app/Controllers/DeweyApiController.php, app/Routes/web.php
Supporto per formato flat (dewey_completo_*.json) con fallback legacy; nuovi endpoint /api/dewey/children, /api/dewey/search, /api/dewey/reseed; wiring rotte merge autori/editori; aggiornamento middleware CsrfMiddleware instanziato senza container; risposte JSON strutturate.
Interfaccia Libri (form & view)
app/Views/libri/partials/book_form.php, app/Views/libri/scheda_libro.php, app/Controllers/LibriController.php, app/Models/BookRepository.php
Rinominato campo a classificazione_dewey; form sostituito con UI a chip dinamica e JS per navigazione/validazione via API; scheda libro risolve dinamicamente il nome Dewey; hook globali (window.setDeweyCode).
Auto‑popolazione & Scraping
app/Support/DeweyAutoPopulator.php, app/Controllers/ScrapeController.php, storage/plugins/z39-server/classes/SbnClient.php, storage/plugins/z39-server/classes/SruClient.php
Nuova classe per upsert locale di voci Dewey da scraping; integrazione post‑scrape; SBN/SRU aggiornati per estrarre classificazione_dewey e nome associato; logica per creare genitori ricorsivi.
CSV & Import
app/Controllers/CsvImportController.php
Supporto classificazione_dewey in esempi CSV, binding DB e flusso di import; arricchimento record da scraping (dewey, anno, lingua, parole chiave).
Schema, Migrazione & Installer
installer/database/schema.sql, installer/database/migrations/migrate_0.3.0.sql, installer/classes/Installer.php
Rimosso table classificazione; rinominata colonna libri.classificazione_doweyclassificazione_dewey; migration SQL aggiunta; Installer aggiornato per non aspettare più la tabella e per includere dewey-editor; defaults Z39 localizzati.
Rimozione legacy & admin
app/Controllers/DeweyController.php (rimosso), app/Views/admin/dewey.php (rimosso), app/Views/admin/plugins.php
Rimosso controller e view legacy basati su DB; aggiunto pulsante "Apri Editor" nel pannello plugin quando dewey-editor attivo.
Plugin Z39 Server & settings
storage/plugins/z39-server/Z39ServerPlugin.php, storage/plugins/z39-server/endpoint.php, storage/plugins/z39-server/wrapper.php, storage/plugins/z39-server/classes/SbnClient.php
Aggiunto helper isSettingEnabled, supporto alla decrittazione ENC: (AES‑256‑GCM) via decryptSettingValue, refactor getSetting; SBN arricchisce classificazione_dewey; wrapper per caricamento plugin.
Assets & Styling
public/assets/418.bundle.js, public/assets/main.bundle.js, public/assets/main.css
Nuovo bundle sanitizer con API pubblica; main.bundle.js semplificato in IIFE minificato; ampia revisione CSS (nuove utility, dark overrides) e rimozione di sezioni CSS esterne.
Locale, versioning & config
locale/en_US.json, version.json, .env.example, .gitignore
Nuove traduzioni e stringhe per merge/Dewey; versione aggiornata a 0.3.0; aggiunta PLUGIN_ENCRYPTION_KEY in .env.example; .gitignore aggiornato per dewey-editor, storage/calendar e artifact.
Routing & .htaccess calendar
.htaccess, storage/calendar/.htaccess
Regole per consentire accesso diretto a .ics in /storage/calendar/, MIME text/calendar e override deny; rewrite pubblico aggiornato per bypass .ics.
Installer UI & vari
installer/steps/step0.php, installer/steps/step5.php, installer/assets/style.css, public/index.php
SVG bandiere in step0, opzione FORCE_HTTPS in step5, CSS .flag-emoji, guardrail redirect canonical in public/index.php.
Autori & Editori: merge e normalizzazione
app/Models/AuthorRepository.php, app/Support/AuthorNormalizer.php, app/Models/PublisherRepository.php, app/Views/autori/index.php, app/Views/editori/index.php, app/Support/MergeHelper.php
Nuovo AuthorNormalizer; AuthorRepository: findByName, findDuplicates, mergeAuthors; PublisherRepository: mergePublishers; helper centralizzato MergeHelper; UI bulk‑merge per autori/editori.
Refactor formati Dewey nei formatter
storage/plugins/z39-server/classes/DublinCoreFormatter.php, storage/plugins/z39-server/classes/MARCXMLFormatter.php, storage/plugins/z39-server/classes/MODSFormatter.php
Formatter aggiornati per leggere classificazione_dewey al posto del vecchio campo.
Varie rimozioni documentazione
z39.50.md (rimosso)
Documentazione Z39.50/SRU eliminata.

Sequence Diagram(s)

sequenceDiagram
    participant Admin as Admin (Browser)
    participant UI as Dewey Editor UI
    participant PluginAPI as DeweyEditorPlugin API
    participant Validator as DeweyValidator
    participant FS as Filesystem (dewey_completo_{locale}.json)

    Admin->>UI: Apri editor
    UI->>PluginAPI: GET /api/dewey-editor/data/{locale}
    PluginAPI->>FS: Leggi JSON locale
    FS-->>PluginAPI: Dati JSON
    PluginAPI-->>UI: Restituisce tree

    Admin->>UI: Modifica/Aggiungi/Elimina nodi (in-memory)
    UI->>UI: Traccia modifiche

    Admin->>UI: Salva
    UI->>PluginAPI: POST /api/dewey-editor/save/{locale}
    PluginAPI->>Validator: Valida struttura
    Validator-->>PluginAPI: OK / Errori
    alt OK
        PluginAPI->>FS: Crea backup
        PluginAPI->>FS: Scrivi JSON aggiornato
        FS-->>PluginAPI: OK
        PluginAPI-->>UI: Successo
    else Errori
        PluginAPI-->>UI: Errori di validazione
    end
Loading
sequenceDiagram
    participant Scraper as Scrape/CsvImport
    participant SBN as SBN Client (Z39)
    participant AutoPop as DeweyAutoPopulator
    participant FS as Filesystem (dewey_completo_{locale}.json)
    participant DeweyAPI as DeweyApiController

    Scraper->>SBN: Richiesta metadati ISBN
    SBN-->>Scraper: Record + classificazione_dewey
    Scraper->>AutoPop: processBookData({code,name,source})
    AutoPop->>AutoPop: Risolve lingua → locale
    AutoPop->>FS: Leggi JSON locale
    AutoPop->>FS: Controlla esistenza entry
    alt Entry mancante
        AutoPop->>FS: Aggiungi entry + genitori ricorsivi
        FS-->>AutoPop: Scrittura OK
    else Entry esistente con nome vuoto
        AutoPop->>FS: Aggiorna name
    end
    AutoPop-->>Scraper: true/false
    Scraper->>DeweyAPI: (opzionale) /api/dewey/children o /api/dewey/search per UI/validazione
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

  • Aree che richiedono attenzione:
    • storage/plugins/dewey-editor/DeweyEditorPlugin.php — merge ricorsivo, backup/cleanup, permessi I/O, sicurezza rotte/API.
    • storage/plugins/dewey-editor/classes/DeweyValidator.php — regole di validazione, unicità, pattern codici.
    • app/Support/DeweyAutoPopulator.php — manipolazioni ricorsive JSON, concorrenza/race conditions, locking e scritture atomiche.
    • app/Views/libri/partials/book_form.php — JS complesso (async API, validazione), integrazione form e hook globali.
    • Migration/schema (migrate_0.3.0.sql, installer/database/schema.sql) — verifica dati esistenti, backup/rollback, impatto chiavi esterne e indici rimossi.
    • storage/plugins/z39-server/* — decrittazione impostazioni (AES‑256‑GCM), gestione chiave environment e fallback sicuro.
    • public/assets/418.bundle.js & public/assets/main.css — revisione sicurezza sanitizer, compatibilità e impatto styling globale.

Possibly related PRs

Poem

🐰
Nel cesto di JSON il coniglio salta piano,
ricuce rami, aggiunge nodi e fa il backup sano,
un click, un merge, l'albero cresce sottile,
codici danzano, libri trovano il loro stile,
il bosco di Dewey ora ha un giardiniere gentile. 📚✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.06% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 descrive chiaramente i cambiamenti principali del PR: rilascio della versione 0.3.0 con l'introduzione del plugin Dewey Editor e la pulizia del codice legacy.
✨ 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 dewey-editor-v2

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (9)
app/Views/libri/scheda_libro.php (2)

7-18: Badge di stato e pulsante danger più leggibili; valutare riuso delle classi

Le nuove classi Tailwind per $statusClasses e $statusBadgeClass producono badge arrotondati con buon contrasto e il nuovo $btnDanger rende più chiara l’azione distruttiva. Per evitare duplicazione e differenze cromatiche, potresti valutare di riutilizzare lo stesso mapping anche nella sezione “Dettagli Libro” (campo “Stato”), dove oggi esiste una seconda mappatura CSS separata.


398-447: Nome Dewey dinamico ben gestito; si può rendere il JS un po’ più robusto

La visualizzazione mostra sempre il codice (in monospace) e tenta poi di risolvere il nome via /api/dewey/search, gestendo sia il formato legacy 300-340-347 (usando l’ultimo segmento) sia i codici decimali con fallback al parent; l’uso di textContent evita problemi di XSS.

Per irrobustire leggermente lo script suggerirei:

  • Memorizzare l’elemento in una variabile e fare early‑return se manca, ad es.:
    const displayEl = document.getElementById('dewey_name_display');
    if (!displayEl || !code) return;
  • Controllare response.ok prima di chiamare response.json(), così in caso di 403/500 o di una risposta HTML (es. errore middleware) eviti eccezioni SyntaxError rumorose in console:
    const response = await fetch(...);
    if (!response.ok) return;
    const result = await response.json();

Nel complesso la funzionalità è corretta; queste sarebbero solo hardening/UX minori.

app/Controllers/DeweyApiController.php (2)

238-251: Nessuna indicazione quando il codice parent non viene trovato.

Quando $parentCode viene fornito ma non esiste nell'albero Dewey, il metodo restituisce silenziosamente un array vuoto. Questo potrebbe confondere i consumer dell'API che non riescono a distinguere tra "parent non trovato" e "parent senza figli".

Considera di restituire un errore 404 quando il parent code specificato non esiste:

             } else {
                 // Cerca il nodo parent e ritorna i suoi figli
                 $parent = $this->findNodeByCode($data, $parentCode);
-                if ($parent && isset($parent['children'])) {
+                if ($parent === null) {
+                    $response->getBody()->write(json_encode(['error' => __('Codice parent non trovato.')], JSON_UNESCAPED_UNICODE));
+                    return $response->withHeader('Content-Type', 'application/json')->withStatus(404);
+                }
+                if (isset($parent['children'])) {
                     foreach ($parent['children'] as $child) {

304-314: search() non supporta il formato legacy.

Il metodo search() verifica solo il nuovo formato e, se i dati sono in formato legacy, $result rimane null causando un errore 404. Sarebbe coerente con getChildren() restituire un errore esplicito per il formato legacy.

         if ($this->isNewFormat($data)) {
             $node = $this->findNodeByCode($data, $code);
             if ($node) {
                 $result = [
                     'code' => $node['code'],
                     'name' => $node['name'],
                     'level' => $node['level'],
                     'has_children' => !empty($node['children'])
                 ];
             }
+        } else {
+            $response->getBody()->write(json_encode(['error' => __('Usa gli endpoint specifici per il formato legacy.')], JSON_UNESCAPED_UNICODE));
+            return $response->withHeader('Content-Type', 'application/json')->withStatus(400);
         }
app/Views/libri/partials/book_form.php (5)

1035-1038: La validazione del codice Dewey potrebbe essere troppo permissiva.

La regex attuale accetta codici con 1-3 cifre intere (es. 4, 59, 599), ma i codici Dewey standard richiedono sempre 3 cifre con zeri iniziali (es. 004, 059, 599).

Considera di modificare la regex per richiedere esattamente 3 cifre:

-  const validateDeweyCode = (code) => {
-    return /^[0-9]{1,3}(\.[0-9]{1,4})?$/.test(code);
-  };
+  const validateDeweyCode = (code) => {
+    return /^[0-9]{3}(\.[0-9]{1,4})?$/.test(code);
+  };

In alternativa, se vuoi supportare l'input senza zeri iniziali e normalizzarlo automaticamente, aggiungi una funzione di normalizzazione.


1068-1092: Gestione errori insufficiente nelle chiamate API.

Le chiamate fetch usano .then(r => r.json()) senza verificare r.ok, il che può causare errori silenti se l'API restituisce un codice di stato non-200.

      try {
-        const result = await fetch(`/api/dewey/search?code=${encodeURIComponent(code)}`, {
-          credentials: 'same-origin'
-        }).then(r => r.json());
+        const response = await fetch(`/api/dewey/search?code=${encodeURIComponent(code)}`, {
+          credentials: 'same-origin'
+        });
+        if (!response.ok) {
+          console.warn('Dewey search API returned', response.status);
+        }
+        const result = response.ok ? await response.json() : null;

        if (result && result.name) {
          currentDeweyName = result.name;
        } else {
          // Non trovato, cerca il parent
          const parentCode = getParentCode(code);
          if (parentCode) {
-            const parentResult = await fetch(`/api/dewey/search?code=${encodeURIComponent(parentCode)}`, {
-              credentials: 'same-origin'
-            }).then(r => r.json());
+            const parentResponse = await fetch(`/api/dewey/search?code=${encodeURIComponent(parentCode)}`, {
+              credentials: 'same-origin'
+            });
+            const parentResult = parentResponse.ok ? await parentResponse.json() : null;

1148-1162: Gestione della risposta API in loadLevel potrebbe fallire silenziosamente.

Simile al problema precedente, la risposta fetch viene convertita in JSON senza verificare lo stato della risposta. Inoltre, se items è null o vuoto, la funzione ritorna undefined senza feedback all'utente.

  const loadLevel = async (parentCode = null, levelIndex = 0) => {
    try {
      const url = parentCode
        ? `/api/dewey/children?parent_code=${encodeURIComponent(parentCode)}`
        : '/api/dewey/children';

-      const items = await fetch(url, { credentials: 'same-origin' }).then(r => r.json());
-
-      if (!items || items.length === 0) return;
+      const response = await fetch(url, { credentials: 'same-origin' });
+      if (!response.ok) {
+        console.error('Dewey children API error:', response.status);
+        return null;
+      }
+      const items = await response.json();
+
+      if (!Array.isArray(items) || items.length === 0) return null;

1118-1133: Utilizzo inconsistente di alert() invece di SweetAlert2.

Il resto del file utilizza Swal.fire() o Toast.fire() per le notifiche utente, ma qui vengono usati alert() nativi che interrompono l'esperienza utente e hanno un aspetto diverso.

  addBtn.addEventListener('click', async () => {
    const code = manualInput.value.trim();

    if (!code) {
-      alert(__('<?= __("Inserisci un codice Dewey") ?>'));
+      Toast.fire({
+        icon: 'warning',
+        title: __('<?= __("Inserisci un codice Dewey") ?>')
+      });
      return;
    }

    if (!validateDeweyCode(code)) {
-      alert(__('<?= __("Formato codice non valido. Usa formato: 599 oppure 599.9 oppure 599.93") ?>'));
+      Toast.fire({
+        icon: 'error',
+        title: __('<?= __("Formato codice non valido") ?>'),
+        text: __('<?= __("Usa formato: 599 oppure 599.9 oppure 599.93") ?>')
+      });
      return;
    }

1114-1115: Pattern i18n funzionante ma potenzialmente confuso.

La struttura __('<?= __("text") ?>') applica la traduzione due volte (PHP poi JS). Funziona perché il PHP restituisce la chiave italiana che viene poi cercata nell'oggetto i18nTranslations del JS, ma può confondere futuri manutentori.

Per maggiore chiarezza, considera di usare un pattern più esplicito o documentare il comportamento atteso.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3fba19 and 87ffd34.

📒 Files selected for processing (12)
  • .gitignore (3 hunks)
  • app/Controllers/DeweyApiController.php (3 hunks)
  • app/Routes/web.php (1 hunks)
  • app/Views/admin/plugins.php (1 hunks)
  • app/Views/libri/partials/book_form.php (2 hunks)
  • app/Views/libri/scheda_libro.php (2 hunks)
  • installer/classes/Installer.php (2 hunks)
  • locale/en_US.json (3 hunks)
  • public/assets/418.bundle.js (1 hunks)
  • public/assets/main.bundle.js (1 hunks)
  • public/assets/main.css (26 hunks)
  • version.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/Routes/web.php (1)
app/Controllers/DeweyApiController.php (1)
  • DeweyApiController (10-366)
app/Controllers/DeweyApiController.php (2)
installer/index.php (1)
  • __ (20-44)
app/Controllers/GeneriApiController.php (1)
  • search (11-56)
app/Views/libri/scheda_libro.php (1)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
app/Views/admin/plugins.php (1)
installer/index.php (1)
  • __ (20-44)
app/Views/libri/partials/book_form.php (1)
installer/index.php (1)
  • __ (20-44)
🪛 Biome (2.1.2)
public/assets/418.bundle.js

[error] 2-2: Unexpected control character in a regular expression.

Control characters are unusual and potentially incorrect inputs, so they are disallowed.

(lint/suspicious/noControlCharactersInRegex)


[error] 2-2: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)

🪛 PHPMD (2.15.0)
app/Controllers/DeweyApiController.php

64-64: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (18)
version.json (1)

3-3: LGTM!

Il bump di versione a 0.2.1 è appropriato per le nuove funzionalità introdotte (plugin Dewey Editor, nuovi endpoint API, miglioramenti UI).

.gitignore (3)

15-15: LGTM!

L'aggiunta della directory .claude/ agli ignore è appropriata per escludere artefatti AI locali dal version control.


219-221: LGTM!

Pattern di ignore corretti per artefatti di build:

  • frontend/storage/ per file di storage temporanei del frontend
  • *.LICENSE.txt per i file di licenza estratti da webpack

Questi file sono generati automaticamente e non devono essere versionati.


346-364: LGTM!

L'esclusione dei file di documentazione di riferimento in docs/reference/ è appropriata per la distribuzione. I pattern sono specifici e non escludono documentazione utente necessaria.

installer/classes/Installer.php (1)

1175-1176: LGTM!

Il commento è stato aggiornato correttamente per riflettere l'aggiunta del plugin dewey-editor alla lista dei plugin predefiniti.

public/assets/main.bundle.js (1)

1-1: LGTM!

Implementazione pulita e robusta del toggle dark mode:

  • Gestione corretta dello stato con localStorage
  • Protezione con try/catch per gestire eccezioni (es. modalità privata del browser)
  • Esposizione corretta dell'API globale window.toggleDarkMode
  • Inizializzazione al DOMContentLoaded per garantire che il DOM sia pronto
app/Views/admin/plugins.php (1)

267-273: Azione “Apri Editor” integrata correttamente

La logica condizionale su name === 'dewey-editor' e is_active è allineata alle altre azioni plugin, e il semplice link a /admin/dewey-editor è adeguato dato che la route è già protetta lato server. Nessun problema da segnalare.

app/Routes/web.php (1)

1450-1452: Nuovi endpoint Dewey coerenti con lo schema esistente

/api/dewey/children e /api/dewey/search sono agganciati al DeweyApiController e protetti da AdminAuthMiddleware, in linea con gli altri endpoint di gestione Dewey. La posizione nel file (subito dopo categories/divisions/specifics) è corretta e non vedo criticità.

app/Controllers/DeweyApiController.php (3)

64-64: Parametro $request non utilizzato - falso positivo.

Il parametro $request fa parte della signature standard PSR-7 per i controller. L'hint dello static analyzer è un falso positivo in questo contesto, dato che la signature è necessaria per la compatibilità con il framework di routing.


211-265: Nuovi endpoint API ben strutturati.

I nuovi endpoint getChildren() e search() sono implementati correttamente con:

  • Gestione appropriata degli errori con logging interno
  • Risposte JSON consistenti con codici HTTP appropriati
  • Supporto per la navigazione gerarchica infinita

L'implementazione ricorsiva di findNodeByCode() è adeguata per la profondità tipica della classificazione Dewey (4-5 livelli).

Also applies to: 286-329


58-62: Potenziale errore di offset indefinito su array vuoto.

Il metodo isNewFormat() accede a $data[0] senza verificare prima che l'array non sia vuoto. Se $data è un array vuoto, questo causerà un warning/errore PHP.

 private function isNewFormat(array $data): bool
 {
-    // Il nuovo formato è un array diretto, non un oggetto con chiave
-    return isset($data[0]['code']) && isset($data[0]['level']) && isset($data[0]['children']);
+    // Il nuovo formato è un array diretto, non un oggetto con chiave
+    return !empty($data) && isset($data[0]['code']) && isset($data[0]['level']) && isset($data[0]['children']);
 }

Likely an incorrect or invalid review comment.

public/assets/main.css (2)

772-788: Override dark theme per SweetAlert2 correttamente implementato.

Il blocco @media (prefers-color-scheme: dark) per [data-swal2-theme=auto] replica correttamente le variabili del tema scuro, permettendo a SweetAlert2 di adattarsi automaticamente alle preferenze di sistema dell'utente.


3247-3249: Nuove utility classes Tailwind generate correttamente.

Le nuove classi utility (z-10, h-[46px], min-h-[400px], text-start, bg-indigo-200, bg-indigo-50, bg-purple-600, text-purple-700, ring-red-200, ecc.) sono output standard di Tailwind CSS e supportano i nuovi componenti UI del Dewey Editor e del form libro menzionati negli obiettivi del PR.

Also applies to: 3531-3533, 3558-3560, 4252-4259, 4576-4578, 5007-5010

app/Views/libri/partials/book_form.php (4)

246-254: Struttura del chip Dewey corretta.

L'implementazione del chip container con gli elementi per codice e nome è ben strutturata. Il pulsante di rimozione ha sia icona che funzionalità accessibile.


269-283: Buon uso del disclosure progressivo per la navigazione.

L'elemento <details> nasconde la navigazione per categorie di default, mantenendo l'interfaccia pulita mentre offre funzionalità avanzate quando necessario.


1225-1264: La funzione getCodePath assume codici Dewey a 3 cifre.

La logica di costruzione del percorso gerarchico usa substring(0, 1) e substring(0, 2) assumendo che il codice abbia almeno 3 caratteri. Se la validazione viene mantenuta permissiva (1-3 cifre), codici come "4" o "59" produrranno percorsi errati.

Verifica che la validazione del codice in input (linee 1036-1038) venga aggiornata per richiedere 3 cifre, oppure aggiungi normalizzazione/padding degli zeri in questa funzione:

// Esempio di normalizzazione
const normalizeCode = (code) => {
  const [intPart, decPart] = code.split('.');
  const paddedInt = intPart.padStart(3, '0');
  return decPart ? `${paddedInt}.${decPart}` : paddedInt;
};

1307-1316: Buona gestione della retrocompatibilità per il formato legacy.

Il codice gestisce correttamente la migrazione dal vecchio formato (es. "300-340-347") estraendo l'ultimo valore come codice finale. Questo assicura che i libri esistenti con classificazioni nel vecchio formato vengano visualizzati correttamente.

locale/en_US.json (1)

192-192: Ottima copertura di traduzioni per il Dewey Editor e miglioramenti al form del libro.

Le nuove stringhe di traduzione aggiunta sono ben strutturate e comprehensive:

  • Gestione Editor Dewey (riga 1097): Descrizione chiara per l'accesso al nuovo plugin
  • Interfaccia Manuale Dewey (righe 3275-3281): Include istruzioni complete, esempi di formato, e guida all'utente
  • Componenti UI (righe 3282-3295): Etichette, messaggi di stato, messaggi di errore, e testo dei pulsanti ben localizzati
  • Messaggi API (riga 3290): Appropriati per il contesto tecnico

Le traduzioni sono naturali, coerenti con la terminologia esistente dell'applicazione, e seguono le convenzioni di stile già presenti nel file (ad es., messaggi di errore con pattern "Errore nel recupero...", esempi con "es. ...").

Also applies to: 1097-1097, 3274-3296

Comment on lines +1289 to +1291
$installPlugin('dewey-editor', [
['name' => 'app.routes.register', 'callback_method' => 'registerRoutes', 'priority' => 10]
]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# First, let's check if the plugin directory exists
echo "=== Checking dewey-editor plugin structure ==="

if [ -d "storage/plugins/dewey-editor" ]; then
    echo "✓ Plugin directory exists"
    
    # Check for plugin.json
    if [ -f "storage/plugins/dewey-editor/plugin.json" ]; then
        echo "✓ plugin.json exists"
        echo "Content:"
        cat storage/plugins/dewey-editor/plugin.json
    else
        echo "✗ plugin.json NOT found"
    fi
    
    # List plugin files
    echo ""
    echo "=== Plugin files ==="
    find storage/plugins/dewey-editor -type f 2>/dev/null
else
    echo "✗ Plugin directory NOT found at storage/plugins/dewey-editor"
fi

echo ""
echo "=== Checking installer/classes/Installer.php ==="
if [ -f "installer/classes/Installer.php" ]; then
    echo "✓ Installer.php exists"
    echo "Lines 1280-1300:"
    sed -n '1280,1300p' installer/classes/Installer.php
else
    echo "✗ Installer.php NOT found"
fi

Repository: fabiodalez-dev/Pinakes

Length of output: 1171


🏁 Script executed:

#!/bin/bash

# Check what plugin directories exist
echo "=== Existing plugin directories ==="
if [ -d "storage/plugins" ]; then
    ls -la storage/plugins/
else
    echo "storage/plugins directory does not exist"
fi

echo ""
echo "=== Search for dewey-editor references in codebase ==="
rg "dewey-editor" --type php --type json -n

echo ""
echo "=== Check if this is a git diff (new plugin being added) ==="
git status | head -20

Repository: fabiodalez-dev/Pinakes

Length of output: 1218


🏁 Script executed:

#!/bin/bash

# Check the git history to see if dewey-editor was recently added or removed
echo "=== Recent git changes related to dewey-editor ==="
git log --oneline --all -20

echo ""
echo "=== Check if dewey-editor is in any other branches ==="
git branch -r | head -10

Repository: fabiodalez-dev/Pinakes

Length of output: 289


The dewey-editor plugin directory is missing from the repository.

While the installation code at lines 1289-1291 follows the correct pattern consistent with other plugins (digital-library, z39-server), the plugin itself does not exist at storage/plugins/dewey-editor. The plugin directory must be added to the repository before this installer code can function correctly. Additionally, the plugin is referenced in app/Views/admin/plugins.php (lines 267-268), indicating it should be a working plugin.

Ensure the storage/plugins/dewey-editor/ directory and its files are included in the repository before approving this change.

🤖 Prompt for AI Agents
In installer/classes/Installer.php around lines 1289-1291, the installer
registers a 'dewey-editor' plugin but the plugin directory
storage/plugins/dewey-editor is missing from the repository; create and add the
storage/plugins/dewey-editor/ directory with the plugin's expected files (entry
PHP file/class, registerRoutes method, config/manifest if used, and any
views/assets referenced by app/Views/admin/plugins.php), ensure the plugin
follows the same structure and naming conventions as digital-library and
z39-server, commit the new files to the repo (git add/commit/push) so the
installer and the admin view reference resolve correctly.

Comment on lines 3282 to 3293
"Nessuna selezione": "No selection",
"Classificazione selezionata:": "Selected classification:",
"Seleziona...": "Select...",
"Codice Dewey trovato e impostato": "Dewey code found and set",
"Codice Dewey non trovato": "Dewey code not found",
"Errore nella ricerca": "Search error",
"Errore nel recupero dei figli": "Error retrieving children",
"Errore nel recupero delle categorie.": "Error retrieving categories.",
"Usa gli endpoint specifici per il formato legacy": "Use specific endpoints for legacy format",
"Parametro code obbligatorio.": "Code parameter required.",
"Cerca": "Search",
"Aggiungi": "Add",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Rimuovere chiavi di traduzione duplicate nel blocco finale.

Sono state identificate 4 chiavi duplicate nelle nuove aggiunte che già esistono nel file e avrebbero lo stesso valore di traduzione:

  • Riga 3282: "Nessuna selezione" (duplica riga 1710)
  • Riga 3284: "Seleziona..." (duplica riga 2435)
  • Riga 3292: "Cerca" (duplica riga 342)
  • Riga 3293: "Aggiungi" (duplica riga 114)

In JSON, le chiavi duplicate causano confusione e solo l'ultima definizione viene utilizzata. Si consiglia di rimuovere queste 4 righe duplicate e riutilizzare le chiavi esistenti nel codice.

Diff suggerito per rimuovere i duplicati:

-  "Nessuna selezione": "No selection",
-  "Classificazione selezionata:": "Selected classification:",
-  "Seleziona...": "Select...",
   "Codice Dewey trovato e impostato": "Dewey code found and set",
   "Codice Dewey non trovato": "Dewey code not found",
   "Errore nella ricerca": "Search error",
   "Errore nel recupero dei figli": "Error retrieving children",
   "Errore nel recupero delle categorie.": "Error retrieving categories.",
   "Usa gli endpoint specifici per il formato legacy": "Use specific endpoints for legacy format",
   "Parametro code obbligatorio.": "Code parameter required.",
-  "Cerca": "Search",
-  "Aggiungi": "Add",
   "Codice Dewey": "Dewey Code",
   "Inserisci un codice Dewey": "Enter a Dewey code",
   "Formato codice non valido. Usa formato: 599 oppure 599.9 oppure 599.93": "Invalid code format. Use format: 599 or 599.9 or 599.93"

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In locale/en_US.json around lines 3282 to 3293, the block contains four
translation keys that duplicate existing keys elsewhere in the file ("Nessuna
selezione" at 3282, "Seleziona..." at 3284, "Cerca" at 3292, "Aggiungi" at
3293); remove these four duplicate entries from this block so the file relies on
the original definitions (ensure JSON remains valid after deletion, i.e., remove
any trailing commas left by the removals).

Comment on lines +1 to +2
/*! For license information please see 418.bundle.js.LICENSE.txt */
"use strict";(self.webpackChunkpinakes_frontend=self.webpackChunkpinakes_frontend||[]).push([[418],{9418:(e,t,n)=>{n.r(t),n.d(t,{default:()=>ne});const{entries:o,setPrototypeOf:r,isFrozen:i,getPrototypeOf:a,getOwnPropertyDescriptor:l}=Object;let{freeze:c,seal:s,create:u}=Object,{apply:m,construct:p}="undefined"!=typeof Reflect&&Reflect;c||(c=function(e){return e}),s||(s=function(e){return e}),m||(m=function(e,t){for(var n=arguments.length,o=new Array(n>2?n-2:0),r=2;r<n;r++)o[r-2]=arguments[r];return e.apply(t,o)}),p||(p=function(e){for(var t=arguments.length,n=new Array(t>1?t-1:0),o=1;o<t;o++)n[o-1]=arguments[o];return new e(...n)});const f=D(Array.prototype.forEach),d=D(Array.prototype.lastIndexOf),h=D(Array.prototype.pop),g=D(Array.prototype.push),T=D(Array.prototype.splice),y=D(String.prototype.toLowerCase),E=D(String.prototype.toString),A=D(String.prototype.match),_=D(String.prototype.replace),b=D(String.prototype.indexOf),S=D(String.prototype.trim),N=D(Object.prototype.hasOwnProperty),w=D(RegExp.prototype.test),R=(C=TypeError,function(){for(var e=arguments.length,t=new Array(e),n=0;n<e;n++)t[n]=arguments[n];return p(C,t)});var C;function D(e){return function(t){t instanceof RegExp&&(t.lastIndex=0);for(var n=arguments.length,o=new Array(n>1?n-1:0),r=1;r<n;r++)o[r-1]=arguments[r];return m(e,t,o)}}function k(e,t){let n=arguments.length>2&&void 0!==arguments[2]?arguments[2]:y;r&&r(e,null);let o=t.length;for(;o--;){let r=t[o];if("string"==typeof r){const e=n(r);e!==r&&(i(t)||(t[o]=e),r=e)}e[r]=!0}return e}function v(e){for(let t=0;t<e.length;t++)N(e,t)||(e[t]=null);return e}function O(e){const t=u(null);for(const[n,r]of o(e))N(e,n)&&(Array.isArray(r)?t[n]=v(r):r&&"object"==typeof r&&r.constructor===Object?t[n]=O(r):t[n]=r);return t}function L(e,t){for(;null!==e;){const n=l(e,t);if(n){if(n.get)return D(n.get);if("function"==typeof n.value)return D(n.value)}e=a(e)}return function(){return null}}const x=c(["a","abbr","acronym","address","area","article","aside","audio","b","bdi","bdo","big","blink","blockquote","body","br","button","canvas","caption","center","cite","code","col","colgroup","content","data","datalist","dd","decorator","del","details","dfn","dialog","dir","div","dl","dt","element","em","fieldset","figcaption","figure","font","footer","form","h1","h2","h3","h4","h5","h6","head","header","hgroup","hr","html","i","img","input","ins","kbd","label","legend","li","main","map","mark","marquee","menu","menuitem","meter","nav","nobr","ol","optgroup","option","output","p","picture","pre","progress","q","rp","rt","ruby","s","samp","search","section","select","shadow","slot","small","source","spacer","span","strike","strong","style","sub","summary","sup","table","tbody","td","template","textarea","tfoot","th","thead","time","tr","track","tt","u","ul","var","video","wbr"]),I=c(["svg","a","altglyph","altglyphdef","altglyphitem","animatecolor","animatemotion","animatetransform","circle","clippath","defs","desc","ellipse","enterkeyhint","exportparts","filter","font","g","glyph","glyphref","hkern","image","inputmode","line","lineargradient","marker","mask","metadata","mpath","part","path","pattern","polygon","polyline","radialgradient","rect","stop","style","switch","symbol","text","textpath","title","tref","tspan","view","vkern"]),M=c(["feBlend","feColorMatrix","feComponentTransfer","feComposite","feConvolveMatrix","feDiffuseLighting","feDisplacementMap","feDistantLight","feDropShadow","feFlood","feFuncA","feFuncB","feFuncG","feFuncR","feGaussianBlur","feImage","feMerge","feMergeNode","feMorphology","feOffset","fePointLight","feSpecularLighting","feSpotLight","feTile","feTurbulence"]),U=c(["animate","color-profile","cursor","discard","font-face","font-face-format","font-face-name","font-face-src","font-face-uri","foreignobject","hatch","hatchpath","mesh","meshgradient","meshpatch","meshrow","missing-glyph","script","set","solidcolor","unknown","use"]),z=c(["math","menclose","merror","mfenced","mfrac","mglyph","mi","mlabeledtr","mmultiscripts","mn","mo","mover","mpadded","mphantom","mroot","mrow","ms","mspace","msqrt","mstyle","msub","msup","msubsup","mtable","mtd","mtext","mtr","munder","munderover","mprescripts"]),P=c(["maction","maligngroup","malignmark","mlongdiv","mscarries","mscarry","msgroup","mstack","msline","msrow","semantics","annotation","annotation-xml","mprescripts","none"]),F=c(["#text"]),H=c(["accept","action","align","alt","autocapitalize","autocomplete","autopictureinpicture","autoplay","background","bgcolor","border","capture","cellpadding","cellspacing","checked","cite","class","clear","color","cols","colspan","controls","controlslist","coords","crossorigin","datetime","decoding","default","dir","disabled","disablepictureinpicture","disableremoteplayback","download","draggable","enctype","enterkeyhint","exportparts","face","for","headers","height","hidden","high","href","hreflang","id","inert","inputmode","integrity","ismap","kind","label","lang","list","loading","loop","low","max","maxlength","media","method","min","minlength","multiple","muted","name","nonce","noshade","novalidate","nowrap","open","optimum","part","pattern","placeholder","playsinline","popover","popovertarget","popovertargetaction","poster","preload","pubdate","radiogroup","readonly","rel","required","rev","reversed","role","rows","rowspan","spellcheck","scope","selected","shape","size","sizes","slot","span","srclang","start","src","srcset","step","style","summary","tabindex","title","translate","type","usemap","valign","value","width","wrap","xmlns","slot"]),B=c(["accent-height","accumulate","additive","alignment-baseline","amplitude","ascent","attributename","attributetype","azimuth","basefrequency","baseline-shift","begin","bias","by","class","clip","clippathunits","clip-path","clip-rule","color","color-interpolation","color-interpolation-filters","color-profile","color-rendering","cx","cy","d","dx","dy","diffuseconstant","direction","display","divisor","dur","edgemode","elevation","end","exponent","fill","fill-opacity","fill-rule","filter","filterunits","flood-color","flood-opacity","font-family","font-size","font-size-adjust","font-stretch","font-style","font-variant","font-weight","fx","fy","g1","g2","glyph-name","glyphref","gradientunits","gradienttransform","height","href","id","image-rendering","in","in2","intercept","k","k1","k2","k3","k4","kerning","keypoints","keysplines","keytimes","lang","lengthadjust","letter-spacing","kernelmatrix","kernelunitlength","lighting-color","local","marker-end","marker-mid","marker-start","markerheight","markerunits","markerwidth","maskcontentunits","maskunits","max","mask","mask-type","media","method","mode","min","name","numoctaves","offset","operator","opacity","order","orient","orientation","origin","overflow","paint-order","path","pathlength","patterncontentunits","patterntransform","patternunits","points","preservealpha","preserveaspectratio","primitiveunits","r","rx","ry","radius","refx","refy","repeatcount","repeatdur","restart","result","rotate","scale","seed","shape-rendering","slope","specularconstant","specularexponent","spreadmethod","startoffset","stddeviation","stitchtiles","stop-color","stop-opacity","stroke-dasharray","stroke-dashoffset","stroke-linecap","stroke-linejoin","stroke-miterlimit","stroke-opacity","stroke","stroke-width","style","surfacescale","systemlanguage","tabindex","tablevalues","targetx","targety","transform","transform-origin","text-anchor","text-decoration","text-rendering","textlength","type","u1","u2","unicode","values","viewbox","visibility","version","vert-adv-y","vert-origin-x","vert-origin-y","width","word-spacing","wrap","writing-mode","xchannelselector","ychannelselector","x","x1","x2","xmlns","y","y1","y2","z","zoomandpan"]),G=c(["accent","accentunder","align","bevelled","close","columnsalign","columnlines","columnspan","denomalign","depth","dir","display","displaystyle","encoding","fence","frame","height","href","id","largeop","length","linethickness","lspace","lquote","mathbackground","mathcolor","mathsize","mathvariant","maxsize","minsize","movablelimits","notation","numalign","open","rowalign","rowlines","rowspacing","rowspan","rspace","rquote","scriptlevel","scriptminsize","scriptsizemultiplier","selection","separator","separators","stretchy","subscriptshift","supscriptshift","symmetric","voffset","width","xmlns"]),W=c(["xlink:href","xml:id","xlink:title","xml:space","xmlns:xlink"]),Y=s(/\{\{[\w\W]*|[\w\W]*\}\}/gm),j=s(/<%[\w\W]*|[\w\W]*%>/gm),X=s(/\$\{[\w\W]*/gm),q=s(/^data-[\-\w.\u00B7-\uFFFF]+$/),$=s(/^aria-[\-\w]+$/),K=s(/^(?:(?:(?:f|ht)tps?|mailto|tel|callto|sms|cid|xmpp|matrix):|[^a-z]|[a-z+.\-]+(?:[^a-z+.\-:]|$))/i),V=s(/^(?:\w+script|data):/i),Z=s(/[\u0000-\u0020\u00A0\u1680\u180E\u2000-\u2029\u205F\u3000]/g),J=s(/^html$/i),Q=s(/^[a-z][.\w]*(-[.\w]+)+$/i);var ee=Object.freeze({__proto__:null,ARIA_ATTR:$,ATTR_WHITESPACE:Z,CUSTOM_ELEMENT:Q,DATA_ATTR:q,DOCTYPE_NAME:J,ERB_EXPR:j,IS_ALLOWED_URI:K,IS_SCRIPT_OR_DATA:V,MUSTACHE_EXPR:Y,TMPLIT_EXPR:X});const te=function(){return"undefined"==typeof window?null:window};var ne=function e(){let t=arguments.length>0&&void 0!==arguments[0]?arguments[0]:te();const n=t=>e(t);if(n.version="3.3.0",n.removed=[],!t||!t.document||9!==t.document.nodeType||!t.Element)return n.isSupported=!1,n;let{document:r}=t;const i=r,a=i.currentScript,{DocumentFragment:l,HTMLTemplateElement:s,Node:m,Element:p,NodeFilter:C,NamedNodeMap:D=t.NamedNodeMap||t.MozNamedAttrMap,HTMLFormElement:v,DOMParser:Y,trustedTypes:j}=t,X=p.prototype,q=L(X,"cloneNode"),$=L(X,"remove"),V=L(X,"nextSibling"),Z=L(X,"childNodes"),Q=L(X,"parentNode");if("function"==typeof s){const e=r.createElement("template");e.content&&e.content.ownerDocument&&(r=e.content.ownerDocument)}let ne,oe="";const{implementation:re,createNodeIterator:ie,createDocumentFragment:ae,getElementsByTagName:le}=r,{importNode:ce}=i;let se={afterSanitizeAttributes:[],afterSanitizeElements:[],afterSanitizeShadowDOM:[],beforeSanitizeAttributes:[],beforeSanitizeElements:[],beforeSanitizeShadowDOM:[],uponSanitizeAttribute:[],uponSanitizeElement:[],uponSanitizeShadowNode:[]};n.isSupported="function"==typeof o&&"function"==typeof Q&&re&&void 0!==re.createHTMLDocument;const{MUSTACHE_EXPR:ue,ERB_EXPR:me,TMPLIT_EXPR:pe,DATA_ATTR:fe,ARIA_ATTR:de,IS_SCRIPT_OR_DATA:he,ATTR_WHITESPACE:ge,CUSTOM_ELEMENT:Te}=ee;let{IS_ALLOWED_URI:ye}=ee,Ee=null;const Ae=k({},[...x,...I,...M,...z,...F]);let _e=null;const be=k({},[...H,...B,...G,...W]);let Se=Object.seal(u(null,{tagNameCheck:{writable:!0,configurable:!1,enumerable:!0,value:null},attributeNameCheck:{writable:!0,configurable:!1,enumerable:!0,value:null},allowCustomizedBuiltInElements:{writable:!0,configurable:!1,enumerable:!0,value:!1}})),Ne=null,we=null;const Re=Object.seal(u(null,{tagCheck:{writable:!0,configurable:!1,enumerable:!0,value:null},attributeCheck:{writable:!0,configurable:!1,enumerable:!0,value:null}}));let Ce=!0,De=!0,ke=!1,ve=!0,Oe=!1,Le=!0,xe=!1,Ie=!1,Me=!1,Ue=!1,ze=!1,Pe=!1,Fe=!0,He=!1,Be=!0,Ge=!1,We={},Ye=null;const je=k({},["annotation-xml","audio","colgroup","desc","foreignobject","head","iframe","math","mi","mn","mo","ms","mtext","noembed","noframes","noscript","plaintext","script","style","svg","template","thead","title","video","xmp"]);let Xe=null;const qe=k({},["audio","video","img","source","image","track"]);let $e=null;const Ke=k({},["alt","class","for","id","label","name","pattern","placeholder","role","summary","title","value","style","xmlns"]),Ve="http://www.w3.org/1998/Math/MathML",Ze="http://www.w3.org/2000/svg",Je="http://www.w3.org/1999/xhtml";let Qe=Je,et=!1,tt=null;const nt=k({},[Ve,Ze,Je],E);let ot=k({},["mi","mo","mn","ms","mtext"]),rt=k({},["annotation-xml"]);const it=k({},["title","style","font","a","script"]);let at=null;const lt=["application/xhtml+xml","text/html"];let ct=null,st=null;const ut=r.createElement("form"),mt=function(e){return e instanceof RegExp||e instanceof Function},pt=function(){let e=arguments.length>0&&void 0!==arguments[0]?arguments[0]:{};if(!st||st!==e){if(e&&"object"==typeof e||(e={}),e=O(e),at=-1===lt.indexOf(e.PARSER_MEDIA_TYPE)?"text/html":e.PARSER_MEDIA_TYPE,ct="application/xhtml+xml"===at?E:y,Ee=N(e,"ALLOWED_TAGS")?k({},e.ALLOWED_TAGS,ct):Ae,_e=N(e,"ALLOWED_ATTR")?k({},e.ALLOWED_ATTR,ct):be,tt=N(e,"ALLOWED_NAMESPACES")?k({},e.ALLOWED_NAMESPACES,E):nt,$e=N(e,"ADD_URI_SAFE_ATTR")?k(O(Ke),e.ADD_URI_SAFE_ATTR,ct):Ke,Xe=N(e,"ADD_DATA_URI_TAGS")?k(O(qe),e.ADD_DATA_URI_TAGS,ct):qe,Ye=N(e,"FORBID_CONTENTS")?k({},e.FORBID_CONTENTS,ct):je,Ne=N(e,"FORBID_TAGS")?k({},e.FORBID_TAGS,ct):O({}),we=N(e,"FORBID_ATTR")?k({},e.FORBID_ATTR,ct):O({}),We=!!N(e,"USE_PROFILES")&&e.USE_PROFILES,Ce=!1!==e.ALLOW_ARIA_ATTR,De=!1!==e.ALLOW_DATA_ATTR,ke=e.ALLOW_UNKNOWN_PROTOCOLS||!1,ve=!1!==e.ALLOW_SELF_CLOSE_IN_ATTR,Oe=e.SAFE_FOR_TEMPLATES||!1,Le=!1!==e.SAFE_FOR_XML,xe=e.WHOLE_DOCUMENT||!1,Ue=e.RETURN_DOM||!1,ze=e.RETURN_DOM_FRAGMENT||!1,Pe=e.RETURN_TRUSTED_TYPE||!1,Me=e.FORCE_BODY||!1,Fe=!1!==e.SANITIZE_DOM,He=e.SANITIZE_NAMED_PROPS||!1,Be=!1!==e.KEEP_CONTENT,Ge=e.IN_PLACE||!1,ye=e.ALLOWED_URI_REGEXP||K,Qe=e.NAMESPACE||Je,ot=e.MATHML_TEXT_INTEGRATION_POINTS||ot,rt=e.HTML_INTEGRATION_POINTS||rt,Se=e.CUSTOM_ELEMENT_HANDLING||{},e.CUSTOM_ELEMENT_HANDLING&&mt(e.CUSTOM_ELEMENT_HANDLING.tagNameCheck)&&(Se.tagNameCheck=e.CUSTOM_ELEMENT_HANDLING.tagNameCheck),e.CUSTOM_ELEMENT_HANDLING&&mt(e.CUSTOM_ELEMENT_HANDLING.attributeNameCheck)&&(Se.attributeNameCheck=e.CUSTOM_ELEMENT_HANDLING.attributeNameCheck),e.CUSTOM_ELEMENT_HANDLING&&"boolean"==typeof e.CUSTOM_ELEMENT_HANDLING.allowCustomizedBuiltInElements&&(Se.allowCustomizedBuiltInElements=e.CUSTOM_ELEMENT_HANDLING.allowCustomizedBuiltInElements),Oe&&(De=!1),ze&&(Ue=!0),We&&(Ee=k({},F),_e=[],!0===We.html&&(k(Ee,x),k(_e,H)),!0===We.svg&&(k(Ee,I),k(_e,B),k(_e,W)),!0===We.svgFilters&&(k(Ee,M),k(_e,B),k(_e,W)),!0===We.mathMl&&(k(Ee,z),k(_e,G),k(_e,W))),e.ADD_TAGS&&("function"==typeof e.ADD_TAGS?Re.tagCheck=e.ADD_TAGS:(Ee===Ae&&(Ee=O(Ee)),k(Ee,e.ADD_TAGS,ct))),e.ADD_ATTR&&("function"==typeof e.ADD_ATTR?Re.attributeCheck=e.ADD_ATTR:(_e===be&&(_e=O(_e)),k(_e,e.ADD_ATTR,ct))),e.ADD_URI_SAFE_ATTR&&k($e,e.ADD_URI_SAFE_ATTR,ct),e.FORBID_CONTENTS&&(Ye===je&&(Ye=O(Ye)),k(Ye,e.FORBID_CONTENTS,ct)),Be&&(Ee["#text"]=!0),xe&&k(Ee,["html","head","body"]),Ee.table&&(k(Ee,["tbody"]),delete Ne.tbody),e.TRUSTED_TYPES_POLICY){if("function"!=typeof e.TRUSTED_TYPES_POLICY.createHTML)throw R('TRUSTED_TYPES_POLICY configuration option must provide a "createHTML" hook.');if("function"!=typeof e.TRUSTED_TYPES_POLICY.createScriptURL)throw R('TRUSTED_TYPES_POLICY configuration option must provide a "createScriptURL" hook.');ne=e.TRUSTED_TYPES_POLICY,oe=ne.createHTML("")}else void 0===ne&&(ne=function(e,t){if("object"!=typeof e||"function"!=typeof e.createPolicy)return null;let n=null;const o="data-tt-policy-suffix";t&&t.hasAttribute(o)&&(n=t.getAttribute(o));const r="dompurify"+(n?"#"+n:"");try{return e.createPolicy(r,{createHTML:e=>e,createScriptURL:e=>e})}catch(e){return console.warn("TrustedTypes policy "+r+" could not be created."),null}}(j,a)),null!==ne&&"string"==typeof oe&&(oe=ne.createHTML(""));c&&c(e),st=e}},ft=k({},[...I,...M,...U]),dt=k({},[...z,...P]),ht=function(e){g(n.removed,{element:e});try{Q(e).removeChild(e)}catch(t){$(e)}},gt=function(e,t){try{g(n.removed,{attribute:t.getAttributeNode(e),from:t})}catch(e){g(n.removed,{attribute:null,from:t})}if(t.removeAttribute(e),"is"===e)if(Ue||ze)try{ht(t)}catch(e){}else try{t.setAttribute(e,"")}catch(e){}},Tt=function(e){let t=null,n=null;if(Me)e="<remove></remove>"+e;else{const t=A(e,/^[\r\n\t ]+/);n=t&&t[0]}"application/xhtml+xml"===at&&Qe===Je&&(e='<html xmlns="http://www.w3.org/1999/xhtml"><head></head><body>'+e+"</body></html>");const o=ne?ne.createHTML(e):e;if(Qe===Je)try{t=(new Y).parseFromString(o,at)}catch(e){}if(!t||!t.documentElement){t=re.createDocument(Qe,"template",null);try{t.documentElement.innerHTML=et?oe:o}catch(e){}}const i=t.body||t.documentElement;return e&&n&&i.insertBefore(r.createTextNode(n),i.childNodes[0]||null),Qe===Je?le.call(t,xe?"html":"body")[0]:xe?t.documentElement:i},yt=function(e){return ie.call(e.ownerDocument||e,e,C.SHOW_ELEMENT|C.SHOW_COMMENT|C.SHOW_TEXT|C.SHOW_PROCESSING_INSTRUCTION|C.SHOW_CDATA_SECTION,null)},Et=function(e){return e instanceof v&&("string"!=typeof e.nodeName||"string"!=typeof e.textContent||"function"!=typeof e.removeChild||!(e.attributes instanceof D)||"function"!=typeof e.removeAttribute||"function"!=typeof e.setAttribute||"string"!=typeof e.namespaceURI||"function"!=typeof e.insertBefore||"function"!=typeof e.hasChildNodes)},At=function(e){return"function"==typeof m&&e instanceof m};function _t(e,t,o){f(e,e=>{e.call(n,t,o,st)})}const bt=function(e){let t=null;if(_t(se.beforeSanitizeElements,e,null),Et(e))return ht(e),!0;const o=ct(e.nodeName);if(_t(se.uponSanitizeElement,e,{tagName:o,allowedTags:Ee}),Le&&e.hasChildNodes()&&!At(e.firstElementChild)&&w(/<[/\w!]/g,e.innerHTML)&&w(/<[/\w!]/g,e.textContent))return ht(e),!0;if(7===e.nodeType)return ht(e),!0;if(Le&&8===e.nodeType&&w(/<[/\w]/g,e.data))return ht(e),!0;if(!(Re.tagCheck instanceof Function&&Re.tagCheck(o))&&(!Ee[o]||Ne[o])){if(!Ne[o]&&Nt(o)){if(Se.tagNameCheck instanceof RegExp&&w(Se.tagNameCheck,o))return!1;if(Se.tagNameCheck instanceof Function&&Se.tagNameCheck(o))return!1}if(Be&&!Ye[o]){const t=Q(e)||e.parentNode,n=Z(e)||e.childNodes;if(n&&t)for(let o=n.length-1;o>=0;--o){const r=q(n[o],!0);r.__removalCount=(e.__removalCount||0)+1,t.insertBefore(r,V(e))}}return ht(e),!0}return e instanceof p&&!function(e){let t=Q(e);t&&t.tagName||(t={namespaceURI:Qe,tagName:"template"});const n=y(e.tagName),o=y(t.tagName);return!!tt[e.namespaceURI]&&(e.namespaceURI===Ze?t.namespaceURI===Je?"svg"===n:t.namespaceURI===Ve?"svg"===n&&("annotation-xml"===o||ot[o]):Boolean(ft[n]):e.namespaceURI===Ve?t.namespaceURI===Je?"math"===n:t.namespaceURI===Ze?"math"===n&&rt[o]:Boolean(dt[n]):e.namespaceURI===Je?!(t.namespaceURI===Ze&&!rt[o])&&!(t.namespaceURI===Ve&&!ot[o])&&!dt[n]&&(it[n]||!ft[n]):!("application/xhtml+xml"!==at||!tt[e.namespaceURI]))}(e)?(ht(e),!0):"noscript"!==o&&"noembed"!==o&&"noframes"!==o||!w(/<\/no(script|embed|frames)/i,e.innerHTML)?(Oe&&3===e.nodeType&&(t=e.textContent,f([ue,me,pe],e=>{t=_(t,e," ")}),e.textContent!==t&&(g(n.removed,{element:e.cloneNode()}),e.textContent=t)),_t(se.afterSanitizeElements,e,null),!1):(ht(e),!0)},St=function(e,t,n){if(Fe&&("id"===t||"name"===t)&&(n in r||n in ut))return!1;if(De&&!we[t]&&w(fe,t));else if(Ce&&w(de,t));else if(Re.attributeCheck instanceof Function&&Re.attributeCheck(t,e));else if(!_e[t]||we[t]){if(!(Nt(e)&&(Se.tagNameCheck instanceof RegExp&&w(Se.tagNameCheck,e)||Se.tagNameCheck instanceof Function&&Se.tagNameCheck(e))&&(Se.attributeNameCheck instanceof RegExp&&w(Se.attributeNameCheck,t)||Se.attributeNameCheck instanceof Function&&Se.attributeNameCheck(t,e))||"is"===t&&Se.allowCustomizedBuiltInElements&&(Se.tagNameCheck instanceof RegExp&&w(Se.tagNameCheck,n)||Se.tagNameCheck instanceof Function&&Se.tagNameCheck(n))))return!1}else if($e[t]);else if(w(ye,_(n,ge,"")));else if("src"!==t&&"xlink:href"!==t&&"href"!==t||"script"===e||0!==b(n,"data:")||!Xe[e])if(ke&&!w(he,_(n,ge,"")));else if(n)return!1;return!0},Nt=function(e){return"annotation-xml"!==e&&A(e,Te)},wt=function(e){_t(se.beforeSanitizeAttributes,e,null);const{attributes:t}=e;if(!t||Et(e))return;const o={attrName:"",attrValue:"",keepAttr:!0,allowedAttributes:_e,forceKeepAttr:void 0};let r=t.length;for(;r--;){const i=t[r],{name:a,namespaceURI:l,value:c}=i,s=ct(a),u=c;let m="value"===a?u:S(u);if(o.attrName=s,o.attrValue=m,o.keepAttr=!0,o.forceKeepAttr=void 0,_t(se.uponSanitizeAttribute,e,o),m=o.attrValue,!He||"id"!==s&&"name"!==s||(gt(a,e),m="user-content-"+m),Le&&w(/((--!?|])>)|<\/(style|title|textarea)/i,m)){gt(a,e);continue}if("attributename"===s&&A(m,"href")){gt(a,e);continue}if(o.forceKeepAttr)continue;if(!o.keepAttr){gt(a,e);continue}if(!ve&&w(/\/>/i,m)){gt(a,e);continue}Oe&&f([ue,me,pe],e=>{m=_(m,e," ")});const p=ct(e.nodeName);if(St(p,s,m)){if(ne&&"object"==typeof j&&"function"==typeof j.getAttributeType)if(l);else switch(j.getAttributeType(p,s)){case"TrustedHTML":m=ne.createHTML(m);break;case"TrustedScriptURL":m=ne.createScriptURL(m)}if(m!==u)try{l?e.setAttributeNS(l,a,m):e.setAttribute(a,m),Et(e)?ht(e):h(n.removed)}catch(t){gt(a,e)}}else gt(a,e)}_t(se.afterSanitizeAttributes,e,null)},Rt=function e(t){let n=null;const o=yt(t);for(_t(se.beforeSanitizeShadowDOM,t,null);n=o.nextNode();)_t(se.uponSanitizeShadowNode,n,null),bt(n),wt(n),n.content instanceof l&&e(n.content);_t(se.afterSanitizeShadowDOM,t,null)};return n.sanitize=function(e){let t=arguments.length>1&&void 0!==arguments[1]?arguments[1]:{},o=null,r=null,a=null,c=null;if(et=!e,et&&(e="\x3c!--\x3e"),"string"!=typeof e&&!At(e)){if("function"!=typeof e.toString)throw R("toString is not a function");if("string"!=typeof(e=e.toString()))throw R("dirty is not a string, aborting")}if(!n.isSupported)return e;if(Ie||pt(t),n.removed=[],"string"==typeof e&&(Ge=!1),Ge){if(e.nodeName){const t=ct(e.nodeName);if(!Ee[t]||Ne[t])throw R("root node is forbidden and cannot be sanitized in-place")}}else if(e instanceof m)o=Tt("\x3c!----\x3e"),r=o.ownerDocument.importNode(e,!0),1===r.nodeType&&"BODY"===r.nodeName||"HTML"===r.nodeName?o=r:o.appendChild(r);else{if(!Ue&&!Oe&&!xe&&-1===e.indexOf("<"))return ne&&Pe?ne.createHTML(e):e;if(o=Tt(e),!o)return Ue?null:Pe?oe:""}o&&Me&&ht(o.firstChild);const s=yt(Ge?e:o);for(;a=s.nextNode();)bt(a),wt(a),a.content instanceof l&&Rt(a.content);if(Ge)return e;if(Ue){if(ze)for(c=ae.call(o.ownerDocument);o.firstChild;)c.appendChild(o.firstChild);else c=o;return(_e.shadowroot||_e.shadowrootmode)&&(c=ce.call(i,c,!0)),c}let u=xe?o.outerHTML:o.innerHTML;return xe&&Ee["!doctype"]&&o.ownerDocument&&o.ownerDocument.doctype&&o.ownerDocument.doctype.name&&w(J,o.ownerDocument.doctype.name)&&(u="<!DOCTYPE "+o.ownerDocument.doctype.name+">\n"+u),Oe&&f([ue,me,pe],e=>{u=_(u,e," ")}),ne&&Pe?ne.createHTML(u):u},n.setConfig=function(){pt(arguments.length>0&&void 0!==arguments[0]?arguments[0]:{}),Ie=!0},n.clearConfig=function(){st=null,Ie=!1},n.isValidAttribute=function(e,t,n){st||pt({});const o=ct(e),r=ct(t);return St(o,r,n)},n.addHook=function(e,t){"function"==typeof t&&g(se[e],t)},n.removeHook=function(e,t){if(void 0!==t){const n=d(se[e],t);return-1===n?void 0:T(se[e],n,1)[0]}return h(se[e])},n.removeHooks=function(e){se[e]=[]},n.removeAllHooks=function(){se={afterSanitizeAttributes:[],afterSanitizeElements:[],afterSanitizeShadowDOM:[],beforeSanitizeAttributes:[],beforeSanitizeElements:[],beforeSanitizeShadowDOM:[],uponSanitizeAttribute:[],uponSanitizeElement:[],uponSanitizeShadowNode:[]}},n}()}}]); No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

DOMPurify latest version 2024 2025 current stable

💡 Result:

Latest stable DOMPurify (as of Dec 3, 2025) is v3.2.6. [1][2]

Sources:

  • DOMPurify Releases (GitHub) — shows v3.2.6 and related changelog. [1]
  • GitLab dependency updates referencing v3.2.6 (usage in projects). [2]

🌐 Web query:

DOMPurify v3.3.0 security vulnerabilities CVE

💡 Result:

Short answer: DOMPurify v3.3.0 is not affected by the known 2024 DOMPurify CVEs — the important CVEs were fixed in the 3.1.3 (or earlier 2.x) patch releases.

Details and sources:

  • CVE-2024-45801 (prototype-pollution / depth-check bypass) — fixed in 3.1.3. [1]
  • CVE-2024-47875 (nesting-based mXSS) — fixed in 3.1.3 (and 2.5.0 for 2.x). [2]
  • (Related prototype-pollution advisories such as CVE-2024-48910 were fixed in earlier 2.x releases.) [3]

Conclusion: v3.3.0 is newer than 3.1.3 and includes those fixes; upgrade to 3.3.0 (or at least ≥3.1.3) if you were on a vulnerable release. Sources: NVD/CVE entries and GitHub advisory.

Sources:
[1] NVD — CVE-2024-45801.
[2] NVD / GitHub advisory — CVE-2024-47875 / GHSA-gx9m-whjm-85jf.
[3] Debian/security tracker — prototype-pollution advisories (CVE-2024-48910).


Update to DOMPurify v3.2.6 — v3.3.0 is not the latest stable release.

The bundled code indicates DOMPurify v3.3.0, but the latest stable release as of December 2025 is v3.2.6. Verify that v3.3.0 is a legitimate release (pre-release, unreleased, or fork) and update to v3.2.6 if this is outdated. Both versions include fixes for CVE-2024-45801 and CVE-2024-47875.

The static analysis warnings about control characters and variable declarations in minified bundles are likely false positives, but the version discrepancy should be resolved.

🧰 Tools
🪛 Biome (2.1.2)

[error] 2-2: Unexpected control character in a regular expression.

Control characters are unusual and potentially incorrect inputs, so they are disallowed.

(lint/suspicious/noControlCharactersInRegex)


[error] 2-2: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)

- Add empty array check in isNewFormat() to prevent undefined index
- Return 404 when parent_code not found in getChildren()
- Handle legacy format explicitly in search() endpoint
- Replace alert() with Toast.fire() for consistent UX
- Add response.ok checks before parsing JSON in fetch calls
- Remove duplicate translation keys in en_US.json
- Add displayEl null check and response.ok in scheda_libro.php
- Add .gitignore rules to track dewey-editor plugin files
- Include DeweyEditorPlugin.php, DeweyValidator.php, plugin.json, editor.php
- Fixes installer reference to missing plugin directory
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (10)
app/Views/libri/partials/book_form.php (1)

1020-1331: initializeDewey: buona integrazione con le nuove API, migliorabile la gestione degli errori di caricamento

La logica di risoluzione codice→nome, path gerarchico e compatibilità col vecchio formato 300-340-347 è ben strutturata e coerente con le API /api/dewey/children e /api/dewey/search. Attualmente però, se fetch in loadLevel fallisce o restituisce HTTP non OK, l’errore viene solo loggato su console, senza feedback all’utente; suggerisco di usare il messaggio già tradotto per mostrare un Toast o alert non bloccante (es. i18nTranslations['Errore caricamento classificazione Dewey']) così l’utente capisce perché l’albero non si popola.

app/Views/libri/scheda_libro.php (2)

7-14: Badge di stato: resa visiva migliorata, da valutare copertura degli stati “secondari”

Le nuove classi per disponibile, prestato, in_ritardo, danneggiato, perso rendono i badge molto più leggibili; gli altri stati possibili (es. “Non Disponibile”, “Riservato”, “In Riparazione”, “Fuori Catalogo”, “Da Inventariare”) però cadranno tutti nel default grigio. Se vuoi mantenere una leggibilità uniforme, vale la pena aggiungere voci dedicate in $statusClasses anche per questi casi, allineandole alla palette usata più sotto nella tabella copie.


398-451: Risoluzione nome Dewey sul dettaglio: implementazione corretta, logica condivisa da centralizzare

Lo script che normalizza classificazione_dowey (gestendo il vecchio formato con -), interroga /api/dewey/search, e fa fallback sul parent via getParentCode è in linea con il comportamento del form e usa solo textContent, quindi sicuro rispetto a XSS. Dato che una logica molto simile esiste anche in initializeDewey() nel form, potresti in futuro estrarla in un helper JS condiviso per evitare divergenze tra pagina di dettaglio e form di modifica.

storage/plugins/dewey-editor/views/editor.php (1)

145-712: Editor Dewey: architettura client‑side chiara, utile riuso di helper per sicurezza e manutenibilità

La struttura generale (stato per locale, deep‑clone originalData, window.DeweyEditor per i callback inline, stats, filtro, gestione unsaved‑changes) è pulita e facile da seguire. Visto che hai già escapeHtml(), può valere la pena riutilizzarlo anche in altri punti dove inietti valori da backend in template literal HTML (es. showBackups() per b.date/b.filename), così mantieni una policy uniforme e riduci il rischio di regressioni XSS future se il backend inizia a riportare messaggi meno controllati.

storage/plugins/dewey-editor/classes/DeweyValidator.php (1)

56-101: Validatore Dewey completo; chiarire (o restringere) le regole gerarchiche e il contratto di canDelete

La combinazione di validate(), validateNode(), controllo di unicità e checkRequiredClasses() copre bene i casi base (formato codice, presenza di 000–900, nome minimo, livello 1–7). Due note di dettaglio:

  • isValidChild() per un parent intero (es. 500) accetta qualsiasi figlio con lo stesso primo digit e 3 cifre (510, 520, 590, ecc.), non solo i multipli di 10 o la “vera” gerarchia Dewey; se il dataset è sempre generato offline in modo corretto va bene, ma se vuoi che il validatore faccia da guardrail più forte, potresti rendere la regola più esplicita (es. permettere solo X10, X20, … come figli di X00).
  • Il commento di canDelete() parla di “level 4+”, ma l’implementazione si basa unicamente sul pattern decimale; dato che i dati reali sembrano coerenti (decimali usati solo sotto il livello 3), forse basta aggiornare il commento per evitare ambiguità, oppure, se il livello è affidabile, verificare anche $node['level'] >= 4 lato plugin quando decidi se esporre il pulsante di delete.

Sono aspetti non bloccanti, ma chiarirli renderà più prevedibile il comportamento del validatore per chi lo userà/estenderà.

Also applies to: 118-140, 200-246

storage/plugins/dewey-editor/DeweyEditorPlugin.php (5)

132-151: Migliorare la gestione degli errori di lettura in getData ed exportData

In getData ed exportData usi file_get_contents senza verificare l’esito. In caso di problemi di permessi o I/O:

  • getData finirà per fare json_decode(false, true)null, restituendo un generico errore di parsing (500), ma la causa reale è un errore di lettura.
  • exportData potrebbe restituire una risposta vuota come se il file fosse valido.

Non è bloccante, ma un controllo esplicito rende gli errori più chiari e robusti:

-        $content = file_get_contents($filePath);
+        $content = file_get_contents($filePath);
+        if ($content === false) {
+            return $this->jsonError($response, __('Errore nella lettura del file Dewey.'), 500);
+        }

e, in getData, poi decodificare $content come già fai.

Also applies to: 217-236


284-307: Gestione più sicura del risultato di glob() e cleanup dei backup

Sia in listBackups che in cleanupOldBackups usi:

$files = glob($pattern);
  • In listBackups, un eventuale false genererebbe un warning su foreach ($files as $file).
  • In cleanupOldBackups, un false può causare warning su count($files), usort($files, ...) e array_slice.

È raro che glob fallisca con un pattern semplice, ma aggiungere una normalizzazione è economico e rende il codice più robusto:

-        $files = glob($pattern);
+        $files = glob($pattern) ?: [];

sia in listBackups che in cleanupOldBackups.

Inoltre, in cleanupOldBackups:

usort($files, fn($a, $b) => filemtime($b) - filemtime($a));

va bene, ma, per evitare differenze troppo grandi e rendere l’intento più esplicito, potresti usare spaceship:

usort($files, fn($a, $b) => filemtime($b) <=> filemtime($a));

(non è obbligatorio, solo una piccola pulizia di leggibilità).

Also applies to: 367-384


348-365: Verifica dell’esito di copy in createBackup

In createBackup:

$backupPath = $this->backupDir . "/dewey_{$locale}_{$timestamp}.json";
copy($sourcePath, $backupPath);

Se copy fallisce (permessi, spazio disco, ecc.), il metodo prosegue comunque e chiama cleanupOldBackups, dando l’impressione che il backup sia riuscito.

Non è critico, ma potresti controllare il ritorno e loggare/ritornare in caso di errore, ad esempio:

-        copy($sourcePath, $backupPath);
+        if (!@copy($sourcePath, $backupPath)) {
+            // TODO: log dell'errore di backup se disponibile (logger/app)
+            return;
+        }

così eventuali problemi di backup non passano inosservati.


114-130: Parametri/variabili “non usati” segnalati da PHPMD

PHPMD segnala:

  • renderEditor: parametro $request e variabili $csrfToken, $content come non usati.
  • getData, exportData, listBackups: parametro $request non usato.

Nel contesto:

  • $csrfToken e $content vengono effettivamente usati dalle view incluse (editor.php e layout.php) tramite include/require, quindi gli avvisi sono falsi positivi.
  • I parametri $request fanno parte della firma richiesta dalle route PSR‑7, quindi non li puoi rimuovere.

Per tenere puliti i report dello static analysis puoi:

  • Aggiungere un commento di soppressione a livello di metodo (es. PHPMD @SuppressWarnings("UnusedFormalParameter") / @SuppressWarnings("UnusedLocalVariable")), oppure
  • Rinominare i parametri inutilizzati con un prefisso _ (es. Request $_request) se la toolchain lo supporta per ignorarli.

Non è un problema funzionale, ma aiuta a mantenere più “verde” la pipeline.

Also applies to: 132-132, 217-217, 284-284


49-56: Considerare la creazione della directory dati in onInstall

onInstall crea solo la directory dei backup, mentre i file Dewey vengono letti/scritti in:

$this->dataDir = dirname(__DIR__, 3) . '/data/dewey';

Se la directory /data/dewey non esiste (nuova installazione, deploy “vuoto”), gli endpoint che salvano o importano dati falliranno con errore di scrittura.

Potresti estendere onInstall per assicurarti che anche la directory dati esista:

    public function onInstall(): void
    {
-        // Create backup directory
-        if (!is_dir($this->backupDir)) {
-            mkdir($this->backupDir, 0755, true);
-        }
+        // Create backup directory
+        if (!is_dir($this->backupDir)) {
+            mkdir($this->backupDir, 0755, true);
+        }
+
+        // Create data directory if missing
+        if (!is_dir($this->dataDir)) {
+            mkdir($this->dataDir, 0755, true);
+        }
    }

Così il plugin è più “self‑contained” al primo install.

Also applies to: 342-346

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87ffd34 and 00f5315.

📒 Files selected for processing (9)
  • .gitignore (4 hunks)
  • app/Controllers/DeweyApiController.php (3 hunks)
  • app/Views/libri/partials/book_form.php (2 hunks)
  • app/Views/libri/scheda_libro.php (2 hunks)
  • locale/en_US.json (3 hunks)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
  • storage/plugins/dewey-editor/classes/DeweyValidator.php (1 hunks)
  • storage/plugins/dewey-editor/plugin.json (1 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • storage/plugins/dewey-editor/plugin.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • locale/en_US.json
🧰 Additional context used
🧬 Code graph analysis (5)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-247)
  • validate (29-47)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
app/Views/libri/scheda_libro.php (1)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
app/Views/libri/partials/book_form.php (1)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/classes/DeweyValidator.php (1)
installer/index.php (1)
  • __ (20-44)
🪛 PHPMD (2.15.0)
app/Controllers/DeweyApiController.php

64-64: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

storage/plugins/dewey-editor/DeweyEditorPlugin.php

114-114: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


116-116: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


121-121: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


132-132: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


217-217: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


284-284: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (6)
.gitignore (3)

118-126: Modello di plugin Dewey coerente con gli altri plugin.

Le voci per storage/plugins/dewey-editor/ seguono il modello stabilito per i plugin esistenti (open-library, z39-server, api-book-scraper, digital-library) e mantengono coerenza strutturale.


228-228: Verifica delle nuove voci di asset.

Le righe aggiungono frontend/storage/ e public/assets/*.LICENSE.txt agli ignores. Confermare che questi artefatti di build non contengono contenuto versionato necessario (ad esempio, dati di configurazione o librerie critiche).

Also applies to: 230-230


358-358: Remove the obsolete .gitignore entry for scripts/generate_dewey_json.py — the script does not exist.

The script listed on line 358 has never been committed to the repository and does not exist in the scripts/ directory, which contains only PHP and JavaScript files. This .gitignore entry is therefore unnecessary and should be removed. The documentation files ignored in lines 355-372 (18 reference files under docs/reference/) are reasonably excluded as non-essential reference materials.

Likely an incorrect or invalid review comment.

app/Views/libri/partials/book_form.php (1)

243-286: Sezione Dewey nel form: struttura solida e retro‑compatibile

La combinazione di hidden classificazione_dowey, chip selezionato, input manuale e navigazione a livelli copre bene sia l’inserimento libero sia la scelta guidata, mantenendo la stessa chiave usata dal backend. Nulla di bloccante da segnalare qui.

app/Controllers/DeweyApiController.php (1)

55-63: Supporto doppio formato e nuovi endpoint children/search ben strutturati

Il rilevamento del nuovo formato via isNewFormat() e i nuovi endpoint getChildren()/search() sono coerenti: gestiscono solo il formato flat, ritornano 400 per il legacy e mappano correttamente code, name, level, has_children, con errori 404/500 significativi. La ricorsione di findNodeByCode() è semplice e sfrutta la struttura ad albero già materializzata in memoria; per il volume (~2k voci) va benissimo.

Also applies to: 211-337

storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

20-112: Struttura generale del plugin ben pensata

Integrazione con HookManager, separazione chiara dei metodi lifecycle e registrazione delle route (con middleware admin + CSRF dove serve) risultano pulite e coerenti con l’uso in area admin. Nulla di bloccante qui.

… fixes

- Add merge import functionality: new /api/dewey-editor/merge/{locale} endpoint
  allows adding/updating entries by code without losing existing data
- Import dialog now offers choice between Merge and Replace modes
- Dynamic language support: uses I18n::getAvailableLocales() instead of
  hardcoded list, generates filenames dynamically (dewey_completo_{lang}.json)
- Fix DeweyValidator hierarchy: allow decimal codes under division codes
  (e.g., 491.7 under 490) for proper Dewey classification support
- Add safety checks in saveData() and restoreBackup() for null/invalid data
- Fix XSS vulnerability: use SweetAlert2 text: instead of html: for errors
- Add English translations for all new merge-related strings
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (2)
app/Controllers/DeweyApiController.php (1)

120-163: Gestione esplicita del nuovo formato nei vecchi endpoint legacy

L’aggiunta del guard rail:

  • in getDivisions() / getSpecifics() verifica isNewFormat($data)
  • in caso positivo risponde 400 con messaggio che invita a usare /api/dewey/children

risolve il problema segnalato nei review precedenti sugli accessi a indici inesistenti con il nuovo JSON e rende il deprecare degli endpoint legacy molto più chiaro lato client.

storage/plugins/dewey-editor/views/editor.php (1)

568-619: Import/merge: XSS corretta rispetto al review precedente

Il nuovo handleImport():

  • usa FormData + header CSRF per POST verso /import o /merge;
  • in caso di errore costruisce un array lines e lo passa a SweetAlert tramite text: lines.join('\n'), quindi i messaggi non vengono più interpretati come HTML.

Questo recepisce esattamente il fix suggerito nel review precedente e chiude la finestra XSS sull’import.

🧹 Nitpick comments (8)
storage/plugins/dewey-editor/classes/DeweyValidator.php (1)

119-153: Regole gerarchiche dei figli potenzialmente troppo permissive per i decimali di divisione

In isValidChild() la parte per i genitori interi che corrispondono a divisioni (/^([0-9])[0-9]0$/) considera valido qualsiasi codice decimale che inizi con la stessa prima cifra (es. 510 potrebbe accettare anche 599.9). Oggi questo non crea errori perché la struttura viene guidata dal JSON, ma per coerenza con Dewey potresti restringere il controllo al range della divisione (stesso prefisso a tre cifre) per i decimali. È un affinamento logico, non un bug blocccante.

locale/en_US.json (1)

192-200: Nuove chiavi Dewey: mapping corretto ma alcune label del nuovo editor non tradotte

"Apri Editor": "Open Editor" e "Errori di validazione dopo il merge.": "Validation errors after merge." sono coerenti con l’inglese dell’interfaccia. Tuttavia nel nuovo view (storage/plugins/dewey-editor/views/editor.php) restano alcune stringhe italiane senza entry dedicate qui ("Editor Classificazione Dewey", "Voci totali", "Classi principali", "Sezioni", "Decimali", "Cerca codice o nome...", "Nome della classificazione", "Formato: 3 cifre + punto + 1-4 cifre decimali", "Aggiungi decimale sotto" ecc.). Valuta di aggiungerle per evitare testo misto IT/EN quando il locale è en_US.

app/Controllers/DeweyApiController.php (3)

19-33: Strategia di caricamento Dewey multi–formato ben strutturata (nuovo + fallback legacy)

La logica di loadDeweyData() che prova prima dewey_completo_{en,it}.json, poi il vecchio dewey_en.json/dewey.json e infine fa fallback su dewey_completo_it.json è sensata e resiliente; il caching per locale riduce l’I/O. Nota solo che tutti i locale diversi da en_US useranno comunque il dataset italiano: se in futuro aggiungi altre lingue dovrai estendere il mapping del filename.


55-87: Rilevamento nuovo formato e getCategories compatibile con entrambi i layout dati

isNewFormat() è semplice e sufficiente per distinguere l’array piatto di nodi (code/level/children) dal vecchio oggetto con chiave classificazione_dewey / dewey_classification. In getCategories() l’estrazione dei nodi con level === 1 per il nuovo formato e il fallback sulla logica legacy (type/descrizione/codice) appare corretta. Valuta eventualmente un controllo difensivo su $data[$deweyKey][0] prima di accedervi, per gestire file corrotti senza notice PHP, ma non è un blocco.


229-287: Nuovo endpoint /api/dewey/children robusto per il nuovo formato, fallback corretto sul legacy

getChildren() implementa una navigazione generica del nuovo albero Dewey:

  • senza parent_code restituisce tutte le voci level === 1
  • con parent_code usa findNodeByCode() e restituisce i figli con has_children calcolato

Il fallback per il vecchio formato che risponde con errore e rimanda agli endpoint legacy è chiaro. Suggerisco solo, per futura estensione, di considerare una limitazione di profondità o un controllo input su parent_code (pattern Dewey) prima di passarlo alla ricerca, ma l’implementazione attuale è funzionalmente corretta.

storage/plugins/dewey-editor/views/editor.php (2)

364-498: Modali di edit/add: buona UX, ma controllo parent-child JS un po’ lasco rispetto al validator

Le modali di modifica/aggiunta:

  • validano localmente lunghezza del nome e formato del codice (^[0-9]{3}\.[0-9]{1,4}$), allineato al DeweyValidator;
  • verificano che il codice parta (grossolanamente) dal prefisso del genitore e l’unicità nel tree.

Il controllo di prefisso in saveAddHandler() è volutamente semplice (può permettere ad esempio di aggiungere un 599.92 sotto 599.93), ma lato server il DeweyValidator::isValidChild() bloccherà comunque strutture non valide. Se vuoi ridurre errori lato UI, potresti allineare la logica JS a quella del validator (usando il parent esatto come prefisso o delegando la convalida gerarchica esclusivamente al backend).


536-566: Salvataggio: considerare l’uso della proprietà text di SweetAlert per messaggi dinamici

In saveData() i messaggi result.message/result.error vengono passati come secondo argomento a Swal.fire(title, html, icon), che tratta il contenuto come HTML. Al momento i messaggi sembrano statici/controllati, ma per difesa preventiva (ed evitare future regressioni tipo quella già corretta su import) è preferibile lo stile:

Swal.fire({
  icon: 'success',
  title: '...',
  text: result.message
});

e analogo per gli errori. Questo evita che eventuali future stringhe con markup vengano interpretate come HTML.

storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

137-153: Variabile $content non utilizzata.

La variabile $content (linea 144) viene catturata ma mai usata, poiché viene sovrascritta da ob_get_clean() alla linea 149. Puoi rimuovere il primo blocco ob_start()/ob_get_clean() e includere direttamente la vista all'interno del layout.

Applica questa diff per semplificare:

 public function renderEditor(Request $request, Response $response): Response
 {
     $csrfToken = $_SESSION['csrf_token'] ?? '';

-    // Render the view content
-    ob_start();
-    include __DIR__ . '/views/editor.php';
-    $content = ob_get_clean();
-
     // Wrap with standard layout (includes sidebar, header, CSS/JS)
     ob_start();
+    $content = ''; // Set empty or include view directly in layout
     require dirname(__DIR__, 3) . '/app/Views/layout.php';
     $html = ob_get_clean();

     $response->getBody()->write($html);
     return $response->withHeader('Content-Type', 'text/html');
 }

Nota: Verifica che layout.php gestisca correttamente l'inclusione di editor.php, altrimenti mantieni la logica attuale.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00f5315 and d3e857a.

📒 Files selected for processing (6)
  • app/Controllers/DeweyApiController.php (5 hunks)
  • locale/en_US.json (7 hunks)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
  • storage/plugins/dewey-editor/classes/DeweyValidator.php (1 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
  • version.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • version.json
🧰 Additional context used
🧬 Code graph analysis (3)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/classes/DeweyValidator.php (1)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

137-137: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


139-139: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


144-144: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


155-155: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


247-247: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


448-448: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

app/Controllers/DeweyApiController.php

64-64: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (15)
storage/plugins/dewey-editor/classes/DeweyValidator.php (1)

29-47: Validazione complessiva robusta e ben strutturata

L’impostazione generale (validate(), reset di stato, ricorsione e controllo delle classi principali) è chiara e solida; buona scelta di regex forti per i codici e di tracciamento dei duplicati via seenCodes. Non vedo problemi funzionali evidenti in questa parte.

locale/en_US.json (2)

1591-1595: Stringhe per modalità di import/merge allineate al flusso UI

Le traduzioni per "Modalità di importazione", le due opzioni "Merge - Aggiungi e aggiorna (mantiene dati esistenti)", "Sostituisci - Sovrascrivi tutto" e i messaggi "Merge completato...", "Seleziona file", "Seleziona una modalità" risultano coerenti con quanto usato nel modal JavaScript e leggibili in inglese. Nulla da segnalare qui.

Also applies to: 2433-2436


3282-3304: Blocchi di testo per API Dewey/backup coerenti con il controller

Le nuove chiavi nel blocco finale (es. "Server SRU aggiuntivi per Copy Cataloging...", i messaggi Dewey "Codice Dewey trovato e impostato", "Codice Dewey non trovato", "Parametro code obbligatorio.", "Codice parent non trovato.", "Questo endpoint è supportato solo per il formato Dewey legacy. Usa /api/dewey/children.", "Errore nella lettura del backup.", "Formato dati non valido.") sono ben allineate alle risposte di DeweyApiController e del plugin, con un inglese chiaro. Buon lavoro nel mantenere coerenza terminologica.

app/Controllers/DeweyApiController.php (1)

292-355: Endpoint /api/dewey/search semplice e coerente con il resto delle API Dewey

search():

  • valida la presenza del parametro code con 400 + messaggio localizzato;
  • usa findNodeByCode() solo nel nuovo formato e rifiuta il legacy con errore esplicito;
  • restituisce un payload minimal (code, name, level, has_children) o 404 se non trovato;
  • ha gestione errori coerente con gli altri metodi (log interno + JSON generico).

La firma e il comportamento sono chiari e allineati agli altri endpoint.

storage/plugins/dewey-editor/views/editor.php (7)

59-127: Struttura UI editor Dewey chiara (header, tab per locale, stats e ricerca)

Il layout complessivo (header con azioni, tab dinamici basati su I18n::getAvailableLocales(), barra statistiche e campo di ricerca) è ben organizzato e usa escaping lato PHP (HtmlHelper::e, htmlspecialchars) dove serve. Nulla da segnalare in termini di correttezza qui.


159-257: Inizializzazione JS e gestione tab/unsaved changes

L’inizializzazione (DOMContentLoadedloadData + bindEvents) e la gestione dei tab con avviso sulle modifiche non salvate (hasChanges + beforeunload) sono implementate in modo pulito. Attenzione solo a garantire che $csrfToken sia sempre definito nel template che include questa view, altrimenti CSRF_TOKEN diventa null e le POST falliranno in modo non ovvio.

Suggerisco di verificare a livello di controller/layout che $csrfToken sia sempre inizializzato prima di includere questa view.


259-289: Caricamento dati per locale e gestione errori utente-friendly

loadData(locale) mostra uno spinner, chiama /api/dewey-editor/data/{locale}, copia profonda in originalData e resetta correttamente hasChanges/bottone Salva. Le condizioni su result.success e la UI di fallback in caso di errore di backend o di rete sono gestite in modo robusto.


291-362: Render dell’albero con escaping corretto e azioni per nodo

renderTree() e renderNode():

  • usano escapeHtml() sia per code sia per name, prevenendo XSS nel markup;
  • mostrano correttamente i pulsanti di azione (edit/add/delete) solo dove consentito (delete solo per decimali);
  • gestiscono l’espansione/chiusura dei figli tramite .dewey-children.open.

Dal punto di vista di sicurezza e UX la soluzione è buona.


419-433: Conferma cancellazione: messaggio chiaro e uso sicuro di SweetAlert

La conferma di eliminazione usa Swal.fire con text e interpolazione di node.code/node.name nel testo, senza HTML, quindi è sicura rispetto a XSS e fornisce un messaggio comprensibile all’admin.


693-742: Calcolo statistiche semplice ed efficace

updateStats() / countStats() iterano ricorsivamente il tree e popolano i contatori per livello/decimali in modo lineare e leggibile. Per ora l’assegnazione “tutto ciò che non è livello 1–3 → decimale” funziona bene nel contesto attuale.


754-760: Esposizione di window.DeweyEditor come API globale mirata

L’oggetto globale window.DeweyEditor espone solo i metodi necessari per gli handler inline (closeModal, saveEdit, saveAdd, restoreBackup), mantenendo il resto dell’implementazione incapsulato nell’IIFE. Buon compromesso tra necessità del template HTML e isolamento dello scope JS.

storage/plugins/dewey-editor/DeweyEditorPlugin.php (4)

190-199: Le criticità segnalate nelle review precedenti sono state corrette.

Ottimo lavoro: hai aggiunto la validazione esplicita del risultato di json_decode (linee 190-199) e il controllo per file_get_contents che restituisce false (linee 496-499). Entrambe le correzioni evitano TypeError e corruzioni dati.

Also applies to: 496-499


92-135: LGTM!

La registrazione delle route è corretta: autenticazione admin su tutti gli endpoint e protezione CSRF sulle operazioni di modifica. Architettura ben strutturata.


388-446: LGTM!

L'algoritmo di merge ricorsivo è ben implementato: indicizza per codice per lookup O(n), gestisce correttamente aggiornamenti/aggiunte/invariati, e mantiene il sorting. Logica solida.


510-516: LGTM!

I metodi helper (getJsonPath, countEntries, jsonSuccess, jsonError) sono implementati correttamente: logica chiara, gestione appropriata dei casi, e nessun problema evidente.

Also applies to: 556-566, 568-582

Comment on lines 68 to 74
public function onInstall(): void
{
// Create backup directory
if (!is_dir($this->backupDir)) {
mkdir($this->backupDir, 0755, true);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Verifica il risultato di mkdir durante l'installazione.

Se mkdir() fallisce (permessi, spazio disco, ecc.), il plugin sembrerà installato ma le successive operazioni di backup falliranno silenziosamente.

Applica questa diff per gestire l'errore:

 public function onInstall(): void
 {
     // Create backup directory
     if (!is_dir($this->backupDir)) {
-        mkdir($this->backupDir, 0755, true);
+        if (!mkdir($this->backupDir, 0755, true)) {
+            error_log("Dewey Editor: impossibile creare directory backup: {$this->backupDir}");
+        }
     }
 }
📝 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
public function onInstall(): void
{
// Create backup directory
if (!is_dir($this->backupDir)) {
mkdir($this->backupDir, 0755, true);
}
}
public function onInstall(): void
{
// Create backup directory
if (!is_dir($this->backupDir)) {
if (!mkdir($this->backupDir, 0755, true)) {
error_log("Dewey Editor: impossibile creare directory backup: {$this->backupDir}");
}
}
}
🤖 Prompt for AI Agents
In storage/plugins/dewey-editor/DeweyEditorPlugin.php around lines 68 to 74, the
onInstall() code calls mkdir() without checking its result which can fail
silently; update the method to attempt to create the backup directory only if
missing, check the boolean return of mkdir (and re-check is_dir after the call),
and if creation fails log a clear error and/or throw an exception so
installation stops (include errno or last error message when available) to
prevent silent failures during later backup operations.

- Verify mkdir() result in onInstall() with error logging
- Use getContents() instead of string cast for PSR-7 streams
- Verify copy() result in createBackup() with error logging
- Handle unlink() errors in cleanupOldBackups() with logging
- Fix XSS in showBackups(): sanitize date/filename with escapeHtml/encodeURIComponent
- Use object syntax in Swal.fire() for saveData() to prevent XSS
- Add missing English translations for Dewey Editor UI strings
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
storage/plugins/dewey-editor/views/editor.php (1)

259-281: Escape result.error when rendering in loadData error branch for consistency

The error message in the loadData() function is inserted into the DOM without sanitization, while other dynamic content (like node.code and node.name) uses escapeHtml(). Although the current backend implementation uses only i18n strings in error messages, applying escapeHtml() provides defense-in-depth and maintains consistent patterns:

const msg = result.error || <?= json_encode(__('Errore nel caricamento.')) ?>;
treeContainer.innerHTML = `<div class="text-center text-red-500 py-8">
  <i class="fas fa-exclamation-circle text-2xl mb-2"></i>
  <p>${escapeHtml(msg)}</p>
</div>`;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3e857a and fa3bce4.

📒 Files selected for processing (3)
  • locale/en_US.json (15 hunks)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

139-139: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


141-141: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


146-146: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


157-157: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


249-249: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


454-454: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (1)
locale/en_US.json (1)

192-192: Nuove stringhe Dewey/editor coerenti e senza duplicati funzionali evidenti

Le nuove chiavi per editor, statistiche, import/merge e inserimento/manuale Dewey sono consistenti con il resto dell’app e non introducono duplicati di chiavi JSON (che romperebbero le traduzioni). Nulla da segnalare lato funzionale qui.

Also applies to: 269-269, 348-348, 392-392, 648-648, 700-700, 722-722, 104-125, 3290-3312

Slim 4's getParsedBody() doesn't automatically parse JSON bodies.
Added getJsonBody() helper that detects Content-Type: application/json
and manually parses the raw body with json_decode().

Fixed "Dati mancanti" (400 Bad Request) error when saving.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

261-285: validateData: aggiungi controllo is_array per evitare TypeError (come in saveData)

Qui, dopo il json_decode, DeweyValidator::validate() viene chiamato direttamente su $data. Con declare(strict_types=1) e firma validate(array $data), JSON validi ma non array (es. "foo", 123) causeranno un TypeError invece di una risposta JSON controllata. È lo stesso problema già evidenziato in una review precedente.

Suggerimento, allineando a saveData:

    if (is_string($data)) {
        $data = json_decode($data, true);
        if ($data === null) {
            return $this->jsonError($response, __('JSON non valido.'), 400);
        }
    }
+
+   if (!is_array($data)) {
+       return $this->jsonError($response, __('Formato dati non valido.'), 400);
+   }
 
    $validator = new DeweyValidator();
    $errors = $validator->validate($data);

Così eviti eccezioni interne e mantieni coerenza con saveData.

🧹 Nitpick comments (4)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (4)

50-61: Verifica che onActivate() non venga chiamato senza pluginId valido

Se onActivate() fosse invocato quando pluginId è ancora null, il bind_param('i', $this->pluginId) userebbe 0 e produrrebbe record in plugin_hooks con plugin_id = 0, potenzialmente orfani. Consiglierei un early‑return con log quando pluginId non è impostato esplicitamente (es. via setPluginId()):

public function onActivate(): void
{
-    // Register hooks in database
+    if ($this->pluginId === null) {
+        error_log('[DeweyEditor] onActivate chiamato senza pluginId valido; registrazione hook annullata.');
+        return;
+    }
+    // Register hooks in database

Also applies to: 83-113


177-193: renderEditor: variabili apparentemente “inutilizzate” in realtà usate nelle view

$csrfToken e $content (Line [179] e Line [184]) risultano inutilizzate per PHPMD, ma sono in scope per views/editor.php e layout.php, quindi non vanno rimosse. Per ridurre i falsi positivi puoi:

  • Aggiungere un commento esplicativo sopra alle variabili.
  • Oppure configurare PHPMD/PHPCS per ignorare questo metodo.

L’implementazione attuale è corretta per il pattern “layout + contenuto bufferizzato”.


195-214: Gestione degli avvisi PHPMD sui parametri $request non usati nei handler

Nei metodi getData, exportData e listBackups il parametro $request risulta non utilizzato, ma è richiesto dalla firma dei route handler compatibili PSR‑7/Slim. Non lo rimuoverei.

Se vuoi eliminare i warning senza cambiare la firma, puoi, ad esempio:

  • Prefissare il nome con _ (Request $_request), oppure
  • Aggiungere un @SuppressWarnings("UnusedFormalParameter") nella docblock, se supportato dallo strumento.

Also applies to: 287-306, 492-515


492-515: listBackups: rendi sicura la gestione del risultato di glob()

glob($pattern) (Line [500]) può restituire false in caso di errore; il foreach ($files as $file) su false genererebbe un warning. Basta normalizzare il risultato:

-        $pattern = $this->backupDir . "/dewey_{$locale}_*.json";
-        $files = glob($pattern);
+        $pattern = $this->backupDir . "/dewey_{$locale}_*.json";
+        $files = glob($pattern) ?: [];

Il resto del metodo continua a funzionare invariato.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0a3f61 and d2ba70e.

📒 Files selected for processing (1)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

177-177: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


179-179: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


184-184: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


195-195: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


287-287: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


492-492: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (1)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

562-606: Backup/cleanup: logica complessiva solida, piccoli dettagli ok

La catena createBackupcleanupOldBackups gestisce bene:

  • Creazione condizionale della directory con controllo su mkdir e log in caso di errore.
  • copy() verificato e loggato; in caso di fallimento non viene eseguita la pulizia.
  • Limitazione a MAX_BACKUPS con ordinamento per filemtime e log sugli unlink falliti.

Non vedo problemi di correttezza evidenti; la gestione failure è adeguata per un plugin admin‑side.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (4)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (4)

261-285: validateData: possibile TypeError passando JSON scalare al DeweyValidator

Dopo la json_decode in validateData() non viene verificato che $data sia effettivamente un array prima di passarlo a new DeweyValidator()->validate($data), che è tipizzato validate(array $data). Con declare(strict_types=1) un JSON valido ma scalare ("foo", 123, true, ecc.) causerà un TypeError invece di una risposta JSON controllata, come già gestito in saveData().

Allinea validateData() a saveData() aggiungendo il controllo di tipo:

     if (is_string($data)) {
         $data = json_decode($data, true);
         if ($data === null) {
             return $this->jsonError($response, __('JSON non valido.'), 400);
         }
     }
 
+    if (!is_array($data)) {
+        return $this->jsonError($response, __('Formato dati non valido.'), 400);
+    }
+
     $validator = new DeweyValidator();
     $errors = $validator->validate($data);

In questo modo eviti eccezioni interne e mantieni la semantica degli errori coerente con gli altri endpoint.


287-306: exportData: file_get_contents() non controllato → rischio export vuoti con 200 OK

In exportData() il risultato di file_get_contents($filePath) non viene verificato. Se la lettura fallisce (permessi, I/O, race), file_get_contents restituisce false e il body della risposta sarà vuoto, ma l’endpoint restituirà comunque 200 con un file apparentemente valido ma in realtà corrotto.

Conviene gestire l’errore esplicitamente:

-        $content = file_get_contents($filePath);
+        $content = file_get_contents($filePath);
+        if ($content === false) {
+            return $this->jsonError($response, __('Errore nella lettura del file.'), 500);
+        }
         $filename = "dewey_completo_{$locale}_" . date('Y-m-d_His') . '.json';

Così l’utente riceve un errore chiaro invece di un download vuoto difficile da diagnosticare.


308-355: importData: manca il controllo che $data sia un array prima della validazione

In importData() dopo json_decode verifichi solo $data === null, ma non che sia un array. Con JSON scalari validi ("foo", 123, ecc.) DeweyValidator::validate(array $data) riceverà un non-array e, con strict_types=1, genererà un TypeError invece di una risposta JSON 400 coerente con gli altri endpoint.

Suggerisco di aggiungere il controllo:

         $content = $stream->getContents();
         $data = json_decode($content, true);
 
         if ($data === null) {
             return $this->jsonError($response, __('JSON non valido.'), 400);
         }
+
+        if (!is_array($data)) {
+            return $this->jsonError($response, __('Formato dati non valido.'), 400);
+        }
 
         // Validate
         $validator = new DeweyValidator();

Questo allinea il comportamento di importData() a saveData() ed evita errori fatali su input formalmente validi ma con formato inatteso.


356-422: mergeImportData: validare che $importData e $existingData siano array e gestire errori di lettura

In mergeImportData() ci sono ancora alcune falle di robustezza:

  • $importData = json_decode($content, true); viene solo controllato per === null, ma non per il tipo: un JSON scalare valido farà fallire mergeByCode(array $existing, array $import, ...) con TypeError/foreach su non-array.
  • Per il file esistente:
    • Se file_get_contents($filePath) restituisce false, passi comunque oltre.
    • json_decode($existingContent, true) ?? [] copre solo il caso null, non i JSON scalari (che restano scalari e rompono mergeByCode).

Una versione più sicura potrebbe essere:

         $content = $stream->getContents();
         $importData = json_decode($content, true);
 
         if ($importData === null) {
             return $this->jsonError($response, __('JSON non valido.'), 400);
         }
+
+        if (!is_array($importData)) {
+            return $this->jsonError($response, __('Formato dati non valido.'), 400);
+        }
@@
-        $filePath = $this->getJsonPath($locale);
-        $existingData = [];
-        if (file_exists($filePath)) {
-            $existingContent = file_get_contents($filePath);
-            if ($existingContent !== false) {
-                $existingData = json_decode($existingContent, true) ?? [];
-            }
-        }
+        $filePath = $this->getJsonPath($locale);
+        $existingData = [];
+        if (file_exists($filePath)) {
+            $existingContent = file_get_contents($filePath);
+            if ($existingContent === false) {
+                return $this->jsonError(
+                    $response,
+                    __('Errore nella lettura del file Dewey esistente.'),
+                    500
+                );
+            }
+
+            $existingData = json_decode($existingContent, true);
+            if ($existingData === null || !is_array($existingData)) {
+                return $this->jsonError(
+                    $response,
+                    __('File Dewey esistente non è un JSON valido o è corrotto.'),
+                    500
+                );
+            }
+        }

In questo modo garantisci che mergeByCode() riceva sempre array ben formati e che eventuali problemi di I/O o JSON corrotto vengano segnalati con codice 500 invece di esplodere con errori PHP.

🧹 Nitpick comments (2)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (2)

83-113: Verifica esplicita di pluginId prima di registrare/cancellare i hook

In onActivate()/deleteHooks() pluginId viene usato per inserire/cancellare righe in plugin_hooks, ma non viene mai verificato che sia valorizzato (può restare null se la query nel costruttore fallisce o se il chiamante dimentica setPluginId()).

Per evitare inserimenti con plugin_id = 0 o comportamenti silenziosi in caso di problemi a monte, suggerirei una guardia esplicita:

     public function onActivate(): void
     {
+        if ($this->pluginId === null) {
+            error_log('[DeweyEditor] pluginId non impostato, impossibile registrare i hook.');
+            return;
+        }
@@
     private function deleteHooks(): void
     {
-        if ($this->pluginId) {
+        if ($this->pluginId !== null) {
             $stmt = $this->db->prepare("DELETE FROM plugin_hooks WHERE plugin_id = ?");

Così rendi più robusto il lifecycle del plugin in caso di configurazioni inconsistente del DB o errori d’installazione.

Also applies to: 120-130


492-515: listBackups: gestire il caso glob() fallisce (edge case non critico)

In listBackups() glob($pattern) può restituire false in caso di errore di I/O; il foreach ($files as $file) darebbe un warning. È un edge case, ma banale da rendere robusto:

-        $files = glob($pattern);
+        $files = glob($pattern) ?: [];

Non è critico (soprattutto in un contesto admin), ma ti evita warning rumorosi in log in caso di problemi sul filesystem.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2ba70e and bdd39c1.

📒 Files selected for processing (1)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (2)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

177-177: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


179-179: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


184-184: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


195-195: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


287-287: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)


492-492: Avoid unused parameters such as '$request'. (undefined)

(UnusedFormalParameter)

- Add is_array check in validateData() before calling validator
- Add file_get_contents() return check in exportData()
- Add is_array check in importData() for uploaded data
- Add proper error handling in mergeImportData() for existing file
- Add pluginId null check in onActivate() early return
- Use strict null comparison in deleteHooks()
- Handle glob() returning false with ?: [] fallback
- Prefix unused $request parameters with underscore
- Fix XSS vulnerability in editor.php error display with escapeHtml()
- Add English translations for new error messages
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

253-263: Gestire eventuali fallimenti di json_encode prima di scrivere i file Dewey

In saveData(), importData() e mergeImportData() il contenuto viene preparato con:

$jsonContent = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
if (file_put_contents($filePath ?? ..., $jsonContent) === false) { ... }

Se json_encode() dovesse fallire (es. per stringhe con encoding non valido), restituirebbe false, che verrebbe convertito in stringa vuota da file_put_contents, producendo un file JSON vuoto senza intercettare l’errore.

Per robustezza, suggerisco di controllare esplicitamente il risultato di json_encode prima di scrivere:

-        $jsonContent = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
-        if (file_put_contents($filePath, $jsonContent) === false) {
-            return $this->jsonError($response, __('Errore nel salvataggio del file.'), 500);
-        }
+        $jsonContent = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
+        if ($jsonContent === false) {
+            return $this->jsonError($response, __('Formato dati non valido.'), 500);
+        }
+        if (file_put_contents($filePath, $jsonContent) === false) {
+            return $this->jsonError($response, __('Errore nel salvataggio del file.'), 500);
+        }

e applicare lo stesso pattern in importData() e mergeImportData(). Così si evita la possibilità (seppur rara) di corrompere silenziosamente il file di classificazione con contenuto vuoto.

Also applies to: 359-363, 429-433

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bdd39c1 and 4c97889.

📒 Files selected for processing (3)
  • locale/en_US.json (15 hunks)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

182-182: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


184-184: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


189-189: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


200-200: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


296-296: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


518-518: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (1)
locale/en_US.json (1)

192-200: Nuove stringhe Dewey coerenti con la vista e il plugin

Le nuove chiavi di traduzione per l’editor Dewey (titolo, statistiche, ricerca, modalità di import/merge, messaggi di validazione e backup/restore) sono coerenti con l’uso in editor.php e DeweyEditorPlugin.php; i placeholder (%d, %s) sono mantenuti correttamente e i testi inglesi sono chiari. Non vedo problemi funzionali in questa sezione.

Also applies to: 269-270, 348-349, 392-393, 648-649, 700-701, 722-723, 104-105, 1597-1600, 2439-2442, 2459-2460, 2857-2858, 3290-3315

Comment on lines 388 to 399
function showAddModal(parentNode) {
const parentCode = parentNode.code;
const suggestedCode = parentCode.includes('.') ? parentCode : parentCode + '.';

modalContent.innerHTML = `
<div class="p-6">
<h3 class="text-lg font-semibold mb-4"><?= __('Aggiungi decimale sotto') ?> ${escapeHtml(parentCode)}</h3>
<div class="mb-4">
<label class="block text-sm font-medium text-gray-700 mb-1"><?= __('Codice') ?></label>
<input type="text" id="add-code" value="${suggestedCode}" placeholder="es. ${parentCode}.1"
class="w-full px-3 py-2 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500">
<p class="text-xs text-gray-500 mt-1"><?= __('Formato: 3 cifre + punto + 1-4 cifre decimali') ?></p>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Controllo prefisso del genitore e placeholder decimali non coerenti con il formato ammesso

Nella modale di aggiunta decimale e nel relativo handler ci sono due incongruenze:

  1. In showAddModal il placeholder usa sempre ${parentCode}.1 (es. per 500.1 mostra 500.1.1), ma la regex in saveAddHandler accetta solo ^[0-9]{3}\.[0-9]{1,4}$ (una sola virgola decimale). Il placeholder quindi suggerisce esempi che verrebbero rifiutati dalla validazione.

  2. In saveAddHandler il controllo del prefisso:

if (!code.startsWith(parentCode.replace('.', ''))) {
    const baseCode = parentCode.includes('.') ? parentCode.split('.')[0] : parentCode;
    if (!code.startsWith(baseCode)) {
        // errore
    }
}

per i genitori decimali (es. 500.1) permette codici figli che iniziano solo con la classe base (500...), non necessariamente con l’intero codice del genitore, mentre il messaggio dice che “il codice deve iniziare con il prefisso del genitore”.

Per evitare confusione agli admin e rendere la UX coerente con la regex, suggerirei:

  • Aggiornare il placeholder per mostrare esempi del tipo XXX.Y / XXX.YZ (senza concatenare un ulteriore .1 se il genitore è già decimale).
  • Semplificare il controllo prefisso a qualcosa come:
-        // Check code starts with parent
-        if (!code.startsWith(parentCode.replace('.', ''))) {
-            const baseCode = parentCode.includes('.') ? parentCode.split('.')[0] : parentCode;
-            if (!code.startsWith(baseCode)) {
-                Swal.fire(...);
-                return;
-            }
-        }
+        // Verifica che il codice inizi davvero con il codice del genitore (stessa radice Dewey)
+        const expectedPrefix = parentCode.includes('.') ? parentCode : parentCode;
+        if (!code.startsWith(expectedPrefix)) {
+            Swal.fire(
+                <?= json_encode(__('Errore')) ?>,
+                <?= json_encode(__('Il codice deve iniziare con il prefisso del genitore.')) ?>,
+                'error'
+            );
+            return;
+        }

o una variante equivalente che rispecchi la semantica desiderata (radice = genitore o solo classe principale).

Also applies to: 452-469

🤖 Prompt for AI Agents
storage/plugins/dewey-editor/views/editor.php around lines 388-399 and 452-469:
the add-decimal modal builds a misleading placeholder and the child-prefix
validation is inconsistent with the allowed regex; change the placeholder
generation so it does not append an extra ".1" when the parent is already
decimal (e.g. set suggestedCode to parentCode if parentCode.includes('.')
otherwise parentCode + '.1', and use that for both the input value and example
placeholder), and simplify the prefix check in saveAddHandler to compute
baseCode = parentCode.includes('.') ? parentCode.split('.')[0] : parentCode and
then require that the new code startsWith(parentCode) OR startsWith(baseCode)
(i.e. if not (code.startsWith(parentCode) || code.startsWith(baseCode)) then
error) so the validation and error message match the intended semantics and the
regex.

BREAKING CHANGES:
- Remove `classificazione` table from database schema (39 tables, was 40)
- Rename column `classificazione_dowey` → `classificazione_dewey` in libri table
- Remove FK columns from scaffali/mensole tables

Migration required for existing installations:
  mysql -u USER -p DATABASE < installer/database/migrations/migrate_0.3.0.sql

Removed:
- app/Controllers/DeweyController.php (legacy, replaced by DeweyApiController)
- app/Views/admin/dewey.php (legacy admin page)
- Routes for legacy DeweyController

Changed:
- All PHP files now use `classificazione_dewey` (was `classificazione_dowey`)
- data_it_IT.sql and data_en_US.sql cleaned (removed classificazione INSERTs)
- EXPECTED_TABLES reduced from 40 to 39 in Installer.php
- version.json bumped to 0.3.0

Fixed:
- scheda_libro.php: Dewey name lookup now falls back to parent code on 404
- book_form.php: Custom Dewey codes not in JSON now display correctly
- dewey-editor plugin: json_encode error handling, add modal validation

Added:
- CHANGELOG.md documenting breaking changes
- Migration script: installer/database/migrations/migrate_0.3.0.sql
@fabiodalez-dev fabiodalez-dev changed the title feat: Dewey Editor plugin and book form improvements feat!: v0.3.0 - Dewey Editor plugin + legacy cleanup Dec 3, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
installer/classes/Installer.php (1)

688-693: Remove query for non-existent classificazione table or align to existing schema

The classificazione table does not exist in installer/database/schema.sql and will cause a PDOException on fresh installations. The schema only includes generi table (which is checked immediately after). Since classificazione_dewey is a field in the libri table—not a separate table—either remove this check or change it to verify the generi table instead:

-        // Check essential data
-        $stmt = $pdo->query("SELECT COUNT(*) as count FROM classificazione");
-        $result = $stmt->fetch();
-        if ($result['count'] == 0) {
-            throw new Exception(__("Dati di classificazione mancanti"));
-        }

         $stmt = $pdo->query("SELECT COUNT(*) as count FROM generi");

Or, if a dedicated classification data check is required, verify the generi table that is already populated with classification data during installation.

🧹 Nitpick comments (3)
app/Routes/web.php (1)

1449-1451: Endpoint Dewey nuovi coerenti; valuta rate limiting opzionale

Gli endpoint /api/dewey/children e /api/dewey/search sono ben allineati agli altri endpoint Dewey ed esposti solo dietro AdminAuthMiddleware, quindi il perimetro di sicurezza è corretto.

Per coerenza con altre API admin ad alto utilizzo (es. impostazioni, statistiche) potresti valutare, in futuro, l’aggiunta di un leggero RateLimitMiddleware, soprattutto su /api/dewey/search, nel caso venga usato in modo intensivo da UI o plugin JS. Non è bloccante per il merge.

app/Views/libri/scheda_libro.php (2)

7-11: Badge di stato e bottone “Elimina” più leggibili; valuta allineamento con il badge secondario

Le nuove classi dei badge (bg-*-500 text-white) migliorano nettamente il contrasto e la leggibilità sul riquadro di copertina, e l’introduzione di $btnDanger rende più coerente lo stile del pulsante di eliminazione.

L’unica nota opzionale: nella sezione dettagli in basso esiste ancora un secondo badge “Stato” con palette diversa (pastello). In un passaggio successivo potresti uniformare visivamente i due badge per coerenza complessiva dell’interfaccia.

Also applies to: 13-13, 17-17


398-448: Risoluzione dinamica del nome Dewey ben strutturata; piccoli miglioramenti opzionali

Mi piace l’approccio di mostrare il codice come testo monospazio e risolvere il nome via /api/dewey/search, con:

  • inserimento del valore PHP in JS tramite json_encode (evita XSS);
  • gestione del vecchio formato a intervalli (300-340-347) prendendo l’ultimo segmento;
  • fallback sul parent tramite getParentCode e uso di textContent (output sicuro).

Due spunti del tutto opzionali:

  • Applicare un trim() lato PHP al valore (classificazione_dewey) prima del json_encode, così da evitare problemi con spazi residui provenienti dal DB.
  • Se in futuro avrai molti livelli decimali, potresti rendere getParentCode iterativa/ricorsiva (provando più antenati) invece di fermarti al primo parent.

Per l’uso attuale è comunque più che adeguato.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c97889 and fb92512.

⛔ Files ignored due to path filters (1)
  • installer/plugins/dewey-editor.zip is excluded by !**/*.zip
📒 Files selected for processing (19)
  • CHANGELOG.md (1 hunks)
  • app/Controllers/DeweyController.php (0 hunks)
  • app/Controllers/LibriController.php (4 hunks)
  • app/Controllers/PublicApiController.php (2 hunks)
  • app/Models/BookRepository.php (2 hunks)
  • app/Routes/web.php (1 hunks)
  • app/Views/admin/dewey.php (0 hunks)
  • app/Views/libri/partials/book_form.php (3 hunks)
  • app/Views/libri/scheda_libro.php (2 hunks)
  • installer/classes/Installer.php (2 hunks)
  • installer/database/migrations/migrate_0.3.0.sql (1 hunks)
  • installer/database/schema.sql (2 hunks)
  • locale/en_US.json (15 hunks)
  • storage/plugins/dewey-editor/DeweyEditorPlugin.php (1 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
  • storage/plugins/z39-server/classes/DublinCoreFormatter.php (1 hunks)
  • storage/plugins/z39-server/classes/MARCXMLFormatter.php (1 hunks)
  • storage/plugins/z39-server/classes/MODSFormatter.php (1 hunks)
  • version.json (1 hunks)
💤 Files with no reviewable changes (2)
  • app/Views/admin/dewey.php
  • app/Controllers/DeweyController.php
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (6)
app/Views/libri/scheda_libro.php (1)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
storage/plugins/z39-server/classes/MODSFormatter.php (2)
storage/plugins/z39-server/classes/DublinCoreFormatter.php (1)
  • createElement (170-173)
storage/plugins/z39-server/classes/SRUServer.php (1)
  • escapeXml (1073-1076)
storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)
storage/plugins/dewey-editor/classes/DeweyValidator.php (2)
  • DeweyValidator (10-260)
  • validate (29-47)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
app/Routes/web.php (1)
app/Controllers/DeweyApiController.php (1)
  • DeweyApiController (10-392)
app/Views/libri/partials/book_form.php (1)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • e (57-60)
🪛 PHPMD (2.15.0)
storage/plugins/dewey-editor/DeweyEditorPlugin.php

182-182: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


184-184: Avoid unused local variables such as '$csrfToken'. (undefined)

(UnusedLocalVariable)


189-189: Avoid unused local variables such as '$content'. (undefined)

(UnusedLocalVariable)


200-200: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


299-299: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)


527-527: Avoid unused parameters such as '$_request'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (20)
installer/classes/Installer.php (2)

1172-1176: Docblock aggiornato e coerente con i plugin di default

Il commento ora elenca correttamente anche dewey-editor tra i plugin installati di default, in linea con il codice sottostante (installPluginsFromZip). Nulla da segnalare lato funzionale.


1278-1290: Installazione di default di dewey-editor allineata al pattern esistente (verificare solo i hook definiti in plugin.json)

L’aggiunta di:

  • $installPlugin('dewey-editor', [...])
    con hook app.routes.register → registerRoutes a priorità 10 è coerente con quanto già fatto per digital-library e usa correttamente l’helper $ensureHooks.

Unico punto da verificare: che il plugin.json di dewey-editor non definisca già un hook app.routes.register con un metodo diverso (es. handleApproutesregister), altrimenti finiresti con due entry distinte in plugin_hooks per lo stesso hook.

Se possibile, tenere tutti i hook “di default” del plugin in plugin.json e usare additionalHooks solo per eccezioni aiuterebbe a evitare divergenze tra installer e manifest del plugin, ma non è strettamente necessario.

version.json (1)

3-3: LGTM! Version bump appropriato per la modifica dello schema.

Il bump di versione a 0.3.0 è corretto considerando la modifica breaking dello schema del database (rinomina colonna).

storage/plugins/z39-server/classes/DublinCoreFormatter.php (1)

108-110: LGTM! Correzione del typo nel nome del campo.

La modifica corregge il typo da classificazione_dowey a classificazione_dewey mantenendo invariata la logica di formattazione Dublin Core.

storage/plugins/z39-server/classes/MODSFormatter.php (1)

142-146: LGTM! Correzione coerente del nome campo.

La modifica allinea il formatter MODS con la correzione del typo classificazione_dewey, mantenendo la logica di classificazione Dewey inalterata.

storage/plugins/z39-server/classes/MARCXMLFormatter.php (1)

67-71: LGTM! Entrambi i riferimenti al campo aggiornati correttamente.

La modifica corregge sia il controllo di presenza (linea 67) che l'assegnazione del valore (linea 69) per il campo classificazione_dewey nel formato MARCXML.

app/Controllers/PublicApiController.php (2)

126-126: LGTM! Campo rinominato nella query SELECT.

La modifica aggiorna correttamente il riferimento al campo classificazione_dewey nella query SELECT.


202-202: LGTM! Chiave risposta API aggiornata coerentemente.

La chiave nell'array di risposta dell'API corrisponde al nome del campo aggiornato nella query SELECT (linea 126).

app/Controllers/LibriController.php (4)

257-257: LGTM! Campo rinominato nel metodo store.

Il campo classificazione_dewey è stato aggiornato correttamente nell'array fields per la creazione di nuovi libri.


698-698: LGTM! Campo rinominato nel metodo update.

Il campo classificazione_dewey è stato aggiornato coerentemente con il metodo store (linea 257).


1816-1816: LGTM! Lettura campo aggiornata nel rendering etichetta portrait.

Il rendering dell'etichetta legge correttamente il campo classificazione_dewey dall'array libro.


1958-1958: LGTM! Lettura campo aggiornata nel rendering etichetta landscape.

Il rendering dell'etichetta landscape è coerente con il rendering portrait (linea 1816).

app/Models/BookRepository.php (3)

317-319: LGTM! Campo rinominato nel metodo createBasic.

Il campo classificazione_dewey è stato aggiornato correttamente con controllo di esistenza della colonna.


529-531: LGTM! Campo rinominato nel metodo updateBasic.

Il campo è coerente con createBasic (linee 317-318) e mantiene il controllo di esistenza della colonna.


317-319: All residual references to the old field name have been successfully removed.

Verification confirms zero occurrences of the typo classificazione_dowey across the entire codebase. All references consistently use the correct field name classificazione_dewey in controllers, views, formatters, and API endpoints. The migration is complete.

installer/database/schema.sql (2)

330-330: Colonna classificazione_dewey coerente con il nuovo modello; attenzione all’upgrade

L’introduzione (o rinomina) della colonna classificazione_dewey su libri è coerente con le modifiche al codice e più chiara del vecchio nome “dowey”; il tipo varchar(50) è più che sufficiente per i codici Dewey con decimali e per gli eventuali formati legacy.

L’unico punto operativo è assicurarsi che, sugli ambienti aggiornati e non fresh install, lo script di migrazione (migrate_0.3.0.sql) esegua correttamente il rename dalla vecchia colonna alla nuova e che tutte le query applicative siano già state adeguate (sembra di sì dal contesto del PR).


661-661: Indice univoco uniq_codice su scaffali.codice: ok per nuovi install, verifica duplicati in upgrade

Rendere scaffali.codice univoco tramite UNIQUE KEY uniq_codice (codice) ha senso per mantenere coerenza e identificare in modo chiaro ogni scaffale tramite il proprio codice.

Per i database già esistenti, conviene solo verificare che non esistano record duplicati prima di applicare lo schema/migrazione, altrimenti l’ALTER fallirà. Un controllo rapido tipo:

SELECT codice, COUNT(*) 
FROM scaffali 
WHERE codice IS NOT NULL 
GROUP BY codice 
HAVING COUNT(*) > 1;

ti permette di evidenziare eventuali casi da ripulire prima dell’upgrade.

storage/plugins/dewey-editor/views/editor.php (1)

582-633: Gestione errori import/backup ora sicura rispetto a XSS

Vedo che:

  • in handleImport gli errori vengono aggregati in un array e passati a SweetAlert tramite proprietà text (lines.join('\n')), senza HTML;
  • in showBackups data/size sono sanificati con escapeHtml e i filename sono encodeURIComponent‑ati prima di finire nell’onclick.

Questo chiude bene i vettori XSS segnalati nelle revisioni precedenti, mantenendo la UX chiara.

Also applies to: 635-661

storage/plugins/dewey-editor/DeweyEditorPlugin.php (1)

221-267: Gestione JSON, validazione e backup del plugin Dewey solida e coerente

La pipeline per saveData/validateData/importData/mergeImportData è ben strutturata:

  • controlli espliciti su locale supportato e forma dei dati (is_array dopo json_decode);
  • uso sistematico di DeweyValidator prima di scrivere/mergiare;
  • creazione backup con controllo di mkdir/copy e cleanup con unlink verificato;
  • restoreBackup/exportData ora verificano anche l’esito di file_get_contents, evitando ripristini/esportazioni “vuote”.

Nel complesso il flusso riduce bene il rischio di TypeError, file corrotti e backup mancati.

Also applies to: 324-457, 597-641, 655-691

locale/en_US.json (1)

1-3316: I duplicati dalla revisione precedente sono stati risolti; le nuove traduzioni per Dewey Editor sono complete e ben integrate.

La revisione precedente segnalava 4 chiavi duplicate (righe ~3282-3293): "Nessuna selezione", "Seleziona...", "Cerca", "Aggiungi". Nella versione attuale, queste stringhe compaiono solo una volta nel file (linee 1721, 2448, 342, 114), indicando che il problema è stato risolto.

Le 22+ nuove voci di traduzione per il Dewey Editor sono ben strutturate e coerenti con lo stile esistente del file:

  • Etichette UI (linee 192, 269, 348, 392, 648, 700, 722, 1104)
  • Modalità di importazione e merge (linee 1597-1600)
  • Guida utente per l'inserimento manuale (linee 3291-3310)
  • Messaggi di errore specifici per il recupero dati Dewey

Il file JSON rimane valido, con tutti gli entry propriamente formattati e le traduzioni coerenti con il vocabolario esistente.

- SbnClient: Extract Dewey classification from SBN API responses
  (classificazioneDewey field in full records)
- DeweyAutoPopulator: Auto-add new Dewey names to JSON when found in
  SBN responses (language-aware - only updates if source matches locale)
- Z39ServerPlugin: Fix encryption key lookup to use PLUGIN_ENCRYPTION_KEY
  (same as PluginManager) instead of just APP_KEY
- book_form.php: Add handler to populate Dewey field from scraped data,
  expose setDeweyCode to global scope for scraping handler
- .env.example: Document PLUGIN_ENCRYPTION_KEY variable
- Fix XSS vulnerability in Dewey breadcrumb (use textContent instead of innerHTML)
- Align frontend validateDeweyCode regex with backend DeweyValidator (require 3 digits)
- Fix migration VARCHAR(20) to VARCHAR(50) to match schema.sql
- Remove obsolete classificazione table check from installer (Dewey now in JSON)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
app/Views/libri/partials/book_form.php (2)

1035-1038: Allineare validateDeweyCode al pattern del backend

Attualmente il validator frontend accetta 1–3 cifre iniziali:

const validateDeweyCode = (code) => {
  return /^[0-9]{1,3}(\.[0-9]{1,4})?$/.test(code);
};

mentre lato PHP (DeweyValidator::PATTERN_ANY_CODE) il pattern richiede esattamente 3 cifre (^[0-9]{3}(\.[0-9]{1,4})?$). Questo disallineamento fa sì che il frontend consideri validi codici che poi saranno rifiutati dal backend o non presenti nello schema standard.

Suggerisco di allineare la regex:

-  const validateDeweyCode = (code) => {
-    return /^[0-9]{1,3}(\.[0-9]{1,4})?$/.test(code);
-  };
+  const validateDeweyCode = (code) => {
+    // 3 cifre principali + parte decimale opzionale, in linea con DeweyValidator lato server
+    return /^[0-9]{3}(\.[0-9]{1,4})?$/.test(code);
+  };

1283-1335: Breadcrumb Dewey con innerHTML e valori non sanificati → rischio XSS

Nel fallback finale di navigateToCode:

if (targetCode !== lastFoundCode) {
  breadcrumb.innerHTML = `<i class="fas fa-home"></i> <span class="text-gray-500">${lastFoundCode || ''} > ${targetCode}</span>`;
  await setDeweyCode(targetCode, null);
}

targetCode e lastFoundCode derivano da INITIAL_BOOK.classificazione_dewey (DB) o da input manuale futuro. Se nel DB fosse mai finito un valore non valido (es. con </">), qui verrebbe iniettato direttamente nel DOM tramite innerHTML, con rischio di XSS (anche se la pagina è admin).

È meglio costruire il breadcrumb con HTML statico e testo dinamico via textContent, come già suggerito nel commento precedente:

-    if (targetCode !== lastFoundCode) {
-      breadcrumb.innerHTML = `<i class="fas fa-home"></i> <span class="text-gray-500">${lastFoundCode || ''} > ${targetCode}</span>`;
-      // setDeweyCode cercherà il nome tramite API (o mostrerà il nome del parent)
-      await setDeweyCode(targetCode, null);
-    }
+    if (targetCode !== lastFoundCode) {
+      // Aggiorna breadcrumb in modo sicuro senza inserire HTML non sanificato
+      breadcrumb.innerHTML = '<i class="fas fa-home"></i> ';
+      const span = document.createElement('span');
+      span.className = 'text-gray-500';
+      const prefix = lastFoundCode ? `${lastFoundCode} > ` : '';
+      span.textContent = prefix + targetCode;
+      breadcrumb.appendChild(span);
+      // setDeweyCode cercherà il nome tramite API (o mostrerà il nome del parent)
+      await setDeweyCode(targetCode, null);
+    }

Per ulteriore hardening potresti applicare lo stesso approccio anche alle altre assegnazioni di breadcrumb.innerHTML che interpolano codici.

🧹 Nitpick comments (5)
storage/plugins/z39-server/classes/SbnClient.php (2)

316-326: Uso di extractDeweyData: aggiungerei un cast a stringa per robustezza verso l’API SBN

La logica di arricchimento e popolamento di classificazione_dewey / _dewey_name_sbn è corretta e coerente con il resto del codice. Per essere un po’ più difensivi rispetto a possibili cambi di formato dell’API SBN (es. campo che in futuro diventa array o numero), con strict_types=1 eviterei possibili TypeError castando esplicitamente a stringa nei due call‑site:

-            if (empty($books[$index]['classificazione_dewey']) && !empty($fullRecord['classificazioneDewey'])) {
-                $deweyData = $this->extractDeweyData($fullRecord['classificazioneDewey']);
+            if (empty($books[$index]['classificazione_dewey']) && !empty($fullRecord['classificazioneDewey'])) {
+                $deweyData = $this->extractDeweyData((string)$fullRecord['classificazioneDewey']);
             }
-        $deweyData = $this->extractDeweyData($record['classificazioneDewey'] ?? '');
+        $deweyData = $this->extractDeweyData((string)($record['classificazioneDewey'] ?? ''));

Also applies to: 415-423


603-639: extractDeweyData: docblock e parsing buoni, ma il tipo di ritorno non riflette name nullable

La funzione è ben documentata e il parsing copre i formati SBN più comuni; c’è solo una piccola incoerenza tra docblock e implementazione:

  • In alcuni casi $name viene lasciato a null, ma il docblock dichiara array{code: string, name: string}|null. Per allineare la firma “informale” a ciò che il codice fa davvero (utile per IDE / static‑analysis), suggerirei:
-     * @return array{code: string, name: string}|null Dewey data or null
+     * @return array{code: string, name: ?string}|null Dewey data or null

Se in futuro SBN dovesse restituire codici con suffissi non strettamente numerici (es. varianti con / o lettere), potresti anche valutare di rendere la regex leggermente più permissiva; per l’uso attuale però il compromesso fra semplicità e robustezza è già buono.

storage/plugins/z39-server/Z39ServerPlugin.php (1)

652-688: getSetting + decryptSettingValue: flusso di decrittazione corretto, aggiungerei solo qualche guardia di hardening

La gestione dei valori cifrati con prefisso ENC: è ben integrata e coerente con l’approccio usato altrove; ci sono però un paio di piccoli punti che puoi rafforzare:

  • In getSetting, se per qualche motivo setting_value in tabella non fosse una stringa (o venisse impostato a NULL da altro codice), str_starts_with lancerebbe un TypeError con PHP 8. Una guardia minima sul tipo rende il metodo più resiliente:
-        $value = $row['setting_value'];
-
-        // Handle encrypted values (ENC: prefix from PluginManager)
-        if (str_starts_with($value, 'ENC:')) {
-            $value = $this->decryptSettingValue($value);
-        }
-
-        return $value ?? $default;
+        $value = $row['setting_value'];
+
+        if (!is_string($value)) {
+            return $default;
+        }
+
+        // Handle encrypted values (ENC: prefix from PluginManager)
+        if (str_starts_with($value, 'ENC:')) {
+            $value = $this->decryptSettingValue($value);
+        }
+
+        return $value ?? $default;
  • In decryptSettingValue, logghi correttamente l’assenza della chiave e le eccezioni, ma quando openssl_decrypt restituisce false (chiave/tag/dato non validi) al momento torni null in silenzio; un log dedicato aiuterebbe la diagnosi in produzione:
-            $decrypted = openssl_decrypt($ciphertext, 'aes-256-gcm', $key, OPENSSL_RAW_DATA, $iv, $tag);
-
-            return $decrypted !== false ? $decrypted : null;
+            $decrypted = openssl_decrypt($ciphertext, 'aes-256-gcm', $key, OPENSSL_RAW_DATA, $iv, $tag);
+
+            if ($decrypted === false) {
+                error_log('[Z39 Server Plugin] Decryption failed: invalid key, IV, tag or ciphertext');
+                return null;
+            }
+
+            return $decrypted;
  • Infine, questa implementazione è molto simile a quella usata in altri plugin (es. OpenLibrary); a medio termine potrebbe valere la pena centralizzare la logica di cifratura/decrittazione in un helper condiviso per evitare divergenze future tra plugin.

Also applies to: 690-735

app/Views/libri/partials/book_form.php (2)

243-285: Nuova sezione UI Dewey: struttura pulita e binding sicuro

Il campo nascosto classificazione_dewey usa HtmlHelper::e, quindi il valore proveniente dal DB è correttamente sanificato lato HTML, e la nuova UI (chip + input manuale + navigazione) è ben isolata e leggibile.
Come micro‑miglioria UX potresti valutare autocomplete="off" su #dewey_manual_input se noti suggerimenti del browser fuorvianti, ma non è bloccante.


1272-1288: Import Dewey dallo scraping: integrazione OK, possibile normalizzazione facoltativa

L’integrazione con lo scraping:

if (data.classificazione_dewey) {
  if (typeof window.setDeweyCode === 'function') {
    await window.setDeweyCode(data.classificazione_dewey, null);
  } else {
    const deweyHidden = document.getElementById('classificazione_dewey');
    if (deweyHidden) {
      deweyHidden.value = data.classificazione_dewey;
    }
  }
}

è coerente con la nuova UI (usa setDeweyCode e cade sul campo hidden se la funzione non è disponibile), quindi funzionalmente va bene.

Opzionale: se le fonti di scraping dovessero restituire ancora il vecchio formato multi‑livello tipo 300-340-347, potresti riusare la stessa normalizzazione usata in initializeDewey per l’initialCode (prendere l’ultimo segmento) prima di chiamare setDeweyCode, così il comportamento resta consistente tra dati esistenti e dati importati.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb92512 and c10b542.

📒 Files selected for processing (6)
  • .env.example (1 hunks)
  • app/Controllers/ScrapeController.php (3 hunks)
  • app/Support/DeweyAutoPopulator.php (1 hunks)
  • app/Views/libri/partials/book_form.php (4 hunks)
  • storage/plugins/z39-server/Z39ServerPlugin.php (2 hunks)
  • storage/plugins/z39-server/classes/SbnClient.php (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Views/libri/partials/book_form.php (1)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • e (57-60)
🔇 Additional comments (11)
app/Controllers/ScrapeController.php (2)

66-68: L'integrazione sembra corretta, ma considera la latenza I/O.

La chiamata a DeweyAutoPopulator::processBookData() esegue operazioni di file I/O in modo sincrono nel percorso della richiesta. Per la maggior parte dei casi d'uso questo è accettabile dato che il metodo ritorna immediatamente se non ci sono dati di classificazione.

Tuttavia, se le prestazioni diventano un problema, considera di spostare questa logica in un job asincrono o in un hook post-risposta.


94-96: Buona duplicazione della logica per il percorso fallback.

La stessa logica di auto-popolazione viene applicata anche ai dati di fallback, garantendo coerenza.

app/Support/DeweyAutoPopulator.php (7)

102-113: LGTM!

Metodo semplice e ben strutturato con validazione appropriata.


118-128: Buona gestione del fallback per la locale.

La catena di fallback (I18n → $_ENV → default) è ragionevole. Nota che $_ENV potrebbe non essere popolato in tutti i contesti (es. CLI senza dotenv).


133-147: Costruzione del path sicura.

Il path viene costruito usando chiavi costanti da LOCALE_FILES, evitando rischi di path traversal.


258-284: Creazione ricorsiva dei nodi padre ben implementata.

La ricorsione è sicura dato che i codici Dewey hanno una profondità limitata. L'uso di nomi vuoti per i nodi padre creati automaticamente è una scelta ragionevole.


152-167: Buona gestione degli errori nel parsing JSON.

Il metodo gestisce correttamente gli errori di lettura file e parsing JSON, con logging appropriato.


400-405: Ordinamento corretto per codici Dewey.

L'uso di strcmp funziona bene per i codici Dewey poiché sono sempre formattati con cifre iniziali (es. "800", non "8").


352-363: The project's composer.json specifies a minimum PHP version of ^8.1, which fully supports str_contains(). No compatibility issue exists—this is an appropriate use of PHP 8+ native functions.

.env.example (1)

30-35: Blocco PLUGIN_ENCRYPTION_KEY chiaro e allineato all’uso lato codice

La spiegazione del formato (base64:…), del comando di generazione e dello scopo (cifrare impostazioni sensibili dei plugin) è completa e coerente con la logica di decrittazione nel plugin; non ho ulteriori osservazioni.

app/Views/libri/partials/book_form.php (1)

24-32: Allineamento campo classificazione_dewey in INITIAL_BOOK

La mappatura su $book['classificazione_dewey'] è coerente con il rename di colonna e con l’uso successivo in JS (INITIAL_BOOK.classificazione_dewey). Non vedo problemi qui.

Comment on lines +73 to +93
// Load existing data
$data = self::loadJson($jsonFile);
if ($data === null) {
return false;
}

// Check if code exists and has a name
$existing = self::findEntry($data, $code);
if ($existing !== null && !empty($existing['name'])) {
// Entry exists with name - don't overwrite
return false;
}

// Add or update the entry
$updated = self::addOrUpdateEntry($data, $code, $name);
if (!$updated) {
return false;
}

// Save back to file
return self::saveJson($jsonFile, $data);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

TOCTOU: il pattern read-modify-write non è atomico.

Il flusso attuale legge il file (linea 74), lo modifica (linea 87), e poi lo salva (linea 93). Se due richieste eseguono questo flusso contemporaneamente, una potrebbe sovrascrivere le modifiche dell'altra.

Considera di usare un file lock esclusivo durante l'intero ciclo:

+    private static function withFileLock(string $path, callable $callback): bool
+    {
+        $lockFile = $path . '.lock';
+        $fp = fopen($lockFile, 'c');
+        if (!$fp || !flock($fp, LOCK_EX)) {
+            return false;
+        }
+        try {
+            return $callback();
+        } finally {
+            flock($fp, LOCK_UN);
+            fclose($fp);
+        }
+    }

Poi wrappa la logica di lettura-modifica-scrittura in withFileLock().

Committable suggestion skipped: line range outside the PR's diff.

CSV Import enhancements:
- Add classificazione_dewey to CSV headers and examples
- Add Dewey field to insertBook() and updateBook() queries
- Add enrichment for: anno_pubblicazione, lingua, parole_chiave
- Dewey classification now imported from SBN scraping

CodeRabbit fixes:
- Add LOCK_EX to DeweyAutoPopulator::saveJson() for race condition
- Fix Toast.fire() → window.Toast.fire() with existence checks
- Fix breadcrumb XSS using textContent instead of innerHTML
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
app/Support/DeweyAutoPopulator.php (1)

43-94: TOCTOU race condition tra lettura e scrittura.

Il flusso read-modify-write non è atomico: tra loadJson (linea 74) e saveJson (linea 93), un'altra richiesta concorrente potrebbe modificare il file, causando la perdita di dati.

Sebbene LOCK_EX sia stato aggiunto a saveJson, non protegge l'intero ciclo. Considera l'uso di un file lock esclusivo che copra l'intera operazione come suggerito nella review precedente.

🧹 Nitpick comments (2)
app/Support/DeweyAutoPopulator.php (2)

197-213: Ricorsione potenzialmente profonda in findEntry.

Con strutture Dewey molto annidate, la ricorsione potrebbe causare stack overflow. Per dati di produzione con gerarchie profonde, considera un approccio iterativo con uno stack esplicito.


289-328: Ricorsione multipla in addToParentRecursive.

La funzione chiama se stessa ricorsivamente e può essere invocata da addToParent che è già ricorsiva. Con codici Dewey molto specifici (es. 808.81234), questo potrebbe creare catene di chiamate profonde.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23f4251 and 7af3e5b.

📒 Files selected for processing (3)
  • app/Controllers/CsvImportController.php (11 hunks)
  • app/Support/DeweyAutoPopulator.php (1 hunks)
  • app/Views/libri/partials/book_form.php (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Views/libri/partials/book_form.php (2)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • e (57-60)
installer/index.php (1)
  • __ (20-44)
🔇 Additional comments (11)
app/Support/DeweyAutoPopulator.php (1)

334-366: Buona implementazione della logica di parentela Dewey.

La funzione getParentCode gestisce correttamente tutti i livelli della gerarchia Dewey: classi principali, divisioni, sezioni e decimali. La logica è chiara e ben documentata con i commenti.

app/Controllers/CsvImportController.php (5)

206-207: Buona aggiunta di campi al CSV di esempio.

L'aggiunta di parole_chiave e classificazione_dewey all'header e ai dati di esempio è corretta e ben documentata con commenti esplicativi.


985-995: Validazione Dewey coerente con il backend.

La regex /^[0-9]{3}(\.[0-9]{1,4})?$/ è allineata con DeweyValidator::PATTERN_ANY_CODE, garantendo consistenza tra validazione CSV e altri flussi.


997-1019: Buona estensione dell'enrichment con dati scraped.

L'aggiunta di anno, lingua e parole chiave segue lo stesso pattern degli altri campi, con validazione appropriata (es. verifica lunghezza anno = 4 cifre).


764-830: Parametri insertBook allineati correttamente.

Il type string 'sssssississdiiissssss' (21 caratteri) corrisponde ai 21 parametri bindati. La logica è corretta.


668-739: The bind_param type string and NULL handling are correct.

mysqli properly handles NULL values for all type specifiers, including 'i' for integers. When $anno or other nullable parameters are NULL, mysqli correctly sends SQL NULL to the database. This works as expected in SET clauses (UPDATE context). The type string length (20 characters) correctly corresponds to the 20 parameters being bound.

app/Views/libri/partials/book_form.php (5)

243-285: Buona implementazione UI chip-based per Dewey.

La nuova interfaccia con chip per la classificazione Dewey è ben strutturata:

  • Input nascosto per il valore effettivo
  • Chip visuale con codice e nome
  • Input manuale con validazione
  • Navigazione opzionale per categorie

L'uso di HtmlHelper::e() per il valore iniziale previene XSS lato server.


1035-1039: Regex di validazione allineata con il backend.

La regex ^[0-9]{3}(\.[0-9]{1,4})?$ è ora coerente con DeweyValidator::PATTERN_ANY_CODE lato server, come richiesto nella review precedente.


1124-1150: Toast usage corretto con window.Toast guard.

Le chiamate a Toast ora usano correttamente window.Toast con verifica di esistenza, come richiesto nella review precedente.


2889-2904: Buona integrazione Dewey nello scraping.

L'uso di window.setDeweyCode per impostare la classificazione Dewey dallo scraping è corretto, con fallback appropriato se la funzione non è disponibile.


2940-2957: Toast calls ora protette correttamente.

Tutte le chiamate Toast.fire() in questa sezione sono ora wrapped con if (window.Toast) come richiesto nella review precedente.

README updates:
- Document Dewey Editor plugin (2,106 entries)
- Document SBN Dewey extraction feature
- Document CSV import with Dewey enrichment
- Update pre-installed plugins count (5)

Security fix:
- Fix remaining breadcrumb XSS at line 1326 (use textContent)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

159-168: Chiarire se “Scraping Pro” è preinstallato o solo scaricabile a parte

Nella sezione plugin ci sono due messaggi diversi su Scraping Pro:

  • Linee 159–165: intestazione “Pre-installed plugins (5 included)” con 5 plugin elencati (senza Scraping Pro).
  • Linee 166–168: “Available as separate download: Scraping Pro”.
  • Linee 304–327: “### 5. Dewey Editor …” seguito da “### 6. Scraping Pro (scraping-pro-v1.0.0.zip)” ancora sotto “Plugins (Pre-installed)”.

Questo può far pensare che Scraping Pro sia contemporaneamente preinstallato e disponibile come download separato. Suggerisco di:

  • o spostare il blocco “### 6. Scraping Pro” sotto una sezione distinta per i plugin opzionali / extra,
  • o aggiornare il titolo della sezione (es. “Plugins disponibili”) e specificare esplicitamente vicino a Scraping Pro che non è incluso di default ma va installato a parte.

Così si evita ambiguità su cosa l’utente troverà subito dopo l’installazione.

Also applies to: 304-327

🧹 Nitpick comments (1)
app/Views/libri/partials/book_form.php (1)

1020-1369: Logica Dewey client‑side robusta; possibili hardening minimi opzionali

La nuova initializeDewey() + integrazione con l’import ISBN è in generale ben progettata:

  • La regex ^[0-9]{3}(\.[0-9]{1,4})?$ è allineata al validator PHP (3 cifre + parte decimale opzionale fino a 4 cifre) e coerente con quanto descritto nel README.
  • Breadcrumb e chip usano textContent anziché innerHTML per i valori dinamici (codice/nome), prevenendo XSS anche con codici personalizzati o dati di ritorno “sporchi”.
  • navigateToCode e getCodePath gestiscono sia codici standard (es. 133.5, 004.6782) sia codici non presenti nel JSON, con fallback che mostra comunque il codice scelto e chiama setDeweyCode.
  • L’import ISBN usa window.setDeweyCode quando disponibile e ha un fallback che scrive direttamente nel campo nascosto, quindi il flusso resta funzionante anche se l’inizializzazione Dewey fallisce.

Suggerimenti opzionali (non bloccanti):

  1. Guard sugli elementi DOM
    All’inizio di initializeDewey() potresti aggiungere un controllo:

    if (!container || !breadcrumb || !hidden || !manualInput || !addBtn || !chipContainer || !chipCode || !chipName || !chipRemove) {
        console.warn('Dewey UI elements missing, skipping initializeDewey');
        return;
    }

    Questo rende la funzione più resiliente nel caso la partial venga riutilizzata/semplificata in futuro.

  2. Segnalazione errori API Dewey all’utente
    Nei catch di loadLevel e del blocco che chiama /api/dewey/search, potresti opzionalmente usare window.Toast (o uno Swal) con la stringa già definita 'Errore caricamento classificazione Dewey', così l’utente capisce perché la navigazione non carica.

  3. Difensività su codici non numerici in getCodePath
    Se dal backend o da plugin esterni arrivasse accidentalmente un valore non conforme (es. stringhe miste), potresti aggiungere un controllo leggero all’inizio di getCodePath che esce subito se !/^[0-9]{3}(\.[0-9]+)?$/.test(code), evitando path “strani” e chiamate API inutili.

Se non avete casi reali problematici, questi punti possono tranquillamente restare come miglioramenti futuri.

Also applies to: 2893-2909

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7af3e5b and 337f7fe.

📒 Files selected for processing (2)
  • README.md (6 hunks)
  • app/Views/libri/partials/book_form.php (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Views/libri/partials/book_form.php (1)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • e (57-60)
🔇 Additional comments (2)
app/Views/libri/partials/book_form.php (2)

29-29: Mapping classificazione_dewey e nuova sezione UI coerenti con la migrazione

  • L’uso di classificazione_dewey in $initialData e nell’<input type="hidden" ...> è coerente con la rinomina del campo DB e con le nuove API Dewey.
  • Il valore iniziale è correttamente sanificato tramite HtmlHelper::e, quindi nessun rischio di injection lato HTML.
  • La nuova UX (chip selezionato + input manuale + navigazione opzionale a livelli) è ben integrata con il campo nascosto e non altera il payload POST esistente.

Non vedo problemi di correttezza o regressioni su questa parte.

Also applies to: 243-285


1793-1797: Uso di window.Toast ora uniforme e con fallback sicuro

Nei vari punti in cui mostri notifiche leggere (suggerimento collocazione, prezzo importato, esito positivo/negativo import ISBN):

  • L’accesso avviene sempre tramite if (window.Toast) window.Toast.fire(...), quindi non ci sono più rischi di ReferenceError se SweetAlert/Toast non sono inizializzati.
  • Le notifiche sono additive (non bloccano il flusso principale) e i messaggi sono coerenti con la UX esistente.

Questa parte è allineata alle linee guida già adottate nel resto del file.

Also applies to: 2745-2750, 2944-2949, 2956-2961

- Fix XSS vulnerability in Dewey Editor onclick handlers (escape quotes)
- Add strict comparison in AuthorRepository::mergeAuthors in_array call
- Add primary_id validation in merge API endpoints
- Add try-catch error handling in merge API endpoints
- Add English translations for new error messages
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
app/Routes/web.php (2)

928-995: Endpoint duplicati/merge autori: logica solida, ma attenzione a input duplicati e JSON non valido

Positivo:

  • /api/autori/duplicates usa AuthorRepository::findDuplicates() e restituisce JSON strutturato (success + duplicates), protetto da AdminAuthMiddleware.
  • /api/autori/merge valida che ci siano almeno 2 ID, che l’eventuale primary_id appartenga alla lista e incapsula mergeAuthors() in un try/catch con log e risposta JSON coerente.

Due punti da rafforzare:

  1. ID duplicati nell’array ids
    Anche se la UI dovrebbe già evitarlo, lato API è meglio difendersi: con un body tipo {"ids":[5,5]} oggi passi il controllo count($ids) < 2 e deleghi al repository, che nella versione attuale può cancellare l’autore primario (vedi commento su mergeAuthors). Ti suggerisco di normalizzare qui:
-        $ids = array_map('intval', $data['ids'] ?? []);
+        $ids = array_values(array_unique(array_map('intval', $data['ids'] ?? [])));
@@
-        if (count($ids) < 2) {
+        if (count($ids) < 2) {
             $response->getBody()->write(json_encode([
                 'success' => false,
                 'error' => __('Seleziona almeno 2 autori da unire')
             ]));
             return $response->withHeader('Content-Type', 'application/json')->withStatus(400);
         }

in modo che l’endpoint rifiuti richieste con meno di 2 ID distinti.

  1. JSON non valido gestito come “nessun input” (già segnalato in un review precedente)
    Con il pattern:
$data = $request->getParsedBody();
if (!$data) {
    $data = json_decode((string) $request->getBody(), true) ?? [];
}

un JSON malformato finisce trattato come array vuoto e produce l’errore “Seleziona almeno 2 autori da unire”, che non è molto esplicito. Se vuoi essere più rigoroso, potresti distinguere esplicitamente:

$raw = (string) $request->getBody();
$data = json_decode($raw, true);
if ($data === null && json_last_error() !== JSON_ERROR_NONE) {
    // 400: JSON non valido
}

e usare getParsedBody() solo per form/urlencoded. Non è bloccante, ma renderebbe gli errori di input più chiari.

Infine, coerentemente con altri endpoint, potresti valutare di usare new CsrfMiddleware($app->getContainer()) al posto di new CsrfMiddleware() se il middleware fa affidamento al container (stessa osservazione emersa in un review precedente).


1574-1633: Endpoint merge editori: stessi rischi/funzionalità del merge autori + duplicazione codice

Il flusso di /api/editori/merge è parallelo a quello degli autori:

  • parsing body (ids, primary_id, new_name),
  • validazione numero minimo di ID e appartenenza di primary_id,
  • chiamata a PublisherRepository::mergePublishers() con try/catch,
  • opzionale rinomina dell’editore primario,
  • risposte JSON localizzate.

Questo è positivo e allinea bene UX e comportamento tra autori/editore.

Due punti da sistemare:

  1. Lo stesso problema di ID duplicati nel repository
    PublisherRepository::mergePublishers() ha la stessa struttura di mergeAuthors() (sort + array_shift senza deduplica). Con un input come {"ids":[5,5]} il repository può scegliere 5 come primario e poi cancellarlo nel loop. Andrebbe applicato un fix analogo a quello suggerito per mergeAuthors() (deduplica, verifica coerenza primaryId, costruzione esplicita di $duplicateIds).

  2. Deduplica anche lato route
    Come per gli autori, puoi rendere l’endpoint più robusto normalizzando e deduplicando gli ID prima della validazione:

-        $ids = array_map('intval', $data['ids'] ?? []);
+        $ids = array_values(array_unique(array_map('intval', $data['ids'] ?? [])));
@@
-        if (count($ids) < 2) {
+        if (count($ids) < 2) {
             $response->getBody()->write(json_encode([
                 'success' => false,
                 'error' => __('Seleziona almeno 2 editori da unire')
             ]));
             return $response->withHeader('Content-Type', 'application/json')->withStatus(400);
         }
  1. Duplicazione logica tra /api/autori/merge e /api/editori/merge (già notata in un review precedente)
    Il blocco è praticamente identico (parse/validazione/try‑catch/rinomina/risposte). Per futura manutenibilità varrebbe la pena valutare un helper condiviso (es. MergeHelper::handleMergeRequest(...)) come suggerito nel commento precedente, così ogni miglioramento (logging, validazioni, gestione errori) andrebbe toccato una sola volta.

Anche qui, valuta se uniformare a new CsrfMiddleware($app->getContainer()) per coerenza con altri endpoint.

🧹 Nitpick comments (2)
app/Models/AuthorRepository.php (2)

175-266: Ricerca autore robusta; possibili micro‑ottimizzazioni

La pipeline di findByName() (match esatto normalizzato → match sull’originale → match case‑insensitive → fuzzy con LIKE + AuthorNormalizer::match) è ben strutturata e migliora molto il matching rispetto alla versione precedente, senza caricare tutta la tabella in memoria.

Un paio di note non bloccanti:

  • La query LOWER(nome) = ? può impedire l’uso di un indice su nome; se la tabella crescesse molto conviene prevedere in futuro una colonna indicizzata normalizzata o una collation/campo dedicato al search form.
  • toSearchForm() richiama già internamente normalize(), quindi la normalizzazione iniziale è in parte ridondante ma trascurabile come costo.

Per la release attuale va bene così.


268-303: findDuplicates corretta ma O(n) in memoria

L’algoritmo che normalizza tutti i nomi e raggruppa in PHP funziona ed è semplice da mantenere, ed è ragionevole per un uso amministrativo sporadico.

Se il numero di autori diventasse molto alto, potresti in futuro:

  • aggiungere una colonna nome_normalized indicizzata e far fare il GROUP BY direttamente in SQL, oppure
  • introdurre una vista/materializzazione periodica dei duplicati.

Non è però un blocco per questa PR.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e26b13a and f254899.

📒 Files selected for processing (4)
  • app/Models/AuthorRepository.php (3 hunks)
  • app/Routes/web.php (3 hunks)
  • locale/en_US.json (32 hunks)
  • storage/plugins/dewey-editor/views/editor.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/Models/AuthorRepository.php (3)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • decode (14-21)
app/Support/AuthorNormalizer.php (4)
  • AuthorNormalizer (16-213)
  • normalize (30-65)
  • toSearchForm (79-98)
  • match (140-162)
app/Models/PublisherRepository.php (1)
  • findByName (176-186)
app/Routes/web.php (3)
app/Models/AuthorRepository.php (4)
  • AuthorRepository (8-373)
  • findDuplicates (273-303)
  • mergeAuthors (315-361)
  • update (132-173)
app/Models/PublisherRepository.php (3)
  • update (131-164)
  • PublisherRepository (8-237)
  • mergePublishers (198-236)
app/Controllers/DeweyApiController.php (1)
  • DeweyApiController (10-392)
storage/plugins/dewey-editor/views/editor.php (2)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
installer/index.php (1)
  • __ (20-44)
🔇 Additional comments (5)
app/Models/AuthorRepository.php (2)

107-130: Normalizzazione su create coerente con la strategia di matching

La decodifica via HtmlHelper::decode e la normalizzazione con AuthorNormalizer::normalize($nome) in create() sono allineate con l’uso di findByName e riducono bene il rischio di duplicati (“Cognome, Nome” vs “Nome Cognome”). Nessun problema funzionale evidente.


144-173: Normalizzazione su update mantiene coerenza dei dati

Applicare AuthorNormalizer::normalize($nome) anche in update() mantiene il campo nome in formato canonico e rende le ricerche consistenti nel tempo. Anche i plugin hook prima/dopo il salvataggio restano intatti. Tutto ok.

app/Routes/web.php (1)

1518-1520: Nuovi endpoint Dewey children/search ben integrati

L’aggiunta di /api/dewey/children e /api/dewey/search aggancia direttamente i metodi del DeweyApiController ed è protetta da AdminAuthMiddleware, in linea con gli altri endpoint Dewey admin‑only. Nessun problema evidente lato routing.

locale/en_US.json (1)

726-741: Nuove stringhe per Editor Dewey / merge: coerenza e copertura OK

Le stringhe aggiunte per titolo editor, azioni di merge autori/editori, note SRU e inserimento/manuale/validazione Dewey risultano coerenti con il flusso implementato in editor.php, hanno traduzioni EN chiare e non introducono chiavi duplicate rispetto al resto del file. Buon allineamento lessicale (es. “Classi principali / Divisioni / Sezioni / Decimali”, messaggi di errore e help‑text).

Also applies to: 1119-1120, 3328-3355

storage/plugins/dewey-editor/views/editor.php (1)

304-363: Logica editor, gestione cambi e hardening XSS complessivamente solidi

La struttura generale dell’editor è pulita: dati in memoria (deweyData), rendering ricorsivo dell’albero con azioni per nodo, tracking delle modifiche tramite hasChanges + beforeunload, salvataggio via fetch con CSRF in header e body, import/merge con selezione modalità, backup/restore con conferme dedicate e barra statistiche aggiornata da updateStats.

Dal punto di vista sicurezza lato client:

  • Le parti dinamiche di markup (node.code, node.name, errori server, liste backup) passano attraverso escapeHtml() prima di finire in innerHTML o in testo dei modali.
  • Gli errori di import vengono mostrati con text in SweetAlert, non come HTML, evitando riflessi XSS da contenuto JSON importato.
  • I pulsanti che richiamano funzioni globali (DeweyEditor.*) usano solo codici Dewey controllati (cifre + punto), riducendo il rischio di injection negli onclick.

Non vedo problemi evidenti di correttezza o race lato client; il codice appare manutenibile e coerente con il resto dell’admin.

Also applies to: 423-507, 538-581, 582-649, 651-731, 732-813, 819-831

…nslations

- Deduplicate IDs at route level before validation (array_unique)
- Use separate $duplicateIds array in repository merge methods to ensure
  primary ID is never included in the delete loop
- Add empty duplicateIds check after filtering
- Add 27 missing Dewey Editor translation keys to en_US.json

Addresses CodeRabbit review feedback about potential data corruption
when duplicate IDs (e.g., [5,5]) are passed to merge endpoints.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (4)
app/Models/PublisherRepository.php (1)

188-246: Gestire esplicitamente il caso di $primaryId non presente nell’elenco

La logica di merge con dedup degli ID e transazione è solida e previene la cancellazione involontaria dell’editore primario in presenza di duplicati. Tuttavia, se viene passato un $primaryId che non è contenuto in $publisherIds, il metodo lo ignora e sceglie automaticamente l’ID più basso: per evitare merge inattesi, sarebbe più sicuro trattare questo scenario come input non valido e restituire null (o lanciare un’eccezione) invece di procedere.

Esempio di variante più esplicita:

if ($primaryId !== null) {
    if (!\in_array($primaryId, $publisherIds, true)) {
        return null; // primaryId incoerente con la lista
    }
    $duplicateIds = array_values(array_filter($publisherIds, fn($id) => $id !== $primaryId));
} else {
    sort($publisherIds);
    $primaryId = array_shift($publisherIds);
    $duplicateIds = $publisherIds;
}
app/Models/AuthorRepository.php (1)

315-370: mergeAuthors risolve il bug sui duplicati, ma valida meglio $primaryId

La deduplicazione iniziale di $authorIds e l’uso di UPDATE IGNORE + cleanup su libri_autori risolvono il bug critico precedentemente segnalato con ID duplicati che potevano portare alla cancellazione dell’autore primario. Rimane però lo stesso spigolo di mergePublishers: se viene passato un $primaryId non presente in $authorIds, il metodo lo ignora e sceglie l’ID più basso, comportamento poco trasparente per il chiamante; suggerirei di trattare esplicitamente questo caso come input non valido (ritornando null subito) invece di procedere, e mantenere la semantica “auto-primario più basso” solo quando $primaryId è null. La stessa logica può essere applicata simmetricamente in PublisherRepository::mergePublishers().

app/Routes/web.php (2)

937-996: Merge autori: buona robustezza generale, ma JSON non valido e CSRF restano migliorabili

Ottimo:

  • Dedup degli ID prima del merge (evita di cancellare il primario).
  • Validazione che primary_id sia negli ids.
  • try/catch con logging ed errori JSON strutturati.

Restano due punti aperti (già notati nei commenti precedenti):

  1. JSON non valido gestito in modo fuorviante
    Se il body è JSON invalido, json_decode(... ) ?? [] produce $data = [] e l’utente riceve solo “Seleziona almeno 2 autori da unire”. Meglio restituire 400 con un errore esplicito di JSON non valido, senza procedere con il merge.

  2. Incoerenza nell’uso di CsrfMiddleware
    Qui usi new CsrfMiddleware() mentre molte altre rotte admin usano new CsrfMiddleware($app->getContainer()). Per coerenza e per eventuale dipendenza dal container, converrebbe allinearsi.

Una possibile correzione (semplificata) potrebbe essere:

-        $data = $request->getParsedBody();
-        if (!$data) {
-            $data = json_decode((string) $request->getBody(), true) ?? [];
-        }
+        $data = $request->getParsedBody();
+        if ($data === null) {
+            $rawBody = (string) $request->getBody();
+            if ($rawBody !== '') {
+                $decoded = json_decode($rawBody, true);
+                if (json_last_error() !== JSON_ERROR_NONE || !is_array($decoded)) {
+                    $response->getBody()->write(json_encode([
+                        'success' => false,
+                        'error' => __('Body JSON non valido')
+                    ]));
+                    return $response->withHeader('Content-Type', 'application/json')->withStatus(400);
+                }
+                $data = $decoded;
+            } else {
+                $data = [];
+            }
+        } elseif (!is_array($data)) {
+            $response->getBody()->write(json_encode([
+                'success' => false,
+                'error' => __('Formato del body non valido')
+            ]));
+            return $response->withHeader('Content-Type', 'application/json')->withStatus(400);
+        }
@@
-    })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware());
+    })->add(new CsrfMiddleware($app->getContainer()))->add(new AdminAuthMiddleware());

Questo rende l’errore molto più chiaro per il client e allinea l’uso del middleware CSRF agli altri endpoint admin.


1575-1635: Merge editori: stessa logica dell’endpoint autori, da rifinire e possibilmente DRY

Questa rotta replica correttamente le migliorie viste per gli autori (dedup ID, validazione primary_id, try/catch, messaggi localizzati). Restano però gli stessi punti da sistemare:

  1. Gestione JSON non valido
    Stessa dinamica dell’endpoint autori: JSON invalido → $data = [] → messaggio “Seleziona almeno 2 editori da unire” invece di un chiaro errore di parsing. Conviene applicare la stessa correzione proposta per /api/autori/merge.

  2. CsrfMiddleware senza container
    Anche qui, sostituire new CsrfMiddleware() con new CsrfMiddleware($app->getContainer()) per coerenza.

  3. Duplicazione con /api/autori/merge
    La logica è quasi identica; per manutenzione futura sarebbe utile estrarla in un helper condiviso (come il MergeHelper suggerito in un commento precedente) parametrizzato su tipo entità e repository, così da avere un solo punto in cui aggiornare validazioni, gestione eccezioni, ecc.

Suggerimento minimo per questo endpoint (analogo a quello sugli autori):

-        $data = $request->getParsedBody();
-        if (!$data) {
-            $data = json_decode((string) $request->getBody(), true) ?? [];
-        }
+        // Vedi logica di parsing robusta proposta per /api/autori/merge
@@
-    })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware());
+    })->add(new CsrfMiddleware($app->getContainer()))->add(new AdminAuthMiddleware());

E, quando hai tempo, valutare l’estrazione in un helper condiviso per ridurre la duplicazione tra merge autori/editori.

🧹 Nitpick comments (2)
app/Models/AuthorRepository.php (1)

268-303: findDuplicates OK per uso admin, ma tenere d’occhio la scalabilità

findDuplicates() legge tutti gli autori, normalizza i nomi e restituisce solo i gruppi duplicati: per una funzione di manutenzione admin questa scansione completa è accettabile, ma se il numero di autori crescesse molto potresti valutare in futuro una colonna normalizzata indicizzata (es. nome_normalized) per evitare di ricalcolare tutto ad ogni chiamata.

app/Routes/web.php (1)

928-935: Endpoint duplicati autori: implementazione chiara

L’endpoint usa correttamente AuthorRepository::findDuplicates() e restituisce una risposta JSON semplice, protetta da AdminAuthMiddleware. Per coerenza con altri endpoint admin potresti in futuro aggiungere anche un piccolo rate‑limit, ma non è bloccante.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f254899 and 9ddaa9a.

📒 Files selected for processing (4)
  • app/Models/AuthorRepository.php (3 hunks)
  • app/Models/PublisherRepository.php (1 hunks)
  • app/Routes/web.php (3 hunks)
  • locale/en_US.json (32 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Routes/web.php (3)
app/Models/AuthorRepository.php (5)
  • AuthorRepository (8-383)
  • findDuplicates (273-303)
  • mergeAuthors (315-371)
  • getById (26-40)
  • update (132-173)
app/Models/PublisherRepository.php (4)
  • getById (25-39)
  • update (131-164)
  • PublisherRepository (8-247)
  • mergePublishers (198-246)
app/Controllers/DeweyApiController.php (1)
  • DeweyApiController (10-392)
🔇 Additional comments (4)
locale/en_US.json (1)

192-3381: Nuove stringhe Dewey/merge coerenti e senza problemi evidenti

Le nuove chiavi per merge autori/editori, editor Dewey, messaggi di validazione/backup e impostazioni HTTPS/HSTS sono tradotte in modo chiaro e coerente con il resto del file; non vedo chiavi duplicate o errori che possano rompere il JSON o l’UI.

app/Models/AuthorRepository.php (2)

103-117: Normalizzazione del nome autore allineata all’obiettivo anti-duplicati

Applicare AuthorNormalizer::normalize() sia in create() che in update() garantisce che i nomi vengano salvati in formato canonico (es. “Nome Cognome” invece di “Cognome, Nome”), migliorando la deduplica tra sorgenti diverse; l’implementazione è coerente con il resto del repository.

Also applies to: 145-154


175-266: findByName ben strutturato (match normalizzato + fallback + fuzzy limitato)

La nuova findByName() copre in modo ordinato i casi di match: prima sul nome normalizzato, poi sul nome originale (per dati legacy), quindi su confronto case-insensitive e infine su fuzzy match con filtro LIKE e limite 50 candidati, evitando di caricare tutta la tabella in memoria; non vedo problemi di correttezza o sicurezza nella logica attuale.

app/Routes/web.php (1)

1519-1521: Nuovi endpoint Dewey: coerenti con le API esistenti

/api/dewey/children e /api/dewey/search sono registrati in modo lineare verso DeweyApiController e protetti da AdminAuthMiddleware, in linea con gli altri endpoint Dewey JSON‑based. Nulla da segnalare.

- Add explicit JSON parsing error handling in merge endpoints
- Return clear "Invalid JSON format" error instead of ambiguous message
- Remove unnecessary $app->getContainer() from CsrfMiddleware (103 calls)
  The middleware has no constructor and ignores the parameter
- Add translation for new error message
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
app/Routes/web.php (1)

1587-1659: Duplicazione di codice significativa tra endpoint di merge.

L'endpoint /api/editori/merge (linee 1588-1659) duplica quasi interamente la logica dell'endpoint /api/autori/merge (linee 937-1008). Le uniche differenze sono:

  • Classe repository: PublisherRepository vs AuthorRepository
  • Metodo chiamato: mergePublishers vs mergeAuthors
  • Messaggi di errore: "editori" vs "autori"

Come già segnalato nelle revisioni precedenti, questa duplicazione massiva (circa 72 linee) viola il principio DRY. La soluzione consigliata di estrarre la logica comune in un helper o metodo controller condiviso (come MergeHelper::handleMergeRequest) migliorerebbe significativamente la manutenibilità.

🧹 Nitpick comments (1)
app/Routes/web.php (1)

1531-1535: Inconsistenza nell'uso dei middleware di autenticazione.

Gli endpoint Dewey mostrano un'inconsistenza nell'uso dei middleware:

  • Linee 1528-1533: Utilizzano AdminAuthMiddleware()
  • Linea 1535: Utilizza AuthMiddleware(['admin'])

Se questi middleware sono funzionalmente equivalenti per gli admin, considera di utilizzare lo stesso per coerenza. Se hanno comportamenti diversi, aggiungi un commento per chiarire la differenza.

Applica questa diff per maggiore coerenza:

-    $app->post('/api/dewey/reseed', [DeweyApiController::class, 'reseed'])->add(new CsrfMiddleware())->add(new AuthMiddleware(['admin']));
+    $app->post('/api/dewey/reseed', [DeweyApiController::class, 'reseed'])->add(new CsrfMiddleware())->add(new AdminAuthMiddleware());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da5a422 and f18e182.

📒 Files selected for processing (2)
  • app/Routes/web.php (54 hunks)
  • locale/en_US.json (32 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • locale/en_US.json
🔇 Additional comments (1)
app/Routes/web.php (1)

928-1008: Ottimo lavoro sul miglioramento della gestione degli errori e della validazione!

Gli endpoint di gestione dei duplicati e merge degli autori mostrano significativi miglioramenti rispetto alle preoccupazioni precedenti:

  1. Validazione JSON robusta: Controllo esplicito di json_last_error() con risposta 400 appropriata (linee 944-949)
  2. Deduplicazione ID: Prevenzione dell'eliminazione accidentale del primario quando vengono passati ID duplicati (linea 956)
  3. Validazione primary_id: Verifica che l'ID primario richiesto sia presente nell'array degli ID (linee 969-975)
  4. Gestione eccezioni: Blocco try-catch per catturare errori imprevisti con logging e risposta 500 (linee 977-1006)
  5. Codici di stato HTTP appropriati: 400 per input non validi, 500 per errori server

Questi miglioramenti aumentano significativamente la robustezza e l'affidabilità dell'endpoint.

- Add EAN column to book queries in AuthorRepository and PublisherRepository
- Update author/publisher detail views to show EAN when ISBN13 is NULL
- Fix installer language selection flags to have equal width (45px)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
app/Views/editori/scheda_editore.php (1)

308-308: Fallback ISBN13→EAN corretto e coerente

La nuova espressione ($libro['isbn13'] ?? $libro['ean'] ?? 'N/D') rende più utile il campo, riutilizzando l’EAN quando l’ISBN13 manca, e l’output è correttamente sanificato con HtmlHelper::e.

Come miglioramento opzionale, se nel DB usate stringhe vuote invece di NULL, potresti rendere il fallback ancora più robusto trattando '' come valore mancante, ad esempio:

<?php
$isbn = trim((string)($libro['isbn13'] ?? ''));
if ($isbn === '') {
    $isbn = trim((string)($libro['ean'] ?? ''));
}
if ($isbn === '') {
    $isbn = 'N/D';
}
echo HtmlHelper::e($isbn);

Valuta anche se l’etichetta dovrebbe indicare chiaramente che può mostrare l’EAN (es. “ISBN/EAN”) per maggiore trasparenza verso l’utente.

app/Views/autori/scheda_autore.php (1)

276-276: Coerenza del fallback ISBN13→EAN anche nella scheda autore

La logica isbn13 ?? ean ?? 'N/D' è corretta, coerente con la vista editore e l’uso di HtmlHelper::e garantisce output sicuro.

Se vuoi evitare duplicazione di logica tra viste e gestire meglio casi come stringhe vuote, potresti estrarre una piccola utility (es. formatIsbn($libro)) riusabile nelle due pagine, applicando lì il trimming e il fallback completo.

app/Models/PublisherRepository.php (1)

198-246: Implementazione solida con corretta gestione delle transazioni.

La deduplicazione degli ID (linea 201) e l'uso del confronto strict (linea 208) prevengono correttamente il bug critico di cancellazione dell'entità primaria segnalato in passato per AuthorRepository.

Considerazione opzionale sulla type-safety: se $publisherIds contiene stringhe (es. da JSON) e $primaryId è int, il confronto strict potrebbe non trovare corrispondenza. Potresti normalizzare i tipi all'ingresso:

-        $publisherIds = array_values(array_unique($publisherIds));
+        $publisherIds = array_values(array_unique(array_map('intval', $publisherIds)));
app/Models/AuthorRepository.php (2)

273-303: Nota sulla scalabilità di findDuplicates.

Il metodo carica tutti gli autori in memoria per il raggruppamento. Per database molto grandi questo potrebbe diventare lento, ma essendo una funzione amministrativa eseguita occasionalmente, è un trade-off accettabile.

Se in futuro la tabella autori crescesse significativamente, potresti considerare l'aggiunta di una colonna indicizzata nome_normalized per eseguire il raggruppamento direttamente in SQL.


315-371: Problemi segnalati in precedenza risolti correttamente.

La deduplicazione degli ID (linea 318), il confronto strict in in_array (linea 325), e la gestione della tabella di relazione libri_autori con UPDATE IGNORE + DELETE sono implementati correttamente.

Per coerenza con il suggerimento fatto per PublisherRepository::mergePublishers(), considera la normalizzazione dei tipi:

-        $authorIds = array_values(array_unique($authorIds));
+        $authorIds = array_values(array_unique(array_map('intval', $authorIds)));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f18e182 and 2e9b87b.

📒 Files selected for processing (5)
  • app/Models/AuthorRepository.php (4 hunks)
  • app/Models/PublisherRepository.php (2 hunks)
  • app/Views/autori/scheda_autore.php (1 hunks)
  • app/Views/editori/scheda_editore.php (1 hunks)
  • installer/steps/step0.php (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • installer/steps/step0.php
🧰 Additional context used
🧬 Code graph analysis (2)
app/Views/editori/scheda_editore.php (1)
app/Support/HtmlHelper.php (1)
  • HtmlHelper (6-285)
app/Models/AuthorRepository.php (3)
app/Support/HtmlHelper.php (2)
  • HtmlHelper (6-285)
  • decode (14-21)
app/Support/AuthorNormalizer.php (4)
  • AuthorNormalizer (16-213)
  • normalize (30-65)
  • toSearchForm (79-98)
  • match (140-162)
app/Models/PublisherRepository.php (1)
  • findByName (176-186)
🔇 Additional comments (5)
app/Models/PublisherRepository.php (1)

43-54: LGTM!

L'aggiunta della colonna l.ean è coerente con la modifica analoga in AuthorRepository::getBooksByAuthorId().

app/Models/AuthorRepository.php (4)

63-75: LGTM!

Aggiunta coerente della colonna l.ean alla query.


113-117: Buona pratica: normalizzazione alla creazione.

La normalizzazione del nome autore prima dell'inserimento previene efficacemente i duplicati provenienti da fonti con formati diversi (es. "Levi, Primo" da SBN vs "Primo Levi" da Google Books).


151-154: LGTM!

Normalizzazione coerente anche in fase di aggiornamento.


230-263: Ottimo miglioramento delle performance per il fuzzy match.

Rispetto all'implementazione precedente che caricava tutti gli autori in memoria, questa versione usa query LIKE per filtrare i candidati nel database prima del matching PHP. L'uso di LIMIT 50 mantiene il set di candidati gestibile.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
installer/steps/step0.php (1)

89-98: Implementazione corretta della bandiera inglese.

L'SVG della Union Jack è implementato correttamente con clipPath e pattern appropriati. I colori (#00247d blu, #cf142b rosso) e le proporzioni sono corrette. Gli attributi di accessibilità sono appropriati.

Suggerimento opzionale: L'id del clipPath (clip-flag-en) potrebbe essere reso più univoco (es. clip-flag-en-installer-step0) per evitare potenziali conflitti se il codice venisse riutilizzato in altri contesti. Nel contesto attuale dell'installer, questo non è un problema pratico.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e9b87b and fbc9be4.

📒 Files selected for processing (1)
  • installer/steps/step0.php (2 hunks)
🔇 Additional comments (1)
installer/steps/step0.php (1)

63-67: Implementazione corretta della bandiera italiana.

L'SVG inline è ben strutturato con i colori corretti (#009246 verde, #fff bianco, #ce2b37 rosso) e le proporzioni appropriate. Gli attributi di accessibilità aria-hidden="true" e focusable="false" sono appropriati per un elemento decorativo.

…nd CsvImportController

- LibriController: autori_new now checks findByName() before creating
  Handles 'Levi, Primo' vs 'Primo Levi' as same author in both store() and update()

- CsvImportController: getOrCreateAuthor() now uses AuthorRepository
  Previously did exact match, now uses normalized findByName()

Fixes author duplication when importing from different sources (SBN, Google Books, etc.)
that return author names in different formats.
- Italian locale: Only SBN (JSON API) enabled by default, no foreign servers
- English locale: SBN + European SRU servers (K10plus, SUDOC, DNB)

This prevents author name format inconsistencies and duplicates.
Foreign SRU servers may return 'Levi, Primo' while SBN returns consistent
format. Combined with AuthorNormalizer, this ensures clean author data.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/Controllers/CsvImportController.php (2)

571-599: Buon refactoring per utilizzare AuthorRepository, ma il pattern di ritorno è fragile.

L'uso di AuthorRepository::findByName() per la normalizzazione è corretto e previene i duplicati. Tuttavia, il metodo create() restituisce già l'ID del nuovo autore, che viene scartato. Il chiamante poi usa $db->insert_id per recuperare l'ID.

Questo funziona nell'attuale contesto transazionale, ma è un pattern fragile. Se in futuro venisse aggiunta un'altra operazione di insert tra create() e l'uso di $db->insert_id, il comportamento sarebbe errato.

Considera di restituire direttamente l'ID dal metodo create():

-        $authRepo->create([
+        $newId = $authRepo->create([
             'nome' => $name,
             'pseudonimo' => '',
             'data_nascita' => null,
             'data_morte' => null,
             'nazionalita' => '',
             'biografia' => '',
             'sito_web' => ''
         ]);

-        return 'created';
+        return ['id' => $newId, 'created' => true];

Questo richiederebbe un aggiornamento del chiamante per gestire il nuovo formato di ritorno.


699-748: Campo Dewey aggiunto correttamente a updateBook.

Il binding del nuovo campo classificazione_dewey è implementato correttamente e il tipo 's' è appropriato per un campo stringa. Tuttavia, la stringa dei tipi bind_param contiene due incoerenze pre-esistenti:

  • Posizione 9: $pagine è castato a (int) ma usa tipo 's'
  • Posizione 10: $genreId è parametro ?int ma usa tipo 's' (dovrebbe essere 'i')

Queste sono problematiche di coerenza di tipo pre-esistenti non introdotte da questo cambiamento.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fbc9be4 and 3b1553c.

📒 Files selected for processing (3)
  • app/Controllers/CsvImportController.php (12 hunks)
  • app/Controllers/LibriController.php (6 hunks)
  • installer/classes/Installer.php (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/Controllers/LibriController.php (1)
app/Models/AuthorRepository.php (1)
  • AuthorRepository (8-383)
app/Controllers/CsvImportController.php (1)
app/Models/AuthorRepository.php (1)
  • AuthorRepository (8-383)
🔇 Additional comments (16)
installer/classes/Installer.php (5)

1294-1301: Documentazione chiara della configurazione locale-specifica.

Il docblock aggiornato spiega efficacemente la logica delle impostazioni predefinite basate sulla locale: installazioni italiane utilizzano solo SBN (API JSON), mentre quelle inglesi includono server SRU europei aggiuntivi. La motivazione sulla coerenza del formato dei nomi autore è ben documentata.


1325-1362: Configurazione dei server corretta, subordinata alla correzione del bug della locale.

Le configurazioni dei server per installazioni inglesi (K10plus, SUDOC, DNB) sono complete e ben strutturate. Gli URL HTTPS, i timeout di 15 secondi e le mappature degli indici ISBN sono appropriati. Tuttavia, questa logica non funzionerà correttamente finché non viene risolto il bug di confronto della locale alla riga 1320.


1367-1367: LGTM! Configurazione SBN ben documentata.

I commenti aggiornati chiariscono che SBN (Servizio Bibliotecario Nazionale) è abilitato di default per tutte le locale, il che è appropriato data l'eccellente copertura per libri italiani e molti titoli internazionali. Il timeout di 15 secondi è coerente con gli altri server.

Also applies to: 1380-1384


1169-1169: No issues found. The dewey-editor plugin directory exists at storage/plugins/dewey-editor with all required files properly configured, including plugin.json, DeweyEditorPlugin.php with the registerRoutes() method implemented, and supporting files. The installation code at lines 1283-1285 and the plugin reference at line 1169 are valid.


8-48: Rimozione della tabella classificazione confermata.

La costante EXPECTED_TABLES ora contiene 39 tabelle invece di 40, coerentemente con la rimozione della tabella classificazione. Il commento alla riga 689 documenta correttamente che i dati Dewey sono ora memorizzati in file JSON nella directory data/dewey/ (levels.json, dewey_completo_en.json, dewey_completo_it.json). Lo script di migrazione migrate_0.3.0.sql è presente e gestisce la transizione.

app/Controllers/LibriController.php (6)

257-257: Correzione typo classificazione_dewey applicata correttamente.

La rinomina del campo da classificazione_dowey a classificazione_dewey è coerente con la migrazione del database e le modifiche correlate nel codebase.


478-502: Ottima implementazione della normalizzazione autori.

L'utilizzo di findByName() per rilevare autori esistenti prima della creazione previene efficacemente i duplicati causati da formati di nome diversi (es. "Cognome, Nome" vs "Nome Cognome"). Il AuthorRepository viene istanziato una sola volta prima del ciclo, mantenendo l'efficienza.


704-704: Rinomina coerente con il metodo store.


974-998: Gestione autori coerente con il metodo store.

La logica di normalizzazione è identica a quella del metodo store(), garantendo comportamento uniforme durante la creazione e l'aggiornamento dei libri.


1828-1828: Campo Dewey aggiornato correttamente per le etichette PDF portrait.


1970-1970: Campo Dewey aggiornato correttamente per le etichette PDF landscape.

app/Controllers/CsvImportController.php (5)

206-208: Intestazioni CSV estese correttamente con il campo Dewey.

L'aggiunta di classificazione_dewey alle intestazioni è coerente con lo schema del database aggiornato.


232-234: Dati di esempio validi per la classificazione Dewey.

I valori di esempio (853 per narrativa italiana, 851.1 per poesia italiana) dimostrano il formato corretto della classificazione Dewey.


773-865: Campo Dewey aggiunto correttamente a insertBook.

L'inserimento del campo classificazione_dewey è implementato correttamente, coerente con la struttura di updateBook. La generazione delle copie fisiche e il ricalcolo della disponibilità rimangono inalterati.


994-1028: Eccellente implementazione dell'arricchimento dati con validazione.

L'arricchimento dei dati scraped è ben strutturato:

  • Classificazione Dewey: la regex /^[0-9]{3}(\.[0-9]{1,4})?$/ valida correttamente il formato standard (es. 853, 851.1, 530.1234)
  • Anno pubblicazione: validazione corretta per anni a 4 cifre
  • Lingua e parole chiave: arricchimento appropriato

La logica rispetta correttamente la priorità dei dati CSV (arricchisce solo campi vuoti) e include logging utile per il debugging.


256-258: Buon esempio di Dewey vuoto per dimostrare l'arricchimento da scraping.

Il commento indica chiaramente che il campo vuoto verrà popolato dallo scraping se abilitato, utile per documentare il comportamento agli utenti.

- Create MergeHelper class to eliminate ~140 lines of duplicate code
  between /api/autori/merge and /api/editori/merge endpoints

- Fix middleware inconsistency: Dewey reseed endpoint now uses
  AdminAuthMiddleware() like all other Dewey endpoints

- Add type safety: mergeAuthors() and mergePublishers() now normalize
  IDs to integers with array_map('intval', ...) to prevent strict
  comparison issues when IDs come from JSON as strings
- Installer.php: Use str_starts_with() for locale check to handle
  potential 'it_IT' format variations more robustly

- CsvImportController: Return array {id, created} from getOrCreateAuthor()
  instead of relying on fragile $db->insert_id pattern. This prevents
  potential bugs if other insert operations occur between create() and
  reading insert_id
- data_it_IT.sql: Remove English templates, keep only Italian (15 templates)
- data_en_US.sql: Add English CMS pages and 15 email templates
- EmailService: Add PLACEHOLDER_ALIASES for bilingual placeholder support
  - English operators can use {{book_title}}, {{first_name}}, etc.
  - Italian operators can use {{libro_titolo}}, {{nome}}, etc.
  - Both are replaced correctly regardless of template language
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
app/Support/EmailService.php (2)

17-59: Mappa alias chiara; attenzione alla manutenzione futura dei nomi placeholder

La costante PLACEHOLDER_ALIASES è ben strutturata (gruppi logici, nomi consistenti) e il commento spiega chiaramente l’intento di permettere placeholder in inglese nei template mantenendo i nomi italiani come “interni”.

Il rovescio della medaglia è che qui si introducono parecchie stringhe “magiche”: se in futuro rinomini un placeholder italiano usato nei template (es. da libro_titolo a qualcos’altro), bisognerà ricordarsi di aggiornare anche questa costante, altrimenti l’alias inglese smetterà di funzionare in silenzio.

Suggerimento (opzionale):

  • Valutare un piccolo test/unit o di integrazione che verifichi che tutti i valori di PLACEHOLDER_ALIASES corrispondano a placeholder effettivamente supportati/attesi dall’applicazione, così qualsiasi rinomina futura fa fallire i test invece di rompere gli alias in produzione.

446-463: Gestione alias in replaceVariables corretta; possibile micro‑ottimizzazione

La logica è coerente con il contratto dei commenti:

  • Si assume che le chiavi di $variables rimangano i nomi placeholder italiani (“canonici”).
  • Per ogni chiave, sostituisci sia {{nome_italiano}} che, se esiste, l’alias inglese associato ({{english_alias}}).
  • L’escaping HTML rimane invariato e continua a proteggere da HTML injection, con l’eccezione esplicita di RAW_HTML_VARIABLES come verify_section.

Quindi il cambiamento è retro‑compatibile e abilita l’uso di placeholder inglesi solo lato template, senza toccare il codice chiamante.

L’unico aspetto migliorabile è l’uso di array_search dentro il foreach, che è O(n) per ogni variabile; con pochi alias è trascurabile, ma si può semplificare e rendere più esplicito invertendo una volta sola la mappa:

-    private function replaceVariables(string $content, array $variables): string {
-        foreach ($variables as $key => $value) {
+    private function replaceVariables(string $content, array $variables): string {
+        static $italianToEnglish = null;
+        if ($italianToEnglish === null) {
+            $italianToEnglish = array_flip(self::PLACEHOLDER_ALIASES);
+        }
+
+        foreach ($variables as $key => $value) {
             $replacement = in_array($key, self::RAW_HTML_VARIABLES, true)
                 ? (string)$value
                 : htmlspecialchars((string)$value, ENT_QUOTES | ENT_HTML5, 'UTF-8');
 
             // Replace the Italian placeholder
             $content = str_replace('{{' . $key . '}}', $replacement, $content);
 
-            // Also replace the English alias if one exists
-            $englishAlias = array_search($key, self::PLACEHOLDER_ALIASES, true);
-            if ($englishAlias !== false) {
-                $content = str_replace('{{' . $englishAlias . '}}', $replacement, $content);
-            }
+            // Also replace the English alias if one exists
+            if (isset($italianToEnglish[$key])) {
+                $content = str_replace('{{' . $italianToEnglish[$key] . '}}', $replacement, $content);
+            }
         }
         return $content;
     }

È solo un micro‑refactor: se preferisci tenere il codice più semplice, la versione attuale è comunque leggibile e sufficiente per il numero ridotto di alias.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee80527 and fd939e8.

📒 Files selected for processing (1)
  • app/Support/EmailService.php (2 hunks)

- Italian locales (it_IT, it_CH) → fallback to it_IT
- All other locales (fr_FR, de_DE, es_ES, etc.) → fallback to en_US
- This ensures international users get English templates with English
  placeholders, making the app more accessible globally
- Cache inverted PLACEHOLDER_ALIASES map using static variable
- Replace O(n) array_search with O(1) isset lookup
- Addresses CodeRabbit review feedback
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
app/Support/EmailService.php (2)

16-59: Alias di placeholder chiari e ben strutturati

La mappatura EN→IT è coerente con i placeholder usati nei template e rende esplicito cosa è “interno” (italiano) e cosa è alias (inglese). Non vedo problemi di correttezza; al massimo valuterei in futuro un test automatico che verifichi che la lista resti allineata ai template predefiniti.


451-454: Rendere la sostituzione dei placeholder simmetrica tra chiavi IT ed EN

Ora i placeholder vengono sempre sostituiti se la chiave nel $variables è italiana (es. libro_titolo), e in quel caso vengono rimpiazzate sia {{libro_titolo}} che {{book_title}}. Se però qualcuno passasse variabili con chiavi inglesi (es. book_title), verrebbero sostituiti solo i placeholder inglesi, non l’eventuale forma italiana.

Per rendere il comportamento più robusto e indipendente da come vengono costruiti gli array di variabili, puoi usare entrambi i mapping (EN→IT e IT→EN) e sostituire tutte le forme equivalenti in un colpo solo:

     private function replaceVariables(string $content, array $variables): string {
         // Cache the inverted map (Italian → English) for O(1) lookups
         static $italianToEnglish = null;
         if ($italianToEnglish === null) {
             $italianToEnglish = array_flip(self::PLACEHOLDER_ALIASES);
         }

         foreach ($variables as $key => $value) {
-            $replacement = in_array($key, self::RAW_HTML_VARIABLES, true)
-                ? (string)$value
-                : htmlspecialchars((string)$value, ENT_QUOTES | ENT_HTML5, 'UTF-8');
-
-            // Replace the Italian placeholder
-            $content = str_replace('{{' . $key . '}}', $replacement, $content);
-
-            // Also replace the English alias if one exists
-            if (isset($italianToEnglish[$key])) {
-                $content = str_replace('{{' . $italianToEnglish[$key] . '}}', $replacement, $content);
-            }
+            $replacement = in_array($key, self::RAW_HTML_VARIABLES, true)
+                ? (string)$value
+                : htmlspecialchars((string)$value, ENT_QUOTES | ENT_HTML5, 'UTF-8');
+
+            // Costruisci l'elenco di placeholder equivalenti (italiano + alias inglese)
+            $placeholders = [$key];
+            if (isset(self::PLACEHOLDER_ALIASES[$key])) {
+                // chiave inglese → interno italiano
+                $placeholders[] = self::PLACEHOLDER_ALIASES[$key];
+            }
+            if (isset($italianToEnglish[$key])) {
+                // chiave italiana → alias inglese
+                $placeholders[] = $italianToEnglish[$key];
+            }
+
+            foreach (array_unique($placeholders) as $placeholder) {
+                $content = str_replace('{{' . $placeholder . '}}', $replacement, $content);
+            }
         }
         return $content;
     }

Così, indipendentemente dal fatto che chi costruisce $variables usi chiavi italiane o inglesi, entrambe le forme di placeholder nel template vengono sempre coperte.

Also applies to: 456-460, 467-473

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd939e8 and 0edbbd0.

📒 Files selected for processing (1)
  • app/Support/EmailService.php (3 hunks)
🔇 Additional comments (1)
app/Support/EmailService.php (1)

222-231: Logica di fallback locale aderente ai requisiti

Il comportamento di fallback (varianti italiane → it_IT, altri → en_US, poi template di default) è chiaro e corrisponde a quanto descritto nel PR. Non vedo edge case critici in questa logica, dato che I18n sembra restituire codici completi tipo it_IT / en_US.

- Fix Branding::fullLogo() to respect user-configured logo
- Add responsive width for login/register forms (lg:max-w-2xl)
- Add header-left wrapper for grouped brand+nav on large screens
- Make hero search bar responsive (900px lg, 1000px xl)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
app/Support/Branding.php (1)

17-21: Considera l'estrazione della logica duplicata.

Il blocco di codice per il recupero del logo configurato è duplicato tra logo() e fullLogo(). Per migliorare la manutenibilità, considera l'estrazione di questa logica in un metodo privato.

Applica questa refactoring per eliminare la duplicazione:

+    /**
+     * Get user-configured logo if it exists.
+     */
+    private static function getConfiguredLogo(): string
+    {
+        $configured = (string)ConfigStore::get('app.logo', '');
+        if ($configured !== '' && self::assetExists($configured)) {
+            return $configured;
+        }
+        return '';
+    }
+
     public static function logo(): string
     {
-        $configured = (string)ConfigStore::get('app.logo', '');
-
-        if ($configured !== '' && self::assetExists($configured)) {
-            return $configured;
+        $configured = self::getConfiguredLogo();
+        if ($configured !== '') {
+            return $configured;
         }

         return self::assetExists(self::DEFAULT_LOGO) ? self::DEFAULT_LOGO : '';
     }

     public static function fullLogo(): string
     {
-        // First check for user-configured logo
-        $configured = (string)ConfigStore::get('app.logo', '');
-        if ($configured !== '' && self::assetExists($configured)) {
-            return $configured;
+        $configured = self::getConfiguredLogo();
+        if ($configured !== '') {
+            return $configured;
         }

         // Fall back to default full logo
         if (self::assetExists(self::DEFAULT_FULL_LOGO)) {
             return self::DEFAULT_FULL_LOGO;
         }

         // Final fallback to small logo
         return self::assetExists(self::DEFAULT_LOGO) ? self::DEFAULT_LOGO : '';
     }

Also applies to: 33-36

public/assets/main.css (1)

5439-5449: Responsive width di hero search e auth form OK, ma .lg:max-w-2xl è definita due volte

  • I nuovi breakpoint per .hero-search-input-group (900px su min-width: 1024px, 1000px su min-width: 1280px) migliorano la leggibilità del search bar sui monitor grandi senza toccare il comportamento mobile.
  • L’utility .lg:max-w-2xl è definita sia nel blocco generato Tailwind (senza !important) sia nella sezione “CUSTOM RESPONSIVE UTILITIES” con !important. Questo garantisce la larghezza desiderata per login/registrazione anche in presenza di altre classi, ma introduce una leggera duplicazione da tenere a mente per futuri refactor (eventuali modifiche vanno fatte in entrambi i punti).

Also applies to: 5445-5449, 5481-5493, 6433-6435, 688-693

frontend/css/input.css (1)

640-650: Breakpoint di ricerca e utility lg:max-w-2xl coerenti con il bundle compilato

I nuovi breakpoint per .hero-search-input-group e l’override manuale di .lg\:max-w-2xl (42rem) in input.css mantengono la sorgente Tailwind allineata a quanto già presente in main.css, garantendo search bar e form di autenticazione ben dimensionati su viewport grandi. In futuro, se spostate questa logica nel config Tailwind, ricordate di rimuovere queste regole manuali per evitare duplicazioni.

Also applies to: 682-693

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0edbbd0 and bf492a1.

📒 Files selected for processing (6)
  • app/Support/Branding.php (1 hunks)
  • app/Views/auth/login.php (1 hunks)
  • app/Views/auth/register.php (1 hunks)
  • app/Views/frontend/layout.php (2 hunks)
  • frontend/css/input.css (2 hunks)
  • public/assets/main.css (33 hunks)
🔇 Additional comments (6)
app/Support/Branding.php (1)

32-36: Miglioramento della coerenza tra i metodi.

La nuova logica rende fullLogo() coerente con logo(), permettendo agli utenti di configurare un logo personalizzato che venga rispettato anche nelle sezioni hero/installer.

app/Views/frontend/layout.php (1)

729-740: Wrapper header-left coerente con il layout responsive

L’uso di .header-left con display: contents su schermi piccoli e flex da 1200px in su raggruppa correttamente brand + navigazione senza introdurre regressioni apparenti nel layout o nel comportamento mobile/desktop.

Also applies to: 1309-1330

public/assets/main.css (2)

772-788: Override dark-mode per il tema “auto” coerente con il resto delle variabili

Il blocco @media (prefers-color-scheme: dark) per [data-swal2-theme=auto] replica correttamente la logica già usata per il tema dark esplicito, mantenendo consistenza su colori di background, testo e progress bar in modalità scura.


3247-3563: Espansione delle utility Tailwind-like ben allineata al design system

Le nuove utility (.z-10, .-mt-2, .contents, altezze/min-height custom, varianti di colore bg-*/border-*/ring-*, stati hover:/focus:, helper dark-mode e helper responsivi sm:/md:) sono coerenti con la nomenclatura Tailwind e con le variabili di design già definite; non vedo problemi di sintassi o di compatibilità evidente con il resto del foglio.

Also applies to: 4104-4107, 4155-4157, 4240-4247, 4271-4273, 4302-4305, 4984-4987, 5542-5657, 5706-5733, 5803-5805, 5869-5871, 5987-5992, 6025-6028, 6084-6087, 6183-6185, 6239-6245, 6317-6319, 6331-6333

app/Views/auth/register.php (1)

26-26: Larghezza container registrazione più comoda sui monitor grandi

L’aggiunta di lg:max-w-2xl al wrapper principale allinea la pagina di registrazione al layout del login e migliora la fruibilità su viewport ampi, mantenendo invariato il comportamento mobile.

app/Views/auth/login.php (1)

27-29: Allineamento larghezza login/registrazione tramite lg:max-w-2xl

La modifica del container a max-w-md lg:max-w-2xl w-full mx-auto rende il form di login più arioso su desktop e coerente con la pagina di registrazione, senza impatti sul flusso o sulla logica del form.

- Remove dark: Tailwind classes from register.php
- Override SweetAlert2 dark theme to force light mode
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
public/assets/main.css (3)

3247-3249: Aggiunta di classi utility Tailwind che dovrebbero essere auto-generate

Le nuove classi .z-10, .-mt-2, .contents, .h-[46px], .min-h-[400px] (e altre nelle sezioni successive) sembrano essere utility standard di Tailwind CSS che dovrebbero essere generate automaticamente durante il build tramite JIT compiler.

Consiglio: Se il file CSS viene generato da Tailwind v3+, queste classi dovrebbero già essere presenti automaticamente negli output generati. Se sono state aggiunte manualmente, potrebbe indicare:

  • Configurazione Tailwind incompleta o non correttamente ottimizzata
  • Build process non configurato per generare tutte le utility necessarie

Verificare il file di configurazione Tailwind (tailwind.config.js) e il processo di build.

Also applies to: 3306-3308, 3456-3458, 3534-3536, 3561-3563


4104-4107: Espansione incoerente delle classi colore utility

Sono state aggiunte nuove classi colore (.border-red-400, .bg-amber-500, .bg-indigo-200, .bg-indigo-50, .bg-purple-600, .bg-slate-500, .text-purple-700) sparse nel file invece di essere raggruppate logicamente.

Consiglio: Se usate Tailwind CSS con PurgeCSS/JIT, questi colori dovrebbero essere generati automaticamente solo quando utilizzati nel markup. L'aggiunta manuale di classi sparse suggerisce possibili gap nella configurazione di Tailwind. Questo potrebbe gonfiare il file CSS con utility inutilizzate.

Also applies to: 4155-4158, 4240-4247, 4271-4274, 4302-4305, 4815-4818


4565-4567: Aggiunta di utility .text-start

La nuova classe .text-start (allineamento testo inizio, responsive per RTL) è una buona aggiunta per supporto i18n/l10n, in particolare per lingue RTL. Tuttavia:

Consiglio: Verificare che il progetto utilizzi effettivamente l'attributo dir="rtl" nel markup quando necessario, altrimenti questa utility non avrà effetto pratico. Se il supporto RTL è pianificato, aggiungere una nota di documentazione nel file o in un commento CSS.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf492a1 and 20aae79.

📒 Files selected for processing (3)
  • app/Views/auth/register.php (14 hunks)
  • frontend/css/input.css (3 hunks)
  • public/assets/main.css (34 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/Views/auth/register.php
  • frontend/css/input.css

Comment on lines +772 to +788
@media (prefers-color-scheme: dark) {
[data-swal2-theme=auto] {
--swal2-dark-theme-black: #19191a;
--swal2-dark-theme-white: #e1e1e1;
--swal2-background: var(--swal2-dark-theme-black);
--swal2-color: var(--swal2-dark-theme-white);
--swal2-footer-border-color: #555;
--swal2-input-background: color-mix(in srgb, var(--swal2-dark-theme-black), var(--swal2-dark-theme-white) 10%);
--swal2-validation-message-background: color-mix(
in srgb,
var(--swal2-dark-theme-black),
var(--swal2-dark-theme-white) 10%
);
--swal2-validation-message-color: var(--swal2-dark-theme-white);
--swal2-timer-progress-bar-background: rgba(255, 255, 255, 0.7);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

wc -l public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 93


🏁 Script executed:

# Get first section around lines 772-788
sed -n '765,795p' public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 1162


🏁 Script executed:

# Get second section around lines 5324-5334
sed -n '5317,5345p' public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 746


🏁 Script executed:

# Check context before first section to understand its purpose
sed -n '750,775p' public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 1090


🏁 Script executed:

# Check for any comments or context explaining the second section
sed -n '5315,5330p' public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 507


🏁 Script executed:

# Verify if there are any other SweetAlert2 theme sections
rg -n "data-swal2-theme" public/assets/main.css

Repository: fabiodalez-dev/Pinakes

Length of output: 160


CSS cascade conflict: Second SweetAlert2 theme section overrides and disables dark mode

The dark mode styling at lines 773–788 is completely overridden by the later light mode section at lines 5326–5334. Both target @media (prefers-color-scheme: dark) with [data-swal2-theme=auto], but the second section forces light colors (#fff, #545454) that take precedence in the cascade. This defeats the first section's dark theme and ignores user dark mode preferences.

Remove the second section (lines 5324–5334) to allow the dark theme styling to apply when users prefer dark mode.

🤖 Prompt for AI Agents
In public/assets/main.css around lines 772–788, the dark-mode SweetAlert2 rules
for [data-swal2-theme=auto] are being negated by a later conflicting block at
lines 5324–5334 that forces light colors; remove the later @media
(prefers-color-scheme: dark) / [data-swal2-theme=auto] block at lines 5324–5334
(or replace it with the correct light-mode @media (prefers-color-scheme: light)
variant) so the initial dark-theme variables take effect for users who prefer
dark mode.

Comment on lines +5451 to +5461
@media (min-width: 1024px) {
.hero-search-input-group {
max-width: 900px !important;
}
}

@media (min-width: 1280px) {
.hero-search-input-group {
max-width: 1000px !important;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Media query duplicate per hero search bar

Sono state aggiunte tre media query @media (min-width: 1024px) e @media (min-width: 1280px) per .hero-search-input-group. Tuttavia, esiste già una regola base (linee 5439-5449) con breakpoint mobile (max-width: 768px).

Problema: La struttura è incoerente. Tutte e tre le media query dovrebbero essere raggruppate insieme o organizzate gerarchicamente con i breakpoint mobile-first.

Consiglio: Consolidare tutti i breakpoint di .hero-search-input-group in un'unica sezione usando approccio mobile-first di Tailwind:

.hero-search-input-group {
  max-width: 700px;  /* mobile default */
}
@media (min-width: 1024px) {
  .hero-search-input-group { max-width: 900px; }
}
@media (min-width: 1280px) {
  .hero-search-input-group { max-width: 1000px; }
}
🤖 Prompt for AI Agents
public/assets/main.css around lines 5439-5461: there are duplicated media
queries for .hero-search-input-group (min-width:1024px and 1280px) separated
from the base/mobile rule at lines 5439-5449, creating incoherent structure;
consolidate the rules by defining a mobile-first base max-width for
.hero-search-input-group near line 5439 and then place the min-width:1024px and
min-width:1280px media queries directly after it in ascending order, remove
unnecessary !important flags, and ensure the breakpoints only override the
earlier base value.

Comment on lines +5492 to +5504

/* ========================================
CUSTOM RESPONSIVE UTILITIES
Manual overrides for Tailwind classes not
generated by PurgeCSS/JIT
======================================== */

/* Large screen max-width for auth forms (login/register) */
@media (min-width: 1024px) {
.lg\:max-w-2xl {
max-width: 42rem !important; /* 672px */
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Nuova sezione: Custom Responsive Utilities

È stata aggiunta una sezione "CUSTOM RESPONSIVE UTILITIES" che duplica regole .lg\:max-w-2xl già definite altrove nel file (linea 6438-6440).

Problema: Ridondanza. Le stesse classi sono definite sia in questa sezione manuale che nelle media query responsive standard di Tailwind.

Consiglio: Rimuovere questa sezione e lasciare che Tailwind generi automaticamente le utility responsive. Se sono necessarie override personalizzate, includerle nel file tailwind.config.js o in un file di override CSS dedicato, non nel file CSS compilato.

🤖 Prompt for AI Agents
In public/assets/main.css around lines 5492 to 5504, there is a duplicated
"CUSTOM RESPONSIVE UTILITIES" block that redefines .lg\:max-w-2xl already
declared at lines ~6438-6440; remove this duplicate block from the compiled CSS
so you don't override Tailwind-generated utilities twice. If you need custom
responsive values, move them into tailwind.config.js under theme/extend or into
a dedicated source override CSS (not the compiled public/assets/main.css), then
rebuild so the compiled CSS contains only one authoritative definition.

- BookDataMerger: Add priority protection for keywords field
  - When _keywords_priority flag is set, skip merging from other sources
  - Fix mergeArrayField() to handle existing string values (comma-separated)
  - Add _keywords_priority to SKIP_FIELDS

- SbnClient: Map BID to numero_inventario for form auto-fill
  - Add 'SBN-' prefix to codiceIdentificativo in parseFullRecord()
  - Add same mapping in parseBriefRecord()

This ensures LibreriaUniversitaria keywords (via scraping-pro) take
absolute priority over other sources like Google Books.
…945-)

Adds hierarchical path: 828 > 828.9 > 828.91 > 828.914
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
storage/plugins/z39-server/classes/SbnClient.php (2)

431-434: Considera l'estrazione della logica di mapping BID.

Il mapping numero_inventario = 'SBN-' . $book['_sbn_bid'] è duplicato in parseFullRecord e parseBriefRecord. Potresti estrarre questa logica in un metodo helper per migliorare la manutenibilità.

Esempio:

private function mapBidToInventoryNumber(string $bid): string
{
    return 'SBN-' . $bid;
}

Poi usarlo in entrambi i metodi:

-    if (!empty($book['_sbn_bid'])) {
-        $book['numero_inventario'] = 'SBN-' . $book['_sbn_bid'];
-    }
+    if (!empty($book['_sbn_bid'])) {
+        $book['numero_inventario'] = $this->mapBidToInventoryNumber($book['_sbn_bid']);
+    }

Also applies to: 500-503


565-593: Refactor to eliminate duplicate normalization logic using AuthorNormalizer.

The cleanAuthorName method duplicates the logic from AuthorNormalizer::normalize(). Since author names are normalized again in AuthorRepository::create() (line 116) and AuthorRepository::update() (line 153), and the repository's findByName() method (lines 184-266) already handles fuzzy matching across both "Name Surname" and "Surname, Name" formats, consider consolidating this logic. The normalization change is safe—the repository's multi-stage matching (exact, case-insensitive, fuzzy word-order matching) prevents data integrity issues. However, this creates redundant calls: SbnClient::cleanAuthorName()AuthorRepository::create()AuthorNormalizer::normalize().

app/Support/BookDataMerger.php (1)

51-54: Buon design per la protezione con priorità, considera di aggiornare la documentazione.

L'introduzione di PRIORITY_PROTECTED_FIELDS rende il meccanismo di priorità estensibile e dichiarativo. Tuttavia, il docblock della classe (linee 6-12) non menziona questa funzionalità. Considera di aggiungere una breve descrizione del meccanismo di priorità nella documentazione della classe.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20aae79 and fce0eea.

📒 Files selected for processing (2)
  • app/Support/BookDataMerger.php (3 hunks)
  • storage/plugins/z39-server/classes/SbnClient.php (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
storage/plugins/z39-server/classes/SbnClient.php (1)
app/Support/AuthorNormalizer.php (1)
  • match (140-162)
🔇 Additional comments (4)
storage/plugins/z39-server/classes/SbnClient.php (2)

316-328: LGTM! Gestione corretta del caso edge '000'.

L'uso del controllo esplicito invece di empty() è fondamentale per permettere codici Dewey validi come '000'. La logica di estrazione ed enrichment è corretta.


417-425: LGTM! Estrazione Dewey implementata correttamente.

L'integrazione con extractDeweyData() è pulita e il commento spiega bene l'uso del prefisso _ per i campi interni.

app/Support/BookDataMerger.php (2)

49-50: Corretta l'aggiunta del flag di priorità a SKIP_FIELDS.

L'inclusione di _keywords_priority in SKIP_FIELDS è appropriata: impedisce che il flag venga elaborato nel ciclo principale di merge (linee 81-125) e viene gestito esplicitamente nella logica di protezione della priorità (linee 100-119).


202-205: Ottimo miglioramento per la gestione delle stringhe esistenti.

L'aggiunta della conversione da stringa ad array per i valori esistenti (che rispecchia la logica già presente per i nuovi valori alle linee 207-210) è un buon esempio di programmazione difensiva. Garantisce un comportamento coerente del merge indipendentemente dal formato di input (array o stringa separata da virgole).

Comment on lines +100 to 119
// Handle array fields - check for priority protection first
if (in_array($key, self::ARRAY_FIELDS, true)) {
// Check if this field has priority protection
if (in_array($key, self::PRIORITY_PROTECTED_FIELDS, true)) {
$priorityKey = "_{$key}_priority";
// If existing data has priority flag set, keep existing value (don't merge)
if (!empty($merged[$priorityKey])) {
continue;
}
// If new data has priority flag, use new value exclusively
if (!empty($new[$priorityKey])) {
$merged[$key] = $newValue;
$merged[$priorityKey] = $new[$priorityKey];
continue;
}
}
// Normal merge for array fields
$merged[$key] = self::mergeArrayField($existingValue, $newValue);
continue;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Chiarire i controlli del flag di priorità e gestire i conflitti.

La logica di protezione della priorità funziona correttamente, ma presenta due aree di miglioramento:

  1. Controlli dei flag non espliciti (linee 106, 110): L'uso di !empty() per verificare i flag di priorità non è esplicito riguardo ai valori attesi. Quali valori indicano "priorità impostata"? true, 1, "1", qualsiasi valore truthy? Un controllo più esplicito migliorerebbe la chiarezza.

  2. Gestione dei conflitti di priorità: Se sia i dati esistenti che quelli nuovi hanno il flag di priorità impostato, il codice mantiene silenziosamente quello esistente (linea 106). Considera di registrare un avviso o documentare esplicitamente questo comportamento per evitare confusione durante il debug.

Applica questa diff per rendere i controlli più espliciti:

-                if (!empty($merged[$priorityKey])) {
+                // If existing data has priority flag explicitly set to true, keep existing value
+                if (isset($merged[$priorityKey]) && $merged[$priorityKey] === true) {
                     continue;
                 }
                 // If new data has priority flag, use new value exclusively
-                if (!empty($new[$priorityKey])) {
+                if (isset($new[$priorityKey]) && $new[$priorityKey] === true) {
                     $merged[$key] = $newValue;
                     $merged[$priorityKey] = $new[$priorityKey];
                     continue;
                 }
🤖 Prompt for AI Agents
In app/Support/BookDataMerger.php around lines 100 to 119, the priority-flag
checks use loose !empty() checks which are ambiguous about which values
constitute "priority" and the case where both existing and new flags are set is
silently ignored; change the checks to explicit boolean/coercion tests (e.g.,
normalize flags via strict comparisons or use filter_var($value,
FILTER_VALIDATE_BOOLEAN) / casting to (bool) or checking for '1'/'true' as the
project expects) so it's clear which inputs count as priority, and add explicit
handling for the conflict case (either prefer existing or new deterministically
and emit a log warning or comment when both flags are true) so the behavior is
documented and observable during debugging.

Comment on lines +633 to +670
/**
* Extract Dewey classification code and name from SBN format
*
* @param string $deweyStr Format: "808.81 (12.) RACCOLTE DI PIU LETTERATURE. POESIA"
* @return array{code: string, name: string|null}|null Dewey data or null
*/
private function extractDeweyData(string $deweyStr): ?array
{
$deweyStr = trim($deweyStr);

if ($deweyStr === '') {
return null;
}

// Extract code, optional edition, and name
// Format: "808.81 (12.) RACCOLTE DI PIU LETTERATURE. POESIA"
// Or: "808.81 RACCOLTE DI PIU LETTERATURE. POESIA"
if (preg_match('/^(\d{3}(?:\.\d+)?)\s*(?:\([^)]+\)\s*)?(.+)?$/u', $deweyStr, $match)) {
$code = $match[1];
$name = isset($match[2]) ? trim($match[2]) : null;

// Clean up the name - remove trailing dots, normalize case
if ($name) {
$name = rtrim($name, '. ');
// Convert from ALL CAPS to Title Case if needed
if ($name === mb_strtoupper($name, 'UTF-8')) {
$name = mb_convert_case(mb_strtolower($name, 'UTF-8'), MB_CASE_TITLE, 'UTF-8');
}
}

return [
'code' => $code,
'name' => $name
];
}

return null;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

La conversione Title Case potrebbe non gestire correttamente gli acronimi.

La logica di parsing del Dewey è robusta e gestisce correttamente i vari formati. Tuttavia, la conversione da ALL CAPS a Title Case alle righe 658-660 potrebbe trasformare acronimi in modo errato (es. "UNESCO BIBLIOTECA" → "Unesco Biblioteca").

Per la maggior parte dei casi questo va bene, ma considera di preservare parole brevi in maiuscolo o implementare una whitelist di acronimi comuni se questo diventa un problema:

// Opzionale: lista acronimi da preservare
private const ACRONYMS = ['UNESCO', 'NATO', 'USA', 'UK', 'EU'];

private function titleCasePreservingAcronyms(string $text): string
{
    $words = explode(' ', $text);
    $result = [];
    
    foreach ($words as $word) {
        if (in_array(strtoupper($word), self::ACRONYMS)) {
            $result[] = strtoupper($word);
        } else {
            $result[] = mb_convert_case(mb_strtolower($word, 'UTF-8'), MB_CASE_TITLE, 'UTF-8');
        }
    }
    
    return implode(' ', $result);
}
🤖 Prompt for AI Agents
In storage/plugins/z39-server/classes/SbnClient.php around lines 633 to 670, the
current ALL CAPS -> Title Case conversion will incorrectly downcase acronyms
(e.g. "UNESCO BIBLIOTECA" → "Unesco Biblioteca"); replace the inline
mb_convert_case logic with a small helper that preserves known acronyms or short
all-caps words by: add a private ACRONYMS constant array of common acronyms,
implement a titleCasePreservingAcronyms(string $text): string that splits the
string into words, for each word if strtoupper($word) is in ACRONYMS or the
original word is all-uppercase and short (e.g. <=3 chars) keep
strtoupper($word), otherwise convert to title case using
mb_convert_case(mb_strtolower(...), MB_CASE_TITLE, 'UTF-8'), and call that
helper instead of the current conversion.

@fabiodalez-dev fabiodalez-dev deleted the dewey-editor-v2 branch December 5, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants