Merge pull request #1175 from bookwyrm-social/dedplicate-boosts

Don't show duplicated statuses after boosts

bookwyrm/activitystreams.py

@@ -201,6 +201,19 @@ def add_status_on_create(sender, instance, created, *args, **kwargs):
     for stream in streams.values():
         stream.add_status(instance)
 
+    if sender != models.Boost:
+        return
+    # remove the original post and other, earlier boosts
+    boosted = instance.boost.boosted_status
+    old_versions = models.Boost.objects.filter(
+        boosted_status__id=boosted.id,
+        created_date__lt=instance.created_date,
+    )
+    for stream in streams.values():
+        stream.remove_object_from_related_stores(boosted)
+        for status in old_versions:
+            stream.remove_object_from_related_stores(status)
+
 
 @receiver(signals.post_delete, sender=models.Boost)
 # pylint: disable=unused-argument
@@ -208,7 +221,10 @@ def remove_boost_on_delete(sender, instance, *args, **kwargs):
     """boosts are deleted"""
     # we're only interested in new statuses
     for stream in streams.values():
+        # remove the boost
         stream.remove_object_from_related_stores(instance)
+        # re-add the original status
+        stream.add_status(instance.boosted_status)
 
 
 @receiver(signals.post_save, sender=models.UserFollows)

bookwyrm/tests/models/test_status_model.py

@@ -16,6 +16,7 @@ from bookwyrm import activitypub, models, settings
 # pylint: disable=too-many-public-methods
 @patch("bookwyrm.models.Status.broadcast")
 @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+@patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
 class Status(TestCase):
     """lotta types of statuses"""
 
@@ -384,7 +385,8 @@ class Status(TestCase):
                 user=self.local_user, notification_type="GLORB"
             )
 
