Ensure we normalize get_decide

In a back compat manner.

Author: haacked

posthog/client.py

@@ -18,7 +18,7 @@
 from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple, handle_in_app
 from posthog.feature_flags import InconclusiveMatchError, match_feature_flag_properties
 from posthog.poller import Poller
-from posthog.request import DEFAULT_HOST, APIError, batch_post, decide, determine_server_host, get, remote_config
+from posthog.request import DEFAULT_HOST, APIError, batch_post, decide, determine_server_host, get, remote_config, normalize_decide_response, DecideResponse
 from posthog.utils import SizeLimitedDict, clean, guess_timezone, remove_trailing_slash
 from posthog.version import VERSION
 
@@ -230,21 +230,21 @@ def get_feature_variants(
         self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
     ):
         resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
-        return resp_data["featureFlags"]
+        return self.to_variants(resp_data)
 
     def get_feature_payloads(
         self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
     ):
         resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
-        return resp_data["featureFlagPayloads"]
+        return self.to_payloads(resp_data)
 
     def get_feature_flags_and_payloads(
         self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
     ):
         resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
         return {
-            "featureFlags": resp_data["featureFlags"],
-            "featureFlagPayloads": resp_data["featureFlagPayloads"],
+            "featureFlags": self.to_variants(resp_data),
+            "featureFlagPayloads": self.to_payloads(resp_data),
         }
 
     def get_decide(self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None):
@@ -267,7 +267,7 @@ def get_decide(self, distinct_id, groups=None, person_properties=None, group_pro
         }
         resp_data = decide(self.api_key, self.host, timeout=self.feature_flags_request_timeout_seconds, **request_data)
 
