Skip to content

Commit 5da56ad

Browse files
authored
Merge branch 'master' into deprecate-payload-method
2 parents 08c3f3b + 80e6e43 commit 5da56ad

File tree

3 files changed

+105
-67
lines changed

3 files changed

+105
-67
lines changed

posthog/client.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -680,15 +680,6 @@ def capture(
680680
f"[FEATURE FLAGS] Unable to get feature variants: {e}"
681681
)
682682

683-
elif self.feature_flags and event != "$feature_flag_called":
684-
# Local evaluation is enabled, flags are loaded, so try and get all flags we can without going to the server
685-
feature_variants = self.get_all_flags(
686-
distinct_id,
687-
groups=(groups or {}),
688-
disable_geoip=disable_geoip,
689-
only_evaluate_locally=True,
690-
)
691-
692683
for feature, variant in (feature_variants or {}).items():
693684
extra_properties[f"$feature/{feature}"] = variant
694685

@@ -1798,7 +1789,7 @@ def get_feature_flag_payload(
17981789
person_properties=None,
17991790
group_properties=None,
18001791
only_evaluate_locally=False,
1801-
send_feature_flag_events=True,
1792+
send_feature_flag_events=False,
18021793
disable_geoip=None,
18031794
):
18041795
"""
@@ -1812,7 +1803,7 @@ def get_feature_flag_payload(
18121803
person_properties: A dictionary of person properties.
18131804
group_properties: A dictionary of group properties.
18141805
only_evaluate_locally: Whether to only evaluate locally.
1815-
send_feature_flag_events: Whether to send feature flag events.
1806+
send_feature_flag_events: Deprecated. Use get_feature_flag() instead if you need events.
18161807
disable_geoip: Whether to disable GeoIP for this request.
18171808
18181809
Examples:
@@ -1834,6 +1825,14 @@ def get_feature_flag_payload(
18341825
DeprecationWarning,
18351826
stacklevel=2,
18361827
)
1828+
if send_feature_flag_events:
1829+
warnings.warn(
1830+
"send_feature_flag_events is deprecated in get_feature_flag_payload() and will be removed "
1831+
"in a future version. Use get_feature_flag() if you want to send $feature_flag_called events.",
1832+
DeprecationWarning,
1833+
stacklevel=2,
1834+
)
1835+
18371836
feature_flag_result = self._get_feature_flag_result(
18381837
key,
18391838
distinct_id,

posthog/test/test_client.py

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,9 @@ def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags):
413413
)
414414
client.feature_flags = [multivariate_flag, basic_flag, false_flag]
415415

416-
msg_uuid = client.capture("python test event", distinct_id="distinct_id")
416+
msg_uuid = client.capture(
417+
"python test event", distinct_id="distinct_id", send_feature_flags=True
418+
)
417419
self.assertIsNotNone(msg_uuid)
418420
self.assertFalse(self.failed)
419421

@@ -569,6 +571,7 @@ def test_dont_override_capture_with_local_flags(self, patch_flags):
569571
"python test event",
570572
distinct_id="distinct_id",
571573
properties={"$feature/beta-feature-local": "my-custom-variant"},
574+
send_feature_flags=True,
572575
)
573576
self.assertIsNotNone(msg_uuid)
574577
self.assertFalse(self.failed)
@@ -750,6 +753,88 @@ def test_basic_capture_with_feature_flags_switched_off_doesnt_send_them(
750753

751754
self.assertEqual(patch_flags.call_count, 0)
752755

756+
@mock.patch("posthog.client.flags")
757+
def test_capture_with_send_feature_flags_false_and_local_evaluation_doesnt_send_flags(
758+
self, patch_flags
759+
):
760+
"""Test that send_feature_flags=False with local evaluation enabled does NOT send flags"""
761+
patch_flags.return_value = {"featureFlags": {"beta-feature": "remote-variant"}}
762+
763+
multivariate_flag = {
764+
"id": 1,
765+
"name": "Beta Feature",
766+
"key": "beta-feature-local",
767+
"active": True,
768+
"rollout_percentage": 100,
769+
"filters": {
770+
"groups": [
771+
{
772+
"rollout_percentage": 100,
773+
},
774+
],
775+
"multivariate": {
776+
"variants": [
777+
{
778+
"key": "first-variant",
779+
"name": "First Variant",
780+
"rollout_percentage": 50,
781+
},
782+
{
783+
"key": "second-variant",
784+
"name": "Second Variant",
785+
"rollout_percentage": 50,
786+
},
787+
]
788+
},
789+
},
790+
}
791+
simple_flag = {
792+
"id": 2,
793+
"name": "Simple Flag",
794+
"key": "simple-flag",
795+
"active": True,
796+
"filters": {
797+
"groups": [
798+
{
799+
"rollout_percentage": 100,
800+
}
801+
],
802+
},
803+
}
804+
805+
with mock.patch("posthog.client.batch_post") as mock_post:
806+
client = Client(
807+
FAKE_TEST_API_KEY,
808+
on_error=self.set_fail,
809+
personal_api_key=FAKE_TEST_API_KEY,
810+
sync_mode=True,
811+
)
812+
client.feature_flags = [multivariate_flag, simple_flag]
813+
814+
msg_uuid = client.capture(
815+
"python test event",
816+
distinct_id="distinct_id",
817+
send_feature_flags=False,
818+
)
819+
self.assertIsNotNone(msg_uuid)
820+
self.assertFalse(self.failed)
821+
822+
# Get the enqueued message from the mock
823+
mock_post.assert_called_once()
824+
batch_data = mock_post.call_args[1]["batch"]
825+
msg = batch_data[0]
826+
827+
self.assertEqual(msg["event"], "python test event")
828+
self.assertEqual(msg["distinct_id"], "distinct_id")
829+
830+
# CRITICAL: Verify local flags are NOT included in the event
831+
self.assertNotIn("$feature/beta-feature-local", msg["properties"])
832+
self.assertNotIn("$feature/simple-flag", msg["properties"])
833+
self.assertNotIn("$active_feature_flags", msg["properties"])
834+
835+
# CRITICAL: Verify the /flags API was NOT called
836+
self.assertEqual(patch_flags.call_count, 0)
837+
753838
@mock.patch("posthog.client.flags")
754839
def test_capture_with_send_feature_flags_true_and_local_evaluation_uses_local_flags(
755840
self, patch_flags

posthog/test/test_feature_flags.py

Lines changed: 9 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3062,6 +3062,7 @@ def test_boolean_feature_flag_payload_decide(self, patch_flags, patch_capture):
30623062
"some-distinct-id",
30633063
match_value=True,
30643064
person_properties={"region": "USA"},
3065+
send_feature_flag_events=True,
30653066
),
30663067
300,
30673068
)
@@ -4051,7 +4052,9 @@ def test_capture_is_called_with_flag_details_and_payload(
40514052

40524053
self.assertEqual(
40534054
client.get_feature_flag_payload(
4054-
"decide-flag-with-payload", "some-distinct-id"
4055+
"decide-flag-with-payload",
4056+
"some-distinct-id",
4057+
send_feature_flag_events=True,
40554058
),
40564059
{"foo": "bar"},
40574060
)
@@ -4127,9 +4130,10 @@ def test_capture_is_called_but_does_not_add_all_flags(self, patch_flags):
41274130

41284131
@mock.patch.object(Client, "capture")
41294132
@mock.patch("posthog.client.flags")
4130-
def test_capture_is_called_in_get_feature_flag_payload(
4133+
def test_get_feature_flag_payload_does_not_send_feature_flag_called_events(
41314134
self, patch_flags, patch_capture
41324135
):
4136+
"""Test that get_feature_flag_payload does NOT send $feature_flag_called events"""
41334137
patch_flags.return_value = {
41344138
"featureFlags": {"person-flag": True},
41354139
"featureFlagPayloads": {"person-flag": 300},
@@ -4151,68 +4155,18 @@ def test_capture_is_called_in_get_feature_flag_payload(
41514155
"rollout_percentage": 100,
41524156
}
41534157
],
4158+
"payloads": {"true": '"payload"'},
41544159
},
41554160
}
41564161
]
41574162

4158-
# Call get_feature_flag_payload with match_value=None to trigger get_feature_flag
4159-
client.get_feature_flag_payload(
4160-
key="person-flag",
4161-
distinct_id="some-distinct-id",
4162-
person_properties={"region": "USA", "name": "Aloha"},
4163-
)
4164-
4165-
# Assert that capture was called once, with the correct parameters
4166-
self.assertEqual(patch_capture.call_count, 1)
4167-
patch_capture.assert_called_with(
4168-
"$feature_flag_called",
4169-
distinct_id="some-distinct-id",
4170-
properties={
4171-
"$feature_flag": "person-flag",
4172-
"$feature_flag_response": True,
4173-
"locally_evaluated": True,
4174-
"$feature/person-flag": True,
4175-
},
4176-
groups={},
4177-
disable_geoip=None,
4178-
)
4179-
4180-
# Reset mocks for further tests
4181-
patch_capture.reset_mock()
4182-
patch_flags.reset_mock()
4183-
4184-
# Call get_feature_flag_payload again for the same user; capture should not be called again because we've already reported an event for this distinct_id + flag
4185-
client.get_feature_flag_payload(
4163+
payload = client.get_feature_flag_payload(
41864164
key="person-flag",
41874165
distinct_id="some-distinct-id",
41884166
person_properties={"region": "USA", "name": "Aloha"},
41894167
)
4190-
4168+
self.assertIsNotNone(payload)
41914169
self.assertEqual(patch_capture.call_count, 0)
4192-
patch_capture.reset_mock()
4193-
4194-
# Call get_feature_flag_payload for a different user; capture should be called
4195-
client.get_feature_flag_payload(
4196-
key="person-flag",
4197-
distinct_id="some-distinct-id2",
4198-
person_properties={"region": "USA", "name": "Aloha"},
4199-
)
4200-
4201-
self.assertEqual(patch_capture.call_count, 1)
4202-
patch_capture.assert_called_with(
4203-
"$feature_flag_called",
4204-
distinct_id="some-distinct-id2",
4205-
properties={
4206-
"$feature_flag": "person-flag",
4207-
"$feature_flag_response": True,
4208-
"locally_evaluated": True,
4209-
"$feature/person-flag": True,
4210-
},
4211-
groups={},
4212-
disable_geoip=None,
4213-
)
4214-
4215-
patch_capture.reset_mock()
42164170

42174171
@mock.patch("posthog.client.flags")
42184172
def test_fallback_to_api_in_get_feature_flag_payload_when_flag_has_static_cohort(

0 commit comments

Comments
 (0)