Feature flags: remove is_enabled_for()

The function essentially did the same thing as `is_enabled()`, but with
overriding the user and not also checking the subreddit, subdomain, or oauth
client.  The former is easily handled with default args, and the latter has
actually potentially been an issue - if you needed to override the user, you
lost the effect of any of those on the feature flag.

r2/r2/config/feature/README.md

@@ -115,13 +115,13 @@ Copied essentially wholesale from Etsy's guidelines:
 To make it easier to push features through the life cycle there are a
 few coding guidelines to observe.
 
-First, the feature name argument to the Feature methods (`is_enabled`,
-`is_enabled_for`) should always be a string literal. This will make it easier
-to find all the places that a particular feature is checked. If you find
-yourself creating feature names at run time and then checking them, you’re
-probably abusing the Feature system. Chances are in such a case you don’t
-really want to be using the Feature API but rather simply driving your code
-with some plain old config data.
+First, the feature name argument to the Feature method (`is_enabled`) should
+always be a string literal. This will make it easier to find all the places
+that a particular feature is checked. If you find yourself creating feature
+names at run time and then checking them, you’re probably abusing the Feature
+system. Chances are in such a case you don’t really want to be using the
+Feature API but rather simply driving your code with some plain old config
+data.
 
 Second, the results of the Feature methods should not be cached, such
 as by calling `feature.is_enabled` once and storing the result in an

r2/r2/config/feature/__init__.py

@@ -20,4 +20,4 @@
 # Inc. All Rights Reserved.
 ###############################################################################
 
-from r2.config.feature.feature import is_enabled, is_enabled_for
+from r2.config.feature.feature import is_enabled

r2/r2/config/feature/feature.py

@@ -30,36 +30,32 @@ _world = World()
 _featurestate_cache = {}
 
 
-def is_enabled(name):
+def is_enabled(name, user=None):
     """Test and return whether a given feature is enabled for this request.
 
     If `feature` is not found, returns False.
 
+    The optional arguments allow overriding that you generally don't want, but
+    is useful outside of request contexts - cron jobs and the like.
+
     :param name string - a given feature name
+    :param user - (optional) an Account
     :return bool
     """
+    if not user:
+        user = _world.current_user()
+    subreddit = _world.current_subreddit()
+    subdomain = _world.current_subdomain()
+    oauth_client = _world.current_oauth_client()
+
     return _get_featurestate(name).is_enabled(
-               user=_world.current_user(),
-               subreddit=_world.current_subreddit(),
-               subdomain=_world.current_subdomain(),
-               oauth_client=_world.current_oauth_client(),
+        user=user,
+        subreddit=subreddit,
+        subdomain=subdomain,
+        oauth_client=oauth_client,
     )
 
 
-def is_enabled_for(name, user):
-    """Test and return whether a given feature is enabled for a user.
-
-    This should only be used in contexts where we want to test outside
-    of a current user context - cron jobs and the like. This is also
-    going to be slower, as featurestates are not cached.
-
-    :param name string - a given feature name
-    :param user - an Account
-    :return bool
-    """
-    return _get_featurestate(name).is_enabled(user)
-
-
 @feature_hooks.on('worker.live_config.update')
 def clear_featurestate_cache():
     global _featurestate_cache

r2/r2/lib/emailer.py

@@ -145,7 +145,7 @@ def message_notification_email(data):
         # In case a user has enabled the preference while it was enabled for
         # them, but we've since turned it off.  We need to explicitly state the
         # user because we're not in the context of an HTTP request from them.
-        if not feature.is_enabled_for('orangereds_as_emails', user):
+        if not feature.is_enabled('orangereds_as_emails', user=user):
             continue
 
         if g.cache.get(MESSAGE_THROTTLE_KEY) > MAX_EMAILS_PER_DAY:

r2/r2/lib/pages/pages.py

@@ -1116,7 +1116,7 @@ class PrefsPage(Reddit):
         ])
 
         # Hide the security tab behind a feature flag while it's being tested
-        if feature.is_enabled_for('allow_force_https', c.user):
+        if feature.is_enabled('allow_force_https'):
             buttons += [NamedButton('security')]
         #if CustomerID.get_id(user):
         #    buttons += [NamedButton('payment')]

r2/r2/lib/validator/preferences.py

@@ -105,7 +105,7 @@ def set_prefs(user, prefs):
 
 def filter_prefs(prefs, user):
     # replace stylesheet_override with other_theme if it doesn't exist
-    if feature.is_enabled_for('stylesheets_everywhere', user):
+    if feature.is_enabled('stylesheets_everywhere', user=user):
         if not prefs["pref_default_theme_sr"]:
             if prefs.get("pref_other_theme", False):
                 prefs["pref_default_theme_sr"] = prefs["pref_other_theme"]
@@ -137,7 +137,7 @@ def filter_prefs(prefs, user):
         prefs['pref_highlight_new_comments'] = True
 
     # check stylesheet override
-    if feature.is_enabled_for('stylesheets_everywhere', user):
+    if feature.is_enabled('stylesheets_everywhere', user=user):
         override_sr = prefs['pref_default_theme_sr']
         if not override_sr:
             del prefs['pref_default_theme_sr']

r2/r2/models/account.py

@@ -745,7 +745,7 @@ class Account(Thing):
     @property
     def https_forced(self):
         """Return whether this account may only be used via HTTPS."""
-        if feature.is_enabled_for("require_https", self):
+        if feature.is_enabled("require_https", user=self):
             return True
         return self.pref_force_https