From 563d1443e41b90181e36567eee8c6713f2f73e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Thu, 8 Feb 2024 17:31:45 +0100 Subject: [PATCH 1/6] feat(hogql): strict order funnel --- .../insights/funnels/__init__.py | 1 + .../insights/funnels/funnel_strict.py | 129 ++++ .../funnels/test/test_funnel_strict.py | 620 ++++++++++++++++++ .../hogql_queries/insights/funnels/utils.py | 5 +- 4 files changed, 752 insertions(+), 3 deletions(-) create mode 100644 posthog/hogql_queries/insights/funnels/funnel_strict.py create mode 100644 posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py diff --git a/posthog/hogql_queries/insights/funnels/__init__.py b/posthog/hogql_queries/insights/funnels/__init__.py index 35dbf5b0045c1..d6cddab2ba293 100644 --- a/posthog/hogql_queries/insights/funnels/__init__.py +++ b/posthog/hogql_queries/insights/funnels/__init__.py @@ -1,2 +1,3 @@ from .base import FunnelBase from .funnel import Funnel +from .funnel_strict import FunnelStrict diff --git a/posthog/hogql_queries/insights/funnels/funnel_strict.py b/posthog/hogql_queries/insights/funnels/funnel_strict.py new file mode 100644 index 0000000000000..c80ad7297f0d5 --- /dev/null +++ b/posthog/hogql_queries/insights/funnels/funnel_strict.py @@ -0,0 +1,129 @@ +from typing import List + +from posthog.hogql import ast +from posthog.hogql.parser import parse_expr +from posthog.hogql_queries.insights.funnels.base import FunnelBase + + +class FunnelStrict(FunnelBase): + def get_query(self): + max_steps = self.context.max_steps + + breakdown_exprs = self._get_breakdown_prop_expr() + + select: List[ast.Expr] = [ + *self._get_count_columns(max_steps), + *self._get_step_time_avgs(max_steps), + *self._get_step_time_median(max_steps), + *breakdown_exprs, + ] + + return ast.SelectQuery( + select=select, + select_from=ast.JoinExpr(table=self.get_step_counts_query()), + group_by=[ast.Field(chain=["prop"])] if len(breakdown_exprs) > 0 else None, + ) + + def get_step_counts_query(self): + max_steps = self.context.max_steps + breakdown_exprs = self._get_breakdown_prop_expr() + inner_timestamps, outer_timestamps = self._get_timestamp_selects() + person_and_group_properties = self._get_person_and_group_properties() + + group_by_columns: List[ast.Expr] = [ + ast.Field(chain=["aggregation_target"]), + ast.Field(chain=["steps"]), + *breakdown_exprs, + ] + + outer_select: List[ast.Expr] = [ + *group_by_columns, + *self._get_step_time_avgs(max_steps, inner_query=True), + *self._get_step_time_median(max_steps, inner_query=True), + *self._get_matching_event_arrays(max_steps), + *breakdown_exprs, + *outer_timestamps, + *person_and_group_properties, + ] + + max_steps_expr = parse_expr( + f"max(steps) over (PARTITION BY aggregation_target {self._get_breakdown_prop()}) as max_steps" + ) + + inner_select: List[ast.Expr] = [ + *group_by_columns, + max_steps_expr, + *self._get_step_time_names(max_steps), + *self._get_matching_events(max_steps), + *breakdown_exprs, + *inner_timestamps, + *person_and_group_properties, + ] + + return ast.SelectQuery( + select=outer_select, + select_from=ast.JoinExpr( + table=ast.SelectQuery( + select=inner_select, + select_from=ast.JoinExpr(table=self.get_step_counts_without_aggregation_query()), + ) + ), + group_by=group_by_columns, + having=ast.CompareOperation( + left=ast.Field(chain=["steps"]), right=ast.Field(chain=["max_steps"]), op=ast.CompareOperationOp.Eq + ), + ) + + def get_step_counts_without_aggregation_query(self): + max_steps = self.context.max_steps + + select_inner: List[ast.Expr] = [ + ast.Field(chain=["aggregation_target"]), + ast.Field(chain=["timestamp"]), + *self._get_partition_cols(1, max_steps), + *self._get_breakdown_prop_expr(group_remaining=True), + *self._get_person_and_group_properties(), + ] + select_from_inner = self._get_inner_event_query(skip_entity_filter=True, skip_step_filter=True) + inner_query = ast.SelectQuery(select=select_inner, select_from=select_from_inner) + + select: List[ast.Expr] = [ + ast.Field(chain=["*"]), + ast.Alias(alias="steps", expr=self._get_sorting_condition(max_steps, max_steps)), + *self._get_step_times(max_steps), + *self._get_matching_events(max_steps), + *self._get_person_and_group_properties(), + ] + select_from = ast.JoinExpr(table=inner_query) + where = ast.CompareOperation( + left=ast.Field(chain=["step_0"]), right=ast.Constant(value=1), op=ast.CompareOperationOp.Eq + ) + return ast.SelectQuery(select=select, select_from=select_from, where=where) + + def _get_partition_cols(self, level_index: int, max_steps: int): + exprs: List[ast.Expr] = [] + + for i in range(0, max_steps): + exprs.append(ast.Field(chain=[f"step_{i}"])) + + if i < level_index: + exprs.append(ast.Field(chain=[f"latest_{i}"])) + + # for field in self.extra_event_fields_and_properties: + # exprs.append(ast.Field(chain=[f'"{field}_{i}"'])) + + else: + exprs.append( + parse_expr( + f"min(latest_{i}) over (PARTITION by aggregation_target {self._get_breakdown_prop()} ORDER BY timestamp DESC ROWS BETWEEN {i} PRECEDING AND {i} PRECEDING) latest_{i}" + ) + ) + + # for field in self.extra_event_fields_and_properties: + # exprs.append( + # parse_expr( + # f'min("{field}_{i}") over (PARTITION by aggregation_target {self._get_breakdown_prop()} ORDER BY timestamp DESC ROWS BETWEEN {i} PRECEDING AND {i} PRECEDING) "{field}_{i}"' + # ) + # ) + + return exprs diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py new file mode 100644 index 0000000000000..40dfb91ba1f90 --- /dev/null +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py @@ -0,0 +1,620 @@ +# from datetime import datetime +from typing import cast + +from posthog.constants import INSIGHT_FUNNELS +from posthog.hogql_queries.insights.funnels.funnel_strict import FunnelStrict +from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner +from posthog.hogql_queries.insights.funnels.test.conversion_time_cases import ( + funnel_conversion_time_test_factory, +) + +# from posthog.hogql_queries.insights.funnels.test.breakdown_cases import ( +# assert_funnel_results_equal, +# funnel_breakdown_test_factory, +# ) +from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query +from posthog.models.action import Action +from posthog.models.action_step import ActionStep +from posthog.models.filters import Filter +from posthog.models.instance_setting import override_instance_config +from posthog.queries.funnels.funnel_strict_persons import ClickhouseFunnelStrictActors +from posthog.schema import FunnelsQuery +from posthog.test.base import ( + APIBaseTest, + ClickhouseTestMixin, + _create_event, + _create_person, +) + +# from posthog.test.test_journeys import journeys_for + +FORMAT_TIME = "%Y-%m-%d 00:00:00" + + +def _create_action(**kwargs): + team = kwargs.pop("team") + name = kwargs.pop("name") + properties = kwargs.pop("properties", {}) + action = Action.objects.create(team=team, name=name) + ActionStep.objects.create(action=action, event=name, properties=properties) + return action + + +# class TestFunnelStrictStepsBreakdown( +# ClickhouseTestMixin, +# funnel_breakdown_test_factory( # type: ignore +# FunnelStrict, +# ClickhouseFunnelStrictActors, +# _create_event, +# _create_action, +# _create_person, +# ), +# ): +# maxDiff = None + +# def test_basic_funnel_default_funnel_days_breakdown_event(self): +# # TODO: This test and the one below it fail, only for strict funnels. Figure out why and how to fix +# pass + +# def test_basic_funnel_default_funnel_days_breakdown_action(self): +# pass + +# def test_basic_funnel_default_funnel_days_breakdown_action_materialized(self): +# pass + +# def test_strict_breakdown_events_with_multiple_properties(self): +# filters = { +# "events": [{"id": "sign up", "order": 0}, {"id": "play movie", "order": 1}], +# "insight": INSIGHT_FUNNELS, +# "date_from": "2020-01-01", +# "date_to": "2020-01-08", +# "funnel_window_days": 7, +# "breakdown_type": "event", +# "breakdown": "$browser", +# } + +# people = journeys_for( +# { +# "person1": [ +# { +# "event": "sign up", +# "timestamp": datetime(2020, 1, 1, 12), +# "properties": {"$browser": "Chrome"}, +# }, +# { +# "event": "blah", +# "timestamp": datetime(2020, 1, 1, 13), +# "properties": {"$browser": "Chrome"}, +# }, +# { +# "event": "play movie", +# "timestamp": datetime(2020, 1, 1, 14), +# "properties": {"$browser": "Chrome"}, +# }, +# ], +# "person2": [ +# { +# "event": "sign up", +# "timestamp": datetime(2020, 1, 2, 13), +# "properties": {"$browser": "Safari"}, +# }, +# { +# "event": "play movie", +# "timestamp": datetime(2020, 1, 2, 14), +# "properties": {"$browser": "Safari"}, +# }, +# ], +# }, +# self.team, +# ) + +# query = cast(FunnelsQuery, filter_to_query(filters)) +# results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + +# assert_funnel_results_equal( +# results[0], +# [ +# { +# "action_id": "sign up", +# "name": "sign up", +# "custom_name": None, +# "order": 0, +# "people": [], +# "count": 1, +# "type": "events", +# "average_conversion_time": None, +# "median_conversion_time": None, +# "breakdown": ["Chrome"], +# "breakdown_value": ["Chrome"], +# }, +# { +# "action_id": "play movie", +# "name": "play movie", +# "custom_name": None, +# "order": 1, +# "people": [], +# "count": 0, +# "type": "events", +# "average_conversion_time": None, +# "median_conversion_time": None, +# "breakdown": ["Chrome"], +# "breakdown_value": ["Chrome"], +# }, +# ], +# ) +# self.assertCountEqual(self._get_actor_ids_at_step(filters, 1, ["Chrome"]), [people["person1"].uuid]) +# self.assertCountEqual(self._get_actor_ids_at_step(filters, 2, ["Chrome"]), []) + +# assert_funnel_results_equal( +# results[1], +# [ +# { +# "action_id": "sign up", +# "name": "sign up", +# "custom_name": None, +# "order": 0, +# "people": [], +# "count": 1, +# "type": "events", +# "average_conversion_time": None, +# "median_conversion_time": None, +# "breakdown": ["Safari"], +# "breakdown_value": ["Safari"], +# }, +# { +# "action_id": "play movie", +# "name": "play movie", +# "custom_name": None, +# "order": 1, +# "people": [], +# "count": 1, +# "type": "events", +# "average_conversion_time": 3600, +# "median_conversion_time": 3600, +# "breakdown": ["Safari"], +# "breakdown_value": ["Safari"], +# }, +# ], +# ) +# self.assertCountEqual(self._get_actor_ids_at_step(filters, 1, ["Safari"]), [people["person2"].uuid]) +# self.assertCountEqual(self._get_actor_ids_at_step(filters, 2, ["Safari"]), [people["person2"].uuid]) + + +class TestFunnelStrictStepsConversionTime( + ClickhouseTestMixin, + funnel_conversion_time_test_factory( # type: ignore + FunnelStrict, + ClickhouseFunnelStrictActors, + _create_event, + _create_person, + ), +): + maxDiff = None + pass + + +class TestFunnelStrictSteps(ClickhouseTestMixin, APIBaseTest): + maxDiff = None + + def _get_actor_ids_at_step(self, filter, funnel_step, breakdown_value=None): + filter = Filter(data=filter, team=self.team) + person_filter = filter.shallow_clone({"funnel_step": funnel_step, "funnel_step_breakdown": breakdown_value}) + _, serialized_result, _ = ClickhouseFunnelStrictActors(person_filter, self.team).get_actors() + + return [val["id"] for val in serialized_result] + + def test_basic_strict_funnel(self): + filters = { + "insight": INSIGHT_FUNNELS, + "events": [ + {"id": "user signed up", "order": 0}, + {"id": "$pageview", "order": 1}, + {"id": "insight viewed", "order": 2}, + ], + } + + person1_stopped_after_signup = _create_person( + distinct_ids=["stopped_after_signup1"], + team_id=self.team.pk, + properties={"test": "okay"}, + ) + _create_event(team=self.team, event="user signed up", distinct_id="stopped_after_signup1") + + person2_stopped_after_one_pageview = _create_person( + distinct_ids=["stopped_after_pageview1"], team_id=self.team.pk + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_pageview1") + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_pageview1", + ) + + person3_stopped_after_insight_view = _create_person( + distinct_ids=["stopped_after_insightview"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_insightview", + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview") + _create_event(team=self.team, event="blaah blaa", distinct_id="stopped_after_insightview") + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview", + ) + + person4_stopped_after_insight_view_not_strict_order = _create_person( + distinct_ids=["stopped_after_insightview2"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview2", + ) + _create_event(team=self.team, event="blaah blaa", distinct_id="stopped_after_insightview2") + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview2") + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_insightview2", + ) + + person5_stopped_after_insight_view_random = _create_person( + distinct_ids=["stopped_after_insightview3"], team_id=self.team.pk + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview3") + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_insightview3", + ) + _create_event(team=self.team, event="blaah blaa", distinct_id="stopped_after_insightview3") + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview3") + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview3", + ) + + person6 = _create_person(distinct_ids=["person6"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person6") + _create_event(team=self.team, event="user signed up", distinct_id="person6") + _create_event(team=self.team, event="blaah blaa", distinct_id="person6") + _create_event(team=self.team, event="$pageview", distinct_id="person6") + + person7 = _create_person(distinct_ids=["person7"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person7") + _create_event(team=self.team, event="user signed up", distinct_id="person7") + _create_event(team=self.team, event="$pageview", distinct_id="person7") + _create_event(team=self.team, event="insight viewed", distinct_id="person7") + _create_event(team=self.team, event="blaah blaa", distinct_id="person7") + + _create_person(distinct_ids=["stopped_after_insightview6"], team_id=self.team.pk) + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview6", + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview6") + + query = cast(FunnelsQuery, filter_to_query(filters)) + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + + self.assertEqual(results[0]["name"], "user signed up") + self.assertEqual(results[1]["name"], "$pageview") + self.assertEqual(results[2]["name"], "insight viewed") + self.assertEqual(results[0]["count"], 7) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 1), + [ + person1_stopped_after_signup.uuid, + person2_stopped_after_one_pageview.uuid, + person3_stopped_after_insight_view.uuid, + person4_stopped_after_insight_view_not_strict_order.uuid, + person5_stopped_after_insight_view_random.uuid, + person6.uuid, + person7.uuid, + ], + ) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 2), + [person3_stopped_after_insight_view.uuid, person7.uuid], + ) + + self.assertCountEqual(self._get_actor_ids_at_step(filters, 3), [person7.uuid]) + + with override_instance_config("AGGREGATE_BY_DISTINCT_IDS_TEAMS", f"{self.team.pk}"): + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + self.assertEqual(results[0]["name"], "user signed up") + self.assertEqual(results[1]["name"], "$pageview") + self.assertEqual(results[2]["name"], "insight viewed") + self.assertEqual(results[0]["count"], 7) + + def test_advanced_strict_funnel(self): + sign_up_action = _create_action( + name="sign up", + team=self.team, + properties=[{"key": "key", "type": "event", "value": ["val"], "operator": "exact"}], + ) + + view_action = _create_action( + name="pageview", + team=self.team, + properties=[{"key": "key", "type": "event", "value": ["val"], "operator": "exact"}], + ) + + filters = { + "events": [ + {"id": "user signed up", "type": "events", "order": 0}, + {"id": "$pageview", "type": "events", "order": 2}, + ], + "actions": [ + {"id": sign_up_action.id, "math": "dau", "order": 1}, + {"id": view_action.id, "math": "weekly_active", "order": 3}, + ], + "insight": INSIGHT_FUNNELS, + } + + person1_stopped_after_signup = _create_person(distinct_ids=["stopped_after_signup1"], team_id=self.team.pk) + _create_event(team=self.team, event="user signed up", distinct_id="stopped_after_signup1") + + person2_stopped_after_one_pageview = _create_person( + distinct_ids=["stopped_after_pageview1"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_pageview1", + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_pageview1") + + person3_stopped_after_insight_view = _create_person( + distinct_ids=["stopped_after_insightview"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_insightview", + ) + _create_event( + team=self.team, + event="sign up", + distinct_id="stopped_after_insightview", + properties={"key": "val"}, + ) + _create_event( + team=self.team, + event="sign up", + distinct_id="stopped_after_insightview", + properties={"key": "val2"}, + ) + _create_event(team=self.team, event="$pageview", distinct_id="stopped_after_insightview") + _create_event(team=self.team, event="blaah blaa", distinct_id="stopped_after_insightview") + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview", + ) + + person4 = _create_person(distinct_ids=["person4"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person4") + _create_event(team=self.team, event="user signed up", distinct_id="person4") + _create_event( + team=self.team, + event="sign up", + distinct_id="person4", + properties={"key": "val"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="person4", + properties={"key": "val"}, + ) + _create_event(team=self.team, event="blaah blaa", distinct_id="person4") + + person5 = _create_person(distinct_ids=["person5"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person5") + _create_event(team=self.team, event="user signed up", distinct_id="person5") + _create_event( + team=self.team, + event="sign up", + distinct_id="person5", + properties={"key": "val"}, + ) + _create_event(team=self.team, event="$pageview", distinct_id="person5") + _create_event(team=self.team, event="blaah blaa", distinct_id="person5") + + person6 = _create_person(distinct_ids=["person6"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person6") + _create_event(team=self.team, event="user signed up", distinct_id="person6") + _create_event( + team=self.team, + event="sign up", + distinct_id="person6", + properties={"key": "val"}, + ) + _create_event(team=self.team, event="$pageview", distinct_id="person6") + _create_event( + team=self.team, + event="pageview", + distinct_id="person6", + properties={"key": "val1"}, + ) + + person7 = _create_person(distinct_ids=["person7"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person7") + _create_event(team=self.team, event="user signed up", distinct_id="person7") + _create_event( + team=self.team, + event="sign up", + distinct_id="person7", + properties={"key": "val"}, + ) + _create_event(team=self.team, event="$pageview", distinct_id="person7") + _create_event(team=self.team, event="user signed up", distinct_id="person7") + _create_event( + team=self.team, + event="pageview", + distinct_id="person7", + properties={"key": "val"}, + ) + + person8 = _create_person(distinct_ids=["person8"], team_id=self.team.pk) + _create_event(team=self.team, event="blaah blaa", distinct_id="person8") + _create_event(team=self.team, event="user signed up", distinct_id="person8") + _create_event(team=self.team, event="user signed up", distinct_id="person8") + _create_event( + team=self.team, + event="sign up", + distinct_id="person8", + properties={"key": "val"}, + ) + _create_event(team=self.team, event="$pageview", distinct_id="person8") + _create_event( + team=self.team, + event="pageview", + distinct_id="person8", + properties={"key": "val"}, + ) + + query = cast(FunnelsQuery, filter_to_query(filters)) + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + + self.assertEqual(results[0]["name"], "user signed up") + self.assertEqual(results[1]["name"], "sign up") + self.assertEqual(results[2]["name"], "$pageview") + self.assertEqual(results[3]["name"], "pageview") + self.assertEqual(results[0]["count"], 8) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 1), + [ + person1_stopped_after_signup.uuid, + person2_stopped_after_one_pageview.uuid, + person3_stopped_after_insight_view.uuid, + person4.uuid, + person5.uuid, + person6.uuid, + person7.uuid, + person8.uuid, + ], + ) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 2), + [ + person3_stopped_after_insight_view.uuid, + person4.uuid, + person5.uuid, + person6.uuid, + person7.uuid, + person8.uuid, + ], + ) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 3), + [person4.uuid, person5.uuid, person6.uuid, person7.uuid, person8.uuid], + ) + + self.assertCountEqual(self._get_actor_ids_at_step(filters, 4), [person8.uuid]) + + def test_basic_strict_funnel_conversion_times(self): + filters = { + "insight": INSIGHT_FUNNELS, + "events": [ + {"id": "user signed up", "order": 0}, + {"id": "$pageview", "order": 1}, + {"id": "insight viewed", "order": 2}, + ], + "date_from": "2021-05-01 00:00:00", + "date_to": "2021-05-07 23:59:59", + } + + person1_stopped_after_signup = _create_person(distinct_ids=["stopped_after_signup1"], team_id=self.team.pk) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_signup1", + timestamp="2021-05-02 00:00:00", + ) + + person2_stopped_after_one_pageview = _create_person( + distinct_ids=["stopped_after_pageview1"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_pageview1", + timestamp="2021-05-02 00:00:00", + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="stopped_after_pageview1", + timestamp="2021-05-02 01:00:00", + ) + + person3_stopped_after_insight_view = _create_person( + distinct_ids=["stopped_after_insightview"], team_id=self.team.pk + ) + _create_event( + team=self.team, + event="user signed up", + distinct_id="stopped_after_insightview", + timestamp="2021-05-02 00:00:00", + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="stopped_after_insightview", + timestamp="2021-05-02 02:00:00", + ) + _create_event( + team=self.team, + event="insight viewed", + distinct_id="stopped_after_insightview", + timestamp="2021-05-02 04:00:00", + ) + + query = cast(FunnelsQuery, filter_to_query(filters)) + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + + self.assertEqual(results[0]["name"], "user signed up") + self.assertEqual(results[1]["name"], "$pageview") + self.assertEqual(results[2]["name"], "insight viewed") + self.assertEqual(results[0]["count"], 3) + + self.assertEqual(results[1]["average_conversion_time"], 5400) + # 1 hour for Person 2, 2 hours for Person 3, average = 1.5 hours + + self.assertEqual(results[2]["average_conversion_time"], 7200) + # 2 hours for Person 3 + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 1), + [ + person1_stopped_after_signup.uuid, + person2_stopped_after_one_pageview.uuid, + person3_stopped_after_insight_view.uuid, + ], + ) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 2), + [ + person2_stopped_after_one_pageview.uuid, + person3_stopped_after_insight_view.uuid, + ], + ) + + self.assertCountEqual( + self._get_actor_ids_at_step(filters, 3), + [person3_stopped_after_insight_view.uuid], + ) diff --git a/posthog/hogql_queries/insights/funnels/utils.py b/posthog/hogql_queries/insights/funnels/utils.py index bd69ce6aefa8a..d5e21e219309f 100644 --- a/posthog/hogql_queries/insights/funnels/utils.py +++ b/posthog/hogql_queries/insights/funnels/utils.py @@ -6,7 +6,7 @@ def get_funnel_order_class(funnelsFilter: FunnelsFilter): from posthog.hogql_queries.insights.funnels import ( Funnel, - # FunnelStrict, + FunnelStrict, # FunnelUnordered, FunnelBase, ) @@ -15,8 +15,7 @@ def get_funnel_order_class(funnelsFilter: FunnelsFilter): return FunnelBase # return FunnelUnordered elif funnelsFilter.funnelOrderType == StepOrderValue.strict: - return FunnelBase - # return FunnelStrict + return FunnelStrict return Funnel From 683a4c553cbeef05307a6325bf9654bc6c03a33d Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 8 Feb 2024 16:49:43 +0000 Subject: [PATCH 2/6] Update query snapshots --- .../funnels/test/__snapshots__/test_funnel.ambr | 4 ++-- .../test_lifecycle_query_runner.ambr | 2 +- .../trends/test/__snapshots__/test_trends.ambr | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr index 33bc454ab2c28..14d50251dbeca 100644 --- a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr +++ b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr @@ -350,7 +350,7 @@ if(and(equals(e.event, 'user signed up'), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 4)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 2)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0)), 1, 0) AS step_0, if(ifNull(equals(step_0, 1), 0), timestamp, NULL) AS latest_0, @@ -871,7 +871,7 @@ if(and(equals(e.event, 'user signed up'), ifNull(in(e__pdi.person_id, (SELECT person_static_cohort.person_id AS person_id FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 2), equals(person_static_cohort.cohort_id, 5)))), 0)), 1, 0) AS step_0, + WHERE and(equals(person_static_cohort.team_id, 2), equals(person_static_cohort.cohort_id, 3)))), 0)), 1, 0) AS step_0, if(ifNull(equals(step_0, 1), 0), timestamp, NULL) AS latest_0, if(equals(e.event, 'paid'), 1, 0) AS step_1, if(ifNull(equals(step_1, 1), 0), timestamp, NULL) AS latest_1 diff --git a/posthog/hogql_queries/insights/test/__snapshots__/test_lifecycle_query_runner.ambr b/posthog/hogql_queries/insights/test/__snapshots__/test_lifecycle_query_runner.ambr index bbca1ba255e1b..ef3b23794866d 100644 --- a/posthog/hogql_queries/insights/test/__snapshots__/test_lifecycle_query_runner.ambr +++ b/posthog/hogql_queries/insights/test/__snapshots__/test_lifecycle_query_runner.ambr @@ -79,7 +79,7 @@ WHERE and(equals(events.team_id, 2), greaterOrEquals(toTimeZone(events.timestamp, 'UTC'), minus(toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-12 00:00:00', 6, 'UTC'))), toIntervalDay(1))), less(toTimeZone(events.timestamp, 'UTC'), plus(toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-19 23:59:59', 6, 'UTC'))), toIntervalDay(1))), ifNull(in(person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 6)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 4)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0), equals(events.event, '$pageview')) GROUP BY person_id) diff --git a/posthog/hogql_queries/insights/trends/test/__snapshots__/test_trends.ambr b/posthog/hogql_queries/insights/trends/test/__snapshots__/test_trends.ambr index e0e2725b16894..d9e0cd6ed6abf 100644 --- a/posthog/hogql_queries/insights/trends/test/__snapshots__/test_trends.ambr +++ b/posthog/hogql_queries/insights/trends/test/__snapshots__/test_trends.ambr @@ -85,7 +85,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-01 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-07 23:59:59', 6, 'UTC'))), ifNull(equals(e__pdi__person.`properties___$bool_prop`, 'x'), 0), and(equals(e.event, 'sign up'), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 7)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 5)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0))) GROUP BY day_start) @@ -172,7 +172,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-01 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-07 23:59:59', 6, 'UTC'))), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, '$bool_prop'), ''), 'null'), '^"|"$', ''), 'x'), 0), and(equals(e.event, 'sign up'), ifNull(in(ifNull(nullIf(e__override.override_person_id, '00000000-0000-0000-0000-000000000000'), e.person_id), (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 8)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 6)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0))) GROUP BY day_start) @@ -688,7 +688,7 @@ WHERE and(equals(e.team_id, 2), and(equals(e.event, '$pageview'), and(or(ifNull(equals(e__pdi__person.properties___name, 'p1'), 0), ifNull(equals(e__pdi__person.properties___name, 'p2'), 0), ifNull(equals(e__pdi__person.properties___name, 'p3'), 0)), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 27)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 25)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0)))) GROUP BY value @@ -757,7 +757,7 @@ WHERE and(equals(e.team_id, 2), and(and(equals(e.event, '$pageview'), and(or(ifNull(equals(e__pdi__person.properties___name, 'p1'), 0), ifNull(equals(e__pdi__person.properties___name, 'p2'), 0), ifNull(equals(e__pdi__person.properties___name, 'p3'), 0)), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 27)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 25)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0))), or(ifNull(equals(transform(ifNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'key'), ''), 'null'), '^"|"$', ''), '$$_posthog_breakdown_null_$$'), ['$$_posthog_breakdown_other_$$', 'val'], ['$$_posthog_breakdown_other_$$', 'val'], '$$_posthog_breakdown_other_$$'), '$$_posthog_breakdown_other_$$'), 0), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'key'), ''), 'null'), '^"|"$', ''), 'val'), 0))), ifNull(greaterOrEquals(timestamp, minus(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-01 00:00:00', 6, 'UTC')), toIntervalDay(7))), 0), ifNull(lessOrEquals(timestamp, assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-12 23:59:59', 6, 'UTC'))), 0)) GROUP BY timestamp, actor_id, @@ -1592,7 +1592,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2019-12-28 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-04 23:59:59', 6, 'UTC'))), and(equals(e.event, 'sign up'), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 40)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 38)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0))) GROUP BY value @@ -1640,7 +1640,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2019-12-28 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-04 23:59:59', 6, 'UTC'))), and(equals(e.event, 'sign up'), ifNull(in(e__pdi.person_id, (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 40)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 38)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0)), or(ifNull(equals(transform(ifNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), '$$_posthog_breakdown_null_$$'), ['$$_posthog_breakdown_other_$$', 'value', 'other_value'], ['$$_posthog_breakdown_other_$$', 'value', 'other_value'], '$$_posthog_breakdown_other_$$'), '$$_posthog_breakdown_other_$$'), 0), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), 'value'), 0), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), 'other_value'), 0))) GROUP BY day_start, @@ -1691,7 +1691,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2019-12-28 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-04 23:59:59', 6, 'UTC'))), and(equals(e.event, 'sign up'), ifNull(in(ifNull(nullIf(e__override.override_person_id, '00000000-0000-0000-0000-000000000000'), e.person_id), (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 41)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 39)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0))) GROUP BY value @@ -1738,7 +1738,7 @@ WHERE and(equals(e.team_id, 2), greaterOrEquals(toTimeZone(e.timestamp, 'UTC'), toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2019-12-28 00:00:00', 6, 'UTC')))), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-04 23:59:59', 6, 'UTC'))), and(equals(e.event, 'sign up'), ifNull(in(ifNull(nullIf(e__override.override_person_id, '00000000-0000-0000-0000-000000000000'), e.person_id), (SELECT cohortpeople.person_id AS person_id FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 41)) + WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 39)) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0)), or(ifNull(equals(transform(ifNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), '$$_posthog_breakdown_null_$$'), ['$$_posthog_breakdown_other_$$', 'value', 'other_value'], ['$$_posthog_breakdown_other_$$', 'value', 'other_value'], '$$_posthog_breakdown_other_$$'), '$$_posthog_breakdown_other_$$'), 0), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), 'value'), 0), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, '$some_property'), ''), 'null'), '^"|"$', ''), 'other_value'), 0))) GROUP BY day_start, From 6345719e2ff32d3f89895de463e5ce3faa1ce6fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Thu, 8 Feb 2024 18:01:19 +0100 Subject: [PATCH 3/6] comment out breakdowns --- .../insights/funnels/funnel_strict.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/funnel_strict.py b/posthog/hogql_queries/insights/funnels/funnel_strict.py index c80ad7297f0d5..a7e1399493898 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_strict.py +++ b/posthog/hogql_queries/insights/funnels/funnel_strict.py @@ -9,31 +9,31 @@ class FunnelStrict(FunnelBase): def get_query(self): max_steps = self.context.max_steps - breakdown_exprs = self._get_breakdown_prop_expr() + # breakdown_exprs = self._get_breakdown_prop_expr() select: List[ast.Expr] = [ *self._get_count_columns(max_steps), *self._get_step_time_avgs(max_steps), *self._get_step_time_median(max_steps), - *breakdown_exprs, + # *breakdown_exprs, ] return ast.SelectQuery( select=select, select_from=ast.JoinExpr(table=self.get_step_counts_query()), - group_by=[ast.Field(chain=["prop"])] if len(breakdown_exprs) > 0 else None, + # group_by=[ast.Field(chain=["prop"])] if len(breakdown_exprs) > 0 else None, ) def get_step_counts_query(self): max_steps = self.context.max_steps - breakdown_exprs = self._get_breakdown_prop_expr() + # breakdown_exprs = self._get_breakdown_prop_expr() inner_timestamps, outer_timestamps = self._get_timestamp_selects() person_and_group_properties = self._get_person_and_group_properties() group_by_columns: List[ast.Expr] = [ ast.Field(chain=["aggregation_target"]), ast.Field(chain=["steps"]), - *breakdown_exprs, + # *breakdown_exprs, ] outer_select: List[ast.Expr] = [ @@ -41,7 +41,7 @@ def get_step_counts_query(self): *self._get_step_time_avgs(max_steps, inner_query=True), *self._get_step_time_median(max_steps, inner_query=True), *self._get_matching_event_arrays(max_steps), - *breakdown_exprs, + # *breakdown_exprs, *outer_timestamps, *person_and_group_properties, ] @@ -55,7 +55,7 @@ def get_step_counts_query(self): max_steps_expr, *self._get_step_time_names(max_steps), *self._get_matching_events(max_steps), - *breakdown_exprs, + # *breakdown_exprs, *inner_timestamps, *person_and_group_properties, ] @@ -81,7 +81,7 @@ def get_step_counts_without_aggregation_query(self): ast.Field(chain=["aggregation_target"]), ast.Field(chain=["timestamp"]), *self._get_partition_cols(1, max_steps), - *self._get_breakdown_prop_expr(group_remaining=True), + # *self._get_breakdown_prop_expr(group_remaining=True), *self._get_person_and_group_properties(), ] select_from_inner = self._get_inner_event_query(skip_entity_filter=True, skip_step_filter=True) From ef59b95e93b855d325552b18c75a5f21fcd9e9a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Thu, 8 Feb 2024 18:02:26 +0100 Subject: [PATCH 4/6] wrap select_from in join expr --- posthog/hogql_queries/insights/funnels/funnel_strict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/hogql_queries/insights/funnels/funnel_strict.py b/posthog/hogql_queries/insights/funnels/funnel_strict.py index a7e1399493898..f7bc5b0d34de8 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_strict.py +++ b/posthog/hogql_queries/insights/funnels/funnel_strict.py @@ -85,7 +85,7 @@ def get_step_counts_without_aggregation_query(self): *self._get_person_and_group_properties(), ] select_from_inner = self._get_inner_event_query(skip_entity_filter=True, skip_step_filter=True) - inner_query = ast.SelectQuery(select=select_inner, select_from=select_from_inner) + inner_query = ast.SelectQuery(select=select_inner, select_from=ast.JoinExpr(table=select_from_inner)) select: List[ast.Expr] = [ ast.Field(chain=["*"]), From 1d89b7f26f74af30366b592be4be85ac456e9611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Fri, 9 Feb 2024 00:20:00 +0100 Subject: [PATCH 5/6] set funnel order type in tests --- .../funnels/test/conversion_time_cases.py | 13 ++++++++----- .../insights/funnels/test/test_funnel.py | 5 ++--- .../insights/funnels/test/test_funnel_strict.py | 15 ++++++--------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/test/conversion_time_cases.py b/posthog/hogql_queries/insights/funnels/test/conversion_time_cases.py index 59b62cc686ffb..63cf914e84cc8 100644 --- a/posthog/hogql_queries/insights/funnels/test/conversion_time_cases.py +++ b/posthog/hogql_queries/insights/funnels/test/conversion_time_cases.py @@ -1,7 +1,7 @@ from datetime import datetime from typing import cast -from posthog.constants import INSIGHT_FUNNELS +from posthog.constants import INSIGHT_FUNNELS, FunnelOrderType from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query from posthog.models.filters import Filter @@ -10,7 +10,7 @@ from posthog.test.test_journeys import journeys_for -def funnel_conversion_time_test_factory(Funnel, FunnelPerson, _create_event, _create_person): +def funnel_conversion_time_test_factory(funnel_order_type: FunnelOrderType, FunnelPerson): class TestFunnelConversionTime(APIBaseTest): def _get_actor_ids_at_step(self, filter, funnel_step, breakdown_value=None): filter = Filter(data=filter, team=self.team) @@ -21,6 +21,8 @@ def _get_actor_ids_at_step(self, filter, funnel_step, breakdown_value=None): def test_funnel_with_multiple_incomplete_tries(self): filters = { + "insight": INSIGHT_FUNNELS, + "funnel_order_type": funnel_order_type, "events": [ {"id": "user signed up", "type": "events", "order": 0}, {"id": "$pageview", "type": "events", "order": 1}, @@ -29,7 +31,6 @@ def test_funnel_with_multiple_incomplete_tries(self): "funnel_window_days": 1, "date_from": "2021-05-01 00:00:00", "date_to": "2021-05-14 00:00:00", - "insight": INSIGHT_FUNNELS, } people = journeys_for( @@ -76,12 +77,13 @@ def test_funnel_with_multiple_incomplete_tries(self): def test_funnel_step_conversion_times(self): filters = { + "insight": INSIGHT_FUNNELS, + "funnel_order_type": funnel_order_type, "events": [ {"id": "sign up", "order": 0}, {"id": "play movie", "order": 1}, {"id": "buy", "order": 2}, ], - "insight": INSIGHT_FUNNELS, "date_from": "2020-01-01", "date_to": "2020-01-08", "funnel_window_days": 7, @@ -120,11 +122,12 @@ def test_funnel_step_conversion_times(self): def test_funnel_times_with_different_conversion_windows(self): filters = { + "insight": INSIGHT_FUNNELS, + "funnel_order_type": funnel_order_type, "events": [ {"id": "user signed up", "type": "events", "order": 0}, {"id": "pageview", "type": "events", "order": 1}, ], - "insight": INSIGHT_FUNNELS, "funnel_window_interval": 14, "funnel_window_interval_unit": "day", "date_from": "2020-01-01", diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel.py b/posthog/hogql_queries/insights/funnels/test/test_funnel.py index d100cc4e399e9..75fd4a2ce9fbe 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel.py @@ -5,7 +5,7 @@ from freezegun import freeze_time from posthog.api.instance_settings import get_instance_setting from posthog.clickhouse.client.execute import sync_execute -from posthog.constants import INSIGHT_FUNNELS +from posthog.constants import INSIGHT_FUNNELS, FunnelOrderType from posthog.hogql.query import execute_hogql_query from posthog.hogql_queries.insights.funnels.funnel_query_context import FunnelQueryContext from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner @@ -61,8 +61,7 @@ def _create_action(**kwargs): class TestFunnelConversionTime( - ClickhouseTestMixin, - funnel_conversion_time_test_factory(Funnel, ClickhouseFunnelActors, _create_event, _create_person), # type: ignore + ClickhouseTestMixin, funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelActors) ): maxDiff = None pass diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py index 40dfb91ba1f90..3b6c7d266d4e4 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py @@ -1,8 +1,7 @@ # from datetime import datetime from typing import cast -from posthog.constants import INSIGHT_FUNNELS -from posthog.hogql_queries.insights.funnels.funnel_strict import FunnelStrict +from posthog.constants import INSIGHT_FUNNELS, FunnelOrderType from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner from posthog.hogql_queries.insights.funnels.test.conversion_time_cases import ( funnel_conversion_time_test_factory, @@ -182,12 +181,7 @@ def _create_action(**kwargs): class TestFunnelStrictStepsConversionTime( ClickhouseTestMixin, - funnel_conversion_time_test_factory( # type: ignore - FunnelStrict, - ClickhouseFunnelStrictActors, - _create_event, - _create_person, - ), + funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelStrictActors), ): maxDiff = None pass @@ -206,6 +200,7 @@ def _get_actor_ids_at_step(self, filter, funnel_step, breakdown_value=None): def test_basic_strict_funnel(self): filters = { "insight": INSIGHT_FUNNELS, + "funnel_order_type": "strict", "events": [ {"id": "user signed up", "order": 0}, {"id": "$pageview", "order": 1}, @@ -349,6 +344,8 @@ def test_advanced_strict_funnel(self): ) filters = { + "insight": INSIGHT_FUNNELS, + "funnel_order_type": "strict", "events": [ {"id": "user signed up", "type": "events", "order": 0}, {"id": "$pageview", "type": "events", "order": 2}, @@ -357,7 +354,6 @@ def test_advanced_strict_funnel(self): {"id": sign_up_action.id, "math": "dau", "order": 1}, {"id": view_action.id, "math": "weekly_active", "order": 3}, ], - "insight": INSIGHT_FUNNELS, } person1_stopped_after_signup = _create_person(distinct_ids=["stopped_after_signup1"], team_id=self.team.pk) @@ -528,6 +524,7 @@ def test_advanced_strict_funnel(self): def test_basic_strict_funnel_conversion_times(self): filters = { "insight": INSIGHT_FUNNELS, + "funnel_order_type": "strict", "events": [ {"id": "user signed up", "order": 0}, {"id": "$pageview", "order": 1}, From 73ca7e7807dafab20fc8365928d79ad99ef0bac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Fri, 9 Feb 2024 02:12:20 +0100 Subject: [PATCH 6/6] re-add type ignore --- posthog/hogql_queries/insights/funnels/test/test_funnel.py | 3 ++- .../hogql_queries/insights/funnels/test/test_funnel_strict.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel.py b/posthog/hogql_queries/insights/funnels/test/test_funnel.py index 75fd4a2ce9fbe..30b0cfd0df0fb 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel.py @@ -61,7 +61,8 @@ def _create_action(**kwargs): class TestFunnelConversionTime( - ClickhouseTestMixin, funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelActors) + ClickhouseTestMixin, + funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelActors), # type: ignore ): maxDiff = None pass diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py index 3b6c7d266d4e4..b101fe718be61 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel_strict.py @@ -181,7 +181,7 @@ def _create_action(**kwargs): class TestFunnelStrictStepsConversionTime( ClickhouseTestMixin, - funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelStrictActors), + funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelStrictActors), # type: ignore ): maxDiff = None pass