fix goodreads import provider error warnings #1362

Author: FuzzyGrim

src/integrations/imports/goodreads.py

@@ -65,8 +65,14 @@ def import_data(self):
         for row in reader:
             try:
                 self._process_row(row)
-            except services.ProviderAPIError:
-                error_msg = f"Error processing entry with ID {row['media_id']} "
+            except services.ProviderAPIError as error:
+                row_description = self._row_description(row)
+                logger.warning(
+                    "Error processing Goodreads entry: %s - %s",
+                    row_description,
+                    error,
+                )
+                error_msg = f"Error processing entry: {row_description} - {error}"
                 self.warnings.append(error_msg)
                 continue
             except Exception as error:
@@ -88,6 +94,19 @@ def import_data(self):
         deduplicated_messages = "\n".join(dict.fromkeys(self.warnings))
         return imported_counts, deduplicated_messages
 
+    def _row_description(self, row):
+        """Return a useful label for warnings without assuming Yamtrack fields."""
+        title = row.get("Title")
+        book_id = row.get("Book Id")
+
+        if title and book_id:
+            return f"{title} (Goodreads ID {book_id})"
+        if title:
+            return title
+        if book_id:
+            return f"Goodreads ID {book_id}"
+        return str(row)
+
     def _process_row(self, row):
         """Process a single row from the CSV file."""
         default_source = Sources.HARDCOVER

src/integrations/tests/imports/test_goodreads.py

@@ -1,12 +1,16 @@
 from pathlib import Path
+from unittest.mock import MagicMock, patch
 
+import requests
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 
 from app.models import (
     Book,
+    Sources,
     Status,
 )
+from app.providers import services
 from integrations.imports import (
     goodreads,
 )
@@ -45,3 +49,32 @@ def test_stored_progress(self):
         read_book = Book.objects.get(status=Status.IN_PROGRESS.value)
         self.assertEqual(read_book.status, Status.IN_PROGRESS.value)
         self.assertEqual(read_book.progress, 0)
+
+
+class ImportGoodreadsProviderErrors(TestCase):
+    """Test GoodReads provider error handling."""
+
+    def setUp(self):
+        """Create user for the tests."""
+        self.credentials = {"username": "test", "password": "12345"}
+        self.user = get_user_model().objects.create_user(**self.credentials)
+
+    @patch("integrations.imports.goodreads.GoodReadsImporter._process_row")
+    def test_provider_error_warns_with_goodreads_fields(self, mock_process_row):
+        """Test provider failures warn and continue without requiring media_id."""
+        response = MagicMock(status_code=408, text='{"error":"Request timeout"}')
+        error = requests.exceptions.HTTPError(response=response)
+        mock_process_row.side_effect = services.ProviderAPIError(
+            Sources.HARDCOVER.value,
+            error,
+        )
+
+        with Path(mock_path / "import_goodreads.csv").open("rb") as file:
+            imported_counts, warnings = goodreads.importer(file, self.user, "new")
+
+        self.assertEqual(imported_counts, {})
+        self.assertIn(
+            "Ghosts of the Tristan Basin (Powder Mage, #0.8) (Goodreads ID 28825810)",
+            warnings,
+        )
+        self.assertIn("There was an error contacting the Hardcover API", warnings)