Skip to content

Commit

Permalink
fix(trends): Fixed trends first time user matching event (#28325)
Browse files Browse the repository at this point in the history
  • Loading branch information
anirudhpillai authored Feb 5, 2025
1 parent 88e6167 commit 583b1c5
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def is_active_users_math(self):
return self.series.math in ["weekly_active", "monthly_active"]

def is_first_time_ever_math(self):
return self.series.math == "first_time_for_user"
return self.series.math in {"first_time_for_user", "first_matching_event_for_user"}

def is_first_matching_event(self):
return self.series.math == "first_matching_event_for_user"
Expand Down Expand Up @@ -465,7 +465,6 @@ def get_first_time_math_query_orchestrator(
events_where_clause: ast.Expr,
sample_value: ast.RatioExpr,
event_name_filter: ast.Expr | None = None,
is_first_matching_event: bool = False,
):
date_placeholders = self.query_date_range.to_placeholders()
date_from = parse_expr(
Expand All @@ -479,6 +478,7 @@ def get_first_time_math_query_orchestrator(

events_query = ast.SelectQuery(select=[])
parent_select = self._first_time_parent_query(events_query)
is_first_matching_event = self.is_first_matching_event()

class QueryOrchestrator:
events_query_builder: FirstTimeForUserEventsQueryAlternator
Expand Down
120 changes: 120 additions & 0 deletions posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -5022,6 +5022,13 @@ def test_trends_aggregation_first_matching_event_for_user(self):
distinct_ids=["p1"],
properties={},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-06T12:00:00Z",
properties={"$browser": "Firefox"},
)
_create_event(
team=self.team,
event="$pageview",
Expand Down Expand Up @@ -5069,3 +5076,116 @@ def test_trends_aggregation_first_matching_event_for_user(self):
assert len(response.results) == 1
assert response.results[0]["count"] == 1
assert response.results[0]["data"] == [0, 0, 1, 0]

def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self):
_create_person(
team=self.team,
distinct_ids=["p1"],
properties={},
)
_create_person(
team=self.team,
distinct_ids=["p2"],
properties={},
)

# these events outside date range should be ignored
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-06T12:00:00Z",
properties={"$browser": "Chrome"},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p2",
timestamp="2020-01-06T12:00:00Z",
properties={"$browser": "Firefox"},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-06T13:00:00Z",
properties={"$browser": "Firefox"},
)

# only this event for p1 should be included
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-08T12:00:00Z",
properties={"$browser": "Chrome"},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-09T12:00:00Z",
properties={"$browser": "Chrome"},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-10T12:00:00Z",
properties={"$browser": "Firefox"},
)

# only this event for p2 should be included
_create_event(
team=self.team,
event="$pageview",
distinct_id="p2",
timestamp="2020-01-10T12:00:00Z",
properties={"$browser": "Firefox"},
)

_create_event(
team=self.team,
event="$pageview",
distinct_id="p1",
timestamp="2020-01-11T12:00:00Z",
properties={"$browser": "Firefox"},
)
_create_event(
team=self.team,
event="$pageview",
distinct_id="p2",
timestamp="2020-01-11T12:00:00Z",
properties={"$browser": "Firefox"},
)

flush_persons_and_events()

response = self._run_trends_query(
"2020-01-08",
"2020-01-11",
IntervalType.DAY,
[
EventsNode(
event="$pageview",
math=BaseMathType.FIRST_MATCHING_EVENT_FOR_USER,
)
],
TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH),
BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"),
)

# one for each breakdown value
assert len(response.results) == 2

# chrome
assert response.results[0]["breakdown_value"] == "Chrome"
assert response.results[0]["count"] == 1
# match on 8th (p1) for first day in time range
assert response.results[0]["data"] == [1, 0, 0, 0]

# firefox
assert response.results[1]["breakdown_value"] == "Firefox"
assert response.results[1]["count"] == 1
# match on 10th (p2) for third day in time range
assert response.results[1]["data"] == [0, 0, 1, 0]
2 changes: 0 additions & 2 deletions posthog/hogql_queries/insights/trends/trends_query_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,11 @@ def _get_events_subquery(
# Just complex series aggregation
elif self._aggregation_operation.requires_query_orchestration() and (
self._aggregation_operation.is_first_time_ever_math()
or self._aggregation_operation.is_first_matching_event()
):
return self._aggregation_operation.get_first_time_math_query_orchestrator(
events_where_clause=events_filter,
sample_value=self._sample_value(),
event_name_filter=self._event_or_action_where_expr(),
is_first_matching_event=self._aggregation_operation.is_first_matching_event(),
).build()
elif self._aggregation_operation.requires_query_orchestration():
return self._aggregation_operation.get_actors_query_orchestrator(
Expand Down
5 changes: 3 additions & 2 deletions posthog/hogql_queries/insights/utils/aggregations.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ def _select_expr(self, date_from: ast.Expr, filters: ast.Expr | None = None, is_
aggregation_filters = date_from if filters is None else ast.And(exprs=[date_from, filters])
min_timestamp_expr = (
ast.Call(name="min", args=[ast.Field(chain=["timestamp"])])
if not is_first_matching_event or filters is None
else ast.Call(name="minIf", args=[ast.Field(chain=["timestamp"]), filters])
if not is_first_matching_event
else ast.Call(name="minIf", args=[ast.Field(chain=["timestamp"]), aggregation_filters])
)

return [
ast.Alias(
alias="min_timestamp",
Expand Down

0 comments on commit 583b1c5

Please sign in to comment.