From 52adfbf5da882461cb84f81bcbb944cf24b2091e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 14 Jun 2021 14:47:59 -0700 Subject: [PATCH 1/7] Don't show duplicated statuses after boosts --- bookwyrm/activitystreams.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index ff3c55fbd..a49a7ce4d 100644 --- a/bookwyrm/activitystreams.py +++ b/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) From ef71da7ef0c84da32a80d09d0d92948d679dc76d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 14 Jun 2021 16:23:17 -0700 Subject: [PATCH 2/7] Fixes interaction tests --- bookwyrm/tests/views/test_interaction.py | 17 +++++++++++++---- bw-dev | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index 876d6053c..ab1d6066e 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -83,7 +83,10 @@ class InteractionViews(TestCase): with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") - view(request, status.id) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ): + view(request, status.id) boost = models.Boost.objects.get() @@ -157,8 +160,11 @@ class InteractionViews(TestCase): with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") - view(request, status.id) - view(request, status.id) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ): + view(request, status.id) + view(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) def test_unboost(self, _): @@ -168,7 +174,10 @@ class InteractionViews(TestCase): request.user = self.remote_user with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") - views.Boost.as_view()(request, status.id) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ): + views.Boost.as_view()(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) diff --git a/bw-dev b/bw-dev index fb2af0e76..763a2b54e 100755 --- a/bw-dev +++ b/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 From c3a09a63317094eba1f20163d9f93ca8b31e4ed4 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 14 Jun 2021 16:39:54 -0700 Subject: [PATCH 3/7] More test fixes --- bookwyrm/tests/models/test_status_model.py | 1 + bookwyrm/tests/test_templatetags.py | 29 ++++++++++--------- .../tests/views/inbox/test_inbox_announce.py | 23 +++++++-------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 4c8930bc4..0c0436df2 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/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""" diff --git a/bookwyrm/tests/test_templatetags.py b/bookwyrm/tests/test_templatetags.py index 4b1aa5946..eccfb2987 100644 --- a/bookwyrm/tests/test_templatetags.py +++ b/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, "

hi

") @@ -157,13 +158,13 @@ class TemplateTags(TestCase): result = markdown.get_markdown("_hi_") self.assertEqual(result, "

hi

") - 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) diff --git a/bookwyrm/tests/views/inbox/test_inbox_announce.py b/bookwyrm/tests/views/inbox/test_inbox_announce.py index 4243dc78e..fdb84d7c5 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_announce.py +++ b/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,12 +155,13 @@ 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 - ) + boost = models.Boost.objects.create( + boosted_status=self.status, user=self.remote_user + ) activity = { "type": "Undo", "actor": "hi", @@ -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) + views.inbox.activity_task(activity) self.assertFalse(models.Boost.objects.exists()) def test_unboost_unknown_boost(self): From 894633202311ed739f3b36656eb3697558a0ef2d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 14 Jun 2021 16:56:47 -0700 Subject: [PATCH 4/7] More fixes for more tests --- bookwyrm/tests/models/test_status_model.py | 2 +- bookwyrm/tests/views/test_interaction.py | 36 +++++++++------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 0c0436df2..d58a6cb95 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -385,7 +385,7 @@ class Status(TestCase): user=self.local_user, notification_type="GLORB" ) - def test_create_broadcast(self, _, broadcast_mock): + def test_create_broadcast(self, 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 diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index ab1d6066e..272f6b40f 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -8,6 +8,9 @@ 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 +43,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 +61,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 +78,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("") @@ -83,10 +86,7 @@ class InteractionViews(TestCase): 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" - ): - view(request, status.id) + view(request, status.id) boost = models.Boost.objects.get() @@ -100,7 +100,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("") @@ -124,7 +124,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("") @@ -139,7 +139,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("") @@ -152,7 +152,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("") @@ -160,24 +160,18 @@ class InteractionViews(TestCase): 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" - ): - view(request, status.id) - view(request, status.id) + view(request, status.id) + view(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) - def test_unboost(self, _): + 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) + views.Boost.as_view()(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) From 75df21019348a004ba3e231f10657151c1beb91d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 14 Jun 2021 17:01:53 -0700 Subject: [PATCH 5/7] Python formatting --- bookwyrm/tests/views/test_interaction.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index 272f6b40f..cb5e97cbd 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -8,9 +8,7 @@ from bookwyrm import models, views @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") -@patch( - "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" -) +@patch("bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores") class InteractionViews(TestCase): """viewing and creating statuses""" From 8b657bbcb1dee069d3c94ba3575b02b3a9a4b808 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 17 Jun 2021 12:58:37 -0700 Subject: [PATCH 6/7] Updates model test --- bookwyrm/tests/models/test_status_model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index d58a6cb95..355caab9b 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -385,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 From 2a6339cf21ff77b6278487c453a52e96ca2b9280 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 17 Jun 2021 13:10:01 -0700 Subject: [PATCH 7/7] Fixes redis interaction mock --- bookwyrm/tests/views/test_interaction.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index cb5e97cbd..3867f57d5 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -162,13 +162,17 @@ class InteractionViews(TestCase): view(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) + @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") + 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)