diff --git a/salt/salt/consumers/init.sls b/salt/salt/consumers/init.sls index d1b4bd2..a05a680 100644 --- a/salt/salt/consumers/init.sls +++ b/salt/salt/consumers/init.sls @@ -1,3 +1,11 @@ +/etc/systemd/system/consumer-topic_interesting_activity_updater.service: + file.managed: + - source: salt://consumers/topic_interesting_activity_updater.service.jinja2 + - template: jinja + - user: root + - group: root + - mode: 644 + /etc/systemd/system/consumer-topic_metadata_generator.service: file.managed: - source: salt://consumers/topic_metadata_generator.service.jinja2 @@ -14,6 +22,10 @@ - group: root - mode: 644 +consumer-topic_interesting_activity_updater.service: + service.running: + - enable: True + consumer-topic_metadata_generator.service: service.running: - enable: True diff --git a/salt/salt/consumers/topic_interesting_activity_updater.service.jinja2 b/salt/salt/consumers/topic_interesting_activity_updater.service.jinja2 new file mode 100644 index 0000000..050ab6c --- /dev/null +++ b/salt/salt/consumers/topic_interesting_activity_updater.service.jinja2 @@ -0,0 +1,16 @@ +{% from 'common.jinja2' import app_dir, bin_dir -%} +[Unit] +Description=Topic Interesting Activity Updater (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 topic_interesting_activity_updater.py +Restart=always +RestartSec=5 + +[Install] +WantedBy=multi-user.target diff --git a/tildes/alembic/versions/cddd7d7ed0ea_add_interesting_activity_topic_sorting.py b/tildes/alembic/versions/cddd7d7ed0ea_add_interesting_activity_topic_sorting.py new file mode 100644 index 0000000..b51c0e1 --- /dev/null +++ b/tildes/alembic/versions/cddd7d7ed0ea_add_interesting_activity_topic_sorting.py @@ -0,0 +1,101 @@ +"""Add "interesting activity" topic sorting + +Revision ID: cddd7d7ed0ea +Revises: a2fda5d4e058 +Create Date: 2019-06-10 20:20:58.652760 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "cddd7d7ed0ea" +down_revision = "a2fda5d4e058" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "topics", + sa.Column( + "last_interesting_activity_time", + sa.TIMESTAMP(timezone=True), + server_default=sa.text("NOW()"), + nullable=False, + ), + ) + op.create_index( + op.f("ix_topics_last_interesting_activity_time"), + "topics", + ["last_interesting_activity_time"], + unique=False, + ) + + op.execute("UPDATE topics SET last_interesting_activity_time = last_activity_time") + + op.execute( + """ + CREATE TRIGGER send_rabbitmq_message_for_comment_delete + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('deleted'); + + + CREATE TRIGGER send_rabbitmq_message_for_comment_undelete + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = true AND NEW.is_deleted = false) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('undeleted'); + + + CREATE TRIGGER send_rabbitmq_message_for_comment_remove + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed = false AND NEW.is_removed = true) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('removed'); + + + CREATE TRIGGER send_rabbitmq_message_for_comment_unremove + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed = true AND NEW.is_removed = false) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('unremoved'); + + + CREATE TRIGGER send_rabbitmq_message_for_comment_label_delete + AFTER DELETE ON comment_labels + FOR EACH ROW + EXECUTE PROCEDURE send_rabbitmq_message_for_comment_label('deleted'); + """ + ) + + # manually commit before disabling the transaction for ALTER TYPE + op.execute("COMMIT") + + # 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 topicsortoption ADD VALUE IF NOT EXISTS 'ALL_ACTIVITY'") + + # re-activate the transaction for any future migrations + if connection is not None: + connection.execution_options(isolation_level="READ_COMMITTED") + + +def downgrade(): + op.execute( + "DROP TRIGGER send_rabbitmq_message_for_comment_label_delete ON comment_labels" + ) + op.execute("DROP TRIGGER send_rabbitmq_message_for_comment_unremove ON comments") + op.execute("DROP TRIGGER send_rabbitmq_message_for_comment_remove ON comments") + op.execute("DROP TRIGGER send_rabbitmq_message_for_comment_undelete ON comments") + op.execute("DROP TRIGGER send_rabbitmq_message_for_comment_delete ON comments") + + op.drop_index(op.f("ix_topics_last_interesting_activity_time"), table_name="topics") + op.drop_column("topics", "last_interesting_activity_time") diff --git a/tildes/consumers/topic_interesting_activity_updater.py b/tildes/consumers/topic_interesting_activity_updater.py new file mode 100644 index 0000000..aadc890 --- /dev/null +++ b/tildes/consumers/topic_interesting_activity_updater.py @@ -0,0 +1,88 @@ +# Copyright (c) 2019 Tildes contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Consumer that updates topics' last_interesting_activity_time.""" + +from datetime import datetime +from typing import Optional + +from amqpy import Message + +from tildes.enums import CommentTreeSortOption +from tildes.lib.amqp import PgsqlQueueConsumer +from tildes.models.comment import Comment, CommentInTree, CommentTree + + +class TopicInterestingActivityUpdater(PgsqlQueueConsumer): + """Consumer that updates topics' last_interesting_activity_time.""" + + def run(self, msg: Message) -> None: + """Process a delivered message.""" + trigger_comment = ( + self.db_session.query(Comment) + .filter_by(comment_id=msg.body["comment_id"]) + .one() + ) + + topic = trigger_comment.topic + + all_comments = self.db_session.query(Comment).filter_by(topic=topic).all() + + tree = CommentTree(all_comments, CommentTreeSortOption.NEWEST) + + # default the last interesting time to the topic's creation + last_interesting_time = topic.created_time + + for comment in tree: + branch_time = self._find_last_interesting_time(comment) + if branch_time and branch_time > last_interesting_time: + last_interesting_time = branch_time + + topic.last_interesting_activity_time = last_interesting_time + + def _find_last_interesting_time(self, comment: CommentInTree) -> Optional[datetime]: + """Recursively find the last "interesting" time from a comment and replies.""" + # if the comment has one of these labels, don't look any deeper down this branch + uninteresting_labels = ("noise", "offtopic", "malice") + if any(comment.is_label_active(label) for label in uninteresting_labels): + return None + + # the comment itself isn't interesting if it's deleted or removed, but one of + # its children still could be, so we still want to keep recursing under it + if not (comment.is_deleted or comment.is_removed): + comment_time = comment.created_time + else: + comment_time = None + + # find the max interesting time from all of this comment's replies + if comment.replies: + reply_time = max( + [self._find_last_interesting_time(reply) for reply in comment.replies] + ) + else: + reply_time = None + + # disregard either time if it's None (or both) + potential_times = [time for time in (comment_time, reply_time) if time] + + if potential_times: + return max(potential_times) + + # will only be reached if both the comment and reply times were None + return None + + +if __name__ == "__main__": + TopicInterestingActivityUpdater( + queue_name="topic_interesting_activity_updater.q", + routing_keys=[ + "comment.created", + "comment.deleted", + "comment.edited", + "comment.removed", + "comment.undeleted", + "comment.unremoved", + "comment_label.created", + "comment_label.deleted", + ], + ).consume_queue() diff --git a/tildes/sql/init/triggers/comment_labels/rabbitmq.sql b/tildes/sql/init/triggers/comment_labels/rabbitmq.sql index aac5f04..7e48c38 100644 --- a/tildes/sql/init/triggers/comment_labels/rabbitmq.sql +++ b/tildes/sql/init/triggers/comment_labels/rabbitmq.sql @@ -28,3 +28,9 @@ CREATE TRIGGER send_rabbitmq_message_for_comment_label_insert AFTER INSERT ON comment_labels FOR EACH ROW EXECUTE PROCEDURE send_rabbitmq_message_for_comment_label('created'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_label_delete + AFTER DELETE ON comment_labels + FOR EACH ROW + EXECUTE PROCEDURE send_rabbitmq_message_for_comment_label('deleted'); diff --git a/tildes/sql/init/triggers/comments/rabbitmq.sql b/tildes/sql/init/triggers/comments/rabbitmq.sql index 457ed1a..2d626b8 100644 --- a/tildes/sql/init/triggers/comments/rabbitmq.sql +++ b/tildes/sql/init/triggers/comments/rabbitmq.sql @@ -32,3 +32,31 @@ CREATE TRIGGER send_rabbitmq_message_for_comment_edit FOR EACH ROW WHEN (OLD.markdown IS DISTINCT FROM NEW.markdown) EXECUTE PROCEDURE send_rabbitmq_message_for_comment('edited'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_delete + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('deleted'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_undelete + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = true AND NEW.is_deleted = false) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('undeleted'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_remove + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed = false AND NEW.is_removed = true) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('removed'); + + +CREATE TRIGGER send_rabbitmq_message_for_comment_unremove + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed = true AND NEW.is_removed = false) + EXECUTE PROCEDURE send_rabbitmq_message_for_comment('unremoved'); diff --git a/tildes/tildes/enums.py b/tildes/tildes/enums.py index fda19bd..862ba61 100644 --- a/tildes/tildes/enums.py +++ b/tildes/tildes/enums.py @@ -117,12 +117,28 @@ class ScraperType(enum.Enum): class TopicSortOption(enum.Enum): - """Enum for the different methods topics can be sorted by.""" + """Enum for the different methods topics can be sorted by. + + Note that there are two sort methods based on activity: + - "All activity" will bump a topic back to the top of the sort whenever *any* new + comments are posted in that topic, similar to how forums behave. This uses the + Topic.last_activity_time value. + - "Activity" tries to only bump topics back up when "interesting" activity + occurs in them, using some checks to decide whether specific comments should be + disregarded. This uses the topic.last_interesting_activity_time value, which is + updated by a separate background process (topic_interesting_activity_updater). + """ + ACTIVITY = enum.auto() VOTES = enum.auto() COMMENTS = enum.auto() NEW = enum.auto() - ACTIVITY = enum.auto() + ALL_ACTIVITY = enum.auto() + + @property + def display_name(self) -> str: + """Return the sort method's name in a format more suitable for display.""" + return self.name.capitalize().replace("_", " ") @property def descending_description(self) -> str: @@ -135,7 +151,9 @@ class TopicSortOption(enum.Enum): if self.name == "NEW": return "newest" elif self.name == "ACTIVITY": - return "activity" + return "relevant activity" + elif self.name == "ALL_ACTIVITY": + return "all activity" return "most {}".format(self.name.lower()) diff --git a/tildes/tildes/models/comment/__init__.py b/tildes/tildes/models/comment/__init__.py index b9cb436..5871321 100644 --- a/tildes/tildes/models/comment/__init__.py +++ b/tildes/tildes/models/comment/__init__.py @@ -6,5 +6,5 @@ from .comment_notification import CommentNotification from .comment_notification_query import CommentNotificationQuery from .comment_query import CommentQuery from .comment_label import CommentLabel -from .comment_tree import CommentTree +from .comment_tree import CommentInTree, CommentTree from .comment_vote import CommentVote diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index b673c33..1f8119a 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -89,6 +89,12 @@ class Topic(DatabaseModel): index=True, server_default=text("NOW()"), ) + last_interesting_activity_time: datetime = Column( + TIMESTAMP(timezone=True), + nullable=False, + index=True, + server_default=text("NOW()"), + ) is_deleted: bool = Column( Boolean, nullable=False, server_default="false", index=True ) diff --git a/tildes/tildes/models/topic/topic_query.py b/tildes/tildes/models/topic/topic_query.py index 1c842a2..74ec553 100644 --- a/tildes/tildes/models/topic/topic_query.py +++ b/tildes/tildes/models/topic/topic_query.py @@ -126,6 +126,8 @@ class TopicQuery(PaginatedQuery): elif sort == TopicSortOption.NEW: self._sort_column = Topic.created_time elif sort == TopicSortOption.ACTIVITY: + self._sort_column = Topic.last_interesting_activity_time + elif sort == TopicSortOption.ALL_ACTIVITY: self._sort_column = Topic.last_activity_time self.sort_desc = desc diff --git a/tildes/tildes/templates/topic_listing.jinja2 b/tildes/tildes/templates/topic_listing.jinja2 index f3703ac..ac8c3bc 100644 --- a/tildes/tildes/templates/topic_listing.jinja2 +++ b/tildes/tildes/templates/topic_listing.jinja2 @@ -36,7 +36,7 @@ {% endif %} - {{ option.descending_description.capitalize() }} + {{ option.display_name }} diff --git a/tildes/tildes/templates/user.jinja2 b/tildes/tildes/templates/user.jinja2 index 4215d08..407e613 100644 --- a/tildes/tildes/templates/user.jinja2 +++ b/tildes/tildes/templates/user.jinja2 @@ -43,7 +43,7 @@
  • - All activity + All posts
  • Topics