fix(grants): address PR review — retry email, on_commit, admin atomic

- create_and_send_voucher_to_grantee: if GRANT/SPEAKER row exists but
  voucher_email_sent_at is None, queue send_conference_voucher_email (Celery
  retry after DB+Pretix succeeded but email enqueue failed).
- Catch IntegrityError on create_conference_voucher; refetch row and queue email
  if still unsent (concurrent task / race).
- sendGrantReply: schedule voucher task via transaction.on_commit so workers
  see committed grant status.
- create_grant_vouchers: drop @transaction.atomic to avoid Pretix orphans on
  rollback.
- Migration 0022: document noop_reverse (irreversible consolidation).
- Tests: voucher already sent vs never sent, IntegrityError path, on_commit
  helper for GraphQL confirm.

Author: estyxx

backend/api/grants/mutations.py

@@ -351,7 +351,11 @@ def send_grant_reply(
         grant.save()
 
         if old_status != grant.status and grant.status == GrantModel.Status.confirmed:
-            create_and_send_voucher_to_grantee.delay(grant_id=grant.id)
+            transaction.on_commit(
+                lambda gid=grant.id: create_and_send_voucher_to_grantee.delay(
+                    grant_id=gid
+                )
+            )
 
         create_change_admin_log_entry(
             request.user, grant, f"Grantee has replied with status {grant.status}."

backend/api/grants/tests/test_send_grant_reply.py

@@ -75,14 +75,17 @@ def test_user_cannot_reply_if_status_is_rejected(graphql_client, user):
     )
 
 
-def test_status_is_updated_when_reply_is_confirmed(graphql_client, user, mocker):
+def test_status_is_updated_when_reply_is_confirmed(
+    graphql_client, user, mocker, django_capture_on_commit_callbacks
+):
     graphql_client.force_login(user)
     grant = GrantFactory(user_id=user.id, status=Grant.Status.waiting_for_confirmation)
     mock_voucher = mocker.patch(
         "api.grants.mutations.create_and_send_voucher_to_grantee"
     )
 
-    response = _send_grant_reply(graphql_client, grant, status="confirmed")
+    with django_capture_on_commit_callbacks(execute=True):
+        response = _send_grant_reply(graphql_client, grant, status="confirmed")
 
     assert response["data"]["sendGrantReply"]["__typename"] == "Grant"
 

backend/grants/admin.py

@@ -4,7 +4,6 @@
 
 from django.contrib import admin, messages
 from django.contrib.admin import SimpleListFilter
-from django.db import transaction
 from django.db.models import Exists, OuterRef
 from django.db.models.query import QuerySet
 from django.urls import reverse
@@ -299,7 +298,6 @@ def send_reply_email_waiting_list_update(modeladmin, request, queryset):
 
 @admin.action(description="Create grant vouchers")
 @validate_single_conference_selection
-@transaction.atomic
 def create_grant_vouchers(modeladmin, request, queryset):
     grants_ordered = list(queryset.order_by("id").select_related("user", "conference"))
     conference_id = grants_ordered[0].conference_id if grants_ordered else None

backend/grants/tasks.py

@@ -3,6 +3,7 @@
 from urllib.parse import urljoin
 
 from django.conf import settings
+from django.db import IntegrityError
 from django.utils import timezone
 
 from conferences.models.conference_voucher import ConferenceVoucher
@@ -49,6 +50,10 @@ def create_and_send_voucher_to_grantee(*, grant_id: int) -> None:
                 user.id,
                 conference.id,
             )
+            if conference_voucher.voucher_email_sent_at is None:
+                send_conference_voucher_email.delay(
+                    conference_voucher_id=conference_voucher.id
+                )
             return
         if conference_voucher.voucher_type == ConferenceVoucher.VoucherType.CO_SPEAKER:
             conference_voucher.voucher_type = ConferenceVoucher.VoucherType.GRANT
@@ -61,11 +66,31 @@ def create_and_send_voucher_to_grantee(*, grant_id: int) -> None:
             )
             return
 
