feat(copies): per-copy labels + loan/return by code + camera scanner (#238)#243
Merged
Conversation
… copy code Phase 1 of physical-copy tracking (discussion #238 requests 2a + 2b). Builds on the existing model (copie.numero_inventario UNIQUE, prestiti.copia_id) — no schema change. 2a — Per-copy labels: GET /admin/books/{id}/copy-labels-pdf prints one label per non-deleted copy, each carrying that copy's numero_inventario as text + a Code128 barcode (the label barcode payload is now parameterizable; the book-level label endpoint is unchanged). "Print copy labels" button added to the copies card. 2b — Loan/return against a specific copy: - GET /admin/copies/by-code?code=<numero_inventario> resolves a copy to {found, copy_id, libro_id, titolo, stato, available}. - The loan-create form takes an optional copy code; PrestitiController::store() resolves it (FOR UPDATE), validates it belongs to the book and is available, and pins that exact copia_id instead of auto-assigning. Empty code = auto-assign as before (backward compatible). - POST /admin/loans/return-by-code finds the single active loan for a scanned/typed code and returns it through the existing processReturn path (no duplicated logic). The camera scanner that fills these code fields is Phase 2. Verified end-to-end against the dev install: by-code resolves real + missing codes, the copy-labels PDF renders (200, application/pdf) and contains the copy's numero_inventario. Soft-delete scoped, admin routes English literals, strings in __() across it/en/de/fr, prepared statements. php -l + PHPStan L5 clean.
… codes Phase 2 of physical-copy tracking (discussion #238 request 3). Adds a "Scan" button next to the copy-code inputs from Phase 1 (loan-create form + return-by-code form) that opens a camera modal, decodes the Code128 barcode printed on the per-copy label, and fills the input — closing the gap between the copy having a code and using it without a hardware scanner. - frontend/js/copy-scanner.js: reusable scanner modal on zxing-wasm. Decodes Code128/EAN/Code39/QR, fills the target input (never auto-submits), and stops all MediaStream tracks on success/cancel/Esc/error (no leaked camera). Graceful fallback messages for permission-denied / no-camera / unsupported; manual typing stays fully usable. - zxing-wasm's .wasm is served SAME-ORIGIN (webpack asset/resource) so the CSP connect-src 'self' isn't violated — the default CDN wasm URL is superseded. CSP script-src gains 'wasm-unsafe-eval' (the minimal directive to compile wasm; not general unsafe-eval), kept in sync in public/.htaccess and public/index.php. - Scan buttons target #copy_code (loan) and #return_copy_code (return). i18n across it/en/de/fr. Built bundle + wasm committed (create-release ships committed assets). Verified in a headless browser: buttons bind, modal opens, self-hosted wasm loads (200 application/wasm), zero CSP/WebAssembly errors, clean teardown. Live camera decode requires a real device/camera (https or localhost) — cannot run headless.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughIntrodotta gestione copie tramite codice inventario: nuovo endpoint ChangesGestione codice copia/inventario e scanner barcode
Estimated code review effort: 4 (Complex) | ~60 minutes 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: 5
🤖 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/CopyController.php`:
- Around line 87-102: The “copy held” SQL predicate is duplicated in
CopyController, including the availability check in this block and the same
logic in updateCopy, which risks future drift. Extract the shared query into a
private helper like isCopyHeld(mysqli $db, int $copyId): bool, have the current
availability logic call that helper, and reuse it from updateCopy so the
predicate stays single-sourced.
In `@app/Controllers/LibriController.php`:
- Around line 2694-2825: `generateCopyLabelsPDF` duplicates the same label setup
logic already present in `generateLabelPDF`, inflating complexity and making
both methods harder to maintain. Extract the repeated settings lookup,
collocazione resolution, and author string building into shared private helpers
such as `resolveLabelSettings()`, `resolveCollocazione()`, and
`buildAuthorString()`, then reuse them from both `generateCopyLabelsPDF` and
`generateLabelPDF` so the label generation flow stays the same but the
duplicated blocks are removed.
In `@app/Controllers/PrestitiController.php`:
- Around line 254-313: The `copyCode` lookup in `PrestitiController::create` is
taking an unnecessary lock on `libri` via the JOIN, which can create lock-order
deadlocks. Remove the `JOIN libri` from the `SELECT ... FOR UPDATE` used to
fetch the copy, and read only from `copie` in that query. Keep the existing
`libro_id` comparison and availability checks in place, since those already
validate the selected book and its state.
In `@app/Views/prestiti/index.php`:
- Around line 774-788: The window.copyScannerI18n block is duplicated in this
view and in crea_prestito.php, so the same scanner translation strings can drift
if only one copy is updated. Move the shared i18n object into a single reusable
partial or shared config include, and have both views render that common source
instead of keeping separate inline copies. Use the existing
window.copyScannerI18n setup and the copy-scanner.bundle.js inclusion as the
place to centralize the shared strings.
In `@frontend/js/copy-scanner.js`:
- Around line 203-228: The camera stream is not released when video.play() fails
after a successful getUserMedia() call in copy-scanner.js. Update the promise
chain around navigator.mediaDevices.getUserMedia() and video.play() so that any
failure after stream assignment stops all tracks on stream before showing the
error message; keep the existing settled guard, and make sure the cleanup
happens in the video.play() failure path as well as in the cancellation path.
🪄 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: 577da0e5-5284-41d7-924f-4e49a2e30571
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonpublic/assets/df0ae38955633a7ef478.wasmis excluded by!**/*.wasm
📒 Files selected for processing (18)
app/Controllers/CopyController.phpapp/Controllers/LibriController.phpapp/Controllers/PrestitiController.phpapp/Routes/web.phpapp/Views/libri/scheda_libro.phpapp/Views/prestiti/crea_prestito.phpapp/Views/prestiti/index.phpfrontend/js/copy-scanner.jsfrontend/package.jsonfrontend/webpack.config.jslocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsonpublic/.htaccesspublic/assets/copy-scanner.bundle.jspublic/assets/main.csspublic/index.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements discussion #238 requests 2 and 3 for physical-copy tracking. Builds on the model that already exists (
copie.numero_inventarioUNIQUE,prestiti.copia_id) — no schema change.Phase 1 — labels + loan/return by copy code (
a39354d4)GET /admin/books/{id}/copy-labels-pdfprints one label per copy, each with itsnumero_inventarioas text + a Code128 barcode (the book-level label is unchanged). "Print copy labels" button on the copies card.GET /admin/copies/by-code?code=…→{found, copy_id, libro_id, titolo, stato, available}.store()resolves it (FOR UPDATE), validates book + availability, and pins that exactcopia_id. Empty = auto-assign as before (backward compatible).POST /admin/loans/return-by-codefinds the active loan for a code and returns it via the existingprocessReturnpath (no duplicated logic).Verified live:
by-coderesolves real + missing codes; the labels PDF renders (200,application/pdf) and contains the copy code.Phase 2 — in-browser camera scanner (
e508db41)zxing-wasmcamera modal, decodes the label barcode, and fills the input (never auto-submits; stops all camera tracks on close)..wasmis served same-origin (webpackasset/resource) so CSPconnect-src selfholds;script-srcgainswasm-unsafe-eval(minimal wasm directive), synced in.htaccess+index.php. Built bundle + wasm committed (release ships committed assets).Verified headless: buttons bind, modal opens, wasm loads (200
application/wasm), zero CSP/WebAssembly errors, clean teardown. Live camera decode needs a real device (https/localhost) — cannot run headless.Soft-delete scoped, admin routes English literals, strings in
__()across it/en/de/fr, prepared statements. PHPStan L5 clean.Remaining for #238: request #1 (subtitle in search) already shipped in #240.
Summary by CodeRabbit
Nuove Funzionalità
Miglioramenti