From 502f29082c04cc652e94090a7d8a0720576b585c Mon Sep 17 00:00:00 2001 From: n2no1 <7995366-n2no1@users.noreply.gitlab.com> Date: Tue, 6 Apr 2021 19:57:57 -0400 Subject: [PATCH 01/16] check the form for initial date values if the book has none --- bookwyrm/templates/book/edit_book.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/book/edit_book.html b/bookwyrm/templates/book/edit_book.html index a9ce651e7..8cf6f31c9 100644 --- a/bookwyrm/templates/book/edit_book.html +++ b/bookwyrm/templates/book/edit_book.html @@ -124,7 +124,7 @@

- +

{% for error in form.first_published_date.errors %}

{{ error | escape }}

@@ -132,7 +132,7 @@

- +

{% for error in form.published_date.errors %}

{{ error | escape }}

From 0941c50c6913d94430d03525396df1b9dca75d3d Mon Sep 17 00:00:00 2001 From: n2no1 <7995366-n2no1@users.noreply.gitlab.com> Date: Tue, 6 Apr 2021 20:43:37 -0400 Subject: [PATCH 02/16] ensure that the book edit confirmation receives initial date data as a datetime --- bookwyrm/templates/book/edit_book.html | 4 ++-- bookwyrm/views/books.py | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/book/edit_book.html b/bookwyrm/templates/book/edit_book.html index 8cf6f31c9..0cde8dff2 100644 --- a/bookwyrm/templates/book/edit_book.html +++ b/bookwyrm/templates/book/edit_book.html @@ -124,7 +124,7 @@

- +

{% for error in form.first_published_date.errors %}

{{ error | escape }}

@@ -132,7 +132,7 @@

- +

{% for error in form.published_date.errors %}

{{ error | escape }}

diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 58886cade..e982a7d3c 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -1,4 +1,5 @@ """ the good stuff! the books! """ +from datetime import datetime from uuid import uuid4 from django.contrib.auth.decorators import login_required, permission_required @@ -172,6 +173,12 @@ class EditBook(View): data["confirm_mode"] = True # this isn't preserved because it isn't part of the form obj data["remove_authors"] = request.POST.getlist("remove_authors") + # we have to make sure the dates are passed in as datetime, they're currently a string + # QueryDicts are immutable, we need to copy + formcopy = data["form"].data.copy() + formcopy["first_published_date"] = datetime.strptime(formcopy["first_published_date"], "%Y-%m-%d") + formcopy["published_date"] = datetime.strptime(formcopy["published_date"], "%Y-%m-%d") + data["form"].data = formcopy return TemplateResponse(request, "book/edit_book.html", data) remove_authors = request.POST.getlist("remove_authors") From 51e16fba97b459eb110294628653db298aa8489c Mon Sep 17 00:00:00 2001 From: n2no1 <7995366-n2no1@users.noreply.gitlab.com> Date: Tue, 6 Apr 2021 20:49:19 -0400 Subject: [PATCH 03/16] run black, add a try/catch around the formcopy fix --- bookwyrm/views/books.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index e982a7d3c..02f61ea77 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -176,8 +176,18 @@ class EditBook(View): # we have to make sure the dates are passed in as datetime, they're currently a string # QueryDicts are immutable, we need to copy formcopy = data["form"].data.copy() - formcopy["first_published_date"] = datetime.strptime(formcopy["first_published_date"], "%Y-%m-%d") - formcopy["published_date"] = datetime.strptime(formcopy["published_date"], "%Y-%m-%d") + try: + formcopy["first_published_date"] = datetime.strptime( + formcopy["first_published_date"], "%Y-%m-%d" + ) + except MultiValueDictKeyError: + pass + try: + formcopy["published_date"] = datetime.strptime( + formcopy["published_date"], "%Y-%m-%d" + ) + except MultiValueDictKeyError: + pass data["form"].data = formcopy return TemplateResponse(request, "book/edit_book.html", data) From 31146b00e47052e450eb93ca2559aaa33cfbdc5c Mon Sep 17 00:00:00 2001 From: n2no1 <7995366-n2no1@users.noreply.gitlab.com> Date: Tue, 6 Apr 2021 21:40:11 -0400 Subject: [PATCH 04/16] import MultiValueDictKeyError to catch with formcopy --- bookwyrm/views/books.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 02f61ea77..2e48942b2 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -11,6 +11,7 @@ from django.db.models import Avg, Q from django.http import HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse +from django.utils.datastructures import MultiValueDictKeyError from django.utils.decorators import method_decorator from django.views import View from django.views.decorators.http import require_POST From 4cea7be77155cc549f3ba47402a2f83b91092c37 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 07:46:15 -0700 Subject: [PATCH 05/16] Gets black back on stable tag --- .github/workflows/black.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index 5fc849d61..afa9cf2b3 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -8,6 +8,6 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 - - uses: psf/black@20.8b1 + - uses: psf/black@stable with: args: ". --check -l 80 -S" From f11d64f984c23d0c867b9f5c1e18544fb12aeafc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 08:09:47 -0700 Subject: [PATCH 06/16] Handle all connector errors in search --- bookwyrm/connectors/connector_manager.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bookwyrm/connectors/connector_manager.py b/bookwyrm/connectors/connector_manager.py index caf6bcbe2..bf3c749d4 100644 --- a/bookwyrm/connectors/connector_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -1,5 +1,6 @@ """ interface with whatever connectors the app has """ import importlib +import logging import re from urllib.parse import urlparse @@ -11,6 +12,8 @@ from requests import HTTPError from bookwyrm import models from bookwyrm.tasks import app +logger = logging.getLogger(__name__) + class ConnectorException(HTTPError): """ when the connector can't do what was asked """ @@ -44,7 +47,9 @@ def search(query, min_confidence=0.1): if result_set in (None, []): try: result_set = connector.search(query, min_confidence=min_confidence) - except (HTTPError, ConnectorException): + except Exception as e: # pylint: disable=broad-except + # we don't want *any* error to crash the whole search page + logger.exception(e) continue # if the search results look the same, ignore them From 45006afdf32707fd16f4492c5efb72030cafe4ab Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 08:50:50 -0700 Subject: [PATCH 07/16] Remove character limit on some book fields --- bookwyrm/models/book.py | 6 +++--- bookwyrm/templates/book/edit_book.html | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 3204c6035..a6824c0ad 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -53,14 +53,14 @@ class Book(BookDataModel): connector = models.ForeignKey("Connector", on_delete=models.PROTECT, null=True) # book/work metadata - title = fields.CharField(max_length=255) + title = fields.TextField(max_length=255) sort_title = fields.CharField(max_length=255, blank=True, null=True) - subtitle = fields.CharField(max_length=255, blank=True, null=True) + subtitle = fields.TextField(max_length=255, blank=True, null=True) description = fields.HtmlField(blank=True, null=True) languages = fields.ArrayField( models.CharField(max_length=255), blank=True, default=list ) - series = fields.CharField(max_length=255, blank=True, null=True) + series = fields.TextField(max_length=255, blank=True, null=True) series_number = fields.CharField(max_length=255, blank=True, null=True) subjects = fields.ArrayField( models.CharField(max_length=255), blank=True, null=True, default=list diff --git a/bookwyrm/templates/book/edit_book.html b/bookwyrm/templates/book/edit_book.html index a9ce651e7..15202d807 100644 --- a/bookwyrm/templates/book/edit_book.html +++ b/bookwyrm/templates/book/edit_book.html @@ -88,12 +88,18 @@

{% trans "Metadata" %}

-

{{ form.title }}

+

+ + +

{% for error in form.title.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.subtitle }}

+

+ + +

{% for error in form.subtitle.errors %}

{{ error | escape }}

