Merge pull request #347 from mouse-reeve/import-bugs

Import bugs

bookwyrm/connectors/bookwyrm_connector.py

@@ -79,10 +79,17 @@ class Connector(AbstractConnector):
         cover_data = data.get('attachment')
         if not cover_data:
             return None
-        cover_url = cover_data[0].get('url')
-        response = requests.get(cover_url)
+        try:
+            cover_url = cover_data[0].get('url')
+        except IndexError:
+            return None
+        try:
+            response = requests.get(cover_url)
+        except ConnectionError:
+            return None
+
         if not response.ok:
-            response.raise_for_status()
+            return None
 
         image_name = str(uuid4()) + '.' + cover_url.split('.')[-1]
         image_content = ContentFile(response.content)

bookwyrm/connectors/openlibrary.py

@@ -177,10 +177,9 @@ class Connector(AbstractConnector):
         ''' load that author '''
         if not re.match(r'^OL\d+A$', olkey):
             raise ValueError('Invalid OpenLibrary author ID')
-        try:
-            return models.Author.objects.get(openlibrary_key=olkey)
-        except models.Author.DoesNotExist:
-            pass
+        author = models.Author.objects.filter(openlibrary_key=olkey).first()
+        if author:
+            return author
 
         url = '%s/authors/%s.json' % (self.base_url, olkey)
         data = get_data(url)

bookwyrm/goodreads_import.py

@@ -20,7 +20,7 @@ def create_job(user, csv_file, include_reviews, privacy):
     )
     for index, entry in enumerate(list(csv.DictReader(csv_file))[:MAX_ENTRIES]):
         if not all(x in entry for x in ('ISBN13', 'Title', 'Author')):
-            raise ValueError("Author, title, and isbn must be in data.")
+            raise ValueError('Author, title, and isbn must be in data.')
         ImportItem(job=job, index=index, data=entry).save()
     return job
 
@@ -41,8 +41,11 @@ def import_data(job_id):
         for item in job.items.all():
             try:
                 item.resolve()
-            except HTTPError:
-                pass
+            except:
+                item.fail_reason = 'Error loading book'
+                item.save()
+                continue
+
             if item.book:
                 item.save()
                 results.append(item)
@@ -51,7 +54,7 @@ def import_data(job_id):
                 outgoing.handle_imported_book(
                     job.user, item, job.include_reviews, job.privacy)
             else:
-                item.fail_reason = "Could not find a match for book"
+                item.fail_reason = 'Could not find a match for book'
                 item.save()
     finally:
         create_notification(job.user, 'IMPORT', related_import=job)

bookwyrm/templates/import.html

@@ -14,7 +14,8 @@
             </label>
         </div>
         <div class="field">
-            <label class="label">Privacy setting for imported reviews
+            <label class="label">
+                <p>Privacy setting for imported reviews:</p>
                 {% include 'snippets/privacy_select.html' with no_label=True %}
             </label>
         </div>

bookwyrm/templates/import_status.html

@@ -6,29 +6,44 @@
     <h1 class="title">Import Status</h1>
 
     <p>
-    Import started: {{ job.created_date | naturaltime }}
+        Import started: {{ job.created_date | naturaltime }}
+    </p>
+    {% if task.successful %}
     <p>
-    {% if task.ready %}
-    Import completed: {{ task.date_done | naturaltime }}
-    {% if task.failed %}
-    <h3><span style="background-color: #ffaaaa;">TASK FAILED</span></h3>
-    <p>
-    {{ task.info }}
+        Import completed: {{ task.date_done | naturaltime }}
+    </p>
+    {% elif task.failed %}
+    <div class="notification is-danger">TASK FAILED</div>
     {% endif %}
 </div>
 
 <div class="block">
-    {% if job.import_status %}
-        {% include 'snippets/status.html' with status=job.import_status %}
-    {% endif %}
-    {% else %}
+    {% if not task.ready %}
     Import still in progress.
     <p>
-    (Hit reload to update!)
+        (Hit reload to update!)
+    </p>
     {% endif %}
 </div>
 
+{% if failed_items %}
 <div class="block">
+    <h2 class="title is-4">Failed to load</h2>
+    <ul>
+    {% for item in failed_items %}
+        <li>
+            Line {{ item.index }}:
+            <strong>{{ item.data|dict_key:'Title' }}</strong> by
+            {{ item.data|dict_key:'Author' }}
+            ({{ item.fail_reason }})
+        </li>
+    {% endfor %}
+    </ul>
+</div>
+{% endif %}
+
+<div class="block">
+    <h2 class="title is-4">Successfully imported</h2>
     <table class="table">
         <tr>
             <th>
@@ -59,9 +74,10 @@
                 {{ item.data|dict_key:'Author' }}
             </td>
             <td>
-                {% if item.book %}✓
-                {% elif item.fail_reason %}
-                    {{ item.fail_reason }}
+                {% if item.book %}
+                <span class="icon icon-check">
+                    <span class="is-sr-only">Imported</span>
+                </span>
                 {% endif %}
             </td>
         </tr>

bookwyrm/views.py

@@ -209,10 +209,14 @@ def import_status(request, job_id):
     if job.user != request.user:
         raise PermissionDenied
     task = app.AsyncResult(job.task_id)
+    items = job.items.order_by('index').all()
+    failed_items = [i for i in items if i.fail_reason]
+    items = [i for i in items if not i.fail_reason]
     return TemplateResponse(request, 'import_status.html', {
         'title': 'Import Status',
         'job': job,
-        'items': job.items.order_by('index').all(),
+        'items': items,
+        'failed_items': failed_items,
         'task': task
     })