From cdd4371d924241d2b1d3da57cfea8aac6e473c2b Mon Sep 17 00:00:00 2001
From: Deimos <deimos@tildes.net>
Date: Tue, 30 Oct 2018 22:05:00 -0600
Subject: [PATCH] Simplify bookmark page querying, drop pagination

Pagination isn't working correctly for bookmarks yet, and it seems like
it'll take some fiddly work to be able to make PaginatedQuery be able to
handle using the bookmark's created_time instead of the post's.

This just simplifies the method by using an inner join on the bookmarks
instead of an EXISTS subquery, and totally removes pagination for now.
---
 tildes/tildes/models/comment/comment_query.py | 10 +++++++++-
 tildes/tildes/models/topic/topic_query.py     | 10 +++++++++-
 tildes/tildes/views/bookmarks.py              | 19 ++++---------------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/tildes/tildes/models/comment/comment_query.py b/tildes/tildes/models/comment/comment_query.py
index 912053c..a894bcf 100644
--- a/tildes/tildes/models/comment/comment_query.py
+++ b/tildes/tildes/models/comment/comment_query.py
@@ -25,6 +25,8 @@ class CommentQuery(PaginatedQuery):
         """
         super().__init__(Comment, request)
 
+        self._only_bookmarked = False
+
     def _attach_extra_data(self) -> "CommentQuery":
         """Attach the extra user data to the query."""
         # pylint: disable=protected-access
@@ -48,12 +50,13 @@ class CommentQuery(PaginatedQuery):
 
     def _attach_bookmark_data(self) -> "CommentQuery":
         """Join the data related to whether the user has bookmarked the comment."""
-        query = self.outerjoin(
+        query = self.join(
             CommentBookmark,
             and_(
                 CommentBookmark.comment_id == Comment.comment_id,
                 CommentBookmark.user == self.request.user,
             ),
+            isouter=(not self._only_bookmarked),
         )
         query = query.add_columns(CommentBookmark.created_time)
 
@@ -74,3 +77,8 @@ class CommentQuery(PaginatedQuery):
             comment.bookmark_created_time = result.created_time
 
         return comment
+
+    def only_bookmarked(self) -> "CommentQuery":
+        """Restrict the comments to ones that the user has bookmarked (generative)."""
+        self._only_bookmarked = True
+        return self
diff --git a/tildes/tildes/models/topic/topic_query.py b/tildes/tildes/models/topic/topic_query.py
index 62c1bc4..1c842a2 100644
--- a/tildes/tildes/models/topic/topic_query.py
+++ b/tildes/tildes/models/topic/topic_query.py
@@ -33,6 +33,8 @@ class TopicQuery(PaginatedQuery):
         """
         super().__init__(Topic, request)
 
+        self._only_bookmarked = False
+
     def _attach_extra_data(self) -> "TopicQuery":
         """Attach the extra user data to the query."""
         if not self.request.user:
@@ -56,12 +58,13 @@ class TopicQuery(PaginatedQuery):
 
     def _attach_bookmark_data(self) -> "TopicQuery":
         """Join the data related to whether the user has bookmarked the topic."""
-        query = self.outerjoin(
+        query = self.join(
             TopicBookmark,
             and_(
                 TopicBookmark.topic_id == Topic.topic_id,
                 TopicBookmark.user == self.request.user,
             ),
+            isouter=(not self._only_bookmarked),
         )
         query = query.add_columns(TopicBookmark.created_time)
 
@@ -164,3 +167,8 @@ class TopicQuery(PaginatedQuery):
     def search(self, query: str) -> "TopicQuery":
         """Restrict the topics to ones that match a search query (generative)."""
         return self.filter(Topic.search_tsv.op("@@")(func.plainto_tsquery(query)))
+
+    def only_bookmarked(self) -> "TopicQuery":
+        """Restrict the topics to ones that the user has bookmarked (generative)."""
+        self._only_bookmarked = True
+        return self
diff --git a/tildes/tildes/views/bookmarks.py b/tildes/tildes/views/bookmarks.py
index 68482c8..9d5e958 100644
--- a/tildes/tildes/views/bookmarks.py
+++ b/tildes/tildes/views/bookmarks.py
@@ -6,8 +6,7 @@ from marshmallow.fields import String
 from marshmallow.validate import OneOf
 from pyramid.request import Request
 from pyramid.view import view_config
-from sqlalchemy.sql import exists, desc
-from sqlalchemy.sql.expression import and_
+from sqlalchemy.sql import desc
 from webargs.pyramidparser import use_kwargs
 
 from tildes.models.comment import Comment, CommentBookmark
@@ -28,6 +27,7 @@ def get_bookmarks(
     post_type: Optional[str] = None,
 ) -> dict:
     """Generate the bookmarks page."""
+    # pylint: disable=unused-argument
     user = request.user
 
     bookmark_cls: Union[Type[CommentBookmark], Type[TopicBookmark]]
@@ -41,18 +41,7 @@ def get_bookmarks(
 
     query = (
         request.query(post_cls)
-        .filter(
-            exists()
-            .where(
-                and_(
-                    bookmark_cls.user == user,
-                    bookmark_cls.topic_id == post_cls.topic_id
-                    if post_cls == Topic
-                    else bookmark_cls.comment_id == post_cls.comment_id,
-                )
-            )
-            .correlate(bookmark_cls)
-        )
+        .only_bookmarked()
         .order_by(desc(bookmark_cls.created_time))
     )
 
@@ -64,6 +53,6 @@ def get_bookmarks(
 
     query = query.join_all_relationships()
 
-    posts = query.get_page(per_page)
+    posts = query.all()
 
     return {"user": user, "posts": posts, "post_type": post_type}