From fc599f8b9a320cd245893dea8e68a35e1bbdcba1 Mon Sep 17 00:00:00 2001 From: Ritik Ranjan Date: Sat, 28 Jan 2023 12:10:06 +0530 Subject: [PATCH 001/104] fixed singularisation/pluralisation edited two files --- bookwyrm/templates/import/import.html | 2 +- bookwyrm/views/imports/import_data.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/bookwyrm/templates/import/import.html b/bookwyrm/templates/import/import.html index f3062a4475..9d278b28d1 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -17,7 +17,7 @@

{% trans "Import Books" %}

{% if site.imports_enabled %} {% if import_size_limit and import_limit_reset %}
-

{% blocktrans %}Currently you are allowed to import {{ import_size_limit }} books every {{ import_limit_reset }} days.{% endblocktrans %}

+

{% blocktrans %}Currently you are allowed to import {{ import_size_limit }} {{book_noun}} every {{ import_limit_reset }} {{day_noun}}.{% endblocktrans %}

{% blocktrans %}You have {{ allowed_imports }} left.{% endblocktrans %}

{% endif %} diff --git a/bookwyrm/views/imports/import_data.py b/bookwyrm/views/imports/import_data.py index 01812e1d57..d09e0257ff 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -64,6 +64,9 @@ def get(self, request, invalid=False): data["import_limit_reset"] = site_settings.import_limit_reset data["allowed_imports"] = site_settings.import_size_limit - imported_books + data["book_noun"] = "books" if site_settings.import_size_limit > 1 else 'book' + data["day_noun"] = "days" if site_settings.import_limit_reset > 1 else 'day' + return TemplateResponse(request, "import/import.html", data) def post(self, request): From e5e9e807cabf4e259bd426a08d34624c2d9f36ed Mon Sep 17 00:00:00 2001 From: Ritik Ranjan Date: Sat, 28 Jan 2023 21:11:20 +0530 Subject: [PATCH 002/104] Splited stirng into two then fixed pluralization error. --- bookwyrm/templates/import/import.html | 9 ++++++++- bookwyrm/views/imports/import_data.py | 3 --- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bookwyrm/templates/import/import.html b/bookwyrm/templates/import/import.html index 9d278b28d1..06817d3024 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -17,7 +17,14 @@

{% trans "Import Books" %}

{% if site.imports_enabled %} {% if import_size_limit and import_limit_reset %}
-

{% blocktrans %}Currently you are allowed to import {{ import_size_limit }} {{book_noun}} every {{ import_limit_reset }} {{day_noun}}.{% endblocktrans %}

+

+ {% blocktrans count books=import_size_limit%} + Currently you are allowed to import {{ import_size_limit }} {{books}} + {% endblocktrans %} + {% blocktrans count days=import_limit_reset%} + every {{ import_limit_reset }} {{days}}. + {% endblocktrans %} +

{% blocktrans %}You have {{ allowed_imports }} left.{% endblocktrans %}

{% endif %} diff --git a/bookwyrm/views/imports/import_data.py b/bookwyrm/views/imports/import_data.py index d09e0257ff..01812e1d57 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -64,9 +64,6 @@ def get(self, request, invalid=False): data["import_limit_reset"] = site_settings.import_limit_reset data["allowed_imports"] = site_settings.import_size_limit - imported_books - data["book_noun"] = "books" if site_settings.import_size_limit > 1 else 'book' - data["day_noun"] = "days" if site_settings.import_limit_reset > 1 else 'day' - return TemplateResponse(request, "import/import.html", data) def post(self, request): From adcf9310a0850cb646af81fdbbfc53c31f3ac41d Mon Sep 17 00:00:00 2001 From: Ritik Ranjan Date: Sat, 28 Jan 2023 21:23:08 +0530 Subject: [PATCH 003/104] Fixed Syntax of pluralisation --- bookwyrm/templates/import/import.html | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/import/import.html b/bookwyrm/templates/import/import.html index 06817d3024..9146d07152 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -19,10 +19,14 @@

{% trans "Import Books" %}

{% blocktrans count books=import_size_limit%} - Currently you are allowed to import {{ import_size_limit }} {{books}} + Currently you are allowed to import {{ import_size_limit }} book + {% plural %} + Currently you are allowed to import {{ import_size_limit }} books {% endblocktrans %} {% blocktrans count days=import_limit_reset%} - every {{ import_limit_reset }} {{days}}. + every {{ import_limit_reset }} day. + {% plural %} + every {{ import_limit_reset }} days. {% endblocktrans %}

{% blocktrans %}You have {{ allowed_imports }} left.{% endblocktrans %}

