Skip to content

Commit 3464cde

Browse files
committed
feat: Include an absolute link to notification.url in the default emails
1 parent 5236b9d commit 3464cde

File tree

8 files changed

+247
-28
lines changed

8 files changed

+247
-28
lines changed

README.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,31 @@ Run migrations:
3838
uv run ./manage.py migrate generic_notifications
3939
```
4040

41+
## Settings
42+
43+
### `NOTIFICATION_BASE_URL`
44+
45+
Configure the base URL for generating absolute URLs in email notifications:
46+
47+
```python
48+
# With protocol (recommended)
49+
NOTIFICATION_BASE_URL = "https://www.example.com"
50+
NOTIFICATION_BASE_URL = "http://localhost:8000"
51+
52+
# Without protocol (auto-detects based on DEBUG setting)
53+
NOTIFICATION_BASE_URL = "www.example.com"
54+
```
55+
56+
**Protocol handling**: If you omit the protocol, it's automatically added:
57+
- `https://` in production (`DEBUG = False`)
58+
- `http://` in development (`DEBUG = True`)
59+
60+
**Fallback order** if `NOTIFICATION_BASE_URL` is not set:
61+
1. `BASE_URL` setting
62+
2. `SITE_URL` setting
63+
3. Django Sites framework (if `django.contrib.sites` is installed)
64+
4. URLs remain relative if no base URL is found (not ideal in emails!)
65+
4166
## Quick Start
4267

4368
### 1. Define a notification type

example/config/settings.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727

2828
ALLOWED_HOSTS = ["*"]
2929

30+
# Base URL for generating absolute URLs in notifications
31+
# In production, set this to your domain: NOTIFICATION_BASE_URL = "https://yourdomain.com"
32+
NOTIFICATION_BASE_URL = "http://localhost:8000"
33+
3034

3135
# Application definition
3236

generic_notifications/channels.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,11 @@ def send_email_now(self, notification: "Notification") -> None:
135135
try:
136136
text_message = render_to_string(text_template, context)
137137
except Exception:
138-
# Fallback to notification's text
138+
# Fallback to notification's text with URL if available
139139
text_message = notification.get_text()
140+
absolute_url = notification.get_absolute_url()
141+
if absolute_url:
142+
text_message += f"\n{absolute_url}"
140143

141144
send_mail(
142145
subject=subject,
@@ -216,6 +219,9 @@ def send_digest_emails(
216219
message_lines = [f"You have {notifications_count} new notification{pluralize(notifications_count)}:\n"]
217220
for notification in notifications[:10]: # Limit to first 10
218221
message_lines.append(f"- {notification.get_text()}")
222+
absolute_url = notification.get_absolute_url()
223+
if absolute_url:
224+
message_lines.append(f" {absolute_url}")
219225
if notifications_count > 10:
220226
message_lines.append(f"... and {notifications_count - 10} more")
221227
text_message = "\n".join(message_lines)

generic_notifications/models.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from django.conf import settings
12
from django.contrib.auth import get_user_model
23
from django.contrib.contenttypes.fields import GenericForeignKey
34
from django.contrib.contenttypes.models import ContentType
@@ -226,3 +227,48 @@ def get_text(self) -> str:
226227
@property
227228
def is_read(self) -> bool:
228229
return self.read is not None
230+
231+
def get_absolute_url(self) -> str:
232+
"""
233+
Get the absolute URL for this notification.
234+
If the URL is already absolute (starts with http:// or https://), return as-is.
235+
Otherwise, prepend the base URL from settings if available.
236+
"""
237+
if not self.url:
238+
return ""
239+
240+
# If already absolute, return as-is
241+
if self.url.startswith(("http://", "https://")):
242+
return self.url
243+
244+
# Get base URL from settings, with fallback
245+
base_url = getattr(settings, "NOTIFICATION_BASE_URL", "")
246+
247+
if not base_url:
248+
# Try common alternatives
249+
base_url = getattr(settings, "BASE_URL", "")
250+
if not base_url:
251+
base_url = getattr(settings, "SITE_URL", "")
252+
253+
if not base_url and "django.contrib.sites" in settings.INSTALLED_APPS:
254+
# Try the Sites framework
255+
from django.contrib.sites.models import Site
256+
257+
try:
258+
base_url = Site.objects.get_current().domain
259+
except Site.DoesNotExist:
260+
pass
261+
262+
if base_url:
263+
# Add protocol if missing
264+
if not base_url.startswith(("http://", "https://")):
265+
protocol = "http" if settings.DEBUG else "https"
266+
base_url = f"{protocol}://{base_url}"
267+
268+
# Ensure base URL doesn't end with slash and relative URL doesn't start with slash
269+
base_url = base_url.rstrip("/")
270+
relative_url = self.url.lstrip("/")
271+
return f"{base_url}/{relative_url}"
272+
273+
# No base URL configured, return relative URL
274+
return self.url

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.1"
3+
version = "1.3.0"
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 = "kevin@loopwerk.io"},

tests/test_channels.py

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ def test_send_digest_emails_basic(self):
269269
self.assertEqual(len(mail.outbox), 1)
270270
email = mail.outbox[0]
271271
self.assertEqual(email.to, [self.user.email])
272-
self.assertIn("3 new notifications", email.subject)
272+
self.assertEqual(email.subject, "Digest - 3 new notifications")
273273

274274
# Check all notifications marked as sent
275275
for notification in notifications:
@@ -288,7 +288,7 @@ def test_send_digest_emails_with_frequency(self):
288288
)
289289

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

