From cd788d3018cb8a9454b80d79fe4e16d5c4e11fca Mon Sep 17 00:00:00 2001 From: Deimos Date: Mon, 1 Oct 2018 19:29:37 -0600 Subject: [PATCH] Paginate previously-read notifications page This has said that pagination is "coming soon" for quite a while. This is a bit messy in a few ways, but should do the job for now. PaginatedQuery/PaginatedResults might be good to refactor a little in the future to make this kind of thing simpler. --- tildes/tildes/__init__.py | 1 + .../comment/comment_notification_query.py | 41 +++++++++++++++++-- tildes/tildes/models/pagination.py | 23 ++++++----- tildes/tildes/templates/notifications.jinja2 | 5 --- .../templates/notifications_unread.jinja2 | 16 ++++++++ tildes/tildes/views/notifications.py | 21 +++++++--- 6 files changed, 84 insertions(+), 23 deletions(-) diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index ba8dfe1..f55d3d1 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -179,6 +179,7 @@ def current_listing_normal_url( normal_vars_by_route: Dict[str, Tuple[str, ...]] = { "group": ("order", "period", "per_page"), "home": ("order", "period", "per_page"), + "notifications": ("per_page",), "search": ("order", "period", "per_page", "q"), "user": ("per_page",), } diff --git a/tildes/tildes/models/comment/comment_notification_query.py b/tildes/tildes/models/comment/comment_notification_query.py index 71cc2ac..447b5f1 100644 --- a/tildes/tildes/models/comment/comment_notification_query.py +++ b/tildes/tildes/models/comment/comment_notification_query.py @@ -8,18 +8,29 @@ from typing import Any from pyramid.request import Request from sqlalchemy.orm import joinedload -from tildes.models import ModelQuery +from tildes.lib.id import id_to_id36 +from tildes.models.pagination import PaginatedQuery, PaginatedResults from .comment_notification import CommentNotification from .comment_vote import CommentVote -class CommentNotificationQuery(ModelQuery): - """Specialized ModelQuery for CommentNotifications.""" +class CommentNotificationQuery(PaginatedQuery): + """Specialized query class for CommentNotifications.""" def __init__(self, request: Request) -> None: """Initialize a CommentNotificationQuery for the request.""" super().__init__(CommentNotification, request) + def _anchor_subquery(self, anchor_id: int) -> Any: + return ( + self.request.db_session.query(*self.sorting_columns) + .filter( + CommentNotification.user == self.request.user, + CommentNotification.comment_id == anchor_id, + ) + .subquery() + ) + def _attach_extra_data(self) -> "CommentNotificationQuery": """Attach the user's comment votes to the query.""" vote_subquery = ( @@ -56,3 +67,27 @@ class CommentNotificationQuery(ModelQuery): notification.comment.user_voted = result.user_voted return notification + + def get_page(self, per_page: int) -> "CommentNotificationResults": + """Get a page worth of results from the query (`per page` items).""" + return CommentNotificationResults(self, per_page) + + +class CommentNotificationResults(PaginatedResults): + """Specialized results class for CommentNotifications.""" + + @property + 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 id_to_id36(self.results[-1].comment_id) + + @property + 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 id_to_id36(self.results[0].comment_id) diff --git a/tildes/tildes/models/pagination.py b/tildes/tildes/models/pagination.py index 162d6ca..1305218 100644 --- a/tildes/tildes/models/pagination.py +++ b/tildes/tildes/models/pagination.py @@ -20,9 +20,6 @@ class PaginatedQuery(ModelQuery): def __init__(self, model_cls: Any, request: Request) -> None: """Initialize a PaginatedQuery for the specified model and request.""" - if len(model_cls.__table__.primary_key) > 1: - raise TypeError("Only single-col primary key tables are supported") - super().__init__(model_cls, request) # default to sorting by created_time descending (newest first) @@ -118,13 +115,7 @@ class PaginatedQuery(ModelQuery): # an upper bound if the sort order is *ascending* is_anchor_upper_bound = not self.sort_desc - # create a subquery to get comparison values for the anchor item - id_column = list(self.model_cls.__table__.primary_key)[0] - subquery = ( - self.request.db_session.query(*self.sorting_columns) - .filter(id_column == anchor_id) - .subquery() - ) + subquery = self._anchor_subquery(anchor_id) # restrict the results to items on the right "side" of the anchor item if is_anchor_upper_bound: @@ -134,6 +125,18 @@ class PaginatedQuery(ModelQuery): return query + def _anchor_subquery(self, anchor_id: int) -> Any: + """Return a subquery to get comparison values for the anchor item.""" + if len(self.model_cls.__table__.primary_key) > 1: + raise TypeError("Only single-col primary key tables are supported") + + id_column = list(self.model_cls.__table__.primary_key)[0] + return ( + self.request.db_session.query(*self.sorting_columns) + .filter(id_column == anchor_id) + .subquery() + ) + def _finalize(self) -> "PaginatedQuery": """Finalize the query before execution.""" query = super()._finalize() diff --git a/tildes/tildes/templates/notifications.jinja2 b/tildes/tildes/templates/notifications.jinja2 index e13ba97..9d5e8ec 100644 --- a/tildes/tildes/templates/notifications.jinja2 +++ b/tildes/tildes/templates/notifications.jinja2 @@ -6,8 +6,3 @@ {% block title %}Previously read notifications{% endblock %} {% block main_heading %}Previously read notifications{% endblock %} - -{% block content %} -

This page shows your most recent, previously read notifications (up to a max of 100, pagination coming soon)

-{{ super() }} -{% endblock %} diff --git a/tildes/tildes/templates/notifications_unread.jinja2 b/tildes/tildes/templates/notifications_unread.jinja2 index e927b22..cf07d81 100644 --- a/tildes/tildes/templates/notifications_unread.jinja2 +++ b/tildes/tildes/templates/notifications_unread.jinja2 @@ -52,6 +52,22 @@ {% endfor %} + + {% if notifications.has_prev_page or notifications.has_next_page %} + + {% endif %} {% else %}

No unread notifications.

Go to previously read notifications

diff --git a/tildes/tildes/views/notifications.py b/tildes/tildes/views/notifications.py index a0baa09..7a84285 100644 --- a/tildes/tildes/views/notifications.py +++ b/tildes/tildes/views/notifications.py @@ -6,9 +6,11 @@ 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.enums import CommentLabelOption from tildes.models.comment import CommentNotification +from tildes.schemas.topic_listing import TopicListingSchema @view_config(route_name="notifications_unread", renderer="notifications_unread.jinja2") @@ -35,9 +37,12 @@ def get_user_unread_notifications(request: Request) -> dict: @view_config(route_name="notifications", renderer="notifications.jinja2") -def get_user_notifications(request: Request) -> dict: - """Show the most recent 100 of the logged-in user's read notifications.""" - notifications = ( +@use_kwargs(TopicListingSchema(only=("after", "before", "per_page"))) +def get_user_notifications( + request: Request, after: str, before: str, per_page: int +) -> dict: + """Show the logged-in user's previously-read notifications.""" + query = ( request.query(CommentNotification) .join_all_relationships() .filter( @@ -45,8 +50,14 @@ def get_user_notifications(request: Request) -> dict: CommentNotification.is_unread == False, # noqa ) .order_by(desc(CommentNotification.created_time)) - .limit(100) - .all() ) + if before: + query = query.before_id36(before) + + if after: + query = query.after_id36(after) + + notifications = query.get_page(per_page) + return {"notifications": notifications, "comment_label_options": CommentLabelOption}