Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions posthog/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import sys
import warnings
from datetime import datetime, timedelta
from typing import Any, Dict, Optional, Union
from typing_extensions import Unpack
Expand Down Expand Up @@ -679,15 +680,6 @@ def capture(
f"[FEATURE FLAGS] Unable to get feature variants: {e}"
)

elif self.feature_flags and event != "$feature_flag_called":
# 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 or {}).items():
extra_properties[f"$feature/{feature}"] = variant

Expand Down Expand Up @@ -1797,7 +1789,7 @@ def get_feature_flag_payload(
person_properties=None,
group_properties=None,
only_evaluate_locally=False,
send_feature_flag_events=True,
send_feature_flag_events=False,
disable_geoip=None,
):
"""
Expand All @@ -1811,7 +1803,7 @@ def get_feature_flag_payload(
person_properties: A dictionary of person properties.
group_properties: A dictionary of group properties.
only_evaluate_locally: Whether to only evaluate locally.
send_feature_flag_events: Whether to send feature flag events.
send_feature_flag_events: Deprecated. Use get_feature_flag() instead if you need events.
disable_geoip: Whether to disable GeoIP for this request.

Examples:
Expand All @@ -1827,6 +1819,14 @@ def get_feature_flag_payload(
Category:
Feature flags
"""
if send_feature_flag_events:
warnings.warn(
"send_feature_flag_events is deprecated in get_feature_flag_payload() and will be removed "
"in a future version. Use get_feature_flag() if you want to send $feature_flag_called events.",
DeprecationWarning,
stacklevel=2,
)

feature_flag_result = self._get_feature_flag_result(
key,
distinct_id,
Expand Down
87 changes: 86 additions & 1 deletion posthog/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,9 @@ def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags):
)
client.feature_flags = [multivariate_flag, basic_flag, false_flag]

msg_uuid = client.capture("python test event", distinct_id="distinct_id")
msg_uuid = client.capture(
"python test event", distinct_id="distinct_id", send_feature_flags=True
)
self.assertIsNotNone(msg_uuid)
self.assertFalse(self.failed)

