From 7ae0db7f4ae3edffdd5d54e25d3fb5fe9339e590 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Thu, 5 May 2022 13:29:07 -0700 Subject: [PATCH 001/261] Ignore VariableDoesNotExist errors in debug logging They're so noisy as to make debug logging useless otherwise --- bookwyrm/settings.py | 4 ++++ bookwyrm/utils/log.py | 12 ++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 bookwyrm/utils/log.py diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index e16c576e1..236413fe8 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -147,6 +147,9 @@ LOGGING = { "require_debug_true": { "()": "django.utils.log.RequireDebugTrue", }, + "ignore_missing_variable": { + "()": "bookwyrm.utils.log.IgnoreVariableDoesNotExist", + }, }, "handlers": { # Overrides the default handler to make it log to console @@ -154,6 +157,7 @@ LOGGING = { # console if DEBUG=False) "console": { "level": LOG_LEVEL, + "filters": ["ignore_missing_variable"], "class": "logging.StreamHandler", }, # This is copied as-is from the default logger, and is diff --git a/bookwyrm/utils/log.py b/bookwyrm/utils/log.py new file mode 100644 index 000000000..8ad86895c --- /dev/null +++ b/bookwyrm/utils/log.py @@ -0,0 +1,12 @@ +import logging + + +class IgnoreVariableDoesNotExist(logging.Filter): + def filter(self, record): + if(record.exc_info): + (errType, errValue, _) = record.exc_info + while errValue: + if type(errValue).__name__ == 'VariableDoesNotExist': + return False + errValue = errValue.__context__ + return True From 7014786fe03a73c7c8fe525aaad19384aa05c159 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Sun, 5 Jun 2022 13:41:00 -0700 Subject: [PATCH 002/261] Run formatters --- bookwyrm/utils/log.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/utils/log.py b/bookwyrm/utils/log.py index 8ad86895c..4ea24d81d 100644 --- a/bookwyrm/utils/log.py +++ b/bookwyrm/utils/log.py @@ -3,10 +3,10 @@ import logging class IgnoreVariableDoesNotExist(logging.Filter): def filter(self, record): - if(record.exc_info): + if record.exc_info: (errType, errValue, _) = record.exc_info while errValue: - if type(errValue).__name__ == 'VariableDoesNotExist': + if type(errValue).__name__ == "VariableDoesNotExist": return False errValue = errValue.__context__ return True From 8f79b362f8c43d37d8c6e82f32f882831416d11b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 5 Aug 2022 09:51:55 -0700 Subject: [PATCH 003/261] Check permissions automatically on form save --- bookwyrm/forms/custom_form.py | 5 +++++ bookwyrm/views/goal.py | 4 +--- bookwyrm/views/group.py | 11 ++++++----- bookwyrm/views/list/list.py | 3 +-- bookwyrm/views/list/list_item.py | 3 +-- bookwyrm/views/list/lists.py | 3 +-- bookwyrm/views/reading.py | 2 +- bookwyrm/views/shelf/shelf.py | 3 +-- bookwyrm/views/shelf/shelf_actions.py | 4 +--- bookwyrm/views/status.py | 4 +--- 10 files changed, 19 insertions(+), 23 deletions(-) diff --git a/bookwyrm/forms/custom_form.py b/bookwyrm/forms/custom_form.py index 74a3417a2..3c2b4685f 100644 --- a/bookwyrm/forms/custom_form.py +++ b/bookwyrm/forms/custom_form.py @@ -24,3 +24,8 @@ class CustomForm(ModelForm): input_type = "textarea" visible.field.widget.attrs["rows"] = 5 visible.field.widget.attrs["class"] = css_classes[input_type] + + def save(self, request, *args, **kwargs): + """Save and check perms""" + self.instance.raise_not_editable(request.user) + return super().save(*args, **kwargs) diff --git a/bookwyrm/views/goal.py b/bookwyrm/views/goal.py index 57ff4bd75..b5fd5bdc2 100644 --- a/bookwyrm/views/goal.py +++ b/bookwyrm/views/goal.py @@ -48,8 +48,6 @@ class Goal(View): year = int(year) user = get_user_from_username(request.user, username) goal = models.AnnualGoal.objects.filter(year=year, user=user).first() - if goal: - goal.raise_not_editable(request.user) form = forms.GoalForm(request.POST, instance=goal) if not form.is_valid(): @@ -59,7 +57,7 @@ class Goal(View): "year": year, } return TemplateResponse(request, "user/goal.html", data) - goal = form.save() + goal = form.save(request) if request.POST.get("post-status"): # create status, if appropriate diff --git a/bookwyrm/views/group.py b/bookwyrm/views/group.py index b2271e78d..1ccfd6849 100644 --- a/bookwyrm/views/group.py +++ b/bookwyrm/views/group.py @@ -52,7 +52,7 @@ class Group(View): form = forms.GroupForm(request.POST, instance=user_group) if not form.is_valid(): return redirect("group", user_group.id) - user_group = form.save() + user_group = form.save(request) # let the other members know something about the group changed memberships = models.GroupMember.objects.filter(group=user_group) @@ -113,10 +113,8 @@ class UserGroups(View): if not form.is_valid(): return redirect(request.user.local_path + "/groups") - group = form.save(commit=False) - group.raise_not_editable(request.user) with transaction.atomic(): - group.save() + group = form.save(request) # add the creator as a group member models.GroupMember.objects.create(group=group, user=request.user) return redirect("group", group.id) @@ -129,10 +127,13 @@ class FindUsers(View): # this is mostly borrowed from the Get Started friend finder def get(self, request, group_id): - """basic profile info""" + """Search for a user to add the a group, or load suggested users cache""" user_query = request.GET.get("user_query") group = get_object_or_404(models.Group, id=group_id) + + # only users who can edit can add users group.raise_not_editable(request.user) + lists = ( models.List.privacy_filter(request.user) .filter(group=group) diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py index d0b5e08f4..35e18d244 100644 --- a/bookwyrm/views/list/list.py +++ b/bookwyrm/views/list/list.py @@ -81,13 +81,12 @@ class List(View): def post(self, request, list_id): """edit a list""" book_list = get_object_or_404(models.List, id=list_id) - book_list.raise_not_editable(request.user) form = forms.ListForm(request.POST, instance=book_list) if not form.is_valid(): # this shouldn't happen raise Exception(form.errors) - book_list = form.save() + book_list = form.save(request) if not book_list.curation == "group": book_list.group = None book_list.save(broadcast=False) diff --git a/bookwyrm/views/list/list_item.py b/bookwyrm/views/list/list_item.py index 6dca908fb..691df4da3 100644 --- a/bookwyrm/views/list/list_item.py +++ b/bookwyrm/views/list/list_item.py @@ -16,10 +16,9 @@ class ListItem(View): def post(self, request, list_id, list_item): """Edit a list item's notes""" list_item = get_object_or_404(models.ListItem, id=list_item, book_list=list_id) - list_item.raise_not_editable(request.user) form = forms.ListItemForm(request.POST, instance=list_item) if form.is_valid(): - item = form.save(commit=False) + item = form.save(request, commit=False) item.notes = to_markdown(item.notes) item.save() else: diff --git a/bookwyrm/views/list/lists.py b/bookwyrm/views/list/lists.py index ee6ff0867..1b2250794 100644 --- a/bookwyrm/views/list/lists.py +++ b/bookwyrm/views/list/lists.py @@ -36,8 +36,7 @@ class Lists(View): form = forms.ListForm(request.POST) if not form.is_valid(): return redirect("lists") - book_list = form.save(commit=False) - book_list.raise_not_editable(request.user) + book_list = form.save(request) # list should not have a group if it is not group curated if not book_list.curation == "group": diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 482da3cd0..328dfd7fa 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -159,7 +159,7 @@ class ReadThrough(View): models.ReadThrough, id=request.POST.get("id") ) return TemplateResponse(request, "readthrough/readthrough.html", data) - form.save() + form.save(request) return redirect("book", book_id) diff --git a/bookwyrm/views/shelf/shelf.py b/bookwyrm/views/shelf/shelf.py index 378b346b3..0c3074902 100644 --- a/bookwyrm/views/shelf/shelf.py +++ b/bookwyrm/views/shelf/shelf.py @@ -113,7 +113,6 @@ class Shelf(View): """edit a shelf""" user = get_user_from_username(request.user, username) shelf = get_object_or_404(user.shelf_set, identifier=shelf_identifier) - shelf.raise_not_editable(request.user) # you can't change the name of the default shelves if not shelf.editable and request.POST.get("name") != shelf.name: @@ -122,7 +121,7 @@ class Shelf(View): form = forms.ShelfForm(request.POST, instance=shelf) if not form.is_valid(): return redirect(shelf.local_path) - shelf = form.save() + shelf = form.save(request) return redirect(shelf.local_path) diff --git a/bookwyrm/views/shelf/shelf_actions.py b/bookwyrm/views/shelf/shelf_actions.py index 7dbb83dea..d2aa7d566 100644 --- a/bookwyrm/views/shelf/shelf_actions.py +++ b/bookwyrm/views/shelf/shelf_actions.py @@ -15,9 +15,7 @@ def create_shelf(request): if not form.is_valid(): return redirect("user-shelves", request.user.localname) - shelf = form.save(commit=False) - shelf.raise_not_editable(request.user) - shelf.save() + shelf = form.save(request) return redirect(shelf.local_path) diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index c0a045f8a..2f957f087 100644 --- a/bookwyrm/views/status.py +++ b/bookwyrm/views/status.py @@ -65,7 +65,6 @@ class CreateStatus(View): existing_status = get_object_or_404( models.Status.objects.select_subclasses(), id=existing_status_id ) - existing_status.raise_not_editable(request.user) existing_status.edited_date = timezone.now() status_type = status_type[0].upper() + status_type[1:] @@ -84,8 +83,7 @@ class CreateStatus(View): return HttpResponseBadRequest() return redirect("/") - status = form.save(commit=False) - status.raise_not_editable(request.user) + status = form.save(request) # save the plain, unformatted version of the status for future editing status.raw_content = status.content if hasattr(status, "quote"): From 5c3bb2da13ce85d6374ebf9ca81e970bdb8e6d97 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 5 Aug 2022 14:04:16 -0700 Subject: [PATCH 004/261] Refactors how mentions are collected This should be quicker, because it queries the users in one go instead of jumping straight to iterating over them, and it checks if a user blocks the poster before allowing them to be tagged. --- bookwyrm/tests/views/test_status.py | 95 ++++++++++++++++++----------- bookwyrm/utils/regex.py | 2 +- bookwyrm/views/helpers.py | 7 ++- bookwyrm/views/status.py | 45 +++++++++----- 4 files changed, 96 insertions(+), 53 deletions(-) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index e05d888e5..022c246c4 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -6,6 +6,7 @@ from django.test import TestCase from django.test.client import RequestFactory from bookwyrm import forms, models, views +from bookwyrm.views.status import find_mentions from bookwyrm.settings import DOMAIN from bookwyrm.tests.validate_html import validate_html @@ -34,6 +35,13 @@ class StatusViews(TestCase): localname="mouse", remote_id="https://example.com/users/mouse", ) + self.another_user = models.User.objects.create_user( + f"nutria@{DOMAIN}", + "nutria@nutria.com", + "password", + local=True, + localname="nutria", + ) with patch("bookwyrm.models.user.set_remote_server"): self.remote_user = models.User.objects.create_user( "rat", @@ -211,51 +219,66 @@ class StatusViews(TestCase): self.assertFalse(self.remote_user in reply.mention_users.all()) self.assertTrue(self.local_user in reply.mention_users.all()) - def test_find_mentions(self, *_): + def test_find_mentions_local(self, *_): """detect and look up @ mentions of users""" - user = models.User.objects.create_user( - f"nutria@{DOMAIN}", - "nutria@nutria.com", - "password", - local=True, - localname="nutria", - ) - self.assertEqual(user.username, f"nutria@{DOMAIN}") + result = find_mentions(self.local_user, "@nutria") + self.assertEqual(result["@nutria"], self.another_user) + self.assertEqual(result[f"@nutria@{DOMAIN}"], self.another_user) + result = find_mentions(self.local_user, f"@nutria@{DOMAIN}") + self.assertEqual(result["@nutria"], self.another_user) + self.assertEqual(result[f"@nutria@{DOMAIN}"], self.another_user) + + result = find_mentions(self.local_user, "leading text @nutria") + self.assertEqual(result["@nutria"], self.another_user) + self.assertEqual(result[f"@nutria@{DOMAIN}"], self.another_user) + + result = find_mentions(self.local_user, "leading @nutria trailing") + self.assertEqual(result["@nutria"], self.another_user) + self.assertEqual(result[f"@nutria@{DOMAIN}"], self.another_user) + + self.assertEqual(find_mentions(self.local_user, "leading@nutria"), {}) + + def test_find_mentions_remote(self, *_): + """detect and look up @ mentions of users""" self.assertEqual( - list(views.status.find_mentions("@nutria"))[0], ("@nutria", user) - ) - self.assertEqual( - list(views.status.find_mentions("leading text @nutria"))[0], - ("@nutria", user), - ) - self.assertEqual( - list(views.status.find_mentions("leading @nutria trailing text"))[0], - ("@nutria", user), - ) - self.assertEqual( - list(views.status.find_mentions("@rat@example.com"))[0], - ("@rat@example.com", self.remote_user), + find_mentions(self.local_user, "@rat@example.com"), + {"@rat@example.com": self.remote_user}, ) - multiple = list(views.status.find_mentions("@nutria and @rat@example.com")) - self.assertEqual(multiple[0], ("@nutria", user)) - self.assertEqual(multiple[1], ("@rat@example.com", self.remote_user)) + def test_find_mentions_multiple(self, *_): + """detect and look up @ mentions of users""" + multiple = find_mentions(self.local_user, "@nutria and @rat@example.com") + self.assertEqual(multiple["@nutria"], self.another_user) + self.assertEqual(multiple[f"@nutria@{DOMAIN}"], self.another_user) + self.assertEqual(multiple["@rat@example.com"], self.remote_user) + self.assertIsNone(multiple.get("@rat")) + def test_find_mentions_unknown(self, *_): + """detect and look up @ mentions of users""" + multiple = find_mentions(self.local_user, "@nutria and @rdkjfgh") + self.assertEqual(multiple["@nutria"], self.another_user) + self.assertEqual(multiple[f"@nutria@{DOMAIN}"], self.another_user) + + def test_find_mentions_blocked(self, *_): + """detect and look up @ mentions of users""" + self.another_user.blocks.add(self.local_user) + + result = find_mentions(self.local_user, "@nutria hello") + self.assertEqual(result, {}) + + def test_find_mentions_unknown_remote(self, *_): + """mention a user that isn't in the database""" with patch("bookwyrm.views.status.handle_remote_webfinger") as rw: - rw.return_value = self.local_user - self.assertEqual( - list(views.status.find_mentions("@beep@beep.com"))[0], - ("@beep@beep.com", self.local_user), - ) + rw.return_value = self.another_user + result = find_mentions(self.local_user, "@beep@beep.com") + self.assertEqual(result["@nutria"], self.another_user) + self.assertEqual(result[f"@nutria@{DOMAIN}"], self.another_user) + with patch("bookwyrm.views.status.handle_remote_webfinger") as rw: rw.return_value = None - self.assertEqual(list(views.status.find_mentions("@beep@beep.com")), []) - - self.assertEqual( - list(views.status.find_mentions(f"@nutria@{DOMAIN}"))[0], - (f"@nutria@{DOMAIN}", user), - ) + result = find_mentions(self.local_user, "@beep@beep.com") + self.assertEqual(result, {}) def test_format_links_simple_url(self, *_): """find and format urls into a tags""" diff --git a/bookwyrm/utils/regex.py b/bookwyrm/utils/regex.py index 38d097f7f..c8a475a3d 100644 --- a/bookwyrm/utils/regex.py +++ b/bookwyrm/utils/regex.py @@ -4,7 +4,7 @@ DOMAIN = r"[\w_\-\.]+\.[a-z\-]{2,}" LOCALNAME = r"@?[a-zA-Z_\-\.0-9]+" STRICT_LOCALNAME = r"@[a-zA-Z_\-\.0-9]+" USERNAME = rf"{LOCALNAME}(@{DOMAIN})?" -STRICT_USERNAME = rf"\B{STRICT_LOCALNAME}(@{DOMAIN})?\b" +STRICT_USERNAME = rf"(\B{STRICT_LOCALNAME}(@{DOMAIN})?\b)" FULL_USERNAME = rf"{LOCALNAME}@{DOMAIN}\b" SLUG = r"/s/(?P[-_a-z0-9]*)" # should match (BookWyrm/1.0.0; or (BookWyrm/99.1.2; diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 7be42a87b..f89ea0dfe 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -59,7 +59,7 @@ def is_bookwyrm_request(request): return True -def handle_remote_webfinger(query): +def handle_remote_webfinger(query, unknown_only=False): """webfingerin' other servers""" user = None @@ -75,6 +75,11 @@ def handle_remote_webfinger(query): try: user = models.User.objects.get(username__iexact=query) + + if unknown_only: + # In this case, we only want to know about previously undiscovered users + # So the fact that we found a match in the database means no results + return None except models.User.DoesNotExist: url = f"https://{domain}/.well-known/webfinger?resource=acct:{query}" try: diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index c0a045f8a..e4eb5ada5 100644 --- a/bookwyrm/views/status.py +++ b/bookwyrm/views/status.py @@ -6,6 +6,7 @@ from urllib.parse import urlparse from django.contrib.auth.decorators import login_required from django.core.validators import URLValidator from django.core.exceptions import ValidationError +from django.db.models import Q from django.http import HttpResponse, HttpResponseBadRequest, Http404 from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse @@ -16,7 +17,6 @@ from django.views.decorators.http import require_POST from markdown import markdown from bookwyrm import forms, models -from bookwyrm.settings import DOMAIN from bookwyrm.utils import regex, sanitizer from .helpers import handle_remote_webfinger, is_api_request from .helpers import load_date_in_user_tz_as_utc @@ -96,14 +96,16 @@ class CreateStatus(View): # inspect the text for user tags content = status.content - for (mention_text, mention_user) in find_mentions(content): + for (mention_text, mention_user) in find_mentions( + request.user, content + ).items(): # add them to status mentions fk status.mention_users.add(mention_user) # turn the mention into a link content = re.sub( - rf"{mention_text}([^@]|$)", - rf'{mention_text}\g<1>', + rf"{mention_text}\b(?!@)", + rf'{mention_text}', content, ) # add reply parent to mentions @@ -199,22 +201,35 @@ def edit_readthrough(request): return redirect("/") -def find_mentions(content): +def find_mentions(user, content): """detect @mentions in raw status content""" if not content: - return - for match in re.finditer(regex.STRICT_USERNAME, content): - username = match.group().strip().split("@")[1:] - if len(username) == 1: - # this looks like a local user (@user), fill in the domain - username.append(DOMAIN) - username = "@".join(username) + return {} + # The regex has nested match groups, so the 0th entry has the full (outer) match + # And beacuse the strict username starts with @, the username is 1st char onward + usernames = [m[0][1:] for m in re.findall(regex.STRICT_USERNAME, content)] - mention_user = handle_remote_webfinger(username) + known_users = ( + models.User.viewer_aware_objects(user) + .filter(Q(username__in=usernames) | Q(localname__in=usernames)) + .distinct() + ) + # Prepare a lookup based on both username and localname + username_dict = { + **{f"@{u.username}": u for u in known_users}, + **{f"@{u.localname}": u for u in known_users.filter(local=True)}, + } + + # Users not captured here could be blocked or not yet loaded on the server + not_found = set(usernames) - set(username_dict.keys()) + for username in not_found: + mention_user = handle_remote_webfinger(username, unknown_only=True) if not mention_user: - # we can ignore users we don't know about + # this user is blocked or can't be found continue - yield (match.group(), mention_user) + username_dict[f"@{mention_user.username}"] = mention_user + username_dict[f"@{mention_user.localname}"] = mention_user + return username_dict def format_links(content): From 15814914716cfc21727aa38d46acac2c6a25e359 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 5 Aug 2022 17:03:56 -0700 Subject: [PATCH 005/261] Removes duplicate version warning --- bookwyrm/templates/settings/dashboard/dashboard.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bookwyrm/templates/settings/dashboard/dashboard.html b/bookwyrm/templates/settings/dashboard/dashboard.html index ea7a8a796..99c0e9621 100644 --- a/bookwyrm/templates/settings/dashboard/dashboard.html +++ b/bookwyrm/templates/settings/dashboard/dashboard.html @@ -57,10 +57,6 @@ {% endif %} - {% if current_version %} - {% include 'settings/dashboard/warnings/update_version.html' with warning_level="warning" fullwidth=True %} - {% endif %} - {% if reports %} {% include 'settings/dashboard/warnings/reports.html' with warning_level="warning" %} {% endif %} From a5c7ca1cfd3706d8ac94347a4db257493eb21e10 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Aug 2022 11:27:52 -0700 Subject: [PATCH 006/261] Removes compilescss command This does not work and doesn't reflect how the stylesheets are compiled any longer. --- bw-dev | 5 ----- complete_bwdev.fish | 1 - complete_bwdev.sh | 1 - complete_bwdev.zsh | 1 - 4 files changed, 8 deletions(-) diff --git a/bw-dev b/bw-dev index fbd19dd7f..bffa358c9 100755 --- a/bw-dev +++ b/bw-dev @@ -189,10 +189,6 @@ case "$CMD" in bookwyrm/static/css/bookwyrm.scss bookwyrm/static/css/bookwyrm/**/*.scss --fix \ --config dev-tools/.stylelintrc.js ;; - compilescss) - runweb python manage.py compilescss - runweb python manage.py collectstatic --no-input - ;; collectstatic_watch) prod_error npm run --prefix dev-tools watch:static @@ -286,7 +282,6 @@ case "$CMD" in echo " prettier" echo " stylelint" echo " formatters" - echo " compilescss" echo " collectstatic_watch" echo " populate_streams [--stream=]" echo " populate_lists_streams" diff --git a/complete_bwdev.fish b/complete_bwdev.fish index c1f28dd51..ac87739f2 100644 --- a/complete_bwdev.fish +++ b/complete_bwdev.fish @@ -23,7 +23,6 @@ black \ prettier \ stylelint \ formatters \ -compilescss \ collectstatic_watch \ populate_streams \ populate_lists_streams \ diff --git a/complete_bwdev.sh b/complete_bwdev.sh index 5dd025673..3bfc95a67 100644 --- a/complete_bwdev.sh +++ b/complete_bwdev.sh @@ -20,7 +20,6 @@ black prettier stylelint formatters -compilescss collectstatic_watch populate_streams populate_lists_streams diff --git a/complete_bwdev.zsh b/complete_bwdev.zsh index 5f7695ee1..963597231 100644 --- a/complete_bwdev.zsh +++ b/complete_bwdev.zsh @@ -22,7 +22,6 @@ black prettier stylelint formatters -compilescss collectstatic_watch populate_streams populate_lists_streams From 0cf9e1033be8df2ddfb432e2e66beb1c9772e04c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Aug 2022 13:26:05 -0700 Subject: [PATCH 007/261] Safer display of book descriptions --- bookwyrm/templatetags/book_display_tags.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bookwyrm/templatetags/book_display_tags.py b/bookwyrm/templatetags/book_display_tags.py index 9db79f8e4..2ae900c9e 100644 --- a/bookwyrm/templatetags/book_display_tags.py +++ b/bookwyrm/templatetags/book_display_tags.py @@ -8,7 +8,12 @@ register = template.Library() @register.filter(name="book_description") def get_book_description(book): """use the work's text if the book doesn't have it""" - return book.description or book.parent_work.description + if book.description: + return book.description + if book.parent_work: + # this shoud always be true + return book.parent_work.description + return None @register.simple_tag(takes_context=False) From c591371b4e7b9e458a11f47a4f322878e74baadd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:08:55 +0000 Subject: [PATCH 008/261] Bump django from 3.2.14 to 3.2.15 Bumps [django](https://github.com/django/django) from 3.2.14 to 3.2.15. - [Release notes](https://github.com/django/django/releases) - [Commits](https://github.com/django/django/compare/3.2.14...3.2.15) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 05aa7c786..a838ce450 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ aiohttp==3.8.1 bleach==5.0.1 celery==5.2.2 colorthief==0.2.1 -Django==3.2.14 +Django==3.2.15 django-celery-beat==2.2.1 django-compressor==2.4.1 django-imagekit==4.1.0 From 22495e40bdd0661260774ad7c57519b19682e814 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 13 Aug 2022 15:40:53 +1000 Subject: [PATCH 009/261] strip spaces from user search Strips leading and trailing spaces from user search to prevent errors when doing webfinger lookup. Prior to this, webfinger user queries included everything after the second '@' as part of the hostname. This resulted in failed webfinger requests when there was one or more trailing spaces. Fixes #2205 --- bookwyrm/views/search.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bookwyrm/views/search.py b/bookwyrm/views/search.py index db33fd330..6bf230ddd 100644 --- a/bookwyrm/views/search.py +++ b/bookwyrm/views/search.py @@ -90,6 +90,7 @@ def user_search(request): """cool kids members only user search""" viewer = request.user query = request.GET.get("q") + query = query.strip() data = {"type": "user", "query": query} # logged out viewers can't search users if not viewer.is_authenticated: From 8d593e4498de1f48bd0464c8c807872253846bca Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 14 Aug 2022 14:17:10 +1000 Subject: [PATCH 010/261] hide replies to posts user cannot see This is in response to #1870 Users should not see links to posts they are not allowed to see, in their feed. The main question is how to stop that happening. This commit hides all replies to posts if the original post was "followers only" and the user is not a follower of the original poster. The privacy of the reply is not considered relevant (except "direct"). I believe this is the cleanest way to deal with the problem, as it avoids orphaned replies and confusing 404s, and a reply without access to the context of the original post is not particularly useful to anyone. This also feels like it respects the wishes of the original poster more accurately, as it does not draw attention from non-followers to the original followers-only post. A less draconian approach might be to remove the link to the original status in the feed interface, however that simply leads to confusion of another kind since it will make the interface inconsistent. This commit does not change any ActivityPub behaviour - it only affects the Bookwyrm user feeds. This means orphaned posts may be sent to external apps like Mastodon. --- bookwyrm/activitystreams.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index a90d7943b..90404f1c3 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -111,8 +111,18 @@ class ActivityStream(RedisStore): Q(id__in=status.user.blocks.all()) | Q(blocks=status.user) # not blocked ) + # don't show replies to statuses the user can't see + if status.reply_parent and status.reply_parent.privacy == "followers": + audience = audience.filter( + Q(id=status.user.id) # if the user is the post's author + | Q(id=status.reply_parent.user.id) # if the user is the OG author + | ( + Q(following=status.user) & Q(following=status.reply_parent.user) + ) # if the user is following both authors + ).distinct() + # only visible to the poster and mentioned users - if status.privacy == "direct": + elif status.privacy == "direct": audience = audience.filter( Q(id=status.user.id) # if the user is the post's author | Q(id__in=status.mention_users.all()) # if the user is mentioned From cc97c52d12bb2f4ce451290aab2d17c5b109bb86 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 21 Aug 2022 09:33:43 +1000 Subject: [PATCH 011/261] make get_audience logic clearer Retains 'direct' messages at the top of the logic tree to make it easier to understand. In practice because direct messages are excluded from feeds anyway, this doesn't seem to make much difference, but it's easier to read. --- bookwyrm/activitystreams.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 90404f1c3..f8312f063 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -111,8 +111,15 @@ class ActivityStream(RedisStore): Q(id__in=status.user.blocks.all()) | Q(blocks=status.user) # not blocked ) + # only visible to the poster and mentioned users + if status.privacy == "direct": + audience = audience.filter( + Q(id=status.user.id) # if the user is the post's author + | Q(id__in=status.mention_users.all()) # if the user is mentioned + ) + # don't show replies to statuses the user can't see - if status.reply_parent and status.reply_parent.privacy == "followers": + elif status.reply_parent and status.reply_parent.privacy == "followers": audience = audience.filter( Q(id=status.user.id) # if the user is the post's author | Q(id=status.reply_parent.user.id) # if the user is the OG author @@ -121,12 +128,6 @@ class ActivityStream(RedisStore): ) # if the user is following both authors ).distinct() - # only visible to the poster and mentioned users - elif status.privacy == "direct": - audience = audience.filter( - Q(id=status.user.id) # if the user is the post's author - | Q(id__in=status.mention_users.all()) # if the user is mentioned - ) # only visible to the poster's followers and tagged users elif status.privacy == "followers": audience = audience.filter( From bc297d663d8d091ab253ea1736a9860fde8de7da Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 27 Aug 2022 19:53:57 +1000 Subject: [PATCH 012/261] fix resetdb Instead of just whining I thought maybe I should fix the problem. This replaces a manual reset of the database with deletion of the bookwyrm volumes using docker. fixes #2276 --- bw-dev | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bw-dev b/bw-dev index bffa358c9..a8f638803 100755 --- a/bw-dev +++ b/bw-dev @@ -84,16 +84,14 @@ case "$CMD" in ;; resetdb) prod_error - clean - # Start just the DB so no one else is using it - docker-compose up --build -d db - rundb dropdb -U ${POSTGRES_USER} ${POSTGRES_DB} - rundb createdb -U ${POSTGRES_USER} ${POSTGRES_DB} - # Now start up web so we can run the migrations - docker-compose up --build -d web + docker-compose rm -svf + docker volume rm -f bookwyrm_media_volume bookwyrm_pgdata bookwyrm_redis_activity_data bookwyrm_redis_broker_data bookwyrm_static_volume + docker-compose build migrate + migrate django_celery_beat initdb - clean + runweb python manage.py collectstatic --no-input + admin_code ;; makemigrations) prod_error From da5fd32196eea9df9aedeec96522f8513b27927e Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 28 Aug 2022 11:05:40 +1000 Subject: [PATCH 013/261] normalise isbn searching ISBNs are always numeric except for when the check digit in ISBN-10s is a ten, indicated with a capital X. These changes ensure that ISBNs are always upper-case so that a lower-case 'x' is not used when searching. Additionally some ancient ISBNs have been printed without a leading zero (i.e. they only have 9 characters on the physical book). This change prepends a zero if something looks like an ISBN but only has 9 chars. --- bookwyrm/book_search.py | 4 +++- bookwyrm/connectors/abstract_connector.py | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/bookwyrm/book_search.py b/bookwyrm/book_search.py index 4b0a6eab9..29c48c936 100644 --- a/bookwyrm/book_search.py +++ b/bookwyrm/book_search.py @@ -30,7 +30,9 @@ def isbn_search(query): """search your local database""" if not query: return [] - + # Up-case the ISBN string to ensure any 'X' check-digit is correct + # If the ISBN has only 9 characters, prepend missing zero + query = query.upper().rjust(10, '0') filters = [{f: query} for f in ["isbn_10", "isbn_13"]] results = models.Edition.objects.filter( reduce(operator.or_, (Q(**f) for f in filters)) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index dc4be4b3d..87cae00c0 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -42,6 +42,9 @@ class AbstractMinimalConnector(ABC): """format the query url""" # Check if the query resembles an ISBN if maybe_isbn(query) and self.isbn_search_url and self.isbn_search_url != "": + # Up-case the ISBN string to ensure any 'X' check-digit is correct + # If the ISBN has only 9 characters, prepend missing zero + query = query.upper().rjust(10, '0') return f"{self.isbn_search_url}{query}" # NOTE: previously, we tried searching isbn and if that produces no results, @@ -325,4 +328,7 @@ def unique_physical_format(format_text): def maybe_isbn(query): """check if a query looks like an isbn""" isbn = re.sub(r"[\W_]", "", query) # removes filler characters - return len(isbn) in [10, 13] # ISBN10 or ISBN13 + # ISBNs must be numeric except an ISBN10 checkdigit can be 'X' + if not isbn.rstrip('X').isnumeric(): + return False + return len(isbn) in [9, 10, 13] # ISBN10 or ISBN13, or maybe ISBN10 missing a prepended zero From f219851f3ab4d9e433a0b261fc114b83312efd5c Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 28 Aug 2022 17:28:00 +1000 Subject: [PATCH 014/261] strip leading and following spaces from ISBN --- bookwyrm/book_search.py | 2 +- bookwyrm/connectors/abstract_connector.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bookwyrm/book_search.py b/bookwyrm/book_search.py index 29c48c936..4ea4c14a8 100644 --- a/bookwyrm/book_search.py +++ b/bookwyrm/book_search.py @@ -32,7 +32,7 @@ def isbn_search(query): return [] # Up-case the ISBN string to ensure any 'X' check-digit is correct # If the ISBN has only 9 characters, prepend missing zero - query = query.upper().rjust(10, '0') + query = query.strip().upper().rjust(10, '0') filters = [{f: query} for f in ["isbn_10", "isbn_13"]] results = models.Edition.objects.filter( reduce(operator.or_, (Q(**f) for f in filters)) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 87cae00c0..32559a307 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -44,9 +44,8 @@ class AbstractMinimalConnector(ABC): if maybe_isbn(query) and self.isbn_search_url and self.isbn_search_url != "": # Up-case the ISBN string to ensure any 'X' check-digit is correct # If the ISBN has only 9 characters, prepend missing zero - query = query.upper().rjust(10, '0') - return f"{self.isbn_search_url}{query}" - + normalized_query = query.strip().upper().rjust(10, '0') + return f"{self.isbn_search_url}{normalized_query}" # NOTE: previously, we tried searching isbn and if that produces no results, # searched as free text. This, instead, only searches isbn if it's isbn-y return f"{self.search_url}{query}" From 18d3d2f85dcd99cabc738cf921ea0b7714393200 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 28 Aug 2022 17:30:46 +1000 Subject: [PATCH 015/261] linting --- bookwyrm/book_search.py | 2 +- bookwyrm/connectors/abstract_connector.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bookwyrm/book_search.py b/bookwyrm/book_search.py index 4ea4c14a8..0dd41fccf 100644 --- a/bookwyrm/book_search.py +++ b/bookwyrm/book_search.py @@ -32,7 +32,7 @@ def isbn_search(query): return [] # Up-case the ISBN string to ensure any 'X' check-digit is correct # If the ISBN has only 9 characters, prepend missing zero - query = query.strip().upper().rjust(10, '0') + query = query.strip().upper().rjust(10, "0") filters = [{f: query} for f in ["isbn_10", "isbn_13"]] results = models.Edition.objects.filter( reduce(operator.or_, (Q(**f) for f in filters)) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 32559a307..dce4e5089 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -44,7 +44,7 @@ class AbstractMinimalConnector(ABC): if maybe_isbn(query) and self.isbn_search_url and self.isbn_search_url != "": # Up-case the ISBN string to ensure any 'X' check-digit is correct # If the ISBN has only 9 characters, prepend missing zero - normalized_query = query.strip().upper().rjust(10, '0') + normalized_query = query.strip().upper().rjust(10, "0") return f"{self.isbn_search_url}{normalized_query}" # NOTE: previously, we tried searching isbn and if that produces no results, # searched as free text. This, instead, only searches isbn if it's isbn-y @@ -328,6 +328,10 @@ def maybe_isbn(query): """check if a query looks like an isbn""" isbn = re.sub(r"[\W_]", "", query) # removes filler characters # ISBNs must be numeric except an ISBN10 checkdigit can be 'X' - if not isbn.rstrip('X').isnumeric(): + if not isbn.rstrip("X").isnumeric(): return False - return len(isbn) in [9, 10, 13] # ISBN10 or ISBN13, or maybe ISBN10 missing a prepended zero + return len(isbn) in [ + 9, + 10, + 13, + ] # ISBN10 or ISBN13, or maybe ISBN10 missing a prepended zero From 252fe7fd6a56c7654e1ab758bc3f671b72a0e47d Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 29 Aug 2022 14:02:04 +1000 Subject: [PATCH 016/261] fix visibility of stars when editing a review Previously the star rating appeared to be five stars when editing a review, regardless of what value was actually stored. Now it will show the actual rating, including half stars. Fixes #2213 --- bookwyrm/templates/snippets/form_rate_stars.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/snippets/form_rate_stars.html b/bookwyrm/templates/snippets/form_rate_stars.html index 302f181ed..47cb241dc 100644 --- a/bookwyrm/templates/snippets/form_rate_stars.html +++ b/bookwyrm/templates/snippets/form_rate_stars.html @@ -37,7 +37,7 @@ type="radio" name="rating" value="{{ forloop.counter0 }}.5" - {% if default_rating == forloop.counter %}checked{% endif %} + {% if default_rating >= forloop.counter0 %}checked{% endif %} /> = forloop.counter %}checked{% endif %} />