From 51bd5a9476286a6bff9c950b082951ca4bc12ece Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Wed, 20 Mar 2024 10:09:19 +0100 Subject: [PATCH 01/21] Use funnel in paths query --- .../utils/filtersToQueryNode.test.ts | 61 +++++- .../InsightQuery/utils/filtersToQueryNode.ts | 13 ++ .../InsightQuery/utils/queryNodeToFilter.ts | 1 + frontend/src/queries/schema.json | 3 + frontend/src/queries/schema.ts | 1 + .../insights/funnels/funnel_query_context.py | 2 +- .../insights/paths_query_runner.py | 49 ++++- posthog/schema.py | 173 +++++++++--------- 8 files changed, 213 insertions(+), 90 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 12f1b95c0a10b..286521e7caa27 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -507,7 +507,25 @@ describe('filtersToQueryNode', () => { end_point: 'b', path_groupings: ['c', 'd'], funnel_paths: FunnelPathType.between, - funnel_filter: { a: 1 }, + funnel_filter: { + events: [ + { + id: '$pageview', + name: '$pageview', + order: 0, + type: 'events', + }, + { + id: null, + order: 1, + type: 'events', + }, + ], + exclusions: [], + funnel_step: 1, + funnel_viz_type: 'steps', + insight: 'FUNNELS', + }, exclude_events: ['e', 'f'], step_limit: 1, path_start_key: 'g', @@ -530,7 +548,46 @@ describe('filtersToQueryNode', () => { endPoint: 'b', pathGroupings: ['c', 'd'], funnelPaths: FunnelPathType.between, - funnelFilter: { a: 1 }, + funnelFilter: { + events: [ + { + id: '$pageview', + name: '$pageview', + order: 0, + type: 'events', + }, + { + id: null, + order: 1, + type: 'events', + }, + ], + exclusions: [], + funnel_step: 1, + funnel_viz_type: 'steps', + insight: 'FUNNELS', + }, + funnelActorsQuery: { + funnelStep: 1, + kind: NodeKind.FunnelsActorsQuery, + source: { + funnelsFilter: { + funnelVizType: FunnelVizType.Steps, + }, + kind: NodeKind.FunnelsQuery, + series: [ + { + event: '$pageview', + kind: NodeKind.EventsNode, + name: '$pageview', + }, + { + event: null, + kind: NodeKind.EventsNode, + }, + ], + }, + }, excludeEvents: ['e', 'f'], stepLimit: 1, pathReplacements: true, diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 40cd113a7d29d..26e9386164793 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -18,6 +18,7 @@ import { EventsNode, FunnelExclusionActionsNode, FunnelExclusionEventsNode, + FunnelsActorsQuery, FunnelsFilter, InsightNodeKind, InsightQueryNode, @@ -474,6 +475,18 @@ export const pathsFilterToQuery = (filters: Partial): PathsFilt pathGroupings: filters.path_groupings, funnelPaths: filters.funnel_paths, funnelFilter: filters.funnel_filter, + funnelActorsQuery: filters.funnel_filter + ? ({ + kind: NodeKind.FunnelsActorsQuery, + source: { + //kind: NodeKind.FunnelsQuery, + funnelsFilter: funnelsFilterToQuery(filters.funnel_filter), + ...filtersToQueryNode(filters.funnel_filter), + }, + funnelStep: filters.funnel_filter.funnel_step, + //series: undefined // TODO + } as FunnelsActorsQuery) + : undefined, excludeEvents: filters.exclude_events, stepLimit: filters.step_limit, pathReplacements: filters.path_replacements, diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index 8c03814ead320..641091b433ed1 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -282,6 +282,7 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.pathsFilter?.maxEdgeWeight delete queryCopy.pathsFilter?.funnelPaths delete queryCopy.pathsFilter?.funnelFilter + delete queryCopy.pathsFilter?.funnelActorsQuery } else if (isStickinessQuery(queryCopy)) { camelCasedStickinessProps.show_legend = queryCopy.stickinessFilter?.showLegend camelCasedStickinessProps.show_values_on_series = queryCopy.stickinessFilter?.showValuesOnSeries diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 898d4e0974004..67a4ad5c0e1e0 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -3240,6 +3240,9 @@ }, "type": "array" }, + "funnelActorsQuery": { + "$ref": "#/definitions/FunnelsActorsQuery" + }, "funnelFilter": { "type": "object" }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 7eaa1734ea463..57c62cdda059c 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -787,6 +787,7 @@ export type PathsFilter = { maxEdgeWeight?: PathsFilterLegacy['max_edge_weight'] funnelPaths?: PathsFilterLegacy['funnel_paths'] funnelFilter?: PathsFilterLegacy['funnel_filter'] + funnelActorsQuery?: FunnelsActorsQuery /** Relevant only within actors query */ pathStartKey?: string diff --git a/posthog/hogql_queries/insights/funnels/funnel_query_context.py b/posthog/hogql_queries/insights/funnels/funnel_query_context.py index 66a0d28ad3d7f..64a14916f5245 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_query_context.py +++ b/posthog/hogql_queries/insights/funnels/funnel_query_context.py @@ -46,7 +46,7 @@ def __init__( timings: Optional[HogQLTimings] = None, modifiers: Optional[HogQLQueryModifiers] = None, limit_context: Optional[LimitContext] = None, - include_timestamp: Optional[bool] = None, + include_timestamp: Optional[bool] = True, include_preceding_timestamp: Optional[bool] = None, include_properties: Optional[List[str]] = None, include_final_matching_events: Optional[bool] = None, diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index c10a5a2320207..29c0afd3be4dd 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -27,6 +27,7 @@ PathCleaningFilter, PathsFilter, PathType, + FunnelPathType, ) from posthog.schema import PathsQuery @@ -133,9 +134,25 @@ def paths_events_query(self) -> ast.SelectQuery: event_hogql = self.construct_event_hogql() event_conditional = parse_expr("ifNull({event_hogql}, '') AS path_item_ungrouped", {"event_hogql": event_hogql}) + funnel_fields = [] + + if self.query.pathsFilter.funnelPaths in ( + FunnelPathType.funnel_path_after_step, + FunnelPathType.funnel_path_before_step, + ): + funnel_fields = [ + ast.Alias(alias="target_timestamp", expr=ast.Field(chain=["funnel_actors", "timestamp"])), + ] + elif self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps: + funnel_fields = [ + ast.Alias(alias="min_timestamp", expr=ast.Field(chain=["funnel_actors", "min_timestamp"])), + ast.Alias(alias="max_timestamp", expr=ast.Field(chain=["funnel_actors", "max_timestamp"])), + ] + fields = [ ast.Field(chain=["events", "timestamp"]), ast.Field(chain=["events", "person_id"]), + *funnel_fields, event_conditional, *[ast.Field(chain=["events", field]) for field in self.extra_event_fields], *[ @@ -226,10 +243,40 @@ def paths_events_query(self) -> ast.SelectQuery: where=ast.And(exprs=event_filters + self._get_event_query()), order_by=[ ast.OrderExpr(expr=ast.Field(chain=["person_id"])), - ast.OrderExpr(expr=ast.Field(chain=["timestamp"])), + ast.OrderExpr(expr=ast.Field(chain=["events", "timestamp"])), ], ) + if funnel_fields: + # get funnels query runner and user actors query + # assemble a funnel query + from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner + + actors_query_runner = InsightActorsQueryRunner( + query=self.query.pathsFilter.funnelActorsQuery, + team=self.team, + timings=self.timings, + modifiers=self.modifiers, + limit_context=self.limit_context, + ) + actors_query_runner.source_runner.funnel_actor_class.context.includeTimestamps = True + actors_query = actors_query_runner.to_query() + query.select_from = ast.JoinExpr( + table=ast.Field(chain=["events"]), + next_join=ast.JoinExpr( + table=actors_query, + join_type="INNER JOIN", + alias="funnel_actors", + constraint=ast.JoinConstraint( + expr=ast.CompareOperation( + op=ast.CompareOperationOp.Eq, + left=ast.Field(chain=["events", "person_id"]), + right=ast.Field(chain=["funnel_actors", "actor_id"]), + ), + ), + ), + ) + if self.query.samplingFactor is not None and isinstance(self.query.samplingFactor, float) and query.select_from: query.select_from.sample = ast.SampleExpr( sample_value=ast.RatioExpr(left=ast.Constant(value=self.query.samplingFactor)) diff --git a/posthog/schema.py b/posthog/schema.py index dc77da163db17..5eef1b94889e4 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -590,29 +590,6 @@ class PathType(str, Enum): hogql = "hogql" -class PathsFilter(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - edgeLimit: Optional[int] = None - endPoint: Optional[str] = None - excludeEvents: Optional[List[str]] = None - funnelFilter: Optional[Dict[str, Any]] = None - funnelPaths: Optional[FunnelPathType] = None - includeEventTypes: Optional[List[PathType]] = None - localPathCleaningFilters: Optional[List[PathCleaningFilter]] = None - maxEdgeWeight: Optional[int] = None - minEdgeWeight: Optional[int] = None - pathDropoffKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathEndKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathGroupings: Optional[List[str]] = None - pathReplacements: Optional[bool] = None - pathStartKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathsHogQLExpression: Optional[str] = None - startPoint: Optional[str] = None - stepLimit: Optional[int] = None - - class PathsFilterLegacy(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2391,12 +2368,6 @@ class HogQLAutocomplete(BaseModel): startPosition: int = Field(..., description="Start position of the editor word") -class InsightFilter( - RootModel[Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter]] -): - root: Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter] - - class PropertyGroupFilter(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2699,6 +2670,57 @@ class NamedParametersTypeofDateRangeForFilter(BaseModel): source: Optional[FilterType] = None +class FunnelsActorsQuery(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + funnelCustomSteps: Optional[List[int]] = Field( + default=None, + description="Custom step numbers to get persons for. This overrides `funnelStep`. Primarily for correlation use.", + ) + funnelStep: Optional[int] = Field( + default=None, + description="Index of the step for which we want to get the timestamp for, per person. Positive for converted persons, negative for dropped of persons.", + ) + funnelStepBreakdown: Optional[Union[str, float, List[Union[str, float]]]] = Field( + default=None, + description="The breakdown value for which to get persons for. This is an array for person and event properties, a string for groups and an integer for cohorts.", + ) + funnelTrendsDropOff: Optional[bool] = None + funnelTrendsEntrancePeriodStart: Optional[str] = Field( + default=None, + description="Used together with `funnelTrendsDropOff` for funnels time conversion date for the persons modal.", + ) + includeRecordings: Optional[bool] = None + kind: Literal["FunnelsActorsQuery"] = "FunnelsActorsQuery" + response: Optional[ActorsQueryResponse] = None + source: FunnelsQuery + + +class PathsFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + edgeLimit: Optional[int] = None + endPoint: Optional[str] = None + excludeEvents: Optional[List[str]] = None + funnelActorsQuery: Optional[FunnelsActorsQuery] = None + funnelFilter: Optional[Dict[str, Any]] = None + funnelPaths: Optional[FunnelPathType] = None + includeEventTypes: Optional[List[PathType]] = None + localPathCleaningFilters: Optional[List[PathCleaningFilter]] = None + maxEdgeWeight: Optional[int] = None + minEdgeWeight: Optional[int] = None + pathDropoffKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathEndKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathGroupings: Optional[List[str]] = None + pathReplacements: Optional[bool] = None + pathStartKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathsHogQLExpression: Optional[str] = None + startPoint: Optional[str] = None + stepLimit: Optional[int] = None + + class PathsQuery(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2734,31 +2756,25 @@ class PathsQuery(BaseModel): samplingFactor: Optional[float] = Field(default=None, description="Sampling rate") -class FunnelsActorsQuery(BaseModel): +class FunnelCorrelationQuery(BaseModel): model_config = ConfigDict( extra="forbid", ) - funnelCustomSteps: Optional[List[int]] = Field( - default=None, - description="Custom step numbers to get persons for. This overrides `funnelStep`. Primarily for correlation use.", - ) - funnelStep: Optional[int] = Field( - default=None, - description="Index of the step for which we want to get the timestamp for, per person. Positive for converted persons, negative for dropped of persons.", - ) - funnelStepBreakdown: Optional[Union[str, float, List[Union[str, float]]]] = Field( - default=None, - description="The breakdown value for which to get persons for. This is an array for person and event properties, a string for groups and an integer for cohorts.", - ) - funnelTrendsDropOff: Optional[bool] = None - funnelTrendsEntrancePeriodStart: Optional[str] = Field( - default=None, - description="Used together with `funnelTrendsDropOff` for funnels time conversion date for the persons modal.", - ) - includeRecordings: Optional[bool] = None - kind: Literal["FunnelsActorsQuery"] = "FunnelsActorsQuery" - response: Optional[ActorsQueryResponse] = None - source: FunnelsQuery + funnelCorrelationEventExcludePropertyNames: Optional[List[str]] = None + funnelCorrelationEventNames: Optional[List[str]] = None + funnelCorrelationExcludeEventNames: Optional[List[str]] = None + funnelCorrelationExcludeNames: Optional[List[str]] = None + funnelCorrelationNames: Optional[List[str]] = None + funnelCorrelationType: FunnelCorrelationResultsType + kind: Literal["FunnelCorrelationQuery"] = "FunnelCorrelationQuery" + response: Optional[FunnelCorrelationResponse] = None + source: FunnelsActorsQuery + + +class InsightFilter( + RootModel[Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter]] +): + root: Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter] class InsightVizNode(BaseModel): @@ -2785,41 +2801,6 @@ class InsightVizNode(BaseModel): vizSpecificOptions: Optional[VizSpecificOptions] = None -class FunnelCorrelationQuery(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - funnelCorrelationEventExcludePropertyNames: Optional[List[str]] = None - funnelCorrelationEventNames: Optional[List[str]] = None - funnelCorrelationExcludeEventNames: Optional[List[str]] = None - funnelCorrelationExcludeNames: Optional[List[str]] = None - funnelCorrelationNames: Optional[List[str]] = None - funnelCorrelationType: FunnelCorrelationResultsType - kind: Literal["FunnelCorrelationQuery"] = "FunnelCorrelationQuery" - response: Optional[FunnelCorrelationResponse] = None - source: FunnelsActorsQuery - - -class InsightActorsQuery(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - breakdown: Optional[Union[str, int]] = None - compare: Optional[Compare] = None - day: Optional[Union[str, int]] = None - includeRecordings: Optional[bool] = None - interval: Optional[int] = Field( - default=None, description="An interval selected out of available intervals in source query." - ) - kind: Literal["InsightActorsQuery"] = "InsightActorsQuery" - response: Optional[ActorsQueryResponse] = None - series: Optional[int] = None - source: Union[TrendsQuery, FunnelsQuery, RetentionQuery, PathsQuery, StickinessQuery, LifecycleQuery] = Field( - ..., discriminator="kind" - ) - status: Optional[str] = None - - class FunnelCorrelationActorsQuery(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2849,6 +2830,26 @@ class FunnelCorrelationActorsQuery(BaseModel): source: FunnelCorrelationQuery +class InsightActorsQuery(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + breakdown: Optional[Union[str, int]] = None + compare: Optional[Compare] = None + day: Optional[Union[str, int]] = None + includeRecordings: Optional[bool] = None + interval: Optional[int] = Field( + default=None, description="An interval selected out of available intervals in source query." + ) + kind: Literal["InsightActorsQuery"] = "InsightActorsQuery" + response: Optional[ActorsQueryResponse] = None + series: Optional[int] = None + source: Union[TrendsQuery, FunnelsQuery, RetentionQuery, PathsQuery, StickinessQuery, LifecycleQuery] = Field( + ..., discriminator="kind" + ) + status: Optional[str] = None + + class InsightActorsQueryOptions(BaseModel): model_config = ConfigDict( extra="forbid", From f4bfbbde953c83a580ac9efb66f2ff3f43e9aa6a Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Wed, 20 Mar 2024 10:14:09 +0100 Subject: [PATCH 02/21] Fix set active view when changing insight --- .../insights/InsightNav/insightNavLogic.tsx | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx index d2f28906bfe95..9dff8c87eeb8c 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx @@ -1,4 +1,5 @@ import { actions, afterMount, connect, kea, key, listeners, path, props, reducers, selectors } from 'kea' +import { urlToAction } from 'kea-router' import { FEATURE_FLAGS } from 'lib/constants' import { LemonTag } from 'lib/lemon-ui/LemonTag/LemonTag' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' @@ -41,7 +42,7 @@ import { isStickinessQuery, isTrendsQuery, } from '~/queries/utils' -import { BaseMathType, InsightLogicProps, InsightType } from '~/types' +import { BaseMathType, FilterType, InsightLogicProps, InsightType } from '~/types' import { MathAvailability } from '../filters/ActionFilter/ActionFilterRow/ActionFilterRow' import type { insightNavLogicType } from './insightNavLogicType' @@ -278,6 +279,23 @@ export const insightNavLogic = kea([ } }, })), + urlToAction(({ actions }) => ({ + '/insights/:shortId(/:mode)(/:subscriptionId)': ( + _, // url params + { dashboard, ...searchParams }, // search params + { filters: _filters } // hash params + ) => { + // capture any filters from the URL, either #filters={} or ?insight=X&bla=foo&bar=baz + const filters: Partial | null = + Object.keys(_filters || {}).length > 0 ? _filters : searchParams.insight ? searchParams : null + + if (!filters?.insight) { + return + } + + actions.setActiveView(filters?.insight) + }, + })), afterMount(({ values, actions }) => { if (values.query && isInsightVizNode(values.query)) { actions.updateQueryPropertyCache(cachePropertiesFromQuery(values.query.source, values.queryPropertyCache)) From 3bf8be972539c546c6bd89bd4be5f56e34de6a1c Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Wed, 20 Mar 2024 12:31:29 +0100 Subject: [PATCH 03/21] Add clauses --- .../insights/funnels/funnel_query_context.py | 2 +- .../insights/paths_query_runner.py | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/funnel_query_context.py b/posthog/hogql_queries/insights/funnels/funnel_query_context.py index 64a14916f5245..66a0d28ad3d7f 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_query_context.py +++ b/posthog/hogql_queries/insights/funnels/funnel_query_context.py @@ -46,7 +46,7 @@ def __init__( timings: Optional[HogQLTimings] = None, modifiers: Optional[HogQLQueryModifiers] = None, limit_context: Optional[LimitContext] = None, - include_timestamp: Optional[bool] = True, + include_timestamp: Optional[bool] = None, include_preceding_timestamp: Optional[bool] = None, include_properties: Optional[List[str]] = None, include_final_matching_events: Optional[bool] = None, diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 29c0afd3be4dd..0f3299a75403b 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -16,6 +16,7 @@ from posthog.hogql.property import property_to_expr from posthog.hogql.query import execute_hogql_query from posthog.hogql.timings import HogQLTimings +from posthog.hogql_queries.insights.funnels.utils import funnel_window_interval_unit_to_sql from posthog.hogql_queries.query_runner import QueryRunner from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.models import Team @@ -127,6 +128,9 @@ def construct_event_hogql(self) -> ast.Expr: return event_hogql + def funnel_window_interval(self) -> Optional[int]: + return self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval + def paths_events_query(self) -> ast.SelectQuery: event_filters = [] pathReplacements: list[PathCleaningFilter] = [] @@ -135,19 +139,52 @@ def paths_events_query(self) -> ast.SelectQuery: event_conditional = parse_expr("ifNull({event_hogql}, '') AS path_item_ungrouped", {"event_hogql": event_hogql}) funnel_fields = [] + funnel_include_timestamp = False + funnel_include_preceding_timestamp = False if self.query.pathsFilter.funnelPaths in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, ): + funnel_include_timestamp = True funnel_fields = [ ast.Alias(alias="target_timestamp", expr=ast.Field(chain=["funnel_actors", "timestamp"])), ] + interval = self.funnel_window_interval() + interval_unit = funnel_window_interval_unit_to_sql(interval) + operator = ">=" if self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step else "<=" + default_case = f"events.timestamp {operator} {{target_timestamp}}" + if ( + self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step + and self.query.pathsFilter.funnelActorsQuery.funnelStep + and self.query.pathsFilter.funnelActorsQuery.funnelStep < 0 + ): + default_case += f" + INTERVAL {interval} {interval_unit}" + event_filters.append( + parse_expr(default_case, {"target_timestamp": ast.Field(chain=["funnel_actors", "timestamp"])}) + ) elif self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps: + funnel_include_preceding_timestamp = True funnel_fields = [ ast.Alias(alias="min_timestamp", expr=ast.Field(chain=["funnel_actors", "min_timestamp"])), ast.Alias(alias="max_timestamp", expr=ast.Field(chain=["funnel_actors", "max_timestamp"])), ] + event_filters.append( + ast.And( + exprs=[ + ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=ast.Field(chain=["events", "timestamp"]), + right=ast.Field(chain=["funnel_actors", "min_timestamp"]), + ), + ast.CompareOperation( + op=ast.CompareOperationOp.LtEq, + left=ast.Field(chain=["events", "timestamp"]), + right=ast.Field(chain=["funnel_actors", "max_timestamp"]), + ), + ] + ) + ) fields = [ ast.Field(chain=["events", "timestamp"]), @@ -259,7 +296,8 @@ def paths_events_query(self) -> ast.SelectQuery: modifiers=self.modifiers, limit_context=self.limit_context, ) - actors_query_runner.source_runner.funnel_actor_class.context.includeTimestamps = True + actors_query_runner.source_runner.context.includeTimestamp = funnel_include_timestamp + actors_query_runner.source_runner.context.includePrecedingTimestamp = funnel_include_preceding_timestamp actors_query = actors_query_runner.to_query() query.select_from = ast.JoinExpr( table=ast.Field(chain=["events"]), From ee9532647ad3dc0cb2fe4c24baca19178a33b729 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Wed, 20 Mar 2024 12:50:21 +0100 Subject: [PATCH 04/21] Clean up --- .../InsightQuery/utils/filtersToQueryNode.ts | 2 - .../hogql_queries/insights/funnels/utils.py | 2 +- .../insights/paths_query_runner.py | 133 +++++++++--------- 3 files changed, 70 insertions(+), 67 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 26e9386164793..e690ac4115b09 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -479,12 +479,10 @@ export const pathsFilterToQuery = (filters: Partial): PathsFilt ? ({ kind: NodeKind.FunnelsActorsQuery, source: { - //kind: NodeKind.FunnelsQuery, funnelsFilter: funnelsFilterToQuery(filters.funnel_filter), ...filtersToQueryNode(filters.funnel_filter), }, funnelStep: filters.funnel_filter.funnel_step, - //series: undefined // TODO } as FunnelsActorsQuery) : undefined, excludeEvents: filters.exclude_events, diff --git a/posthog/hogql_queries/insights/funnels/utils.py b/posthog/hogql_queries/insights/funnels/utils.py index 47c1487e5fbcc..1222959535e3e 100644 --- a/posthog/hogql_queries/insights/funnels/utils.py +++ b/posthog/hogql_queries/insights/funnels/utils.py @@ -57,7 +57,7 @@ def funnel_window_interval_unit_to_sql( elif funnelWindowIntervalUnit == "day": return "DAY" else: - raise ValidationError("{funnelWindowIntervalUnit} not supported") + raise ValidationError(f"{funnelWindowIntervalUnit} not supported") def get_breakdown_expr( diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 0f3299a75403b..d9ed5c7dd9383 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -128,30 +128,17 @@ def construct_event_hogql(self) -> ast.Expr: return event_hogql - def funnel_window_interval(self) -> Optional[int]: - return self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval - - def paths_events_query(self) -> ast.SelectQuery: - event_filters = [] - pathReplacements: list[PathCleaningFilter] = [] - - event_hogql = self.construct_event_hogql() - event_conditional = parse_expr("ifNull({event_hogql}, '') AS path_item_ungrouped", {"event_hogql": event_hogql}) - - funnel_fields = [] - funnel_include_timestamp = False - funnel_include_preceding_timestamp = False - + def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: if self.query.pathsFilter.funnelPaths in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, ): - funnel_include_timestamp = True funnel_fields = [ ast.Alias(alias="target_timestamp", expr=ast.Field(chain=["funnel_actors", "timestamp"])), ] - interval = self.funnel_window_interval() - interval_unit = funnel_window_interval_unit_to_sql(interval) + interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval + unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit + interval_unit = funnel_window_interval_unit_to_sql(unit) operator = ">=" if self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step else "<=" default_case = f"events.timestamp {operator} {{target_timestamp}}" if ( @@ -160,31 +147,77 @@ def paths_events_query(self) -> ast.SelectQuery: and self.query.pathsFilter.funnelActorsQuery.funnelStep < 0 ): default_case += f" + INTERVAL {interval} {interval_unit}" - event_filters.append( - parse_expr(default_case, {"target_timestamp": ast.Field(chain=["funnel_actors", "timestamp"])}) + event_filter = parse_expr( + default_case, {"target_timestamp": ast.Field(chain=["funnel_actors", "timestamp"])} ) + return funnel_fields, event_filter elif self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps: - funnel_include_preceding_timestamp = True funnel_fields = [ ast.Alias(alias="min_timestamp", expr=ast.Field(chain=["funnel_actors", "min_timestamp"])), ast.Alias(alias="max_timestamp", expr=ast.Field(chain=["funnel_actors", "max_timestamp"])), ] - event_filters.append( - ast.And( - exprs=[ - ast.CompareOperation( - op=ast.CompareOperationOp.GtEq, - left=ast.Field(chain=["events", "timestamp"]), - right=ast.Field(chain=["funnel_actors", "min_timestamp"]), - ), - ast.CompareOperation( - op=ast.CompareOperationOp.LtEq, - left=ast.Field(chain=["events", "timestamp"]), - right=ast.Field(chain=["funnel_actors", "max_timestamp"]), - ), - ] - ) + event_filter = ast.And( + exprs=[ + ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=ast.Field(chain=["events", "timestamp"]), + right=ast.Field(chain=["funnel_actors", "min_timestamp"]), + ), + ast.CompareOperation( + op=ast.CompareOperationOp.LtEq, + left=ast.Field(chain=["events", "timestamp"]), + right=ast.Field(chain=["funnel_actors", "max_timestamp"]), + ), + ] ) + return funnel_fields, event_filter + + return [], None + + def funnel_join(self) -> ast.JoinExpr: + from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner + + actors_query_runner = InsightActorsQueryRunner( + query=self.query.pathsFilter.funnelActorsQuery, + team=self.team, + timings=self.timings, + modifiers=self.modifiers, + limit_context=self.limit_context, + ) + actors_query_runner.source_runner.context.includeTimestamp = self.query.pathsFilter.funnelPaths in ( + FunnelPathType.funnel_path_after_step, + FunnelPathType.funnel_path_before_step, + ) + actors_query_runner.source_runner.context.includePrecedingTimestamp = ( + self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps + ) + actors_query = actors_query_runner.to_query() + return ast.JoinExpr( + table=ast.Field(chain=["events"]), + next_join=ast.JoinExpr( + table=actors_query, + join_type="INNER JOIN", + alias="funnel_actors", + constraint=ast.JoinConstraint( + expr=ast.CompareOperation( + op=ast.CompareOperationOp.Eq, + left=ast.Field(chain=["events", "person_id"]), + right=ast.Field(chain=["funnel_actors", "actor_id"]), + ), + ), + ), + ) + + def paths_events_query(self) -> ast.SelectQuery: + event_filters = [] + pathReplacements: list[PathCleaningFilter] = [] + + event_hogql = self.construct_event_hogql() + event_conditional = parse_expr("ifNull({event_hogql}, '') AS path_item_ungrouped", {"event_hogql": event_hogql}) + + funnel_fields, funnel_event_filter = self.handle_funnel() + if funnel_event_filter: + event_filters.append(funnel_event_filter) fields = [ ast.Field(chain=["events", "timestamp"]), @@ -285,35 +318,7 @@ def paths_events_query(self) -> ast.SelectQuery: ) if funnel_fields: - # get funnels query runner and user actors query - # assemble a funnel query - from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner - - actors_query_runner = InsightActorsQueryRunner( - query=self.query.pathsFilter.funnelActorsQuery, - team=self.team, - timings=self.timings, - modifiers=self.modifiers, - limit_context=self.limit_context, - ) - actors_query_runner.source_runner.context.includeTimestamp = funnel_include_timestamp - actors_query_runner.source_runner.context.includePrecedingTimestamp = funnel_include_preceding_timestamp - actors_query = actors_query_runner.to_query() - query.select_from = ast.JoinExpr( - table=ast.Field(chain=["events"]), - next_join=ast.JoinExpr( - table=actors_query, - join_type="INNER JOIN", - alias="funnel_actors", - constraint=ast.JoinConstraint( - expr=ast.CompareOperation( - op=ast.CompareOperationOp.Eq, - left=ast.Field(chain=["events", "person_id"]), - right=ast.Field(chain=["funnel_actors", "actor_id"]), - ), - ), - ), - ) + query.select_from = self.funnel_join() if self.query.samplingFactor is not None and isinstance(self.query.samplingFactor, float) and query.select_from: query.select_from.sample = ast.SampleExpr( From 458c62b04ccf011e644611608b99827ff73c3052 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 20 Mar 2024 12:43:57 +0000 Subject: [PATCH 05/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index fad3c08168d0b..c2982e4f8526f 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -778,7 +778,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_2') + 'user_one_0') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 0414b29367ac38ecef789565b81c849cc00bb46a Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 20 Mar 2024 13:01:40 +0000 Subject: [PATCH 06/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index c2982e4f8526f..fad3c08168d0b 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -778,7 +778,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_0') + 'user_one_2') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 7c6cf174ea0a741f1fd4a08c6ab6ba715a9195e2 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Thu, 21 Mar 2024 16:35:43 +0100 Subject: [PATCH 07/21] Adjust session threshold calculation --- .../insights/paths_query_runner.py | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index d9ed5c7dd9383..5e10a41e9851e 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -29,6 +29,7 @@ PathsFilter, PathType, FunnelPathType, + FunnelConversionWindowTimeUnit, ) from posthog.schema import PathsQuery @@ -129,6 +130,12 @@ def construct_event_hogql(self) -> ast.Expr: return event_hogql def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: + if ( + not self.query.pathsFilter.funnelActorsQuery + or not self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter + ): + raise ValueError("Funnel actors query is required for funnel paths") + if self.query.pathsFilter.funnelPaths in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, @@ -136,7 +143,7 @@ def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: funnel_fields = [ ast.Alias(alias="target_timestamp", expr=ast.Field(chain=["funnel_actors", "timestamp"])), ] - interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval + interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval or 14 unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit interval_unit = funnel_window_interval_unit_to_sql(unit) operator = ">=" if self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step else "<=" @@ -175,6 +182,9 @@ def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: return [], None def funnel_join(self) -> ast.JoinExpr: + if not self.query.pathsFilter.funnelActorsQuery: + raise ValueError("Funnel actors query is required for funnel paths") + from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner actors_query_runner = InsightActorsQueryRunner( @@ -184,6 +194,8 @@ def funnel_join(self) -> ast.JoinExpr: modifiers=self.modifiers, limit_context=self.limit_context, ) + + assert actors_query_runner.source_runner.context is not None actors_query_runner.source_runner.context.includeTimestamp = self.query.pathsFilter.funnelPaths in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, @@ -192,6 +204,7 @@ def funnel_join(self) -> ast.JoinExpr: self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps ) actors_query = actors_query_runner.to_query() + return ast.JoinExpr( table=ast.Field(chain=["events"]), next_join=ast.JoinExpr( @@ -498,6 +511,28 @@ def get_target_clause(self) -> list[ast.Expr]: else: return self.get_filtered_path_ordering() + def get_session_threshold_clause(self) -> ast.Expr: + if self.query.pathsFilter.funnelPaths: + interval = 14 + interval_unit = FunnelConversionWindowTimeUnit.day + + if self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval: + interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval + unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit + interval_unit = funnel_window_interval_unit_to_sql(unit) + # TODO: Figure out if funnelWindowDays still used + # elif self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowDays: + # interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowDays + + return parse_expr( + f"arraySplit(x -> if(toDateTime('2018-01-01') + toIntervalSecond(_toInt64(x.3)) < toDateTime('2018-01-01') + INTERVAL {interval} {interval_unit}, 0, 1), paths_tuple)" + ) + + return parse_expr( + "arraySplit(x -> if(x.3 < ({session_time_threshold}), 0, 1), paths_tuple)", + {"session_time_threshold": ast.Constant(value=SESSION_TIME_THRESHOLD_DEFAULT_SECONDS)}, + ) + def paths_per_person_query(self) -> ast.SelectQuery: target_point = self.query.pathsFilter.endPoint or self.query.pathsFilter.startPoint target_point = ( @@ -521,15 +556,8 @@ def paths_per_person_query(self) -> ast.SelectQuery: "path_event_query": self.paths_events_query(), "boundary_event_filter": ast.Constant(value=None), "target_point": ast.Constant(value=target_point), - "session_threshold_clause": ast.Constant(value=None), - "session_time_threshold": ast.Constant(value=SESSION_TIME_THRESHOLD_DEFAULT_SECONDS), + "session_threshold_clause": self.get_session_threshold_clause(), "path_tuples_expr": path_tuples_expr, - # TODO: "extra_final_select_statements": ast.Constant(value=None), - "extra_joined_path_tuple_select_statements": ast.Constant(value=None), - "extra_array_filter_select_statements": ast.Constant(value=None), - "extra_limited_path_tuple_elements": ast.Constant(value=None), - "extra_path_time_tuple_select_statements": ast.Constant(value=None), - "extra_group_array_select_statements": ast.Constant(value=None), } select = cast( ast.SelectQuery, @@ -565,7 +593,7 @@ def paths_per_person_query(self) -> ast.SelectQuery: /* path_time_tuple.x added below if required */ session_index, {path_tuples_expr} as paths_tuple, - arraySplit(x -> if(x.3 < ({session_time_threshold}), 0, 1), paths_tuple) as session_paths + {session_threshold_clause} as session_paths FROM ( SELECT person_id, From 5795cb272d967636dd7adac9d92efeb63adefee9 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Fri, 22 Mar 2024 11:23:30 +0100 Subject: [PATCH 08/21] Fix code review suggestions --- .../InsightQuery/utils/filtersToQueryNode.ts | 18 ++++++++---------- .../insights/paths_query_runner.py | 3 --- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index e690ac4115b09..61c7ce5584d8b 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -475,16 +475,14 @@ export const pathsFilterToQuery = (filters: Partial): PathsFilt pathGroupings: filters.path_groupings, funnelPaths: filters.funnel_paths, funnelFilter: filters.funnel_filter, - funnelActorsQuery: filters.funnel_filter - ? ({ - kind: NodeKind.FunnelsActorsQuery, - source: { - funnelsFilter: funnelsFilterToQuery(filters.funnel_filter), - ...filtersToQueryNode(filters.funnel_filter), - }, - funnelStep: filters.funnel_filter.funnel_step, - } as FunnelsActorsQuery) - : undefined, + funnelActorsQuery: + filters.funnel_filter && Object.keys(filters.funnel_filter).length > 0 + ? ({ + kind: NodeKind.FunnelsActorsQuery, + source: filtersToQueryNode(filters.funnel_filter), + funnelStep: filters.funnel_filter.funnel_step, + } as FunnelsActorsQuery) + : undefined, excludeEvents: filters.exclude_events, stepLimit: filters.step_limit, pathReplacements: filters.path_replacements, diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 5e10a41e9851e..6bd34722fbe5b 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -520,9 +520,6 @@ def get_session_threshold_clause(self) -> ast.Expr: interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit interval_unit = funnel_window_interval_unit_to_sql(unit) - # TODO: Figure out if funnelWindowDays still used - # elif self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowDays: - # interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowDays return parse_expr( f"arraySplit(x -> if(toDateTime('2018-01-01') + toIntervalSecond(_toInt64(x.3)) < toDateTime('2018-01-01') + INTERVAL {interval} {interval_unit}, 0, 1), paths_tuple)" From da35c14dd628189340c8e6db20ec21aad9030912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 14:18:47 +0100 Subject: [PATCH 09/21] change schema --- frontend/src/queries/schema.json | 25 +++++++++--- frontend/src/queries/schema.ts | 10 ++++- posthog/schema.py | 70 ++++++++++++++++++-------------- 3 files changed, 67 insertions(+), 38 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 67a4ad5c0e1e0..a6999df515ff9 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -1959,6 +1959,21 @@ "enum": ["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"], "type": "string" }, + "FunnelPathsFilter": { + "additionalProperties": false, + "properties": { + "funnelPathType": { + "$ref": "#/definitions/FunnelPathType" + }, + "funnelSource": { + "$ref": "#/definitions/FunnelsQuery" + }, + "funnelStep": { + "type": "integer" + } + }, + "type": "object" + }, "FunnelStepReference": { "enum": ["total", "previous"], "type": "string" @@ -3240,12 +3255,6 @@ }, "type": "array" }, - "funnelActorsQuery": { - "$ref": "#/definitions/FunnelsActorsQuery" - }, - "funnelFilter": { - "type": "object" - }, "funnelPaths": { "$ref": "#/definitions/FunnelPathType" }, @@ -3379,6 +3388,10 @@ "description": "Exclude internal and test users by applying the respective filters", "type": "boolean" }, + "funnelPathsFilter": { + "$ref": "#/definitions/FunnelPathsFilter", + "description": "Used for displaying paths in relation to funnel steps." + }, "kind": { "const": "PathsQuery", "type": "string" diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 57c62cdda059c..c6230c464b088 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -786,8 +786,6 @@ export type PathsFilter = { minEdgeWeight?: PathsFilterLegacy['min_edge_weight'] maxEdgeWeight?: PathsFilterLegacy['max_edge_weight'] funnelPaths?: PathsFilterLegacy['funnel_paths'] - funnelFilter?: PathsFilterLegacy['funnel_filter'] - funnelActorsQuery?: FunnelsActorsQuery /** Relevant only within actors query */ pathStartKey?: string @@ -797,11 +795,19 @@ export type PathsFilter = { pathDropoffKey?: string } +export type FunnelPathsFilter = { + funnelPathType?: PathsFilterLegacy['funnel_paths'] + funnelSource?: FunnelsQuery + funnelStep?: integer +} + export interface PathsQuery extends InsightsQueryBase { kind: NodeKind.PathsQuery response?: PathsQueryResponse /** Properties specific to the paths insight */ pathsFilter: PathsFilter + /** Used for displaying paths in relation to funnel steps. */ + funnelPathsFilter?: FunnelPathsFilter } /** `StickinessFilterType` minus everything inherited from `FilterType` and persons modal related params diff --git a/posthog/schema.py b/posthog/schema.py index 5eef1b94889e4..cc86af986713d 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -590,6 +590,28 @@ class PathType(str, Enum): hogql = "hogql" +class PathsFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + edgeLimit: Optional[int] = None + endPoint: Optional[str] = None + excludeEvents: Optional[List[str]] = None + funnelPaths: Optional[FunnelPathType] = None + includeEventTypes: Optional[List[PathType]] = None + localPathCleaningFilters: Optional[List[PathCleaningFilter]] = None + maxEdgeWeight: Optional[int] = None + minEdgeWeight: Optional[int] = None + pathDropoffKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathEndKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathGroupings: Optional[List[str]] = None + pathReplacements: Optional[bool] = None + pathStartKey: Optional[str] = Field(default=None, description="Relevant only within actors query") + pathsHogQLExpression: Optional[str] = None + startPoint: Optional[str] = None + stepLimit: Optional[int] = None + + class PathsFilterLegacy(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2368,6 +2390,12 @@ class HogQLAutocomplete(BaseModel): startPosition: int = Field(..., description="Start position of the editor word") +class InsightFilter( + RootModel[Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter]] +): + root: Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter] + + class PropertyGroupFilter(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2670,6 +2698,15 @@ class NamedParametersTypeofDateRangeForFilter(BaseModel): source: Optional[FilterType] = None +class FunnelPathsFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + funnelPathType: Optional[FunnelPathType] = None + funnelSource: Optional[FunnelsQuery] = None + funnelStep: Optional[int] = None + + class FunnelsActorsQuery(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2697,30 +2734,6 @@ class FunnelsActorsQuery(BaseModel): source: FunnelsQuery -class PathsFilter(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - edgeLimit: Optional[int] = None - endPoint: Optional[str] = None - excludeEvents: Optional[List[str]] = None - funnelActorsQuery: Optional[FunnelsActorsQuery] = None - funnelFilter: Optional[Dict[str, Any]] = None - funnelPaths: Optional[FunnelPathType] = None - includeEventTypes: Optional[List[PathType]] = None - localPathCleaningFilters: Optional[List[PathCleaningFilter]] = None - maxEdgeWeight: Optional[int] = None - minEdgeWeight: Optional[int] = None - pathDropoffKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathEndKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathGroupings: Optional[List[str]] = None - pathReplacements: Optional[bool] = None - pathStartKey: Optional[str] = Field(default=None, description="Relevant only within actors query") - pathsHogQLExpression: Optional[str] = None - startPoint: Optional[str] = None - stepLimit: Optional[int] = None - - class PathsQuery(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2730,6 +2743,9 @@ class PathsQuery(BaseModel): filterTestAccounts: Optional[bool] = Field( default=None, description="Exclude internal and test users by applying the respective filters" ) + funnelPathsFilter: Optional[FunnelPathsFilter] = Field( + default=None, description="Used for displaying paths in relation to funnel steps." + ) kind: Literal["PathsQuery"] = "PathsQuery" pathsFilter: PathsFilter = Field(..., description="Properties specific to the paths insight") properties: Optional[ @@ -2771,12 +2787,6 @@ class FunnelCorrelationQuery(BaseModel): source: FunnelsActorsQuery -class InsightFilter( - RootModel[Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter]] -): - root: Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter] - - class InsightVizNode(BaseModel): model_config = ConfigDict( extra="forbid", From f9b8ae0cb50f173605be4ebb5ccdf5ffd7894265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 14:54:58 +0100 Subject: [PATCH 10/21] remove leftover todo --- posthog/hogql_queries/insights/paths_query_runner.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 6bd34722fbe5b..9cc133ea8e570 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -895,8 +895,6 @@ def to_actors_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: else: conditions.append(parse_expr("1=1")) - # TODO: Funnel? - actors_query = parse_select( """ SELECT From 7de91a6c1d24c3e04a3911f240c89b3f22f96280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 14:56:49 +0100 Subject: [PATCH 11/21] remove funnelPaths from funnelFilter --- frontend/src/queries/schema.json | 3 --- frontend/src/queries/schema.ts | 1 - posthog/schema.py | 1 - 3 files changed, 5 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index a6999df515ff9..b8b499839e339 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -3255,9 +3255,6 @@ }, "type": "array" }, - "funnelPaths": { - "$ref": "#/definitions/FunnelPathType" - }, "includeEventTypes": { "items": { "$ref": "#/definitions/PathType" diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index c6230c464b088..6640848c89108 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -785,7 +785,6 @@ export type PathsFilter = { localPathCleaningFilters?: PathsFilterLegacy['local_path_cleaning_filters'] minEdgeWeight?: PathsFilterLegacy['min_edge_weight'] maxEdgeWeight?: PathsFilterLegacy['max_edge_weight'] - funnelPaths?: PathsFilterLegacy['funnel_paths'] /** Relevant only within actors query */ pathStartKey?: string diff --git a/posthog/schema.py b/posthog/schema.py index cc86af986713d..8c0c5344a0797 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -597,7 +597,6 @@ class PathsFilter(BaseModel): edgeLimit: Optional[int] = None endPoint: Optional[str] = None excludeEvents: Optional[List[str]] = None - funnelPaths: Optional[FunnelPathType] = None includeEventTypes: Optional[List[PathType]] = None localPathCleaningFilters: Optional[List[PathCleaningFilter]] = None maxEdgeWeight: Optional[int] = None From 79cb03c94c7c4fcd8e9d2c1cfdcf37f0909dbbb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 14:58:22 +0100 Subject: [PATCH 12/21] make funnelPathType and funnelSource require in FunnelPathsFilter --- frontend/src/queries/schema.json | 1 + frontend/src/queries/schema.ts | 4 ++-- posthog/schema.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index b8b499839e339..aaff1e00a4315 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -1972,6 +1972,7 @@ "type": "integer" } }, + "required": ["funnelSource"], "type": "object" }, "FunnelStepReference": { diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 6640848c89108..774c4eb88e790 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -795,8 +795,8 @@ export type PathsFilter = { } export type FunnelPathsFilter = { - funnelPathType?: PathsFilterLegacy['funnel_paths'] - funnelSource?: FunnelsQuery + funnelPathType: PathsFilterLegacy['funnel_paths'] + funnelSource: FunnelsQuery funnelStep?: integer } diff --git a/posthog/schema.py b/posthog/schema.py index 8c0c5344a0797..6de155791d5aa 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -2702,7 +2702,7 @@ class FunnelPathsFilter(BaseModel): extra="forbid", ) funnelPathType: Optional[FunnelPathType] = None - funnelSource: Optional[FunnelsQuery] = None + funnelSource: FunnelsQuery funnelStep: Optional[int] = None From 97c9132ad61f4bf73147266078574409ba6d55ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 15:09:49 +0100 Subject: [PATCH 13/21] adapt backend --- .../insights/paths_query_runner.py | 63 +++++++++++-------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 9cc133ea8e570..624156064fc1e 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -23,6 +23,8 @@ from posthog.models.filters.mixins.utils import cached_property from posthog.queries.util import correct_result_for_sampling from posthog.schema import ( + FunnelsActorsQuery, + FunnelsFilter, HogQLQueryModifiers, PathsQueryResponse, PathCleaningFilter, @@ -130,35 +132,35 @@ def construct_event_hogql(self) -> ast.Expr: return event_hogql def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: - if ( - not self.query.pathsFilter.funnelActorsQuery - or not self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter - ): - raise ValueError("Funnel actors query is required for funnel paths") + if not self.query.funnelPathsFilter: + return [], None - if self.query.pathsFilter.funnelPaths in ( + funnelPathType, funnelSource, funnelStep = ( + self.query.funnelPathsFilter.funnelPathType, + self.query.funnelPathsFilter.funnelSource, + self.query.funnelPathsFilter.funnelStep, + ) + funnelSourceFilter = funnelSource.funnelsFilter or FunnelsFilter() + + if funnelPathType in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, ): funnel_fields = [ ast.Alias(alias="target_timestamp", expr=ast.Field(chain=["funnel_actors", "timestamp"])), ] - interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval or 14 - unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit + interval = funnelSourceFilter.funnelWindowInterval or 14 + unit = funnelSourceFilter.funnelWindowIntervalUnit interval_unit = funnel_window_interval_unit_to_sql(unit) - operator = ">=" if self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step else "<=" + operator = ">=" if funnelPathType == FunnelPathType.funnel_path_after_step else "<=" default_case = f"events.timestamp {operator} {{target_timestamp}}" - if ( - self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_after_step - and self.query.pathsFilter.funnelActorsQuery.funnelStep - and self.query.pathsFilter.funnelActorsQuery.funnelStep < 0 - ): + if funnelPathType == FunnelPathType.funnel_path_after_step and funnelStep and funnelStep < 0: default_case += f" + INTERVAL {interval} {interval_unit}" event_filter = parse_expr( default_case, {"target_timestamp": ast.Field(chain=["funnel_actors", "timestamp"])} ) return funnel_fields, event_filter - elif self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps: + elif funnelPathType == FunnelPathType.funnel_path_between_steps: funnel_fields = [ ast.Alias(alias="min_timestamp", expr=ast.Field(chain=["funnel_actors", "min_timestamp"])), ast.Alias(alias="max_timestamp", expr=ast.Field(chain=["funnel_actors", "max_timestamp"])), @@ -178,17 +180,24 @@ def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: ] ) return funnel_fields, event_filter - - return [], None + else: + raise ValueError("Unexpected `funnelPathType` for funnel path filter.") def funnel_join(self) -> ast.JoinExpr: - if not self.query.pathsFilter.funnelActorsQuery: - raise ValueError("Funnel actors query is required for funnel paths") + if not self.query.funnelPathsFilter: + raise ValueError("Funnel paths filter is required for funnel paths.") from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner + funnelPathType, funnelSource, funnelStep = ( + self.query.funnelPathsFilter.funnelPathType, + self.query.funnelPathsFilter.funnelSource, + self.query.funnelPathsFilter.funnelStep, + ) + + actor_query = FunnelsActorsQuery(source=funnelSource, funnelStep=funnelStep) actors_query_runner = InsightActorsQueryRunner( - query=self.query.pathsFilter.funnelActorsQuery, + query=actor_query, team=self.team, timings=self.timings, modifiers=self.modifiers, @@ -196,12 +205,12 @@ def funnel_join(self) -> ast.JoinExpr: ) assert actors_query_runner.source_runner.context is not None - actors_query_runner.source_runner.context.includeTimestamp = self.query.pathsFilter.funnelPaths in ( + actors_query_runner.source_runner.context.includeTimestamp = funnelPathType in ( FunnelPathType.funnel_path_after_step, FunnelPathType.funnel_path_before_step, ) actors_query_runner.source_runner.context.includePrecedingTimestamp = ( - self.query.pathsFilter.funnelPaths == FunnelPathType.funnel_path_between_steps + funnelPathType == FunnelPathType.funnel_path_between_steps ) actors_query = actors_query_runner.to_query() @@ -512,13 +521,15 @@ def get_target_clause(self) -> list[ast.Expr]: return self.get_filtered_path_ordering() def get_session_threshold_clause(self) -> ast.Expr: - if self.query.pathsFilter.funnelPaths: + if self.query.funnelPathsFilter: + funnelSourceFilter = self.query.funnelPathsFilter.funnelSource.funnelsFilter or FunnelsFilter() + interval = 14 interval_unit = FunnelConversionWindowTimeUnit.day - if self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval: - interval = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowInterval - unit = self.query.pathsFilter.funnelActorsQuery.source.funnelsFilter.funnelWindowIntervalUnit + if funnelSourceFilter.funnelWindowInterval: + interval = funnelSourceFilter.funnelWindowInterval + unit = funnelSourceFilter.funnelWindowIntervalUnit interval_unit = funnel_window_interval_unit_to_sql(unit) return parse_expr( From 432588b74d08efc0f5cac37220049f722ea358f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 15:41:21 +0100 Subject: [PATCH 14/21] adapt filtersToQueryNode --- .../utils/filtersToQueryNode.test.ts | 62 +++++++------------ .../InsightQuery/utils/filtersToQueryNode.ts | 26 ++++---- 2 files changed, 36 insertions(+), 52 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 286521e7caa27..bd25eb4fb9e87 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -547,47 +547,6 @@ describe('filtersToQueryNode', () => { startPoint: 'a', endPoint: 'b', pathGroupings: ['c', 'd'], - funnelPaths: FunnelPathType.between, - funnelFilter: { - events: [ - { - id: '$pageview', - name: '$pageview', - order: 0, - type: 'events', - }, - { - id: null, - order: 1, - type: 'events', - }, - ], - exclusions: [], - funnel_step: 1, - funnel_viz_type: 'steps', - insight: 'FUNNELS', - }, - funnelActorsQuery: { - funnelStep: 1, - kind: NodeKind.FunnelsActorsQuery, - source: { - funnelsFilter: { - funnelVizType: FunnelVizType.Steps, - }, - kind: NodeKind.FunnelsQuery, - series: [ - { - event: '$pageview', - kind: NodeKind.EventsNode, - name: '$pageview', - }, - { - event: null, - kind: NodeKind.EventsNode, - }, - ], - }, - }, excludeEvents: ['e', 'f'], stepLimit: 1, pathReplacements: true, @@ -596,6 +555,27 @@ describe('filtersToQueryNode', () => { minEdgeWeight: 1, maxEdgeWeight: 1, }, + funnelPathsFilter: { + funnelPathType: FunnelPathType.between, + funnelStep: 1, + funnelSource: { + funnelsFilter: { + funnelVizType: FunnelVizType.Steps, + }, + kind: NodeKind.FunnelsQuery, + series: [ + { + event: '$pageview', + kind: NodeKind.EventsNode, + name: '$pageview', + }, + { + event: null, + kind: NodeKind.EventsNode, + }, + ], + }, + }, } expect(result).toEqual(query) }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 61c7ce5584d8b..5fc6382abce5e 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -18,8 +18,9 @@ import { EventsNode, FunnelExclusionActionsNode, FunnelExclusionEventsNode, - FunnelsActorsQuery, + FunnelPathsFilter, FunnelsFilter, + FunnelsQuery, InsightNodeKind, InsightQueryNode, InsightsQueryBase, @@ -398,6 +399,7 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo // paths filter if (isPathsFilter(filters) && isPathsQuery(query)) { query.pathsFilter = pathsFilterToQuery(filters) + query.funnelPathsFilter = filtersToFunnelPathsQuery(filters) } // stickiness filter @@ -473,16 +475,6 @@ export const pathsFilterToQuery = (filters: Partial): PathsFilt startPoint: filters.start_point, endPoint: filters.end_point, pathGroupings: filters.path_groupings, - funnelPaths: filters.funnel_paths, - funnelFilter: filters.funnel_filter, - funnelActorsQuery: - filters.funnel_filter && Object.keys(filters.funnel_filter).length > 0 - ? ({ - kind: NodeKind.FunnelsActorsQuery, - source: filtersToQueryNode(filters.funnel_filter), - funnelStep: filters.funnel_filter.funnel_step, - } as FunnelsActorsQuery) - : undefined, excludeEvents: filters.exclude_events, stepLimit: filters.step_limit, pathReplacements: filters.path_replacements, @@ -493,6 +485,18 @@ export const pathsFilterToQuery = (filters: Partial): PathsFilt }) } +export const filtersToFunnelPathsQuery = (filters: Partial): FunnelPathsFilter | undefined => { + if (filters.funnel_paths === undefined || filters.funnel_filter === undefined) { + return undefined + } + + return { + funnelPathType: filters.funnel_paths, + funnelSource: filtersToQueryNode(filters.funnel_filter) as FunnelsQuery, + funnelStep: filters.funnel_filter?.funnel_step, + } +} + export const stickinessFilterToQuery = (filters: Record): StickinessFilter => { return objectCleanWithEmpty({ display: filters.display, From 76bda03667534035b91bf497d9d2b03a898299be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Mon, 25 Mar 2024 15:41:51 +0100 Subject: [PATCH 15/21] adapt queryNodeToFilters --- .../utils/queryNodeToFilter.test.ts | 101 +++++++++++++++++- .../InsightQuery/utils/queryNodeToFilter.ts | 14 ++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts index 9b9dd04421962..94777bfa254c1 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts @@ -1,16 +1,19 @@ import { FunnelLayout } from 'lib/constants' import { hiddenLegendItemsToKeys, queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' -import { FunnelsQuery, LifecycleQuery, NodeKind, TrendsQuery } from '~/queries/schema' +import { FunnelsQuery, LifecycleQuery, NodeKind, PathsQuery, TrendsQuery } from '~/queries/schema' import { BreakdownAttributionType, ChartDisplayType, FunnelConversionWindowTimeUnit, + FunnelPathType, FunnelsFilterType, FunnelStepReference, FunnelVizType, InsightType, LifecycleFilterType, + PathsFilterType, + PathType, StepOrderValue, TrendsFilterType, } from '~/types' @@ -178,6 +181,102 @@ describe('queryNodeToFilter', () => { } expect(result).toEqual(filters) }) + + test('converts a pathsFilter and funnelPathsFilter into filter properties', () => { + const query: PathsQuery = { + kind: NodeKind.PathsQuery, + pathsFilter: { + includeEventTypes: [PathType.Screen, PathType.PageView], + startPoint: 'a', + endPoint: 'b', + pathGroupings: ['c', 'd'], + excludeEvents: ['e', 'f'], + stepLimit: 1, + pathReplacements: true, + localPathCleaningFilters: [{ alias: 'home' }], + edgeLimit: 1, + minEdgeWeight: 1, + maxEdgeWeight: 1, + }, + funnelPathsFilter: { + funnelPathType: FunnelPathType.between, + funnelStep: 1, + funnelSource: { + funnelsFilter: { + funnelVizType: FunnelVizType.Steps, + }, + kind: NodeKind.FunnelsQuery, + series: [ + { + event: '$pageview', + kind: NodeKind.EventsNode, + name: '$pageview', + }, + { + event: null, + kind: NodeKind.EventsNode, + }, + ], + }, + }, + } + + const result = queryNodeToFilter(query) + + const filters: Partial = { + insight: InsightType.PATHS, + include_event_types: [PathType.Screen, PathType.PageView], + start_point: 'a', + end_point: 'b', + path_groupings: ['c', 'd'], + funnel_paths: FunnelPathType.between, + entity_type: 'events', + funnel_filter: { + entity_type: 'events', + events: [ + { + id: '$pageview', + name: '$pageview', + order: 0, + type: 'events', + }, + { + id: null, + order: 1, + type: 'events', + }, + ], + exclusions: undefined, + funnel_step: 1, + funnel_viz_type: 'steps', + insight: 'FUNNELS', + bin_count: undefined, + breakdown_attribution_type: undefined, + breakdown_attribution_value: undefined, + funnel_aggregate_by_hogql: undefined, + funnel_from_step: undefined, + funnel_to_step: undefined, + funnel_order_type: undefined, + funnel_step_reference: undefined, + funnel_window_interval: undefined, + funnel_window_interval_unit: undefined, + hidden_legend_keys: undefined, + interval: undefined, + }, + exclude_events: ['e', 'f'], + step_limit: 1, + // path_start_key: 'g', + // path_end_key: 'h', + // path_dropoff_key: 'i', + path_replacements: true, + local_path_cleaning_filters: [{ alias: 'home' }], + edge_limit: 1, + min_edge_weight: 1, + max_edge_weight: 1, + paths_hogql_expression: undefined, + } + expect(result).toEqual(filters) + }) }) describe('hiddenLegendItemsToKeys', () => { diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index 641091b433ed1..125d138e8779d 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -266,8 +266,14 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial camelCasedPathsProps.local_path_cleaning_filters = queryCopy.pathsFilter?.localPathCleaningFilters camelCasedPathsProps.min_edge_weight = queryCopy.pathsFilter?.minEdgeWeight camelCasedPathsProps.max_edge_weight = queryCopy.pathsFilter?.maxEdgeWeight - camelCasedPathsProps.funnel_paths = queryCopy.pathsFilter?.funnelPaths - camelCasedPathsProps.funnel_filter = queryCopy.pathsFilter?.funnelFilter + camelCasedPathsProps.funnel_paths = queryCopy.funnelPathsFilter?.funnelPathType + camelCasedPathsProps.funnel_filter = + queryCopy.funnelPathsFilter !== undefined + ? { + ...queryNodeToFilter(queryCopy.funnelPathsFilter.funnelSource), + funnel_step: queryCopy.funnelPathsFilter.funnelStep, + } + : undefined delete queryCopy.pathsFilter?.edgeLimit delete queryCopy.pathsFilter?.pathsHogQLExpression delete queryCopy.pathsFilter?.includeEventTypes @@ -280,9 +286,7 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.pathsFilter?.localPathCleaningFilters delete queryCopy.pathsFilter?.minEdgeWeight delete queryCopy.pathsFilter?.maxEdgeWeight - delete queryCopy.pathsFilter?.funnelPaths - delete queryCopy.pathsFilter?.funnelFilter - delete queryCopy.pathsFilter?.funnelActorsQuery + delete queryCopy.funnelPathsFilter } else if (isStickinessQuery(queryCopy)) { camelCasedStickinessProps.show_legend = queryCopy.stickinessFilter?.showLegend camelCasedStickinessProps.show_values_on_series = queryCopy.stickinessFilter?.showValuesOnSeries From 294b351f392a0ff50908cd644fb6cb5f16eea749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 27 Mar 2024 16:06:06 +0100 Subject: [PATCH 16/21] adapt filter_to_query --- .../legacy_compatibility/filter_to_query.py | 23 +++++++- .../test/test_filter_to_query.py | 58 +++++++++++++------ 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index 2b8f59f88a421..aea2ffe5e5227 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -12,6 +12,7 @@ EventsNode, FunnelExclusionActionsNode, FunnelExclusionEventsNode, + FunnelPathsFilter, FunnelsFilter, FunnelsQuery, LifecycleFilter, @@ -451,9 +452,8 @@ def _insight_filter(filter: Dict): edgeLimit=filter.get("edge_limit"), minEdgeWeight=filter.get("min_edge_weight"), maxEdgeWeight=filter.get("max_edge_weight"), - funnelPaths=filter.get("funnel_paths"), - funnelFilter=filter.get("funnel_filter"), - ) + ), + "funnelPathsFilter": filters_to_funnel_paths_query(filter), } elif _insight_type(filter) == "LIFECYCLE": insight_filter = { @@ -480,6 +480,23 @@ def _insight_filter(filter: Dict): return insight_filter +def filters_to_funnel_paths_query(filter: Dict) -> FunnelPathsFilter | None: + funnel_paths = filter.get("funnel_paths") + funnel_filter = filter.get("funnel_filter") + + if funnel_paths is None or funnel_filter is None: + return None + + funnel_query = filter_to_query(funnel_filter) + assert isinstance(funnel_query, FunnelsQuery) + + return FunnelPathsFilter( + funnelPathType=funnel_paths, + funnelSource=funnel_query, + funnelStep=funnel_filter["funnel_step"], + ) + + def _insight_type(filter: Dict) -> INSIGHT_TYPE: if filter.get("insight") == "SESSIONS": return "TRENDS" diff --git a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py index 9421ac41be854..327eeccd94663 100644 --- a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py @@ -10,6 +10,7 @@ ChartDisplayType, CohortPropertyFilter, CountPerActorMathType, + DateRange, ElementPropertyFilter, EventPropertyFilter, EventsNode, @@ -17,26 +18,34 @@ FunnelExclusionActionsNode, FunnelExclusionEventsNode, FunnelPathType, + FunnelPathsFilter, FunnelVizType, + FunnelsQuery, GroupPropertyFilter, HogQLPropertyFilter, Key, + LifecycleQuery, LifecycleToggle, + MathGroupTypeIndex, PathCleaningFilter, PathType, + PathsQuery, PersonPropertyFilter, PropertyMathType, PropertyOperator, RetentionPeriod, + RetentionQuery, RetentionType, SessionPropertyFilter, StepOrderValue, + StickinessQuery, TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter, + TrendsQuery, ) from posthog.test.base import BaseTest @@ -954,6 +963,7 @@ def test_date_range(self): query = filter_to_query(filter) + assert isinstance(query.dateRange, DateRange) self.assertEqual(query.dateRange.date_from, "-14d") self.assertEqual(query.dateRange.date_to, "-7d") @@ -962,6 +972,7 @@ def test_interval(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual(query.interval, "hour") def test_series_default(self): @@ -969,6 +980,7 @@ def test_series_default(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual(query.series, []) def test_series_custom(self): @@ -979,6 +991,7 @@ def test_series_custom(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.series, [ @@ -1000,6 +1013,7 @@ def test_series_order(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.series, [ @@ -1038,6 +1052,7 @@ def test_series_math(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.series, [ @@ -1057,7 +1072,7 @@ def test_series_math(self): event="$pageview", name="$pageview", math="unique_group", - math_group_type_index=0, + math_group_type_index=MathGroupTypeIndex.number_0, ), EventsNode( event="$pageview", @@ -1164,6 +1179,7 @@ def test_series_properties(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.series, [ @@ -1252,6 +1268,7 @@ def test_breakdown(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.breakdownFilter, BreakdownFilter(breakdown_type=BreakdownType.event, breakdown="$browser"), @@ -1262,6 +1279,7 @@ def test_breakdown_converts_multi(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.breakdownFilter, BreakdownFilter(breakdown_type=BreakdownType.event, breakdown="$browser"), @@ -1272,6 +1290,7 @@ def test_breakdown_type_default(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.breakdownFilter, BreakdownFilter(breakdown_type=BreakdownType.event, breakdown="some_prop"), @@ -1294,6 +1313,7 @@ def test_trends_filter(self): query = filter_to_query(filter) + assert isinstance(query, TrendsQuery) self.assertEqual( query.trendsFilter, TrendsFilter( @@ -1359,6 +1379,7 @@ def test_funnels_filter(self): query = filter_to_query(filter) + assert isinstance(query, FunnelsQuery) self.assertEqual( query.funnelsFilter, FunnelsFilter( @@ -1407,6 +1428,7 @@ def test_retention_filter(self): query = filter_to_query(filter) + assert isinstance(query, RetentionQuery) self.assertEqual( query.retentionFilter, RetentionFilter( @@ -1467,6 +1489,7 @@ def test_paths_filter(self): query = filter_to_query(filter) + assert isinstance(query, PathsQuery) self.assertEqual( query.pathsFilter, PathsFilter( @@ -1484,24 +1507,21 @@ def test_paths_filter(self): excludeEvents=["http://localhost:8000/events"], stepLimit=5, pathGroupings=["/merchant/*/payment"], - funnelPaths=FunnelPathType.funnel_path_between_steps, - funnelFilter={ - "insight": "FUNNELS", - "events": [ - { - "type": "events", - "id": "$pageview", - "order": 0, - "name": "$pageview", - "math": "total", - }, - {"type": "events", "id": None, "order": 1, "math": "total"}, + ), + ) + self.assertEqual( + query.funnelPathsFilter, + FunnelPathsFilter( + funnelPathType=FunnelPathType.funnel_path_between_steps, + funnelSource=FunnelsQuery( + series=[ + EventsNode(event="$pageview", name="$pageview"), + EventsNode(event=None, name="All events"), ], - "funnel_viz_type": "steps", - "exclusions": [], - "filter_test_accounts": True, - "funnel_step": 2, - }, + filterTestAccounts=True, + funnelsFilter=FunnelsFilter(funnelVizType=FunnelVizType.steps, exclusions=[]), + ), + funnelStep=2, ), ) @@ -1516,6 +1536,7 @@ def test_stickiness_filter(self): query = filter_to_query(filter) + assert isinstance(query, StickinessQuery) self.assertEqual( query.stickinessFilter, StickinessFilter(compare=True, showLegend=True, showValuesOnSeries=True), @@ -1531,6 +1552,7 @@ def test_lifecycle_filter(self): query = filter_to_query(filter) + assert isinstance(query, LifecycleQuery) self.assertEqual( query.lifecycleFilter, LifecycleFilter( From 7c8323bc5ea74b932163aceb446ea43f16c13332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 27 Mar 2024 16:18:37 +0100 Subject: [PATCH 17/21] convert timestamp to utc before adding interval --- posthog/hogql_queries/insights/paths_query_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index 624156064fc1e..d2c519033aa3d 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -153,7 +153,7 @@ def handle_funnel(self) -> tuple[list, Optional[ast.Expr]]: unit = funnelSourceFilter.funnelWindowIntervalUnit interval_unit = funnel_window_interval_unit_to_sql(unit) operator = ">=" if funnelPathType == FunnelPathType.funnel_path_after_step else "<=" - default_case = f"events.timestamp {operator} {{target_timestamp}}" + default_case = f"events.timestamp {operator} toTimeZone({{target_timestamp}}, 'UTC')" if funnelPathType == FunnelPathType.funnel_path_after_step and funnelStep and funnelStep < 0: default_case += f" + INTERVAL {interval} {interval_unit}" event_filter = parse_expr( From ce3b487de2b4889ef0f6112b967fbc5cffd40906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 27 Mar 2024 16:46:07 +0100 Subject: [PATCH 18/21] fix mypy issues --- mypy-baseline.txt | 56 ------------------- .../insights/paths_query_runner.py | 4 +- .../legacy_compatibility/filter_to_query.py | 6 +- 3 files changed, 6 insertions(+), 60 deletions(-) diff --git a/mypy-baseline.txt b/mypy-baseline.txt index 2143c119ea5c2..d8231681ce336 100644 --- a/mypy-baseline.txt +++ b/mypy-baseline.txt @@ -468,63 +468,7 @@ posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Need type annotation for "properties_3" (hint: "properties_3: Dict[, ] = ...") [var-annotated] posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Need type annotation for "filter" (hint: "filter: Dict[, ] = ...") [var-annotated] posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Need type annotation for "filter" (hint: "filter: Dict[, ] = ...") [var-annotated] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "None" of "DateRange | None" has no attribute "date_from" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "None" of "DateRange | None" has no attribute "date_to" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "interval" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "interval" [union-attr] posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Need type annotation for "filter" (hint: "filter: Dict[, ] = ...") [var-annotated] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "series" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "breakdownFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "FunnelsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "trendsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "trendsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "trendsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "trendsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "trendsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "TrendsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "funnelsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "funnelsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "funnelsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "funnelsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "funnelsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "TrendsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "retentionFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "FunnelsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "retentionFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "retentionFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "retentionFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "retentionFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "TrendsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "pathsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "FunnelsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "pathsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "pathsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "pathsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "pathsFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "TrendsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "stickinessFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "FunnelsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "stickinessFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "stickinessFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "stickinessFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "LifecycleQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "stickinessFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "TrendsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "lifecycleFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "FunnelsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "lifecycleFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "RetentionQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "lifecycleFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "PathsQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "lifecycleFilter" [union-attr] -posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py:0: error: Item "StickinessQuery" of "TrendsQuery | FunnelsQuery | RetentionQuery | PathsQuery | StickinessQuery | LifecycleQuery" has no attribute "lifecycleFilter" [union-attr] posthog/hogql_queries/insights/trends/test/test_aggregation_operations.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select" [union-attr] posthog/hogql_queries/insights/trends/test/test_aggregation_operations.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select" [union-attr] posthog/hogql_queries/insights/trends/test/test_aggregation_operations.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "group_by" [union-attr] diff --git a/posthog/hogql_queries/insights/paths_query_runner.py b/posthog/hogql_queries/insights/paths_query_runner.py index d2c519033aa3d..052fe2a586f47 100644 --- a/posthog/hogql_queries/insights/paths_query_runner.py +++ b/posthog/hogql_queries/insights/paths_query_runner.py @@ -16,6 +16,7 @@ from posthog.hogql.property import property_to_expr from posthog.hogql.query import execute_hogql_query from posthog.hogql.timings import HogQLTimings +from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner from posthog.hogql_queries.insights.funnels.utils import funnel_window_interval_unit_to_sql from posthog.hogql_queries.query_runner import QueryRunner from posthog.hogql_queries.utils.query_date_range import QueryDateRange @@ -204,6 +205,7 @@ def funnel_join(self) -> ast.JoinExpr: limit_context=self.limit_context, ) + assert isinstance(actors_query_runner.source_runner, FunnelsQueryRunner) assert actors_query_runner.source_runner.context is not None actors_query_runner.source_runner.context.includeTimestamp = funnelPathType in ( FunnelPathType.funnel_path_after_step, @@ -530,7 +532,7 @@ def get_session_threshold_clause(self) -> ast.Expr: if funnelSourceFilter.funnelWindowInterval: interval = funnelSourceFilter.funnelWindowInterval unit = funnelSourceFilter.funnelWindowIntervalUnit - interval_unit = funnel_window_interval_unit_to_sql(unit) + interval_unit = funnel_window_interval_unit_to_sql(unit) # type: ignore return parse_expr( f"arraySplit(x -> if(toDateTime('2018-01-01') + toIntervalSecond(_toInt64(x.3)) < toDateTime('2018-01-01') + INTERVAL {interval} {interval_unit}, 0, 1), paths_tuple)" diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index aea2ffe5e5227..2714c84db438e 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -1,7 +1,7 @@ import copy from enum import Enum import json -from typing import List, Dict, Literal +from typing import Any, List, Dict, Literal from posthog.models.entity.entity import Entity as LegacyEntity from posthog.schema import ( ActionsNode, @@ -453,7 +453,7 @@ def _insight_filter(filter: Dict): minEdgeWeight=filter.get("min_edge_weight"), maxEdgeWeight=filter.get("max_edge_weight"), ), - "funnelPathsFilter": filters_to_funnel_paths_query(filter), + "funnelPathsFilter": filters_to_funnel_paths_query(filter), # type: ignore } elif _insight_type(filter) == "LIFECYCLE": insight_filter = { @@ -480,7 +480,7 @@ def _insight_filter(filter: Dict): return insight_filter -def filters_to_funnel_paths_query(filter: Dict) -> FunnelPathsFilter | None: +def filters_to_funnel_paths_query(filter: Dict[str, Any]) -> FunnelPathsFilter | None: funnel_paths = filter.get("funnel_paths") funnel_filter = filter.get("funnel_filter") From e102cc8e78f99688f6e7387df4b533365a24cd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 27 Mar 2024 19:02:29 +0100 Subject: [PATCH 19/21] fix paths frontend --- .../insights/EditorFilters/PathsTarget.tsx | 46 +++++++++++-------- .../scenes/insights/insightVizDataLogic.ts | 1 + frontend/src/scenes/paths/pathsDataLogic.ts | 3 +- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx index 73c5a243e180f..a716b2a962d96 100644 --- a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx +++ b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx @@ -6,6 +6,8 @@ import { IconFunnelVertical } from 'lib/lemon-ui/icons' import { LemonButton } from 'lib/lemon-ui/LemonButton' import { pathsDataLogic } from 'scenes/paths/pathsDataLogic' +import { queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' +import { FunnelsQuery } from '~/queries/schema' import { EditorFilterProps, FunnelPathType } from '~/types' export function PathsTargetStart(props: EditorFilterProps): JSX.Element { @@ -21,13 +23,14 @@ type PathTargetProps = { } & EditorFilterProps function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { - const { pathsFilter, taxonomicGroupTypes } = useValues(pathsDataLogic(insightProps)) - const { updateInsightFilter } = useActions(pathsDataLogic(insightProps)) + const { pathsFilter, funnelPathsFilter, taxonomicGroupTypes } = useValues(pathsDataLogic(insightProps)) + const { updateInsightFilter, updateQuerySource } = useActions(pathsDataLogic(insightProps)) - const { funnelPaths, funnelFilter, startPoint, endPoint, pathGroupings } = pathsFilter || {} + const { startPoint, endPoint, pathGroupings } = pathsFilter || {} + const { funnelPathType, funnelSource, funnelStep } = funnelPathsFilter || {} - const overrideStartInput = funnelPaths && [FunnelPathType.between, FunnelPathType.after].includes(funnelPaths) - const overrideEndInput = funnelPaths && [FunnelPathType.between, FunnelPathType.before].includes(funnelPaths) + const overrideStartInput = funnelPathType && [FunnelPathType.between, FunnelPathType.after].includes(funnelPathType) + const overrideEndInput = funnelPathType && [FunnelPathType.between, FunnelPathType.before].includes(funnelPathType) const overrideInputs = overrideStartInput || overrideEndInput const key = position === 'start' ? 'startPoint' : 'endPoint' @@ -35,7 +38,7 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { updateInsightFilter({ [key]: item }) } const onReset = (): void => { - updateInsightFilter({ [key]: undefined, funnelFilter: undefined, funnelPaths: undefined }) + updateQuerySource({ pathsFilter: { ...pathsFilter, [key]: undefined }, funnelPathsFilter: undefined }) } function _getStepNameAtIndex(filters: Record, index: number): string { @@ -50,14 +53,14 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { return targetEntity?.name || '' } - function _getStepLabel(funnelFilters?: Record, index?: number, shift: number = 0): JSX.Element { - if (funnelFilters && index) { + function _getStepLabel(funnelSource?: FunnelsQuery, index?: number, shift: number = 0): JSX.Element { + if (funnelSource && index) { return (
{`${ index > 0 ? 'Funnel step ' + (index + shift) : 'Funnel dropoff ' + index * -1 - }: ${_getStepNameAtIndex(funnelFilters, index > 0 ? index + shift : index * -1)}`} + }: ${_getStepNameAtIndex(funnelSource, index > 0 ? index + shift : index * -1)}`}
) } else { @@ -66,12 +69,12 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { } function getStartPointLabel(): JSX.Element { - if (funnelPaths) { - if (funnelPaths === FunnelPathType.after) { - return _getStepLabel(funnelFilter, funnelFilter?.funnel_step) - } else if (funnelPaths === FunnelPathType.between) { + if (funnelPathType) { + if (funnelPathType === FunnelPathType.after) { + return _getStepLabel(funnelSource, funnelStep) + } else if (funnelPathType === FunnelPathType.between) { // funnel_step targets the later of the 2 events when specifying between so the start point index is shifted back 1 - return _getStepLabel(funnelFilter, funnelFilter?.funnel_step, -1) + return _getStepLabel(funnelSource, funnelStep, -1) } else { return } @@ -85,9 +88,9 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { } function getEndPointLabel(): JSX.Element { - if (funnelPaths) { - if (funnelPaths === FunnelPathType.before || funnelPaths === FunnelPathType.between) { - return _getStepLabel(funnelFilter, funnelFilter?.funnel_step) + if (funnelPathType) { + if (funnelPathType === FunnelPathType.before || funnelPathType === FunnelPathType.between) { + return _getStepLabel(funnelSource, funnelStep) } else { return } @@ -107,7 +110,7 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { pathItem: startPoint, closeButtonEnabled: startPoint || overrideStartInput, disabled: overrideEndInput && !overrideStartInput, - funnelFilterLink: funnelFilter && overrideStartInput, + funnelFilterLink: funnelSource && overrideStartInput, }, end: { index: 1, @@ -115,7 +118,7 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { pathItem: endPoint, closeButtonEnabled: endPoint || overrideEndInput, disabled: overrideStartInput && !overrideEndInput, - funnelFilterLink: funnelFilter && overrideEndInput, + funnelFilterLink: funnelSource && overrideEndInput, }, }[position] @@ -139,7 +142,10 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { positionOptions.funnelFilterLink ? () => { router.actions.push( - combineUrl('/insights', encodeParams(funnelFilter as Record, '?')).url + combineUrl( + '/insights', + encodeParams(queryNodeToFilter(funnelSource as FunnelsQuery), '?') + ).url ) } : () => {} diff --git a/frontend/src/scenes/insights/insightVizDataLogic.ts b/frontend/src/scenes/insights/insightVizDataLogic.ts index a73ffdc89afa9..250ac40172add 100644 --- a/frontend/src/scenes/insights/insightVizDataLogic.ts +++ b/frontend/src/scenes/insights/insightVizDataLogic.ts @@ -178,6 +178,7 @@ export const insightVizDataLogic = kea([ pathsFilter: [(s) => [s.querySource], (q) => (isPathsQuery(q) ? q.pathsFilter : null)], stickinessFilter: [(s) => [s.querySource], (q) => (isStickinessQuery(q) ? q.stickinessFilter : null)], lifecycleFilter: [(s) => [s.querySource], (q) => (isLifecycleQuery(q) ? q.lifecycleFilter : null)], + funnelPathsFilter: [(s) => [s.querySource], (q) => (isPathsQuery(q) ? q.funnelPathsFilter : null)], isUsingSessionAnalysis: [ (s) => [s.series, s.breakdownFilter, s.properties], diff --git a/frontend/src/scenes/paths/pathsDataLogic.ts b/frontend/src/scenes/paths/pathsDataLogic.ts index 750490efc64f4..0e2edb8fb1b48 100644 --- a/frontend/src/scenes/paths/pathsDataLogic.ts +++ b/frontend/src/scenes/paths/pathsDataLogic.ts @@ -50,12 +50,13 @@ export const pathsDataLogic = kea([ 'insightDataLoading', 'insightDataError', 'pathsFilter', + 'funnelPathsFilter', 'dateRange', ], featureFlagLogic, ['featureFlags'], ], - actions: [insightVizDataLogic(props), ['updateInsightFilter']], + actions: [insightVizDataLogic(props), ['updateInsightFilter', 'updateQuerySource']], })), actions({ From d47bd8cdff6c54dd797b57cc7e109d1420c40d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 27 Mar 2024 19:33:51 +0100 Subject: [PATCH 20/21] more fixes --- .../insights/EditorFilters/PathsTarget.tsx | 7 +++++-- frontend/src/scenes/paths/PathNodeCard.tsx | 12 ++++++++---- frontend/src/scenes/paths/Paths.tsx | 16 ++++++++++++++-- frontend/src/scenes/paths/pathUtils.ts | 18 ++++++++++++------ frontend/src/scenes/paths/renderPaths.ts | 8 +++++--- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx index a716b2a962d96..e864588381a45 100644 --- a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx +++ b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx @@ -7,7 +7,7 @@ import { LemonButton } from 'lib/lemon-ui/LemonButton' import { pathsDataLogic } from 'scenes/paths/pathsDataLogic' import { queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' -import { FunnelsQuery } from '~/queries/schema' +import { FunnelsQuery, PathsQuery } from '~/queries/schema' import { EditorFilterProps, FunnelPathType } from '~/types' export function PathsTargetStart(props: EditorFilterProps): JSX.Element { @@ -38,7 +38,10 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { updateInsightFilter({ [key]: item }) } const onReset = (): void => { - updateQuerySource({ pathsFilter: { ...pathsFilter, [key]: undefined }, funnelPathsFilter: undefined }) + updateQuerySource({ + pathsFilter: { ...pathsFilter, [key]: undefined }, + funnelPathsFilter: undefined, + } as Partial) } function _getStepNameAtIndex(filters: Record, index: number): string { diff --git a/frontend/src/scenes/paths/PathNodeCard.tsx b/frontend/src/scenes/paths/PathNodeCard.tsx index de3becb0f4293..38baea7bee48d 100644 --- a/frontend/src/scenes/paths/PathNodeCard.tsx +++ b/frontend/src/scenes/paths/PathNodeCard.tsx @@ -2,6 +2,7 @@ import { Tooltip } from '@posthog/lemon-ui' import { Dropdown } from 'antd' import { useActions, useValues } from 'kea' +import { FunnelPathsFilter } from '~/queries/schema' import { InsightLogicProps } from '~/types' import { PATH_NODE_CARD_LEFT_OFFSET, PATH_NODE_CARD_TOP_OFFSET, PATH_NODE_CARD_WIDTH } from './constants' @@ -16,10 +17,11 @@ export type PathNodeCardProps = { } export function PathNodeCard({ insightProps, node }: PathNodeCardProps): JSX.Element | null { - const { pathsFilter } = useValues(pathsDataLogic(insightProps)) + const { pathsFilter: _pathsFilter, funnelPathsFilter: _funnelPathsFilter } = useValues(pathsDataLogic(insightProps)) const { updateInsightFilter, openPersonsModal, viewPathToFunnel } = useActions(pathsDataLogic(insightProps)) - const filter = pathsFilter || {} + const pathsFilter = _pathsFilter || {} + const funnelPathsFilter = _funnelPathsFilter || ({} as FunnelPathsFilter) if (!node.visible) { return null @@ -64,7 +66,9 @@ export function PathNodeCard({ insightProps, node }: PathNodeCardProps): JSX.Ele ? node.y0 + PATH_NODE_CARD_TOP_OFFSET : // use middle for end nodes node.y0 + (node.y1 - node.y0) / 2, - border: `1px solid ${isSelectedPathStartOrEnd(filter, node) ? 'purple' : 'var(--border)'}`, + border: `1px solid ${ + isSelectedPathStartOrEnd(pathsFilter, funnelPathsFilter, node) ? 'purple' : 'var(--border)' + }`, }} data-attr="path-node-card-button" > @@ -75,7 +79,7 @@ export function PathNodeCard({ insightProps, node }: PathNodeCardProps): JSX.Ele viewPathToFunnel={viewPathToFunnel} openPersonsModal={openPersonsModal} setFilter={updateInsightFilter} - filter={filter} + filter={pathsFilter} /> diff --git a/frontend/src/scenes/paths/Paths.tsx b/frontend/src/scenes/paths/Paths.tsx index e865923142e17..933f2825567f9 100644 --- a/frontend/src/scenes/paths/Paths.tsx +++ b/frontend/src/scenes/paths/Paths.tsx @@ -6,6 +6,8 @@ import { useEffect, useRef, useState } from 'react' import { InsightEmptyState, InsightErrorState } from 'scenes/insights/EmptyStates' import { insightLogic } from 'scenes/insights/insightLogic' +import { FunnelPathsFilter } from '~/queries/schema' + import { PathNodeCard } from './PathNodeCard' import { pathsDataLogic } from './pathsDataLogic' import type { PathNodeData } from './pathUtils' @@ -23,7 +25,9 @@ export function Paths(): JSX.Element { const [nodeCards, setNodeCards] = useState([]) const { insight, insightProps } = useValues(insightLogic) - const { paths, pathsFilter, insightDataLoading, insightDataError } = useValues(pathsDataLogic(insightProps)) + const { paths, pathsFilter, funnelPathsFilter, insightDataLoading, insightDataError } = useValues( + pathsDataLogic(insightProps) + ) const id = `'${insight?.short_id || DEFAULT_PATHS_ID}'` @@ -35,7 +39,15 @@ export function Paths(): JSX.Element { const elements = document?.getElementById(id)?.querySelectorAll(`.Paths__canvas`) elements?.forEach((node) => node?.parentNode?.removeChild(node)) - renderPaths(canvasRef, canvasWidth, canvasHeight, paths, pathsFilter || {}, setNodeCards) + renderPaths( + canvasRef, + canvasWidth, + canvasHeight, + paths, + pathsFilter || {}, + funnelPathsFilter || ({} as FunnelPathsFilter), + setNodeCards + ) }, [paths, !insightDataLoading, canvasWidth, canvasHeight]) if (insightDataError) { diff --git a/frontend/src/scenes/paths/pathUtils.ts b/frontend/src/scenes/paths/pathUtils.ts index fd0336483d3c7..16b82d61b2395 100644 --- a/frontend/src/scenes/paths/pathUtils.ts +++ b/frontend/src/scenes/paths/pathUtils.ts @@ -1,6 +1,6 @@ import { RGBColor } from 'd3' -import { PathsFilter } from '~/queries/schema' +import { FunnelPathsFilter, PathsFilter } from '~/queries/schema' import { FunnelPathType } from '~/types' export interface PathTargetLink { @@ -114,16 +114,22 @@ export function pageUrl(d: PathNodeData, display?: boolean): string { : name } -export const isSelectedPathStartOrEnd = (pathsFilter: PathsFilter, pathItemCard: PathNodeData): boolean => { +export const isSelectedPathStartOrEnd = ( + pathsFilter: PathsFilter, + funnelPathsFilter: FunnelPathsFilter, + pathItemCard: PathNodeData +): boolean => { const cardName = pageUrl(pathItemCard) const isPathStart = pathItemCard.targetLinks.length === 0 const isPathEnd = pathItemCard.sourceLinks.length === 0 - const { startPoint, endPoint, funnelPaths, funnelFilter } = pathsFilter + const { startPoint, endPoint } = pathsFilter + const { funnelPathType, funnelSource, funnelStep } = funnelPathsFilter || {} + return ( (startPoint === cardName && isPathStart) || (endPoint === cardName && isPathEnd) || - (funnelPaths === FunnelPathType.between && - ((cardName === funnelFilter?.events[funnelFilter.funnel_step - 1].name && isPathEnd) || - (cardName === funnelFilter?.events[funnelFilter.funnel_step - 2].name && isPathStart))) + (funnelPathType === FunnelPathType.between && + ((cardName === funnelSource?.series[funnelStep! - 1].name && isPathEnd) || + (cardName === funnelSource?.series[funnelStep! - 2].name && isPathStart))) ) } diff --git a/frontend/src/scenes/paths/renderPaths.ts b/frontend/src/scenes/paths/renderPaths.ts index f30a0c28346e1..2e8c6bbe6cafd 100644 --- a/frontend/src/scenes/paths/renderPaths.ts +++ b/frontend/src/scenes/paths/renderPaths.ts @@ -4,7 +4,7 @@ import { D3Selector } from 'lib/hooks/useD3' import { stripHTTP } from 'lib/utils' import { Dispatch, RefObject, SetStateAction } from 'react' -import { PathsFilter } from '~/queries/schema' +import { FunnelPathsFilter, PathsFilter } from '~/queries/schema' import { FALLBACK_CANVAS_WIDTH, HIDE_PATH_CARD_HEIGHT } from './Paths' import { PathNode } from './pathsDataLogic' @@ -34,6 +34,7 @@ const appendPathNodes = ( svg: any, nodes: PathNodeData[], pathsFilter: PathsFilter, + funnelPathsFilter: FunnelPathsFilter, setNodeCards: Dispatch> ): void => { svg.append('g') @@ -62,7 +63,7 @@ const appendPathNodes = ( } } } - if (isSelectedPathStartOrEnd(pathsFilter, d)) { + if (isSelectedPathStartOrEnd(pathsFilter, funnelPathsFilter, d)) { return d3.color('purple') } const startNodeColor = c && d3.color(c) ? d3.color(c) : d3.color('#5375ff') @@ -201,6 +202,7 @@ export function renderPaths( canvasHeight: number, paths: { links: PathNode[]; nodes: any[] }, pathsFilter: PathsFilter, + funnelPathsFilter: FunnelPathsFilter, setNodeCards: Dispatch> ): void { if (!paths || paths.nodes.length === 0) { @@ -227,7 +229,7 @@ export function renderPaths( setNodeCards(nodes.map((node: PathNodeData) => ({ ...node, visible: node.y1 - node.y0 > HIDE_PATH_CARD_HEIGHT }))) - appendPathNodes(svg, nodes, pathsFilter, setNodeCards) + appendPathNodes(svg, nodes, pathsFilter, funnelPathsFilter, setNodeCards) appendDropoffs(svg) appendPathLinks(svg, links, nodes, setNodeCards) addChartAxisLines(svg, height, nodes, maxLayer) From 5ff98e1c9b7f22f5ffd196223ba568bae401bc7d Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Wed, 3 Apr 2024 15:41:57 +0200 Subject: [PATCH 21/21] Fix query not working and adjust display of start and end --- .../scenes/insights/EditorFilters/PathsTarget.tsx | 12 ++---------- .../scenes/insights/InsightNav/insightNavLogic.tsx | 6 ++++++ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx index e864588381a45..27c7c68ad0fc5 100644 --- a/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx +++ b/frontend/src/scenes/insights/EditorFilters/PathsTarget.tsx @@ -44,16 +44,8 @@ function PathsTarget({ position, insightProps }: PathTargetProps): JSX.Element { } as Partial) } - function _getStepNameAtIndex(filters: Record, index: number): string { - const targetEntity = - filters.events?.filter((event: Record) => { - return event.order === index - 1 - })?.[0] || - filters.actions?.filter((action: Record) => { - return action.order === index - 1 - })?.[0] - - return targetEntity?.name || '' + function _getStepNameAtIndex(filters: FunnelsQuery, index: number): string { + return filters.series[index - 1].name ?? '' } function _getStepLabel(funnelSource?: FunnelsQuery, index?: number, shift: number = 0): JSX.Element { diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx index 9dff8c87eeb8c..15a58a8c13e1d 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx @@ -38,6 +38,7 @@ import { isInsightQueryWithSeries, isInsightVizNode, isLifecycleQuery, + isPathsQuery, isRetentionQuery, isStickinessQuery, isTrendsQuery, @@ -385,6 +386,11 @@ const mergeCachedProperties = (query: InsightQueryNode, cache: QueryPropertyCach mergedQuery.breakdownFilter = cache.breakdownFilter } + // funnel paths filter + if (isPathsQuery(mergedQuery) && cache.funnelPathsFilter) { + mergedQuery.funnelPathsFilter = cache.funnelPathsFilter + } + // insight specific filter const filterKey = filterKeyForQuery(mergedQuery) if (cache[filterKey] || cache.commonFilter) {