From 4f5d23e785dc8f070bfcf36fe075115a275c03df Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:28:43 -0800 Subject: [PATCH 1/5] Replace image_serialzier helper with built-in serializers --- bookwyrm/models/fields.py | 13 ------------- bookwyrm/models/status.py | 12 ++---------- bookwyrm/tests/models/test_fields.py | 9 --------- 3 files changed, 2 insertions(+), 32 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index a490c1336..03305f6bc 100644 --- a/bookwyrm/models/fields.py +++ b/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""" diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 2b395ec8b..a298b132f 100644 --- a/bookwyrm/models/status.py +++ b/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,8 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): if hasattr(activity, "name"): activity.name = self.pure_name activity.type = self.pure_type - activity.attachment = [ - image_serializer(b.cover, b.alt_text) - for b in self.mention_books.all()[:4] - if b.cover - ] - if hasattr(self, "book") and self.book.cover: - activity.attachment.append( - image_serializer(self.book.cover, self.book.alt_text) - ) + covers = [b.to_activity().get("cover") for b in [getattr(self, "book", None)] + list(self.mention_books.all()) if b] + activity.attachment = covers return activity def to_activity(self, pure=False): # pylint: disable=arguments-differ diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 6796f8d3a..74f4c48bd 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/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() From 9815e9e1009db30b46c814238da949e99708bad2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:30:18 -0800 Subject: [PATCH 2/5] Python formatting --- bookwyrm/models/status.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index a298b132f..767841c86 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -189,7 +189,11 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): if hasattr(activity, "name"): activity.name = self.pure_name activity.type = self.pure_type - covers = [b.to_activity().get("cover") for b in [getattr(self, "book", None)] + list(self.mention_books.all()) if b] + covers = [ + b.to_activity().get("cover") + for b in [getattr(self, "book", None)] + list(self.mention_books.all()) + if b + ] activity.attachment = covers return activity From d61595abb9df3c816a60f60332aa07a3fbdadde8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:50:05 -0800 Subject: [PATCH 3/5] Clearer syntax --- bookwyrm/models/status.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 767841c86..d3fcc2545 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -189,10 +189,9 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): if hasattr(activity, "name"): activity.name = self.pure_name activity.type = self.pure_type + books = [getattr(self, "book", None)] + list(self.mention_books.all()) covers = [ - b.to_activity().get("cover") - for b in [getattr(self, "book", None)] + list(self.mention_books.all()) - if b + b.to_activity().get("cover") for b in books if b ] activity.attachment = covers return activity From 717da918cfefdb1bb6586956028490e9ebce7d60 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:58:02 -0800 Subject: [PATCH 4/5] Use social media preview images --- bookwyrm/models/fields.py | 24 ++++++++++++------------ bookwyrm/models/status.py | 19 ++++++++++++++++--- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 03305f6bc..361079906 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -415,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 @@ -456,19 +456,19 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) - # pylint: disable=no-self-use - def get_absolute_url(self, value): - """returns an absolute URL for the image""" - name = getattr(value, "name") - if not name: - return None - url = filepath_to_uri(name) - if url is not None: - url = url.lstrip("/") - url = urljoin(MEDIA_FULL_URL, url) +def get_absolute_url(value): + """returns an absolute URL for the image""" + name = getattr(value, "name") + if not name: + return None - return url + url = filepath_to_uri(name) + if url is not None: + url = url.lstrip("/") + url = urljoin(MEDIA_FULL_URL, url) + + return url class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index d3fcc2545..a52af123c 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -190,9 +190,22 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): activity.name = self.pure_name activity.type = self.pure_type books = [getattr(self, "book", None)] + list(self.mention_books.all()) - covers = [ - b.to_activity().get("cover") for b in books if b - ] + if len(books) == 1 and books[0].preview_image: + covers = [ + activitypub.Document( + url=fields.get_absolute_url(books[0].preview_image), + name=books[0].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 From 8c4e8361f2c69041c58dd28889da1405578a5e18 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 11 Nov 2021 13:35:56 -0800 Subject: [PATCH 5/5] Fixes tests --- bookwyrm/tests/models/test_status_model.py | 50 ++++++++++++++-------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index f5013422c..7d0dd138b 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/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( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -221,9 +223,11 @@ class Status(TestCase): f'test content

(comment on "Test Edition")

', ) self.assertEqual(activity["attachment"][0].type, "Document") - self.assertEqual( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -258,9 +262,11 @@ class Status(TestCase): f'a sickening sense

-- "Test Edition"

test content', ) self.assertEqual(activity["attachment"][0].type, "Document") - self.assertEqual( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) 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( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) 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( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -342,9 +352,11 @@ class Status(TestCase): f'rated {self.book.title}: 3 stars', ) self.assertEqual(activity["attachment"][0].type, "Document") - self.assertEqual( - activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), + self.assertTrue( + re.match( + r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg", + activity["attachment"][0].url, + ) ) self.assertEqual(activity["attachment"][0].name, "Test Edition")