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