diff --git a/backend/app/services/book_import.py b/backend/app/services/book_import.py index 13b3be53..fbbe5082 100644 --- a/backend/app/services/book_import.py +++ b/backend/app/services/book_import.py @@ -205,7 +205,7 @@ async def _run_hc() -> None: for e in hc_events: yield e - results = _merge_and_deduplicate(ol_results, hc_results) + results = _merge_results(ol_results, hc_results) if not results: if not api_key: @@ -288,7 +288,7 @@ async def search( logger.info("Open Library returned %d result(s) for %r", len(ol_results), query) logger.info("Hardcover returned %d result(s) for %r", len(hc_results), query) - results = _merge_and_deduplicate(ol_results, hc_results) + results = _merge_results(ol_results, hc_results) if not results: if not api_key: @@ -829,33 +829,18 @@ def map_hardcover(edition: dict) -> BookImportCandidate | None: # ── Merge / Deduplicate ─────────────────────────────────────────────────────── -def _merge_and_deduplicate( +def _merge_results( primary: list[BookImportCandidate], secondary: list[BookImportCandidate], ) -> list[BookImportCandidate]: - """Merge two candidate lists, deduplicating by (isbn, page_count, language). + """Merge two candidate lists, preserving every candidate in input order. - Primary list items come first in the result. - Same ISBN with different page_count/language is kept as separate candidates. - When two candidates collide, the one with a cover image is preferred. + The frontend is responsible for grouping variants that represent the same + book (e.g. by ISBN) and letting the user pick the best record. A user can + only own one book per exact ISBN string, so keeping all provider-specific + records lets the user compare data quality before importing. """ - seen: dict[str, BookImportCandidate] = {} - - def _key(c: BookImportCandidate) -> str: - isbn = (c.isbn or "").replace("-", "").replace(" ", "") - pages = str(c.page_count or "") - lang = (c.language or "").upper() - return f"isbn:{isbn}|pages:{pages}|lang:{lang}" - - for c in primary + secondary: - k = _key(c) - existing = seen.get(k) - if existing is None: - seen[k] = c - elif existing.cover_url is None and c.cover_url is not None: - seen[k] = c - - return list(seen.values()) + return primary + secondary # ── Helpers ─────────────────────────────────────────────────────────────────── diff --git a/backend/tests/test_book_import.py b/backend/tests/test_book_import.py index 023fde88..2cfb7850 100644 --- a/backend/tests/test_book_import.py +++ b/backend/tests/test_book_import.py @@ -1006,22 +1006,23 @@ def test_hardcover_dedup_key_full() -> None: assert key == ("9781234567897", 300, "en") -# ── _merge_and_deduplicate ───────────────────────────────────────────────────── +# ── _merge_results ───────────────────────────────────────────────────────────── -def test_merge_and_deduplicate_cover_preference() -> None: +def test_merge_results_preserves_all_candidates() -> None: a = BookImportCandidate(title="A", isbn="123", cover_url=None, source="ol") b = BookImportCandidate(title="B", isbn="123", cover_url="https://x.jpg", source="gb") - result = bi._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].cover_url == "https://x.jpg" - - -def test_merge_and_deduplicate_no_cover_override() -> None: - a = BookImportCandidate(title="A", isbn="123", cover_url="https://a.jpg", source="ol") - b = BookImportCandidate(title="B", isbn="123", cover_url="https://b.jpg", source="gb") - result = bi._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].cover_url == "https://a.jpg" + result = bi._merge_results([a], [b]) + assert len(result) == 2 + assert result[0] is a + assert result[1] is b + + +def test_merge_results_preserves_order() -> None: + a = BookImportCandidate(title="A", isbn="123", source="ol") + b = BookImportCandidate(title="B", isbn="123", source="gb") + c = BookImportCandidate(title="C", isbn="456", source="hc") + result = bi._merge_results([a, b], [c]) + assert [r.title for r in result] == ["A", "B", "C"] # ── _pick_isbn ───────────────────────────────────────────────────────────────── diff --git a/backend/tests/test_import.py b/backend/tests/test_import.py index f5d97559..60646a33 100644 --- a/backend/tests/test_import.py +++ b/backend/tests/test_import.py @@ -289,77 +289,44 @@ def test_map_hardcover_language_uppercased() -> None: assert result.language == "DE" -# ── _merge_and_deduplicate unit tests ──────────────────────────────────────── - -def _make_candidate(title: str, isbn: str | None = None, pages: int | None = None, lang: str | None = None) -> BookImportCandidate: - """Create a BookImportCandidate with default values for reuse in dedup tests.""" +# ── _merge_results unit tests ───────────────────────────────────────────────── + +def _make_candidate( + title: str, + isbn: str | None = None, + pages: int | None = None, + lang: str | None = None, + source: str = "open_library", +) -> BookImportCandidate: + """Create a BookImportCandidate with default values for reuse in merge tests.""" return BookImportCandidate( title=title, author="Author", isbn=isbn, page_count=pages, language=lang, - source="open_library", + source=source, ) -def test_merge_and_dedup_same_isbn_pages_lang() -> None: - a = _make_candidate("Dune", "9780441013593", 412, "EN") - b = _make_candidate("Dune", "9780441013593", 412, "EN") - result = book_import._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].title == "Dune" - - -def test_merge_and_dedup_same_isbn_diff_pages() -> None: +def test_merge_results_preserves_all_candidates() -> None: a = _make_candidate("Dune", "9780441013593", 412, "EN") - b = _make_candidate("Dune HC", "9780441013593", 688, "EN") - result = book_import._merge_and_deduplicate([a], [b]) + b = _make_candidate("Dune", "9780441013593", 412, "EN", source="hardcover") + result = book_import._merge_results([a], [b]) assert len(result) == 2 + assert result[0] is a + assert result[1] is b -def test_merge_and_dedup_same_isbn_diff_lang() -> None: - a = _make_candidate("Dune", "9780441013593", 412, "EN") - b = _make_candidate("Dune DE", "9780441013593", 412, "DE") - result = book_import._merge_and_deduplicate([a], [b]) - assert len(result) == 2 - - -def test_merge_and_dedup_ol_first_order() -> None: +def test_merge_results_preserves_order() -> None: ol = _make_candidate("OL Book", "9781111111111", 200, "EN") - hc = _make_candidate("HC Book", "9782222222222", 300, "DE") - result = book_import._merge_and_deduplicate([ol], [hc]) - assert len(result) == 2 + hc = _make_candidate("HC Book", "9782222222222", 300, "DE", source="hardcover") + gb = _make_candidate("GB Book", "9783333333333", 250, "FR", source="google_books") + result = book_import._merge_results([ol], [hc, gb]) + assert len(result) == 3 assert result[0].title == "OL Book" assert result[1].title == "HC Book" - - -def test_merge_and_dedup_prefers_candidate_with_cover() -> None: - a = _make_candidate("No Cover", "9780441013593", 412, "EN") - b = _make_candidate("Has Cover", "9780441013593", 412, "EN") - b.cover_url = "https://example.com/cover.jpg" - result = book_import._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].title == "Has Cover" - - -def test_merge_and_dedup_prefers_cover_when_primary_missing_cover() -> None: - a = _make_candidate("OL No Cover", "9780441013593", 412, "EN") - b = _make_candidate("HC Has Cover", "9780441013593", 412, "EN") - b.cover_url = "https://example.com/cover.jpg" - result = book_import._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].title == "HC Has Cover" - - -def test_merge_and_dedup_keeps_primary_cover_when_both_have_cover() -> None: - a = _make_candidate("OL Cover", "9780441013593", 412, "EN") - a.cover_url = "https://ol-cover.jpg" - b = _make_candidate("HC Cover", "9780441013593", 412, "EN") - b.cover_url = "https://hc-cover.jpg" - result = book_import._merge_and_deduplicate([a], [b]) - assert len(result) == 1 - assert result[0].title == "OL Cover" + assert result[2].title == "GB Book" # ── _hardcover_dedup_key tests ─────────────────────────────────────────────── diff --git a/docs/guide/using-librislog/library.md b/docs/guide/using-librislog/library.md index 48a2084d..439e30de 100644 --- a/docs/guide/using-librislog/library.md +++ b/docs/guide/using-librislog/library.md @@ -63,7 +63,26 @@ Search external sources for book metadata: - **Google Books** — Requires API key (set in `.env`) - **Hardcover.app** — Requires API token (set in `.env`) -The search automatically tries Open Library first, then falls back to other sources. For ISBN searches, all available sources are queried in parallel. +Open Library and Hardcover (if an API token is configured) are queried **in parallel** for both title and ISBN searches. Google Books is only used as a **fallback** when the other sources return no results — or on demand via the **Search Google Books too** button, which adds Google Books results to the current results. + +While a search is running, the **Search** button changes to **Cancel**, so you can stop the request at any time and refine your query. + +#### How results are grouped + +Different providers often describe the same book slightly differently (title language, page count, publisher, cover). Instead of dropping these variants, LibrisLog keeps every result and groups the ones that represent the same book. Each group shows a **"N results"** badge with a **Show editions** toggle — expand it to review the individual records and pick the one you want to import. + +Results are grouped by this rule: + +- **Same ISBN** — if two results carry the same ISBN, they are grouped together. ISBN-10 and ISBN-13 forms of the same ISBN count as equal (e.g. `0441013597` and `9780441013593`). +- **No ISBN — same title + same authors** — results without an ISBN are grouped by a normalized title (case- and whitespace-insensitive) together with the same sorted author names. + +Consequences you may notice: + +- Two results with the *same title* but **different ISBNs** are **not** grouped — they are different editions (different language, publisher, or page count) and appear as separate entries. +- A result with an ISBN and a result without one are never grouped, even if the title and authors match. +- Results that differ only in metadata (cover, publisher, page count, description) but share an ISBN or title+author are grouped so you can compare them side by side. + +Because an ISBN can only be owned once per user, a group with a shared ISBN always represents a single book — importing one variant is enough. ### ISBN Barcode Scan diff --git a/frontend/src/lib/api.test.ts b/frontend/src/lib/api.test.ts index ff1fb970..7c2eafd6 100644 --- a/frontend/src/lib/api.test.ts +++ b/frontend/src/lib/api.test.ts @@ -285,3 +285,44 @@ describe('api.statistics.gamification', () => { expect(body).toMatchObject({ goal_pages_per_day_enabled: true, goal_pages_per_day: 25 }); }); }); + +describe('api.import.searchStream', () => { + beforeEach(() => { + apiKey.set(null); + csrfToken.set(null); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('passes abort signal to fetch', async () => { + const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + body: new ReadableStream({ start(controller) { controller.close(); } }), + } as unknown as Response); + + const controller = new AbortController(); + const gen = api.import.searchStream('dune', 'title', 'auto', controller.signal); + await gen.next(); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(init.signal).toBe(controller.signal); + }); + + it('builds the stream URL with query, type and mode', async () => { + const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + body: new ReadableStream({ start(controller) { controller.close(); } }), + } as unknown as Response); + + const gen = api.import.searchStream('dune', 'isbn', 'google_only'); + await gen.next(); + + const [url] = fetchMock.mock.calls[0] as [string]; + expect(url).toContain('/import/search/stream'); + expect(url).toContain('q=dune'); + expect(url).toContain('type=isbn'); + expect(url).toContain('mode=google_only'); + }); +}); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 3dcdd415..573f457d 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -483,11 +483,12 @@ export const api = { async *searchStream( q: string, type: 'title' | 'isbn' = 'title', - mode: ImportSearchMode = 'auto' + mode: ImportSearchMode = 'auto', + signal?: AbortSignal ): AsyncGenerator { const res = await fetch( `${BASE}/import/search/stream?q=${encodeURIComponent(q)}&type=${type}&mode=${mode}`, - { headers: authHeaders() } + { headers: authHeaders(), signal } ); if (!res.ok || !res.body) { const detail = await res.json().catch(() => ({})); diff --git a/frontend/src/lib/components/AddBookModal.svelte b/frontend/src/lib/components/AddBookModal.svelte index 9bb46695..581abd39 100644 --- a/frontend/src/lib/components/AddBookModal.svelte +++ b/frontend/src/lib/components/AddBookModal.svelte @@ -43,6 +43,18 @@ let cover_url = $state(null); $effect(() => { status = defaultStatus; }); + // Close on Escape right away — the backdrop only receives key events + // after it has been clicked, so listen at the window level instead. + // Skip while the nested barcode scanner is open. + $effect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !scannerOpen) open = false; + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }); + function reset() { title = ''; subtitle = ''; @@ -268,7 +280,6 @@ isbn = detected; }} /> - -
e.key === 'Escape' && (open = false)}>
+
{/if} diff --git a/frontend/src/lib/components/AutoSearchCoverModal.svelte b/frontend/src/lib/components/AutoSearchCoverModal.svelte index e7722e72..24433aea 100644 --- a/frontend/src/lib/components/AutoSearchCoverModal.svelte +++ b/frontend/src/lib/components/AutoSearchCoverModal.svelte @@ -23,6 +23,17 @@ function close() { onCancel?.(); } + + // Close on Escape right away — the modal-backdrop only receives key events + // after it has been clicked, so listen at the window level instead. + $effect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') close(); + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }); {#if open} @@ -48,6 +59,6 @@ - +
{/if} diff --git a/frontend/src/lib/components/AutoSearchCoverModal.test.ts b/frontend/src/lib/components/AutoSearchCoverModal.test.ts index 7bc84f49..baafb80a 100644 --- a/frontend/src/lib/components/AutoSearchCoverModal.test.ts +++ b/frontend/src/lib/components/AutoSearchCoverModal.test.ts @@ -100,13 +100,21 @@ describe('AutoSearchCoverModal', () => { expect(onCancel).toHaveBeenCalledOnce(); }); - it('calls onCancel when backdrop clicked', async () => { + it('does not call onCancel when backdrop clicked', async () => { render(AutoSearchCoverModal, { props: { open: true, loading: false, candidates: [], error: null, onCancel, onSelect } }); const backdrop = document.querySelector('.modal-backdrop'); expect(backdrop).toBeTruthy(); await fireEvent.click(backdrop as Element); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it('calls onCancel when Escape is pressed', async () => { + render(AutoSearchCoverModal, { + props: { open: true, loading: false, candidates: [], error: null, onCancel, onSelect } + }); + await fireEvent.keyDown(window, { key: 'Escape' }); expect(onCancel).toHaveBeenCalledOnce(); }); diff --git a/frontend/src/lib/components/BarcodeScanner.svelte b/frontend/src/lib/components/BarcodeScanner.svelte index adc55d5f..6ec53386 100644 --- a/frontend/src/lib/components/BarcodeScanner.svelte +++ b/frontend/src/lib/components/BarcodeScanner.svelte @@ -339,6 +339,17 @@ } }); + // Close on Escape right away — the backdrop only receives key events + // after it has been clicked, so listen at the window level instead. + $effect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') void closeScanner(); + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }); + onDestroy(() => { void stopScanner(); }); @@ -346,14 +357,7 @@ {#if open}
-
e.key === 'Escape' && closeScanner()} - role="button" - tabindex="0" - aria-label={$_('scanner.close')} - >
+
{/if}
-
+
{/if} diff --git a/frontend/src/lib/components/ImportSearch.svelte b/frontend/src/lib/components/ImportSearch.svelte index 07917ad2..4f79ece0 100644 --- a/frontend/src/lib/components/ImportSearch.svelte +++ b/frontend/src/lib/components/ImportSearch.svelte @@ -1,12 +1,17 @@
@@ -493,13 +504,7 @@
{#if coverViewer} -
e.key === 'Escape' && closeCoverViewer()} - >
+