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 f3062a447..9d278b28d 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -17,7 +17,7 @@ {% 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 01812e1d5..d09e0257f 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -64,6 +64,9 @@ class Import(View): 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 9d278b28d..06817d302 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -17,7 +17,14 @@ {% 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 d09e0257f..01812e1d5 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -64,9 +64,6 @@ class Import(View): 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 06817d302..9146d0715 100644 --- a/bookwyrm/templates/import/import.html +++ b/bookwyrm/templates/import/import.html @@ -19,10 +19,14 @@

{% 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 05ca44476..8449b4f50 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -4,7 +4,7 @@ import sys 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 3102f8da2..10d500f1c 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 89e644db8..867b2b971 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 8449b4f50..05ca44476 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -4,7 +4,7 @@ import sys 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 867b2b971..da32ebdb0 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 10d500f1c..3dfde4cb1 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 1fcc9ee75..bd2a98f35 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -136,10 +136,16 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): # 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 6cfe4c10c..c11a24630 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -371,7 +371,7 @@ class TagField(ManyToManyField): 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 83ca90b0a..83a7dd998 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 3dfde4cb1..2a1f2e72a 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 da32ebdb0..9eef6792f 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 bd2a98f35..cabf2cf8d 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -137,6 +137,8 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): 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 cabf2cf8d..1561295a7 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -137,9 +137,9 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): 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 c11a24630..168270973 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -379,7 +379,12 @@ class TagField(ManyToManyField): 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 1561295a7..a620f09b1 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -136,10 +136,9 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): # 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 83a7dd998..5e52576aa 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 168270973..62dc8f0d9 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -383,7 +383,7 @@ class TagField(ManyToManyField): # 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 a620f09b1..2f1999bf2 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -138,7 +138,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): 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 2a1f2e72a..d3475edf6 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 9eef6792f..5670f35b9 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 5e52576aa..d8103c9c3 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 d8103c9c3..3e04fa408 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 d3475edf6..9730ec6a1 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 5670f35b9..1c6c64228 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -3,7 +3,6 @@ import json 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 3e04fa408..d9942746a 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 9730ec6a1..08780b731 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 961fbd522..fc8d7387d 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -404,7 +404,7 @@ class ModelFields(TestCase): 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 cde193f08..12a164eb5 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -87,7 +87,7 @@ class Signature(TestCase): 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 d9942746a..ee8b0d26a 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 12a164eb5..ca68d3fd6 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -111,6 +111,7 @@ class Signature(TestCase): 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 @@ class Signature(TestCase): 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 @@ class Signature(TestCase): "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 ca68d3fd6..37d841b33 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -159,7 +159,7 @@ class Signature(TestCase): "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 97b012db8..829b7a6c0 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -102,11 +102,13 @@ class CreateBook(View): "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 @@ class CreateBook(View): 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 829b7a6c0..d5b3e2573 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -103,12 +103,12 @@ class CreateBook(View): } 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 @@ class CreateBook(View): 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 d5b3e2573..df67dac5e 100644 --- a/bookwyrm/views/books/edit_book.py +++ b/bookwyrm/views/books/edit_book.py @@ -143,10 +143,11 @@ class CreateBook(View): 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 647db3bfe..f5008baa3 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 000000000..5c9ae39a3 --- /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 4e7ffcad3..2bad36a50 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.preview_images import generate_edition_preview_image_task from bookwyrm.settings import ( DOMAIN, DEFAULT_LANGUAGE, + LANGUAGE_ARTICLES, ENABLE_PREVIEW_IMAGES, ENABLE_THUMBNAIL_GENERATION, ) @@ -363,6 +365,16 @@ class Edition(Book): 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 8dcf90fcb..f9940bcd5 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -312,6 +312,9 @@ LANGUAGES = [ ("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 79ec241be..7d0035ed3 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 0c3074902..dbbcc2d3a 100644 --- a/bookwyrm/views/shelf/shelf.py +++ b/bookwyrm/views/shelf/shelf.py @@ -128,7 +128,7 @@ class Shelf(View): 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 30d6f970a..660bd1a62 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 33854b3d6..50ff8c7e1 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -132,3 +132,16 @@ class Book(TestCase): 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 5c9ae39a3..7f79fb321 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 2bad36a50..3cc89a45d 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -366,14 +366,14 @@ class Edition(Book): 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 f9940bcd5..23fa3b064 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -313,7 +313,7 @@ LANGUAGES = [ ] 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 50ff8c7e1..fda74cb38 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -140,8 +140,6 @@ class Book(TestCase): 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 3cc89a45d..c25f8fee2 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -369,7 +369,10 @@ class Edition(Book): 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 fda74cb38..825f42b87 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -142,4 +142,4 @@ class Book(TestCase): 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 7f79fb321..a5055c5e1 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 623beaa04..3a3979e2c 100644 --- a/bookwyrm/forms/books.py +++ b/bookwyrm/forms/books.py @@ -20,6 +20,7 @@ class EditionForm(CustomForm): model = models.Edition fields = [ "title", + "sort_title", "subtitle", "description", "series", @@ -45,6 +46,9 @@ class EditionForm(CustomForm): ] 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 e85164444..72d80e9cf 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" %} +
+