293293
@override_settings(DEFAULT_FROM_EMAIL="test@example.com")
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 notification", email.subject)
305+
self.assertEqual(email.subject, "Digest - 1 new notification")
306306

307307
@override_settings(DEFAULT_FROM_EMAIL="test@example.com")
308308
def test_send_digest_emails_text_limit(self):
@@ -322,7 +322,7 @@ def test_send_digest_emails_text_limit(self):
322322
# The implementation may not have this feature, so we'll just check that email was sent
323323
self.assertEqual(len(mail.outbox), 1)
324324
email = mail.outbox[0]
325-
self.assertIn("15 new notifications", email.subject)
325+
self.assertEqual(email.subject, "Digest - 15 new notifications")
326326

327327
@override_settings(DEFAULT_FROM_EMAIL="test@example.com")
328328
@patch("generic_notifications.channels.render_to_string")
@@ -354,3 +354,59 @@ def test_send_digest_emails_with_html_template(self, mock_render):
354354
self.assertEqual(text_call[0][0], "notifications/email/digest/message.txt")
355355
self.assertEqual(html_call[0][1]["user"], self.user)
356356
self.assertEqual(html_call[0][1]["count"], 1)
357+
358+
def test_send_email_now_fallback_includes_url(self):
359+
"""Test that fallback email content includes URL when available"""
360+
notification = Notification.objects.create(
361+
recipient=self.user,
362+
notification_type=TestNotificationType.key,
363+
channels=["email"],
364+
subject="Test Subject",
365+
text="Test notification text",
366+
url="https://example.com/test/url/123",
367+
)
368+
369+
EmailChannel().send_email_now(notification)
370+
371+
# Check that one email was sent
372+
self.assertEqual(len(mail.outbox), 1)
373+
sent_email = mail.outbox[0]
374+
375+
# Check the exact email body content
376+
expected_body = "Test notification text\nhttps://example.com/test/url/123"
377+
self.assertEqual(sent_email.body, expected_body)
378+
379+
def test_send_digest_emails_fallback_includes_urls(self):
380+
"""Test that digest fallback email content includes URLs when available"""
381+
# Create notifications with URLs
382+
Notification.objects.create(
383+
recipient=self.user,
384+
notification_type=TestNotificationType.key,
385+
channels=["email"],
386+
text="First notification",
387+
url="https://example.com/url/1",
388+
)
389+
Notification.objects.create(
390+
recipient=self.user,
391+
notification_type=TestNotificationType.key,
392+
channels=["email"],
393+
text="Second notification",
394+
url="https://example.com/url/2",
395+
)
396+
397+
EmailChannel.send_digest_emails(
398+
self.user, Notification.objects.filter(recipient=self.user, email_sent_at__isnull=True)
399+
)
400+
401+
# Check that one email was sent
402+
self.assertEqual(len(mail.outbox), 1)
403+
sent_email = mail.outbox[0]
404+
405+
# Check the exact digest email body content
406+
expected_body = """You have 2 new notifications:
407+
408+
- Second notification
409+
https://example.com/url/2
410+
- First notification
411+
https://example.com/url/1"""
412+
self.assertEqual(sent_email.body, expected_body)

tests/test_management_commands.py

Lines changed: 18 additions & 22 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 notification", email.subject)
102-
self.assertIn("This is a test notification", email.body)
101+
self.assertEqual(email.subject, "Daily digest - 1 new notification")
102+
self.assertEqual(email.body, "You have 1 new notification:\n\n- This is a test notification")
103103

104104
# Verify notification was marked as sent
105105
notification.refresh_from_db()
@@ -150,8 +150,7 @@ def test_only_includes_unread_notifications(self):
150150
# Should send one email with only unread notification
151151
self.assertEqual(len(mail.outbox), 1)
152152
email = mail.outbox[0]
153-
self.assertIn("Unread notification text", email.body)
154-
self.assertNotIn("Read notification text", email.body)
153+
self.assertEqual(email.body, "You have 1 new notification:\n\n- Unread notification text")
155154

156155
# Only unread notification should be marked as sent
157156
read_notification.refresh_from_db()
@@ -184,8 +183,7 @@ def test_only_includes_unsent_notifications(self):
184183
# Should send one email with only unsent notification
185184
self.assertEqual(len(mail.outbox), 1)
186185
email = mail.outbox[0]
187-
self.assertIn("Unsent notification text", email.body)
188-
self.assertNotIn("Sent notification text", email.body)
186+
self.assertEqual(email.body, "You have 1 new notification:\n\n- Unsent notification text")
189187

