From 03f5450414155483fff26d5c82f54f5eabd2225c Mon Sep 17 00:00:00 2001 From: Deimos Date: Thu, 2 Aug 2018 23:58:30 -0600 Subject: [PATCH 1/5] PaginatedResults: add props for before/after IDs This adds two new properties to PaginatedResults - .next_page_after_id and .prev_page_before_id. These can be used when creating the links to the before/after pages, instead of needing to access the right element/property "manually". This will be most useful in listings that contain items of multiple types (such as comments and topics), since we won't necessarily know which type the first/last elements are. --- tildes/tildes/models/pagination.py | 18 +++++++++++++++++- tildes/tildes/templates/topic_listing.jinja2 | 4 ++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/tildes/tildes/models/pagination.py b/tildes/tildes/models/pagination.py index c4c6d8c..d80ff64 100644 --- a/tildes/tildes/models/pagination.py +++ b/tildes/tildes/models/pagination.py @@ -3,7 +3,7 @@ from typing import Any, Iterator, List, Optional, TypeVar from pyramid.request import Request -from sqlalchemy import Column, func +from sqlalchemy import Column, func, inspect from tildes.lib.id import id36_to_id from .model_query import ModelQuery @@ -204,3 +204,19 @@ class PaginatedResults: def __len__(self) -> int: """Return the number of results.""" return len(self.results) + + @property + def next_page_after_id(self) -> int: + """Return "after" ID that should be used to fetch the next page.""" + if not self.has_next_page: + raise AttributeError + + return inspect(self.results[-1]).identity[0] + + @property + def prev_page_before_id(self) -> int: + """Return "before" ID that should be used to fetch the prev page.""" + if not self.has_prev_page: + raise AttributeError + + return inspect(self.results[0]).identity[0] diff --git a/tildes/tildes/templates/topic_listing.jinja2 b/tildes/tildes/templates/topic_listing.jinja2 index 611fed2..2439d8d8 100644 --- a/tildes/tildes/templates/topic_listing.jinja2 +++ b/tildes/tildes/templates/topic_listing.jinja2 @@ -154,13 +154,13 @@ From f95a504ca4ead4a9093138f8eb598d161c3ea259 Mon Sep 17 00:00:00 2001 From: Deimos Date: Fri, 3 Aug 2018 00:05:10 -0600 Subject: [PATCH 2/5] PaginatedQuery: set a default for _sort_column Previously, _sort_column wasn't set by default and needed to be set by the query, but some of the other methods would fail if it hadn't been set. This just defaults it to use the created_time column as the default sort, which should always be present on models being queried by this class (for now, at least). --- tildes/tildes/models/pagination.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tildes/tildes/models/pagination.py b/tildes/tildes/models/pagination.py index d80ff64..9e47562 100644 --- a/tildes/tildes/models/pagination.py +++ b/tildes/tildes/models/pagination.py @@ -22,7 +22,8 @@ class PaginatedQuery(ModelQuery): super().__init__(model_cls, request) - self._sort_column: Optional[Column] = None + # default to sorting by created_time descending (newest first) + self._sort_column = model_cls.created_time self.sort_desc = True self.after_id: Optional[int] = None @@ -135,17 +136,16 @@ class PaginatedQuery(ModelQuery): """Finalize the query before execution.""" query = super()._finalize() - if self._sort_column: - # if the query is reversed, we need to sort in the opposite dir - # (basically self.sort_desc XOR self.is_reversed) - desc = self.sort_desc - if self.is_reversed: - desc = not desc + # if the query is reversed, we need to sort in the opposite dir + # (basically self.sort_desc XOR self.is_reversed) + desc = self.sort_desc + if self.is_reversed: + desc = not desc - if desc: - query = query.order_by(*self.sorting_columns_desc) - else: - query = query.order_by(*self.sorting_columns) + if desc: + query = query.order_by(*self.sorting_columns_desc) + else: + query = query.order_by(*self.sorting_columns) # pylint: disable=protected-access query = query._apply_before_or_after() From 7276b15e395e4b84ff05e3e48741b3edb995afb6 Mon Sep 17 00:00:00 2001 From: Deimos Date: Fri, 3 Aug 2018 00:23:14 -0600 Subject: [PATCH 3/5] Add paginated topics/comments pages to own profile This needs some more work still to clean up a few things, but it should be good enough for now. This allows users to see (only on their own user page), separate "tabs" for Topics and Comments, and those separate listings are paginated. --- tildes/tildes/__init__.py | 7 +- tildes/tildes/models/comment/comment_query.py | 4 +- .../tildes/templates/macros/user_menu.jinja2 | 2 +- tildes/tildes/templates/user.jinja2 | 42 +++++++++++- tildes/tildes/views/user.py | 66 +++++++++++++++++-- 5 files changed, 105 insertions(+), 16 deletions(-) diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index 02df50d..655aee9 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -137,10 +137,11 @@ def current_listing_base_url( The `query` argument allows adding query variables to the generated url. """ - if request.matched_route.name not in ('home', 'group'): + if request.matched_route.name not in ('home', 'group', 'user'): raise AttributeError('Current route is not supported.') - base_view_vars = ('order', 'period', 'per_page', 'tag', 'unfiltered') + base_view_vars = ( + 'order', 'period', 'per_page', 'tag', 'type', 'unfiltered') query_vars = { key: val for key, val in request.GET.copy().items() if key in base_view_vars @@ -165,7 +166,7 @@ def current_listing_normal_url( The `query` argument allows adding query variables to the generated url. """ - if request.matched_route.name not in ('home', 'group'): + if request.matched_route.name not in ('home', 'group', 'user'): raise AttributeError('Current route is not supported.') normal_view_vars = ('order', 'period', 'per_page') diff --git a/tildes/tildes/models/comment/comment_query.py b/tildes/tildes/models/comment/comment_query.py index 672367b..fb029f8 100644 --- a/tildes/tildes/models/comment/comment_query.py +++ b/tildes/tildes/models/comment/comment_query.py @@ -4,12 +4,12 @@ from typing import Any from pyramid.request import Request -from tildes.models import ModelQuery +from tildes.models.pagination import PaginatedQuery from .comment import Comment from .comment_vote import CommentVote -class CommentQuery(ModelQuery): +class CommentQuery(PaginatedQuery): """Specialized ModelQuery for Comments.""" def __init__(self, request: Request) -> None: diff --git a/tildes/tildes/templates/macros/user_menu.jinja2 b/tildes/tildes/templates/macros/user_menu.jinja2 index 761a85a..0033ab9 100644 --- a/tildes/tildes/templates/macros/user_menu.jinja2 +++ b/tildes/tildes/templates/macros/user_menu.jinja2 @@ -7,7 +7,7 @@ diff --git a/tildes/tildes/templates/user.jinja2 b/tildes/tildes/templates/user.jinja2 index 1e3a1f1..a043528 100644 --- a/tildes/tildes/templates/user.jinja2 +++ b/tildes/tildes/templates/user.jinja2 @@ -10,13 +10,33 @@ {{ user.username }} {% endblock %} -{% block main_heading %}{{ user.username }}'s recent activity{% endblock %} +{# Only show the heading if they can't see the type tabs #} +{% block main_heading %} + {% if not request.has_permission('view_types', user) %} + {{ user.username }}'s recent activity + {% endif %} +{% endblock %} {% block content %} +{% if request.has_permission('view_types', user) %} +
+ +
  • + Recent activity +
    • +
  • + Topics +
    • +
  • + Comments +
    • +     +
    +{% endif %} -{% if merged_posts %} +{% if posts %}
      - {% for post in merged_posts if request.has_permission('view', post) %} + {% for post in posts if request.has_permission('view', post) %}
    1. {% if post is topic %} {{ render_topic_for_listing(post, show_group=True) }} @@ -27,6 +47,22 @@
      2. {% endfor %}
    + + {% if post_type and (posts.has_prev_page or posts.has_next_page) %} + + {% endif %} {% else %}

    This user hasn't made any posts

    diff --git a/tildes/tildes/views/user.py b/tildes/tildes/views/user.py index f5ced8a..c7da379 100644 --- a/tildes/tildes/views/user.py +++ b/tildes/tildes/views/user.py @@ -1,19 +1,24 @@ """Views related to a specific user.""" +from typing import List, Union + +from marshmallow.fields import String +from marshmallow.validate import OneOf from pyramid.request import Request from pyramid.view import view_config from sqlalchemy.sql.expression import desc +from webargs.pyramidparser import use_kwargs from tildes.models.comment import Comment from tildes.models.topic import Topic -from tildes.models.user import UserInviteCode - +from tildes.models.user import User, UserInviteCode +from tildes.schemas.topic_listing import TopicListingSchema -@view_config(route_name='user', renderer='user.jinja2') -def get_user(request: Request) -> dict: - """Generate the main user info page.""" - user = request.context +def _get_user_recent_activity( + request: Request, + user: User, +) -> List[Union[Comment, Topic]]: page_size = 20 # Since we don't know how many comments or topics will be needed to make @@ -54,9 +59,56 @@ def get_user(request: Request) -> dict: ) merged_posts = merged_posts[:page_size] + return merged_posts + + +@view_config(route_name='user', renderer='user.jinja2') +@use_kwargs(TopicListingSchema(only=('after', 'before', 'per_page'))) +@use_kwargs({ + 'post_type': String( + load_from='type', + validate=OneOf(('topic', 'comment')), + ), +}) +def get_user( + request: Request, + after: str, + before: str, + per_page: int, + post_type: str = None, +) -> dict: + """Generate the main user history page.""" + user = request.context + + if not request.has_permission('view_types', user): + post_type = None + + if post_type: + if post_type == 'topic': + query = request.query(Topic).filter(Topic.user == user) + elif post_type == 'comment': + query = request.query(Comment).filter(Comment.user == user) + + if before: + query = query.before_id36(before) + + if after: + query = query.after_id36(after) + + query = query.join_all_relationships() + + # include removed posts if the user's looking at their own page + if user == request.user: + query = query.include_removed() + + posts = query.get_page(per_page) + else: + posts = _get_user_recent_activity(request, user) + return { 'user': user, - 'merged_posts': merged_posts, + 'posts': posts, + 'post_type': post_type, } From 2de03d719e5bf03a3f3c548d52e4e9a7dcbc9787 Mon Sep 17 00:00:00 2001 From: Deimos Date: Fri, 3 Aug 2018 14:06:34 -0600 Subject: [PATCH 4/5] PaginatedResults: use id36s for before/after This was previously using regular integer IDs, but it should have been ID36s. --- tildes/tildes/models/pagination.py | 16 +++++++++------- tildes/tildes/templates/topic_listing.jinja2 | 4 ++-- tildes/tildes/templates/user.jinja2 | 4 ++-- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/tildes/tildes/models/pagination.py b/tildes/tildes/models/pagination.py index 9e47562..cd52d2f 100644 --- a/tildes/tildes/models/pagination.py +++ b/tildes/tildes/models/pagination.py @@ -5,7 +5,7 @@ from typing import Any, Iterator, List, Optional, TypeVar from pyramid.request import Request from sqlalchemy import Column, func, inspect -from tildes.lib.id import id36_to_id +from tildes.lib.id import id_to_id36, id36_to_id from .model_query import ModelQuery @@ -206,17 +206,19 @@ class PaginatedResults: return len(self.results) @property - def next_page_after_id(self) -> int: - """Return "after" ID that should be used to fetch the next page.""" + def next_page_after_id36(self) -> str: + """Return "after" ID36 that should be used to fetch the next page.""" if not self.has_next_page: raise AttributeError - return inspect(self.results[-1]).identity[0] + next_id = inspect(self.results[-1]).identity[0] + return id_to_id36(next_id) @property - def prev_page_before_id(self) -> int: - """Return "before" ID that should be used to fetch the prev page.""" + def prev_page_before_id36(self) -> str: + """Return "before" ID36 that should be used to fetch the prev page.""" if not self.has_prev_page: raise AttributeError - return inspect(self.results[0]).identity[0] + prev_id = inspect(self.results[0]).identity[0] + return id_to_id36(prev_id) diff --git a/tildes/tildes/templates/topic_listing.jinja2 b/tildes/tildes/templates/topic_listing.jinja2 index 2439d8d8..ab4bf51 100644 --- a/tildes/tildes/templates/topic_listing.jinja2 +++ b/tildes/tildes/templates/topic_listing.jinja2 @@ -154,13 +154,13 @@ diff --git a/tildes/tildes/templates/user.jinja2 b/tildes/tildes/templates/user.jinja2 index a043528..b6362fe 100644 --- a/tildes/tildes/templates/user.jinja2 +++ b/tildes/tildes/templates/user.jinja2 @@ -52,13 +52,13 @@ From b6c5be0593ed93348f10ca4c59764f96fe74ce5c Mon Sep 17 00:00:00 2001 From: Deimos Date: Fri, 3 Aug 2018 17:06:01 -0600 Subject: [PATCH 5/5] Fix excessive margin on
      at end of block The bottom margin on
        was causing some weird spacing when it's the last element inside a block. This is most noticeable on a blockquote where the background color extends further down than it should. --- tildes/scss/_base.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tildes/scss/_base.scss b/tildes/scss/_base.scss index c1087e2..b74483c 100644 --- a/tildes/scss/_base.scss +++ b/tildes/scss/_base.scss @@ -161,6 +161,10 @@ ol { margin-top: 0.2rem; max-width: $paragraph-max-width - 2rem; } + + &:last-child { + margin-bottom: 0.2rem; + } } p {