From 67b876569e5f4917bfae3c0501fa181f38a05562 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 9 Dec 2024 17:37:08 -0300 Subject: [PATCH] chore: Extract shared functions to parent Now that all of the queries share a `conversionGoal` parameter, Python is happy with this! --- .../web_analytics/stats_table.py | 66 +------------------ .../web_analytics_query_runner.py | 56 +++++++++++++++- .../web_analytics/web_overview.py | 56 +--------------- 3 files changed, 57 insertions(+), 121 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 0a0612b572aa7..a85e1a47dc7b3 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -1,4 +1,4 @@ -from typing import Optional, Union +from typing import Union from posthog.hogql import ast from posthog.hogql.constants import LimitContext @@ -9,18 +9,13 @@ get_property_value, get_property_type, get_property_key, - action_to_expr, ) from posthog.hogql_queries.insights.paginators import HogQLHasMorePaginator from posthog.hogql_queries.web_analytics.web_analytics_query_runner import ( WebAnalyticsQueryRunner, map_columns, ) -from posthog.models import Action -from posthog.models.filters.mixins.utils import cached_property from posthog.schema import ( - ActionConversionGoal, - CustomEventConversionGoal, CachedWebStatsTableQueryResponse, WebStatsTableQuery, WebStatsBreakdown, @@ -382,65 +377,6 @@ def _current_period_aggregate(self, function_name, column_name): def _previous_period_aggregate(self, function_name, column_name): return self.period_aggregate(function_name, column_name, self._date_from_previous_period(), self._date_from()) - # Reproduction from `web_analytics/web_overview.py` - # Update in both places - @cached_property - def conversion_goal_expr(self) -> Optional[ast.Expr]: - if isinstance(self.query.conversionGoal, ActionConversionGoal): - action = Action.objects.get(pk=self.query.conversionGoal.actionId, team__project_id=self.team.project_id) - return action_to_expr(action) - elif isinstance(self.query.conversionGoal, CustomEventConversionGoal): - return ast.CompareOperation( - left=ast.Field(chain=["events", "event"]), - op=ast.CompareOperationOp.Eq, - right=ast.Constant(value=self.query.conversionGoal.customEventName), - ) - else: - return None - - # Reproduction from `web_analytics/web_overview.py` - # Update in both places - @cached_property - def conversion_count_expr(self) -> Optional[ast.Expr]: - if self.conversion_goal_expr: - return ast.Call(name="countIf", args=[self.conversion_goal_expr]) - else: - return None - - # Reproduction from `web_analytics/web_overview.py` - # Update in both places - @cached_property - def conversion_person_id_expr(self) -> Optional[ast.Expr]: - if self.conversion_goal_expr: - return ast.Call( - name="any", - args=[ - ast.Call( - name="if", - args=[ - self.conversion_goal_expr, - ast.Field(chain=["events", "person_id"]), - ast.Constant(value=None), - ], - ) - ], - ) - else: - return None - - # Reproduction from `web_analytics/web_overview.py` - # Update in both places - @cached_property - def event_type_expr(self) -> ast.Expr: - pageview_expr = ast.CompareOperation( - op=ast.CompareOperationOp.Eq, left=ast.Field(chain=["event"]), right=ast.Constant(value="$pageview") - ) - - if self.conversion_goal_expr: - return ast.Call(name="or", args=[pageview_expr, self.conversion_goal_expr]) - else: - return pageview_expr - def _event_properties(self) -> ast.Expr: properties = [ p for p in self.query.properties + self._test_account_filters if get_property_type(p) in ["event", "person"] diff --git a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py index b73772ef79a90..016e1e50e8dad 100644 --- a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py +++ b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py @@ -11,12 +11,15 @@ from posthog.caching.insights_api import BASE_MINIMUM_INSIGHT_REFRESH_INTERVAL, REDUCED_MINIMUM_INSIGHT_REFRESH_INTERVAL from posthog.hogql import ast from posthog.hogql.parser import parse_expr, parse_select -from posthog.hogql.property import property_to_expr +from posthog.hogql.property import property_to_expr, action_to_expr from posthog.hogql.query import execute_hogql_query from posthog.hogql_queries.query_runner import QueryRunner from posthog.hogql_queries.utils.query_date_range import QueryDateRange +from posthog.models import Action from posthog.models.filters.mixins.utils import cached_property from posthog.schema import ( + ActionConversionGoal, + CustomEventConversionGoal, EventPropertyFilter, WebOverviewQuery, WebStatsTableQuery, @@ -57,6 +60,57 @@ def property_filters_without_pathname( ) -> list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]]: return [p for p in self.query.properties if p.key != "$pathname"] + @cached_property + def conversion_goal_expr(self) -> Optional[ast.Expr]: + if isinstance(self.query.conversionGoal, ActionConversionGoal): + action = Action.objects.get(pk=self.query.conversionGoal.actionId, team__project_id=self.team.project_id) + return action_to_expr(action) + elif isinstance(self.query.conversionGoal, CustomEventConversionGoal): + return ast.CompareOperation( + left=ast.Field(chain=["events", "event"]), + op=ast.CompareOperationOp.Eq, + right=ast.Constant(value=self.query.conversionGoal.customEventName), + ) + else: + return None + + @cached_property + def conversion_count_expr(self) -> Optional[ast.Expr]: + if self.conversion_goal_expr: + return ast.Call(name="countIf", args=[self.conversion_goal_expr]) + else: + return None + + @cached_property + def conversion_person_id_expr(self) -> Optional[ast.Expr]: + if self.conversion_goal_expr: + return ast.Call( + name="any", + args=[ + ast.Call( + name="if", + args=[ + self.conversion_goal_expr, + ast.Field(chain=["events", "person_id"]), + ast.Constant(value=None), + ], + ) + ], + ) + else: + return None + + @cached_property + def event_type_expr(self) -> ast.Expr: + pageview_expr = ast.CompareOperation( + op=ast.CompareOperationOp.Eq, left=ast.Field(chain=["event"]), right=ast.Constant(value="$pageview") + ) + + if self.conversion_goal_expr: + return ast.Call(name="or", args=[pageview_expr, self.conversion_goal_expr]) + else: + return pageview_expr + def period_aggregate(self, function_name, column_name, start, end, alias=None, params=None): expr = ast.Call( name=function_name + "If", diff --git a/posthog/hogql_queries/web_analytics/web_overview.py b/posthog/hogql_queries/web_analytics/web_overview.py index 2a41455a6ad29..62149a1eb7289 100644 --- a/posthog/hogql_queries/web_analytics/web_overview.py +++ b/posthog/hogql_queries/web_analytics/web_overview.py @@ -5,20 +5,17 @@ from posthog.hogql import ast from posthog.hogql.parser import parse_select -from posthog.hogql.property import property_to_expr, get_property_type, action_to_expr +from posthog.hogql.property import property_to_expr, get_property_type from posthog.hogql.query import execute_hogql_query from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.hogql_queries.web_analytics.web_analytics_query_runner import ( WebAnalyticsQueryRunner, ) -from posthog.models import Action from posthog.models.filters.mixins.utils import cached_property from posthog.schema import ( CachedWebOverviewQueryResponse, WebOverviewQueryResponse, WebOverviewQuery, - ActionConversionGoal, - CustomEventConversionGoal, SessionTableVersion, ) @@ -97,39 +94,6 @@ def session_properties(self) -> ast.Expr: ] return property_to_expr(properties, team=self.team, scope="event") - @cached_property - def conversion_goal_expr(self) -> Optional[ast.Expr]: - if isinstance(self.query.conversionGoal, ActionConversionGoal): - action = Action.objects.get(pk=self.query.conversionGoal.actionId, team__project_id=self.team.project_id) - return action_to_expr(action) - elif isinstance(self.query.conversionGoal, CustomEventConversionGoal): - return ast.CompareOperation( - left=ast.Field(chain=["events", "event"]), - op=ast.CompareOperationOp.Eq, - right=ast.Constant(value=self.query.conversionGoal.customEventName), - ) - else: - return None - - @cached_property - def conversion_person_id_expr(self) -> Optional[ast.Expr]: - if self.conversion_goal_expr: - return ast.Call( - name="any", - args=[ - ast.Call( - name="if", - args=[ - self.conversion_goal_expr, - ast.Field(chain=["events", "person_id"]), - ast.Constant(value=None), - ], - ) - ], - ) - else: - return None - @cached_property def pageview_count_expression(self) -> ast.Expr: if self.conversion_goal_expr: @@ -146,24 +110,6 @@ def pageview_count_expression(self) -> ast.Expr: else: return ast.Call(name="count", args=[]) - @cached_property - def conversion_count_expr(self) -> Optional[ast.Expr]: - if self.conversion_goal_expr: - return ast.Call(name="countIf", args=[self.conversion_goal_expr]) - else: - return None - - @cached_property - def event_type_expr(self) -> ast.Expr: - pageview_expr = ast.CompareOperation( - op=ast.CompareOperationOp.Eq, left=ast.Field(chain=["event"]), right=ast.Constant(value="$pageview") - ) - - if self.conversion_goal_expr and self.conversion_goal_expr != ast.Constant(value=None): - return ast.Call(name="or", args=[pageview_expr, self.conversion_goal_expr]) - else: - return pageview_expr - @cached_property def inner_select(self) -> ast.SelectQuery: start = self.query_date_range.previous_period_date_from_as_hogql()