From 8c8aae2c92b0eb2bdb26f8ba71d79b339aa2aa92 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 11:59:14 -0800 Subject: [PATCH 1/3] Check if a book is already shelved after import --- bookwyrm/connectors/abstract_connector.py | 2 +- bookwyrm/outgoing.py | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 82b99378..d63bd135 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -205,7 +205,7 @@ def get_data(url): 'User-Agent': settings.USER_AGENT, }, ) - except RequestError: + except (RequestError, SSLError): raise ConnectorException() if not resp.ok: resp.raise_for_status() diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index fd4dff8f..172563bb 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -166,22 +166,24 @@ def handle_imported_book(user, item, include_reviews, privacy): if not item.book: return - if item.shelf: + existing_shelf = models.ShelfBook.objects.filter( + book=item.book, added_by=user).exists() + + # shelve the book if it hasn't been shelved already + if item.shelf and not existing_shelf: desired_shelf = models.Shelf.objects.get( identifier=item.shelf, user=user ) - # shelve the book if it hasn't been shelved already - shelf_book, created = models.ShelfBook.objects.get_or_create( + shelf_book = models.ShelfBook.objects.create( book=item.book, shelf=desired_shelf, added_by=user) - if created: - broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) + broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) - # only add new read-throughs if the item isn't already shelved - for read in item.reads: - read.book = item.book - read.user = user - read.save() + # only add new read-throughs if the item isn't already shelved + for read in item.reads: + read.book = item.book + read.user = user + read.save() if include_reviews and (item.rating or item.review): review_title = 'Review of {!r} on Goodreads'.format( From b2c22c5b7f77feb9ef430da78a6bece16641db6f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 13:09:40 -0800 Subject: [PATCH 2/3] Tests handle import shelving --- bookwyrm/tests/test_outgoing.py | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index 14d67aeb..f0125907 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -1,4 +1,5 @@ ''' sending out activities ''' +import csv import json import pathlib from unittest.mock import patch @@ -258,6 +259,60 @@ class Outgoing(TestCase): self.assertEqual(self.shelf.books.count(), 0) + def test_handle_imported_book(self): + ''' goodreads 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/goodreads.csv') + csv_file = open(datafile, 'r') + for index, entry in enumerate(list(csv.DictReader(csv_file))): + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book) + break + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'public') + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + + readthrough = models.ReadThrough.objects.get(user=self.local_user) + self.assertEqual(readthrough.book, self.book) + self.assertEqual(readthrough.start_date.year, 2020) + self.assertEqual(readthrough.start_date.month, 10) + self.assertEqual(readthrough.start_date.day, 21) + self.assertEqual(readthrough.finish_date.year, 2020) + self.assertEqual(readthrough.finish_date.month, 10) + self.assertEqual(readthrough.finish_date.day, 25) + + + def test_handle_imported_book_already_shelved(self): + ''' goodreads import added a book, this adds related connections ''' + shelf = self.local_user.shelf_set.filter(identifier='to-read').first() + models.ShelfBook.objects.create( + shelf=shelf, added_by=self.local_user, book=self.book) + + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + for index, entry in enumerate(list(csv.DictReader(csv_file))): + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book) + break + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'public') + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + self.assertIsNone( + self.local_user.shelf_set.get(identifier='read').books.first()) + + def test_handle_status(self): ''' create a status ''' form = forms.CommentForm({ From 22f5fa154caf2a03b90c144a1fbee328ed350d27 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 13:26:42 -0800 Subject: [PATCH 3/3] Add readthroughs even when a book is already shelved --- bookwyrm/outgoing.py | 9 +++--- bookwyrm/tests/data/goodreads.csv | 2 +- bookwyrm/tests/test_outgoing.py | 47 +++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index 172563bb..88377d33 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -179,11 +179,10 @@ def handle_imported_book(user, item, include_reviews, privacy): book=item.book, shelf=desired_shelf, added_by=user) broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) - # only add new read-throughs if the item isn't already shelved - for read in item.reads: - read.book = item.book - read.user = user - read.save() + for read in item.reads: + read.book = item.book + read.user = user + read.save() if include_reviews and (item.rating or item.review): review_title = 'Review of {!r} on Goodreads'.format( diff --git a/bookwyrm/tests/data/goodreads.csv b/bookwyrm/tests/data/goodreads.csv index b96a0c26..5f124edc 100644 --- a/bookwyrm/tests/data/goodreads.csv +++ b/bookwyrm/tests/data/goodreads.csv @@ -1,4 +1,4 @@ Book Id,Title,Author,Author l-f,Additional Authors,ISBN,ISBN13,My Rating,Average Rating,Publisher,Binding,Number of Pages,Year Published,Original Publication Year,Date Read,Date Added,Bookshelves,Bookshelves with positions,Exclusive Shelf,My Review,Spoiler,Private Notes,Read Count,Recommended For,Recommended By,Owned Copies,Original Purchase Date,Original Purchase Location,Condition,Condition Description,BCID 42036538,Gideon the Ninth (The Locked Tomb #1),Tamsyn Muir,"Muir, Tamsyn",,"=""1250313198""","=""9781250313195""",0,4.20,Tor,Hardcover,448,2019,2019,2020/10/25,2020/10/21,,,read,,,,1,,,0,,,,, 52691223,Subcutanean,Aaron A. Reed,"Reed, Aaron A.",,"=""""","=""""",0,4.45,,Paperback,232,2020,,2020/03/06,2020/03/05,,,read,,,,1,,,0,,,,, -28694510,Patisserie at Home,Mélanie Dupuis,"Dupuis, Mélanie",Anne Cazor,"=""0062445316""","=""9780062445315""",2,4.60,Harper Design,Hardcover,288,2016,,,2019/07/08,,,read,"The good:
- Well illustrated and good photographs
- I loved the organization, with sections for base recipes like pie crust, full recipes, and skills
- I loved the madeleines and sweet pie crust recipe
- Very precise measurements

