mirror of
https://github.com/suitenumerique/docs.git
synced 2026-10-15 20:55:25 +02:00
✨(backend) send thread reply notification asynchronously
Queue a Celery task when a comment is added to an existing thread, so the SMTP round-trips stay out of the request. Queuing is best effort and a failure to send never fails the creation of the comment. The first comment of a thread is created with the thread by another code path and is not notified. Signed-off-by: Anthony LC <anthony.le-courric@mail.numerique.gouv.fr>
This commit is contained in:
@@ -6,6 +6,10 @@ and this project adheres to
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
-✨(backend) email thread participants on reply #2802
|
||||
|
||||
## [v6.0.0] - 2026-10-05
|
||||
|
||||
### Added
|
||||
|
||||
@@ -71,7 +71,11 @@ from core.services.search_indexers import (
|
||||
from core.services.yhub_services import YHubError, YHubService
|
||||
from core.tasks.access import reset_service_connections_on_commit
|
||||
from core.tasks.documents import sync_service_restorations_in_cascade
|
||||
from core.tasks.mail import send_ask_for_access_mail, send_mention_notification_mail
|
||||
from core.tasks.mail import (
|
||||
send_ask_for_access_mail,
|
||||
send_mention_notification_mail,
|
||||
send_thread_reply_notification_mail,
|
||||
)
|
||||
from core.tasks.search import trigger_batch_document_indexer
|
||||
from core.utils.analytics import PosthogEventName, posthog_capture
|
||||
from core.utils.dicts import lowercase_keys
|
||||
@@ -3282,7 +3286,9 @@ class CommentViewSet(
|
||||
return context
|
||||
|
||||
def perform_create(self, serializer):
|
||||
"""Attach the request user as the comment author."""
|
||||
"""Attach the request user as the comment author and notify the
|
||||
participants of the thread, a comment here always replies to an existing
|
||||
thread (the first comment of a thread is created with the thread)."""
|
||||
user = self.request.user if self.request.user.is_authenticated else None
|
||||
comment = serializer.save(user=user)
|
||||
|
||||
@@ -3293,6 +3299,15 @@ class CommentViewSet(
|
||||
document=self.get_document_or_404(),
|
||||
)
|
||||
|
||||
# Notifying is best effort: the comment is already saved and a broker
|
||||
# outage must not turn its creation into an error
|
||||
try:
|
||||
send_thread_reply_notification_mail.delay(str(comment.id))
|
||||
except Exception: # pylint: disable=broad-exception-caught
|
||||
logger.exception(
|
||||
"unable to queue the notification of comment %s", comment.id
|
||||
)
|
||||
|
||||
@drf.decorators.action(
|
||||
detail=True,
|
||||
methods=["post", "delete"],
|
||||
|
||||
@@ -45,3 +45,18 @@ def send_mention_notification_mail(mention_id):
|
||||
},
|
||||
document=mention.document,
|
||||
)
|
||||
|
||||
|
||||
@app.task
|
||||
def send_thread_reply_notification_mail(comment_id):
|
||||
"""Notify the participants of a thread of a new reply, outside the
|
||||
request/response cycle."""
|
||||
try:
|
||||
comment = models.Comment.objects.select_related(
|
||||
"user", "thread__creator", "thread__document"
|
||||
).get(id=comment_id)
|
||||
except models.Comment.DoesNotExist:
|
||||
# The comment was deleted before the task ran, nothing to announce
|
||||
return
|
||||
|
||||
comment.notify_thread_participants()
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
"""
|
||||
Tests for the email notification of the participants of a thread when a
|
||||
comment is added to it, through the comments API and the Celery task.
|
||||
"""
|
||||
|
||||
from unittest import mock
|
||||
|
||||
from django.core import mail
|
||||
|
||||
import pytest
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from core import factories, models
|
||||
from core.tasks.mail import send_thread_reply_notification_mail
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
def _post_comment(user, document, thread, body="reply"):
|
||||
"""Post a comment on a thread as the given user."""
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
return client.post(
|
||||
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/",
|
||||
{"body": body},
|
||||
)
|
||||
|
||||
|
||||
def _setup():
|
||||
"""Return a document, a thread with a first comment and the replying user."""
|
||||
document = factories.DocumentFactory(link_reach="restricted", title="My doc")
|
||||
creator = factories.UserFactory(language="en-us")
|
||||
factories.UserDocumentAccessFactory(
|
||||
document=document, user=creator, role="commenter"
|
||||
)
|
||||
thread = factories.ThreadFactory(document=document, creator=creator)
|
||||
factories.CommentFactory(thread=thread, user=creator)
|
||||
replier = factories.UserFactory(full_name="Replying User")
|
||||
factories.UserDocumentAccessFactory(document=document, user=replier, role="editor")
|
||||
return document, thread, creator, replier
|
||||
|
||||
|
||||
def test_api_comments_reply_notifies_participants():
|
||||
"""Replying in a thread emails the participants, not the author of the reply."""
|
||||
document, thread, creator, replier = _setup()
|
||||
earlier = factories.UserFactory(language="en-us")
|
||||
factories.UserDocumentAccessFactory(document=document, user=earlier, role="editor")
|
||||
factories.CommentFactory(thread=thread, user=earlier)
|
||||
# Collaborator who did not take part in the thread
|
||||
bystander = factories.UserFactory()
|
||||
factories.UserDocumentAccessFactory(
|
||||
document=document, user=bystander, role="editor"
|
||||
)
|
||||
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
# pylint: disable=no-member
|
||||
assert sorted(email.to[0] for email in mail.outbox) == sorted(
|
||||
[creator.email, earlier.email]
|
||||
)
|
||||
email = next(email for email in mail.outbox if email.to == [creator.email])
|
||||
assert email.subject.lower() == (
|
||||
'replying user replied in a comment thread in "my doc"'
|
||||
)
|
||||
assert f"docs/{document.id!s}/" in email.body
|
||||
|
||||
|
||||
def test_api_comments_reply_does_not_notify_on_thread_creation():
|
||||
"""Creating a thread (and its first comment) does not notify anybody."""
|
||||
document = factories.DocumentFactory(link_reach="restricted")
|
||||
author = factories.UserFactory()
|
||||
factories.UserDocumentAccessFactory(document=document, user=author, role="editor")
|
||||
other = factories.UserFactory()
|
||||
factories.UserDocumentAccessFactory(document=document, user=other, role="editor")
|
||||
factories.ThreadFactory(document=document, creator=other)
|
||||
|
||||
client = APIClient()
|
||||
client.force_login(author)
|
||||
response = client.post(
|
||||
f"/api/v1.0/documents/{document.id!s}/threads/", {"body": "first"}
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert models.Comment.objects.filter(thread_id=response.json()["id"]).count() == 1
|
||||
# pylint: disable-next=no-member
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
|
||||
def test_api_comments_reply_nobody_to_notify():
|
||||
"""Replying to your own thread notifies nobody."""
|
||||
document, thread, creator, _replier = _setup()
|
||||
|
||||
response = _post_comment(creator, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
# pylint: disable-next=no-member
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
|
||||
def test_api_comments_reply_skips_reader_and_user_without_access():
|
||||
"""Participants who cannot see the thread anymore are not notified."""
|
||||
document, thread, creator, replier = _setup()
|
||||
reader = factories.UserFactory()
|
||||
factories.UserDocumentAccessFactory(document=document, user=reader, role="reader")
|
||||
former = factories.UserFactory()
|
||||
factories.CommentFactory(thread=thread, user=reader)
|
||||
factories.CommentFactory(thread=thread, user=former)
|
||||
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
# pylint: disable=no-member
|
||||
assert [email.to for email in mail.outbox] == [[creator.email]]
|
||||
|
||||
|
||||
def test_api_comments_reply_with_mention_sends_a_single_email():
|
||||
"""
|
||||
A user mentioned by the replier is emailed once: by the mention
|
||||
notification, not again by the reply notification.
|
||||
"""
|
||||
document, thread, creator, replier = _setup()
|
||||
mentioned = factories.UserFactory()
|
||||
factories.UserDocumentAccessFactory(
|
||||
document=document, user=mentioned, role="commenter"
|
||||
)
|
||||
factories.CommentFactory(thread=thread, user=mentioned)
|
||||
|
||||
client = APIClient()
|
||||
client.force_login(replier)
|
||||
response = client.post(
|
||||
f"/api/v1.0/documents/{document.id!s}/mention/",
|
||||
{
|
||||
"anchor_id": "3f2b0f4e-5c55-4f0b-9a3e-2d1f5c1b0a11",
|
||||
"mentioned_user_id": str(mentioned.id),
|
||||
"thread_id": str(thread.id),
|
||||
},
|
||||
)
|
||||
assert response.status_code == 201
|
||||
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
# pylint: disable=no-member
|
||||
recipients = [email.to[0] for email in mail.outbox]
|
||||
assert sorted(recipients) == sorted([mentioned.email, creator.email])
|
||||
assert recipients.count(mentioned.email) == 1
|
||||
|
||||
|
||||
def test_api_comments_reply_notification_is_asynchronous():
|
||||
"""The notification is queued, the request does not wait for the emails."""
|
||||
document, thread, _creator, replier = _setup()
|
||||
|
||||
with mock.patch(
|
||||
"core.api.viewsets.send_thread_reply_notification_mail.delay"
|
||||
) as mock_delay:
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
mock_delay.assert_called_once_with(response.json()["id"])
|
||||
# pylint: disable-next=no-member
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
|
||||
def test_api_comments_reply_queue_failure_does_not_fail_the_creation():
|
||||
"""A broker outage must not turn the creation of a comment into an error."""
|
||||
document, thread, _creator, replier = _setup()
|
||||
|
||||
with mock.patch(
|
||||
"core.api.viewsets.send_thread_reply_notification_mail.delay",
|
||||
side_effect=ConnectionError("broker down"),
|
||||
):
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert models.Comment.objects.filter(id=response.json()["id"]).exists()
|
||||
|
||||
|
||||
def test_api_comments_reply_mail_failure_does_not_fail_the_creation():
|
||||
"""A failure while sending an email must not fail the creation of a comment."""
|
||||
document, thread, _creator, replier = _setup()
|
||||
|
||||
with mock.patch.object(models.Document, "send_email", side_effect=RuntimeError):
|
||||
response = _post_comment(replier, document, thread)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert models.Comment.objects.filter(id=response.json()["id"]).exists()
|
||||
|
||||
|
||||
def test_tasks_send_thread_reply_notification_mail():
|
||||
"""The task emails the participants of the thread of the comment."""
|
||||
document, thread, creator, replier = _setup()
|
||||
reply = factories.CommentFactory(thread=thread, user=replier)
|
||||
|
||||
send_thread_reply_notification_mail(str(reply.id))
|
||||
|
||||
# pylint: disable=no-member
|
||||
assert [email.to for email in mail.outbox] == [[creator.email]]
|
||||
assert document.title.lower() in mail.outbox[0].subject.lower()
|
||||
|
||||
|
||||
def test_tasks_send_thread_reply_notification_mail_deleted_comment():
|
||||
"""The task does nothing if the comment was deleted in the meantime."""
|
||||
_document, thread, _creator, replier = _setup()
|
||||
reply = factories.CommentFactory(thread=thread, user=replier)
|
||||
comment_id = str(reply.id)
|
||||
reply.delete()
|
||||
|
||||
send_thread_reply_notification_mail(comment_id)
|
||||
|
||||
# pylint: disable-next=no-member
|
||||
assert len(mail.outbox) == 0
|
||||
Reference in New Issue
Block a user