Merge pull request #2057 from bookwyrm-social/list-privacy

Re-consider list privacy on edit
This commit is contained in:
Mouse Reeve 2022-07-02 20:07:00 -07:00 committed by GitHub
commit 4ccbfb6b31
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 93 additions and 41 deletions

View file

@ -114,12 +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):
"""add newly created lists streamsstreams"""
if not created:
def add_list_on_create(sender, instance, created, *args, update_fields=None, **kwargs):
"""add newly created lists streams"""
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)
@ -217,7 +225,7 @@ def populate_lists_task(user_id):
@app.task(queue=MEDIUM)
def remove_list_task(list_id):
def remove_list_task(list_id, re_add=False):
"""remove a list from any stream it might be in"""
stores = models.User.objects.filter(local=True, is_active=True).values_list(
"id", flat=True
@ -227,6 +235,9 @@ def remove_list_task(list_id):
stores = [ListsStream().stream_id(idx) for idx in stores]
ListsStream().remove_object_from_related_stores(list_id, stores=stores)
if re_add:
add_list_task.delay(list_id)
@app.task(queue=HIGH)
def add_list_task(list_id):

View file

@ -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)

View file

@ -32,9 +32,10 @@ class ListsStreamSignals(TestCase):
def test_add_list_on_create_command(self, _):
"""a new lists has entered"""
book_list = models.List.objects.create(
user=self.remote_user, name="hi", privacy="public"
)
with patch("bookwyrm.lists_stream.remove_list_task.delay"):
book_list = models.List.objects.create(
user=self.remote_user, name="hi", privacy="public"
)
with patch("bookwyrm.lists_stream.add_list_task.delay") as mock:
lists_stream.add_list_on_create_command(book_list.id)
self.assertEqual(mock.call_count, 1)
@ -43,9 +44,10 @@ class ListsStreamSignals(TestCase):
def test_remove_list_on_delete(self, _):
"""delete a list"""
book_list = models.List.objects.create(
user=self.remote_user, name="hi", privacy="public"
)
with patch("bookwyrm.lists_stream.remove_list_task.delay"):
book_list = models.List.objects.create(
user=self.remote_user, name="hi", privacy="public"
)
with patch("bookwyrm.lists_stream.remove_list_task.delay") as mock:
lists_stream.remove_list_on_delete(models.List, book_list)
args = mock.call_args[0]

View file

@ -11,6 +11,7 @@ from bookwyrm import lists_stream, models
@patch("bookwyrm.activitystreams.add_book_statuses_task.delay")
@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
@patch("bookwyrm.activitystreams.populate_stream_task.delay")
@patch("bookwyrm.lists_stream.remove_list_task.delay")
class ListsStream(TestCase):
"""using redis to build activity streams"""

View file

@ -35,7 +35,9 @@ class Activitystreams(TestCase):
inbox="https://example.com/users/rat/inbox",
outbox="https://example.com/users/rat/outbox",
)
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(
user=self.local_user, name="hi", privacy="public"
)

View file

@ -80,7 +80,9 @@ class Group(TestCase):
"""follower-only group booklists should not be excluded from group booklist
listing for group members who do not follower list owner"""
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"):
followers_list = models.List.objects.create(
name="Followers List",
curation="group",
@ -101,8 +103,9 @@ class Group(TestCase):
"""private group booklists should not be excluded from group booklist listing
for group members"""
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"):
private_list = models.List.objects.create(
name="Private List",
privacy="direct",

View file

@ -23,7 +23,9 @@ class List(TestCase):
def test_remote_id(self, _):
"""shelves use custom remote ids"""
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"):
book_list = models.List.objects.create(
name="Test List", user=self.local_user
)
@ -32,7 +34,9 @@ class List(TestCase):
def test_to_activity(self, _):
"""jsonify it"""
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"):
book_list = models.List.objects.create(
name="Test List", user=self.local_user
)
@ -46,7 +50,9 @@ class List(TestCase):
def test_list_item(self, _):
"""a list entry"""
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"):
book_list = models.List.objects.create(
name="Test List", user=self.local_user, privacy="unlisted"
)
@ -64,7 +70,9 @@ class List(TestCase):
def test_list_item_pending(self, _):
"""a list entry"""
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"):
book_list = models.List.objects.create(
name="Test List", user=self.local_user
)
@ -84,7 +92,9 @@ class List(TestCase):
def test_embed_key(self, _):
"""embed_key should never be empty"""
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"):
book_list = models.List.objects.create(
name="Test List", user=self.local_user
)

View file

@ -75,7 +75,9 @@ class InboxRemove(TestCase):
def test_handle_remove_book_from_list(self):
"""listing a book"""
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="test list",
user=self.local_user,

View file

@ -50,7 +50,9 @@ class InboxUpdate(TestCase):
def test_update_list(self):
"""a new list"""
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"):
book_list = models.List.objects.create(
name="hi", remote_id="https://example.com/list/22", user=self.local_user
)
@ -69,7 +71,8 @@ class InboxUpdate(TestCase):
"curation": "curated",
"@context": "https://www.w3.org/ns/activitystreams",
}
views.inbox.activity_task(activity)
with patch("bookwyrm.lists_stream.remove_list_task.delay"):
views.inbox.activity_task(activity)
book_list.refresh_from_db()
self.assertEqual(book_list.name, "Test List")
self.assertEqual(book_list.curation, "curated")

View file

@ -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
)

View file

@ -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
)

View file

@ -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(
"",
{

View file

@ -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
)

View file

@ -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)

View file

@ -140,7 +140,9 @@ class Views(TestCase):
def test_search_lists(self):
"""searches remote connectors"""
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(
user=self.local_user, name="test list"
)