Skip to content

Commit 1d46bf9

Browse files
fix: CodeRabbit round 8 — 8 fixes
1. Discogs: register scrape.isbn.validate for EAN/UPC barcodes 2. mergeBookData: dynamic source from payload, not hardcoded 'discogs' 3. enrichWithDiscogsData: resolveTipoMedia (honors explicit tipo_media) 4. Authors array normalize (handle structured ['name' => '...']) 5. strip_tags: br→newline before stripping in tracklist rendering 6. PluginManager getPluginInstance: try/catch \Throwable → null 7. Updater: backup cleanup non-fatal after successful swap 8. Migration: AFTER formato guard (check column exists)
1 parent 5c0d069 commit 1d46bf9

11 files changed

Lines changed: 230 additions & 1129 deletions

File tree

app/Support/PluginManager.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,15 @@ public function getPluginInstance(int $pluginId): ?object
360360
return null;
361361
}
362362

363-
return $this->instantiatePlugin($plugin);
363+
try {
364+
return $this->instantiatePlugin($plugin);
365+
} catch (\Throwable $e) {
366+
SecureLogger::warning("[PluginManager] Failed to instantiate plugin {$plugin['name']}", [
367+
'plugin_id' => $pluginId,
368+
'error' => $e->getMessage(),
369+
]);
370+
return null;
371+
}
364372
}
365373

366374
/**

app/Support/Updater.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2259,7 +2259,15 @@ private function updateBundledPlugins(string $sourcePath): void
22592259
}
22602260

22612261
if (is_dir($backupPath)) {
2262-
$this->removeDirectoryTree($backupPath);
2262+
try {
2263+
$this->removeDirectoryTree($backupPath);
2264+
} catch (\Throwable $cleanupError) {
2265+
$this->debugLog('WARNING', 'Impossibile rimuovere backup plugin', [
2266+
'plugin' => $pluginSlug,
2267+
'backup' => $backupPath,
2268+
'error' => $cleanupError->getMessage(),
2269+
]);
2270+
}
22632271
}
22642272
} catch (\Throwable $e) {
22652273
if (is_dir($stagingPath)) {

app/Views/libri/scheda_libro.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,12 @@ class="inline-flex items-center gap-1 px-2 py-1 text-xs font-medium rounded-full
601601
<div class="card-body">
602602
<div class="prose prose-sm max-w-none text-gray-700">
603603
<?php if ($isMusic): ?>
604-
<?= \App\Support\MediaLabels::formatTracklist(strip_tags($libro['descrizione'])) ?>
604+
<?php
605+
$descText = preg_replace('/<br\s*\/?>/i', "\n", $libro['descrizione']);
606+
$descText = preg_replace('/<\/(?:p|div|li|h[1-6])>/i', "\n", (string) $descText);
607+
$descText = strip_tags((string) $descText);
608+
?>
609+
<?= \App\Support\MediaLabels::formatTracklist($descText) ?>
605610
<?php else: ?>
606611
<?php echo App\Support\HtmlHelper::sanitizeHtml(nl2br($libro['descrizione'], false)); ?>
607612
<?php endif; ?>

data/dewey/dewey_completo_it.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6822,6 +6822,12 @@
68226822
"name": "Narrativa inglese",
68236823
"level": 3,
68246824
"children": [
6825+
{
6826+
"code": "823.7",
6827+
"name": "Narrativa Inglese, 1800-1837",
6828+
"level": 4,
6829+
"children": []
6830+
},
68256831
{
68266832
"code": "823.9",
68276833
"name": "Narrativa Inglese",

installer/database/migrations/migrate_0.5.4.sql

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,14 @@ SET @col_exists = (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
55
WHERE TABLE_SCHEMA = DATABASE()
66
AND TABLE_NAME = 'libri'
77
AND COLUMN_NAME = 'tipo_media');
8+
SET @formato_exists = (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
9+
WHERE TABLE_SCHEMA = DATABASE()
10+
AND TABLE_NAME = 'libri'
11+
AND COLUMN_NAME = 'formato');
812
SET @sql = IF(@col_exists = 0,
9-
"ALTER TABLE libri ADD COLUMN tipo_media ENUM('libro','disco','audiolibro','dvd','altro') NOT NULL DEFAULT 'libro' AFTER formato",
13+
IF(@formato_exists > 0,
14+
"ALTER TABLE libri ADD COLUMN tipo_media ENUM('libro','disco','audiolibro','dvd','altro') NOT NULL DEFAULT 'libro' AFTER formato",
15+
"ALTER TABLE libri ADD COLUMN tipo_media ENUM('libro','disco','audiolibro','dvd','altro') NOT NULL DEFAULT 'libro'"),
1016
'SELECT 1');
1117
PREPARE stmt FROM @sql;
1218
EXECUTE stmt;

storage/plugins/discogs/DiscogsPlugin.php

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ public function activate(): void
4646
Hooks::add('scrape.sources', [$this, 'addDiscogsSource'], 8);
4747
Hooks::add('scrape.fetch.custom', [$this, 'fetchFromDiscogs'], 8);
4848
Hooks::add('scrape.data.modify', [$this, 'enrichWithDiscogsData'], 15);
49+
Hooks::add('scrape.isbn.validate', [$this, 'validateBarcode'], 5);
50+
}
51+
52+
/**
53+
* Validate barcode: accept ISBN, EAN-13, and UPC-A codes
54+
*/
55+
public function validateBarcode(bool $isValid, string $isbn): bool
56+
{
57+
// If already valid (ISBN), keep it
58+
if ($isValid) {
59+
return true;
60+
}
61+
// Accept EAN-13 (13 digits) and UPC-A (12 digits)
62+
$clean = preg_replace('/[^0-9]/', '', $isbn);
63+
return strlen((string) $clean) === 13 || strlen((string) $clean) === 12;
4964
}
5065

