Merge branch 'main' into import-field-names

.env.dev.example

@@ -36,7 +36,7 @@ FLOWER_PORT=8888
 #FLOWER_USER=mouse
 #FLOWER_PASSWORD=changeme
 
-EMAIL_HOST="smtp.mailgun.org"
+EMAIL_HOST=smtp.mailgun.org
 EMAIL_PORT=587
 EMAIL_HOST_USER=mail@your.domain.here
 EMAIL_HOST_PASSWORD=emailpassword123

.env.prod.example

@@ -36,7 +36,7 @@ FLOWER_PORT=8888
 FLOWER_USER=mouse
 FLOWER_PASSWORD=changeme
 
-EMAIL_HOST="smtp.mailgun.org"
+EMAIL_HOST=smtp.mailgun.org
 EMAIL_PORT=587
 EMAIL_HOST_USER=mail@your.domain.here
 EMAIL_HOST_PASSWORD=emailpassword123

bookwyrm/models/fields.py

@@ -3,6 +3,7 @@ from dataclasses import MISSING
 import imghdr
 import re
 from uuid import uuid4
+from urllib.parse import urljoin
 
 import dateutil.parser
 from dateutil.parser import ParserError
@@ -13,11 +14,12 @@ from django.db import models
 from django.forms import ClearableFileInput, ImageField as DjangoImageField
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
+from django.utils.encoding import filepath_to_uri
 
 from bookwyrm import activitypub
 from bookwyrm.connectors import get_image
 from bookwyrm.sanitize_html import InputHtmlParser
-from bookwyrm.settings import DOMAIN
+from bookwyrm.settings import MEDIA_FULL_URL
 
 
 def validate_remote_id(value):
@@ -381,17 +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 not url[:4] == "http":
-        url = f"https://{DOMAIN}{url}"
-    return activitypub.Document(url=url, name=alt)
-
-
 class ImageField(ActivitypubFieldMixin, models.ImageField):
     """activitypub-aware image field"""
 
@@ -424,7 +415,12 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         activity[key] = formatted
 
     def field_to_activity(self, value, alt=None):
-        return image_serializer(value, alt)
+        url = get_absolute_url(value)
+
+        if not url:
+            return None
+
+        return activitypub.Document(url=url, name=alt)
 
     def field_from_activity(self, value):
         image_slug = value
@@ -461,6 +457,20 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         )
 
 
+def get_absolute_url(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)
+
+    return url
+
+
 class DateTimeField(ActivitypubFieldMixin, models.DateTimeField):
     """activitypub-aware datetime field"""
 

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/templates/discover/card-header.html

@@ -1,10 +1,24 @@
 {% load i18n %}
 {% load utilities %}
 
-{% with user_path=status.user.local_path username=status.user.display_name book_path=status.book.local_poth book_title=book|book_title %}
+{% with user_path=status.user.local_path username=status.user.display_name book_path=book.local_path book_title=book|book_title %}
 
 {% if status.status_type == 'GeneratedNote' %}
-    {{ status.content|safe }}
+    {% if status.content == 'wants to read' %}
+        {% blocktrans trimmed %}
+        <a href="{{ user_path}}">{{ username }}</a> wants to read <a href="{{ book_path }}">{{ book_title }}</a>
+        {% endblocktrans %}
+    {% endif %}
+    {% if status.content == 'finished reading' %}
+        {% blocktrans trimmed %}
+        <a href="{{ user_path}}">{{ username }}</a> finished reading <a href="{{ book_path }}">{{ book_title }}</a>
+        {% endblocktrans %}
+    {% endif %}
+    {% if status.content == 'started reading' %}
+        {% blocktrans trimmed %}
+        <a href="{{ user_path}}">{{ username }}</a> started reading <a href="{{ book_path }}">{{ book_title }}</a>
+        {% endblocktrans %}
+    {% endif %}
 {% elif status.status_type == 'Rating' %}
     {% blocktrans trimmed %}
     <a href="{{ user_path}}">{{ username }}</a> rated <a href="{{ book_path }}">{{ book_title }}</a>

bookwyrm/templates/import/import.html

@@ -46,10 +46,10 @@
                 </label>
             </div>
             <div class="field">
-                <label>
+                <label class="label" for="privacy_import">
-                    <span class="label">{% trans "Privacy setting for imported reviews:" %}</span>
+                    {% trans "Privacy setting for imported reviews:" %}
-                    {% include 'snippets/privacy_select.html' with no_label=True %}
                 </label>
+                {% include 'snippets/privacy_select.html' with no_label=True privacy_uuid="import" %}
             </div>
         </div>
         </div>

bookwyrm/templates/import/import_status.html

@@ -10,18 +10,17 @@
     <h1 class="title">{% trans "Import Status" %}</h1>
     <a href="{% url 'import' %}" class="has-text-weight-normal help subtitle is-link">{% trans "Back to imports" %}</a>
 
+    {% if task.failed %}
+    <div class="notification is-danger">{% trans "TASK FAILED" %}</div>
+    {% endif %}
+
     <dl>
-        <div class="is-flex">
+        <dt class="is-pulled-left mr-5">{% trans "Import started:" %}</dt>
-            <dt class="has-text-weight-medium">{% trans "Import started:" %}</dt>
+        <dd>{{ job.created_date | naturaltime }}</dd>
-            <dd class="ml-2">{{ job.created_date | naturaltime }}</dd>
+
-        </div>
         {% if job.complete %}
-        <div class="is-flex">
+        <dt class="is-pulled-left mr-5">{% trans "Import completed:" %}</dt>
-            <dt class="has-text-weight-medium">{% trans "Import completed:" %}</dt>
+        <dd>{{ task.date_done | naturaltime }}</dd>
-            <dd class="ml-2">{{ task.date_done | naturaltime }}</dd>
-        </div>
-        {% elif task.failed %}
-        <div class="notification is-danger">{% trans "TASK FAILED" %}</div>
         {% endif %}
     </dl>
 </div>

