Conversation
… bulk import, and camera scanning.
📝 WalkthroughWalkthroughAdds a tabbed multi-mode ISBN import system: camera-based scanning, bulk text import, and manual lookup, with new UI components, a scanning composable, ISBN extraction utilities, and a concurrent backend bulk-add endpoint. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Add Book Page
participant Scanner as IsbnScanner
participant Composable as useIsbnScanner
participant API as Backend API
participant DB as Database
rect rgba(100, 150, 200, 0.5)
note over User,UI: Single ISBN Lookup Flow
User->>UI: Enter ISBN / tap lookup
UI->>Composable: addIsbn(isbn)
Composable->>API: POST /api/books/lookup
API->>DB: Query book data
DB-->>API: Book details
API-->>Composable: BookLookupResult
Composable-->>UI: Update state (found / not_found)
UI->>User: Show BookPreview
end
rect rgba(150, 200, 100, 0.5)
note over User,DB: Bulk Scan / Import Flow
User->>UI: Choose bulk or scan mode
alt Camera Scan
User->>Scanner: Point camera at barcode
Scanner->>Composable: detected(isbn)
Composable->>Composable: addIsbn(isbn)
else Text Import
User->>UI: Paste list of ISBNs
UI->>Composable: addMultipleIsbns(text)
end
Composable->>API: POST /api/books/lookup (per ISBN)
API->>DB: Query each ISBN
DB-->>API: Results
API-->>Composable: Lookup results
Composable-->>UI: Render BulkScanReview with statuses
User->>UI: Select & click "Add Selected"
UI->>Composable: addSelectedToLibrary()
Composable->>API: POST /api/books/bulk-add
API->>DB: Concurrently add books
DB-->>API: Success/fail per ISBN
API-->>Composable: {added, failed}
Composable-->>UI: Update UI & navigate on success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/components/add/CameraScanTab.vue`:
- Around line 114-119: The reset() function currently clears lookupResult and
showScanner but leaves continuous-mode state (scannedBooks) intact; update
reset() to also call the composable's clearAll() (or equivalent) to purge
scannedBooks so switching tabs doesn't retain stale continuous-scan data —
locate reset() and add a call to clearAll() from the scanner/composable (the
same module that exposes scannedBooks) before defineExpose({ reset }).
In `@app/components/IsbnScanner.vue`:
- Around line 80-93: The setTimeout calls inside the continuous scan branch
(where scanCooldown.value is set and the auto-hide uses lastScanned.value) can
fire after component unmount causing a memory leak; capture the returned timer
IDs (e.g., push them into a local array or refs like scanTimeouts) when calling
setTimeout and implement an onUnmounted hook that iterates those IDs and calls
clearTimeout on each, also clear the array and avoid touching refs after
unmount; ensure any places that reset timeouts (if added) also clear prior
timers by ID.
In `@app/composables/useIsbnScanner.ts`:
- Around line 100-106: The isLookingUp flag can remain true if any addIsbn
rejects because the await Promise.all(inputs.map(isbn => addIsbn(isbn))) is not
guarded; wrap that await in a try/finally so that isLookingUp.value = false runs
in the finally block (inside the same function that references isLookingUp,
inputs, and addIsbn), preserving error propagation (don’t swallow the error)
while guaranteeing cleanup.
In `@shared/utils/schemas.ts`:
- Around line 46-56: The current bookIsbnSchema applies .min()/.max() before the
.transform(), causing inputs like "9781234567890 59099" to fail length checks
before extractIsbn runs; change the schema to perform normalization first and
apply validation after by replacing the inline .transform(...) with a
transformation pipeline (use .transform or .pipe to first map the isbn via
extractIsbn/normalization and then apply z.string().min(...).max(...)) so that
extractIsbn is invoked before length checks; update references in bookIsbnSchema
and keep extractIsbn as the normalizer function used in the pipeline.
🧹 Nitpick comments (9)
shared/utils/schemas.ts (1)
3-44: Consider checksum validation (or adjust “valid” wording).
This helper accepts any 10/13‑digit pattern; check digits aren’t verified, so malformed ISBNs can slip through and trigger unnecessary lookups. Consider adding checksum checks (or softening the doc comment) to align behavior with “valid.”♻️ Suggested checksum validation
+function isValidIsbn10(isbn: string): boolean { + if (!/^\d{9}[\dX]$/.test(isbn)) return false + let sum = 0 + for (let i = 0; i < 9; i++) sum += (10 - i) * Number(isbn[i]) + const check = isbn[9] === 'X' ? 10 : Number(isbn[9]) + sum += check + return sum % 11 === 0 +} + +function isValidIsbn13(isbn: string): boolean { + if (!/^97[89]\d{10}$/.test(isbn)) return false + let sum = 0 + for (let i = 0; i < 12; i++) { + sum += Number(isbn[i]) * (i % 2 === 0 ? 1 : 3) + } + const check = (10 - (sum % 10)) % 10 + return check === Number(isbn[12]) +} + export function extractIsbn(input: string): string | null { // Normalize: uppercase and remove hyphens/spaces const normalized = input.toUpperCase().replace(/[-\s]/g, '') // Check for valid ISBN-10 (9 digits + digit or X) - if (normalized.length === 10) { - if (/^\d{9}[\dX]$/.test(normalized)) { - return normalized - } - } + if (normalized.length === 10 && isValidIsbn10(normalized)) { + return normalized + } // Check for valid ISBN-13 (13 digits starting with 978 or 979) - if (normalized.length === 13) { - if (/^97[89]\d{10}$/.test(normalized)) { - return normalized - } - } + if (normalized.length === 13 && isValidIsbn13(normalized)) { + return normalized + } // Try to extract ISBN-13 from longer barcode (price code suffix) const digits = normalized.replace(/\D/g, '') if (digits.length >= 13) { const isbn13Match = digits.match(/^(97[89]\d{10})/) - if (isbn13Match && isbn13Match[1]) { + if (isbn13Match?.[1] && isValidIsbn13(isbn13Match[1])) { return isbn13Match[1] } }app/components/IsbnScanner.vue (2)
19-20: Unused reactive statedetectedBarcodes.The
detectedBarcodesref is populated inonDetectbut never consumed in the template or elsewhere. Consider removing it or implementing the visual bounding box feedback it was intended for.
176-194: Inconsistent prop access:continuousvsprops.continuous.Line 179 uses
continuousdirectly in the template, while line 81 usesprops.continuousin the script. Both work, but consistency improves readability. Prefer usingprops.continuouseverywhere or destructure withtoRefsif you want shorthand access.app/composables/useIsbnScanner.ts (1)
67-76: Repeated error message extraction pattern.The same error extraction logic appears in
addIsbn(lines 72-74) andaddSelectedToLibrary(lines 217-219). Consider extracting a helper function for consistency and DRY.Example helper
function extractErrorMessage(err: unknown, fallback = 'An error occurred'): string { if (err instanceof Error) return err.message const data = (err as { data?: { message?: string } })?.data return data?.message || fallback }Also applies to: 215-226
app/components/add/BulkImportTab.vue (2)
20-34: Input cleared regardless of lookup outcome.Line 33 clears
bulkIsbnTextafteraddMultipleIsbnscompletes, even if lookups failed. Users may want to retry with the same input. Consider preserving input on partial failures or providing a way to recover.
5-6: Redundant loading state management.
isBulkLookingUpis managed locally here, butuseIsbnScanneralso exposesisLookingUp. Consider using the composable's state directly to avoid potential inconsistencies.Proposed simplification
-const isBulkLookingUp = ref(false) - const { scannedBooks, isAddingBooks, counts, addMultipleIsbns, + isLookingUp, // ... } = useIsbnScanner() async function handleBulkImport() { // ... - isBulkLookingUp.value = true await addMultipleIsbns(bulkIsbnText.value) - isBulkLookingUp.value = false bulkIsbnText.value = '' }Then use
isLookingUpin the template instead ofisBulkLookingUp.Also applies to: 30-32
app/components/add/CameraScanTab.vue (1)
36-66: Duplicated lookup and add logic between single mode and composable.
lookupSingleIsbnandaddBookToLibraryreplicate patterns already present inuseIsbnScanner. Consider refactoring single mode to also use the composable (withscannedBooks.length === 1detection) to reduce duplication and ensure consistent behavior.Also applies to: 69-99
app/components/add/IsbnLookupTab.vue (1)
85-86: Naming inconsistency:resetLookupvsresetin other tabs.
BulkImportTabandCameraScanTabexposereset(), but this component exposesresetLookup(). For a consistent API when the parent calls reset on tab change, consider renaming toreset.Proposed fix
-function resetLookup() { +function reset() { lookupResult.value = null formState.isbn = '' } // Expose for parent to reset on tab change -defineExpose({ resetLookup }) +defineExpose({ reset })Update template references from
resetLookuptoresetaccordingly.app/pages/library/add.vue (1)
76-80: Scope the “manual entry” teaser to the relevant tab (optional).Right now the message shows in scan/bulk modes too. If it’s meant only for the manual flow, consider gating it.
💡 Optional tweak
- <div class="mt-6 text-center"> + <div v-if="inputMode === 'manual'" class="mt-6 text-center"> <p class="text-sm text-muted"> Can't find your book? Manual entry coming soon. </p> </div>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.