Add pagination on main user page (topics+comments)

I hate almost everything about this, but it seems to work fine overall.

Pagination is still restricted to users themselves currently, so you can
only page back through history and switch to topics or comments only on
yourself. This won't stay like this for much longer though.

tildes/tildes/models/pagination.py

@@ -3,7 +3,8 @@
 
 """Contains the PaginatedQuery and PaginatedResults classes."""
 
-from typing import Any, Iterator, List, Optional, TypeVar
+from itertools import chain
+from typing import Any, Iterator, List, Optional, Sequence, TypeVar
 
 from pyramid.request import Request
 from sqlalchemy import Column, func, inspect
@@ -29,6 +30,8 @@ class PaginatedQuery(ModelQuery):
         self.after_id: Optional[int] = None
         self.before_id: Optional[int] = None
 
+        self._anchor_table = model_cls.__table__
+
     def __iter__(self) -> Iterator[ModelType]:
         """Iterate over the results of the query, reversed if necessary."""
         if not self.is_reversed:
@@ -44,14 +47,22 @@ class PaginatedQuery(ModelQuery):
         if not self._sort_column:
             raise AttributeError
 
-        # always add a final sort by the ID so keyset pagination works properly
-        return [self._sort_column] + list(self.model_cls.__table__.primary_key)
+        if self.is_anchor_same_type:
+            # add a final sort by the ID so keyset pagination works properly
+            return [self._sort_column] + list(self.model_cls.__table__.primary_key)
+        else:
+            return [self._sort_column]
 
     @property
     def sorting_columns_desc(self) -> List[Column]:
         """Return descending versions of the sorting columns."""
         return [col.desc() for col in self.sorting_columns]
 
+    @property
+    def is_anchor_same_type(self) -> bool:
+        """Return whether the anchor type is the same as the overall model_cls."""
+        return self._anchor_table == self.model_cls.__table__
+
     @property
     def is_reversed(self) -> bool:
         """Return whether the query is operating "in reverse".
@@ -73,6 +84,13 @@ class PaginatedQuery(ModelQuery):
         """
         return bool(self.before_id)
 
+    def anchor_type(self, anchor_type: str) -> "PaginatedQuery":
+        """Set the type of the "anchor" (before/after item) (generative)."""
+        anchor_table_name = anchor_type + "s"
+        self._anchor_table = self.model_cls.metadata.tables.get(anchor_table_name)
+
+        return self
+
     def after_id36(self, id36: str) -> "PaginatedQuery":
         """Restrict the query to results after an id36 (generative)."""
         if self.before_id:
@@ -127,12 +145,21 @@ class PaginatedQuery(ModelQuery):
 
     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:
+        if len(self._anchor_table.primary_key) > 1:
             raise TypeError("Only single-col primary key tables are supported")
 
-        id_column = list(self.model_cls.__table__.primary_key)[0]
+        id_column = list(self._anchor_table.primary_key)[0]
+
+        if self.is_anchor_same_type:
+            columns = self.sorting_columns
+        else:
+            columns = [
+                self._anchor_table.columns.get(column.name)
+                for column in self.sorting_columns
+            ]
+
         return (
-            self.request.db_session.query(*self.sorting_columns)
+            self.request.db_session.query(*columns)
             .filter(id_column == anchor_id)
             .subquery()
         )
@@ -170,6 +197,7 @@ class PaginatedResults:
 
     def __init__(self, query: PaginatedQuery, per_page: int):
         """Fetch results from a PaginatedQuery."""
+        self.query = query
         self.per_page = per_page
 
         # if the query had `before` or `after` restrictions, there must be a page in
@@ -227,3 +255,67 @@ class PaginatedResults:
 
         prev_id = inspect(self.results[0]).identity[0]
         return id_to_id36(prev_id)
