Merge branch 'main' into production

bookwyrm/models/activitypub_mixin.py

@@ -30,6 +30,7 @@ logger = logging.getLogger(__name__)
 PropertyField = namedtuple("PropertyField", ("set_activity_from_field"))
 
 
+# pylint: disable=invalid-name
 def set_activity_from_property_field(activity, obj, field):
     """assign a model property value to the activity json"""
     activity[field[1]] = getattr(obj, field[0])
@@ -318,7 +319,9 @@ class OrderedCollectionPageMixin(ObjectMixin):
 
         remote_id = remote_id or self.remote_id
         if page:
-            return to_ordered_collection_page(queryset, remote_id, **kwargs)
+            if isinstance(page, list) and len(page) > 0:
+                page = page[0]
+            return to_ordered_collection_page(queryset, remote_id, page=page, **kwargs)
 
         if collection_only or not hasattr(self, "activity_serializer"):
             serializer = activitypub.OrderedCollection

bookwyrm/settings.py

@@ -166,6 +166,7 @@ LANGUAGES = [
     ("es", _("Spanish")),
     ("fr-fr", _("French")),
     ("zh-hans", _("Simplified Chinese")),
+    ("zh-hant", _("Traditional Chinese")),
 ]
 
 

bookwyrm/tasks.py

@@ -7,6 +7,5 @@ from bookwyrm import settings
 # set the default Django settings module for the 'celery' program.
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "celerywyrm.settings")
 app = Celery(
-    "tasks",
+    "tasks", broker=settings.CELERY_BROKER, backend=settings.CELERY_RESULT_BACKEND
-    broker=settings.CELERY_BROKER,
 )

bookwyrm/templates/book/book.html

@@ -47,7 +47,7 @@
 
         {% if user_authenticated and can_edit_book %}
         <div class="column is-narrow">
-            <a href="{{ book.id }}/edit">
+            <a href="{% url 'edit-book' book.id %}">
                 <span class="icon icon-pencil" title="{% trans "Edit Book" %}" aria-hidden=True></span>
                 <span class="is-hidden-mobile">{% trans "Edit Book" %}</span>
             </a>
@@ -209,24 +209,24 @@
                     <ul>
                         {% url 'book' book.id as tab_url %}
                         <li {% if tab_url == request.path %}class="is-active"{% endif %}>
-                            <a href="{{ tab_url }}">{% trans "Reviews" %} ({{ review_count }})</a>
+                            <a href="{{ tab_url }}#reviews">{% trans "Reviews" %} ({{ review_count }})</a>
                         </li>
                         {% if user_statuses.review_count %}
                         {% url 'book-user-statuses' book.id 'review' as tab_url %}
                         <li {% if tab_url == request.path %}class="is-active"{% endif %}>
-                            <a href="{{ tab_url }}">{% trans "Your reviews" %} ({{ user_statuses.review_count }})</a>
+                            <a href="{{ tab_url }}#reviews">{% trans "Your reviews" %} ({{ user_statuses.review_count }})</a>
                         </li>
                         {% endif %}
                         {% if user_statuses.comment_count %}
                         {% url 'book-user-statuses' book.id 'comment' as tab_url %}
                         <li {% if tab_url == request.path %}class="is-active"{% endif %}>
-                            <a href="{{ tab_url }}">{% trans "Your comments" %} ({{ user_statuses.comment_count }})</a>
+                            <a href="{{ tab_url }}#reviews">{% trans "Your comments" %} ({{ user_statuses.comment_count }})</a>
                         </li>
                         {% endif %}
                         {% if user_statuses.quotation_count %}
                         {% url 'book-user-statuses' book.id 'quote' as tab_url %}
                         <li {% if tab_url == request.path %}class="is-active"{% endif %}>
-                            <a href="{{ tab_url }}">{% trans "Your quotes" %} ({{ user_statuses.quotation_count }})</a>
+                            <a href="{{ tab_url }}#reviews">{% trans "Your quotes" %} ({{ user_statuses.quotation_count }})</a>
                         </li>
                         {% endif %}
                     </ul>

