Experiments: Handle 'newer_than' correctly in R2Experiments

Only check 'newer_than' targeting after checking 'logged_in'.
Brblol · Aug 23, 2017 · aa1a9c5 · aa1a9c5
1 parent 6302798
commit aa1a9c5
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 6 deletions.
diff --git a/baseplate/experiments/providers/r2.py b/baseplate/experiments/providers/r2.py
@@ -207,12 +207,14 @@ def _is_enabled(self, **kwargs):
                     else:
                         final_value = value.lower()
                     if final_value in allowed_values:
-                        return True
-
-        user_created = kwargs.get("user_created")
-        if self.newer_than and user_created and user_created > self.newer_than:
-            return True
-
+                        if targeting_param == "logged_in" and self.newer_than:
+                            user_created = kwargs.get("user_created")
+                            if (user_created and
+                                    user_created > self.newer_than):
+                                return True
+
+                        else:
+                            return True
         return False
 
     def _calculate_bucket(self, bucket_val):

diff --git a/tests/unit/experiments/providers/feature_flag_tests.py b/tests/unit/experiments/providers/feature_flag_tests.py
@@ -785,6 +785,9 @@ def test_is_newer_than(self):
             "type": "feature_flag",
             "expires": (datetime.utcnow() + THIRTY_DAYS).strftime(ISO_DATE_FMT),
             "experiment": {
+                "targeting": {
+                    "logged_in": [True],
+                },
                 "newer_than": int(time.time()) - THIRTY_DAYS.total_seconds(),
                 "variants": {
                     "active": 100,
@@ -809,6 +812,9 @@ def test_is_not_newer_than(self):
             "type": "feature_flag",
             "expires": (datetime.utcnow() + THIRTY_DAYS).strftime(ISO_DATE_FMT),
             "experiment": {
+                "targeting": {
+                    "logged_in": [True],
+                },
                 "newer_than": int(time.time()) + THIRTY_DAYS.total_seconds(),
                 "variants": {
                     "active": 100,
@@ -825,3 +831,32 @@ def test_is_not_newer_than(self):
             user_id=self.user_id,
             logged_in=self.user_logged_in,
         ), "active")
+
+
+    def test_newer_than_only_on_logged_in_check(self):
+        cfg = {
+            "id": 1,
+            "name": "test_feature",
+            "type": "feature_flag",
+            "expires": (datetime.utcnow() + THIRTY_DAYS).strftime(ISO_DATE_FMT),
+            "experiment": {
+                "targeting": {
+                    "logged_in": [True],
+                    "user_name": ["gary"],
+                },
+                "newer_than": int(time.time()) + THIRTY_DAYS.total_seconds(),
+                "variants": {
+                    "active": 100,
+                },
+            },
+        }
+        feature_flag = parse_experiment(cfg)
+        self.assertEqual(feature_flag.variant(
+            user_id=self.user_id,
+            logged_in=self.user_logged_in,
+            user_name="gary",
+        ), "active")
+        self.assertNotEqual(feature_flag.variant(
+            user_id=self.user_id,
+            logged_in=self.user_logged_in,
+        ), "active")