+
+
+class MixedPaginatedResults(PaginatedResults):
+    """Merged result from multiple PaginatedResults, consisting of different types."""
+
+    def __init__(self, paginated_results: Sequence[PaginatedResults]):
+        # pylint: disable=super-init-not-called,protected-access
+        """Merge all the supplied results into a single one."""
+        sort_column_name = paginated_results[0].query._sort_column.name
+        if any(
+            [r.query._sort_column.name != sort_column_name for r in paginated_results]
+        ):
+            raise ValueError("All results must by sorted by the same column.")
+
+        reverse_sort = paginated_results[0].query.sort_desc
+        if any([r.query.sort_desc != reverse_sort for r in paginated_results]):
+            raise ValueError("All results must by sorted in the same direction.")
+
+        # merge all the results into one list and sort it
+        self.results = sorted(
+            chain.from_iterable(paginated_results),
+            key=lambda post: getattr(post, sort_column_name),
+            reverse=reverse_sort,
+        )
+
+        self.per_page = min([r.per_page for r in paginated_results])
+
+        if len(self.results) > self.per_page:
+            self.has_next_page = True
+            self.results = self.results[: self.per_page]
+        else:
+            self.has_next_page = any([r.has_next_page for r in paginated_results])
+
+        self.has_prev_page = any([r.has_prev_page for r in paginated_results])
+
+    @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
+
+        item = self.results[-1]
+
+        next_id = inspect(item).identity[0]
+        next_id36 = id_to_id36(next_id)
+
+        type_char = item.__class__.__name__.lower()[0]
+
+        return f"{type_char}-{next_id36}"
+
+    @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
+
+        item = self.results[0]
+
+        prev_id = inspect(item).identity[0]
+        prev_id36 = id_to_id36(prev_id)
+
+        type_char = item.__class__.__name__.lower()[0]
+
+        return f"{type_char}-{prev_id36}"

tildes/tildes/schemas/listing.py

@@ -8,7 +8,7 @@ from marshmallow.fields import Boolean, Integer
 from marshmallow.validate import Range
 
 from tildes.enums import TopicSortOption
-from tildes.schemas.fields import Enum, ID36, Ltree, ShortTimePeriod
+from tildes.schemas.fields import Enum, ID36, Ltree, PostType, ShortTimePeriod
 
 
 class PaginatedListingSchema(Schema):
@@ -46,3 +46,42 @@ class TopicListingSchema(PaginatedListingSchema):
             data["rank_start"] = 1
 
         return data
+
+
+class MixedListingSchema(PaginatedListingSchema):
+    """Marshmallow schema to validate arguments for a "mixed" listing page.
+
+    By "mixed", this means that the listing can contain topics and/or comments, instead
+    of just one or the other.
+    """
+
+    anchor_type = PostType()
+
+    @pre_load
+    def set_anchor_type_from_before_or_after(self, data: dict) -> dict:
+        """Set the anchor_type if before or after has a special value indicating type.
+
+        For example, if after or before looks like "t-123" that means it is referring
+        to the topic with ID36 "123". "c-123" also works, for comments.
+        """
+        keys = ("after", "before")
+
+        for key in keys:
+            value = data.get(key)
+            if not value:
+                continue
+
+            type_char, _, id36 = value.partition("-")
+            if not id36:
+                continue
+
+            if type_char == "c":
+                data["anchor_type"] = "comment"
+            elif type_char == "t":
+                data["anchor_type"] = "topic"
+            else:
+                continue
+
+            data[key] = id36
+
+        return data

tildes/tildes/templates/user.jinja2

