🐛(backend) release mention guard when notification fails

The cache key claimed before sending outlived a failed notification,
so the next mention in the same context was silently dropped until the
key expired although nobody had been notified. Delete it unless the
email went out and `notified_at` was saved.

Signed-off-by: BOUKERFA Mohamed El Amine <boukerfa.ma@gmail.com>

src/backend/core/models.py

@@ -1954,6 +1954,12 @@ class Reaction(BaseModel):
         return f"Reaction {self.emoji} on comment {self.comment.id}"
 
 
+# The notification guard only has to cover the window between the cooldown
+# check and the `notified_at` save: a leaked key (e.g. killed worker) must not
+# silence a context for the whole cooldown period
+MENTION_NOTIFICATION_GUARD_TIMEOUT_SECONDS = 60
+
+
 class Mention(BaseModel):
     """A mention of a user in a document body or in a comment thread.
 
@@ -2062,7 +2068,7 @@ class Mention(BaseModel):
         if not cache.add(
             self.notification_guard_key,
             str(self.pk),
-            timeout=settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES * 60,
+            timeout=MENTION_NOTIFICATION_GUARD_TIMEOUT_SECONDS,
         ):
             return False
 
@@ -2095,10 +2101,17 @@ class Mention(BaseModel):
                 "link": f"{domain}/docs/{self.document_id}/#{self.anchor_id}",
             }
 
-        self.document.send_email(subject, [user.email], context, language)
+        sent = False
+        try:
+            self.document.send_email(subject, [user.email], context, language)
+            self.notified_at = timezone.now()
+            self.save(update_fields=["notified_at", "updated_at"])
+            sent = True
+        finally:
+            # Release the context so the next mention can notify
+            if not sent:
+                cache.delete(self.notification_guard_key)
 
-        self.notified_at = timezone.now()
-        self.save(update_fields=["notified_at", "updated_at"])
         return True
 
 

src/backend/core/tests/test_models_mentions.py

@@ -2,7 +2,10 @@
 Unit tests for the Mention model
 """
 
+from unittest import mock
+
 from django.core import mail
+from django.core.cache import cache
 
 import pytest
 from rest_framework.exceptions import ValidationError
@@ -106,3 +109,29 @@ def test_models_mentions_notify_concurrent_duplicate():
     assert second.notified_at is None
     # pylint: disable-next=no-member
     assert len(mail.outbox) == 1
+
+
+def test_models_mentions_notify_failure_releases_guard():
+    """A failed notification should release the context guard.
+
+    Otherwise the next mention in the same context would be silently dropped
+    until the guard expires, although nobody was notified.
+    """
+    document = factories.DocumentFactory()
+    mentioned_user = factories.UserFactory()
+    first = factories.MentionFactory(document=document, mentioned_user=mentioned_user)
+    second = factories.MentionFactory(document=document, mentioned_user=mentioned_user)
+
+    with (
+        mock.patch.object(models.Document, "send_email", side_effect=RuntimeError),
+        pytest.raises(RuntimeError),
+    ):
+        first.notify()
+
+    first.refresh_from_db()
+    assert first.notified_at is None
+    assert cache.get(first.notification_guard_key) is None
+
+    assert second.notify() is True
+    # pylint: disable-next=no-member
+    assert len(mail.outbox) == 1