Merge pull request #1599 from bookwyrm-social/remove-serialize-image-helper

Remove serialize image helper

bookwyrm/models/fields.py

@@ -383,19 +383,6 @@ class CustomImageField(DjangoImageField):
     widget = ClearableFileInputWithWarning
 
 
-def image_serializer(value, alt):
-    """helper for serializing images"""
-    if value and hasattr(value, "url"):
-        url = value.url
-    else:
-        return None
-    if url is not None:
-        url = url.lstrip("/")
-    url = urljoin(MEDIA_FULL_URL, url)
-
-    return activitypub.Document(url=url, name=alt)
-
-
 class ImageField(ActivitypubFieldMixin, models.ImageField):
     """activitypub-aware image field"""
 
@@ -428,7 +415,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         activity[key] = formatted
 
     def field_to_activity(self, value, alt=None):
-        url = self.get_absolute_url(value)
+        url = get_absolute_url(value)
 
         if not url:
             return None
@@ -469,8 +456,8 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
             }
         )
 
-    # pylint: disable=no-self-use
+
-    def get_absolute_url(self, value):
+def get_absolute_url(value):
     """returns an absolute URL for the image"""
     name = getattr(value, "name")
     if not name:

bookwyrm/models/status.py

@@ -19,7 +19,6 @@ from bookwyrm.settings import ENABLE_PREVIEW_IMAGES
 from .activitypub_mixin import ActivitypubMixin, ActivityMixin
 from .activitypub_mixin import OrderedCollectionPageMixin
 from .base_model import BookWyrmModel
-from .fields import image_serializer
 from .readthrough import ProgressMode
 from . import fields
 
@@ -190,15 +189,24 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
             if hasattr(activity, "name"):
                 activity.name = self.pure_name
             activity.type = self.pure_type
-            activity.attachment = [
+            books = [getattr(self, "book", None)] + list(self.mention_books.all())
-                image_serializer(b.cover, b.alt_text)
+            if len(books) == 1 and books[0].preview_image:
-                for b in self.mention_books.all()[:4]
+                covers = [
-                if b.cover
+                    activitypub.Document(
-            ]
+                        url=fields.get_absolute_url(books[0].preview_image),
-            if hasattr(self, "book") and self.book.cover:
+                        name=books[0].alt_text,
-                activity.attachment.append(
-                    image_serializer(self.book.cover, self.book.alt_text)
                     )
+                ]
+            else:
+                covers = [
+                    activitypub.Document(
+                        url=fields.get_absolute_url(b.cover),
+                        name=b.alt_text,
+                    )
+                    for b in books
+                    if b and b.cover
+                ]
+            activity.attachment = covers
         return activity
 
     def to_activity(self, pure=False):  # pylint: disable=arguments-differ

bookwyrm/tests/models/test_fields.py

@@ -447,15 +447,6 @@ class ModelFields(TestCase):
         self.assertIsInstance(loaded_image, list)
         self.assertIsInstance(loaded_image[1], ContentFile)
 
-    def test_image_serialize(self, *_):
-        """make sure we're creating sensible image paths"""
-        ValueMock = namedtuple("ValueMock", ("url"))
-        value_mock = ValueMock("https://your.domain.here/images/fish.jpg")
-        result = fields.image_serializer(value_mock, "hello")
-        self.assertEqual(result.type, "Document")
-        self.assertEqual(result.url, "https://your.domain.here/images/fish.jpg")
-        self.assertEqual(result.name, "hello")
-
     def test_datetime_field(self, *_):
         """this one is pretty simple, it just has to use isoformat"""
         instance = fields.DateTimeField()

bookwyrm/tests/models/test_status_model.py

@@ -2,7 +2,7 @@
 from unittest.mock import patch
 from io import BytesIO
 import pathlib
-from urllib.parse import urljoin
+import re
 
 from django.http import Http404
 from django.core.files.base import ContentFile
@@ -191,9 +191,11 @@ class Status(TestCase):
         self.assertEqual(activity["sensitive"], False)
         self.assertIsInstance(activity["attachment"], list)
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -221,9 +223,11 @@ class Status(TestCase):
             f'test content<p>(comment on <a href="{self.book.remote_id}">"Test Edition"</a>)</p>',
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -258,9 +262,11 @@ class Status(TestCase):
             f'a sickening sense <p>-- <a href="{self.book.remote_id}">"Test Edition"</a></p>test content',
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -299,9 +305,11 @@ class Status(TestCase):
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -321,9 +329,11 @@ class Status(TestCase):
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -342,9 +352,11 @@ class Status(TestCase):
             f'rated <em><a href="{self.book.remote_id}">{self.book.title}</a></em>: 3 stars',
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
+            re.match(
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
                 activity["attachment"][0].url,
-            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")