Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ export function FeatureFlagReleaseConditions({
<div className="basis-full flex flex-col mt-1 text-secondary tabular-nums">
<span>
Filters match: <b>~{pluralize(affected, singularName, pluralName)}</b>
{filters.aggregation_group_type_index == null && (
{resolvedGroupTypeIndex == null && (
<Tooltip title={MATCHING_ESTIMATE_TOOLTIP} interactive>
<IconInfo className="text-muted text-xs ml-0.5" />
</Tooltip>
Expand Down
7 changes: 2 additions & 5 deletions frontend/src/scenes/feature-flags/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@ export const MATCHING_ESTIMATE_TOOLTIP = (
</Link>
</div>
<div className="mt-1">
Estimated from{' '}
<Link to="https://posthog.com/docs/data/anonymous-vs-identified-events" target="_blank">
identified users
</Link>{' '}
only. Anonymous visitors can still match this flag, so the actual number may be higher.
Estimated from persons active in the last 30 days, so people who have not been seen recently are not
counted.
Comment thread
posthog[bot] marked this conversation as resolved.
Resolved
</div>
</>
)
13 changes: 11 additions & 2 deletions products/feature_flags/backend/api/feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -2602,9 +2602,18 @@ class UserBlastRadiusRequestSerializer(serializers.Serializer):

class UserBlastRadiusResponseSerializer(serializers.Serializer):
affected = serializers.IntegerField(
help_text="Number of entities matching the condition (users or groups depending on group_type_index)"
help_text=(
"Number of entities matching the condition. For person-based flags this counts persons active in the "
"last 30 days; for group-based flags it counts all groups of this type."
)
)
total = serializers.IntegerField(
help_text=(
"Denominator the affected count is shown against. For person-based flags this is the number of persons "
"active in the last 30 days; for group-based flags it is the all-time number of groups of this type in "
"the project."
)
)
total = serializers.IntegerField(help_text="Total number of entities of this type in the project")


# HYPERCACHE CONTRACT: This serializer defines the JSON schema that the Rust feature-flags
Expand Down

Large diffs are not rendered by default.

88 changes: 64 additions & 24 deletions products/feature_flags/backend/api/test/test_feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
APIBaseTest,
ClickhouseTestMixin,
FuzzyInt,
_create_event,
_create_person,
flush_persons_and_events,
snapshot_clickhouse_queries,
Expand Down Expand Up @@ -9052,11 +9053,23 @@ def test_insert_failure_is_recorded_as_failure_not_success(self, mock_batch_eval
self.assertEqual(history.error_code, CohortErrorCode.UNKNOWN)


def _create_active_person(*args, **kwargs):
# Flag sizing counts persons active in the recent window, so every person a sizing test relies
# on needs a recent event. Wrap person creation to emit one $pageview per person.
person = _create_person(*args, **kwargs)
team = kwargs.get("team")
team_id = kwargs.get("team_id") or (team.pk if team else None)
distinct_ids = kwargs.get("distinct_ids") or []
if distinct_ids:
_create_event(team_id=team_id, event="$pageview", distinct_id=distinct_ids[0])
return person


class TestBlastRadius(ClickhouseTestMixin, APIBaseTest):
@snapshot_clickhouse_queries
def test_user_blast_radius(self):
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9084,6 +9097,33 @@ def test_user_blast_radius(self):
response_json = response.json()
self.assertLessEqual({"affected": 4, "total": 10}.items(), response_json.items())

def test_user_blast_radius_excludes_inactive_matching_persons(self):
# A person matching the filter but with no recent activity is not reachable audience, so it
# must not count toward "affected" — otherwise inactive matches inflate the sized audience,
# which is the bug this fix guards against.
_create_active_person(team_id=self.team.pk, distinct_ids=["active-match-1"], properties={"group": "match"})
_create_active_person(team_id=self.team.pk, distinct_ids=["active-match-2"], properties={"group": "match"})
# Matches the filter but emits no event in the window, so it drops out of the count.
_create_person(team_id=self.team.pk, distinct_ids=["inactive-match"], properties={"group": "match"})
# Active but does not match, so it keeps the total above the matched count.
_create_active_person(team_id=self.team.pk, distinct_ids=["active-nonmatch"], properties={"group": "other"})
flush_persons_and_events()

response = self.client.post(
f"/api/projects/{self.team.id}/feature_flags/user_blast_radius",
{
"condition": {
"properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}],
"rollout_percentage": 100,
}
},
)

self.assertEqual(response.status_code, status.HTTP_200_OK)
# Only the two active matches count; the inactive match is excluded, and the active
# non-match keeps the total at three.
self.assertLessEqual({"affected": 2, "total": 3}.items(), response.json().items())

@parameterized.expand(
[
(
Expand Down Expand Up @@ -9138,7 +9178,7 @@ def test_user_blast_radius_endpoint_returns_400_for_unevaluable_filters(self):

def test_user_blast_radius_with_flag_dependency(self):
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9174,7 +9214,7 @@ def test_user_blast_radius_with_flag_dependency(self):

def test_user_blast_radius_with_flag_dependency_and_person_property(self):
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9320,7 +9360,7 @@ def test_user_blast_radius_persons_with_groups_and_flag_dependency(self):
@freeze_time("2024-01-11")
def test_user_blast_radius_with_relative_date_filters(self):
for i in range(8):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}", "created_at": f"2023-0{i + 1}-04"},
Expand Down Expand Up @@ -9373,7 +9413,7 @@ def test_user_blast_radius_with_zero_users(self):

def test_user_blast_radius_with_zero_selected_users(self):
for i in range(5):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9403,7 +9443,7 @@ def test_user_blast_radius_with_zero_selected_users(self):

def test_user_blast_radius_with_all_selected_users(self):
for i in range(5):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand All @@ -9424,7 +9464,7 @@ def test_user_blast_radius_with_distinct_id_filter(self):
# person_distinct_id2 table and must be joined via pdi. Filtering by distinct_id
# in a release condition should match the persons that own that distinct_id.
for i in range(5):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9455,12 +9495,12 @@ def test_user_blast_radius_with_distinct_id_filter(self):
def test_user_blast_radius_with_distinct_id_filter_multiple_distinct_ids_per_person(self):
# A single person can own multiple distinct_ids; filtering by any one should still
# count that person exactly once.
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=["alias-a", "alias-b"],
properties={"group": "0"},
)
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=["other"],
properties={"group": "1"},
Expand Down Expand Up @@ -9492,7 +9532,7 @@ def test_user_blast_radius_with_distinct_id_filter_multiple_distinct_ids_per_per
def test_user_blast_radius_with_single_cohort(self):
# Just to shake things up, we're using integers for the group property
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": i},
Expand Down Expand Up @@ -9558,7 +9598,7 @@ def test_user_blast_radius_with_single_cohort(self):
@snapshot_clickhouse_queries
def test_user_blast_radius_with_multiple_precalculated_cohorts(self):
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9635,7 +9675,7 @@ def test_user_blast_radius_with_multiple_precalculated_cohorts(self):
@snapshot_clickhouse_queries
def test_user_blast_radius_with_multiple_static_cohorts(self):
for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"group": f"{i}"},
Expand Down Expand Up @@ -9959,17 +9999,17 @@ def test_user_blast_radius_with_group_key_property(self):

def test_user_blast_radius_with_integer_property_values(self):
"""Test that integer property values are correctly normalized to strings for matching"""
_create_person(
_create_active_person(
distinct_ids=["p1"],
team_id=self.team.pk,
properties={"age": 25, "score": 100},
)
_create_person(
_create_active_person(
distinct_ids=["p2"],
team_id=self.team.pk,
properties={"age": "25", "score": "100"},
)
_create_person(
_create_active_person(
distinct_ids=["p3"],
team_id=self.team.pk,
properties={"age": 30, "score": 200},
Expand Down Expand Up @@ -10143,12 +10183,12 @@ def test_user_blast_radius_with_dynamic_cohort(self):
},
)

_create_person(
_create_active_person(
distinct_ids=["p1"],
team_id=self.team.pk,
properties={"email": "user@posthog.com"},
)
_create_person(
_create_active_person(
distinct_ids=["p2"],
team_id=self.team.pk,
properties={"email": "user@example.com"},
Expand Down Expand Up @@ -10476,7 +10516,7 @@ def test_user_blast_radius_with_semver_operators(self):
"2.1.0",
]
for version in versions:
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person_{version}"],
properties={"app_version": version},
Expand Down Expand Up @@ -10694,7 +10734,7 @@ def test_user_blast_radius_with_semver_caret_0x_versions(self):
"1.0.0",
]
for version in versions:
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person_{version}"],
properties={"app_version": version},
Expand Down Expand Up @@ -10764,7 +10804,7 @@ def test_user_blast_radius_with_semver_operators_on_groups(self):

# Create persons in organizations
for i, version in enumerate(versions):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person_{i}"],
properties={"$group_0": f"org-{version}"},
Expand Down Expand Up @@ -10803,7 +10843,7 @@ def test_user_blast_radius_person_condition_separate_from_group_condition(self):
)

for i in range(10):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"plan": "pro" if i < 6 else "free"},
Expand Down Expand Up @@ -10864,7 +10904,7 @@ def test_user_blast_radius_person_condition_separate_from_group_condition(self):

def test_user_blast_radius_pure_person_condition_has_no_group_counts(self):
for i in range(5):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"plan": "pro" if i < 3 else "free"},
Expand Down Expand Up @@ -10944,7 +10984,7 @@ def test_user_blast_radius_cohort_condition_and_group_condition_separate(self):
)

for i in range(8):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"plan": "pro" if i < 5 else "free"},
Expand Down Expand Up @@ -11021,7 +11061,7 @@ def test_user_blast_radius_cohort_condition_and_group_condition_separate(self):

def test_user_blast_radius_no_error_fields_for_successful_queries(self):
for i in range(3):
_create_person(
_create_active_person(
team_id=self.team.pk,
distinct_ids=[f"person{i}"],
properties={"plan": "pro"},
Expand Down
Loading