diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 35b786f71..c67c5dcad 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/base_activity.py b/bookwyrm/activitypub/base_activity.py index 452f61e03..3d922b473 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/activitypub/ordered_collection.py b/bookwyrm/activitypub/ordered_collection.py index 6da608322..650f6a407 100644 --- a/bookwyrm/activitypub/ordered_collection.py +++ b/bookwyrm/activitypub/ordered_collection.py @@ -50,3 +50,30 @@ 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 3686b3f3e..c3c84ee5b 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,37 +141,27 @@ 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 """ - 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) + """ 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(Verb): +class Remove(Add): """Remove activity """ - target: ActivityObject type: str = "Remove" 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(save=False, allow_create=False) obj.delete() diff --git a/bookwyrm/migrations/0064_auto_20210408_2208.py b/bookwyrm/migrations/0064_auto_20210408_2208.py new file mode 100644 index 000000000..84a1a1284 --- /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/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 8dce42e4e..d0ab829d9 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,49 +357,59 @@ 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 + + @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 """ - 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 - activity = self.to_add_activity() + 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() + 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): + 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, + id="{:s}#add".format(collection_field.remote_id), + 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, + id="{:s}#remove".format(collection_field.remote_id), + 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 880c41229..4d6b53cde 100644 --- a/bookwyrm/models/list.py +++ b/bookwyrm/models/list.py @@ -59,11 +59,9 @@ class ListItem(CollectionItemMixin, BookWyrmModel): """ ok """ book = fields.ForeignKey( - "Edition", on_delete=models.PROTECT, activitypub_field="object" - ) - book_list = fields.ForeignKey( - "List", on_delete=models.CASCADE, activitypub_field="target" + "Edition", on_delete=models.PROTECT, activitypub_field="book" ) + book_list = models.ForeignKey("List", on_delete=models.CASCADE) user = fields.ForeignKey( "User", on_delete=models.PROTECT, activitypub_field="actor" ) @@ -72,8 +70,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 3209da5d9..5bbb84b9b 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -66,17 +66,14 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel): """ many to many join table for books and shelves """ book = fields.ForeignKey( - "Edition", on_delete=models.PROTECT, activitypub_field="object" - ) - shelf = fields.ForeignKey( - "Shelf", on_delete=models.PROTECT, activitypub_field="target" + "Edition", on_delete=models.PROTECT, activitypub_field="book" ) + shelf = models.ForeignKey("Shelf", on_delete=models.PROTECT) user = fields.ForeignKey( "User", on_delete=models.PROTECT, activitypub_field="actor" ) - activity_serializer = activitypub.Add - object_field = "book" + activity_serializer = activitypub.ShelfItem collection_field = "shelf" def save(self, *args, **kwargs): diff --git a/bookwyrm/tests/models/test_list.py b/bookwyrm/tests/models/test_list.py index 6bc4b796b..48f918a04 100644 --- a/bookwyrm/tests/models/test_list.py +++ b/bookwyrm/tests/models/test_list.py @@ -11,45 +11,64 @@ 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]) diff --git a/bookwyrm/tests/models/test_shelf_model.py b/bookwyrm/tests/models/test_shelf_model.py index ebda04999..45ae1fa13 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 diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py index 638d56d77..b2b653381 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_add.py +++ b/bookwyrm/tests/views/inbox/test_inbox_add.py @@ -8,20 +8,20 @@ from bookwyrm import models, views # pylint: disable=too-many-public-methods -class InboxActivities(TestCase): +class InboxAdd(TestCase): """ inbox tests """ 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", @@ -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,20 @@ 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", + "actor": self.remote_user.remote_id, + "type": "ShelfItem", + "book": self.book.remote_id, + "id": "https://bookwyrm.social/shelfbook/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,10 +90,10 @@ 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", + "actor": self.remote_user.remote_id, + "type": "ListItem", + "book": self.book.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,49 +101,7 @@ 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) - - @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) + self.assertEqual(booklist.books.first(), self.book) + self.assertEqual(listitem.remote_id, "https://bookwyrm.social/listbook/6189") diff --git a/bookwyrm/tests/views/inbox/test_inbox_announce.py b/bookwyrm/tests/views/inbox/test_inbox_announce.py index a730045a4..954d4e647 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 @@ -185,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) diff --git a/bookwyrm/tests/views/inbox/test_inbox_create.py b/bookwyrm/tests/views/inbox/test_inbox_create.py index 437f6ffc4..f8ed6a84a 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 b3e42bbd1..a17154d11 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 = { @@ -49,13 +58,44 @@ class InboxActivities(TestCase): "type": "Remove", "actor": "https://example.com/users/rat", "object": { - "type": "Edition", - "title": "Test Title", - "work": work.remote_id, - "id": "https://bookwyrm.social/book/37292", + "actor": self.remote_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", } 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": { + "actor": self.remote_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", + } + views.inbox.activity_task(activity) + + self.assertEqual(booklist.books.count(), 0) diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py index c2c754539..d669307cc 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[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[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[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,13 @@ class ListViews(TestCase): ) request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, self.list.id) + view(request, self.list.id) + 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 +255,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[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 +281,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[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 +307,25 @@ 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[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) - 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 +337,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[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 +361,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 +369,9 @@ 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()) - models.ListItem.broadcast = real_broadcast def test_remove_book_unauthorized(self): """ take an item off a list """ @@ -426,5 +391,4 @@ class ListViews(TestCase): request.user = self.rat views.list.remove_book(request, self.list.id) - self.assertTrue(self.list.listitem_set.exists()) diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/test_shelf.py index a308fe56a..7ab015624 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) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 34bd2e1cc..8c645159e 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): diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 7724cd137..adf9840d4 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)