Skip to content

Commit

Permalink
fix(experiments): fix funnel events validation (#20127)
Browse files Browse the repository at this point in the history
  • Loading branch information
jurajmajerik authored Feb 5, 2024
1 parent c6cf384 commit d937315
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 4 deletions.
23 changes: 20 additions & 3 deletions ee/clickhouse/queries/experiments/funnel_experiment_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,28 @@ def validate_event_variants(funnel_results, variants):
if event.get("order") == 0:
eventsWithOrderZero.append(event)

missing_variants = set(variants)
missing_variants = []

# Check if "control" is present
control_found = False
for event in eventsWithOrderZero:
event_variant = event.get("breakdown_value")[0]
if event_variant == "control":
control_found = True
break
if not control_found:
missing_variants.append("control")

# Check if at least one of the test variants is present
test_variants = [variant for variant in variants if variant != "control"]
test_variant_found = False
for event in eventsWithOrderZero:
event_variant = event.get("breakdown_value")[0]
if event_variant in missing_variants:
missing_variants.discard(event_variant)
if event_variant in test_variants:
test_variant_found = True
break
if not test_variant_found:
missing_variants.extend(test_variants)

if not len(missing_variants) == 0:
missing_variants_str = ", ".join(missing_variants)
Expand Down
60 changes: 60 additions & 0 deletions ee/clickhouse/queries/test/test_experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,45 @@ def test_validate_event_variants_missing_variants(self):

self.assertEqual(expected_code, context.exception.detail[0].code)

def test_validate_event_variants_missing_control(self):
funnel_results = [
[
{
"action_id": "step-a-1",
"name": "step-a-1",
"custom_name": None,
"order": 0,
"people": [],
"count": 1,
"type": "events",
"average_conversion_time": None,
"median_conversion_time": None,
"breakdown": ["test_1"],
"breakdown_value": ["test_1"],
},
{
"action_id": "step-a-2",
"name": "step-a-2",
"custom_name": None,
"order": 1,
"people": [],
"count": 0,
"type": "events",
"average_conversion_time": None,
"median_conversion_time": None,
"breakdown": ["test_1"],
"breakdown_value": ["test_1"],
},
]
]

# Only 1 test variant is required to return results
expected_code = "missing-flag-variants::control"
with self.assertRaises(ValidationError) as context:
validate_funnel_event_variants(funnel_results, ["control", "test_1", "test_2"])

self.assertEqual(expected_code, context.exception.detail[0].code)

def test_validate_event_variants_ignore_old_variant(self):
funnel_results = [
[
Expand Down Expand Up @@ -121,6 +160,27 @@ def test_validate_event_variants_missing_variants(self):

self.assertEqual(expected_code, context.exception.detail[0].code)

def test_validate_event_variants_missing_control(self):
insight_results = [
{
"action": {
"id": "step-b-0",
"type": "events",
"order": 0,
"name": "step-b-0",
},
"label": "test_1",
"breakdown_value": "test_1",
}
]

# Only 1 test variant is required to return results
expected_code = "missing-flag-variants::control"
with self.assertRaises(ValidationError) as context:
validate_trend_event_variants(insight_results, ["control", "test_1", "test_2"])

self.assertEqual(expected_code, context.exception.detail[0].code)

def test_validate_event_variants_ignore_old_variant(self):
insight_results = [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# serializer version: 1
# name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results
'''
/* user_id:130 celery:posthog.tasks.tasks.sync_insight_caching_state */
/* user_id:127 celery:posthog.tasks.tasks.sync_insight_caching_state */
SELECT team_id,
date_diff('second', max(timestamp), now()) AS age
FROM events
Expand Down

0 comments on commit d937315

Please sign in to comment.