-        return resp_data
+        return normalize_decide_response(resp_data)
 
     def capture(
         self,
@@ -967,16 +967,17 @@ def get_all_flags(
         group_properties={},
         only_evaluate_locally=False,
         disable_geoip=None,
-    ):
-        flags = self.get_all_flags_and_payloads(
+    ) -> dict[str, bool | str]:
+        response = self.get_all_flags_and_payloads(
             distinct_id,
             groups=groups,
             person_properties=person_properties,
             group_properties=group_properties,
             only_evaluate_locally=only_evaluate_locally,
             disable_geoip=disable_geoip,
         )
-        return flags["featureFlags"]
+
+        return self.to_variants(response)
 
     def get_all_flags_and_payloads(
         self,
@@ -987,7 +988,7 @@ def get_all_flags_and_payloads(
         group_properties={},
         only_evaluate_locally=False,
         disable_geoip=None,
-    ):
+    ) -> DecideResponse:
         if self.disabled:
             return {"featureFlags": None, "featureFlagPayloads": None}
 
@@ -998,7 +999,8 @@ def get_all_flags_and_payloads(
         flags, payloads, fallback_to_decide = self._get_all_flags_and_payloads_locally(
             distinct_id, groups=groups, person_properties=person_properties, group_properties=group_properties
         )
-        response = {"featureFlags": flags, "featureFlagPayloads": payloads}
+        
+        response = normalize_decide_response({"featureFlags": flags, "featureFlagPayloads": payloads})
 
         if fallback_to_decide and not only_evaluate_locally:
             try:
@@ -1069,6 +1071,18 @@ def _add_local_person_and_group_properties(self, distinct_id, groups, person_pro
 
         return all_person_properties, all_group_properties
 
+    def to_variants(self, response: any) -> dict[str, bool | str]:
+        if "flags" not in response:
+            return None
+
+        return {key: value.get_value() for key, value in response.get("flags", {}).items()}
+
+    def to_payloads(self, response: any) -> dict[str, str]:
+        if "flags" not in response:
+            return None
+
+        return {key: value.metadata.payload for key, value in response.get("flags", {}).items() if value.enabled}
+
 
 def require(name, field, data_type):
     """Require that the named `field` has the right `data_type`"""

posthog/test/test_client.py

@@ -11,7 +11,7 @@
 from posthog.request import APIError
 from posthog.test.test_utils import FAKE_TEST_API_KEY
 from posthog.version import VERSION
-
+from posthog.types import FeatureFlag, LegacyFlagMetadata
 
 class TestClient(unittest.TestCase):
     @classmethod
@@ -1212,3 +1212,53 @@ def test_mock_system_context(
                 }
 
                 assert context == expected_context
+
+    @mock.patch("posthog.client.decide")    
+    def test_get_decide_returns_normalized_decide_response(self, patch_decide):
+        patch_decide.return_value = {
+            "featureFlags": {"beta-feature": "random-variant", "alpha-feature": True, "off-feature": False},
+            "featureFlagPayloads": {"beta-feature": "{\"some\": \"data\"}"},
+            "errorsWhileComputingFlags": False,
+            "requestId": "test-id",
+        }
+
+        client = Client(FAKE_TEST_API_KEY)
+        distinct_id = "test_distinct_id"
+        groups = {"test_group_type": "test_group_id"}
+        person_properties = {"test_property": "test_value"}
+        
+        response = client.get_decide(distinct_id, groups, person_properties)
+        
+        assert response == {
+            "flags": {
+                "beta-feature": FeatureFlag(
+                    key="beta-feature",
+                    enabled=True,
+                    variant="random-variant",
+                    reason=None,
+                    metadata=LegacyFlagMetadata(
+                        payload="{\"some\": \"data\"}",
+                    )
+                ),
+                "alpha-feature": FeatureFlag(
+                    key="alpha-feature",
+                    enabled=True,
+                    variant=None,
+                    reason=None,
+                    metadata=LegacyFlagMetadata(
+                        payload=None,
+                    )
+                ),
+                "off-feature": FeatureFlag(
+                    key="off-feature",
+                    enabled=False,
+                    variant=None,
+                    reason=None,
+                    metadata=LegacyFlagMetadata(
+                        payload=None,
+                    )
+                )
+            },
+            "errorsWhileComputingFlags": False,
+            "requestId": "test-id",
+        }

posthog/test/test_feature_flags.py

@@ -642,8 +642,9 @@ def test_get_all_flags_and_payloads_with_fallback(self, patch_decide, patch_capt
             },
         ]
         # beta-feature value overridden by /decide
+        flags_and_payloads = client.get_all_flags_and_payloads("distinct_id")
         self.assertEqual(
-            client.get_all_flags_and_payloads("distinct_id")["featureFlagPayloads"],
+            client.to_payloads(flags_and_payloads),
             {
                 "beta-feature": 100,
                 "beta-feature2": 300,
@@ -675,8 +676,9 @@ def test_get_all_flags_and_payloads_with_fallback_empty_local_flags(self, patch_
         client = self.client
         client.feature_flags = []
         # beta-feature value overridden by /decide
+        flags_and_payloads = client.get_all_flags_and_payloads("distinct_id")
         self.assertEqual(
-            client.get_all_flags_and_payloads("distinct_id")["featureFlagPayloads"],
+            client.to_payloads(flags_and_payloads),
             {"beta-feature": 100, "beta-feature2": 300},
         )
         self.assertEqual(patch_decide.call_count, 1)
@@ -726,7 +728,6 @@ def test_get_all_flags_with_no_fallback(self, patch_decide, patch_capture):
     @mock.patch.object(Client, "capture")
     @mock.patch("posthog.client.decide")
     def test_get_all_flags_and_payloads_with_no_fallback(self, patch_decide, patch_capture):
-        patch_decide.return_value = {"featureFlags": {"beta-feature": "variant-1", "beta-feature2": "variant-2"}}
         client = self.client
         basic_flag = {
             "id": 1,
@@ -768,8 +769,9 @@ def test_get_all_flags_and_payloads_with_no_fallback(self, patch_decide, patch_c
             disabled_flag,
         ]
         client.feature_flags_by_key = {"beta-feature": basic_flag, "disabled-feature": disabled_flag}
+        all_flags_and_payloads = client.get_all_flags_and_payloads("distinct_id")
         self.assertEqual(
-            client.get_all_flags_and_payloads("distinct_id")["featureFlagPayloads"], {"beta-feature": "new"}
+            client.to_payloads(all_flags_and_payloads), {"beta-feature": "new"}
         )
         # decide not called because this can be evaluated locally
         self.assertEqual(patch_decide.call_count, 0)
@@ -900,8 +902,9 @@ def test_get_all_flags_and_payloads_with_fallback_but_only_local_evaluation_set(
         ]
         client.feature_flags_by_key = {"beta-feature": flag_1, "disabled-feature": flag_2, "beta-feature2": flag_3}
         # beta-feature2 has no value
+        flags_and_payloads = client.get_all_flags_and_payloads("distinct_id", only_evaluate_locally=True)
         self.assertEqual(
-            client.get_all_flags_and_payloads("distinct_id", only_evaluate_locally=True)["featureFlagPayloads"],
+            client.to_payloads(flags_and_payloads),
             {"beta-feature": "some-payload"},
         )
         self.assertEqual(patch_decide.call_count, 0)

posthog/types.py

@@ -36,10 +36,11 @@ def get_value(self):
 
     @classmethod
     def from_value_and_payload(cls, key: str, value: any, payload: any) -> "FeatureFlag":
+        enabled, variant = (True, value) if isinstance(value, str) else (value, None)
         return cls(
             key=key,
-            enabled=True if isinstance(value, str) else value,
-            variant=value if isinstance(value, str) else None,
+            enabled=enabled,
+            variant=variant,
             reason=None,
             metadata=LegacyFlagMetadata(
                 payload=payload,