From 4217f7c07742f49c4bf454ea342df459cec7d607 Mon Sep 17 00:00:00 2001 From: BOUKERFA Mohamed El Amine Date: Tue, 15 Sep 2026 17:38:19 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20release=20mention=20gua?= =?UTF-8?q?rd=20when=20notification=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/backend/core/models.py | 21 +++++++++++--- .../core/tests/test_models_mentions.py | 29 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index a1e303e6e..5b9867e82 100644 --- a/src/backend/core/models.py +++ b/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 diff --git a/src/backend/core/tests/test_models_mentions.py b/src/backend/core/tests/test_models_mentions.py index da6197e5b..ae4de53ee 100644 --- a/src/backend/core/tests/test_models_mentions.py +++ b/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