Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/Controllers/CopyController.php
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ public function createCopy(Request $request, Response $response, mysqli $db, int
// Re-evaluate after sanitisation: an explicit value made only of
// control characters must fall back to automatic allocation.
if ($numero === '') {
$base = !empty($book['numero_inventario']) ? (string) $book['numero_inventario'] : "LIB-{$bookId}";
$base = CopyRepository::inventoryBase($bookId, $book['numero_inventario'] ?? null);
$newCopyIds = $repo->createManyForBookWithIdsAndNote(
$bookId,
$base,
Expand Down
4 changes: 1 addition & 3 deletions app/Controllers/LibriController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1335,9 +1335,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons
// Genera copie fisiche del libro
$copyRepo = new \App\Models\CopyRepository($db);
$copieTotali = (int) $fields['copie_totali'];
$baseInventario = !empty($fields['numero_inventario'])
? $fields['numero_inventario']
: "LIB-{$id}";
$baseInventario = \App\Models\CopyRepository::inventoryBase($id, $fields['numero_inventario'] ?? null);

// Create the requested holding set with one atomic multi-row INSERT:
// a request for three copies must never leave a partial 1/3 or 2/3
Expand Down
16 changes: 16 additions & 0 deletions app/Models/CopyRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,22 @@ public function getById(int $id): ?array
return $copia ?: null;
}

/**
* Base of the "{base}-C{N}" codes given to new copies of a book: the book's
* own "Prefisso inventario copie" (libri.numero_inventario) when it has
* one, otherwise "LIB-{id}". Every path that allocates copy codes reads the
* rule from here, so a library with its own inventory scheme gets it
* whichever screen, plugin or API created the copy (#489).
*
* $bookPrefix is the raw column/form value. A blank or whitespace-only
* value means "no prefix"; surrounding whitespace is never part of a code.
*/
public static function inventoryBase(int $bookId, mixed $bookPrefix): string
{
$prefix = is_scalar($bookPrefix) ? trim((string) $bookPrefix) : '';
return $prefix !== '' ? $prefix : "LIB-{$bookId}";
}

/**
* Crea una nuova copia
*/
Expand Down
4 changes: 1 addition & 3 deletions app/Routes/web.php
Original file line number Diff line number Diff line change
Expand Up @@ -2658,9 +2658,7 @@
// manually-inflated copie_totali and a partial copy set behind. No manual
// UPDATE, one transaction, rolled back on any failure.
$copyRepo = new \App\Models\CopyRepository($db);
$baseInventario = !empty($book['numero_inventario'])
? $book['numero_inventario']
: "LIB-{$bookId}";
$baseInventario = \App\Models\CopyRepository::inventoryBase($bookId, $book['numero_inventario'] ?? null);

$reassignmentService = null;
$reservationManager = null;
Expand Down
16 changes: 10 additions & 6 deletions storage/plugins/desiderata/DesiderataPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ private function registerCopies(int $bookId, int $howMany): void
{
$this->db->begin_transaction();
try {
$stmt = $this->db->prepare('SELECT titolo FROM libri l WHERE l.id = ? AND l.deleted_at IS NULL AND l.is_desiderata = 1 AND NOT EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = l.id) FOR UPDATE');
$stmt = $this->db->prepare('SELECT titolo, numero_inventario FROM libri l WHERE l.id = ? AND l.deleted_at IS NULL AND l.is_desiderata = 1 AND NOT EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = l.id) FOR UPDATE');
if ($stmt === false) { throw new RuntimeException($this->db->error); }
$stmt->bind_param('i', $bookId);
$stmt->execute();
Expand All @@ -565,8 +565,10 @@ private function registerCopies(int $bookId, int $howMany): void
if (!$book) { $this->db->rollback(); return; }
$title = (string) ($book['titolo'] ?? '');
$this->assertNoOpenOffers($bookId);
// The book's own inventory prefix, as every core path uses it (#489).
$base = \App\Models\CopyRepository::inventoryBase($bookId, $book['numero_inventario'] ?? null);
$ids = (new \App\Models\CopyRepository($this->db))
->createManyForBookWithIdsAndNote($bookId, 'LIB-' . $bookId, $howMany, 'disponibile', 'Donazione diretta');
->createManyForBookWithIdsAndNote($bookId, $base, $howMany, 'disponibile', 'Donazione diretta');
if (count($ids) !== $howMany) { throw new RuntimeException('Copy creation failed'); }
$copyId = $ids[0];
$stmt = $this->db->prepare('UPDATE libri SET is_desiderata = 0' . \App\Support\BookVisibility::catalogueStamp($this->db) . ' WHERE id = ? AND deleted_at IS NULL');
Expand Down Expand Up @@ -1550,7 +1552,7 @@ public function receiveDirect(Request $q, Response $r, int $bookId): Response
// inconsistency DataIntegrity exists to repair, and without this
// clause a receipt recorded in that window would answer it by
// creating a SECOND inventory copy rather than refusing.
$stmt = $this->db->prepare('SELECT titolo FROM libri l WHERE id = ? AND deleted_at IS NULL AND is_desiderata = 1 AND NOT EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = l.id) FOR UPDATE');
$stmt = $this->db->prepare('SELECT titolo, numero_inventario FROM libri l WHERE id = ? AND deleted_at IS NULL AND is_desiderata = 1 AND NOT EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = l.id) FOR UPDATE');
if ($stmt === false) { throw new RuntimeException($this->db->error); }
$stmt->bind_param('i', $bookId);
$stmt->execute();
Expand All @@ -1559,7 +1561,8 @@ public function receiveDirect(Request $q, Response $r, int $bookId): Response
if (!$book) { throw new InvalidArgumentException(__('Il libro non è più una richiesta aperta.')); }
$title = (string) ($book['titolo'] ?? '');
$this->assertNoOpenOffers($bookId);
$copyId = (new \App\Models\CopyRepository($this->db))->createWithAllocatedInventoryCode($bookId, 'LIB-' . $bookId, 'disponibile', 'Donazione diretta');
$base = \App\Models\CopyRepository::inventoryBase($bookId, $book['numero_inventario'] ?? null);
$copyId = (new \App\Models\CopyRepository($this->db))->createWithAllocatedInventoryCode($bookId, $base, 'disponibile', 'Donazione diretta');
if ($copyId <= 0) { throw new RuntimeException('Copy creation failed'); }
// Self-guarding, per ABSOLUTE RULE 2, and kept even though
// DataIntegrity clears the flag by itself once a copy exists: the
Expand Down Expand Up @@ -1933,13 +1936,14 @@ public function manage(Request $q, Response $r, int $id): Response
throw new InvalidArgumentException(__('Seleziona la scheda del libro ricevuto.'));
}
$bookId = (int)$rawBookId;
$stmt = $this->db->prepare('SELECT id FROM libri WHERE id=? AND deleted_at IS NULL FOR UPDATE');
$stmt = $this->db->prepare('SELECT id, numero_inventario FROM libri WHERE id=? AND deleted_at IS NULL FOR UPDATE');
if ($stmt === false) { throw new RuntimeException('Book lock prepare failed: ' . $this->db->error); }
$stmt->bind_param('i', $bookId); $stmt->execute();
$bookRow = $stmt->get_result()->fetch_assoc();
$stmt->close();
if (!$bookRow) { throw new InvalidArgumentException(__('Per la ricezione scegli una scheda libro esistente. Puoi prima crearla con zero copie.')); }
$copyId = (new \App\Models\CopyRepository($this->db))->createWithAllocatedInventoryCode($bookId, 'LIB-' . $bookId, 'disponibile', 'Donazione #' . $id);
$base = \App\Models\CopyRepository::inventoryBase($bookId, $bookRow['numero_inventario'] ?? null);
$copyId = (new \App\Models\CopyRepository($this->db))->createWithAllocatedInventoryCode($bookId, $base, 'disponibile', 'Donazione #' . $id);
if ($copyId <= 0) { throw new RuntimeException('Copy creation failed'); }
// Self-guarding, per ABSOLUTE RULE 2. Provably a no-op here —
// the row was locked FOR UPDATE and checked for deleted_at two
Expand Down
2 changes: 1 addition & 1 deletion storage/plugins/desiderata/plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "desiderata",
"display_name": "Desiderata e donazioni",
"description": "Libri cercati dalla biblioteca, ricerca pubblica e proposte di donazione con ricezione controllata.",
"version": "1.2.0",
"version": "1.2.1",
"author": "Pinakes",
"main_file": "wrapper.php",
"requires_php": "8.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,7 @@ public function addCopies(ServerRequestInterface $request, ResponseInterface $re
return ResponseEnvelope::error($response, 'not_found', __('Libro non trovato.'), 404);
}
$before = \App\Support\ActivityLog::loadBookSnapshot($this->db, $bookId);
$base = trim((string) ($book['numero_inventario'] ?? '')) !== ''
? (string) $book['numero_inventario']
: 'LIB-' . $bookId;
$base = \App\Models\CopyRepository::inventoryBase($bookId, $book['numero_inventario'] ?? null);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Same steps as POST /api/libri/{id}/increase-copies: one
// transaction, book row locked first, every new copy first repairs
Expand Down
163 changes: 163 additions & 0 deletions tests/desiderata-inventory-prefix-489.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// @ts-check
/**
* #489 — a copy received through Desiderata carries the book's own
* "Prefisso inventario copie", through the real admin screens.
*
* Each wanted book is created from the admin book form with a prefix, then
* received through one of the three buttons an operator actually has:
*
* A. edit the record, untick "Desiderata", save with two copies
* B. "Libro donato: registra la copia" on /admin/desiderata
* C. a donor's proposal: "Accetta proposta", then
* "Libro arrivato: registra una copia"
*
* The code is read where the operator reads it: the Copie Fisiche table of the
* book page. A fourth book without a prefix keeps the LIB-<id>-C1 default.
*
* Run: /tmp/run-e2e.sh tests/desiderata-inventory-prefix-489.spec.js --config=tests/playwright.config.js --workers=1
*/
const { test, expect } = require('@playwright/test');
const { execFileSync } = require('node:child_process');
const crypto = require('node:crypto');
const { ensurePluginActive } = require('./helpers/plugin-activation');

const ADMIN_EMAIL = process.env.E2E_ADMIN_EMAIL || '';
const ADMIN_PASS = process.env.E2E_ADMIN_PASS || '';
const DB_USER = process.env.E2E_DB_USER || '';
const DB_PASS = process.env.E2E_DB_PASS || '';
const DB_NAME = process.env.E2E_DB_NAME || '';
const DB_SOCKET = process.env.E2E_DB_SOCKET || '';

test.skip(!ADMIN_EMAIL || !ADMIN_PASS || !DB_NAME, 'Requires an installed app, E2E_ADMIN_* and E2E_DB_* credentials');
test.describe.configure({ mode: 'serial' });

// Unique per run: copy codes are globally unique, so a fixed prefix would
// continue a previous run's -C<n> family instead of starting at -C1.
const TAG = 'E2E489' + crypto.randomBytes(4).toString('hex');
const bookIds = [];

function dbQuery(sql) {
const args = ['-u', DB_USER, DB_NAME, '-N', '-B', '-e', sql];
if (DB_SOCKET) args.splice(2, 0, '-S', DB_SOCKET);
return execFileSync('mysql', args, { encoding: 'utf-8', timeout: 10000, env: { ...process.env, MYSQL_PWD: DB_PASS } }).trim();
}

async function login(page) {
await page.goto('/accedi');
await page.getByRole('textbox', { name: 'Email', exact: true }).fill(ADMIN_EMAIL);
await page.getByRole('textbox', { name: 'Password', exact: true }).fill(ADMIN_PASS);
await page.getByRole('button', { name: 'Accedi', exact: true }).click();
await page.waitForURL(/\/admin/);
}

/** Submit the core book form, confirming its SweetAlert when it asks. */
async function submitBookForm(page) {
await page.locator('#bookForm button[type="submit"]').click();
const confirm = page.locator('.swal2-confirm');
if (await confirm.isVisible({ timeout: 5000 }).catch(() => false)) {
await confirm.click();
}
await page.waitForURL(/\/admin\/books\/(?!create|edit)\d+/, { timeout: 15000 });
}

/** A wanted book created from the admin form, as an operator does it. */
async function createWanted(page, label, prefix) {
const title = `${TAG} ${label}`;
await page.goto('/admin/books/create');
await page.fill('#titolo', title);
await page.getByRole('checkbox', { name: 'Desiderata: cerchiamo questo libro', exact: true }).check();
if (prefix !== null) {
await page.fill('#numero_inventario', prefix);
}
await submitBookForm(page);
const id = Number(dbQuery(`SELECT id FROM libri WHERE titolo = '${title}' AND deleted_at IS NULL ORDER BY id DESC LIMIT 1`));
expect(id).toBeGreaterThan(0);
bookIds.push(id);
expect(dbQuery(`SELECT is_desiderata FROM libri WHERE id = ${id}`)).toBe('1');
return { id, title };
}

/** The codes the operator sees in the book page's Copie Fisiche table. */
async function expectCopyCodes(page, bookId, codes) {
await page.goto(`/admin/books/${bookId}#physical-copies`);
const section = page.locator('#physical-copies');
for (const code of codes) {
await expect(section).toContainText(code);
}
const stored = dbQuery(`SELECT GROUP_CONCAT(numero_inventario ORDER BY id SEPARATOR ',') FROM copie WHERE libro_id = ${bookId}`);
expect(stored).toBe(codes.join(','));
}

test.beforeAll(async ({ browser }) => {
const context = await browser.newContext();
try {
const page = await context.newPage();
await login(page);
await ensurePluginActive(page, 'desiderata', { smokePath: '/desiderata' });
} finally {
await context.close();
}
});

test.afterAll(() => {
if (bookIds.length === 0) return;
const list = bookIds.join(',');
dbQuery(`DELETE FROM desiderata_offers WHERE book_id IN (${list}) OR received_book_id IN (${list})`);
dbQuery(`DELETE FROM admin_notifications WHERE title LIKE '%${TAG}%' OR message LIKE '%${TAG}%'`);
dbQuery(`DELETE FROM copie WHERE libro_id IN (${list})`);
dbQuery(`DELETE FROM log_modifiche WHERE tabella = 'libri' AND record_id IN (${list})`);
dbQuery(`DELETE FROM libri WHERE id IN (${list})`);
});

test('A — closing the request from the book form uses the prefix', async ({ page }) => {
await login(page);
const prefix = `BIB-${TAG}A`;
const book = await createWanted(page, 'form', prefix);
await page.goto(`/admin/books/edit/${book.id}`);
await page.getByRole('checkbox', { name: 'Desiderata: cerchiamo questo libro', exact: true }).uncheck();
await page.fill('#desiderata_copies', '2');
await submitBookForm(page);
await expectCopyCodes(page, book.id, [`${prefix}-C1`, `${prefix}-C2`]);
});

test('B — "Libro donato" on the requests list uses the prefix', async ({ page }) => {
await login(page);
const prefix = `BIB-${TAG}B`;
const book = await createWanted(page, 'direct', prefix);
page.on('dialog', (dialog) => dialog.accept());
await page.goto('/admin/desiderata');
const row = page.locator('li').filter({ hasText: book.title });
await row.getByRole('button', { name: 'Libro donato: registra la copia', exact: true }).click();
await page.waitForURL(/\/admin\/desiderata/);
await expectCopyCodes(page, book.id, [`${prefix}-C1`]);
});

test('C — a donor proposal received through the admin buttons uses the prefix', async ({ page }) => {
await login(page);
const prefix = `BIB-${TAG}C`;
const book = await createWanted(page, 'offer', prefix);
// The donor's side is not under test here: the proposal row is what the
// public form writes, inserted directly so this spec does not depend on the
// anonymous form's rate limiter or captcha settings.
dbQuery(`INSERT INTO desiderata_offers (book_id, donor_name, donor_email, title, notes, status) VALUES (${book.id}, 'E2E donor', 'donor-${TAG}@example.invalid', '${book.title}', '', 'pending')`);
page.on('dialog', (dialog) => dialog.accept());
await page.goto('/admin/desiderata');
let offer = page.locator('article').filter({ hasText: book.title });
await offer.getByRole('button', { name: 'Accetta proposta', exact: true }).click();
offer = page.locator('article').filter({ hasText: book.title });
await expect(offer).toContainText('In attesa della consegna');
await offer.getByRole('button', { name: 'Libro arrivato: registra una copia', exact: true }).click();
await expect(page.locator('article').filter({ hasText: book.title })).toContainText('Ricevuta');
await expectCopyCodes(page, book.id, [`${prefix}-C1`]);
});

test('without a prefix the copy keeps the LIB-<id> default', async ({ page }) => {
await login(page);
const book = await createWanted(page, 'no prefix', null);
page.on('dialog', (dialog) => dialog.accept());
await page.goto('/admin/desiderata');
const row = page.locator('li').filter({ hasText: book.title });
await row.getByRole('button', { name: 'Libro donato: registra la copia', exact: true }).click();
await page.waitForURL(/\/admin\/desiderata/);
await expectCopyCodes(page, book.id, [`LIB-${book.id}-C1`]);
});
Loading
Loading