Merge pull request #987 from bookwyrm-social/duplicate-boosts

Don't broadcast boosts twice

bookwyrm/models/activitypub_mixin.py

@@ -204,7 +204,9 @@ class ObjectMixin(ActivitypubMixin):
         created = created or not bool(self.id)
         # first off, we want to save normally no matter what
         super().save(*args, **kwargs)
-        if not broadcast:
+        if not broadcast or (
+            hasattr(self, "status_type") and self.status_type == "Announce"
+        ):
             return
 
         # this will work for objects owned by a user (lists, shelves)

bookwyrm/models/status.py

@@ -351,6 +351,16 @@ class Boost(ActivityMixin, Status):
 
     def save(self, *args, **kwargs):
         """ save and notify """
+        # This constraint can't work as it would cross tables.
+        # class Meta:
+        #     unique_together = ('user', 'boosted_status')
+        if (
+            Boost.objects.filter(boosted_status=self.boosted_status, user=self.user)
+            .exclude(id=self.id)
+            .exists()
+        ):
+            return
+
         super().save(*args, **kwargs)
         if not self.boosted_status.user.local or self.boosted_status.user == self.user:
             return

bookwyrm/tests/views/inbox/test_inbox_announce.py

@@ -51,7 +51,7 @@ class InboxActivities(TestCase):
         models.SiteSettings.objects.create()
 
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
-    def test_handle_boost(self, _):
+    def test_boost(self, redis_mock):
         """ boost a status """
         self.assertEqual(models.Notification.objects.count(), 0)
         activity = {
@@ -66,16 +66,23 @@ class InboxActivities(TestCase):
         with patch("bookwyrm.models.status.Status.ignore_activity") as discarder:
             discarder.return_value = False
             views.inbox.activity_task(activity)
+
+        # boost added to redis activitystreams
+        self.assertTrue(redis_mock.called)
+
+        # boost created of correct status
         boost = models.Boost.objects.get()
         self.assertEqual(boost.boosted_status, self.status)
+
+        # notification sent to original poster
         notification = models.Notification.objects.get()
         self.assertEqual(notification.user, self.local_user)
         self.assertEqual(notification.related_status, self.status)
 
     @responses.activate
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
-    def test_handle_boost_remote_status(self, redis_mock):
-        """ boost a status """
+    def test_boost_remote_status(self, redis_mock):
+        """ boost a status from a remote server """
         work = models.Work.objects.create(title="work title")
         book = models.Edition.objects.create(
             title="Test",
@@ -123,7 +130,7 @@ class InboxActivities(TestCase):
         self.assertEqual(boost.boosted_status.comment.book, book)
 
     @responses.activate
-    def test_handle_discarded_boost(self):
+    def test_discarded_boost(self):
         """ test a boost of a mastodon status that will be discarded """
         status = models.Status(
             content="hi",
@@ -146,7 +153,7 @@ class InboxActivities(TestCase):
         views.inbox.activity_task(activity)
         self.assertEqual(models.Boost.objects.count(), 0)
 
-    def test_handle_unboost(self):
+    def test_unboost(self):
         """ undo a boost """
         with patch("bookwyrm.activitystreams.ActivityStream.add_status"):
             boost = models.Boost.objects.create(
@@ -175,7 +182,7 @@ class InboxActivities(TestCase):
             self.assertTrue(redis_mock.called)
         self.assertFalse(models.Boost.objects.exists())
 
-    def test_handle_unboost_unknown_boost(self):
+    def test_unboost_unknown_boost(self):
         """ undo a boost """
         activity = {
             "type": "Undo",

bookwyrm/tests/views/test_interaction.py

@@ -1,4 +1,5 @@
 """ test for app action functionality """
+import json
 from unittest.mock import patch
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -39,7 +40,7 @@ class InteractionViews(TestCase):
             parent_work=work,
         )
 
-    def test_handle_favorite(self, _):
+    def test_favorite(self, _):
         """ create and broadcast faving a status """
         view = views.Favorite.as_view()
         request = self.factory.post("")
@@ -57,7 +58,7 @@ class InteractionViews(TestCase):
         self.assertEqual(notification.user, self.local_user)
         self.assertEqual(notification.related_user, self.remote_user)
 
-    def test_handle_unfavorite(self, _):
+    def test_unfavorite(self, _):
         """ unfav a status """
         view = views.Unfavorite.as_view()
         request = self.factory.post("")
@@ -74,7 +75,7 @@ class InteractionViews(TestCase):
         self.assertEqual(models.Favorite.objects.count(), 0)
         self.assertEqual(models.Notification.objects.count(), 0)
 
-    def test_handle_boost(self, _):
+    def test_boost(self, _):
         """ boost a status """
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -85,6 +86,7 @@ class InteractionViews(TestCase):
             view(request, status.id)
 
         boost = models.Boost.objects.get()
+
         self.assertEqual(boost.boosted_status, status)
         self.assertEqual(boost.user, self.remote_user)
         self.assertEqual(boost.privacy, "public")
@@ -95,7 +97,7 @@ class InteractionViews(TestCase):
         self.assertEqual(notification.related_user, self.remote_user)
         self.assertEqual(notification.related_status, status)
 
-    def test_handle_self_boost(self, _):
+    def test_self_boost(self, _):
         """ boost your own status """
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -103,8 +105,15 @@ class InteractionViews(TestCase):
         with patch("bookwyrm.activitystreams.ActivityStream.add_status"):
             status = models.Status.objects.create(user=self.local_user, content="hi")
 
+            with patch(
+                "bookwyrm.models.activitypub_mixin.broadcast_task.delay"
+            ) as broadcast_mock:
                 view(request, status.id)
 
+        self.assertEqual(broadcast_mock.call_count, 1)
+        activity = json.loads(broadcast_mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Announce")
+
         boost = models.Boost.objects.get()
         self.assertEqual(boost.boosted_status, status)
         self.assertEqual(boost.user, self.local_user)
@@ -112,7 +121,7 @@ class InteractionViews(TestCase):
 
         self.assertFalse(models.Notification.objects.exists())
 
-    def test_handle_boost_unlisted(self, _):
+    def test_boost_unlisted(self, _):
         """ boost a status """
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -127,7 +136,7 @@ class InteractionViews(TestCase):
         boost = models.Boost.objects.get()
         self.assertEqual(boost.privacy, "unlisted")
 
-    def test_handle_boost_private(self, _):
+    def test_boost_private(self, _):
         """ boost a status """
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -140,7 +149,7 @@ class InteractionViews(TestCase):
             view(request, status.id)
         self.assertFalse(models.Boost.objects.exists())
 
-    def test_handle_boost_twice(self, _):
+    def test_boost_twice(self, _):
         """ boost a status """
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -152,7 +161,7 @@ class InteractionViews(TestCase):
             view(request, status.id)
         self.assertEqual(models.Boost.objects.count(), 1)
 
-    def test_handle_unboost(self, _):
+    def test_unboost(self, _):
         """ undo a boost """
         view = views.Unboost.as_view()
         request = self.factory.post("")