Merge branch 'main' into production

bookwyrm/activitypub/base_activity.py

@@ -8,6 +8,10 @@ from django.db.models.fields.related_descriptors \
         import ForwardManyToOneDescriptor
 
 
+class ActivitySerializerError(ValueError):
+    ''' routine problems serializing activitypub json '''
+
+
 class ActivityEncoder(JSONEncoder):
     '''  used to convert an Activity object into json '''
     def default(self, o):
@@ -66,7 +70,8 @@ class ActivityObject:
                 value = kwargs[field.name]
             except KeyError:
                 if field.default == MISSING:
-                    raise TypeError('Missing required field: %s' % field.name)
+                    raise ActivitySerializerError(\
+                            'Missing required field: %s' % field.name)
                 value = field.default
             setattr(self, field.name, value)
 
@@ -74,7 +79,7 @@ class ActivityObject:
     def to_model(self, model, instance=None):
         ''' convert from an activity to a model instance '''
         if not isinstance(self, model.activity_serializer):
-            raise TypeError('Wrong activity type for model')
+            raise ActivitySerializerError('Wrong activity type for model')
 
         # check for an existing instance, if we're not updating a known obj
         if not instance:
@@ -136,6 +141,7 @@ def resolve_foreign_key(model, remote_id):
     ).first()
 
     if not result:
