From 2ec015e5606e7799193623511c4ee396cec7b0aa Mon Sep 17 00:00:00 2001
From: Deimos <deimos@tildes.net>
Date: Mon, 9 Sep 2019 21:32:35 -0600
Subject: [PATCH] Update theme cookie tween to only set when needed

Previously, the "theme cookie tween" was setting the theme cookie on
every single GET request, in order to make sure that the domain was set
on it so that it would be accessible from the Blog and Docs sites. We've
been doing this for a long time now though, and almost every cookie
should have been set properly now.

Now, this tween will only set the cookie when it's actually necessary -
when the user is logged-in and has a default theme, but doesn't already
have a theme cookie.
---
 tildes/tildes/tweens.py | 45 ++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/tildes/tildes/tweens.py b/tildes/tildes/tweens.py
index c49e8e1..5d25785 100644
--- a/tildes/tildes/tweens.py
+++ b/tildes/tildes/tweens.py
@@ -71,33 +71,36 @@ def theme_cookie_tween_factory(handler: Callable, registry: Registry) -> Callabl
     """Return a tween function that sets the theme cookie."""
 
     def theme_cookie_tween(request: Request) -> Response:
-        """Set the theme cookie if needed (currently always, see comment below)."""
+        """Set the theme cookie if needed.
+
+        Will only set a cookie if the user has a default theme set for their account
+        but doesn't already have a theme cookie. This is necessary so that their default
+        theme will apply to the Blog and Docs sites as well, since those sites are
+        static and can't look up the user's default theme in the database.
+        """
         response = handler(request)
 
         # only set the cookie on GET requests
         if request.method.upper() != "GET":
             return response
 
-        current_theme = request.cookies.get("theme", "")
-        if not current_theme and request.user:
-            current_theme = request.user.theme_default
-
-        # Currently, we want to always set the theme cookie. This is because we
-        # recently started setting the domain on this cookie (to be able to apply the
-        # user's theme to the Blog/Docs sites), and all older cookies won't have a
-        # domain set. This will basically let us convert the old no-domain cookies to
-        # new ones. After a decent amount of time (maybe sometime in April 2019), we
-        # can change this to only set the cookie when it's not already present and the
-        # user has a default theme set (so their default theme will work for Blog/Docs).
-        if current_theme:
-            response.set_cookie(
-                "theme",
-                current_theme,
-                max_age=315360000,
-                secure=True,
-                domain="." + request.domain,
-            )
-            incr_counter("theme_cookie_tween_sets")
+        # if they already have a theme cookie, we don't need to do anything
+        if request.cookies.get("theme", ""):
+            return response
+
+        # if the user doesn't have a default theme, we don't need to do anything
+        if not request.user or not request.user.theme_default:
+            return response
+
+        # set a cookie with the user's default theme
+        response.set_cookie(
+            "theme",
+            request.user.theme_default,
+            max_age=315360000,
+            secure=True,
+            domain="." + request.domain,
+        )
+        incr_counter("theme_cookie_tween_sets")
 
         return response