Merge branch 'main' into production

bookwyrm/activitypub/book.py

@@ -11,6 +11,7 @@ class Book(ActivityObject):
     """ serializes an edition or work, abstract """
 
     title: str
+    lastEditedBy: str = None
     sortTitle: str = ""
     subtitle: str = ""
     description: str = ""
@@ -64,6 +65,7 @@ class Author(ActivityObject):
     """ author of a book """
 
     name: str
+    lastEditedBy: str = None
     born: str = None
     died: str = None
     aliases: List[str] = field(default_factory=lambda: [])

bookwyrm/models/activitypub_mixin.py

@@ -148,14 +148,18 @@ class ActivitypubMixin:
         mentions = self.recipients if hasattr(self, "recipients") else []
 
         # we always send activities to explicitly mentioned users' inboxes
-        recipients = [u.inbox for u in mentions or []]
+        recipients = [u.inbox for u in mentions or [] if not u.local]
 
         # unless it's a dm, all the followers should receive the activity
         if privacy != "direct":
             # we will send this out to a subset of all remote users
-            queryset = user_model.viewer_aware_objects(user).filter(
+            queryset = (
+                user_model.viewer_aware_objects(user)
+                .filter(
                     local=False,
                 )
+                .distinct()
+            )
             # filter users first by whether they're using the desired software
             # this lets us send book updates only to other bw servers
             if software:
@@ -175,7 +179,7 @@ class ActivitypubMixin:
                 "inbox", flat=True
             )
             recipients += list(shared_inboxes) + list(inboxes)
-        return recipients
+        return list(set(recipients))
 
     def to_activity_dataclass(self):
         """ convert from a model to an activity """

bookwyrm/models/book.py

@@ -26,7 +26,11 @@ class BookDataModel(ObjectMixin, BookWyrmModel):
         max_length=255, blank=True, null=True, deduplication_field=True
     )
 
-    last_edited_by = models.ForeignKey("User", on_delete=models.PROTECT, null=True)
+    last_edited_by = fields.ForeignKey(
+        "User",
+        on_delete=models.PROTECT,
+        null=True,
+    )
 
     class Meta:
         """ can't initialize this model, that wouldn't make sense """

bookwyrm/models/relationship.py

@@ -101,12 +101,15 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship):
 
     def save(self, *args, broadcast=True, **kwargs):
         """ make sure the follow or block relationship doesn't already exist """
-        # don't create a request if a follow already exists
+        # if there's a request for a follow that already exists, accept it
+        # without changing the local database state
         if UserFollows.objects.filter(
             user_subject=self.user_subject,
             user_object=self.user_object,
         ).exists():
-            raise IntegrityError()
+            self.accept(broadcast_only=True)
+            return
+
         # blocking in either direction is a no-go
         if UserBlocks.objects.filter(
             Q(
@@ -141,9 +144,9 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship):
         """ get id for sending an accept or reject of a local user """
 
         base_path = self.user_object.remote_id
-        return "%s#%s/%d" % (base_path, status, self.id)
+        return "%s#%s/%d" % (base_path, status, self.id or 0)
 
-    def accept(self):
+    def accept(self, broadcast_only=False):
         """ turn this request into the real deal"""
         user = self.user_object
         if not self.user_subject.local:
@@ -153,6 +156,9 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship):
                 object=self.to_activity(),
             ).serialize()
             self.broadcast(activity, user)
+        if broadcast_only:
+            return
+
         with transaction.atomic():
             UserFollows.from_request(self)
             self.delete()

bookwyrm/tests/data/bw_edition.json