{% endfor %} From 63d37c281d59d26f1c83a13a6d62c3977a78e847 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 08:59:33 -0700 Subject: [PATCH 08/16] Handle invalid authors when importing books --- bookwyrm/connectors/abstract_connector.py | 6 +++++- bookwyrm/connectors/openlibrary.py | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 00b5c5c9e..2483cc62b 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -179,7 +179,11 @@ class AbstractConnector(AbstractMinimalConnector): data = get_data(remote_id) mapped_data = dict_from_mappings(data, self.author_mappings) - activity = activitypub.Author(**mapped_data) + try: + activity = activitypub.Author(**mapped_data) + except activitypub.ActivitySerializerError: + return None + # this will dedupe return activity.to_model(model=models.Author) diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index 9be0266cd..8ee738eb8 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -93,7 +93,10 @@ class Connector(AbstractConnector): # this id is "/authors/OL1234567A" author_id = author_blob["key"] url = "%s%s" % (self.base_url, author_id) - yield self.get_or_create_author(url) + author = self.get_or_create_author(url) + if not author: + continue + yield author def get_cover_url(self, cover_blob, size="L"): """ ask openlibrary for the cover """ From 514afdc12d6c42053e10ba08ea106682773ca93b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 09:02:39 -0700 Subject: [PATCH 09/16] Use run instead of exec for bw-dev web commands The issue I had with this initially was the `clean` part, not the `run` part --- bw-dev | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/bw-dev b/bw-dev index b9c4b2a1f..42fb4a2e7 100755 --- a/bw-dev +++ b/bw-dev @@ -19,7 +19,6 @@ function clean { function runweb { docker-compose run --rm web "$@" - clean } function execdb { @@ -64,17 +63,16 @@ case "$CMD" in clean ;; makemigrations) - execweb python manage.py makemigrations "$@" + runweb python manage.py makemigrations "$@" ;; migrate) - execweb python manage.py rename_app fedireads bookwyrm - execweb python manage.py migrate "$@" + runweb python manage.py migrate "$@" ;; bash) - execweb bash + runweb bash ;; shell) - execweb python manage.py shell + runweb python manage.py shell ;; dbshell) execdb psql -U ${POSTGRES_USER} ${POSTGRES_DB} @@ -83,22 +81,19 @@ case "$CMD" in docker-compose restart celery_worker ;; test) - execweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@" + runweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@" ;; pytest) - execweb pytest --no-cov-on-fail "$@" - ;; - test_report) - execweb coverage report + runweb pytest --no-cov-on-fail "$@" ;; collectstatic) - execweb python manage.py collectstatic --no-input + runweb python manage.py collectstatic --no-input ;; makemessages) - execweb django-admin makemessages --no-wrap --ignore=venv3 $@ + runweb django-admin makemessages --no-wrap --ignore=venv3 $@ ;; compilemessages) - execweb django-admin compilemessages --ignore venv3 $@ + runweb django-admin compilemessages --ignore venv3 $@ ;; build) docker-compose build @@ -110,7 +105,7 @@ case "$CMD" in makeitblack ;; populate_streams) - execweb python manage.py populate_streams + runweb python manage.py populate_streams ;; *) echo "Unrecognised command. Try: build, clean, up, initdb, resetdb, makemigrations, migrate, bash, shell, dbshell, restart_celery, test, pytest, test_report, black, populate_feeds" From e3d01c6736b233ca59126ed3b6b6a1b467c85865 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 09:17:04 -0700 Subject: [PATCH 10/16] Gracefully handle errors in webfinger during search --- bookwyrm/views/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 75c5da8fe..41e6a6082 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -138,7 +138,7 @@ def handle_remote_webfinger(query): user = activitypub.resolve_remote_id( link["href"], model=models.User ) - except KeyError: + except (KeyError, activitypub.ActivitySerializerError): return None return user From ef12b077dd9e81ebd2c57fb07ed69146577a7751 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 10:32:16 -0700 Subject: [PATCH 11/16] Adds following field to actor serialization --- bookwyrm/activitypub/person.py | 1 + bookwyrm/models/activitypub_mixin.py | 19 +++++++++++++++++-- bookwyrm/models/user.py | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/bookwyrm/activitypub/person.py b/bookwyrm/activitypub/person.py index 4ab9f08e6..9231bd955 100644 --- a/bookwyrm/activitypub/person.py +++ b/bookwyrm/activitypub/person.py @@ -23,6 +23,7 @@ class Person(ActivityObject): inbox: str publicKey: PublicKey followers: str = None + following: str = None outbox: str = None endpoints: Dict = None name: str = None diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index a253207ac..1c4a274c2 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -1,5 +1,6 @@ """ activitypub model functionality """ from base64 import b64encode +from collections import namedtuple from functools import reduce import json import operator @@ -25,6 +26,15 @@ from bookwyrm.models.fields import ImageField, ManyToManyField logger = logging.getLogger(__name__) # I tried to separate these classes into mutliple files but I kept getting # circular import errors so I gave up. I'm sure it could be done though! + +PropertyField = namedtuple("PropertyField", ("set_activity_from_field")) + + +def set_activity_from_property_field(activity, obj, field): + """ assign a model property value to the activity json """ + activity[field[1]] = getattr(obj, field[0]) + + class ActivitypubMixin: """ add this mixin for models that are AP serializable """ @@ -52,6 +62,11 @@ class ActivitypubMixin: self.activity_fields = ( self.image_fields + self.many_to_many_fields + self.simple_fields ) + if hasattr(self, "property_fields"): + self.activity_fields += [ + PropertyField(lambda a, o: set_activity_from_property_field(a, o, f)) + for f in self.property_fields + ] # these are separate to avoid infinite recursion issues self.deserialize_reverse_fields = ( @@ -430,7 +445,7 @@ def generate_activity(obj): ) in obj.serialize_reverse_fields: related_field = getattr(obj, model_field_name) activity[activity_field_name] = unfurl_related_field( - related_field, sort_field + related_field, sort_field=sort_field ) if not activity.get("id"): @@ -440,7 +455,7 @@ def generate_activity(obj): def unfurl_related_field(related_field, sort_field=None): """ load reverse lookups (like public key owner or Status attachment """ - if hasattr(related_field, "all"): + if sort_field and hasattr(related_field, "all"): return [ unfurl_related_field(i) for i in related_field.order_by(sort_field).all() ] diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 33dedc9ef..dcc4162eb 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -112,6 +112,12 @@ class User(OrderedCollectionPageMixin, AbstractUser): ) name_field = "username" + property_fields = [("following_link", "following")] + + @property + def following_link(self): + """ just how to find out the following info """ + return "{:s}/following".format(self.remote_id) @property def alt_text(self): From 7c5f078682b128eef96fe52398fa27b695959f1a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 10:33:56 -0700 Subject: [PATCH 12/16] Adds missing migration for #898 --- .../migrations/0062_auto_20210407_1545.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 bookwyrm/migrations/0062_auto_20210407_1545.py diff --git a/bookwyrm/migrations/0062_auto_20210407_1545.py b/bookwyrm/migrations/0062_auto_20210407_1545.py new file mode 100644 index 000000000..3a1566371 --- /dev/null +++ b/bookwyrm/migrations/0062_auto_20210407_1545.py @@ -0,0 +1,33 @@ +# Generated by Django 3.1.6 on 2021-04-07 15:45 + +import bookwyrm.models.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0061_auto_20210402_1435"), + ] + + operations = [ + migrations.AlterField( + model_name="book", + name="series", + field=bookwyrm.models.fields.TextField( + blank=True, max_length=255, null=True + ), + ), + migrations.AlterField( + model_name="book", + name="subtitle", + field=bookwyrm.models.fields.TextField( + blank=True, max_length=255, null=True + ), + ), + migrations.AlterField( + model_name="book", + name="title", + field=bookwyrm.models.fields.TextField(max_length=255), + ), + ] From 954958b6f98a66638e567d5403e036c9339f11a1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 10:54:00 -0700 Subject: [PATCH 13/16] Handle arbitrary errors in isbn search --- bookwyrm/connectors/connector_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bookwyrm/connectors/connector_manager.py b/bookwyrm/connectors/connector_manager.py index bf3c749d4..53198c0a9 100644 --- a/bookwyrm/connectors/connector_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -40,8 +40,9 @@ def search(query, min_confidence=0.1): else: try: result_set = connector.isbn_search(isbn) - except (HTTPError, ConnectorException): - pass + except Exception as e: # pylint: disable=broad-except + logger.exception(e) + continue # if no isbn search or results, we fallback to generic search if result_set in (None, []): From 5427790c4e14a03bc8d57377560f17d0fd9c2f3b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 11:02:56 -0700 Subject: [PATCH 14/16] Safer serialization of shelve and unshelve activities --- bookwyrm/models/activitypub_mixin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index a253207ac..a4f4f353a 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -370,7 +370,7 @@ class CollectionItemMixin(ActivitypubMixin): object_field = getattr(self, self.object_field) collection_field = getattr(self, self.collection_field) return activitypub.Add( - id=self.remote_id, + id=self.get_remote_id(), actor=self.user.remote_id, object=object_field, target=collection_field.remote_id, @@ -381,7 +381,7 @@ class CollectionItemMixin(ActivitypubMixin): object_field = getattr(self, self.object_field) collection_field = getattr(self, self.collection_field) return activitypub.Remove( - id=self.remote_id, + id=self.get_remote_id(), actor=self.user.remote_id, object=object_field, target=collection_field.remote_id, From ac86c194d4550903e73414f4ad7ad9d93b21c5a8 Mon Sep 17 00:00:00 2001 From: n2no1 <7995366-n2no1@users.noreply.gitlab.com> Date: Wed, 7 Apr 2021 14:11:13 -0400 Subject: [PATCH 15/16] move from strptime to dateutil for parsing date inputs --- bookwyrm/views/books.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 2e48942b2..b1b2d0656 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -2,6 +2,7 @@ from datetime import datetime from uuid import uuid4 +from dateutil.parser import parse as dateparse from django.contrib.auth.decorators import login_required, permission_required from django.contrib.postgres.search import SearchRank, SearchVector from django.core.files.base import ContentFile @@ -178,15 +179,13 @@ class EditBook(View): # QueryDicts are immutable, we need to copy formcopy = data["form"].data.copy() try: - formcopy["first_published_date"] = datetime.strptime( - formcopy["first_published_date"], "%Y-%m-%d" + formcopy["first_published_date"] = dateparse( + formcopy["first_published_date"] ) except MultiValueDictKeyError: pass try: - formcopy["published_date"] = datetime.strptime( - formcopy["published_date"], "%Y-%m-%d" - ) + formcopy["published_date"] = dateparse(formcopy["published_date"]) except MultiValueDictKeyError: pass data["form"].data = formcopy From 89af144105c74d413bf1b87aeced7b5f34bb442a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 7 Apr 2021 18:38:26 -0700 Subject: [PATCH 16/16] Avoid showing "None" for title and subtitle fields --- bookwyrm/templates/book/edit_book.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/book/edit_book.html b/bookwyrm/templates/book/edit_book.html index 963b2b75f..1da7c3f70 100644 --- a/bookwyrm/templates/book/edit_book.html +++ b/bookwyrm/templates/book/edit_book.html @@ -90,7 +90,7 @@

{% trans "Metadata" %}

- +

{% for error in form.title.errors %}

{{ error | escape }}

@@ -98,7 +98,7 @@

- +

{% for error in form.subtitle.errors %}

{{ error | escape }}