From 7fd1c3e72ee3847568b541565578f0bc29ca7cee Mon Sep 17 00:00:00 2001 From: Deimos Date: Wed, 20 Nov 2019 13:07:30 -0700 Subject: [PATCH] Close voting after 30 days, delete vote records This makes it so that posts (both topics and comments) can no longer be voted on after they're over 30 days old. An hourly cronjob makes this "official" by updating a flag on the post indicating that voting is closed. The daily clean_private_data script then deletes all individual vote records for posts with closed voting, and the triggers on the voting tables have been updated to not decrement the vote totals when these deletions happen. The net result of this is that Tildes only stores users' votes for a maximum of 30 days, removing a lot of sensitive/private data that builds up over the long term. --- salt/salt/cronjobs.sls | 7 + ...1a468_add_is_voting_closed_to_comments_.py | 129 ++++++++++++++++++ tildes/scripts/clean_private_data.py | 21 ++- tildes/scripts/close_voting_on_old_posts.py | 31 +++++ tildes/scss/modules/_btn.scss | 1 - tildes/scss/modules/_comment.scss | 3 +- .../init/triggers/comment_votes/comments.sql | 5 +- .../sql/init/triggers/topic_votes/topics.sql | 5 +- tildes/tildes/models/comment/__init__.py | 2 +- tildes/tildes/models/comment/comment.py | 27 +++- tildes/tildes/models/comment/comment_vote.py | 2 +- tildes/tildes/models/topic/__init__.py | 2 +- tildes/tildes/models/topic/topic.py | 29 +++- tildes/tildes/models/topic/topic_vote.py | 2 +- .../tildes/templates/macros/comments.jinja2 | 10 +- 15 files changed, 258 insertions(+), 18 deletions(-) create mode 100644 tildes/alembic/versions/4d352e61a468_add_is_voting_closed_to_comments_.py create mode 100644 tildes/scripts/close_voting_on_old_posts.py diff --git a/salt/salt/cronjobs.sls b/salt/salt/cronjobs.sls index b6cafd6..ea0e4f3 100644 --- a/salt/salt/cronjobs.sls +++ b/salt/salt/cronjobs.sls @@ -1,5 +1,12 @@ {% from 'common.jinja2' import app_dir, app_username, bin_dir %} +close-voting-cronjob: + cron.present: + - name: {{ bin_dir }}/python -c "from scripts.close_voting_on_old_posts import close_voting_on_old_posts; close_voting_on_old_posts('{{ app_dir }}/{{ pillar['ini_file'] }}')" + - user: {{ app_username }} + - hour: '*' + - minute: 3 + data-cleanup-cronjob: cron.present: - name: {{ bin_dir }}/python -c "from scripts.clean_private_data import clean_all_data; clean_all_data('{{ app_dir }}/{{ pillar['ini_file'] }}')" diff --git a/tildes/alembic/versions/4d352e61a468_add_is_voting_closed_to_comments_.py b/tildes/alembic/versions/4d352e61a468_add_is_voting_closed_to_comments_.py new file mode 100644 index 0000000..e8da240 --- /dev/null +++ b/tildes/alembic/versions/4d352e61a468_add_is_voting_closed_to_comments_.py @@ -0,0 +1,129 @@ +"""Add is_voting_closed to comments and topics + +Revision ID: 4d352e61a468 +Revises: 879588c5729d +Create Date: 2019-11-15 23:58:09.613684 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "4d352e61a468" +down_revision = "879588c5729d" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "comments", + sa.Column( + "is_voting_closed", sa.Boolean(), server_default="false", nullable=False + ), + ) + op.create_index( + op.f("ix_comments_is_voting_closed"), + "comments", + ["is_voting_closed"], + unique=False, + ) + op.add_column( + "topics", + sa.Column( + "is_voting_closed", sa.Boolean(), server_default="false", nullable=False + ), + ) + op.create_index( + op.f("ix_topics_is_voting_closed"), "topics", ["is_voting_closed"], unique=False + ) + + op.execute( + """ + CREATE OR REPLACE FUNCTION update_comment_num_votes() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE comments + SET num_votes = num_votes + 1 + WHERE comment_id = NEW.comment_id; + ELSIF (TG_OP = 'DELETE') THEN + UPDATE comments + SET num_votes = num_votes - 1 + WHERE comment_id = OLD.comment_id + AND is_voting_closed = FALSE; + END IF; + + RETURN NULL; + END + $$ LANGUAGE plpgsql; + """ + ) + + op.execute( + """ + CREATE OR REPLACE FUNCTION update_topic_num_votes() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE topics + SET num_votes = num_votes + 1 + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'DELETE') THEN + UPDATE topics + SET num_votes = num_votes - 1 + WHERE topic_id = OLD.topic_id + AND is_voting_closed = FALSE; + END IF; + + RETURN NULL; + END + $$ LANGUAGE plpgsql; + """ + ) + + +def downgrade(): + op.drop_index(op.f("ix_topics_is_voting_closed"), table_name="topics") + op.drop_column("topics", "is_voting_closed") + op.drop_index(op.f("ix_comments_is_voting_closed"), table_name="comments") + op.drop_column("comments", "is_voting_closed") + + op.execute( + """ + CREATE OR REPLACE FUNCTION update_comment_num_votes() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE comments + SET num_votes = num_votes + 1 + WHERE comment_id = NEW.comment_id; + ELSIF (TG_OP = 'DELETE') THEN + UPDATE comments + SET num_votes = num_votes - 1 + WHERE comment_id = OLD.comment_id; + END IF; + + RETURN NULL; + END + $$ LANGUAGE plpgsql; + """ + ) + + op.execute( + """ + CREATE OR REPLACE FUNCTION update_topic_num_votes() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE topics + SET num_votes = num_votes + 1 + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'DELETE') THEN + UPDATE topics + SET num_votes = num_votes - 1 + WHERE topic_id = OLD.topic_id; + END IF; + + RETURN NULL; + END + $$ LANGUAGE plpgsql; + """ + ) diff --git a/tildes/scripts/clean_private_data.py b/tildes/scripts/clean_private_data.py index dac5114..66eb42f 100644 --- a/tildes/scripts/clean_private_data.py +++ b/tildes/scripts/clean_private_data.py @@ -4,7 +4,6 @@ """Script for cleaning up private/deleted data. Other things that should probably be added here eventually: - - Delete individual votes on comments/topics after voting has been closed - Delete which users labeled comments after labeling has been closed - Delete old used invite codes (30 days after used?) """ @@ -65,6 +64,7 @@ class DataCleaner: self.delete_old_log_entries() self.delete_old_topic_visits() + self.delete_old_votes() self.clean_old_deleted_comments() self.clean_old_deleted_topics() @@ -96,6 +96,25 @@ class DataCleaner: self.db_session.commit() logging.info(f"Deleted {deleted} old topic visits.") + def delete_old_votes(self) -> None: + """Delete individual votes on posts with their voting closed.""" + self.db_session.execute( + """ + DELETE FROM comment_votes USING comments + WHERE comments.comment_id = comment_votes.comment_id + AND comments.is_voting_closed = TRUE + """ + ) + self.db_session.execute( + """ + DELETE FROM topic_votes USING topics + WHERE topics.topic_id = topic_votes.topic_id + AND topics.is_voting_closed = TRUE + """ + ) + + self.db_session.commit() + def clean_old_deleted_comments(self) -> None: """Clean the data of old deleted comments. diff --git a/tildes/scripts/close_voting_on_old_posts.py b/tildes/scripts/close_voting_on_old_posts.py new file mode 100644 index 0000000..6eed1a7 --- /dev/null +++ b/tildes/scripts/close_voting_on_old_posts.py @@ -0,0 +1,31 @@ +# Copyright (c) 2019 Tildes contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Simple script to "officially" close voting on old posts. + +This script should be set up to run regularly (such as every hour). It's not totally +essential since the application can generally prevent voting using the same logic, but +the more often it's run, the more correct the is_voting_closed column will be. +""" + +from tildes.lib.database import get_session_from_config +from tildes.lib.datetime import utc_now +from tildes.models.comment import Comment, VOTING_PERIOD as COMMENT_VOTING_PERIOD +from tildes.models.topic import Topic, VOTING_PERIOD as TOPIC_VOTING_PERIOD + + +def close_voting_on_old_posts(config_path: str) -> None: + """Update is_voting_closed column on all posts older than the voting period.""" + db_session = get_session_from_config(config_path) + + db_session.query(Comment).filter( + Comment.created_time < utc_now() - COMMENT_VOTING_PERIOD, + Comment._is_voting_closed == False, # noqa + ).update({"_is_voting_closed": True}, synchronize_session=False) + + db_session.query(Topic).filter( + Topic.created_time < utc_now() - TOPIC_VOTING_PERIOD, + Topic._is_voting_closed == False, # noqa + ).update({"_is_voting_closed": True}, synchronize_session=False) + + db_session.commit() diff --git a/tildes/scss/modules/_btn.scss b/tildes/scss/modules/_btn.scss index 6725995..6cad365 100644 --- a/tildes/scss/modules/_btn.scss +++ b/tildes/scss/modules/_btn.scss @@ -83,7 +83,6 @@ align-items: center; justify-content: space-between; margin: 0; - padding: 0 0.2rem; // The buttons don't need to be spaced widely on a desktop @media (min-width: $size-md) { diff --git a/tildes/scss/modules/_comment.scss b/tildes/scss/modules/_comment.scss index 9daeaa9..39c053c 100644 --- a/tildes/scss/modules/_comment.scss +++ b/tildes/scss/modules/_comment.scss @@ -166,8 +166,7 @@ .comment-votes { font-size: 0.6rem; - font-weight: bold; - margin: 0 0.4rem; + margin: 0.2rem 0.4rem; } .is-comment-by-op { diff --git a/tildes/sql/init/triggers/comment_votes/comments.sql b/tildes/sql/init/triggers/comment_votes/comments.sql index 1bd31d1..8eb1d66 100644 --- a/tildes/sql/init/triggers/comment_votes/comments.sql +++ b/tildes/sql/init/triggers/comment_votes/comments.sql @@ -8,9 +8,12 @@ BEGIN SET num_votes = num_votes + 1 WHERE comment_id = NEW.comment_id; ELSIF (TG_OP = 'DELETE') THEN + -- Exclude comments with closed voting from decrements so that individual vote + -- records can be deleted while retaining the final vote total. UPDATE comments SET num_votes = num_votes - 1 - WHERE comment_id = OLD.comment_id; + WHERE comment_id = OLD.comment_id + AND is_voting_closed = FALSE; END IF; RETURN NULL; diff --git a/tildes/sql/init/triggers/topic_votes/topics.sql b/tildes/sql/init/triggers/topic_votes/topics.sql index d52f8ad..1dd1e5e 100644 --- a/tildes/sql/init/triggers/topic_votes/topics.sql +++ b/tildes/sql/init/triggers/topic_votes/topics.sql @@ -8,9 +8,12 @@ BEGIN SET num_votes = num_votes + 1 WHERE topic_id = NEW.topic_id; ELSIF (TG_OP = 'DELETE') THEN + -- Exclude topics with closed voting from decrements so that individual vote + -- records can be deleted while retaining the final vote total. UPDATE topics SET num_votes = num_votes - 1 - WHERE topic_id = OLD.topic_id; + WHERE topic_id = OLD.topic_id + AND is_voting_closed = FALSE; END IF; RETURN NULL; diff --git a/tildes/tildes/models/comment/__init__.py b/tildes/tildes/models/comment/__init__.py index e7879c5..7d77348 100644 --- a/tildes/tildes/models/comment/__init__.py +++ b/tildes/tildes/models/comment/__init__.py @@ -1,6 +1,6 @@ """Contains models related to comments.""" -from .comment import Comment, EDIT_GRACE_PERIOD +from .comment import Comment, EDIT_GRACE_PERIOD, VOTING_PERIOD from .comment_bookmark import CommentBookmark from .comment_label import CommentLabel from .comment_notification import CommentNotification diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index 4670296..e7a86f9 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -31,6 +31,9 @@ else: # edits inside this period after creation will not mark the comment as edited EDIT_GRACE_PERIOD = timedelta(minutes=5) +# comments can no longer be voted on when they're older than this +VOTING_PERIOD = timedelta(days=30) + class Comment(DatabaseModel): """Model for a comment on the site. @@ -38,7 +41,7 @@ class Comment(DatabaseModel): Trigger behavior: Incoming: - num_votes will be incremented and decremented by insertions and deletions in - comment_votes. + comment_votes (but no decrementing if voting is closed). Outgoing: - Inserting or deleting rows, or updating is_deleted/is_removed to change visibility will increment or decrement num_comments accordingly on the @@ -88,6 +91,9 @@ class Comment(DatabaseModel): rendered_html: str = Column(Text, nullable=False) excerpt: str = Column(Text, nullable=False, server_default="") num_votes: int = Column(Integer, nullable=False, server_default="0", index=True) + _is_voting_closed: bool = Column( + "is_voting_closed", Boolean, nullable=False, server_default="false", index=True + ) search_tsv: Any = deferred(Column(TSVECTOR)) user: User = relationship("User", lazy=False, innerjoin=True) @@ -170,10 +176,14 @@ class Comment(DatabaseModel): # vote: # - removed comments can't be voted on by anyone + # - if voting has been closed, nobody can vote # - otherwise, logged-in users except the author can vote if self.is_removed: acl.append((Deny, Everyone, "vote")) + if self.is_voting_closed: + acl.append((Deny, Everyone, "vote")) + acl.append((Deny, self.user_id, "vote")) acl.append((Allow, Authenticated, "vote")) @@ -259,6 +269,21 @@ class Comment(DatabaseModel): return f"{self.topic.permalink}#comment-{self.parent_comment_id36}" + @property + def is_voting_closed(self) -> bool: + """Return whether voting on the comment is closed. + + Note that if any logic is changed in here, the close_voting_on_old posts script + should be updated to match. + """ + if self._is_voting_closed: + return True + + if self.age > VOTING_PERIOD: + return True + + return False + @property def label_counts(self) -> Counter: """Counter for number of times each label is on this comment.""" diff --git a/tildes/tildes/models/comment/comment_vote.py b/tildes/tildes/models/comment/comment_vote.py index fa9e03f..a53dc6d 100644 --- a/tildes/tildes/models/comment/comment_vote.py +++ b/tildes/tildes/models/comment/comment_vote.py @@ -22,7 +22,7 @@ class CommentVote(DatabaseModel): Trigger behavior: Outgoing: - Inserting or deleting a row will increment or decrement the num_votes column - for the relevant comment. + for the relevant comment (but no decrementing if its voting is closed). """ __tablename__ = "comment_votes" diff --git a/tildes/tildes/models/topic/__init__.py b/tildes/tildes/models/topic/__init__.py index 4ae5432..5034c38 100644 --- a/tildes/tildes/models/topic/__init__.py +++ b/tildes/tildes/models/topic/__init__.py @@ -1,6 +1,6 @@ """Contains models related to topics.""" -from .topic import EDIT_GRACE_PERIOD, Topic +from .topic import EDIT_GRACE_PERIOD, Topic, VOTING_PERIOD from .topic_bookmark import TopicBookmark from .topic_query import TopicQuery from .topic_schedule import TopicSchedule diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index e3feb3a..3c9aebd 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -47,6 +47,9 @@ else: # edits inside this period after creation will not mark the topic as edited EDIT_GRACE_PERIOD = timedelta(minutes=5) +# topics can no longer be voted on when they're older than this +VOTING_PERIOD = timedelta(days=30) + class Topic(DatabaseModel): """Model for a topic on the site. @@ -54,7 +57,7 @@ class Topic(DatabaseModel): Trigger behavior: Incoming: - num_votes will be incremented and decremented by insertions and deletions in - topic_votes. + topic_votes (but no decrementing if voting is closed). - num_comments will be incremented and decremented by insertions, deletions, and updates to is_deleted in comments. - last_activity_time will be updated by insertions, deletions, and updates to @@ -123,6 +126,9 @@ class Topic(DatabaseModel): ) num_comments: int = Column(Integer, nullable=False, server_default="0", index=True) num_votes: int = Column(Integer, nullable=False, server_default="0", index=True) + _is_voting_closed: bool = Column( + "is_voting_closed", Boolean, nullable=False, server_default="false", index=True + ) tags: List[str] = Column(TagList, nullable=False, server_default="{}") is_official: bool = Column(Boolean, nullable=False, server_default="false") is_locked: bool = Column(Boolean, nullable=False, server_default="false") @@ -235,7 +241,7 @@ class Topic(DatabaseModel): def _update_creation_metric(self) -> None: incr_counter("topics", type=self.topic_type.name.lower()) - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> Sequence[Tuple[str, Any, str]]: # noqa """Pyramid security ACL.""" acl = [] @@ -274,10 +280,14 @@ class Topic(DatabaseModel): # vote: # - removed topics can't be voted on by anyone + # - if voting has been closed, nobody can vote # - otherwise, logged-in users except the author can vote if self.is_removed: acl.append((Deny, Everyone, "vote")) + if self.is_voting_closed: + acl.append((Deny, Everyone, "vote")) + acl.append((Deny, self.user_id, "vote")) acl.append((Allow, Authenticated, "vote")) @@ -402,6 +412,21 @@ class Topic(DatabaseModel): """Return whether the topic is marked as a spoiler.""" return self.has_tag("spoiler") + @property + def is_voting_closed(self) -> bool: + """Return whether voting on the topic is closed. + + Note that if any logic is changed in here, the close_voting_on_old posts script + should be updated to match. + """ + if self._is_voting_closed: + return True + + if self.age > VOTING_PERIOD: + return True + + return False + def has_tag(self, check_tag: str) -> bool: """Return whether the topic has a tag or any sub-tag of it.""" if check_tag in self.tags: diff --git a/tildes/tildes/models/topic/topic_vote.py b/tildes/tildes/models/topic/topic_vote.py index 4b6a37f..06af20b 100644 --- a/tildes/tildes/models/topic/topic_vote.py +++ b/tildes/tildes/models/topic/topic_vote.py @@ -22,7 +22,7 @@ class TopicVote(DatabaseModel): Trigger behavior: Outgoing: - Inserting or deleting a row will increment or decrement the num_votes column - for the relevant topic. + for the relevant topic (but no decrementing if its voting is closed). """ __tablename__ = "topic_votes" diff --git a/tildes/tildes/templates/macros/comments.jinja2 b/tildes/tildes/templates/macros/comments.jinja2 index aa1b89a..6871161 100644 --- a/tildes/tildes/templates/macros/comments.jinja2 +++ b/tildes/tildes/templates/macros/comments.jinja2 @@ -144,12 +144,12 @@ {{ comment.rendered_html|safe }} - {# Show votes at the bottom only if the viewer is logged out #} - {% if not request.user and comment.num_votes > 0 %} -
{{ pluralize(comment.num_votes, "vote") }}
- {% endif %} - + {# Show non-button vote count if the viewer can't vote and it's not their comment #} + {% if not request.has_permission("vote", comment) and comment.num_votes > 0 and comment.user != request.user %} +
{{ pluralize(comment.num_votes, "vote") }}
+ {% endif %} + {% if request.has_permission('vote', comment) %} {% if comment.user_voted is defined and comment.user_voted %}