From 5e1197b0c6f77dde01572404bfcda33fe9526a92 Mon Sep 17 00:00:00 2001 From: Deimos Date: Wed, 12 Jun 2019 14:33:11 -0600 Subject: [PATCH] Base activity sorting on "interesting" activity This changes the "activity" topic-sorting method to look for "interesting" activity instead of everything, and adds a new "All activity" method that retains the previous behavior. Currently, "interesting activity" excludes any comments that have active Noise, Offtopic, or Malice labels, or any of their children. These checks are also done based on labeling activity, so for example if someone posts a new comment it will bump the thread initially, but if that comment is then labeled as Noise, the thread will "un-bump" and go back to its previous position in the Activity sort. There were also some other minor changes made to appearance to support adding another sorting option, such as shortening the displayed names on the "tabs", like showing "Votes" instead of "Most votes". This probably needs some further work, but is okay for now. --- salt/salt/consumers/init.sls | 12 +++ ...nteresting_activity_updater.service.jinja2 | 16 +++ ..._add_interesting_activity_topic_sorting.py | 101 ++++++++++++++++++ .../topic_interesting_activity_updater.py | 88 +++++++++++++++ .../init/triggers/comment_labels/rabbitmq.sql | 6 ++ .../sql/init/triggers/comments/rabbitmq.sql | 28 +++++ tildes/tildes/enums.py | 24 ++++- tildes/tildes/models/comment/__init__.py | 2 +- tildes/tildes/models/topic/topic.py | 6 ++ tildes/tildes/models/topic/topic_query.py | 2 + tildes/tildes/templates/topic_listing.jinja2 | 2 +- tildes/tildes/templates/user.jinja2 | 2 +- 12 files changed, 283 insertions(+), 6 deletions(-) create mode 100644 salt/salt/consumers/topic_interesting_activity_updater.service.jinja2 create mode 100644 tildes/alembic/versions/cddd7d7ed0ea_add_interesting_activity_topic_sorting.py create mode 100644 tildes/consumers/topic_interesting_activity_updater.py 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