From a898ebce4038d5967c1e37503bb58121c469e9a8 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 22 Oct 2022 18:56:38 +1100 Subject: [PATCH] Fix email notification setting (#3832) * Coerce setting value to a boolean * Ignore inactive users when sending notification emails * Only send UI notifications to active users * Fixes for unit tests --- InvenTree/build/test_build.py | 6 ++++-- InvenTree/common/notifications.py | 5 +++-- InvenTree/common/tests.py | 5 +++-- InvenTree/order/test_sales_order.py | 2 +- InvenTree/order/tests.py | 7 ++++++- .../plugin/builtin/integration/core_notifications.py | 8 +++++++- 6 files changed, 24 insertions(+), 9 deletions(-) diff --git a/InvenTree/build/test_build.py b/InvenTree/build/test_build.py index 23925eba17..6561e08903 100644 --- a/InvenTree/build/test_build.py +++ b/InvenTree/build/test_build.py @@ -557,11 +557,13 @@ class BuildTest(BuildTestBase): category='build.new_build', ) - self.assertEqual(messages.count(), 2) + self.assertEqual(messages.count(), 1) self.assertFalse(messages.filter(user__pk=2).exists()) - self.assertTrue(messages.filter(user__pk=3).exists()) + # Inactive users do not receive notifications + self.assertFalse(messages.filter(user__pk=3).exists()) + self.assertTrue(messages.filter(user__pk=4).exists()) diff --git a/InvenTree/common/notifications.py b/InvenTree/common/notifications.py index 1d4e7ae47b..0928fad6d0 100644 --- a/InvenTree/common/notifications.py +++ b/InvenTree/common/notifications.py @@ -243,8 +243,9 @@ class UIMessageNotification(SingleNotificationMethod): METHOD_NAME = 'ui_message' def get_targets(self): - """Just return the targets - no tricks here.""" - return self.targets + """Only send notifications for active users""" + + return [target for target in self.targets if target.is_active] def send(self, target): """Send a UI notification to a user.""" diff --git a/InvenTree/common/tests.py b/InvenTree/common/tests.py index 9720508d10..b305073045 100644 --- a/InvenTree/common/tests.py +++ b/InvenTree/common/tests.py @@ -778,7 +778,8 @@ class NotificationTest(InvenTreeAPITestCase): messages = NotificationMessage.objects.all() # As there are three staff users (including the 'test' user) we expect 30 notifications - self.assertEqual(messages.count(), 30) + # However, one user is marked as i nactive + self.assertEqual(messages.count(), 20) # Only 10 messages related to *this* user my_notifications = messages.filter(user=self.user) @@ -822,7 +823,7 @@ class NotificationTest(InvenTreeAPITestCase): # Only 7 notifications should have been deleted, # as the notifications associated with other users must remain untouched - self.assertEqual(NotificationMessage.objects.count(), 23) + self.assertEqual(NotificationMessage.objects.count(), 13) self.assertEqual(NotificationMessage.objects.filter(user=self.user).count(), 3) diff --git a/InvenTree/order/test_sales_order.py b/InvenTree/order/test_sales_order.py index 2d00f69ac6..98db41ff42 100644 --- a/InvenTree/order/test_sales_order.py +++ b/InvenTree/order/test_sales_order.py @@ -267,7 +267,7 @@ class SalesOrderTest(TestCase): category='order.overdue_sales_order', ) - self.assertEqual(len(messages), 2) + self.assertEqual(len(messages), 1) def test_new_so_notification(self): """Test that a notification is sent when a new SalesOrder is created. diff --git a/InvenTree/order/tests.py b/InvenTree/order/tests.py index 8c66b3a53a..4a758c3d6e 100644 --- a/InvenTree/order/tests.py +++ b/InvenTree/order/tests.py @@ -326,7 +326,12 @@ class OrderTest(TestCase): user__id=user_id, ) - self.assertTrue(messages.exists()) + # User ID 3 is inactive, and thus should not receive notifications + if user_id == 3: + self.assertFalse(messages.exists()) + continue + else: + self.assertTrue(messages.exists()) msg = messages.first() diff --git a/InvenTree/plugin/builtin/integration/core_notifications.py b/InvenTree/plugin/builtin/integration/core_notifications.py index d21dc8c46f..2176ab9feb 100644 --- a/InvenTree/plugin/builtin/integration/core_notifications.py +++ b/InvenTree/plugin/builtin/integration/core_notifications.py @@ -6,6 +6,7 @@ from django.utils.translation import gettext_lazy as _ from allauth.account.models import EmailAddress import common.models +import InvenTree.helpers import InvenTree.tasks from plugin import InvenTreePlugin from plugin.mixins import BulkNotificationMethod, SettingsMixin @@ -61,7 +62,12 @@ class CoreNotificationsPlugin(SettingsMixin, InvenTreePlugin): allowed_users = [] for user in self.targets: - allows_emails = self.usersetting(user) + + if not user.is_active: + # Ignore any users who have been deactivated + continue + + allows_emails = InvenTree.helpers.str2bool(self.usersetting(user)) if allows_emails: allowed_users.append(user)