openlibrary icon indicating copy to clipboard operation
openlibrary copied to clipboard

Fix cover on author page

Open jjessieyang opened this issue 10 months ago • 2 comments

Closes #9064

Technical

I found covers that were not appearing on the author page from the local-production dataset and found that their covers were from archive.org rather than covers.openlibrary.org/something. It seems that before, if there was no cover in openlibrary, the function would simply go to return None in return_cover_url in openlibrary/book_providers.py and ignore the rest of the logic.

Testing

I found a few examples of this happening when you look up "web design" books in my local version.

Screenshot

image image

after changes:

image

Stakeholders

@scottbarnes

jjessieyang avatar Apr 21 '24 00:04 jjessieyang

@jjessieyang seems your code editor changed all the quotes in that file. Can you please change then back? It should be a config in your editor/formatter. Also @scottbarnes is lead so I think he'll review this change

RayBB avatar Apr 21 '24 08:04 RayBB

@jjessieyang need any help with the feedback here?

RayBB avatar May 16 '24 18:05 RayBB

@jjessieyang, if you want to submit this PR in part, please feel free to re-open it with this diff:

diff --git a/openlibrary/book_providers.py b/openlibrary/book_providers.py
index 758e498a6..6227e2c5f 100644
--- a/openlibrary/book_providers.py
+++ b/openlibrary/book_providers.py
@@ -241,22 +241,21 @@ def get_cover_url(ed_or_solr: Edition | dict) -> str | None:
         return cover.url(size) if cover else None
 
     # Solr edition
-    elif ed_or_solr['key'].startswith('/books/'):
-        if ed_or_solr.get('cover_i'):
-            return (
-                get_coverstore_public_url()
-                + f'/b/id/{ed_or_solr["cover_i"]}-{size}.jpg'
-            )
-        else:
-            # Solr document augmented with availability
-            availability = ed_or_solr.get('availability', {}) or {}
-
-            if availability.get('openlibrary_edition'):
-                olid = availability.get('openlibrary_edition')
-                return f"{get_coverstore_public_url()}/b/olid/{olid}-{size}.jpg"
-            if availability.get('identifier'):
-                ocaid = ed_or_solr['availability']['identifier']
-                return f"https://archive.org/download/{ocaid}/page/cover_w180_h360.jpg"
+    elif ed_or_solr['key'].startswith('/books/') and ed_or_solr.get('cover_i'):
+        return (
+            get_coverstore_public_url()
+            + f'/b/id/{ed_or_solr["cover_i"]}-{size}.jpg'
+        )
+
+    # Solr document augmented with availability
+    availability = ed_or_solr.get('availability', {}) or {}
+
+    if availability.get('openlibrary_edition'):
+        olid = availability.get('openlibrary_edition')
+        return f"{get_coverstore_public_url()}/b/olid/{olid}-{size}.jpg"
+    if availability.get('identifier'):
+        ocaid = ed_or_solr['availability']['identifier']
+        return f"https://archive.org/download/{ocaid}/page/cover_w180_h360.jpg"

That will take care of half of the issue, at least.

scottbarnes avatar Aug 20 '24 17:08 scottbarnes