more pr feedback

Author: dmarticus

posthog/client.py

@@ -398,50 +398,28 @@ def capture(
         extra_properties: dict[str, Any] = {}
         feature_variants: Optional[dict[str, Union[bool, str]]] = {}
 
-        # Parse send_feature_flags parameter
-        should_send_flags = False
-        only_evaluate_locally = None
-        flag_person_properties = None
-        flag_group_properties = None
+        # Parse and normalize send_feature_flags parameter
+        flag_options = self._parse_send_feature_flags(send_feature_flags)
 
-        if isinstance(send_feature_flags, dict):
-            # SendFeatureFlagsOptions object
-            should_send_flags = True
-            only_evaluate_locally = send_feature_flags.get("only_evaluate_locally")
-            flag_person_properties = send_feature_flags.get("person_properties")
-            flag_group_properties = send_feature_flags.get("group_properties")
-        elif send_feature_flags:
-            # Boolean True
-            should_send_flags = True
-
-        if should_send_flags:
+        if flag_options["should_send"]:
             try:
-                if only_evaluate_locally is True:
+                if flag_options["only_evaluate_locally"] is True:
                     # Only use local evaluation
                     feature_variants = self.get_all_flags(
                         distinct_id,
                         groups=(groups or {}),
-                        person_properties=flag_person_properties,
-                        group_properties=flag_group_properties,
+                        person_properties=flag_options["person_properties"],
+                        group_properties=flag_options["group_properties"],
                         disable_geoip=disable_geoip,
                         only_evaluate_locally=True,
                     )
-                elif only_evaluate_locally is False:
-                    # Force remote evaluation via /decide
-                    feature_variants = self.get_feature_variants(
-                        distinct_id,
-                        groups,
-                        person_properties=flag_person_properties,
-                        group_properties=flag_group_properties,
-                        disable_geoip=disable_geoip,
-                    )
                 else:
                     # Default behavior - use remote evaluation
                     feature_variants = self.get_feature_variants(
                         distinct_id,
                         groups,
-                        person_properties=flag_person_properties,
-                        group_properties=flag_group_properties,
+                        person_properties=flag_options["person_properties"],
+                        group_properties=flag_options["group_properties"],
                         disable_geoip=disable_geoip,
                     )
             except Exception as e:
@@ -474,6 +452,42 @@ def capture(
 
         return self._enqueue(msg, disable_geoip)
 
+    def _parse_send_feature_flags(self, send_feature_flags) -> dict:
+        """
+        Parse and normalize send_feature_flags parameter into a standard format.
+
+        Args:
+            send_feature_flags: Either bool or SendFeatureFlagsOptions dict
+
+        Returns:
+            dict: Normalized options with keys: should_send, only_evaluate_locally,
+                  person_properties, group_properties
+
+        Raises:
+            TypeError: If send_feature_flags is not bool or dict
+        """
+        if isinstance(send_feature_flags, dict):
+            return {
+                "should_send": True,
+                "only_evaluate_locally": send_feature_flags.get(
+                    "only_evaluate_locally"
+                ),
+                "person_properties": send_feature_flags.get("person_properties"),
+                "group_properties": send_feature_flags.get("group_properties"),
+            }
+        elif isinstance(send_feature_flags, bool):
+            return {
+                "should_send": send_feature_flags,
+                "only_evaluate_locally": None,
+                "person_properties": None,
+                "group_properties": None,
+            }
+        else:
+            raise TypeError(
+                f"Invalid type for send_feature_flags: {type(send_feature_flags)}. "
+                f"Expected bool or dict."
+            )
+
     def set(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
         distinct_id = kwargs.get("distinct_id", None)
         properties = kwargs.get("properties", None)

posthog/test/test_client.py

@@ -2173,3 +2173,76 @@ def test_get_remote_config_payload_requires_personal_api_key(self):
         result = client.get_remote_config_payload("test-flag")
 
         self.assertIsNone(result)
+
+    def test_parse_send_feature_flags_method(self):
+        """Test the _parse_send_feature_flags helper method"""
+        client = Client(FAKE_TEST_API_KEY, sync_mode=True)
+
+        # Test boolean True
+        result = client._parse_send_feature_flags(True)
+        expected = {
+            "should_send": True,
+            "only_evaluate_locally": None,
+            "person_properties": None,
+            "group_properties": None,
+        }
+        self.assertEqual(result, expected)
+
+        # Test boolean False
+        result = client._parse_send_feature_flags(False)
+        expected = {
+            "should_send": False,
+            "only_evaluate_locally": None,
+            "person_properties": None,
+            "group_properties": None,
+        }
+        self.assertEqual(result, expected)
+
+        # Test options dict with all fields
+        options = {
+            "only_evaluate_locally": True,
+            "person_properties": {"plan": "premium"},
+            "group_properties": {"company": {"type": "enterprise"}},
+        }
+        result = client._parse_send_feature_flags(options)
+        expected = {
+            "should_send": True,
+            "only_evaluate_locally": True,
+            "person_properties": {"plan": "premium"},
+            "group_properties": {"company": {"type": "enterprise"}},
+        }
+        self.assertEqual(result, expected)
+
+        # Test options dict with partial fields
+        options = {"person_properties": {"user_id": "123"}}
+        result = client._parse_send_feature_flags(options)
+        expected = {
+            "should_send": True,
+            "only_evaluate_locally": None,
+            "person_properties": {"user_id": "123"},
+            "group_properties": None,
+        }
+        self.assertEqual(result, expected)
+
+        # Test empty dict
+        result = client._parse_send_feature_flags({})
+        expected = {
+            "should_send": True,
+            "only_evaluate_locally": None,
+            "person_properties": None,
+            "group_properties": None,
+        }
+        self.assertEqual(result, expected)
+
+        # Test invalid types
+        with self.assertRaises(TypeError) as cm:
+            client._parse_send_feature_flags("invalid")
+        self.assertIn("Invalid type for send_feature_flags", str(cm.exception))
+
+        with self.assertRaises(TypeError) as cm:
+            client._parse_send_feature_flags(123)
+        self.assertIn("Invalid type for send_feature_flags", str(cm.exception))
+
+        with self.assertRaises(TypeError) as cm:
+            client._parse_send_feature_flags(None)
+        self.assertIn("Invalid type for send_feature_flags", str(cm.exception))

posthog/types.py

@@ -15,7 +15,7 @@ class SendFeatureFlagsOptions(TypedDict, total=False):
     Args:
         only_evaluate_locally: Whether to only use local evaluation for feature flags.
             If True, only flags that can be evaluated locally will be included.
-            If False, remote evaluation via /decide API will be used when needed.
+            If False, remote evaluation via /flags API will be used when needed.
         person_properties: Properties to use for feature flag evaluation specific to this event.
             These properties will be merged with any existing person properties.
         group_properties: Group properties to use for feature flag evaluation specific to this event.