-        raise ValueError('Could not resolve remote_id in %s model: %s' % \
+        raise ActivitySerializerError(
+            'Could not resolve remote_id in %s model: %s' % \
                 (model.__name__, remote_id))
     return result

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/incoming.py

@@ -238,7 +238,12 @@ def handle_create(activity):
 @app.task
 def handle_delete_status(activity):
     ''' remove a status '''
-    status_id = activity['object']['id']
+    try:
+        status_id = activity['object']['id']
+    except TypeError:
+        # this isn't a great fix, because you hit this when mastadon
+        # is trying to delete a user.
+        return
     try:
         status = models.Status.objects.select_subclasses().get(
             remote_id=status_id
@@ -282,7 +287,11 @@ def handle_unfavorite(activity):
 @app.task
 def handle_boost(activity):
     ''' someone gave us a boost! '''
-    boost = activitypub.Boost(**activity).to_model(models.Boost)
+    try:
+        boost = activitypub.Boost(**activity).to_model(models.Boost)
+    except activitypub.ActivitySerializerError:
+        # this probably just means we tried to boost an unknown status
+        return
 
     if not boost.user.local:
         status_builder.create_notification(

bookwyrm/remote_user.py

@@ -37,10 +37,14 @@ def get_or_create_remote_user(actor):
 
 def fetch_user_data(actor):
     ''' load the user's info from the actor url '''
-    response = requests.get(
-        actor,
-        headers={'Accept': 'application/activity+json'}
-    )
+    try:
+        response = requests.get(
+            actor,
+            headers={'Accept': 'application/activity+json'}
+        )
+    except ConnectionError:
+        return None
+
     if not response.ok:
         response.raise_for_status()
     data = response.json()
@@ -83,7 +87,10 @@ def get_avatar(data):
 @app.task
 def get_remote_reviews(user_id):
     ''' ingest reviews by a new remote bookwyrm user '''
-    user = models.User.objects.get(id=user_id)
+    try:
+        user = models.User.objects.get(id=user_id)
+    except models.User.DoesNotExist:
+        return
     outbox_page = user.outbox + '?page=true'
     response = requests.get(
         outbox_page,

bookwyrm/static/js/shared.js

@@ -20,6 +20,7 @@ function reply(e) {
     return true;
 }
 
+
 function rate_stars(e) {
     e.preventDefault();
     ajaxPost(e.target);

bookwyrm/templates/book.html

@@ -55,7 +55,7 @@
             <div class="block">
                 <h3 class="field is-grouped">{% include 'snippets/stars.html' with rating=rating %} ({{ reviews|length }} review{{ reviews|length|pluralize }})</h3>
 
-                {% include 'snippets/book_description.html' %}
+                {% include 'snippets/trimmed_text.html' with full=book|book_description %}
 
                 {% if book.parent_work.edition_set.count > 1 %}
                 <p><a href="/book/{{ book.parent_work.id }}/editions">{{ book.parent_work.edition_set.count }} editions</a></p>
@@ -77,12 +77,12 @@
                         {% endif %}
                     </dl>
                     <div class="field is-grouped">
-                        <label class="button is-small" for="edit-readthrough-{{ readthrough.id }}">
+                        <label class="button is-small" for="edit-readthrough-{{ readthrough.id }}" role="button" tabindex="0">
                             <span class="icon icon-pencil">
                                 <span class="is-sr-only">Edit read-through dates</span>
                             </span>
                         </label>
-                        <label class="button is-small" for="delete-readthrough-{{ readthrough.id }}">
+                        <label class="button is-small" for="delete-readthrough-{{ readthrough.id }}" role="button" tabindex="0">
                             <span class="icon icon-x">
                                 <span class="is-sr-only">Delete this read-through</span>
                             </span>

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/templates/snippets/book_description.html

@@ -1,26 +0,0 @@
-{% load fr_display %}
-{% with book.id|uuid as uuid %}
-{% with book|book_description as full %}
-{% if full %}
-    {% with full|text_overflow as trimmed %}
-    {% if trimmed != full %}
-    <div>
-        <input type="radio" name="show-hide-{{ book.id }}-{{ uuid }}" id="show-{{ book.id }}-{{ uuid }}" class="toggle-control" checked>
-        <blockquote class="content toggle-content hidden">{{ trimmed }}
-            <label class="button is-small" for="hide-{{ book.id }}-{{ uuid }}"><div role="button" tabindex="0">show more</div></label>
-        </blockquote>
-    </div>
-    <div>
-        <input type="radio" name="show-hide-{{ book.id }}-{{ uuid }}" id="hide-{{ book.id }}-{{ uuid }}" class="toggle-control">
-        <blockquote class="content toggle-content hidden">{{ full }}
-            <label class="button is-small" for="show-{{ book.id }}-{{ uuid }}"><div role="button" tabindex="0">show less</div></label>
-        </blockquote>
-    </div>
-    {% else %}
-    <blockquote class="content">{{ full }}
-    </blockquote>
-    {% endif %}
-    {% endwith %}
-{% endif %}
-{% endwith %}
-{% endwith %}

bookwyrm/templates/snippets/book_preview.html

@@ -1,3 +1,4 @@
+{% load fr_display %}
 <div class="columns">
     <div class="column is-narrow">
         <div>
@@ -7,6 +8,6 @@
     </div>
     <div class="column">
         <h3 class="title is-6">{% include 'snippets/book_titleby.html' with book=book %}</h3>
-        {% include 'snippets/book_description.html' with book=book %}
+        {% include 'snippets/trimmed_text.html' with full=book|book_description %}
     </div>
 </div>

bookwyrm/templates/snippets/status_body.html

@@ -18,24 +18,13 @@
     </div>
 
     <footer>
-        {% if request.user.is_authenticated %}
-        <input class="toggle-control" type="checkbox" name="show-comment-{{ status.id }}" id="show-comment-{{ status.id }}">
-        <div class="toggle-content hidden">
-            <div class="card-footer">
-                <div class="card-footer-item">
-                    {% include 'snippets/reply_form.html' with status=status %}
-                </div>
-            </div>
-        </div>
-        {% endif %}
         <div class="card-footer">
             <div class="card-footer-item">
                 {% if request.user.is_authenticated %}
-
                 <label class="button is-small" for="show-comment-{{ status.id }}">
                     <div role="button" tabindex="0">
                         <span class="icon icon-comment">
-                            <span class="is-sr-only">Comment</span>
+                            <span class="is-sr-only">Reply</span>
                         </span>
                     </div>
                 </label>
@@ -44,17 +33,17 @@
 
                 {% else %}
                 <a href="/login">
-                <span class="icon icon-comment">
-                    <span class="is-sr-only">Comment</span>
-                </span>
+                    <span class="icon icon-comment">
+                        <span class="is-sr-only">Reply</span>
+                    </span>
 
-                <span class="icon icon-boost">
-                    <span class="is-sr-only">Boost status</span>
-                </span>
+                    <span class="icon icon-boost">
+                        <span class="is-sr-only">Boost status</span>
+                    </span>
 
-                <span class="icon icon-heart">
-                    <span class="is-sr-only">Like status</span>
-                </span>
+                    <span class="icon icon-heart">
+                        <span class="is-sr-only">Like status</span>
+                    </span>
                 </a>
                 {% endif %}
             </div>
@@ -77,6 +66,17 @@
             {% endif %}
         </div>
 
+        {% if request.user.is_authenticated %}
+        <input class="toggle-control" type="checkbox" name="show-comment-{{ status.id }}" id="show-comment-{{ status.id }}">
+        <div class="toggle-content hidden">
+            <div class="card-footer">
+                <div class="card-footer-item">
+                    {% include 'snippets/reply_form.html' with status=status %}
+                </div>
+            </div>
+        </div>
+        {% endif %}
+
         {% if status.user == request.user %}
         <div>
             <input class="toggle-control" type="checkbox" name="more-info-{{ status.id }}" id="more-info-{{ status.id }}">

bookwyrm/templates/snippets/status_content.html

@@ -16,7 +16,7 @@
     {% endif %}
 
     {% if status.content and status.status_type != 'GeneratedNote' and status.status_type != 'Boost' %}
-        <blockquote>{{ status.content | safe }}</blockquote>
+    {% include 'snippets/trimmed_text.html' with full=status.content|safe %}
     {% endif %}
 </div>
 

bookwyrm/templates/snippets/trimmed_text.html

@@ -0,0 +1,26 @@
+{% load fr_display %}
+{% with 0|uuid as uuid %}
+{% if full %}
+
+{% with full|text_overflow as trimmed %}
+{% if trimmed != full %}
+<div>
+    <input type="radio" name="show-hide-{{ uuid }}" id="show-{{ uuid }}" class="toggle-control" checked>
+    <blockquote class="content toggle-content hidden">{{ trimmed }}
+        <label class="button is-small" for="hide-{{ uuid }}"><div role="button" tabindex="0">show more</div></label>
+    </blockquote>
+</div>
+<div>
+    <input type="radio" name="show-hide-{{ uuid }}" id="hide-{{ uuid }}" class="toggle-control">
+    <blockquote class="content toggle-content hidden">{{ full }}
+        <label class="button is-small" for="show-{{ uuid }}"><div role="button" tabindex="0">show less</div></label>
+    </blockquote>
+</div>
+{% else %}
+<blockquote class="content">{{ full }}</blockquote>
+{% endif %}
+{% endwith %}
+
+{% endif %}
+{% endwith %}
+

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
     })
 
@@ -511,7 +515,11 @@ def book_page(request, book_id):
     except ValueError:
         page = 1
 
-    book = models.Book.objects.select_subclasses().get(id=book_id)
+    try:
+        book = models.Book.objects.select_subclasses().get(id=book_id)
+    except models.Book.DoesNotExist:
+        return HttpResponseNotFound()
+
     if is_api_request(request):
         return JsonResponse(book.to_activity(), encoder=ActivityEncoder)