Implement Draft SentEmail (#4785)

Author: marcoacierno

backend/notifications/admin/admins.py

@@ -1,3 +1,7 @@
+import functools
+import logging
+from django.db import transaction
+from django.utils import timezone
 from typing import Any
 from django.http import HttpResponseRedirect
 from django.http.request import HttpRequest
@@ -15,6 +19,10 @@
 
 from notifications.models import EmailTemplate, SentEmail, SentEmailEvent
 from django.forms import Textarea
+from django.db.models import QuerySet
+from notifications.tasks import send_pending_email
+
+logger = logging.getLogger(__name__)
 
 
 class SentEmailEventInline(admin.TabularInline):
@@ -111,6 +119,17 @@ def get_view_on_site_url(self, obj) -> str | None:
         return reverse("admin:view-email-template", args=(obj.id,))
 
 
+def _submit_emails_for_sending(sent_emails_ids: list[int]) -> None:
+    for sent_email_id in sent_emails_ids:
+        try:
+            send_pending_email.delay(sent_email_id)
+        except Exception:
+            logger.exception(
+                "Could not queue sent_email_id=%s, leaving it pending",
+                sent_email_id,
+            )
+
+
 @admin.register(SentEmail)
 class SentEmailAdmin(admin.ModelAdmin):
     list_display = [
@@ -146,6 +165,7 @@ class SentEmailAdmin(admin.ModelAdmin):
     ordering = ["-sent_at"]
     autocomplete_fields = ["recipient"]
     inlines = [SentEmailEventInline]
+    actions = ["send_email"]
 
     def email_template_display_name(self, obj):
         if obj.email_template.is_custom:
@@ -182,3 +202,29 @@ def get_queryset(self, request: HttpRequest) -> Any:
             qs = qs.filter(email_template__is_system_template=False)
 
         return qs
+
+    @transaction.atomic
+    def send_email(self, request: HttpRequest, queryset: QuerySet[SentEmail]):
+        affected_emails_ids = list(
+            queryset.filter(
+                status__in=[
+                    SentEmail.Status.draft,
+                    SentEmail.Status.pending,
+                    SentEmail.Status.failed,
+                ]
+            )
+            .select_for_update(skip_locked=True)
+            .values_list("id", flat=True)
+        )
+
+        SentEmail.objects.filter(id__in=affected_emails_ids).update(
+            status=SentEmail.Status.pending,
+            modified=timezone.now(),
+        )
+
+        transaction.on_commit(
+            functools.partial(_submit_emails_for_sending, affected_emails_ids)
+        )
+        self.message_user(
+            request, f"Emails queued for sending: {len(affected_emails_ids)}"
+        )

backend/notifications/migrations/0023_alter_sentemail_status.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.2.8 on 2026-08-15 10:04
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('notifications', '0022_remove_grant_voucher_code_template_identifier'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='sentemail',
+            name='status',
+            field=models.CharField(choices=[('draft', 'Draft'), ('pending', 'Pending'), ('sent', 'Sent'), ('failed', 'Failed')], db_index=True, default='pending', max_length=200, verbose_name='status'),
+        ),
+    ]

backend/notifications/models.py

@@ -211,18 +211,17 @@ def render(
             placeholders=placeholders,
         )
 
-    def send_email(
+    def _prepare_email(
         self,
         *,
+        status: "SentEmail.Status",
         recipient: User | None = None,
         recipient_email: str | None = None,
         placeholders: dict = None,
     ):
         if not recipient and not recipient_email:
             raise ValueError("Either recipient or recipient_email must be provided")
 
-        from notifications.tasks import send_pending_email
-
         recipient_email = recipient_email or recipient.email
 
         placeholders = placeholders or {}
@@ -236,7 +235,8 @@ def send_email(
                 or settings.DEFAULT_FROM_EMAIL
             )
 
-        sent_email = SentEmail.objects.create(
+        return SentEmail.objects.create(
+            status=status,
             email_template=self,
             conference=self.conference,
             from_email=from_email,
@@ -252,6 +252,35 @@ def send_email(
             bcc_addresses=self.bcc_addresses,
         )
 
+    def draft_email(
+        self,
+        *,
+        recipient: User | None = None,
+        recipient_email: str | None = None,
+        placeholders: dict = None,
+    ):
+        return self._prepare_email(
+            status=SentEmail.Status.draft,
+            recipient=recipient,
+            recipient_email=recipient_email,
+            placeholders=placeholders,
+        )
+
+    def send_email(
+        self,
+        *,
+        recipient: User | None = None,
+        recipient_email: str | None = None,
+        placeholders: dict = None,
+    ):
+        from notifications.tasks import send_pending_email
+
+        sent_email = self._prepare_email(
+            status=SentEmail.Status.pending,
+            recipient=recipient,
+            recipient_email=recipient_email,
+            placeholders=placeholders,
+        )
         transaction.on_commit(lambda: send_pending_email.delay(sent_email.id))
 
     @property
@@ -284,6 +313,7 @@ class Meta:
 
 class SentEmail(TimeStampedModel):
     class Status(models.TextChoices):
+        draft = "draft", _("Draft")
         pending = "pending", _("Pending")
         sent = "sent", _("Sent")
         failed = "failed", _("Failed")

backend/notifications/tests/admin/test_sent_email_admin.py

@@ -98,3 +98,102 @@ def test_email_template_display_name():
     visible_name = admin.email_template_display_name(sent_email)
 
     assert visible_name == sent_email.email_template.get_identifier_display()
+
+
+def test_send_email_action(rf, admin_user, django_capture_on_commit_callbacks, mocker):
+    mock_send_pending_email = mocker.patch(
+        "notifications.admin.admins.send_pending_email.delay"
+    )
+    admin = SentEmailAdmin(
+        model=SentEmail,
+        admin_site=AdminSite(),
+    )
+    admin.message_user = mocker.Mock()
+
+    request = rf.post("/")
+    request.user = admin_user
+
+    draft_email_1 = SentEmailFactory(status=SentEmail.Status.draft)
+    draft_email_2 = SentEmailFactory(status=SentEmail.Status.draft)
+    pending_email = SentEmailFactory(status=SentEmail.Status.pending)
+    sent_email = SentEmailFactory(status=SentEmail.Status.sent)
+    failed_email = SentEmailFactory(status=SentEmail.Status.failed)
+
+    with django_capture_on_commit_callbacks(execute=True):
+        admin.send_email(request, SentEmail.objects.all())
+
+    # drafts, pending and failed emails are all (re)queued for sending
+    queued_ids = {call.args[0] for call in mock_send_pending_email.call_args_list}
+    assert queued_ids == {
+        draft_email_1.id,
+        draft_email_2.id,
+        pending_email.id,
+        failed_email.id,
+    }
+    assert mock_send_pending_email.call_count == 4
+
+    for email in (draft_email_1, draft_email_2, pending_email, failed_email):
+        email.refresh_from_db()
+        assert email.status == SentEmail.Status.pending
+
+    # already sent emails are never touched
+    sent_email.refresh_from_db()
+    assert sent_email.status == SentEmail.Status.sent
+
+    admin.message_user.assert_called_once_with(request, "Emails queued for sending: 4")
+
+
+def test_send_email_action_with_a_status_filtered_queryset(
+    rf, admin_user, django_capture_on_commit_callbacks, mocker
+):
+    """The changelist queryset carries the active list_filter, so the action
+    receives a queryset already narrowed to a single status."""
+    mock_send_pending_email = mocker.patch(
+        "notifications.admin.admins.send_pending_email.delay"
+    )
+    admin = SentEmailAdmin(
+        model=SentEmail,
+        admin_site=AdminSite(),
+    )
+    admin.message_user = mocker.Mock()
+
+    request = rf.post("/")
+    request.user = admin_user
+
+    draft_email = SentEmailFactory(status=SentEmail.Status.draft)
+
+    with django_capture_on_commit_callbacks(execute=True):
+        admin.send_email(
+            request, SentEmail.objects.filter(status=SentEmail.Status.draft)
+        )
+
+    mock_send_pending_email.assert_called_once_with(draft_email.id)
+
+    draft_email.refresh_from_db()
+    assert draft_email.status == SentEmail.Status.pending
+
+
+def test_send_email_action_keeps_queueing_after_a_broker_failure(
+    rf, admin_user, django_capture_on_commit_callbacks, mocker
+):
+    mock_send_pending_email = mocker.patch(
+        "notifications.admin.admins.send_pending_email.delay",
+        side_effect=[Exception("broker is down"), None],
+    )
+    admin = SentEmailAdmin(
+        model=SentEmail,
+        admin_site=AdminSite(),
+    )
+    admin.message_user = mocker.Mock()
+
+    request = rf.post("/")
+    request.user = admin_user
+
+    SentEmailFactory(status=SentEmail.Status.draft)
+    SentEmailFactory(status=SentEmail.Status.draft)
+
+    with django_capture_on_commit_callbacks(execute=True):
+        admin.send_email(request, SentEmail.objects.all())
+
+    # the first publish blowing up must not strand the remaining emails
+    assert mock_send_pending_email.call_count == 2

backend/notifications/tests/test_models.py

@@ -264,3 +264,87 @@ def test_email_template_system_templates_filter():
 
     assert EmailTemplate.objects.system_templates().count() == 1
     assert EmailTemplate.objects.system_templates().first().id == email_template.id
+
+
+def test_draft_email_template_to_recipient_email(
+    mocker, django_capture_on_commit_callbacks
+):
+    email_template = EmailTemplateFactory(
+        subject="Subject {{ test }}",
+        body="Body {{ test }}",
+        preview_text="Preview {{ test }}",
+        reply_to="replyto@example.com",
+    )
+
+    mock_send_pending_email = mocker.patch(
+        "notifications.tasks.send_pending_email.delay"
+    )
+
+    with django_capture_on_commit_callbacks(execute=True):
+        sent_email = email_template.draft_email(
+            recipient_email="example@example.com",
+            placeholders={
+                "test": "abc",
+            },
+        )
+
+    # drafts are only queued once the action in the admin is used
+    mock_send_pending_email.assert_not_called()
+
+    assert sent_email.status == SentEmail.Status.draft
+    assert sent_email.sent_at is None
+    assert sent_email.recipient is None
+    assert sent_email.recipient_email == "example@example.com"
+
+    assert sent_email.subject == "Subject abc"
+    assert "Body abc" in sent_email.body
+    assert sent_email.preview_text == "Preview abc"
+    assert sent_email.reply_to == "replyto@example.com"
+
+
+def test_draft_email_template_to_recipient_user():
+    user = UserFactory()
+    email_template = EmailTemplateFactory(
+        subject="Subject {{ test }}",
+        body="Body {{ test }}",
+        preview_text="Preview {{ test }}",
+    )
+
+    sent_email = email_template.draft_email(
+        recipient=user,
+        placeholders={
+            "test": "abc",
+        },
+    )
+
+    assert sent_email.status == SentEmail.Status.draft
+    assert sent_email.recipient == user
+    assert sent_email.recipient_email == user.email
+    assert sent_email.subject == "Subject abc"
+
+
+def test_draft_email_requires_a_recipient():
+    email_template = EmailTemplateFactory()
+
+    with pytest.raises(
+        ValueError, match="Either recipient or recipient_email must be provided"
+    ):
+        email_template.draft_email(placeholders={})
+
+    assert not SentEmail.objects.exists()
+
+
+def test_send_email_creates_a_pending_email(mocker, django_capture_on_commit_callbacks):
+    email_template = EmailTemplateFactory()
+
+    mock_send_pending_email = mocker.patch(
+        "notifications.tasks.send_pending_email.delay"
+    )
+
+    with django_capture_on_commit_callbacks(execute=True):
+        email_template.send_email(recipient_email="example@example.com")
+
+    sent_email = SentEmail.objects.get(email_template=email_template)
+
+    assert sent_email.status == SentEmail.Status.pending
+    mock_send_pending_email.assert_called_once_with(sent_email.id)