From 812221456b7ce37f2344f8c2c3da613b1a6378f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 28 Jul 2024 05:56:25 -0300 Subject: [PATCH 1/8] Add failing test case for comment progress clobbering start_date --- bookwyrm/tests/views/test_status.py | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 52582a235..21dc11dcc 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -1,9 +1,12 @@ """ test for app action functionality """ import json +from unittest import expectedFailure from unittest.mock import patch +import dateutil from django.core.exceptions import PermissionDenied from django.test import TestCase, TransactionTestCase from django.test.client import RequestFactory +from django.utils import timezone from bookwyrm import forms, models, views from bookwyrm.views.status import find_mentions, find_or_create_hashtags @@ -167,6 +170,38 @@ class StatusViews(TestCase): self.assertEqual(status.rating, 4.0) self.assertIsNone(status.edited_date) + @expectedFailure # https://github.com/bookwyrm-social/bookwyrm/issues/3164 + def test_create_status_progress(self, *_): + """create a status that updates a readthrough""" + start_date = timezone.make_aware(dateutil.parser.parse("2024-07-27")) + readthrough = models.ReadThrough.objects.create( + book=self.book, user=self.local_user, start_date=start_date + ) + + self.assertEqual(start_date, readthrough.start_date) + self.assertIsNone(readthrough.progress) + + view = views.CreateStatus.as_view() + form = forms.CommentForm( + { + "progress": 1, + "progress_mode": "PG", + "content": "I started the book", + "id": readthrough.id, + "book": self.book.id, + "user": self.local_user.id, + "privacy": "public", + } + ) + request = self.factory.post("", form.data) + request.user = self.local_user + + view(request, "comment") + readthrough.refresh_from_db() + + self.assertEqual(1, readthrough.progress) + self.assertEqual(start_date, readthrough.start_date) + def test_create_status_wrong_user(self, *_): """You can't compose statuses for someone else""" view = views.CreateStatus.as_view() From 041f2fc41304ccbd8199f57c297a9858cfe9e218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 28 Jul 2024 05:57:57 -0300 Subject: [PATCH 2/8] edit_readthrough: set start_date/finish_date iff present in request Fixes: #3164 --- bookwyrm/tests/views/test_status.py | 4 +--- bookwyrm/views/status.py | 11 +++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 21dc11dcc..af8a6dc3f 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -1,6 +1,5 @@ """ test for app action functionality """ import json -from unittest import expectedFailure from unittest.mock import patch import dateutil from django.core.exceptions import PermissionDenied @@ -170,7 +169,6 @@ class StatusViews(TestCase): self.assertEqual(status.rating, 4.0) self.assertIsNone(status.edited_date) - @expectedFailure # https://github.com/bookwyrm-social/bookwyrm/issues/3164 def test_create_status_progress(self, *_): """create a status that updates a readthrough""" start_date = timezone.make_aware(dateutil.parser.parse("2024-07-27")) @@ -200,7 +198,7 @@ class StatusViews(TestCase): readthrough.refresh_from_db() self.assertEqual(1, readthrough.progress) - self.assertEqual(start_date, readthrough.start_date) + self.assertEqual(start_date, readthrough.start_date) # not overwritten def test_create_status_wrong_user(self, *_): """You can't compose statuses for someone else""" diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index f2f03405f..85c41e3ec 100644 --- a/bookwyrm/views/status.py +++ b/bookwyrm/views/status.py @@ -201,12 +201,11 @@ def edit_readthrough(request): # TODO: remove this, it duplicates the code in the ReadThrough view readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id")) - readthrough.start_date = load_date_in_user_tz_as_utc( - request.POST.get("start_date"), request.user - ) - readthrough.finish_date = load_date_in_user_tz_as_utc( - request.POST.get("finish_date"), request.user - ) + if start_date := request.POST.get("start_date"): + readthrough.start_date = load_date_in_user_tz_as_utc(start_date, request.user) + + if finish_date := request.POST.get("finish_date"): + readthrough.finish_date = load_date_in_user_tz_as_utc(finish_date, request.user) progress = request.POST.get("progress") try: From f4133e0236c89cf4902b91bd2dcadb1514a4c31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Thu, 30 Nov 2023 22:37:10 -0300 Subject: [PATCH 3/8] celerywyrm: allow broker and result backend from the environment This allows to easily configure an in-memory transport for tests. --- celerywyrm/settings.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/celerywyrm/settings.py b/celerywyrm/settings.py index 20f194a12..3ca9b2748 100644 --- a/celerywyrm/settings.py +++ b/celerywyrm/settings.py @@ -15,8 +15,12 @@ REDIS_BROKER_URL = env( f"redis://:{REDIS_BROKER_PASSWORD}@{REDIS_BROKER_HOST}:{REDIS_BROKER_PORT}/{REDIS_BROKER_DB_INDEX}", ) -CELERY_BROKER_URL = REDIS_BROKER_URL.replace("unix:", "redis+socket:") -CELERY_RESULT_BACKEND = REDIS_BROKER_URL.replace("unix:", "redis+socket:") +CELERY_BROKER_URL = env( + "CELERY_BROKER_URL", REDIS_BROKER_URL.replace("unix:", "redis+socket:") +) +CELERY_RESULT_BACKEND = env( + "CELERY_RESULT_BACKEND", REDIS_BROKER_URL.replace("unix:", "redis+socket:") +) CELERY_DEFAULT_QUEUE = "low_priority" CELERY_CREATE_MISSING_QUEUES = True From 3d28019146e58a9be3502bd22d1525ba9ecf1750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Thu, 30 Nov 2023 22:43:35 -0300 Subject: [PATCH 4/8] Use in-memory backends for Celery in tests and workflows At the moment, this doesn't have much of an effect since most task calls are mock'd out. --- pytest.ini | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pytest.ini b/pytest.ini index b963fb316..74970e877 100644 --- a/pytest.ini +++ b/pytest.ini @@ -15,11 +15,8 @@ env = ALLOWED_HOSTS = your.domain.here BOOKWYRM_DATABASE_BACKEND = postgres MEDIA_ROOT = images/ - CELERY_BROKER = - REDIS_BROKER_PORT = 6379 - REDIS_BROKER_PASSWORD = beep - REDIS_ACTIVITY_PORT = 6379 - REDIS_ACTIVITY_PASSWORD = beep + CELERY_BROKER_URL = memory:// + CELERY_RESULT_BACKEND = cache+memory:// USE_DUMMY_CACHE = true FLOWER_PORT = 8888 EMAIL_HOST = smtp.mailgun.org From 69962bb7c98b0e5b4c59fdcfd44de1949517afab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 28 Jul 2024 21:58:26 -0300 Subject: [PATCH 5/8] Avoid empty in templates Newer versions of tidylib complain about this. --- bookwyrm/templates/ostatus/remote_follow.html | 2 ++ bookwyrm/templates/ostatus/success.html | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/bookwyrm/templates/ostatus/remote_follow.html b/bookwyrm/templates/ostatus/remote_follow.html index ca47c529b..c54e6bfa7 100644 --- a/bookwyrm/templates/ostatus/remote_follow.html +++ b/bookwyrm/templates/ostatus/remote_follow.html @@ -3,8 +3,10 @@ {% load utilities %} {% block heading %} +{% block title %} {% blocktrans with username=user.localname sitename=site.name %}Follow {{ username }} on the fediverse{% endblocktrans %} {% endblock %} +{% endblock %} {% block content %} <div class="block card"> diff --git a/bookwyrm/templates/ostatus/success.html b/bookwyrm/templates/ostatus/success.html index 66577e83f..c2b8edd75 100644 --- a/bookwyrm/templates/ostatus/success.html +++ b/bookwyrm/templates/ostatus/success.html @@ -2,6 +2,10 @@ {% load i18n %} {% load utilities %} +{% block title %} +{% blocktrans with display_name=user.display_name %}You are now following {{ display_name }}!{% endblocktrans %} +{% endblock %} + {% block content %} <div class="block card"> <div class="card-content"> From b45435803a3abf2187d35940a67f413c1a8904d6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:33:05 +0000 Subject: [PATCH 6/8] Bump django from 4.2.14 to 4.2.15 Bumps [django](https://github.com/django/django) from 4.2.14 to 4.2.15. - [Commits](https://github.com/django/django/compare/4.2.14...4.2.15) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index d93ff9a74..49bc810bc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ boto3==1.34.74 bw-file-resubmit==0.6.0rc2 celery==5.3.6 colorthief==0.2.1 -Django==4.2.14 +Django==4.2.15 django-celery-beat==2.6.0 django-compressor==4.4 django-csp==3.8 From 2bb77d9bf81aa9de84d259309c2b452fcbb84552 Mon Sep 17 00:00:00 2001 From: Tim Rogers <rogers.timothy.john@gmail.com> Date: Sun, 11 Aug 2024 13:01:48 -0500 Subject: [PATCH 7/8] Updated search view to trim leading and trailing whitespace for author, book, and list query values --- bookwyrm/views/search.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bookwyrm/views/search.py b/bookwyrm/views/search.py index 13695a7d4..95845db64 100644 --- a/bookwyrm/views/search.py +++ b/bookwyrm/views/search.py @@ -53,7 +53,7 @@ class Search(View): def api_book_search(request): """Return books via API response""" - query = request.GET.get("q") + query = request.GET.get("q").strip() query = isbn_check_and_format(query) min_confidence = request.GET.get("min_confidence", 0) # only return local book results via json so we don't cascade @@ -65,7 +65,7 @@ def api_book_search(request): def book_search(request): """the real business is elsewhere""" - query = request.GET.get("q") + query = request.GET.get("q").strip() # check if query is isbn query = isbn_check_and_format(query) min_confidence = request.GET.get("min_confidence", 0) @@ -123,8 +123,7 @@ def author_search(request): def user_search(request): """user search: search for a user""" viewer = request.user - query = request.GET.get("q") - query = query.strip() + query = request.GET.get("q").strip() data = {"type": "user", "query": query} # use webfinger for mastodon style account@domain.com username to load the user if @@ -162,7 +161,7 @@ def user_search(request): def list_search(request): """any relevent lists?""" - query = request.GET.get("q") + query = request.GET.get("q").strip() data = {"query": query, "type": "list"} results = ( models.List.privacy_filter( From f68e33ffc6b10488823fa06cd6d41a3c9aa46fd2 Mon Sep 17 00:00:00 2001 From: Tim Rogers <rogers.timothy.john@gmail.com> Date: Sun, 11 Aug 2024 14:02:24 -0500 Subject: [PATCH 8/8] Added additional test cases for searches with leading and trailing whitespace --- bookwyrm/tests/views/test_search.py | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/bookwyrm/tests/views/test_search.py b/bookwyrm/tests/views/test_search.py index 6c7e41cf3..3b3e78e17 100644 --- a/bookwyrm/tests/views/test_search.py +++ b/bookwyrm/tests/views/test_search.py @@ -103,6 +103,20 @@ class Views(TestCase): connector_results = response.context_data["remote_results"] self.assertEqual(connector_results[0]["results"][0].title, "Mock Book") + def test_search_books_extra_whitespace(self): + """just the search page""" + view = views.Search.as_view() + request = self.factory.get("", {"q": " Test Book ", "remote": False}) + request.user = self.local_user + with patch("bookwyrm.views.search.is_api_request") as is_api: + is_api.return_value = False + response = view(request) + self.assertIsInstance(response, TemplateResponse) + validate_html(response.render()) + + local_results = response.context_data["results"] + self.assertEqual(local_results[0].title, "Test Book") + def test_search_book_anonymous(self): """Don't search remote for logged out user""" view = views.Search.as_view() @@ -150,6 +164,17 @@ class Views(TestCase): validate_html(response.render()) self.assertEqual(response.context_data["results"][0], self.local_user) + def test_search_users_extra_whitespace(self): + """searches remote connectors""" + view = views.Search.as_view() + request = self.factory.get("", {"q": " mouse ", "type": "user"}) + request.user = self.local_user + response = view(request) + + self.assertIsInstance(response, TemplateResponse) + validate_html(response.render()) + self.assertEqual(response.context_data["results"][0], self.local_user) + def test_search_users_logged_out(self): """searches remote connectors""" view = views.Search.as_view() @@ -181,3 +206,21 @@ class Views(TestCase): self.assertIsInstance(response, TemplateResponse) validate_html(response.render()) self.assertEqual(response.context_data["results"][0], booklist) + + def test_search_lists_extra_whitespace(self): + """searches remote connectors""" + with ( + patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"), + patch("bookwyrm.lists_stream.remove_list_task.delay"), + ): + booklist = models.List.objects.create( + user=self.local_user, name="test list" + ) + view = views.Search.as_view() + request = self.factory.get("", {"q": " test ", "type": "list"}) + request.user = self.local_user + response = view(request) + + self.assertIsInstance(response, TemplateResponse) + validate_html(response.render()) + self.assertEqual(response.context_data["results"][0], booklist)