5166
/**
@@ -107,6 +122,7 @@ private function registerHooks(): void
107122
['scrape.sources', 'addDiscogsSource', 8],
108123
['scrape.fetch.custom', 'fetchFromDiscogs', 8],
109124
['scrape.data.modify', 'enrichWithDiscogsData', 15],
125+
['scrape.isbn.validate', 'validateBarcode', 5],
110126
];
111127

112128
$stmt = null;
@@ -310,7 +326,12 @@ private function searchDiscogsByTitleArtist($currentResult, ?string $token): ?ar
310326
$artist = trim((string) ($currentResult['author'] ?? ''));
311327
if ($artist === '' && !empty($currentResult['authors'])) {
312328
if (is_array($currentResult['authors'])) {
313-
$artist = trim((string) ($currentResult['authors'][0] ?? ''));
329+
$firstAuthor = $currentResult['authors'][0] ?? '';
330+
if (is_array($firstAuthor)) {
331+
$artist = trim((string) ($firstAuthor['name'] ?? ''));
332+
} else {
333+
$artist = trim((string) $firstAuthor);
334+
}
314335
} else {
315336
$artist = trim((string) $currentResult['authors']);
316337
}
@@ -367,9 +388,11 @@ public function enrichWithDiscogsData(array $data, string $isbn, array $source,
367388
}
368389

369390
// Only enrich from Deezer for music sources (avoid attaching music covers to books)
370-
$resolvedType = \App\Support\MediaLabels::inferTipoMedia($data['format'] ?? $data['formato'] ?? '');
371-
$isMusicSource = ($data['tipo_media'] ?? '') === 'disco'
372-
|| $resolvedType === 'disco'
391+
$resolvedType = \App\Support\MediaLabels::resolveTipoMedia(
392+
$data['format'] ?? $data['formato'] ?? null,
393+
$data['tipo_media'] ?? null
394+
);
395+
$isMusicSource = $resolvedType === 'disco'
373396
|| ($data['source'] ?? '') === 'discogs'
374397
|| ($data['source'] ?? '') === 'musicbrainz';
375398

@@ -938,7 +961,8 @@ private function mergeBookData(?array $existing, ?array $new): ?array
938961
{
939962
// Use BookDataMerger if available
940963
if (class_exists('\\App\\Support\\BookDataMerger')) {
941-
return \App\Support\BookDataMerger::merge($existing, $new, 'discogs');
964+
$mergeSource = $new['source'] ?? ($existing['source'] ?? 'discogs');
965+
return \App\Support\BookDataMerger::merge($existing, $new, $mergeSource);
942966
}
943967

944968
// Fallback: simple merge

tests/seed-catalog.spec.js

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// @ts-check
2+
/**
3+
* Seed the catalog with books and music records.
4+
* This is a SEEDER — it does NOT clean up. Records persist for manual testing.
5+
* Run: /tmp/run-e2e.sh tests/seed-catalog.spec.js --config=tests/playwright.config.js --workers=1
6+
*/
7+
const { test, expect } = require('@playwright/test');
8+
const { execFileSync } = require('child_process');
9+
10+
const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081';
11+
const ADMIN_EMAIL = process.env.E2E_ADMIN_EMAIL || '';
12+
const ADMIN_PASS = process.env.E2E_ADMIN_PASS || '';
13+
14+
// 10 music records via Discogs barcode scraping
15+
const MUSIC_BARCODES = [
16+
{ barcode: '0720642442524', note: 'Nirvana - Nevermind' },
17+
{ barcode: '5099902894225', note: 'Pink Floyd - Meddle' },
18+
{ barcode: '0094638246824', note: 'Beatles - Abbey Road' },
19+
{ barcode: '0888837168625', note: 'Daft Punk - RAM' },
20+
{ barcode: '5099751076322', note: 'AC/DC' },
21+
{ barcode: '0602547428714', note: 'Adele - 25' },
22+
{ barcode: '0602537615810', note: 'Arctic Monkeys - AM' },
23+
{ barcode: '0886971592924', note: 'Muse - The Resistance' },
24+
{ barcode: '0602527947747', note: 'Coldplay - Mylo Xyloto' },
25+
{ barcode: '0602557048032', note: 'Metallica - Hardwired' },
26+
];
27+
28+
// 5 books via ISBN scraping
29+
const BOOK_ISBNS = [
30+
{ isbn: '9780061120084', note: 'To Kill a Mockingbird' },
31+
{ isbn: '9780451524935', note: '1984' },
32+
{ isbn: '9780141439518', note: 'Pride and Prejudice' },
33+
{ isbn: '9780060935467', note: 'Don Quixote' },
34+
{ isbn: '9780142437230', note: 'Moby Dick' },
35+
];
36+
37+
// 1 manual entry (punk split without barcode)
38+
const MANUAL_ENTRIES = [
39+
{ titolo: 'Zeromila / Orsetti HC — Split', formato: 'vinile', tipo_media: 'disco' },
40+
];
41+
42+
test.describe.serial('Seed Catalog (books + music)', () => {
43+
/** @type {import('@playwright/test').Page} */
44+
let page;
45+
/** @type {import('@playwright/test').BrowserContext} */
46+
let context;
47+
48+
test.beforeAll(async ({ browser }) => {
49+
test.skip(!ADMIN_EMAIL || !ADMIN_PASS, 'Missing env vars');
50+
context = await browser.newContext();
51+
page = await context.newPage();
52+
53+
await page.goto(`${BASE}/accedi`);
54+
await page.fill('input[name="email"]', ADMIN_EMAIL);
55+
await page.fill('input[name="password"]', ADMIN_PASS);
56+
await page.click('button[type="submit"]');
57+
await page.waitForURL(/\/admin\//, { timeout: 15000 });
58+
});
59+
60+
test.afterAll(async () => {
61+
// DO NOT clean up — this is a seeder
62+
await context?.close();
63+
});
64+
65+
// Seed music records via barcode scraping
66+
for (let i = 0; i < MUSIC_BARCODES.length; i++) {
67+
const rec = MUSIC_BARCODES[i];
68+
test(`Music ${i + 1}: ${rec.note}`, async () => {
69+
test.setTimeout(30000);
70+
await page.goto(`${BASE}/admin/libri/crea`);
71+
await page.waitForLoadState('domcontentloaded');
72+
73+
const importBtn = page.locator('#btnImportIsbn');
74+
if (await importBtn.isVisible({ timeout: 3000 }).catch(() => false)) {
75+
await page.fill('#importIsbn', rec.barcode);
76+
await importBtn.click();
77+
await page.waitForTimeout(8000);
78+
79+
const title = await page.locator('input[name="titolo"]').inputValue();
80+
if (!title) {
81+
await page.locator('input[name="titolo"]').fill(`CD (${rec.barcode})`);
82+
await page.locator('input[name="ean"]').fill(rec.barcode);
83+
await page.locator('input[name="formato"]').fill('cd_audio');
84+
}
85+
} else {
86+
await page.locator('input[name="titolo"]').fill(`CD (${rec.barcode})`);
87+
await page.locator('input[name="ean"]').fill(rec.barcode);
88+
await page.locator('input[name="formato"]').fill('cd_audio');
89+
}
90+
91+
const copie = await page.locator('input[name="copie_totali"]').inputValue();
92+
if (!copie || copie === '0') await page.locator('input[name="copie_totali"]').fill('1');
93+
94+
await page.locator('button[type="submit"]').first().click();
95+
const swal = page.locator('.swal2-confirm');
96+
if (await swal.isVisible({ timeout: 3000 }).catch(() => false)) await swal.click();
97+
await page.waitForURL(/\/admin\/libri\/\d+/, { timeout: 15000 }).catch(() => {});
98+
console.log(` ✓ ${rec.note}`);
99+
});
100+
}
101+
102+
// Seed books via ISBN
103+
for (let i = 0; i < BOOK_ISBNS.length; i++) {
104+
const book = BOOK_ISBNS[i];
105+
test(`Book ${i + 1}: ${book.note}`, async () => {
106+
test.setTimeout(30000);
107+
await page.goto(`${BASE}/admin/libri/crea`);
108+
await page.waitForLoadState('domcontentloaded');
109+
110+
const importBtn = page.locator('#btnImportIsbn');
111+
if (await importBtn.isVisible({ timeout: 3000 }).catch(() => false)) {
112+
await page.fill('#importIsbn', book.isbn);
113+
await importBtn.click();
114+
await page.waitForTimeout(8000);
115+
116+
const title = await page.locator('input[name="titolo"]').inputValue();
117+
if (!title) {
118+
await page.locator('input[name="titolo"]').fill(book.note);
119+
await page.locator('input[name="isbn13"]').fill(book.isbn);
120+
}
121+
} else {
122+
await page.locator('input[name="titolo"]').fill(book.note);
123+
await page.locator('input[name="isbn13"]').fill(book.isbn);
124+
}
125+
126+
const copie = await page.locator('input[name="copie_totali"]').inputValue();
127+
if (!copie || copie === '0') await page.locator('input[name="copie_totali"]').fill('1');
128+
129+
await page.locator('button[type="submit"]').first().click();
130+
const swal = page.locator('.swal2-confirm');
131+
if (await swal.isVisible({ timeout: 3000 }).catch(() => false)) await swal.click();
132+
await page.waitForURL(/\/admin\/libri\/\d+/, { timeout: 15000 }).catch(() => {});
133+
console.log(` ✓ ${book.note}`);
134+
});
135+
}
136+
137+
// Manual entries
138+
for (let i = 0; i < MANUAL_ENTRIES.length; i++) {
139+
const entry = MANUAL_ENTRIES[i];
140+
test(`Manual ${i + 1}: ${entry.titolo}`, async () => {
141+
test.setTimeout(15000);
142+
await page.goto(`${BASE}/admin/libri/crea`);
143+
await page.waitForLoadState('domcontentloaded');
144+
145+
await page.locator('input[name="titolo"]').fill(entry.titolo);
146+
await page.locator('input[name="formato"]').fill(entry.formato);
147+
if (entry.tipo_media) {
148+
const sel = page.locator('#tipo_media');
149+
if (await sel.isVisible({ timeout: 2000 }).catch(() => false)) {
150+
await sel.selectOption(entry.tipo_media);
151+
}
152+
}
153+
await page.locator('input[name="copie_totali"]').fill('1');
154+
155+
await page.locator('button[type="submit"]').first().click();
156+
const swal = page.locator('.swal2-confirm');
157+
if (await swal.isVisible({ timeout: 3000 }).catch(() => false)) await swal.click();
158+
await page.waitForURL(/\/admin\/libri\/\d+/, { timeout: 15000 }).catch(() => {});
159+
console.log(` ✓ ${entry.titolo}`);
160+
});
161+
}
162+
});

0 commit comments

Comments
 (0)