From ed6237f283bb374887275eaf2f2cc6c72467fcee Mon Sep 17 00:00:00 2001 From: Daniel-VM Date: Wed, 26 Aug 2026 13:13:39 +0200 Subject: [PATCH 1/3] Thread access request notification emails --- core/api/services/access_requests.py | 100 ++++++++++++++++++++++----- core/tests.py | 75 ++++++++++++++++++-- 2 files changed, 151 insertions(+), 24 deletions(-) diff --git a/core/api/services/access_requests.py b/core/api/services/access_requests.py index ce822d3..74ac4f0 100644 --- a/core/api/services/access_requests.py +++ b/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"" + ) + + +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}" diff --git a/core/tests.py b/core/tests.py index 9ac8acf..7d16dea 100644 --- a/core/tests.py +++ b/core/tests.py @@ -1523,7 +1523,17 @@ 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"" + ), + ) def test_legacy_v1_alias_still_accepts_access_request(self): response = self.client.post( @@ -1612,7 +1622,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 +1652,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"" + ), + ) + def test_admin_can_list_pending_access_requests(self): models.AccessRequest.objects.create(**self._request_payload()) @@ -1674,7 +1709,17 @@ 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"" + ), + ) self.assertIn("MEPRAM (view)", mail.outbox[0].body) self.assertIn( "https://mepram-datahub.ciberisciii.es/use-cases/mepram", @@ -1702,7 +1747,17 @@ 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"" + ), + ) 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 +1791,17 @@ 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"" + ), + ) 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) From 6f140cef69d8d4b33d5538b81760503749827883 Mon Sep 17 00:00:00 2001 From: Daniel-VM Date: Wed, 26 Aug 2026 13:14:05 +0200 Subject: [PATCH 2/3] Update changelog for access request email threads --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa0f53c..846666a 100644 --- a/CHANGELOG.md +++ b/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` From 42ba77df5aef0cdd7f9f0f7482c0ffc3e83a67b0 Mon Sep 17 00:00:00 2001 From: Daniel-VM Date: Wed, 26 Aug 2026 13:21:38 +0200 Subject: [PATCH 3/3] fix linting --- core/tests.py | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/core/tests.py b/core/tests.py index 7d16dea..c1658fd 100644 --- a/core/tests.py +++ b/core/tests.py @@ -1529,10 +1529,7 @@ def test_public_user_can_create_pending_access_request(self): ) self.assertEqual( mail.outbox[0].extra_headers["Message-ID"], - ( - f"" - ), + (f""), ) def test_legacy_v1_alias_still_accepts_access_request(self): @@ -1715,10 +1712,7 @@ def test_admin_can_approve_access_request(self, provision_mock): ) self.assertEqual( mail.outbox[0].extra_headers["In-Reply-To"], - ( - f"" - ), + (f""), ) self.assertIn("MEPRAM (view)", mail.outbox[0].body) self.assertIn( @@ -1753,10 +1747,7 @@ def test_admin_can_reject_access_request(self, emails_mock): ) self.assertEqual( mail.outbox[0].extra_headers["In-Reply-To"], - ( - f"" - ), + (f""), ) self.assertIn("Reason: Missing project justification", mail.outbox[0].body) self.assertIn("contact: mepram.admin@example.org", mail.outbox[0].body) @@ -1797,10 +1788,7 @@ def test_admin_can_revoke_approved_access_request(self, emails_mock, revoke_mock ) self.assertEqual( mail.outbox[0].extra_headers["In-Reply-To"], - ( - f"" - ), + (f""), ) self.assertIn("Reason: Access no longer required", mail.outbox[0].body) self.assertIn("contact: mepram.admin@example.org", mail.outbox[0].body)