The bad:
- I found the index very hard to use, I would have much preferred a regular, alphabetical index of everything instead of breaking it down in to categories like a table of contents
- The primary unit of measure is ounces, which I found very hard to work with, and in fraction form which my food scale definitely does not do. One recipe calls for 1/32 oz, which I have absolutely no way of measuring out
- Some of the measurements were baffling, like 1/3 tablespoon. 1/3 tablespoon is 1 teaspoon, why would you write 1/3 tablespoon??
- The croissant dough recipe said to allow the pastry to get to room temperature before rolling which meant the butter squirted out and made a huge mess. I don't know why it said to do this??? Rolling works just fine if it's chilled.
- The financier recipe just tells you to add egg whites and has no other raising agent so if you just add the egg whites it will obviously not rise. Either there should have been a raising agent or the egg whites should have been beaten? I don't know.",,,2,,,0,,,,, +28694510,Patisserie at Home,Mélanie Dupuis,"Dupuis, Mélanie",Anne Cazor,"=""0062445316""","=""9780062445315""",2,4.60,Harper Design,Hardcover,288,2016,,,2019/07/08,,,read,"mixed feelings",,,2,,,0,,,,, diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index f0125907..5e585b1d 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -281,6 +281,7 @@ class Outgoing(TestCase): readthrough = models.ReadThrough.objects.get(user=self.local_user) self.assertEqual(readthrough.book, self.book) + # I can't remember how to create dates and I don't want to look it up. self.assertEqual(readthrough.start_date.year, 2020) self.assertEqual(readthrough.start_date.month, 10) self.assertEqual(readthrough.start_date.day, 21) @@ -311,6 +312,52 @@ class Outgoing(TestCase): self.assertEqual(shelf.books.first(), self.book) self.assertIsNone( self.local_user.shelf_set.get(identifier='read').books.first()) + readthrough = models.ReadThrough.objects.get(user=self.local_user) + self.assertEqual(readthrough.book, self.book) + self.assertEqual(readthrough.start_date.year, 2020) + self.assertEqual(readthrough.start_date.month, 10) + self.assertEqual(readthrough.start_date.day, 21) + self.assertEqual(readthrough.finish_date.year, 2020) + self.assertEqual(readthrough.finish_date.month, 10) + self.assertEqual(readthrough.finish_date.day, 25) + + + def test_handle_imported_book_review(self): + ''' goodreads review import ''' + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + entry = list(csv.DictReader(csv_file))[2] + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book) + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, True, 'unlisted') + review = models.Review.objects.get(book=self.book, user=self.local_user) + self.assertEqual(review.content, 'mixed feelings') + self.assertEqual(review.rating, 2) + self.assertEqual(review.published_date.year, 2019) + self.assertEqual(review.published_date.month, 7) + self.assertEqual(review.published_date.day, 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.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + entry = list(csv.DictReader(csv_file))[2] + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book) + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'unlisted') + self.assertFalse(models.Review.objects.filter( + book=self.book, user=self.local_user + ).exists()) def test_handle_status(self):