fix(feature flags): Guard for None values when comparing person Properties (#129)

* Guard for None values when comparing person Properties

Author: Phani Raj

posthog/feature_flags.py

@@ -13,6 +13,8 @@
 
 log = logging.getLogger("posthog")
 
+NONE_VALUES_ALLOWED_OPERATORS = ["is_not"]
+
 
 class InconclusiveMatchError(Exception):
     pass
@@ -119,6 +121,9 @@ def match_property(property, property_values) -> bool:
 
     override_value = property_values[key]
 
+    if (operator not in NONE_VALUES_ALLOWED_OPERATORS) and override_value is None:
+        return False
+
     if operator in ("exact", "is_not"):
 
         def compute_exact_match(value, override_value):

posthog/test/test_feature_flags.py

@@ -960,6 +960,60 @@ def test_compute_inactive_flags_locally(self, patch_decide, patch_capture):
         self.assertEqual(patch_decide.call_count, 0)
         self.assertEqual(patch_capture.call_count, 0)
 
+    @mock.patch("posthog.client.decide")
+    @mock.patch("posthog.client.get")
+    def test_feature_flags_local_evaluation_None_values(self, patch_get, patch_decide):
+        client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+        client.feature_flags = [
+            {
+                id: 1,
+                "name": "Beta Feature",
+                "key": "beta-feature",
+                "is_simple_flag": True,
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "variant": None,
+                            "properties": [
+                                {"key": "latestBuildVersion", "type": "person", "value": ".+", "operator": "regex"},
+                                {"key": "latestBuildVersionMajor", "type": "person", "value": "23", "operator": "gt"},
+                                {"key": "latestBuildVersionMinor", "type": "person", "value": "31", "operator": "gt"},
+                                {"key": "latestBuildVersionPatch", "type": "person", "value": "0", "operator": "gt"},
+                            ],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            },
+        ]
+
+        feature_flag_match = client.get_feature_flag(
+            "beta-feature",
+            "some-distinct-id",
+            person_properties={
+                "latestBuildVersion": None,
+                "latestBuildVersionMajor": None,
+                "latestBuildVersionMinor": None,
+                "latestBuildVersionPatch": None,
+            },
+        )
+
+        self.assertEqual(feature_flag_match, False)
+        self.assertEqual(patch_decide.call_count, 0)
+        self.assertEqual(patch_get.call_count, 0)
+
+        feature_flag_match = client.get_feature_flag(
+            "beta-feature",
+            "some-distinct-id",
+            person_properties={
+                "latestBuildVersion": "24.32..1",
+                "latestBuildVersionMajor": "24",
+                "latestBuildVersionMinor": "32",
+                "latestBuildVersionPatch": "1",
+            },
+        )
+
     @mock.patch("posthog.client.decide")
     @mock.patch("posthog.client.get")
     def test_feature_flags_local_evaluation_for_cohorts(self, patch_get, patch_decide):
@@ -1714,7 +1768,7 @@ def test_match_properties_is_set(self):
         self.assertTrue(match_property(property_a, {"key": "value"}))
         self.assertTrue(match_property(property_a, {"key": "value2"}))
         self.assertTrue(match_property(property_a, {"key": ""}))
-        self.assertTrue(match_property(property_a, {"key": None}))
+        self.assertFalse(match_property(property_a, {"key": None}))
 
         with self.assertRaises(InconclusiveMatchError):
             match_property(property_a, {"key2": "value"})
@@ -1980,20 +2034,20 @@ def test_none_property_value_with_all_operators(self):
         self.assertTrue(match_property(property_a, {"key": "non"}))
 
         property_b = self.property(key="key", value=None, operator="is_set")
-        self.assertTrue(match_property(property_b, {"key": None}))
+        self.assertFalse(match_property(property_b, {"key": None}))
 
         property_c = self.property(key="key", value="no", operator="icontains")
-        self.assertTrue(match_property(property_c, {"key": None}))
+        self.assertFalse(match_property(property_c, {"key": None}))
         self.assertFalse(match_property(property_c, {"key": "smh"}))
 
         property_d = self.property(key="key", value="No", operator="regex")
-        self.assertTrue(match_property(property_d, {"key": None}))
+        self.assertFalse(match_property(property_d, {"key": None}))
 
         property_d_lower_case = self.property(key="key", value="no", operator="regex")
         self.assertFalse(match_property(property_d_lower_case, {"key": None}))
 
         property_e = self.property(key="key", value=1, operator="gt")
-        self.assertTrue(match_property(property_e, {"key": None}))
+        self.assertFalse(match_property(property_e, {"key": None}))
 
         property_f = self.property(key="key", value=1, operator="lt")
         self.assertFalse(match_property(property_f, {"key": None}))
@@ -2002,15 +2056,13 @@ def test_none_property_value_with_all_operators(self):
         self.assertFalse(match_property(property_g, {"key": None}))
 
         property_h = self.property(key="key", value="Oo", operator="lte")
-        self.assertTrue(match_property(property_h, {"key": None}))
+        self.assertFalse(match_property(property_h, {"key": None}))
 
         property_i = self.property(key="key", value="2022-05-01", operator="is_date_before")
-        with self.assertRaises(InconclusiveMatchError):
-            self.assertFalse(match_property(property_i, {"key": None}))
+        self.assertFalse(match_property(property_i, {"key": None}))
 
         property_j = self.property(key="key", value="2022-05-01", operator="is_date_after")
-        with self.assertRaises(InconclusiveMatchError):
-            self.assertFalse(match_property(property_j, {"key": None}))
+        self.assertFalse(match_property(property_j, {"key": None}))
 
         property_k = self.property(key="key", value="2022-05-01", operator="is_date_before")
         with self.assertRaises(InconclusiveMatchError):