fix(flags): Don't override existing props when adding flags (#110)

Author: neilkakkar

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 3.3.1 - 2024-01-10
+
+1. Make sure we don't override any existing feature flag properties when adding locally evaluated feature flag properties.
+
 ## 3.3.0 - 2024-01-09
 
 1. When local evaluation is enabled, we automatically add flag information to all events sent to PostHog, whenever possible. This makes it easier to use these events in experiments.

posthog/client.py

@@ -137,16 +137,6 @@ def get_feature_variants(
         resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
         return resp_data["featureFlags"]
 
-    def _get_active_feature_variants(
-        self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
-    ):
-        feature_variants = self.get_feature_variants(
-            distinct_id, groups, person_properties, group_properties, disable_geoip
-        )
-        return {
-            k: v for (k, v) in feature_variants.items() if v is not False
-        }  # explicitly test for false to account for values that may seem falsy (ex: 0)
-
     def get_feature_payloads(
         self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
     ):
@@ -206,26 +196,29 @@ def capture(
             require("groups", groups, dict)
             msg["properties"]["$groups"] = groups
 
+        extra_properties = {}
+        feature_variants = {}
         if send_feature_flags:
             try:
-                feature_variants = self._get_active_feature_variants(distinct_id, groups, disable_geoip=disable_geoip)
+                feature_variants = self.get_feature_variants(distinct_id, groups, disable_geoip=disable_geoip)
             except Exception as e:
                 self.log.exception(f"[FEATURE FLAGS] Unable to get feature variants: {e}")
-            else:
-                for feature, variant in feature_variants.items():
-                    msg["properties"][f"$feature/{feature}"] = variant
-                msg["properties"]["$active_feature_flags"] = list(feature_variants.keys())
+
         elif self.feature_flags:
             # Local evaluation is enabled, flags are loaded, so try and get all flags we can without going to the server
             feature_variants = self.get_all_flags(
                 distinct_id, groups=(groups or {}), disable_geoip=disable_geoip, only_evaluate_locally=True
             )
-            for feature, variant in feature_variants.items():
-                msg["properties"][f"$feature/{feature}"] = variant
 
-            active_feature_flags = [key for (key, value) in feature_variants.items() if value is not False]
-            if active_feature_flags:
-                msg["properties"]["$active_feature_flags"] = active_feature_flags
+        for feature, variant in feature_variants.items():
+            extra_properties[f"$feature/{feature}"] = variant
+
+        active_feature_flags = [key for (key, value) in feature_variants.items() if value is not False]
+        if active_feature_flags:
+            extra_properties["$active_feature_flags"] = active_feature_flags
+
+        if extra_properties:
+            msg["properties"] = {**extra_properties, **msg["properties"]}
 
         return self._enqueue(msg, disable_geoip)
 

posthog/test/test_client.py

@@ -210,24 +210,83 @@ def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_decide):
         assert "$active_feature_flags" not in msg["properties"]
 
     @mock.patch("posthog.client.decide")
-    def test_get_active_feature_flags(self, patch_decide):
-        patch_decide.return_value = {
-            "featureFlags": {"beta-feature": "random-variant", "alpha-feature": True, "off-feature": False}
+    def test_dont_override_capture_with_local_flags(self, patch_decide):
+        patch_decide.return_value = {"featureFlags": {"beta-feature": "random-variant"}}
+        client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY)
+
+        multivariate_flag = {
+            "id": 1,
+            "name": "Beta Feature",
+            "key": "beta-feature-local",
+            "is_simple_flag": False,
+            "active": True,
+            "rollout_percentage": 100,
+            "filters": {
+                "groups": [
+                    {
+                        "properties": [
+                            {"key": "email", "type": "person", "value": "[email protected]", "operator": "exact"}
+                        ],
+                        "rollout_percentage": 100,
+                    },
+                    {
+                        "rollout_percentage": 50,
+                    },
+                ],
+                "multivariate": {
+                    "variants": [
+                        {"key": "first-variant", "name": "First Variant", "rollout_percentage": 50},
+                        {"key": "second-variant", "name": "Second Variant", "rollout_percentage": 25},
+                        {"key": "third-variant", "name": "Third Variant", "rollout_percentage": 25},
+                    ]
+                },
+                "payloads": {"first-variant": "some-payload", "third-variant": {"a": "json"}},
+            },
         }
+        basic_flag = {
+            "id": 1,
+            "name": "Beta Feature",
+            "key": "person-flag",
+            "is_simple_flag": True,
+            "active": True,
+            "filters": {
+                "groups": [
+                    {
+                        "properties": [
+                            {
+                                "key": "region",
+                                "operator": "exact",
+                                "value": ["USA"],
+                                "type": "person",
+                            }
+                        ],
+                        "rollout_percentage": 100,
+                    }
+                ],
+                "payloads": {"true": 300},
+            },
+        }
+        client.feature_flags = [multivariate_flag, basic_flag]
 
-        client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY)
-        variants = client._get_active_feature_variants("some_id", None, None, None, False)
-        self.assertEqual(variants, {"beta-feature": "random-variant", "alpha-feature": True})
-        patch_decide.assert_called_with(
-            "random_key",
-            None,
-            timeout=10,
-            distinct_id="some_id",
-            groups={},
-            person_properties=None,
-            group_properties=None,
-            disable_geoip=False,
+        success, msg = client.capture(
+            "distinct_id", "python test event", {"$feature/beta-feature-local": "my-custom-variant"}
         )
+        client.flush()
+        self.assertTrue(success)
+        self.assertFalse(self.failed)
+
+        self.assertEqual(msg["event"], "python test event")
+        self.assertTrue(isinstance(msg["timestamp"], str))
+        self.assertIsNone(msg.get("uuid"))
+        self.assertEqual(msg["distinct_id"], "distinct_id")
+        self.assertEqual(msg["properties"]["$lib"], "posthog-python")
+        self.assertEqual(msg["properties"]["$lib_version"], VERSION)
+        self.assertEqual(msg["properties"]["$feature/beta-feature-local"], "my-custom-variant")
+        self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature-local"])
+        assert "$feature/beta-feature" not in msg["properties"]
+        assert "$feature/person-flag" not in msg["properties"]
+
+        self.assertEqual(patch_decide.call_count, 0)
 
     @mock.patch("posthog.client.decide")
     def test_basic_capture_with_feature_flags_returns_active_only(self, patch_decide):

posthog/version.py

@@ -1,4 +1,4 @@
-VERSION = "3.3.0"
+VERSION = "3.3.1"
 
 if __name__ == "__main__":
     print(VERSION, end="")  # noqa: T201