From e19c4620ceab8197436fefca6f7841c362f65155 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 8 Nov 2021 12:00:08 -0800 Subject: [PATCH 1/7] Don't broadcast imported reviews outside bookwyrm --- bookwyrm/importers/importer.py | 6 ++++-- bookwyrm/models/activitypub_mixin.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/bookwyrm/importers/importer.py b/bookwyrm/importers/importer.py index 6d898a2a3..a5243cd3f 100644 --- a/bookwyrm/importers/importer.py +++ b/bookwyrm/importers/importer.py @@ -136,7 +136,7 @@ def handle_imported_book(source, user, item, include_reviews, privacy): if item.review else "" ) - models.Review.objects.create( + review = models.Review( user=user, book=item.book, name=review_title, @@ -147,10 +147,12 @@ def handle_imported_book(source, user, item, include_reviews, privacy): ) else: # just a rating - models.ReviewRating.objects.create( + review = models.ReviewRating( user=user, book=item.book, rating=item.rating, published_date=published_date_guess, privacy=privacy, ) + # only broadcast this review to other bookwyrm instances + review.save(software="bookwyrm") diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 3a88c5249..031963aad 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -195,7 +195,7 @@ class ActivitypubMixin: class ObjectMixin(ActivitypubMixin): """add this mixin for object models that are AP serializable""" - def save(self, *args, created=None, **kwargs): + def save(self, *args, created=None, software=None, **kwargs): """broadcast created/updated/deleted objects as appropriate""" broadcast = kwargs.get("broadcast", True) # this bonus kwarg would cause an error in the base save method @@ -219,15 +219,16 @@ class ObjectMixin(ActivitypubMixin): return try: - software = None # do we have a "pure" activitypub version of this for mastodon? - if hasattr(self, "pure_content"): + if not software and hasattr(self, "pure_content"): pure_activity = self.to_create_activity(user, pure=True) self.broadcast(pure_activity, user, software="other") + # set bookwyrm so that that type is also sent software = "bookwyrm" - # sends to BW only if we just did a pure version for masto - activity = self.to_create_activity(user) - self.broadcast(activity, user, software=software) + if software == "bookwyrm": + # sends to BW only if we just did a pure version for masto + activity = self.to_create_activity(user) + self.broadcast(activity, user, software=software) except AttributeError: # janky as heck, this catches the mutliple inheritence chain # for boosts and ignores this auxilliary broadcast @@ -241,8 +242,7 @@ class ObjectMixin(ActivitypubMixin): if isinstance(self, user_model): user = self # book data tracks last editor - elif hasattr(self, "last_edited_by"): - user = self.last_edited_by + user = user or getattr(self, "last_edited_by", None) # again, if we don't know the user or they're remote, don't bother if not user or not user.local: return From 20c6a3ea1c7e32055e1bbfea9b69a3d18c79495b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 08:56:28 -0800 Subject: [PATCH 2/7] Creates generic importer test file And removes some tests that duplicate the generic tests --- .../tests/importers/test_goodreads_import.py | 123 +------- bookwyrm/tests/importers/test_importer.py | 289 ++++++++++++++++++ .../importers/test_librarything_import.py | 50 --- 3 files changed, 290 insertions(+), 172 deletions(-) create mode 100644 bookwyrm/tests/importers/test_importer.py diff --git a/bookwyrm/tests/importers/test_goodreads_import.py b/bookwyrm/tests/importers/test_goodreads_import.py index d2b0ea7d3..a24512921 100644 --- a/bookwyrm/tests/importers/test_goodreads_import.py +++ b/bookwyrm/tests/importers/test_goodreads_import.py @@ -1,5 +1,4 @@ """ testing import """ -from collections import namedtuple import csv import pathlib from unittest.mock import patch @@ -7,11 +6,10 @@ import datetime import pytz from django.test import TestCase -import responses from bookwyrm import models from bookwyrm.importers import GoodreadsImporter -from bookwyrm.importers.importer import import_data, handle_imported_book +from bookwyrm.importers.importer import handle_imported_book def make_date(*args): @@ -48,9 +46,6 @@ class GoodreadsImport(TestCase): def test_create_job(self, *_): """creates the import job entry and checks csv""" import_job = self.importer.create_job(self.user, self.csv, False, "public") - self.assertEqual(import_job.user, self.user) - self.assertEqual(import_job.include_reviews, False) - self.assertEqual(import_job.privacy, "public") import_items = models.ImportItem.objects.filter(job=import_job).all() self.assertEqual(len(import_items), 3) @@ -79,33 +74,6 @@ class GoodreadsImport(TestCase): self.assertEqual(retry_items[1].index, 1) self.assertEqual(retry_items[1].data["Book Id"], "52691223") - def test_start_import(self, *_): - """begin loading books""" - import_job = self.importer.create_job(self.user, self.csv, False, "unlisted") - MockTask = namedtuple("Task", ("id")) - mock_task = MockTask(7) - with patch("bookwyrm.importers.importer.import_data.delay") as start: - start.return_value = mock_task - self.importer.start_import(import_job) - import_job.refresh_from_db() - self.assertEqual(import_job.task_id, "7") - - @responses.activate - def test_import_data(self, *_): - """resolve entry""" - import_job = self.importer.create_job(self.user, self.csv, False, "unlisted") - book = models.Edition.objects.create(title="Test Book") - - with patch( - "bookwyrm.models.import_job.ImportItem.get_book_from_isbn" - ) as resolve: - resolve.return_value = book - with patch("bookwyrm.importers.importer.handle_imported_book"): - import_data(self.importer.service, import_job.id) - - import_item = models.ImportItem.objects.get(job=import_job, index=0) - self.assertEqual(import_item.book.id, book.id) - def test_handle_imported_book(self, *_): """goodreads import added a book, this adds related connections""" shelf = self.user.shelf_set.filter(identifier="read").first() @@ -137,76 +105,6 @@ class GoodreadsImport(TestCase): self.assertEqual(readthrough.start_date, make_date(2020, 10, 21)) self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25)) - def test_handle_imported_book_already_shelved(self, *_): - """goodreads import added a book, this adds related connections""" - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - shelf = self.user.shelf_set.filter(identifier="to-read").first() - models.ShelfBook.objects.create( - shelf=shelf, - user=self.user, - book=self.book, - shelved_date=make_date(2020, 2, 2), - ) - - import_job = models.ImportJob.objects.create(user=self.user) - datafile = pathlib.Path(__file__).parent.joinpath("../data/goodreads.csv") - csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding - for index, entry in enumerate(list(csv.DictReader(csv_file))): - entry = self.importer.parse_fields(entry) - import_item = models.ImportItem.objects.create( - job_id=import_job.id, index=index, data=entry, book=self.book - ) - break - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - handle_imported_book( - self.importer.service, self.user, import_item, False, "public" - ) - - shelf.refresh_from_db() - self.assertEqual(shelf.books.first(), self.book) - self.assertEqual( - shelf.shelfbook_set.first().shelved_date, make_date(2020, 2, 2) - ) - self.assertIsNone(self.user.shelf_set.get(identifier="read").books.first()) - - readthrough = models.ReadThrough.objects.get(user=self.user) - self.assertEqual(readthrough.book, self.book) - self.assertEqual(readthrough.start_date, make_date(2020, 10, 21)) - self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25)) - - def test_handle_import_twice(self, *_): - """re-importing books""" - shelf = self.user.shelf_set.filter(identifier="read").first() - import_job = models.ImportJob.objects.create(user=self.user) - datafile = pathlib.Path(__file__).parent.joinpath("../data/goodreads.csv") - csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding - for index, entry in enumerate(list(csv.DictReader(csv_file))): - entry = self.importer.parse_fields(entry) - import_item = models.ImportItem.objects.create( - job_id=import_job.id, index=index, data=entry, book=self.book - ) - break - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - handle_imported_book( - self.importer.service, self.user, import_item, False, "public" - ) - handle_imported_book( - self.importer.service, self.user, import_item, False, "public" - ) - - shelf.refresh_from_db() - self.assertEqual(shelf.books.first(), self.book) - self.assertEqual( - shelf.shelfbook_set.first().shelved_date, make_date(2020, 10, 21) - ) - - readthrough = models.ReadThrough.objects.get(user=self.user) - self.assertEqual(readthrough.book, self.book) - self.assertEqual(readthrough.start_date, make_date(2020, 10, 21)) - self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25)) - @patch("bookwyrm.activitystreams.add_status_task.delay") def test_handle_imported_book_review(self, *_): """goodreads review import""" @@ -252,22 +150,3 @@ class GoodreadsImport(TestCase): self.assertEqual(review.rating, 2) self.assertEqual(review.published_date, make_date(2019, 7, 8)) self.assertEqual(review.privacy, "unlisted") - - def test_handle_imported_book_reviews_disabled(self, *_): - """goodreads review import""" - import_job = models.ImportJob.objects.create(user=self.user) - datafile = pathlib.Path(__file__).parent.joinpath("../data/goodreads.csv") - csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding - entry = list(csv.DictReader(csv_file))[2] - entry = self.importer.parse_fields(entry) - import_item = models.ImportItem.objects.create( - job_id=import_job.id, index=0, data=entry, book=self.book - ) - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - handle_imported_book( - self.importer.service, self.user, import_item, False, "unlisted" - ) - self.assertFalse( - models.Review.objects.filter(book=self.book, user=self.user).exists() - ) diff --git a/bookwyrm/tests/importers/test_importer.py b/bookwyrm/tests/importers/test_importer.py new file mode 100644 index 000000000..f5d9af302 --- /dev/null +++ b/bookwyrm/tests/importers/test_importer.py @@ -0,0 +1,289 @@ +""" testing import """ +from collections import namedtuple +import csv +import pathlib +from unittest.mock import patch +import datetime +import pytz + +from django.test import TestCase +import responses + +from bookwyrm import models +from bookwyrm.importers import Importer +from bookwyrm.importers.importer import import_data, handle_imported_book + + +def make_date(*args): + """helper function to easily generate a date obj""" + return datetime.datetime(*args, tzinfo=pytz.UTC) + + +# pylint: disable=consider-using-with +@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") +@patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.activitystreams.add_book_statuses_task.delay") +class GenericImporter(TestCase): + """importing from csv""" + + def setUp(self): + """use a test csv""" + + class TestImporter(Importer): + """basic importer""" + + mandatory_fields = ["title", "author"] + + def parse_fields(self, entry): + return { + "id": entry["id"], + "Title": entry["title"], + "Author": entry["author"], + "ISBN13": entry["ISBN"], + "Star Rating": entry["rating"], + "My Rating": entry["rating"], + "My Review": entry["review"], + "Exclusive Shelf": entry["shelf"], + "Date Added": entry["added"], + "Date Read": None, + } + + self.importer = TestImporter() + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + self.csv = open(datafile, "r", encoding=self.importer.encoding) + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True + ) + + work = models.Work.objects.create(title="Test Work") + self.book = models.Edition.objects.create( + title="Example Edition", + remote_id="https://example.com/book/1", + parent_work=work, + ) + + def test_create_job(self, *_): + """creates the import job entry and checks csv""" + import_job = self.importer.create_job( + self.local_user, self.csv, False, "public" + ) + self.assertEqual(import_job.user, self.local_user) + self.assertEqual(import_job.include_reviews, False) + self.assertEqual(import_job.privacy, "public") + + import_items = models.ImportItem.objects.filter(job=import_job).all() + self.assertEqual(len(import_items), 4) + self.assertEqual(import_items[0].index, 0) + self.assertEqual(import_items[0].data["id"], "38") + self.assertEqual(import_items[1].index, 1) + self.assertEqual(import_items[1].data["id"], "48") + self.assertEqual(import_items[2].index, 2) + self.assertEqual(import_items[2].data["id"], "23") + self.assertEqual(import_items[3].index, 3) + self.assertEqual(import_items[3].data["id"], "10") + + def test_create_retry_job(self, *_): + """trying again with items that didn't import""" + import_job = self.importer.create_job( + self.local_user, self.csv, False, "unlisted" + ) + import_items = models.ImportItem.objects.filter(job=import_job).all()[:2] + + retry = self.importer.create_retry_job( + self.local_user, import_job, import_items + ) + self.assertNotEqual(import_job, retry) + self.assertEqual(retry.user, self.local_user) + self.assertEqual(retry.include_reviews, False) + self.assertEqual(retry.privacy, "unlisted") + + retry_items = models.ImportItem.objects.filter(job=retry).all() + self.assertEqual(len(retry_items), 2) + self.assertEqual(retry_items[0].index, 0) + self.assertEqual(retry_items[0].data["id"], "38") + self.assertEqual(retry_items[1].index, 1) + self.assertEqual(retry_items[1].data["id"], "48") + + def test_start_import(self, *_): + """check that a task was created""" + import_job = self.importer.create_job( + self.local_user, self.csv, False, "unlisted" + ) + MockTask = namedtuple("Task", ("id")) + mock_task = MockTask(7) + with patch("bookwyrm.importers.importer.import_data.delay") as start: + start.return_value = mock_task + self.importer.start_import(import_job) + import_job.refresh_from_db() + self.assertEqual(import_job.task_id, "7") + + @responses.activate + def test_import_data(self, *_): + """resolve entry""" + import_job = self.importer.create_job( + self.local_user, self.csv, False, "unlisted" + ) + book = models.Edition.objects.create(title="Test Book") + + with patch( + "bookwyrm.models.import_job.ImportItem.get_book_from_isbn" + ) as resolve: + resolve.return_value = book + with patch("bookwyrm.importers.importer.handle_imported_book"): + import_data(self.importer.service, import_job.id) + + import_item = models.ImportItem.objects.get(job=import_job, index=0) + self.assertEqual(import_item.book.id, book.id) + + def test_handle_imported_book(self, *_): + """import added a book, this adds related connections""" + shelf = self.local_user.shelf_set.filter(identifier="read").first() + self.assertIsNone(shelf.books.first()) + + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + for index, entry in enumerate(list(csv.DictReader(csv_file))): + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book + ) + break + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.local_user, import_item, False, "public" + ) + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + + def test_handle_imported_book_already_shelved(self, *_): + """import added a book, this adds related connections""" + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + shelf = self.local_user.shelf_set.filter(identifier="to-read").first() + models.ShelfBook.objects.create( + shelf=shelf, + user=self.local_user, + book=self.book, + shelved_date=make_date(2020, 2, 2), + ) + + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + for index, entry in enumerate(list(csv.DictReader(csv_file))): + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book + ) + break + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.local_user, import_item, False, "public" + ) + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + self.assertEqual( + shelf.shelfbook_set.first().shelved_date, make_date(2020, 2, 2) + ) + self.assertIsNone( + self.local_user.shelf_set.get(identifier="read").books.first() + ) + + def test_handle_import_twice(self, *_): + """re-importing books""" + shelf = self.local_user.shelf_set.filter(identifier="read").first() + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + for index, entry in enumerate(list(csv.DictReader(csv_file))): + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book + ) + break + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.local_user, import_item, False, "public" + ) + handle_imported_book( + self.importer.service, self.local_user, import_item, False, "public" + ) + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + + @patch("bookwyrm.activitystreams.add_status_task.delay") + def test_handle_imported_book_review(self, *_): + """review import""" + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + entry = list(csv.DictReader(csv_file))[3] + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book + ) + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.models.Status.broadcast") as broadcast_mock: + handle_imported_book( + self.importer.service, + self.local_user, + import_item, + True, + "unlisted", + ) + kwargs = broadcast_mock.call_args.kwargs + self.assertEqual(kwargs["software"], "bookwyrm") + review = models.Review.objects.get(book=self.book, user=self.local_user) + self.assertEqual(review.content, "mixed feelings") + self.assertEqual(review.rating, 2.0) + self.assertEqual(review.privacy, "unlisted") + + @patch("bookwyrm.activitystreams.add_status_task.delay") + def test_handle_imported_book_rating(self, *_): + """rating import""" + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + entry = list(csv.DictReader(csv_file))[1] + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book + ) + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.local_user, import_item, True, "unlisted" + ) + review = models.ReviewRating.objects.get(book=self.book, user=self.local_user) + self.assertIsInstance(review, models.ReviewRating) + self.assertEqual(review.rating, 3.0) + self.assertEqual(review.privacy, "unlisted") + + def test_handle_imported_book_reviews_disabled(self, *_): + """review import""" + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/generic.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + entry = list(csv.DictReader(csv_file))[2] + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book + ) + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.local_user, import_item, False, "unlisted" + ) + self.assertFalse( + models.Review.objects.filter(book=self.book, user=self.local_user).exists() + ) diff --git a/bookwyrm/tests/importers/test_librarything_import.py b/bookwyrm/tests/importers/test_librarything_import.py index ab92c11b1..f76666a78 100644 --- a/bookwyrm/tests/importers/test_librarything_import.py +++ b/bookwyrm/tests/importers/test_librarything_import.py @@ -157,37 +157,6 @@ class LibrarythingImport(TestCase): self.assertEqual(readthrough.start_date, make_date(2007, 4, 16)) self.assertEqual(readthrough.finish_date, make_date(2007, 5, 8)) - def test_handle_import_twice(self, *_): - """re-importing books""" - shelf = self.user.shelf_set.filter(identifier="read").first() - import_job = models.ImportJob.objects.create(user=self.user) - datafile = pathlib.Path(__file__).parent.joinpath("../data/librarything.tsv") - csv_file = open(datafile, "r", encoding=self.importer.encoding) - for index, entry in enumerate( - list(csv.DictReader(csv_file, delimiter=self.importer.delimiter)) - ): - entry = self.importer.parse_fields(entry) - import_item = models.ImportItem.objects.create( - job_id=import_job.id, index=index, data=entry, book=self.book - ) - break - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - handle_imported_book( - self.importer.service, self.user, import_item, False, "public" - ) - handle_imported_book( - self.importer.service, self.user, import_item, False, "public" - ) - - shelf.refresh_from_db() - self.assertEqual(shelf.books.first(), self.book) - - readthrough = models.ReadThrough.objects.get(user=self.user) - self.assertEqual(readthrough.book, self.book) - self.assertEqual(readthrough.start_date, make_date(2007, 4, 16)) - self.assertEqual(readthrough.finish_date, make_date(2007, 5, 8)) - @patch("bookwyrm.activitystreams.add_status_task.delay") def test_handle_imported_book_review(self, *_): """librarything review import""" @@ -209,22 +178,3 @@ class LibrarythingImport(TestCase): self.assertEqual(review.rating, 5) self.assertEqual(review.published_date, make_date(2007, 5, 8)) self.assertEqual(review.privacy, "unlisted") - - def test_handle_imported_book_reviews_disabled(self, *_): - """librarything review import""" - import_job = models.ImportJob.objects.create(user=self.user) - datafile = pathlib.Path(__file__).parent.joinpath("../data/librarything.tsv") - csv_file = open(datafile, "r", encoding=self.importer.encoding) - entry = list(csv.DictReader(csv_file, delimiter=self.importer.delimiter))[2] - entry = self.importer.parse_fields(entry) - import_item = models.ImportItem.objects.create( - job_id=import_job.id, index=0, data=entry, book=self.book - ) - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - handle_imported_book( - self.importer.service, self.user, import_item, False, "unlisted" - ) - self.assertFalse( - models.Review.objects.filter(book=self.book, user=self.user).exists() - ) From aeef472ee1542e01e6d7e0d204faab725c39a43e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 09:33:00 -0800 Subject: [PATCH 3/7] Fixes flow in checking software for broadcast --- bookwyrm/models/activitypub_mixin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 031963aad..fb2771ae6 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -220,7 +220,7 @@ class ObjectMixin(ActivitypubMixin): try: # do we have a "pure" activitypub version of this for mastodon? - if not software and hasattr(self, "pure_content"): + if not software == "bookwyrm" and hasattr(self, "pure_content"): pure_activity = self.to_create_activity(user, pure=True) self.broadcast(pure_activity, user, software="other") # set bookwyrm so that that type is also sent From 1b9d08414f729857a2ba9080ea86de74cb53033f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 09:53:52 -0800 Subject: [PATCH 4/7] Adds storygraph tests --- bookwyrm/importers/storygraph_import.py | 2 +- bookwyrm/tests/data/generic.csv | 5 + bookwyrm/tests/data/storygraph.csv | 3 + .../tests/importers/test_storygraph_import.py | 122 ++++++++++++++++++ 4 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 bookwyrm/tests/data/generic.csv create mode 100644 bookwyrm/tests/data/storygraph.csv create mode 100644 bookwyrm/tests/importers/test_storygraph_import.py diff --git a/bookwyrm/importers/storygraph_import.py b/bookwyrm/importers/storygraph_import.py index 25498432c..bf532e58e 100644 --- a/bookwyrm/importers/storygraph_import.py +++ b/bookwyrm/importers/storygraph_import.py @@ -21,7 +21,7 @@ class StorygraphImporter(Importer): data["ISBN13"] = entry["ISBN"] data["My Review"] = entry["Review"] if entry["Star Rating"]: - data["My Rating"] = math.ceil(float(entry["Star Rating"])) + data["My Rating"] = float(entry["Star Rating"]) else: data["My Rating"] = "" diff --git a/bookwyrm/tests/data/generic.csv b/bookwyrm/tests/data/generic.csv new file mode 100644 index 000000000..a081a6421 --- /dev/null +++ b/bookwyrm/tests/data/generic.csv @@ -0,0 +1,5 @@ +id,title,author,ISBN,rating,shelf,review,added +38,Gideon the Ninth (The Locked Tomb #1),Tamsyn Muir,"9781250313195",,read,,2021-11-10 +48,Harrow the Ninth (The Locked Tomb #2),Tamsyn Muir,,3,read,,2021-11-10 +23,Subcutanean,Aaron A. Reed,,,read,,2021-11-10 +10,Patisserie at Home,Mélanie Dupuis,"9780062445315",2,read,"mixed feelings",2021-11-10 diff --git a/bookwyrm/tests/data/storygraph.csv b/bookwyrm/tests/data/storygraph.csv new file mode 100644 index 000000000..4dd0b16e4 --- /dev/null +++ b/bookwyrm/tests/data/storygraph.csv @@ -0,0 +1,3 @@ +Title,Authors,Contributors,ISBN,Format,Read Status,Date Added,Last Date Read,Dates Read,Read Count,Moods,Pace,Character- or Plot-Driven?,Strong Character Development?,Loveable Characters?,Diverse Characters?,Flawed Characters?,Star Rating,Review,Content Warnings,Content Warning Description,Tags,Owned? +Always Coming Home,"Ursula K. Le Guin, Todd Barton, Margaret Chodos-Irvine","",,,to-read,2021/05/10,"","",0,"",,,,,,,,,"",,"",No +Subprime Attention Crisis,Tim Hwang,"",,,read,2021/05/10,"","",1,informative,fast,,,,,,5.0,"","","","",No diff --git a/bookwyrm/tests/importers/test_storygraph_import.py b/bookwyrm/tests/importers/test_storygraph_import.py new file mode 100644 index 000000000..addf362cb --- /dev/null +++ b/bookwyrm/tests/importers/test_storygraph_import.py @@ -0,0 +1,122 @@ +""" testing import """ +import csv +import pathlib +from unittest.mock import patch +import datetime +import pytz + +from django.test import TestCase + +from bookwyrm import models +from bookwyrm.importers import StorygraphImporter +from bookwyrm.importers.importer import handle_imported_book + + +def make_date(*args): + """helper function to easily generate a date obj""" + return datetime.datetime(*args, tzinfo=pytz.UTC) + + +# pylint: disable=consider-using-with +@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") +@patch("bookwyrm.activitystreams.populate_stream_task.delay") +@patch("bookwyrm.activitystreams.add_book_statuses_task.delay") +class StorygraphImport(TestCase): + """importing from storygraph csv""" + + def setUp(self): + """use a test csv""" + self.importer = StorygraphImporter() + datafile = pathlib.Path(__file__).parent.joinpath("../data/storygraph.csv") + self.csv = open(datafile, "r", encoding=self.importer.encoding) + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + self.user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True + ) + + work = models.Work.objects.create(title="Test Work") + self.book = models.Edition.objects.create( + title="Example Edition", + remote_id="https://example.com/book/1", + parent_work=work, + ) + + def test_create_job(self, *_): + """creates the import job entry and checks csv""" + import_job = self.importer.create_job(self.user, self.csv, False, "public") + + import_items = models.ImportItem.objects.filter(job=import_job).all() + self.assertEqual(len(import_items), 2) + self.assertEqual(import_items[0].index, 0) + self.assertEqual(import_items[0].data["Title"], "Always Coming Home") + self.assertEqual(import_items[1].index, 1) + self.assertEqual(import_items[1].data["Title"], "Subprime Attention Crisis") + self.assertEqual(import_items[1].data["My Rating"], 5.0) + + def test_create_retry_job(self, *_): + """trying again with items that didn't import""" + import_job = self.importer.create_job(self.user, self.csv, False, "unlisted") + import_items = models.ImportItem.objects.filter(job=import_job).all()[:2] + + retry = self.importer.create_retry_job(self.user, import_job, import_items) + self.assertNotEqual(import_job, retry) + self.assertEqual(retry.user, self.user) + self.assertEqual(retry.include_reviews, False) + self.assertEqual(retry.privacy, "unlisted") + + retry_items = models.ImportItem.objects.filter(job=retry).all() + self.assertEqual(len(retry_items), 2) + self.assertEqual(retry_items[0].index, 0) + self.assertEqual(retry_items[0].data["Title"], "Always Coming Home") + self.assertEqual(retry_items[1].index, 1) + self.assertEqual(retry_items[1].data["Title"], "Subprime Attention Crisis") + + def test_handle_imported_book(self, *_): + """storygraph import added a book, this adds related connections""" + shelf = self.user.shelf_set.filter(identifier="to-read").first() + self.assertIsNone(shelf.books.first()) + + import_job = models.ImportJob.objects.create(user=self.user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/storygraph.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + for index, entry in enumerate(list(csv.DictReader(csv_file))): + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book + ) + break + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.user, import_item, False, "public" + ) + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + self.assertEqual( + shelf.shelfbook_set.first().shelved_date, make_date(2021, 5, 10) + ) + + @patch("bookwyrm.activitystreams.add_status_task.delay") + def test_handle_imported_book_rating(self, *_): + """storygraph rating import""" + import_job = models.ImportJob.objects.create(user=self.user) + datafile = pathlib.Path(__file__).parent.joinpath("../data/storygraph.csv") + csv_file = open(datafile, "r") # pylint: disable=unspecified-encoding + entry = list(csv.DictReader(csv_file))[1] + entry = self.importer.parse_fields(entry) + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book + ) + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + handle_imported_book( + self.importer.service, self.user, import_item, True, "unlisted" + ) + review = models.ReviewRating.objects.get(book=self.book, user=self.user) + self.assertIsInstance(review, models.ReviewRating) + self.assertEqual(review.rating, 5.0) + self.assertEqual(review.published_date, make_date(2021, 5, 10)) + self.assertEqual(review.privacy, "unlisted") From 97a71f5e39ea4cee4d96af9f7f6b3b7971aee27f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 09:55:56 -0800 Subject: [PATCH 5/7] Cleans up software check syntax --- bookwyrm/models/activitypub_mixin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index fb2771ae6..0cb8a2cce 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -220,7 +220,7 @@ class ObjectMixin(ActivitypubMixin): try: # do we have a "pure" activitypub version of this for mastodon? - if not software == "bookwyrm" and hasattr(self, "pure_content"): + if software != "bookwyrm" and hasattr(self, "pure_content"): pure_activity = self.to_create_activity(user, pure=True) self.broadcast(pure_activity, user, software="other") # set bookwyrm so that that type is also sent From 7e784fa705521be9fa0688276918ca105e5ab20f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:35:37 -0800 Subject: [PATCH 6/7] Removes used import --- bookwyrm/importers/storygraph_import.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bookwyrm/importers/storygraph_import.py b/bookwyrm/importers/storygraph_import.py index bf532e58e..1333b8b9d 100644 --- a/bookwyrm/importers/storygraph_import.py +++ b/bookwyrm/importers/storygraph_import.py @@ -1,6 +1,5 @@ """ handle reading a csv from librarything """ import re -import math from . import Importer From cf477a03aee26373fa4105862100b904bfa17907 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Nov 2021 10:39:51 -0800 Subject: [PATCH 7/7] Corrects broadcast flow for objects --- bookwyrm/models/activitypub_mixin.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 0cb8a2cce..e1276d465 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -225,10 +225,9 @@ class ObjectMixin(ActivitypubMixin): self.broadcast(pure_activity, user, software="other") # set bookwyrm so that that type is also sent software = "bookwyrm" - if software == "bookwyrm": - # sends to BW only if we just did a pure version for masto - activity = self.to_create_activity(user) - self.broadcast(activity, user, software=software) + # sends to BW only if we just did a pure version for masto + activity = self.to_create_activity(user) + self.broadcast(activity, user, software=software) except AttributeError: # janky as heck, this catches the mutliple inheritence chain # for boosts and ignores this auxilliary broadcast