@@ -1,5 +1,6 @@
 {
     "id": "https://bookwyrm.social/book/5989",
+    "lastEditedBy": "https://example.com/users/rat",
     "type": "Edition",
     "authors": [
         "https://bookwyrm.social/author/417"

bookwyrm/tests/views/inbox/test_inbox_follow.py

@@ -1,4 +1,5 @@
 """ tests incoming activities"""
+import json
 from unittest.mock import patch
 
 from django.test import TestCase
@@ -34,7 +35,7 @@ class InboxRelationships(TestCase):
 
         models.SiteSettings.objects.create()
 
-    def test_handle_follow(self):
+    def test_follow(self):
         """ remote user wants to follow local user """
         activity = {
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -48,6 +49,8 @@ class InboxRelationships(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.inbox.activity_task(activity)
             self.assertEqual(mock.call_count, 1)
+            response_activity = json.loads(mock.call_args[0][1])
+            self.assertEqual(response_activity["type"], "Accept")
 
         # notification created
         notification = models.Notification.objects.get()
@@ -61,7 +64,34 @@ class InboxRelationships(TestCase):
         follow = models.UserFollows.objects.get(user_object=self.local_user)
         self.assertEqual(follow.user_subject, self.remote_user)
 
-    def test_handle_follow_manually_approved(self):
+    def test_follow_duplicate(self):
+        """ remote user wants to follow local user twice """
+        activity = {
+            "@context": "https://www.w3.org/ns/activitystreams",
+            "id": "https://example.com/users/rat/follows/123",
+            "type": "Follow",
+            "actor": "https://example.com/users/rat",
+            "object": "https://example.com/user/mouse",
+        }
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.inbox.activity_task(activity)
+
+        # the follow relationship should exist
+        follow = models.UserFollows.objects.get(user_object=self.local_user)
+        self.assertEqual(follow.user_subject, self.remote_user)
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.inbox.activity_task(activity)
+            self.assertEqual(mock.call_count, 1)
+            response_activity = json.loads(mock.call_args[0][1])
+            self.assertEqual(response_activity["type"], "Accept")
+
+        # the follow relationship should STILL exist
+        follow = models.UserFollows.objects.get(user_object=self.local_user)
+        self.assertEqual(follow.user_subject, self.remote_user)
+
+    def test_follow_manually_approved(self):
         """ needs approval before following """
         activity = {
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -91,7 +121,7 @@ class InboxRelationships(TestCase):
         follow = models.UserFollows.objects.all()
         self.assertEqual(list(follow), [])
 
-    def test_handle_undo_follow_request(self):
+    def test_undo_follow_request(self):
         """ the requester cancels a follow request """
         self.local_user.manually_approves_followers = True
         self.local_user.save(broadcast=False)
@@ -121,7 +151,7 @@ class InboxRelationships(TestCase):
 
         self.assertFalse(self.local_user.follower_requests.exists())
 
-    def test_handle_unfollow(self):
+    def test_unfollow(self):
         """ remove a relationship """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollows.objects.create(
@@ -146,7 +176,7 @@ class InboxRelationships(TestCase):
         views.inbox.activity_task(activity)
         self.assertIsNone(self.local_user.followers.first())
 
-    def test_handle_follow_accept(self):
+    def test_follow_accept(self):
         """ a remote user approved a follow request from local """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollowRequest.objects.create(
@@ -177,7 +207,7 @@ class InboxRelationships(TestCase):
         self.assertEqual(follows.count(), 1)
         self.assertEqual(follows.first(), self.local_user)
 
-    def test_handle_follow_reject(self):
+    def test_follow_reject(self):
         """ turn down a follow request """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollowRequest.objects.create(

bookwyrm/tests/views/inbox/test_inbox_update.py

@@ -23,6 +23,16 @@ class InboxUpdate(TestCase):
         )
         self.local_user.remote_id = "https://example.com/user/mouse"
         self.local_user.save(broadcast=False)
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
+            self.remote_user = models.User.objects.create_user(
+                "rat",
+                "rat@rat.com",
+                "ratword",
+                local=False,
+                remote_id="https://example.com/users/rat",
+                inbox="https://example.com/users/rat/inbox",
+                outbox="https://example.com/users/rat/outbox",
+            )
 
         self.create_json = {
             "id": "hi",
@@ -34,7 +44,7 @@ class InboxUpdate(TestCase):
         }
         models.SiteSettings.objects.create()
 
-    def test_handle_update_list(self):
+    def test_update_list(self):
         """ a new list """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             book_list = models.List.objects.create(
@@ -68,7 +78,7 @@ class InboxUpdate(TestCase):
         self.assertEqual(book_list.description, "summary text")
         self.assertEqual(book_list.remote_id, "https://example.com/list/22")
 
-    def test_handle_update_user(self):
+    def test_update_user(self):
         """ update an existing user """
         # we only do this with remote users
         self.local_user.local = False
@@ -94,7 +104,7 @@ class InboxUpdate(TestCase):
         self.assertEqual(user.localname, "mouse")
         self.assertTrue(user.discoverable)
 
-    def test_handle_update_edition(self):
+    def test_update_edition(self):
         """ update an existing edition """
         datafile = pathlib.Path(__file__).parent.joinpath("../../data/bw_edition.json")
         bookdata = json.loads(datafile.read_bytes())
@@ -122,8 +132,9 @@ class InboxUpdate(TestCase):
             )
         book = models.Edition.objects.get(id=book.id)
         self.assertEqual(book.title, "Piranesi")
+        self.assertEqual(book.last_edited_by, self.remote_user)
 
-    def test_handle_update_work(self):
+    def test_update_work(self):
         """ update an existing edition """
         datafile = pathlib.Path(__file__).parent.joinpath("../../data/bw_work.json")
         bookdata = json.loads(datafile.read_bytes())