-    def test_create_broadcast(self, _, broadcast_mock):
+    # pylint: disable=unused-argument
+    def test_create_broadcast(self, one, two, broadcast_mock, *_):
         """should send out two verions of a status on create"""
         models.Comment.objects.create(
             content="hi", user=self.local_user, book=self.book

bookwyrm/tests/test_templatetags.py

@@ -16,6 +16,7 @@ from bookwyrm.templatetags import (
 
 
 @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+@patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
 class TemplateTags(TestCase):
     """lotta different things here"""
 
@@ -38,28 +39,28 @@ class TemplateTags(TestCase):
             )
         self.book = models.Edition.objects.create(title="Test Book")
 
-    def test_get_user_rating(self, _):
+    def test_get_user_rating(self, *_):
         """get a user's most recent rating of a book"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             models.Review.objects.create(user=self.user, book=self.book, rating=3)
         self.assertEqual(bookwyrm_tags.get_user_rating(self.book, self.user), 3)
 
-    def test_get_user_rating_doesnt_exist(self, _):
+    def test_get_user_rating_doesnt_exist(self, *_):
         """there is no rating available"""
         self.assertEqual(bookwyrm_tags.get_user_rating(self.book, self.user), 0)
 
-    def test_get_user_identifer_local(self, _):
+    def test_get_user_identifer_local(self, *_):
         """fall back to the simplest uid available"""
         self.assertNotEqual(self.user.username, self.user.localname)
         self.assertEqual(utilities.get_user_identifier(self.user), "mouse")
 
-    def test_get_user_identifer_remote(self, _):
+    def test_get_user_identifer_remote(self, *_):
         """for a remote user, should be their full username"""
         self.assertEqual(
             utilities.get_user_identifier(self.remote_user), "rat@example.com"
         )
 
-    def test_get_replies(self, _):
+    def test_get_replies(self, *_):
         """direct replies to a status"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             parent = models.Review.objects.create(
@@ -87,7 +88,7 @@ class TemplateTags(TestCase):
         self.assertTrue(second_child in replies)
         self.assertFalse(third_child in replies)
 
-    def test_get_parent(self, _):
+    def test_get_parent(self, *_):
         """get the reply parent of a status"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             parent = models.Review.objects.create(
@@ -101,7 +102,7 @@ class TemplateTags(TestCase):
         self.assertEqual(result, parent)
         self.assertIsInstance(result, models.Review)
 
-    def test_get_user_liked(self, _):
+    def test_get_user_liked(self, *_):
         """did a user like a status"""
         status = models.Review.objects.create(user=self.remote_user, book=self.book)
 
@@ -110,7 +111,7 @@ class TemplateTags(TestCase):
             models.Favorite.objects.create(user=self.user, status=status)
         self.assertTrue(interaction.get_user_liked(self.user, status))
 
-    def test_get_user_boosted(self, _):
+    def test_get_user_boosted(self, *_):
         """did a user boost a status"""
         status = models.Review.objects.create(user=self.remote_user, book=self.book)
 
@@ -119,7 +120,7 @@ class TemplateTags(TestCase):
             models.Boost.objects.create(user=self.user, boosted_status=status)
         self.assertTrue(interaction.get_user_boosted(self.user, status))
 
-    def test_get_boosted(self, _):
+    def test_get_boosted(self, *_):
         """load a boosted status"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             status = models.Review.objects.create(user=self.remote_user, book=self.book)
@@ -128,7 +129,7 @@ class TemplateTags(TestCase):
         self.assertIsInstance(boosted, models.Review)
         self.assertEqual(boosted, status)
 
-    def test_get_book_description(self, _):
+    def test_get_book_description(self, *_):
         """grab it from the edition or the parent"""
         work = models.Work.objects.create(title="Test Work")
         self.book.parent_work = work
@@ -144,12 +145,12 @@ class TemplateTags(TestCase):
         self.book.save()
         self.assertEqual(bookwyrm_tags.get_book_description(self.book), "hello")
 
-    def test_get_uuid(self, _):
+    def test_get_uuid(self, *_):
         """uuid functionality"""
         uuid = utilities.get_uuid("hi")
         self.assertTrue(re.match(r"hi[A-Za-z0-9\-]", uuid))
 
-    def test_get_markdown(self, _):
+    def test_get_markdown(self, *_):
         """mardown format data"""
         result = markdown.get_markdown("_hi_")
         self.assertEqual(result, "<p><em>hi</em></p>")
@@ -157,13 +158,13 @@ class TemplateTags(TestCase):
         result = markdown.get_markdown("<marquee>_hi_</marquee>")
         self.assertEqual(result, "<p><em>hi</em></p>")
 
-    def test_get_mentions(self, _):
+    def test_get_mentions(self, *_):
         """list of people mentioned"""
         status = models.Status.objects.create(content="hi", user=self.remote_user)
         result = status_display.get_mentions(status, self.user)
         self.assertEqual(result, "@rat@example.com ")
 
-    def test_related_status(self, _):
+    def test_related_status(self, *_):
         """gets the subclass model for a notification status"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             status = models.Status.objects.create(content="hi", user=self.user)

bookwyrm/tests/views/inbox/test_inbox_announce.py

@@ -51,7 +51,8 @@ class InboxActivities(TestCase):
         models.SiteSettings.objects.create()
 
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
-    def test_boost(self, redis_mock):
+    @patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
+    def test_boost(self, redis_mock, _):
         """boost a status"""
         self.assertEqual(models.Notification.objects.count(), 0)
         activity = {
@@ -81,7 +82,8 @@ class InboxActivities(TestCase):
 
     @responses.activate
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
-    def test_boost_remote_status(self, redis_mock):
+    @patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
+    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(
@@ -153,9 +155,10 @@ class InboxActivities(TestCase):
         views.inbox.activity_task(activity)
         self.assertEqual(models.Boost.objects.count(), 0)
 
-    def test_unboost(self):
+    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+    @patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
+    def test_unboost(self, *_):
         """undo a boost"""
-        with patch("bookwyrm.activitystreams.ActivityStream.add_status"):
         boost = models.Boost.objects.create(
             boosted_status=self.status, user=self.remote_user
         )
@@ -175,11 +178,7 @@ class InboxActivities(TestCase):
                 "published": "Mon, 25 May 2020 19:31:20 GMT",
             },
         }
-        with patch(
-            "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores"
-        ) as redis_mock:
         views.inbox.activity_task(activity)
-            self.assertTrue(redis_mock.called)
         self.assertFalse(models.Boost.objects.exists())
 
     def test_unboost_unknown_boost(self):

bookwyrm/tests/views/test_interaction.py

@@ -8,6 +8,7 @@ from bookwyrm import models, views
 
 
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
+@patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores")
 class InteractionViews(TestCase):
     """viewing and creating statuses"""
 
@@ -40,7 +41,7 @@ class InteractionViews(TestCase):
             parent_work=work,
         )
 
-    def test_favorite(self, _):
+    def test_favorite(self, *_):
         """create and broadcast faving a status"""
         view = views.Favorite.as_view()
         request = self.factory.post("")
@@ -58,7 +59,7 @@ class InteractionViews(TestCase):
         self.assertEqual(notification.user, self.local_user)
         self.assertEqual(notification.related_user, self.remote_user)
 
-    def test_unfavorite(self, _):
+    def test_unfavorite(self, *_):
         """unfav a status"""
         view = views.Unfavorite.as_view()
         request = self.factory.post("")
@@ -75,7 +76,7 @@ class InteractionViews(TestCase):
         self.assertEqual(models.Favorite.objects.count(), 0)
         self.assertEqual(models.Notification.objects.count(), 0)
 
-    def test_boost(self, _):
+    def test_boost(self, *_):
         """boost a status"""
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -97,7 +98,7 @@ class InteractionViews(TestCase):
         self.assertEqual(notification.related_user, self.remote_user)
         self.assertEqual(notification.related_status, status)
 
-    def test_self_boost(self, _):
+    def test_self_boost(self, *_):
         """boost your own status"""
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -121,7 +122,7 @@ class InteractionViews(TestCase):
 
         self.assertFalse(models.Notification.objects.exists())
 
-    def test_boost_unlisted(self, _):
+    def test_boost_unlisted(self, *_):
         """boost a status"""
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -136,7 +137,7 @@ class InteractionViews(TestCase):
         boost = models.Boost.objects.get()
         self.assertEqual(boost.privacy, "unlisted")
 
-    def test_boost_private(self, _):
+    def test_boost_private(self, *_):
         """boost a status"""
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -149,7 +150,7 @@ class InteractionViews(TestCase):
             view(request, status.id)
         self.assertFalse(models.Boost.objects.exists())
 
-    def test_boost_twice(self, _):
+    def test_boost_twice(self, *_):
         """boost a status"""
         view = views.Boost.as_view()
         request = self.factory.post("")
@@ -161,13 +162,17 @@ class InteractionViews(TestCase):
             view(request, status.id)
         self.assertEqual(models.Boost.objects.count(), 1)
 
-    def test_unboost(self, _):
+    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+    def test_unboost(self, *_):
         """undo a boost"""
         view = views.Unboost.as_view()
         request = self.factory.post("")
         request.user = self.remote_user
-        with patch("bookwyrm.activitystreams.ActivityStream.add_status"):
         status = models.Status.objects.create(user=self.local_user, content="hi")
+
+        with patch(
+            "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores"
+        ):
             views.Boost.as_view()(request, status.id)
 
         self.assertEqual(models.Boost.objects.count(), 1)

bw-dev

@@ -84,7 +84,7 @@ case "$CMD" in
         runweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@"
         ;;
     pytest)
-        runweb pytest --no-cov-on-fail "$@"
+        execweb pytest --no-cov-on-fail "$@"
         ;;
     collectstatic)
         runweb python manage.py collectstatic --no-input