-    new_voucher = create_conference_voucher(
-        conference=conference,
-        user=user,
-        voucher_type=ConferenceVoucher.VoucherType.GRANT,
-    )
+    try:
+        new_voucher = create_conference_voucher(
+            conference=conference,
+            user=user,
+            voucher_type=ConferenceVoucher.VoucherType.GRANT,
+        )
+    except IntegrityError:
+        logger.warning(
+            "IntegrityError creating grant voucher for user %s conference %s "
+            "(likely concurrent task); syncing email state",
+            user.id,
+            conference.id,
+        )
+        existing_after_race = (
+            ConferenceVoucher.objects.for_conference(conference).for_user(user).first()
+        )
+        if (
+            existing_after_race is not None
+            and existing_after_race.voucher_email_sent_at is None
+        ):
+            send_conference_voucher_email.delay(
+                conference_voucher_id=existing_after_race.id
+            )
+        return
+
     send_conference_voucher_email.delay(conference_voucher_id=new_voucher.id)
 
 

backend/grants/tests/test_tasks.py

@@ -1,7 +1,9 @@
 from datetime import datetime, timezone
 from decimal import Decimal
+
 import pytest
 import time_machine
+from django.db import IntegrityError
 
 from conferences.models.conference_voucher import ConferenceVoucher
 from conferences.tests.factories import (
@@ -537,6 +539,7 @@ def test_create_and_send_voucher_to_grantee_does_nothing_if_voucher_exists(
         conference=grant.conference,
         user=grant.user,
         voucher_type=voucher_type,
+        voucher_email_sent_at=datetime(2020, 1, 1, tzinfo=timezone.utc),
     )
 
     create_and_send_voucher_to_grantee(grant_id=grant.id)
@@ -545,6 +548,64 @@ def test_create_and_send_voucher_to_grantee_does_nothing_if_voucher_exists(
     mock_send_email.delay.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    "voucher_type",
+    [
+        ConferenceVoucher.VoucherType.SPEAKER,
+        ConferenceVoucher.VoucherType.GRANT,
+    ],
+)
+def test_create_and_send_voucher_to_grantee_queues_email_when_voucher_exists_but_never_sent(
+    mocker, voucher_type
+):
+    mock_create = mocker.patch("conferences.vouchers.create_voucher")
+    mock_send_email = mocker.patch("conferences.tasks.send_conference_voucher_email")
+
+    grant = GrantFactory(status=Grant.Status.confirmed)
+    voucher = ConferenceVoucherFactory(
+        conference=grant.conference,
+        user=grant.user,
+        voucher_type=voucher_type,
+        voucher_email_sent_at=None,
+    )
+
+    create_and_send_voucher_to_grantee(grant_id=grant.id)
+
+    mock_create.assert_not_called()
+    mock_send_email.delay.assert_called_once_with(conference_voucher_id=voucher.id)
+
+
+def test_create_and_send_voucher_to_grantee_integrity_error_queues_email_if_unsent(
+    mocker,
+):
+    mock_send_email = mocker.patch("conferences.tasks.send_conference_voucher_email")
+
+    def another_worker_won_race_then_integrity(**kwargs):
+        ConferenceVoucherFactory(
+            conference=kwargs["conference"],
+            user=kwargs["user"],
+            voucher_type=kwargs["voucher_type"],
+            voucher_email_sent_at=None,
+        )
+        raise IntegrityError()
+
+    mock_create = mocker.patch(
+        "grants.tasks.create_conference_voucher",
+        side_effect=another_worker_won_race_then_integrity,
+    )
+
+    grant = GrantFactory(status=Grant.Status.confirmed)
+
+    create_and_send_voucher_to_grantee(grant_id=grant.id)
+
+    mock_create.assert_called_once()
+    voucher = ConferenceVoucher.objects.get(
+        conference=grant.conference,
+        user=grant.user,
+    )
+    mock_send_email.delay.assert_called_once_with(conference_voucher_id=voucher.id)
+
+
 def test_create_and_send_voucher_to_grantee_upgrades_co_speaker(mocker):
     mock_create = mocker.patch("conferences.vouchers.create_voucher")
     mock_send_email = mocker.patch("conferences.tasks.send_conference_voucher_email")

backend/notifications/migrations/0022_remove_grant_voucher_code_template_identifier.py

@@ -11,6 +11,8 @@ def forwards_grant_voucher_to_voucher_code(apps, schema_editor):
 
 
 def noop_reverse(apps, schema_editor):
+    # Intentionally empty: reversing would not restore grant_voucher_code rows in a
+    # distinguishable way from existing voucher_code templates (data loss on rollback).
     pass