Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Funnel Trends Persons with month/week granularity #5277

Merged
merged 3 commits into from
Jul 22, 2021
Merged

Fix Funnel Trends Persons with month/week granularity #5277

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90e87ba
resolve #5275
neilkakkar Jul 22, 2021
e3b887c
address comment
neilkakkar Jul 22, 2021
5f9d665
rm unused piece
neilkakkar Jul 22, 2021
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
3 changes: 2 additions & 1 deletion ee/clickhouse/queries/funnels/funnel_trends.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from posthog.models.team import Team

TIMESTAMP_FORMAT = "%Y-%m-%d %H:%M:%S"
DATE_FORMAT = "%Y-%m-%d"
macobo marked this conversation as resolved.
Show resolved Hide resolved
HUMAN_READABLE_TIMESTAMP_FORMAT = "%a. %-d %b"


Expand Down Expand Up @@ -78,7 +79,7 @@ def get_step_counts_without_aggregation_query(
FROM (
{steps_per_person_query}
)
{"WHERE entrance_period_start = %(entrance_period_start)s" if specific_entrance_period_start else ""}
{"WHERE toDateTime(entrance_period_start) = %(entrance_period_start)s" if specific_entrance_period_start else ""}
GROUP BY person_id, entrance_period_start"""

def get_query(self) -> str:
Expand Down
103 changes: 66 additions & 37 deletions ee/clickhouse/queries/funnels/test/test_funnel_trends.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ def _create_event(**kwargs):
class TestFunnelTrends(ClickhouseTestMixin, APIBaseTest):
maxDiff = None

def _get_people_at_step(self, filter, entrance_period_start, drop_off, funnel_class=ClickhouseFunnel):
person_filter = filter.with_data({"entrance_period_start": entrance_period_start, "drop_off": drop_off})
return ClickhouseFunnelTrendsPersons(person_filter, self.team, funnel_class).run()

def _create_sample_data(self):
# five people, three steps
_create_person(distinct_ids=["user_one"], team=self.team)
Expand Down Expand Up @@ -177,11 +181,9 @@ def test_only_one_user_reached_one_step(self):
)

# 1 user who dropped off starting 2021-06-07
funnel_trends_persons_existent_dropped_off_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-06-07 00:00:00", "drop_off": True}),
self.team,
ClickhouseFunnel,
).run()
funnel_trends_persons_existent_dropped_off_results, _ = self._get_people_at_step(
filter, "2021-06-07 00:00:00", True
)

self.assertEqual(
len(funnel_trends_persons_existent_dropped_off_results), 1,
Expand All @@ -191,22 +193,18 @@ def test_only_one_user_reached_one_step(self):
)

# No users converted 2021-06-07
funnel_trends_persons_nonexistent_converted_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-06-07 00:00:00", "drop_off": False}),
self.team,
ClickhouseFunnel,
).run()
funnel_trends_persons_nonexistent_converted_results, _ = self._get_people_at_step(
filter, "2021-06-07 00:00:00", False
)

self.assertEqual(
len(funnel_trends_persons_nonexistent_converted_results), 0,
)

# No users dropped off 2021-06-08
funnel_trends_persons_nonexistent_converted_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-06-08 00:00:00", "drop_off": True}),
self.team,
ClickhouseFunnel,
).run()
funnel_trends_persons_nonexistent_converted_results, _ = self._get_people_at_step(
filter, "2021-06-08 00:00:00", True
)

self.assertEqual(
len(funnel_trends_persons_nonexistent_converted_results), 0,
Expand Down Expand Up @@ -248,8 +246,21 @@ def test_day_interval(self):
],
}
)
_create_person(distinct_ids=["user_one"], team=self.team)

# full run
_create_event(event="step one", distinct_id="user_one", team=self.team, timestamp="2021-05-01 00:00:00")
_create_event(event="step two", distinct_id="user_one", team=self.team, timestamp="2021-05-01 01:00:00")
_create_event(event="step three", distinct_id="user_one", team=self.team, timestamp="2021-05-01 02:00:00")

results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query()
self.assertEqual(len(results), 7)
self.assertEqual(7, len(results))

persons, _ = self._get_people_at_step(filter, "2021-05-01 00:00:00", False)

self.assertEqual(
[person["distinct_ids"] for person in persons], [["user_one"]],
)

def test_week_interval(self):
filter = Filter(
Expand All @@ -267,8 +278,21 @@ def test_week_interval(self):
],
}
)

_create_person(distinct_ids=["user_one"], team=self.team)

# full run
_create_event(event="step one", distinct_id="user_one", team=self.team, timestamp="2021-05-01 00:00:00")
_create_event(event="step two", distinct_id="user_one", team=self.team, timestamp="2021-05-01 01:00:00")
_create_event(event="step three", distinct_id="user_one", team=self.team, timestamp="2021-05-01 02:00:00")

results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query()
persons, _ = self._get_people_at_step(filter, "2021-04-25 00:00:00", False)

self.assertEqual(2, len(results))
self.assertEqual(
macobo marked this conversation as resolved.
Show resolved Hide resolved
[person["distinct_ids"] for person in persons], [["user_one"]],
)

def test_month_interval(self):
filter = Filter(
Expand All @@ -286,8 +310,21 @@ def test_month_interval(self):
],
}
)
_create_person(distinct_ids=["user_one"], team=self.team)

# full run
_create_event(event="step one", distinct_id="user_one", team=self.team, timestamp="2021-05-01 00:00:00")
_create_event(event="step two", distinct_id="user_one", team=self.team, timestamp="2021-05-01 01:00:00")
_create_event(event="step three", distinct_id="user_one", team=self.team, timestamp="2021-05-01 02:00:00")

results = ClickhouseFunnelTrends(filter, self.team, ClickhouseFunnel)._exec_query()
self.assertEqual(len(results), 1)
self.assertEqual(1, len(results))

persons, _ = self._get_people_at_step(filter, "2021-05-01 00:00:00", False)

self.assertEqual(
[person["distinct_ids"] for person in persons], [["user_one"]],
)

def test_all_results_for_day_interval(self):
self._create_sample_data()
Expand Down Expand Up @@ -604,11 +641,9 @@ def test_one_person_in_multiple_periods_and_windows(self):
self.assertEqual(day_4["is_period_final"], True)

# 1 user who dropped off starting # 2021-05-04
funnel_trends_persons_existent_dropped_off_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-05-04 00:00:00", "drop_off": True}),
self.team,
ClickhouseFunnel,
).run()
funnel_trends_persons_existent_dropped_off_results, _ = self._get_people_at_step(
filter, "2021-05-04 00:00:00", True
)

self.assertEqual(
len(funnel_trends_persons_existent_dropped_off_results), 1,
Expand All @@ -618,11 +653,9 @@ def test_one_person_in_multiple_periods_and_windows(self):
)

# 1 user who converted starting # 2021-05-04
funnel_trends_persons_existent_dropped_off_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-05-04 00:00:00", "drop_off": False}),
self.team,
ClickhouseFunnel,
).run()
funnel_trends_persons_existent_dropped_off_results, _ = self._get_people_at_step(
filter, "2021-05-04 00:00:00", False
)

self.assertEqual(
len(funnel_trends_persons_existent_dropped_off_results), 1,
Expand Down Expand Up @@ -845,11 +878,9 @@ def test_one_person_in_multiple_periods_and_windows_in_unordered_funnel(self):
self.assertEqual(day_4["is_period_final"], True)

# 1 user who dropped off starting # 2021-05-04
funnel_trends_persons_existent_dropped_off_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-05-04 00:00:00", "drop_off": True}),
self.team,
ClickhouseFunnelUnordered,
).run()
funnel_trends_persons_existent_dropped_off_results, _ = self._get_people_at_step(
filter, "2021-05-04 00:00:00", True, ClickhouseFunnelUnordered
)

self.assertEqual(
len(funnel_trends_persons_existent_dropped_off_results), 1,
Expand All @@ -859,11 +890,9 @@ def test_one_person_in_multiple_periods_and_windows_in_unordered_funnel(self):
)

# 1 user who converted starting # 2021-05-04
funnel_trends_persons_existent_dropped_off_results, _ = ClickhouseFunnelTrendsPersons(
Filter({**filter._data, "entrance_period_start": "2021-05-04 00:00:00", "drop_off": False}),
self.team,
ClickhouseFunnelUnordered,
).run()
funnel_trends_persons_existent_dropped_off_results, _ = self._get_people_at_step(
filter, "2021-05-04 00:00:00", False, ClickhouseFunnelUnordered
)

self.assertEqual(
len(funnel_trends_persons_existent_dropped_off_results), 1,
Expand Down