diff --git a/PROJECT.org b/PROJECT.org
index 41b746e..9a605aa 100644
--- a/PROJECT.org
+++ b/PROJECT.org
@@ -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
diff --git a/tests/books_tests/test_book_source_links.py b/tests/books_tests/test_book_source_links.py
new file mode 100644
index 0000000..f93c415
--- /dev/null
+++ b/tests/books_tests/test_book_source_links.py
@@ -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
diff --git a/vrobbler/apps/books/management/commands/cleanup_book_metadata.py b/vrobbler/apps/books/management/commands/cleanup_book_metadata.py
index 91d4222..e69a959 100644
--- a/vrobbler/apps/books/management/commands/cleanup_book_metadata.py
+++ b/vrobbler/apps/books/management/commands/cleanup_book_metadata.py
@@ -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,10 +188,11 @@ class Command(BaseCommand):
book_dict = {}
cv_data = None
- if book.comicvine_id:
- cv_data = lookup_issue_by_comicvine_id(str(book.comicvine_id))
- if not cv_data:
- cv_data = lookup_comic_from_comicvine(title)
+ if book.is_comic:
+ if book.comicvine_id:
+ cv_data = lookup_issue_by_comicvine_id(str(book.comicvine_id))
+ if not cv_data:
+ cv_data = lookup_comic_from_comicvine(title)
if cv_data:
book_dict.update(cv_data)
diff --git a/vrobbler/apps/books/models.py b/vrobbler/apps/books/models.py
index 08dfb15..066cca8 100644
--- a/vrobbler/apps/books/models.py
+++ b/vrobbler/apps/books/models.py
@@ -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))
diff --git a/vrobbler/apps/scrobbles/static/images/bookshop-logo.png b/vrobbler/apps/scrobbles/static/images/bookshop-logo.png
new file mode 100644
index 0000000..d2e17c0
Binary files /dev/null and b/vrobbler/apps/scrobbles/static/images/bookshop-logo.png differ
diff --git a/vrobbler/apps/scrobbles/static/images/openlibrary-logo.png b/vrobbler/apps/scrobbles/static/images/openlibrary-logo.png
new file mode 100644
index 0000000..8ea28b5
Binary files /dev/null and b/vrobbler/apps/scrobbles/static/images/openlibrary-logo.png differ
diff --git a/vrobbler/settings.py b/vrobbler/settings.py
index d7cc2a5..f90404f 100644
--- a/vrobbler/settings.py
+++ b/vrobbler/settings.py
@@ -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 = [
diff --git a/vrobbler/templates/books/book_detail.html b/vrobbler/templates/books/book_detail.html
index 39b402b..db2dc22 100644
--- a/vrobbler/templates/books/book_detail.html
+++ b/vrobbler/templates/books/book_detail.html
@@ -20,8 +20,8 @@
{% endif %}
- 
-
+ {% if object.openlibrary_link %}{% endif %}
+ {% if object.bookshop_link %}{% endif %}
diff --git a/vrobbler/templates/bricksets/brickset_detail.html b/vrobbler/templates/bricksets/brickset_detail.html
index f463439..ca4eaba 100644
--- a/vrobbler/templates/bricksets/brickset_detail.html
+++ b/vrobbler/templates/bricksets/brickset_detail.html
@@ -19,10 +19,6 @@
{{object.summary|safe|linebreaks|truncatewords:160}}
{% endif %}
-
-
-
-