@@ -15,7 +15,7 @@
 
 {# Only show the heading if they can't see the type tabs and the user isn't deleted #}
 {% block main_heading %}
-  {% if not user.is_deleted and not request.has_permission('view_types', user) %}
+  {% if not user.is_deleted and not request.has_permission("view_history", user) %}
     {{ user.username }}'s recent activity
   {% endif %}
 {% endblock %}
@@ -27,11 +27,11 @@
     <div class="empty-action"><a href="/" class="btn btn-primary">Back to the home page</a></div>
   </div>
 {% else %}
-  {% if request.has_permission('view_types', user) %}
+  {% if request.has_permission("view_history", user) %}
     <div class="listing-options">
       <menu class="tab tab-listing-order">
         <li class="tab-item{{' active' if not post_type else ''}}">
-          <a href="{{ request.current_listing_normal_url() }}">Recent activity</a>
+          <a href="{{ request.current_listing_normal_url() }}">All activity</a>
         </li>
         <li class="tab-item{{' active' if post_type == 'topic' else ''}}">
           <a href="{{ request.current_listing_normal_url({'type': 'topic'}) }}">Topics</a>
@@ -57,7 +57,7 @@
     {% endfor %}
     </ol>
 
-    {% if post_type and (posts.has_prev_page or posts.has_next_page) %}
+    {% if request.has_permission("view_history", user) and (posts.has_prev_page or posts.has_next_page) %}
       <div class="pagination">
         {% if posts.has_prev_page %}
           <a class="page-item btn" id="prev-page"

tildes/tildes/views/user.py

@@ -3,7 +3,7 @@
 
 """Views related to a specific user."""
 
-from typing import List, Optional, Union
+from typing import List, Optional, Type, Union
 
 from pyramid.request import Request
 from pyramid.view import view_config
@@ -12,79 +12,51 @@ from webargs.pyramidparser import use_kwargs
 
 from tildes.enums import CommentLabelOption
 from tildes.models.comment import Comment
+from tildes.models.pagination import MixedPaginatedResults
 from tildes.models.topic import Topic
-from tildes.models.user import User, UserInviteCode
+from tildes.models.user import UserInviteCode
 from tildes.schemas.fields import PostType
-from tildes.schemas.listing import PaginatedListingSchema
-
-
-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 up a page,
-    # we'll fetch the full page size of both types, merge them, and then trim down to
-    # the size afterwards
-    query = (
-        request.query(Comment)
-        .filter(Comment.user == user)
-        .order_by(desc(Comment.created_time))
-        .limit(page_size)
-        .join_all_relationships()
-    )
-
-    # include removed comments if the user's looking at their own page or is an admin
-    if user == request.user or request.user.is_admin:
-        query = query.include_removed()
-
-    comments = query.all()
-
-    query = (
-        request.query(Topic)
-        .filter(Topic.user == user)
-        .order_by(desc(Topic.created_time))
-        .limit(page_size)
-        .join_all_relationships()
-    )
-
-    # include removed topics if the user's looking at their own page or is an admin
-    if user == request.user or request.user.is_admin:
-        query = query.include_removed()
-
-    topics = query.all()
-
-    merged_posts = sorted(
-        comments + topics,  # this order so topic comes first when times match
-        key=lambda post: post.created_time,
-        reverse=True,
-    )
-    merged_posts = merged_posts[:page_size]
-
-    return merged_posts
+from tildes.schemas.listing import MixedListingSchema
 
 
 @view_config(route_name="user", renderer="user.jinja2")
-@use_kwargs(PaginatedListingSchema())
+@use_kwargs(MixedListingSchema())
 @use_kwargs({"post_type": PostType(load_from="type")})
 def get_user(
     request: Request,
-    after: str,
-    before: str,
+    after: Optional[str],
+    before: Optional[str],
     per_page: int,
+    anchor_type: Optional[str],
     post_type: Optional[str] = None,
 ) -> dict:
+    # pylint: disable=too-many-arguments
     """Generate the main user history page."""
     user = request.context
 
-    if not request.has_permission("view_types", user):
+    if not request.has_permission("view_history", user):
         post_type = None
+        after = None
+        before = None
+        anchor_type = None
+        per_page = 20
+
+    types_to_query: List[Union[Type[Topic], Type[Comment]]]
+    if post_type == "topic":
+        types_to_query = [Topic]
+    elif post_type == "comment":
+        types_to_query = [Comment]
+    else:
+        # the order here is important so items are in the right order when the results
+        # are merged at the end (we want topics to come first when times match)
+        types_to_query = [Comment, Topic]
+
+    result_sets = []
+    for type_to_query in types_to_query:
+        query = request.query(type_to_query).filter(type_to_query.user == user)
 
-    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 anchor_type:
+            query = query.anchor_type(anchor_type)
 
         if before:
             query = query.before_id36(before)
@@ -98,9 +70,12 @@ def get_user(
         if user == request.user or request.user.is_admin:
             query = query.include_removed()
 
-        posts = query.get_page(per_page)
+        result_sets.append(query.get_page(per_page))
+
+    if len(result_sets) == 1:
+        posts = result_sets[0]
     else:
-        posts = _get_user_recent_activity(request, user)
+        posts = MixedPaginatedResults(result_sets)
 
     return {
         "user": user,