From 6617cede375f335e9722b09d69afa0a3b666f716 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:15:58 -0700
Subject: [PATCH 01/22] Loudly throw errors in inbox task

---
 bookwyrm/views/inbox.py | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index 34bd2e1c..8c645159 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -58,18 +58,11 @@ class Inbox(View):
 def activity_task(activity_json):
     """ do something with this json we think is legit """
     # lets see if the activitypub module can make sense of this json
-    try:
-        activity = activitypub.parse(activity_json)
-    except activitypub.ActivitySerializerError:
-        return
+    activity = activitypub.parse(activity_json)
 
     # cool that worked, now we should do the action described by the type
     # (create, update, delete, etc)
-    try:
-        activity.action()
-    except activitypub.ActivitySerializerError:
-        # this is raised if the activity is discarded
-        return
+    activity.action()
 
 
 def has_valid_signature(request, activity):

From 98161b9041ce807194670db10f3427d9577e1aa9 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:16:34 -0700
Subject: [PATCH 02/22] Use CollectionItem objects

---
 bookwyrm/activitypub/__init__.py           |  1 +
 bookwyrm/activitypub/ordered_collection.py | 24 ++++++++++++++++++++++
 bookwyrm/activitypub/verbs.py              | 12 ++++-------
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py
index 35b786f7..c67c5dca 100644
--- a/bookwyrm/activitypub/__init__.py
+++ b/bookwyrm/activitypub/__init__.py
@@ -10,6 +10,7 @@ from .note import Note, GeneratedNote, Article, Comment, Quotation
 from .note import Review, Rating
 from .note import Tombstone
 from .ordered_collection import OrderedCollection, OrderedCollectionPage
+from .ordered_collection import CollectionItem, ListItem, ShelfItem
 from .ordered_collection import BookList, Shelf
 from .person import Person, PublicKey
 from .response import ActivitypubResponse
diff --git a/bookwyrm/activitypub/ordered_collection.py b/bookwyrm/activitypub/ordered_collection.py
index 6da60832..135c50d3 100644
--- a/bookwyrm/activitypub/ordered_collection.py
+++ b/bookwyrm/activitypub/ordered_collection.py
@@ -50,3 +50,27 @@ class OrderedCollectionPage(ActivityObject):
     next: str = None
     prev: str = None
     type: str = "OrderedCollectionPage"
+
+
+@dataclass(init=False)
+class CollectionItem(ActivityObject):
+    """ an item in a collection """
+    actor: str
+    type: str = "CollectionItem"
+
+
+@dataclass(init=False)
+class ListItem(CollectionItem):
+    """ a book on a list """
+    book: str
+    notes: str = None
+    approved: bool = True
+    order: int = None
+    type: str = "ListItem"
+
+
+@dataclass(init=False)
+class ShelfItem(CollectionItem):
+    """ a book on a list """
+    book: str
+    type: str = "ShelfItem"
diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py
index 3686b3f3..a56eb2fe 100644
--- a/bookwyrm/activitypub/verbs.py
+++ b/bookwyrm/activitypub/verbs.py
@@ -4,7 +4,7 @@ from typing import List
 from django.apps import apps
 
 from .base_activity import ActivityObject, Signature, resolve_remote_id
-from .book import Edition
+from .ordered_collection import CollectionItem
 
 
 @dataclass(init=False)
@@ -141,12 +141,9 @@ class Reject(Verb):
 class Add(Verb):
     """Add activity """
 
-    target: str
-    object: Edition
+    target: ActivityObject
+    object: CollectionItem
     type: str = "Add"
-    notes: str = None
-    order: int = 0
-    approved: bool = True
 
     def action(self):
         """ add obj to collection """
@@ -159,10 +156,9 @@ class Add(Verb):
 
 
 @dataclass(init=False)
-class Remove(Verb):
+class Remove(Add):
     """Remove activity """
 
-    target: ActivityObject
     type: str = "Remove"
 
     def action(self):

From 31077a97900691b1104767e41c427d662ecedf7d Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:17:29 -0700
Subject: [PATCH 03/22] Update ordered collection classes to use CollectionItem

---
 bookwyrm/models/activitypub_mixin.py | 22 ++++++++++------------
 bookwyrm/models/list.py              |  3 +--
 bookwyrm/models/shelf.py             |  5 ++---
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 8dce42e4..9b104456 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -64,6 +64,7 @@ class ActivitypubMixin:
         )
         if hasattr(self, "property_fields"):
             self.activity_fields += [
+                # pylint: disable=cell-var-from-loop
                 PropertyField(lambda a, o: set_activity_from_property_field(a, o, f))
                 for f in self.property_fields
             ]
@@ -356,8 +357,7 @@ class OrderedCollectionMixin(OrderedCollectionPageMixin):
 class CollectionItemMixin(ActivitypubMixin):
     """ for items that are part of an (Ordered)Collection """
 
-    activity_serializer = activitypub.Add
-    object_field = collection_field = None
+    activity_serializer = activitypub.CollectionItem
 
     def save(self, *args, broadcast=True, **kwargs):
         """ broadcast updated """
@@ -370,35 +370,33 @@ class CollectionItemMixin(ActivitypubMixin):
             return
 
         # adding an obj to the collection
-        activity = self.to_add_activity()
+        activity = self.to_add_activity(self.user)
         self.broadcast(activity, self.user)
 
     def delete(self, *args, **kwargs):
         """ broadcast a remove activity """
-        activity = self.to_remove_activity()
+        activity = self.to_remove_activity(self.user)
         super().delete(*args, **kwargs)
         if self.user.local:
             self.broadcast(activity, self.user)
 
-    def to_add_activity(self):
+    def to_add_activity(self, user):
         """ AP for shelving a book"""
-        object_field = getattr(self, self.object_field)
         collection_field = getattr(self, self.collection_field)
         return activitypub.Add(
             id=self.get_remote_id(),
-            actor=self.user.remote_id,
-            object=object_field,
+            actor=user.remote_id,
+            object=self.to_activity_dataclass(),
             target=collection_field.remote_id,
         ).serialize()
 
-    def to_remove_activity(self):
+    def to_remove_activity(self, user):
         """ AP for un-shelving a book"""
-        object_field = getattr(self, self.object_field)
         collection_field = getattr(self, self.collection_field)
         return activitypub.Remove(
             id=self.get_remote_id(),
-            actor=self.user.remote_id,
-            object=object_field,
+            actor=user.remote_id,
+            object=self.to_activity_dataclass(),
             target=collection_field.remote_id,
         ).serialize()
 
diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py
index 880c4122..b90eafe3 100644
--- a/bookwyrm/models/list.py
+++ b/bookwyrm/models/list.py
@@ -72,8 +72,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
     order = fields.IntegerField(blank=True, null=True)
     endorsement = models.ManyToManyField("User", related_name="endorsers")
 
-    activity_serializer = activitypub.Add
-    object_field = "book"
+    activity_serializer = activitypub.ListItem
     collection_field = "book_list"
 
     def save(self, *args, **kwargs):
diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py
index 3209da5d..8a7095d0 100644
--- a/bookwyrm/models/shelf.py
+++ b/bookwyrm/models/shelf.py
@@ -32,6 +32,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
     )
 
     activity_serializer = activitypub.Shelf
+    collection_field = "shelf"
 
     def save(self, *args, **kwargs):
         """ set the identifier """
@@ -75,9 +76,7 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
 
