Thread access request notification emails

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#24](https://github.com/BIPLAT-CIBERINFEC/pathocore-api/pull/24) Prefix PathoCore API host settings
 - [#27](https://github.com/BIPLAT-CIBERINFEC/pathocore-api/pull/27) Move API documentation under `/v1`
 - [#28](https://github.com/BIPLAT-CIBERINFEC/pathocore-api/pull/28) Keep API documentation public
+- [#30](https://github.com/BIPLAT-CIBERINFEC/pathocore-api/pull/30) Thread access request notification emails
 
 ### `Added`
 

core/api/services/access_requests.py

@@ -1,6 +1,8 @@
+from email.utils import parseaddr
+
 from django.conf import settings
 from django.contrib.auth import get_user_model
-from django.core.mail import send_mail
+from django.core.mail import EmailMessage
 from django.db import transaction
 from django.db.models import Q
 from django.utils import timezone
@@ -327,8 +329,8 @@ def build_group_path_from_scope(scope):
 
 def notify_access_request_created(access_request):
     requested_access = _requested_access_label(access_request)
-    send_mail(
-        subject="PathoCore access request received",
+    _send_access_request_email(
+        access_request,
         message=_with_email_footer(
             f"Hello {access_request.first_name},\n\n"
             "We have received your PathoCore access request and it is pending "
@@ -337,24 +339,23 @@ def notify_access_request_created(access_request):
             "You will receive another notification once the request has been "
             "reviewed.\n"
         ),
-        from_email=getattr(settings, "DEFAULT_FROM_EMAIL", None),
         recipient_list=[access_request.email],
-        fail_silently=True,
+        message_key="received",
     )
 
     recipients = _access_request_admin_recipients(access_request)
     if not recipients:
         return
-    send_mail(
-        subject=f"PathoCore access request pending: {access_request.username}",
+    _send_access_request_email(
+        access_request,
         message=_with_email_footer(
             f"User: {access_request.username} <{access_request.email}>\n"
             f"Requested: {requested_access}\n"
             f"Message: {access_request.message or '-'}"
         ),
-        from_email=getattr(settings, "DEFAULT_FROM_EMAIL", None),
         recipient_list=recipients,
-        fail_silently=True,
+        message_key="admin-pending",
+        reply_to_thread=True,
     )
 
 
@@ -385,9 +386,9 @@ def notify_access_request_reviewed(access_request):
     requested_access = _requested_access_label(access_request)
     section_url = _use_case_section_url(access_request)
     section_line = (
-        f"Web section: {section_url}\n"
+        f"Web section: {section_url}\n\n"
         if section_url
-        else f"Web section: {access_request.requested_use_case}\n"
+        else f"Web section: {access_request.requested_use_case}\n\n"
     )
 
     if access_request.status == core.models.AccessRequest.STATUS_APPROVED:
@@ -420,31 +421,92 @@ def notify_access_request_reviewed(access_request):
             f"Review note: {access_request.review_note or '-'}\n"
         )
 
-    send_mail(
-        subject=f"PathoCore access request {access_request.status}",
+    _send_access_request_email(
+        access_request,
         message=_with_email_footer(message),
-        from_email=getattr(settings, "DEFAULT_FROM_EMAIL", None),
         recipient_list=[access_request.email],
-        fail_silently=True,
+        message_key=access_request.status,
+        reply_to_thread=True,
     )
 
 
 def notify_access_request_revoked(access_request):
     revoked_access = _requested_access_label(access_request)
-    send_mail(
-        subject="PathoCore access revoked",
+    _send_access_request_email(
+        access_request,
         message=_with_email_footer(
             "Your PathoCore access has been revoked.\n\n"
             f"Revoked access: {revoked_access}\n"
             f"Reason: {access_request.review_note or '-'}\n"
             f"{_admin_contact_line(access_request)}"
         ),
-        from_email=getattr(settings, "DEFAULT_FROM_EMAIL", None),
         recipient_list=[access_request.email],
-        fail_silently=True,
+        message_key="revoked",
+        reply_to_thread=True,
     )
 
 
+def _send_access_request_email(
+    access_request,
+    *,
+    message,
+    recipient_list,
+    message_key,
+    reply_to_thread=False,
+):
+    email = EmailMessage(
+        subject=_access_request_email_subject(access_request),
+        body=message,
+        from_email=getattr(settings, "DEFAULT_FROM_EMAIL", None),
+        to=recipient_list,
+        headers=_access_request_email_headers(
+            access_request,
+            message_key,
+            reply_to_thread=reply_to_thread,
+        ),
+    )
+    email.send(fail_silently=True)
+
+
+def _access_request_email_subject(access_request):
+    requested_access = _requested_access_label(access_request)
+    return (
+        f"[PathoCore access #{access_request.pk}] "
+        f"{requested_access} - {access_request.username}"
+    )
+
+
+def _access_request_email_headers(
+    access_request,
+    message_key,
+    *,
+    reply_to_thread=False,
+):
+    root_message_id = _access_request_message_id(access_request, "received")
+    headers = {
+        "Message-ID": _access_request_message_id(access_request, message_key),
+    }
+    if reply_to_thread:
+        headers["In-Reply-To"] = root_message_id
+        headers["References"] = root_message_id
+    return headers
+
+
+def _access_request_message_id(access_request, message_key):
+    request_id = access_request.pk or "new"
+    return (
+        f"<pathocore-access-request-{request_id}-{message_key}"
+        f"@{_message_id_domain()}>"
+    )
+
+
+def _message_id_domain():
+    _, address = parseaddr(getattr(settings, "DEFAULT_FROM_EMAIL", "") or "")
+    if "@" not in address:
+        return "pathocore.local"
+    return address.rsplit("@", 1)[-1].lower()
+
+
 def _with_email_footer(message):
     return f"{message.rstrip()}{EMAIL_FOOTER}"
 

core/tests.py

@@ -1523,7 +1523,14 @@ def test_public_user_can_create_pending_access_request(self):
         self.assertEqual(models.AccessRequest.objects.count(), 1)
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(mail.outbox[0].to, ["new.user@example.org"])
-        self.assertIn("received", mail.outbox[0].subject.lower())
+        self.assertEqual(
+            mail.outbox[0].subject,
+            f"[PathoCore access #{response.data['id']}] MEPRAM (view) - new_user",
+        )
+        self.assertEqual(
+            mail.outbox[0].extra_headers["Message-ID"],
+            (f"<pathocore-access-request-{response.data['id']}-received" "@localhost>"),
+        )
 
     def test_legacy_v1_alias_still_accepts_access_request(self):
         response = self.client.post(
@@ -1612,7 +1619,15 @@ def test_access_request_notifies_keycloak_use_case_admins(self, emails_mock):
             mail.outbox[1].to,
             ["mepram.admin@example.org", "other.admin@example.org"],
         )
-        self.assertIn("pending", mail.outbox[1].subject.lower())
+        self.assertEqual(mail.outbox[1].subject, mail.outbox[0].subject)
+        self.assertEqual(
+            mail.outbox[1].extra_headers["In-Reply-To"],
+            mail.outbox[0].extra_headers["Message-ID"],
+        )
+        self.assertEqual(
+            mail.outbox[1].extra_headers["References"],
+            mail.outbox[0].extra_headers["Message-ID"],
+        )
         self.assertIn("https://github.com/BU-ISCIII", mail.outbox[1].body)
 
     @override_settings(PATHOCORE_ACCESS_REQUEST_ADMIN_EMAILS=["fallback@example.org"])
@@ -1634,6 +1649,23 @@ def test_access_request_uses_fallback_admin_email_when_keycloak_fails(
         self.assertEqual(len(mail.outbox), 2)
         self.assertEqual(mail.outbox[1].to, ["fallback@example.org"])
 
+    @override_settings(DEFAULT_FROM_EMAIL="bioinformatica.infec@ciberisciii.es")
+    def test_access_request_email_thread_uses_sender_domain(self):
+        response = self.client.post(
+            "/v1/access-requests",
+            data=self._request_payload(),
+            format="json",
+        )
+
+        self.assertEqual(response.status_code, 201)
+        self.assertEqual(
+            mail.outbox[0].extra_headers["Message-ID"],
+            (
+                f"<pathocore-access-request-{response.data['id']}-received"
+                "@ciberisciii.es>"
+            ),
+        )
+
     def test_admin_can_list_pending_access_requests(self):
         models.AccessRequest.objects.create(**self._request_payload())
 
@@ -1674,7 +1706,14 @@ def test_admin_can_approve_access_request(self, provision_mock):
         self.assertEqual(response.data["keycloak_user_id"], "keycloak-user-1")
         provision_mock.assert_called_once()
         self.assertEqual(len(mail.outbox), 1)
-        self.assertIn("approved", mail.outbox[0].subject.lower())
+        self.assertEqual(
+            mail.outbox[0].subject,
+            f"[PathoCore access #{access_request.pk}] MEPRAM (view) - new_user",
+        )
+        self.assertEqual(
+            mail.outbox[0].extra_headers["In-Reply-To"],
+            (f"<pathocore-access-request-{access_request.pk}-received" "@localhost>"),
+        )
         self.assertIn("MEPRAM (view)", mail.outbox[0].body)
         self.assertIn(
             "https://mepram-datahub.ciberisciii.es/use-cases/mepram",
@@ -1702,7 +1741,14 @@ def test_admin_can_reject_access_request(self, emails_mock):
         )
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(mail.outbox[0].to, ["new.user@example.org"])
-        self.assertIn("rejected", mail.outbox[0].subject.lower())
+        self.assertEqual(
+            mail.outbox[0].subject,
+            f"[PathoCore access #{access_request.pk}] MEPRAM (view) - new_user",
+        )
+        self.assertEqual(
+            mail.outbox[0].extra_headers["In-Reply-To"],
+            (f"<pathocore-access-request-{access_request.pk}-received" "@localhost>"),
+        )
         self.assertIn("Reason: Missing project justification", mail.outbox[0].body)
         self.assertIn("contact: mepram.admin@example.org", mail.outbox[0].body)
         self.assertIn("Technical platforms:", mail.outbox[0].body)
@@ -1736,7 +1782,14 @@ def test_admin_can_revoke_approved_access_request(self, emails_mock, revoke_mock
         revoke_mock.assert_called_once()
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(mail.outbox[0].to, ["new.user@example.org"])
-        self.assertIn("revoked", mail.outbox[0].subject.lower())
+        self.assertEqual(
+            mail.outbox[0].subject,
+            f"[PathoCore access #{access_request.pk}] MEPRAM (view) - new_user",
+        )
+        self.assertEqual(
+            mail.outbox[0].extra_headers["In-Reply-To"],
+            (f"<pathocore-access-request-{access_request.pk}-received" "@localhost>"),
+        )
         self.assertIn("Reason: Access no longer required", mail.outbox[0].body)
         self.assertIn("contact: mepram.admin@example.org", mail.outbox[0].body)
         self.assertIn("Technical platforms:", mail.outbox[0].body)