From 420ea5a15bb3fe80639aa8df599a7367590f10ad Mon Sep 17 00:00:00 2001 From: Celeo Date: Sun, 5 Aug 2018 23:25:54 -0600 Subject: [PATCH] Add notifications for users being mentioned This detects mentions of users in comments using the same pattern as the markdown parsing uses to generate user links. Mentioned users are sent a notification, and mentions are added/deleted if needed on comment edits. As part of this, setup was done to generate rabbitmq messages for comment creation and edits, and the mentions are handled by an async consumer of these messages. --- ...ent_user_mentions_generator.service.jinja2 | 16 ++ salt/salt/consumers/init.sls | 12 ++ ...dded_user_tag_type_comment_notification.py | 72 +++++++++ .../comment_user_mentions_generator.py | 41 +++++ .../sql/init/triggers/comments/rabbitmq.sql | 31 ++++ tildes/tests/test_comment_user_mentions.py | 149 ++++++++++++++++++ tildes/tildes/enums.py | 1 + tildes/tildes/models/comment/comment.py | 3 + .../models/comment/comment_notification.py | 99 +++++++++++- .../templates/notifications_unread.jinja2 | 6 +- 10 files changed, 428 insertions(+), 2 deletions(-) create mode 100644 salt/salt/consumers/comment_user_mentions_generator.service.jinja2 create mode 100644 tildes/alembic/versions/f1ecbf24c212_added_user_tag_type_comment_notification.py create mode 100644 tildes/consumers/comment_user_mentions_generator.py create mode 100644 tildes/sql/init/triggers/comments/rabbitmq.sql create mode 100644 tildes/tests/test_comment_user_mentions.py diff --git a/salt/salt/consumers/comment_user_mentions_generator.service.jinja2 b/salt/salt/consumers/comment_user_mentions_generator.service.jinja2 new file mode 100644 index 0000000..4b8988e --- /dev/null +++ b/salt/salt/consumers/comment_user_mentions_generator.service.jinja2 @@ -0,0 +1,16 @@ +{% from 'common.jinja2' import app_dir, bin_dir -%} +[Unit] +Description=Comment User Mention Generator (Queue Consumer) +Requires=rabbitmq-server.service +After=rabbitmq-server.service +PartOf=rabbitmq-server.service + +[Service] +WorkingDirectory={{ app_dir }}/consumers +Environment="INI_FILE={{ app_dir }}/{{ pillar['ini_file'] }}" +ExecStart={{ bin_dir }}/python comment_user_mentions_generator.py +Restart=always +RestartSec=5 + +[Install] +WantedBy=multi-user.target diff --git a/salt/salt/consumers/init.sls b/salt/salt/consumers/init.sls index 73437ae..a97a251 100644 --- a/salt/salt/consumers/init.sls +++ b/salt/salt/consumers/init.sls @@ -6,6 +6,18 @@ - group: root - mode: 644 +/etc/systemd/system/consumer-comment_user_mentions_generator.service: + file.managed: + - source: salt://consumers/comment_user_mentions_generator.service.jinja2 + - template: jinja + - user: root + - group: root + - mode: 644 + consumer-topic_metadata_generator.service: service.running: - enable: True + +consumer-comment_user_mentions_generator.service: + service.running: + - enable: True diff --git a/tildes/alembic/versions/f1ecbf24c212_added_user_tag_type_comment_notification.py b/tildes/alembic/versions/f1ecbf24c212_added_user_tag_type_comment_notification.py new file mode 100644 index 0000000..a675a0e --- /dev/null +++ b/tildes/alembic/versions/f1ecbf24c212_added_user_tag_type_comment_notification.py @@ -0,0 +1,72 @@ +"""Add user mention notifications from comments + +Revision ID: f1ecbf24c212 +Revises: de83b8750123 +Create Date: 2018-07-19 02:32:43.684716 + +""" +from alembic import op + + +# revision identifiers, used by Alembic. +revision = 'f1ecbf24c212' +down_revision = 'de83b8750123' +branch_labels = None +depends_on = None + + +def upgrade(): + # ALTER TYPE doesn't work from inside a transaction, disable it + connection = None + if not op.get_context().as_sql: + connection = op.get_bind() + connection.execution_options(isolation_level='AUTOCOMMIT') + + op.execute( + "ALTER TYPE commentnotificationtype " + "ADD VALUE IF NOT EXISTS 'USER_MENTION'" + ) + + # re-activate the transaction for any future migrations + if connection is not None: + connection.execution_options(isolation_level='READ_COMMITTED') + + op.execute(''' + CREATE OR REPLACE FUNCTION send_rabbitmq_message_for_comment() RETURNS TRIGGER AS $$ + DECLARE + affected_row RECORD; + payload TEXT; + BEGIN + IF (TG_OP = 'INSERT' OR TG_OP = 'UPDATE') THEN + affected_row := NEW; + ELSIF (TG_OP = 'DELETE') THEN + affected_row := OLD; + END IF; + + payload := json_build_object('comment_id', affected_row.comment_id, 'event_type', TG_OP); + + PERFORM send_rabbitmq_message('comment.' || TG_ARGV[0], payload); + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + ''') + op.execute(''' + CREATE TRIGGER send_rabbitmq_message_for_comment_insert + AFTER INSERT ON comments + FOR EACH ROW + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('created'); + ''') + op.execute(''' + CREATE TRIGGER send_rabbitmq_message_for_comment_edit + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.markdown IS DISTINCT FROM NEW.markdown) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('edited'); + ''') + + +def downgrade(): + op.execute('DROP TRIGGER send_rabbitmq_message_for_comment_insert ON comments') + op.execute('DROP TRIGGER send_rabbitmq_message_for_comment_edit ON comments') + op.execute('DROP FUNCTION send_rabbitmq_message_for_comment') diff --git a/tildes/consumers/comment_user_mentions_generator.py b/tildes/consumers/comment_user_mentions_generator.py new file mode 100644 index 0000000..dea5c0f --- /dev/null +++ b/tildes/consumers/comment_user_mentions_generator.py @@ -0,0 +1,41 @@ +"""Consumer that generates user mentions for comments.""" + +from amqpy import Message + +from tildes.lib.amqp import PgsqlQueueConsumer +from tildes.models.comment import Comment, CommentNotification + + +class CommentUserMentionGenerator(PgsqlQueueConsumer): + """Consumer that generates user mentions for comments.""" + + def run(self, msg: Message) -> None: + """Process a delivered message.""" + comment = ( + self.db_session.query(Comment) + .filter_by(comment_id=msg.body['comment_id']) + .one() + ) + new_mentions = CommentNotification.get_mentions_for_comment( + self.db_session, comment) + + if msg.delivery_info['routing_key'] == 'comment.created': + for user_mention in new_mentions: + self.db_session.add(user_mention) + elif msg.delivery_info['routing_key'] == 'comment.edited': + to_delete, to_add = ( + CommentNotification.prevent_duplicate_notifications( + self.db_session, comment, new_mentions)) + + for user_mention in to_delete: + self.db_session.delete(user_mention) + + for user_mention in to_add: + self.db_session.add(user_mention) + + +if __name__ == '__main__': + CommentUserMentionGenerator( + queue_name='comment_user_mentions_generator.q', + routing_keys=['comment.created', 'comment.edited'], + ).consume_queue() diff --git a/tildes/sql/init/triggers/comments/rabbitmq.sql b/tildes/sql/init/triggers/comments/rabbitmq.sql new file mode 100644 index 0000000..819ed1a --- /dev/null +++ b/tildes/sql/init/triggers/comments/rabbitmq.sql @@ -0,0 +1,31 @@ +CREATE OR REPLACE FUNCTION send_rabbitmq_message_for_comment() RETURNS TRIGGER AS $$ +DECLARE + affected_row RECORD; + payload TEXT; +BEGIN + IF (TG_OP = 'INSERT' OR TG_OP = 'UPDATE') THEN + affected_row := NEW; + ELSIF (TG_OP = 'DELETE') THEN + affected_row := OLD; + END IF; + + payload := json_build_object('comment_id', affected_row.comment_id, 'event_type', TG_OP); + + PERFORM send_rabbitmq_message('comment.' || TG_ARGV[0], payload); + + RETURN NULL; +END; +$$ LANGUAGE plpgsql; + + +CREATE TRIGGER send_rabbitmq_message_for_comment_insert + AFTER INSERT ON comments + FOR EACH ROW + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('created'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_edit + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.markdown IS DISTINCT FROM NEW.markdown) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('edited'); diff --git a/tildes/tests/test_comment_user_mentions.py b/tildes/tests/test_comment_user_mentions.py new file mode 100644 index 0000000..ec9ec15 --- /dev/null +++ b/tildes/tests/test_comment_user_mentions.py @@ -0,0 +1,149 @@ +from pytest import fixture + +from sqlalchemy import and_ + +from tildes.enums import CommentNotificationType +from tildes.models.comment import ( + Comment, + CommentNotification, +) +from tildes.models.topic import Topic +from tildes.models.user import User + + +@fixture +def topic(db, session_group, session_user): + """Create a topic in the db, delete it as teardown (including comments).""" + new_topic = Topic.create_text_topic( + session_group, session_user, 'Some title', 'some text') + db.add(new_topic) + db.commit() + + yield new_topic + + db.query(Comment).filter_by(topic_id=new_topic.topic_id).delete() + db.delete(new_topic) + db.commit() + + +@fixture +def comment(topic, session_user): + """Return an unsaved comment.""" + return Comment(topic, session_user, 'Comment content.') + + +@fixture +def user_list(db): + """Create several users.""" + users = [] + for name in ['foo', 'bar', 'baz']: + user = User(name, 'password') + users.append(user) + db.add(user) + db.commit() + + yield users + + for user in users: + db.delete(user) + db.commit() + + +def test_get_mentions_for_comment(db, user_list, comment): + """Test that notifications are generated and returned.""" + comment.markdown = '@foo @bar. @baz!' + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert len(mentions) == 3 + for index, user in enumerate(user_list): + assert mentions[index].user == user + + +def test_mention_filtering_parent_comment( + mocker, 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, + ) + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert not mentions + + +def test_mention_filtering_self_mention(db, user_list, topic): + """Test notification filtering for self-mentions.""" + comment = Comment(topic, user_list[0], f'@{user_list[0]}') + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert not mentions + + +def test_mention_filtering_top_level(db, user_list, session_group): + """Test notification filtering for top-level comments.""" + topic = Topic.create_text_topic( + session_group, user_list[0], 'Some title', 'some text') + comment = Comment(topic, user_list[1], f'@{user_list[0].username}') + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert not mentions + + +def test_prevent_duplicate_notifications(db, user_list, topic): + """Test that notifications are cleaned up for edits. + + Flow: + 1. A comment is created by user A that mentions user B. Notifications + are generated, and yield A mentioning B. + 2. The comment is edited to mention C and not B. + 3. The comment is edited to mention B and C. + 4. The comment is deleted. + """ + # 1 + comment = Comment(topic, user_list[0], f'@{user_list[1].username}') + db.add(comment) + db.commit() + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert len(mentions) == 1 + assert mentions[0].user == user_list[1] + db.add_all(mentions) + db.commit() + + # 2 + comment.markdown = f'@{user_list[2].username}' + db.commit() + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert len(mentions) == 1 + to_delete, to_add = CommentNotification.prevent_duplicate_notifications( + db, comment, mentions) + assert len(to_delete) == 1 + assert mentions == to_add + assert to_delete[0].user.username == user_list[1].username + + # 3 + comment.markdown = f'@{user_list[1].username} @{user_list[2].username}' + db.commit() + mentions = CommentNotification.get_mentions_for_comment( + db, comment) + assert len(mentions) == 2 + to_delete, to_add = CommentNotification.prevent_duplicate_notifications( + db, comment, mentions) + assert not to_delete + assert len(to_add) == 1 + + # 4 + comment.is_deleted = True + db.commit() + notifications = ( + db.query(CommentNotification.user_id) + .filter(and_( + CommentNotification.comment_id == comment.comment_id, + CommentNotification.notification_type == + CommentNotificationType.USER_MENTION, + )).all()) + assert not notifications diff --git a/tildes/tildes/enums.py b/tildes/tildes/enums.py index ebb75a4..6822cd4 100644 --- a/tildes/tildes/enums.py +++ b/tildes/tildes/enums.py @@ -8,6 +8,7 @@ class CommentNotificationType(enum.Enum): COMMENT_REPLY = enum.auto() TOPIC_REPLY = enum.auto() + USER_MENTION = enum.auto() class CommentSortOption(enum.Enum): diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index d706e3e..437ff40 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -50,6 +50,8 @@ class Comment(DatabaseModel): - Setting is_deleted to true will decrement num_comments on all topic_visit rows for the relevant topic, where the visit_time was after the time the comment was originally posted. + - Inserting a row or updating markdown will send a rabbitmq message + for "comment.created" or "comment.edited" respectively. Internal: - deleted_time will be set when is_deleted is set to true """ @@ -95,6 +97,7 @@ class Comment(DatabaseModel): user: User = relationship('User', lazy=False, innerjoin=True) topic: Topic = relationship('Topic', innerjoin=True) + parent_comment: 'Comment' = relationship('Comment', innerjoin=True) @hybrid_property def markdown(self) -> str: diff --git a/tildes/tildes/models/comment/comment_notification.py b/tildes/tildes/models/comment/comment_notification.py index 5e80d2f..192c975 100644 --- a/tildes/tildes/models/comment/comment_notification.py +++ b/tildes/tildes/models/comment/comment_notification.py @@ -1,13 +1,16 @@ """Contains the CommentNotification class.""" from datetime import datetime +import re +from typing import List, Tuple from sqlalchemy import Boolean, Column, ForeignKey, Integer, TIMESTAMP from sqlalchemy.dialects.postgresql import ENUM -from sqlalchemy.orm import relationship +from sqlalchemy.orm import relationship, Session from sqlalchemy.sql.expression import text from tildes.enums import CommentNotificationType +from tildes.lib.markdown import LinkifyFilter from tildes.models import DatabaseModel from tildes.models.user import User from .comment import Comment @@ -72,3 +75,97 @@ class CommentNotification(DatabaseModel): def is_topic_reply(self) -> bool: """Return whether this is a topic reply notification.""" return self.notification_type == CommentNotificationType.TOPIC_REPLY + + @property + def is_mention(self) -> bool: + """Return whether this is a mention notification.""" + return self.notification_type == CommentNotificationType.USER_MENTION + + @classmethod + def get_mentions_for_comment( + cls, + db_session: Session, + comment: Comment, + ) -> List['CommentNotification']: + """Get a list of notifications for user mentions in the comment.""" + notifications = [] + + raw_names = re.findall( + LinkifyFilter.USERNAME_REFERENCE_REGEX, + comment.markdown, + ) + users_to_mention = ( + db_session.query(User) + .filter(User.username.in_(raw_names)) # type: ignore + .all() + ) + + parent_comment = comment.parent_comment + + for user in users_to_mention: + # prevent the user from mentioning themselves + if comment.user_id == user.user_id: + continue + + if parent_comment: + # prevent comment replies from mentioning that comment's poster + if parent_comment.user_id == user.user_id: + continue + # prevent top-level comments from mentioning the thread creator + elif comment.topic.user_id == user.user_id: + continue + + mention_notification = cls( + user, comment, CommentNotificationType.USER_MENTION) + notifications.append(mention_notification) + + return notifications + + @staticmethod + def prevent_duplicate_notifications( + db_session: Session, + comment: Comment, + new_notifications: List['CommentNotification'], + ) -> Tuple[List['CommentNotification'], List['CommentNotification']]: + """Filter new notifications for edited comments. + + Protect against sending a notification for the same comment to + the same user twice. Edits can sent notifications to users + now mentioned in the content, but only if they weren't sent + a notification for that comment before. + + This method returns a tuple of lists of this class. The first + item is the notifications that were previously sent for this + comment but need to be deleted (i.e. mentioned username was edited + out of the comment), and the second item is the notifications + that need to be added, as they're new. + """ + previous_notifications = ( + db_session + .query(CommentNotification) + .filter( + CommentNotification.comment_id == comment.comment_id, + CommentNotification.notification_type == + CommentNotificationType.USER_MENTION, + ).all() + ) + + new_mention_user_ids = [ + notification.user.user_id for notification in new_notifications + ] + + previous_mention_user_ids = [ + notification.user_id for notification in previous_notifications + ] + + to_delete = [ + notification for notification in previous_notifications + if notification.user.user_id not in new_mention_user_ids + ] + + to_add = [ + notification for notification in new_notifications + if notification.user.user_id not in previous_mention_user_ids + ] + + return (to_delete, to_add) diff --git a/tildes/tildes/templates/notifications_unread.jinja2 b/tildes/tildes/templates/notifications_unread.jinja2 index b580f1f..beab3d8 100644 --- a/tildes/tildes/templates/notifications_unread.jinja2 +++ b/tildes/tildes/templates/notifications_unread.jinja2 @@ -1,7 +1,7 @@ {% extends 'base_user_menu.jinja2' %} {% from 'macros/comments.jinja2' import comment_tag_options_template, render_single_comment with context %} -{% from 'macros/links.jinja2' import group_linked %} +{% from 'macros/links.jinja2' import group_linked, username_linked %} {% block title %}Unread notifications{% endblock %} @@ -16,6 +16,10 @@

Reply to your comment on {{ notification.comment.topic.title }} in {{ group_linked(notification.comment.topic.group.path) }}

{% elif notification.is_topic_reply %}

Reply to your topic {{ notification.comment.topic.title }} in {{ group_linked(notification.comment.topic.group.path) }}

+ {% elif notification.is_mention %} +

+ You were mentioned in a comment on {{ notification.comment.topic.title }} in {{ group_linked(notification.comment.topic.group.path) }} +

{% endif %} {% if notification.is_unread and not request.user.auto_mark_notifications_read %}