From 52db30cdad2bdd856349764ddc06d7f0fabefe25 Mon Sep 17 00:00:00 2001
From: Deimos <deimos@tildes.net>
Date: Wed, 25 Jul 2018 19:02:08 -0600
Subject: [PATCH] Remove handling of None in some query methods

The previous setup was a bit unusual and confusing - various functions
that restricted the query somehow were accepting None as an argument and
simply doing nothing if it was passed. This made calling them a little
simpler, but overall didn't make a lot of sense - if you don't want to
apply the restriction, you shouldn't call the function in the first
place.
---
 tildes/tildes/models/pagination.py        | 10 ++--------
 tildes/tildes/models/topic/topic_query.py | 13 ++-----------
 tildes/tildes/views/topic.py              | 20 ++++++++++++++++----
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/tildes/tildes/models/pagination.py b/tildes/tildes/models/pagination.py
index f7afe47..c4c6d8c 100644
--- a/tildes/tildes/models/pagination.py
+++ b/tildes/tildes/models/pagination.py
@@ -74,11 +74,8 @@ class PaginatedQuery(ModelQuery):
         """
         return bool(self.before_id)
 
-    def after_id36(self, id36: Optional[str]) -> 'PaginatedQuery':
+    def after_id36(self, id36: str) -> 'PaginatedQuery':
         """Restrict the query to results after an id36 (generative)."""
-        if not id36:
-            return self
-
         if self.before_id:
             raise ValueError("Can't set both before and after restrictions")
 
@@ -86,11 +83,8 @@ class PaginatedQuery(ModelQuery):
 
         return self
 
-    def before_id36(self, id36: Optional[str]) -> 'PaginatedQuery':
+    def before_id36(self, id36: str) -> 'PaginatedQuery':
         """Restrict the query to results before an id36 (generative)."""
-        if not id36:
-            return self
-
         if self.after_id:
             raise ValueError("Can't set both before and after restrictions")
 
diff --git a/tildes/tildes/models/topic/topic_query.py b/tildes/tildes/models/topic/topic_query.py
index eb1c76b..9816d40 100644
--- a/tildes/tildes/models/topic/topic_query.py
+++ b/tildes/tildes/models/topic/topic_query.py
@@ -1,6 +1,6 @@
 """Contains the TopicQuery class."""
 
-from typing import Any, Optional, Sequence
+from typing import Any, Sequence
 
 from pyramid.request import Request
 from sqlalchemy.sql.expression import and_, null
@@ -119,21 +119,12 @@ class TopicQuery(PaginatedQuery):
         return self.filter(
             Topic.group_id.in_(subgroup_subquery))  # type: ignore
 
-    def inside_time_period(
-            self,
-            period: Optional[SimpleHoursPeriod],
-    ) -> 'TopicQuery':
+    def inside_time_period(self, period: SimpleHoursPeriod) -> 'TopicQuery':
         """Restrict the topics to inside a time period (generative)."""
-        if not period:
-            return self
-
         return self.filter(Topic.created_time > utc_now() - period.timedelta)
 
     def has_tag(self, tag: Ltree) -> 'TopicQuery':
         """Restrict the topics to ones with a specific tag (generative)."""
-        if not tag:
-            return self
-
         # casting tag to string really shouldn't be necessary, but some kind of
         # strange interaction seems to be happening with the ArrayOfLtree
         # class, this will need some investigation
diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py
index 198534c..f376c02 100644
--- a/tildes/tildes/views/topic.py
+++ b/tildes/tildes/views/topic.py
@@ -122,17 +122,29 @@ def get_group_topics(
     if period is missing:
         period = default_settings.period
 
+    # set up the basic query for topics
     query = (
         request.query(Topic)
         .join_all_relationships()
         .inside_groups(groups)
-        .inside_time_period(period)
-        .has_tag(tag)
         .apply_sort_option(order)
-        .after_id36(after)
-        .before_id36(before)
     )
 
+    # restrict the time period, if not set to "all time"
+    if period:
+        query = query.inside_time_period(period)
+
+    # restrict to a specific tag, if we're viewing a single one
+    if tag:
+        query = query.has_tag(tag)
+
+    # apply before/after pagination restrictions if relevant
+    if before:
+        query = query.before_id36(before)
+
+    if after:
+        query = query.after_id36(after)
+
     # apply topic tag filters unless they're disabled or viewing a single tag
     if not (tag or unfiltered):
         # pylint: disable=protected-access