[books] Fix dead source icon links and gate ComicVine lookups
Some checks failed
ci / test (push) Has been cancelled
ci / build-and-deploy (push) Has been cancelled

This commit is contained in:
2026-09-01 13:00:20 -04:00
parent c400f8a18d
commit 102380a8ff
9 changed files with 181 additions and 15 deletions

View File

@ -616,6 +616,50 @@ author name (like Track does with =musicbrainz_id=).
the author to the Google lookup and persists =google_books_id= on backfill.
- Migration ~0040_book_google_books_id~ adds the field.
** DONE [#C] Fix dead source icon links on book and brick set pages :books:templates:
:PROPERTIES:
:ID: 35ce01a2-0dca-4bd3-83ea-ca826c734c36
:END:
*** Description
The book detail page rendered OpenLibrary and Amazon source icons via
=object.openlibrary_link= and =object.amazon_link=, but neither property
existed on =Book= (the model has no Amazon ASIN field at all), so both icons
were dead links around broken images. The brick set detail page was a copy of
the book template and had the same broken icons for a type with no
OpenLibrary/Amazon presence.
*** Implementation
- ~vrobbler/apps/books/models.py~: added =Book.openlibrary_link= (builds
=https://openlibrary.org/works/{olid}= or =/books/{olid}= for edition IDs)
and =Book.bookshop_link= (uses the ISBN, in =/a/{affiliate_id}/{isbn}=
affiliate form when =BOOKSHOP_AFFILIATE_ID= is set, otherwise a
=/books?keywords={isbn}= search link).
- ~vrobbler/settings.py~: added =BOOKSHOP_AFFILIATE_ID= read from
=VROBBLER_BOOKSHOP_AFFILIATE_ID=.
- ~vrobbler/templates/books/book_detail.html~: source icons now render
conditionally (only when a link exists, like the music pages), link to
OpenLibrary and Bookshop, and the dead Amazon icon is gone.
- ~vrobbler/templates/bricksets/brickset_detail.html~: removed the dead
OpenLibrary/Amazon source icon row entirely.
- Added official OpenLibrary and Bookshop logos at
~vrobbler/apps/scrobbles/static/images/~
(=openlibrary-logo.png=, =bookshop-logo.png=).
- Tests in ~tests/books_tests/test_book_source_links.py= cover both link
properties (work vs edition OLIDs, ISBN-10/13, with/without affiliate ID).
- ~vrobbler/apps/books/management/commands/cleanup_book_metadata.py~: added a
=--missing-source_id= filter flag for books lacking any of
=openlibrary_id= / =google_books_id= / =comicvine_id=.
- ~vrobbler/apps/books/models.py~: added =Book.is_comic= so ComicVine is only
queried for books that already carry comic source data
(=comicvine_id=, =volume_comicvine_id=, =readcomics_url=, issue/volume
fields) or whose title contains "Issue "/"Volume ". The blanket ComicVine
fallback in =find_or_create()=, =fix_metadata()=, and
~cleanup_book_metadata.py~ now respects this, so unrelated prose books no
longer get matched to comics.
** DONE [#B] Books created via koreader do not get enriched :books:metadata:bug:
:PROPERTIES:
:ID: ddfed612-1f4e-b0bb-5310-19adf9cf9336
@ -683,7 +727,6 @@ include their scrobbles in global aggregation. If that check box is not selected
- Tests in ~vrobbler/apps/charts/tests/test_views.py~ and new global chart
tests in ~vrobbler/apps/charts/tests/test_utils.py~.
** DONE [#B] Make it possible for anonymous users to view media objects :media:templates:
:PROPERTIES:
:ID: f12fd0b4-b728-4fbc-adf7-d51695d8c155

View File

@ -0,0 +1,84 @@
import pytest
from books.models import Book
from django.test import override_settings
@pytest.mark.django_db
def test_openlibrary_link_work_id():
book = Book.objects.create(title="Test Book", openlibrary_id="OL1234567W")
assert book.openlibrary_link == "https://openlibrary.org/works/OL1234567W"
@pytest.mark.django_db
def test_openlibrary_link_edition_id():
book = Book.objects.create(title="Test Book", openlibrary_id="OL1234567M")
assert book.openlibrary_link == "https://openlibrary.org/books/OL1234567M"
@pytest.mark.django_db
def test_openlibrary_link_empty():
book = Book.objects.create(title="Test Book")
assert book.openlibrary_link == ""
@pytest.mark.django_db
def test_bookshop_link_isbn_13_without_affiliate():
book = Book.objects.create(title="Test Book", isbn_13="9783161484100")
assert book.bookshop_link == "https://bookshop.org/books?keywords=9783161484100"
@pytest.mark.django_db
def test_bookshop_link_isbn_10_without_affiliate():
book = Book.objects.create(title="Test Book", isbn_10="0805061762")
assert book.bookshop_link == "https://bookshop.org/books?keywords=0805061762"
@pytest.mark.django_db
@override_settings(BOOKSHOP_AFFILIATE_ID="abc123")
def test_bookshop_link_with_affiliate():
book = Book.objects.create(title="Test Book", isbn_13="9783161484100")
assert book.bookshop_link == "https://bookshop.org/a/abc123/9783161484100"
@pytest.mark.django_db
def test_bookshop_link_empty():
book = Book.objects.create(title="Test Book")
assert book.bookshop_link == ""
@pytest.mark.django_db
def test_is_comic_false_for_prose_book():
book = Book.objects.create(title="A Gentle Madness")
assert book.is_comic is False
@pytest.mark.django_db
def test_is_comic_true_for_comicvine_id():
book = Book.objects.create(title="Spider-Man", comicvine_id="538480")
assert book.is_comic is True
@pytest.mark.django_db
def test_is_comic_true_for_readcomics_url():
book = Book.objects.create(
title="Saga", readcomics_url="https://readcomicsonline.ru/comic/saga/1"
)
assert book.is_comic is True
@pytest.mark.django_db
def test_is_comic_true_for_issue_number():
book = Book.objects.create(title="Watchmen", issue_number=1)
assert book.is_comic is True
@pytest.mark.django_db
def test_is_comic_true_for_issue_in_title():
book = Book.objects.create(title="Star Wars Issue 42")
assert book.is_comic is True
@pytest.mark.django_db
def test_is_comic_true_for_volume_in_title():
book = Book.objects.create(title="East of West Volume 2")
assert book.is_comic is True

View File

@ -16,6 +16,7 @@ MISSING_ALL = [
"language",
"publisher",
"publish_year",
"source_id",
]
@ -36,6 +37,9 @@ MISSING_GROUPS = {
"language": lambda b: not b.language,
"publisher": lambda b: not b.publisher,
"publish_year": lambda b: b.first_publish_year is None,
"source_id": lambda b: not (
b.openlibrary_id or b.google_books_id or b.comicvine_id
),
}
@ -184,6 +188,7 @@ class Command(BaseCommand):
book_dict = {}
cv_data = None
if book.is_comic:
if book.comicvine_id:
cv_data = lookup_issue_by_comicvine_id(str(book.comicvine_id))
if not cv_data:

View File

@ -240,12 +240,48 @@ class Book(LongPlayScrobblableMixin):
def get_absolute_url(self):
return reverse("books:book_detail", kwargs={"slug": self.uuid})
@property
def openlibrary_link(self) -> str:
if not self.openlibrary_id:
return ""
if self.openlibrary_id.endswith("M"):
return f"https://openlibrary.org/books/{self.openlibrary_id}"
return f"https://openlibrary.org/works/{self.openlibrary_id}"
@property
def bookshop_link(self) -> str:
isbn = self.isbn_13 or self.isbn_10
if not isbn:
return ""
affiliate_id = getattr(settings, "BOOKSHOP_AFFILIATE_ID", "")
if affiliate_id:
return f"https://bookshop.org/a/{affiliate_id}/{isbn}"
return f"https://bookshop.org/books?keywords={isbn}"
@property
def resume_start_url(self):
return (
reverse("scrobbles:start", kwargs={"media_uuid": self.uuid}) + "?resume=1"
)
@property
def is_comic(self) -> bool:
"""True if this book looks like a comic rather than prose.
We don't want to send every book title through ComicVine, since it
will happily return comic matches for unrelated titles. Only treat a
book as a comic when it already has comic source data, or its title
smells like an issue/volume.
"""
if self.comicvine_id or self.volume_comicvine_id:
return True
if self.readcomics_url or self.next_readcomics_url:
return True
if self.issue_number or self.volume_number or self.volume:
return True
title = self.original_title or self.title or ""
return "Issue " in title or "Volume " in title
@classmethod
def get_from_comicvine(
cls, title: str, overwrite: bool = False, force_new: bool = False
@ -333,9 +369,9 @@ class Book(LongPlayScrobblableMixin):
if ol_data and ol_data.get("cover_url"):
book_dict["cover_url"] = ol_data["cover_url"]
# Always try ComicVine as a fallback — it may recognize books that
# OL/Google don't flag as comics
if not tried_comicvine:
# Try ComicVine as a fallback, but only for titles that look like
# comics — otherwise it matches unrelated prose books to comics.
if not tried_comicvine and ("Issue " in title or "Volume " in title):
cv_data = lookup_comic_from_comicvine(title)
if cv_data:
for k, v in cv_data.items():
@ -443,7 +479,7 @@ class Book(LongPlayScrobblableMixin):
logger.warn(f"Checking LOCG for {self.title}")
data = lookup_comic_from_locg(str(self.title))
if not data and COMICVINE_API_KEY:
if not data and COMICVINE_API_KEY and self.is_comic:
if self.comicvine_id:
logger.warn(f"Checking ComicVine by ID for {self.title}")
data = lookup_issue_by_comicvine_id(str(self.comicvine_id))

Binary file not shown.

After

Width:  |  Height:  |  Size: 25 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 16 KiB

View File

@ -143,6 +143,8 @@ AMAZON_PAAPI_SECRET_KEY = os.getenv("VROBBLER_AMAZON_PAAPI_SECRET_KEY", "")
AMAZON_PAAPI_ASSOCIATE_TAG = os.getenv("VROBBLER_AMAZON_PAAPI_ASSOCIATE_TAG", "")
AMAZON_PAAPI_COUNTRY = os.getenv("VROBBLER_AMAZON_PAAPI_COUNTRY", "US")
BOOKSHOP_AFFILIATE_ID = os.getenv("VROBBLER_BOOKSHOP_AFFILIATE_ID", "")
FASTCORK_API_KEY = os.getenv("VROBBLER_FASTCORK_API_KEY", "")
DEFAULT_TASK_CONTEXT_TAGS = [

View File

@ -20,8 +20,8 @@
<hr />
{% endif %}
<p style="float:right;">
<a href="{{object.openlibrary_link}}"><img src="{% static " images/openlibrary-logo.png" %}" width=35></a>
<a href="{{object.amazon_link}}"><img src="{% static " images/amazon-logo.png" %}" width=35></a>
{% if object.openlibrary_link %}<a href="{{object.openlibrary_link}}"><img src="{% static "images/openlibrary-logo.png" %}" height=35></a>{% endif %}
{% if object.bookshop_link %}<a href="{{object.bookshop_link}}"><img src="{% static "images/bookshop-logo.png" %}" height=35></a>{% endif %}
</p>
</div>
</div>

View File

@ -19,10 +19,6 @@
<p>{{object.summary|safe|linebreaks|truncatewords:160}}</p>
<hr />
{% endif %}
<p style="float:right;">
<a href="{{object.openlibrary_link}}"><img src="{% static " images/openlibrary-logo.png" %}" width=35></a>
<a href="{{object.amazon_link}}"><img src="{% static " images/amazon-logo.png" %}" width=35></a>
</p>
</div>
</div>
<div class="row">