diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6633bb0ebb..a10ec48668 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -37,6 +37,7 @@ Changelog * Fix: Ensure comment notifications dropdown handles longer translations without overflowing content (Krzysztof Jeziorny) * Fix: Set `default_auto_field` in `postgres_search` `AppConfig` (Nick Moreton) * Fix: Ensure admin tab JS events are handled on page load (Andrew Stone) + * Fix: `EmailNotificationMixin` and `send_notification` should only send emails to active users (Bryan Williams) 2.14.1 (12.08.2021) diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index 9342d628cf..a95b19b8a3 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -538,6 +538,7 @@ Contributors * Desai Akshata * Krzysztof Jeziorny * Nick Moreton +* Bryan Williams Translators =========== diff --git a/docs/releases/2.15.rst b/docs/releases/2.15.rst index 420ca14b86..db4ea49501 100644 --- a/docs/releases/2.15.rst +++ b/docs/releases/2.15.rst @@ -52,6 +52,7 @@ Bug fixes * Ensure comment notifications dropdown handles longer translations without overflowing content (Krzysztof Jeziorny) * Set ``default_auto_field`` in ``postgres_search`` ``AppConfig`` (Nick Moreton) * Ensure admin tab JS events are handled on page load (Andrew Stone) + * `EmailNotificationMixin` and `send_notification` should only send emails to active users (Bryan Williams) Upgrade considerations ====================== diff --git a/wagtail/admin/mail.py b/wagtail/admin/mail.py index b5b53d236e..a5d1f00b36 100644 --- a/wagtail/admin/mail.py +++ b/wagtail/admin/mail.py @@ -84,7 +84,7 @@ def send_notification(recipient_users, notification, extra_context): # Get list of email addresses email_recipients = [ recipient for recipient in recipient_users - if recipient.email and getattr( + if recipient.is_active and recipient.email and getattr( UserProfile.get_for_user(recipient), notification + '_notifications' ) @@ -206,11 +206,13 @@ class EmailNotificationMixin: def get_valid_recipients(self, instance, **kwargs): """Filters notification recipients to those allowing the notification type on their UserProfile, and those with an email address""" - - return {recipient for recipient in self.get_recipient_users(instance, **kwargs) if recipient.email and getattr( - UserProfile.get_for_user(recipient), - self.notification + '_notifications' - )} + return { + recipient for recipient in self.get_recipient_users(instance, **kwargs) + if recipient.is_active and recipient.email and getattr( + UserProfile.get_for_user(recipient), + self.notification + '_notifications' + ) + } def get_template_set(self, instance, **kwargs): """Return a dictionary of template paths for the templates for the email subject and the text and html diff --git a/wagtail/admin/tests/pages/test_edit_page.py b/wagtail/admin/tests/pages/test_edit_page.py index 797a920776..c6d4082a63 100644 --- a/wagtail/admin/tests/pages/test_edit_page.py +++ b/wagtail/admin/tests/pages/test_edit_page.py @@ -2564,3 +2564,39 @@ class TestCommenting(TestCase, WagtailTestUtils): # This time, no emails should be submitted because the only subscriber has disabled these emails globally self.assertEqual(len(mail.outbox), 0) + + def test_updated_comments_notifications_active_users_only(self): + # subscriber is inactive + self.subscriber.is_active = False + self.subscriber.save() + + post_data = { + 'title': "I've been edited!", + 'content': "Some content", + 'slug': 'hello-world', + 'comments-TOTAL_FORMS': '1', + 'comments-INITIAL_FORMS': '0', + 'comments-MIN_NUM_FORMS': '0', + 'comments-MAX_NUM_FORMS': '', + 'comments-0-DELETE': '', + 'comments-0-resolved': '', + 'comments-0-id': '', + 'comments-0-contentpath': 'title', + 'comments-0-text': 'A test comment', + 'comments-0-position': '', + 'comments-0-replies-TOTAL_FORMS': '0', + 'comments-0-replies-INITIAL_FORMS': '0', + 'comments-0-replies-MIN_NUM_FORMS': '0', + 'comments-0-replies-MAX_NUM_FORMS': '0' + } + + response = self.client.post(reverse('wagtailadmin_pages:edit', args=[self.child_page.id]), post_data) + + self.assertRedirects(response, reverse('wagtailadmin_pages:edit', args=[self.child_page.id])) + + # Check the comment was added + comment = self.child_page.comments.get() + self.assertEqual(comment.text, 'A test comment') + + # No emails should be submitted because subscriber is inactive + self.assertEqual(len(mail.outbox), 0) diff --git a/wagtail/admin/tests/test_workflows.py b/wagtail/admin/tests/test_workflows.py index 03dbe1f7df..8259acb905 100644 --- a/wagtail/admin/tests/test_workflows.py +++ b/wagtail/admin/tests/test_workflows.py @@ -1317,6 +1317,33 @@ class TestNotificationPreferences(TestCase, WagtailTestUtils): # with `WAGTAILADMIN_NOTIFICATION_INCLUDE_SUPERUSERS` off, the superuser should not get a workflow email self.assertNotIn(self.superuser.email, workflow_submission_emailed_addresses) + @override_settings(WAGTAILADMIN_NOTIFICATION_INCLUDE_SUPERUSERS=True) + def test_submit_notification_active_users_only(self): + # moderator2 is inactive + self.moderator2.is_active = False + self.moderator2.save() + + # superuser is inactive + self.superuser.is_active = False + self.superuser.save() + + # Submit + self.login(self.submitter) + self.submit() + + workflow_submission_emails = [email for email in mail.outbox if "workflow" in email.subject] + workflow_submission_emailed_addresses = [address for email in workflow_submission_emails for address in + email.to] + task_submission_emails = [email for email in mail.outbox if "task" in email.subject] + task_submission_emailed_addresses = [address for email in task_submission_emails for address in email.to] + + # Check that moderator2 didn't receive a task submitted email + self.assertNotIn(self.moderator2.email, task_submission_emailed_addresses) + + # Check that the superuser didn't receive a workflow or task email + self.assertNotIn(self.superuser.email, task_submission_emailed_addresses) + self.assertNotIn(self.superuser.email, workflow_submission_emailed_addresses) + @override_settings(WAGTAILADMIN_NOTIFICATION_INCLUDE_SUPERUSERS=True) def test_submit_notification_preferences_respected(self): # moderator2 doesn't want emails