diff --git a/.sampo/changesets/mixed-targeting-local-eval.md b/.sampo/changesets/mixed-targeting-local-eval.md new file mode 100644 index 00000000..4a78a23d --- /dev/null +++ b/.sampo/changesets/mixed-targeting-local-eval.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Support mixed user+group targeting in local flag evaluation. diff --git a/posthog/client.py b/posthog/client.py index c0c5491c..1b7f0a34 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -1454,8 +1454,9 @@ def _compute_flag_locally( flag_filters = feature_flag.get("filters") or {} aggregation_group_type_index = flag_filters.get("aggregation_group_type_index") + group_type_mapping = self.group_type_mapping or {} + if aggregation_group_type_index is not None: - group_type_mapping = self.group_type_mapping or {} group_name = group_type_mapping.get(str(aggregation_group_type_index)) if not group_name: @@ -1493,6 +1494,9 @@ def _compute_flag_locally( evaluation_cache=evaluation_cache, device_id=device_id, bucketing_value=group_key, + group_type_mapping=group_type_mapping, + groups=groups, + group_properties=group_properties, ) else: bucketing_value = resolve_bucketing_value( @@ -1507,6 +1511,9 @@ def _compute_flag_locally( evaluation_cache=evaluation_cache, device_id=device_id, bucketing_value=bucketing_value, + group_type_mapping=group_type_mapping, + groups=groups, + group_properties=group_properties, ) def feature_enabled( diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index acd2d999..edfdbef9 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -293,6 +293,9 @@ def match_feature_flag_properties( evaluation_cache=None, device_id=None, bucketing_value=None, + group_type_mapping=None, + groups=None, + group_properties=None, ) -> FlagValue: if bucketing_value is None: warnings.warn( @@ -305,32 +308,65 @@ def match_feature_flag_properties( flag_filters = flag.get("filters") or {} flag_conditions = flag_filters.get("groups") or [] + flag_aggregation = flag_filters.get("aggregation_group_type_index") is_inconclusive = False cohort_properties = cohort_properties or {} + groups = groups or {} + group_properties = group_properties or {} + group_type_mapping = group_type_mapping or {} # Some filters can be explicitly set to null, which require accessing variants like so flag_variants = (flag_filters.get("multivariate") or {}).get("variants") or [] valid_variant_keys = [variant["key"] for variant in flag_variants] for condition in flag_conditions: try: - # if any one condition resolves to True, we can shortcircuit and return - # the matching variant + # Per-condition aggregation overrides only when the condition explicitly + # sets its own aggregation_group_type_index (mixed targeting). + # When absent, use the properties/bucketing already resolved by the caller. + condition_aggregation = condition.get( + "aggregation_group_type_index", flag_aggregation + ) + + # Mixed-override path: condition-level aggregation differs from flag-level. + # This assumes flag-level aggregation is None for mixed flags. + if condition_aggregation != flag_aggregation: + if condition_aggregation is not None: + group_name = group_type_mapping.get(str(condition_aggregation)) + if not group_name or group_name not in groups: + log.debug( + "Skipping group condition for flag '%s': group type index %s not available", + flag.get("key", ""), + condition_aggregation, + ) + continue + if group_name not in group_properties: + is_inconclusive = True + continue + effective_properties = group_properties[group_name] + effective_bucketing = groups[group_name] + else: + effective_properties = properties + effective_bucketing = bucketing_value + else: + effective_properties = properties + effective_bucketing = bucketing_value + if is_condition_match( flag, distinct_id, condition, - properties, + effective_properties, cohort_properties, flags_by_key, evaluation_cache, - bucketing_value=bucketing_value, + bucketing_value=effective_bucketing, device_id=device_id, ): variant_override = condition.get("variant") if variant_override and variant_override in valid_variant_keys: variant = variant_override else: - variant = get_matching_variant(flag, bucketing_value) + variant = get_matching_variant(flag, effective_bucketing) return variant or True except RequiresServerEvaluation: # Static cohort or other missing server-side data - must fallback to API diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index 2e7168ed..0a8269a5 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -4,6 +4,7 @@ from unittest import mock from dateutil import parser, tz from freezegun import freeze_time +from parameterized import parameterized from posthog.client import Client from posthog.feature_flags import ( @@ -3706,6 +3707,145 @@ def test_group_flag_dependency_ignores_device_id_bucketing_identifier( self.assertTrue(result) self.assertEqual(patch_flags.call_count, 0) + MIXED_FLAG = { + "id": 1, + "key": "mixed-flag", + "active": True, + "filters": { + "aggregation_group_type_index": None, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [ + { + "key": "plan", + "value": "enterprise", + "operator": "exact", + "type": "group", + "group_type_index": 0, + } + ], + "rollout_percentage": 100, + }, + { + "aggregation_group_type_index": None, + "properties": [ + { + "key": "email", + "value": "test@example.com", + "operator": "exact", + "type": "person", + } + ], + "rollout_percentage": 100, + }, + ], + }, + } + + @parameterized.expand( + [ + ( + "person_condition_matches", + {"person_properties": {"email": "test@example.com"}}, + True, + ), + ( + "group_condition_matches", + { + "groups": {"company": "acme"}, + "group_properties": {"company": {"plan": "enterprise"}}, + }, + True, + ), + ( + "no_match", + { + "person_properties": {"email": "wrong@example.com"}, + "groups": {"company": "acme"}, + "group_properties": {"company": {"plan": "free"}}, + }, + False, + ), + ] + ) + @mock.patch("posthog.client.flags") + def test_mixed_targeting(self, _name, call_kwargs, expected, patch_flags): + client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY) + client.feature_flags = [self.MIXED_FLAG] + client.group_type_mapping = {"0": "company"} + + result = client.get_feature_flag("mixed-flag", "user-123", **call_kwargs) + self.assertEqual(bool(result), expected) + self.assertEqual(patch_flags.call_count, 0) + + @mock.patch("posthog.client.flags") + def test_mixed_targeting_only_group_conditions_no_groups_passed(self, patch_flags): + client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY) + client.feature_flags = [ + { + "id": 1, + "key": "mixed-flag", + "active": True, + "filters": { + "aggregation_group_type_index": None, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [], + "rollout_percentage": 100, + }, + ], + }, + } + ] + client.group_type_mapping = {"0": "company"} + + # No groups passed, no person condition — all conditions skip, returns False + result = client.get_feature_flag( + "mixed-flag", + "user-123", + ) + self.assertFalse(result) + self.assertEqual(patch_flags.call_count, 0) + + @mock.patch("posthog.client.flags") + def test_mixed_targeting_rollout_uses_correct_bucketing(self, patch_flags): + client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY) + client.feature_flags = [ + { + "id": 1, + "key": "mixed-flag", + "active": True, + "filters": { + "aggregation_group_type_index": None, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [], + "rollout_percentage": 100, + }, + { + "aggregation_group_type_index": None, + "properties": [], + "rollout_percentage": 0, + }, + ], + }, + } + ] + client.group_type_mapping = {"0": "company"} + + # Group condition at 100% matches, person condition at 0% doesn't matter + result = client.get_feature_flag( + "mixed-flag", + "user-123", + groups={"company": "acme"}, + group_properties={"company": {}}, + ) + self.assertTrue(result) + self.assertEqual(patch_flags.call_count, 0) + @mock.patch("posthog.client.flags") def test_get_all_flags_with_device_id_bucketing(self, patch_flags): """