-    activity_serializer = activitypub.Add
-    object_field = "book"
-    collection_field = "shelf"
+    activity_serializer = activitypub.ShelfItem
 
     def save(self, *args, **kwargs):
         if not self.user:

From af58455ae9ea6af1bf82d1da786facf7114f0604 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:20:16 -0700
Subject: [PATCH 04/22] Updates mock paradigm in lists tests

---
 bookwyrm/tests/views/test_list.py | 175 +++++++++++++-----------------
 1 file changed, 74 insertions(+), 101 deletions(-)

diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index c2c75453..04e8aabe 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -1,4 +1,5 @@
 """ test for app action functionality """
+import json
 from unittest.mock import patch
 
 from django.contrib.auth.models import AnonymousUser
@@ -71,16 +72,6 @@ class ListViews(TestCase):
 
     def test_lists_create(self):
         """ create list view """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Create")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-
-        models.List.broadcast = mock_broadcast
-
         view = views.Lists.as_view()
         request = self.factory.post(
             "",
@@ -93,13 +84,19 @@ class ListViews(TestCase):
             },
         )
         request.user = self.local_user
-        result = view(request)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            result = view(request)
+
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Create")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+
         self.assertEqual(result.status_code, 302)
         new_list = models.List.objects.filter(name="A list").get()
         self.assertEqual(new_list.description, "wow")
         self.assertEqual(new_list.privacy, "unlisted")
         self.assertEqual(new_list.curation, "open")
-        models.List.broadcast = real_broadcast
 
     def test_list_page(self):
         """ there are so many views, this just makes sure it LOADS """
@@ -138,17 +135,6 @@ class ListViews(TestCase):
 
     def test_list_edit(self):
         """ edit a list """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Update")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["id"], self.list.remote_id)
