Skip to content

Commit b88327f

Browse files
committed
[tests] Improved coverage
1 parent 941982d commit b88327f

File tree

6 files changed

+172
-66
lines changed

6 files changed

+172
-66
lines changed

openwisp_notifications/migrations/0008_alter_notificationsetting_organization_and_more.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import django.db.models.deletion
44
from django.db import migrations, models
55

6+
from openwisp_notifications.types import NOTIFICATION_CHOICES
7+
68

79
class Migration(migrations.Migration):
810
dependencies = [
@@ -26,10 +28,7 @@ class Migration(migrations.Migration):
2628
name="type",
2729
field=models.CharField(
2830
blank=True,
29-
choices=[
30-
("default", "Default Type"),
31-
("generic_message", "Generic Message Type"),
32-
],
31+
choices=NOTIFICATION_CHOICES,
3332
max_length=30,
3433
null=True,
3534
verbose_name="Notification Type",

openwisp_notifications/tasks.py

+10-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from datetime import timedelta
23

34
from celery import shared_task
@@ -22,6 +23,8 @@
2223
from openwisp_utils.admin_theme.email import send_email
2324
from openwisp_utils.tasks import OpenwispCeleryTask
2425

26+
logger = logging.getLogger(__name__)
27+
2528
User = get_user_model()
2629

2730
Notification = load_model('Notification')
@@ -91,15 +94,12 @@ def create_notification_settings(user, organizations, notification_types):
9194
)
9295

9396
for type in notification_types:
94-
notification_config = types.get_notification_configuration(type)
9597
for org in organizations:
9698
NotificationSetting.objects.update_or_create(
9799
defaults={
98100
'deleted': False,
99-
'email': global_setting.email
100-
and notification_config.get('email_notification'),
101-
'web': global_setting.web
102-
and notification_config.get('web_notification'),
101+
'email': None if global_setting.email else False,
102+
'web': None if global_setting.email else False,
103103
},
104104
user=user,
105105
type=type,
@@ -233,7 +233,11 @@ def send_batched_email_notifications(instance_id):
233233
"""
234234
Sends a summary of notifications to the specified email address.
235235
"""
236-
if not instance_id:
236+
if not User.objects.filter(id=instance_id).exists():
237+
logger.error(
238+
'Failed to send batched email notifications:'
239+
f' User with ID {instance_id} not found in the database.'
240+
)
237241
return
238242

239243
cache_key = f'email_batch_{instance_id}'

openwisp_notifications/templates/openwisp_notifications/emails/batch_email.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ <h2>
106106
{% if notification.url %}
107107
<a href="{{ notification.url }}" target="_blank"><p>{{ notification.email_message|striptags }}</p></a>
108108
{% else %}
109-
{{ notification.email_message }}
109+
<p>{{ notification.email_message }}</p>
110110
{% endif %}
111111
</span>
112112
</h2>
+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
_test_batch_email_notification_email_body = """
2+
[example.com] 4 new notifications since {datetime_str}
3+
4+
5+
- Default notification with default verb and level info by Tester Tester (test org)
6+
Description: Test Notification
7+
Date & Time: {datetime_str}
8+
URL: https://example.com/api/v1/notifications/notification/{notification_id}/redirect/
9+
10+
- Test Notification
11+
Description: Test Notification
12+
Date & Time: {datetime_str}
13+
14+
- Test Notification
15+
Description: Test Notification
16+
Date & Time: {datetime_str}
17+
URL: https://localhost:8000/admin
18+
19+
- Test Notification
20+
Description: Test Notification
21+
Date & Time: {datetime_str}
22+
URL: https://localhost:8000/admin
23+
"""
24+
25+
_test_batch_email_notification_email_html = """
26+
<div class="subject">[example.com] 4 new notifications since {datetime_str}</div>
27+
<div>
28+
<div class="alert info">
29+
<h2><span class="badge info">INFO</span><span class="title"><a
30+
href="https://example.com/api/v1/notifications/notification/{notification_id}/redirect/"
31+
target="_blank">
32+
<p>Default notification with default verb and level info by Tester Tester (test org)</p>
33+
</a></span></h2>
34+
<p>{datetime_str}</p>
35+
<p>
36+
<p>Test Notification</p>
37+
</p>
38+
</div>
39+
<div class="alert info">
40+
<h2><span class="badge info">INFO</span><span class="title">
41+
<p>Test Notification</p>
42+
</span></h2>
43+
<p>{datetime_str}</p>
44+
<p>
45+
<p>Test Notification</p>
46+
</p>
47+
</div>
48+
<div class="alert info">
49+
<h2><span class="badge info">INFO</span><span class="title"><a href="https://localhost:8000/admin"
50+
target="_blank">
51+
<p>Test Notification</p>
52+
</a></span></h2>
53+
<p>{datetime_str}</p>
54+
<p>
55+
<p>Test Notification</p>
56+
</p>
57+
</div>
58+
<div class="alert info">
59+
<h2><span class="badge info">INFO</span><span class="title"><a href="https://localhost:8000/admin"
60+
target="_blank">
61+
<p>Test Notification</p>
62+
</a></span></h2>
63+
<p>{datetime_str}</p>
64+
<p>
65+
<p>Test Notification</p>
66+
</p>
67+
</div>
68+
</div>
69+
"""

openwisp_notifications/tests/test_notifications.py

+86-50
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from django.test import TransactionTestCase
1717
from django.urls import reverse
1818
from django.utils import timezone
19+
from django.utils.html import strip_spaces_between_tags
1920
from django.utils.timesince import timesince
2021
from freezegun import freeze_time
2122

@@ -42,6 +43,11 @@
4243
from openwisp_users.tests.utils import TestOrganizationMixin
4344
from openwisp_utils.tests import capture_any_output
4445

46+
from . import (
47+
_test_batch_email_notification_email_body,
48+
_test_batch_email_notification_email_html,
49+
)
50+
4551
User = get_user_model()
4652

4753
Notification = load_model('Notification')
@@ -136,10 +142,16 @@ def test_superuser_notifications_disabled(self):
136142
organization_id=target_obj.organization.pk,
137143
type='default',
138144
)
139-
self.assertTrue(notification_preference.email)
145+
# Database field is set to None
146+
self.assertEqual(notification_preference.email, None)
147+
# The fallback is taked from notification type
148+
self.assertTrue(notification_preference.email_notification)
140149
notification_preference.web = False
150+
notification_preference.full_clean()
141151
notification_preference.save()
142152
notification_preference.refresh_from_db()
153+
# The database field has been updated to override the default
154+
# value in notification type.
143155
self.assertEqual(notification_preference.email, False)
144156
self._create_notification()
145157
self.assertEqual(notification_queryset.count(), 0)
@@ -953,77 +965,101 @@ def test_notification_for_unverified_email(self):
953965
# we don't send emails to unverified email addresses
954966
self.assertEqual(len(mail.outbox), 0)
955967

956-
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
957-
def test_send_batched_email_notifications_no_instance_id(self, mock_send_email):
958-
# Ensure no emails are sent if instance_id is None or empty
968+
@patch('logging.Logger.error')
969+
def test_send_batched_email_notifications_no_instance_id(self, mocked_logger):
970+
# Ensure no emails are sent if instance_id is invalid
959971
tasks.send_batched_email_notifications(None)
960972
self.assertEqual(len(mail.outbox), 0)
973+
mocked_logger.assert_called_once_with(
974+
'Failed to send batched email notifications:'
975+
f' User with ID {None} not found in the database.'
976+
)
977+
mocked_logger.reset_mock()
961978

962-
tasks.send_batched_email_notifications("")
979+
invalid_id = uuid4()
980+
tasks.send_batched_email_notifications(str(invalid_id))
963981
self.assertEqual(len(mail.outbox), 0)
982+
mocked_logger.assert_called_once_with(
983+
'Failed to send batched email notifications:'
984+
f' User with ID {invalid_id} not found in the database.'
985+
)
964986

965987
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
966988
def test_send_batched_email_notifications_single_notification(
967989
self, mock_send_email
968990
):
969-
# Ensure no batch email template is used for a single batched notification
991+
# The first notification will always be sent immediately
970992
self._create_notification()
971-
n = self._create_notification().pop()[1][0]
972-
973-
tasks.send_batched_email_notifications(self.admin.id)
974-
975-
self.assertEqual(len(mail.outbox), 2)
976-
self.assertIn(n.message, mail.outbox[0].body)
993+
mail.outbox.clear()
994+
# There's only one notification in the batch, it should
995+
# not use summary of batched email.
996+
self._create_notification()
997+
# The task is mocked to prevent immediate execution in test environment.
998+
# In tests, Celery runs in EAGER mode which would execute tasks immediately,
999+
# preventing us from testing the batching behavior properly.
1000+
tasks.send_batched_email_notifications(str(self.admin.id))
1001+
self.assertEqual(len(mail.outbox), 1)
1002+
self.assertNotIn('new notifications since', mail.outbox[0].body)
9771003

978-
# @override_settings(TIME_ZONE='UTC')
9791004
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
9801005
def test_batch_email_notification(self, mock_send_email):
9811006
fixed_datetime = timezone.localtime(
9821007
datetime(2024, 7, 26, 11, 40, tzinfo=timezone.utc)
9831008
)
9841009
datetime_str = fixed_datetime.strftime('%B %-d, %Y, %-I:%M %p %Z')
9851010

1011+
# Add multiple notifications with slightly different timestamps
1012+
# to maintain consistent order in generated email text across test runs
1013+
for _ in range(3):
1014+
with freeze_time(fixed_datetime):
1015+
# Create notifications with URL (from self.notification_options)
1016+
self._create_notification()
1017+
fixed_datetime += timedelta(
1018+
microseconds=100
1019+
) # Increment time for ordering consistency
1020+
# Notification without URL
1021+
self.notification_options.pop('url')
9861022
with freeze_time(fixed_datetime):
987-
for _ in range(5):
988-
notify.send(recipient=self.admin, **self.notification_options)
989-
990-
# Check if only one mail is sent initially
991-
self.assertEqual(len(mail.outbox), 1)
992-
993-
# Call the task
994-
tasks.send_batched_email_notifications(self.admin.id)
995-
996-
# Check if the rest of the notifications are sent in a batch
997-
self.assertEqual(len(mail.outbox), 2)
998-
999-
expected_subject = f'[example.com] 4 new notifications since {datetime_str}'
1000-
expected_body = f"""
1001-
[example.com] 4 new notifications since {datetime_str}
1002-
1003-
1004-
- Test Notification
1005-
Description: Test Notification
1006-
Date & Time: {datetime_str}
1007-
URL: https://localhost:8000/admin
1008-
1009-
- Test Notification
1010-
Description: Test Notification
1011-
Date & Time: {datetime_str}
1012-
URL: https://localhost:8000/admin
1023+
self._create_notification()
1024+
fixed_datetime += timedelta(microseconds=100)
1025+
# Notification with a type and target object
1026+
self.notification_options.update(
1027+
{'type': 'default', 'target': self._get_org_user()}
1028+
)
1029+
with freeze_time(fixed_datetime):
1030+
default = self._create_notification().pop()[1][0]
10131031

1014-
- Test Notification
1015-
Description: Test Notification
1016-
Date & Time: {datetime_str}
1017-
URL: https://localhost:8000/admin
1032+
# Check if only one mail is sent initially
1033+
self.assertEqual(len(mail.outbox), 1)
10181034

1019-
- Test Notification
1020-
Description: Test Notification
1021-
Date & Time: {datetime_str}
1022-
URL: https://localhost:8000/admin
1023-
"""
1035+
# Call the task
1036+
tasks.send_batched_email_notifications(self.admin.id)
10241037

1025-
self.assertEqual(mail.outbox[1].subject, expected_subject)
1026-
self.assertEqual(mail.outbox[1].body.strip(), expected_body.strip())
1038+
# Check if the rest of the notifications are sent in a batch
1039+
self.assertEqual(len(mail.outbox), 2)
1040+
email = mail.outbox[1]
1041+
expected_subject = f'[example.com] 4 new notifications since {datetime_str}'
1042+
self.assertEqual(email.subject, expected_subject)
1043+
self.assertEqual(
1044+
email.body.strip(),
1045+
_test_batch_email_notification_email_body.format(
1046+
datetime_str=datetime_str,
1047+
notification_id=default.id,
1048+
).strip(),
1049+
)
1050+
html_email = email.alternatives[0][0]
1051+
self.maxDiff = None
1052+
self.assertIn(
1053+
strip_spaces_between_tags(
1054+
_test_batch_email_notification_email_html.format(
1055+
datetime_str=datetime_str,
1056+
notification_id=default.id,
1057+
)
1058+
.replace('\n', '')
1059+
.replace(' ', ' ')
1060+
),
1061+
strip_spaces_between_tags(html_email),
1062+
)
10271063

10281064
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
10291065
def test_batch_email_notification_with_call_to_action(self, mock_send_email):

tests/openwisp2/sample_notifications/migrations/0003_alter_notificationsetting_organization_and_more.py

+3-5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import django.db.models.deletion
44
from django.db import migrations, models
55

6+
from openwisp_notifications.types import NOTIFICATION_CHOICES
7+
68

79
class Migration(migrations.Migration):
810
dependencies = [
@@ -26,11 +28,7 @@ class Migration(migrations.Migration):
2628
name="type",
2729
field=models.CharField(
2830
blank=True,
29-
choices=[
30-
("default", "Default Type"),
31-
("generic_message", "Generic Message Type"),
32-
("object_created", "Object created"),
33-
],
31+
choices=NOTIFICATION_CHOICES,
3432
max_length=30,
3533
null=True,
3634
verbose_name="Notification Type",

0 commit comments

Comments
 (0)