190188
# Unsent notification should now be marked as sent
191189
unsent_notification.refresh_from_db()
@@ -220,9 +218,10 @@ def test_sends_all_unsent_notifications(self):
220218
# Should send one email with both notifications (no time window filtering)
221219
self.assertEqual(len(mail.outbox), 1)
222220
email = mail.outbox[0]
223-
self.assertIn("Recent notification text", email.body)
224-
self.assertIn("Old notification text", email.body)
225-
self.assertIn("2 new notifications", email.subject)
221+
self.assertEqual(email.subject, "Daily digest - 2 new notifications")
222+
self.assertEqual(
223+
email.body, "You have 2 new notifications:\n\n- Recent notification text\n- Old notification text"
224+
)
226225

227226
# Both notifications should be marked as sent
228227
old_notification.refresh_from_db()
@@ -258,7 +257,7 @@ def test_specific_frequency_filter(self):
258257
self.assertEqual(len(mail.outbox), 1)
259258
email = mail.outbox[0]
260259
self.assertEqual(email.to, [self.user1.email])
261-
self.assertIn("Daily user notification text", email.body)
260+
self.assertEqual(email.body, "You have 1 new notification:\n\n- Daily user notification text")
262261

263262
# Clear outbox and test weekly frequency
264263
mail.outbox.clear()
@@ -268,7 +267,7 @@ def test_specific_frequency_filter(self):
268267
self.assertEqual(len(mail.outbox), 1)
269268
email = mail.outbox[0]
270269
self.assertEqual(email.to, [self.user2.email])
271-
self.assertIn("Weekly user notification text", email.body)
270+
self.assertEqual(email.body, "You have 1 new notification:\n\n- Weekly user notification text")
272271

273272
def test_multiple_notification_types_for_user(self):
274273
# Set up user with multiple notification types for daily frequency
@@ -297,9 +296,10 @@ def test_multiple_notification_types_for_user(self):
297296
self.assertEqual(len(mail.outbox), 1)
298297
email = mail.outbox[0]
299298
self.assertEqual(email.to, [self.user1.email])
300-
self.assertIn("2 new notifications", email.subject)
301-
self.assertIn("Test type notification text", email.body)
302-
self.assertIn("Other type notification text", email.body)
299+
self.assertEqual(email.subject, "Daily digest - 2 new notifications")
300+
self.assertEqual(
301+
email.body, "You have 2 new notifications:\n\n- Other type notification text\n- Test type notification text"
302+
)
303303

304304
# Both notifications should be marked as sent
305305
notification1.refresh_from_db()
@@ -357,8 +357,7 @@ def test_users_with_default_frequencies_get_digest(self):
357357
self.assertEqual(len(mail.outbox), 1)
358358
email = mail.outbox[0]
359359
self.assertEqual(email.to, [self.user1.email])
360-
self.assertIn("This is a test notification", email.body)
361-
self.assertNotIn("This is another type of notification", email.body)
360+
self.assertEqual(email.body, "You have 1 new notification:\n\n- This is a test notification")
362361

363362
# Verify only test notification was marked as sent
364363
test_notification.refresh_from_db()
@@ -396,8 +395,7 @@ def test_mixed_explicit_and_default_preferences(self):
396395
call_command("send_digest_emails", "--frequency", "weekly")
397396
self.assertEqual(len(mail.outbox), 1)
398397
email = mail.outbox[0]
399-
self.assertIn("Test notification text", email.body)
400-
self.assertNotIn("Other notification text", email.body)
398+
self.assertEqual(email.body, "You have 1 new notification:\n\n- Test notification text")
401399

402400
def test_multiple_users_default_and_explicit_mix(self):
403401
"""Test digest emails work correctly with multiple users having different preference configurations."""
@@ -423,13 +421,11 @@ def test_multiple_users_default_and_explicit_mix(self):
423421
self.assertEqual(len(mail.outbox), 2)
424422

425423
recipients = {email.to[0] for email in mail.outbox}
426-
self.assertIn(self.user1.email, recipients)
427-
self.assertIn(self.user3.email, recipients)
428-
self.assertNotIn(self.user2.email, recipients)
424+
self.assertEqual(set(recipients), {self.user1.email, self.user3.email})
429425

430426
# Clear outbox and run weekly digest - should get user2
431427
mail.outbox.clear()
432428
call_command("send_digest_emails", "--frequency", "weekly")
433429
self.assertEqual(len(mail.outbox), 1)
434430
self.assertEqual(mail.outbox[0].to[0], self.user2.email)
435-
self.assertIn("Test notification for user 2 text", mail.outbox[0].body)
431+
self.assertEqual(mail.outbox[0].body, "You have 1 new notification:\n\n- Test notification for user 2 text")

0 commit comments

Comments
 (0)