From 44ab79d0a59309eb86f3fabf9586129eab035139 Mon Sep 17 00:00:00 2001 From: Deimos Date: Mon, 7 Oct 2019 13:50:09 -0600 Subject: [PATCH] Don't create reply notifications for "fake" users Now that there are topics being posted automatically by the "fake" user, we don't want to be generating reply notifications for that user whenever someone posts a comment in them. These serve no purpose and would just pile up forever without being read. The same problem could have already been happening if people replied back to a comment with the "unknown user" author. --- tildes/tests/test_comment_user_mentions.py | 9 ++---- tildes/tildes/models/comment/comment.py | 10 ++++++- .../models/comment/comment_notification.py | 29 ++++++++++++++----- tildes/tildes/views/api/web/comment.py | 4 +-- tildes/tildes/views/topic.py | 2 +- 5 files changed, 35 insertions(+), 19 deletions(-) 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 )