From f18b6c0e5d558d730f2dc4bc1b3a11c7777c8787 Mon Sep 17 00:00:00 2001 From: Mohamed El Amine BOUKERFA Date: Fri, 12 Jun 2026 00:35:56 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20mention=20endpoint?= =?UTF-8?q?=20with=20email=20notification?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow users with at least the commenter role on a document to mention other collaborators in the document body or in a comment thread via POST /documents/{id}/mention/. The mentioned user must already have access to the document. The mention record is always created, and the mentioned user is notified by email with a link to the anchor, unless a notification was already sent in the same context (document body or specific thread) within a configurable cooldown period (15 minutes by default). Signed-off-by: Mohamed El Amine BOUKERFA --- CHANGELOG.md | 3 + src/backend/core/api/serializers.py | 61 +++ src/backend/core/api/viewsets.py | 46 ++ src/backend/core/factories.py | 12 + src/backend/core/migrations/0035_mention.py | 107 ++++ src/backend/core/models.py | 131 +++++ .../documents/test_api_documents_mention.py | 506 ++++++++++++++++++ .../documents/test_api_documents_retrieve.py | 5 + .../documents/test_api_documents_trashbin.py | 2 + .../core/tests/test_models_documents.py | 12 + .../core/tests/test_models_mentions.py | 83 +++ src/backend/core/utils/analytics.py | 3 + src/backend/impress/settings.py | 5 + 13 files changed, 976 insertions(+) create mode 100644 src/backend/core/migrations/0035_mention.py create mode 100644 src/backend/core/tests/documents/test_api_documents_mention.py create mode 100644 src/backend/core/tests/test_models_mentions.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c6e9bde2e..f3b29188d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to ### Added +- ✨(backend) add mention endpoint with cooldown-limited email + notification #2447 - 🚩(setting) add feature flag on Duplicate with Children #2721 - 💄(frontend) redesign 404 error standalone page #2696 - 💄(frontend) redesign 403 access denied page #2720 @@ -49,6 +51,7 @@ and this project adheres to - ✨(collaboration) add an admin reset-connections endpoint on yhub - ✨(collaboration) add a create-ydoc endpoint on yhub - 🔧(backend) fine tune redis cache options +- ✨(frontend) make the full last-update date available #1215 - ✨(collaboration) soft-migrate legacy S3 documents into yhub - ✨(collaboration) replay legacy s3 version history into yhub - ✨(backend) add a service to call the yhub REST API diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 75290127b..3e5e38194 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -983,6 +983,67 @@ class ThreadSerializer(serializers.ModelSerializer): return {} +class MentionSerializer(serializers.ModelSerializer): + """Serialize mentions of users in a document body or comment thread. + + Expects the document on which the mention is created in the context. + """ + + document_id = serializers.PrimaryKeyRelatedField(source="document", read_only=True) + mentioned_user_id = serializers.PrimaryKeyRelatedField( + queryset=models.User.objects.filter(is_active=True), + source="mentioned_user", + ) + mentioned_by_user_id = serializers.PrimaryKeyRelatedField( + source="mentioned_by_user", read_only=True + ) + thread_id = serializers.PrimaryKeyRelatedField( + queryset=models.Thread.objects.all(), + source="thread", + required=False, + allow_null=True, + default=None, + ) + + class Meta: + model = models.Mention + fields = [ + "id", + "document_id", + "anchor_id", + "thread_id", + "mentioned_user_id", + "mentioned_by_user_id", + "created_at", + "notified_at", + ] + read_only_fields = [ + "id", + "document_id", + "mentioned_by_user_id", + "created_at", + "notified_at", + ] + + def validate_mentioned_user_id(self, user): + """Ensure the mentioned user has access to the document.""" + document = models.Document.objects.get(pk=self.context["document"].pk) + if document.get_role(user) is None: + raise serializers.ValidationError( + "This user does not have access to the document." + ) + return user + + def validate_thread_id(self, thread): + """Ensure the thread belongs to the document on which the mention is created.""" + document = self.context["document"] + if thread is not None and thread.document_id != document.id: + raise serializers.ValidationError( + "The thread does not belong to this document." + ) + return thread + + class SearchQueryParamDocumentSerializer(serializers.Serializer): """Serializer for fulltext search requests through Find application""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 3118ee701..32bf33606 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -515,6 +515,16 @@ class DocumentViewSet( 13. **AI Proxy**: Proxy an AI request to an external AI service. Example: POST /api/v1.0/documents//ai-proxy + 16. **Mention**: Mention a user on the document and notify them by email. + Example: POST /documents/{id}/mention/ + Expected data: + - anchor_id (str): The location of the mention, used for the email deeplink. + - mentioned_user_id (uuid): The user being mentioned, must have access + to the document. + - thread_id (uuid, optional): The comment thread in which the mention + occurs. Omit for mentions in the document body. + Returns: 201 with the created mention. + ### Ordering: created_at, updated_at, is_favorite, title Example: @@ -1867,6 +1877,42 @@ class DocumentViewSet( status=drf.status.HTTP_200_OK, ) + @drf.decorators.action(detail=True, methods=["post"], url_path="mention") + def mention(self, request, *args, **kwargs): + """Mention a user on the document and notify them by email. + + The mention record is always created; the email notification is + suppressed when the same user was already notified in the same context + (document body or thread) within the cooldown period. + """ + # Check permissions first + document = self.get_object() + + serializer = serializers.MentionSerializer( + data=request.data, + context={**self.get_serializer_context(), "document": document}, + ) + serializer.is_valid(raise_exception=True) + mention = serializer.save(document=document, mentioned_by_user=request.user) + + mention.notify() + + posthog_capture( + PosthogEventName.MENTION_CREATED, + request.user, + { + "mention_id": str(mention.id), + "mentioned_user_id": str(mention.mentioned_user_id), + "thread_id": str(mention.thread_id) if mention.thread_id else None, + "notified": mention.notified_at is not None, + }, + document=document, + ) + + return drf.response.Response( + serializer.data, status=drf.status.HTTP_201_CREATED + ) + @drf.decorators.action(detail=True, methods=["post"], url_path="attachment-upload") def attachment_upload(self, request, *args, **kwargs): """Upload a file related to a given document""" diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 7d3d49054..8684cb413 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -256,3 +256,15 @@ class ReactionFactory(factory.django.DjangoModelFactory): # Add the iterable of groups using bulk addition self.users.add(*extracted) + + +class MentionFactory(factory.django.DjangoModelFactory): + """A factory to create mentions of users on a document""" + + class Meta: + model = models.Mention + + document = factory.SubFactory(DocumentFactory) + anchor_id = factory.Sequence(lambda n: f"block-{n}") + mentioned_user = factory.SubFactory(UserFactory) + mentioned_by_user = factory.SubFactory(UserFactory) diff --git a/src/backend/core/migrations/0035_mention.py b/src/backend/core/migrations/0035_mention.py new file mode 100644 index 000000000..d5b0eaf5a --- /dev/null +++ b/src/backend/core/migrations/0035_mention.py @@ -0,0 +1,107 @@ +# Generated by Django 5.2 - create the mention model + +import uuid + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0034_documentmigration"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="Mention", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + help_text="primary key for the record as UUID", + primary_key=True, + serialize=False, + verbose_name="id", + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="date and time at which a record was created", + verbose_name="created on", + ), + ), + ( + "updated_at", + models.DateTimeField( + auto_now=True, + help_text="date and time at which a record was last updated", + verbose_name="updated on", + ), + ), + ("anchor_id", models.TextField()), + ("notified_at", models.DateTimeField(blank=True, null=True)), + ( + "document", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="mentions", + to="core.document", + ), + ), + ( + "mentioned_by_user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="mentions_sent", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "mentioned_user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="mentions_received", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "thread", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="mentions", + to="core.thread", + ), + ), + ], + options={ + "verbose_name": "Mention", + "verbose_name_plural": "Mentions", + "db_table": "impress_mention", + "ordering": ("-created_at",), + }, + ), + migrations.AddIndex( + model_name="mention", + index=models.Index( + fields=["mentioned_user", "-created_at"], + name="mention_user_created_idx", + ), + ), + migrations.AddIndex( + model_name="mention", + index=models.Index( + fields=["document", "mentioned_user"], + name="mention_document_user_idx", + ), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 054e1f94f..e2da00912 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1323,6 +1323,7 @@ class Document(MP_Node, BaseModel): "link_configuration": is_owner_or_admin, "invite_owner": is_owner and not is_deleted, "leave": can_leave, + "mention": can_comment and user.is_authenticated, "move": is_owner_or_admin and not is_deleted, "partial_update": can_update, "restore": is_owner and bool(self.deleted_at), @@ -1953,6 +1954,136 @@ class Reaction(BaseModel): return f"Reaction {self.emoji} on comment {self.comment.id}" +class Mention(BaseModel): + """A mention of a user in a document body or in a comment thread. + + A mention record is always created, but the email notification is only + sent if no notification was already sent to the same user in the same + context (the document or a specific thread) within the cooldown period. + """ + + document = models.ForeignKey( + Document, + on_delete=models.CASCADE, + related_name="mentions", + ) + anchor_id = models.TextField() + thread = models.ForeignKey( + Thread, + on_delete=models.CASCADE, + related_name="mentions", + null=True, + blank=True, + ) + mentioned_user = models.ForeignKey( + User, + on_delete=models.SET_NULL, + related_name="mentions_received", + null=True, + blank=True, + ) + mentioned_by_user = models.ForeignKey( + User, + on_delete=models.CASCADE, + related_name="mentions_sent", + ) + notified_at = models.DateTimeField(null=True, blank=True) + + class Meta: + db_table = "impress_mention" + ordering = ("-created_at",) + verbose_name = _("Mention") + verbose_name_plural = _("Mentions") + indexes = [ + models.Index( + fields=["mentioned_user", "-created_at"], + name="mention_user_created_idx", + ), + models.Index( + fields=["document", "mentioned_user"], + name="mention_document_user_idx", + ), + ] + + def __str__(self): + mentioned = self.mentioned_user or _("a deleted user") + return ( + f"{self.mentioned_by_user!s} mentioned {mentioned!s} on {self.document!s}" + ) + + def clean(self): + """Validate that the thread, if any, belongs to the mention's document.""" + super().clean() + if self.thread_id and self.thread.document_id != self.document_id: + raise ValidationError( + {"thread_id": [_("The thread does not belong to this document.")]} + ) + + def is_notification_in_cooldown(self): + """Return whether the mentioned user was already notified in the same context + (same document and thread, the document body when the thread is null) within + the cooldown period.""" + cooldown_start = timezone.now() - timedelta( + minutes=settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES + ) + return ( + self._meta.model.objects.filter( + document_id=self.document_id, + mentioned_user_id=self.mentioned_user_id, + thread_id=self.thread_id, + notified_at__isnull=False, + created_at__gt=cooldown_start, + ) + .exclude(pk=self.pk) + .exists() + ) + + def notify(self, language=None): + """Send the mention notification email unless the context is in cooldown. + + Set `notified_at` on the mention and return True if an email was sent, + return False otherwise. + """ + user = self.mentioned_user + if user is None or not user.email or self.is_notification_in_cooldown(): + return False + + sender = self.mentioned_by_user + language = ( + language or user.language or sender.language or settings.LANGUAGE_CODE + ) + sender_name = sender.full_name or sender.email + domain = settings.EMAIL_URL_APP or Site.objects.get_current().domain + + with override(language): + title = self.document.title or str(_("Untitled Document")) + if self.thread_id: + subject = _( + "You were mentioned in a comment on the document {title}" + ).format(title=title) + message = _( + "{name} mentioned you in a comment on the following document:" + ).format(name=sender_name) + else: + subject = _("You were mentioned in the document {title}").format( + title=title + ) + message = _("{name} mentioned you in the following document:").format( + name=sender_name + ) + context = { + "title": subject, + "message": message, + "link": f"{domain}/docs/{self.document_id}/#{self.anchor_id}", + } + + self.document.send_email(subject, [user.email], context, language) + + self.notified_at = timezone.now() + self.save(update_fields=["notified_at", "updated_at"]) + return True + + class Invitation(BaseModel): """User invitation to a document.""" diff --git a/src/backend/core/tests/documents/test_api_documents_mention.py b/src/backend/core/tests/documents/test_api_documents_mention.py new file mode 100644 index 000000000..b1fa23c5d --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_mention.py @@ -0,0 +1,506 @@ +""" +Tests for Documents API endpoint in impress's core app: mention +""" + +import random +from datetime import timedelta + +from django.core import mail +from django.utils import timezone + +import pytest +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +def test_api_documents_mention_anonymous(): + """Anonymous users should not be allowed to mention users on a document.""" + document = factories.DocumentFactory() + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + response = APIClient().post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 401 + assert models.Mention.objects.exists() is False + assert len(mail.outbox) == 0 + + +def test_api_documents_mention_anonymous_public_document(): + """ + Anonymous users should not be allowed to mention users, even on public + documents with a commenter link role. + """ + document = factories.DocumentFactory(link_reach="public", link_role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + response = APIClient().post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 401 + assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_authenticated_no_access(): + """ + Authenticated users with no access to a restricted document should not be + allowed to mention users on it. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 403 + assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_authenticated_reader(): + """Users with a reader role on a document should not be allowed to mention.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="reader") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 403 + assert models.Mention.objects.exists() is False + + +@pytest.mark.parametrize("role", ["commenter", "editor", "administrator", "owner"]) +def test_api_documents_mention_authenticated_success(role): + """ + Users with at least a commenter role should be allowed to mention another + collaborator; the mention is created and an email notification is sent. + """ + user = factories.UserFactory(full_name="Mentioning User") + document = factories.DocumentFactory(link_reach="restricted", title="My doc") + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + mentioned_user = factories.UserFactory(language="en-us") + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 201 + + mention = models.Mention.objects.get() + assert mention.document == document + assert mention.anchor_id == "block-1" + assert mention.thread is None + assert mention.mentioned_user == mentioned_user + assert mention.mentioned_by_user == user + assert mention.notified_at is not None + + content = response.json() + assert content == { + "id": str(mention.id), + "document_id": str(document.id), + "anchor_id": "block-1", + "thread_id": None, + "mentioned_user_id": str(mentioned_user.id), + "mentioned_by_user_id": str(user.id), + "created_at": mention.created_at.isoformat().replace("+00:00", "Z"), + "notified_at": mention.notified_at.isoformat().replace("+00:00", "Z"), + } + + assert len(mail.outbox) == 1 + email = mail.outbox[0] + assert email.to == [mentioned_user.email] + assert "you were mentioned in the document my doc" in email.subject.lower() + email_content = " ".join(email.body.split()) + assert "Mentioning User mentioned you in the following document" in email_content + assert f"docs/{document.id!s}/#block-1" in email_content + + +def test_api_documents_mention_via_link_role(): + """ + Authenticated users allowed to comment via the document link role should be + allowed to mention collaborators. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="public", link_role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 201 + assert len(mail.outbox) == 1 + + +def test_api_documents_mention_missing_anchor_id(): + """The anchor_id field should be required.""" + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 400 + assert response.json() == {"anchor_id": ["This field is required."]} + assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_unknown_user(): + """Mentioning a user that does not exist should receive a 400 error.""" + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + { + "anchor_id": "block-1", + "mentioned_user_id": "8f850ee5-86b2-4c9f-acdb-ef9ccb8a4bbc", + }, + ) + + assert response.status_code == 400 + assert "mentioned_user_id" in response.json() + assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_user_without_access(): + """Mentioning a user that has no access to the document should be impossible.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="public", link_role="editor") + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 400 + assert response.json() == { + "mentioned_user_id": ["This user does not have access to the document."] + } + assert models.Mention.objects.exists() is False + assert len(mail.outbox) == 0 + + +def test_api_documents_mention_user_with_access_on_ancestor(): + """Users with access inherited from an ancestor document can be mentioned.""" + user = factories.UserFactory() + parent = factories.DocumentFactory() + document = factories.DocumentFactory(parent=parent) + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=parent, user=mentioned_user) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 201 + assert models.Mention.objects.count() == 1 + assert len(mail.outbox) == 1 + + +def test_api_documents_mention_user_with_access_via_team(mock_user_teams): + """Users with access via a team can be mentioned.""" + mock_user_teams.return_value = ["lasuite"] + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.TeamDocumentAccessFactory(document=document, team="lasuite") + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 201 + assert len(mail.outbox) == 1 + + +def test_api_documents_mention_thread(): + """ + Mentions can be attached to a comment thread of the document, in which case + the email notification mentions the comment. + """ + user = factories.UserFactory(full_name="Mentioning User") + document = factories.DocumentFactory(title="My doc") + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + thread = factories.ThreadFactory(document=document) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + { + "anchor_id": "comment-1", + "mentioned_user_id": str(mentioned_user.id), + "thread_id": str(thread.id), + }, + ) + + assert response.status_code == 201 + + mention = models.Mention.objects.get() + assert mention.thread == thread + assert response.json()["thread_id"] == str(thread.id) + + assert len(mail.outbox) == 1 + email = mail.outbox[0] + assert ( + "you were mentioned in a comment on the document my doc" + in email.subject.lower() + ) + email_content = " ".join(email.body.split()) + assert ( + "Mentioning User mentioned you in a comment on the following document" + in email_content + ) + assert f"docs/{document.id!s}/#comment-1" in email_content + + +def test_api_documents_mention_thread_other_document(): + """Mentions cannot be attached to a thread from another document.""" + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + other_thread = factories.ThreadFactory() + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + { + "anchor_id": "comment-1", + "mentioned_user_id": str(mentioned_user.id), + "thread_id": str(other_thread.id), + }, + ) + + assert response.status_code == 400 + assert response.json() == { + "thread_id": ["The thread does not belong to this document."] + } + assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_cooldown_same_context(): + """ + A user mentioned several times in the same context within the cooldown + period should be notified only once, but all mentions should be recorded. + """ + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + payload = {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)} + + response = client.post(f"/api/v1.0/documents/{document.id!s}/mention/", payload) + assert response.status_code == 201 + assert response.json()["notified_at"] is not None + assert len(mail.outbox) == 1 + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-2", "mentioned_user_id": str(mentioned_user.id)}, + ) + assert response.status_code == 201 + assert response.json()["notified_at"] is None + assert len(mail.outbox) == 1 + + assert models.Mention.objects.count() == 2 + + +def test_api_documents_mention_cooldown_distinct_contexts(): + """ + The notification cooldown applies per context: the document body and each + thread are separate contexts. + """ + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + thread1, thread2 = factories.ThreadFactory.create_batch(2, document=document) + + client = APIClient() + client.force_login(user) + + for i, thread_id in enumerate([None, str(thread1.id), str(thread2.id)]): + payload = { + "anchor_id": f"block-{i}", + "mentioned_user_id": str(mentioned_user.id), + } + if thread_id: + payload["thread_id"] = thread_id + response = client.post(f"/api/v1.0/documents/{document.id!s}/mention/", payload) + assert response.status_code == 201 + assert response.json()["notified_at"] is not None + + assert len(mail.outbox) == 3 + + # Mentioning again in one of the contexts should not send a new email + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + { + "anchor_id": "block-4", + "mentioned_user_id": str(mentioned_user.id), + "thread_id": str(thread1.id), + }, + ) + assert response.status_code == 201 + assert response.json()["notified_at"] is None + assert len(mail.outbox) == 3 + + # The cooldown should apply per mentioned user + other_mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=other_mentioned_user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + { + "anchor_id": "block-5", + "mentioned_user_id": str(other_mentioned_user.id), + "thread_id": str(thread1.id), + }, + ) + assert response.status_code == 201 + assert response.json()["notified_at"] is not None + assert len(mail.outbox) == 4 + + +def test_api_documents_mention_cooldown_expired(settings): + """A user mentioned again after the cooldown period should be notified again.""" + settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES = random.randint(1, 120) + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + assert response.status_code == 201 + assert len(mail.outbox) == 1 + + # Age the first mention beyond the cooldown period + expired = timezone.now() - timedelta( + minutes=settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES + 1 + ) + models.Mention.objects.update(created_at=expired, notified_at=expired) + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-2", "mentioned_user_id": str(mentioned_user.id)}, + ) + assert response.status_code == 201 + assert response.json()["notified_at"] is not None + assert len(mail.outbox) == 2 + + +def test_api_documents_mention_cooldown_only_considers_notified_mentions(): + """ + Mentions that did not trigger a notification should not be taken into + account by the cooldown check. + """ + user = factories.UserFactory() + document = factories.DocumentFactory() + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + factories.MentionFactory( + document=document, + mentioned_user=mentioned_user, + mentioned_by_user=user, + notified_at=None, + ) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 201 + assert response.json()["notified_at"] is not None + assert len(mail.outbox) == 1 + + +def test_api_documents_mention_soft_deleted_document(): + """Mentions should not be allowed on soft deleted documents.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") + mentioned_user = factories.UserFactory() + factories.UserDocumentAccessFactory(document=document, user=mentioned_user) + document.soft_delete() + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)}, + ) + + assert response.status_code == 404 + assert models.Mention.objects.exists() is False diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index bd6d0f697..4f03706f7 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -54,6 +54,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "leave": False, "media_auth": True, "media_check": True, + "mention": False, "move": False, "partial_update": document.link_role == "editor", "restore": False, @@ -128,6 +129,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "leave": False, "media_auth": True, "media_check": True, + "mention": False, "move": False, "partial_update": grand_parent.link_role == "editor", "restore": False, @@ -235,6 +237,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "leave": True, "media_auth": True, "media_check": True, + "mention": document.link_role in ["commenter", "editor"], "move": False, "partial_update": document.link_role == "editor", "restore": False, @@ -317,6 +320,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "leave": True, "media_auth": True, "media_check": True, + "mention": grand_parent.link_role in ["commenter", "editor"], "partial_update": grand_parent.link_role == "editor", "restore": False, "retrieve": True, @@ -511,6 +515,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "leave": access.role not in ["administrator", "owner"], "media_auth": True, "media_check": True, + "mention": access.role != "reader", "move": access.role in ["administrator", "owner"], "partial_update": access.role not in ["reader", "commenter"], "restore": False, diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 5e87a738c..cf5407130 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -103,6 +103,7 @@ def test_api_documents_trashbin_format(): "leave": False, "media_auth": False, "media_check": False, + "mention": False, "move": False, # Can't move a deleted document "partial_update": False, "restore": True, @@ -166,6 +167,7 @@ def test_api_documents_trashbin_format(): "leave": False, "media_auth": False, "media_check": False, + "mention": False, "move": False, # Can't move a deleted document "partial_update": False, "restore": True, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 60d2387ce..fd835f6ff 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -172,6 +172,7 @@ def test_models_documents_get_abilities_forbidden( "leave": False, "media_auth": False, "media_check": False, + "mention": False, "move": False, "link_configuration": False, "link_select_options": { @@ -242,6 +243,7 @@ def test_models_documents_get_abilities_reader( "leave": False, "media_auth": True, "media_check": True, + "mention": False, "move": False, "partial_update": False, "restore": False, @@ -311,6 +313,7 @@ def test_models_documents_get_abilities_commenter( "leave": False, "media_auth": True, "media_check": True, + "mention": is_authenticated, "move": False, "partial_update": False, "restore": False, @@ -377,6 +380,7 @@ def test_models_documents_get_abilities_editor( "leave": False, "media_auth": True, "media_check": True, + "mention": is_authenticated, "move": False, "partial_update": True, "restore": False, @@ -432,6 +436,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "leave": False, "media_auth": True, "media_check": True, + "mention": True, "move": True, "partial_update": True, "restore": False, @@ -473,6 +478,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "leave": False, "media_auth": False, "media_check": False, + "mention": False, "move": False, "partial_update": False, "restore": True, @@ -518,6 +524,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "leave": False, "media_auth": True, "media_check": True, + "mention": True, "move": True, "partial_update": True, "restore": False, @@ -573,6 +580,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "leave": True, "media_auth": True, "media_check": True, + "mention": True, "move": False, "partial_update": True, "restore": False, @@ -636,6 +644,8 @@ def test_models_documents_get_abilities_reader_user( "leave": True, "media_auth": True, "media_check": True, + "mention": document.link_reach != "restricted" + and document.link_role in ["commenter", "editor"], "move": False, "partial_update": access_from_link, "restore": False, @@ -700,6 +710,7 @@ def test_models_documents_get_abilities_commenter_user( "leave": True, "media_auth": True, "media_check": True, + "mention": True, "move": False, "partial_update": access_from_link, "restore": False, @@ -760,6 +771,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "leave": True, "media_auth": True, "media_check": True, + "mention": False, "move": False, "partial_update": False, "restore": False, diff --git a/src/backend/core/tests/test_models_mentions.py b/src/backend/core/tests/test_models_mentions.py new file mode 100644 index 000000000..b906299c7 --- /dev/null +++ b/src/backend/core/tests/test_models_mentions.py @@ -0,0 +1,83 @@ +""" +Unit tests for the Mention model +""" + +from django.core import mail + +import pytest +from rest_framework.exceptions import ValidationError + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +def test_models_mentions_str(): + """The str representation should mention both users and the document.""" + mention = factories.MentionFactory( + document__title="My doc", + mentioned_user__email="mentioned@example.com", + mentioned_user__full_name=None, + mentioned_by_user__email="author@example.com", + mentioned_by_user__full_name=None, + ) + assert ( + str(mention) == "author@example.com mentioned mentioned@example.com on My doc" + ) + + +def test_models_mentions_thread_other_document(): + """A mention cannot reference a thread from another document.""" + thread = factories.ThreadFactory() + + with pytest.raises(ValidationError) as excinfo: + factories.MentionFactory(thread=thread) + + assert "The thread does not belong to this document." in str(excinfo.value) + + +def test_models_mentions_document_deletion_cascades(): + """Deleting a document should delete its mentions.""" + mention = factories.MentionFactory() + mention.document.delete() + assert models.Mention.objects.exists() is False + + +def test_models_mentions_thread_deletion_cascades(): + """Deleting a thread should delete the mentions linked to it.""" + document = factories.DocumentFactory() + thread = factories.ThreadFactory(document=document) + factories.MentionFactory(document=document, thread=thread) + mention_in_body = factories.MentionFactory(document=document) + + thread.delete() + + assert list(models.Mention.objects.all()) == [mention_in_body] + + +def test_models_mentions_mentioned_user_deletion_sets_null(): + """Deleting the mentioned user should preserve the mention with a null user.""" + mention = factories.MentionFactory() + mention.mentioned_user.delete() + + mention.refresh_from_db() + assert mention.mentioned_user is None + + +def test_models_mentions_mentioned_by_user_deletion_cascades(): + """Deleting the mentioning user should delete the mention.""" + mention = factories.MentionFactory() + mention.mentioned_by_user.delete() + assert models.Mention.objects.exists() is False + + +def test_models_mentions_notify_mentioned_user_deleted(): + """Notifying a mention whose mentioned user was deleted should do nothing.""" + mention = factories.MentionFactory() + mention.mentioned_user.delete() + mention.refresh_from_db() + + assert mention.notify() is False + assert mention.notified_at is None + # pylint: disable-next=no-member + assert len(mail.outbox) == 0 diff --git a/src/backend/core/utils/analytics.py b/src/backend/core/utils/analytics.py index b4b6c2d54..14df8e90e 100644 --- a/src/backend/core/utils/analytics.py +++ b/src/backend/core/utils/analytics.py @@ -34,6 +34,9 @@ class PosthogEventName(StrEnum): # Comment COMMENT_CREATED = "comment_created" + # Mention + MENTION_CREATED = "mention_created" + # User USER_LOGIN = "user_login" diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 7b2c9cf6a..5d93fa548 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -517,6 +517,11 @@ class Base(Configuration): 30, environ_name="TRASHBIN_CUTOFF_DAYS", environ_prefix=None ) + # Mentions + MENTION_NOTIFICATION_COOLDOWN_MINUTES = values.IntegerValue( + 15, environ_name="MENTION_NOTIFICATION_COOLDOWN_MINUTES", environ_prefix=None + ) + # Mail EMAIL_BACKEND = values.Value("django.core.mail.backends.smtp.EmailBackend") EMAIL_BRAND_NAME = values.Value(None)