diff --git a/ee/clickhouse/queries/funnels/funnel_time_to_convert.py b/ee/clickhouse/queries/funnels/funnel_time_to_convert.py index 4e2a03110b4..9987bddfeee 100644 --- a/ee/clickhouse/queries/funnels/funnel_time_to_convert.py +++ b/ee/clickhouse/queries/funnels/funnel_time_to_convert.py @@ -1,20 +1,16 @@ -from typing import Type - from rest_framework.exceptions import ValidationError from ee.clickhouse.queries.funnels.base import ClickhouseFunnelBase -from ee.clickhouse.queries.funnels.funnel import ClickhouseFunnel +from ee.clickhouse.queries.funnels.utils import get_funnel_order_class from posthog.constants import FUNNEL_TO_STEP from posthog.models.filters.filter import Filter from posthog.models.team import Team class ClickhouseFunnelTimeToConvert(ClickhouseFunnelBase): - def __init__( - self, filter: Filter, team: Team, funnel_order_class: Type[ClickhouseFunnelBase] = ClickhouseFunnel - ) -> None: + def __init__(self, filter: Filter, team: Team) -> None: super().__init__(filter, team) - self.funnel_order = funnel_order_class(filter, team) + self.funnel_order = get_funnel_order_class(filter)(filter, team) def _format_results(self, results: list) -> dict: return { diff --git a/ee/clickhouse/queries/funnels/funnel_trends.py b/ee/clickhouse/queries/funnels/funnel_trends.py index 8a9edf87c16..a6c2131b05a 100644 --- a/ee/clickhouse/queries/funnels/funnel_trends.py +++ b/ee/clickhouse/queries/funnels/funnel_trends.py @@ -1,11 +1,11 @@ from datetime import date, datetime from itertools import groupby -from typing import List, Optional, Tuple, Type, Union, cast +from typing import List, Optional, Tuple, Union, cast from dateutil.relativedelta import relativedelta from ee.clickhouse.queries.funnels.base import ClickhouseFunnelBase -from ee.clickhouse.queries.funnels.funnel import ClickhouseFunnel +from ee.clickhouse.queries.funnels.utils import get_funnel_order_class from ee.clickhouse.queries.util import ( format_ch_timestamp, get_earliest_timestamp, @@ -54,13 +54,11 @@ class ClickhouseFunnelTrends(ClickhouseFunnelBase): If no people have reached step {from_step} in the period, {conversion_rate} is zero. """ - def __init__( - self, filter: Filter, team: Team, funnel_order_class: Type[ClickhouseFunnelBase] = ClickhouseFunnel - ) -> None: + def __init__(self, filter: Filter, team: Team) -> None: super().__init__(filter, team) - self.funnel_order = funnel_order_class(filter, team) + self.funnel_order = get_funnel_order_class(filter)(filter, team) def _exec_query(self): return self._summarize_data(super()._exec_query()) diff --git a/ee/clickhouse/queries/funnels/test/test_funnel_time_to_convert.py b/ee/clickhouse/queries/funnels/test/test_funnel_time_to_convert.py index 74dad02c356..8658f2ca80a 100644 --- a/ee/clickhouse/queries/funnels/test/test_funnel_time_to_convert.py +++ b/ee/clickhouse/queries/funnels/test/test_funnel_time_to_convert.py @@ -2,10 +2,9 @@ import unittest from uuid import uuid4 from ee.clickhouse.models.event import create_event -from ee.clickhouse.queries.funnels import ClickhouseFunnel, ClickhouseFunnelStrict, ClickhouseFunnelUnordered from ee.clickhouse.queries.funnels.funnel_time_to_convert import ClickhouseFunnelTimeToConvert from ee.clickhouse.util import ClickhouseTestMixin -from posthog.constants import INSIGHT_FUNNELS, TRENDS_LINEAR +from posthog.constants import INSIGHT_FUNNELS, TRENDS_LINEAR, FunnelOrderType from posthog.models.filters import Filter from posthog.models.person import Person from posthog.test.base import APIBaseTest @@ -62,7 +61,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() # Autobinned using the minimum time to convert, maximum time to convert, and sample count @@ -118,7 +117,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() # Autobinned using the minimum time to convert, maximum time to convert, and sample count @@ -171,7 +170,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() # 7 bins, autoscaled to work best with minimum time to convert and maximum time to convert at hand @@ -223,7 +222,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() self.assertEqual( @@ -241,7 +240,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): # Let's verify that behavior with steps unspecified is the same as when first and last steps specified funnel_trends_steps_specified = ClickhouseFunnelTimeToConvert( - Filter(data={**filter._data, "funnel_from_step": 0, "funnel_to_step": 2,}), self.team, ClickhouseFunnel + Filter(data={**filter._data, "funnel_from_step": 0, "funnel_to_step": 2,}), self.team ) results_steps_specified = funnel_trends_steps_specified.run() @@ -275,6 +274,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "funnel_from_step": 0, "funnel_to_step": 1, "funnel_window_days": 7, + "funnel_order_type": FunnelOrderType.UNORDERED, "events": [ {"id": "step one", "order": 0}, {"id": "step two", "order": 1}, @@ -283,7 +283,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnelUnordered) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() # Autobinned using the minimum time to convert, maximum time to convert, and sample count @@ -336,6 +336,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "funnel_from_step": 0, "funnel_to_step": 1, "funnel_window_days": 7, + "funnel_order_type": FunnelOrderType.STRICT, "events": [ {"id": "step one", "order": 0}, {"id": "step two", "order": 1}, @@ -344,7 +345,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team, ClickhouseFunnelStrict) + funnel_trends = ClickhouseFunnelTimeToConvert(filter, self.team) results = funnel_trends.run() # Autobinned using the minimum time to convert, maximum time to convert, and sample count diff --git a/ee/clickhouse/queries/funnels/test/test_funnel_trends.py b/ee/clickhouse/queries/funnels/test/test_funnel_trends.py index 3ac34d383e0..bbaf21c75de 100644 --- a/ee/clickhouse/queries/funnels/test/test_funnel_trends.py +++ b/ee/clickhouse/queries/funnels/test/test_funnel_trends.py @@ -1,18 +1,13 @@ from datetime import date, datetime, timedelta -from typing import List, cast -from uuid import uuid4 import pytz from freezegun.api import freeze_time -from ee.clickhouse.models.event import create_event -from ee.clickhouse.queries.actor_base_query import SerializedGroup, SerializedPerson -from ee.clickhouse.queries.funnels import ClickhouseFunnel, ClickhouseFunnelStrict, ClickhouseFunnelUnordered from ee.clickhouse.queries.funnels.funnel_trends import ClickhouseFunnelTrends from ee.clickhouse.queries.funnels.funnel_trends_persons import ClickhouseFunnelTrendsActors from ee.clickhouse.test.test_journeys import journeys_for from ee.clickhouse.util import ClickhouseTestMixin -from posthog.constants import INSIGHT_FUNNELS, TRENDS_LINEAR +from posthog.constants import INSIGHT_FUNNELS, TRENDS_LINEAR, FunnelOrderType from posthog.models.cohort import Cohort from posthog.models.filters import Filter from posthog.models.person import Person @@ -30,9 +25,9 @@ def _create_person(**kwargs): class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): maxDiff = None - def _get_actors_at_step(self, filter, entrance_period_start, drop_off, funnel_class=ClickhouseFunnel): + def _get_actors_at_step(self, filter, entrance_period_start, drop_off): person_filter = filter.with_data({"entrance_period_start": entrance_period_start, "drop_off": drop_off}) - funnel_query_builder = ClickhouseFunnelTrendsActors(person_filter, self.team, funnel_class) + funnel_query_builder = ClickhouseFunnelTrendsActors(person_filter, self.team) _, serialized_result = funnel_query_builder.get_actors() return serialized_result @@ -90,7 +85,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): } ) - funnel_trends = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTrends(filter, self.team) results = funnel_trends._exec_query() formatted_results = funnel_trends._format_results(results) @@ -115,7 +110,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - funnel_trends = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTrends(filter, self.team) results = funnel_trends._exec_query() self.assertEqual( @@ -220,7 +215,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 145) def test_day_interval(self): @@ -251,7 +246,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): self.team, ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(7, len(results)) persons = self._get_actors_at_step(filter, "2021-05-01 00:00:00", False) @@ -288,7 +283,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): self.team, ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() persons = self._get_actors_at_step(filter, "2021-04-25 00:00:00", False) self.assertEqual(2, len(results)) @@ -323,7 +318,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): self.team, ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual( results, [ @@ -413,7 +408,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ) with freeze_time("2021-05-20T13:01:01Z"): - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(20, len(results)) persons = self._get_actors_at_step(filter, "2021-05-01 00:00:00", False) @@ -440,7 +435,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() saturday = results[0] # 5/1 self.assertEqual(3, saturday["reached_to_step_count"]) @@ -502,7 +497,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() saturday = results[0] # 5/1 self.assertEqual(1, saturday["reached_to_step_count"]) @@ -575,7 +570,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 2) @@ -630,7 +625,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 1) @@ -667,7 +662,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 1) @@ -716,7 +711,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 4) @@ -811,7 +806,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 2) @@ -870,7 +865,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 2) @@ -918,6 +913,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "date_from": "2021-05-01 00:00:00", "date_to": "2021-05-04 23:59:59", "funnel_window_days": 1, + "funnel_order_type": FunnelOrderType.UNORDERED, "events": [ {"id": "step one", "order": 0}, {"id": "step two", "order": 1}, @@ -925,7 +921,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnelUnordered)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 4) @@ -955,7 +951,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): # 1 user who dropped off starting # 2021-05-04 funnel_trends_persons_existent_dropped_off_results = self._get_actors_at_step( - filter, "2021-05-04 00:00:00", True, ClickhouseFunnelUnordered + filter, "2021-05-04 00:00:00", True ) self.assertEqual( @@ -967,7 +963,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): # 1 user who converted starting # 2021-05-04 funnel_trends_persons_existent_dropped_off_results = self._get_actors_at_step( - filter, "2021-05-04 00:00:00", False, ClickhouseFunnelUnordered + filter, "2021-05-04 00:00:00", False ) self.assertEqual( @@ -1014,6 +1010,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "interval": "day", "date_from": "2021-05-01 00:00:00", "date_to": "2021-05-04 23:59:59", + "funnel_order_type": FunnelOrderType.STRICT, "funnel_window_days": 1, "events": [ {"id": "step one", "order": 0}, @@ -1022,7 +1019,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): ], } ) - results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnelStrict)._exec_query() + results = ClickhouseFunnelTrends(filter, self.team)._exec_query() self.assertEqual(len(results), 4) @@ -1089,7 +1086,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "breakdown": "$browser", } ) - funnel_trends = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTrends(filter, self.team) result = funnel_trends.run() self.assertEqual(len(result), 2) @@ -1150,7 +1147,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "breakdown": "$browser", } ) - funnel_trends = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTrends(filter, self.team) result = funnel_trends.run() self.assertEqual(len(result), 2) @@ -1213,7 +1210,7 @@ class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest): "breakdown": [cohort.pk], } ) - funnel_trends = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel) + funnel_trends = ClickhouseFunnelTrends(filter, self.team) result = funnel_trends.run() self.assertEqual(len(result), 1) diff --git a/ee/clickhouse/queries/funnels/test/test_utils.py b/ee/clickhouse/queries/funnels/test/test_utils.py new file mode 100644 index 00000000000..758d88f1e9c --- /dev/null +++ b/ee/clickhouse/queries/funnels/test/test_utils.py @@ -0,0 +1,23 @@ +from ee.clickhouse.queries.funnels import ClickhouseFunnel, ClickhouseFunnelStrict, ClickhouseFunnelUnordered +from ee.clickhouse.queries.funnels.utils import get_funnel_order_class +from posthog.constants import FunnelOrderType +from posthog.models.filters import Filter +from posthog.test.base import BaseTest + + +class TestGetFunnelOrderClass(BaseTest): + def test_filter_missing_order(self): + filter = Filter({"foo": "bar"}) + self.assertEqual(get_funnel_order_class(filter), ClickhouseFunnel) + + def test_unordered(self): + filter = Filter({"funnel_order_type": FunnelOrderType.UNORDERED}) + self.assertEqual(get_funnel_order_class(filter), ClickhouseFunnelUnordered) + + def test_strict(self): + filter = Filter({"funnel_order_type": FunnelOrderType.STRICT}) + self.assertEqual(get_funnel_order_class(filter), ClickhouseFunnelStrict) + + def test_ordered(self): + filter = Filter({"funnel_order_type": FunnelOrderType.ORDERED}) + self.assertEqual(get_funnel_order_class(filter), ClickhouseFunnel) diff --git a/ee/clickhouse/queries/funnels/utils.py b/ee/clickhouse/queries/funnels/utils.py new file mode 100644 index 00000000000..829d1f82364 --- /dev/null +++ b/ee/clickhouse/queries/funnels/utils.py @@ -0,0 +1,15 @@ +from typing import Type + +from ee.clickhouse.queries.funnels import ClickhouseFunnelBase +from posthog.constants import FunnelOrderType +from posthog.models.filters import Filter + + +def get_funnel_order_class(filter: Filter) -> Type[ClickhouseFunnelBase]: + from ee.clickhouse.queries.funnels import ClickhouseFunnel, ClickhouseFunnelStrict, ClickhouseFunnelUnordered + + if filter.funnel_order_type == FunnelOrderType.UNORDERED: + return ClickhouseFunnelUnordered + elif filter.funnel_order_type == FunnelOrderType.STRICT: + return ClickhouseFunnelStrict + return ClickhouseFunnel diff --git a/ee/clickhouse/views/insights.py b/ee/clickhouse/views/insights.py index bb05679b6e9..c40b799d364 100644 --- a/ee/clickhouse/views/insights.py +++ b/ee/clickhouse/views/insights.py @@ -1,19 +1,13 @@ import json -from typing import Any, Dict, Type +from typing import Any, Dict from rest_framework.decorators import action from rest_framework.request import Request from rest_framework.response import Response -from ee.clickhouse.queries.funnels import ( - ClickhouseFunnel, - ClickhouseFunnelBase, - ClickhouseFunnelStrict, - ClickhouseFunnelTimeToConvert, - ClickhouseFunnelTrends, - ClickhouseFunnelUnordered, -) +from ee.clickhouse.queries.funnels import ClickhouseFunnelTimeToConvert, ClickhouseFunnelTrends from ee.clickhouse.queries.funnels.funnel_correlation import FunnelCorrelation +from ee.clickhouse.queries.funnels.utils import get_funnel_order_class from ee.clickhouse.queries.paths import ClickhousePaths from ee.clickhouse.queries.retention.clickhouse_retention import ClickhouseRetention from ee.clickhouse.queries.stickiness.clickhouse_stickiness import ClickhouseStickiness @@ -26,7 +20,6 @@ from posthog.constants import ( INSIGHT_STICKINESS, PATHS_INCLUDE_EVENT_TYPES, TRENDS_STICKINESS, - FunnelOrderType, FunnelVizType, ) from posthog.decorators import cached_function @@ -78,23 +71,12 @@ class ClickhouseInsightsViewSet(InsightViewSet): team = self.team filter = Filter(request=request, data={"insight": INSIGHT_FUNNELS}, team=self.team) - funnel_order_class: Type[ClickhouseFunnelBase] = ClickhouseFunnel - if filter.funnel_order_type == FunnelOrderType.UNORDERED: - funnel_order_class = ClickhouseFunnelUnordered - elif filter.funnel_order_type == FunnelOrderType.STRICT: - funnel_order_class = ClickhouseFunnelStrict - if filter.funnel_viz_type == FunnelVizType.TRENDS: - return { - "result": ClickhouseFunnelTrends(team=team, filter=filter, funnel_order_class=funnel_order_class).run() - } + return {"result": ClickhouseFunnelTrends(team=team, filter=filter).run()} elif filter.funnel_viz_type == FunnelVizType.TIME_TO_CONVERT: - return { - "result": ClickhouseFunnelTimeToConvert( - team=team, filter=filter, funnel_order_class=funnel_order_class - ).run() - } + return {"result": ClickhouseFunnelTimeToConvert(team=team, filter=filter).run()} else: + funnel_order_class = get_funnel_order_class(filter) return {"result": funnel_order_class(team=team, filter=filter).run()} # ****************************************** diff --git a/ee/clickhouse/views/test/funnel/test_clickhouse_funnel_trends_person.py b/ee/clickhouse/views/test/funnel/test_clickhouse_funnel_trends_person.py index 85d4a6b4d5d..80943a56936 100644 --- a/ee/clickhouse/views/test/funnel/test_clickhouse_funnel_trends_person.py +++ b/ee/clickhouse/views/test/funnel/test_clickhouse_funnel_trends_person.py @@ -6,7 +6,7 @@ from rest_framework import status from ee.clickhouse.models.event import create_event from ee.clickhouse.util import ClickhouseTestMixin -from posthog.constants import INSIGHT_FUNNELS, FunnelVizType +from posthog.constants import INSIGHT_FUNNELS, FunnelOrderType, FunnelVizType from posthog.models.person import Person from posthog.test.base import APIBaseTest @@ -71,3 +71,99 @@ class TestFunnelTrendsPerson(ClickhouseTestMixin, APIBaseTest): self.assertEqual(response_3.status_code, status.HTTP_200_OK) self.assertEqual([person["id"] for person in response_3_data["results"][0]["people"]], []) + + def test_strict_order(self): + user_a = _create_person(distinct_ids=["user a"], team=self.team) + user_b = _create_person(distinct_ids=["user b"], team=self.team) + + _create_event(event="step one", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:00") + _create_event(event="step two", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:01") + _create_event(event="step one", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:02") + _create_event(event="step three", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:03") + + _create_event(event="step one", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:00") + _create_event(event="step two", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:01") + _create_event(event="step three", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:03") + + common_request_data = { + "insight": INSIGHT_FUNNELS, + "funnel_viz_type": FunnelVizType.TRENDS, + "interval": "day", + "date_from": "2021-06-07", + "date_to": "2021-06-13 23:59:59", + "funnel_window_days": 7, + "funnel_order_type": FunnelOrderType.STRICT, + "events": json.dumps( + [{"id": "step one", "order": 0}, {"id": "step two", "order": 1}, {"id": "step three", "order": 2},] + ), + "properties": json.dumps([]), + "funnel_window_days": 7, + "new_entity": json.dumps([]), + } + + # 1 user who dropped off + response_1 = self.client.get( + "/api/person/funnel/", + data={**common_request_data, "entrance_period_start": "2021-06-07", "drop_off": True,}, + ) + response_1_data = response_1.json() + + self.assertEqual(response_1.status_code, status.HTTP_200_OK) + self.assertEqual([person["id"] for person in response_1_data["results"][0]["people"]], [str(user_a.uuid)]) + + # 1 user who successfully converted + response_1 = self.client.get( + "/api/person/funnel/", + data={**common_request_data, "entrance_period_start": "2021-06-07", "drop_off": False,}, + ) + response_1_data = response_1.json() + + self.assertEqual(response_1.status_code, status.HTTP_200_OK) + self.assertEqual([person["id"] for person in response_1_data["results"][0]["people"]], [str(user_b.uuid)]) + + def test_unordered(self): + user_a = _create_person(distinct_ids=["user a"], team=self.team) + user_b = _create_person(distinct_ids=["user b"], team=self.team) + + _create_event(event="step one", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:00") + _create_event(event="step three", distinct_id="user a", team=self.team, timestamp="2021-06-07 19:00:03") + + _create_event(event="step one", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:00") + _create_event(event="step three", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:01") + _create_event(event="step two", distinct_id="user b", team=self.team, timestamp="2021-06-07 19:00:02") + + common_request_data = { + "insight": INSIGHT_FUNNELS, + "funnel_viz_type": FunnelVizType.TRENDS, + "interval": "day", + "date_from": "2021-06-07", + "date_to": "2021-06-13 23:59:59", + "funnel_window_days": 7, + "funnel_order_type": FunnelOrderType.UNORDERED, + "events": json.dumps( + [{"id": "step one", "order": 0}, {"id": "step two", "order": 1}, {"id": "step three", "order": 2},] + ), + "properties": json.dumps([]), + "funnel_window_days": 7, + "new_entity": json.dumps([]), + } + + # 1 user who dropped off + response_1 = self.client.get( + "/api/person/funnel/", + data={**common_request_data, "entrance_period_start": "2021-06-07", "drop_off": True,}, + ) + response_1_data = response_1.json() + + self.assertEqual(response_1.status_code, status.HTTP_200_OK) + self.assertEqual([person["id"] for person in response_1_data["results"][0]["people"]], [str(user_a.uuid)]) + + # 1 user who successfully converted + response_1 = self.client.get( + "/api/person/funnel/", + data={**common_request_data, "entrance_period_start": "2021-06-07", "drop_off": False,}, + ) + response_1_data = response_1.json() + + self.assertEqual(response_1.status_code, status.HTTP_200_OK) + self.assertEqual([person["id"] for person in response_1_data["results"][0]["people"]], [str(user_b.uuid)]) diff --git a/posthog/tasks/test/test_update_cache.py b/posthog/tasks/test/test_update_cache.py index bb243e389e7..67ebd70885a 100644 --- a/posthog/tasks/test/test_update_cache.py +++ b/posthog/tasks/test/test_update_cache.py @@ -120,11 +120,11 @@ class TestUpdateCache(APIBaseTest): self.assertEqual(updated_dashboard_item.last_refresh, now()) @freeze_time("2012-01-15") - @patch("posthog.tasks.update_cache.ClickhouseFunnelUnordered", create=True) - @patch("posthog.tasks.update_cache.ClickhouseFunnelStrict", create=True) + @patch("ee.clickhouse.queries.funnels.ClickhouseFunnelUnordered", create=True) + @patch("ee.clickhouse.queries.funnels.ClickhouseFunnelStrict", create=True) @patch("posthog.tasks.update_cache.ClickhouseFunnelTimeToConvert", create=True) @patch("posthog.tasks.update_cache.ClickhouseFunnelTrends", create=True) - @patch("posthog.tasks.update_cache.ClickhouseFunnel", create=True) + @patch("ee.clickhouse.queries.funnels.ClickhouseFunnel", create=True) def test_update_cache_item_calls_right_funnel_class_clickhouse( self, funnel_mock: MagicMock, @@ -162,25 +162,9 @@ class TestUpdateCache(APIBaseTest): CacheType.FUNNEL, {"filter": filter.toJSON(), "team_id": self.team.pk,}, ) - funnel_trends_mock.assert_called_once() - self.assertEqual(funnel_trends_mock.call_args[1]["funnel_order_class"], funnel_mock) - funnel_trends_mock.reset_mock() - # trends unordered funnel - filter = base_filter.with_data({"funnel_viz_type": "trends", "funnel_order_type": "unordered"}) - funnel_trends_mock.return_value.run.return_value = {} - update_cache_item( - generate_cache_key("{}_{}".format(filter.toJSON(), self.team.pk)), - CacheType.FUNNEL, - {"filter": filter.toJSON(), "team_id": self.team.pk,}, - ) - - funnel_trends_mock.assert_called_once() - self.assertEqual(funnel_trends_mock.call_args[1]["funnel_order_class"], funnel_unordered_mock) - funnel_trends_mock.reset_mock() - - # time to convert strict funnel + # time to convert funnel filter = base_filter.with_data({"funnel_viz_type": "time_to_convert", "funnel_order_type": "strict"}) funnel_time_to_convert_mock.return_value.run.return_value = {} update_cache_item( @@ -188,10 +172,7 @@ class TestUpdateCache(APIBaseTest): CacheType.FUNNEL, {"filter": filter.toJSON(), "team_id": self.team.pk,}, ) - funnel_time_to_convert_mock.assert_called_once() - self.assertEqual(funnel_time_to_convert_mock.call_args[1]["funnel_order_class"], funnel_strict_mock) - funnel_time_to_convert_mock.reset_mock() # strict funnel filter = base_filter.with_data({"funnel_order_type": "strict"}) @@ -201,9 +182,18 @@ class TestUpdateCache(APIBaseTest): CacheType.FUNNEL, {"filter": filter.toJSON(), "team_id": self.team.pk,}, ) - funnel_strict_mock.assert_called_once() + # unordered funnel + filter = base_filter.with_data({"funnel_order_type": "unordered"}) + funnel_unordered_mock.return_value.run.return_value = {} + update_cache_item( + generate_cache_key("{}_{}".format(filter.toJSON(), self.team.pk)), + CacheType.FUNNEL, + {"filter": filter.toJSON(), "team_id": self.team.pk,}, + ) + funnel_unordered_mock.assert_called_once() + def _test_refresh_dashboard_cache_types( self, filter: FilterType, cache_type: CacheType, patch_update_cache_item: MagicMock, ) -> None: diff --git a/posthog/tasks/update_cache.py b/posthog/tasks/update_cache.py index 439da401c94..a14c3bd781c 100644 --- a/posthog/tasks/update_cache.py +++ b/posthog/tasks/update_cache.py @@ -8,7 +8,7 @@ from dateutil.relativedelta import relativedelta from django.conf import settings from django.core.cache import cache from django.db.models import Q -from django.db.models.expressions import F, Subquery +from django.db.models.expressions import F from django.utils import timezone from sentry_sdk import capture_exception from statshog.defaults.django import statsd @@ -21,7 +21,6 @@ from posthog.constants import ( INSIGHT_STICKINESS, INSIGHT_TRENDS, TRENDS_STICKINESS, - FunnelOrderType, FunnelVizType, ) from posthog.decorators import CacheType @@ -35,14 +34,8 @@ PARALLEL_INSIGHT_CACHE = int(os.environ.get("PARALLEL_DASHBOARD_ITEM_CACHE", 5)) logger = structlog.get_logger(__name__) -from ee.clickhouse.queries.funnels import ( - ClickhouseFunnel, - ClickhouseFunnelBase, - ClickhouseFunnelStrict, - ClickhouseFunnelTimeToConvert, - ClickhouseFunnelTrends, - ClickhouseFunnelUnordered, -) +from ee.clickhouse.queries.funnels import ClickhouseFunnelTimeToConvert, ClickhouseFunnelTrends +from ee.clickhouse.queries.funnels.utils import get_funnel_order_class from ee.clickhouse.queries.paths import ClickhousePaths from ee.clickhouse.queries.retention.clickhouse_retention import ClickhouseRetention from ee.clickhouse.queries.stickiness.clickhouse_stickiness import ClickhouseStickiness @@ -169,17 +162,12 @@ def _calculate_by_filter(filter: FilterType, key: str, team_id: int, cache_type: def _calculate_funnel(filter: Filter, key: str, team_id: int) -> List[Dict[str, Any]]: team = Team(pk=team_id) - funnel_order_class: Type[ClickhouseFunnelBase] = ClickhouseFunnel - if filter.funnel_order_type == FunnelOrderType.UNORDERED: - funnel_order_class = ClickhouseFunnelUnordered - elif filter.funnel_order_type == FunnelOrderType.STRICT: - funnel_order_class = ClickhouseFunnelStrict - if filter.funnel_viz_type == FunnelVizType.TRENDS: - result = ClickhouseFunnelTrends(team=team, filter=filter, funnel_order_class=funnel_order_class).run() + result = ClickhouseFunnelTrends(team=team, filter=filter).run() elif filter.funnel_viz_type == FunnelVizType.TIME_TO_CONVERT: - result = ClickhouseFunnelTimeToConvert(team=team, filter=filter, funnel_order_class=funnel_order_class).run() + result = ClickhouseFunnelTimeToConvert(team=team, filter=filter).run() else: + funnel_order_class = get_funnel_order_class(filter) result = funnel_order_class(team=team, filter=filter).run() return result