From 7c30ee365fdc0a5392313e6c102f96768a8f2de3 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 2 Dec 2024 19:08:08 -0300 Subject: [PATCH 01/17] WIP adding support to our queries to display only results related to the currently selected `conversionGoal` --- frontend/src/queries/schema.json | 11 + frontend/src/queries/schema.ts | 2 + .../web-analytics/webAnalyticsLogic.tsx | 8 +- .../web_analytics/stats_table.py | 214 +++++++++++++----- posthog/schema.py | 3 + 5 files changed, 180 insertions(+), 58 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index b4d28dbbf3776..fd5e7c729d302 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -12666,6 +12666,17 @@ "$ref": "#/definitions/CompareFilter", "description": "Compare to date range" }, + "conversionGoal": { + "anyOf": [ + { + "$ref": "#/definitions/WebAnalyticsConversionGoal" + }, + { + "type": "null" + } + ], + "description": "Whether we should be comparing against a specific conversion goal" + }, "dateRange": { "$ref": "#/definitions/InsightDateRange", "description": "Date range for the query" diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index cddaec321af3c..f2000c0351138 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -910,6 +910,8 @@ export interface TrendsQuery extends InsightsQueryBase { breakdownFilter?: BreakdownFilter /** Compare to date range */ compareFilter?: CompareFilter + /** Whether we should be comparing against a specific conversion goal */ + conversionGoal?: WebAnalyticsConversionGoal | null } export type AssistantArrayPropertyFilterOperator = PropertyOperator.Exact | PropertyOperator.IsNot diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index cec7aad2059ee..949e172b18bf3 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -622,6 +622,7 @@ export const webAnalyticsLogic = kea([ }, compareFilter: compareFilter || { compare: false }, filterTestAccounts, + conversionGoal, properties: webAnalyticsFilters, }, hidePersonsModal: true, @@ -662,6 +663,7 @@ export const webAnalyticsLogic = kea([ compareFilter, limit: 10, filterTestAccounts, + conversionGoal, ...(source || {}), }, embedded: false, @@ -862,6 +864,7 @@ export const webAnalyticsLogic = kea([ sampling, limit: 10, filterTestAccounts, + conversionGoal, stripQueryParams: shouldStripQueryParams, }, embedded: false, @@ -1136,6 +1139,7 @@ export const webAnalyticsLogic = kea([ trendsFilter: { display: ChartDisplayType.WorldMap, }, + conversionGoal, filterTestAccounts, properties: webAnalyticsFilters, }, @@ -1239,7 +1243,8 @@ export const webAnalyticsLogic = kea([ ), }, }, - featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOALS] + // Hiding if conversionGoal is set already because values aren't representative + !conversionGoal && featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOALS] ? { kind: 'query', tileId: TileId.GOALS, @@ -1300,6 +1305,7 @@ export const webAnalyticsLogic = kea([ }, } : null, + // TODO: What to do with this one if conversion goal is set? Hide it? Disable it? featureFlags[FEATURE_FLAGS.ERROR_TRACKING] ? { kind: 'error_tracking', diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 1633c4389879d..198eb3c9b40cb 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 Union +from typing import Optional, Union from posthog.hogql import ast from posthog.hogql.constants import LimitContext @@ -9,13 +9,18 @@ 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, @@ -27,6 +32,8 @@ BREAKDOWN_NULL_DISPLAY = "(none)" +# TODO: Extend `conversion_goal` support to queries besides `to_main_query` +# TODO: Add test cases for conversion goal, both action, and event-based ones class WebStatsTableQueryRunner(WebAnalyticsQueryRunner): query: WebStatsTableQuery response: WebStatsTableQueryResponse @@ -57,58 +64,48 @@ def to_query(self) -> ast.SelectQuery: def to_main_query(self) -> ast.SelectQuery: with self.timings.measure("stats_table_query"): - query = parse_select( - """ -WITH - start_timestamp >= {date_from} AND start_timestamp < {date_to} AS current_period_segment, - start_timestamp >= {date_from_previous_period} AND start_timestamp < {date_from} AS previous_period_segment -SELECT - {processed_breakdown_value} AS "context.columns.breakdown_value", - tuple( - uniqIf(filtered_person_id, current_period_segment), - uniqIf(filtered_person_id, previous_period_segment) - ) AS "context.columns.visitors", - tuple( - sumIf(filtered_pageview_count, current_period_segment), - sumIf(filtered_pageview_count, previous_period_segment) - ) AS "context.columns.views" -FROM ( - SELECT - any(person_id) AS filtered_person_id, - count() AS filtered_pageview_count, - {breakdown_value} AS breakdown_value, - min(session.$start_timestamp) as start_timestamp - FROM events - WHERE and( - timestamp >= {date_from_previous_period}, - timestamp < {date_to}, - events.event == '$pageview', - {all_properties}, - {where_breakdown} - ) - GROUP BY events.`$session_id`, breakdown_value -) -GROUP BY "context.columns.breakdown_value" -ORDER BY "context.columns.visitors" DESC, -"context.columns.views" DESC, -"context.columns.breakdown_value" ASC -""", - timings=self.timings, - placeholders={ - "breakdown_value": self._counts_breakdown_value(), - "processed_breakdown_value": self._processed_breakdown_value(), - "where_breakdown": self.where_breakdown(), - "all_properties": self._all_properties(), - "date_from_previous_period": self._date_from_previous_period(), - "date_from": self._date_from(), - "date_to": self._date_to(), - }, - ) - - assert isinstance(query, ast.SelectQuery) + # Base selects, always returns the breakdown value, and the total number of visitors + selects = [ + ast.Alias(alias="context.columns.breakdown_value", expr=self._processed_breakdown_value()), + self._period_comparison_tuple("filtered_person_id", "context.columns.visitors", "uniq"), + ] + + if self.query.conversionGoal is not None: + selects.extend( + [ + self._period_comparison_tuple("conversion_count", "context.columns.total_conversions", "sum"), + self._period_comparison_tuple( + "conversion_person_id", "context.columns.unique_conversions", "uniq" + ), + ] + ) + else: + selects.append( + self._period_comparison_tuple("filtered_pageview_count", "context.columns.views", "sum"), + ) - if self._include_extra_aggregation_value(): - query.select.append(self._extra_aggregation_value()) + if self._include_extra_aggregation_value(): + selects.append(self._extra_aggregation_value()) + + query = ast.SelectQuery( + select=selects, + select_from=ast.JoinExpr(table=self._main_inner_query()), + group_by=[ast.Field(chain=["context.columns.breakdown_value"])], + order_by=[ + ast.OrderExpr(expr=ast.Field(chain=["context.columns.visitors"]), order="DESC"), + ast.OrderExpr( + expr=ast.Field( + chain=[ + "context.columns.views" + if self.query.conversionGoal is None + else "context.columns.total_conversions" + ] + ), + order="DESC", + ), + ast.OrderExpr(expr=ast.Field(chain=["context.columns.breakdown_value"]), order="ASC"), + ], + ) return query @@ -438,6 +435,105 @@ def to_path_bounce_query(self) -> ast.SelectQuery: assert isinstance(query, ast.SelectQuery) return query + def _main_inner_query(self): + query = parse_select( + """ +SELECT + any(person_id) AS filtered_person_id, + count() AS filtered_pageview_count, + {breakdown_value} AS breakdown_value, + min(session.$start_timestamp) as start_timestamp +FROM events +WHERE and( + timestamp >= {date_from}, + timestamp < {date_to}, + events.event == '$pageview', + {all_properties}, + {where_breakdown} +) +GROUP BY events.`$session_id`, breakdown_value +""", + timings=self.timings, + placeholders={ + "breakdown_value": self._counts_breakdown_value(), + "date_from": self._date_from_previous_period(), + "date_to": self._date_to(), + "all_properties": self._all_properties(), + "where_breakdown": self.where_breakdown(), + }, + ) + + assert isinstance(query, ast.SelectQuery) + + if self.conversion_count_expr and self.conversion_person_id_expr: + query.select.append(ast.Alias(alias="conversion_count", expr=self.conversion_count_expr)) + query.select.append(ast.Alias(alias="conversion_person_id", expr=self.conversion_person_id_expr)) + + return query + + def _period_comparison_tuple(self, column, alias, function_name): + return ast.Alias( + alias=alias, + expr=ast.Tuple( + exprs=[ + self._current_period_aggregate(function_name, column), + self._previous_period_aggregate(function_name, column), + ] + ), + ) + + def _current_period_aggregate(self, function_name, column_name): + return self.period_aggregate(function_name, column_name, self._date_from(), self._date_to()) + + 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 + 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"] @@ -496,6 +592,7 @@ def _date_from(self) -> ast.Expr: def _date_from_previous_period(self) -> ast.Expr: return self.query_date_range.previous_period_date_from_as_hogql() + # TODO: Calculate conversion rate def calculate(self): query = self.to_query() response = self.paginator.execute_hogql_query( @@ -513,11 +610,14 @@ def calculate(self): results, { 0: self._join_with_aggregation_value, # breakdown_value - 1: lambda tuple, row: (self._unsample(tuple[0], row), self._unsample(tuple[1], row)), # Views (tuple) - 2: lambda tuple, row: ( + 1: lambda tuple, row: ( # Views (tuple) + self._unsample(tuple[0], row), + self._unsample(tuple[1], row), + ), + 2: lambda tuple, row: ( # Visitors (tuple) self._unsample(tuple[0], row), self._unsample(tuple[1], row), - ), # Visitors (tuple) + ), }, ) @@ -525,9 +625,9 @@ def calculate(self): if self.query.breakdownBy == WebStatsBreakdown.LANGUAGE: # Keep only first 3 columns, we don't need the aggregation value in the frontend - results_mapped = [[column for idx, column in enumerate(row) if idx < 3] for row in results_mapped] + # Remove both the value and the column (used to generate table headers) + results_mapped = [row[:3] for row in results_mapped] - # Remove this before returning it to the frontend columns = ( [column for column in response.columns if column != "context.columns.aggregation_value"] if response.columns is not None diff --git a/posthog/schema.py b/posthog/schema.py index e01ba99563750..11d82df2dcbcd 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -5823,6 +5823,9 @@ class TrendsQuery(BaseModel): aggregation_group_type_index: Optional[int] = Field(default=None, description="Groups aggregation") breakdownFilter: Optional[BreakdownFilter] = Field(default=None, description="Breakdown of the events and actions") compareFilter: Optional[CompareFilter] = Field(default=None, description="Compare to date range") + conversionGoal: Optional[Union[ActionConversionGoal, CustomEventConversionGoal]] = Field( + default=None, description="Whether we should be comparing against a specific conversion goal" + ) dateRange: Optional[InsightDateRange] = Field(default=None, description="Date range for the query") filterTestAccounts: Optional[bool] = Field( default=False, description="Exclude internal and test users by applying the respective filters" From 8ae1f0cbb1273e09ea007601d883bad4b3984e6a Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Tue, 3 Dec 2024 14:40:53 -0300 Subject: [PATCH 02/17] Stop displaying unrelated-to-conversion-goal sections --- .../web-analytics/tiles/WebAnalyticsTile.tsx | 5 + .../web-analytics/webAnalyticsLogic.tsx | 119 +++++++++--------- 2 files changed, 65 insertions(+), 59 deletions(-) diff --git a/frontend/src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx b/frontend/src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx index 63ef78e423bce..cda07bc69ee55 100644 --- a/frontend/src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx +++ b/frontend/src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx @@ -321,6 +321,11 @@ export const webAnalyticsDataTableQueryContext: QueryContext = { render: VariationCell(), align: 'right', }, + unique_conversions: { + title: Unique Conversions, + render: VariationCell(), + align: 'right', + }, conversion_rate: { title: Conversion Rate, render: VariationCell({ isPercentage: true }), diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 949e172b18bf3..58814401a2dde 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -1187,62 +1187,64 @@ export const webAnalyticsLogic = kea([ ], } : null, - { - kind: 'query', - tileId: TileId.RETENTION, - title: 'Retention', - layout: { - colSpanClassName: 'md:col-span-2', - }, - query: { - kind: NodeKind.InsightVizNode, - source: { - kind: NodeKind.RetentionQuery, - properties: webAnalyticsFilters, - dateRange, - filterTestAccounts, - retentionFilter: { - retentionType: RETENTION_FIRST_TIME, - retentionReference: 'total', - totalIntervals: isGreaterThanMd ? 8 : 5, - period: RetentionPeriod.Week, - }, - }, - vizSpecificOptions: { - [InsightType.RETENTION]: { - hideLineGraph: true, - hideSizeColumn: !isGreaterThanMd, - useSmallLayout: !isGreaterThanMd, - }, - }, - embedded: true, - }, - insightProps: createInsightProps(TileId.RETENTION), - canOpenInsight: false, - canOpenModal: true, - docs: { - url: 'https://posthog.com/docs/web-analytics/dashboard#retention', - title: 'Retention', - description: ( - <> -
-

- Retention creates a cohort of unique users who performed any event for the - first time in the last week. It then tracks the percentage of users who - return to perform any event in the following weeks. -

-

- You want the numbers numbers to be the highest possible, suggesting that - people that come to your page continue coming to your page - and performing - an actions. Also, the further down the table the higher the numbers should - be (or at least as high), which would indicate that you're either increasing - or keeping your retention at the same level. -

-
- - ), - }, - }, + !conversionGoal + ? { + kind: 'query', + tileId: TileId.RETENTION, + title: 'Retention', + layout: { + colSpanClassName: 'md:col-span-2', + }, + query: { + kind: NodeKind.InsightVizNode, + source: { + kind: NodeKind.RetentionQuery, + properties: webAnalyticsFilters, + dateRange, + filterTestAccounts, + retentionFilter: { + retentionType: RETENTION_FIRST_TIME, + retentionReference: 'total', + totalIntervals: isGreaterThanMd ? 8 : 5, + period: RetentionPeriod.Week, + }, + }, + vizSpecificOptions: { + [InsightType.RETENTION]: { + hideLineGraph: true, + hideSizeColumn: !isGreaterThanMd, + useSmallLayout: !isGreaterThanMd, + }, + }, + embedded: true, + }, + insightProps: createInsightProps(TileId.RETENTION), + canOpenInsight: false, + canOpenModal: true, + docs: { + url: 'https://posthog.com/docs/web-analytics/dashboard#retention', + title: 'Retention', + description: ( + <> +
+

+ Retention creates a cohort of unique users who performed any event for + the first time in the last week. It then tracks the percentage of + users who return to perform any event in the following weeks. +

+

+ You want the numbers numbers to be the highest possible, suggesting + that people that come to your page continue coming to your page - and + performing an actions. Also, the further down the table the higher the + numbers should be (or at least as high), which would indicate that + you're either increasing or keeping your retention at the same level. +

+
+ + ), + }, + } + : null, // Hiding if conversionGoal is set already because values aren't representative !conversionGoal && featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOALS] ? { @@ -1290,7 +1292,7 @@ export const webAnalyticsLogic = kea([ }, } : null, - featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_REPLAY] + !conversionGoal && featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_REPLAY] ? { kind: 'replay', tileId: TileId.REPLAY, @@ -1305,8 +1307,7 @@ export const webAnalyticsLogic = kea([ }, } : null, - // TODO: What to do with this one if conversion goal is set? Hide it? Disable it? - featureFlags[FEATURE_FLAGS.ERROR_TRACKING] + !conversionGoal && featureFlags[FEATURE_FLAGS.ERROR_TRACKING] ? { kind: 'error_tracking', tileId: TileId.ERROR_TRACKING, From 7b2c7128f7f22e5ac57d2f5222d8279bbdc4a32a Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Tue, 3 Dec 2024 16:01:02 -0300 Subject: [PATCH 03/17] Add `conversion` support to session and entry bounce queries --- .../web_analytics/stats_table.py | 192 ++++-------------- 1 file changed, 42 insertions(+), 150 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 198eb3c9b40cb..aeef63fe97e82 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -57,12 +57,11 @@ def to_query(self) -> ast.SelectQuery: if self.query.includeBounceRate: return self.to_entry_bounce_query() - if self._has_session_properties(): - return self._to_main_query_with_session_properties() - - return self.to_main_query() + return self.to_main_query( + self._counts_breakdown_value(), include_session_properties=self._has_session_properties() + ) - def to_main_query(self) -> ast.SelectQuery: + def to_main_query(self, breakdown, *, include_session_properties=False) -> ast.SelectQuery: with self.timings.measure("stats_table_query"): # Base selects, always returns the breakdown value, and the total number of visitors selects = [ @@ -87,150 +86,43 @@ def to_main_query(self) -> ast.SelectQuery: if self._include_extra_aggregation_value(): selects.append(self._extra_aggregation_value()) - query = ast.SelectQuery( - select=selects, - select_from=ast.JoinExpr(table=self._main_inner_query()), - group_by=[ast.Field(chain=["context.columns.breakdown_value"])], - order_by=[ - ast.OrderExpr(expr=ast.Field(chain=["context.columns.visitors"]), order="DESC"), - ast.OrderExpr( - expr=ast.Field( - chain=[ - "context.columns.views" - if self.query.conversionGoal is None - else "context.columns.total_conversions" - ] - ), - order="DESC", + query = ast.SelectQuery( + select=selects, + select_from=ast.JoinExpr( + table=self._main_inner_query( + breakdown, + include_session_properties=include_session_properties, + ) ), - ast.OrderExpr(expr=ast.Field(chain=["context.columns.breakdown_value"]), order="ASC"), - ], - ) - - return query - - def _to_main_query_with_session_properties(self) -> ast.SelectQuery: - with self.timings.measure("stats_table_query"): - query = parse_select( - """ -WITH - start_timestamp >= {date_from} AND start_timestamp < {date_to} AS current_period_segment, - start_timestamp >= {date_from_previous_period} AND start_timestamp < {date_from} AS previous_period_segment -SELECT - {processed_breakdown_value} AS "context.columns.breakdown_value", - tuple( - uniqIf(filtered_person_id, current_period_segment), - uniqIf(filtered_person_id, previous_period_segment) - ) AS "context.columns.visitors", - tuple( - sumIf(filtered_pageview_count, current_period_segment), - sumIf(filtered_pageview_count, previous_period_segment) - ) AS "context.columns.views" -FROM ( - SELECT - any(person_id) AS filtered_person_id, - count() AS filtered_pageview_count, - {breakdown_value} AS breakdown_value, - session.session_id AS session_id, - min(session.$start_timestamp) as start_timestamp - FROM events - WHERE and( - timestamp >= {date_from_previous_period}, - timestamp < {date_to}, - events.event == '$pageview', - {event_properties}, - {session_properties}, - {where_breakdown} - ) - GROUP BY session_id, breakdown_value -) -GROUP BY "context.columns.breakdown_value" -ORDER BY "context.columns.visitors" DESC, -"context.columns.views" DESC, -"context.columns.breakdown_value" ASC -""", - timings=self.timings, - placeholders={ - "breakdown_value": self._counts_breakdown_value(), - "processed_breakdown_value": self._processed_breakdown_value(), - "where_breakdown": self.where_breakdown(), - "event_properties": self._event_properties(), - "session_properties": self._session_properties(), - "date_from_previous_period": self._date_from_previous_period(), - "date_from": self._date_from(), - "date_to": self._date_to(), - }, + group_by=[ast.Field(chain=["context.columns.breakdown_value"])], + order_by=[ + ast.OrderExpr(expr=ast.Field(chain=["context.columns.visitors"]), order="DESC"), + ast.OrderExpr( + expr=ast.Field( + chain=[ + "context.columns.views" + if self.query.conversionGoal is None + else "context.columns.total_conversions" + ] + ), + order="DESC", + ), + ast.OrderExpr(expr=ast.Field(chain=["context.columns.breakdown_value"]), order="ASC"), + ], ) - assert isinstance(query, ast.SelectQuery) - - if self.query.breakdownBy == WebStatsBreakdown.LANGUAGE: - query.select.append(self._extra_aggregation_value()) return query def to_entry_bounce_query(self) -> ast.SelectQuery: - with self.timings.measure("stats_table_query"): - query = parse_select( - """ -WITH - start_timestamp >= {date_from} AND start_timestamp < {date_to} AS current_period_segment, - start_timestamp >= {date_from_previous_period} AND start_timestamp < {date_from} AS previous_period_segment -SELECT - breakdown_value AS "context.columns.breakdown_value", - tuple( - uniqIf(filtered_person_id, current_period_segment), - uniqIf(filtered_person_id, previous_period_segment) - ) AS "context.columns.visitors", - tuple( - sumIf(filtered_pageview_count, current_period_segment), - sumIf(filtered_pageview_count, previous_period_segment) - ) AS "context.columns.views", - tuple( - avgIf(is_bounce, current_period_segment), - avgIf(is_bounce, previous_period_segment) - ) AS "context.columns.bounce_rate", -FROM ( - SELECT - {bounce_breakdown} AS breakdown_value, - any(person_id) AS filtered_person_id, - count() AS filtered_pageview_count, - any(session.$is_bounce) AS is_bounce, - session.session_id AS session_id, - min(session.$start_timestamp) as start_timestamp - FROM events - WHERE and( - timestamp >= {date_from_previous_period}, - timestamp < {date_to}, - events.event == '$pageview', - {event_properties}, - {session_properties}, - {where_breakdown} - ) - GROUP BY session_id, breakdown_value -) -GROUP BY "context.columns.breakdown_value" -ORDER BY "context.columns.visitors" DESC, -"context.columns.views" DESC, -"context.columns.breakdown_value" ASC -""", - timings=self.timings, - placeholders={ - "bounce_breakdown": self._bounce_entry_pathname_breakdown(), - "where_breakdown": self.where_breakdown(), - "session_properties": self._session_properties(), - "event_properties": self._event_properties(), - "date_from_previous_period": self._date_from_previous_period(), - "date_from": self._date_from(), - "date_to": self._date_to(), - }, - ) - assert isinstance(query, ast.SelectQuery) + query = self.to_main_query(self._bounce_entry_pathname_breakdown(), include_session_properties=True) + + if self.query.conversionGoal is None: + query.select.append(self._period_comparison_tuple("is_bounce", "context.columns.bounce_rate", "avg")) + return query + # TODO: Support conversion goal def to_path_scroll_bounce_query(self) -> ast.SelectQuery: - if self.query.breakdownBy != WebStatsBreakdown.PAGE: - raise NotImplementedError("Scroll depth is only supported for page breakdowns") - with self.timings.measure("stats_table_bounce_query"): query = parse_select( """ @@ -349,6 +241,7 @@ def to_path_scroll_bounce_query(self) -> ast.SelectQuery: assert isinstance(query, ast.SelectQuery) return query + # TODO: Support conversion goal def to_path_bounce_query(self) -> ast.SelectQuery: if self.query.breakdownBy not in [WebStatsBreakdown.INITIAL_PAGE, WebStatsBreakdown.PAGE]: raise NotImplementedError("Bounce rate is only supported for page breakdowns") @@ -435,27 +328,23 @@ def to_path_bounce_query(self) -> ast.SelectQuery: assert isinstance(query, ast.SelectQuery) return query - def _main_inner_query(self): + def _main_inner_query(self, breakdown, *, include_session_properties=False): query = parse_select( """ SELECT any(person_id) AS filtered_person_id, count() AS filtered_pageview_count, {breakdown_value} AS breakdown_value, + session.session_id AS session_id, + any(session.$is_bounce) AS is_bounce, min(session.$start_timestamp) as start_timestamp FROM events -WHERE and( - timestamp >= {date_from}, - timestamp < {date_to}, - events.event == '$pageview', - {all_properties}, - {where_breakdown} -) -GROUP BY events.`$session_id`, breakdown_value +WHERE and(timestamp >= {date_from}, timestamp < {date_to}, events.event == '$pageview', {all_properties}, {where_breakdown}) +GROUP BY session_id, breakdown_value """, timings=self.timings, placeholders={ - "breakdown_value": self._counts_breakdown_value(), + "breakdown_value": breakdown, "date_from": self._date_from_previous_period(), "date_to": self._date_to(), "all_properties": self._all_properties(), @@ -465,6 +354,9 @@ def _main_inner_query(self): assert isinstance(query, ast.SelectQuery) + if include_session_properties: + query.where.args.append(self._session_properties()) # query.where is an `ast.Call` + if self.conversion_count_expr and self.conversion_person_id_expr: query.select.append(ast.Alias(alias="conversion_count", expr=self.conversion_count_expr)) query.select.append(ast.Alias(alias="conversion_person_id", expr=self.conversion_person_id_expr)) From 34efc506fd3825b4a1d75205f196752afc8a4c58 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Tue, 3 Dec 2024 16:06:31 -0300 Subject: [PATCH 04/17] Add conversion rate column --- .../hogql_queries/web_analytics/stats_table.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index aeef63fe97e82..698c49cc229d8 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -526,6 +526,23 @@ def calculate(self): else response.columns ) + # Add last conversion rate column + if self.query.conversionGoal is not None: + for result in results_mapped: + unique_visitors = result[1] + unique_conversions = result[-1] + + # Keep them in the same tuple format we already have + result.append( + ( + unique_conversions[0] / unique_visitors[0] if unique_visitors[0] != 0 else None, + unique_conversions[1] / unique_visitors[1] if unique_visitors[1] != 0 else None, + ) + ) + + # Guarantee new column exists + columns.append("context.columns.conversion_rate") + return WebStatsTableQueryResponse( columns=columns, results=results_mapped, From 18f44e55aa86bf5f624a43a2029fa6207b5df774 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Tue, 3 Dec 2024 18:30:08 -0300 Subject: [PATCH 05/17] Add conversion tests --- .../test/test_web_stats_table.py | 242 +++++++++++++++++- 1 file changed, 239 insertions(+), 3 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py index ae4b48b0632c1..dbbfb0b3a9d5b 100644 --- a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py +++ b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py @@ -3,7 +3,7 @@ from freezegun import freeze_time from posthog.hogql_queries.web_analytics.stats_table import WebStatsTableQueryRunner -from posthog.models import Cohort +from posthog.models import Action, Cohort, Element from posthog.models.utils import uuid7 from posthog.schema import ( DateRange, @@ -13,6 +13,8 @@ PropertyOperator, SessionTableVersion, HogQLQueryModifiers, + CustomEventConversionGoal, + ActionConversionGoal, ) from posthog.test.base import ( APIBaseTest, @@ -38,13 +40,27 @@ def _create_events(self, data, event="$pageview"): }, ) ) - for timestamp, session_id, pathname in timestamps: + for timestamp, session_id, *extra in timestamps: + url = None + elements = None + if event == "$pageview": + url = extra[0] if extra else None + elif event == "$autocapture": + elements = extra[0] if extra else None + properties = extra[1] if extra and len(extra) > 1 else {} + _create_event( team=self.team, event=event, distinct_id=id, timestamp=timestamp, - properties={"$session_id": session_id, "$pathname": pathname}, + properties={ + "$session_id": session_id, + "$pathname": url, + "$current_url": url, + **properties, + }, + elements=elements, ) return person_result @@ -107,6 +123,8 @@ def _run_web_stats_table_query( include_bounce_rate=False, include_scroll_depth=False, properties=None, + action: Optional[Action] = None, + custom_event: Optional[str] = None, session_table_version: SessionTableVersion = SessionTableVersion.V2, filter_test_accounts: Optional[bool] = False, ): @@ -119,6 +137,11 @@ def _run_web_stats_table_query( doPathCleaning=bool(path_cleaning_filters), includeBounceRate=include_bounce_rate, includeScrollDepth=include_scroll_depth, + conversionGoal=ActionConversionGoal(actionId=action.id) + if action + else CustomEventConversionGoal(customEventName=custom_event) + if custom_event + else None, filterTestAccounts=filter_test_accounts, ) self.team.path_cleaning_filters = path_cleaning_filters or [] @@ -1255,3 +1278,216 @@ def test_timezone_filter_with_empty_timezone(self): # Don't crash, treat all of them null assert results == [] + + def test_conversion_goal_no_conversions(self): + s1 = str(uuid7("2023-12-01")) + self._create_events( + [ + ("p1", [("2023-12-01", s1, "https://www.example.com/foo")]), + ] + ) + + action = Action.objects.create( + team=self.team, + name="Visited Bar", + steps_json=[{"event": "$pageview", "url": "https://www.example.com/bar", "url_matching": "regex"}], + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + def test_conversion_goal_one_pageview_conversion(self): + s1 = str(uuid7("2023-12-01")) + self._create_events( + [ + ("p1", [("2023-12-01", s1, "https://www.example.com/foo")]), + ] + ) + + action = Action.objects.create( + team=self.team, + name="Visited Foo", + steps_json=[ + { + "event": "$pageview", + "url": "https://www.example.com/foo", + "url_matching": "regex", + } + ], + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + assert [["https://www.example.com/foo", (1, 0), (1, 0), (1, 0), (1, None)]] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + def test_conversion_goal_one_custom_event_conversion(self): + s1 = str(uuid7("2023-12-01")) + self._create_events( + [ + ("p1", [("2023-12-01", s1, "https://www.example.com/foo")]), + ], + event="custom_event", + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, custom_event="custom_event" + ) + + # TODO: It's returning empty, I don't know why + assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + def test_conversion_goal_one_custom_action_conversion(self): + s1 = str(uuid7("2023-12-01")) + self._create_events( + [ + ("p1", [("2023-12-01", s1)]), + ], + event="custom_event", + ) + + action = Action.objects.create( + team=self.team, + name="Did Custom Event", + steps_json=[ + { + "event": "custom_event", + } + ], + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + # TODO: It's returning empty for some reason, check why + assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + def test_conversion_goal_one_autocapture_conversion(self): + s1 = str(uuid7("2023-12-01")) + self._create_events( + [ + ("p1", [("2023-12-01", s1, [Element(nth_of_type=1, nth_child=0, tag_name="button", text="Pay $10")])]), + ], + event="$autocapture", + ) + + action = Action.objects.create( + team=self.team, + name="Paid $10", + steps_json=[ + { + "event": "$autocapture", + "tag_name": "button", + "text": "Pay $10", + } + ], + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + # TODO: It's returning empty I don't know why, fix this + assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + def test_conversion_rate(self): + s1 = str(uuid7("2023-12-01")) + s2 = str(uuid7("2023-12-01")) + s3 = str(uuid7("2023-12-01")) + + self._create_events( + [ + ( + "p1", + [ + ("2023-12-01", s1, "https://www.example.com/foo"), + ("2023-12-01", s1, "https://www.example.com/foo"), + ], + ), + ( + "p2", + [ + ("2023-12-01", s2, "https://www.example.com/foo"), + ("2023-12-01", s2, "https://www.example.com/bar"), + ], + ), + ("p3", [("2023-12-01", s3, "https://www.example.com/bar")]), + ] + ) + + action = Action.objects.create( + team=self.team, + name="Visited Foo", + steps_json=[ + { + "event": "$pageview", + "url": "https://www.example.com/foo", + "url_matching": "regex", + } + ], + ) + + response = self._run_web_stats_table_query( + "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + ) + + # TODO: Check if conversion rate is accurate here, or if it should be a rational number + assert [ + ["https://www.example.com/foo", (2, 0), (3, 0), (2, 0), (1, None)], + ["https://www.example.com/bar", (2, 0), (0, 0), (0, 0), (0, None)], + ] == response.results + assert [ + "context.columns.breakdown_value", + "context.columns.visitors", + "context.columns.total_conversions", + "context.columns.unique_conversions", + "context.columns.conversion_rate", + ] == response.columns + + # TODO: Guarantee this matches above + # conversion_rate = results[3] + # self.assertAlmostEqual(conversion_rate.value, 100 * 2 / 3) From f934f57fbdf8c026be8ee56e195e5ad100321873 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 19:35:29 -0300 Subject: [PATCH 06/17] Render session replays relevant to the chosen conversion goal --- .../tiles/WebAnalyticsRecordings.tsx | 1 + .../web-analytics/webAnalyticsLogic.tsx | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/frontend/src/scenes/web-analytics/tiles/WebAnalyticsRecordings.tsx b/frontend/src/scenes/web-analytics/tiles/WebAnalyticsRecordings.tsx index ee32168dd7817..b04d7cdaa8953 100644 --- a/frontend/src/scenes/web-analytics/tiles/WebAnalyticsRecordings.tsx +++ b/frontend/src/scenes/web-analytics/tiles/WebAnalyticsRecordings.tsx @@ -16,6 +16,7 @@ export function WebAnalyticsRecordingsTile({ tile }: { tile: ReplayTile }): JSX. const { layout } = tile const { replayFilters, webAnalyticsFilters } = useValues(webAnalyticsLogic) const { currentTeam } = useValues(teamLogic) + const sessionRecordingsListLogicInstance = sessionRecordingsPlaylistLogic({ logicKey: 'webAnalytics', filters: replayFilters, diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 58814401a2dde..59b8e92cf8c56 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -45,6 +45,7 @@ import { PropertyOperator, RecordingUniversalFilters, RetentionPeriod, + UniversalFiltersGroupValue, } from '~/types' import type { webAnalyticsLogicType } from './webAnalyticsLogicType' @@ -1292,7 +1293,7 @@ export const webAnalyticsLogic = kea([ }, } : null, - !conversionGoal && featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_REPLAY] + featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_REPLAY] ? { kind: 'replay', tileId: TileId.REPLAY, @@ -1440,12 +1441,22 @@ export const webAnalyticsLogic = kea([ }, ], replayFilters: [ - (s) => [s.webAnalyticsFilters, s.dateFilter, s.shouldFilterTestAccounts], + (s) => [s.webAnalyticsFilters, s.dateFilter, s.shouldFilterTestAccounts, s.conversionGoal], ( webAnalyticsFilters: WebAnalyticsPropertyFilters, dateFilter, - shouldFilterTestAccounts + shouldFilterTestAccounts, + conversionGoal ): RecordingUniversalFilters => { + const filters: UniversalFiltersGroupValue[] = [...webAnalyticsFilters] + if (conversionGoal) { + const actionId = (conversionGoal as ActionConversionGoal).actionId + const customEventName = (conversionGoal as CustomEventConversionGoal).customEventName + const id = actionId || customEventName + + filters.push({ id, name: String(id), type: customEventName ? 'events' : 'actions' }) + } + return { filter_test_accounts: shouldFilterTestAccounts, @@ -1456,7 +1467,7 @@ export const webAnalyticsLogic = kea([ values: [ { type: FilterLogicalOperator.And, - values: webAnalyticsFilters || [], + values: filters, }, ], }, From 988e85b969fc6262e2291828e855331ed32c4757 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 19:52:31 -0300 Subject: [PATCH 07/17] Move conversation rate computation to HogQL Much more performant, and much simpler code --- .../web_analytics/stats_table.py | 31 ++++++++----------- .../test/test_web_stats_table.py | 5 --- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 698c49cc229d8..8f7d6fee0e96d 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -76,6 +76,19 @@ def to_main_query(self, breakdown, *, include_session_properties=False) -> ast.S self._period_comparison_tuple( "conversion_person_id", "context.columns.unique_conversions", "uniq" ), + ast.Alias( + alias="context.columns.conversion_rate", + expr=ast.Tuple( + exprs=[ + parse_expr( + "if(`context.columns.visitors`.1 = 0, NULL, `context.columns.unique_conversions`.1 / `context.columns.visitors`.1)" + ), + parse_expr( + "if(`context.columns.visitors`.2 = 0, NULL, `context.columns.unique_conversions`.2 / `context.columns.visitors`.2)" + ), + ] + ), + ), ] ) else: @@ -484,7 +497,6 @@ def _date_from(self) -> ast.Expr: def _date_from_previous_period(self) -> ast.Expr: return self.query_date_range.previous_period_date_from_as_hogql() - # TODO: Calculate conversion rate def calculate(self): query = self.to_query() response = self.paginator.execute_hogql_query( @@ -526,23 +538,6 @@ def calculate(self): else response.columns ) - # Add last conversion rate column - if self.query.conversionGoal is not None: - for result in results_mapped: - unique_visitors = result[1] - unique_conversions = result[-1] - - # Keep them in the same tuple format we already have - result.append( - ( - unique_conversions[0] / unique_visitors[0] if unique_visitors[0] != 0 else None, - unique_conversions[1] / unique_visitors[1] if unique_visitors[1] != 0 else None, - ) - ) - - # Guarantee new column exists - columns.append("context.columns.conversion_rate") - return WebStatsTableQueryResponse( columns=columns, results=results_mapped, diff --git a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py index dbbfb0b3a9d5b..1dd61238bd4f8 100644 --- a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py +++ b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py @@ -1475,7 +1475,6 @@ def test_conversion_rate(self): "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action ) - # TODO: Check if conversion rate is accurate here, or if it should be a rational number assert [ ["https://www.example.com/foo", (2, 0), (3, 0), (2, 0), (1, None)], ["https://www.example.com/bar", (2, 0), (0, 0), (0, 0), (0, None)], @@ -1487,7 +1486,3 @@ def test_conversion_rate(self): "context.columns.unique_conversions", "context.columns.conversion_rate", ] == response.columns - - # TODO: Guarantee this matches above - # conversion_rate = results[3] - # self.assertAlmostEqual(conversion_rate.value, 100 * 2 / 3) From 25c4977a53b700a018e9898ba85d38b66a289097 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 21:53:24 -0300 Subject: [PATCH 08/17] Fix all conversion-related test --- .../web_analytics/stats_table.py | 16 ++++++++++++- .../test/test_web_stats_table.py | 24 ++++++++++++------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 8f7d6fee0e96d..b5f6ca3e599a5 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -352,7 +352,7 @@ def _main_inner_query(self, breakdown, *, include_session_properties=False): any(session.$is_bounce) AS is_bounce, min(session.$start_timestamp) as start_timestamp FROM events -WHERE and(timestamp >= {date_from}, timestamp < {date_to}, events.event == '$pageview', {all_properties}, {where_breakdown}) +WHERE and(timestamp >= {date_from}, timestamp < {date_to}, {event_where}, {all_properties}, {where_breakdown}) GROUP BY session_id, breakdown_value """, timings=self.timings, @@ -360,6 +360,7 @@ def _main_inner_query(self, breakdown, *, include_session_properties=False): "breakdown_value": breakdown, "date_from": self._date_from_previous_period(), "date_to": self._date_to(), + "event_where": self.event_type_expr, "all_properties": self._all_properties(), "where_breakdown": self.where_breakdown(), }, @@ -439,6 +440,19 @@ def conversion_person_id_expr(self) -> Optional[ast.Expr]: 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/test/test_web_stats_table.py b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py index 1dd61238bd4f8..f59c95931dd07 100644 --- a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py +++ b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py @@ -1353,11 +1353,13 @@ def test_conversion_goal_one_custom_event_conversion(self): ) response = self._run_web_stats_table_query( - "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, custom_event="custom_event" + "2023-12-01", + "2023-12-03", + breakdown_by=WebStatsBreakdown.INITIAL_UTM_SOURCE, # Allow the breakdown value to be non-null + custom_event="custom_event", ) - # TODO: It's returning empty, I don't know why - assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [[None, (1, 0), (1, 0), (1, 0), (1, None)]] == response.results assert [ "context.columns.breakdown_value", "context.columns.visitors", @@ -1386,11 +1388,13 @@ def test_conversion_goal_one_custom_action_conversion(self): ) response = self._run_web_stats_table_query( - "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + "2023-12-01", + "2023-12-03", + breakdown_by=WebStatsBreakdown.INITIAL_UTM_SOURCE, # Allow the breakdown value to be non-null + action=action, ) - # TODO: It's returning empty for some reason, check why - assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [[None, (1, 0), (1, 0), (1, 0), (1, None)]] == response.results assert [ "context.columns.breakdown_value", "context.columns.visitors", @@ -1421,11 +1425,13 @@ def test_conversion_goal_one_autocapture_conversion(self): ) response = self._run_web_stats_table_query( - "2023-12-01", "2023-12-03", breakdown_by=WebStatsBreakdown.PAGE, action=action + "2023-12-01", + "2023-12-03", + breakdown_by=WebStatsBreakdown.INITIAL_UTM_SOURCE, # Allow the breakdown value to be non-null + action=action, ) - # TODO: It's returning empty I don't know why, fix this - assert [["https://www.example.com/foo", (1, 0), (0, 0), (0, 0), (0, None)]] == response.results + assert [[None, (1, 0), (1, 0), (1, 0), (1, None)]] == response.results assert [ "context.columns.breakdown_value", "context.columns.visitors", From 1efe865ed1fc61286a827a878b718c40a0191123 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 21:57:21 -0300 Subject: [PATCH 09/17] Span Replay the whole width if we're not rendering error tracking --- frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 59b8e92cf8c56..1dc5c1b92d288 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -1298,7 +1298,7 @@ export const webAnalyticsLogic = kea([ kind: 'replay', tileId: TileId.REPLAY, layout: { - colSpanClassName: 'md:col-span-1', + colSpanClassName: conversionGoal ? 'md:col-span-full' : 'md:col-span-1', }, docs: { url: 'https://posthog.com/docs/session-replay', From 3fae09d3d5d557c446646336072f4c360a18be54 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 22:13:10 -0300 Subject: [PATCH 10/17] Return conversion for `PAGE` queries Simply use the main query, scroll bounce/bounce doesn't make sense --- posthog/hogql_queries/web_analytics/stats_table.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index b5f6ca3e599a5..3913242c13dd6 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -48,7 +48,9 @@ def __init__(self, *args, **kwargs): def to_query(self) -> ast.SelectQuery: if self.query.breakdownBy == WebStatsBreakdown.PAGE: - if self.query.includeScrollDepth and self.query.includeBounceRate: + if self.query.conversionGoal: + return self.to_main_query(self._counts_breakdown_value()) + elif self.query.includeScrollDepth and self.query.includeBounceRate: return self.to_path_scroll_bounce_query() elif self.query.includeBounceRate: return self.to_path_bounce_query() From 61c32bc45053c204a81b6e1f9afd93b5bba6bc6c Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Wed, 4 Dec 2024 23:01:00 -0300 Subject: [PATCH 11/17] Fix mypy complaints --- posthog/hogql_queries/web_analytics/stats_table.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 3913242c13dd6..8dea9c8a648c2 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -371,7 +371,13 @@ def _main_inner_query(self, breakdown, *, include_session_properties=False): assert isinstance(query, ast.SelectQuery) if include_session_properties: - query.where.args.append(self._session_properties()) # query.where is an `ast.Call` + # Append session properties to the where clause if it exists + # + # We know `query.where` is an `ast.Call` because it's a `WHERE` clause + # and `ast.Call` is the only kind of expression that `query.where` can be + # but we need to convince mypy + if query.where is not None and isinstance(query.where, ast.Call): + query.where.args.append(self._session_properties()) if self.conversion_count_expr and self.conversion_person_id_expr: query.select.append(ast.Alias(alias="conversion_count", expr=self.conversion_count_expr)) From 34100259bbcbcf96030e7fbf529f77fb19b2700c Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Thu, 5 Dec 2024 12:16:34 -0300 Subject: [PATCH 12/17] Simplify TS checks --- .../scenes/web-analytics/webAnalyticsLogic.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 1dc5c1b92d288..07b5ff86c5ba4 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -1450,11 +1450,19 @@ export const webAnalyticsLogic = kea([ ): RecordingUniversalFilters => { const filters: UniversalFiltersGroupValue[] = [...webAnalyticsFilters] if (conversionGoal) { - const actionId = (conversionGoal as ActionConversionGoal).actionId - const customEventName = (conversionGoal as CustomEventConversionGoal).customEventName - const id = actionId || customEventName - - filters.push({ id, name: String(id), type: customEventName ? 'events' : 'actions' }) + if ('actionId' in conversionGoal) { + filters.push({ + id: conversionGoal.actionId, + name: String(conversionGoal.actionId), + type: 'actions', + }) + } else if ('customEventName' in conversionGoal) { + filters.push({ + id: conversionGoal.customEventName, + name: conversionGoal.customEventName, + type: 'events', + }) + } } return { From 5cc0b7a077bc16209030ff9d0a92dc368408672c Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Thu, 5 Dec 2024 12:27:49 -0300 Subject: [PATCH 13/17] Protect conversion goal behind feature flag --- frontend/src/lib/constants.tsx | 1 + .../web-analytics/webAnalyticsLogic.tsx | 27 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 6bd204a0aba84..21d891ff9b051 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -240,6 +240,7 @@ export const FEATURE_FLAGS = { REPLAY_LIST_RECORDINGS_AS_QUERY: 'replay-list-recordings-as-query', // owner: @pauldambra #team-replay BILLING_SKIP_FORECASTING: 'billing-skip-forecasting', // owner: @zach WEB_ANALYTICS_PERIOD_COMPARISON: 'web-analytics-period-comparison', // owner: @rafaeelaudibert #team-web-analytics + WEB_ANALYTICS_CONVERSION_GOAL_FILTERS: 'web-analytics-conversion-goal-filters', // owner: @rafaeelaudibert #team-web-analytics } as const export type FeatureFlagKey = (typeof FEATURE_FLAGS)[keyof typeof FEATURE_FLAGS] diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 07b5ff86c5ba4..2ec8e04161b75 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -623,7 +623,9 @@ export const webAnalyticsLogic = kea([ }, compareFilter: compareFilter || { compare: false }, filterTestAccounts, - conversionGoal, + conversionGoal: featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOAL_FILTERS] + ? conversionGoal + : undefined, properties: webAnalyticsFilters, }, hidePersonsModal: true, @@ -664,7 +666,9 @@ export const webAnalyticsLogic = kea([ compareFilter, limit: 10, filterTestAccounts, - conversionGoal, + conversionGoal: featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOAL_FILTERS] + ? conversionGoal + : undefined, ...(source || {}), }, embedded: false, @@ -865,7 +869,11 @@ export const webAnalyticsLogic = kea([ sampling, limit: 10, filterTestAccounts, - conversionGoal, + conversionGoal: featureFlags[ + FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOAL_FILTERS + ] + ? conversionGoal + : undefined, stripQueryParams: shouldStripQueryParams, }, embedded: false, @@ -1140,7 +1148,11 @@ export const webAnalyticsLogic = kea([ trendsFilter: { display: ChartDisplayType.WorldMap, }, - conversionGoal, + conversionGoal: featureFlags[ + FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOAL_FILTERS + ] + ? conversionGoal + : undefined, filterTestAccounts, properties: webAnalyticsFilters, }, @@ -1441,15 +1453,16 @@ export const webAnalyticsLogic = kea([ }, ], replayFilters: [ - (s) => [s.webAnalyticsFilters, s.dateFilter, s.shouldFilterTestAccounts, s.conversionGoal], + (s) => [s.webAnalyticsFilters, s.dateFilter, s.shouldFilterTestAccounts, s.conversionGoal, s.featureFlags], ( webAnalyticsFilters: WebAnalyticsPropertyFilters, dateFilter, shouldFilterTestAccounts, - conversionGoal + conversionGoal, + featureFlags ): RecordingUniversalFilters => { const filters: UniversalFiltersGroupValue[] = [...webAnalyticsFilters] - if (conversionGoal) { + if (conversionGoal && featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_CONVERSION_GOAL_FILTERS]) { if ('actionId' in conversionGoal) { filters.push({ id: conversionGoal.actionId, From 9acd99614ebab677655ae2d2b742338d2daa4de4 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Fri, 6 Dec 2024 18:47:28 -0300 Subject: [PATCH 14/17] docs: Improve documentation when conversion goal is selected --- .../web-analytics/webAnalyticsLogic.tsx | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 2ec8e04161b75..a535b5b54ed76 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -790,14 +790,21 @@ export const webAnalyticsLogic = kea([ accessed in your application, regardless of when they were accessed through the lifetime of a user session.

