diff --git a/tildes/tests/test_comment_user_mentions.py b/tildes/tests/test_comment_user_mentions.py index b019b63..eacbc52 100644 --- a/tildes/tests/test_comment_user_mentions.py +++ b/tildes/tests/test_comment_user_mentions.py @@ -36,15 +36,10 @@ def test_get_mentions_for_comment(db, user_list, comment): assert mentions[index].user == user -def test_mention_filtering_parent_comment(mocker, db, topic, user_list): +def test_mention_filtering_parent_comment(db, topic, user_list): """Test notification filtering for parent comments.""" parent_comment = Comment(topic, user_list[0], "Comment content.") - parent_comment.user_id = user_list[0].user_id - comment = mocker.Mock( - user_id=user_list[1].user_id, - markdown=f"@{user_list[0].username}", - parent_comment=parent_comment, - ) + comment = Comment(topic, user_list[1], f"@{user_list[0].username}", parent_comment) mentions = CommentNotification.get_mentions_for_comment(db, comment) assert not mentions diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index 872a0e3..9701bed 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -5,7 +5,7 @@ from collections import Counter from datetime import datetime, timedelta -from typing import Any, Optional, Sequence, Tuple, TYPE_CHECKING +from typing import Any, Optional, Sequence, Tuple, TYPE_CHECKING, Union from pyramid.security import Allow, Authenticated, Deny, DENY_ALL, Everyone from sqlalchemy import Boolean, Column, ForeignKey, Integer, Text, TIMESTAMP @@ -228,6 +228,14 @@ class Comment(DatabaseModel): """Return the comment's ID in ID36 format.""" return id_to_id36(self.comment_id) + @property + def parent(self) -> Union["Comment", Topic]: + """Return what the comment is replying to (a topic or another comment).""" + if self.parent_comment: + return self.parent_comment + + return self.topic + @property def parent_comment_id36(self) -> str: """Return the parent comment's ID in ID36 format.""" diff --git a/tildes/tildes/models/comment/comment_notification.py b/tildes/tildes/models/comment/comment_notification.py index 68c34de..162c5aa 100644 --- a/tildes/tildes/models/comment/comment_notification.py +++ b/tildes/tildes/models/comment/comment_notification.py @@ -55,6 +55,12 @@ class CommentNotification(DatabaseModel): self, user: User, comment: Comment, notification_type: CommentNotificationType ): """Create a new notification for a user from a comment.""" + if notification_type in ( + CommentNotificationType.COMMENT_REPLY, + CommentNotificationType.TOPIC_REPLY, + ) and not self.should_create_reply_notification(comment): + raise ValueError("That comment shouldn't create a reply notification") + self.user = user self.comment = comment self.notification_type = notification_type @@ -66,6 +72,18 @@ class CommentNotification(DatabaseModel): acl.append(DENY_ALL) return acl + @classmethod + def should_create_reply_notification(cls, comment: Comment) -> bool: + """Return whether a comment should generate a reply notification.""" + # User is replying to their own post + if comment.parent.user == comment.user: + return False + + if not comment.parent.user.is_real_user: + return False + + return True + @property def is_comment_reply(self) -> bool: """Return whether this is a comment reply notification.""" @@ -95,19 +113,14 @@ class CommentNotification(DatabaseModel): .all() ) - parent_comment = comment.parent_comment - for user in users_to_mention: # prevent the user from mentioning themselves if comment.user == user: continue - if parent_comment: - # prevent comment replies from mentioning that comment's poster - if parent_comment.user == user: - continue - # prevent top-level comments from mentioning the thread creator - elif comment.topic.user == user: + # prevent mentioning the user they're replying to + # (they'll already get a reply notification) + if comment.parent.user == user: continue mention_notification = cls( diff --git a/tildes/tildes/views/api/web/comment.py b/tildes/tildes/views/api/web/comment.py index 8d300e0..903bffc 100644 --- a/tildes/tildes/views/api/web/comment.py +++ b/tildes/tildes/views/api/web/comment.py @@ -95,7 +95,7 @@ def post_toplevel_comment(request: Request, markdown: str) -> dict: request.db_session.add(LogComment(LogEventType.COMMENT_POST, request, new_comment)) - if topic.user != request.user and not topic.is_deleted: + if CommentNotification.should_create_reply_notification(new_comment): notification = CommentNotification( topic.user, new_comment, CommentNotificationType.TOPIC_REPLY ) @@ -135,7 +135,7 @@ def post_comment_reply(request: Request, markdown: str) -> dict: request.db_session.add(LogComment(LogEventType.COMMENT_POST, request, new_comment)) - if parent_comment.user != request.user: + if CommentNotification.should_create_reply_notification(new_comment): notification = CommentNotification( parent_comment.user, new_comment, CommentNotificationType.COMMENT_REPLY ) diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index af879b9..c21b021 100644 --- a/tildes/tildes/views/topic.py +++ b/tildes/tildes/views/topic.py @@ -415,7 +415,7 @@ def post_comment_on_topic(request: Request, markdown: str) -> HTTPFound: request.db_session.add(LogComment(LogEventType.COMMENT_POST, request, new_comment)) - if topic.user != request.user and not topic.is_deleted: + if CommentNotification.should_create_reply_notification(new_comment): notification = CommentNotification( topic.user, new_comment, CommentNotificationType.TOPIC_REPLY )