Contributor roles as entities + pseudonym search & display (#237)#254
Open
fabiodalez-dev wants to merge 17 commits into
Open
Contributor roles as entities + pseudonym search & display (#237)#254fabiodalez-dev wants to merge 17 commits into
fabiodalez-dev wants to merge 17 commits into
Conversation
Illustrators, translators, curators and the new colorist role are now first-class author entities via libri_autori.ruolo, and authors are findable/shown by pseudonym. Pseudonym (#237 part 2): - SearchController::authors matches nome OR pseudonimo, so an author is findable by pen name; new App\Support\AuthorName centralizes the display form 'Pseudonimo (Nome)' (PHP + SQL), used on the list/detail/dashboard queries and the book form chips. No pseudonym → just the real name (unchanged). Contributor roles (#237 part 1): - Schema: add 'colorista' to libri_autori.ruolo (migrate_0.7.36.sql, guarded/idempotent). - Book form: illustrator/translator/curator/colorist are Choices.js entity pickers (generic initContributorPicker) with the same /api/search/autori autocomplete as authors, replacing the free-text inputs. They post <role>_ids[] / <role>_new[]. - Save: BookRepository::syncAuthors → syncContributors writes every role; the controller resolves each role's picker (existing + new names) to author ids. - getById returns contributors grouped by role; detail views (admin + public) show them grouped, with App\Support\ContributorRoles for localized role labels. The public author page already lists a contributor's books, so an illustrator now appears in the authors system. - The legacy free-text columns are kept and only touched when explicitly provided (array_key_exists guard), so nothing is nulled before the backfill. Upgrade: App\Support\ContributorBackfill converts existing free-text illustratore/traduttore/curatore into entities once, invoked from MaintenanceService::runAll() (cron + admin login), guarded by a system_settings marker and INSERT IGNORE — the .sql migration runner can't do the split/find-or-create. Verified end-to-end in the browser: pseudonym search + 'Leo (Luiz…)' display, no-pseudonym shows plain name, saving an illustrator entity persists a libri_autori illustratore row and shows on the book detail + the illustrator's author page. Migration unit test 19/19 (enum ALTER idempotent, splitNames, real backfill run in a rolled-back transaction, drift guards); PHPStan L5 full-tree clean; locale parity 6535x4.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLa versione 0.7.36 normalizza i ruoli contributori come relazioni autore, abilita ricerca e visualizzazione tramite pseudonimo, migra i dati legacy e aggiorna API, plugin, schema, interfacce, localizzazioni e verifiche di installazione e release. ChangesRuoli contributori, pseudonimi e persistenza
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Form as book_form.php
participant Search as SearchController::authors
participant DB as Database autori
participant Save as LibriController::store/update
participant Repo as BookRepository::syncContributors
Form->>Search: ricerca nome o pseudonimo
Search->>DB: esegue la ricerca autore
DB-->>Search: restituisce label formattate
Search-->>Form: aggiorna le opzioni Choices.js
Form->>Save: invia ID e nuovi nomi per ruolo
Save->>Repo: sincronizza i contributori
Repo->>DB: aggiorna libri_autori
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Models/BookRepository.php`:
- Around line 1073-1110: Update syncContributors() to mirror syncPublishers():
prepare and validate the INSERT statement before executing the DELETE, and
handle preparation failure through SecureLogger::error() with a
\RuntimeException rather than returning silently. Preserve the contributor
synchronization flow while ensuring an INSERT preparation failure cannot wipe
existing contributors.
In `@app/Support/AuthorName.php`:
- Around line 48-52: Aggiorna la costruzione dell’espressione in `AuthorName`
per gestire `nome` NULL o vuoto come fa `display()`: usa `COALESCE` per evitare
confronti NULL e un `CASE` annidato che restituisca solo il nome quando è vuoto,
senza generare `Pseudonimo ()`; conserva invece il formato pseudonimo con nome
quando entrambi sono valorizzati e differenti.
In `@app/Views/libri/partials/book_form.php`:
- Around line 274-283: Sostituisci gli utilizzi di HtmlHelper::e() nella view
del ciclo contributorFields con htmlspecialchars(..., ENT_QUOTES, 'UTF-8') per
l’output di $meta['label'] e $meta['help']; mantieni invariati il markup e gli
altri dati del form.
- Around line 2275-2359: Update initContributorPicker to apply the same
_onEnterKey override used by authorsChoice and publishersChoice, ensuring Enter
creates the currently typed contributor text instead of selecting a highlighted
Choices.js result. Prefer extracting the existing Enter-handling logic into a
shared helper and reuse it for all relevant pickers without changing their other
behavior.
In `@app/Views/libri/scheda_libro.php`:
- Around line 504-527: Replace every App\Support\HtmlHelper::e() call in the
contributor rendering block with htmlspecialchars(..., ENT_QUOTES, 'UTF-8'),
including labels, author display names, and fallback text, while preserving the
existing output and iteration behavior.
In `@locale/fr_FR.json`:
- Line 6535: Update the locale entry for “Curatori dell'opera (autori veri, con
autocomplete)” to use the catalog’s established “Directeurs de publication”
terminology, preserving the parenthetical meaning and autocomplete wording.
In `@tests/migration-0.7.36.unit.php`:
- Around line 108-157: Update Test C to create a throwaway book inside the
transaction using the existing database/repository APIs, setting at least
titolo, then use its inserted ID as bookId instead of selecting an arbitrary row
from libri. Remove the dependency on pre-existing non-deleted books while
preserving the subsequent illustratore seeding and backfill assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0cdb7ea-9bf6-434c-be11-90e7e4557824
📒 Files selected for processing (22)
README.mdapp/Controllers/LibriController.phpapp/Controllers/SearchController.phpapp/Models/BookRepository.phpapp/Models/DashboardStats.phpapp/Models/LoanRepository.phpapp/Support/AuthorName.phpapp/Support/ContributorBackfill.phpapp/Support/ContributorRoles.phpapp/Support/MaintenanceService.phpapp/Views/frontend/book-detail.phpapp/Views/libri/partials/book_form.phpapp/Views/libri/scheda_libro.phpdocs/superpowers/specs/2026-07-14-author-roles-pseudonym-237-design.mdinstaller/database/migrations/migrate_0.7.36.sqlinstaller/database/schema.sqllocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsontests/migration-0.7.36.unit.phpversion.json
| "Nessun risultato, premi Invio per aggiungerne uno nuovo": "Aucun résultat, appuyez sur Entrée pour en ajouter un nouveau", | ||
| "Illustratori dell'opera (autori veri, con autocomplete)": "Illustrateurs (vrais auteurs, avec autocomplétion)", | ||
| "Traduttori (autori veri, con autocomplete)": "Traducteurs (vrais auteurs, avec autocomplétion)", | ||
| "Curatori dell'opera (autori veri, con autocomplete)": "Éditeurs (vrais auteurs, avec autocomplétion)", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correggi la traduzione del ruolo “Curatore”.
"Curatori dell'opera" è tradotto come "Éditeurs", creando ambiguità con il ruolo di editore. Usa il termine già adottato dal catalogo, ad esempio "Directeurs de publication de l'œuvre (vrais auteurs, avec autocomplétion)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@locale/fr_FR.json` at line 6535, Update the locale entry for “Curatori
dell'opera (autori veri, con autocomplete)” to use the catalog’s established
“Directeurs de publication” terminology, preserving the parenthetical meaning
and autocomplete wording.
…+ interop Hardens the contributor-roles/pseudonym work after a full cross-plugin review. Correctness: - New App\Support\ContributorSync centralizes name→entity resolution for the form, CSV import, LibraryThing import and the legacy backfill. It resolves ONLY against the canonical autori.nome (never the pseudonym), so an SBN/VIAF name that collides with someone's pen name creates the distinct canonical entity. splitNames no longer blindly splits on commas: 'Levi, Primo' stays one inverted personal name (canonicalized by AuthorNormalizer), while a real list 'Mario Rossi, Luigi Bianchi' still splits — fixing a real bug in the original backfill split. - AuthorRepository::findByCanonicalName; role loss during partial upgrades / enum-FK errors eliminated; pseudonyms folded into the FULLTEXT search index and refreshed on author edit/merge/delete; picker create-on-Enter no longer risks duplicating an author when 'Pseudonym (Real name)' is picked. Semantics + interop: - Authors/co-authors are separated from translators/curators/illustrators/ colorists across catalog, feed, dashboard, sitemap, Book Club, FRBR/LRM and the API. OAI/BIBFRAME/OpenURL/NCIP/Z39.50-SRU/UNIMARC emit canonical names + correct role codes (UNIMARC 730 translator, 340 editor, 440 illustrator, 410 colorist, per IFLA UNIMARC / Dublin Core / LOC relators). - Central points: ContributorSync, AuthorRepository::findByCanonicalName, BookRepository::syncContributors, OaiPmhServerPlugin. Backfill now also rebuilds the search index for affected + pseudonymous books on the first post-upgrade pass. Verified: PHPStan L5 full-tree 0 errors; migration 20/20 (hermetic fixture + pseudonym index rebuild), contributor roles 28/28, plugin compatibility 30/30, book-club migration 6/6; real headless install + simulated 0.7.35→0.7.36 upgrade + idempotent backfill + SBN-name-equals-pseudonym distinct-entity case.
…ay, tests)
Applies every finding from the review pass on this PR.
Correctness / data loss:
- CSV + LibraryThing importers scoped their DELETE FROM libri_autori to
ruolo='principale' — a blanket delete-all silently wiped illustrator/curator/
colorist ENTITY links on every re-import (they only re-add principale + a subset).
- scheda_libro dropped the free-text fallback: it was never cleared, so removing
all entity contributors of a role resurrected the just-deleted name (and diverged
from the public page, which has no fallback). Entities are the single source of
truth; legacy columns stay only for import/export round-trip.
- syncContributors() now prunes stale links only within the roles the caller
actually supplied, so a partial caller can't wipe roles it never mentioned.
Consistency / display:
- Book Club repos now use AuthorName::displaySql() or its full guarded CASE literal
(in const-context SELECTs) instead of an inline CONCAT missing the nome-non-empty
and pseudonimo<>nome guards ('Pseudonimo ()', 'X (X)').
- Public book page: contributor role no longer double-parenthesised
('Pseudonym (Name) (Role)' -> 'Pseudonym (Name) . Role').
- Author landing page (title/H1/alt) now uses AuthorName::display, so a clicked
pseudonym carries through instead of collapsing to the real name.
UX / tests / docs:
- The 4 contributor pickers now Toast on create/duplicate like the authors picker;
their help microcopy is consistent user-facing copy.
- migration-0.7.36 Test A now runs the REAL migration .sql (retargeted at a sandbox
table via multi_query) so the guarded PREPARE/EXECUTE path is exercised — 21/21.
- Stale docblocks/design-doc updated to insert-first/delete-stale + comma-preserving split.
PHPStan L5 full-tree clean; contributor roles 28/28, plugin compatibility 30/30,
book-club migration 6/6; locale parity 6539x4.
- Views: use htmlspecialchars(..., ENT_QUOTES, 'UTF-8') instead of HtmlHelper::e() for the new contributor label/help (book_form) and role label (scheda), per the repo's view-escaping path instruction (don't add new HtmlHelper::e() in views). - Locale: drop the four now-orphaned contributor help strings (the form's help copy was unified in the previous commit); this also removes the ambiguous French 'Éditeurs' rendering of 'Curatori dell'opera' CodeRabbit flagged. The visible role label 'Curatore' already reads 'Directeur de publication' in fr. The other five CodeRabbit findings were on an earlier commit and are already resolved: syncContributors is insert-first/delete-stale; AuthorName::displaySql uses the COALESCE+nested-CASE form; the contributor picker has the _onEnterKey wrapper; the migration test creates a hermetic book fixture. Locale parity 6535x4; contributor tests green.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Controllers/FrontendController.php (1)
1953-1977: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRisolvi il troncamento della lista autori nei libri correlati.
Filtrando
la.autore_id IN (...)direttamente nellaWHEREprincipale, la precedenteLEFT JOINsi comporta come unaINNER JOIN, scartando dalla concatenazione gli altri co-autori del libro che non rientrano nel filtro iniziale.
Inoltre, eseguire unGROUP BY l.idsull'intera tabellalibriè inefficiente. Puoi correggere il bug e migliorare le performance sostituendo ilGROUP_CONCATcon una subquery scalare e usandoEXISTSper il filtro, esattamente come implementato già inSearchController.🐛 Soluzione proposta
- $query = " - SELECT DISTINCT l.*, - GROUP_CONCAT(DISTINCT " . \App\Support\AuthorName::displaySql('a') . " SEPARATOR ', ') as autori - FROM libri l - LEFT JOIN libri_autori la ON l.id = la.libro_id AND la.ruolo IN ('principale', 'co-autore') - LEFT JOIN autori a ON la.autore_id = a.id - WHERE la.autore_id IN ($authorPlaceholders) - AND l.id NOT IN ($excludePlaceholders) - AND l.deleted_at IS NULL - GROUP BY l.id - ORDER BY l.created_at DESC - LIMIT ? - "; + $query = " + SELECT l.*, + (SELECT GROUP_CONCAT(" . \App\Support\AuthorName::displaySql('a') . " ORDER BY a.nome SEPARATOR ', ') + FROM libri_autori la2 JOIN autori a ON la2.autore_id = a.id + WHERE la2.libro_id = l.id AND la2.ruolo IN ('principale', 'co-autore')) AS autori + FROM libri l + WHERE EXISTS ( + SELECT 1 FROM libri_autori la_f + WHERE la_f.libro_id = l.id AND la_f.autore_id IN ($authorPlaceholders) AND la_f.ruolo IN ('principale', 'co-autore') + ) + AND l.id NOT IN ($excludePlaceholders) + AND l.deleted_at IS NULL + ORDER BY l.created_at DESC + LIMIT ? + ";(Puoi applicare questo stesso pattern di subquery scalare anche per
Priority 2ePriority 3eliminando le rispettiveLEFT JOINe clausoleGROUP BY l.idnon più necessarie).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Controllers/FrontendController.php` around lines 1953 - 1977, Correggi la query nel blocco che costruisce $query sostituendo GROUP_CONCAT, le LEFT JOIN su libri_autori/autori e GROUP BY l.id con una subquery scalare per autori, così da includere tutti i co-autori dei libri correlati. Sostituisci il filtro la.autore_id IN (...) con EXISTS correlato agli autori filtrati, mantenendo esclusioni, ordinamento e limite; applica lo stesso pattern alle sezioni Priority 2 e Priority 3 se presenti.app/Controllers/PublicApiController.php (1)
83-94: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winWildcard LIKE non sanificati nella ricerca per autore/pseudonimo.
$authorfinisce inLIKE '%...%'senza escape di%/_. Un valore comeauthor=%fa matchare tutti i libri, bypassando il filtro. Nel resto del PR (es.ArchivesPlugin::searchAutori) i nuovi input LIKE vengono sanificati conaddcslashes($value, '%_\\')— qui manca la stessa protezione, sia pernomesia per il nuovopseudonimo.🛡️ Fix proposto
if ($author !== null && $author !== '') { $conditions[] = 'EXISTS ( SELECT 1 FROM libri_autori la JOIN autori a ON la.autore_id = a.id WHERE la.libro_id = l.id AND la.ruolo IN (\'principale\', \'co-autore\') AND (a.nome LIKE ? OR a.pseudonimo LIKE ?) )'; - $params[] = '%' . $author . '%'; - $params[] = '%' . $author . '%'; + $likeAuthor = '%' . addcslashes($author, '%_\\') . '%'; + $params[] = $likeAuthor; + $params[] = $likeAuthor; $types .= 'ss'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Controllers/PublicApiController.php` around lines 83 - 94, Sanitize $author before building the LIKE parameters in the author filter within PublicApiController, escaping %, _, and backslashes with the same addcslashes($value, '%_\\') pattern used by ArchivesPlugin::searchAutori. Reuse the escaped value for both nome and pseudonimo parameters while preserving the surrounding wildcard search.
♻️ Duplicate comments (2)
app/Views/libri/scheda_libro.php (1)
522-524: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRimuovi
HtmlHelper::e()in favore dihtmlspecialchars.Come specificato nelle path instructions, le view non devono utilizzare
HtmlHelper::e(). Usa invecehtmlspecialchars(..., ENT_QUOTES, 'UTF-8')per garantire un escaping uniforme e sicuro.♻️ Proposta di refactor
- <dt class="text-xs uppercase text-gray-500"><?php echo App\Support\HtmlHelper::e($label); ?></dt> + <dt class="text-xs uppercase text-gray-500"><?= htmlspecialchars($label, ENT_QUOTES, 'UTF-8') ?></dt> <dd class="text-gray-900 font-medium"> - <?php foreach ($people as $idx => $p): ?><?php echo $idx ? ', ' : ''; ?><a href="<?= htmlspecialchars(url('/admin/authors/' . (int)($p['id'] ?? 0)), ENT_QUOTES, 'UTF-8') ?>" class="hover:text-blue-600 transition"><?php echo App\Support\HtmlHelper::e(\App\Support\AuthorName::display($p)); ?></a><?php endforeach; ?> + <?php foreach ($people as $idx => $p): ?><?= $idx ? ', ' : '' ?><a href="<?= htmlspecialchars(url('/admin/authors/' . (int)($p['id'] ?? 0)), ENT_QUOTES, 'UTF-8') ?>" class="hover:text-blue-600 transition"><?= htmlspecialchars(\App\Support\AuthorName::display($p), ENT_QUOTES, 'UTF-8') ?></a><?php endforeach; ?>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Views/libri/scheda_libro.php` around lines 522 - 524, Rimuovi l’uso di HtmlHelper::e() nella view, sia per $label sia per il risultato di AuthorName::display($p), sostituendolo con htmlspecialchars(..., ENT_QUOTES, 'UTF-8'). Mantieni invariati il ciclo, la formattazione delle virgole e la costruzione del link.Source: Path instructions
app/Views/libri/partials/book_form.php (1)
276-285: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
HtmlHelper::e()ancora usato nel loop contributor — non risolto dalla review precedente.Righe 278 e 283 usano
HtmlHelper::e(). Come da regola di progetto per le view, servehtmlspecialchars(..., ENT_QUOTES, 'UTF-8').🔧 Fix
- <label for="<?= $roleKey ?>_select" class="form-label"><?= HtmlHelper::e($meta['label']) ?></label> + <label for="<?= $roleKey ?>_select" class="form-label"><?= htmlspecialchars($meta['label'], ENT_QUOTES, 'UTF-8') ?></label> <select id="<?= $roleKey ?>_select" name="<?= $roleKey ?>_select[]" multiple placeholder="<?= __('Cerca o aggiungi...') ?>" data-initial-contributors="<?php echo $initialContributorsJson[$roleKey]; ?>"></select> <div id="<?= $roleKey ?>_hidden"></div> - <p class="text-xs text-gray-500 mt-1"><?= HtmlHelper::e($meta['help']) ?></p> + <p class="text-xs text-gray-500 mt-1"><?= htmlspecialchars($meta['help'], ENT_QUOTES, 'UTF-8') ?></p>As per path instructions: "Mai usare
HtmlHelper::e()nelle view — usarehtmlspecialchars(..., ENT_QUOTES, 'UTF-8')."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Views/libri/partials/book_form.php` around lines 276 - 285, Replace both HtmlHelper::e() calls in the contributorFields loop with htmlspecialchars(..., ENT_QUOTES, 'UTF-8'), preserving the escaped label and help text output and the existing loop structure.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Controllers/CsvImportController.php`:
- Around line 375-387: Update ContributorSync::linkLegacyValues and both import
call sites in app/Controllers/CsvImportController.php lines 375-387 and
app/Controllers/LibraryThingImportController.php lines 447-455 so reimports
replace only contributors previously created from legacy traduttore/illustratore
values, removing stale links before adding current values while preserving
manually linked contributors.
In `@app/Models/BookRepository.php`:
- Around line 1116-1131: Make the contributor synchronization in the
create/update path atomic by wrapping the entire insert loop in a database
transaction, including transaction start, commit on success, and rollback for
any prepare or execute failure. Update the surrounding repository method so a
failed insert leaves no contributor changes persisted, while preserving the
existing desired-list and INSERT IGNORE behavior.
In `@app/Support/ContributorSync.php`:
- Around line 136-146: In the contributor insert loop, check the return value of
execute() on the prepared statement created by ContributorSync before
continuing. When execution fails, immediately throw or otherwise propagate a
RuntimeException containing the database error, while preserving the existing
duplicate-ignore behavior and closing the statement on successful completion.
In `@app/Views/frontend/archive.php`:
- Line 457: Update the htmlspecialchars call rendering $archiveDisplayName in
the archive title to pass ENT_QUOTES and 'UTF-8' as explicit escaping
parameters, preserving the existing output value.
In `@locale/de_DE.json`:
- Line 6535: Update the German translation for “Curatori dell'opera (autori
veri, con autocomplete)” to use “Kuratoren” instead of “Herausgeber”, preserving
the rest of the label and its autocomplete wording.
In `@locale/it_IT.json`:
- Around line 6536-6540: Add the missing "Coloristi (utile per i fumetti)"
translation key to the locale entries for en_US, de_DE, and fr_FR, matching the
existing key and preserving each file’s language-specific translation
conventions.
In `@storage/plugins/book-club/src/LendingRepo.php`:
- Around line 111-116: Replace the inline author-name CASE expression in
LendingRepo’s LOAN_SELECT with the shared
\App\Support\AuthorName::displaySql('a') helper, matching its usage in
BookRepository, DashboardStats, and SeriesRepository. Preserve the existing
GROUP_CONCAT ordering, separator, joins, and author-role filtering.
In `@storage/plugins/book-club/src/Repo.php`:
- Around line 645-649: Replace the duplicated author-name CASE expressions with
the centralized \App\Support\AuthorName::displaySql('a') helper in Repo.php at
lines 645-649, 874-877, and 1008-1011, preserving each surrounding
GROUP_CONCAT/select query in the Repo methods.
In `@storage/plugins/openurl-resolver/OpenUrlResolverPlugin.php`:
- Around line 395-397: Aggiorna l’ORDER BY della query che calcola
autore_principale aggiungendo la priorità decrescente per la condizione la.ruolo
= 'principale' prima degli ordinamenti esistenti. Mantieni invariati
COALESCE(la.ordine_credito, 0), la.autore_id e il limite, così in caso di parità
viene selezionato l’autore principale.
- Around line 441-443: Aggiorna l’ORDER BY della query in OpenUrlResolverPlugin
per ordinare prima le righe con ruolo “principale” e poi quelle “co-autore”,
mantenendo come criteri successivi COALESCE(la.ordine_credito, 0) e la priorità
esistente tramite lautore_id.
In `@tests/contributor-plugin-compatibility-237.unit.php`:
- Around line 27-29: Update the manifest-count assertion in the contributor
plugin compatibility test to compare against the centralized
BundledPlugins::LIST count instead of the hardcoded 20. Keep the existing
manifest discovery and JSON validity checks unchanged.
---
Outside diff comments:
In `@app/Controllers/FrontendController.php`:
- Around line 1953-1977: Correggi la query nel blocco che costruisce $query
sostituendo GROUP_CONCAT, le LEFT JOIN su libri_autori/autori e GROUP BY l.id
con una subquery scalare per autori, così da includere tutti i co-autori dei
libri correlati. Sostituisci il filtro la.autore_id IN (...) con EXISTS
correlato agli autori filtrati, mantenendo esclusioni, ordinamento e limite;
applica lo stesso pattern alle sezioni Priority 2 e Priority 3 se presenti.
In `@app/Controllers/PublicApiController.php`:
- Around line 83-94: Sanitize $author before building the LIKE parameters in the
author filter within PublicApiController, escaping %, _, and backslashes with
the same addcslashes($value, '%_\\') pattern used by
ArchivesPlugin::searchAutori. Reuse the escaped value for both nome and
pseudonimo parameters while preserving the surrounding wildcard search.
---
Duplicate comments:
In `@app/Views/libri/partials/book_form.php`:
- Around line 276-285: Replace both HtmlHelper::e() calls in the
contributorFields loop with htmlspecialchars(..., ENT_QUOTES, 'UTF-8'),
preserving the escaped label and help text output and the existing loop
structure.
In `@app/Views/libri/scheda_libro.php`:
- Around line 522-524: Rimuovi l’uso di HtmlHelper::e() nella view, sia per
$label sia per il risultato di AuthorName::display($p), sostituendolo con
htmlspecialchars(..., ENT_QUOTES, 'UTF-8'). Mantieni invariati il ciclo, la
formattazione delle virgole e la costruzione del link.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4645ded3-b3fa-4be0-970c-8ef42b92b550
📒 Files selected for processing (60)
app/Controllers/CollocazioneController.phpapp/Controllers/CsvImportController.phpapp/Controllers/FeedController.phpapp/Controllers/FrontendController.phpapp/Controllers/LibraryThingImportController.phpapp/Controllers/LibriApiController.phpapp/Controllers/LibriController.phpapp/Controllers/PublicApiController.phpapp/Controllers/ReservationManager.phpapp/Controllers/SearchController.phpapp/Controllers/UserDashboardController.phpapp/Models/AuthorRepository.phpapp/Models/BookRepository.phpapp/Models/DashboardStats.phpapp/Models/LoanRepository.phpapp/Models/PublisherRepository.phpapp/Models/SeriesRepository.phpapp/Services/ReservationReassignmentService.phpapp/Support/AuthorName.phpapp/Support/ContributorBackfill.phpapp/Support/ContributorSync.phpapp/Support/NotificationService.phpapp/Support/SearchIndexBuilder.phpapp/Support/SitemapGenerator.phpapp/Views/frontend/archive.phpapp/Views/frontend/book-detail.phpapp/Views/libri/partials/book_form.phpapp/Views/libri/scheda_libro.phpdocs/superpowers/specs/2026-07-14-author-roles-pseudonym-237-design.mdlocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsonstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/bibframe-linked-data/BibframeLinkedDataPlugin.phpstorage/plugins/book-club/src/AffinityRepo.phpstorage/plugins/book-club/src/ChallengeRepo.phpstorage/plugins/book-club/src/LendingRepo.phpstorage/plugins/book-club/src/LibraryRepo.phpstorage/plugins/book-club/src/QuoteRepo.phpstorage/plugins/book-club/src/ReadingRepo.phpstorage/plugins/book-club/src/Repo.phpstorage/plugins/book-club/src/StatsRepo.phpstorage/plugins/frbr-lrm/EspressioniRepository.phpstorage/plugins/frbr-lrm/FrbrLrmPlugin.phpstorage/plugins/frbr-lrm/OpereRepository.phpstorage/plugins/mobile-api/src/Controllers/ActionsController.phpstorage/plugins/mobile-api/src/Controllers/CatalogController.phpstorage/plugins/mobile-api/src/Controllers/OpenApiController.phpstorage/plugins/mobile-api/src/Controllers/ReviewsController.phpstorage/plugins/ncip-server/NcipServerPlugin.phpstorage/plugins/oai-pmh-server/OaiPmhServerPlugin.phpstorage/plugins/openurl-resolver/OpenUrlResolverPlugin.phpstorage/plugins/z39-server/Z39ServerPlugin.phpstorage/plugins/z39-server/classes/SRUServer.phpstorage/plugins/z39-server/classes/UnimarcLibriParser.phptests/contributor-plugin-compatibility-237.unit.phptests/contributor-roles-237.unit.phptests/migration-0.7.36.unit.phptests/oai-pmh-server.spec.js
| "Nessun risultato, premi Invio per aggiungerne uno nuovo": "Keine Ergebnisse, Enter drücken, um einen neuen hinzuzufügen", | ||
| "Illustratori dell'opera (autori veri, con autocomplete)": "Illustratoren (echte Autoren, mit Autovervollständigung)", | ||
| "Traduttori (autori veri, con autocomplete)": "Übersetzer (echte Autoren, mit Autovervollständigung)", | ||
| "Curatori dell'opera (autori veri, con autocomplete)": "Herausgeber (echte Autoren, mit Autovervollständigung)", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tradurre “Curatori” come “Kuratoren”
Herausgeber indica editori/redattori, non curatori. Questa label mostrerebbe quindi un ruolo contributor errato nell’interfaccia tedesca.
Proposta
- "Curatori dell'opera (autori veri, con autocomplete)": "Herausgeber (echte Autoren, mit Autovervollständigung)",
+ "Curatori dell'opera (autori veri, con autocomplete)": "Kuratoren (echte Autoren, mit Autovervollständigung)",📝 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
| "Curatori dell'opera (autori veri, con autocomplete)": "Herausgeber (echte Autoren, mit Autovervollständigung)", | |
| "Curatori dell'opera (autori veri, con autocomplete)": "Kuratoren (echte Autoren, mit Autovervollständigung)", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@locale/de_DE.json` at line 6535, Update the German translation for “Curatori
dell'opera (autori veri, con autocomplete)” to use “Kuratoren” instead of
“Herausgeber”, preserving the rest of the label and its autocomplete wording.
Source: Path instructions
…d escaping Second review pass over the contributor-roles/pseudonym work. - Contributor sync is atomic: rolls back on FK/enum violations or invalid/zero ids so a book never ends up partially wiped; co-authors are preserved and never duplicated as principals. - Per-source provenance for CSV / LibraryThing / backfill: each importer removes only the links it owns, and a contributor an admin confirmed becomes 'manual' and can no longer be dropped by a re-import. - SBN and other providers compare only the canonical name, never the pseudonym. - Backfill runs immediately from the updater, Docker boot and manual upgrade. - Related-books query keeps the complete co-author list; OpenURL always gives the principal creator priority. - Public API escapes %, _ and backslash as literals (ESCAPE clause). - Every Book Club repo (QuoteRepo included) uses the single AuthorName SQL helper. - Escaping, plugin compatibility and the dynamic bundled-plugin count corrected. - Adds tests/issue-237-contributors.spec.js (Playwright E2E). PHPStan L5 full-tree clean; migration real-file 24/24, contributor roles 44/44, plugin compatibility 33/33, book-club suites green; locale parity 6535x4. Verified: real Docker upgrade 0.7.35->0.7.36 (enum, provenance table, backfill, marker, version) on MySQL 8 / PHP 8.4.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Support/ContributorBackfill.php (1)
44-58: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNon impostare il marker quando una query del backfill fallisce.
query()/prepare()falliti vengono trattati come risultati vuoti o colonne assenti;run()prosegue e salvacontributors_backfilled=1, impedendo ogni retry e lasciando contributor o indici incompleti. Propaga invece unaRuntimeException, così ilcatchrestituiscefalsesenza impostare il marker.Correzione proposta
$res = $db->query($sql); if (!($res instanceof \mysqli_result)) { - return $bookIds; + throw new \RuntimeException('Unable to read legacy contributors: ' . $db->error); } $res = $db->query(/* pseudonym query */); -if ($res instanceof \mysqli_result) { - // ... +if (!($res instanceof \mysqli_result)) { + throw new \RuntimeException('Unable to read pseudonym books: ' . $db->error); } if ($stmt === false) { - return false; + throw new \RuntimeException('Unable to inspect schema: ' . $db->error); }Also applies to: 71-75, 95-102, 124-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Support/ContributorBackfill.php` around lines 44 - 58, Update the backfill query and schema helpers used by run()—including booksWithPseudonyms(), hasColumn(), and backfillColumn()—to throw a RuntimeException when query() or prepare() fails instead of treating failures as empty results or missing columns. Preserve normal empty-result behavior for successful queries, and ensure run() reaches the existing catch path so the marker is not set and the method returns false.app/Models/BookRepository.php (1)
640-641: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRendi atomico il salvataggio del libro
createBasic()/updateBasic()scrivonolibriprima disyncContributors()esyncPublishers(): se uno di questi passi fallisce, restano dati parziali. Apri una sola transazione per l’intero flusso di salvataggio e aggiungi un test che con un contributor ID invalido il libro venga rollbackato.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/BookRepository.php` around lines 640 - 641, Rendi atomico il flusso di salvataggio in createBasic() e updateBasic(): racchiudi la scrittura di libri e le chiamate a syncContributors() e syncPublishers() in un’unica transazione, eseguendo il rollback quando uno dei passaggi fallisce. Aggiungi un test che usi un contributor ID invalido e verifichi che il libro non rimanga salvato.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Controllers/FrontendController.php`:
- Around line 1939-1947: Remove DISTINCT from the GROUP_CONCAT expression
assigned to $allCreatorsSelect, while preserving the existing displaySql
expression, ordering, filtering, and separator so creator ordering remains
unchanged.
In `@app/Views/frontend/archive.php`:
- Around line 512-513: Valida il valore di archive_info['sito_web'] con la
stessa allowlist HTTP/HTTPS usata per i link autore prima di inserirlo
nell’attributo href. Aggiorna il blocco che genera il link dell’editore
mantenendo l’escaping esistente e impedendo schemi non consentiti come
javascript:.
In `@tests/issue-237-contributors.spec.js`:
- Around line 67-71: After input.fill(query) in the contributor suggestion flow,
wait briefly for the Choices.js debounce to finish before creating or querying
the suggestion locator, then preserve the existing visibility assertion and
click behavior.
- Around line 79-83: Rendi obbligatoria la conferma SweetAlert nel flusso del
test dopo `button.click()`: individua `.swal2-confirm` e attendi che sia
visibile senza usare un controllo opzionale con fallback a `false`, quindi
cliccala. Mantieni il selettore esistente e assicurati che il test non possa
proseguire lasciando il form bloccato.
---
Outside diff comments:
In `@app/Models/BookRepository.php`:
- Around line 640-641: Rendi atomico il flusso di salvataggio in createBasic() e
updateBasic(): racchiudi la scrittura di libri e le chiamate a
syncContributors() e syncPublishers() in un’unica transazione, eseguendo il
rollback quando uno dei passaggi fallisce. Aggiungi un test che usi un
contributor ID invalido e verifichi che il libro non rimanga salvato.
In `@app/Support/ContributorBackfill.php`:
- Around line 44-58: Update the backfill query and schema helpers used by
run()—including booksWithPseudonyms(), hasColumn(), and backfillColumn()—to
throw a RuntimeException when query() or prepare() fails instead of treating
failures as empty results or missing columns. Preserve normal empty-result
behavior for successful queries, and ensure run() reaches the existing catch
path so the marker is not set and the method returns false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5181bc71-743b-4c26-8f4b-a92e2fca7e60
📒 Files selected for processing (29)
app/Controllers/CsvImportController.phpapp/Controllers/FrontendController.phpapp/Controllers/LibraryThingImportController.phpapp/Controllers/PublicApiController.phpapp/Models/BookRepository.phpapp/Support/AuthorName.phpapp/Support/ContributorBackfill.phpapp/Support/ContributorSync.phpapp/Support/MaintenanceService.phpapp/Support/Updater.phpapp/Views/frontend/archive.phpapp/Views/libri/partials/book_form.phpapp/Views/libri/scheda_libro.phpdocs/superpowers/specs/2026-07-14-author-roles-pseudonym-237-design.mdinstaller/database/migrations/migrate_0.7.36.sqlinstaller/database/schema.sqllocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsonscripts/manual-upgrade.phpstorage/plugins/book-club/src/LendingRepo.phpstorage/plugins/book-club/src/QuoteRepo.phpstorage/plugins/book-club/src/Repo.phpstorage/plugins/openurl-resolver/OpenUrlResolverPlugin.phptests/contributor-plugin-compatibility-237.unit.phptests/contributor-roles-237.unit.phptests/issue-237-contributors.spec.jstests/migration-0.7.36.unit.php
💤 Files with no reviewable changes (4)
- locale/en_US.json
- locale/fr_FR.json
- locale/it_IT.json
- locale/de_DE.json
Comment on lines
+1939
to
+1947
| $allCreatorsSelect = " | ||
| (SELECT GROUP_CONCAT(DISTINCT " . \App\Support\AuthorName::displaySql('a_all') . " | ||
| ORDER BY (la_all.ruolo = 'principale') DESC, | ||
| COALESCE(la_all.ordine_credito, 0), la_all.autore_id | ||
| SEPARATOR ', ') | ||
| FROM libri_autori la_all | ||
| JOIN autori a_all ON a_all.id = la_all.autore_id | ||
| WHERE la_all.libro_id = l.id | ||
| AND la_all.ruolo IN ('principale', 'co-autore'))"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Errore SQL con GROUP_CONCAT(DISTINCT ... ORDER BY ...)
In MySQL 5.7+ e 8.0, quando si usa DISTINCT all'interno di GROUP_CONCAT, tutte le colonne specificate nella clausola ORDER BY devono essere necessariamente parte dell'espressione concatenata. Poiché l'ordinamento utilizza la_all.ruolo, la_all.ordine_credito e la_all.autore_id che non fanno parte dell'espressione SQL del nome (displaySql), la query fallirà a runtime (Error 3029) rompendo il caricamento dei libri correlati. Rimuovi DISTINCT per risolvere il problema (i duplicati sono di base impediti dal vincolo di unicità in DB su ruolo/autore).
🐛 Proposta di correzione
$allCreatorsSelect = "
- (SELECT GROUP_CONCAT(DISTINCT " . \App\Support\AuthorName::displaySql('a_all') . "
+ (SELECT GROUP_CONCAT(" . \App\Support\AuthorName::displaySql('a_all') . "
ORDER BY (la_all.ruolo = 'principale') DESC,
COALESCE(la_all.ordine_credito, 0), la_all.autore_id
SEPARATOR ', ')📝 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
| $allCreatorsSelect = " | |
| (SELECT GROUP_CONCAT(DISTINCT " . \App\Support\AuthorName::displaySql('a_all') . " | |
| ORDER BY (la_all.ruolo = 'principale') DESC, | |
| COALESCE(la_all.ordine_credito, 0), la_all.autore_id | |
| SEPARATOR ', ') | |
| FROM libri_autori la_all | |
| JOIN autori a_all ON a_all.id = la_all.autore_id | |
| WHERE la_all.libro_id = l.id | |
| AND la_all.ruolo IN ('principale', 'co-autore'))"; | |
| $allCreatorsSelect = " | |
| (SELECT GROUP_CONCAT(" . \App\Support\AuthorName::displaySql('a_all') . " | |
| ORDER BY (la_all.ruolo = 'principale') DESC, | |
| COALESCE(la_all.ordine_credito, 0), la_all.autore_id | |
| SEPARATOR ', ') | |
| FROM libri_autori la_all | |
| JOIN autori a_all ON a_all.id = la_all.autore_id | |
| WHERE la_all.libro_id = l.id | |
| AND la_all.ruolo IN ('principale', 'co-autore'))"; |
🧰 Tools
🪛 PHPMD (2.15.0)
[error] 1940-1940: Avoid using static access to class '\App\Support\AuthorName' in method 'getRelatedBooks'. (undefined)
(StaticAccess)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Controllers/FrontendController.php` around lines 1939 - 1947, Remove
DISTINCT from the GROUP_CONCAT expression assigned to $allCreatorsSelect, while
preserving the existing displaySql expression, ordering, filtering, and
separator so creator ordering remains unchanged.
Comment on lines
512
to
+513
| <?php if (!empty($archive_info['sito_web'])): ?> | ||
| <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener"><?= htmlspecialchars($archive_info['sito_web']) ?></a></p> | ||
| <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener"><?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?></a></p> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Valida lo schema dell’URL dell’editore prima di inserirlo in href.
L’escaping dell’attributo non blocca valori come javascript:. Applica la stessa allowlist HTTP/HTTPS già usata per i link autore.
Correzione proposta
- <?php if (!empty($archive_info['sito_web'])): ?>
+ <?php
+ $publisherSite = trim((string)($archive_info['sito_web'] ?? ''));
+ $hasPublisherSite = filter_var($publisherSite, FILTER_VALIDATE_URL)
+ && preg_match('#^https?://#i', $publisherSite) === 1;
+ ?>
+ <?php if ($hasPublisherSite): ?>
- <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener"><?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?></a></p>
+ <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($publisherSite, ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener noreferrer"><?= htmlspecialchars($publisherSite, ENT_QUOTES, 'UTF-8') ?></a></p>📝 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
| <?php if (!empty($archive_info['sito_web'])): ?> | |
| <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener"><?= htmlspecialchars($archive_info['sito_web']) ?></a></p> | |
| <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener"><?= htmlspecialchars($archive_info['sito_web'], ENT_QUOTES, 'UTF-8') ?></a></p> | |
| <?php | |
| $publisherSite = trim((string)($archive_info['sito_web'] ?? '')); | |
| $hasPublisherSite = filter_var($publisherSite, FILTER_VALIDATE_URL) | |
| && preg_match('#^https?://#i', $publisherSite) === 1; | |
| ?> | |
| <?php if ($hasPublisherSite): ?> | |
| <p><i class="fas fa-globe"></i><a href="<?= htmlspecialchars($publisherSite, ENT_QUOTES, 'UTF-8') ?>" target="_blank" rel="noopener noreferrer"><?= htmlspecialchars($publisherSite, ENT_QUOTES, 'UTF-8') ?></a></p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Views/frontend/archive.php` around lines 512 - 513, Valida il valore di
archive_info['sito_web'] con la stessa allowlist HTTP/HTTPS usata per i link
autore prima di inserirlo nell’attributo href. Aggiorna il blocco che genera il
link dell’editore mantenendo l’escaping esistente e impedendo schemi non
consentiti come javascript:.
Comment on lines
+67
to
+71
| await input.fill(query); | ||
| const suggestion = wrapper.locator('.choices__list--dropdown .choices__item--selectable') | ||
| .filter({ hasText: expectedLabel }).first(); | ||
| await expect(suggestion).toBeVisible({ timeout: 10_000 }); | ||
| await suggestion.click(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Attendi il debounce di Choices.js prima di cercare il suggerimento.
Senza una breve attesa dopo fill, la lista può ancora contenere i risultati della ricerca precedente.
Correzione proposta
await input.fill(query);
+ await page.waitForTimeout(300);
const suggestion = wrapper.locator('.choices__list--dropdown .choices__item--selectable')As per path instructions, «Choices.js: usare fill + waitForTimeout + click suggestion».
📝 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
| await input.fill(query); | |
| const suggestion = wrapper.locator('.choices__list--dropdown .choices__item--selectable') | |
| .filter({ hasText: expectedLabel }).first(); | |
| await expect(suggestion).toBeVisible({ timeout: 10_000 }); | |
| await suggestion.click(); | |
| await input.fill(query); | |
| await page.waitForTimeout(300); | |
| const suggestion = wrapper.locator('.choices__list--dropdown .choices__item--selectable') | |
| .filter({ hasText: expectedLabel }).first(); | |
| await expect(suggestion).toBeVisible({ timeout: 10_000 }); | |
| await suggestion.click(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/issue-237-contributors.spec.js` around lines 67 - 71, After
input.fill(query) in the contributor suggestion flow, wait briefly for the
Choices.js debounce to finish before creating or querying the suggestion
locator, then preserve the existing visibility assertion and click behavior.
Source: Path instructions
Comment on lines
+79
to
+83
| await button.click(); | ||
| const confirm = page.locator('.swal2-confirm:visible'); | ||
| if (await confirm.isVisible({ timeout: 3000 }).catch(() => false)) { | ||
| await confirm.click(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Rendi obbligatoria la conferma SweetAlert.
Il controllo opzionale può saltare il click se il dialog viene renderizzato in ritardo, lasciando il test bloccato sul form.
Correzione proposta
await button.click();
const confirm = page.locator('.swal2-confirm:visible');
- if (await confirm.isVisible({ timeout: 3000 }).catch(() => false)) {
- await confirm.click();
- }
+ await expect(confirm).toBeVisible({ timeout: 10_000 });
+ await confirm.click();As per path instructions, «SweetAlert: dopo form submit, verificare e cliccare .swal2-confirm».
📝 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
| await button.click(); | |
| const confirm = page.locator('.swal2-confirm:visible'); | |
| if (await confirm.isVisible({ timeout: 3000 }).catch(() => false)) { | |
| await confirm.click(); | |
| } | |
| await button.click(); | |
| const confirm = page.locator('.swal2-confirm:visible'); | |
| await expect(confirm).toBeVisible({ timeout: 10_000 }); | |
| await confirm.click(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/issue-237-contributors.spec.js` around lines 79 - 83, Rendi
obbligatoria la conferma SweetAlert nel flusso del test dopo `button.click()`:
individua `.swal2-confirm` e attendi che sia visibile senza usare un controllo
opzionale con fallback a `false`, quindi cliccala. Mantieni il selettore
esistente e assicurati che il test non possa proseguire lasciando il form
bloccato.
Source: Path instructions
…CORE_TABLES The 0.7.36 migration + schema.sql add the libri_autori_import_sources provenance table, but schema-integrity.spec.js's hardcoded CORE_TABLES list (and its schema.sql cross-check) didn't know about it, so the E2E 'exactly N core tables' test failed. Add the table to CORE_TABLES (count is derived from the list length, now 58) — schema.sql already creates it, so both the count and the cross-check now agree.
A test that hardcodes the core-table list/count or the bundled-plugin list only re-asserts the source and fails on every legitimate addition — it isn't a real invariant. It also goes stale silently: the plugin lists were pinned at 16 while BundledPlugins::LIST already had 19 (book-club, frbr-lrm, mobile-api were never verified), and adding libri_autori_import_sources (0.7.36) broke the hardcoded 'exactly 57 core tables' assertion. Both now derive from the app's own sources of truth: - schema-integrity.spec.js: CORE_TABLES parsed from every CREATE TABLE in installer/database/schema.sql; the count follows the parsed length; the test asserts the installed DB contains every table the schema declares. Dropped the redundant hardcoded-vs-schema cross-check. - plugin-integrity.spec.js: EXPECTED_BUNDLED parsed from BundledPlugins::LIST (now 19, so the three previously-unverified plugins are covered). - Both skip when the source file isn't readable (remote-only CI target). Adding a table or plugin no longer requires editing these tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/plugin-integrity.spec.js`:
- Around line 28-38: Extract the shared parseBundledPluginsFromSource
implementation into a common tests utility module, then replace the duplicate
definition in tests/plugin-integrity.spec.js at lines 28-38 with an import and
remove the duplicate in tests/schema-integrity.spec.js at lines 105-115, using
the imported function in both tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dba22568-9e73-4c1a-a81d-c7a72fa0b5cb
📒 Files selected for processing (2)
tests/plugin-integrity.spec.jstests/schema-integrity.spec.js
…dcode Extend the "derive, don't hardcode" invariant to every place that still listed core tables or bundled plugins by hand, so a legitimate schema or plugin addition can't break a check that only re-asserted the source. - Installer::expectedTablesFromSchema() parses CREATE TABLE from schema.sql for the post-import and final verify; step7 renders the count via %d. - create-release.sh, ci-upgrade-smoke.yml, reinstall-test.sh and verify-schema.sh read tables/plugins from scripts/list-source-expectations.php. - Each plugin's expectedTables() derives from its own schema map; book-club aggregates all 40 module tables through a static schemaSteps() contract (declaredTables()/expectedTables() added to ModuleInterface). - New tests/helpers/source-expectations.js and PHP unit tests; the schema-integrity and plugin-integrity specs derive their sets. plugin- integrity now asserts "bundled does not mean active" (external music plugins stay inactive on install) and that every activatable plugin's schema lands. code-quality guards against reintroducing a hardcoded CORE_TABLES array. - Installer table-count string collapses to one %d form across all four locales. reinstall-test.sh also picks a MySQL-native mysqldump when a MariaDB one shadows it in PATH, so the backup no longer aborts empty on the MySQL-only --set-gtid-purged flag.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
storage/plugins/oai-pmh-server/OaiPmhServerPlugin.php (1)
2060-2064: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAggiungi il fallback per i contributori legacy nel formato MAG.
Nel blocco
writeBookOaiDc(e in altri formati comemarcxmlemods) è stato aggiunto un fallback che include i contributori salvati sui campi testuali legacy (traduttore,illustratore, ecc.) se non sono già mappati come entità. Nel formato MAG (che genera un sotto-record Dublin Core) questa logica è assente, rischiando di omettere dei contributori in attesa del completamento del backfill.💡 Proposta di modifica
foreach ($authors as $a) { $role = (string) ($a['ruolo'] ?? ''); $element = in_array($role, ['principale', 'co-autore'], true) ? 'creator' : 'contributor'; $xw->writeElementNs('dc', $element, null, (string) $a['nome']); } + + $entityRoles = array_map(static fn (array $a): string => (string) ($a['ruolo'] ?? ''), $authors); + foreach (['traduttore', 'illustratore', 'curatore', 'colorista'] as $col) { + if (!empty($row[$col]) && !in_array($col, $entityRoles, true)) { + $xw->writeElementNs('dc', 'contributor', null, (string) $row[$col]); + } + } foreach ($publishers as $pub) { if (!empty($pub['nome'])) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@storage/plugins/oai-pmh-server/OaiPmhServerPlugin.php` around lines 2060 - 2064, Aggiorna il blocco MAG in writeBookOaiDc, vicino al ciclo sugli authors, per includere anche i contributori provenienti dai campi testuali legacy come traduttore e illustratore quando non sono già rappresentati come entità. Riutilizza la stessa logica di fallback adottata da writeBookOaiDc negli altri formati, mantenendo creator per principale/co-autore e contributor per gli altri ruoli.locale/it_IT.json (1)
6527-6534: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAllinea le chiavi in
locale/en_US.jsonelocale/de_DE.json.
Le chiaviContributore "%s" è già selezionatoeContributore "%s" pronto per essere creatosono presenti inlocale/it_IT.jsonma mancano in entrambe le altre traduzioni, quindi la sincronizzazione richiesta non è rispettata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@locale/it_IT.json` around lines 6527 - 6534, Add the missing translation keys “Contributore "%s" è già selezionato” and “Contributore "%s" pronto per essere creato” to the corresponding sections of en_US.json and de_DE.json, using appropriate English and German translations while preserving the existing JSON structure.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/README.MD`:
- Around line 146-147: Aggiorna la sezione “Sistema a Plugin” aggiungendo una
riga vuota dopo il titolo e specificando il linguaggio del code fence come text.
In `@installer/steps/step7.php`:
- Around line 39-40: Wrap the Installer::parseCreateTableNames call in step7.php
with try-catch, including the file_get_contents fallback path, and assign a safe
fallback table count when parsing throws. Preserve the existing successful
parsing behavior and allow the installation success screen to continue
rendering.
In `@tests/helpers/source-expectations.js`:
- Around line 56-66: Update parseBundledPluginNames to validate every non-empty,
non-comment line inside the extracted BundledPlugins::LIST block, rejecting any
unrecognized plugin entry instead of silently skipping it. Preserve valid quoted
plugin parsing, comment and blank-line filtering, duplicate detection, and
sorted output; raise a clear error when a remaining line cannot be parsed.
---
Outside diff comments:
In `@locale/it_IT.json`:
- Around line 6527-6534: Add the missing translation keys “Contributore "%s" è
già selezionato” and “Contributore "%s" pronto per essere creato” to the
corresponding sections of en_US.json and de_DE.json, using appropriate English
and German translations while preserving the existing JSON structure.
In `@storage/plugins/oai-pmh-server/OaiPmhServerPlugin.php`:
- Around line 2060-2064: Aggiorna il blocco MAG in writeBookOaiDc, vicino al
ciclo sugli authors, per includere anche i contributori provenienti dai campi
testuali legacy come traduttore e illustratore quando non sono già rappresentati
come entità. Riutilizza la stessa logica di fallback adottata da writeBookOaiDc
negli altri formati, mantenendo creator per principale/co-autore e contributor
per gli altri ruoli.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46e5922f-e9eb-4be8-8ae1-9e90990263bf
📒 Files selected for processing (52)
.github/workflows/ci-upgrade-smoke.ymlREADME.mdSTATUS.mdbin/build-release.shdocs/CREATING_UPDATES.mddocs/README.MDinstaller/README.mdinstaller/classes/Installer.phpinstaller/steps/step7.phplocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsonscripts/create-release.shscripts/list-source-expectations.phpscripts/verify-schema.shstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/book-club/BookClubPlugin.phpstorage/plugins/book-club/README.mdstorage/plugins/book-club/src/Modules/AbstractModule.phpstorage/plugins/book-club/src/Modules/AffinityModule.phpstorage/plugins/book-club/src/Modules/AiModule.phpstorage/plugins/book-club/src/Modules/BuddyModule.phpstorage/plugins/book-club/src/Modules/ChallengesModule.phpstorage/plugins/book-club/src/Modules/DiscussionsModule.phpstorage/plugins/book-club/src/Modules/GamificationModule.phpstorage/plugins/book-club/src/Modules/GovernanceModule.phpstorage/plugins/book-club/src/Modules/LendingModule.phpstorage/plugins/book-club/src/Modules/LibraryModule.phpstorage/plugins/book-club/src/Modules/ModuleInterface.phpstorage/plugins/book-club/src/Modules/QuotesModule.phpstorage/plugins/book-club/src/Modules/ReadingModule.phpstorage/plugins/book-club/src/Modules/Registry.phpstorage/plugins/book-club/src/Modules/SeasonsModule.phpstorage/plugins/book-club/src/Modules/SprintsModule.phpstorage/plugins/book-club/src/Modules/StatsModule.phpstorage/plugins/book-club/src/Modules/SurveysModule.phpstorage/plugins/frbr-lrm/FrbrLrmPlugin.phpstorage/plugins/mobile-api/MobileApiPlugin.phpstorage/plugins/ncip-server/NcipServerPlugin.phpstorage/plugins/oai-pmh-server/OaiPmhServerPlugin.phpstorage/plugins/viaf-authority/ViafAuthorityPlugin.phpstorage/plugins/z39-server/Z39ServerPlugin.phptests/code-quality.spec.jstests/helpers/plugin-schema-source.phptests/helpers/source-expectations.jstests/plugin-integrity.spec.jstests/plugin-schema-expectations-static.unit.phptests/plugin-schema-guard.unit.phptests/plugin-schema-selfheal.unit.phptests/schema-integrity.spec.jstests/source-expectations.unit.php
Cut the 0.7.36 release candidate and resolve the outstanding review
findings on the schema-derivation work.
Release candidate:
- version.json -> 0.7.36-rc.1 (a hyphen marks it a GitHub prerelease).
- Rename migrate_0.7.36.sql -> migrate_0.7.36-rc.1.sql: a pre-release sorts
BELOW its final, so version_compare('0.7.36','0.7.36-rc.1','<=') is false
and the updater would skip the migration on the RC. The rc-named file runs
on the RC, on the eventual stable 0.7.36, and for 0.7.35 upgraders alike.
- verify-schema.sh derives the release migration test from version.json
instead of a hardcoded name that had gone stale (pinned to 0.7.31, so it
never ran the 0.7.36 test), and fails loudly if a migration ships untested.
- Bump oai-pmh-server 1.1.0 -> 1.1.1 for the MAG change below.
Review findings:
- installer/steps/step7.php: guard the schema table-count parse so an
unreadable/unparseable schema.sql on the final success screen cannot fatal
it (parseCreateTableNames throws by design).
- oai-pmh-server: the MAG Dublin Core sub-record now includes contributors
still held on the legacy free-text columns (mirrors oai_dc/marcxml) so they
aren't dropped before the backfill completes.
- tests/helpers/source-expectations.js: the BundledPlugins::LIST parser now
rejects any unparseable entry instead of silently skipping it.
- docs: blank line + fenced-code language on the plugin-system section.
Owner
Author
Code reviewBranch: Found 15 findings across all lanes:
Deep lane — correctness & security✓ Auto-fixable (7)
Details and fix proposalsF001 — RC-1 upgrade never runs ContributorBackfill because version_compare($toVersion,'0.7.36','>=') is false when $toVersion is '0.7.36-rc.1' (prerelease sorts before final), so this release's production/Docker upgrade path silently skips the legacy contributor conversion the comment guarantees.File: F004 — htmlspecialchars($authorDisplay) called WITHOUT ENT_QUOTES/'UTF-8' (required by CLAUDE.md rule 3 and used elsewhere in the same hunk) — inconsistent escaping in new contributor-role display code.File: F009 — Admin form no longer persists libri.traduttore/illustratore/curatore (writes guarded by array_key_exists; controller removed those keys from $fields), but consumers still READ the columns: CSV export (LibriController:3370) and public API (PublicApiController:125/203). A translator/curator set via the new #237 entity picker vanishes from CSV export and the API's traduttore field. Also weakens prior fix d179730 (persist curatore free-text).File: F011 — processAuthorId changed from returning 0 (skip one bad id) to throwing InvalidArgumentException on non-positive/unresolved/malformed id. Since syncContributors runs it in the contributor transaction, one malformed/zero id in autori_ids now rolls back the ENTIRE book save instead of dropping one contributor — a robustness regression for form resubmissions.File: F015 — Same guard bug in the standalone upgrader: version_compare($targetVersion,'0.7.36','>=') is FALSE when $targetVersion is '0.7.36-rc.1' (read verbatim from the package version.json, no prerelease stripping), so the backfill block never runs for this release, diverging from the 'equivalent to in-app and Docker paths' guarantee the comment states.File: F017 — Autocomplete author search interpolates words into LIKE patterns without escaping SQL wildcards (%,_); sibling PublicApiController was hardened with ESCAPE in the same PR but this path (touched to add the pseudonimo OR-clause) was left unescaped.File: F019 — Mobile API catalog author filter (extended in this PR to match a.pseudonimo) interpolates query/author into LIKE without escaping %,_ — unlike PublicApiController hardened in the same PR.File: ℹ Uncertain (2)
Phase 4 couldn't confirm decisively. Re-run Light lane — ux, policy, architecture
|
…or cache, escaping) - Updater.php + manual-upgrade.php: the ContributorBackfill gate compared the target version against the bare '0.7.36' with '>=', which is FALSE for the shipping '0.7.36-rc.1' (a pre-release sorts below its final), so the synchronous backfill was skipped on the RC upgrade path (Docker + standalone upgrader). Compare against '0.7.36-rc.1' so the RC runs it too; the stable 0.7.36 still qualifies. Verified against a real bibliodoc upgrade. - BookRepository::syncContributors: the admin form stopped writing the free-text libri.traduttore/illustratore/curatore columns, but CSV export and the public REST API still read them, so picker-set contributors vanished there. Refresh those columns from the synced entity names (per role actually provided; colorista has no free-text column) so every reader stays consistent. - SearchController + mobile-api CatalogController: author LIKE clauses now use ESCAPE and escape %/_ in the bound value, matching the PublicApiController hardening so a query wildcard can't broaden the match. - book-detail.php: htmlspecialchars($authorDisplay) now passes ENT_QUOTES/UTF-8 like the sibling lines in the same hunk. Regression: PHPStan L5 = 0, full-test.spec.js 136 passed, issue-237 E2E green.
Project exception-handling rule: strict_types TypeErrors extend \Error, so a bare \Exception catch would miss them. Aligns the new test with the convention. (code-review finding F013)
The scraping handler reused the contributor picker's addName() to populate translator/illustrator, so each fired a "ready to be created" success toast that stacked on the final "import complete" toast — unlike the author path, which populates silently. addName() now takes a silent flag; the scraping call sites pass it. Manual Enter-key entry is unchanged. (code-review F016)
…tors (F011) processAuthorId throws on a non-positive / unresolved-new_ / wrong-type id (a frontend contract slip). syncContributors ran it inside the contributor transaction, so one bad id rolled back the entire book save. Catch the InvalidArgumentException per contributor, log a warning, and skip just that entry — the empty/null case still returns 0 and is skipped as before. (code-review walkthrough, finding F011)
Owner
Author
Walkthrough decisions
Walking the Qualifying scope: of 2 non-auto-eligible finding(s), 1 promoted & fixed, 1 skipped. Promoted & applied
Skipped
Decisions log: append-only audit. Current state: see the main review comment. |
The docstring said splitNames "does NOT split on a lone comma", but it does split a comma list when every part is a multi-word name (e.g. "Mario Rossi, Gianni Verdi"), while preserving single-word inverted "Surname, Forename" forms. Rewrote the comment to match the delegated implementation. (code-review walkthrough, finding F005)
Owner
Author
Walkthrough decisions
Walking the Full skip set scope: of 4 non-auto-eligible finding(s), 1 promoted & fixed, 3 skipped. Promoted & applied
Skipped (assessed, no change)
Decisions log: append-only audit. Current state: see the main review comment. |
Address a deeper review of the contributor-roles work that surfaced three data-loss bugs and three correctness gaps: - Atomic save (P1): createBasic()/updateBasic() now wrap the libri row, contributor links, legacy compatibility cache and publisher links in one transaction (savepoint when the caller already owns one). An invalid FK or malformed id rolls the whole write back instead of leaving an orphan book or a half-saved title. Publisher inserts propagate errors too. - Malformed contributor id (P1): a non-positive / unresolved / wrong-type id throws again (rolled back by the atomic scope) instead of being swallowed — swallowing could empty $desired and delete every existing author link. - Legacy compatibility cache (P1): the free-text illustratore/traduttore/ curatore columns are only overwritten when a role has entity links or an explicit removal; untouched legacy text awaiting the backfill is preserved. - Comma splitting (P2): splitNames() no longer splits on a comma at all — a multi-word inverted "García Márquez, Gabriel José" SBN name is one person, not two. AuthorNormalizer still canonicalises the inverted form. - Primary-author priority (P2): OAI-PMH, BIBFRAME, NCIP and Z39.50 order contributors by role (principale first) so a co-author/illustrator with a NULL ordine_credito can't take the main-author slot. - Upgrade smoke (P2): ci-upgrade-smoke now runs the real Updater::runMigrations() so the PHP backfill + marker are exercised, not just the raw SQL. Regression: PHPStan L5 = 0, full-test.spec.js 136 passed, issue-237 E2E green, contributor-roles unit 50/50 (the three previously-failing CI cases now pass).
Add tests/issue-237-regression.unit.php — a reusable 25-check suite pinning every invariant the contributor-roles review rounds proved easy to regress: RC-safe version guards, migration/schema enum parity, provenance PK co-ownership, comma-safe splitNames, pseudonym display, atomic saves (no orphan book, rollback on malformed id), the legacy compatibility cache, per-source re-import semantics, exhaustive LIKE-ESCAPE and principale-first interop ordering. Static checks parse the shipped sources; DB checks run against the real schema and skip cleanly without one. The suite caught three real gaps on its first run, fixed here: - SearchController: two of the three author LIKE paths (searchAuthors, searchAuthorsWithDetails) shipped WITHOUT ESCAPE — an earlier replace-all fix silently missed their indentation. All three now escape wildcards. - mobile-api CatalogController: the free-text q search bound unescaped wildcards across all nine LIKE fields; now escaped with ESCAPE like the author filter and PublicApiController. Also address the follow-up review of the atomicity work (all P3): - rollbackWriteScope no longer masks the original error when a deadlock has already discarded the savepoints (best-effort rollback, rethrow intact). - The contributor cache is joined in PHP instead of GROUP_CONCAT (which truncates silently at group_concat_max_len mid-name) and capped at the varchar(255) column on a name boundary, so a strict-mode 1406 can never abort a book save. - A stale scraped_translator/illustrator no longer clobbers the entity-derived cache when the submit carries the authoritative *_ids picker lists. - The cache UPDATE carries the deleted_at IS NULL guard (project rule 2). Regression: PHPStan L5 = 0, new suite 25/25, contributor-roles 50/50, migration 26/26, issue-237 E2E green, full-test.spec.js 136 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #237.
Two independent gaps @ctariel reported, both now addressed.
1. Contributor roles (illustrator / translator / curator / colorist)
For comics and illustrated books, contributors other than the main author were plain free-text columns on
libri— no autocomplete, and they never appeared as authors. Thelibri_autori.ruoloenum already modeled these roles but the form only ever wroteprincipale.Now each role is a first-class author entity via
libri_autori.ruolo:coloristato the enum (migrate_0.7.36.sql, guarded/idempotent)./api/search/autoriautocomplete as authors, replacing the free-text inputs. A genericinitContributorPickerdrives all four without touching the bespoke authors picker.BookRepository::syncAuthors→syncContributorswrites every role; the controller resolves each picker (existing + new names, find-or-created).App\Support\ContributorRolesfor localized labels). The public author page already lists a contributor's books, so an illustrator now shows up in the authors system and on their own page.2. Pseudonym search & display
nomeORpseudonimo, so typing "Leo" finds the author.App\Support\AuthorNamecentralizes the display form "Pseudonym (Real name)" (PHP + SQL), applied to the list/detail/dashboard queries and the form chips. No pseudonym → just the real name, unchanged.Upgrade path
The
.sqlmigration runner can't do the free-text→entity conversion (comma-split + find-or-create), soApp\Support\ContributorBackfilldoes it once as a guarded self-heal fromMaintenanceService::runAll()(cron + admin login), idempotent via asystem_settingsmarker +INSERT IGNORE. Legacy free-text columns are retained and only written when explicitly provided, so nothing is nulled before the backfill.Verification
tests/migration-0.7.36.unit.php— 19/19: enum ALTER idempotency,splitNames, the realContributorBackfill::run()in a rolled-back transaction (comma-split, find-or-create reuse, marker, idempotency), and drift guards.Leo (Luiz Eduardo de Oliveira); the book detail shows the pseudonym while a no-pseudonym author shows plain; saving a new illustrator entity persists alibri_autoriillustrator row and shows on the book detail and the illustrator's author page; public book page renders.Design doc:
docs/superpowers/specs/2026-07-14-author-roles-pseudonym-237-design.md.Summary by CodeRabbit