Expand Down Expand Up @@ -565,6 +567,7 @@ def test_dont_override_capture_with_local_flags(self, patch_flags):
"python test event",
distinct_id="distinct_id",
properties={"$feature/beta-feature-local": "my-custom-variant"},
send_feature_flags=True,
)
self.assertIsNotNone(msg_uuid)
self.assertFalse(self.failed)
Expand Down Expand Up @@ -746,6 +749,88 @@ def test_basic_capture_with_feature_flags_switched_off_doesnt_send_them(

self.assertEqual(patch_flags.call_count, 0)

@mock.patch("posthog.client.flags")
def test_capture_with_send_feature_flags_false_and_local_evaluation_doesnt_send_flags(
self, patch_flags
):
"""Test that send_feature_flags=False with local evaluation enabled does NOT send flags"""
patch_flags.return_value = {"featureFlags": {"beta-feature": "remote-variant"}}

multivariate_flag = {
"id": 1,
"name": "Beta Feature",
"key": "beta-feature-local",
"active": True,
"rollout_percentage": 100,
"filters": {
"groups": [
{
"rollout_percentage": 100,
},
],
"multivariate": {
"variants": [
{
"key": "first-variant",
"name": "First Variant",
"rollout_percentage": 50,
},
{
"key": "second-variant",
"name": "Second Variant",
"rollout_percentage": 50,
},
]
},
},
}
simple_flag = {
"id": 2,
"name": "Simple Flag",
"key": "simple-flag",
"active": True,
"filters": {
"groups": [
{
"rollout_percentage": 100,
}
],
},
}

with mock.patch("posthog.client.batch_post") as mock_post:
client = Client(
FAKE_TEST_API_KEY,
on_error=self.set_fail,
personal_api_key=FAKE_TEST_API_KEY,
sync_mode=True,
)
client.feature_flags = [multivariate_flag, simple_flag]

msg_uuid = client.capture(
"python test event",
distinct_id="distinct_id",
send_feature_flags=False,
)
self.assertIsNotNone(msg_uuid)
self.assertFalse(self.failed)

# Get the enqueued message from the mock
mock_post.assert_called_once()
batch_data = mock_post.call_args[1]["batch"]
msg = batch_data[0]

self.assertEqual(msg["event"], "python test event")
self.assertEqual(msg["distinct_id"], "distinct_id")

# CRITICAL: Verify local flags are NOT included in the event
self.assertNotIn("$feature/beta-feature-local", msg["properties"])
self.assertNotIn("$feature/simple-flag", msg["properties"])
self.assertNotIn("$active_feature_flags", msg["properties"])

# CRITICAL: Verify the /flags API was NOT called
self.assertEqual(patch_flags.call_count, 0)

@mock.patch("posthog.client.flags")
def test_capture_with_send_feature_flags_true_and_local_evaluation_uses_local_flags(
self, patch_flags
Expand Down
64 changes: 9 additions & 55 deletions posthog/test/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -3062,6 +3062,7 @@ def test_boolean_feature_flag_payload_decide(self, patch_flags, patch_capture):
"some-distinct-id",
match_value=True,
person_properties={"region": "USA"},
send_feature_flag_events=True,
),
300,
)
Expand Down Expand Up @@ -4051,7 +4052,9 @@ def test_capture_is_called_with_flag_details_and_payload(

self.assertEqual(
client.get_feature_flag_payload(
"decide-flag-with-payload", "some-distinct-id"
"decide-flag-with-payload",
"some-distinct-id",
send_feature_flag_events=True,
),
{"foo": "bar"},
)
Expand Down Expand Up @@ -4127,9 +4130,10 @@ def test_capture_is_called_but_does_not_add_all_flags(self, patch_flags):

@mock.patch.object(Client, "capture")
@mock.patch("posthog.client.flags")
def test_capture_is_called_in_get_feature_flag_payload(
def test_get_feature_flag_payload_does_not_send_feature_flag_called_events(
self, patch_flags, patch_capture
):
"""Test that get_feature_flag_payload does NOT send $feature_flag_called events"""
patch_flags.return_value = {
"featureFlags": {"person-flag": True},
"featureFlagPayloads": {"person-flag": 300},
Expand All @@ -4151,68 +4155,18 @@ def test_capture_is_called_in_get_feature_flag_payload(
"rollout_percentage": 100,
}
],
"payloads": {"true": '"payload"'},
},
}
]

# Call get_feature_flag_payload with match_value=None to trigger get_feature_flag
client.get_feature_flag_payload(
key="person-flag",
distinct_id="some-distinct-id",
person_properties={"region": "USA", "name": "Aloha"},
)

# Assert that capture was called once, with the correct parameters
self.assertEqual(patch_capture.call_count, 1)
patch_capture.assert_called_with(
"$feature_flag_called",
distinct_id="some-distinct-id",
properties={
"$feature_flag": "person-flag",
"$feature_flag_response": True,
"locally_evaluated": True,
"$feature/person-flag": True,
},
groups={},
disable_geoip=None,
)

# Reset mocks for further tests
patch_capture.reset_mock()
patch_flags.reset_mock()

# 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
client.get_feature_flag_payload(
payload = client.get_feature_flag_payload(
key="person-flag",
distinct_id="some-distinct-id",
person_properties={"region": "USA", "name": "Aloha"},
)

self.assertIsNotNone(payload)
self.assertEqual(patch_capture.call_count, 0)
patch_capture.reset_mock()

# Call get_feature_flag_payload for a different user; capture should be called
client.get_feature_flag_payload(
key="person-flag",
distinct_id="some-distinct-id2",
person_properties={"region": "USA", "name": "Aloha"},
)

self.assertEqual(patch_capture.call_count, 1)
patch_capture.assert_called_with(
"$feature_flag_called",
distinct_id="some-distinct-id2",
properties={
"$feature_flag": "person-flag",
"$feature_flag_response": True,
"locally_evaluated": True,
"$feature/person-flag": True,
},
groups={},
disable_geoip=None,
)

patch_capture.reset_mock()

@mock.patch("posthog.client.flags")
def test_fallback_to_api_in_get_feature_flag_payload_when_flag_has_static_cohort(
Expand Down
Loading