From ae5e7447312727169cde797ecbfc04a1c889e6ad Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 07:29:09 -0700 Subject: [PATCH 1/6] Save last edited by user --- bookwyrm/activitypub/book.py | 1 + bookwyrm/models/activitypub_mixin.py | 6 +++--- bookwyrm/models/book.py | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 7615adcf7..538fa571e 100644 --- a/bookwyrm/activitypub/book.py +++ b/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 = "" diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index f687e96cb..71e02082b 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -148,14 +148,14 @@ 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( 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 +175,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 """ diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index a6824c0ad..5280c7aaf 100644 --- a/bookwyrm/models/book.py +++ b/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 """ From c405580e8e2b28b4738d1140d99e5b8ffbf0b0dd Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 07:37:12 -0700 Subject: [PATCH 2/6] Updates edition federation test --- bookwyrm/tests/data/bw_edition.json | 1 + .../tests/views/inbox/test_inbox_update.py | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/bookwyrm/tests/data/bw_edition.json b/bookwyrm/tests/data/bw_edition.json index 0cc17d29a..6194e4090 100644 --- a/bookwyrm/tests/data/bw_edition.json +++ b/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" diff --git a/bookwyrm/tests/views/inbox/test_inbox_update.py b/bookwyrm/tests/views/inbox/test_inbox_update.py index 012343e78..114eae23d 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_update.py +++ b/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()) @@ -123,7 +133,7 @@ class InboxUpdate(TestCase): book = models.Edition.objects.get(id=book.id) self.assertEqual(book.title, "Piranesi") - 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()) From db09ca43312fbbfafcda1d792db130cb358f2548 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 07:51:06 -0700 Subject: [PATCH 3/6] Adds last edited by to author --- bookwyrm/activitypub/book.py | 1 + bookwyrm/models/activitypub_mixin.py | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 538fa571e..c5b896e34 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -65,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: []) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 71e02082b..82a45ac86 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -153,9 +153,13 @@ class ActivitypubMixin: # 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( - local=False, - ).distinct() + 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: From dd0aa7a12394fec47d37c7999f30e6e566d17d19 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 08:08:03 -0700 Subject: [PATCH 4/6] Test that remote user is being set on books --- bookwyrm/tests/views/inbox/test_inbox_update.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bookwyrm/tests/views/inbox/test_inbox_update.py b/bookwyrm/tests/views/inbox/test_inbox_update.py index 114eae23d..fbd377388 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_update.py +++ b/bookwyrm/tests/views/inbox/test_inbox_update.py @@ -132,6 +132,7 @@ 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_update_work(self): """ update an existing edition """ From d5b27e2202d806f52460e83c28d2d6ea49aa2ac7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 08:31:32 -0700 Subject: [PATCH 5/6] Test re-following a user --- .../tests/views/inbox/test_inbox_follow.py | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/bookwyrm/tests/views/inbox/test_inbox_follow.py b/bookwyrm/tests/views/inbox/test_inbox_follow.py index c549c31bd..b0177cb88 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_follow.py +++ b/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( From 6b84e53ddde07d68f293393a42158911a90c3686 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 22 Apr 2021 08:40:32 -0700 Subject: [PATCH 6/6] Send accepts to duplicate follow requests --- bookwyrm/models/relationship.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 927c87407..3f849597a 100644 --- a/bookwyrm/models/relationship.py +++ b/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()