Change how goodread import writes reviews

- adds published date
 - broadcasts review imports
 - completes review and shelve actions as it goes
 - some small connector fixes

 fixes #247

bookwyrm/connectors/abstract_connector.py

@@ -2,14 +2,16 @@
 from abc import ABC, abstractmethod
 from dateutil import parser
 import pytz
+from urllib3.exceptions import ProtocolError
 import requests
+from requests import HTTPError
 
 from django.db import transaction
 
 from bookwyrm import models
 
 
-class ConnectorException(Exception):
+class ConnectorException(HTTPError):
     ''' when the connector can't do what was asked '''
 
 
@@ -155,9 +157,11 @@ class AbstractConnector(ABC):
         ''' for creating a new book or syncing with data '''
         book = update_from_mappings(book, data, self.book_mappings)
 
+        author_text = []
         for author in self.get_authors_from_data(data):
             book.authors.add(author)
-        book.author_text = ', '.join(a.display_name for a in book.authors.all())
+            author_text += author.display_name
+        book.author_text = ', '.join(author_text)
         book.save()
 
         if not update_cover:
@@ -287,12 +291,15 @@ def get_date(date_string):
 
 def get_data(url):
     ''' wrapper for request.get '''
-    resp = requests.get(
+    try:
-        url,
+        resp = requests.get(
-        headers={
+            url,
-            'Accept': 'application/json; charset=utf-8',
+            headers={
-        },
+                'Accept': 'application/json; charset=utf-8',
-    )
+            },
+        )
+    except ProtocolError:
+        raise ConnectorException()
     if not resp.ok:
         resp.raise_for_status()
     data = resp.json()

bookwyrm/goodreads_import.py

@@ -42,13 +42,10 @@ def import_data(job_id):
             if item.book:
                 item.save()
                 results.append(item)
+                # shelves book and handles reviews
+                outgoing.handle_imported_book(job.user, item)
             else:
-                item.fail_reason = "Could not match book on OpenLibrary"
+                item.fail_reason = "Could not find a match for book"
                 item.save()
-
-        status = outgoing.handle_import_books(job.user, results)
-        if status:
-            job.import_status = status
-            job.save()
     finally:
         create_notification(job.user, 'IMPORT', related_import=job)

bookwyrm/models/import_job.py

@@ -40,8 +40,7 @@ class ImportJob(models.Model):
     user = models.ForeignKey(User, on_delete=models.CASCADE)
     created_date = models.DateTimeField(default=timezone.now)
     task_id = models.CharField(max_length=100, null=True)
-    import_status = models.ForeignKey(
+
-        'Status', null=True, on_delete=models.PROTECT)
 
 class ImportItem(models.Model):
     ''' a single line of a csv being imported '''
@@ -71,6 +70,8 @@ class ImportItem(models.Model):
                 return books_manager.get_or_create_book(search_result.key)
             except ConnectorException:
                 pass
+        return None
+
 
     def get_book_from_title_author(self):
         ''' search by title and author '''
@@ -84,6 +85,8 @@ class ImportItem(models.Model):
                 return books_manager.get_or_create_book(search_result.key)
             except ConnectorException:
                 pass
+        return None
+
 
     @property
     def isbn(self):
@@ -95,6 +98,7 @@ class ImportItem(models.Model):
         ''' the goodreads shelf field '''
         if self.data['Exclusive Shelf']:
             return GOODREADS_SHELVES.get(self.data['Exclusive Shelf'])
+        return None
 
     @property
     def review(self):
@@ -111,12 +115,14 @@ class ImportItem(models.Model):
         ''' when the book was added to this dataset '''
         if self.data['Date Added']:
             return dateutil.parser.parse(self.data['Date Added'])
+        return None
 
     @property
     def date_read(self):
         ''' the date a book was completed '''
         if self.data['Date Read']:
             return dateutil.parser.parse(self.data['Date Read'])
+        return None
 
     @property
     def reads(self):
@@ -126,6 +132,7 @@ class ImportItem(models.Model):
             return [ReadThrough(start_date=self.date_added)]
         if self.date_read:
             return [ReadThrough(
+                start_date=self.date_added,
                 finish_date=self.date_read,
             )]
         return []

bookwyrm/outgoing.py

@@ -155,51 +155,49 @@ def handle_unshelve(user, book, shelf):
     broadcast(user, activity)
 
 
-def handle_import_books(user, items):
+def handle_imported_book(user, item):
     ''' process a goodreads csv and then post about it '''
-    new_books = []
+    if isinstance(item.book, models.Work):
-    for item in items:
+        item.book = item.book.default_edition
-        if item.shelf:
+    if not item.book:
-            desired_shelf = models.Shelf.objects.get(
+        return
-                identifier=item.shelf,
-                user=user
-            )
-            if isinstance(item.book, models.Work):
-                item.book = item.book.default_edition
-            if not item.book:
-                continue
-            shelf_book, created = models.ShelfBook.objects.get_or_create(
-                book=item.book, shelf=desired_shelf, added_by=user)
-            if created:
-                new_books.append(item.book)
-                activity = shelf_book.to_add_activity(user)
-                broadcast(user, activity)
 
-                if item.rating or item.review:
+    if item.shelf:
-                    review_title = 'Review of {!r} on Goodreads'.format(
+        desired_shelf = models.Shelf.objects.get(
-                        item.book.title,
+            identifier=item.shelf,
-                    ) if item.review else ''
+            user=user
+        )
+        # shelve the book if it hasn't been shelved already
+        shelf_book, created = models.ShelfBook.objects.get_or_create(
+            book=item.book, shelf=desired_shelf, added_by=user)
+        if created:
+            broadcast(user, shelf_book.to_add_activity(user))
 
-                    models.Review.objects.create(
+            # only add new read-throughs if the item isn't already shelved
-                        user=user,
+            for read in item.reads:
-                        book=item.book,
+                read.book = item.book
-                        name=review_title,
+                read.user = user
-                        content=item.review,
+                read.save()
-                        rating=item.rating,
-                    )
-                for read in item.reads:
-                    read.book = item.book
-                    read.user = user
-                    read.save()
 
-    if new_books:
+    if item.rating or item.review:
-        message = 'imported {} books'.format(len(new_books))
+        review_title = 'Review of {!r} on Goodreads'.format(
-        status = create_generated_note(user, message, mention_books=new_books)
+            item.book.title,
-        status.save()
+        ) if item.review else ''
 
-        broadcast(user, status.to_create_activity(user))
+        # we don't know the publication date of the review,
-        return status
+        # but "now" is a bad guess
-    return None
+        published_date_guess = item.date_read or item.date_added
+        review = models.Review.objects.create(
+            user=user,
+            book=item.book,
+            name=review_title,
+            content=item.review,
+            rating=item.rating,
+            published_date=published_date_guess,
+        )
+        # we don't need to send out pure activities because non-bookwyrm
+        # instances don't need this data
+        broadcast(user, review.to_create_activity(user))
 
 
 def handle_delete_status(user, status):

bookwyrm/views.py

@@ -489,7 +489,8 @@ def book_page(request, book_id):
         ).values_list('identifier', flat=True)
 
         readthroughs = models.ReadThrough.objects.filter(
-            user=request.user
+            user=request.user,
+            book=book,
         ).order_by('start_date')