Skip to content

Commit 5236b9d

Browse files
committed
fix: Fix pluralization issue in the digest emails (no more "1 new notifications")
1 parent cd6c851 commit 5236b9d

File tree

4 files changed

+85
-37
lines changed

4 files changed

+85
-37
lines changed

generic_notifications/channels.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from django.conf import settings
66
from django.core.mail import send_mail
77
from django.db.models import QuerySet
8+
from django.template.defaultfilters import pluralize
89
from django.template.loader import render_to_string
910
from django.utils import timezone
1011

@@ -190,13 +191,15 @@ def send_digest_emails(
190191
html_template = "notifications/email/digest/message.html"
191192
text_template = "notifications/email/digest/message.txt"
192193

194+
notifications_count = notifications.count()
195+
193196
# Load subject
194197
try:
195198
subject = render_to_string(subject_template, context).strip()
196199
except Exception:
197200
# Fallback subject
198201
frequency_name = frequency.name if frequency else "Digest"
199-
subject = f"{frequency_name} - {notifications.count()} new notifications"
202+
subject = f"{frequency_name} - {notifications_count} new notification{pluralize(notifications_count)}"
200203

201204
# Load HTML message
202205
try:
@@ -210,11 +213,11 @@ def send_digest_emails(
210213
text_message = render_to_string(text_template, context)
211214
except Exception:
212215
# Fallback if template doesn't exist
213-
message_lines = [f"You have {notifications.count()} new notifications:"]
216+
message_lines = [f"You have {notifications_count} new notification{pluralize(notifications_count)}:\n"]
214217
for notification in notifications[:10]: # Limit to first 10
215-
message_lines.append(f"- {notification.get_subject()}")
216-
if notifications.count() > 10:
217-
message_lines.append(f"... and {notifications.count() - 10} more")
218+
message_lines.append(f"- {notification.get_text()}")
219+
if notifications_count > 10:
220+
message_lines.append(f"... and {notifications_count - 10} more")
218221
text_message = "\n".join(message_lines)
219222

220223
send_mail(

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "django-generic-notifications"
3-
version = "1.2.0"
3+
version = "1.2.1"
44
description = "A flexible, multi-channel notification system for Django applications with built-in support for email digests, user preferences, and extensible delivery channels."
55
authors = [
66
{name = "Kevin Renskers", email = "[email protected]"},

tests/test_channels.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ def test_send_digest_emails_with_frequency(self):
288288
)
289289

290290
email = mail.outbox[0]
291-
self.assertIn("1 new notifications", email.subject)
291+
self.assertIn("1 new notification", email.subject)
292292

293293
@override_settings(DEFAULT_FROM_EMAIL="[email protected]")
294294
def test_send_digest_emails_without_frequency(self):
@@ -302,7 +302,7 @@ def test_send_digest_emails_without_frequency(self):
302302
)
303303

304304
email = mail.outbox[0]
305-
self.assertIn("Digest - 1 new notifications", email.subject)
305+
self.assertIn("Digest - 1 new notification", email.subject)
306306

307307
@override_settings(DEFAULT_FROM_EMAIL="[email protected]")
308308
def test_send_digest_emails_text_limit(self):

tests/test_management_commands.py

Lines changed: 74 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ def test_send_digest_emails_basic_flow(self):
9898

9999
# Check email details
100100
self.assertEqual(email.to, [self.user1.email])
101-
self.assertIn("1 new notifications", email.subject)
102-
self.assertIn("Test notification", email.body)
101+
self.assertIn("1 new notification", email.subject)
102+
self.assertIn("This is a test notification", email.body)
103103

104104
# Verify notification was marked as sent
105105
notification.refresh_from_db()
@@ -129,21 +129,29 @@ def test_only_includes_unread_notifications(self):
129129

130130
# Create read and unread notifications
131131
read_notification = Notification.objects.create(
132-
recipient=self.user1, notification_type="test_type", subject="Read notification", channels=["email"]
132+
recipient=self.user1,
133+
notification_type="test_type",
134+
subject="Read notification subject",
135+
text="Read notification text",
136+
channels=["email"],
133137
)
134138
read_notification.mark_as_read()
135139

136140
unread_notification = Notification.objects.create(
137-
recipient=self.user1, notification_type="test_type", subject="Unread notification", channels=["email"]
141+
recipient=self.user1,
142+
notification_type="test_type",
143+
subject="Unread notification subject",
144+
text="Unread notification text",
145+
channels=["email"],
138146
)
139147

140148
call_command("send_digest_emails", "--frequency", "daily")
141149

142150
# Should send one email with only unread notification
143151
self.assertEqual(len(mail.outbox), 1)
144152
email = mail.outbox[0]
145-
self.assertIn("Unread notification", email.body)
146-
self.assertNotIn("Read notification", email.body)
153+
self.assertIn("Unread notification text", email.body)
154+
self.assertNotIn("Read notification text", email.body)
147155

148156
# Only unread notification should be marked as sent
149157
read_notification.refresh_from_db()
@@ -164,16 +172,20 @@ def test_only_includes_unsent_notifications(self):
164172
)
165173

166174
unsent_notification = Notification.objects.create(
167-
recipient=self.user1, notification_type="test_type", subject="Unsent notification", channels=["email"]
175+
recipient=self.user1,
176+
notification_type="test_type",
177+
subject="Unsent notification subject",
178+
text="Unsent notification text",
179+
channels=["email"],
168180
)
169181

170182
call_command("send_digest_emails", "--frequency", "daily")
171183

172184
# Should send one email with only unsent notification
173185
self.assertEqual(len(mail.outbox), 1)
174186
email = mail.outbox[0]
175-
self.assertIn("Unsent notification", email.body)
176-
self.assertNotIn("Sent notification", email.body)
187+
self.assertIn("Unsent notification text", email.body)
188+
self.assertNotIn("Sent notification text", email.body)
177189

178190
# Unsent notification should now be marked as sent
179191
unsent_notification.refresh_from_db()
@@ -184,24 +196,32 @@ def test_sends_all_unsent_notifications(self):
184196

185197
# Create notification older than time window (>1 day ago)
186198
old_notification = Notification.objects.create(
187-
recipient=self.user1, notification_type="test_type", subject="Old notification", channels=["email"]
199+
recipient=self.user1,
200+
notification_type="test_type",
201+
subject="Old notification subject",
202+
text="Old notification text",
203+
channels=["email"],
188204
)
189205
# Manually set old timestamp
190206
old_time = timezone.now() - timedelta(days=2)
191207
Notification.objects.filter(id=old_notification.id).update(added=old_time)
192208

193209
# Create recent notification
194210
recent_notification = Notification.objects.create(
195-
recipient=self.user1, notification_type="test_type", subject="Recent notification", channels=["email"]
211+
recipient=self.user1,
212+
notification_type="test_type",
213+
subject="Recent notification subject",
214+
text="Recent notification text",
215+
channels=["email"],
196216
)
197217

198218
call_command("send_digest_emails", "--frequency", "daily")
199219

200220
# Should send one email with both notifications (no time window filtering)
201221
self.assertEqual(len(mail.outbox), 1)
202222
email = mail.outbox[0]
203-
self.assertIn("Recent notification", email.body)
204-
self.assertIn("Old notification", email.body)
223+
self.assertIn("Recent notification text", email.body)
224+
self.assertIn("Old notification text", email.body)
205225
self.assertIn("2 new notifications", email.subject)
206226

207227
# Both notifications should be marked as sent
@@ -218,10 +238,18 @@ def test_specific_frequency_filter(self):
218238

219239
# Create notifications for both
220240
Notification.objects.create(
221-
recipient=self.user1, notification_type="test_type", subject="Daily user notification", channels=["email"]
241+
recipient=self.user1,
242+
notification_type="test_type",
243+
subject="Daily user notification subject",
244+
text="Daily user notification text",
245+
channels=["email"],
222246
)
223247
Notification.objects.create(
224-
recipient=self.user2, notification_type="test_type", subject="Weekly user notification", channels=["email"]
248+
recipient=self.user2,
249+
notification_type="test_type",
250+
subject="Weekly user notification subject",
251+
text="Weekly user notification text",
252+
channels=["email"],
225253
)
226254

227255
call_command("send_digest_emails", "--frequency", "daily")
@@ -230,7 +258,7 @@ def test_specific_frequency_filter(self):
230258
self.assertEqual(len(mail.outbox), 1)
231259
email = mail.outbox[0]
232260
self.assertEqual(email.to, [self.user1.email])
233-
self.assertIn("Daily user notification", email.body)
261+
self.assertIn("Daily user notification text", email.body)
234262

235263
# Clear outbox and test weekly frequency
236264
mail.outbox.clear()
@@ -240,7 +268,7 @@ def test_specific_frequency_filter(self):
240268
self.assertEqual(len(mail.outbox), 1)
241269
email = mail.outbox[0]
242270
self.assertEqual(email.to, [self.user2.email])
243-
self.assertIn("Weekly user notification", email.body)
271+
self.assertIn("Weekly user notification text", email.body)
244272

245273
def test_multiple_notification_types_for_user(self):
246274
# Set up user with multiple notification types for daily frequency
@@ -249,10 +277,18 @@ def test_multiple_notification_types_for_user(self):
249277

250278
# Create notifications of both types
251279
notification1 = Notification.objects.create(
252-
recipient=self.user1, notification_type="test_type", subject="Test type notification", channels=["email"]
280+
recipient=self.user1,
281+
notification_type="test_type",
282+
subject="Test type notification subject",
283+
text="Test type notification text",
284+
channels=["email"],
253285
)
254286
notification2 = Notification.objects.create(
255-
recipient=self.user1, notification_type="other_type", subject="Other type notification", channels=["email"]
287+
recipient=self.user1,
288+
notification_type="other_type",
289+
subject="Other type notification subject",
290+
text="Other type notification text",
291+
channels=["email"],
256292
)
257293

258294
call_command("send_digest_emails", "--frequency", "daily")
@@ -262,8 +298,8 @@ def test_multiple_notification_types_for_user(self):
262298
email = mail.outbox[0]
263299
self.assertEqual(email.to, [self.user1.email])
264300
self.assertIn("2 new notifications", email.subject)
265-
self.assertIn("Test type notification", email.body)
266-
self.assertIn("Other type notification", email.body)
301+
self.assertIn("Test type notification text", email.body)
302+
self.assertIn("Other type notification text", email.body)
267303

268304
# Both notifications should be marked as sent
269305
notification1.refresh_from_db()
@@ -321,8 +357,8 @@ def test_users_with_default_frequencies_get_digest(self):
321357
self.assertEqual(len(mail.outbox), 1)
322358
email = mail.outbox[0]
323359
self.assertEqual(email.to, [self.user1.email])
324-
self.assertIn("Test notification", email.body)
325-
self.assertNotIn("Other notification", email.body)
360+
self.assertIn("This is a test notification", email.body)
361+
self.assertNotIn("This is another type of notification", email.body)
326362

327363
# Verify only test notification was marked as sent
328364
test_notification.refresh_from_db()
@@ -338,10 +374,18 @@ def test_mixed_explicit_and_default_preferences(self):
338374

339375
# Create notifications
340376
Notification.objects.create(
341-
recipient=self.user1, notification_type="test_type", subject="Test notification", channels=["email"]
377+
recipient=self.user1,
378+
notification_type="test_type",
379+
subject="Test notification subject",
380+
text="Test notification text",
381+
channels=["email"],
342382
)
343383
Notification.objects.create(
344-
recipient=self.user1, notification_type="other_type", subject="Other notification", channels=["email"]
384+
recipient=self.user1,
385+
notification_type="other_type",
386+
subject="Other notification subject",
387+
text="Other notification text",
388+
channels=["email"],
345389
)
346390

347391
# Run daily digest - should get nothing (test_type is weekly, other_type is realtime)
@@ -352,8 +396,8 @@ def test_mixed_explicit_and_default_preferences(self):
352396
call_command("send_digest_emails", "--frequency", "weekly")
353397
self.assertEqual(len(mail.outbox), 1)
354398
email = mail.outbox[0]
355-
self.assertIn("Test notification", email.body)
356-
self.assertNotIn("Other notification", email.body)
399+
self.assertIn("Test notification text", email.body)
400+
self.assertNotIn("Other notification text", email.body)
357401

358402
def test_multiple_users_default_and_explicit_mix(self):
359403
"""Test digest emails work correctly with multiple users having different preference configurations."""
@@ -369,7 +413,8 @@ def test_multiple_users_default_and_explicit_mix(self):
369413
Notification.objects.create(
370414
recipient=user,
371415
notification_type="test_type",
372-
subject=f"Test notification for user {i}",
416+
subject=f"Test notification for user {i} subject",
417+
text=f"Test notification for user {i} text",
373418
channels=["email"],
374419
)
375420

@@ -387,4 +432,4 @@ def test_multiple_users_default_and_explicit_mix(self):
387432
call_command("send_digest_emails", "--frequency", "weekly")
388433
self.assertEqual(len(mail.outbox), 1)
389434
self.assertEqual(mail.outbox[0].to[0], self.user2.email)
390-
self.assertIn("Test notification for user 2", mail.outbox[0].body)
435+
self.assertIn("Test notification for user 2 text", mail.outbox[0].body)

0 commit comments

Comments
 (0)