Fix cover on author page

[jjessieyang]

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

after changes:

Stakeholders

@scottbarnes

[RayBB]

@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

[scottbarnes]

Thanks for this, and sorry for the delay in the review, @jjessieyang.

I can't seem to leave the comment the way I want, but what about something like this, starting on line 239:

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"
 
     # Plain solr - we don't know which edition is which here, so this is most
     # preferable

Both seem to work equally well, but I'm a bit wary of changing the original logic flow more than necessary. Originally, the "Solr document augmented with availability" code was running on everything that fell through the "Editions" if/elif/else beginning on line 239, and I am hesitant to move it under the "Solr edition" conditional on line 243, unless there is a compelling reason to do so -- and there may be, as I haven't looked super closely at this yet. :)

Also, have you had a chance yet to look at the other half of the issue, relating to the merge tool, as shown in this image from the issue:

That will likely involve editing utils/SelectionManager/SelectionManager.js.

[RayBB]

@jjessieyang need any help with the feedback here?