diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 927c8740..3f849597 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() diff --git a/bookwyrm/tests/views/inbox/test_inbox_follow.py b/bookwyrm/tests/views/inbox/test_inbox_follow.py index c549c31b..b0177cb8 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(