From fc520fdbdcabb7518ca20802d7fbe2e85d4f5a5b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Nov 2021 12:39:39 -0800 Subject: [PATCH 01/38] Adds quick first pass on lists stream manager --- bookwyrm/lists_stream.py | 238 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 bookwyrm/lists_stream.py diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py new file mode 100644 index 00000000..82cef5e4 --- /dev/null +++ b/bookwyrm/lists_stream.py @@ -0,0 +1,238 @@ +""" access the activity streams stored in redis """ +from django.dispatch import receiver +from django.db import transaction +from django.db.models import signals, Q + +from bookwyrm import models +from bookwyrm.redis_store import RedisStore +from bookwyrm.tasks import app, LOW, MEDIUM, HIGH + + +class ListsStream(RedisStore): + """a category of activity stream (like home, local, books)""" + + def stream_id(self, user): # pylint: disable=no-self-use + """the redis key for this user's instance of this stream""" + return f"{user.id}-lists" + + def get_rank(self, obj): # pylint: disable=no-self-use + """lists are sorted by date published""" + return obj.updated_date.timestamp() + + def add_list(self, book_list): + """add a list to users' feeds""" + # the pipeline contains all the add-to-stream activities + self.add_object_to_related_stores(book_list) + + def add_user_lists(self, viewer, user): + """add a user's listes to another user's feed""" + # only add the listes that the viewer should be able to see + lists = models.List.filter(user=user).privacy_filter(viewer) + self.bulk_add_objects_to_store(lists, self.stream_id(viewer)) + + def remove_user_lists(self, viewer, user): + """remove a user's list from another user's feed""" + # remove all so that followers only lists are removed + lists = user.list_set.all() + self.bulk_remove_objects_from_store(lists, self.stream_id(viewer)) + + def get_activity_stream(self, user): + """load the lists to be displayed""" + lists = self.get_store(self.stream_id(user)) + return ( + models.List.objects.filter(id__in=lists) + .select_related( + "user", + ) + .prefetch_related("listitem_set") + .order_by("-updated_date") + ) + + def populate_streams(self, user): + """go from zero to a timeline""" + self.populate_store(self.stream_id(user)) + + def get_audience(self, book_list): # pylint: disable=no-self-use + """given a list, what users should see it""" + # everybody who could plausibly see this list + audience = models.User.objects.filter( + is_active=True, + local=True, # we only create feeds for users of this instance + ).exclude( # not blocked + Q(id__in=book_list.user.blocks.all()) | Q(blocks=book_list.user) + ) + + # TODO: groups + + # only visible to the poster and mentioned users + if book_list.privacy == "direct": + audience = audience.filter( + Q(id=list.user.id) # if the user is the post's author + ) + # only visible to the poster's followers and tagged users + elif book_list.privacy == "followers": + audience = audience.filter( + Q(id=book_list.user.id) # if the user is the post's author + | Q(following=book_list.user) # if the user is following the author + ) + return audience.distinct() + + def get_stores_for_object(self, obj): + return [self.stream_id(u) for u in self.get_audience(obj)] + + def get_lists_for_user(self, user): # pylint: disable=no-self-use + """given a user, what lists should they see on this stream""" + return models.List.privacy_filter( + user, + privacy_levels=["public", "followers"], + ) + + def get_objects_for_store(self, store): + user = models.User.objects.get(id=store.split("-")[0]) + return self.get_lists_for_user(user) + + +@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 to activity feeds""" + # we're only interested in new lists + if not issubclass(sender, models.List): + return + + if instance.deleted: + remove_list_task.delay(instance.id) + return + + # when creating new things, gotta wait on the transaction + transaction.on_commit(lambda: add_list_on_create_command(instance, created)) + + +def add_list_on_create_command(instance, created): + """runs this code only after the database commit completes""" + priority = HIGH + # check if this is an old list, de-prioritize if so + # (this will happen if federation is very slow, or, more expectedly, on csv import) + one_day = 60 * 60 * 24 + if (instance.created_date - instance.published_date).seconds > one_day: + priority = LOW + + add_list_task.apply_async( + args=(instance.id,), + kwargs={"increment_unread": created}, + queue=priority, + ) + + +@receiver(signals.post_save, sender=models.UserFollows) +# pylint: disable=unused-argument +def add_lists_on_follow(sender, instance, created, *args, **kwargs): + """add a newly followed user's lists to feeds""" + if not created or not instance.user_subject.local: + return + add_user_lists_task.delay( + instance.user_subject.id, instance.user_object.id, stream_list=["home"] + ) + + +@receiver(signals.post_delete, sender=models.UserFollows) +# pylint: disable=unused-argument +def remove_lists_on_unfollow(sender, instance, *args, **kwargs): + """remove lists from a feed on unfollow""" + if not instance.user_subject.local: + return + remove_user_lists_task.delay( + instance.user_subject.id, instance.user_object.id, stream_list=["home"] + ) + + +@receiver(signals.post_save, sender=models.UserBlocks) +# pylint: disable=unused-argument +def remove_lists_on_block(sender, instance, *args, **kwargs): + """remove lists from all feeds on block""" + # blocks apply ot all feeds + if instance.user_subject.local: + remove_user_lists_task.delay(instance.user_subject.id, instance.user_object.id) + + # and in both directions + if instance.user_object.local: + remove_user_lists_task.delay(instance.user_object.id, instance.user_subject.id) + + +@receiver(signals.post_delete, sender=models.UserBlocks) +# pylint: disable=unused-argument +def add_lists_on_unblock(sender, instance, *args, **kwargs): + """add lists back to all feeds on unblock""" + # make sure there isn't a block in the other direction + if models.UserBlocks.objects.filter( + user_subject=instance.user_object, + user_object=instance.user_subject, + ).exists(): + return + + # add lists back to streams with lists from anyone + if instance.user_subject.local: + add_user_lists_task.delay( + instance.user_subject.id, + instance.user_object.id, + ) + + # add lists back to streams with lists from anyone + if instance.user_object.local: + add_user_lists_task.delay( + instance.user_object.id, + instance.user_subject.id, + ) + + +@receiver(signals.post_save, sender=models.User) +# pylint: disable=unused-argument +def populate_streams_on_account_create(sender, instance, created, *args, **kwargs): + """build a user's feeds when they join""" + if not created or not instance.local: + return + + populate_lists_stream_task.delay(instance.id) + + +# ---- TASKS +@app.task(queue=MEDIUM) +def populate_lists_stream_task(user_id): + """background task for populating an empty activitystream""" + user = models.User.objects.get(id=user_id) + ListsStream().populate_streams(user) + + +@app.task(queue=MEDIUM) +def remove_list_task(list_ids): + """remove a list from any stream it might be in""" + # this can take an id or a list of ids + if not isinstance(list_ids, list): + list_ids = [list_ids] + lists = models.List.objects.filter(id__in=list_ids) + + for book_list in lists: + ListsStream().remove_object_from_related_stores(book_list) + + +@app.task(queue=HIGH) +def add_list_task(list_id): + """add a list to any stream it should be in""" + book_list = models.List.objects.get(id=list_id) + ListsStream().add_list(book_list) + + +@app.task(queue=MEDIUM) +def remove_user_lists_task(viewer_id, user_id): + """remove all lists by a user from a viewer's stream""" + viewer = models.User.objects.get(id=viewer_id) + user = models.User.objects.get(id=user_id) + ListsStream().remove_user_lists(viewer, user) + + +@app.task(queue=MEDIUM) +def add_user_lists_task(viewer_id, user_id): + """add all lists by a user to a viewer's stream""" + viewer = models.User.objects.get(id=viewer_id) + user = models.User.objects.get(id=user_id) + ListsStream().add_user_lists(viewer, user) From 1d28c7e73dff221597fe8083e8aecc2ac408cb66 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Nov 2021 09:21:12 -0800 Subject: [PATCH 02/38] Load lists from redis cache --- bookwyrm/lists_stream.py | 10 ++++++---- bookwyrm/views/list.py | 16 +++------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 82cef5e4..1aef95bd 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -1,7 +1,7 @@ """ access the activity streams stored in redis """ from django.dispatch import receiver from django.db import transaction -from django.db.models import signals, Q +from django.db.models import signals, Count, Q from bookwyrm import models from bookwyrm.redis_store import RedisStore @@ -41,11 +41,13 @@ class ListsStream(RedisStore): lists = self.get_store(self.stream_id(user)) return ( models.List.objects.filter(id__in=lists) - .select_related( - "user", - ) + .annotate(item_count=Count("listitem", filter=Q(listitem__approved=True))) + # hide lists with no approved books + .filter(item_count__gt=0) + .select_related("user") .prefetch_related("listitem_set") .order_by("-updated_date") + .distinct() ) def populate_streams(self, user): diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 97eaf9d6..1049f070 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -5,7 +5,7 @@ from urllib.parse import urlencode from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator from django.db import IntegrityError, transaction -from django.db.models import Avg, Count, DecimalField, Q, Max +from django.db.models import Avg, DecimalField, Q, Max from django.db.models.functions import Coalesce from django.http import HttpResponseBadRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect @@ -17,6 +17,7 @@ from django.views.decorators.http import require_POST from bookwyrm import book_search, forms, models from bookwyrm.activitypub import ActivitypubResponse +from bookwyrm.lists_stream import ListsStream from bookwyrm.settings import PAGE_LENGTH from .helpers import is_api_request from .helpers import get_user_from_username @@ -28,18 +29,7 @@ class Lists(View): def get(self, request): """display a book list""" - # hide lists with no approved books - lists = ( - models.List.privacy_filter( - request.user, privacy_levels=["public", "followers"] - ) - .annotate(item_count=Count("listitem", filter=Q(listitem__approved=True))) - .filter(item_count__gt=0) - .select_related("user") - .prefetch_related("listitem_set") - .order_by("-updated_date") - .distinct() - ) + lists = ListsStream().get_activity_stream(request.user) paginated = Paginator(lists, 12) data = { "lists": paginated.get_page(request.GET.get("page")), From 903aaaf4c4d7664246001e1102e3a70c048a6e0c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Nov 2021 10:41:08 -0800 Subject: [PATCH 03/38] Adds management and bw-dev commands --- .../commands/populate_lists_streams.py | 34 +++++++++++++++++++ .../management/commands/populate_streams.py | 6 ++-- bw-dev | 3 ++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 bookwyrm/management/commands/populate_lists_streams.py diff --git a/bookwyrm/management/commands/populate_lists_streams.py b/bookwyrm/management/commands/populate_lists_streams.py new file mode 100644 index 00000000..1cf488df --- /dev/null +++ b/bookwyrm/management/commands/populate_lists_streams.py @@ -0,0 +1,34 @@ +""" Re-create user streams """ +from django.core.management.base import BaseCommand +from bookwyrm import lists_stream, models + + +def populate_lists_streams(): + """build all the lists streams for all the users""" + print("Populating lists streams") + users = models.User.objects.filter( + local=True, + is_active=True, + ).order_by("-last_active_date") + print("This may take a long time! Please be patient.") + for user in users: + print(".", end="") + lists_stream.populate_lists_stream_task.delay(user.id) + + +class Command(BaseCommand): + """start all over with user streams""" + + help = "Populate streams for all users" + + def add_arguments(self, parser): + parser.add_argument( + "--stream", + default=None, + help="Specifies which time of stream to populate", + ) + + # pylint: disable=no-self-use,unused-argument + def handle(self, *args, **options): + """run feed builder""" + populate_lists_streams() diff --git a/bookwyrm/management/commands/populate_streams.py b/bookwyrm/management/commands/populate_streams.py index a04d7f5a..6b537650 100644 --- a/bookwyrm/management/commands/populate_streams.py +++ b/bookwyrm/management/commands/populate_streams.py @@ -1,18 +1,20 @@ """ Re-create user streams """ from django.core.management.base import BaseCommand -from bookwyrm import activitystreams, models +from bookwyrm import activitystreams, lists_stream, models def populate_streams(stream=None): """build all the streams for all the users""" streams = [stream] if stream else activitystreams.streams.keys() - print("Populations streams", streams) + print("Populating streams", streams) users = models.User.objects.filter( local=True, is_active=True, ).order_by("-last_active_date") print("This may take a long time! Please be patient.") for user in users: + print(".", end="") + lists_stream.populate_lists_stream_task.delay(user.id) for stream_key in streams: print(".", end="") activitystreams.populate_stream_task.delay(stream_key, user.id) diff --git a/bw-dev b/bw-dev index 35f3694e..c5ae7c9e 100755 --- a/bw-dev +++ b/bw-dev @@ -120,6 +120,9 @@ case "$CMD" in populate_streams) runweb python manage.py populate_streams $@ ;; + populate_lists_streams) + runweb python manage.py populate_lists_streams $@ + ;; populate_suggestions) runweb python manage.py populate_suggestions ;; From 4cb572f4c7f78c6a8288b72cbb3b310905e72033 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Nov 2021 11:17:49 -0800 Subject: [PATCH 04/38] Updates management tests --- .../management/test_populate_lists_streams.py | 54 +++++++++++++++++++ .../tests/management/test_populate_streams.py | 15 +++++- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 bookwyrm/tests/management/test_populate_lists_streams.py diff --git a/bookwyrm/tests/management/test_populate_lists_streams.py b/bookwyrm/tests/management/test_populate_lists_streams.py new file mode 100644 index 00000000..0a586d86 --- /dev/null +++ b/bookwyrm/tests/management/test_populate_lists_streams.py @@ -0,0 +1,54 @@ +""" test populating user streams """ +from unittest.mock import patch +from django.test import TestCase + +from bookwyrm import models +from bookwyrm.management.commands.populate_lists_streams import populate_lists_streams + + +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") +class Activitystreams(TestCase): + """using redis to build activity streams""" + + def setUp(self): + """we need some stuff""" + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" + ) + self.another_user = models.User.objects.create_user( + "nutria", + "nutria@nutria.nutria", + "password", + local=True, + localname="nutria", + ) + models.User.objects.create_user( + "gerbil", + "gerbil@nutria.nutria", + "password", + local=True, + localname="gerbil", + is_active=False, + ) + with patch("bookwyrm.models.user.set_remote_server.delay"): + self.remote_user = models.User.objects.create_user( + "rat", + "rat@rat.com", + "ratword", + local=False, + remote_id="https://example.com/users/rat", + inbox="https://example.com/users/rat/inbox", + outbox="https://example.com/users/rat/outbox", + ) + self.book = models.Edition.objects.create(title="test book") + + def test_populate_streams(self, _): + """make sure the function on the redis manager gets called""" + with patch( + "bookwyrm.lists_stream.populate_lists_stream_task.delay" + ) as list_mock: + populate_lists_streams() + self.assertEqual(list_mock.call_count, 2) # 2 users diff --git a/bookwyrm/tests/management/test_populate_streams.py b/bookwyrm/tests/management/test_populate_streams.py index 5be1774d..cee8b381 100644 --- a/bookwyrm/tests/management/test_populate_streams.py +++ b/bookwyrm/tests/management/test_populate_streams.py @@ -25,6 +25,14 @@ class Activitystreams(TestCase): local=True, localname="nutria", ) + models.User.objects.create_user( + "gerbil", + "gerbil@gerbil.gerbil", + "password", + local=True, + localname="gerbil", + is_active=False, + ) with patch("bookwyrm.models.user.set_remote_server.delay"): self.remote_user = models.User.objects.create_user( "rat", @@ -44,6 +52,11 @@ class Activitystreams(TestCase): user=self.local_user, content="hi", book=self.book ) - with patch("bookwyrm.activitystreams.populate_stream_task.delay") as redis_mock: + with patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ) as redis_mock, patch( + "bookwyrm.lists_stream.populate_lists_stream_task.delay" + ) as list_mock: populate_streams() self.assertEqual(redis_mock.call_count, 6) # 2 users x 3 streams + self.assertEqual(list_mock.call_count, 2) # 2 users From 157d8916813d8718b9ad5c31a35ac2678029e926 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Nov 2021 09:10:28 -0800 Subject: [PATCH 05/38] Adds tests files --- bookwyrm/lists_stream.py | 17 +- bookwyrm/tests/lists_stream/__init__.py | 1 + bookwyrm/tests/lists_stream/test_signals.py | 118 +++++++++++ bookwyrm/tests/lists_stream/test_stream.py | 113 ++++++++++ bookwyrm/tests/lists_stream/test_tasks.py | 218 ++++++++++++++++++++ 5 files changed, 458 insertions(+), 9 deletions(-) create mode 100644 bookwyrm/tests/lists_stream/__init__.py create mode 100644 bookwyrm/tests/lists_stream/test_signals.py create mode 100644 bookwyrm/tests/lists_stream/test_stream.py create mode 100644 bookwyrm/tests/lists_stream/test_tasks.py diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 1aef95bd..ec6726a9 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -69,7 +69,7 @@ class ListsStream(RedisStore): # only visible to the poster and mentioned users if book_list.privacy == "direct": audience = audience.filter( - Q(id=list.user.id) # if the user is the post's author + Q(id=book_list.user.id) # if the user is the post's author ) # only visible to the poster's followers and tagged users elif book_list.privacy == "followers": @@ -98,18 +98,17 @@ class ListsStream(RedisStore): # pylint: disable=unused-argument def add_list_on_create(sender, instance, created, *args, **kwargs): """add newly created lists to activity feeds""" - # we're only interested in new lists - if not issubclass(sender, models.List): - return - - if instance.deleted: - remove_list_task.delay(instance.id) - return - # when creating new things, gotta wait on the transaction transaction.on_commit(lambda: add_list_on_create_command(instance, created)) +@receiver(signals.pre_delete, sender=models.List) +# pylint: disable=unused-argument +def remove_list_on_delete(sender, instance, created, *args, **kwargs): + """add newly created lists to activity feeds""" + remove_list_task.delay(instance.id) + + def add_list_on_create_command(instance, created): """runs this code only after the database commit completes""" priority = HIGH diff --git a/bookwyrm/tests/lists_stream/__init__.py b/bookwyrm/tests/lists_stream/__init__.py new file mode 100644 index 00000000..b6e690fd --- /dev/null +++ b/bookwyrm/tests/lists_stream/__init__.py @@ -0,0 +1 @@ +from . import * diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py new file mode 100644 index 00000000..34aeb947 --- /dev/null +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -0,0 +1,118 @@ +""" testing activitystreams """ +from unittest.mock import patch +from django.test import TestCase +from bookwyrm import activitystreams, models + + +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") +class ActivitystreamsSignals(TestCase): + """using redis to build activity streams""" + + def setUp(self): + """use a test csv""" + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" + ) + self.another_user = models.User.objects.create_user( + "fish", "fish@fish.fish", "password", local=True, localname="fish" + ) + with patch("bookwyrm.models.user.set_remote_server.delay"): + self.remote_user = models.User.objects.create_user( + "rat", + "rat@rat.com", + "ratword", + local=False, + remote_id="https://example.com/users/rat", + inbox="https://example.com/users/rat/inbox", + outbox="https://example.com/users/rat/outbox", + ) + work = models.Work.objects.create(title="test work") + self.book = models.Edition.objects.create(title="test book", parent_work=work) + + def test_add_status_on_create_ignore(self, _): + """a new statuses has entered""" + activitystreams.add_status_on_create(models.User, self.local_user, False) + + def test_add_status_on_create_deleted(self, _): + """a new statuses has entered""" + with patch("bookwyrm.activitystreams.remove_status_task.delay"): + status = models.Status.objects.create( + user=self.remote_user, content="hi", privacy="public", deleted=True + ) + with patch("bookwyrm.activitystreams.remove_status_task.delay") as mock: + activitystreams.add_status_on_create(models.Status, status, False) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[0] + self.assertEqual(args[0], status.id) + + def test_add_status_on_create_created(self, _): + """a new statuses has entered""" + status = models.Status.objects.create( + user=self.remote_user, content="hi", privacy="public" + ) + with patch("bookwyrm.activitystreams.add_status_task.apply_async") as mock: + activitystreams.add_status_on_create_command(models.Status, status, False) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[1] + self.assertEqual(args["args"][0], status.id) + self.assertEqual(args["queue"], "high_priority") + + def test_populate_streams_on_account_create(self, _): + """create streams for a user""" + with patch("bookwyrm.activitystreams.populate_stream_task.delay") as mock: + activitystreams.populate_streams_on_account_create( + models.User, self.local_user, True + ) + self.assertEqual(mock.call_count, 3) + args = mock.call_args[0] + self.assertEqual(args[0], "books") + self.assertEqual(args[1], self.local_user.id) + + def test_remove_statuses_on_block(self, _): + """don't show statuses from blocked users""" + with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") as mock: + models.UserBlocks.objects.create( + user_subject=self.local_user, + user_object=self.remote_user, + ) + + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user.id) + self.assertEqual(args[1], self.remote_user.id) + + def test_add_statuses_on_unblock(self, _): + """re-add statuses on unblock""" + with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): + block = models.UserBlocks.objects.create( + user_subject=self.local_user, + user_object=self.remote_user, + ) + + with patch("bookwyrm.activitystreams.add_user_statuses_task.delay") as mock: + block.delete() + + args = mock.call_args[0] + kwargs = mock.call_args.kwargs + self.assertEqual(args[0], self.local_user.id) + self.assertEqual(args[1], self.remote_user.id) + self.assertEqual(kwargs["stream_list"], ["local", "books"]) + + def test_add_statuses_on_unblock_reciprocal_block(self, _): + """re-add statuses on unblock""" + with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): + block = models.UserBlocks.objects.create( + user_subject=self.local_user, + user_object=self.remote_user, + ) + block = models.UserBlocks.objects.create( + user_subject=self.remote_user, + user_object=self.local_user, + ) + + with patch("bookwyrm.activitystreams.add_user_statuses_task.delay") as mock: + block.delete() + + self.assertEqual(mock.call_count, 0) diff --git a/bookwyrm/tests/lists_stream/test_stream.py b/bookwyrm/tests/lists_stream/test_stream.py new file mode 100644 index 00000000..25241793 --- /dev/null +++ b/bookwyrm/tests/lists_stream/test_stream.py @@ -0,0 +1,113 @@ +""" testing activitystreams """ +from unittest.mock import patch +from django.test import TestCase +from bookwyrm import lists_stream, models + + +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") +@patch("bookwyrm.activitystreams.add_status_task.delay") +@patch("bookwyrm.activitystreams.add_book_statuses_task.delay") +@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") +@patch("bookwyrm.activitystreams.populate_stream_task.delay") +class ListsStream(TestCase): + """using redis to build activity streams""" + + def setUp(self): + """use a test csv""" + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" + ) + self.another_user = models.User.objects.create_user( + "nutria", + "nutria@nutria.nutria", + "password", + local=True, + localname="nutria", + ) + with patch("bookwyrm.models.user.set_remote_server.delay"): + self.remote_user = models.User.objects.create_user( + "rat", + "rat@rat.com", + "ratword", + local=False, + remote_id="https://example.com/users/rat", + inbox="https://example.com/users/rat/inbox", + outbox="https://example.com/users/rat/outbox", + ) + self.stream = lists_stream.ListsStream() + + def test_lists_stream_ids(self, *_): + """the abstract base class for stream objects""" + self.assertEqual( + self.stream.stream_id(self.local_user), + f"{self.local_user.id}-lists", + ) + + def test_get_audience(self, *_): + """get a list of users that should see a list""" + book_list = models.List.objects.create( + user=self.remote_user, name="hi", privacy="public" + ) + users = self.stream.get_audience(book_list) + # remote users don't have feeds + self.assertFalse(self.remote_user in users) + self.assertTrue(self.local_user in users) + self.assertTrue(self.another_user in users) + + def test_get_audience_direct(self, *_): + """get a list of users that should see a list""" + book_list = models.List.objects.create( + user=self.remote_user, + name="hi", + privacy="direct", + ) + users = self.stream.get_audience(book_list) + self.assertFalse(users.exists()) + + book_list = models.List.objects.create( + user=self.local_user, + name="hi", + privacy="direct", + ) + users = self.stream.get_audience(book_list) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + def test_get_audience_followers_remote_user(self, *_): + """get a list of users that should see a list""" + book_list = models.List.objects.create( + user=self.remote_user, + name="hi", + privacy="followers", + ) + users = self.stream.get_audience(book_list) + self.assertFalse(users.exists()) + + def test_get_audience_followers_self(self, *_): + """get a list of users that should see a list""" + book_list = models.List.objects.create( + user=self.local_user, + name="hi", + privacy="direct", + ) + users = self.stream.get_audience(book_list) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + def test_get_audience_followers_with_relationship(self, *_): + """get a list of users that should see a list""" + self.remote_user.followers.add(self.local_user) + book_list = models.List.objects.create( + user=self.remote_user, + name="hi", + privacy="direct", + ) + users = self.stream.get_audience(book_list) + self.assertFalse(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py new file mode 100644 index 00000000..f5750763 --- /dev/null +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -0,0 +1,218 @@ +""" testing activitystreams """ +from unittest.mock import patch +from django.test import TestCase +from bookwyrm import activitystreams, models + + +class Activitystreams(TestCase): + """using redis to build activity streams""" + + def setUp(self): + """use a test csv""" + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" + ) + self.another_user = models.User.objects.create_user( + "nutria", + "nutria@nutria.nutria", + "password", + local=True, + localname="nutria", + ) + with patch("bookwyrm.models.user.set_remote_server.delay"): + self.remote_user = models.User.objects.create_user( + "rat", + "rat@rat.com", + "ratword", + local=False, + remote_id="https://example.com/users/rat", + inbox="https://example.com/users/rat/inbox", + outbox="https://example.com/users/rat/outbox", + ) + work = models.Work.objects.create(title="test work") + self.book = models.Edition.objects.create(title="test book", parent_work=work) + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + self.status = models.Status.objects.create( + content="hi", user=self.local_user + ) + + def test_add_book_statuses_task(self): + """statuses related to a book""" + with patch("bookwyrm.activitystreams.BooksStream.add_book_statuses") as mock: + activitystreams.add_book_statuses_task(self.local_user.id, self.book.id) + self.assertTrue(mock.called) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.book) + + def test_remove_book_statuses_task(self): + """remove stauses related to a book""" + with patch("bookwyrm.activitystreams.BooksStream.remove_book_statuses") as mock: + activitystreams.remove_book_statuses_task(self.local_user.id, self.book.id) + self.assertTrue(mock.called) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.book) + + def test_populate_stream_task(self): + """populate a given stream""" + with patch("bookwyrm.activitystreams.BooksStream.populate_streams") as mock: + activitystreams.populate_stream_task("books", self.local_user.id) + self.assertTrue(mock.called) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + + with patch("bookwyrm.activitystreams.HomeStream.populate_streams") as mock: + activitystreams.populate_stream_task("home", self.local_user.id) + self.assertTrue(mock.called) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + + def test_remove_status_task(self): + """remove a status from all streams""" + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ) as mock: + activitystreams.remove_status_task(self.status.id) + self.assertEqual(mock.call_count, 3) + args = mock.call_args[0] + self.assertEqual(args[0], self.status) + + def test_add_status_task(self): + """add a status to all streams""" + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as mock: + activitystreams.add_status_task(self.status.id) + self.assertEqual(mock.call_count, 3) + args = mock.call_args[0] + self.assertEqual(args[0], self.status) + + def test_remove_user_statuses_task(self): + """remove all statuses by a user from another users' feeds""" + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_user_statuses" + ) as mock: + activitystreams.remove_user_statuses_task( + self.local_user.id, self.another_user.id + ) + self.assertEqual(mock.call_count, 3) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) + + with patch("bookwyrm.activitystreams.HomeStream.remove_user_statuses") as mock: + activitystreams.remove_user_statuses_task( + self.local_user.id, self.another_user.id, stream_list=["home"] + ) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) + + def test_add_user_statuses_task(self): + """add a user's statuses to another users feeds""" + with patch("bookwyrm.activitystreams.ActivityStream.add_user_statuses") as mock: + activitystreams.add_user_statuses_task( + self.local_user.id, self.another_user.id + ) + self.assertEqual(mock.call_count, 3) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) + + with patch("bookwyrm.activitystreams.HomeStream.add_user_statuses") as mock: + activitystreams.add_user_statuses_task( + self.local_user.id, self.another_user.id, stream_list=["home"] + ) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + def test_boost_to_another_timeline(self, *_): + """boost from a non-follower doesn't remove original status from feed""" + status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.handle_boost_task.delay"): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.handle_boost_task(boost.id) + + self.assertTrue(mock.called) + self.assertEqual(mock.call_count, 1) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertEqual(call_args[1]["stores"], [f"{self.another_user.id}-home"]) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + def test_boost_to_another_timeline_remote(self, *_): + """boost from a remote non-follower doesn't remove original status from feed""" + status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.handle_boost_task.delay"): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.remote_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.handle_boost_task(boost.id) + + self.assertTrue(mock.called) + self.assertEqual(mock.call_count, 1) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertEqual(call_args[1]["stores"], []) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + def test_boost_to_following_timeline(self, *_): + """add a boost and deduplicate the boosted status on the timeline""" + self.local_user.following.add(self.another_user) + status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.handle_boost_task.delay"): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.handle_boost_task(boost.id) + self.assertTrue(mock.called) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertTrue(f"{self.another_user.id}-home" in call_args[1]["stores"]) + self.assertTrue(f"{self.local_user.id}-home" in call_args[1]["stores"]) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + def test_boost_to_same_timeline(self, *_): + """add a boost and deduplicate the boosted status on the timeline""" + status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.handle_boost_task.delay"): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.local_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.handle_boost_task(boost.id) + self.assertTrue(mock.called) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertEqual(call_args[1]["stores"], [f"{self.local_user.id}-home"]) From 3bf1121fa64f6b854f0a0a22ed346a12ffa1c154 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Nov 2021 09:38:02 -0800 Subject: [PATCH 06/38] Signals tests --- bookwyrm/lists_stream.py | 29 +++---- bookwyrm/tests/lists_stream/test_signals.py | 84 +++++++++------------ 2 files changed, 46 insertions(+), 67 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index ec6726a9..04a0725c 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -5,7 +5,7 @@ from django.db.models import signals, Count, Q from bookwyrm import models from bookwyrm.redis_store import RedisStore -from bookwyrm.tasks import app, LOW, MEDIUM, HIGH +from bookwyrm.tasks import app, MEDIUM, HIGH class ListsStream(RedisStore): @@ -98,31 +98,22 @@ class ListsStream(RedisStore): # pylint: disable=unused-argument def add_list_on_create(sender, instance, created, *args, **kwargs): """add newly created lists to activity feeds""" + if not created: + return # when creating new things, gotta wait on the transaction - transaction.on_commit(lambda: add_list_on_create_command(instance, created)) + transaction.on_commit(lambda: add_list_on_create_command(instance)) @receiver(signals.pre_delete, sender=models.List) # pylint: disable=unused-argument -def remove_list_on_delete(sender, instance, created, *args, **kwargs): +def remove_list_on_delete(sender, instance, *args, **kwargs): """add newly created lists to activity feeds""" remove_list_task.delay(instance.id) -def add_list_on_create_command(instance, created): +def add_list_on_create_command(instance): """runs this code only after the database commit completes""" - priority = HIGH - # check if this is an old list, de-prioritize if so - # (this will happen if federation is very slow, or, more expectedly, on csv import) - one_day = 60 * 60 * 24 - if (instance.created_date - instance.published_date).seconds > one_day: - priority = LOW - - add_list_task.apply_async( - args=(instance.id,), - kwargs={"increment_unread": created}, - queue=priority, - ) + add_list_task.delay(instance.id) @receiver(signals.post_save, sender=models.UserFollows) @@ -188,17 +179,17 @@ def add_lists_on_unblock(sender, instance, *args, **kwargs): @receiver(signals.post_save, sender=models.User) # pylint: disable=unused-argument -def populate_streams_on_account_create(sender, instance, created, *args, **kwargs): +def populate_lists_on_account_create(sender, instance, created, *args, **kwargs): """build a user's feeds when they join""" if not created or not instance.local: return - populate_lists_stream_task.delay(instance.id) + populate_lists_task.delay(instance.id) # ---- TASKS @app.task(queue=MEDIUM) -def populate_lists_stream_task(user_id): +def populate_lists_task(user_id): """background task for populating an empty activitystream""" user = models.User.objects.get(id=user_id) ListsStream().populate_streams(user) diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 34aeb947..7640ea98 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -1,11 +1,11 @@ -""" testing activitystreams """ +""" testing lists_stream """ from unittest.mock import patch from django.test import TestCase -from bookwyrm import activitystreams, models +from bookwyrm import lists_stream, models @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") -class ActivitystreamsSignals(TestCase): +class ListsStreamSignals(TestCase): """using redis to build activity streams""" def setUp(self): @@ -29,51 +29,41 @@ class ActivitystreamsSignals(TestCase): inbox="https://example.com/users/rat/inbox", outbox="https://example.com/users/rat/outbox", ) - work = models.Work.objects.create(title="test work") - self.book = models.Edition.objects.create(title="test book", parent_work=work) - def test_add_status_on_create_ignore(self, _): - """a new statuses has entered""" - activitystreams.add_status_on_create(models.User, self.local_user, False) - - def test_add_status_on_create_deleted(self, _): - """a new statuses has entered""" - with patch("bookwyrm.activitystreams.remove_status_task.delay"): - status = models.Status.objects.create( - user=self.remote_user, content="hi", privacy="public", deleted=True - ) - with patch("bookwyrm.activitystreams.remove_status_task.delay") as mock: - activitystreams.add_status_on_create(models.Status, status, False) + def test_add_list_on_create(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.add_list_task.delay") as mock: + lists_stream.add_list_on_create_command(book_list) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] - self.assertEqual(args[0], status.id) + self.assertEqual(args[0], book_list.id) - def test_add_status_on_create_created(self, _): - """a new statuses has entered""" - status = models.Status.objects.create( - user=self.remote_user, content="hi", privacy="public" + 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.activitystreams.add_status_task.apply_async") as mock: - activitystreams.add_status_on_create_command(models.Status, status, False) - self.assertEqual(mock.call_count, 1) - args = mock.call_args[1] - self.assertEqual(args["args"][0], status.id) - self.assertEqual(args["queue"], "high_priority") + 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] + self.assertEqual(args[0], book_list.id) - def test_populate_streams_on_account_create(self, _): + def test_populate_lists_on_account_create(self, _): """create streams for a user""" - with patch("bookwyrm.activitystreams.populate_stream_task.delay") as mock: - activitystreams.populate_streams_on_account_create( + with patch("bookwyrm.lists_stream.populate_lists_task.delay") as mock: + lists_stream.populate_lists_on_account_create( models.User, self.local_user, True ) - self.assertEqual(mock.call_count, 3) + self.assertEqual(mock.call_count, 1) args = mock.call_args[0] - self.assertEqual(args[0], "books") - self.assertEqual(args[1], self.local_user.id) + self.assertEqual(args[0], self.local_user.id) - def test_remove_statuses_on_block(self, _): - """don't show statuses from blocked users""" - with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") as mock: + def test_remove_lists_on_block(self, _): + """don't show lists from blocked users""" + with patch("bookwyrm.lists_stream.remove_user_lists_task.delay") as mock: models.UserBlocks.objects.create( user_subject=self.local_user, user_object=self.remote_user, @@ -83,26 +73,24 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args[0], self.local_user.id) self.assertEqual(args[1], self.remote_user.id) - def test_add_statuses_on_unblock(self, _): - """re-add statuses on unblock""" - with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): + def test_add_lists_on_unblock(self, _): + """re-add lists on unblock""" + with patch("bookwyrm.lists_stream.remove_user_lists_task.delay"): block = models.UserBlocks.objects.create( user_subject=self.local_user, user_object=self.remote_user, ) - with patch("bookwyrm.activitystreams.add_user_statuses_task.delay") as mock: + with patch("bookwyrm.lists_stream.add_user_lists_task.delay") as mock: block.delete() args = mock.call_args[0] - kwargs = mock.call_args.kwargs self.assertEqual(args[0], self.local_user.id) self.assertEqual(args[1], self.remote_user.id) - self.assertEqual(kwargs["stream_list"], ["local", "books"]) - def test_add_statuses_on_unblock_reciprocal_block(self, _): - """re-add statuses on unblock""" - with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): + def test_add_lists_on_unblock_reciprocal_block(self, _): + """dont' re-add lists on unblock if there's a block the other way""" + with patch("bookwyrm.lists_stream.remove_user_lists_task.delay"): block = models.UserBlocks.objects.create( user_subject=self.local_user, user_object=self.remote_user, @@ -112,7 +100,7 @@ class ActivitystreamsSignals(TestCase): user_object=self.local_user, ) - with patch("bookwyrm.activitystreams.add_user_statuses_task.delay") as mock: + with patch("bookwyrm.lists_stream.add_user_lists_task.delay") as mock: block.delete() - self.assertEqual(mock.call_count, 0) + self.assertFalse(mock.called) From b206aae32be8e21c9bd2780f9617d4a9d7cfcab2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Nov 2021 09:47:24 -0800 Subject: [PATCH 07/38] Tasks tests --- bookwyrm/tests/lists_stream/test_tasks.py | 195 +++++----------------- 1 file changed, 41 insertions(+), 154 deletions(-) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py index f5750763..1e5f7a98 100644 --- a/bookwyrm/tests/lists_stream/test_tasks.py +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -1,7 +1,7 @@ -""" testing activitystreams """ +""" testing lists_stream """ from unittest.mock import patch from django.test import TestCase -from bookwyrm import activitystreams, models +from bookwyrm import lists_stream, models class Activitystreams(TestCase): @@ -32,187 +32,74 @@ class Activitystreams(TestCase): inbox="https://example.com/users/rat/inbox", outbox="https://example.com/users/rat/outbox", ) - work = models.Work.objects.create(title="test work") - self.book = models.Edition.objects.create(title="test book", parent_work=work) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): - self.status = models.Status.objects.create( - content="hi", user=self.local_user - ) + self.list = models.List.objects.create( + user=self.local_user, name="hi", privacy="public" + ) - def test_add_book_statuses_task(self): - """statuses related to a book""" - with patch("bookwyrm.activitystreams.BooksStream.add_book_statuses") as mock: - activitystreams.add_book_statuses_task(self.local_user.id, self.book.id) - self.assertTrue(mock.called) - args = mock.call_args[0] - self.assertEqual(args[0], self.local_user) - self.assertEqual(args[1], self.book) - - def test_remove_book_statuses_task(self): - """remove stauses related to a book""" - with patch("bookwyrm.activitystreams.BooksStream.remove_book_statuses") as mock: - activitystreams.remove_book_statuses_task(self.local_user.id, self.book.id) - self.assertTrue(mock.called) - args = mock.call_args[0] - self.assertEqual(args[0], self.local_user) - self.assertEqual(args[1], self.book) - - def test_populate_stream_task(self): - """populate a given stream""" - with patch("bookwyrm.activitystreams.BooksStream.populate_streams") as mock: - activitystreams.populate_stream_task("books", self.local_user.id) + def test_populate_lists_task(self): + """populate lists cache""" + with patch("bookwyrm.lists_stream.ListsStream.populate_streams") as mock: + lists_stream.populate_lists_task(self.local_user.id) self.assertTrue(mock.called) args = mock.call_args[0] self.assertEqual(args[0], self.local_user) - with patch("bookwyrm.activitystreams.HomeStream.populate_streams") as mock: - activitystreams.populate_stream_task("home", self.local_user.id) + with patch("bookwyrm.lists_stream.ListsStream.populate_streams") as mock: + lists_stream.populate_lists_task(self.local_user.id) self.assertTrue(mock.called) args = mock.call_args[0] self.assertEqual(args[0], self.local_user) - def test_remove_status_task(self): - """remove a status from all streams""" + def test_remove_list_task(self): + """remove a list from all streams""" with patch( - "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + "bookwyrm.lists_stream.ListsStream.remove_object_from_related_stores" ) as mock: - activitystreams.remove_status_task(self.status.id) - self.assertEqual(mock.call_count, 3) + lists_stream.remove_list_task(self.list.id) + self.assertEqual(mock.call_count, 1) args = mock.call_args[0] - self.assertEqual(args[0], self.status) + self.assertEqual(args[0], self.list) - def test_add_status_task(self): - """add a status to all streams""" - with patch("bookwyrm.activitystreams.ActivityStream.add_status") as mock: - activitystreams.add_status_task(self.status.id) - self.assertEqual(mock.call_count, 3) + def test_add_list_task(self): + """add a list to all streams""" + with patch("bookwyrm.lists_stream.ListsStream.add_list") as mock: + lists_stream.add_list_task(self.list.id) + self.assertEqual(mock.call_count, 1) args = mock.call_args[0] - self.assertEqual(args[0], self.status) + self.assertEqual(args[0], self.list) - def test_remove_user_statuses_task(self): - """remove all statuses by a user from another users' feeds""" - with patch( - "bookwyrm.activitystreams.ActivityStream.remove_user_statuses" - ) as mock: - activitystreams.remove_user_statuses_task( + def test_remove_user_lists_task(self): + """remove all lists by a user from another users' feeds""" + with patch("bookwyrm.lists_stream.ListsStream.remove_user_lists") as mock: + lists_stream.remove_user_lists_task( self.local_user.id, self.another_user.id ) - self.assertEqual(mock.call_count, 3) - args = mock.call_args[0] - self.assertEqual(args[0], self.local_user) - self.assertEqual(args[1], self.another_user) - - with patch("bookwyrm.activitystreams.HomeStream.remove_user_statuses") as mock: - activitystreams.remove_user_statuses_task( - self.local_user.id, self.another_user.id, stream_list=["home"] - ) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], self.local_user) self.assertEqual(args[1], self.another_user) - def test_add_user_statuses_task(self): - """add a user's statuses to another users feeds""" - with patch("bookwyrm.activitystreams.ActivityStream.add_user_statuses") as mock: - activitystreams.add_user_statuses_task( + with patch("bookwyrm.lists_stream.ListsStream.remove_user_lists") as mock: + lists_stream.remove_user_lists_task( self.local_user.id, self.another_user.id ) - self.assertEqual(mock.call_count, 3) - args = mock.call_args[0] - self.assertEqual(args[0], self.local_user) - self.assertEqual(args[1], self.another_user) - - with patch("bookwyrm.activitystreams.HomeStream.add_user_statuses") as mock: - activitystreams.add_user_statuses_task( - self.local_user.id, self.another_user.id, stream_list=["home"] - ) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], self.local_user) self.assertEqual(args[1], self.another_user) - @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") - @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") - @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_boost_to_another_timeline(self, *_): - """boost from a non-follower doesn't remove original status from feed""" - status = models.Status.objects.create(user=self.local_user, content="hi") - with patch("bookwyrm.activitystreams.handle_boost_task.delay"): - boost = models.Boost.objects.create( - boosted_status=status, - user=self.another_user, - ) - with patch( - "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" - ) as mock: - activitystreams.handle_boost_task(boost.id) - - self.assertTrue(mock.called) + def test_add_user_lists_task(self): + """add a user's lists to another users feeds""" + with patch("bookwyrm.lists_stream.ListsStream.add_user_lists") as mock: + lists_stream.add_user_lists_task(self.local_user.id, self.another_user.id) self.assertEqual(mock.call_count, 1) - call_args = mock.call_args - self.assertEqual(call_args[0][0], status) - self.assertEqual(call_args[1]["stores"], [f"{self.another_user.id}-home"]) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) - @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") - @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") - @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_boost_to_another_timeline_remote(self, *_): - """boost from a remote non-follower doesn't remove original status from feed""" - status = models.Status.objects.create(user=self.local_user, content="hi") - with patch("bookwyrm.activitystreams.handle_boost_task.delay"): - boost = models.Boost.objects.create( - boosted_status=status, - user=self.remote_user, - ) - with patch( - "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" - ) as mock: - activitystreams.handle_boost_task(boost.id) - - self.assertTrue(mock.called) + with patch("bookwyrm.lists_stream.ListsStream.add_user_lists") as mock: + lists_stream.add_user_lists_task(self.local_user.id, self.another_user.id) self.assertEqual(mock.call_count, 1) - call_args = mock.call_args - self.assertEqual(call_args[0][0], status) - self.assertEqual(call_args[1]["stores"], []) - - @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") - @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") - @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_boost_to_following_timeline(self, *_): - """add a boost and deduplicate the boosted status on the timeline""" - self.local_user.following.add(self.another_user) - status = models.Status.objects.create(user=self.local_user, content="hi") - with patch("bookwyrm.activitystreams.handle_boost_task.delay"): - boost = models.Boost.objects.create( - boosted_status=status, - user=self.another_user, - ) - with patch( - "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" - ) as mock: - activitystreams.handle_boost_task(boost.id) - self.assertTrue(mock.called) - call_args = mock.call_args - self.assertEqual(call_args[0][0], status) - self.assertTrue(f"{self.another_user.id}-home" in call_args[1]["stores"]) - self.assertTrue(f"{self.local_user.id}-home" in call_args[1]["stores"]) - - @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") - @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") - @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_boost_to_same_timeline(self, *_): - """add a boost and deduplicate the boosted status on the timeline""" - status = models.Status.objects.create(user=self.local_user, content="hi") - with patch("bookwyrm.activitystreams.handle_boost_task.delay"): - boost = models.Boost.objects.create( - boosted_status=status, - user=self.local_user, - ) - with patch( - "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" - ) as mock: - activitystreams.handle_boost_task(boost.id) - self.assertTrue(mock.called) - call_args = mock.call_args - self.assertEqual(call_args[0][0], status) - self.assertEqual(call_args[1]["stores"], [f"{self.local_user.id}-home"]) + args = mock.call_args[0] + self.assertEqual(args[0], self.local_user) + self.assertEqual(args[1], self.another_user) From eb4a39947252e8d97f9812daae15cde52562e97a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 12:42:07 -0800 Subject: [PATCH 08/38] Updates models tests --- bookwyrm/lists_stream.py | 10 ++--- .../tests/models/test_activitypub_mixin.py | 8 ++-- bookwyrm/tests/models/test_base_model.py | 2 +- bookwyrm/tests/models/test_fields.py | 1 + bookwyrm/tests/models/test_group.py | 19 ++++---- bookwyrm/tests/models/test_import_model.py | 2 +- bookwyrm/tests/models/test_list.py | 6 +-- .../tests/models/test_readthrough_model.py | 2 +- .../tests/models/test_relationship_models.py | 45 +++++++++++++++---- bookwyrm/tests/models/test_shelf_model.py | 5 ++- bookwyrm/tests/models/test_site.py | 2 +- bookwyrm/tests/models/test_status_model.py | 2 +- bookwyrm/tests/models/test_user_model.py | 2 +- 13 files changed, 68 insertions(+), 38 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 04a0725c..da8cdb21 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -9,7 +9,7 @@ from bookwyrm.tasks import app, MEDIUM, HIGH class ListsStream(RedisStore): - """a category of activity stream (like home, local, books)""" + """all the lists you can see""" def stream_id(self, user): # pylint: disable=no-self-use """the redis key for this user's instance of this stream""" @@ -122,9 +122,7 @@ def add_lists_on_follow(sender, instance, created, *args, **kwargs): """add a newly followed user's lists to feeds""" if not created or not instance.user_subject.local: return - add_user_lists_task.delay( - instance.user_subject.id, instance.user_object.id, stream_list=["home"] - ) + add_user_lists_task.delay(instance.user_subject.id, instance.user_object.id) @receiver(signals.post_delete, sender=models.UserFollows) @@ -133,9 +131,7 @@ def remove_lists_on_unfollow(sender, instance, *args, **kwargs): """remove lists from a feed on unfollow""" if not instance.user_subject.local: return - remove_user_lists_task.delay( - instance.user_subject.id, instance.user_object.id, stream_list=["home"] - ) + remove_user_lists_task.delay(instance.user_subject.id, instance.user_object.id) @receiver(signals.post_save, sender=models.UserBlocks) diff --git a/bookwyrm/tests/models/test_activitypub_mixin.py b/bookwyrm/tests/models/test_activitypub_mixin.py index 91a1fe7c..f1279ddf 100644 --- a/bookwyrm/tests/models/test_activitypub_mixin.py +++ b/bookwyrm/tests/models/test_activitypub_mixin.py @@ -29,7 +29,7 @@ class ActivitypubMixins(TestCase): """shared data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.com", "mouseword", local=True, localname="mouse" ) @@ -332,7 +332,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(activity["id"], "https://example.com/status/1/activity") self.assertEqual(activity["actor"], self.local_user.remote_id) self.assertEqual(activity["type"], "Delete") - self.assertEqual(activity["to"], ["%s/followers" % self.local_user.remote_id]) + self.assertEqual(activity["to"], [f"{self.local_user.remote_id}/followers"]) self.assertEqual( activity["cc"], ["https://www.w3.org/ns/activitystreams#Public"] ) @@ -374,7 +374,7 @@ class ActivitypubMixins(TestCase): for number in range(0, 2 * PAGE_LENGTH): models.Status.objects.create( user=self.local_user, - content="test status {:d}".format(number), + content=f"test status {number}", ) page_1 = to_ordered_collection_page( models.Status.objects.all(), "http://fish.com/", page=1 @@ -400,7 +400,7 @@ class ActivitypubMixins(TestCase): for number in range(0, 2 * PAGE_LENGTH): models.Status.objects.create( user=self.local_user, - content="test status {:d}".format(number), + content=f"test status {number}", ) MockSelf = namedtuple("Self", ("remote_id")) diff --git a/bookwyrm/tests/models/test_base_model.py b/bookwyrm/tests/models/test_base_model.py index 7c7b48de..ae6d1207 100644 --- a/bookwyrm/tests/models/test_base_model.py +++ b/bookwyrm/tests/models/test_base_model.py @@ -16,7 +16,7 @@ class BaseModel(TestCase): """shared data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.com", "mouseword", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 8028a305..c3402a25 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -27,6 +27,7 @@ from bookwyrm.settings import DOMAIN # pylint: disable=too-many-public-methods @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") class ModelFields(TestCase): """overwrites standard model feilds to work with activitypub""" diff --git a/bookwyrm/tests/models/test_group.py b/bookwyrm/tests/models/test_group.py index 2dd3cee1..3439a1bf 100644 --- a/bookwyrm/tests/models/test_group.py +++ b/bookwyrm/tests/models/test_group.py @@ -2,7 +2,7 @@ from unittest.mock import patch from django.test import TestCase -from bookwyrm import models, settings +from bookwyrm import models @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @@ -14,21 +14,21 @@ class Group(TestCase): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.owner_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.rat = models.User.objects.create_user( "rat", "rat@rat.rat", "ratword", local=True, localname="rat" ) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.badger = models.User.objects.create_user( "badger", "badger@badger.badger", @@ -39,7 +39,7 @@ class Group(TestCase): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.capybara = models.User.objects.create_user( "capybara", "capybara@capybara.capybara", @@ -76,7 +76,8 @@ class Group(TestCase): models.GroupMember.objects.create(group=self.public_group, user=self.capybara) def test_group_members_can_see_private_groups(self, _): - """direct privacy group should not be excluded from group listings for group members viewing""" + """direct privacy group should not be excluded from group listings for group + members viewing""" rat_groups = models.Group.privacy_filter(self.rat).all() badger_groups = models.Group.privacy_filter(self.badger).all() @@ -85,7 +86,8 @@ class Group(TestCase): self.assertTrue(self.private_group in badger_groups) def test_group_members_can_see_followers_only_lists(self, _): - """follower-only group booklists should not be excluded from group booklist listing for group members who do not follower list owner""" + """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"): followers_list = models.List.objects.create( @@ -105,7 +107,8 @@ class Group(TestCase): self.assertTrue(followers_list in capybara_lists) def test_group_members_can_see_private_lists(self, _): - """private group booklists should not be excluded from group booklist listing for group members""" + """private group booklists should not be excluded from group booklist listing + for group members""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index f48143d0..225466f3 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -20,7 +20,7 @@ class ImportJob(TestCase): """data is from a goodreads export of The Raven Tower""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True ) diff --git a/bookwyrm/tests/models/test_list.py b/bookwyrm/tests/models/test_list.py index 824de835..e4ecfe89 100644 --- a/bookwyrm/tests/models/test_list.py +++ b/bookwyrm/tests/models/test_list.py @@ -1,7 +1,7 @@ """ testing models """ +from uuid import UUID from unittest.mock import patch from django.test import TestCase -from uuid import UUID from bookwyrm import models, settings @@ -14,7 +14,7 @@ class List(TestCase): """look, a list""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) @@ -27,7 +27,7 @@ class List(TestCase): book_list = models.List.objects.create( name="Test List", user=self.local_user ) - expected_id = "https://%s/list/%d" % (settings.DOMAIN, book_list.id) + expected_id = f"https://{settings.DOMAIN}/list/{book_list.id}" self.assertEqual(book_list.get_remote_id(), expected_id) def test_to_activity(self, _): diff --git a/bookwyrm/tests/models/test_readthrough_model.py b/bookwyrm/tests/models/test_readthrough_model.py index 5d8450a1..7e3963cf 100644 --- a/bookwyrm/tests/models/test_readthrough_model.py +++ b/bookwyrm/tests/models/test_readthrough_model.py @@ -15,7 +15,7 @@ class ReadThrough(TestCase): """look, a shelf""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/models/test_relationship_models.py b/bookwyrm/tests/models/test_relationship_models.py index 2b388398..a5b4dbff 100644 --- a/bookwyrm/tests/models/test_relationship_models.py +++ b/bookwyrm/tests/models/test_relationship_models.py @@ -1,12 +1,16 @@ """ testing models """ import json from unittest.mock import patch +from django.db import IntegrityError from django.test import TestCase from bookwyrm import models @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") +@patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") +@patch("bookwyrm.lists_stream.add_user_lists_task.delay") +@patch("bookwyrm.lists_stream.remove_user_lists_task.delay") class Relationship(TestCase): """following, blocking, stuff like that""" @@ -24,14 +28,39 @@ class Relationship(TestCase): ) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.com", "mouseword", local=True, localname="mouse" ) self.local_user.remote_id = "http://local.com/user/mouse" self.local_user.save(broadcast=False, update_fields=["remote_id"]) - def test_user_follows_from_request(self, _): + def test_user_follows(self, *_): + """basic functionality of user follows""" + relationship = models.UserFollows.objects.create( + user_subject=self.local_user, user_object=self.remote_user + ) + self.assertEqual(relationship.status, "follows") + activity = relationship.to_activity() + self.assertEqual(activity.type, "Follow") + self.assertEqual( + relationship.remote_id, + f"http://local.com/user/mouse#follows/{relationship.id}", + ) + + def test_user_follows_blocks(self, *_): + """can't follow if you're blocked""" + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + models.UserBlocks.objects.create( + user_subject=self.local_user, user_object=self.remote_user + ) + + with self.assertRaises(IntegrityError): + models.UserFollows.objects.create( + user_subject=self.local_user, user_object=self.remote_user + ) + + def test_user_follows_from_request(self, *_): """convert a follow request into a follow""" with patch( "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" @@ -42,19 +71,19 @@ class Relationship(TestCase): activity = json.loads(mock.call_args[1]["args"][1]) self.assertEqual(activity["type"], "Follow") self.assertEqual( - request.remote_id, "http://local.com/user/mouse#follows/%d" % request.id + request.remote_id, f"http://local.com/user/mouse#follows/{request.id}" ) self.assertEqual(request.status, "follow_request") rel = models.UserFollows.from_request(request) self.assertEqual( - rel.remote_id, "http://local.com/user/mouse#follows/%d" % request.id + rel.remote_id, f"http://local.com/user/mouse#follows/{request.id}" ) self.assertEqual(rel.status, "follows") self.assertEqual(rel.user_subject, self.local_user) self.assertEqual(rel.user_object, self.remote_user) - def test_user_follows_from_request_custom_remote_id(self, _): + def test_user_follows_from_request_custom_remote_id(self, *_): """store a specific remote id for a relationship provided by remote""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): request = models.UserFollowRequest.objects.create( @@ -72,7 +101,7 @@ class Relationship(TestCase): self.assertEqual(rel.user_object, self.remote_user) @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_follow_request_activity(self, broadcast_mock, _): + def test_follow_request_activity(self, broadcast_mock, *_): """accept a request and make it a relationship""" models.UserFollowRequest.objects.create( user_subject=self.local_user, @@ -84,7 +113,7 @@ class Relationship(TestCase): self.assertEqual(activity["type"], "Follow") @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_follow_request_accept(self, broadcast_mock, _): + def test_follow_request_accept(self, broadcast_mock, *_): """accept a request and make it a relationship""" self.local_user.manually_approves_followers = True self.local_user.save( @@ -110,7 +139,7 @@ class Relationship(TestCase): self.assertEqual(rel.user_object, self.local_user) @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") - def test_follow_request_reject(self, broadcast_mock, _): + def test_follow_request_reject(self, broadcast_mock, *_): """accept a request and make it a relationship""" self.local_user.manually_approves_followers = True self.local_user.save( diff --git a/bookwyrm/tests/models/test_shelf_model.py b/bookwyrm/tests/models/test_shelf_model.py index 0683fbef..4f7f3589 100644 --- a/bookwyrm/tests/models/test_shelf_model.py +++ b/bookwyrm/tests/models/test_shelf_model.py @@ -9,6 +9,7 @@ from bookwyrm import models, settings # pylint: disable=unused-argument @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.activitystreams.add_book_statuses_task.delay") @patch("bookwyrm.activitystreams.remove_book_statuses_task.delay") class Shelf(TestCase): @@ -18,7 +19,7 @@ class Shelf(TestCase): """look, a shelf""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) @@ -31,7 +32,7 @@ class Shelf(TestCase): shelf = models.Shelf.objects.create( name="Test Shelf", identifier="test-shelf", user=self.local_user ) - expected_id = "https://%s/user/mouse/books/test-shelf" % settings.DOMAIN + expected_id = f"https://{settings.DOMAIN}/user/mouse/books/test-shelf" self.assertEqual(shelf.get_remote_id(), expected_id) def test_to_activity(self, *_): diff --git a/bookwyrm/tests/models/test_site.py b/bookwyrm/tests/models/test_site.py index b7c19c4b..d23f7988 100644 --- a/bookwyrm/tests/models/test_site.py +++ b/bookwyrm/tests/models/test_site.py @@ -16,7 +16,7 @@ class SiteModels(TestCase): """we need basic test data and mocks""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 822d837a..6520094c 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -28,7 +28,7 @@ class Status(TestCase): """useful things for creating a status""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 389928cd..aa62dce3 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -15,7 +15,7 @@ class User(TestCase): def setUp(self): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.user = models.User.objects.create_user( "mouse@%s" % DOMAIN, "mouse@mouse.mouse", From 7a89552892703648cff1fa6616a3f52dcec9b64e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 13:01:50 -0800 Subject: [PATCH 09/38] Updates test_* tests --- bookwyrm/tests/test_emailing.py | 2 +- bookwyrm/tests/test_preview_images.py | 2 +- bookwyrm/tests/test_signing.py | 10 +++++----- bookwyrm/tests/test_suggested_users.py | 3 ++- bookwyrm/tests/test_templatetags.py | 19 +++++++++++++------ 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/bookwyrm/tests/test_emailing.py b/bookwyrm/tests/test_emailing.py index 2f24122e..ecfbd944 100644 --- a/bookwyrm/tests/test_emailing.py +++ b/bookwyrm/tests/test_emailing.py @@ -16,7 +16,7 @@ class Emailing(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/test_preview_images.py b/bookwyrm/tests/test_preview_images.py index f95e623c..79ee195d 100644 --- a/bookwyrm/tests/test_preview_images.py +++ b/bookwyrm/tests/test_preview_images.py @@ -32,7 +32,7 @@ class PreviewImages(TestCase): ) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "possum@local.com", "possum@possum.possum", diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index d33687a5..da67a8de 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -39,19 +39,19 @@ class Signature(TestCase): """create users and test data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.mouse = models.User.objects.create_user( - "mouse@%s" % DOMAIN, + f"mouse@{DOMAIN}", "mouse@example.com", "", local=True, localname="mouse", ) self.rat = models.User.objects.create_user( - "rat@%s" % DOMAIN, "rat@example.com", "", local=True, localname="rat" + f"rat@{DOMAIN}", "rat@example.com", "", local=True, localname="rat" ) self.cat = models.User.objects.create_user( - "cat@%s" % DOMAIN, "cat@example.com", "", local=True, localname="cat" + f"cat@{DOMAIN}", "cat@example.com", "", local=True, localname="cat" ) private_key, public_key = create_key_pair() @@ -75,7 +75,7 @@ class Signature(TestCase): "HTTP_DIGEST": digest, "HTTP_CONTENT_TYPE": "application/activity+json; charset=utf-8", "HTTP_HOST": DOMAIN, - } + }, ) def send_test_request( # pylint: disable=too-many-arguments diff --git a/bookwyrm/tests/test_suggested_users.py b/bookwyrm/tests/test_suggested_users.py index dce5d770..91677f19 100644 --- a/bookwyrm/tests/test_suggested_users.py +++ b/bookwyrm/tests/test_suggested_users.py @@ -13,6 +13,7 @@ from bookwyrm.suggested_users import suggested_users, get_annotated_users @patch("bookwyrm.activitystreams.add_status_task.delay") @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.activitystreams.add_book_statuses_task.delay") @patch("bookwyrm.suggested_users.rerank_user_task.delay") @patch("bookwyrm.suggested_users.remove_user_task.delay") @@ -23,7 +24,7 @@ class SuggestedUsers(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/test_templatetags.py b/bookwyrm/tests/test_templatetags.py index ed4466f5..b7b1d748 100644 --- a/bookwyrm/tests/test_templatetags.py +++ b/bookwyrm/tests/test_templatetags.py @@ -24,7 +24,7 @@ class TemplateTags(TestCase): """create some filler objects""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.mouse", @@ -42,6 +42,18 @@ class TemplateTags(TestCase): ) self.book = models.Edition.objects.create(title="Test Book") + 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_title(self, *_): + """the title of a book""" + self.assertEqual(utilities.get_title(None), "") + self.assertEqual(utilities.get_title(self.book), "Test Book") + book = models.Edition.objects.create(title="Oh", subtitle="oh my") + self.assertEqual(utilities.get_title(book), "Oh: oh my") + def test_get_user_rating(self, *_): """get a user's most recent rating of a book""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): @@ -145,11 +157,6 @@ class TemplateTags(TestCase): self.book.save() self.assertEqual(bookwyrm_tags.get_book_description(self.book), "hello") - 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, *_): """mardown format data""" result = markdown.get_markdown("_hi_") From 93dbe2daa0fd5ffc8d3f38749c423e9af1d0d09e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 15:02:53 -0800 Subject: [PATCH 10/38] Updates inbox tests --- bookwyrm/tests/views/inbox/test_inbox.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_add.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_announce.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_block.py | 8 +++++--- bookwyrm/tests/views/inbox/test_inbox_create.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_delete.py | 10 ++++++---- bookwyrm/tests/views/inbox/test_inbox_follow.py | 5 +++-- bookwyrm/tests/views/inbox/test_inbox_like.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_remove.py | 2 +- bookwyrm/tests/views/inbox/test_inbox_update.py | 3 ++- 10 files changed, 22 insertions(+), 16 deletions(-) diff --git a/bookwyrm/tests/views/inbox/test_inbox.py b/bookwyrm/tests/views/inbox/test_inbox.py index 47e6a86e..e328b1ba 100644 --- a/bookwyrm/tests/views/inbox/test_inbox.py +++ b/bookwyrm/tests/views/inbox/test_inbox.py @@ -22,7 +22,7 @@ class Inbox(TestCase): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py index 33e6c55b..a9a80982 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_add.py +++ b/bookwyrm/tests/views/inbox/test_inbox_add.py @@ -15,7 +15,7 @@ class InboxAdd(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_announce.py b/bookwyrm/tests/views/inbox/test_inbox_announce.py index a291552d..01580c92 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_announce.py +++ b/bookwyrm/tests/views/inbox/test_inbox_announce.py @@ -15,7 +15,7 @@ class InboxActivities(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_block.py b/bookwyrm/tests/views/inbox/test_inbox_block.py index f6898fc6..eb73af09 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_block.py +++ b/bookwyrm/tests/views/inbox/test_inbox_block.py @@ -14,7 +14,7 @@ class InboxBlock(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", @@ -57,7 +57,7 @@ class InboxBlock(TestCase): with patch( "bookwyrm.activitystreams.remove_user_statuses_task.delay" - ) as redis_mock: + ) as redis_mock, patch("bookwyrm.lists_stream.remove_user_lists_task.delay"): views.inbox.activity_task(activity) self.assertTrue(redis_mock.called) views.inbox.activity_task(activity) @@ -70,7 +70,9 @@ class InboxBlock(TestCase): self.assertFalse(models.UserFollowRequest.objects.exists()) @patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") - def test_handle_unblock(self, _): + @patch("bookwyrm.lists_stream.add_user_lists_task.delay") + @patch("bookwyrm.lists_stream.remove_user_lists_task.delay") + def test_handle_unblock(self, *_): """unblock a user""" self.remote_user.blocks.add(self.local_user) diff --git a/bookwyrm/tests/views/inbox/test_inbox_create.py b/bookwyrm/tests/views/inbox/test_inbox_create.py index 53b17d68..4ee366cf 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_create.py +++ b/bookwyrm/tests/views/inbox/test_inbox_create.py @@ -19,7 +19,7 @@ class InboxCreate(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_delete.py b/bookwyrm/tests/views/inbox/test_inbox_delete.py index dd603352..b4863aad 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_delete.py +++ b/bookwyrm/tests/views/inbox/test_inbox_delete.py @@ -15,7 +15,7 @@ class InboxActivities(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", @@ -51,7 +51,7 @@ class InboxActivities(TestCase): "type": "Delete", "to": ["https://www.w3.org/ns/activitystreams#Public"], "cc": ["https://example.com/user/mouse/followers"], - "id": "%s/activity" % self.status.remote_id, + "id": f"{self.status.remote_id}/activity", "actor": self.remote_user.remote_id, "object": {"id": self.status.remote_id, "type": "Tombstone"}, } @@ -80,7 +80,7 @@ class InboxActivities(TestCase): "type": "Delete", "to": ["https://www.w3.org/ns/activitystreams#Public"], "cc": ["https://example.com/user/mouse/followers"], - "id": "%s/activity" % self.status.remote_id, + "id": f"{self.status.remote_id}/activity", "actor": self.remote_user.remote_id, "object": {"id": self.status.remote_id, "type": "Tombstone"}, } @@ -152,5 +152,7 @@ class InboxActivities(TestCase): "cc": [], }, } - views.inbox.activity_task(activity) + with patch("bookwyrm.lists_stream.remove_list_task.delay") as mock: + views.inbox.activity_task(activity) + self.assertTrue(mock.called) self.assertFalse(models.List.objects.exists()) diff --git a/bookwyrm/tests/views/inbox/test_inbox_follow.py b/bookwyrm/tests/views/inbox/test_inbox_follow.py index 71f101ca..13e46ff8 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_follow.py +++ b/bookwyrm/tests/views/inbox/test_inbox_follow.py @@ -15,7 +15,7 @@ class InboxRelationships(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", @@ -188,7 +188,8 @@ class InboxRelationships(TestCase): self.assertIsNone(self.local_user.followers.first()) @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") - def test_follow_accept(self, _): + @patch("bookwyrm.lists_stream.add_user_lists_task.delay") + def test_follow_accept(self, *_): """a remote user approved a follow request from local""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): rel = models.UserFollowRequest.objects.create( diff --git a/bookwyrm/tests/views/inbox/test_inbox_like.py b/bookwyrm/tests/views/inbox/test_inbox_like.py index 2f1b6629..ea4d4a65 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_like.py +++ b/bookwyrm/tests/views/inbox/test_inbox_like.py @@ -14,7 +14,7 @@ class InboxActivities(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_remove.py b/bookwyrm/tests/views/inbox/test_inbox_remove.py index 55cc8120..53288e0d 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_remove.py +++ b/bookwyrm/tests/views/inbox/test_inbox_remove.py @@ -14,7 +14,7 @@ class InboxRemove(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/inbox/test_inbox_update.py b/bookwyrm/tests/views/inbox/test_inbox_update.py index 248c1ad2..052b47c4 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_update.py +++ b/bookwyrm/tests/views/inbox/test_inbox_update.py @@ -16,7 +16,7 @@ class InboxUpdate(TestCase): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@example.com", "mouse@mouse.com", @@ -78,6 +78,7 @@ class InboxUpdate(TestCase): @patch("bookwyrm.suggested_users.rerank_user_task.delay") @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") + @patch("bookwyrm.lists_stream.add_user_lists_task.delay") def test_update_user(self, *_): """update an existing user""" models.UserFollows.objects.create( From 2d63bfb791cd13ff38c30dc9b3886e4903c52d76 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 15:03:01 -0800 Subject: [PATCH 11/38] Updates views tests --- bookwyrm/tests/views/test_author.py | 2 +- bookwyrm/tests/views/test_directory.py | 3 ++- bookwyrm/tests/views/test_discover.py | 7 +++--- bookwyrm/tests/views/test_feed.py | 25 ++++++++++++++++++--- bookwyrm/tests/views/test_follow.py | 26 ++++++++++++---------- bookwyrm/tests/views/test_get_started.py | 2 +- bookwyrm/tests/views/test_goal.py | 2 +- bookwyrm/tests/views/test_group.py | 2 +- bookwyrm/tests/views/test_helpers.py | 2 +- bookwyrm/tests/views/test_interaction.py | 2 +- bookwyrm/tests/views/test_isbn.py | 12 +++++++++- bookwyrm/tests/views/test_list.py | 24 ++++++++++++-------- bookwyrm/tests/views/test_list_actions.py | 10 ++++----- bookwyrm/tests/views/test_notifications.py | 2 +- bookwyrm/tests/views/test_outbox.py | 2 +- bookwyrm/tests/views/test_reading.py | 2 +- bookwyrm/tests/views/test_readthrough.py | 2 +- bookwyrm/tests/views/test_rss_feed.py | 2 +- bookwyrm/tests/views/test_search.py | 2 +- bookwyrm/tests/views/test_status.py | 3 ++- bookwyrm/tests/views/test_updates.py | 2 +- bookwyrm/tests/views/test_user.py | 2 +- bookwyrm/tests/views/test_wellknown.py | 2 +- bookwyrm/views/feed.py | 4 +++- 24 files changed, 92 insertions(+), 52 deletions(-) diff --git a/bookwyrm/tests/views/test_author.py b/bookwyrm/tests/views/test_author.py index 03166932..ad5c069d 100644 --- a/bookwyrm/tests/views/test_author.py +++ b/bookwyrm/tests/views/test_author.py @@ -21,7 +21,7 @@ class AuthorViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_directory.py b/bookwyrm/tests/views/test_directory.py index 4fe2aa05..bceb0e7a 100644 --- a/bookwyrm/tests/views/test_directory.py +++ b/bookwyrm/tests/views/test_directory.py @@ -18,7 +18,7 @@ class DirectoryViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -34,6 +34,7 @@ class DirectoryViews(TestCase): @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") + @patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.suggested_users.rerank_user_task.delay") def test_directory_page(self, *_): """there are so many views, this just makes sure it LOADS""" diff --git a/bookwyrm/tests/views/test_discover.py b/bookwyrm/tests/views/test_discover.py index b2a82241..bff420c8 100644 --- a/bookwyrm/tests/views/test_discover.py +++ b/bookwyrm/tests/views/test_discover.py @@ -6,6 +6,7 @@ from django.test.client import RequestFactory from bookwyrm import models from bookwyrm import views +from bookwyrm.tests.validate_html import validate_html class DiscoverViews(TestCase): @@ -16,7 +17,7 @@ class DiscoverViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", @@ -39,7 +40,7 @@ class DiscoverViews(TestCase): result = view(request) self.assertEqual(mock.call_count, 1) self.assertEqual(result.status_code, 200) - result.render() + validate_html(result.render()) @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @patch("bookwyrm.activitystreams.add_status_task.delay") @@ -67,7 +68,7 @@ class DiscoverViews(TestCase): result = view(request) self.assertEqual(mock.call_count, 1) self.assertEqual(result.status_code, 200) - result.render() + validate_html(result.render()) def test_discover_page_logged_out(self): """there are so many views, this just makes sure it LOADS""" diff --git a/bookwyrm/tests/views/test_feed.py b/bookwyrm/tests/views/test_feed.py index 5c6a4dd3..7f8db948 100644 --- a/bookwyrm/tests/views/test_feed.py +++ b/bookwyrm/tests/views/test_feed.py @@ -10,8 +10,7 @@ from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory -from bookwyrm import models -from bookwyrm import views +from bookwyrm import forms, models, views from bookwyrm.activitypub import ActivitypubResponse @@ -19,6 +18,7 @@ from bookwyrm.activitypub import ActivitypubResponse @patch("bookwyrm.activitystreams.add_status_task.delay") @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.suggested_users.remove_user_task.delay") class FeedViews(TestCase): """activity feed, statuses, dms""" @@ -28,7 +28,7 @@ class FeedViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", @@ -54,6 +54,25 @@ class FeedViews(TestCase): result.render() self.assertEqual(result.status_code, 200) + @patch("bookwyrm.suggested_users.SuggestedUsers.get_suggestions") + def test_save_feed_settings(self, *_): + """update display preferences""" + self.assertEqual( + self.local_user.feed_status_types, + ["review", "comment", "quotation", "everything"], + ) + view = views.Feed.as_view() + form = forms.FeedStatusTypesForm(instance=self.local_user) + form.data["feed_status_types"] = "review" + request = self.factory.post("", form.data) + request.user = self.local_user + + result = view(request, "home") + + self.assertEqual(result.status_code, 200) + self.local_user.refresh_from_db() + self.assertEqual(self.local_user.feed_status_types, ["review"]) + def test_status_page(self, *_): """there are so many views, this just makes sure it LOADS""" view = views.Status.as_view() diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index 046fe3d9..d18e24f8 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -13,6 +13,7 @@ from bookwyrm.tests.validate_html import validate_html @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") +@patch("bookwyrm.lists_stream.add_user_lists_task.delay") class FollowViews(TestCase): """follows""" @@ -22,7 +23,7 @@ class FollowViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -56,7 +57,7 @@ class FollowViews(TestCase): parent_work=self.work, ) - def test_handle_follow_remote(self, _): + def test_handle_follow_remote(self, *_): """send a follow request""" request = self.factory.post("", {"user": self.remote_user.username}) request.user = self.local_user @@ -71,11 +72,11 @@ class FollowViews(TestCase): self.assertEqual(rel.user_object, self.remote_user) self.assertEqual(rel.status, "follow_request") - def test_handle_follow_local_manually_approves(self, _): + def test_handle_follow_local_manually_approves(self, *_): """send a follow request""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): rat = models.User.objects.create_user( "rat@local.com", "rat@rat.com", @@ -97,11 +98,11 @@ class FollowViews(TestCase): self.assertEqual(rel.user_object, rat) self.assertEqual(rel.status, "follow_request") - def test_handle_follow_local(self, _): + def test_handle_follow_local(self, *_): """send a follow request""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): rat = models.User.objects.create_user( "rat@local.com", "rat@rat.com", @@ -124,6 +125,7 @@ class FollowViews(TestCase): self.assertEqual(rel.status, "follows") @patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") + @patch("bookwyrm.lists_stream.remove_user_lists_task.delay") def test_handle_unfollow(self, *_): """send an unfollow""" request = self.factory.post("", {"user": self.remote_user.username}) @@ -140,7 +142,7 @@ class FollowViews(TestCase): self.assertEqual(self.remote_user.followers.count(), 0) - def test_handle_accept(self, _): + def test_handle_accept(self, *_): """accept a follow request""" self.local_user.manually_approves_followers = True self.local_user.save( @@ -159,7 +161,7 @@ class FollowViews(TestCase): # follow relationship should exist self.assertEqual(self.local_user.followers.first(), self.remote_user) - def test_handle_reject(self, _): + def test_handle_reject(self, *_): """reject a follow request""" self.local_user.manually_approves_followers = True self.local_user.save( @@ -178,7 +180,7 @@ class FollowViews(TestCase): # follow relationship should not exist self.assertEqual(models.UserFollows.objects.filter(id=rel.id).count(), 0) - def test_ostatus_follow_request(self, _): + def test_ostatus_follow_request(self, *_): """check ostatus subscribe template loads""" request = self.factory.get( "", {"acct": "https%3A%2F%2Fexample.com%2Fusers%2Frat"} @@ -189,7 +191,7 @@ class FollowViews(TestCase): validate_html(result.render()) self.assertEqual(result.status_code, 200) - def test_remote_follow_page(self, _): + def test_remote_follow_page(self, *_): """check remote follow page loads""" request = self.factory.get("", {"acct": "mouse@local.com"}) request.user = self.remote_user @@ -198,7 +200,7 @@ class FollowViews(TestCase): validate_html(result.render()) self.assertEqual(result.status_code, 200) - def test_ostatus_follow_success(self, _): + def test_ostatus_follow_success(self, *_): """check remote follow success page loads""" request = self.factory.get("") request.user = self.remote_user @@ -208,7 +210,7 @@ class FollowViews(TestCase): validate_html(result.render()) self.assertEqual(result.status_code, 200) - def test_remote_follow(self, _): + def test_remote_follow(self, *_): """check follow from remote page loads""" request = self.factory.post("", {"user": self.remote_user.id}) request.user = self.remote_user diff --git a/bookwyrm/tests/views/test_get_started.py b/bookwyrm/tests/views/test_get_started.py index 6d1819a4..a54940e3 100644 --- a/bookwyrm/tests/views/test_get_started.py +++ b/bookwyrm/tests/views/test_get_started.py @@ -16,7 +16,7 @@ class GetStartedViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/test_goal.py b/bookwyrm/tests/views/test_goal.py index 73207240..0faeef11 100644 --- a/bookwyrm/tests/views/test_goal.py +++ b/bookwyrm/tests/views/test_goal.py @@ -20,7 +20,7 @@ class GoalViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_group.py b/bookwyrm/tests/views/test_group.py index b18ce6b4..87776dc5 100644 --- a/bookwyrm/tests/views/test_group.py +++ b/bookwyrm/tests/views/test_group.py @@ -19,7 +19,7 @@ class GroupViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/test_helpers.py b/bookwyrm/tests/views/test_helpers.py index 1aae830f..8fe04f51 100644 --- a/bookwyrm/tests/views/test_helpers.py +++ b/bookwyrm/tests/views/test_helpers.py @@ -23,7 +23,7 @@ class ViewsHelpers(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): with patch("bookwyrm.suggested_users.rerank_user_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index aa402952..1d729f9a 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -17,7 +17,7 @@ class InteractionViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_isbn.py b/bookwyrm/tests/views/test_isbn.py index bdf72f75..7c18b4ab 100644 --- a/bookwyrm/tests/views/test_isbn.py +++ b/bookwyrm/tests/views/test_isbn.py @@ -18,7 +18,7 @@ class IsbnViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -49,3 +49,13 @@ class IsbnViews(TestCase): self.assertEqual(len(data), 1) self.assertEqual(data[0]["title"], "Test Book") self.assertEqual(data[0]["key"], f"https://{DOMAIN}/book/{self.book.id}") + + def test_isbn_html_response(self): + """searches local data only and returns book data in json format""" + view = views.Isbn.as_view() + request = self.factory.get("") + with patch("bookwyrm.views.isbn.is_api_request") as is_api: + is_api.return_value = False + response = view(request, isbn="1234567890123") + self.assertEqual(response.status_code, 200) + response.render() diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py index fd4d2d76..aa942a39 100644 --- a/bookwyrm/tests/views/test_list.py +++ b/bookwyrm/tests/views/test_list.py @@ -20,7 +20,7 @@ class ListViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -71,10 +71,13 @@ class ListViews(TestCase): models.SiteSettings.objects.create() - def test_lists_page(self): + @patch("bookwyrm.lists_stream.ListsStream.get_activity_stream") + def test_lists_page(self, _): """there are so many views, this just makes sure it LOADS""" view = views.Lists.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.add_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 @@ -345,13 +348,16 @@ class ListViews(TestCase): def test_curate_page(self): """there are so many views, this just makes sure it LOADS""" view = views.Curate.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): - models.List.objects.create(name="Public list", user=self.local_user) - models.List.objects.create( - name="Private list", privacy="direct", user=self.local_user - ) request = self.factory.get("") request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=False, + order=1, + ) result = view(request, self.list.id) self.assertIsInstance(result, TemplateResponse) @@ -404,7 +410,7 @@ class ListViews(TestCase): with patch("bookwyrm.views.list.is_api_request") as is_api: is_api.return_value = False with self.assertRaises(Http404): - result = view(request, self.list.id, "") + view(request, self.list.id, "") def test_embed_call_with_key(self): """there are so many views, this just makes sure it LOADS""" diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/test_list_actions.py index 1d9f46b3..0d04a8f2 100644 --- a/bookwyrm/tests/views/test_list_actions.py +++ b/bookwyrm/tests/views/test_list_actions.py @@ -18,7 +18,7 @@ class ListActionViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -90,8 +90,9 @@ class ListActionViews(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") as redis_mock: views.delete_list(request, self.list.id) + self.assertTrue(redis_mock.called) activity = json.loads(mock.call_args[1]["args"][1]) self.assertEqual(activity["type"], "Delete") self.assertEqual(activity["actor"], self.local_user.remote_id) @@ -123,10 +124,7 @@ class ListActionViews(TestCase): request = self.factory.post( "", - { - "item": pending.id, - "approved": "true", - }, + {"item": pending.id, "approved": "true"}, ) request.user = self.local_user diff --git a/bookwyrm/tests/views/test_notifications.py b/bookwyrm/tests/views/test_notifications.py index 5df62b1d..2a5cf798 100644 --- a/bookwyrm/tests/views/test_notifications.py +++ b/bookwyrm/tests/views/test_notifications.py @@ -17,7 +17,7 @@ class NotificationViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/test_outbox.py b/bookwyrm/tests/views/test_outbox.py index 5c5d47b0..598cce51 100644 --- a/bookwyrm/tests/views/test_outbox.py +++ b/bookwyrm/tests/views/test_outbox.py @@ -20,7 +20,7 @@ class OutboxView(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_reading.py b/bookwyrm/tests/views/test_reading.py index 4ec50165..0d6c13aa 100644 --- a/bookwyrm/tests/views/test_reading.py +++ b/bookwyrm/tests/views/test_reading.py @@ -20,7 +20,7 @@ class ReadingViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_readthrough.py b/bookwyrm/tests/views/test_readthrough.py index 5b554748..6697c0e0 100644 --- a/bookwyrm/tests/views/test_readthrough.py +++ b/bookwyrm/tests/views/test_readthrough.py @@ -27,7 +27,7 @@ class ReadThrough(TestCase): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.user = models.User.objects.create_user( "cinco", "cinco@example.com", "seissiete", local=True, localname="cinco" ) diff --git a/bookwyrm/tests/views/test_rss_feed.py b/bookwyrm/tests/views/test_rss_feed.py index 409c306d..8b3ecf58 100644 --- a/bookwyrm/tests/views/test_rss_feed.py +++ b/bookwyrm/tests/views/test_rss_feed.py @@ -15,7 +15,7 @@ class RssFeedView(TestCase): def setUp(self): with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "rss_user", "rss@test.rss", "password", local=True ) diff --git a/bookwyrm/tests/views/test_search.py b/bookwyrm/tests/views/test_search.py index f8045511..a7822f1c 100644 --- a/bookwyrm/tests/views/test_search.py +++ b/bookwyrm/tests/views/test_search.py @@ -22,7 +22,7 @@ class Views(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index b5d7ac16..35507157 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -13,6 +13,7 @@ from bookwyrm.tests.validate_html import validate_html # pylint: disable=invalid-name @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.activitystreams.remove_status_task.delay") @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") class StatusViews(TestCase): @@ -23,7 +24,7 @@ class StatusViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_updates.py b/bookwyrm/tests/views/test_updates.py index e7b466cc..d5d1d881 100644 --- a/bookwyrm/tests/views/test_updates.py +++ b/bookwyrm/tests/views/test_updates.py @@ -17,7 +17,7 @@ class UpdateViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/test_user.py b/bookwyrm/tests/views/test_user.py index ddb029cc..869d04af 100644 --- a/bookwyrm/tests/views/test_user.py +++ b/bookwyrm/tests/views/test_user.py @@ -20,7 +20,7 @@ class UserViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/test_wellknown.py b/bookwyrm/tests/views/test_wellknown.py index ecb6a67a..465f39b4 100644 --- a/bookwyrm/tests/views/test_wellknown.py +++ b/bookwyrm/tests/views/test_wellknown.py @@ -18,7 +18,7 @@ class WellknownViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index ba6c3af5..4dd76499 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -28,7 +28,9 @@ class Feed(View): settings_saved = False form = forms.FeedStatusTypesForm(request.POST, instance=request.user) if form.is_valid(): - form.save() + # workaround to avoid broadcasting this change + user = form.save(commit=False) + user.save(broadcast=False, update_fields=["feed_status_types"]) settings_saved = True return self.get(request, tab, settings_saved) From 074c2cfb957b0b4cb3851cc79fc8917b62263856 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 18:16:34 -0800 Subject: [PATCH 12/38] Gets updates view to 100% test coverage --- bookwyrm/tests/views/test_updates.py | 11 ++++++++++- bookwyrm/views/updates.py | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/views/test_updates.py b/bookwyrm/tests/views/test_updates.py index d5d1d881..4ca704fc 100644 --- a/bookwyrm/tests/views/test_updates.py +++ b/bookwyrm/tests/views/test_updates.py @@ -2,7 +2,7 @@ import json from unittest.mock import patch -from django.http import JsonResponse +from django.http import Http404, JsonResponse from django.test import TestCase from django.test.client import RequestFactory @@ -54,6 +54,7 @@ class UpdateViews(TestCase): "bookwyrm.activitystreams.ActivityStream.get_unread_count" ) as mock_count: with patch( + # pylint:disable=line-too-long "bookwyrm.activitystreams.ActivityStream.get_unread_count_by_status_type" ) as mock_count_by_status: mock_count.return_value = 3 @@ -64,3 +65,11 @@ class UpdateViews(TestCase): data = json.loads(result.getvalue()) self.assertEqual(data["count"], 3) self.assertEqual(data["count_by_type"]["review"], 5) + + def test_get_unread_status_count_invalid_stream(self): + """there are so many views, this just makes sure it LOADS""" + request = self.factory.get("") + request.user = self.local_user + + with self.assertRaises(Http404): + views.get_unread_status_count(request, "fish") diff --git a/bookwyrm/views/updates.py b/bookwyrm/views/updates.py index 2bbc5477..765865ef 100644 --- a/bookwyrm/views/updates.py +++ b/bookwyrm/views/updates.py @@ -1,6 +1,6 @@ """ endpoints for getting updates about activity """ from django.contrib.auth.decorators import login_required -from django.http import JsonResponse +from django.http import Http404, JsonResponse from bookwyrm import activitystreams @@ -21,7 +21,7 @@ def get_unread_status_count(request, stream="home"): """any unread statuses for this feed?""" stream = activitystreams.streams.get(stream) if not stream: - return JsonResponse({}) + raise Http404 return JsonResponse( { "count": stream.get_unread_count(request.user), From d9d0919ad4cd845ba3b8c4e5f09df56e62c1f1dd Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 19:14:10 -0800 Subject: [PATCH 13/38] More mocks for more tests --- bookwyrm/tests/lists_stream/test_signals.py | 2 +- bookwyrm/tests/lists_stream/test_stream.py | 2 +- bookwyrm/tests/lists_stream/test_tasks.py | 2 +- bookwyrm/tests/management/test_populate_lists_streams.py | 2 +- bookwyrm/tests/management/test_populate_streams.py | 2 +- bookwyrm/tests/views/admin/test_announcements.py | 2 +- bookwyrm/tests/views/admin/test_dashboard.py | 2 +- bookwyrm/tests/views/admin/test_email_blocks.py | 2 +- bookwyrm/tests/views/admin/test_federation.py | 2 +- bookwyrm/tests/views/admin/test_ip_blocklist.py | 2 +- bookwyrm/tests/views/admin/test_reports.py | 2 +- bookwyrm/tests/views/admin/test_user_admin.py | 2 +- bookwyrm/tests/views/books/test_book.py | 2 +- bookwyrm/tests/views/books/test_edit_book.py | 2 +- bookwyrm/tests/views/books/test_editions.py | 2 +- bookwyrm/tests/views/landing/test_invite.py | 2 +- bookwyrm/tests/views/landing/test_landing.py | 2 +- bookwyrm/tests/views/landing/test_login.py | 2 +- bookwyrm/tests/views/landing/test_password.py | 2 +- bookwyrm/tests/views/landing/test_register.py | 3 ++- bookwyrm/tests/views/preferences/test_block.py | 2 +- bookwyrm/tests/views/preferences/test_change_password.py | 2 +- bookwyrm/tests/views/preferences/test_delete_user.py | 2 +- bookwyrm/tests/views/preferences/test_edit_user.py | 2 +- bookwyrm/tests/views/shelf/test_shelf.py | 3 ++- bookwyrm/tests/views/shelf/test_shelf_actions.py | 5 +++-- 26 files changed, 30 insertions(+), 27 deletions(-) diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 7640ea98..7d7cfee7 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -12,7 +12,7 @@ class ListsStreamSignals(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/lists_stream/test_stream.py b/bookwyrm/tests/lists_stream/test_stream.py index 25241793..e7157152 100644 --- a/bookwyrm/tests/lists_stream/test_stream.py +++ b/bookwyrm/tests/lists_stream/test_stream.py @@ -16,7 +16,7 @@ class ListsStream(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py index 1e5f7a98..70c112bd 100644 --- a/bookwyrm/tests/lists_stream/test_tasks.py +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -11,7 +11,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/management/test_populate_lists_streams.py b/bookwyrm/tests/management/test_populate_lists_streams.py index 0a586d86..45c5eb80 100644 --- a/bookwyrm/tests/management/test_populate_lists_streams.py +++ b/bookwyrm/tests/management/test_populate_lists_streams.py @@ -14,7 +14,7 @@ class Activitystreams(TestCase): """we need some stuff""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/management/test_populate_streams.py b/bookwyrm/tests/management/test_populate_streams.py index cee8b381..c527b27b 100644 --- a/bookwyrm/tests/management/test_populate_streams.py +++ b/bookwyrm/tests/management/test_populate_streams.py @@ -14,7 +14,7 @@ class Activitystreams(TestCase): """we need some stuff""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/views/admin/test_announcements.py b/bookwyrm/tests/views/admin/test_announcements.py index 44b5d5b6..08cab350 100644 --- a/bookwyrm/tests/views/admin/test_announcements.py +++ b/bookwyrm/tests/views/admin/test_announcements.py @@ -15,7 +15,7 @@ class AnnouncementViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_dashboard.py b/bookwyrm/tests/views/admin/test_dashboard.py index f0238eef..d05772c2 100644 --- a/bookwyrm/tests/views/admin/test_dashboard.py +++ b/bookwyrm/tests/views/admin/test_dashboard.py @@ -16,7 +16,7 @@ class DashboardViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_email_blocks.py b/bookwyrm/tests/views/admin/test_email_blocks.py index 6d676fd2..4fe9412e 100644 --- a/bookwyrm/tests/views/admin/test_email_blocks.py +++ b/bookwyrm/tests/views/admin/test_email_blocks.py @@ -17,7 +17,7 @@ class EmailBlocklistViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_federation.py b/bookwyrm/tests/views/admin/test_federation.py index fbbd540e..deed5bd3 100644 --- a/bookwyrm/tests/views/admin/test_federation.py +++ b/bookwyrm/tests/views/admin/test_federation.py @@ -19,7 +19,7 @@ class FederationViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_ip_blocklist.py b/bookwyrm/tests/views/admin/test_ip_blocklist.py index 25694d07..e23abd8b 100644 --- a/bookwyrm/tests/views/admin/test_ip_blocklist.py +++ b/bookwyrm/tests/views/admin/test_ip_blocklist.py @@ -16,7 +16,7 @@ class IPBlocklistViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_reports.py b/bookwyrm/tests/views/admin/test_reports.py index 8b9fe9f5..137d17cb 100644 --- a/bookwyrm/tests/views/admin/test_reports.py +++ b/bookwyrm/tests/views/admin/test_reports.py @@ -18,7 +18,7 @@ class ReportViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/admin/test_user_admin.py b/bookwyrm/tests/views/admin/test_user_admin.py index 486fe45e..4cb3702d 100644 --- a/bookwyrm/tests/views/admin/test_user_admin.py +++ b/bookwyrm/tests/views/admin/test_user_admin.py @@ -18,7 +18,7 @@ class UserAdminViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/books/test_book.py b/bookwyrm/tests/views/books/test_book.py index cb73e381..4d937071 100644 --- a/bookwyrm/tests/views/books/test_book.py +++ b/bookwyrm/tests/views/books/test_book.py @@ -28,7 +28,7 @@ class BookViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/books/test_edit_book.py b/bookwyrm/tests/views/books/test_edit_book.py index cd957858..cfb0d766 100644 --- a/bookwyrm/tests/views/books/test_edit_book.py +++ b/bookwyrm/tests/views/books/test_edit_book.py @@ -21,7 +21,7 @@ class EditBookViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/books/test_editions.py b/bookwyrm/tests/views/books/test_editions.py index 17f15654..70a95051 100644 --- a/bookwyrm/tests/views/books/test_editions.py +++ b/bookwyrm/tests/views/books/test_editions.py @@ -18,7 +18,7 @@ class BookViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/landing/test_invite.py b/bookwyrm/tests/views/landing/test_invite.py index a93821a9..a5877187 100644 --- a/bookwyrm/tests/views/landing/test_invite.py +++ b/bookwyrm/tests/views/landing/test_invite.py @@ -19,7 +19,7 @@ class InviteViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/landing/test_landing.py b/bookwyrm/tests/views/landing/test_landing.py index 82991917..37ce6cfc 100644 --- a/bookwyrm/tests/views/landing/test_landing.py +++ b/bookwyrm/tests/views/landing/test_landing.py @@ -18,7 +18,7 @@ class LandingViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/landing/test_login.py b/bookwyrm/tests/views/landing/test_login.py index 0f86fb79..24987c8e 100644 --- a/bookwyrm/tests/views/landing/test_login.py +++ b/bookwyrm/tests/views/landing/test_login.py @@ -21,7 +21,7 @@ class LoginViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@your.domain.here", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/landing/test_password.py b/bookwyrm/tests/views/landing/test_password.py index f01565ef..b1f7e59f 100644 --- a/bookwyrm/tests/views/landing/test_password.py +++ b/bookwyrm/tests/views/landing/test_password.py @@ -21,7 +21,7 @@ class PasswordViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/landing/test_register.py b/bookwyrm/tests/views/landing/test_register.py index 99f38da2..deb16552 100644 --- a/bookwyrm/tests/views/landing/test_register.py +++ b/bookwyrm/tests/views/landing/test_register.py @@ -16,6 +16,7 @@ from bookwyrm.tests.validate_html import validate_html # pylint: disable=too-many-public-methods @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") class RegisterViews(TestCase): """login and password management""" @@ -24,7 +25,7 @@ class RegisterViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@your.domain.here", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/preferences/test_block.py b/bookwyrm/tests/views/preferences/test_block.py index 975142a1..262eee74 100644 --- a/bookwyrm/tests/views/preferences/test_block.py +++ b/bookwyrm/tests/views/preferences/test_block.py @@ -18,7 +18,7 @@ class BlockViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/preferences/test_change_password.py b/bookwyrm/tests/views/preferences/test_change_password.py index d13c2af5..61837c4e 100644 --- a/bookwyrm/tests/views/preferences/test_change_password.py +++ b/bookwyrm/tests/views/preferences/test_change_password.py @@ -17,7 +17,7 @@ class ChangePasswordViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/preferences/test_delete_user.py b/bookwyrm/tests/views/preferences/test_delete_user.py index b6d87ccd..95bfa4df 100644 --- a/bookwyrm/tests/views/preferences/test_delete_user.py +++ b/bookwyrm/tests/views/preferences/test_delete_user.py @@ -20,7 +20,7 @@ class DeleteUserViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/preferences/test_edit_user.py b/bookwyrm/tests/views/preferences/test_edit_user.py index 7a845fbe..11d33340 100644 --- a/bookwyrm/tests/views/preferences/test_edit_user.py +++ b/bookwyrm/tests/views/preferences/test_edit_user.py @@ -23,7 +23,7 @@ class EditUserViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/shelf/test_shelf.py b/bookwyrm/tests/views/shelf/test_shelf.py index ab88de0a..5f788391 100644 --- a/bookwyrm/tests/views/shelf/test_shelf.py +++ b/bookwyrm/tests/views/shelf/test_shelf.py @@ -14,6 +14,7 @@ from bookwyrm.tests.validate_html import validate_html @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.activitystreams.add_book_statuses_task.delay") @patch("bookwyrm.activitystreams.remove_book_statuses_task.delay") class ShelfViews(TestCase): @@ -24,7 +25,7 @@ class ShelfViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/shelf/test_shelf_actions.py b/bookwyrm/tests/views/shelf/test_shelf_actions.py index 1a7d56fd..93ff0a38 100644 --- a/bookwyrm/tests/views/shelf/test_shelf_actions.py +++ b/bookwyrm/tests/views/shelf/test_shelf_actions.py @@ -12,6 +12,7 @@ from bookwyrm import forms, models, views @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.lists_stream.populate_lists_task.delay") @patch("bookwyrm.activitystreams.add_book_statuses_task.delay") @patch("bookwyrm.activitystreams.remove_book_statuses_task.delay") class ShelfActionViews(TestCase): @@ -22,7 +23,7 @@ class ShelfActionViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", @@ -178,7 +179,7 @@ class ShelfActionViews(TestCase): """delete a brand new custom shelf""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): rat = models.User.objects.create_user( "rat@local.com", "rat@mouse.mouse", From acde30887d70e948ace4c3e2765f8000c499c304 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 20:04:47 -0800 Subject: [PATCH 14/38] Importer tests --- .../tests/importers/test_goodreads_import.py | 2 +- bookwyrm/tests/importers/test_importer.py | 2 +- .../importers/test_librarything_import.py | 2 +- .../tests/importers/test_storygraph_import.py | 2 +- bookwyrm/tests/views/imports/test_import.py | 44 +++++++++++++--- .../tests/views/imports/test_import_review.py | 2 +- .../views/imports/test_import_troubleshoot.py | 2 +- bookwyrm/views/imports/import_data.py | 51 +++++++++---------- 8 files changed, 69 insertions(+), 38 deletions(-) diff --git a/bookwyrm/tests/importers/test_goodreads_import.py b/bookwyrm/tests/importers/test_goodreads_import.py index 0a421df4..04fb886b 100644 --- a/bookwyrm/tests/importers/test_goodreads_import.py +++ b/bookwyrm/tests/importers/test_goodreads_import.py @@ -30,7 +30,7 @@ class GoodreadsImport(TestCase): self.csv = open(datafile, "r", encoding=self.importer.encoding) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True ) diff --git a/bookwyrm/tests/importers/test_importer.py b/bookwyrm/tests/importers/test_importer.py index 5c3b2031..87f0d209 100644 --- a/bookwyrm/tests/importers/test_importer.py +++ b/bookwyrm/tests/importers/test_importer.py @@ -33,7 +33,7 @@ class GenericImporter(TestCase): self.csv = open(datafile, "r", encoding=self.importer.encoding) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True ) diff --git a/bookwyrm/tests/importers/test_librarything_import.py b/bookwyrm/tests/importers/test_librarything_import.py index 49354b36..57d55520 100644 --- a/bookwyrm/tests/importers/test_librarything_import.py +++ b/bookwyrm/tests/importers/test_librarything_import.py @@ -32,7 +32,7 @@ class LibrarythingImport(TestCase): self.csv = open(datafile, "r", encoding=self.importer.encoding) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mmai", "mmai@mmai.mmai", "password", local=True ) diff --git a/bookwyrm/tests/importers/test_storygraph_import.py b/bookwyrm/tests/importers/test_storygraph_import.py index 09cf32dc..e6d1c5c0 100644 --- a/bookwyrm/tests/importers/test_storygraph_import.py +++ b/bookwyrm/tests/importers/test_storygraph_import.py @@ -30,7 +30,7 @@ class StorygraphImport(TestCase): self.csv = open(datafile, "r", encoding=self.importer.encoding) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True ) diff --git a/bookwyrm/tests/views/imports/test_import.py b/bookwyrm/tests/views/imports/test_import.py index b8b8b328..362cb199 100644 --- a/bookwyrm/tests/views/imports/test_import.py +++ b/bookwyrm/tests/views/imports/test_import.py @@ -1,13 +1,14 @@ """ test for app action functionality """ import pathlib from unittest.mock import patch + from django.core.files.uploadedfile import SimpleUploadedFile from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory -from bookwyrm.tests.validate_html import validate_html from bookwyrm import forms, models, views +from bookwyrm.tests.validate_html import validate_html class ImportViews(TestCase): @@ -18,7 +19,7 @@ class ImportViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", @@ -44,15 +45,29 @@ class ImportViews(TestCase): import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) request = self.factory.get("") request.user = self.local_user - with patch("bookwyrm.tasks.app.AsyncResult") as async_result: - async_result.return_value = [] - result = view(request, import_job.id) + + result = view(request, import_job.id) + self.assertIsInstance(result, TemplateResponse) validate_html(result.render()) self.assertEqual(result.status_code, 200) + def test_import_status_reformat(self): + """there are so many views, this just makes sure it LOADS""" + view = views.ImportStatus.as_view() + import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) + request = self.factory.post("") + request.user = self.local_user + with patch( + "bookwyrm.importers.goodreads_import.GoodreadsImporter.update_legacy_job" + ) as mock: + result = view(request, import_job.id) + self.assertEqual(mock.call_args[0][0], import_job) + + self.assertEqual(result.status_code, 302) + def test_start_import(self): - """retry failed items""" + """start a job""" view = views.Import.as_view() form = forms.ImportForm() form.data["source"] = "Goodreads" @@ -74,3 +89,20 @@ class ImportViews(TestCase): job = models.ImportJob.objects.get() self.assertFalse(job.include_reviews) self.assertEqual(job.privacy, "public") + + def test_retry_item(self): + """try again on a single row""" + job = models.ImportJob.objects.create(user=self.local_user, mappings={}) + item = models.ImportItem.objects.create( + index=0, + job=job, + book_guess=self.book, + fail_reason="no match", + data={}, + normalized_data={}, + ) + request = self.factory.post("") + request.user = self.local_user + with patch("bookwyrm.importers.importer.import_item_task.delay") as mock: + views.retry_item(request, job.id, item.id) + self.assertEqual(mock.call_count, 1) diff --git a/bookwyrm/tests/views/imports/test_import_review.py b/bookwyrm/tests/views/imports/test_import_review.py index 2ab48468..9ed4532e 100644 --- a/bookwyrm/tests/views/imports/test_import_review.py +++ b/bookwyrm/tests/views/imports/test_import_review.py @@ -16,7 +16,7 @@ class ImportManualReviewViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/tests/views/imports/test_import_troubleshoot.py b/bookwyrm/tests/views/imports/test_import_troubleshoot.py index 5359cc1e..b39f6d9e 100644 --- a/bookwyrm/tests/views/imports/test_import_troubleshoot.py +++ b/bookwyrm/tests/views/imports/test_import_troubleshoot.py @@ -16,7 +16,7 @@ class ImportTroubleshootViews(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.mouse", diff --git a/bookwyrm/views/imports/import_data.py b/bookwyrm/views/imports/import_data.py index 7f6a4d13..80386b3d 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -37,33 +37,32 @@ class Import(View): def post(self, request): """ingest a goodreads csv""" form = forms.ImportForm(request.POST, request.FILES) - if form.is_valid(): - include_reviews = request.POST.get("include_reviews") == "on" - privacy = request.POST.get("privacy") - source = request.POST.get("source") + if not form.is_valid(): + return HttpResponseBadRequest() - importer = None - if source == "LibraryThing": - importer = LibrarythingImporter() - elif source == "Storygraph": - importer = StorygraphImporter() - else: - # Default : Goodreads - importer = GoodreadsImporter() + include_reviews = request.POST.get("include_reviews") == "on" + privacy = request.POST.get("privacy") + source = request.POST.get("source") - try: - job = importer.create_job( - request.user, - TextIOWrapper( - request.FILES["csv_file"], encoding=importer.encoding - ), - include_reviews, - privacy, - ) - except (UnicodeDecodeError, ValueError, KeyError): - return HttpResponseBadRequest(_("Not a valid csv file")) + importer = None + if source == "LibraryThing": + importer = LibrarythingImporter() + elif source == "Storygraph": + importer = StorygraphImporter() + else: + # Default : Goodreads + importer = GoodreadsImporter() - importer.start_import(job) + try: + job = importer.create_job( + request.user, + TextIOWrapper(request.FILES["csv_file"], encoding=importer.encoding), + include_reviews, + privacy, + ) + except (UnicodeDecodeError, ValueError, KeyError): + return HttpResponseBadRequest(_("Not a valid csv file")) - return redirect(f"/import/{job.id}") - return HttpResponseBadRequest() + importer.start_import(job) + + return redirect(f"/import/{job.id}") From 5e6b28bbc0b392a37880015968825570c98db0ca Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 20:30:53 -0800 Subject: [PATCH 15/38] Prefs mocks --- bookwyrm/tests/activitypub/test_base_activity.py | 2 +- bookwyrm/tests/views/preferences/test_block.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index b951c7ab..973e57bf 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -31,7 +31,7 @@ class BaseActivity(TestCase): """we're probably going to re-use this so why copy/paste""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/views/preferences/test_block.py b/bookwyrm/tests/views/preferences/test_block.py index 262eee74..46de8f48 100644 --- a/bookwyrm/tests/views/preferences/test_block.py +++ b/bookwyrm/tests/views/preferences/test_block.py @@ -61,7 +61,9 @@ class BlockViews(TestCase): request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): + with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"), patch( + "bookwyrm.lists_stream.remove_user_lists_task.delay" + ): view(request, self.remote_user.id) block = models.UserBlocks.objects.get() self.assertEqual(block.user_subject, self.local_user) @@ -76,7 +78,9 @@ class BlockViews(TestCase): request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.activitystreams.add_user_statuses_task.delay"): + with patch("bookwyrm.activitystreams.add_user_statuses_task.delay"), patch( + "bookwyrm.lists_stream.add_user_lists_task.delay" + ): views.unblock(request, self.remote_user.id) self.assertFalse(models.UserBlocks.objects.exists()) From 0c193b6ce18c3793947cbf4dafdeee7976913d83 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 20:33:11 -0800 Subject: [PATCH 16/38] ActivityStream mocks --- bookwyrm/tests/activitystreams/test_abstractstream.py | 2 +- bookwyrm/tests/activitystreams/test_booksstream.py | 2 +- bookwyrm/tests/activitystreams/test_homestream.py | 2 +- bookwyrm/tests/activitystreams/test_localstream.py | 2 +- bookwyrm/tests/activitystreams/test_signals.py | 2 +- bookwyrm/tests/activitystreams/test_tasks.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bookwyrm/tests/activitystreams/test_abstractstream.py b/bookwyrm/tests/activitystreams/test_abstractstream.py index 17a1b587..2c5cf610 100644 --- a/bookwyrm/tests/activitystreams/test_abstractstream.py +++ b/bookwyrm/tests/activitystreams/test_abstractstream.py @@ -16,7 +16,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/activitystreams/test_booksstream.py b/bookwyrm/tests/activitystreams/test_booksstream.py index 347e7a94..c001d6dd 100644 --- a/bookwyrm/tests/activitystreams/test_booksstream.py +++ b/bookwyrm/tests/activitystreams/test_booksstream.py @@ -16,7 +16,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/activitystreams/test_homestream.py b/bookwyrm/tests/activitystreams/test_homestream.py index d48bdfba..10c806c8 100644 --- a/bookwyrm/tests/activitystreams/test_homestream.py +++ b/bookwyrm/tests/activitystreams/test_homestream.py @@ -16,7 +16,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/activitystreams/test_localstream.py b/bookwyrm/tests/activitystreams/test_localstream.py index fa1a6741..d8bfb4fa 100644 --- a/bookwyrm/tests/activitystreams/test_localstream.py +++ b/bookwyrm/tests/activitystreams/test_localstream.py @@ -16,7 +16,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/activitystreams/test_signals.py b/bookwyrm/tests/activitystreams/test_signals.py index 34aeb947..15f2b640 100644 --- a/bookwyrm/tests/activitystreams/test_signals.py +++ b/bookwyrm/tests/activitystreams/test_signals.py @@ -12,7 +12,7 @@ class ActivitystreamsSignals(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) diff --git a/bookwyrm/tests/activitystreams/test_tasks.py b/bookwyrm/tests/activitystreams/test_tasks.py index f5750763..af05cf18 100644 --- a/bookwyrm/tests/activitystreams/test_tasks.py +++ b/bookwyrm/tests/activitystreams/test_tasks.py @@ -11,7 +11,7 @@ class Activitystreams(TestCase): """use a test csv""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" ) From 59e6b67bc8936e13ceca52600e9d4883ff8b4cb9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 9 Dec 2021 20:38:44 -0800 Subject: [PATCH 17/38] Activitystreams mocks --- bookwyrm/tests/activitystreams/test_signals.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/bookwyrm/tests/activitystreams/test_signals.py b/bookwyrm/tests/activitystreams/test_signals.py index 15f2b640..c5c7e698 100644 --- a/bookwyrm/tests/activitystreams/test_signals.py +++ b/bookwyrm/tests/activitystreams/test_signals.py @@ -5,6 +5,8 @@ from bookwyrm import activitystreams, models @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") +@patch("bookwyrm.lists_stream.add_user_lists_task.delay") +@patch("bookwyrm.lists_stream.remove_user_lists_task.delay") class ActivitystreamsSignals(TestCase): """using redis to build activity streams""" @@ -32,11 +34,11 @@ class ActivitystreamsSignals(TestCase): work = models.Work.objects.create(title="test work") self.book = models.Edition.objects.create(title="test book", parent_work=work) - def test_add_status_on_create_ignore(self, _): + def test_add_status_on_create_ignore(self, *_): """a new statuses has entered""" activitystreams.add_status_on_create(models.User, self.local_user, False) - def test_add_status_on_create_deleted(self, _): + def test_add_status_on_create_deleted(self, *_): """a new statuses has entered""" with patch("bookwyrm.activitystreams.remove_status_task.delay"): status = models.Status.objects.create( @@ -48,7 +50,7 @@ class ActivitystreamsSignals(TestCase): args = mock.call_args[0] self.assertEqual(args[0], status.id) - def test_add_status_on_create_created(self, _): + def test_add_status_on_create_created(self, *_): """a new statuses has entered""" status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" @@ -60,7 +62,7 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args["args"][0], status.id) self.assertEqual(args["queue"], "high_priority") - def test_populate_streams_on_account_create(self, _): + def test_populate_streams_on_account_create(self, *_): """create streams for a user""" with patch("bookwyrm.activitystreams.populate_stream_task.delay") as mock: activitystreams.populate_streams_on_account_create( @@ -71,7 +73,7 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args[0], "books") self.assertEqual(args[1], self.local_user.id) - def test_remove_statuses_on_block(self, _): + def test_remove_statuses_on_block(self, *_): """don't show statuses from blocked users""" with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") as mock: models.UserBlocks.objects.create( @@ -83,7 +85,7 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args[0], self.local_user.id) self.assertEqual(args[1], self.remote_user.id) - def test_add_statuses_on_unblock(self, _): + def test_add_statuses_on_unblock(self, *_): """re-add statuses on unblock""" with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): block = models.UserBlocks.objects.create( @@ -100,7 +102,7 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args[1], self.remote_user.id) self.assertEqual(kwargs["stream_list"], ["local", "books"]) - def test_add_statuses_on_unblock_reciprocal_block(self, _): + def test_add_statuses_on_unblock_reciprocal_block(self, *_): """re-add statuses on unblock""" with patch("bookwyrm.activitystreams.remove_user_statuses_task.delay"): block = models.UserBlocks.objects.create( From 94250dab4240c65d8c67b8fe03b4c06acd316bf5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 10 Dec 2021 09:19:32 -0800 Subject: [PATCH 18/38] Mocks for list signals tests --- bookwyrm/tests/lists_stream/test_signals.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 7d7cfee7..91c6b48f 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -61,7 +61,8 @@ class ListsStreamSignals(TestCase): args = mock.call_args[0] self.assertEqual(args[0], self.local_user.id) - def test_remove_lists_on_block(self, _): + @patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") + def test_remove_lists_on_block(self, *_): """don't show lists from blocked users""" with patch("bookwyrm.lists_stream.remove_user_lists_task.delay") as mock: models.UserBlocks.objects.create( @@ -73,7 +74,9 @@ class ListsStreamSignals(TestCase): self.assertEqual(args[0], self.local_user.id) self.assertEqual(args[1], self.remote_user.id) - def test_add_lists_on_unblock(self, _): + @patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") + @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") + def test_add_lists_on_unblock(self, *_): """re-add lists on unblock""" with patch("bookwyrm.lists_stream.remove_user_lists_task.delay"): block = models.UserBlocks.objects.create( @@ -88,7 +91,9 @@ class ListsStreamSignals(TestCase): self.assertEqual(args[0], self.local_user.id) self.assertEqual(args[1], self.remote_user.id) - def test_add_lists_on_unblock_reciprocal_block(self, _): + @patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") + @patch("bookwyrm.activitystreams.add_user_statuses_task.delay") + def test_add_lists_on_unblock_reciprocal_block(self, *_): """dont' re-add lists on unblock if there's a block the other way""" with patch("bookwyrm.lists_stream.remove_user_lists_task.delay"): block = models.UserBlocks.objects.create( From f7c8a550cf843c87edd3a12c09d74b304ef04cc1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 10 Dec 2021 09:34:17 -0800 Subject: [PATCH 19/38] Fixes references to populate lists task --- bookwyrm/management/commands/populate_lists_streams.py | 2 +- bookwyrm/management/commands/populate_streams.py | 2 +- bookwyrm/tests/management/test_populate_lists_streams.py | 6 ++++-- bookwyrm/tests/management/test_populate_streams.py | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/bookwyrm/management/commands/populate_lists_streams.py b/bookwyrm/management/commands/populate_lists_streams.py index 1cf488df..502b3b92 100644 --- a/bookwyrm/management/commands/populate_lists_streams.py +++ b/bookwyrm/management/commands/populate_lists_streams.py @@ -13,7 +13,7 @@ def populate_lists_streams(): print("This may take a long time! Please be patient.") for user in users: print(".", end="") - lists_stream.populate_lists_stream_task.delay(user.id) + lists_stream.populate_lists_task.delay(user.id) class Command(BaseCommand): diff --git a/bookwyrm/management/commands/populate_streams.py b/bookwyrm/management/commands/populate_streams.py index 6b537650..5f83670c 100644 --- a/bookwyrm/management/commands/populate_streams.py +++ b/bookwyrm/management/commands/populate_streams.py @@ -14,7 +14,7 @@ def populate_streams(stream=None): print("This may take a long time! Please be patient.") for user in users: print(".", end="") - lists_stream.populate_lists_stream_task.delay(user.id) + lists_stream.populate_lists_task.delay(user.id) for stream_key in streams: print(".", end="") activitystreams.populate_stream_task.delay(stream_key, user.id) diff --git a/bookwyrm/tests/management/test_populate_lists_streams.py b/bookwyrm/tests/management/test_populate_lists_streams.py index 45c5eb80..f5047460 100644 --- a/bookwyrm/tests/management/test_populate_lists_streams.py +++ b/bookwyrm/tests/management/test_populate_lists_streams.py @@ -7,6 +7,8 @@ from bookwyrm.management.commands.populate_lists_streams import populate_lists_s @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") +@patch("bookwyrm.activitystreams.add_status_task.delay") +@patch("bookwyrm.activitystreams.populate_stream_task.delay") class Activitystreams(TestCase): """using redis to build activity streams""" @@ -45,10 +47,10 @@ class Activitystreams(TestCase): ) self.book = models.Edition.objects.create(title="test book") - def test_populate_streams(self, _): + def test_populate_streams(self, *_): """make sure the function on the redis manager gets called""" with patch( - "bookwyrm.lists_stream.populate_lists_stream_task.delay" + "bookwyrm.lists_stream.populate_lists_task.delay" ) as list_mock: populate_lists_streams() self.assertEqual(list_mock.call_count, 2) # 2 users diff --git a/bookwyrm/tests/management/test_populate_streams.py b/bookwyrm/tests/management/test_populate_streams.py index c527b27b..c20a21ac 100644 --- a/bookwyrm/tests/management/test_populate_streams.py +++ b/bookwyrm/tests/management/test_populate_streams.py @@ -55,7 +55,7 @@ class Activitystreams(TestCase): with patch( "bookwyrm.activitystreams.populate_stream_task.delay" ) as redis_mock, patch( - "bookwyrm.lists_stream.populate_lists_stream_task.delay" + "bookwyrm.lists_stream.populate_lists_task.delay" ) as list_mock: populate_streams() self.assertEqual(redis_mock.call_count, 6) # 2 users x 3 streams From b890e93533442aecc7e084c0603990fa55b67ee4 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 10 Dec 2021 09:53:17 -0800 Subject: [PATCH 20/38] Adds saved list view test --- .../management/test_populate_lists_streams.py | 4 +-- bookwyrm/tests/views/test_list_actions.py | 27 ++++++++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/bookwyrm/tests/management/test_populate_lists_streams.py b/bookwyrm/tests/management/test_populate_lists_streams.py index f5047460..2cce7b7a 100644 --- a/bookwyrm/tests/management/test_populate_lists_streams.py +++ b/bookwyrm/tests/management/test_populate_lists_streams.py @@ -49,8 +49,6 @@ class Activitystreams(TestCase): def test_populate_streams(self, *_): """make sure the function on the redis manager gets called""" - with patch( - "bookwyrm.lists_stream.populate_lists_task.delay" - ) as list_mock: + with patch("bookwyrm.lists_stream.populate_lists_task.delay") as list_mock: populate_lists_streams() self.assertEqual(list_mock.call_count, 2) # 2 users diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/test_list_actions.py index 0d04a8f2..7f57fae3 100644 --- a/bookwyrm/tests/views/test_list_actions.py +++ b/bookwyrm/tests/views/test_list_actions.py @@ -551,12 +551,7 @@ class ListActionViews(TestCase): ) self.assertTrue(self.list.listitem_set.exists()) - request = self.factory.post( - "", - { - "item": item.id, - }, - ) + request = self.factory.post("", {"item": item.id}) request.user = self.local_user with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): @@ -570,14 +565,22 @@ class ListActionViews(TestCase): book_list=self.list, user=self.local_user, book=self.book, order=1 ) self.assertTrue(self.list.listitem_set.exists()) - request = self.factory.post( - "", - { - "item": item.id, - }, - ) + request = self.factory.post("", {"item": item.id}) request.user = self.rat with self.assertRaises(PermissionDenied): views.list.remove_book(request, self.list.id) self.assertTrue(self.list.listitem_set.exists()) + + def test_save_unsave_list(self): + """bookmark a list""" + self.assertFalse(self.local_user.saved_lists.exists()) + request = self.factory.post("") + request.user = self.local_user + views.save_list(request, self.list.id) + self.local_user.refresh_from_db() + self.assertEqual(self.local_user.saved_lists.first(), self.list) + + views.unsave_list(request, self.list.id) + self.local_user.refresh_from_db() + self.assertFalse(self.local_user.saved_lists.exists()) From 9f04919becbb2c133ffd09745fdc97c8eff55b7f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 10 Dec 2021 11:44:38 -0800 Subject: [PATCH 21/38] Remove unrelated tests changes these will happen elsewhere --- bookwyrm/tests/views/imports/test_import.py | 42 ++--------------- bookwyrm/views/imports/import_data.py | 51 +++++++++++---------- 2 files changed, 31 insertions(+), 62 deletions(-) diff --git a/bookwyrm/tests/views/imports/test_import.py b/bookwyrm/tests/views/imports/test_import.py index 362cb199..763c2a52 100644 --- a/bookwyrm/tests/views/imports/test_import.py +++ b/bookwyrm/tests/views/imports/test_import.py @@ -1,14 +1,13 @@ """ test for app action functionality """ import pathlib from unittest.mock import patch - from django.core.files.uploadedfile import SimpleUploadedFile from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory +from bookwyrm.tests.validate_html import validate_html from bookwyrm import forms, models, views -from bookwyrm.tests.validate_html import validate_html class ImportViews(TestCase): @@ -45,29 +44,15 @@ class ImportViews(TestCase): import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) request = self.factory.get("") request.user = self.local_user - - result = view(request, import_job.id) - + with patch("bookwyrm.tasks.app.AsyncResult") as async_result: + async_result.return_value = [] + result = view(request, import_job.id) self.assertIsInstance(result, TemplateResponse) validate_html(result.render()) self.assertEqual(result.status_code, 200) - def test_import_status_reformat(self): - """there are so many views, this just makes sure it LOADS""" - view = views.ImportStatus.as_view() - import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) - request = self.factory.post("") - request.user = self.local_user - with patch( - "bookwyrm.importers.goodreads_import.GoodreadsImporter.update_legacy_job" - ) as mock: - result = view(request, import_job.id) - self.assertEqual(mock.call_args[0][0], import_job) - - self.assertEqual(result.status_code, 302) - def test_start_import(self): - """start a job""" + """retry failed items""" view = views.Import.as_view() form = forms.ImportForm() form.data["source"] = "Goodreads" @@ -89,20 +74,3 @@ class ImportViews(TestCase): job = models.ImportJob.objects.get() self.assertFalse(job.include_reviews) self.assertEqual(job.privacy, "public") - - def test_retry_item(self): - """try again on a single row""" - job = models.ImportJob.objects.create(user=self.local_user, mappings={}) - item = models.ImportItem.objects.create( - index=0, - job=job, - book_guess=self.book, - fail_reason="no match", - data={}, - normalized_data={}, - ) - request = self.factory.post("") - request.user = self.local_user - with patch("bookwyrm.importers.importer.import_item_task.delay") as mock: - views.retry_item(request, job.id, item.id) - self.assertEqual(mock.call_count, 1) diff --git a/bookwyrm/views/imports/import_data.py b/bookwyrm/views/imports/import_data.py index 80386b3d..7f6a4d13 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -37,32 +37,33 @@ class Import(View): def post(self, request): """ingest a goodreads csv""" form = forms.ImportForm(request.POST, request.FILES) - if not form.is_valid(): - return HttpResponseBadRequest() + if form.is_valid(): + include_reviews = request.POST.get("include_reviews") == "on" + privacy = request.POST.get("privacy") + source = request.POST.get("source") - include_reviews = request.POST.get("include_reviews") == "on" - privacy = request.POST.get("privacy") - source = request.POST.get("source") + importer = None + if source == "LibraryThing": + importer = LibrarythingImporter() + elif source == "Storygraph": + importer = StorygraphImporter() + else: + # Default : Goodreads + importer = GoodreadsImporter() - importer = None - if source == "LibraryThing": - importer = LibrarythingImporter() - elif source == "Storygraph": - importer = StorygraphImporter() - else: - # Default : Goodreads - importer = GoodreadsImporter() + try: + job = importer.create_job( + request.user, + TextIOWrapper( + request.FILES["csv_file"], encoding=importer.encoding + ), + include_reviews, + privacy, + ) + except (UnicodeDecodeError, ValueError, KeyError): + return HttpResponseBadRequest(_("Not a valid csv file")) - try: - job = importer.create_job( - request.user, - TextIOWrapper(request.FILES["csv_file"], encoding=importer.encoding), - include_reviews, - privacy, - ) - except (UnicodeDecodeError, ValueError, KeyError): - return HttpResponseBadRequest(_("Not a valid csv file")) + importer.start_import(job) - importer.start_import(job) - - return redirect(f"/import/{job.id}") + return redirect(f"/import/{job.id}") + return HttpResponseBadRequest() From 8d6059ae32aaf47bfa34e03464d4c04ae20aa9cb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 10 Dec 2021 12:32:05 -0800 Subject: [PATCH 22/38] Fixes mocks on group model tests --- bookwyrm/tests/models/test_group.py | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/bookwyrm/tests/models/test_group.py b/bookwyrm/tests/models/test_group.py index 3439a1bf..1fd071df 100644 --- a/bookwyrm/tests/models/test_group.py +++ b/bookwyrm/tests/models/test_group.py @@ -2,7 +2,7 @@ from unittest.mock import patch from django.test import TestCase -from bookwyrm import models +from bookwyrm import models, settings @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @@ -19,16 +19,10 @@ class Group(TestCase): "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse" ) - with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( - "bookwyrm.activitystreams.populate_stream_task.delay" - ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.rat = models.User.objects.create_user( "rat", "rat@rat.rat", "ratword", local=True, localname="rat" ) - with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( - "bookwyrm.activitystreams.populate_stream_task.delay" - ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.badger = models.User.objects.create_user( "badger", "badger@badger.badger", @@ -37,9 +31,6 @@ class Group(TestCase): localname="badger", ) - with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( - "bookwyrm.activitystreams.populate_stream_task.delay" - ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.capybara = models.User.objects.create_user( "capybara", "capybara@capybara.capybara", @@ -76,8 +67,7 @@ class Group(TestCase): models.GroupMember.objects.create(group=self.public_group, user=self.capybara) def test_group_members_can_see_private_groups(self, _): - """direct privacy group should not be excluded from group listings for group - members viewing""" + """direct privacy group should not be excluded from group listings for group members viewing""" rat_groups = models.Group.privacy_filter(self.rat).all() badger_groups = models.Group.privacy_filter(self.badger).all() @@ -86,8 +76,7 @@ class Group(TestCase): self.assertTrue(self.private_group in badger_groups) def test_group_members_can_see_followers_only_lists(self, _): - """follower-only group booklists should not be excluded from group booklist - listing for group members who do not follower list owner""" + """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"): followers_list = models.List.objects.create( @@ -107,8 +96,7 @@ class Group(TestCase): self.assertTrue(followers_list in capybara_lists) def test_group_members_can_see_private_lists(self, _): - """private group booklists should not be excluded from group booklist listing - for group members""" + """private group booklists should not be excluded from group booklist listing for group members""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): From 3358e4508673799f2f4156aa3d7703ced323a700 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 14 Dec 2021 09:31:57 -0800 Subject: [PATCH 23/38] Updates mocks on list stream tasks --- bookwyrm/tests/lists_stream/test_tasks.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py index 70c112bd..4c673a3a 100644 --- a/bookwyrm/tests/lists_stream/test_tasks.py +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -4,6 +4,9 @@ from django.test import TestCase from bookwyrm import lists_stream, models +@patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.activitystreams.remove_user_statuses_task.delay") +@patch("bookwyrm.activitystreams.add_user_statuses_task.delay") class Activitystreams(TestCase): """using redis to build activity streams""" @@ -32,11 +35,12 @@ class Activitystreams(TestCase): inbox="https://example.com/users/rat/inbox", outbox="https://example.com/users/rat/outbox", ) - self.list = models.List.objects.create( - user=self.local_user, name="hi", privacy="public" - ) + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + self.list = models.List.objects.create( + user=self.local_user, name="hi", privacy="public" + ) - def test_populate_lists_task(self): + def test_populate_lists_task(self, *_): """populate lists cache""" with patch("bookwyrm.lists_stream.ListsStream.populate_streams") as mock: lists_stream.populate_lists_task(self.local_user.id) @@ -50,7 +54,7 @@ class Activitystreams(TestCase): args = mock.call_args[0] self.assertEqual(args[0], self.local_user) - def test_remove_list_task(self): + def test_remove_list_task(self, *_): """remove a list from all streams""" with patch( "bookwyrm.lists_stream.ListsStream.remove_object_from_related_stores" @@ -60,7 +64,7 @@ class Activitystreams(TestCase): args = mock.call_args[0] self.assertEqual(args[0], self.list) - def test_add_list_task(self): + def test_add_list_task(self, *_): """add a list to all streams""" with patch("bookwyrm.lists_stream.ListsStream.add_list") as mock: lists_stream.add_list_task(self.list.id) @@ -68,7 +72,7 @@ class Activitystreams(TestCase): args = mock.call_args[0] self.assertEqual(args[0], self.list) - def test_remove_user_lists_task(self): + def test_remove_user_lists_task(self, *_): """remove all lists by a user from another users' feeds""" with patch("bookwyrm.lists_stream.ListsStream.remove_user_lists") as mock: lists_stream.remove_user_lists_task( @@ -88,7 +92,7 @@ class Activitystreams(TestCase): self.assertEqual(args[0], self.local_user) self.assertEqual(args[1], self.another_user) - def test_add_user_lists_task(self): + def test_add_user_lists_task(self, *_): """add a user's lists to another users feeds""" with patch("bookwyrm.lists_stream.ListsStream.add_user_lists") as mock: lists_stream.add_user_lists_task(self.local_user.id, self.another_user.id) From 0012f4464d80b30b5f4b4698ef3b34ab43e99c16 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 14 Dec 2021 11:07:36 -0800 Subject: [PATCH 24/38] Consider group membership for list cache --- bookwyrm/lists_stream.py | 31 +++++++++++++++------- bookwyrm/tests/lists_stream/test_stream.py | 26 +++++++++++++++--- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index da8cdb21..f44ef028 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -64,19 +64,32 @@ class ListsStream(RedisStore): Q(id__in=book_list.user.blocks.all()) | Q(blocks=book_list.user) ) - # TODO: groups - + group = book_list.group # only visible to the poster and mentioned users if book_list.privacy == "direct": - audience = audience.filter( - Q(id=book_list.user.id) # if the user is the post's author - ) + if group: + audience = audience.filter( + Q(id=book_list.user.id) # if the user is the post's author + | ~Q(groups=group.memberships) # if the user is in the group + ) + else: + audience = audience.filter( + Q(id=book_list.user.id) # if the user is the post's author + ) # only visible to the poster's followers and tagged users elif book_list.privacy == "followers": - audience = audience.filter( - Q(id=book_list.user.id) # if the user is the post's author - | Q(following=book_list.user) # if the user is following the author - ) + if group: + audience = audience.filter( + Q(id=book_list.user.id) # if the user is the list's owner + | Q(following=book_list.user) # if the user is following the pwmer + # if a user is in the group + | Q(memberships__group__id=book_list.group.id) + ) + else: + audience = audience.filter( + Q(id=book_list.user.id) # if the user is the list's owner + | Q(following=book_list.user) # if the user is following the pwmer + ) return audience.distinct() def get_stores_for_object(self, obj): diff --git a/bookwyrm/tests/lists_stream/test_stream.py b/bookwyrm/tests/lists_stream/test_stream.py index e7157152..d5357b17 100644 --- a/bookwyrm/tests/lists_stream/test_stream.py +++ b/bookwyrm/tests/lists_stream/test_stream.py @@ -92,7 +92,7 @@ class ListsStream(TestCase): book_list = models.List.objects.create( user=self.local_user, name="hi", - privacy="direct", + privacy="followers", ) users = self.stream.get_audience(book_list) self.assertTrue(self.local_user in users) @@ -105,9 +105,29 @@ class ListsStream(TestCase): book_list = models.List.objects.create( user=self.remote_user, name="hi", - privacy="direct", + privacy="followers", + ) + users = self.stream.get_audience(book_list) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + + def test_get_audience_followers_with_group(self, *_): + """get a list of users that should see a list""" + group = models.Group.objects.create(name="test group", user=self.remote_user) + models.GroupMember.objects.create( + group=group, + user=self.local_user, + ) + + book_list = models.List.objects.create( + user=self.remote_user, name="hi", privacy="followers", curation="group" ) users = self.stream.get_audience(book_list) self.assertFalse(self.local_user in users) + + book_list.group = group + book_list.save(broadcast=False) + + users = self.stream.get_audience(book_list) + self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) - self.assertFalse(self.remote_user in users) From 65ec626573f02a2d6381a9e49bc40d1bdfbb09d6 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 14 Dec 2021 11:11:05 -0800 Subject: [PATCH 25/38] Don't trim stream if max length is unset --- bookwyrm/redis_store.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py index 78f373a2..3b1b54a4 100644 --- a/bookwyrm/redis_store.py +++ b/bookwyrm/redis_store.py @@ -27,7 +27,8 @@ class RedisStore(ABC): # add the status to the feed pipeline.zadd(store, value) # trim the store - pipeline.zremrangebyrank(store, 0, -1 * self.max_length) + if self.max_length: + pipeline.zremrangebyrank(store, 0, -1 * self.max_length) if not execute: return pipeline # and go! @@ -46,7 +47,7 @@ class RedisStore(ABC): pipeline = r.pipeline() for obj in objs[: self.max_length]: pipeline.zadd(store, self.get_value(obj)) - if objs: + if objs and self.max_length: pipeline.zremrangebyrank(store, 0, -1 * self.max_length) pipeline.execute() @@ -70,7 +71,7 @@ class RedisStore(ABC): pipeline.zadd(store, self.get_value(obj)) # only trim the store if objects were added - if queryset.exists(): + if queryset.exists() and self.max_length: pipeline.zremrangebyrank(store, 0, -1 * self.max_length) pipeline.execute() From 2640c26bb1d0fbfa8cad8ea26c82ba0a97d8a0c5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 29 Dec 2021 17:39:14 -0800 Subject: [PATCH 26/38] Fixes wording in comments --- bookwyrm/lists_stream.py | 12 ++++++------ .../management/commands/populate_lists_streams.py | 6 +++--- bookwyrm/tests/lists_stream/test_signals.py | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index f44ef028..6c353545 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -1,4 +1,4 @@ -""" access the activity streams stored in redis """ +""" access the list streams stored in redis """ from django.dispatch import receiver from django.db import transaction from django.db.models import signals, Count, Q @@ -25,8 +25,8 @@ class ListsStream(RedisStore): self.add_object_to_related_stores(book_list) def add_user_lists(self, viewer, user): - """add a user's listes to another user's feed""" - # only add the listes that the viewer should be able to see + """add a user's lists to another user's feed""" + # only add the lists that the viewer should be able to see lists = models.List.filter(user=user).privacy_filter(viewer) self.bulk_add_objects_to_store(lists, self.stream_id(viewer)) @@ -110,7 +110,7 @@ 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 to activity feeds""" + """add newly created lists streamsstreams""" if not created: return # when creating new things, gotta wait on the transaction @@ -120,7 +120,7 @@ def add_list_on_create(sender, instance, created, *args, **kwargs): @receiver(signals.pre_delete, sender=models.List) # pylint: disable=unused-argument def remove_list_on_delete(sender, instance, *args, **kwargs): - """add newly created lists to activity feeds""" + """remove deleted lists to streams""" remove_list_task.delay(instance.id) @@ -199,7 +199,7 @@ def populate_lists_on_account_create(sender, instance, created, *args, **kwargs) # ---- TASKS @app.task(queue=MEDIUM) def populate_lists_task(user_id): - """background task for populating an empty activitystream""" + """background task for populating an empty list stream""" user = models.User.objects.get(id=user_id) ListsStream().populate_streams(user) diff --git a/bookwyrm/management/commands/populate_lists_streams.py b/bookwyrm/management/commands/populate_lists_streams.py index 502b3b92..c08b36d5 100644 --- a/bookwyrm/management/commands/populate_lists_streams.py +++ b/bookwyrm/management/commands/populate_lists_streams.py @@ -1,4 +1,4 @@ -""" Re-create user streams """ +""" Re-create list streams """ from django.core.management.base import BaseCommand from bookwyrm import lists_stream, models @@ -17,9 +17,9 @@ def populate_lists_streams(): class Command(BaseCommand): - """start all over with user streams""" + """start all over with lists streams""" - help = "Populate streams for all users" + help = "Populate list streams for all users" def add_arguments(self, parser): parser.add_argument( diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 91c6b48f..f081dfd5 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -36,7 +36,7 @@ class ListsStreamSignals(TestCase): 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) + lists_stream.add_list_on_create(models.List, book_list, True) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], book_list.id) From 9f6918767b97a45d55869723ad6790e850f7be56 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 29 Dec 2021 17:41:31 -0800 Subject: [PATCH 27/38] Corrects documentation on sort order --- bookwyrm/lists_stream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 6c353545..8609a4d1 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -16,7 +16,7 @@ class ListsStream(RedisStore): return f"{user.id}-lists" def get_rank(self, obj): # pylint: disable=no-self-use - """lists are sorted by date published""" + """lists are sorted by updated date""" return obj.updated_date.timestamp() def add_list(self, book_list): From 906e0c9c7c0304c168883ff9c2be877377cb4d7f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 30 Dec 2021 10:40:26 -0800 Subject: [PATCH 28/38] Renames function for loading lists --- bookwyrm/lists_stream.py | 2 +- bookwyrm/views/list.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 8609a4d1..c373a261 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -36,7 +36,7 @@ class ListsStream(RedisStore): lists = user.list_set.all() self.bulk_remove_objects_from_store(lists, self.stream_id(viewer)) - def get_activity_stream(self, user): + def get_list_stream(self, user): """load the lists to be displayed""" lists = self.get_store(self.stream_id(user)) return ( diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 8a4e03cd..59be18fe 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -30,7 +30,7 @@ class Lists(View): def get(self, request): """display a book list""" - lists = ListsStream().get_activity_stream(request.user) + lists = ListsStream().get_list_stream(request.user) paginated = Paginator(lists, 12) data = { "lists": paginated.get_page(request.GET.get("page")), From 4de406afe10a1a2a07c264a1923565b69ab6d4d0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 30 Dec 2021 11:07:04 -0800 Subject: [PATCH 29/38] Some tests fixes --- bookwyrm/tests/importers/test_openlibrary_import.py | 2 +- bookwyrm/tests/views/test_annual_summary.py | 2 +- bookwyrm/tests/views/test_list.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/importers/test_openlibrary_import.py b/bookwyrm/tests/importers/test_openlibrary_import.py index 6a25c191..a775c596 100644 --- a/bookwyrm/tests/importers/test_openlibrary_import.py +++ b/bookwyrm/tests/importers/test_openlibrary_import.py @@ -30,7 +30,7 @@ class OpenLibraryImport(TestCase): self.csv = open(datafile, "r", encoding=self.importer.encoding) with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse", "mouse@mouse.mouse", "password", local=True ) diff --git a/bookwyrm/tests/views/test_annual_summary.py b/bookwyrm/tests/views/test_annual_summary.py index 32439eab..2d597be7 100644 --- a/bookwyrm/tests/views/test_annual_summary.py +++ b/bookwyrm/tests/views/test_annual_summary.py @@ -26,7 +26,7 @@ class AnnualSummary(TestCase): self.factory = RequestFactory() with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( "bookwyrm.activitystreams.populate_stream_task.delay" - ): + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( "mouse@local.com", "mouse@mouse.com", diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py index cb0ea6fd..d36f88e5 100644 --- a/bookwyrm/tests/views/test_list.py +++ b/bookwyrm/tests/views/test_list.py @@ -72,7 +72,7 @@ class ListViews(TestCase): models.SiteSettings.objects.create() - @patch("bookwyrm.lists_stream.ListsStream.get_activity_stream") + @patch("bookwyrm.lists_stream.ListsStream.get_list_stream") def test_lists_page(self, _): """there are so many views, this just makes sure it LOADS""" view = views.Lists.as_view() From cc37d7404e021d25e18da0015a4ff829c66a1b6a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 30 Dec 2021 11:58:27 -0800 Subject: [PATCH 30/38] Fixes calls to add lists --- bookwyrm/lists_stream.py | 2 +- bookwyrm/tests/lists_stream/test_stream.py | 38 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index c373a261..f98a82cb 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -27,7 +27,7 @@ class ListsStream(RedisStore): def add_user_lists(self, viewer, user): """add a user's lists to another user's feed""" # only add the lists that the viewer should be able to see - lists = models.List.filter(user=user).privacy_filter(viewer) + lists = models.List.privacy_filter(viewer).filter(user=user) self.bulk_add_objects_to_store(lists, self.stream_id(viewer)) def remove_user_lists(self, viewer, user): diff --git a/bookwyrm/tests/lists_stream/test_stream.py b/bookwyrm/tests/lists_stream/test_stream.py index d5357b17..4d8aa52b 100644 --- a/bookwyrm/tests/lists_stream/test_stream.py +++ b/bookwyrm/tests/lists_stream/test_stream.py @@ -1,5 +1,7 @@ """ testing activitystreams """ +from datetime import datetime from unittest.mock import patch + from django.test import TestCase from bookwyrm import lists_stream, models @@ -46,6 +48,42 @@ class ListsStream(TestCase): f"{self.local_user.id}-lists", ) + def test_get_rank(self, *_): + """sort order for lists""" + book_list = models.List.objects.create( + user=self.remote_user, name="hi", privacy="public" + ) + book_list.updated_date = datetime(2020, 1, 1, 0, 0, 0) + self.assertEqual(self.stream.get_rank(book_list), 1577836800.0) + + def test_add_user_lists(self, *_): + """add all of a user's lists""" + book_list = models.List.objects.create( + user=self.remote_user, name="hi", privacy="public" + ) + with patch( + "bookwyrm.lists_stream.ListsStream.bulk_add_objects_to_store" + ) as mock: + self.stream.add_user_lists(self.local_user, self.remote_user) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[0] + self.assertEqual(args[0][0], book_list) + self.assertEqual(args[1], f"{self.local_user.id}-lists") + + def test_remove_user_lists(self, *_): + """remove user's lists""" + book_list = models.List.objects.create( + user=self.remote_user, name="hi", privacy="public" + ) + with patch( + "bookwyrm.lists_stream.ListsStream.bulk_remove_objects_from_store" + ) as mock: + self.stream.remove_user_lists(self.local_user, self.remote_user) + self.assertEqual(mock.call_count, 1) + args = mock.call_args[0] + self.assertEqual(args[0][0], book_list) + self.assertEqual(args[1], f"{self.local_user.id}-lists") + def test_get_audience(self, *_): """get a list of users that should see a list""" book_list = models.List.objects.create( From b090490cd97bb924f4551184136e8c61b572c816 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 30 Dec 2021 12:06:22 -0800 Subject: [PATCH 31/38] Revert test of add list on create command --- bookwyrm/tests/lists_stream/test_signals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index f081dfd5..2b48ef9b 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -30,13 +30,13 @@ class ListsStreamSignals(TestCase): outbox="https://example.com/users/rat/outbox", ) - def test_add_list_on_create(self, _): + 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.add_list_task.delay") as mock: - lists_stream.add_list_on_create(models.List, book_list, True) + lists_stream.add_list_on_create_command(book_list) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], book_list.id) From 3dd7847d7b544581e01b4dfd06aa389be7993553 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 4 Jan 2022 13:46:21 -0800 Subject: [PATCH 32/38] Only remove non-public lists on unfollow --- bookwyrm/lists_stream.py | 17 +++++++++++------ complete_bwdev.sh | 1 + 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index f98a82cb..bb0ae1c8 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -30,11 +30,13 @@ class ListsStream(RedisStore): lists = models.List.privacy_filter(viewer).filter(user=user) self.bulk_add_objects_to_store(lists, self.stream_id(viewer)) - def remove_user_lists(self, viewer, user): + def remove_user_lists(self, viewer, user, exclude_privacy=None): """remove a user's list from another user's feed""" # remove all so that followers only lists are removed - lists = user.list_set.all() - self.bulk_remove_objects_from_store(lists, self.stream_id(viewer)) + lists = user.list_set + if exclude_privacy: + lists = lists.exclude(privacy=exclude_privacy) + self.bulk_remove_objects_from_store(lists.all(), self.stream_id(viewer)) def get_list_stream(self, user): """load the lists to be displayed""" @@ -144,7 +146,10 @@ def remove_lists_on_unfollow(sender, instance, *args, **kwargs): """remove lists from a feed on unfollow""" if not instance.user_subject.local: return - remove_user_lists_task.delay(instance.user_subject.id, instance.user_object.id) + # remove all but public lists + remove_user_lists_task.delay( + instance.user_subject.id, instance.user_object.id, exclude_privacy="public" + ) @receiver(signals.post_save, sender=models.UserBlocks) @@ -224,11 +229,11 @@ def add_list_task(list_id): @app.task(queue=MEDIUM) -def remove_user_lists_task(viewer_id, user_id): +def remove_user_lists_task(viewer_id, user_id, exclude_privacy=None): """remove all lists by a user from a viewer's stream""" viewer = models.User.objects.get(id=viewer_id) user = models.User.objects.get(id=user_id) - ListsStream().remove_user_lists(viewer, user) + ListsStream().remove_user_lists(viewer, user, exclude_privacy=exclude_privacy) @app.task(queue=MEDIUM) diff --git a/complete_bwdev.sh b/complete_bwdev.sh index e7a036cc..85115fba 100644 --- a/complete_bwdev.sh +++ b/complete_bwdev.sh @@ -22,6 +22,7 @@ clean black prettier populate_streams +populate_lists_streams populate_suggestions generate_thumbnails generate_preview_images From 23e498879e73eb5f0b21bdcdfc7b65613f170a38 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 4 Jan 2022 14:17:14 -0800 Subject: [PATCH 33/38] Fixes account create tasks --- bookwyrm/activitystreams.py | 8 +++++++- bookwyrm/lists_stream.py | 16 ++++++++++------ bookwyrm/suggested_users.py | 8 +++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 4cba9939..f2dd43fb 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -397,9 +397,15 @@ def populate_streams_on_account_create(sender, instance, created, *args, **kwarg """build a user's feeds when they join""" if not created or not instance.local: return + transaction.on_commit( + lambda: populate_streams_on_account_create_command(instance.id) + ) + +def populate_streams_on_account_create_command(instance_id): + """wait for the transaction to complete""" for stream in streams: - populate_stream_task.delay(stream, instance.id) + populate_stream_task.delay(stream, instance_id) @receiver(signals.pre_save, sender=models.ShelfBook) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index bb0ae1c8..79fabe43 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -52,7 +52,7 @@ class ListsStream(RedisStore): .distinct() ) - def populate_streams(self, user): + def populate_lists(self, user): """go from zero to a timeline""" self.populate_store(self.stream_id(user)) @@ -116,7 +116,7 @@ def add_list_on_create(sender, instance, created, *args, **kwargs): if not created: return # when creating new things, gotta wait on the transaction - transaction.on_commit(lambda: add_list_on_create_command(instance)) + transaction.on_commit(lambda: add_list_on_create_command(instance.id)) @receiver(signals.pre_delete, sender=models.List) @@ -126,9 +126,9 @@ def remove_list_on_delete(sender, instance, *args, **kwargs): remove_list_task.delay(instance.id) -def add_list_on_create_command(instance): +def add_list_on_create_command(instance_id): """runs this code only after the database commit completes""" - add_list_task.delay(instance.id) + add_list_task.delay(instance_id) @receiver(signals.post_save, sender=models.UserFollows) @@ -197,8 +197,12 @@ def populate_lists_on_account_create(sender, instance, created, *args, **kwargs) """build a user's feeds when they join""" if not created or not instance.local: return + transaction.on_commit(lambda: add_list_on_account_create_command(instance.id)) - populate_lists_task.delay(instance.id) + +def add_list_on_account_create_command(user_id): + """wait for the transaction to complete""" + populate_lists_task.delay(user_id) # ---- TASKS @@ -206,7 +210,7 @@ def populate_lists_on_account_create(sender, instance, created, *args, **kwargs) def populate_lists_task(user_id): """background task for populating an empty list stream""" user = models.User.objects.get(id=user_id) - ListsStream().populate_streams(user) + ListsStream().populate_lists(user) @app.task(queue=MEDIUM) diff --git a/bookwyrm/suggested_users.py b/bookwyrm/suggested_users.py index 86c181a2..a4b7ca65 100644 --- a/bookwyrm/suggested_users.py +++ b/bookwyrm/suggested_users.py @@ -2,6 +2,7 @@ import math import logging from django.dispatch import receiver +from django.db import transaction from django.db.models import signals, Count, Q from bookwyrm import models @@ -197,7 +198,7 @@ def update_user(sender, instance, created, update_fields=None, **kwargs): """an updated user, neat""" # a new user is found, create suggestions for them if created and instance.local: - rerank_suggestions_task.delay(instance.id) + transaction.on_commit(lambda: update_new_user_command(instance.id)) # we know what fields were updated and discoverability didn't change if not instance.bookwyrm_user or ( @@ -217,6 +218,11 @@ def update_user(sender, instance, created, update_fields=None, **kwargs): remove_user_task.delay(instance.id) +def update_new_user_command(instance_id): + """wait for transaction to complete""" + rerank_suggestions_task.delay(instance_id) + + @receiver(signals.post_save, sender=models.FederatedServer) def domain_level_update(sender, instance, created, update_fields=None, **kwargs): """remove users on a domain block""" From 8722778ed00a46e8367a4ce0a2f90753b42fe03b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 6 Jan 2022 12:10:36 -0800 Subject: [PATCH 34/38] Updates tests --- bookwyrm/tests/activitystreams/test_signals.py | 6 +++--- bookwyrm/tests/lists_stream/test_signals.py | 8 +++----- bookwyrm/tests/lists_stream/test_tasks.py | 4 ++-- bookwyrm/tests/test_suggested_users.py | 9 --------- 4 files changed, 8 insertions(+), 19 deletions(-) diff --git a/bookwyrm/tests/activitystreams/test_signals.py b/bookwyrm/tests/activitystreams/test_signals.py index c5c7e698..4db1875f 100644 --- a/bookwyrm/tests/activitystreams/test_signals.py +++ b/bookwyrm/tests/activitystreams/test_signals.py @@ -62,11 +62,11 @@ class ActivitystreamsSignals(TestCase): self.assertEqual(args["args"][0], status.id) self.assertEqual(args["queue"], "high_priority") - def test_populate_streams_on_account_create(self, *_): + def test_populate_streams_on_account_create_command(self, *_): """create streams for a user""" with patch("bookwyrm.activitystreams.populate_stream_task.delay") as mock: - activitystreams.populate_streams_on_account_create( - models.User, self.local_user, True + activitystreams.populate_streams_on_account_create_command( + self.local_user.id ) self.assertEqual(mock.call_count, 3) args = mock.call_args[0] diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 2b48ef9b..42e60327 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -36,7 +36,7 @@ class ListsStreamSignals(TestCase): 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) + lists_stream.add_list_on_create_command(book_list.id) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], book_list.id) @@ -51,12 +51,10 @@ class ListsStreamSignals(TestCase): args = mock.call_args[0] self.assertEqual(args[0], book_list.id) - def test_populate_lists_on_account_create(self, _): + def test_populate_lists_on_account_create_command(self, _): """create streams for a user""" with patch("bookwyrm.lists_stream.populate_lists_task.delay") as mock: - lists_stream.populate_lists_on_account_create( - models.User, self.local_user, True - ) + lists_stream.populate_lists_on_account_create_command(self.local_user.id) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], self.local_user.id) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py index 4c673a3a..c99ed665 100644 --- a/bookwyrm/tests/lists_stream/test_tasks.py +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -42,13 +42,13 @@ class Activitystreams(TestCase): def test_populate_lists_task(self, *_): """populate lists cache""" - with patch("bookwyrm.lists_stream.ListsStream.populate_streams") as mock: + with patch("bookwyrm.lists_stream.ListsStream.populate_lists") as mock: lists_stream.populate_lists_task(self.local_user.id) self.assertTrue(mock.called) args = mock.call_args[0] self.assertEqual(args[0], self.local_user) - with patch("bookwyrm.lists_stream.ListsStream.populate_streams") as mock: + with patch("bookwyrm.lists_stream.ListsStream.populate_lists") as mock: lists_stream.populate_lists_task(self.local_user.id) self.assertTrue(mock.called) args = mock.call_args[0] diff --git a/bookwyrm/tests/test_suggested_users.py b/bookwyrm/tests/test_suggested_users.py index 91677f19..77b82e7e 100644 --- a/bookwyrm/tests/test_suggested_users.py +++ b/bookwyrm/tests/test_suggested_users.py @@ -236,12 +236,3 @@ class SuggestedUsers(TestCase): ) user_1_annotated = result.get(id=user_1.id) self.assertEqual(user_1_annotated.mutuals, 3) - - def test_create_user_signal(self, *_): - """build suggestions for new users""" - with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") as mock: - models.User.objects.create_user( - "nutria", "nutria@nu.tria", "password", local=True, localname="nutria" - ) - - self.assertEqual(mock.call_count, 1) From 7e1a4bc363aef8988ccb462399c76c8a6c3afac7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 7 Jan 2022 10:32:40 -0800 Subject: [PATCH 35/38] Ticks version number --- bookwyrm/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 5920ed80..e22d0329 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -9,7 +9,7 @@ from django.utils.translation import gettext_lazy as _ env = Env() env.read_env() DOMAIN = env("DOMAIN") -VERSION = "0.1.0" +VERSION = "0.1.1" PAGE_LENGTH = env("PAGE_LENGTH", 15) DEFAULT_LANGUAGE = env("DEFAULT_LANGUAGE", "English") From 0580b66c3bc9f6b0408b1cd5988e5652dc2fc2d5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 7 Jan 2022 10:34:45 -0800 Subject: [PATCH 36/38] Fixes test --- bookwyrm/tests/lists_stream/test_signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/lists_stream/test_signals.py b/bookwyrm/tests/lists_stream/test_signals.py index 42e60327..f82dba3a 100644 --- a/bookwyrm/tests/lists_stream/test_signals.py +++ b/bookwyrm/tests/lists_stream/test_signals.py @@ -54,7 +54,7 @@ class ListsStreamSignals(TestCase): def test_populate_lists_on_account_create_command(self, _): """create streams for a user""" with patch("bookwyrm.lists_stream.populate_lists_task.delay") as mock: - lists_stream.populate_lists_on_account_create_command(self.local_user.id) + lists_stream.add_list_on_account_create_command(self.local_user.id) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] self.assertEqual(args[0], self.local_user.id) From 8a8ce0c0d4313847cded1abbbd988211eff5070d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 7 Jan 2022 13:30:11 -0800 Subject: [PATCH 37/38] Fixes deletion of lists --- bookwyrm/lists_stream.py | 19 +++++++++++-------- bookwyrm/redis_store.py | 7 ++++++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/bookwyrm/lists_stream.py b/bookwyrm/lists_stream.py index 79fabe43..f6a35cc2 100644 --- a/bookwyrm/lists_stream.py +++ b/bookwyrm/lists_stream.py @@ -13,6 +13,9 @@ class ListsStream(RedisStore): def stream_id(self, user): # pylint: disable=no-self-use """the redis key for this user's instance of this stream""" + if isinstance(user, int): + # allows the function to take an int or an obj + return f"{user}-lists" return f"{user.id}-lists" def get_rank(self, obj): # pylint: disable=no-self-use @@ -119,7 +122,7 @@ def add_list_on_create(sender, instance, created, *args, **kwargs): transaction.on_commit(lambda: add_list_on_create_command(instance.id)) -@receiver(signals.pre_delete, sender=models.List) +@receiver(signals.post_delete, sender=models.List) # pylint: disable=unused-argument def remove_list_on_delete(sender, instance, *args, **kwargs): """remove deleted lists to streams""" @@ -214,15 +217,15 @@ def populate_lists_task(user_id): @app.task(queue=MEDIUM) -def remove_list_task(list_ids): +def remove_list_task(list_id): """remove a list from any stream it might be in""" - # this can take an id or a list of ids - if not isinstance(list_ids, list): - list_ids = [list_ids] - lists = models.List.objects.filter(id__in=list_ids) + stores = models.User.objects.filter(local=True, is_active=True).values_list( + "id", flat=True + ) - for book_list in lists: - ListsStream().remove_object_from_related_stores(book_list) + # delete for every store + stores = [ListsStream().stream_id(idx) for idx in stores] + ListsStream().remove_object_from_related_stores(list_id, stores=stores) @app.task(queue=HIGH) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py index 263d7fa2..964409e8 100644 --- a/bookwyrm/redis_store.py +++ b/bookwyrm/redis_store.py @@ -39,10 +39,15 @@ class RedisStore(ABC): def remove_object_from_related_stores(self, obj, stores=None): """remove an object from all stores""" + # if the stoers are provided, the object can just be an id + if stores and isinstance(obj, int): + obj_id = obj + else: + obj_id = obj.id stores = self.get_stores_for_object(obj) if stores is None else stores pipeline = r.pipeline() for store in stores: - pipeline.zrem(store, -1, obj.id) + pipeline.zrem(store, -1, obj_id) pipeline.execute() def bulk_add_objects_to_store(self, objs, store): From f43a9570e2b294084d7ec35c3a5eb23afeb3101e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 7 Jan 2022 13:40:20 -0800 Subject: [PATCH 38/38] Updates test --- bookwyrm/tests/lists_stream/test_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/lists_stream/test_tasks.py b/bookwyrm/tests/lists_stream/test_tasks.py index c99ed665..1da36b71 100644 --- a/bookwyrm/tests/lists_stream/test_tasks.py +++ b/bookwyrm/tests/lists_stream/test_tasks.py @@ -62,7 +62,7 @@ class Activitystreams(TestCase): lists_stream.remove_list_task(self.list.id) self.assertEqual(mock.call_count, 1) args = mock.call_args[0] - self.assertEqual(args[0], self.list) + self.assertEqual(args[0], self.list.id) def test_add_list_task(self, *_): """add a list to all streams"""