-

- The{' '} - - bounce rate - {' '} - indicates the percentage of users who left your page immediately - after visiting without capturing any event. -

+ {conversionGoal ? ( +

+ The conversion rate is the percentage of users who completed + the conversion goal in this specific path. +

+ ) : ( +

+ The{' '} + + bounce rate + {' '} + indicates the percentage of users who left your page + immediately after visiting without capturing any event. +

+ )} ), }, @@ -821,8 +828,17 @@ export const webAnalyticsLogic = kea([ title: 'Entry Path', description: (
- Entry paths are the paths a user session started, i.e. the first - path they saw when they opened your website. +

+ Entry paths are the paths a user session started, i.e. the first + path they saw when they opened your website. +

+ {conversionGoal && ( +

+ The conversion rate is the percentage of users who completed + the conversion goal after the first path in their session + being this path. +

+ )}
), }, From 643cba7d99871979fa4cd5be39dfbee03db6a2ec Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 9 Dec 2024 16:59:07 -0300 Subject: [PATCH 15/17] Remove outdated TODO comments --- posthog/hogql_queries/web_analytics/stats_table.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 8dea9c8a648c2..12bf790f7829e 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -32,8 +32,6 @@ BREAKDOWN_NULL_DISPLAY = "(none)" -# TODO: Extend `conversion_goal` support to queries besides `to_main_query` -# TODO: Add test cases for conversion goal, both action, and event-based ones class WebStatsTableQueryRunner(WebAnalyticsQueryRunner): query: WebStatsTableQuery response: WebStatsTableQueryResponse From 6948ee9ada06b0b5fbfe0def1691762582ce0e2b Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 9 Dec 2024 17:20:52 -0300 Subject: [PATCH 16/17] perf: Remove session optimization See https://github.com/PostHog/posthog/pull/26586#discussion_r1875855669 --- .../web_analytics/stats_table.py | 44 +++---------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 12bf790f7829e..0a0612b572aa7 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -57,11 +57,9 @@ def to_query(self) -> ast.SelectQuery: if self.query.includeBounceRate: return self.to_entry_bounce_query() - return self.to_main_query( - self._counts_breakdown_value(), include_session_properties=self._has_session_properties() - ) + return self.to_main_query(self._counts_breakdown_value()) - def to_main_query(self, breakdown, *, include_session_properties=False) -> ast.SelectQuery: + def to_main_query(self, breakdown) -> ast.SelectQuery: with self.timings.measure("stats_table_query"): # Base selects, always returns the breakdown value, and the total number of visitors selects = [ @@ -101,12 +99,7 @@ def to_main_query(self, breakdown, *, include_session_properties=False) -> ast.S query = ast.SelectQuery( select=selects, - select_from=ast.JoinExpr( - table=self._main_inner_query( - breakdown, - include_session_properties=include_session_properties, - ) - ), + select_from=ast.JoinExpr(table=self._main_inner_query(breakdown)), group_by=[ast.Field(chain=["context.columns.breakdown_value"])], order_by=[ ast.OrderExpr(expr=ast.Field(chain=["context.columns.visitors"]), order="DESC"), @@ -127,14 +120,13 @@ def to_main_query(self, breakdown, *, include_session_properties=False) -> ast.S return query def to_entry_bounce_query(self) -> ast.SelectQuery: - query = self.to_main_query(self._bounce_entry_pathname_breakdown(), include_session_properties=True) + query = self.to_main_query(self._bounce_entry_pathname_breakdown()) if self.query.conversionGoal is None: query.select.append(self._period_comparison_tuple("is_bounce", "context.columns.bounce_rate", "avg")) return query - # TODO: Support conversion goal def to_path_scroll_bounce_query(self) -> ast.SelectQuery: with self.timings.measure("stats_table_bounce_query"): query = parse_select( @@ -254,7 +246,6 @@ def to_path_scroll_bounce_query(self) -> ast.SelectQuery: assert isinstance(query, ast.SelectQuery) return query - # TODO: Support conversion goal def to_path_bounce_query(self) -> ast.SelectQuery: if self.query.breakdownBy not in [WebStatsBreakdown.INITIAL_PAGE, WebStatsBreakdown.PAGE]: raise NotImplementedError("Bounce rate is only supported for page breakdowns") @@ -341,7 +332,7 @@ def to_path_bounce_query(self) -> ast.SelectQuery: assert isinstance(query, ast.SelectQuery) return query - def _main_inner_query(self, breakdown, *, include_session_properties=False): + def _main_inner_query(self, breakdown): query = parse_select( """ SELECT @@ -368,15 +359,6 @@ def _main_inner_query(self, breakdown, *, include_session_properties=False): assert isinstance(query, ast.SelectQuery) - if include_session_properties: - # Append session properties to the where clause if it exists - # - # We know `query.where` is an `ast.Call` because it's a `WHERE` clause - # and `ast.Call` is the only kind of expression that `query.where` can be - # but we need to convince mypy - if query.where is not None and isinstance(query.where, ast.Call): - query.where.args.append(self._session_properties()) - if self.conversion_count_expr and self.conversion_person_id_expr: query.select.append(ast.Alias(alias="conversion_count", expr=self.conversion_count_expr)) query.select.append(ast.Alias(alias="conversion_person_id", expr=self.conversion_person_id_expr)) @@ -482,22 +464,6 @@ def map_scroll_property(property: Union[EventPropertyFilter, PersonPropertyFilte ] return property_to_expr(properties, team=self.team, scope="event") - def _has_session_properties(self) -> bool: - return any( - get_property_type(p) == "session" for p in self.query.properties + self._test_account_filters - ) or self.query.breakdownBy in { - WebStatsBreakdown.INITIAL_CHANNEL_TYPE, - WebStatsBreakdown.INITIAL_REFERRING_DOMAIN, - WebStatsBreakdown.INITIAL_UTM_SOURCE, - WebStatsBreakdown.INITIAL_UTM_CAMPAIGN, - WebStatsBreakdown.INITIAL_UTM_MEDIUM, - WebStatsBreakdown.INITIAL_UTM_TERM, - WebStatsBreakdown.INITIAL_UTM_CONTENT, - WebStatsBreakdown.INITIAL_PAGE, - WebStatsBreakdown.EXIT_PAGE, - WebStatsBreakdown.INITIAL_UTM_SOURCE_MEDIUM_CAMPAIGN, - } - def _session_properties(self) -> ast.Expr: properties = [ p for p in self.query.properties + self._test_account_filters if get_property_type(p) == "session" From 67b876569e5f4917bfae3c0501fa181f38a05562 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 9 Dec 2024 17:37:08 -0300 Subject: [PATCH 17/17] 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()