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 %}