From 244bdda52663c93d9aa7eca6585087d7b690cc9c Mon Sep 17 00:00:00 2001 From: Ivan Fonseca Date: Fri, 17 Aug 2018 13:58:30 -0400 Subject: [PATCH] Add bookmarking for topics and comments Allows users to "bookmark" topics and comments, and view their list of bookmarks through their user page. --- ...4_add_topic_and_comment_bookmark_tables.py | 87 +++++++++++++++++++ tildes/tildes/__init__.py | 2 + tildes/tildes/database_models.py | 3 +- tildes/tildes/models/comment/__init__.py | 1 + tildes/tildes/models/comment/comment.py | 4 + .../tildes/models/comment/comment_bookmark.py | 38 ++++++++ tildes/tildes/models/comment/comment_query.py | 21 ++++- tildes/tildes/models/topic/__init__.py | 1 + tildes/tildes/models/topic/topic.py | 4 + tildes/tildes/models/topic/topic_bookmark.py | 38 ++++++++ tildes/tildes/models/topic/topic_query.py | 19 +++- tildes/tildes/routes.py | 8 ++ tildes/tildes/templates/bookmarks.jinja2 | 59 +++++++++++++ .../tildes/templates/macros/comments.jinja2 | 23 +++++ .../tildes/templates/macros/user_menu.jinja2 | 19 ++-- tildes/tildes/templates/topic.jinja2 | 24 ++++- tildes/tildes/views/api/web/comment.py | 63 ++++++++++++++ tildes/tildes/views/api/web/topic.py | 40 ++++++++- tildes/tildes/views/bookmarks.py | 69 +++++++++++++++ 19 files changed, 511 insertions(+), 12 deletions(-) create mode 100644 tildes/alembic/versions/53567981cdf4_add_topic_and_comment_bookmark_tables.py create mode 100644 tildes/tildes/models/comment/comment_bookmark.py create mode 100644 tildes/tildes/models/topic/topic_bookmark.py create mode 100644 tildes/tildes/templates/bookmarks.jinja2 create mode 100644 tildes/tildes/views/bookmarks.py diff --git a/tildes/alembic/versions/53567981cdf4_add_topic_and_comment_bookmark_tables.py b/tildes/alembic/versions/53567981cdf4_add_topic_and_comment_bookmark_tables.py new file mode 100644 index 0000000..0c75c21 --- /dev/null +++ b/tildes/alembic/versions/53567981cdf4_add_topic_and_comment_bookmark_tables.py @@ -0,0 +1,87 @@ +"""Add topic and comment bookmark tables + +Revision ID: 53567981cdf4 +Revises: 5a7dc1032efc +Create Date: 2018-08-17 17:57:22.171858 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "53567981cdf4" +down_revision = "5a7dc1032efc" +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + "topic_bookmarks", + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("topic_id", sa.Integer(), nullable=False), + sa.Column( + "created_time", + sa.TIMESTAMP(timezone=True), + server_default=sa.text("NOW()"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["topic_id"], + ["topics.topic_id"], + name=op.f("fk_topic_bookmarks_topic_id_topics"), + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["users.user_id"], + name=op.f("fk_topic_bookmarks_user_id_users"), + ), + sa.PrimaryKeyConstraint("user_id", "topic_id", name=op.f("pk_topic_bookmarks")), + ) + op.create_index( + op.f("ix_topic_bookmarks_created_time"), + "topic_bookmarks", + ["created_time"], + unique=False, + ) + + op.create_table( + "comment_bookmarks", + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("comment_id", sa.Integer(), nullable=False), + sa.Column( + "created_time", + sa.TIMESTAMP(timezone=True), + server_default=sa.text("NOW()"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["comment_id"], + ["comments.comment_id"], + name=op.f("fk_comment_bookmarks_comment_id_comments"), + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["users.user_id"], + name=op.f("fk_comment_bookmarks_user_id_users"), + ), + sa.PrimaryKeyConstraint( + "user_id", "comment_id", name=op.f("pk_comment_bookmarks") + ), + ) + op.create_index( + op.f("ix_comment_bookmarks_created_time"), + "comment_bookmarks", + ["created_time"], + unique=False, + ) + + +def downgrade(): + op.drop_index( + op.f("ix_comment_bookmarks_created_time"), table_name="comment_bookmarks" + ) + op.drop_table("comment_bookmarks") + op.drop_index(op.f("ix_topic_bookmarks_created_time"), table_name="topic_bookmarks") + op.drop_table("topic_bookmarks") diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index f55d3d1..cc5bc04 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -143,6 +143,7 @@ def current_listing_base_url( The `query` argument allows adding query variables to the generated url. """ base_vars_by_route: Dict[str, Tuple[str, ...]] = { + "bookmarks": ("per_page", "type"), "group": ("order", "period", "per_page", "tag", "unfiltered"), "home": ("order", "period", "per_page", "tag", "unfiltered"), "search": ("order", "period", "per_page", "q"), @@ -177,6 +178,7 @@ def current_listing_normal_url( The `query` argument allows adding query variables to the generated url. """ normal_vars_by_route: Dict[str, Tuple[str, ...]] = { + "bookmarks": ("order", "period", "per_page"), "group": ("order", "period", "per_page"), "home": ("order", "period", "per_page"), "notifications": ("per_page",), diff --git a/tildes/tildes/database_models.py b/tildes/tildes/database_models.py index 089be4b..4139832 100644 --- a/tildes/tildes/database_models.py +++ b/tildes/tildes/database_models.py @@ -7,6 +7,7 @@ both Alembic and the script for initializing the database can simply import * fr from tildes.models.comment import ( Comment, + CommentBookmark, CommentLabel, CommentNotification, CommentVote, @@ -15,5 +16,5 @@ from tildes.models.group import Group, GroupSubscription from tildes.models.log import Log from tildes.models.message import MessageConversation, MessageReply from tildes.models.scraper import ScraperResult -from tildes.models.topic import Topic, TopicVisit, TopicVote +from tildes.models.topic import Topic, TopicBookmark, TopicVisit, TopicVote from tildes.models.user import User, UserGroupSettings, UserInviteCode diff --git a/tildes/tildes/models/comment/__init__.py b/tildes/tildes/models/comment/__init__.py index fa574c5..b9cb436 100644 --- a/tildes/tildes/models/comment/__init__.py +++ b/tildes/tildes/models/comment/__init__.py @@ -1,6 +1,7 @@ """Contains models related to comments.""" from .comment import Comment, EDIT_GRACE_PERIOD +from .comment_bookmark import CommentBookmark from .comment_notification import CommentNotification from .comment_notification_query import CommentNotificationQuery from .comment_query import CommentQuery diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index 89a4a6e..cf11c9f 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -203,6 +203,10 @@ class Comment(DatabaseModel): # - logged-in users can mark comments read acl.append((Allow, Authenticated, "mark_read")) + # bookmark: + # - logged-in users can bookmark comments + acl.append((Allow, Authenticated, "bookmark")) + # tools that require specifically granted permissions acl.append((Allow, "admin", "remove")) acl.append((Allow, "admin", "view_labels")) diff --git a/tildes/tildes/models/comment/comment_bookmark.py b/tildes/tildes/models/comment/comment_bookmark.py new file mode 100644 index 0000000..81ad333 --- /dev/null +++ b/tildes/tildes/models/comment/comment_bookmark.py @@ -0,0 +1,38 @@ +"""Contains the CommentBookmark class.""" + +from datetime import datetime + +from sqlalchemy import Column, ForeignKey, Integer, TIMESTAMP +from sqlalchemy.orm import relationship +from sqlalchemy.sql.expression import text + +from tildes.models import DatabaseModel +from tildes.models.comment import Comment +from tildes.models.user import User + + +class CommentBookmark(DatabaseModel): + """Model for a comment bookmark.""" + + __tablename__ = "comment_bookmarks" + + user_id: int = Column( + Integer, ForeignKey("users.user_id"), nullable=False, primary_key=True + ) + comment_id: int = Column( + Integer, ForeignKey("comments.comment_id"), nullable=False, primary_key=True + ) + created_time: datetime = Column( + TIMESTAMP(timezone=True), + nullable=False, + index=True, + server_default=text("NOW()"), + ) + + user: User = relationship("User", innerjoin=True) + comment: Comment = relationship("Comment", innerjoin=True) + + def __init__(self, user: User, comment: Comment) -> None: + """Create a new comment bookmark.""" + self.user = user + self.comment = comment diff --git a/tildes/tildes/models/comment/comment_query.py b/tildes/tildes/models/comment/comment_query.py index 5b35d05..912053c 100644 --- a/tildes/tildes/models/comment/comment_query.py +++ b/tildes/tildes/models/comment/comment_query.py @@ -6,9 +6,11 @@ from typing import Any from pyramid.request import Request +from sqlalchemy.sql.expression import and_ from tildes.models.pagination import PaginatedQuery from .comment import Comment +from .comment_bookmark import CommentBookmark from .comment_vote import CommentVote @@ -25,10 +27,11 @@ class CommentQuery(PaginatedQuery): def _attach_extra_data(self) -> "CommentQuery": """Attach the extra user data to the query.""" + # pylint: disable=protected-access if not self.request.user: return self - return self._attach_vote_data() + return self._attach_vote_data()._attach_bookmark_data() def _attach_vote_data(self) -> "CommentQuery": """Add a subquery to include whether the user has voted.""" @@ -43,6 +46,19 @@ class CommentQuery(PaginatedQuery): ) return self.add_columns(vote_subquery) + def _attach_bookmark_data(self) -> "CommentQuery": + """Join the data related to whether the user has bookmarked the comment.""" + query = self.outerjoin( + CommentBookmark, + and_( + CommentBookmark.comment_id == Comment.comment_id, + CommentBookmark.user == self.request.user, + ), + ) + query = query.add_columns(CommentBookmark.created_time) + + return query + @staticmethod def _process_result(result: Any) -> Comment: """Merge additional user-context data in result onto the comment.""" @@ -50,8 +66,11 @@ class CommentQuery(PaginatedQuery): # the result is already a Comment, no merging needed comment = result comment.user_voted = False + comment.bookmark_created_time = None else: comment = result.Comment comment.user_voted = result.user_voted + comment.bookmark_created_time = result.created_time + return comment diff --git a/tildes/tildes/models/topic/__init__.py b/tildes/tildes/models/topic/__init__.py index f488182..2255085 100644 --- a/tildes/tildes/models/topic/__init__.py +++ b/tildes/tildes/models/topic/__init__.py @@ -1,6 +1,7 @@ """Contains models related to topics.""" from .topic import EDIT_GRACE_PERIOD, Topic +from .topic_bookmark import TopicBookmark from .topic_query import TopicQuery from .topic_visit import TopicVisit from .topic_vote import TopicVote diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index 168ad2b..8dc4a73 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -297,6 +297,10 @@ class Topic(DatabaseModel): acl.append((Allow, "admin", "edit_title")) acl.append((Allow, "topic.edit_title", "edit_title")) + # bookmark: + # - logged-in users can bookmark topics + acl.append((Allow, Authenticated, "bookmark")) + acl.append(DENY_ALL) return acl diff --git a/tildes/tildes/models/topic/topic_bookmark.py b/tildes/tildes/models/topic/topic_bookmark.py new file mode 100644 index 0000000..8c27860 --- /dev/null +++ b/tildes/tildes/models/topic/topic_bookmark.py @@ -0,0 +1,38 @@ +"""Contains the TopicBookmark class.""" + +from datetime import datetime + +from sqlalchemy import Column, ForeignKey, Integer, TIMESTAMP +from sqlalchemy.orm import relationship +from sqlalchemy.sql.expression import text + +from tildes.models import DatabaseModel +from tildes.models.topic import Topic +from tildes.models.user import User + + +class TopicBookmark(DatabaseModel): + """Model for a topic bookmark.""" + + __tablename__ = "topic_bookmarks" + + user_id: int = Column( + Integer, ForeignKey("users.user_id"), nullable=False, primary_key=True + ) + topic_id: int = Column( + Integer, ForeignKey("topics.topic_id"), nullable=False, primary_key=True + ) + created_time: datetime = Column( + TIMESTAMP(timezone=True), + nullable=False, + index=True, + server_default=text("NOW()"), + ) + + user: User = relationship("User", innerjoin=True) + topic: Topic = relationship("Topic", innerjoin=True) + + def __init__(self, user: User, topic: Topic) -> None: + """Create a new topic bookmark.""" + self.user = user + self.topic = topic diff --git a/tildes/tildes/models/topic/topic_query.py b/tildes/tildes/models/topic/topic_query.py index 751f4d3..62c1bc4 100644 --- a/tildes/tildes/models/topic/topic_query.py +++ b/tildes/tildes/models/topic/topic_query.py @@ -15,6 +15,7 @@ from tildes.lib.datetime import SimpleHoursPeriod, utc_now from tildes.models.group import Group from tildes.models.pagination import PaginatedQuery from .topic import Topic +from .topic_bookmark import TopicBookmark from .topic_visit import TopicVisit from .topic_vote import TopicVote @@ -38,7 +39,7 @@ class TopicQuery(PaginatedQuery): return self # pylint: disable=protected-access - return self._attach_vote_data()._attach_visit_data() + return self._attach_vote_data()._attach_visit_data()._attach_bookmark_data() def _attach_vote_data(self) -> "TopicQuery": """Add a subquery to include whether the user has voted.""" @@ -53,6 +54,19 @@ class TopicQuery(PaginatedQuery): ) return self.add_columns(vote_subquery) + def _attach_bookmark_data(self) -> "TopicQuery": + """Join the data related to whether the user has bookmarked the topic.""" + query = self.outerjoin( + TopicBookmark, + and_( + TopicBookmark.topic_id == Topic.topic_id, + TopicBookmark.user == self.request.user, + ), + ) + query = query.add_columns(TopicBookmark.created_time) + + return query + def _attach_visit_data(self) -> "TopicQuery": """Join the data related to the user's last visit to the topic(s).""" # pylint: disable=assignment-from-no-return @@ -80,6 +94,7 @@ class TopicQuery(PaginatedQuery): # the result is already a Topic, no merging needed topic = result topic.user_voted = False + topic.bookmark_created_time = None topic.last_visit_time = None topic.comments_since_last_visit = None else: @@ -87,6 +102,8 @@ class TopicQuery(PaginatedQuery): topic.user_voted = result.user_voted + topic.bookmark_created_time = result.created_time + topic.last_visit_time = result.visit_time if result.num_comments is not None: new_comments = topic.num_comments - result.num_comments diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index 5e91beb..6d9b697 100644 --- a/tildes/tildes/routes.py +++ b/tildes/tildes/routes.py @@ -83,6 +83,8 @@ def includeme(config: Configurator) -> None: "settings_password_change", "/settings/password_change", factory=LoggedInFactory ) + config.add_route("bookmarks", "/bookmarks", factory=LoggedInFactory) + config.add_route("invite", "/invite", factory=LoggedInFactory) # Route to expose metrics to Prometheus @@ -122,6 +124,9 @@ def add_intercooler_routes(config: Configurator) -> None: add_ic_route("topic_title", "/topics/{topic_id36}/title", factory=topic_by_id36) add_ic_route("topic_vote", "/topics/{topic_id36}/vote", factory=topic_by_id36) add_ic_route("topic_tags", "/topics/{topic_id36}/tags", factory=topic_by_id36) + add_ic_route( + "topic_bookmark", "/topics/{topic_id36}/bookmark", factory=topic_by_id36 + ) add_ic_route("comment", "/comments/{comment_id36}", factory=comment_by_id36) add_ic_route( @@ -138,6 +143,9 @@ def add_intercooler_routes(config: Configurator) -> None: "/comments/{comment_id36}/labels/{name}", factory=comment_by_id36, ) + add_ic_route( + "comment_bookmark", "/comments/{comment_id36}/bookmark", factory=comment_by_id36 + ) add_ic_route( "comment_mark_read", "/comments/{comment_id36}/mark_read", diff --git a/tildes/tildes/templates/bookmarks.jinja2 b/tildes/tildes/templates/bookmarks.jinja2 new file mode 100644 index 0000000..8b77003 --- /dev/null +++ b/tildes/tildes/templates/bookmarks.jinja2 @@ -0,0 +1,59 @@ +{% extends 'base_user_menu.jinja2' %} + +{% from 'macros/comments.jinja2' import render_single_comment with context %} +{% from 'macros/links.jinja2' import group_linked, username_linked %} +{% from 'macros/topics.jinja2' import render_topic_for_listing with context %} + +{% block title %}Bookmarks{% endblock %} + +{% block main_heading %}Bookmarks{% endblock %} + +{% block content %} + +
+ +
  • + Topics +
    • +
  • + Comments +
    • +     +
    + +{% if posts %} +
      + {% for post in posts if request.has_permission('view', post) %} +
    1. + {% if post is topic %} + {{ render_topic_for_listing(post, show_group=True) }} + {% elif post is comment %} +

      Comment on {{ post.topic.title }} in {{ group_linked(post.topic.group.path) }}

      + {{ render_single_comment(post) }} + {% endif %} +
      2. + {% endfor %} +
    + + {% if post_type and (posts.has_prev_page or posts.has_next_page) %} + + {% endif %} +{% else %} +
    +

    You haven't bookmarked any posts

    +
    +{% endif %} + +{% endblock %} diff --git a/tildes/tildes/templates/macros/comments.jinja2 b/tildes/tildes/templates/macros/comments.jinja2 index 6ac9439..2657bad 100644 --- a/tildes/tildes/templates/macros/comments.jinja2 +++ b/tildes/tildes/templates/macros/comments.jinja2 @@ -194,6 +194,29 @@ >Delete {% endif %} + {% if request.has_permission('bookmark', comment) %} + {% if comment.bookmark_created_time %} +
  • Bookmarked + {% else %} +
  • Bookmark + {% endif %} +
    • + {% endif %} + {% if request.has_permission("remove", comment) %}
  • {% if not comment.is_removed %} diff --git a/tildes/tildes/templates/macros/user_menu.jinja2 b/tildes/tildes/templates/macros/user_menu.jinja2 index 4f2b042..a7c6e55 100644 --- a/tildes/tildes/templates/macros/user_menu.jinja2 +++ b/tildes/tildes/templates/macros/user_menu.jinja2 @@ -54,13 +54,18 @@
  • Misc