From de8b7fbc5b8379fa2ef6b91b28739ccc9e8c0124 Mon Sep 17 00:00:00 2001 From: Anett Seeker Date: Wed, 18 Dec 2024 10:39:50 +0100 Subject: [PATCH 1/3] Add tests --- froide/foirequest/tests/test_misc.py | 127 ++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/froide/foirequest/tests/test_misc.py b/froide/foirequest/tests/test_misc.py index 4acf69775..1fc179074 100644 --- a/froide/foirequest/tests/test_misc.py +++ b/froide/foirequest/tests/test_misc.py @@ -1,4 +1,5 @@ from datetime import timedelta +from unittest.mock import MagicMock, patch from django.contrib.contenttypes.models import ContentType from django.contrib.sites.models import Site @@ -11,7 +12,11 @@ from froide.comments.models import FroideComment from froide.foirequest.models import FoiMessage, FoiRequest -from froide.foirequest.notifications import batch_update_requester +from froide.foirequest.notifications import ( + Notification, + batch_update_requester, + send_update, +) from froide.foirequest.tasks import ( classification_reminder, detect_asleep, @@ -121,6 +126,126 @@ def test_requester_batch_update(self): self.assertIn("Update on one of your request", mail.outbox[0].subject) +class SendUpdateTest(TestCase): + def setUp(self): + self.user1 = factories.UserFactory.create() + self.user2 = factories.UserFactory.create() + self.user3 = factories.UserFactory.create() + + # Some requests, all created by user 1. + self.request1a = self.get_request(self.user1) + self.request1b = self.get_request(self.user1) + self.request1c = self.get_request(self.user1) + + # Notifications for comments by user X on request Y. + self.comment_request1a_user1 = self.get_notification( + self.request1a, self.user1, "Comment 1" + ) + self.comment_request1a_user2 = self.get_notification( + self.request1a, self.user2, "Comment 2" + ) + self.comment_request1b_user2 = self.get_notification( + self.request1b, self.user2, "Comment 3" + ) + self.comment_request1c_user3 = self.get_notification( + self.request1c, self.user3, "Comment 4" + ) + self.comment_request1c_user2 = self.get_notification( + self.request1c, self.user2, "Comment 5" + ) + + # Path to the send function to be patched. + self.send_path = "froide.foirequest.notifications.update_requester_email.send" + + def get_request(self, user): + request = factories.FoiRequestFactory.create() + request.user = user + return request + + def get_notification(self, request, user, event_name): + notification = MagicMock(spec=Notification) + notification.object = request + notification.user_id = user.id + notification.event = self.get_event(event_name) + return notification + + def get_event(self, event_name): + event = MagicMock() + event.as_text.return_value = event_name + return event + + def test_does_not_send_email_if_no_notifications(self): + notifications = [] + + with patch(self.send_path) as mock_send: + send_update(notifications, self.user1) + mock_send.assert_not_called() + + def test_does_not_include_requests_with_only_requester_comments(self): + notifications = [self.comment_request1a_user1] + + with patch(self.send_path) as mock_send: + send_update(notifications, self.user1) + mock_send.assert_not_called() + + def test_includes_requests_with_only_non_requester_comments(self): + notifications = [self.comment_request1a_user2] + + with patch(self.send_path) as mock_send: + send_update(notifications, self.user1) + mock_send.assert_called_once() + + context = mock_send.call_args[1]["context"] + request_list = context["request_list"] + + assert context["user"] == self.user1 + assert context["count"] == 1 + assert len(request_list) == 1 + assert request_list[0]["request"] == self.request1a + assert request_list[0]["events"] == ["Comment 2"] + assert mock_send.call_args[1]["subject"] == "Update on one of your request" + + def test_includes_requests_with_both_requester_and_non_requester_comments(self): + notifications = [self.comment_request1a_user1, self.comment_request1a_user2] + + with patch(self.send_path) as mock_send: + send_update(notifications, self.user1) + mock_send.assert_called_once() + + context = mock_send.call_args[1]["context"] + request_list = context["request_list"] + + assert context["user"] == self.user1 + assert context["count"] == 1 + assert len(request_list) == 1 + assert request_list[0]["request"] == self.request1a + assert request_list[0]["events"] == ["Comment 1", "Comment 2"] + assert mock_send.call_args[1]["subject"] == "Update on one of your request" + + def test_includes_multiple_requests_with_comments_from_multiple_users(self): + notifications = [ + self.comment_request1b_user2, + self.comment_request1c_user3, + self.comment_request1c_user2, + ] + + with patch(self.send_path) as mock_send: + send_update(notifications, self.user1) + mock_send.assert_called_once() + + context = mock_send.call_args[1]["context"] + request_list = context["request_list"] + + assert context["user"] == self.user1 + assert context["count"] == 2 + assert len(request_list) == 2 + assert request_list[0]["request"] == self.request1b + assert request_list[0]["events"] == ["Comment 3"] + assert request_list[1]["request"] == self.request1c + assert request_list[1]["events"] == ["Comment 4", "Comment 5"] + assert mock_send.call_args[1]["subject"] == "Update on 2 of your requests" + + class MailAttachmentSizeCheckerTest(TestCase): def test_attachment_size_checker(self): files = [ From fc02316d509baa52259f0d4a0f315046f79024e2 Mon Sep 17 00:00:00 2001 From: Anett Seeker Date: Wed, 18 Dec 2024 10:40:53 +0100 Subject: [PATCH 2/3] Fix counting of requests with updates --- froide/foirequest/notifications.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/froide/foirequest/notifications.py b/froide/foirequest/notifications.py index 66408fc62..8601f2242 100644 --- a/froide/foirequest/notifications.py +++ b/froide/foirequest/notifications.py @@ -118,11 +118,6 @@ def key_func(n): def send_update(notifications: List[Notification], user): translation.activate(user.language or settings.LANGUAGE_CODE) - count = len(notifications) - subject = ngettext_lazy( - "Update on one of your request", "Update on %(count)s of your requests", count - ) % {"count": count} - # Add additional info to template context request_list = [] grouped_notifications = groupby(notifications, lambda n: n.object.id) @@ -146,6 +141,11 @@ def send_update(notifications: List[Notification], user): if not request_list: return + count = len(request_list) + subject = ngettext_lazy( + "Update on one of your request", "Update on %(count)s of your requests", count + ) % {"count": count} + update_requester_email.send( user=user, subject=subject, From 1eda61173120a256d5d7956112b1f8e6adf6aa92 Mon Sep 17 00:00:00 2001 From: Anett Seeker Date: Wed, 18 Dec 2024 10:47:24 +0100 Subject: [PATCH 3/3] Fix subject text --- froide/foirequest/notifications.py | 2 +- froide/foirequest/tests/test_misc.py | 6 +++--- froide/locale/de/LC_MESSAGES/django.po | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/froide/foirequest/notifications.py b/froide/foirequest/notifications.py index 8601f2242..21642c366 100644 --- a/froide/foirequest/notifications.py +++ b/froide/foirequest/notifications.py @@ -143,7 +143,7 @@ def send_update(notifications: List[Notification], user): count = len(request_list) subject = ngettext_lazy( - "Update on one of your request", "Update on %(count)s of your requests", count + "Update on one of your requests", "Update on %(count)s of your requests", count ) % {"count": count} update_requester_email.send( diff --git a/froide/foirequest/tests/test_misc.py b/froide/foirequest/tests/test_misc.py index 1fc179074..68c8a9474 100644 --- a/froide/foirequest/tests/test_misc.py +++ b/froide/foirequest/tests/test_misc.py @@ -123,7 +123,7 @@ def test_requester_batch_update(self): ) batch_update_requester(start=start, end=end) self.assertEqual(len(mail.outbox), 1) - self.assertIn("Update on one of your request", mail.outbox[0].subject) + self.assertIn("Update on one of your requests", mail.outbox[0].subject) class SendUpdateTest(TestCase): @@ -203,7 +203,7 @@ def test_includes_requests_with_only_non_requester_comments(self): assert len(request_list) == 1 assert request_list[0]["request"] == self.request1a assert request_list[0]["events"] == ["Comment 2"] - assert mock_send.call_args[1]["subject"] == "Update on one of your request" + assert mock_send.call_args[1]["subject"] == "Update on one of your requests" def test_includes_requests_with_both_requester_and_non_requester_comments(self): notifications = [self.comment_request1a_user1, self.comment_request1a_user2] @@ -220,7 +220,7 @@ def test_includes_requests_with_both_requester_and_non_requester_comments(self): assert len(request_list) == 1 assert request_list[0]["request"] == self.request1a assert request_list[0]["events"] == ["Comment 1", "Comment 2"] - assert mock_send.call_args[1]["subject"] == "Update on one of your request" + assert mock_send.call_args[1]["subject"] == "Update on one of your requests" def test_includes_multiple_requests_with_comments_from_multiple_users(self): notifications = [ diff --git a/froide/locale/de/LC_MESSAGES/django.po b/froide/locale/de/LC_MESSAGES/django.po index f6fec4f3d..4ec325b4e 100644 --- a/froide/locale/de/LC_MESSAGES/django.po +++ b/froide/locale/de/LC_MESSAGES/django.po @@ -4137,7 +4137,7 @@ msgstr "Nutzer:in wurde eine E-Mail gesendet." #: froide/foirequest/notifications.py #, python-format -msgid "Update on one of your request" +msgid "Update on one of your requests" msgid_plural "Update on %(count)s of your requests" msgstr[0] "Neues bei einer Ihrer Anfragen" msgstr[1] "Neues bei %(count)s Ihrer Anfragen"