diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index c40d2b534..0977ad8c2 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -114,15 +114,20 @@ class ListsStream(RedisStore): @receiver(signals.post_save, sender=models.List) # pylint: disable=unused-argument -def add_list_on_create(sender, instance, created, *args, **kwargs): +def add_list_on_create(sender, instance, created, *args, update_fields=None, **kwargs): """add newly created lists streams""" - if not created: - # the privacy may have changed, so we need to re-do the whole thing - remove_list_task.delay(instance.id, re_add=True) + if created: + # when creating new things, gotta wait on the transaction + transaction.on_commit(lambda: add_list_on_create_command(instance.id)) return - # when creating new things, gotta wait on the transaction - transaction.on_commit(lambda: add_list_on_create_command(instance.id)) + # if update_fields was specified, we can check if privacy was updated, but if + # it wasn't specified (ie, by an activitypub update), there's no way to know + if update_fields and "privacy" not in update_fields: + return + + # the privacy may have changed, so we need to re-do the whole thing + remove_list_task.delay(instance.id, re_add=True) @receiver(signals.post_delete, sender=models.List) diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py index ea524cc54..0195020e0 100644 --- a/bookwyrm/models/list.py +++ b/bookwyrm/models/list.py @@ -129,7 +129,7 @@ class List(OrderedCollectionMixin, BookWyrmModel): """on save, update embed_key and avoid clash with existing code""" if not self.embed_key: self.embed_key = uuid.uuid4() - return super().save(*args, **kwargs) + super().save(*args, **kwargs) class ListItem(CollectionItemMixin, BookWyrmModel): @@ -156,7 +156,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel): super().save(*args, **kwargs) # tick the updated date on the parent list self.book_list.updated_date = timezone.now() - self.book_list.save(broadcast=False) + self.book_list.save(broadcast=False, update_fields=["updated_date"]) list_owner = self.book_list.user model = apps.get_model("bookwyrm.Notification", require_ready=True) diff --git a/bookwyrm/tests/views/lists/test_curate.py b/bookwyrm/tests/views/lists/test_curate.py index 9be8c2a15..9f3427b2c 100644 --- a/bookwyrm/tests/views/lists/test_curate.py +++ b/bookwyrm/tests/views/lists/test_curate.py @@ -36,7 +36,9 @@ class ListViews(TestCase): parent_work=work, ) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): self.list = models.List.objects.create( name="Test List", user=self.local_user ) diff --git a/bookwyrm/tests/views/lists/test_embed.py b/bookwyrm/tests/views/lists/test_embed.py index dc61736d3..4191ffe0d 100644 --- a/bookwyrm/tests/views/lists/test_embed.py +++ b/bookwyrm/tests/views/lists/test_embed.py @@ -36,7 +36,9 @@ class ListViews(TestCase): parent_work=work, ) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): self.list = models.List.objects.create( name="Test List", user=self.local_user ) diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py index bcec0822f..98b0a461a 100644 --- a/bookwyrm/tests/views/lists/test_list.py +++ b/bookwyrm/tests/views/lists/test_list.py @@ -65,7 +65,9 @@ class ListViews(TestCase): parent_work=work_four, ) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): self.list = models.List.objects.create( name="Test List", user=self.local_user ) @@ -244,7 +246,7 @@ class ListViews(TestCase): with patch( "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" - ) as mock: + ) as mock, patch("bookwyrm.lists_stream.remove_list_task.delay"): result = view(request, self.list.id) self.assertEqual(mock.call_count, 1) @@ -596,7 +598,7 @@ class ListViews(TestCase): def test_add_book_outsider(self): """put a book on a list""" self.list.curation = "open" - self.list.save(broadcast=False) + self.list.save(broadcast=False, update_fields=["curation"]) request = self.factory.post( "", { @@ -625,7 +627,7 @@ class ListViews(TestCase): def test_add_book_pending(self): """put a book on a list awaiting approval""" self.list.curation = "curated" - self.list.save(broadcast=False) + self.list.save(broadcast=False, update_fields=["curation"]) request = self.factory.post( "", { @@ -658,7 +660,7 @@ class ListViews(TestCase): def test_add_book_self_curated(self): """put a book on a list automatically approved""" self.list.curation = "curated" - self.list.save(broadcast=False) + self.list.save(broadcast=False, update_fields=["curation"]) request = self.factory.post( "", { @@ -687,7 +689,7 @@ class ListViews(TestCase): def test_add_book_permission_denied(self): """you can't add to that list""" self.list.curation = "closed" - self.list.save(broadcast=False) + self.list.save(broadcast=False, update_fields=["curation"]) request = self.factory.post( "", { diff --git a/bookwyrm/tests/views/lists/test_list_item.py b/bookwyrm/tests/views/lists/test_list_item.py index 50be3c286..b95282bef 100644 --- a/bookwyrm/tests/views/lists/test_list_item.py +++ b/bookwyrm/tests/views/lists/test_list_item.py @@ -32,7 +32,9 @@ class ListItemViews(TestCase): remote_id="https://example.com/book/1", parent_work=work, ) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): self.list = models.List.objects.create( name="Test List", user=self.local_user ) diff --git a/bookwyrm/tests/views/lists/test_lists.py b/bookwyrm/tests/views/lists/test_lists.py index f65d2e4c2..c2263b933 100644 --- a/bookwyrm/tests/views/lists/test_lists.py +++ b/bookwyrm/tests/views/lists/test_lists.py @@ -39,7 +39,9 @@ class ListViews(TestCase): view = views.Lists.as_view() with patch( "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" - ), patch("bookwyrm.lists_stream.add_list_task.delay"): + ), patch("bookwyrm.lists_stream.add_list_task.delay"), patch( + "bookwyrm.lists_stream.remove_list_task.delay" + ): models.List.objects.create(name="Public list", user=self.local_user) models.List.objects.create( name="Private list", privacy="direct", user=self.local_user @@ -62,7 +64,9 @@ class ListViews(TestCase): def test_saved_lists_page(self): """there are so many views, this just makes sure it LOADS""" view = views.SavedLists.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): booklist = models.List.objects.create( name="Public list", user=self.local_user ) @@ -82,7 +86,9 @@ class ListViews(TestCase): def test_saved_lists_page_empty(self): """there are so many views, this just makes sure it LOADS""" view = views.SavedLists.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): models.List.objects.create(name="Public list", user=self.local_user) models.List.objects.create( name="Private list", privacy="direct", user=self.local_user @@ -108,7 +114,9 @@ class ListViews(TestCase): def test_user_lists_page(self): """there are so many views, this just makes sure it LOADS""" view = views.UserLists.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.lists_stream.remove_list_task.delay"): models.List.objects.create(name="Public list", user=self.local_user) models.List.objects.create( name="Private list", privacy="direct", user=self.local_user @@ -146,7 +154,7 @@ class ListViews(TestCase): request.user = self.local_user with patch( "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" - ) as mock: + ) as mock, patch("bookwyrm.lists_stream.remove_list_task.delay"): result = view(request) self.assertEqual(mock.call_count, 1)