bookwyrm/tests/models/test_fields.py

@@ -22,6 +22,7 @@ from bookwyrm.activitypub.base_activity import ActivityObject
 from bookwyrm.models import fields, User, Status
 from bookwyrm.models.base_model import BookWyrmModel
 from bookwyrm.models.activitypub_mixin import ActivitypubMixin
+from bookwyrm.settings import DOMAIN
 
 # pylint: disable=too-many-public-methods
 @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
@@ -424,21 +425,18 @@ class ModelFields(TestCase):
         image.save(output, format=image.format)
         user.avatar.save("test.jpg", ContentFile(output.getvalue()))
 
-        output = fields.image_serializer(user.avatar, alt="alt text")
+        instance = fields.ImageField()
+
+        output = instance.field_to_activity(user.avatar)
         self.assertIsNotNone(
             re.match(
-                r".*\.jpg",
+                fr"https:\/\/{DOMAIN}\/.*\.jpg",
                 output.url,
             )
         )
-        self.assertEqual(output.name, "alt text")
+        self.assertEqual(output.name, "")
         self.assertEqual(output.type, "Document")
 
-        instance = fields.ImageField()
-
-        output = fields.image_serializer(user.avatar, alt=None)
-        self.assertEqual(instance.field_to_activity(user.avatar), output)
-
         responses.add(
             responses.GET,
             "http://www.example.com/image.jpg",
@@ -449,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("/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,6 +2,7 @@
 from unittest.mock import patch
 from io import BytesIO
 import pathlib
+import re
 
 from django.http import Http404
 from django.core.files.base import ContentFile
@@ -190,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(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -220,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(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -257,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(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -298,9 +305,11 @@ class Status(TestCase):
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -320,9 +329,11 @@ class Status(TestCase):
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
-        self.assertEqual(
+        self.assertTrue(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -341,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(
-            activity["attachment"][0].url,
+            re.match(
-            f"https://{settings.DOMAIN}{self.book.cover.url}",
+                r"https:\/\/your.domain.here\/images\/covers\/test_[A-z0-9]+.jpg",
+                activity["attachment"][0].url,
+            )
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 

bookwyrm/tests/views/test_import.py

@@ -5,6 +5,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
+from bookwyrm.tests.validate_html import validate_html
 
 from bookwyrm import forms, models, views
 
@@ -34,7 +35,7 @@ class ImportViews(TestCase):
         request.user = self.local_user
         result = view(request)
         self.assertIsInstance(result, TemplateResponse)
-        result.render()
+        validate_html(result.render())
         self.assertEqual(result.status_code, 200)
 
     def test_import_status(self):
@@ -47,7 +48,7 @@ class ImportViews(TestCase):
             async_result.return_value = []
             result = view(request, import_job.id)
         self.assertIsInstance(result, TemplateResponse)
-        result.render()
+        validate_html(result.render())
         self.assertEqual(result.status_code, 200)
 
     def test_start_import(self):
@@ -59,7 +60,10 @@ class ImportViews(TestCase):
         form.data["include_reviews"] = False
         csv_file = pathlib.Path(__file__).parent.joinpath("../data/goodreads.csv")
         form.data["csv_file"] = SimpleUploadedFile(
-            csv_file, open(csv_file, "rb").read(), content_type="text/csv"
+            # pylint: disable=consider-using-with
+            csv_file,
+            open(csv_file, "rb").read(),
+            content_type="text/csv",
         )
 
         request = self.factory.post("", form.data)

bw-dev

@@ -61,7 +61,7 @@ case "$CMD" in
     up)
         docker-compose up --build "$@"
         ;;
-    run)
+    service_ports_web)
         docker-compose run --rm --service-ports web
         ;;
     initdb)
@@ -96,9 +96,6 @@ case "$CMD" in
     restart_celery)
         docker-compose restart celery_worker
         ;;
-    test)
-        runweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@"
-        ;;
     pytest)
         execweb pytest --no-cov-on-fail "$@"
         ;;
@@ -148,14 +145,11 @@ case "$CMD" in
     runweb)
         runweb "$@"
         ;;
-    rundb)
-        rundb "$@"
-        ;;
     *)
         set +x # No need to echo echo
         echo "Unrecognised command. Try:"
         echo "    up [container]"
-        echo "    run"
+        echo "    service_ports_web"
         echo "    initdb"
         echo "    resetdb"
         echo "    makemigrations [migration]"
@@ -164,10 +158,8 @@ case "$CMD" in
         echo "    shell"
         echo "    dbshell"
         echo "    restart_celery"
-        echo "    test [path]"
         echo "    pytest [path]"
         echo "    collectstatic"
-        echo "    add_locale [locale]"
         echo "    makemessages"
         echo "    compilemessages [locale]"
         echo "    build"
@@ -180,6 +172,5 @@ case "$CMD" in
         echo "    copy_media_to_s3"
         echo "    set_cors_to_s3 [cors file]"
         echo "    runweb [command]"
-        echo "    rundb [command]"
         ;;
 esac

nginx/production

@@ -27,7 +27,7 @@ server {
 #
 #     client_max_body_size 3M;
 #
-#     if ($host != "you-domain.com") {
+#     if ($host != "your-domain.com") {
 #         return 301 $scheme://your-domain.com$request_uri;
 #     }
 #

requirements.txt

@@ -20,7 +20,6 @@ django-storages==1.11.1
 
 # Dev
 black==21.4b0
-coverage==5.1
 pytest-django==4.1.0
 pytest==6.1.2
 pytest-cov==2.10.1