Skip to content

Commit 1df1f87

Browse files
committed
move all email text into file-based templates (closes #110)
1 parent be63927 commit 1df1f87

28 files changed

Lines changed: 628 additions & 277 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Changed
1111

12+
- **All outgoing emails now use file-based Django templates** (closes #110). Email bodies and subjects are stored in `works/templates/email/*.en.txt` — one file per email type (12 templates total). Subject lines are on the first line of each template; a blank line separates them from the body. Every subject now carries a `[OPTIMAP]` prefix and a relevant emoji. Autoescape is disabled for plain-text output so URLs are never HTML-encoded. Future language variants drop in as `*.de.txt` etc. with no code changes required. Missing content assertions for the magic-link, email-change, and account-deletion emails were added as part of this change.
13+
1214
- **Login, logout, and email-change flows now redirect to `/` with a flash message** instead of rendering dedicated single-alert pages. Removed `login_response.html`, `logout.html`, `changeuser.html`, and dead `deleteaccount.html` templates; removed corresponding dead `delete_account` view. Login and email-change messages use `extra_tags="persist"` so they stay visible until manually dismissed.
1315
- **Per-message auto-close TTL for flash alerts.** Pass `extra_tags="persist"` to any `messages.*()` call to suppress auto-close entirely; `error` and `warning` level messages default to 8 s, `info`/`success` to 5 s (previously all server-rendered alerts shared a single 5 s timeout). `OPTIMAP_FLASH` JS alerts for `warning` now also get 8 s to match. The Bootstrap level tag (`alert-danger` etc.) is now derived from `message.level_tag` rather than the combined `message.tags` string, so `extra_tags` values no longer bleed into the CSS class.
1416

CLAUDE.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,61 @@ All deployment-specific config uses `OPTIMAP_*` environment variables loaded fro
341341
- Email confirmation for account changes
342342
- CSRF tokens required - use `localhost` domain during development (not 127.0.0.1)
343343

344+
### Email notifications
345+
346+
All outgoing emails use file-based plain-text templates in `works/templates/email/`. Adding a new email notification follows a fixed pattern:
347+
348+
**1. Create the template** at `works/templates/email/<name>.en.txt`. The **first line is the subject**, a **blank line separates it from the body**. Subjects use `[OPTIMAP]` prefix and an emoji:
349+
350+
```
351+
[OPTIMAP] 🔔 Something happened — {{ title }}
352+
353+
Hello {{ username }},
354+
355+
Here is the detail: {{ detail_url }}
356+
```
357+
358+
Autoescape is disabled (see `works/utils/email.py`), so URLs with `&` render correctly without `&amp;`.
359+
360+
**2. Render the template** using the shared helper:
361+
362+
```python
363+
from works.utils.email import render_email
364+
365+
subject, body = render_email('email/<name>.en.txt', {
366+
'title': work.title,
367+
'detail_url': absolute_url,
368+
})
369+
send_mail(subject, body, settings.EMAIL_HOST_USER, [recipient])
370+
```
371+
372+
For harvest completion/failure emails use `render_harvest_email` from `works.harvesting.common` (same helper, re-exported for convenience).
373+
374+
**3. Queue it** via `django_q.tasks.async_task` for any email that is not a direct user-action response (i.e. everything except magic-link and email-change confirmation). This keeps request latency low and survives SMTP hiccups.
375+
376+
**4. Write a content assertion test.** Every email must have at least one test that checks a key substring in `mail.outbox[0].body` — not just that an email was sent. See `tests/test_auth_emails.py`, `tests/test_work_notifications.py`, and `tests/test_regular_harvesting.py` for examples. Use `@override_settings(EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend")`.
377+
378+
**Complete email inventory** (12 templates, 20 distinct sends):
379+
380+
| Template | Trigger | Sender |
381+
|----------|---------|--------|
382+
| `harvest_success.en.txt` | Harvest completes (OAI, RSS, Crossref, MaRESS, OpenAlex) | `render_harvest_email` in each harvester |
383+
| `harvest_failure.en.txt` | Harvest fails (same 5 harvesters) | same |
384+
| `magic_link.en.txt` | User requests login | `works/views/auth.py::loginres` (synchronous) |
385+
| `email_change_confirm.en.txt` | User requests email change | `works/views/auth.py::change_useremail` |
386+
| `email_changed_notify.en.txt` | Email change confirmed | `works/views/auth.py::confirm_email_change` |
387+
| `account_deletion_confirm.en.txt` | User requests account deletion | `works/views/auth.py::request_delete` |
388+
| `contribution_review.en.txt` | Work contributed — notifies admins/curators | `works/notifications.py` via Django-Q |
389+
| `publication_to_contributor.en.txt` | Work published — notifies contributor | same |
390+
| `curator_change.en.txt` | Curator added/removed from collection | same |
391+
| `new_user_admin.en.txt` | New user confirmed first login | same |
392+
| `monthly_digest.en.txt` | Scheduled monthly digest | `works/tasks.py::send_monthly_email` |
393+
| `subscription_regional.en.txt` | Scheduled regional subscription digest | `works/tasks.py::send_subscription_based_email` |
394+
395+
**Opt-out**: work-event emails (contribution/publish) respect `UserProfile.notify_work_events` (opt-out, default True). Monthly digest respects `UserProfile.notify_new_manuscripts`. Blocked senders are checked via `BlockedEmail`/`BlockedDomain`. All sends are logged to `EmailLog` for audit (harvest emails are the exception).
396+
397+
**Future i18n**: swap `.en.txt` for `.de.txt` etc. and pick the template name based on the user's locale — no other code change needed.
398+
344399
### Testing Notes
345400

346401
- UI tests use Helium/Selenium (set `headless=False` for debugging)

tests/test_account_deletion.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
import django
66
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "optimap.settings")
77
django.setup()
8-
from django.test import TestCase, Client
8+
from django.core import mail
9+
from django.test import TestCase, Client, override_settings
910
from django.contrib.auth import get_user_model
1011
from django.core.cache import cache
1112
from django.urls import reverse
@@ -21,12 +22,25 @@ def setUp(self):
2122
self.delete_token = uuid.uuid4().hex
2223
cache.set(f"user_delete_token_{self.delete_token}", self.user.id, timeout=600)
2324

25+
@override_settings(EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend")
2426
def test_request_delete_account(self):
2527
"""Test that a user can request account deletion"""
28+
mail.outbox = []
2629
response = self.client.post(reverse("optimap:request_delete"))
2730
self.assertEqual(response.status_code, 302)
2831
self.assertIn("message=Check%20your%20email", response.url)
2932

33+
@override_settings(EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend")
34+
def test_deletion_email_contains_confirmation_link(self):
35+
"""Deletion confirmation email includes the token link and timeout."""
36+
mail.outbox = []
37+
self.client.post(reverse("optimap:request_delete"))
38+
self.assertEqual(len(mail.outbox), 1)
39+
email = mail.outbox[0]
40+
self.assertIn("deletion", email.subject.lower())
41+
self.assertIn("/confirm-delete/", email.body)
42+
self.assertIn("10", email.body) # timeout_minutes
43+
3044
def test_confirm_delete_account(self):
3145
"""Test that a user can confirm account deletion"""
3246
response = self.client.get(reverse("optimap:confirm_delete", args=[self.delete_token]))

tests/test_auth_emails.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# SPDX-FileCopyrightText: 2026 OPTIMETA and KOMET projects <https://projects.tib.eu/komet>
2+
# SPDX-License-Identifier: GPL-3.0-or-later
3+
4+
"""Content assertions for auth-flow emails (magic link, email change, account deletion).
5+
6+
These emails had no body-content assertions before the template migration —
7+
only redirect/status checks existed. The tests here ensure that moving the
8+
text to template files doesn't silently break the email content.
9+
"""
10+
11+
from django.contrib.auth import get_user_model
12+
from django.core import mail
13+
from django.core.cache import cache
14+
from django.test import Client, TestCase, override_settings
15+
from django.urls import reverse
16+
17+
User = get_user_model()
18+
19+
EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend"
20+
21+
22+
@override_settings(EMAIL_BACKEND=EMAIL_BACKEND, EMAIL_HOST_USER="noreply@optimap.test")
23+
class MagicLinkEmailContentTests(TestCase):
24+
def setUp(self):
25+
self.client = Client(SERVER_NAME="testserver")
26+
27+
def test_magic_link_email_contains_link_and_validity(self):
28+
"""Magic-link email body contains the token URL and the validity period."""
29+
mail.outbox = []
30+
response = self.client.post(reverse("optimap:login_response"), {"email": "user@example.com"}) # noqa: F841
31+
# The view redirects on success (may render error.html if SMTP fails — we use locmem).
32+
self.assertEqual(len(mail.outbox), 1)
33+
email = mail.outbox[0]
34+
self.assertIn("user@example.com", email.to)
35+
self.assertIn("/login/", email.body) # token URL
36+
self.assertIn("10", email.body) # validity period in minutes
37+
self.assertIn("user@example.com", email.body)
38+
39+
40+
@override_settings(
41+
EMAIL_BACKEND=EMAIL_BACKEND,
42+
EMAIL_HOST_USER="noreply@optimap.test",
43+
BASE_URL="http://testserver",
44+
)
45+
class EmailChangeEmailContentTests(TestCase):
46+
def setUp(self):
47+
self.client = Client()
48+
self.user = User.objects.create_user(
49+
username="old@example.com", email="old@example.com", password="pass"
50+
)
51+
self.client.force_login(self.user)
52+
53+
def test_confirmation_email_contains_old_and_new_address_and_link(self):
54+
"""Email-change confirmation email contains both addresses and the confirm URL."""
55+
mail.outbox = []
56+
self.client.post(
57+
reverse("optimap:changeuser"),
58+
{"form": "email", "email_new": "new@example.com"},
59+
)
60+
# One email sent to the new address.
61+
self.assertEqual(len(mail.outbox), 1)
62+
email = mail.outbox[0]
63+
self.assertEqual(email.to, ["new@example.com"])
64+
self.assertIn("old@example.com", email.body)
65+
self.assertIn("new@example.com", email.body)
66+
self.assertIn("/confirm-email/", email.body)
67+
self.assertIn("10", email.body) # expiry in minutes
68+
69+
def test_notification_email_sent_to_old_address_after_confirmation(self):
70+
"""After confirming an email change, the old address receives a security notice."""
71+
# Key format: EMAIL_CONFIRMATION_TOKEN_PREFIX + "_" + email_new = "email_confirmation__new@..."
72+
cache.set("email_confirmation__new@example.com", {
73+
"token": "testtoken123",
74+
"old_email": "old@example.com",
75+
}, timeout=600)
76+
mail.outbox = []
77+
self.client.get(
78+
reverse("optimap:confirm_email_change", args=["testtoken123", "new@example.com"])
79+
)
80+
# Exactly one email is expected — the security notice to the old address.
81+
self.assertEqual(len(mail.outbox), 1, "Expected one security-notice email")
82+
notify = mail.outbox[0]
83+
self.assertIn("old@example.com", notify.to)
84+
self.assertIn("old@example.com", notify.body)
85+
self.assertIn("new@example.com", notify.body)
86+
self.assertIn("contact", notify.body.lower())

tests/test_new_user_admin_notification.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,15 @@ def test_first_confirmed_login_notifies_all_admins(self):
7575
recipients, ["admin1@optimap.example", "admin2@optimap.example"]
7676
)
7777
subject = mail.outbox[0].subject
78-
self.assertIn("new user registered", subject)
78+
self.assertIn("New user registered", subject)
7979
self.assertIn("brand-new@example.org", subject)
8080
# Body carries the admin user page link.
8181
body = mail.outbox[0].body
8282
self.assertIn("brand-new@example.org", body)
8383
self.assertIn("/admin/works/customuser/", body)
8484

8585
# EmailLog rows recorded.
86-
logs = EmailLog.objects.filter(subject__contains="new user registered")
86+
logs = EmailLog.objects.filter(subject__contains="New user registered")
8787
self.assertEqual(logs.count(), 2)
8888
self.assertTrue(all(log.status == "success" for log in logs))
8989

@@ -100,7 +100,7 @@ def test_existing_user_login_does_not_notify(self):
100100
self.assertEqual(response.status_code, 302)
101101
self.assertEqual(len(mail.outbox), 0)
102102
self.assertFalse(
103-
EmailLog.objects.filter(subject__contains="new user registered").exists()
103+
EmailLog.objects.filter(subject__contains="New user registered").exists()
104104
)
105105

106106
def test_unconfirmed_first_visit_does_not_notify(self):

tests/test_subscription_emails.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ def test_email_sent_for_subscribed_regions(self, mock_email):
125125

126126
# Check subject
127127
subject = call_args[0][0]
128-
self.assertIn("New Publications", subject)
128+
self.assertIn("new publications", subject)
129129

130130
# Check content includes region name and publication
131131
content = call_args[0][1]
@@ -260,7 +260,7 @@ def test_email_shows_correct_publication_count(self, mock_email):
260260

261261
# Check count in subject
262262
subject = mock_email.call_args[0][0]
263-
self.assertIn("2 New Publications", subject)
263+
self.assertIn("2 new publications", subject)
264264

265265
# Check count per region
266266
self.assertIn("Africa (Continent) - 2 work(s)", content)

works/harvesting/common.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,3 +434,13 @@ def send_harvest_email(user, subject, body, fail_silently=False):
434434
)
435435
except Exception as e: # noqa: BLE001 — email failure must not crash the harvest
436436
logger.error("Failed to send harvest email to %s: %s", user.email, e)
437+
438+
439+
def render_harvest_email(template_name, context):
440+
"""Render a harvest email template and split subject from body.
441+
442+
Returns ``(subject, body)``. Delegates to ``works.utils.email.render_email``
443+
so autoescape is off (plain-text output — no HTML entities in URLs, etc.).
444+
"""
445+
from works.utils.email import render_email
446+
return render_email(template_name, context)

works/harvesting/crossref.py

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
_save_or_update_work,
3030
complete_harvest,
3131
fail_harvest,
32+
render_harvest_email,
3233
resolve_user,
3334
send_harvest_email,
3435
)
@@ -358,42 +359,53 @@ def harvest_crossref_prefix(
358359

359360
spatial_count, temporal_count = complete_harvest(event, stats, warning_collector)
360361

361-
send_harvest_email(
362-
user,
363-
f"✅ Crossref Harvesting Completed for {source.name}",
364-
(
365-
f"Crossref harvest details:\n\n"
366-
f"DOI prefix: {resolved_prefix}\n"
367-
f"Container-title filters: "
368-
f"{', '.join(journal_titles) if journal_titles else '<all>'}\n"
369-
f"Records seen: {seen}\n"
370-
f"New works saved: {stats.created}\n"
371-
f"Updated works: {stats.updated}\n"
372-
f"Articles with spatial metadata: {spatial_count}\n"
373-
f"Articles with temporal metadata: {temporal_count}\n"
374-
f"Started: {event.started_at:%Y-%m-%d %H:%M:%S}\n"
375-
f"Completed: {event.completed_at:%Y-%m-%d %H:%M:%S}\n"
376-
f"\n{warning_collector.get_summary()}"
377-
),
378-
)
362+
subject, body = render_harvest_email('email/harvest_success.en.txt', {
363+
'subject_prefix': 'Crossref ',
364+
'source_label': source.name,
365+
'detail_header': 'Crossref harvest details:',
366+
'source_name': source.name,
367+
'source_url': None,
368+
'url_label': None,
369+
'collection_label': None,
370+
'records_added_label': 'New works saved',
371+
'records_added': stats.created,
372+
'records_updated_label': 'Updated works',
373+
'records_updated': stats.updated,
374+
'spatial_label': 'Articles with spatial metadata',
375+
'spatial_count': spatial_count,
376+
'temporal_label': 'Articles with temporal metadata',
377+
'temporal_count': temporal_count,
378+
'event_started': f'{event.started_at:%Y-%m-%d %H:%M:%S}',
379+
'event_completed': f'{event.completed_at:%Y-%m-%d %H:%M:%S}',
380+
'warning_summary': warning_collector.get_summary(),
381+
'resolved_prefix': resolved_prefix,
382+
'container_title_filters': ', '.join(journal_titles) if journal_titles else '<all>',
383+
'openalex_source_id': None,
384+
'records_seen': seen,
385+
'records_processed': None,
386+
})
387+
send_harvest_email(user, subject, body)
379388

380389
except Exception as e:
381390
logger.error(
382391
"Crossref harvesting failed for source %s: %s",
383392
source.url_field, str(e),
384393
)
385394
fail_harvest(event, e, warning_collector)
386-
send_harvest_email(
387-
user,
388-
f"❌ Crossref Harvesting Failed for {source.name}",
389-
(
390-
f"The Crossref harvest failed.\n\n"
391-
f"Source: {source.name}\n"
392-
f"DOI prefix: {resolved_prefix}\n"
393-
f"Error: {e}\n"
394-
),
395-
fail_silently=True,
396-
)
395+
subject, body = render_harvest_email('email/harvest_failure.en.txt', {
396+
'subject_prefix': 'Crossref ',
397+
'source_label': source.name,
398+
'source_type_label': 'Crossref',
399+
'source_name': source.name,
400+
'source_url': None,
401+
'collection_label': None,
402+
'resolved_prefix': resolved_prefix,
403+
'event_started': None,
404+
'event_failed': None,
405+
'error': str(e),
406+
'warning_summary': '',
407+
})
408+
send_harvest_email(user, subject, body, fail_silently=True)
397409
raise
398410
finally:
399411
logger.removeHandler(warning_collector)

0 commit comments

Comments
 (0)