Make notification.deliver idempotent on retry

service/notification/migrations/0002_add_expired_delivery_status.py

@@ -0,0 +1,22 @@
+# Generated by Django 6.1 on 2026-08-29 13:05
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("notification", "0001_initial"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="notificationdelivery",
+            name="status",
+            field=models.CharField(
+                choices=[("pending", "Pending"), ("sent", "Sent"), ("failed", "Failed"), ("expired", "Expired")],
+                default="pending",
+                max_length=20,
+            ),
+        ),
+    ]

service/notification/models.py

@@ -1,5 +1,6 @@
 from django.contrib.auth import get_user_model
 from django.db import models
+from django.utils import timezone
 
 from common.models import BaseModel
 
@@ -62,6 +63,14 @@ def broadcast(self, notifications, spec):
         deliver.defer(delivery_ids=[d.id for d in deliveries])
         return deliveries
 
+    def expire_stale(self, deliveries, max_age):
+        cutoff = timezone.now() - max_age
+        stale = [d for d in deliveries if d.created_at < cutoff]
+        if not stale:
+            return deliveries
+        self.filter(id__in=[d.id for d in stale]).update(status=self.model.Status.EXPIRED)
+        return [d for d in deliveries if d.created_at >= cutoff]
+
     def _email_enabled_ids(self, recipient_ids):
         return set(
             User.objects.filter(
@@ -79,6 +88,7 @@ class Status(models.TextChoices):
         PENDING = "pending"
         SENT = "sent"
         FAILED = "failed"
+        EXPIRED = "expired"
 
     objects = NotificationDeliveryManager()
 

service/notification/tasks.py

@@ -11,17 +11,23 @@
 logger = logging.getLogger(__name__)
 
 PRUNE_AFTER_DAYS = 30
+DELIVER_MAX_AGE_HOURS = 1
 
 
 @app.task(name="notification.deliver", retry=3)
 def deliver(delivery_ids):
     deliveries = list(
-        NotificationDelivery.objects.filter(id__in=delivery_ids).select_related("notification")
+        NotificationDelivery.objects.filter(
+            id__in=delivery_ids, status=NotificationDelivery.Status.PENDING
+        ).select_related("notification")
     )
     if not deliveries:
         return
-    _deliver_channel(deliveries, NotificationDelivery.Channel.PUSH, _send_push)
-    _deliver_channel(deliveries, NotificationDelivery.Channel.EMAIL, _send_email)
+    fresh = NotificationDelivery.objects.expire_stale(deliveries, timedelta(hours=DELIVER_MAX_AGE_HOURS))
+    if not fresh:
+        return
+    _deliver_channel(fresh, NotificationDelivery.Channel.PUSH, _send_push)
+    _deliver_channel(fresh, NotificationDelivery.Channel.EMAIL, _send_email)
 
 
 def _deliver_channel(deliveries, channel, send):

service/notification/tests.py

@@ -16,7 +16,7 @@
 from notification.models import Notification, NotificationDelivery
 from notification.registry import REGISTRY as NOTIFICATION_REGISTRY
 from notification.registry import ChannelMessageSpec
-from notification.tasks import PRUNE_AFTER_DAYS, deliver, prune
+from notification.tasks import DELIVER_MAX_AGE_HOURS, PRUNE_AFTER_DAYS, deliver, prune
 from notification.utils import PUSH_TTL, build_push_message
 from phase.models import Phase
 from user_profile.models import UserProfile
@@ -55,6 +55,21 @@ def _rendered_push(**overrides):
     return [content]
 
 
+def _push_delivery(user, status=NotificationDelivery.Status.PENDING):
+    notification = Notification.objects.create(recipient=user, event_type="game_start")
+    return NotificationDelivery.objects.create(
+        notification=notification,
+        channel=NotificationDelivery.Channel.PUSH,
+        heading="Game",
+        body="Started",
+        status=status,
+    )
+
+
+def _backdate(delivery, age):
+    NotificationDelivery.objects.filter(id=delivery.id).update(created_at=timezone.now() - age)
+
+
 class _StubSpec:
     def __init__(self, rendered):
         self.channels = [content["channel"] for content in rendered]
@@ -868,6 +883,56 @@ def test_missing_ids_is_noop(self, mock_send_notification_to_users, in_memory_pr
 
         mock_send_notification_to_users.assert_not_called()
 
+    @pytest.mark.django_db
+    def test_already_sent_delivery_is_not_resent(self, user_factory, mock_send_notification_to_users):
+        delivery = _push_delivery(user_factory(), status=NotificationDelivery.Status.SENT)
+
+        deliver(delivery_ids=[delivery.id])
+
+        mock_send_notification_to_users.assert_not_called()
+
+    @pytest.mark.django_db
+    def test_pending_delivery_in_a_partly_sent_batch_is_still_sent(
+        self, user_factory, mock_send_notification_to_users
+    ):
+        one, two = user_factory(), user_factory()
+        sent = _push_delivery(one, status=NotificationDelivery.Status.SENT)
+        pending = _push_delivery(two)
+
+        deliver(delivery_ids=[sent.id, pending.id])
+
+        assert mock_send_notification_to_users.call_args.kwargs["user_ids"] == [two.id]
+
+    @pytest.mark.django_db
+    def test_delivery_older_than_max_age_is_expired_not_sent(
+        self, user_factory, mock_send_notification_to_users
+    ):
+        delivery = _push_delivery(user_factory())
+        _backdate(delivery, timedelta(hours=DELIVER_MAX_AGE_HOURS, minutes=1))
+
+        deliver(delivery_ids=[delivery.id])
+
+        mock_send_notification_to_users.assert_not_called()
+        delivery.refresh_from_db()
+        assert delivery.status == NotificationDelivery.Status.EXPIRED
+
+    @pytest.mark.django_db
+    def test_fresh_delivery_in_a_partly_stale_batch_is_still_sent(
+        self, user_factory, mock_send_notification_to_users
+    ):
+        one, two = user_factory(), user_factory()
+        stale = _push_delivery(one)
+        _backdate(stale, timedelta(hours=DELIVER_MAX_AGE_HOURS, minutes=1))
+        fresh = _push_delivery(two)
+
+        deliver(delivery_ids=[stale.id, fresh.id])
+
+        assert mock_send_notification_to_users.call_args.kwargs["user_ids"] == [two.id]
+        stale.refresh_from_db()
+        fresh.refresh_from_db()
+        assert stale.status == NotificationDelivery.Status.EXPIRED
+        assert fresh.status == NotificationDelivery.Status.SENT
+
 
 class TestNotificationPrune:
     @pytest.mark.django_db