-
-        models.List.broadcast = mock_broadcast
-
         view = views.List.as_view()
         request = self.factory.post(
             "",
@@ -162,7 +148,15 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        result = view(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            result = view(request, self.list.id)
+
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Update")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["object"]["id"], self.list.remote_id)
+
         self.assertEqual(result.status_code, 302)
 
         self.list.refresh_from_db()
@@ -170,7 +164,6 @@ class ListViews(TestCase):
         self.assertEqual(self.list.description, "wow")
         self.assertEqual(self.list.privacy, "direct")
         self.assertEqual(self.list.curation, "curated")
-        models.List.broadcast = real_broadcast
 
     def test_curate_page(self):
         """ there are so many views, this just makes sure it LOADS """
@@ -194,17 +187,6 @@ class ListViews(TestCase):
 
     def test_curate_approve(self):
         """ approve a pending item """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
-
         view = views.Curate.as_view()
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             pending = models.ListItem.objects.create(
@@ -223,12 +205,19 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        view(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            view(request, self.list.id)
+
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
         pending.refresh_from_db()
         self.assertEqual(self.list.books.count(), 1)
         self.assertEqual(self.list.listitem_set.first(), pending)
         self.assertTrue(pending.approved)
-        models.ListItem.broadcast = real_broadcast
 
     def test_curate_reject(self):
         """ approve a pending item """
@@ -250,23 +239,15 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             view(request, self.list.id)
+            self.assertEqual(mock.call_count, 1)
+            #activity = json.loads(mock.call_args_list[1][0][1])
         self.assertFalse(self.list.books.exists())
         self.assertFalse(models.ListItem.objects.exists())
 
     def test_add_book(self):
         """ put a book on a list """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
         request = self.factory.post(
             "",
             {
@@ -276,25 +257,21 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        views.list.add_book(request)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
         item = self.list.listitem_set.get()
         self.assertEqual(item.book, self.book)
         self.assertEqual(item.user, self.local_user)
         self.assertTrue(item.approved)
-        models.ListItem.broadcast = real_broadcast
 
     def test_add_book_outsider(self):
         """ put a book on a list """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user):
-            """ ok """
-            self.assertEqual(user.remote_id, self.rat.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.rat.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
         self.list.curation = "open"
         self.list.save(broadcast=False)
         request = self.factory.post(
@@ -306,26 +283,21 @@ class ListViews(TestCase):
         )
         request.user = self.rat
 
-        views.list.add_book(request)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.rat.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
         item = self.list.listitem_set.get()
         self.assertEqual(item.book, self.book)
         self.assertEqual(item.user, self.rat)
         self.assertTrue(item.approved)
-        models.ListItem.broadcast = real_broadcast
 
     def test_add_book_pending(self):
         """ put a book on a list awaiting approval """
-        real_broadcast = models.List.broadcast
-
-        def mock_broadcast(_, activity, user):
-            """ ok """
-            self.assertEqual(user.remote_id, self.rat.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.rat.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-            self.assertEqual(activity["object"]["id"], self.book.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
         self.list.curation = "curated"
         self.list.save(broadcast=False)
         request = self.factory.post(
@@ -337,26 +309,21 @@ class ListViews(TestCase):
         )
         request.user = self.rat
 
-        views.list.add_book(request)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.rat.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+            self.assertEqual(activity["object"]["id"], self.book.remote_id)
         item = self.list.listitem_set.get()
         self.assertEqual(item.book, self.book)
         self.assertEqual(item.user, self.rat)
         self.assertFalse(item.approved)
-        models.ListItem.broadcast = real_broadcast
 
     def test_add_book_self_curated(self):
         """ put a book on a list automatically approved """
-        real_broadcast = models.ListItem.broadcast
-
-        def mock_broadcast(_, activity, user):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
-
         self.list.curation = "curated"
         self.list.save(broadcast=False)
         request = self.factory.post(
@@ -368,16 +335,21 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        views.list.add_book(request)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
         item = self.list.listitem_set.get()
         self.assertEqual(item.book, self.book)
         self.assertEqual(item.user, self.local_user)
         self.assertTrue(item.approved)
-        models.ListItem.broadcast = real_broadcast
 
     def test_remove_book(self):
         """ take an item off a list """
-        real_broadcast = models.ListItem.broadcast
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             item = models.ListItem.objects.create(
@@ -387,14 +359,6 @@ class ListViews(TestCase):
             )
         self.assertTrue(self.list.listitem_set.exists())
 
-        def mock_broadcast(_, activity, user):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Remove")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        models.ListItem.broadcast = mock_broadcast
         request = self.factory.post(
             "",
             {
@@ -403,10 +367,15 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        views.list.remove_book(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.remove_book(request, self.list.id)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Remove")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
 
         self.assertFalse(self.list.listitem_set.exists())
-        models.ListItem.broadcast = real_broadcast
 
     def test_remove_book_unauthorized(self):
         """ take an item off a list """
@@ -425,6 +394,10 @@ class ListViews(TestCase):
         )
         request.user = self.rat
 
-        views.list.remove_book(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.list.remove_book(request, self.list.id)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args_list[1][0][1])
+            self.assertEqual(activity["type"], "Remove")
 
         self.assertTrue(self.list.listitem_set.exists())

From e550f8cde61459793ec4f7846761df5b10e3e4c7 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:47:38 -0700
Subject: [PATCH 05/22] Fixes list item fields and broadcast updates

---
 bookwyrm/models/activitypub_mixin.py | 9 ++++-----
 bookwyrm/models/list.py              | 6 +++---
 bookwyrm/models/shelf.py             | 8 ++++----
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 9b104456..41940fdf 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -361,12 +361,11 @@ class CollectionItemMixin(ActivitypubMixin):
 
     def save(self, *args, broadcast=True, **kwargs):
         """ broadcast updated """
-        created = not bool(self.id)
         # first off, we want to save normally no matter what
         super().save(*args, **kwargs)
 
-        # these shouldn't be edited, only created and deleted
-        if not broadcast or not created or not self.user.local:
+        # list items can be updateda, normally you would only broadcast on created
+        if not broadcast or not self.user.local:
             return
 
         # adding an obj to the collection
@@ -384,7 +383,7 @@ class CollectionItemMixin(ActivitypubMixin):
         """ AP for shelving a book"""
         collection_field = getattr(self, self.collection_field)
         return activitypub.Add(
-            id=self.get_remote_id(),
+            id="{:s}#add".format(collection_field.remote_id),
             actor=user.remote_id,
             object=self.to_activity_dataclass(),
             target=collection_field.remote_id,
@@ -394,7 +393,7 @@ class CollectionItemMixin(ActivitypubMixin):
         """ AP for un-shelving a book"""
         collection_field = getattr(self, self.collection_field)
         return activitypub.Remove(
-            id=self.get_remote_id(),
+            id="{:s}#remove".format(collection_field.remote_id),
             actor=user.remote_id,
             object=self.to_activity_dataclass(),
             target=collection_field.remote_id,
diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py
index b90eafe3..057c2a69 100644
--- a/bookwyrm/models/list.py
+++ b/bookwyrm/models/list.py
@@ -59,10 +59,10 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
     """ ok """
 
     book = fields.ForeignKey(
-        "Edition", on_delete=models.PROTECT, activitypub_field="object"
+        "Edition", on_delete=models.PROTECT, activitypub_field="book"
     )
-    book_list = fields.ForeignKey(
-        "List", on_delete=models.CASCADE, activitypub_field="target"
+    book_list = models.ForeignKey(
+        "List", on_delete=models.CASCADE
     )
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py
index 8a7095d0..22582859 100644
--- a/bookwyrm/models/shelf.py
+++ b/bookwyrm/models/shelf.py
@@ -32,7 +32,6 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
     )
 
     activity_serializer = activitypub.Shelf
-    collection_field = "shelf"
 
     def save(self, *args, **kwargs):
         """ set the identifier """
@@ -67,16 +66,17 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
     """ many to many join table for books and shelves """
 
     book = fields.ForeignKey(
-        "Edition", on_delete=models.PROTECT, activitypub_field="object"
+        "Edition", on_delete=models.PROTECT, activitypub_field="book"
     )
-    shelf = fields.ForeignKey(
-        "Shelf", on_delete=models.PROTECT, activitypub_field="target"
+    shelf = models.ForeignKey(
+        "Shelf", on_delete=models.PROTECT
     )
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
 
     activity_serializer = activitypub.ShelfItem
+    collection_field = "shelf"
 
     def save(self, *args, **kwargs):
         if not self.user:

From 1f930ba8211994beb26ad95d8dbaf7e968513522 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:51:52 -0700
Subject: [PATCH 06/22] Updates list view tests

---
 bookwyrm/tests/views/test_list.py | 72 +++++++++++++------------------
 1 file changed, 31 insertions(+), 41 deletions(-)

diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index 04e8aabe..0a851bf0 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -87,10 +87,10 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             result = view(request)
 
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Create")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Create")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
 
         self.assertEqual(result.status_code, 302)
         new_list = models.List.objects.filter(name="A list").get()
@@ -151,11 +151,11 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             result = view(request, self.list.id)
 
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Update")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["id"], self.list.remote_id)
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Update")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["id"], self.list.remote_id)
 
         self.assertEqual(result.status_code, 302)
 
@@ -208,11 +208,11 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             view(request, self.list.id)
 
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Add")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["target"], self.list.remote_id)
 
         pending.refresh_from_db()
         self.assertEqual(self.list.books.count(), 1)
@@ -239,10 +239,8 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
-            view(request, self.list.id)
-            self.assertEqual(mock.call_count, 1)
-            #activity = json.loads(mock.call_args_list[1][0][1])
+        view(request, self.list.id)
+
         self.assertFalse(self.list.books.exists())
         self.assertFalse(models.ListItem.objects.exists())
 
@@ -260,7 +258,7 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.list.add_book(request)
             self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
+            activity = json.loads(mock.call_args[0][1])
             self.assertEqual(activity["type"], "Add")
             self.assertEqual(activity["actor"], self.local_user.remote_id)
             self.assertEqual(activity["target"], self.list.remote_id)
@@ -286,7 +284,7 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.list.add_book(request)
             self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
+            activity = json.loads(mock.call_args[0][1])
             self.assertEqual(activity["type"], "Add")
             self.assertEqual(activity["actor"], self.rat.remote_id)
             self.assertEqual(activity["target"], self.list.remote_id)
@@ -311,13 +309,17 @@ class ListViews(TestCase):
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.list.add_book(request)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.rat.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-            self.assertEqual(activity["object"]["id"], self.book.remote_id)
+
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+
+        self.assertEqual(activity["type"], "Add")
+        self.assertEqual(activity["actor"], self.rat.remote_id)
+        self.assertEqual(activity["target"], self.list.remote_id)
+
         item = self.list.listitem_set.get()
+        self.assertEqual(activity["object"]["id"], item.remote_id)
+
         self.assertEqual(item.book, self.book)
         self.assertEqual(item.user, self.rat)
         self.assertFalse(item.approved)
@@ -338,7 +340,7 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.list.add_book(request)
             self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
+            activity = json.loads(mock.call_args[0][1])
             self.assertEqual(activity["type"], "Add")
             self.assertEqual(activity["actor"], self.local_user.remote_id)
             self.assertEqual(activity["target"], self.list.remote_id)
@@ -367,14 +369,7 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
-            views.list.remove_book(request, self.list.id)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Remove")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
+        views.list.remove_book(request, self.list.id)
         self.assertFalse(self.list.listitem_set.exists())
 
     def test_remove_book_unauthorized(self):
@@ -394,10 +389,5 @@ class ListViews(TestCase):
         )
         request.user = self.rat
 
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
-            views.list.remove_book(request, self.list.id)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args_list[1][0][1])
-            self.assertEqual(activity["type"], "Remove")
-
+        views.list.remove_book(request, self.list.id)
         self.assertTrue(self.list.listitem_set.exists())

From b1491c2ac69822b00a524f9c8d2a5801834b2ba3 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 14:53:45 -0700
Subject: [PATCH 07/22] Adds privacy and recipient details to list items

---
 bookwyrm/models/activitypub_mixin.py | 14 ++++++
 bookwyrm/models/list.py              |  4 +-
 bookwyrm/models/shelf.py             |  4 +-
 bookwyrm/tests/models/test_list.py   | 64 +++++++++++++++++++---------
 4 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 41940fdf..714dc485 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -359,6 +359,20 @@ class CollectionItemMixin(ActivitypubMixin):
 
     activity_serializer = activitypub.CollectionItem
 
+    @property
+    def privacy(self):
+        """ inherit the privacy of the list, or direct if pending """
+        collection_field = getattr(self, self.collection_field)
+        if self.approved:
+            return collection_field.privacy
+        return "direct"
+
+    @property
+    def recipients(self):
+        """ the owner of the list is a direct recipient """
+        collection_field = getattr(self, self.collection_field)
+        return [collection_field.user]
+
     def save(self, *args, broadcast=True, **kwargs):
         """ broadcast updated """
         # first off, we want to save normally no matter what
diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py
index 057c2a69..4d6b53cd 100644
--- a/bookwyrm/models/list.py
+++ b/bookwyrm/models/list.py
@@ -61,9 +61,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
     book = fields.ForeignKey(
         "Edition", on_delete=models.PROTECT, activitypub_field="book"
     )
-    book_list = models.ForeignKey(
-        "List", on_delete=models.CASCADE
-    )
+    book_list = models.ForeignKey("List", on_delete=models.CASCADE)
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py
index 22582859..5bbb84b9 100644
--- a/bookwyrm/models/shelf.py
+++ b/bookwyrm/models/shelf.py
@@ -68,9 +68,7 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
     book = fields.ForeignKey(
         "Edition", on_delete=models.PROTECT, activitypub_field="book"
     )
-    shelf = models.ForeignKey(
-        "Shelf", on_delete=models.PROTECT
-    )
+    shelf = models.ForeignKey("Shelf", on_delete=models.PROTECT)
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
diff --git a/bookwyrm/tests/models/test_list.py b/bookwyrm/tests/models/test_list.py
index 6bc4b796..f6ed0c96 100644
--- a/bookwyrm/tests/models/test_list.py
+++ b/bookwyrm/tests/models/test_list.py
@@ -11,45 +11,67 @@ class List(TestCase):
 
     def setUp(self):
         """ look, a list """
-        self.user = models.User.objects.create_user(
+        self.local_user = models.User.objects.create_user(
             "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse"
         )
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            self.list = models.List.objects.create(name="Test List", user=self.user)
+        work = models.Work.objects.create(title="hello")
+        self.book = models.Edition.objects.create(title="hi", parent_work=work)
 
     def test_remote_id(self, _):
         """ shelves use custom remote ids """
-        expected_id = "https://%s/list/%d" % (settings.DOMAIN, self.list.id)
-        self.assertEqual(self.list.get_remote_id(), expected_id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            book_list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
+        expected_id = "https://%s/list/%d" % (settings.DOMAIN, book_list.id)
+        self.assertEqual(book_list.get_remote_id(), expected_id)
 
     def test_to_activity(self, _):
         """ jsonify it """
-        activity_json = self.list.to_activity()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            book_list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
+        activity_json = book_list.to_activity()
         self.assertIsInstance(activity_json, dict)
-        self.assertEqual(activity_json["id"], self.list.remote_id)
+        self.assertEqual(activity_json["id"], book_list.remote_id)
         self.assertEqual(activity_json["totalItems"], 0)
         self.assertEqual(activity_json["type"], "BookList")
         self.assertEqual(activity_json["name"], "Test List")
-        self.assertEqual(activity_json["owner"], self.user.remote_id)
+        self.assertEqual(activity_json["owner"], self.local_user.remote_id)
 
     def test_list_item(self, _):
         """ a list entry """
-        work = models.Work.objects.create(title="hello")
-        book = models.Edition.objects.create(title="hi", parent_work=work)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            book_list = models.List.objects.create(
+                name="Test List", user=self.local_user, privacy="unlisted"
+            )
+
         item = models.ListItem.objects.create(
-            book_list=self.list,
-            book=book,
-            user=self.user,
+            book_list=book_list,
+            book=self.book,
+            user=self.local_user,
         )
 
         self.assertTrue(item.approved)
+        self.assertEqual(item.privacy, "unlisted")
+        self.assertEqual(item.recipients, [self.local_user])
 
-        add_activity = item.to_add_activity()
-        self.assertEqual(add_activity["actor"], self.user.remote_id)
-        self.assertEqual(add_activity["object"]["id"], book.remote_id)
-        self.assertEqual(add_activity["target"], self.list.remote_id)
+    def test_list_item_pending(self, _):
+        """ a list entry """
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            book_list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
 
-        remove_activity = item.to_remove_activity()
-        self.assertEqual(remove_activity["actor"], self.user.remote_id)
-        self.assertEqual(remove_activity["object"]["id"], book.remote_id)
-        self.assertEqual(remove_activity["target"], self.list.remote_id)
+        item = models.ListItem.objects.create(
+            book_list=book_list,
+            book=self.book,
+            user=self.local_user,
+            approved=False
+        )
+
+        self.assertFalse(item.approved)
+        self.assertEqual(item.book_list.privacy, "public")
+        self.assertEqual(item.privacy, "direct")
+        self.assertEqual(item.recipients, [self.local_user])

From ee6f37d7d28d7b4aceb7868e137d337d700db8b9 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:11:16 -0700
Subject: [PATCH 08/22] python formatting

---
 bookwyrm/activitypub/ordered_collection.py    |  3 ++
 .../migrations/0064_auto_20210408_2208.py     | 28 +++++++++++++++++++
 bookwyrm/tests/models/test_list.py            |  5 +---
 3 files changed, 32 insertions(+), 4 deletions(-)
 create mode 100644 bookwyrm/migrations/0064_auto_20210408_2208.py

diff --git a/bookwyrm/activitypub/ordered_collection.py b/bookwyrm/activitypub/ordered_collection.py
index 135c50d3..650f6a40 100644
--- a/bookwyrm/activitypub/ordered_collection.py
+++ b/bookwyrm/activitypub/ordered_collection.py
@@ -55,6 +55,7 @@ class OrderedCollectionPage(ActivityObject):
 @dataclass(init=False)
 class CollectionItem(ActivityObject):
     """ an item in a collection """
+
     actor: str
     type: str = "CollectionItem"
 
@@ -62,6 +63,7 @@ class CollectionItem(ActivityObject):
 @dataclass(init=False)
 class ListItem(CollectionItem):
     """ a book on a list """
+
     book: str
     notes: str = None
     approved: bool = True
@@ -72,5 +74,6 @@ class ListItem(CollectionItem):
 @dataclass(init=False)
 class ShelfItem(CollectionItem):
     """ a book on a list """
+
     book: str
     type: str = "ShelfItem"
diff --git a/bookwyrm/migrations/0064_auto_20210408_2208.py b/bookwyrm/migrations/0064_auto_20210408_2208.py
new file mode 100644
index 00000000..84a1a128
--- /dev/null
+++ b/bookwyrm/migrations/0064_auto_20210408_2208.py
@@ -0,0 +1,28 @@
+# Generated by Django 3.1.6 on 2021-04-08 22:08
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0063_auto_20210408_1556"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="listitem",
+            name="book_list",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE, to="bookwyrm.list"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="shelfbook",
+            name="shelf",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.PROTECT, to="bookwyrm.shelf"
+            ),
+        ),
+    ]
diff --git a/bookwyrm/tests/models/test_list.py b/bookwyrm/tests/models/test_list.py
index f6ed0c96..48f918a0 100644
--- a/bookwyrm/tests/models/test_list.py
+++ b/bookwyrm/tests/models/test_list.py
@@ -65,10 +65,7 @@ class List(TestCase):
             )
 
         item = models.ListItem.objects.create(
-            book_list=book_list,
-            book=self.book,
-            user=self.local_user,
-            approved=False
+            book_list=book_list, book=self.book, user=self.local_user, approved=False
         )
 
         self.assertFalse(item.approved)

From 43f0d4e6037dca9ee82e531ad16ae69dc0337756 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:16:49 -0700
Subject: [PATCH 09/22] Adds activity tests to shelf views tests

---
 bookwyrm/tests/views/test_shelf.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/test_shelf.py
index a308fe56..7ab01562 100644
--- a/bookwyrm/tests/views/test_shelf.py
+++ b/bookwyrm/tests/views/test_shelf.py
@@ -1,4 +1,5 @@
 """ test for app action functionality """
+import json
 from unittest.mock import patch
 from django.template.response import TemplateResponse
 from django.test import TestCase
@@ -120,8 +121,15 @@ class ShelfViews(TestCase):
             "", {"book": self.book.id, "shelf": self.shelf.identifier}
         )
         request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.shelve(request)
+
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Add")
+
+        item = models.ShelfBook.objects.get()
+        self.assertEqual(activity["object"]["id"], item.remote_id)
         # make sure the book is on the shelf
         self.assertEqual(self.shelf.books.get(), self.book)
 
@@ -170,10 +178,15 @@ class ShelfViews(TestCase):
             models.ShelfBook.objects.create(
                 book=self.book, user=self.local_user, shelf=self.shelf
             )
+        item = models.ShelfBook.objects.get()
+
         self.shelf.save()
         self.assertEqual(self.shelf.books.count(), 1)
         request = self.factory.post("", {"book": self.book.id, "shelf": self.shelf.id})
         request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.unshelve(request)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Remove")
+        self.assertEqual(activity["object"]["id"], item.remote_id)
         self.assertEqual(self.shelf.books.count(), 0)

From 24685187e87f6af0daeaa058e2dcd2f7de3c5081 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:22:06 -0700
Subject: [PATCH 10/22] Updates collection item add/remove handlers

---
 bookwyrm/activitypub/verbs.py | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py
index a56eb2fe..3a7910cf 100644
--- a/bookwyrm/activitypub/verbs.py
+++ b/bookwyrm/activitypub/verbs.py
@@ -145,15 +145,6 @@ class Add(Verb):
     object: CollectionItem
     type: str = "Add"
 
-    def action(self):
-        """ add obj to collection """
-        target = resolve_remote_id(self.target, refresh=False)
-        # we want to get the related field that isn't the book, this is janky af sorry
-        model = [t for t in type(target)._meta.related_objects if t.name != "edition"][
-            0
-        ].related_model
-        self.to_model(model=model)
-
 
 @dataclass(init=False)
 class Remove(Add):
@@ -163,11 +154,7 @@ class Remove(Add):
 
     def action(self):
         """ find and remove the activity object """
-        target = resolve_remote_id(self.target, refresh=False)
-        model = [t for t in type(target)._meta.related_objects if t.name != "edition"][
-            0
-        ].related_model
-        obj = self.to_model(model=model, save=False, allow_create=False)
+        obj = self.object.to_model(model=model, save=False, allow_create=False)
         obj.delete()
 
 

From a7375c4c139cff2866fe7e8df68dfa39cd7c2a11 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:23:56 -0700
Subject: [PATCH 11/22] Adds remove list item test and updates add/create tests

---
 bookwyrm/tests/views/inbox/test_inbox_add.py  |  9 +--
 .../tests/views/inbox/test_inbox_create.py    |  2 +-
 .../tests/views/inbox/test_inbox_remove.py    | 62 +++++++++++++++----
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py
index 638d56d7..1f396caf 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_add.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_add.py
@@ -97,10 +97,9 @@ class InboxActivities(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
-                "type": "Edition",
-                "title": "Test Title",
-                "work": work.remote_id,
-                "id": "https://bookwyrm.social/book/37292",
+                "type": "ListItem",
+                "book": self.edition.remote_id,
+                "id": "https://bookwyrm.social/listbook/6189",
             },
             "target": "https://bookwyrm.social/user/mouse/list/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -108,8 +107,10 @@ class InboxActivities(TestCase):
         views.inbox.activity_task(activity)
 
         booklist = models.List.objects.get()
+        listitem = models.ListItem.objects.get()
         self.assertEqual(booklist.name, "Test List")
         self.assertEqual(booklist.books.first(), book)
+        self.assertEqual(listitem.remote_id, "https://bookwyrm.social/listbook/6189")
 
     @responses.activate
     def test_handle_tag_book(self):
diff --git a/bookwyrm/tests/views/inbox/test_inbox_create.py b/bookwyrm/tests/views/inbox/test_inbox_create.py
index 437f6ffc..f8ed6a84 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_create.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_create.py
@@ -9,7 +9,7 @@ from bookwyrm import models, views
 
 
 # pylint: disable=too-many-public-methods
-class InboxActivities(TestCase):
+class InboxCreate(TestCase):
     """ readthrough tests """
 
     def setUp(self):
diff --git a/bookwyrm/tests/views/inbox/test_inbox_remove.py b/bookwyrm/tests/views/inbox/test_inbox_remove.py
index b3e42bbd..778f5077 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_remove.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_remove.py
@@ -7,11 +7,20 @@ from bookwyrm import models, views
 
 
 # pylint: disable=too-many-public-methods
-class InboxActivities(TestCase):
+class InboxRemove(TestCase):
     """ inbox tests """
 
     def setUp(self):
         """ basic user and book data """
+        self.local_user = models.User.objects.create_user(
+            "mouse@example.com",
+            "mouse@mouse.com",
+            "mouseword",
+            local=True,
+            localname="mouse",
+        )
+        self.local_user.remote_id = "https://example.com/user/mouse"
+        self.local_user.save(broadcast=False)
         with patch("bookwyrm.models.user.set_remote_server.delay"):
             self.remote_user = models.User.objects.create_user(
                 "rat",
@@ -22,26 +31,26 @@ class InboxActivities(TestCase):
                 inbox="https://example.com/users/rat/inbox",
                 outbox="https://example.com/users/rat/outbox",
             )
+        self.work = models.Work.objects.create(title="work title")
+        self.book = models.Edition.objects.create(
+            title="Test",
+            remote_id="https://bookwyrm.social/book/37292",
+            parent_work=self.work,
+        )
 
         models.SiteSettings.objects.create()
 
     def test_handle_unshelve_book(self):
         """ remove a book from a shelf """
-        work = models.Work.objects.create(title="work title")
-        book = models.Edition.objects.create(
-            title="Test",
-            remote_id="https://bookwyrm.social/book/37292",
-            parent_work=work,
-        )
         shelf = models.Shelf.objects.create(user=self.remote_user, name="Test Shelf")
         shelf.remote_id = "https://bookwyrm.social/user/mouse/shelf/to-read"
         shelf.save()
 
         shelfbook = models.ShelfBook.objects.create(
-            user=self.remote_user, shelf=shelf, book=book
+            user=self.remote_user, shelf=shelf, book=self.book
         )
 
-        self.assertEqual(shelf.books.first(), book)
+        self.assertEqual(shelf.books.first(), self.book)
         self.assertEqual(shelf.books.count(), 1)
 
         activity = {
@@ -51,11 +60,42 @@ class InboxActivities(TestCase):
             "object": {
                 "type": "Edition",
                 "title": "Test Title",
-                "work": work.remote_id,
-                "id": "https://bookwyrm.social/book/37292",
+                "work": self.work.remote_id,
+                "id": "https//bookwyrm.social/book/37292",
             },
             "target": "https://bookwyrm.social/user/mouse/shelf/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",
         }
         views.inbox.activity_task(activity)
         self.assertFalse(shelf.books.exists())
+
+    def test_handle_remove_book_from_list(self):
+        """ listing a book """
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            booklist = models.List.objects.create(
+                name="test list",
+                user=self.local_user,
+            )
+            listitem = models.ListItem.objects.create(
+                user=self.local_user,
+                book=self.book,
+                book_list=booklist,
+            )
+        self.assertEqual(booklist.books.count(), 1)
+
+        activity = {
+            "id": listitem.remote_id,
+            "type": "Remove",
+            "actor": "https://example.com/users/rat",
+            "object": {
+                "type": "Edition",
+                "title": "Test Title",
+                "work": self.work.remote_id,
+                "id": "https://bookwyrm.social/book/37292",
+            },
+            "target": booklist.remote_id,
+            "@context": "https://www.w3.org/ns/activitystreams",
+        }
+        views.inbox.activity_task(activity)
+
+        self.assertEqual(booklist.books.count(), 0)

From ddd05a68cfb94cccf1cb5a5150f03f0961df15f3 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:40:02 -0700
Subject: [PATCH 12/22] Resolve and assign target collection for collection
 items

---
 bookwyrm/activitypub/verbs.py                |  9 ++-
 bookwyrm/tests/views/inbox/test_inbox_add.py | 79 ++++----------------
 2 files changed, 22 insertions(+), 66 deletions(-)

diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py
index 3a7910cf..c3c84ee5 100644
--- a/bookwyrm/activitypub/verbs.py
+++ b/bookwyrm/activitypub/verbs.py
@@ -145,6 +145,13 @@ class Add(Verb):
     object: CollectionItem
     type: str = "Add"
 
+    def action(self):
+        """ figure out the target to assign the item to a collection  """
+        target = resolve_remote_id(self.target)
+        item = self.object.to_model(save=False)
+        setattr(item, item.collection_field, target)
+        item.save()
+
 
 @dataclass(init=False)
 class Remove(Add):
@@ -154,7 +161,7 @@ class Remove(Add):
 
     def action(self):
         """ find and remove the activity object """
-        obj = self.object.to_model(model=model, save=False, allow_create=False)
+        obj = self.object.to_model(save=False, allow_create=False)
         obj.delete()
 
 
diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py
index 1f396caf..f494efa6 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_add.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_add.py
@@ -8,7 +8,7 @@ from bookwyrm import models, views
 
 
 # pylint: disable=too-many-public-methods
-class InboxActivities(TestCase):
+class InboxAdd(TestCase):
     """ inbox tests """
 
     def setUp(self):
@@ -32,17 +32,17 @@ class InboxActivities(TestCase):
                 inbox="https://example.com/users/rat/inbox",
                 outbox="https://example.com/users/rat/outbox",
             )
+        work = models.Work.objects.create(title="work title")
+        self.book = models.Edition.objects.create(
+            title="Test",
+            remote_id="https://bookwyrm.social/book/37292",
+            parent_work=work,
+        )
 
         models.SiteSettings.objects.create()
 
     def test_handle_add_book_to_shelf(self):
         """ shelving a book """
-        work = models.Work.objects.create(title="work title")
-        book = models.Edition.objects.create(
-            title="Test",
-            remote_id="https://bookwyrm.social/book/37292",
-            parent_work=work,
-        )
         shelf = models.Shelf.objects.create(user=self.remote_user, name="Test Shelf")
         shelf.remote_id = "https://bookwyrm.social/user/mouse/shelf/to-read"
         shelf.save()
@@ -52,27 +52,19 @@ class InboxActivities(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
-                "type": "Edition",
-                "title": "Test Title",
-                "work": work.remote_id,
-                "id": "https://bookwyrm.social/book/37292",
+                "type": "ShelfItem",
+                "book": self.book.remote_id,
+                "id": "https://bookwyrm.social/listbook/6189",
             },
             "target": "https://bookwyrm.social/user/mouse/shelf/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",
         }
         views.inbox.activity_task(activity)
-        self.assertEqual(shelf.books.first(), book)
+        self.assertEqual(shelf.books.first(), self.book)
 
     @responses.activate
     def test_handle_add_book_to_list(self):
         """ listing a book """
-        work = models.Work.objects.create(title="work title")
-        book = models.Edition.objects.create(
-            title="Test",
-            remote_id="https://bookwyrm.social/book/37292",
-            parent_work=work,
-        )
-
         responses.add(
             responses.GET,
             "https://bookwyrm.social/user/mouse/list/to-read",
@@ -97,8 +89,9 @@ class InboxActivities(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
+                "actor": self.local_user.remote_id,
                 "type": "ListItem",
-                "book": self.edition.remote_id,
+                "book": self.book.remote_id,
                 "id": "https://bookwyrm.social/listbook/6189",
             },
             "target": "https://bookwyrm.social/user/mouse/list/to-read",
@@ -109,49 +102,5 @@ class InboxActivities(TestCase):
         booklist = models.List.objects.get()
         listitem = models.ListItem.objects.get()
         self.assertEqual(booklist.name, "Test List")
-        self.assertEqual(booklist.books.first(), book)
+        self.assertEqual(booklist.books.first(), self.book)
         self.assertEqual(listitem.remote_id, "https://bookwyrm.social/listbook/6189")
-
-    @responses.activate
-    def test_handle_tag_book(self):
-        """ listing a book """
-        work = models.Work.objects.create(title="work title")
-        book = models.Edition.objects.create(
-            title="Test",
-            remote_id="https://bookwyrm.social/book/37292",
-            parent_work=work,
-        )
-
-        responses.add(
-            responses.GET,
-            "https://www.example.com/tag/cool-tag",
-            json={
-                "id": "https://1b1a78582461.ngrok.io/tag/tag",
-                "type": "OrderedCollection",
-                "totalItems": 0,
-                "first": "https://1b1a78582461.ngrok.io/tag/tag?page=1",
-                "last": "https://1b1a78582461.ngrok.io/tag/tag?page=1",
-                "name": "cool tag",
-                "@context": "https://www.w3.org/ns/activitystreams",
-            },
-        )
-
-        activity = {
-            "id": "https://bookwyrm.social/listbook/6189#add",
-            "type": "Add",
-            "actor": "https://example.com/users/rat",
-            "object": {
-                "type": "Edition",
-                "title": "Test Title",
-                "work": work.remote_id,
-                "id": "https://bookwyrm.social/book/37292",
-            },
-            "target": "https://www.example.com/tag/cool-tag",
-            "@context": "https://www.w3.org/ns/activitystreams",
-        }
-        views.inbox.activity_task(activity)
-
-        tag = models.Tag.objects.get()
-        self.assertFalse(models.List.objects.exists())
-        self.assertEqual(tag.name, "cool tag")
-        self.assertEqual(tag.books.first(), book)

From 6310532ddfdd9839d3ec7fcb5178d2b4e047f9ae Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:43:19 -0700
Subject: [PATCH 13/22] Updates inbox add tests

---
 bookwyrm/tests/views/inbox/test_inbox_add.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py
index f494efa6..65d18860 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_add.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_add.py
@@ -52,6 +52,7 @@ class InboxAdd(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
+                "actor": self.local_user.remote_id,
                 "type": "ShelfItem",
                 "book": self.book.remote_id,
                 "id": "https://bookwyrm.social/listbook/6189",

From 32bf5107556562c537acbf1920a89c476db28f5d Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:46:06 -0700
Subject: [PATCH 14/22] Updates inbox remove tests

---
 bookwyrm/tests/views/inbox/test_inbox_remove.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_remove.py b/bookwyrm/tests/views/inbox/test_inbox_remove.py
index 778f5077..e19ea11b 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_remove.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_remove.py
@@ -58,10 +58,10 @@ class InboxRemove(TestCase):
             "type": "Remove",
             "actor": "https://example.com/users/rat",
             "object": {
-                "type": "Edition",
-                "title": "Test Title",
-                "work": self.work.remote_id,
-                "id": "https//bookwyrm.social/book/37292",
+                "actor": self.local_user.remote_id,
+                "type": "ShelfItem",
+                "book": self.book.remote_id,
+                "id": shelfbook.remote_id,
             },
             "target": "https://bookwyrm.social/user/mouse/shelf/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -88,10 +88,10 @@ class InboxRemove(TestCase):
             "type": "Remove",
             "actor": "https://example.com/users/rat",
             "object": {
-                "type": "Edition",
-                "title": "Test Title",
-                "work": self.work.remote_id,
-                "id": "https://bookwyrm.social/book/37292",
+                "actor": self.local_user.remote_id,
+                "type": "ListItem",
+                "book": self.book.remote_id,
+                "id": listitem.remote_id,
             },
             "target": booklist.remote_id,
             "@context": "https://www.w3.org/ns/activitystreams",

From 37aacee9c6819d75573d04be68711ca96ce7d306 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 15:50:21 -0700
Subject: [PATCH 15/22] Fixes actors in inbox add/remove tests

---
 bookwyrm/tests/views/inbox/test_inbox_add.py    | 12 ++++++------
 bookwyrm/tests/views/inbox/test_inbox_remove.py |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py
index 65d18860..b2b65338 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_add.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_add.py
@@ -13,15 +13,15 @@ class InboxAdd(TestCase):
 
     def setUp(self):
         """ basic user and book data """
-        self.local_user = models.User.objects.create_user(
+        local_user = models.User.objects.create_user(
             "mouse@example.com",
             "mouse@mouse.com",
             "mouseword",
             local=True,
             localname="mouse",
         )
-        self.local_user.remote_id = "https://example.com/user/mouse"
-        self.local_user.save(broadcast=False)
+        local_user.remote_id = "https://example.com/user/mouse"
+        local_user.save(broadcast=False)
         with patch("bookwyrm.models.user.set_remote_server.delay"):
             self.remote_user = models.User.objects.create_user(
                 "rat",
@@ -52,10 +52,10 @@ class InboxAdd(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
-                "actor": self.local_user.remote_id,
+                "actor": self.remote_user.remote_id,
                 "type": "ShelfItem",
                 "book": self.book.remote_id,
-                "id": "https://bookwyrm.social/listbook/6189",
+                "id": "https://bookwyrm.social/shelfbook/6189",
             },
             "target": "https://bookwyrm.social/user/mouse/shelf/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -90,7 +90,7 @@ class InboxAdd(TestCase):
             "type": "Add",
             "actor": "https://example.com/users/rat",
             "object": {
-                "actor": self.local_user.remote_id,
+                "actor": self.remote_user.remote_id,
                 "type": "ListItem",
                 "book": self.book.remote_id,
                 "id": "https://bookwyrm.social/listbook/6189",
diff --git a/bookwyrm/tests/views/inbox/test_inbox_remove.py b/bookwyrm/tests/views/inbox/test_inbox_remove.py
index e19ea11b..a17154d1 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_remove.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_remove.py
@@ -58,7 +58,7 @@ class InboxRemove(TestCase):
             "type": "Remove",
             "actor": "https://example.com/users/rat",
             "object": {
-                "actor": self.local_user.remote_id,
+                "actor": self.remote_user.remote_id,
                 "type": "ShelfItem",
                 "book": self.book.remote_id,
                 "id": shelfbook.remote_id,
@@ -88,7 +88,7 @@ class InboxRemove(TestCase):
             "type": "Remove",
             "actor": "https://example.com/users/rat",
             "object": {
-                "actor": self.local_user.remote_id,
+                "actor": self.remote_user.remote_id,
                 "type": "ListItem",
                 "book": self.book.remote_id,
                 "id": listitem.remote_id,

From af6cab2e3fe9012c4d1dd2cc293e090e772b4dbb Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 20:03:29 -0700
Subject: [PATCH 16/22] Don't reutnr an error when discarding activity objects

---
 bookwyrm/activitypub/base_activity.py             | 2 +-
 bookwyrm/tests/views/inbox/test_inbox_announce.py | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py
index 452f61e0..3d922b47 100644
--- a/bookwyrm/activitypub/base_activity.py
+++ b/bookwyrm/activitypub/base_activity.py
@@ -111,7 +111,7 @@ class ActivityObject:
             and hasattr(model, "ignore_activity")
             and model.ignore_activity(self)
         ):
-            raise ActivitySerializerError()
+            return None
 
         # check for an existing instance
         instance = instance or model.find_existing(self.serialize())
diff --git a/bookwyrm/tests/views/inbox/test_inbox_announce.py b/bookwyrm/tests/views/inbox/test_inbox_announce.py
index a730045a..95b70b5c 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_announce.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_announce.py
@@ -136,6 +136,9 @@ class InboxActivities(TestCase):
             "id": "http://www.faraway.com/boost/12",
             "actor": self.remote_user.remote_id,
             "object": status.remote_id,
+            "to": ["https://www.w3.org/ns/activitystreams#public"],
+            "cc": ["https://example.com/user/mouse/followers"],
+            "published": "Mon, 25 May 2020 19:31:20 GMT",
         }
         responses.add(
             responses.GET, status.remote_id, json=status.to_activity(), status=200

From c149003d89072f65adf3041373dc3a69704f197e Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 20:34:22 -0700
Subject: [PATCH 17/22] Fixes fields on unknown boost test

---
 bookwyrm/tests/views/inbox/test_inbox_announce.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_announce.py b/bookwyrm/tests/views/inbox/test_inbox_announce.py
index 95b70b5c..954d4e64 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_announce.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_announce.py
@@ -188,6 +188,7 @@ class InboxActivities(TestCase):
                 "id": "http://fake.com/unknown/boost",
                 "actor": self.remote_user.remote_id,
                 "object": self.status.remote_id,
+                "published": "Mon, 25 May 2020 19:31:20 GMT",
             },
         }
         views.inbox.activity_task(activity)

From 14b0b978a32582a1080371d4f56602b222c6a49c Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 8 Apr 2021 20:58:15 -0700
Subject: [PATCH 18/22] Updates shelf models tests

---
 bookwyrm/tests/models/test_shelf_model.py | 125 ++++++++--------------
 1 file changed, 46 insertions(+), 79 deletions(-)

diff --git a/bookwyrm/tests/models/test_shelf_model.py b/bookwyrm/tests/models/test_shelf_model.py
index ebda0499..45ae1fa1 100644
--- a/bookwyrm/tests/models/test_shelf_model.py
+++ b/bookwyrm/tests/models/test_shelf_model.py
@@ -1,4 +1,6 @@
 """ testing models """
+import json
+from unittest.mock import patch
 from django.test import TestCase
 
 from bookwyrm import models, settings
@@ -18,30 +20,19 @@ class Shelf(TestCase):
 
     def test_remote_id(self):
         """ shelves use custom remote ids """
-        real_broadcast = models.Shelf.broadcast
-
-        def broadcast_mock(_, activity, user, **kwargs):
-            """ nah """
-
-        models.Shelf.broadcast = broadcast_mock
-        shelf = models.Shelf.objects.create(
-            name="Test Shelf", identifier="test-shelf", user=self.local_user
-        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            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
         self.assertEqual(shelf.get_remote_id(), expected_id)
-        models.Shelf.broadcast = real_broadcast
 
     def test_to_activity(self):
         """ jsonify it """
-        real_broadcast = models.Shelf.broadcast
-
-        def empty_mock(_, activity, user, **kwargs):
-            """ nah """
-
-        models.Shelf.broadcast = empty_mock
-        shelf = models.Shelf.objects.create(
-            name="Test Shelf", identifier="test-shelf", user=self.local_user
-        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            shelf = models.Shelf.objects.create(
+                name="Test Shelf", identifier="test-shelf", user=self.local_user
+            )
         activity_json = shelf.to_activity()
         self.assertIsInstance(activity_json, dict)
         self.assertEqual(activity_json["id"], shelf.remote_id)
@@ -49,77 +40,53 @@ class Shelf(TestCase):
         self.assertEqual(activity_json["type"], "Shelf")
         self.assertEqual(activity_json["name"], "Test Shelf")
         self.assertEqual(activity_json["owner"], self.local_user.remote_id)
-        models.Shelf.broadcast = real_broadcast
 
     def test_create_update_shelf(self):
         """ create and broadcast shelf creation """
-        real_broadcast = models.Shelf.broadcast
 
-        def create_mock(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Create")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["name"], "Test Shelf")
-
-        models.Shelf.broadcast = create_mock
-
-        shelf = models.Shelf.objects.create(
-            name="Test Shelf", identifier="test-shelf", user=self.local_user
-        )
-
-        def update_mock(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Update")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["name"], "arthur russel")
-
-        models.Shelf.broadcast = update_mock
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            shelf = models.Shelf.objects.create(
+                name="Test Shelf", identifier="test-shelf", user=self.local_user
+            )
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Create")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["name"], "Test Shelf")
 
         shelf.name = "arthur russel"
-        shelf.save()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            shelf.save()
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Update")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["name"], "arthur russel")
         self.assertEqual(shelf.name, "arthur russel")
-        models.Shelf.broadcast = real_broadcast
 
     def test_shelve(self):
         """ create and broadcast shelf creation """
-        real_broadcast = models.Shelf.broadcast
-        real_shelfbook_broadcast = models.ShelfBook.broadcast
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            shelf = models.Shelf.objects.create(
+                name="Test Shelf", identifier="test-shelf", user=self.local_user
+            )
 
-        def add_mock(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["id"], self.book.remote_id)
-            self.assertEqual(activity["target"], shelf.remote_id)
-
-        def remove_mock(_, activity, user, **kwargs):
-            """ ok """
-            self.assertEqual(user.remote_id, self.local_user.remote_id)
-            self.assertEqual(activity["type"], "Remove")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["object"]["id"], self.book.remote_id)
-            self.assertEqual(activity["target"], shelf.remote_id)
-
-        def empty_mock(_, activity, user, **kwargs):
-            """ nah """
-
-        models.Shelf.broadcast = empty_mock
-        shelf = models.Shelf.objects.create(
-            name="Test Shelf", identifier="test-shelf", user=self.local_user
-        )
-
-        models.ShelfBook.broadcast = add_mock
-        shelf_book = models.ShelfBook.objects.create(
-            shelf=shelf, user=self.local_user, book=self.book
-        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            shelf_book = models.ShelfBook.objects.create(
+                shelf=shelf, user=self.local_user, book=self.book
+            )
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Add")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["id"], shelf_book.remote_id)
+        self.assertEqual(activity["target"], shelf.remote_id)
         self.assertEqual(shelf.books.first(), self.book)
 
-        models.ShelfBook.broadcast = remove_mock
-        shelf_book.delete()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            shelf_book.delete()
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[0][1])
+        self.assertEqual(activity["type"], "Remove")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["id"], shelf_book.remote_id)
+        self.assertEqual(activity["target"], shelf.remote_id)
         self.assertFalse(shelf.books.exists())
-
-        models.ShelfBook.broadcast = real_shelfbook_broadcast
-        models.Shelf.broadcast = real_broadcast

From fea298a1de5977cf9895f3bf73b5ba204cd85230 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Sat, 10 Apr 2021 16:08:10 -0700
Subject: [PATCH 19/22] Run tests in verbose mode

---
 .github/workflows/django-tests.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml
index 3ce368ec..ab0b0cdb 100644
--- a/.github/workflows/django-tests.yml
+++ b/.github/workflows/django-tests.yml
@@ -65,4 +65,4 @@ jobs:
         EMAIL_HOST_PASSWORD: ""
         EMAIL_USE_TLS: true
       run: |
-        python manage.py test
+        python manage.py test -v 2

From 98ff66cd16daa440d5b6b88b30a9b7d9e1b42d22 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Sat, 10 Apr 2021 18:15:13 -0700
Subject: [PATCH 20/22] Don't broadcast curation rejects

---
 bookwyrm/models/activitypub_mixin.py | 4 ++--
 bookwyrm/views/list.py               | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 714dc485..d0ab829d 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -386,11 +386,11 @@ class CollectionItemMixin(ActivitypubMixin):
         activity = self.to_add_activity(self.user)
         self.broadcast(activity, self.user)
 
-    def delete(self, *args, **kwargs):
+    def delete(self, *args, broadcast=True, **kwargs):
         """ broadcast a remove activity """
         activity = self.to_remove_activity(self.user)
         super().delete(*args, **kwargs)
-        if self.user.local:
+        if self.user.local and broadcast:
             self.broadcast(activity, self.user)
 
     def to_add_activity(self, user):
diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py
index 7724cd13..adf9840d 100644
--- a/bookwyrm/views/list.py
+++ b/bookwyrm/views/list.py
@@ -168,7 +168,7 @@ class Curate(View):
             suggestion.approved = True
             suggestion.save()
         else:
-            suggestion.delete()
+            suggestion.delete(broadcast=False)
         return redirect("list-curate", book_list.id)
 
 

From 6d76782ee188e5d3b89ef9ffb469941d519a3ec2 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Sun, 11 Apr 2021 09:00:44 -0700
Subject: [PATCH 21/22] Mocks list item remove test

---
 bookwyrm/tests/views/test_list.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index 0a851bf0..d669307c 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -369,7 +369,8 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        views.list.remove_book(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.remove_book(request, self.list.id)
         self.assertFalse(self.list.listitem_set.exists())
 
     def test_remove_book_unauthorized(self):

From 274ddeed85305d6712b4e9707a75c8801376df01 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Sun, 11 Apr 2021 09:34:28 -0700
Subject: [PATCH 22/22] Removes verbose flag on unit tests

---
 .github/workflows/django-tests.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml
index ab0b0cdb..3ce368ec 100644
--- a/.github/workflows/django-tests.yml
+++ b/.github/workflows/django-tests.yml
@@ -65,4 +65,4 @@ jobs:
         EMAIL_HOST_PASSWORD: ""
         EMAIL_USE_TLS: true
       run: |
-        python manage.py test -v 2
+        python manage.py test