bookwyrm/tests/models/test_activitypub_mixin.py

@@ -9,11 +9,19 @@ from django.test import TestCase
 from bookwyrm.activitypub.base_activity import ActivityObject
 from bookwyrm import models
 from bookwyrm.models import base_model
-from bookwyrm.models.activitypub_mixin import ActivitypubMixin
+from bookwyrm.models.activitypub_mixin import (
-from bookwyrm.models.activitypub_mixin import ActivityMixin, ObjectMixin
+    ActivitypubMixin,
+    ActivityMixin,
+    ObjectMixin,
+    OrderedCollectionMixin,
+    to_ordered_collection_page,
+)
+from bookwyrm.settings import PAGE_LENGTH
 
 
+# pylint: disable=invalid-name
 @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+@patch("bookwyrm.preview_images.generate_user_preview_image_task.delay")
 class ActivitypubMixins(TestCase):
     """functionality shared across models"""
 
@@ -45,8 +53,7 @@ class ActivitypubMixins(TestCase):
             "published": "2020-12-04T17:52:22.623807+00:00",
         }
 
-    # ActivitypubMixin
+    def test_to_activity(self, *_):
-    def test_to_activity(self, _):
         """model to ActivityPub json"""
 
         @dataclass(init=False)
@@ -67,7 +74,7 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(activity["id"], "https://www.example.com/test")
         self.assertEqual(activity["type"], "Test")
 
-    def test_find_existing_by_remote_id(self, _):
+    def test_find_existing_by_remote_id(self, *_):
         """attempt to match a remote id to an object in the db"""
         # uses a different remote id scheme
         # this isn't really part of this test directly but it's helpful to state
@@ -101,7 +108,7 @@ class ActivitypubMixins(TestCase):
         # test subclass match
         result = models.Status.find_existing_by_remote_id("https://comment.net")
 
