chore: Extract shared functions to parent

Now that all of the queries share a `conversionGoal` parameter, Python is happy with this!
PostHog · Dec 9, 2024 · 67b8765 · 67b8765
1 parent 6948ee9
commit 67b8765
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 121 deletions.
diff --git 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
@@ -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
@@ -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()