From f08197a69444f4e9fc3be9e048c4adf96c580418 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Thu, 20 Jun 2024 00:52:14 +0530 Subject: [PATCH 1/4] [feat] Implement global notification setting for email/web --- openwisp_notifications/api/urls.py | 5 ++++ openwisp_notifications/api/views.py | 27 ++++++++++++++++++ openwisp_notifications/base/models.py | 2 ++ openwisp_notifications/handlers.py | 28 ++++++++++--------- ..._alter_notificationsetting_organization.py | 25 +++++++++++++++++ .../tests/test_notifications.py | 22 +++++++++++++++ 6 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 openwisp_notifications/migrations/0008_alter_notificationsetting_organization.py diff --git a/openwisp_notifications/api/urls.py b/openwisp_notifications/api/urls.py index 597d2a74..ae1f88ca 100644 --- a/openwisp_notifications/api/urls.py +++ b/openwisp_notifications/api/urls.py @@ -37,4 +37,9 @@ def get_api_urls(api_views=None): views.ignore_object_notification, name='ignore_object_notification', ), + path( + 'preference/', + views.notification_preference, + name='notification_preference', + ), ] diff --git a/openwisp_notifications/api/views.py b/openwisp_notifications/api/views.py index 7320c72e..a8f74759 100644 --- a/openwisp_notifications/api/views.py +++ b/openwisp_notifications/api/views.py @@ -198,6 +198,32 @@ def perform_create(self, serializer): ) +class NotificationPreferenceView(BaseNotificationView): + def post(self, request): + data = request.data + + email = data.get('email') + web = data.get('web') + + if not isinstance(email, bool) or not isinstance(web, bool): + return Response({'error': 'Invalid values for email or web'}, status=400) + + notification_settings, created = NotificationSetting.objects.update_or_create( + user=request.user, + organization=None, + type=None, + defaults={'email': email, 'web': web}, + ) + + return Response( + { + 'success': True, + 'email': notification_settings.email, + 'web': notification_settings.web, + } + ) + + notifications_list = NotificationListView.as_view() notification_detail = NotificationDetailView.as_view() notifications_read_all = NotificationReadAllView.as_view() @@ -206,3 +232,4 @@ def perform_create(self, serializer): notification_setting = NotificationSettingView.as_view() ignore_object_notification_list = IgnoreObjectNotificationListView.as_view() ignore_object_notification = IgnoreObjectNotificationView.as_view() +notification_preference = NotificationPreferenceView.as_view() diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index bbcf3793..2bfb4f5c 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -218,6 +218,8 @@ class AbstractNotificationSetting(UUIDModel): organization = models.ForeignKey( get_model_name('openwisp_users', 'Organization'), on_delete=models.CASCADE, + null=True, + blank=True, ) web = models.BooleanField( _('web notifications'), null=True, blank=True, help_text=_(_RECEIVE_HELP) diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index c14efdf3..c6ed7d96 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -168,20 +168,22 @@ def send_email_notification(sender, instance, created, **kwargs): return # Get email preference of user for this type of notification. target_org = getattr(getattr(instance, 'target', None), 'organization_id', None) + notification_setting = None if instance.type and target_org: - try: - notification_setting = instance.recipient.notificationsetting_set.get( - organization=target_org, type=instance.type - ) - except NotificationSetting.DoesNotExist: - return - email_preference = notification_setting.email_notification - else: - # We can not check email preference if notification type is absent, - # or if target_org is not present - # therefore send email anyway. - email_preference = True - + # Check for specific notification setting for the target organization and type + notification_setting = instance.recipient.notificationsetting_set.filter( + organization=target_org, type=instance.type + ).first() + if not notification_setting: + # Check for global notification setting + notification_setting = instance.recipient.notificationsetting_set.filter( + organization=None, type=None + ).first() + + # Send email anyway if no notification setting + email_preference = ( + notification_setting.email_notification if notification_setting else True + ) email_verified = instance.recipient.emailaddress_set.filter( verified=True, email=instance.recipient.email ).exists() diff --git a/openwisp_notifications/migrations/0008_alter_notificationsetting_organization.py b/openwisp_notifications/migrations/0008_alter_notificationsetting_organization.py new file mode 100644 index 00000000..67f59942 --- /dev/null +++ b/openwisp_notifications/migrations/0008_alter_notificationsetting_organization.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.11 on 2024-06-18 13:05 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_users", "0020_populate_password_updated_field"), + ("openwisp_notifications", "0007_notificationsetting_deleted"), + ] + + operations = [ + migrations.AlterField( + model_name="notificationsetting", + name="organization", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="openwisp_users.organization", + ), + ), + ] diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 04894ffd..f19e48c1 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -728,6 +728,28 @@ def test_notification_type_web_notification_setting_true(self): self._create_notification() self.assertEqual(notification_queryset.count(), 0) + @mock_notification_types + def test_global_email_notification_setting(self): + with self.subTest('Test email global preference is "False"'): + NotificationSetting.objects.update_or_create( + user=self.admin, + organization=None, + type=None, + defaults={'email': False, 'web': False}, + ) + self._create_notification() + self.assertEqual(len(mail.outbox), 0) + + with self.subTest('Test email global preference is "True"'): + NotificationSetting.objects.update_or_create( + user=self.admin, + organization=None, + type=None, + defaults={'email': True, 'web': True}, + ) + self._create_notification() + self.assertEqual(len(mail.outbox), 1) + @mock_notification_types def test_notification_type_web_notification_setting_false(self): target_obj = self._get_org_user() From f4181bec0c0381b777af7983609a28180e417eb0 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Thu, 20 Jun 2024 18:34:57 +0530 Subject: [PATCH 2/4] [fix] QA checks --- ..._alter_notificationsetting_organization.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/openwisp2/sample_notifications/migrations/0003_alter_notificationsetting_organization.py diff --git a/tests/openwisp2/sample_notifications/migrations/0003_alter_notificationsetting_organization.py b/tests/openwisp2/sample_notifications/migrations/0003_alter_notificationsetting_organization.py new file mode 100644 index 00000000..0ca0ff27 --- /dev/null +++ b/tests/openwisp2/sample_notifications/migrations/0003_alter_notificationsetting_organization.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.11 on 2024-06-20 13:02 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_users", "0020_populate_password_updated_field"), + ("sample_notifications", "0002_testapp"), + ] + + operations = [ + migrations.AlterField( + model_name="notificationsetting", + name="organization", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="openwisp_users.organization", + ), + ), + ] From 471ff4f7f6cf539579cd1b0d024cda513d94ed70 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Thu, 20 Jun 2024 18:56:46 +0530 Subject: [PATCH 3/4] [fix] Tests --- openwisp_notifications/handlers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index 08181889..45606b94 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -183,6 +183,9 @@ def send_email_notification(sender, instance, created, **kwargs): organization=None, type=None ).first() + if instance.type and target_org and not notification_setting: + return + # Send email anyway if no notification setting email_preference = ( notification_setting.email_notification if notification_setting else True From ed05f2eeddfc74a9ace221041b7bd4288d3bffa3 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Fri, 21 Jun 2024 23:57:10 +0530 Subject: [PATCH 4/4] [chore] Global Web Notifications handling --- openwisp_notifications/handlers.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index 45606b94..73d54a8e 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -91,8 +91,15 @@ def notify_handler(**kwargs): notificationsetting__organization_id=target_org, notificationsetting__deleted=False, ) - where = where & notification_setting - where_group = where_group & notification_setting + + global_setting = web_notification & Q( + notificationsetting__type=None, + notificationsetting__organization_id=None, + notificationsetting__deleted=False, + ) + + where = where & (notification_setting | global_setting) + where_group = where_group & (notification_setting | global_setting) # Ensure notifications are only sent to active user where = where & Q(is_active=True)