-    def test_find_existing(self, _):
+    def test_find_existing(self, *_):
         """match a blob of data to a model"""
         with patch("bookwyrm.preview_images.generate_edition_preview_image_task.delay"):
             book = models.Edition.objects.create(
@@ -112,7 +119,7 @@ class ActivitypubMixins(TestCase):
         result = models.Edition.find_existing({"openlibraryKey": "OL1234"})
         self.assertEqual(result, book)
 
-    def test_get_recipients_public_object(self, _):
+    def test_get_recipients_public_object(self, *_):
         """determines the recipients for an object's broadcast"""
         MockSelf = namedtuple("Self", ("privacy"))
         mock_self = MockSelf("public")
@@ -120,7 +127,7 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(len(recipients), 1)
         self.assertEqual(recipients[0], self.remote_user.inbox)
 
-    def test_get_recipients_public_user_object_no_followers(self, _):
+    def test_get_recipients_public_user_object_no_followers(self, *_):
         """determines the recipients for a user's object broadcast"""
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
@@ -128,7 +135,7 @@ class ActivitypubMixins(TestCase):
         recipients = ActivitypubMixin.get_recipients(mock_self)
         self.assertEqual(len(recipients), 0)
 
-    def test_get_recipients_public_user_object(self, _):
+    def test_get_recipients_public_user_object(self, *_):
         """determines the recipients for a user's object broadcast"""
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
@@ -138,22 +145,21 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(len(recipients), 1)
         self.assertEqual(recipients[0], self.remote_user.inbox)
 
-    def test_get_recipients_public_user_object_with_mention(self, _):
+    def test_get_recipients_public_user_object_with_mention(self, *_):
         """determines the recipients for a user's object broadcast"""
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
         self.local_user.followers.add(self.remote_user)
-        with patch("bookwyrm.preview_images.generate_user_preview_image_task.delay"):
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
-            with patch("bookwyrm.models.user.set_remote_server.delay"):
+            another_remote_user = models.User.objects.create_user(
-                another_remote_user = models.User.objects.create_user(
+                "nutria",
-                    "nutria",
+                "nutria@nutria.com",
-                    "nutria@nutria.com",
+                "nutriaword",
-                    "nutriaword",
+                local=False,
-                    local=False,
+                remote_id="https://example.com/users/nutria",
-                    remote_id="https://example.com/users/nutria",
+                inbox="https://example.com/users/nutria/inbox",
-                    inbox="https://example.com/users/nutria/inbox",
+                outbox="https://example.com/users/nutria/outbox",
-                    outbox="https://example.com/users/nutria/outbox",
+            )
-                )
         MockSelf = namedtuple("Self", ("privacy", "user", "recipients"))
         mock_self = MockSelf("public", self.local_user, [another_remote_user])
 
@@ -162,22 +168,21 @@ class ActivitypubMixins(TestCase):
         self.assertTrue(another_remote_user.inbox in recipients)
         self.assertTrue(self.remote_user.inbox in recipients)
 
-    def test_get_recipients_direct(self, _):
+    def test_get_recipients_direct(self, *_):
         """determines the recipients for a user's object broadcast"""
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
         self.local_user.followers.add(self.remote_user)
-        with patch("bookwyrm.preview_images.generate_user_preview_image_task.delay"):
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
-            with patch("bookwyrm.models.user.set_remote_server.delay"):
+            another_remote_user = models.User.objects.create_user(
-                another_remote_user = models.User.objects.create_user(
+                "nutria",
-                    "nutria",
+                "nutria@nutria.com",
-                    "nutria@nutria.com",
+                "nutriaword",
-                    "nutriaword",
+                local=False,
-                    local=False,
+                remote_id="https://example.com/users/nutria",
-                    remote_id="https://example.com/users/nutria",
+                inbox="https://example.com/users/nutria/inbox",
-                    inbox="https://example.com/users/nutria/inbox",
+                outbox="https://example.com/users/nutria/outbox",
-                    outbox="https://example.com/users/nutria/outbox",
+            )
-                )
         MockSelf = namedtuple("Self", ("privacy", "user", "recipients"))
         mock_self = MockSelf("direct", self.local_user, [another_remote_user])
 
@@ -185,22 +190,21 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(len(recipients), 1)
         self.assertEqual(recipients[0], another_remote_user.inbox)
 
-    def test_get_recipients_combine_inboxes(self, _):
+    def test_get_recipients_combine_inboxes(self, *_):
         """should combine users with the same shared_inbox"""
         self.remote_user.shared_inbox = "http://example.com/inbox"
         self.remote_user.save(broadcast=False)
-        with patch("bookwyrm.preview_images.generate_user_preview_image_task.delay"):
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
-            with patch("bookwyrm.models.user.set_remote_server.delay"):
+            another_remote_user = models.User.objects.create_user(
-                another_remote_user = models.User.objects.create_user(
+                "nutria",
-                    "nutria",
+                "nutria@nutria.com",
-                    "nutria@nutria.com",
+                "nutriaword",
-                    "nutriaword",
+                local=False,
-                    local=False,
+                remote_id="https://example.com/users/nutria",
-                    remote_id="https://example.com/users/nutria",
+                inbox="https://example.com/users/nutria/inbox",
-                    inbox="https://example.com/users/nutria/inbox",
+                shared_inbox="http://example.com/inbox",
-                    shared_inbox="http://example.com/inbox",
+                outbox="https://example.com/users/nutria/outbox",
-                    outbox="https://example.com/users/nutria/outbox",
+            )
-                )
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
         self.local_user.followers.add(self.remote_user)
@@ -210,20 +214,19 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(len(recipients), 1)
         self.assertEqual(recipients[0], "http://example.com/inbox")
 
-    def test_get_recipients_software(self, _):
+    def test_get_recipients_software(self, *_):
         """should differentiate between bookwyrm and other remote users"""
-        with patch("bookwyrm.preview_images.generate_user_preview_image_task.delay"):
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
-            with patch("bookwyrm.models.user.set_remote_server.delay"):
+            another_remote_user = models.User.objects.create_user(
-                another_remote_user = models.User.objects.create_user(
+                "nutria",
-                    "nutria",
+                "nutria@nutria.com",
-                    "nutria@nutria.com",
+                "nutriaword",
-                    "nutriaword",
+                local=False,
-                    local=False,
+                remote_id="https://example.com/users/nutria",
-                    remote_id="https://example.com/users/nutria",
+                inbox="https://example.com/users/nutria/inbox",
-                    inbox="https://example.com/users/nutria/inbox",
+                outbox="https://example.com/users/nutria/outbox",
-                    outbox="https://example.com/users/nutria/outbox",
+                bookwyrm_user=False,
-                    bookwyrm_user=False,
+            )
-                )
         MockSelf = namedtuple("Self", ("privacy", "user"))
         mock_self = MockSelf("public", self.local_user)
         self.local_user.followers.add(self.remote_user)
@@ -241,7 +244,7 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(recipients[0], another_remote_user.inbox)
 
     # ObjectMixin
-    def test_object_save_create(self, _):
+    def test_object_save_create(self, *_):
         """should save uneventufully when broadcast is disabled"""
 
         class Success(Exception):
@@ -272,7 +275,7 @@ class ActivitypubMixins(TestCase):
         ObjectModel(user=self.local_user).save(broadcast=False)
         ObjectModel(user=None).save()
 
-    def test_object_save_update(self, _):
+    def test_object_save_update(self, *_):
         """should save uneventufully when broadcast is disabled"""
 
         class Success(Exception):
@@ -298,7 +301,7 @@ class ActivitypubMixins(TestCase):
         with self.assertRaises(Success):
             UpdateObjectModel(id=1, last_edited_by=self.local_user).save()
 
-    def test_object_save_delete(self, _):
+    def test_object_save_delete(self, *_):
         """should create delete activities when objects are deleted by flag"""
 
         class ActivitySuccess(Exception):
@@ -320,7 +323,7 @@ class ActivitypubMixins(TestCase):
         with self.assertRaises(ActivitySuccess):
             DeletableObjectModel(id=1, user=self.local_user, deleted=True).save()
 
-    def test_to_delete_activity(self, _):
+    def test_to_delete_activity(self, *_):
         """wrapper for Delete activity"""
         MockSelf = namedtuple("Self", ("remote_id", "to_activity"))
         mock_self = MockSelf(
@@ -335,7 +338,7 @@ class ActivitypubMixins(TestCase):
             activity["cc"], ["https://www.w3.org/ns/activitystreams#Public"]
         )
 
-    def test_to_update_activity(self, _):
+    def test_to_update_activity(self, *_):
         """ditto above but for Update"""
         MockSelf = namedtuple("Self", ("remote_id", "to_activity"))
         mock_self = MockSelf(
@@ -352,8 +355,7 @@ class ActivitypubMixins(TestCase):
         )
         self.assertIsInstance(activity["object"], dict)
 
-    # Activity mixin
+    def test_to_undo_activity(self, *_):
-    def test_to_undo_activity(self, _):
         """and again, for Undo"""
         MockSelf = namedtuple("Self", ("remote_id", "to_activity", "user"))
         mock_self = MockSelf(
@@ -366,3 +368,59 @@ class ActivitypubMixins(TestCase):
         self.assertEqual(activity["actor"], self.local_user.remote_id)
         self.assertEqual(activity["type"], "Undo")
         self.assertIsInstance(activity["object"], dict)
+
+    @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
+    def test_to_ordered_collection_page(self, *_):
+        """make sure the paged results of an ordered collection work"""
+        self.assertEqual(PAGE_LENGTH, 15)
+        for number in range(0, 2 * PAGE_LENGTH):
+            models.Status.objects.create(
+                user=self.local_user,
+                content="test status {:d}".format(number),
+            )
+        page_1 = to_ordered_collection_page(
+            models.Status.objects.all(), "http://fish.com/", page=1
+        )
+        self.assertEqual(page_1.partOf, "http://fish.com/")
+        self.assertEqual(page_1.id, "http://fish.com/?page=1")
+        self.assertEqual(page_1.next, "http://fish.com/?page=2")
+        self.assertEqual(page_1.orderedItems[0]["content"], "test status 29")
+        self.assertEqual(page_1.orderedItems[1]["content"], "test status 28")
+
+        page_2 = to_ordered_collection_page(
+            models.Status.objects.all(), "http://fish.com/", page=2
+        )
+        self.assertEqual(page_2.partOf, "http://fish.com/")
+        self.assertEqual(page_2.id, "http://fish.com/?page=2")
+        self.assertEqual(page_2.orderedItems[0]["content"], "test status 14")
+        self.assertEqual(page_2.orderedItems[-1]["content"], "test status 0")
+
+    @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
+    def test_to_ordered_collection(self, *_):
+        """convert a queryset into an ordered collection object"""
+        self.assertEqual(PAGE_LENGTH, 15)
+
+        for number in range(0, 2 * PAGE_LENGTH):
+            models.Status.objects.create(
+                user=self.local_user,
+                content="test status {:d}".format(number),
+            )
+
+        MockSelf = namedtuple("Self", ("remote_id"))
+        mock_self = MockSelf("")
+
+        collection = OrderedCollectionMixin.to_ordered_collection(
+            mock_self, models.Status.objects.all(), remote_id="http://fish.com/"
+        )
+
+        self.assertEqual(collection.totalItems, 30)
+        self.assertEqual(collection.first, "http://fish.com/?page=1")
+        self.assertEqual(collection.last, "http://fish.com/?page=2")
+
+        page_2 = OrderedCollectionMixin.to_ordered_collection(
+            mock_self, models.Status.objects.all(), remote_id="http://fish.com/", page=2
+        )
+        self.assertEqual(page_2.partOf, "http://fish.com/")
+        self.assertEqual(page_2.id, "http://fish.com/?page=2")
+        self.assertEqual(page_2.orderedItems[0]["content"], "test status 14")
+        self.assertEqual(page_2.orderedItems[-1]["content"], "test status 0")

bookwyrm/tests/views/test_status.py

@@ -8,6 +8,7 @@ from bookwyrm import forms, models, views
 from bookwyrm.settings import DOMAIN
 
 
+# pylint: disable=invalid-name
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
 class StatusViews(TestCase):
     """viewing and creating statuses"""
@@ -318,6 +319,15 @@ class StatusViews(TestCase):
             '<p><em>hi</em> and <a href="http://fish.com">fish.com</a> ' "is rad</p>",
         )
 
+    def test_to_markdown_detect_url(self, _):
+        """this is mostly handled in other places, but nonetheless"""
+        text = "http://fish.com/@hello#okay"
+        result = views.status.to_markdown(text)
+        self.assertEqual(
+            result,
+            '<p><a href="http://fish.com/@hello#okay">fish.com/@hello#okay</a></p>',
+        )
+
     def test_to_markdown_link(self, _):
         """this is mostly handled in other places, but nonetheless"""
         text = "[hi](http://fish.com) is <marquee>rad</marquee>"

bookwyrm/urls.py

@@ -192,8 +192,8 @@ urlpatterns = [
     re_path(r"^import/?$", views.Import.as_view(), name="import"),
     re_path(r"^import/(\d+)/?$", views.ImportStatus.as_view(), name="import-status"),
     # users
-    re_path(r"%s/?$" % USER_PATH, views.User.as_view(), name="user-feed"),
     re_path(r"%s\.json$" % USER_PATH, views.User.as_view()),
+    re_path(r"%s/?$" % USER_PATH, views.User.as_view(), name="user-feed"),
     re_path(r"%s/rss" % USER_PATH, views.rss_feed.RssFeed(), name="user-rss"),
     re_path(
         r"%s/followers(.json)?/?$" % USER_PATH,
@@ -295,7 +295,7 @@ urlpatterns = [
         views.Book.as_view(),
         name="book-user-statuses",
     ),
-    re_path(r"%s/edit/?$" % BOOK_PATH, views.EditBook.as_view()),
+    re_path(r"%s/edit/?$" % BOOK_PATH, views.EditBook.as_view(), name="edit-book"),
     re_path(r"%s/confirm/?$" % BOOK_PATH, views.ConfirmEditBook.as_view()),
     re_path(r"^create-book/?$", views.EditBook.as_view(), name="create-book"),
     re_path(r"^create-book/confirm?$", views.ConfirmEditBook.as_view()),

bookwyrm/views/books.py

@@ -59,7 +59,7 @@ class Book(View):
                 queryset = book.comment_set
             else:
                 queryset = book.quotation_set
-            queryset = queryset.filter(user=request.user)
+            queryset = queryset.filter(user=request.user, deleted=False)
         else:
             queryset = reviews.exclude(Q(content__isnull=True) | Q(content=""))
         queryset = queryset.select_related("user")
@@ -102,10 +102,11 @@ class Book(View):
                 book__parent_work=book.parent_work,
             ).select_related("shelf", "book")
 
+            filters = {"user": request.user, "deleted": False}
             data["user_statuses"] = {
-                "review_count": book.review_set.filter(user=request.user).count(),
+                "review_count": book.review_set.filter(**filters).count(),
-                "comment_count": book.comment_set.filter(user=request.user).count(),
+                "comment_count": book.comment_set.filter(**filters).count(),
-                "quotation_count": book.quotation_set.filter(user=request.user).count(),
+                "quotation_count": book.quotation_set.filter(**filters).count(),
             }
 
         return TemplateResponse(request, "book/book.html", data)

bookwyrm/views/import_data.py

@@ -84,7 +84,9 @@ class ImportStatus(View):
 
         try:
             task = app.AsyncResult(job.task_id)
-        except ValueError:
+            # triggers attribute error if the task won't load
+            task.status  # pylint: disable=pointless-statement
+        except (ValueError, AttributeError):
             task = None
 
         items = job.items.order_by("index").all()

bookwyrm/views/status.py

@@ -150,7 +150,7 @@ def find_mentions(content):
 def format_links(content):
     """detect and format links"""
     return re.sub(
-        r'([^(href=")]|^|\()(https?:\/\/(%s([\w\.\-_\/+&\?=:;,])*))' % regex.DOMAIN,
+        r'([^(href=")]|^|\()(https?:\/\/(%s([\w\.\-_\/+&\?=:;,@#])*))' % regex.DOMAIN,
         r'\g<1><a href="\g<2>">\g<3></a>',
         content,
     )

bw-dev

@@ -32,7 +32,9 @@ function initdb {
 }
 
 CMD=$1
-shift
+if [ -n "$CMD" ]; then
+    shift
+fi
 
 # show commands as they're executed
 set -x
@@ -47,6 +49,23 @@ case "$CMD" in
     initdb)
         initdb
         ;;
+<<<<<<< HEAD
+=======
+    resetdb)
+        clean
+        # Start just the DB so no one else is using it
+        docker-compose up --build -d db
+        execdb dropdb -U ${POSTGRES_USER} ${POSTGRES_DB}
+        execdb createdb -U ${POSTGRES_USER} ${POSTGRES_DB}
+        # Now start up web so we can run the migrations
+        docker-compose up --build -d web
+        initdb
+        clean
+        ;;
+    makemigrations)
+        runweb python manage.py makemigrations "$@"
+        ;;
+>>>>>>> main
     migrate)
         runweb python manage.py migrate "$@"
         ;;
@@ -81,7 +100,14 @@ case "$CMD" in
     generate_preview_images)
         runweb python manage.py generate_preview_images $@
         ;;
+    runweb)
+        runweb "$@"
+        ;;
+    rundb)
+        rundb "$@"
+        ;;
     *)
+        set +x # No need to echo echo
         echo "Unrecognised command. Try: build, up, initdb, migrate, bash, shell, dbshell, restart_celery, update, populate_streams, generate_preview_images"
         ;;
 esac