From 632e3844b9c2b0dd1f67567b9c4411bf6b022a2e Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 10 Apr 2023 17:32:49 +1000 Subject: [PATCH 004/104] Don't assume user id is key id minus fragment Fixes #2801 Related to #2794 It is legitimate to use any url for the user's key id. We have been assuming this id is the user id plus a fragment (#key-id) but this is not always the case, notably in the case of GoToSocial it is at /key-id. This commit instead checks the remote user's information to see if the key id listed matches the key id of the message allegedly received from them. Whilst troubleshooting this it also became apparent that there is a mismatch between Bookwyrm users' keyId and the KeyId we claim to be using in signed requests (there is a forward slash missing). Since everything after the slash is a fragment, this usually slips through but we should be consistent so I updated that. --- bookwyrm/activitypub/__init__.py | 2 +- bookwyrm/signatures.py | 3 ++- bookwyrm/views/inbox.py | 10 ++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 05ca444766..8449b4f503 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -4,7 +4,7 @@ from .base_activity import ActivityEncoder, Signature, naive_parse from .base_activity import Link, Mention, Hashtag -from .base_activity import ActivitySerializerError, resolve_remote_id +from .base_activity import ActivitySerializerError, resolve_remote_id, get_activitypub_data from .image import Document, Image from .note import Note, GeneratedNote, Article, Comment, Quotation from .note import Review, Rating diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 3102f8da21..10d500f1c3 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -38,8 +38,9 @@ def make_signature(method, sender, destination, date, digest=None): message_to_sign = "\n".join(signature_headers) signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8"))) + # TODO: this should be /#main-key to match our user key ids, even though that was probably a mistake in hindsight. signature = { - "keyId": f"{sender.remote_id}#main-key", + "keyId": f"{sender.remote_id}/#main-key", "algorithm": "rsa-sha256", "headers": headers, "signature": b64encode(signed_message).decode("utf8"), diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 89e644db8a..867b2b971a 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -130,15 +130,13 @@ def has_valid_signature(request, activity): """verify incoming signature""" try: signature = Signature.parse(request) - - key_actor = urldefrag(signature.key_id).url - if key_actor != activity.get("actor"): - raise ValueError("Wrong actor created signature.") - - remote_user = activitypub.resolve_remote_id(key_actor, model=models.User) + remote_user = activitypub.resolve_remote_id(activity.get("actor"), model=models.User) if not remote_user: return False + if signature.key_id != remote_user.key_pair.remote_id: + raise ValueError("Wrong actor created signature.") + try: signature.verify(remote_user.key_pair.public_key, request) except ValueError: From 49758f2383a675ebedac29fd21e8d43b2d13751d Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 10 Apr 2023 17:50:25 +1000 Subject: [PATCH 005/104] formatting fixes --- bookwyrm/activitypub/__init__.py | 2 +- bookwyrm/views/inbox.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 8449b4f503..05ca444766 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -4,7 +4,7 @@ from .base_activity import ActivityEncoder, Signature, naive_parse from .base_activity import Link, Mention, Hashtag -from .base_activity import ActivitySerializerError, resolve_remote_id, get_activitypub_data +from .base_activity import ActivitySerializerError, resolve_remote_id from .image import Document, Image from .note import Note, GeneratedNote, Article, Comment, Quotation from .note import Review, Rating diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 867b2b971a..da32ebdb0c 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -130,7 +130,9 @@ def has_valid_signature(request, activity): """verify incoming signature""" try: signature = Signature.parse(request) - remote_user = activitypub.resolve_remote_id(activity.get("actor"), model=models.User) + remote_user = activitypub.resolve_remote_id( + activity.get("actor"), model=models.User + ) if not remote_user: return False From e112718d2d08369581dc14f0f12fef1525d0be74 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 10 Apr 2023 20:34:45 +1000 Subject: [PATCH 006/104] clean up troubleshooting comment --- bookwyrm/signatures.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 10d500f1c3..3dfde4cb1f 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -38,7 +38,6 @@ def make_signature(method, sender, destination, date, digest=None): message_to_sign = "\n".join(signature_headers) signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8"))) - # TODO: this should be /#main-key to match our user key ids, even though that was probably a mistake in hindsight. signature = { "keyId": f"{sender.remote_id}/#main-key", "algorithm": "rsa-sha256", From ef85394a1633fb599df2ec31b29707c26042ce30 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 10 Apr 2023 20:35:13 +1000 Subject: [PATCH 007/104] Allow for tag value to be object Previously the 'tag' value in an activitypub object was assumed to be a List (array). Some AP software sends 'tag' as a Dict (object) if there is only a single tag value. It's somewhat debatable whether this is spec compliant but we should aim to be robust. This commit puts an individual mention tag inside a list if necessary. --- bookwyrm/models/status.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 1fcc9ee757..bd2a98f354 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -136,10 +136,16 @@ def ignore_activity(cls, activity): # pylint: disable=too-many-return-statement # keep notes if they mention local users if activity.tag == MISSING or activity.tag is None: return True - tags = [l["href"] for l in activity.tag if l["type"] == "Mention"] + + tags = activity.tag if type(activity.tag) == list else [activity.tag] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: - if user_model.objects.filter(remote_id=tag, local=True).exists(): + if ( + tag["type"] == "Mention" + and user_model.objects.filter( + remote_id=tag["href"], local=True + ).exists() + ): # we found a mention of a known use boost return False return True From c9dcd4f7add416601464167f0d8a599ea44cff99 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 10 Apr 2023 20:38:20 +1000 Subject: [PATCH 008/104] Include initial '@' in mention tag name GoToSocial expects the 'name' value of a mention tag to have an initial '@' symbol. Mastodon doesn't seem to mind either way. --- bookwyrm/models/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 6cfe4c10c2..c11a24630f 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -371,7 +371,7 @@ def field_to_activity(self, value): tags.append( activitypub.Link( href=item.remote_id, - name=getattr(item, item.name_field), + name=f"@{getattr(item, item.name_field)}", type=activity_type, ) ) From 03f21b0f35609c07fedfc71e6b32979dbbda927f Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Tue, 11 Apr 2023 15:45:06 +1000 Subject: [PATCH 009/104] Use correct keyId with legacy fallback Bookwyrm keyIds are at `userpath/#main-key`, however when signing AP objects we have claimed in the headers that the keyId is at `userpath#main-key`. This is incorrect, and makes GoToSocial's strict checking break. Simply updating the signatures to use the correct KeyId breaks legacy Bookwyrm's signature checks, becuase it assumes that the keyId path is the same as the user path plus a fragment. This commit allows for either option, by sending the request a second time with the incorrect keyId if sending with the correct one causes an error. --- bookwyrm/models/activitypub_mixin.py | 13 ++++++++++++- bookwyrm/signatures.py | 6 ++++-- bookwyrm/views/inbox.py | 3 ++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 83ca90b0a5..83a7dd9982 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -540,10 +540,11 @@ async def sign_and_send( digest = make_digest(data) + headers = { "Date": now, "Digest": digest, - "Signature": make_signature("post", sender, destination, now, digest), + "Signature": make_signature("post", sender, destination, now, digest, False), "Content-Type": "application/activity+json; charset=utf-8", "User-Agent": USER_AGENT, } @@ -554,6 +555,16 @@ async def sign_and_send( logger.exception( "Failed to send broadcast to %s: %s", destination, response.reason ) + logger.info("Trying again with legacy keyId") + # try with incorrect keyId to enable communication with legacy Bookwyrm servers + legacy_signature = make_signature("post", sender, destination, now, digest, True) + headers["Signature"] = legacy_signature + async with session.post(destination, data=data, headers=headers) as response: + if not response.ok: + logger.exception( + "Failed to send broadcast with legacy keyId to %s: %s", destination, response.reason + ) + return response except asyncio.TimeoutError: logger.info("Connection timed out for url: %s", destination) diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 3dfde4cb1f..2a1f2e72a7 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -22,7 +22,7 @@ def create_key_pair(): return private_key, public_key -def make_signature(method, sender, destination, date, digest=None): +def make_signature(method, sender, destination, date, digest=None, use_legacy_key=False): """uses a private key to sign an outgoing message""" inbox_parts = urlparse(destination) signature_headers = [ @@ -38,8 +38,10 @@ def make_signature(method, sender, destination, date, digest=None): message_to_sign = "\n".join(signature_headers) signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8"))) + # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions + key_id = f"{sender.remote_id}#main-key" if use_legacy_key else f"{sender.remote_id}/#main-key" signature = { - "keyId": f"{sender.remote_id}/#main-key", + "keyId": key_id, "algorithm": "rsa-sha256", "headers": headers, "signature": b64encode(signed_message).decode("utf8"), diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index da32ebdb0c..9eef6792f4 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -137,7 +137,8 @@ def has_valid_signature(request, activity): return False if signature.key_id != remote_user.key_pair.remote_id: - raise ValueError("Wrong actor created signature.") + if signature.key_id != f"{remote_user.remote_id}#main-key": # legacy Bookwyrm + raise ValueError("Wrong actor created signature.") try: signature.verify(remote_user.key_pair.public_key, request) From 279fa3851b6d043410804b789954f5e42a049348 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Tue, 11 Apr 2023 16:49:11 +1000 Subject: [PATCH 010/104] add comment --- bookwyrm/models/status.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index bd2a98f354..cabf2cf8d8 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -137,6 +137,8 @@ def ignore_activity(cls, activity): # pylint: disable=too-many-return-statement if activity.tag == MISSING or activity.tag is None: return True + # BUG: this fixes the TypeError but we still don't get any notifs + # and DMs are dropped tags = activity.tag if type(activity.tag) == list else [activity.tag] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: From c450947eeee26727dd48a5dd63c42d6ed45bc889 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Tue, 11 Apr 2023 18:57:55 +1000 Subject: [PATCH 011/104] update comment to identify bug --- bookwyrm/models/status.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index cabf2cf8d8..1561295a78 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -137,9 +137,9 @@ def ignore_activity(cls, activity): # pylint: disable=too-many-return-statement if activity.tag == MISSING or activity.tag is None: return True - # BUG: this fixes the TypeError but we still don't get any notifs - # and DMs are dropped - tags = activity.tag if type(activity.tag) == list else [activity.tag] + # BUG: this fixes the TypeError but if there is only one user mentioned + # we still don't get any notifs and DMs are dropped. + tags = activity.tag if type(activity.tag) == list else [ activity.tag ] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: if ( From e3261c6b88c8683bd50f4be94541bae1a134c1eb Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Thu, 13 Apr 2023 13:21:05 +1000 Subject: [PATCH 012/104] fix incoming GTS mentions and DMs GoToSocial sends 'tag' values as a single object if there is only one user mentioned, rather than an array with an object inside it. This causes Bookwyrm to reject the tag since it comes through as a dict rather than a list. This commit fixes this at the point the incoming AP object is transformed so that "mention" tags are turned into a mention_user. --- bookwyrm/models/fields.py | 7 ++++++- bookwyrm/models/status.py | 7 +++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index c11a24630f..1682709739 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -379,7 +379,12 @@ def field_to_activity(self, value): def field_from_activity(self, value, allow_external_connections=True): if not isinstance(value, list): - return None + # GoToSocial DMs and single-user mentions are + # sent as objects, not as an array of objects + if isinstance(value, dict): + value = [value] + else: + return None items = [] for link_json in value: link = activitypub.Link(**link_json) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 1561295a78..a620f09b16 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -136,10 +136,9 @@ def ignore_activity(cls, activity): # pylint: disable=too-many-return-statement # keep notes if they mention local users if activity.tag == MISSING or activity.tag is None: return True - - # BUG: this fixes the TypeError but if there is only one user mentioned - # we still don't get any notifs and DMs are dropped. - tags = activity.tag if type(activity.tag) == list else [ activity.tag ] + # GoToSocial sends single tags as objects + # not wrapped in a list + tags = activity.tag if isinstance(activity.tag, list) else [ activity.tag ] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: if ( From a6676718cbd64cc2d259e81cf7cff339f533743a Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Thu, 13 Apr 2023 13:27:51 +1000 Subject: [PATCH 013/104] formatting --- bookwyrm/models/activitypub_mixin.py | 17 +++++++++++------ bookwyrm/models/fields.py | 2 +- bookwyrm/models/status.py | 2 +- bookwyrm/signatures.py | 10 ++++++++-- bookwyrm/views/inbox.py | 4 +++- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 83a7dd9982..5e52576aa6 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -540,7 +540,6 @@ async def sign_and_send( digest = make_digest(data) - headers = { "Date": now, "Digest": digest, @@ -557,14 +556,20 @@ async def sign_and_send( ) logger.info("Trying again with legacy keyId") # try with incorrect keyId to enable communication with legacy Bookwyrm servers - legacy_signature = make_signature("post", sender, destination, now, digest, True) + legacy_signature = make_signature( + "post", sender, destination, now, digest, True + ) headers["Signature"] = legacy_signature - async with session.post(destination, data=data, headers=headers) as response: + async with session.post( + destination, data=data, headers=headers + ) as response: if not response.ok: logger.exception( - "Failed to send broadcast with legacy keyId to %s: %s", destination, response.reason - ) - + "Failed to send broadcast with legacy keyId to %s: %s", + destination, + response.reason, + ) + return response except asyncio.TimeoutError: logger.info("Connection timed out for url: %s", destination) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 1682709739..62dc8f0d99 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -383,7 +383,7 @@ def field_from_activity(self, value, allow_external_connections=True): # sent as objects, not as an array of objects if isinstance(value, dict): value = [value] - else: + else: return None items = [] for link_json in value: diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index a620f09b16..2f1999bf25 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -138,7 +138,7 @@ def ignore_activity(cls, activity): # pylint: disable=too-many-return-statement return True # GoToSocial sends single tags as objects # not wrapped in a list - tags = activity.tag if isinstance(activity.tag, list) else [ activity.tag ] + tags = activity.tag if isinstance(activity.tag, list) else [activity.tag] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: if ( diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 2a1f2e72a7..d3475edf61 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -22,7 +22,9 @@ def create_key_pair(): return private_key, public_key -def make_signature(method, sender, destination, date, digest=None, use_legacy_key=False): +def make_signature( + method, sender, destination, date, digest=None, use_legacy_key=False +): """uses a private key to sign an outgoing message""" inbox_parts = urlparse(destination) signature_headers = [ @@ -39,7 +41,11 @@ def make_signature(method, sender, destination, date, digest=None, use_legacy_ke signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8"))) # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions - key_id = f"{sender.remote_id}#main-key" if use_legacy_key else f"{sender.remote_id}/#main-key" + key_id = ( + f"{sender.remote_id}#main-key" + if use_legacy_key + else f"{sender.remote_id}/#main-key" + ) signature = { "keyId": key_id, "algorithm": "rsa-sha256", diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 9eef6792f4..5670f35b99 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -137,7 +137,9 @@ def has_valid_signature(request, activity): return False if signature.key_id != remote_user.key_pair.remote_id: - if signature.key_id != f"{remote_user.remote_id}#main-key": # legacy Bookwyrm + if ( + signature.key_id != f"{remote_user.remote_id}#main-key" + ): # legacy Bookwyrm raise ValueError("Wrong actor created signature.") try: From c7adb62831f1fac673645bba67369955ffd00a39 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Thu, 13 Apr 2023 19:48:20 +1000 Subject: [PATCH 014/104] make get_legacy_key more DRY --- bookwyrm/models/activitypub_mixin.py | 29 +++++++++++++--------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 5e52576aa6..d8103c9c3c 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -529,7 +529,7 @@ async def async_broadcast(recipients: List[str], sender, data: str): async def sign_and_send( - session: aiohttp.ClientSession, sender, data: str, destination: str + session: aiohttp.ClientSession, sender, data: str, destination: str, **kwargs ): """Sign the messages and send them in an asynchronous bundle""" now = http_date() @@ -539,11 +539,14 @@ async def sign_and_send( raise ValueError("No private key found for sender") digest = make_digest(data) + signature = make_signature( + "post", sender, destination, now, digest, kwargs.get("use_legacy_key") + ) headers = { "Date": now, "Digest": digest, - "Signature": make_signature("post", sender, destination, now, digest, False), + "Signature": signature, "Content-Type": "application/activity+json; charset=utf-8", "User-Agent": USER_AGENT, } @@ -554,21 +557,15 @@ async def sign_and_send( logger.exception( "Failed to send broadcast to %s: %s", destination, response.reason ) - logger.info("Trying again with legacy keyId") - # try with incorrect keyId to enable communication with legacy Bookwyrm servers - legacy_signature = make_signature( - "post", sender, destination, now, digest, True - ) - headers["Signature"] = legacy_signature - async with session.post( - destination, data=data, headers=headers - ) as response: - if not response.ok: - logger.exception( - "Failed to send broadcast with legacy keyId to %s: %s", - destination, - response.reason, + if kwargs.get("use_legacy_key") is not True: + # try with incorrect keyId to enable communication with legacy Bookwyrm servers + logger.info("Trying again with legacy keyId header value") + + asyncio.ensure_future( + sign_and_send( + session, sender, data, destination, use_legacy_key=True ) + ) return response except asyncio.TimeoutError: From 56a062d01f58be9088de22774f09a17ab08f32b3 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Thu, 13 Apr 2023 20:21:35 +1000 Subject: [PATCH 015/104] pylint fixes --- bookwyrm/models/activitypub_mixin.py | 2 +- bookwyrm/signatures.py | 5 +++-- bookwyrm/views/inbox.py | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index d8103c9c3c..3e04fa408d 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -540,7 +540,7 @@ async def sign_and_send( digest = make_digest(data) signature = make_signature( - "post", sender, destination, now, digest, kwargs.get("use_legacy_key") + "post", sender, destination, now, digest=digest, use_legacy_key=kwargs.get("use_legacy_key") ) headers = { diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index d3475edf61..9730ec6a1e 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -23,7 +23,7 @@ def create_key_pair(): def make_signature( - method, sender, destination, date, digest=None, use_legacy_key=False + method, sender, destination, date, **kwargs ): """uses a private key to sign an outgoing message""" inbox_parts = urlparse(destination) @@ -33,6 +33,7 @@ def make_signature( f"date: {date}", ] headers = "(request-target) host date" + digest = kwargs.get("digest") if digest is not None: signature_headers.append(f"digest: {digest}") headers = "(request-target) host date digest" @@ -43,7 +44,7 @@ def make_signature( # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions key_id = ( f"{sender.remote_id}#main-key" - if use_legacy_key + if kwargs.get("use_legacy_key") else f"{sender.remote_id}/#main-key" ) signature = { diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 5670f35b99..1c6c642284 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -3,7 +3,6 @@ import re import logging -from urllib.parse import urldefrag import requests from django.http import HttpResponse, Http404 From 123628c66a2f332c48b26e261200ad81e3041d45 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Thu, 13 Apr 2023 22:33:54 +1000 Subject: [PATCH 016/104] fix tests and formatting --- bookwyrm/models/activitypub_mixin.py | 7 ++++++- bookwyrm/signatures.py | 4 +--- bookwyrm/tests/models/test_fields.py | 2 +- bookwyrm/tests/test_signing.py | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 3e04fa408d..d9942746a6 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -540,7 +540,12 @@ async def sign_and_send( digest = make_digest(data) signature = make_signature( - "post", sender, destination, now, digest=digest, use_legacy_key=kwargs.get("use_legacy_key") + "post", + sender, + destination, + now, + digest=digest, + use_legacy_key=kwargs.get("use_legacy_key"), ) headers = { diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 9730ec6a1e..08780b7317 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -22,9 +22,7 @@ def create_key_pair(): return private_key, public_key -def make_signature( - method, sender, destination, date, **kwargs -): +def make_signature(method, sender, destination, date, **kwargs): """uses a private key to sign an outgoing message""" inbox_parts = urlparse(destination) signature_headers = [ diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 961fbd5224..fc8d7387da 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -404,7 +404,7 @@ def test_tag_field(self, *_): self.assertIsInstance(result, list) self.assertEqual(len(result), 1) self.assertEqual(result[0].href, "https://e.b/c") - self.assertEqual(result[0].name, "Name") + self.assertEqual(result[0].name, "@Name") self.assertEqual(result[0].type, "Serializable") def test_tag_field_from_activity(self, *_): diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index cde193f080..12a164eb5b 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -87,7 +87,7 @@ def send_test_request( # pylint: disable=too-many-arguments data = json.dumps(get_follow_activity(sender, self.rat)) digest = digest or make_digest(data) signature = make_signature( - "post", signer or sender, self.rat.inbox, now, digest + "post", signer or sender, self.rat.inbox, now, digest=digest ) with patch("bookwyrm.views.inbox.activity_task.apply_async"): with patch("bookwyrm.models.user.set_remote_server.delay"): From 8a8af4e90927540077908b4474a10b427f9d7672 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Fri, 14 Apr 2023 18:03:51 +1000 Subject: [PATCH 017/104] fix tests and make pylint happier --- bookwyrm/models/activitypub_mixin.py | 2 -- bookwyrm/tests/test_signing.py | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index d9942746a6..ee8b0d26a6 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -563,9 +563,7 @@ async def sign_and_send( "Failed to send broadcast to %s: %s", destination, response.reason ) if kwargs.get("use_legacy_key") is not True: - # try with incorrect keyId to enable communication with legacy Bookwyrm servers logger.info("Trying again with legacy keyId header value") - asyncio.ensure_future( sign_and_send( session, sender, data, destination, use_legacy_key=True diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 12a164eb5b..ca68d3fd61 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -111,6 +111,7 @@ def test_remote_signer(self): datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json") data = json.loads(datafile.read_bytes()) data["id"] = self.fake_remote.remote_id + data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key" data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key del data["icon"] # Avoid having to return an avatar. responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200) @@ -138,6 +139,7 @@ def test_key_needs_refresh(self): datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json") data = json.loads(datafile.read_bytes()) data["id"] = self.fake_remote.remote_id + data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key" data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key del data["icon"] # Avoid having to return an avatar. responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200) @@ -157,7 +159,7 @@ def test_key_needs_refresh(self): "bookwyrm.models.relationship.UserFollowRequest.accept" ) as accept_mock: response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 200) #BUG this is 401 self.assertTrue(accept_mock.called) # Old key is cached, so still works: From 98726585f6dc8a36447519c630d093f81ef0adea Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Fri, 14 Apr 2023 18:20:06 +1000 Subject: [PATCH 018/104] oops black --- bookwyrm/tests/test_signing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index ca68d3fd61..37d841b33c 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -159,7 +159,7 @@ def test_key_needs_refresh(self): "bookwyrm.models.relationship.UserFollowRequest.accept" ) as accept_mock: response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) #BUG this is 401 + self.assertEqual(response.status_code, 200) # BUG this is 401 self.assertTrue(accept_mock.called) # Old key is cached, so still works: From fb3cb229e95e2e2f53b023303a4ac982753db464 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 16 Apr 2023 17:58:03 +1000 Subject: [PATCH 019/104] Fixes #2571 Persist book.subjects and add_author when form validation fails. This does not resolve the problem of cover image uploads being dropped because this is a broader problem and is covered separately in #2760 (we should investigate the plugin ` django-file-resubmit`) --- bookwyrm/views/books/edit_book.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bookwyrm/views/books/edit_book.py b/bookwyrm/views/books/edit_book.py index 97b012db81..829b7a6c0c 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -102,11 +102,13 @@ def post(self, request): "authors": authors, } - ensure_transient_values_persist(request, data) - if not form.is_valid(): + ensure_transient_values_persist(request, data, form) return TemplateResponse(request, "book/edit/edit_book.html", data) + # we have to call this twice because it requires form.cleaned_data + # which only exists after we validate the form + ensure_transient_values_persist(request, data, form) data = add_authors(request, data) # check if this is an edition of an existing work @@ -139,8 +141,11 @@ def post(self, request): return redirect(f"/book/{book.id}") -def ensure_transient_values_persist(request, data): +def ensure_transient_values_persist(request, data, form): """ensure that values of transient form fields persist when re-rendering the form""" + data["book"] = data.get("book") or {} + data["book"]["subjects"] = form.cleaned_data["subjects"] + data["add_author"] = request.POST.getlist("add_author") data["cover_url"] = request.POST.get("cover-url") From 6f025af99fc1bbdb9dd8829da552d801672a0c36 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 16 Apr 2023 19:06:04 +1000 Subject: [PATCH 020/104] fix ensure_transient_values_persist call --- bookwyrm/views/books/edit_book.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bookwyrm/views/books/edit_book.py b/bookwyrm/views/books/edit_book.py index 829b7a6c0c..d5b3e2573e 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -103,12 +103,12 @@ def post(self, request): } if not form.is_valid(): - ensure_transient_values_persist(request, data, form) + ensure_transient_values_persist(request, data, form=form) return TemplateResponse(request, "book/edit/edit_book.html", data) # we have to call this twice because it requires form.cleaned_data # which only exists after we validate the form - ensure_transient_values_persist(request, data, form) + ensure_transient_values_persist(request, data, form=form) data = add_authors(request, data) # check if this is an edition of an existing work @@ -141,10 +141,10 @@ def post(self, request): return redirect(f"/book/{book.id}") -def ensure_transient_values_persist(request, data, form): +def ensure_transient_values_persist(request, data, **kwargs): """ensure that values of transient form fields persist when re-rendering the form""" data["book"] = data.get("book") or {} - data["book"]["subjects"] = form.cleaned_data["subjects"] + data["book"]["subjects"] = kwargs["form"].cleaned_data["subjects"] data["add_author"] = request.POST.getlist("add_author") data["cover_url"] = request.POST.get("cover-url") From 5f5886edea05c045fed4a026529889f675c0d181 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 16 Apr 2023 19:58:53 +1000 Subject: [PATCH 021/104] Actually fix ensure_transient_values_persist call oops --- bookwyrm/views/books/edit_book.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bookwyrm/views/books/edit_book.py b/bookwyrm/views/books/edit_book.py index d5b3e2573e..df67dac5ee 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -143,10 +143,11 @@ def post(self, request): def ensure_transient_values_persist(request, data, **kwargs): """ensure that values of transient form fields persist when re-rendering the form""" - data["book"] = data.get("book") or {} - data["book"]["subjects"] = kwargs["form"].cleaned_data["subjects"] - data["add_author"] = request.POST.getlist("add_author") data["cover_url"] = request.POST.get("cover-url") + if kwargs and kwargs.get("form"): + data["book"] = data.get("book") or {} + data["book"]["subjects"] = kwargs["form"].cleaned_data["subjects"] + data["add_author"] = request.POST.getlist("add_author") def add_authors(request, data): From a94a4732ec726cc09bb9b810e4f5d9313fe84dba Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Mon, 24 Apr 2023 23:29:55 -0500 Subject: [PATCH 022/104] add support for title sort to ignore initial article --- bookwyrm/forms/lists.py | 2 +- .../migrations/0179_populate_sort_title.py | 41 +++++++++++++++++++ bookwyrm/models/book.py | 12 ++++++ bookwyrm/settings.py | 3 ++ bookwyrm/templates/shelf/shelf.html | 2 +- 5 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 bookwyrm/migrations/0179_populate_sort_title.py diff --git a/bookwyrm/forms/lists.py b/bookwyrm/forms/lists.py index 647db3bfe9..f5008baa3c 100644 --- a/bookwyrm/forms/lists.py +++ b/bookwyrm/forms/lists.py @@ -24,7 +24,7 @@ class SortListForm(forms.Form): sort_by = ChoiceField( choices=( ("order", _("List Order")), - ("title", _("Book Title")), + ("sort_title", _("Book Title")), ("rating", _("Rating")), ), label=_("Sort By"), diff --git a/bookwyrm/migrations/0179_populate_sort_title.py b/bookwyrm/migrations/0179_populate_sort_title.py new file mode 100644 index 0000000000..5c9ae39a36 --- /dev/null +++ b/bookwyrm/migrations/0179_populate_sort_title.py @@ -0,0 +1,41 @@ +import re +from itertools import chain + +from django.db import migrations, transaction +from django.db.models import Q + +from bookwyrm.settings import LANGUAGE_ARTICLES + + +@transaction.atomic +def populate_sort_title(apps, schema_editor): + Edition = apps.get_model("bookwyrm", "Edition") + db_alias = schema_editor.connection.alias + editions_wo_sort_title = Edition.objects.using(db_alias).filter( + Q(sort_title__isnull=True) | Q(sort_title__exact="") + ) + for edition in editions_wo_sort_title: + articles = chain( + *(LANGUAGE_ARTICLES.get(language, ()) for language in edition.languages) + ) + if articles: + icase_articles = ( + f"[{a[0].capitalize()}{a[0].lower()}]{a[1:]}" for a in articles + ) + edition.sort_title = re.sub( + f'^{" |^".join(icase_articles)} ', "", edition.title + ) + else: + edition.sort_title = edition.title + edition.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0178_auto_20230328_2132"), + ] + + operations = [ + migrations.RunPython(populate_sort_title), + ] diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 4e7ffcad30..2bad36a50f 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -1,4 +1,5 @@ """ database schema for books and shelves """ +from itertools import chain import re from django.contrib.postgres.search import SearchVectorField @@ -17,6 +18,7 @@ from bookwyrm.settings import ( DOMAIN, DEFAULT_LANGUAGE, + LANGUAGE_ARTICLES, ENABLE_PREVIEW_IMAGES, ENABLE_THUMBNAIL_GENERATION, ) @@ -363,6 +365,16 @@ def save(self, *args, **kwargs): for author_id in self.authors.values_list("id", flat=True): cache.delete(f"author-books-{author_id}") + # Create sort title by removing articles from title + if self.sort_title is None: + articles = chain( + *(LANGUAGE_ARTICLES[language] for language in self.languages) + ) + icase_articles = ( + f"[{a[0].capitalize()}{a[0].lower()}]{a[1:]}" for a in articles + ) + self.sort_title = re.sub(f'^{" |^".join(icase_articles)} ', "", self.title) + return super().save(*args, **kwargs) @classmethod diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 8dcf90fcb2..f9940bcd54 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -312,6 +312,9 @@ ("zh-hant", _("繁體中文 (Traditional Chinese)")), ] +LANGUAGE_ARTICLES = { + "English": {"The", "A", "An"}, +} TIME_ZONE = "UTC" diff --git a/bookwyrm/templates/shelf/shelf.html b/bookwyrm/templates/shelf/shelf.html index 79ec241bee..7d0035ed3b 100644 --- a/bookwyrm/templates/shelf/shelf.html +++ b/bookwyrm/templates/shelf/shelf.html @@ -145,7 +145,7 @@

{% trans "Cover"%} - {% trans "Title" as text %}{% include 'snippets/table-sort-header.html' with field="title" sort=sort text=text %} + {% trans "Title" as text %}{% include 'snippets/table-sort-header.html' with field="sort_title" sort=sort text=text %} {% trans "Author" as text %}{% include 'snippets/table-sort-header.html' with field="author" sort=sort text=text %} {% if request.user.is_authenticated %} {% if is_self %} From 21d9cb5fe56c5f37429610abbbdb894940b4ec87 Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 00:15:58 -0500 Subject: [PATCH 023/104] updating shelf view --- bookwyrm/views/shelf/shelf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/views/shelf/shelf.py b/bookwyrm/views/shelf/shelf.py index 0c3074902b..dbbcc2d3aa 100644 --- a/bookwyrm/views/shelf/shelf.py +++ b/bookwyrm/views/shelf/shelf.py @@ -128,7 +128,7 @@ def post(self, request, username, shelf_identifier): def sort_books(books, sort): """Books in shelf sorting""" sort_fields = [ - "title", + "sort_title", "author", "shelved_date", "start_date", From a3013c6224e91dbc6d836a6b4f660fdbefe6663f Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 00:20:54 -0500 Subject: [PATCH 024/104] updating list view --- bookwyrm/views/list/list.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py index 30d6f970a8..660bd1a62c 100644 --- a/bookwyrm/views/list/list.py +++ b/bookwyrm/views/list/list.py @@ -129,7 +129,7 @@ def sort_list(request, items): """helper to handle the surprisingly involved sorting""" # sort_by shall be "order" unless a valid alternative is given sort_by = request.GET.get("sort_by", "order") - if sort_by not in ("order", "title", "rating"): + if sort_by not in ("order", "sort_title", "rating"): sort_by = "order" # direction shall be "ascending" unless a valid alternative is given @@ -139,7 +139,7 @@ def sort_list(request, items): directional_sort_by = { "order": "order", - "title": "book__title", + "sort_title": "book__sort_title", "rating": "average_rating", }[sort_by] if direction == "descending": From 6b39052fcca4aebba6f9d9e4d049e4afd1b0d66d Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 07:17:23 -0500 Subject: [PATCH 025/104] Adding test for sort_title population --- bookwyrm/tests/models/test_book_model.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bookwyrm/tests/models/test_book_model.py b/bookwyrm/tests/models/test_book_model.py index 33854b3d6d..50ff8c7e12 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -132,3 +132,16 @@ def test_thumbnail_fields(self): self.assertIsNotNone(book.cover_bw_book_xlarge_jpg.url) self.assertIsNotNone(book.cover_bw_book_xxlarge_webp.url) self.assertIsNotNone(book.cover_bw_book_xxlarge_jpg.url) + + def test_populate_sort_title(self): + """The sort title should remove the initial article on save""" + books = ( + models.Edition.objects.create( + title=f"{article} Test Edition", languages=[langauge] + ) + for langauge, articles in settings.LANGUAGE_ARTICLES.items() + for article in article + ) + self.assertEqual( + all([book.sort_title == "Test Edition" for book in books]) + ) From 575e1bac4cd6837639ed849cb19040a53502e6d9 Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 19:46:38 -0500 Subject: [PATCH 026/104] responding to review comments --- bookwyrm/migrations/0179_populate_sort_title.py | 14 ++++---------- bookwyrm/models/book.py | 16 ++++++++-------- bookwyrm/settings.py | 2 +- bookwyrm/tests/models/test_book_model.py | 6 ++---- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/bookwyrm/migrations/0179_populate_sort_title.py b/bookwyrm/migrations/0179_populate_sort_title.py index 5c9ae39a36..7f79fb321f 100644 --- a/bookwyrm/migrations/0179_populate_sort_title.py +++ b/bookwyrm/migrations/0179_populate_sort_title.py @@ -18,16 +18,10 @@ def populate_sort_title(apps, schema_editor): articles = chain( *(LANGUAGE_ARTICLES.get(language, ()) for language in edition.languages) ) - if articles: - icase_articles = ( - f"[{a[0].capitalize()}{a[0].lower()}]{a[1:]}" for a in articles - ) - edition.sort_title = re.sub( - f'^{" |^".join(icase_articles)} ', "", edition.title - ) - else: - edition.sort_title = edition.title - edition.save() + edition.sort_title = re.sub( + f'^{" |^".join(articles)} ', "", str(edition.title).lower() + ) + Edition.objects.bulk_update(editions_wo_sort_title, ["sort_title"]) class Migration(migrations.Migration): diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 2bad36a50f..3cc89a45d5 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -366,14 +366,14 @@ def save(self, *args, **kwargs): cache.delete(f"author-books-{author_id}") # Create sort title by removing articles from title - if self.sort_title is None: - articles = chain( - *(LANGUAGE_ARTICLES[language] for language in self.languages) - ) - icase_articles = ( - f"[{a[0].capitalize()}{a[0].lower()}]{a[1:]}" for a in articles - ) - self.sort_title = re.sub(f'^{" |^".join(icase_articles)} ', "", self.title) + if self.sort_title in [None, ""]: + if self.sort_title in [None, ""]: + articles = chain( + *(LANGUAGE_ARTICLES.get(language, ()) for language in self.languages) + ) + self.sort_title = re.sub( + f'^{" |^".join(articles)} ', "", str(self.title).lower() + ) return super().save(*args, **kwargs) diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index f9940bcd54..23fa3b0647 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -313,7 +313,7 @@ ] LANGUAGE_ARTICLES = { - "English": {"The", "A", "An"}, + "English": {"the", "a", "an"}, } TIME_ZONE = "UTC" diff --git a/bookwyrm/tests/models/test_book_model.py b/bookwyrm/tests/models/test_book_model.py index 50ff8c7e12..fda74cb38d 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -140,8 +140,6 @@ def test_populate_sort_title(self): title=f"{article} Test Edition", languages=[langauge] ) for langauge, articles in settings.LANGUAGE_ARTICLES.items() - for article in article - ) - self.assertEqual( - all([book.sort_title == "Test Edition" for book in books]) + for article in articles ) + self.assertTrue(all(book.sort_title == "Test Edition" for book in books)) From f43d7f8c70bc7f5b60a66ff0854e1e13e48ac352 Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 21:00:16 -0500 Subject: [PATCH 027/104] fixing test and other checks --- bookwyrm/models/book.py | 5 ++++- bookwyrm/tests/models/test_book_model.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 3cc89a45d5..c25f8fee2b 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -369,7 +369,10 @@ def save(self, *args, **kwargs): if self.sort_title in [None, ""]: if self.sort_title in [None, ""]: articles = chain( - *(LANGUAGE_ARTICLES.get(language, ()) for language in self.languages) + *( + LANGUAGE_ARTICLES.get(language, ()) + for language in tuple(self.languages) + ) ) self.sort_title = re.sub( f'^{" |^".join(articles)} ', "", str(self.title).lower() diff --git a/bookwyrm/tests/models/test_book_model.py b/bookwyrm/tests/models/test_book_model.py index fda74cb38d..825f42b87c 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -142,4 +142,4 @@ def test_populate_sort_title(self): for langauge, articles in settings.LANGUAGE_ARTICLES.items() for article in articles ) - self.assertTrue(all(book.sort_title == "Test Edition" for book in books)) + self.assertTrue(all(book.sort_title == "test edition" for book in books)) From 858a93e98acdecf80264c6b04174bea5710a3bd6 Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Tue, 25 Apr 2023 21:05:11 -0500 Subject: [PATCH 028/104] fixing migration --- bookwyrm/migrations/0179_populate_sort_title.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bookwyrm/migrations/0179_populate_sort_title.py b/bookwyrm/migrations/0179_populate_sort_title.py index 7f79fb321f..a5055c5e17 100644 --- a/bookwyrm/migrations/0179_populate_sort_title.py +++ b/bookwyrm/migrations/0179_populate_sort_title.py @@ -16,7 +16,10 @@ def populate_sort_title(apps, schema_editor): ) for edition in editions_wo_sort_title: articles = chain( - *(LANGUAGE_ARTICLES.get(language, ()) for language in edition.languages) + *( + LANGUAGE_ARTICLES.get(language, ()) + for language in tuple(edition.languages) + ) ) edition.sort_title = re.sub( f'^{" |^".join(articles)} ', "", str(edition.title).lower() From a6e5939ad2e37d64993392149080024eb0ae4add Mon Sep 17 00:00:00 2001 From: Zach Flanders Date: Wed, 26 Apr 2023 23:05:03 -0500 Subject: [PATCH 029/104] adding sort title to edit book form --- bookwyrm/forms/books.py | 4 ++++ bookwyrm/templates/book/edit/edit_book_form.html | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/bookwyrm/forms/books.py b/bookwyrm/forms/books.py index 623beaa042..3a3979e2ca 100644 --- a/bookwyrm/forms/books.py +++ b/bookwyrm/forms/books.py @@ -20,6 +20,7 @@ class Meta: model = models.Edition fields = [ "title", + "sort_title", "subtitle", "description", "series", @@ -45,6 +46,9 @@ class Meta: ] widgets = { "title": forms.TextInput(attrs={"aria-describedby": "desc_title"}), + "sort_title": forms.TextInput( + attrs={"aria-describedby": "desc_sort_title"} + ), "subtitle": forms.TextInput(attrs={"aria-describedby": "desc_subtitle"}), "description": forms.Textarea( attrs={"aria-describedby": "desc_description"} diff --git a/bookwyrm/templates/book/edit/edit_book_form.html b/bookwyrm/templates/book/edit/edit_book_form.html index e85164444f..72d80e9cf7 100644 --- a/bookwyrm/templates/book/edit/edit_book_form.html +++ b/bookwyrm/templates/book/edit/edit_book_form.html @@ -28,6 +28,15 @@

{% include 'snippets/form_errors.html' with errors_list=form.title.errors id="desc_title" %}

+
+ + + + {% include 'snippets/form_errors.html' with errors_list=form.sort_title.errors id="desc_sort_title" %} +
+