From c8ae3664390b22d6eead75bacb07d804e9a31445 Mon Sep 17 00:00:00 2001 From: Mohamed El Amine BOUKERFA Date: Thu, 11 Jun 2026 23:47:00 +0200 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=9B=82(backend)=20make=20document=20a?= =?UTF-8?q?ccess=20list=20visible=20to=20all=20collaborators?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return all accesses on the document and its ancestors to any user with access, keeping the limited user details serializer and adding the user id to it, so that collaborators allowed to comment can identify and mention each other. Signed-off-by: Mohamed El Amine BOUKERFA --- CHANGELOG.md | 1 + src/backend/core/api/serializers.py | 4 ++-- src/backend/core/api/viewsets.py | 6 ++--- .../documents/test_api_document_accesses.py | 24 ++++++++++--------- .../documents/test_api_documents_comments.py | 6 +++++ .../documents/test_api_documents_threads.py | 4 ++++ .../serializers/test_user_light_serializer.py | 2 ++ 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d6e0721c6..94b5ee6ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to ### Changed +- 🛂(backend) make document access list visible to all collaborators - 👷(CI) remove test-e2e-other-browser job #2404 - ♿️(frontend) use heading element for pinned documents section title #2380 - ♿️(frontend) use anchor links for table of contents entries #2390 diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 4e32f84b46..55682d7459 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -73,8 +73,8 @@ class UserLightSerializer(UserSerializer): class Meta: model = models.User - fields = ["full_name", "short_name"] - read_only_fields = ["full_name", "short_name"] + fields = ["id", "full_name", "short_name"] + read_only_fields = ["id", "full_name", "short_name"] class ListDocumentSerializer(serializers.ModelSerializer): diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index f7e650edba..e9766bf0b5 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -2719,11 +2719,11 @@ def list(self, request, *args, **kwargs): | models.Document.objects.filter(pk=self.document.pk) ).filter(ancestors_deleted_at__isnull=True) + # All users with access see the full list of accesses (with limited + # user details for unprivileged roles) so that any collaborator + # allowed to comment can mention the others. queryset = self.get_queryset().filter(document__in=ancestors) - if role not in choices.PRIVILEGED_ROLES: - queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES) - accesses = list(queryset.order_by("document__path")) # Annotate more information on roles diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index 0feda4a7bc..fcdcb99fc8 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -80,8 +80,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( via, role, mock_user_teams, django_assert_num_queries ): """ - Authenticated users with no privileged role should only be able to list document - accesses associated with privileged roles for a document, including from ancestors. + Authenticated users with no privileged role should be able to list all document + accesses, including from ancestors, but with limited user information, so that + any collaborator allowed to comment can mention the others. """ user = factories.UserFactory() client = APIClient() @@ -108,18 +109,20 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( factories.UserDocumentAccessFactory(document=child) if via == USER: - models.DocumentAccess.objects.create( + user_access = models.DocumentAccess.objects.create( document=document, user=user, role=role, ) elif via == TEAM: mock_user_teams.return_value = ["lasuite", "unknown"] - models.DocumentAccess.objects.create( + user_access = models.DocumentAccess.objects.create( document=document, team="lasuite", role=role, ) + else: + raise RuntimeError() # Accesses for other documents to which the user is related should not be listed either other_access = factories.UserDocumentAccessFactory(user=user) @@ -131,11 +134,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( assert response.status_code == 200 content = response.json() - # Make sure only privileged roles are returned - privileged_accesses = [ - acc for acc in accesses if acc.role in choices.PRIVILEGED_ROLES - ] - assert len(content) == len(privileged_accesses) + # All accesses on the document and its ancestors are returned + all_accesses = [*accesses, user_access] + assert len(content) == len(all_accesses) assert sorted(content, key=lambda x: x["id"]) == sorted( [ @@ -147,6 +148,7 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( "depth": access.document.depth, }, "user": { + "id": str(access.user.id), "full_name": access.user.full_name, "short_name": access.user.short_name, } @@ -159,12 +161,12 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( "abilities": { "destroy": False, "partial_update": False, - "retrieve": False, + "retrieve": access.user is not None and access.user.id == user.id, "set_role_to": [], "update": False, }, } - for access in privileged_accesses + for access in all_accesses ], key=lambda x: x["id"], ) diff --git a/src/backend/core/tests/documents/test_api_documents_comments.py b/src/backend/core/tests/documents/test_api_documents_comments.py index e9a3b2eeb6..3f7901ddc4 100644 --- a/src/backend/core/tests/documents/test_api_documents_comments.py +++ b/src/backend/core/tests/documents/test_api_documents_comments.py @@ -41,6 +41,7 @@ def test_list_comments_anonymous_user_public_document(): "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(comment1.user.id), "full_name": comment1.user.full_name, "short_name": comment1.user.short_name, }, @@ -53,6 +54,7 @@ def test_list_comments_anonymous_user_public_document(): "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(comment2.user.id), "full_name": comment2.user.full_name, "short_name": comment2.user.short_name, }, @@ -110,6 +112,7 @@ def test_list_comments_authenticated_user_accessible_document(): "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(comment1.user.id), "full_name": comment1.user.full_name, "short_name": comment1.user.short_name, }, @@ -122,6 +125,7 @@ def test_list_comments_authenticated_user_accessible_document(): "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(comment2.user.id), "full_name": comment2.user.full_name, "short_name": comment2.user.short_name, }, @@ -263,6 +267,7 @@ def test_create_comment_authenticated_user_accessible_document(): "created_at": response.json()["created_at"], "updated_at": response.json()["updated_at"], "user": { + "id": str(user.id), "full_name": user.full_name, "short_name": user.short_name, }, @@ -317,6 +322,7 @@ def test_retrieve_comment_anonymous_user_public_document(): "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(comment.user.id), "full_name": comment.user.full_name, "short_name": comment.user.short_name, }, diff --git a/src/backend/core/tests/documents/test_api_documents_threads.py b/src/backend/core/tests/documents/test_api_documents_threads.py index 34bd34d12f..b10ec2b2ba 100644 --- a/src/backend/core/tests/documents/test_api_documents_threads.py +++ b/src/backend/core/tests/documents/test_api_documents_threads.py @@ -175,6 +175,7 @@ def test_api_documents_threads_restricted_document_editor(role): "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), "creator": { + "id": str(user.id), "full_name": user.full_name, "short_name": user.short_name, }, @@ -185,6 +186,7 @@ def test_api_documents_threads_restricted_document_editor(role): "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(user.id), "full_name": user.full_name, "short_name": user.short_name, }, @@ -296,6 +298,7 @@ def test_api_documents_threads_authenticated_document(link_role): "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), "creator": { + "id": str(user.id), "full_name": user.full_name, "short_name": user.short_name, }, @@ -306,6 +309,7 @@ def test_api_documents_threads_authenticated_document(link_role): "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), "user": { + "id": str(user.id), "full_name": user.full_name, "short_name": user.short_name, }, diff --git a/src/backend/core/tests/serializers/test_user_light_serializer.py b/src/backend/core/tests/serializers/test_user_light_serializer.py index 05bd33b4fe..f2f75478d8 100644 --- a/src/backend/core/tests/serializers/test_user_light_serializer.py +++ b/src/backend/core/tests/serializers/test_user_light_serializer.py @@ -16,8 +16,10 @@ def test_user_light_serializer(): short_name="John", ) serializer = UserLightSerializer(user) + assert serializer.data["id"] == str(user.id) assert serializer.data["full_name"] == "John Doe" assert serializer.data["short_name"] == "John" + assert "email" not in serializer.data def test_user_light_serializer_no_full_name(): From d3142b45f005e42efd23016c6bf904947ad2116d Mon Sep 17 00:00:00 2001 From: Mohamed El Amine BOUKERFA Date: Fri, 12 Jun 2026 00:35:56 +0200 Subject: [PATCH 2/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20let=20callers?= =?UTF-8?q?=20override=20document=20email=20context=20defaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need this so mention notification emails can replace the default share link with a link to the anchor of the mention. --- src/backend/core/models.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index e2cd88de2c..b25714a195 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1413,21 +1413,22 @@ def get_abilities(self, user): # pylint: disable=too-many-locals } def send_email(self, subject, emails, context=None, language=None): - """Generate and send email from a template.""" - context = context or {} + """Generate and send email from a template. + + Keys passed in `context` take precedence over the default values. + """ domain = settings.EMAIL_URL_APP or Site.objects.get_current().domain language = language or get_language() - context.update( - { - "brandname": settings.EMAIL_BRAND_NAME, - "document": self, - "domain": domain, - "link": f"{domain}/docs/{self.id}/?utm_source=docssharelink&utm_campaign={self.id}", - "link_label": self.title or str(_("Untitled Document")), - "button_label": _("Open"), - "logo_img": settings.EMAIL_LOGO_IMG, - } - ) + context = { + "brandname": settings.EMAIL_BRAND_NAME, + "document": self, + "domain": domain, + "link": f"{domain}/docs/{self.id}/?utm_source=docssharelink&utm_campaign={self.id}", + "link_label": self.title or str(_("Untitled Document")), + "button_label": _("Open"), + "logo_img": settings.EMAIL_LOGO_IMG, + **(context or {}), + } with override(language): msg_html = render_to_string("mail/html/template.html", context) From 34e0cca53a756735f2a1ce7601978f7f1f270743 Mon Sep 17 00:00:00 2001 From: Mohamed El Amine BOUKERFA Date: Fri, 12 Jun 2026 00:35:56 +0200 Subject: [PATCH 3/5] =?UTF-8?q?=E2=9C=A8(backend)=20add=20mention=20endpoi?= =?UTF-8?q?nt=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 | 1 + src/backend/core/api/serializers.py | 61 +++ src/backend/core/api/viewsets.py | 46 ++ src/backend/core/factories.py | 12 + src/backend/core/migrations/0033_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, 974 insertions(+) create mode 100644 src/backend/core/migrations/0033_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 94b5ee6ea9..02623d7958 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to ### Added - ✨(backend) add limit on distinct reactions per comment #1978 +- ✨(backend) add mention endpoint with cooldown-limited email notification - ✨(frontend) leave a document #2410 - ✨(frontend) add top parent on sub docs search #1952 - ✨(frontend) unauthenticated users can search #2407 diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 55682d7459..e634488f94 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1005,6 +1005,67 @@ def get_abilities(self, thread): 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 e9766bf0b5..a6a7ecff0d 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -517,6 +517,16 @@ class DocumentViewSet( 15. **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: @@ -1857,6 +1867,42 @@ def favorite(self, request, *args, **kwargs): 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 eeefa8f4b7..45fbd88c7d 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -246,3 +246,15 @@ def users(self, create, extracted, **kwargs): # 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/0033_mention.py b/src/backend/core/migrations/0033_mention.py new file mode 100644 index 0000000000..6d9d6bbd6d --- /dev/null +++ b/src/backend/core/migrations/0033_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", "0032_remove_linktrace_is_masked"), + 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 b25714a195..533a5a9be0 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1398,6 +1398,7 @@ def get_abilities(self, user): # pylint: disable=too-many-locals "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, @@ -2030,6 +2031,136 @@ def __str__(self): 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 0000000000..b1fa23c5d0 --- /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 db7907ffb9..6d2a0dc963 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -57,6 +57,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, @@ -136,6 +137,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, @@ -248,6 +250,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, @@ -335,6 +338,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, @@ -534,6 +538,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": access.role == "owner", 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 a6f5668eb9..d1979dd695 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -106,6 +106,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, @@ -174,6 +175,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 e9405ce38f..5d42de82d6 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -176,6 +176,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": { @@ -251,6 +252,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, @@ -325,6 +327,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, @@ -396,6 +399,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, @@ -456,6 +460,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": True, @@ -502,6 +507,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, @@ -552,6 +558,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, @@ -612,6 +619,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, @@ -680,6 +688,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, @@ -749,6 +759,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, @@ -814,6 +825,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 0000000000..b906299c70 --- /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 b4b6c2d544..14df8e90e9 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 8ba5b11497..7983522503 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -487,6 +487,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) From 9b74a2fe8953b3c50c211038ce62d4cd81239194 Mon Sep 17 00:00:00 2001 From: BOUKERFA Mohamed El Amine Date: Fri, 19 Jun 2026 08:31:55 +0200 Subject: [PATCH 4/5] =?UTF-8?q?=E2=9A=A1=EF=B8=8F(backend)=20send=20mentio?= =?UTF-8?q?n=20notification=20email=20asynchronously?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the mention email off the request/response cycle into a Celery task so the SMTP round-trip no longer delays the API response. Signed-off-by: BOUKERFA Mohamed El Amine --- src/backend/core/api/viewsets.py | 23 +++++-------------- src/backend/core/tasks/mail.py | 23 +++++++++++++++++++ .../documents/test_api_documents_mention.py | 23 +++++++++++-------- 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a6a7ecff0d..07c3e99bfb 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -68,7 +68,7 @@ get_document_indexer, get_visited_document_ids_of, ) -from core.tasks.mail import send_ask_for_access_mail +from core.tasks.mail import send_ask_for_access_mail, send_mention_notification_mail from core.utils.analytics import PosthogEventName, posthog_capture from core.utils.paths import filter_descendants from core.utils.s3_response_stream import content_stream @@ -1871,9 +1871,10 @@ def favorite(self, request, *args, **kwargs): 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. + The mention record is created synchronously; the email notification is + sent asynchronously by a Celery task, which suppresses it 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() @@ -1885,19 +1886,7 @@ def mention(self, request, *args, **kwargs): 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, - ) + send_mention_notification_mail.delay(str(mention.id)) return drf.response.Response( serializer.data, status=drf.status.HTTP_201_CREATED diff --git a/src/backend/core/tasks/mail.py b/src/backend/core/tasks/mail.py index 483c961486..51602d969d 100644 --- a/src/backend/core/tasks/mail.py +++ b/src/backend/core/tasks/mail.py @@ -3,6 +3,7 @@ from django.conf import settings from core import models +from core.utils.analytics import PosthogEventName, posthog_capture from impress.celery_app import app @@ -22,3 +23,25 @@ def send_ask_for_access_mail(ask_for_access_id): access.user.email, access.user.language or settings.LANGUAGE_CODE, ) + + +@app.task +def send_mention_notification_mail(mention_id): + """Notify a mentioned user by email, outside the request/response cycle.""" + mention = models.Mention.objects.select_related( + "document", "mentioned_user", "mentioned_by_user" + ).get(id=mention_id) + + notified = mention.notify() + + posthog_capture( + PosthogEventName.MENTION_CREATED, + mention.mentioned_by_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": notified, + }, + document=mention.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 index b1fa23c5d0..d066f7a833 100644 --- a/src/backend/core/tests/documents/test_api_documents_mention.py +++ b/src/backend/core/tests/documents/test_api_documents_mention.py @@ -128,7 +128,9 @@ def test_api_documents_mention_authenticated_success(role): "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"), + # The email is sent asynchronously, so the create response carries no + # notification timestamp even though the stored mention is notified. + "notified_at": None, } assert len(mail.outbox) == 1 @@ -352,7 +354,7 @@ def test_api_documents_mention_cooldown_same_context(): 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 models.Mention.objects.get(anchor_id="block-1").notified_at is not None assert len(mail.outbox) == 1 response = client.post( @@ -360,7 +362,7 @@ def test_api_documents_mention_cooldown_same_context(): {"anchor_id": "block-2", "mentioned_user_id": str(mentioned_user.id)}, ) assert response.status_code == 201 - assert response.json()["notified_at"] is None + assert models.Mention.objects.get(anchor_id="block-2").notified_at is None assert len(mail.outbox) == 1 assert models.Mention.objects.count() == 2 @@ -390,7 +392,9 @@ def test_api_documents_mention_cooldown_distinct_contexts(): 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 ( + models.Mention.objects.get(anchor_id=f"block-{i}").notified_at is not None + ) assert len(mail.outbox) == 3 @@ -404,7 +408,7 @@ def test_api_documents_mention_cooldown_distinct_contexts(): }, ) assert response.status_code == 201 - assert response.json()["notified_at"] is None + assert models.Mention.objects.get(anchor_id="block-4").notified_at is None assert len(mail.outbox) == 3 # The cooldown should apply per mentioned user @@ -419,7 +423,7 @@ def test_api_documents_mention_cooldown_distinct_contexts(): }, ) assert response.status_code == 201 - assert response.json()["notified_at"] is not None + assert models.Mention.objects.get(anchor_id="block-5").notified_at is not None assert len(mail.outbox) == 4 @@ -453,7 +457,7 @@ def test_api_documents_mention_cooldown_expired(settings): {"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 models.Mention.objects.get(anchor_id="block-2").notified_at is not None assert len(mail.outbox) == 2 @@ -467,7 +471,7 @@ def test_api_documents_mention_cooldown_only_considers_notified_mentions(): factories.UserDocumentAccessFactory(document=document, user=user, role="commenter") mentioned_user = factories.UserFactory() factories.UserDocumentAccessFactory(document=document, user=mentioned_user) - factories.MentionFactory( + existing_mention = factories.MentionFactory( document=document, mentioned_user=mentioned_user, mentioned_by_user=user, @@ -482,7 +486,8 @@ def test_api_documents_mention_cooldown_only_considers_notified_mentions(): ) assert response.status_code == 201 - assert response.json()["notified_at"] is not None + new_mention = models.Mention.objects.exclude(pk=existing_mention.pk).get() + assert new_mention.notified_at is not None assert len(mail.outbox) == 1 From 708363e3b248af9244faf3a8e15e74de9805edec Mon Sep 17 00:00:00 2001 From: BOUKERFA Mohamed El Amine Date: Sat, 20 Jun 2026 10:18:22 +0200 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=94=92=EF=B8=8F(backend)=20throttle?= =?UTF-8?q?=20the=20document=20mention=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a dedicated "mention" throttle scope limiting calls to the document mention endpoint to 30 requests per minute. Signed-off-by: BOUKERFA Mohamed El Amine --- src/backend/core/api/viewsets.py | 7 ++- .../documents/test_api_documents_mention.py | 62 +++++++++++++++++++ src/backend/impress/settings.py | 5 ++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 07c3e99bfb..3feb0772d8 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1867,7 +1867,12 @@ def favorite(self, request, *args, **kwargs): status=drf.status.HTTP_200_OK, ) - @drf.decorators.action(detail=True, methods=["post"], url_path="mention") + @drf.decorators.action( + detail=True, + methods=["post"], + url_path="mention", + throttle_scope="mention", + ) def mention(self, request, *args, **kwargs): """Mention a user on the document and notify them by email. diff --git a/src/backend/core/tests/documents/test_api_documents_mention.py b/src/backend/core/tests/documents/test_api_documents_mention.py index d066f7a833..c8e529f26b 100644 --- a/src/backend/core/tests/documents/test_api_documents_mention.py +++ b/src/backend/core/tests/documents/test_api_documents_mention.py @@ -509,3 +509,65 @@ def test_api_documents_mention_soft_deleted_document(): assert response.status_code == 404 assert models.Mention.objects.exists() is False + + +def test_api_documents_mention_throttling(settings): + """Mention requests should be throttled once the mention rate is exceeded.""" + current_rate = settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] + settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] = "3/minute" + + 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) + + client = APIClient() + client.force_login(user) + payload = {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)} + + # The first three requests within the minute are allowed. + for _i in range(3): + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", payload + ) + assert response.status_code == 201 + + # The fourth request is throttled. + response = client.post(f"/api/v1.0/documents/{document.id!s}/mention/", payload) + assert response.status_code == 429 + + # Restore original rate + settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] = current_rate + + +def test_api_documents_mention_throttling_y_provider_exempted(settings): + """ + Collaboration-server requests bypass the mention throttle, just like other + document endpoints relying on the document throttle exemption. + """ + current_rate = settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] + settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] = "3/minute" + settings.Y_PROVIDER_API_KEY = "test-y-provider-key" + + 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) + + client = APIClient() + client.force_login(user) + payload = {"anchor_id": "block-1", "mentioned_user_id": str(mentioned_user.id)} + + # More requests than the rate allows all succeed with the y-provider key. + for _i in range(5): + response = client.post( + f"/api/v1.0/documents/{document.id!s}/mention/", + payload, + HTTP_X_Y_PROVIDER_KEY="test-y-provider-key", + ) + assert response.status_code == 201 + + # Restore original rate + settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["mention"] = current_rate diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 7983522503..c806b7dea7 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -456,6 +456,11 @@ class Base(Configuration): environ_name="API_DOCUMENT_ASK_FOR_ACCESS_THROTTLE_RATE", environ_prefix=None, ), + "mention": values.Value( + default="30/minute", + environ_name="API_MENTION_THROTTLE_RATE", + environ_prefix=None, + ), "config": values.Value( default="30/minute", environ_name="API_CONFIG_THROTTLE_RATE",