From e634a5c22baef3e564d42900b91eed4edb1a23d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Oberm=C3=BCller?= Date: Mon, 5 Feb 2024 12:59:31 +0100 Subject: [PATCH] feat(insights): improve series and exclusion entity handling (#20089) --- .../queries/nodes/InsightQuery/defaults.ts | 3 +- .../utils/filtersToQueryNode.test.ts | 44 ++-- .../InsightQuery/utils/filtersToQueryNode.ts | 126 +++++++--- .../nodes/InsightQuery/utils/legacy.ts | 9 + .../utils/queryNodeToFilter.test.ts | 93 +++++++- .../InsightQuery/utils/queryNodeToFilter.ts | 5 +- .../queries/nodes/InsightViz/TrendsSeries.tsx | 15 +- frontend/src/queries/schema.json | 224 ++++++++++++++---- frontend/src/queries/schema.ts | 21 +- .../src/scenes/experiments/MetricSelector.tsx | 10 +- .../src/scenes/funnels/funnelUtils.test.ts | 9 +- frontend/src/scenes/funnels/funnelUtils.ts | 7 +- .../EditorFilters/FunnelsQuerySteps.tsx | 4 +- .../insights/EmptyStates/EmptyStates.tsx | 5 +- .../InsightNav/insightNavLogic.test.ts | 1 - .../insights/InsightNav/insightNavLogic.tsx | 71 ++++-- .../filters/ActionFilter/ActionFilter.tsx | 8 +- .../ActionFilterRow/ActionFilterRow.tsx | 8 +- .../ExclusionRowSuffix.tsx | 40 +--- .../FunnelExclusionsFilter.tsx | 12 +- .../scenes/insights/insightVizDataLogic.ts | 14 +- .../notebooks/Notebook/migrations/migrate.ts | 13 +- frontend/src/types.ts | 24 +- .../api/test/__snapshots__/test_api_docs.ambr | 1 - .../legacy_compatibility/filter_to_query.py | 111 ++++++--- .../test/test_filter_to_query.py | 28 ++- posthog/schema.py | 180 +++++++++++--- 27 files changed, 811 insertions(+), 275 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/defaults.ts b/frontend/src/queries/nodes/InsightQuery/defaults.ts index 3580fe50f7821..00f9633c1be38 100644 --- a/frontend/src/queries/nodes/InsightQuery/defaults.ts +++ b/frontend/src/queries/nodes/InsightQuery/defaults.ts @@ -71,7 +71,7 @@ const stickinessQueryDefault: StickinessQuery = { kind: NodeKind.EventsNode, name: '$pageview', event: '$pageview', - math: BaseMathType.TotalCount, + math: BaseMathType.UniqueUsers, }, ], stickinessFilter: {}, @@ -84,7 +84,6 @@ const lifecycleQueryDefault: LifecycleQuery = { kind: NodeKind.EventsNode, name: '$pageview', event: '$pageview', - math: BaseMathType.TotalCount, }, ], } diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index e7a85f6d1394b..b563e9f4f6541 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -1,4 +1,5 @@ import { FunnelLayout, ShownAsValue } from 'lib/constants' +import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow' import { FunnelsQuery, @@ -52,7 +53,7 @@ describe('actionsAndEventsToSeries', () => { { id: '$autocapture', type: 'events', order: 2, name: 'item3' }, ] - const result = actionsAndEventsToSeries({ actions, events }) + const result = actionsAndEventsToSeries({ actions, events }, false, MathAvailability.None) expect(result[0].name).toEqual('item1') expect(result[1].name).toEqual('item2') @@ -66,7 +67,7 @@ describe('actionsAndEventsToSeries', () => { { id: '$autocapture', type: 'events', order: 2, name: 'item2' }, ] - const result = actionsAndEventsToSeries({ actions, events }) + const result = actionsAndEventsToSeries({ actions, events }, false, MathAvailability.None) expect(result[0].name).toEqual('itemWithOrder') expect(result[1].name).toEqual('item1') @@ -76,7 +77,7 @@ describe('actionsAndEventsToSeries', () => { it('assumes typeless series is an event series', () => { const events: ActionFilter[] = [{ id: '$pageview', order: 0, name: 'item1' } as any] - const result = actionsAndEventsToSeries({ events }) + const result = actionsAndEventsToSeries({ events }, false, MathAvailability.None) expect(result[0].kind === NodeKind.EventsNode) }) @@ -362,8 +363,20 @@ describe('filtersToQueryNode', () => { funnel_order_type: StepOrderValue.ORDERED, exclusions: [ { - funnel_from_step: 0, - funnel_to_step: 1, + id: '$pageview', + type: 'events', + order: 0, + name: '$pageview', + funnel_from_step: 1, + funnel_to_step: 2, + }, + { + id: 3, + type: 'actions', + order: 1, + name: 'Some action', + funnel_from_step: 1, + funnel_to_step: 2, }, ], funnel_correlation_person_entity: { a: 1 }, @@ -393,8 +406,18 @@ describe('filtersToQueryNode', () => { funnelOrderType: StepOrderValue.ORDERED, exclusions: [ { - funnelFromStep: 0, - funnelToStep: 1, + event: '$pageview', + kind: NodeKind.EventsNode, + name: '$pageview', + funnelFromStep: 1, + funnelToStep: 2, + }, + { + id: 3, + kind: NodeKind.ActionsNode, + name: 'Some action', + funnelFromStep: 1, + funnelToStep: 2, }, ], layout: FunnelLayout.horizontal, @@ -749,7 +772,6 @@ describe('filtersToQueryNode', () => { kind: NodeKind.ActionsNode, id: 1, name: 'Interacted with file', - math: BaseMathType.TotalCount, }, ], interval: 'day', @@ -1074,7 +1096,6 @@ describe('filtersToQueryNode', () => { }, ], custom_name: 'Viewed homepage', - math: BaseMathType.TotalCount, }, { kind: NodeKind.EventsNode, @@ -1089,14 +1110,12 @@ describe('filtersToQueryNode', () => { }, ], custom_name: 'Viewed signup page', - math: BaseMathType.TotalCount, }, { kind: NodeKind.EventsNode, event: 'signed_up', name: 'signed_up', custom_name: 'Signed up', - math: BaseMathType.TotalCount, }, ], filterTestAccounts: true, @@ -1153,20 +1172,17 @@ describe('filtersToQueryNode', () => { event: 'signed_up', name: 'signed_up', custom_name: 'Signed up', - math: BaseMathType.TotalCount, }, { kind: NodeKind.ActionsNode, id: 1, name: 'Interacted with file', - math: BaseMathType.TotalCount, }, { kind: NodeKind.EventsNode, event: 'upgraded_plan', name: 'upgraded_plan', custom_name: 'Upgraded plan', - math: BaseMathType.TotalCount, }, ], filterTestAccounts: true, diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 19f3008fa98a6..594faeca69d0d 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -1,6 +1,7 @@ import * as Sentry from '@sentry/react' import { objectCleanWithEmpty } from 'lib/utils' import { transformLegacyHiddenLegendKeys } from 'scenes/funnels/funnelUtils' +import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow' import { isFunnelsFilter, isLifecycleFilter, @@ -14,6 +15,8 @@ import { ActionsNode, BreakdownFilter, EventsNode, + FunnelExclusionActionsNode, + FunnelExclusionEventsNode, FunnelsFilter, InsightNodeKind, InsightQueryNode, @@ -38,9 +41,13 @@ import { import { ActionFilter, AnyPropertyFilter, + BaseMathType, FilterLogicalOperator, FilterType, + FunnelExclusionLegacy, FunnelsFilterType, + GroupMathType, + HogQLMathType, InsightType, PathsFilterType, PropertyFilterType, @@ -60,39 +67,83 @@ const reverseInsightMap: Record { +export const legacyEntityToNode = ( + entity: ActionFilter, + includeProperties: boolean, + mathAvailability: MathAvailability +): EventsNode | ActionsNode => { + let shared: Partial = { + name: entity.name || undefined, + custom_name: entity.custom_name || undefined, + } + + if (includeProperties) { + shared = { ...shared, properties: cleanProperties(entity.properties) } as any + } + + if (mathAvailability !== MathAvailability.None) { + // only trends and stickiness insights support math. + // transition to then default math for stickiness, when an unsupported math type is encountered. + if (mathAvailability === MathAvailability.ActorsOnly && !actorsOnlyMathTypes.includes(entity.math as any)) { + shared = { + ...shared, + math: BaseMathType.UniqueUsers, + } + } else { + shared = { + ...shared, + math: entity.math || 'total', + math_property: entity.math_property, + math_hogql: entity.math_hogql, + math_group_type_index: entity.math_group_type_index, + } as any + } + } + + if (entity.type === 'actions') { + return objectCleanWithEmpty({ + kind: NodeKind.ActionsNode, + id: entity.id, + ...shared, + }) as any + } else { + return objectCleanWithEmpty({ + kind: NodeKind.EventsNode, + event: entity.id, + ...shared, + }) as any + } +} + +export const exlusionEntityToNode = ( + entity: FunnelExclusionLegacy +): FunnelExclusionEventsNode | FunnelExclusionActionsNode => { + const baseEntity = legacyEntityToNode(entity as ActionFilter, false, MathAvailability.None) + return { + ...baseEntity, + funnelFromStep: entity.funnel_from_step, + funnelToStep: entity.funnel_to_step, + } +} + +export const actionsAndEventsToSeries = ( + { actions, events, new_entity }: FilterTypeActionsAndEvents, + includeProperties: boolean, + includeMath: MathAvailability +): (EventsNode | ActionsNode)[] => { const series: any = [...(actions || []), ...(events || []), ...(new_entity || [])] .sort((a, b) => (a.order || b.order ? (!a.order ? -1 : !b.order ? 1 : a.order - b.order) : 0)) - .map((f) => { - const shared = objectCleanWithEmpty({ - name: f.name || undefined, - custom_name: f.custom_name, - properties: cleanProperties(f.properties), - math: f.math || 'total', - math_property: f.math_property, - math_hogql: f.math_hogql, - math_group_type_index: f.math_group_type_index, - }) - if (f.type === 'actions') { - return { - kind: NodeKind.ActionsNode, - id: f.id, - ...shared, - } - } else { - return { - kind: NodeKind.EventsNode, - event: f.id, - ...shared, - } - } - }) + .map((f) => legacyEntityToNode(f, includeProperties, includeMath)) return series } @@ -235,9 +286,16 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo // series + interval if (isInsightQueryWithSeries(query)) { + let includeMath = MathAvailability.None + const includeProperties = true + if (isTrendsQuery(query)) { + includeMath = MathAvailability.All + } else if (isStickinessQuery(query)) { + includeMath = MathAvailability.ActorsOnly + } + const { events, actions } = filters - const series = actionsAndEventsToSeries({ actions, events } as any) - query.series = series + query.series = actionsAndEventsToSeries({ actions, events } as any, includeProperties, includeMath) query.interval = filters.interval } @@ -336,11 +394,7 @@ export const funnelsFilterToQuery = (filters: Partial): Funne funnelOrderType: filters.funnel_order_type, exclusions: filters.exclusions !== undefined - ? filters.exclusions.map(({ funnel_from_step, funnel_to_step, ...rest }) => ({ - funnelFromStep: funnel_from_step, - funnelToStep: funnel_to_step, - ...rest, - })) + ? filters.exclusions.map((entity) => exlusionEntityToNode(entity)) : undefined, layout: filters.layout, hidden_legend_breakdowns: cleanHiddenLegendSeries(filters.hidden_legend_keys), diff --git a/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts b/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts index e3b5d73d333d4..b36c90bf36672 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts @@ -40,6 +40,15 @@ export const isLegacyFunnelsFilter = (filters: Record | undefined): return legacyKeys.some((key) => key in filters) } +export const isLegacyFunnelsExclusion = (filters: Record | undefined): boolean => { + if (filters == null) { + return false + } + + const exclusions = filters.exclusions || [] + return exclusions.some((exclusion: Record) => 'type' in exclusion) +} + export const isLegacyRetentionFilter = (filters: Record | undefined): boolean => { if (filters == null) { return false diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts index 332c625daec4c..9b9dd04421962 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts @@ -1,6 +1,19 @@ +import { FunnelLayout } from 'lib/constants' + import { hiddenLegendItemsToKeys, queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' -import { LifecycleQuery, NodeKind, TrendsQuery } from '~/queries/schema' -import { ChartDisplayType, InsightType, LifecycleFilterType, TrendsFilterType } from '~/types' +import { FunnelsQuery, LifecycleQuery, NodeKind, TrendsQuery } from '~/queries/schema' +import { + BreakdownAttributionType, + ChartDisplayType, + FunnelConversionWindowTimeUnit, + FunnelsFilterType, + FunnelStepReference, + FunnelVizType, + InsightType, + LifecycleFilterType, + StepOrderValue, + TrendsFilterType, +} from '~/types' describe('queryNodeToFilter', () => { test('converts a query node to a filter', () => { @@ -89,6 +102,82 @@ describe('queryNodeToFilter', () => { } expect(result).toEqual(filters) }) + + test('converts a funnelsFilter into filter properties', () => { + const query: FunnelsQuery = { + kind: NodeKind.FunnelsQuery, + funnelsFilter: { + funnelVizType: FunnelVizType.Steps, + funnelFromStep: 1, + funnelToStep: 2, + funnelStepReference: FunnelStepReference.total, + breakdownAttributionType: BreakdownAttributionType.AllSteps, + breakdownAttributionValue: 1, + binCount: 'auto', + funnelWindowIntervalUnit: FunnelConversionWindowTimeUnit.Day, + funnelWindowInterval: 7, + funnelOrderType: StepOrderValue.ORDERED, + exclusions: [ + { + event: '$pageview', + kind: NodeKind.EventsNode, + name: '$pageview', + funnelFromStep: 1, + funnelToStep: 2, + }, + { + id: 3, + kind: NodeKind.ActionsNode, + name: 'Some action', + funnelFromStep: 1, + funnelToStep: 2, + }, + ], + layout: FunnelLayout.horizontal, + }, + series: [], + } + + const result = queryNodeToFilter(query) + + const filters: Partial = { + insight: InsightType.FUNNELS, + funnel_viz_type: FunnelVizType.Steps, + funnel_from_step: 1, + funnel_to_step: 2, + funnel_step_reference: FunnelStepReference.total, + breakdown_attribution_type: BreakdownAttributionType.AllSteps, + breakdown_attribution_value: 1, + bin_count: 'auto', + funnel_window_interval_unit: FunnelConversionWindowTimeUnit.Day, + funnel_window_interval: 7, + funnel_order_type: StepOrderValue.ORDERED, + exclusions: [ + { + id: '$pageview', + type: 'events', + order: 0, + name: '$pageview', + funnel_from_step: 1, + funnel_to_step: 2, + }, + { + id: 3, + type: 'actions', + order: 1, + name: 'Some action', + funnel_from_step: 1, + funnel_to_step: 2, + }, + ], + layout: FunnelLayout.horizontal, + interval: undefined, + hidden_legend_keys: undefined, + funnel_aggregate_by_hogql: undefined, + entity_type: 'events', + } + 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 0da74e81849bc..3d50497bf0862 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -182,10 +182,11 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.trendsFilter?.showValuesOnSeries } else if (isFunnelsQuery(queryCopy)) { camelCasedFunnelsProps.exclusions = queryCopy.funnelsFilter?.exclusions - ? queryCopy.funnelsFilter.exclusions.map(({ funnelFromStep, funnelToStep, ...rest }) => ({ + ? queryCopy.funnelsFilter.exclusions.map(({ funnelFromStep, funnelToStep, ...rest }, index) => ({ funnel_from_step: funnelFromStep, funnel_to_step: funnelToStep, - ...rest, + order: index, + ...seriesNodeToFilter(rest), })) : undefined camelCasedFunnelsProps.bin_count = queryCopy.funnelsFilter?.binCount diff --git a/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx b/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx index fa0f8c49d2aef..b7e2c780a3b7a 100644 --- a/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx +++ b/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx @@ -41,6 +41,11 @@ export function TrendsSeries(): JSX.Element | null { } const filters = queryNodeToFilter(querySource) + const mathAvailability = isLifecycle + ? MathAvailability.None + : isStickiness + ? MathAvailability.ActorsOnly + : MathAvailability.All return ( <> @@ -52,7 +57,7 @@ export function TrendsSeries(): JSX.Element | null { ): void => { - updateQuerySource({ series: actionsAndEventsToSeries(payload as any) } as + updateQuerySource({ series: actionsAndEventsToSeries(payload as any, true, mathAvailability) } as | TrendsQuery | FunnelsQuery | StickinessQuery @@ -67,13 +72,7 @@ export function TrendsSeries(): JSX.Element | null { ? 1 : alphabet.length } - mathAvailability={ - isLifecycle - ? MathAvailability.None - : isStickiness - ? MathAvailability.ActorsOnly - : MathAvailability.All - } + mathAvailability={mathAvailability} propertiesTaxonomicGroupTypes={propertiesTaxonomicGroupTypes} /> diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index dd5c94cdf0946..34346526bdad2 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -221,6 +221,16 @@ } ] }, + "AnyEntityNode": { + "anyOf": [ + { + "$ref": "#/definitions/EventsNode" + }, + { + "$ref": "#/definitions/ActionsNode" + } + ] + }, "AnyPropertyFilter": { "anyOf": [ { @@ -1323,33 +1333,183 @@ "type": "string" }, "FunnelExclusion": { + "anyOf": [ + { + "$ref": "#/definitions/FunnelExclusionEventsNode" + }, + { + "$ref": "#/definitions/FunnelExclusionActionsNode" + } + ] + }, + "FunnelExclusionActionsNode": { "additionalProperties": false, "properties": { "custom_name": { - "type": ["string", "null"] + "type": "string" + }, + "fixedProperties": { + "description": "Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person)", + "items": { + "$ref": "#/definitions/AnyPropertyFilter" + }, + "type": "array" }, "funnelFromStep": { - "type": "number" + "type": "integer" }, "funnelToStep": { - "type": "number" + "type": "integer" }, "id": { - "type": ["string", "number", "null"] + "type": "integer" }, - "index": { + "kind": { + "const": "ActionsNode", + "type": "string" + }, + "math": { + "anyOf": [ + { + "$ref": "#/definitions/BaseMathType" + }, + { + "$ref": "#/definitions/PropertyMathType" + }, + { + "$ref": "#/definitions/CountPerActorMathType" + }, + { + "$ref": "#/definitions/GroupMathType" + }, + { + "$ref": "#/definitions/HogQLMathType" + } + ] + }, + "math_group_type_index": { + "enum": [0, 1, 2, 3, 4], "type": "number" }, + "math_hogql": { + "type": "string" + }, + "math_property": { + "type": "string" + }, "name": { + "type": "string" + }, + "properties": { + "description": "Properties configurable in the interface", + "items": { + "$ref": "#/definitions/AnyPropertyFilter" + }, + "type": "array" + }, + "response": { + "description": "Cached query response", + "type": "object" + } + }, + "required": ["id", "kind"], + "type": "object" + }, + "FunnelExclusionEventsNode": { + "additionalProperties": false, + "properties": { + "custom_name": { + "type": "string" + }, + "event": { + "description": "The event or `null` for all events.", "type": ["string", "null"] }, - "order": { + "fixedProperties": { + "description": "Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person)", + "items": { + "$ref": "#/definitions/AnyPropertyFilter" + }, + "type": "array" + }, + "funnelFromStep": { + "type": "integer" + }, + "funnelToStep": { + "type": "integer" + }, + "kind": { + "const": "EventsNode", + "type": "string" + }, + "limit": { + "type": "integer" + }, + "math": { + "anyOf": [ + { + "$ref": "#/definitions/BaseMathType" + }, + { + "$ref": "#/definitions/PropertyMathType" + }, + { + "$ref": "#/definitions/CountPerActorMathType" + }, + { + "$ref": "#/definitions/GroupMathType" + }, + { + "$ref": "#/definitions/HogQLMathType" + } + ] + }, + "math_group_type_index": { + "enum": [0, 1, 2, 3, 4], "type": "number" }, - "type": { - "$ref": "#/definitions/EntityType" + "math_hogql": { + "type": "string" + }, + "math_property": { + "type": "string" + }, + "name": { + "type": "string" + }, + "orderBy": { + "description": "Columns to order by", + "items": { + "type": "string" + }, + "type": "array" + }, + "properties": { + "description": "Properties configurable in the interface", + "items": { + "$ref": "#/definitions/AnyPropertyFilter" + }, + "type": "array" + }, + "response": { + "additionalProperties": false, + "description": "Return a limited set of data", + "properties": { + "next": { + "type": "string" + }, + "results": { + "items": { + "$ref": "#/definitions/EventType" + }, + "type": "array" + } + }, + "required": ["results"], + "type": "object" } }, + "required": ["kind"], "type": "object" }, "FunnelExclusionLegacy": { @@ -1382,6 +1542,18 @@ }, "type": "object" }, + "FunnelExclusionSteps": { + "additionalProperties": false, + "properties": { + "funnelFromStep": { + "type": "integer" + }, + "funnelToStep": { + "type": "integer" + } + }, + "type": "object" + }, "FunnelLayout": { "enum": ["horizontal", "vertical"], "type": "string" @@ -1559,14 +1731,7 @@ "series": { "description": "Events and actions to include", "items": { - "anyOf": [ - { - "$ref": "#/definitions/EventsNode" - }, - { - "$ref": "#/definitions/ActionsNode" - } - ] + "$ref": "#/definitions/AnyEntityNode" }, "type": "array" } @@ -2353,14 +2518,7 @@ "series": { "description": "Events and actions to include", "items": { - "anyOf": [ - { - "$ref": "#/definitions/EventsNode" - }, - { - "$ref": "#/definitions/ActionsNode" - } - ] + "$ref": "#/definitions/AnyEntityNode" }, "type": "array" } @@ -4197,14 +4355,7 @@ "series": { "description": "Events and actions to include", "items": { - "anyOf": [ - { - "$ref": "#/definitions/EventsNode" - }, - { - "$ref": "#/definitions/ActionsNode" - } - ] + "$ref": "#/definitions/AnyEntityNode" }, "type": "array" }, @@ -4526,14 +4677,7 @@ "series": { "description": "Events and actions to include", "items": { - "anyOf": [ - { - "$ref": "#/definitions/EventsNode" - }, - { - "$ref": "#/definitions/ActionsNode" - } - ] + "$ref": "#/definitions/AnyEntityNode" }, "type": "array" }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 2719878250da9..98189913e031b 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -9,7 +9,6 @@ import { EventPropertyFilter, EventType, FilterType, - FunnelExclusion, FunnelsFilterType, GroupMathType, HogQLMathType, @@ -281,6 +280,8 @@ export interface ActionsNode extends EntityNode { id: number } +export type AnyEntityNode = EventsNode | ActionsNode + export interface QueryTiming { /** Key. Shortened to 'k' to save on data. */ k: string @@ -548,7 +549,7 @@ export interface TrendsQuery extends InsightsQueryBase { /** Granularity of the response. Can be one of `hour`, `day`, `week` or `month` */ interval?: IntervalType /** Events and actions to include */ - series: (EventsNode | ActionsNode)[] + series: AnyEntityNode[] /** Properties specific to the trends insight */ trendsFilter?: TrendsFilter /** Breakdown of the events and actions */ @@ -571,6 +572,16 @@ export type FunnelsFilterLegacy = Omit< | 'funnel_custom_steps' > +export interface FunnelExclusionSteps { + /** @asType integer */ + funnelFromStep?: number + /** @asType integer */ + funnelToStep?: number +} +export interface FunnelExclusionEventsNode extends EventsNode, FunnelExclusionSteps {} +export interface FunnelExclusionActionsNode extends ActionsNode, FunnelExclusionSteps {} +export type FunnelExclusion = FunnelExclusionEventsNode | FunnelExclusionActionsNode + export type FunnelsFilter = { exclusions?: FunnelExclusion[] layout?: FunnelsFilterLegacy['layout'] @@ -593,7 +604,7 @@ export interface FunnelsQuery extends InsightsQueryBase { /** Granularity of the response. Can be one of `hour`, `day`, `week` or `month` */ interval?: IntervalType /** Events and actions to include */ - series: (EventsNode | ActionsNode)[] + series: AnyEntityNode[] /** Properties specific to the funnels insight */ funnelsFilter?: FunnelsFilter /** Breakdown of the events and actions */ @@ -702,7 +713,7 @@ export interface StickinessQuery extends Omit): void => { - updateQuerySource({ series: actionsAndEventsToSeries(payload as any) } as - | TrendsQuery - | FunnelsQuery) + updateQuerySource({ + series: actionsAndEventsToSeries( + payload as any, + true, + isTrends ? MathAvailability.All : MathAvailability.None + ), + } as TrendsQuery | FunnelsQuery) }} typeKey={`experiment-${isTrends ? InsightType.TRENDS : InsightType.FUNNELS}-secondary-metric`} mathAvailability={isTrends ? undefined : MathAvailability.None} diff --git a/frontend/src/scenes/funnels/funnelUtils.test.ts b/frontend/src/scenes/funnels/funnelUtils.test.ts index 0c83d4f0a826a..bd638215192a0 100644 --- a/frontend/src/scenes/funnels/funnelUtils.test.ts +++ b/frontend/src/scenes/funnels/funnelUtils.test.ts @@ -1,12 +1,11 @@ import { dayjs } from 'lib/dayjs' -import { EventsNode, FunnelsQuery, NodeKind } from '~/queries/schema' +import { EventsNode, FunnelExclusionSteps, FunnelsQuery, NodeKind } from '~/queries/schema' import { FunnelConversionWindowTimeUnit, FunnelCorrelation, FunnelCorrelationResultsType, FunnelCorrelationType, - FunnelExclusion, } from '~/types' import { @@ -174,7 +173,7 @@ describe('getIncompleteConversionWindowStartDate()', () => { describe('getClampedStepRange', () => { it('prefers step range to existing filters', () => { - const stepRange: FunnelExclusion = { + const stepRange: FunnelExclusionSteps = { funnelFromStep: 0, funnelToStep: 1, } @@ -197,7 +196,7 @@ describe('getClampedStepRange', () => { }) it('ensures step range is clamped to step range', () => { - const stepRange: FunnelExclusion = {} + const stepRange: FunnelExclusionSteps = {} const query: FunnelsQuery = { kind: NodeKind.FunnelsQuery, funnelsFilter: { @@ -218,7 +217,7 @@ describe('getClampedStepRange', () => { }) it('returns undefined if the incoming filters are undefined', () => { - const stepRange: FunnelExclusion = {} + const stepRange: FunnelExclusionSteps = {} const query: FunnelsQuery = { kind: NodeKind.FunnelsQuery, funnelsFilter: { diff --git a/frontend/src/scenes/funnels/funnelUtils.ts b/frontend/src/scenes/funnels/funnelUtils.ts index 1623a0d87f1e6..373b01edcab6e 100644 --- a/frontend/src/scenes/funnels/funnelUtils.ts +++ b/frontend/src/scenes/funnels/funnelUtils.ts @@ -6,7 +6,7 @@ import { elementsToAction } from 'scenes/events/createActionFromEvent' import { teamLogic } from 'scenes/teamLogic' import { Noun } from '~/models/groupsModel' -import { FunnelsQuery } from '~/queries/schema' +import { FunnelExclusionSteps, FunnelsQuery } from '~/queries/schema' import { AnyPropertyFilter, Breakdown, @@ -17,7 +17,6 @@ import { FunnelConversionWindow, FunnelCorrelation, FunnelCorrelationResultsType, - FunnelExclusion, FunnelResultType, FunnelStep, FunnelStepReference, @@ -226,9 +225,9 @@ export const getClampedStepRange = ({ stepRange, query, }: { - stepRange?: FunnelExclusion + stepRange?: FunnelExclusionSteps query: FunnelsQuery -}): FunnelExclusion => { +}): FunnelExclusionSteps => { const maxStepIndex = Math.max((query.series.length || 0) - 1, 1) let funnelFromStep = findFirstNumber([stepRange?.funnelFromStep, query.funnelsFilter?.funnelFromStep]) diff --git a/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx b/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx index 265dbfa4267c7..7f87def330757 100644 --- a/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx +++ b/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx @@ -29,7 +29,9 @@ export function FunnelsQuerySteps({ insightProps }: EditorFilterProps): JSX.Elem const actionFilters = queryNodeToFilter(querySource) const setActionFilters = (payload: Partial): void => { - updateQuerySource({ series: actionsAndEventsToSeries(payload as any) } as FunnelsQuery) + updateQuerySource({ + series: actionsAndEventsToSeries(payload as any, true, MathAvailability.None), + } as FunnelsQuery) } const { groupsTaxonomicTypes, showGroupsOptions } = useValues(groupsModel) diff --git a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx index 3d301dd7b882a..8950d4e00834b 100644 --- a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx +++ b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx @@ -25,6 +25,7 @@ import { FunnelsQuery } from '~/queries/schema' import { FilterType, InsightLogicProps, SavedInsightsTabs } from '~/types' import { samplingFilterLogic } from '../EditorFilters/samplingFilterLogic' +import { MathAvailability } from '../filters/ActionFilter/ActionFilterRow/ActionFilterRow' export function InsightEmptyState({ heading = 'There are no matching events for this query', @@ -194,7 +195,9 @@ export function FunnelSingleStepState({ actionable = true }: FunnelSingleStepSta const filters = series ? seriesToActionsAndEvents(series) : {} const setFilters = (payload: Partial): void => { - updateQuerySource({ series: actionsAndEventsToSeries(payload as any) } as Partial) + updateQuerySource({ + series: actionsAndEventsToSeries(payload as any, true, MathAvailability.None), + } as Partial) } const { addFilter } = useActions(entityFilterLogic({ setFilters, filters, typeKey: 'EditFunnel-action' })) diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts b/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts index 3babb46e20b34..53e1dff4493ed 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts @@ -71,7 +71,6 @@ describe('insightNavLogic', () => { { event: '$pageview', kind: 'EventsNode', - math: 'total', name: '$pageview', }, ], diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx index a857419ad25af..2028279e40573 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx @@ -37,9 +37,12 @@ import { isInsightVizNode, isLifecycleQuery, isRetentionQuery, + isStickinessQuery, + isTrendsQuery, } from '~/queries/utils' -import { InsightLogicProps, InsightType } from '~/types' +import { BaseMathType, InsightLogicProps, InsightType } from '~/types' +import { MathAvailability } from '../filters/ActionFilter/ActionFilterRow/ActionFilterRow' import type { insightNavLogicType } from './insightNavLogicType' export interface Tab { @@ -66,6 +69,34 @@ export interface QueryPropertyCache commonFilter: CommonInsightFilter } +const cleanSeriesEntityMath = ( + entity: EventsNode | ActionsNode, + mathAvailability: MathAvailability +): EventsNode | ActionsNode => { + const { math, math_property, math_group_type_index, math_hogql, ...baseEntity } = entity + + // TODO: This should be improved to keep a math that differs from the default. + // For this we need to know wether the math was actively changed e.g. + // On which insight type the math properties have been set. + if (mathAvailability === MathAvailability.All) { + // return entity with default all availability math set + return { ...baseEntity, math: BaseMathType.TotalCount } + } else if (mathAvailability === MathAvailability.ActorsOnly) { + // return entity with default actors only availability math set + return { ...baseEntity, math: BaseMathType.UniqueUsers } + } else { + // return entity without math properties for insights that don't support it + return baseEntity + } +} + +const cleanSeriesMath = ( + series: (EventsNode | ActionsNode)[], + mathAvailability: MathAvailability +): (EventsNode | ActionsNode)[] => { + return series.map((entity) => cleanSeriesEntityMath(entity, mathAvailability)) +} + export const insightNavLogic = kea([ props({} as InsightLogicProps), key(keyForInsightLogicProps('new')), @@ -297,26 +328,32 @@ const mergeCachedProperties = (query: InsightQueryNode, cache: QueryPropertyCach if (isInsightQueryWithSeries(mergedQuery)) { if (cache.series) { if (isLifecycleQuery(mergedQuery)) { - mergedQuery.series = cache.series.slice(0, 1) + mergedQuery.series = cleanSeriesMath(cache.series.slice(0, 1), MathAvailability.None) } else { - mergedQuery.series = cache.series + const mathAvailability = isTrendsQuery(mergedQuery) + ? MathAvailability.All + : isStickinessQuery(mergedQuery) + ? MathAvailability.ActorsOnly + : MathAvailability.None + mergedQuery.series = cleanSeriesMath(cache.series, mathAvailability) } - } else if (cache.retentionFilter?.targetEntity || cache.retentionFilter?.returningEntity) { - mergedQuery.series = [ - ...(cache.retentionFilter.targetEntity - ? [cache.retentionFilter.targetEntity as EventsNode | ActionsNode] - : []), - ...(cache.retentionFilter.returningEntity - ? [cache.retentionFilter.returningEntity as EventsNode | ActionsNode] - : []), - ] } + // else if (cache.retentionFilter?.targetEntity || cache.retentionFilter?.returningEntity) { + // mergedQuery.series = [ + // ...(cache.retentionFilter.targetEntity + // ? [cache.retentionFilter.targetEntity as EventsNode | ActionsNode] + // : []), + // ...(cache.retentionFilter.returningEntity + // ? [cache.retentionFilter.returningEntity as EventsNode | ActionsNode] + // : []), + // ] + // } } else if (isRetentionQuery(mergedQuery) && cache.series) { - mergedQuery.retentionFilter = { - ...mergedQuery.retentionFilter, - ...(cache.series.length > 0 ? { targetEntity: cache.series[0] } : {}), - ...(cache.series.length > 1 ? { returningEntity: cache.series[1] } : {}), - } + // mergedQuery.retentionFilter = { + // ...mergedQuery.retentionFilter, + // ...(cache.series.length > 0 ? { targetEntity: cache.series[0] } : {}), + // ...(cache.series.length > 1 ? { returningEntity: cache.series[1] } : {}), + // } } // interval diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx index b6561c9cff1aa..771032e58eb8c 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx @@ -13,7 +13,7 @@ import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import React, { useEffect } from 'react' import { RenameModal } from 'scenes/insights/filters/ActionFilter/RenameModal' -import { ActionFilter as ActionFilterType, FilterType, FunnelExclusion, InsightType, Optional } from '~/types' +import { ActionFilter as ActionFilterType, FilterType, FunnelExclusionLegacy, InsightType, Optional } from '~/types' import { teamLogic } from '../../../teamLogic' import { ActionFilterRow, MathAvailability } from './ActionFilterRow/ActionFilterRow' @@ -53,7 +53,11 @@ export interface ActionFilterProps { customRowSuffix?: | string | JSX.Element - | ((props: { filter: ActionFilterType | FunnelExclusion; index: number; onClose: () => void }) => JSX.Element) + | ((props: { + filter: ActionFilterType | FunnelExclusionLegacy + index: number + onClose: () => void + }) => JSX.Element) /** Show nested arrows to the left of property filter buttons */ showNestedArrow?: boolean /** Which tabs to show for actions selector */ diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx index 23558681d74cf..b445efbfd1b66 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx @@ -40,7 +40,7 @@ import { CountPerActorMathType, EntityType, EntityTypes, - FunnelExclusion, + FunnelExclusionLegacy, HogQLMathType, PropertyFilterValue, PropertyMathType, @@ -94,7 +94,11 @@ export interface ActionFilterRowProps { customRowSuffix?: | string | JSX.Element - | ((props: { filter: ActionFilterType | FunnelExclusion; index: number; onClose: () => void }) => JSX.Element) // Custom suffix element to show in each row + | ((props: { + filter: ActionFilterType | FunnelExclusionLegacy + index: number + onClose: () => void + }) => JSX.Element) // Custom suffix element to show in each row hasBreakdown: boolean // Whether the current graph has a breakdown filter applied showNestedArrow?: boolean // Show nested arrows to the left of property filter buttons actionsTaxonomicGroupTypes?: TaxonomicFilterGroupType[] // Which tabs to show for actions selector diff --git a/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/ExclusionRowSuffix.tsx b/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/ExclusionRowSuffix.tsx index 99189d392ad95..0297f28bdf4c8 100644 --- a/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/ExclusionRowSuffix.tsx +++ b/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/ExclusionRowSuffix.tsx @@ -6,44 +6,26 @@ import { getClampedStepRange } from 'scenes/funnels/funnelUtils' import { insightLogic } from 'scenes/insights/insightLogic' import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' -import { FunnelsFilter, FunnelsQuery } from '~/queries/schema' -import { ActionFilter as ActionFilterType, FunnelExclusion } from '~/types' +import { FunnelsQuery } from '~/queries/schema' type ExclusionRowSuffixComponentBaseProps = { - filter: ActionFilterType | FunnelExclusion index: number onClose?: () => void isVertical: boolean } export function ExclusionRowSuffix({ - filter, index, onClose, isVertical, }: ExclusionRowSuffixComponentBaseProps): JSX.Element | null { const { insightProps } = useValues(insightLogic) - const { querySource, insightFilter, series, isFunnelWithEnoughSteps, exclusionDefaultStepRange } = useValues( + const { querySource, funnelsFilter, series, isFunnelWithEnoughSteps, exclusionDefaultStepRange } = useValues( insightVizDataLogic(insightProps) ) const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps)) - const setOneEventExclusionFilter = (eventFilter: FunnelExclusion, index: number): void => { - const exclusions = ((insightFilter as FunnelsFilter)?.exclusions || []).map((e, e_i) => - e_i === index - ? getClampedStepRange({ - stepRange: eventFilter, - query: querySource as FunnelsQuery, - }) - : e - ) - - updateInsightFilter({ - exclusions, - }) - } - - const exclusions = (insightFilter as FunnelsFilter)?.exclusions + const exclusions = funnelsFilter?.exclusions const numberOfSeries = series?.length || 0 const stepRange = { @@ -55,14 +37,14 @@ export function ExclusionRowSuffix({ funnelFromStep: number | undefined = stepRange.funnelFromStep, funnelToStep: number | undefined = stepRange.funnelToStep ): void => { - setOneEventExclusionFilter( - { - ...filter, - funnelFromStep, - funnelToStep, - }, - index + const newStepRange = getClampedStepRange({ + stepRange: { funnelFromStep, funnelToStep }, + query: querySource as FunnelsQuery, + }) + const newExclusions = funnelsFilter?.exclusions?.map((exclusion, exclusionIndex) => + exclusionIndex === index ? { ...exclusion, ...newStepRange } : exclusion ) + updateInsightFilter({ exclusions: newExclusions }) } return ( @@ -90,10 +72,12 @@ export function ExclusionRowSuffix({ disabled={!isFunnelWithEnoughSteps} /> } onClick={onClose} data-attr="delete-prop-exclusion-filter" title="Delete event exclusion series" + className="ml-1" /> ) diff --git a/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/FunnelExclusionsFilter.tsx b/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/FunnelExclusionsFilter.tsx index 5641541a242be..cc0664f972989 100644 --- a/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/FunnelExclusionsFilter.tsx +++ b/frontend/src/scenes/insights/filters/FunnelExclusionsFilter/FunnelExclusionsFilter.tsx @@ -8,7 +8,8 @@ import { insightLogic } from 'scenes/insights/insightLogic' import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils' -import { EntityTypes, FilterType, FunnelExclusion } from '~/types' +import { legacyEntityToNode } from '~/queries/nodes/InsightQuery/utils/filtersToQueryNode' +import { ActionFilter as ActionFilterType, EntityTypes, FilterType, FunnelExclusionLegacy } from '~/types' import { ExclusionRow } from './ExclusionRow' import { ExclusionRowSuffix } from './ExclusionRowSuffix' @@ -25,11 +26,10 @@ export function FunnelExclusionsFilter(): JSX.Element { const isVerticalLayout = !!width && width < 450 // If filter container shrinks below 500px, initiate verticality const setFilters = (filters: Partial): void => { - const exclusions = (filters.events as FunnelExclusion[]).map((e) => ({ - ...e, - funnelFromStep: e.funnelFromStep || exclusionDefaultStepRange.funnelFromStep, - funnelToStep: e.funnelToStep || exclusionDefaultStepRange.funnelToStep, - })) + const exclusions = filters.events?.map((entity: FunnelExclusionLegacy) => { + const baseEntity = legacyEntityToNode(entity as ActionFilterType, false, MathAvailability.None) + return { ...baseEntity, funnelFromStep: entity.funnel_from_step, funnelToStep: entity.funnel_to_step } + }) updateInsightFilter({ exclusions }) } diff --git a/frontend/src/scenes/insights/insightVizDataLogic.ts b/frontend/src/scenes/insights/insightVizDataLogic.ts index 269fadbb21570..e0a07f15d0b55 100644 --- a/frontend/src/scenes/insights/insightVizDataLogic.ts +++ b/frontend/src/scenes/insights/insightVizDataLogic.ts @@ -17,7 +17,7 @@ import { sceneLogic } from 'scenes/sceneLogic' import { filterTestAccountsDefaultsLogic } from 'scenes/settings/project/filterTestAccountDefaultsLogic' import { BASE_MATH_DEFINITIONS } from 'scenes/trends/mathsLogic' -import { queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' +import { queryNodeToFilter, seriesNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' import { getBreakdown, getCompare, @@ -33,6 +33,7 @@ import { import { BreakdownFilter, DateRange, + FunnelExclusionSteps, FunnelsQuery, InsightFilter, InsightQueryNode, @@ -55,7 +56,7 @@ import { isTrendsQuery, nodeKindToFilterProperty, } from '~/queries/utils' -import { BaseMathType, ChartDisplayType, FilterType, FunnelExclusion, InsightLogicProps, IntervalType } from '~/types' +import { BaseMathType, ChartDisplayType, FilterType, InsightLogicProps, IntervalType } from '~/types' import { insightLogic } from './insightLogic' import type { insightVizDataLogicType } from './insightVizDataLogicType' @@ -297,7 +298,7 @@ export const insightVizDataLogic = kea([ // Exclusion filters exclusionDefaultStepRange: [ (s) => [s.querySource], - (querySource: FunnelsQuery): Omit => ({ + (querySource: FunnelsQuery): FunnelExclusionSteps => ({ funnelFromStep: 0, funnelToStep: (querySource.series || []).length > 1 ? querySource.series.length - 1 : 1, }), @@ -305,7 +306,12 @@ export const insightVizDataLogic = kea([ exclusionFilters: [ (s) => [s.funnelsFilter], (funnelsFilter): FilterType => ({ - events: funnelsFilter?.exclusions, + events: funnelsFilter?.exclusions?.map(({ funnelFromStep, funnelToStep, ...rest }, index) => ({ + funnel_from_step: funnelFromStep, + funnel_to_step: funnelToStep, + order: index, + ...seriesNodeToFilter(rest), + })), }), ], }), diff --git a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts index e44cd04bffc7e..544e96057e16c 100644 --- a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts +++ b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts @@ -2,6 +2,7 @@ import { JSONContent } from '@tiptap/core' import { breakdownFilterToQuery, + exlusionEntityToNode, funnelsFilterToQuery, lifecycleFilterToQuery, pathsFilterToQuery, @@ -10,6 +11,7 @@ import { trendsFilterToQuery, } from '~/queries/nodes/InsightQuery/utils/filtersToQueryNode' import { + isLegacyFunnelsExclusion, isLegacyFunnelsFilter, isLegacyLifecycleFilter, isLegacyPathsFilter, @@ -103,8 +105,15 @@ function convertInsightQueriesToNewSchema(content: JSONContent[]): JSONContent[] query.trendsFilter = trendsFilterToQuery(query.trendsFilter as any) } - if (query.kind === NodeKind.FunnelsQuery && isLegacyFunnelsFilter(query.funnelsFilter as any)) { - query.funnelsFilter = funnelsFilterToQuery(query.funnelsFilter as any) + if (query.kind === NodeKind.FunnelsQuery) { + if (isLegacyFunnelsFilter(query.funnelsFilter as any)) { + query.funnelsFilter = funnelsFilterToQuery(query.funnelsFilter as any) + } else if (isLegacyFunnelsExclusion(query.funnelsFilter as any)) { + query.funnelsFilter = { + ...query.funnelsFilter, + exclusions: query.funnelsFilter!.exclusions!.map((entity) => exlusionEntityToNode(entity)), + } + } } if (query.kind === NodeKind.RetentionQuery && isLegacyRetentionFilter(query.retentionFilter as any)) { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index ce64eb5ba3c8a..534cb8d7c531a 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -890,16 +890,21 @@ export type EntityFilter = { order?: number } +export interface ActionFilter extends EntityFilter { + math?: string + math_property?: string + math_group_type_index?: number | null + math_hogql?: string + properties?: AnyPropertyFilter[] + type: EntityType + days?: string[] // TODO: why was this added here? +} + export interface FunnelExclusionLegacy extends Partial { funnel_from_step?: number funnel_to_step?: number } -export interface FunnelExclusion extends Partial { - funnelFromStep?: number - funnelToStep?: number -} - export type EntityFilterTypes = EntityFilter | ActionFilter | null export interface PersonType { @@ -2112,15 +2117,6 @@ export interface SystemStatusAnalyzeResult { flamegraphs: Record } -export interface ActionFilter extends EntityFilter { - days?: string[] - math?: string - math_property?: string - math_group_type_index?: number | null - math_hogql?: string - properties?: AnyPropertyFilter[] - type: EntityType -} export interface TrendAPIResponse { type: 'Trends' is_cached: boolean diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr index 0a372c02bc193..c59eda95b7757 100644 --- a/posthog/api/test/__snapshots__/test_api_docs.ambr +++ b/posthog/api/test/__snapshots__/test_api_docs.ambr @@ -142,7 +142,6 @@ "/home/runner/work/posthog/posthog/posthog/api/property_definition.py: Warning [PropertyDefinitionViewSet]: could not resolve authenticator . There was no OpenApiAuthenticationExtension registered for that class. Try creating one by subclassing it. Ignoring for now.", '/home/runner/work/posthog/posthog/posthog/api/property_definition.py: Warning [PropertyDefinitionViewSet]: could not derive type of path parameter "id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. ) or annotating the parameter type with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/posthog/api/query.py: Warning [QueryViewSet]: could not derive type of path parameter "project_id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. ) or annotating the parameter type with @extend_schema. Defaulting to "string".', - '/home/runner/.virtualenvs/.venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py: Warning [QueryViewSet > ModelMetaclass]: Encountered 2 components with identical names "FunnelExclusion" and different classes and . This will very likely result in an incorrect schema. Try renaming one.', '/home/runner/.virtualenvs/.venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py: Warning [QueryViewSet > ModelMetaclass]: Encountered 2 components with identical names "Person" and different classes and . This will very likely result in an incorrect schema. Try renaming one.', "/home/runner/work/posthog/posthog/posthog/api/query.py: Warning [QueryViewSet]: could not resolve authenticator . There was no OpenApiAuthenticationExtension registered for that class. Try creating one by subclassing it. Ignoring for now.", '/home/runner/work/posthog/posthog/posthog/api/query.py: Warning [QueryViewSet]: could not derive type of path parameter "id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. ) or annotating the parameter type with @extend_schema. Defaulting to "string".', diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index b5ec1c7bb648d..55880fce903ca 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -1,13 +1,16 @@ +from enum import Enum import json -from typing import List, Dict -from posthog.models.entity.entity import Entity as BackendEntity +from typing import List, Dict, Literal +from posthog.models.entity.entity import Entity as LegacyEntity from posthog.schema import ( ActionsNode, + BaseMathType, BreakdownFilter, ChartDisplayType, DateRange, EventsNode, - FunnelExclusion, + FunnelExclusionActionsNode, + FunnelExclusionEventsNode, FunnelsFilter, FunnelsQuery, LifecycleFilter, @@ -25,6 +28,21 @@ from posthog.types import InsightQueryNode +class MathAvailability(str, Enum): + Unavailable = ("Unavailable",) + All = ("All",) + ActorsOnly = "ActorsOnly" + + +actors_only_math_types = [ + BaseMathType.dau, + BaseMathType.weekly_active, + BaseMathType.monthly_active, + Literal["unique_group"], + Literal["hogql"], +] + + def is_property_with_operator(property: Dict): return property.get("type") not in ("cohort", "hogql") @@ -114,7 +132,12 @@ def clean_display(display: str): return display -def entity_to_node(entity: BackendEntity, include_properties: bool, include_math: bool) -> EventsNode | ActionsNode: +def legacy_entity_to_node( + entity: LegacyEntity, include_properties: bool, math_availability: MathAvailability +) -> EventsNode | ActionsNode: + """ + Takes a legacy entity and converts it into an EventsNode or ActionsNode. + """ shared = { "name": entity.name, "custom_name": entity.custom_name, @@ -126,14 +149,24 @@ def entity_to_node(entity: BackendEntity, include_properties: bool, include_math "properties": clean_entity_properties(entity._data.get("properties", None)), } - if include_math: - shared = { - **shared, - "math": entity.math, - "math_property": entity.math_property, - "math_hogql": entity.math_hogql, - "math_group_type_index": entity.math_group_type_index, - } + if math_availability != MathAvailability.Unavailable: + # only trends and stickiness insights support math. + # transition to then default math for stickiness, when an unsupported math type is encountered. + + if ( + entity.math is not None + and math_availability == MathAvailability.ActorsOnly + and entity.math not in actors_only_math_types + ): + shared = {**shared, "math": BaseMathType.dau} + else: + shared = { + **shared, + "math": entity.math, + "math_property": entity.math_property, + "math_hogql": entity.math_hogql, + "math_group_type_index": entity.math_group_type_index, + } if entity.type == "actions": return ActionsNode(id=entity.id, **shared) @@ -141,6 +174,28 @@ def entity_to_node(entity: BackendEntity, include_properties: bool, include_math return EventsNode(event=entity.id, **shared) +def exlusion_entity_to_node(entity) -> FunnelExclusionEventsNode | FunnelExclusionActionsNode: + """ + Takes a legacy exclusion entity and converts it into an FunnelExclusionEventsNode or FunnelExclusionActionsNode. + """ + base_entity = legacy_entity_to_node( + LegacyEntity(entity), include_properties=False, math_availability=MathAvailability.Unavailable + ) + if isinstance(base_entity, EventsNode): + return FunnelExclusionEventsNode( + **base_entity.model_dump(), + funnelFromStep=entity.get("funnel_from_step"), + funnelToStep=entity.get("funnel_to_step"), + ) + else: + return FunnelExclusionActionsNode( + **base_entity.model_dump(), + funnelFromStep=entity.get("funnel_from_step"), + funnelToStep=entity.get("funnel_to_step"), + ) + + +# TODO: remove this method that returns legacy entities def to_base_entity_dict(entity: Dict): return { "type": entity.get("type"), @@ -160,6 +215,8 @@ def to_base_entity_dict(entity: Dict): "STICKINESS": StickinessQuery, } +INSIGHT_TYPE = Literal["TRENDS", "FUNNELS", "RETENTION", "PATHS", "LIFECYCLE", "STICKINESS"] + def _date_range(filter: Dict): date_range = DateRange(date_from=filter.get("date_from"), date_to=filter.get("date_to")) @@ -188,14 +245,17 @@ def _series(filter: Dict): if filter.get("events") is not None: filter["events"] = [event for event in filter.get("events") if not (isinstance(event, str))] - include_math = True - include_properties = True - if _insight_type(filter) == "LIFECYCLE": - include_math = False + math_availability: MathAvailability = MathAvailability.Unavailable + include_properties: bool = True + + if _insight_type(filter) == "TRENDS": + math_availability = MathAvailability.All + elif _insight_type(filter) == "STICKINESS": + math_availability = MathAvailability.ActorsOnly return { "series": [ - entity_to_node(entity, include_properties, include_math) + legacy_entity_to_node(entity, include_properties, math_availability) for entity in _entities(filter) if entity.id is not None ] @@ -203,19 +263,19 @@ def _series(filter: Dict): def _entities(filter: Dict): - processed_entities: List[BackendEntity] = [] + processed_entities: List[LegacyEntity] = [] # add actions actions = filter.get("actions", []) if isinstance(actions, str): actions = json.loads(actions) - processed_entities.extend([BackendEntity({**entity, "type": "actions"}) for entity in actions]) + processed_entities.extend([LegacyEntity({**entity, "type": "actions"}) for entity in actions]) # add events events = filter.get("events", []) if isinstance(events, str): events = json.loads(events) - processed_entities.extend([BackendEntity({**entity, "type": "events"}) for entity in events]) + processed_entities.extend([LegacyEntity({**entity, "type": "events"}) for entity in events]) # order by order processed_entities.sort(key=lambda entity: entity.order if entity.order else -1) @@ -342,14 +402,7 @@ def _insight_filter(filter: Dict): breakdownAttributionType=filter.get("breakdown_attribution_type"), breakdownAttributionValue=filter.get("breakdown_attribution_value"), binCount=filter.get("bin_count"), - exclusions=[ - FunnelExclusion( - **to_base_entity_dict(entity), - funnelFromStep=entity.get("funnel_from_step"), - funnelToStep=entity.get("funnel_to_step"), - ) - for entity in filter.get("exclusions", []) - ], + exclusions=[exlusion_entity_to_node(entity) for entity in filter.get("exclusions", [])], layout=filter.get("layout"), # hidden_legend_breakdowns: cleanHiddenLegendSeries(filter.get('hidden_legend_keys')), funnelAggregateByHogQL=filter.get("funnel_aggregate_by_hogql"), @@ -414,7 +467,7 @@ def _insight_filter(filter: Dict): return insight_filter -def _insight_type(filter: Dict) -> str: +def _insight_type(filter: Dict) -> INSIGHT_TYPE: if filter.get("insight") == "SESSIONS": return "TRENDS" return filter.get("insight", "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 ca72ea039c6fd..9421ac41be854 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 @@ -11,11 +11,11 @@ CohortPropertyFilter, CountPerActorMathType, ElementPropertyFilter, - EntityType, EventPropertyFilter, EventsNode, FunnelConversionWindowTimeUnit, - FunnelExclusion, + FunnelExclusionActionsNode, + FunnelExclusionEventsNode, FunnelPathType, FunnelVizType, GroupPropertyFilter, @@ -1328,7 +1328,15 @@ def test_funnels_filter(self): "name": "$pageview", "funnel_from_step": 1, "funnel_to_step": 2, - } + }, + { + "id": 3, + "type": "actions", + "order": 1, + "name": "Some action", + "funnel_from_step": 1, + "funnel_to_step": 2, + }, ], "bin_count": 15, # used in time to convert: number of bins to show in histogram "funnel_from_step": 1, # used in time to convert: initial step index to compute time to convert @@ -1363,14 +1371,18 @@ def test_funnels_filter(self): breakdownAttributionValue=2, funnelOrderType=StepOrderValue.strict, exclusions=[ - FunnelExclusion( - id="$pageview", - type=EntityType.events, - order=0, + FunnelExclusionEventsNode( + event="$pageview", name="$pageview", funnelFromStep=1, funnelToStep=2, - ) + ), + FunnelExclusionActionsNode( + id=3, + name="Some action", + funnelFromStep=1, + funnelToStep=2, + ), ], binCount=15, funnelAggregateByHogQL="person_id", diff --git a/posthog/schema.py b/posthog/schema.py index de99275188527..7e064d4b0dfb7 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -218,13 +218,13 @@ class FunnelConversionWindowTimeUnit(str, Enum): month = "month" -class FunnelExclusion(BaseModel): +class FunnelExclusionLegacy(BaseModel): model_config = ConfigDict( extra="forbid", ) custom_name: Optional[str] = None - funnelFromStep: Optional[float] = None - funnelToStep: Optional[float] = None + funnel_from_step: Optional[float] = None + funnel_to_step: Optional[float] = None id: Optional[Union[str, float]] = None index: Optional[float] = None name: Optional[str] = None @@ -232,18 +232,12 @@ class FunnelExclusion(BaseModel): type: Optional[EntityType] = None -class FunnelExclusionLegacy(BaseModel): +class FunnelExclusionSteps(BaseModel): model_config = ConfigDict( extra="forbid", ) - custom_name: Optional[str] = None - funnel_from_step: Optional[float] = None - funnel_to_step: Optional[float] = None - id: Optional[Union[str, float]] = None - index: Optional[float] = None - name: Optional[str] = None - order: Optional[float] = None - type: Optional[EntityType] = None + funnelFromStep: Optional[int] = None + funnelToStep: Optional[int] = None class FunnelLayout(str, Enum): @@ -1056,26 +1050,6 @@ class FeaturePropertyFilter(BaseModel): value: Optional[Union[str, float, List[Union[str, float]]]] = None -class FunnelsFilter(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - binCount: Optional[Union[float, str]] = None - breakdownAttributionType: Optional[BreakdownAttributionType] = None - breakdownAttributionValue: Optional[float] = None - exclusions: Optional[List[FunnelExclusion]] = None - funnelAggregateByHogQL: Optional[str] = None - funnelFromStep: Optional[float] = None - funnelOrderType: Optional[StepOrderValue] = None - funnelStepReference: Optional[FunnelStepReference] = None - funnelToStep: Optional[float] = None - funnelVizType: Optional[FunnelVizType] = None - funnelWindowInterval: Optional[float] = None - funnelWindowIntervalUnit: Optional[FunnelConversionWindowTimeUnit] = None - hidden_legend_breakdowns: Optional[List[str]] = None - layout: Optional[FunnelLayout] = None - - class FunnelsFilterLegacy(BaseModel): model_config = ConfigDict( extra="forbid", @@ -1708,6 +1682,116 @@ class EventsQuery(BaseModel): where: Optional[List[str]] = Field(default=None, description="HogQL filters to apply on returned data") +class FunnelExclusionActionsNode(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + custom_name: Optional[str] = None + fixedProperties: Optional[ + List[ + Union[ + EventPropertyFilter, + PersonPropertyFilter, + ElementPropertyFilter, + SessionPropertyFilter, + CohortPropertyFilter, + RecordingDurationFilter, + GroupPropertyFilter, + FeaturePropertyFilter, + HogQLPropertyFilter, + EmptyPropertyFilter, + ] + ] + ] = Field( + default=None, + description="Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person)", + ) + funnelFromStep: Optional[int] = None + funnelToStep: Optional[int] = None + id: int + kind: Literal["ActionsNode"] = "ActionsNode" + math: Optional[ + Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + ] = None + math_group_type_index: Optional[MathGroupTypeIndex] = None + math_hogql: Optional[str] = None + math_property: Optional[str] = None + name: Optional[str] = None + properties: Optional[ + List[ + Union[ + EventPropertyFilter, + PersonPropertyFilter, + ElementPropertyFilter, + SessionPropertyFilter, + CohortPropertyFilter, + RecordingDurationFilter, + GroupPropertyFilter, + FeaturePropertyFilter, + HogQLPropertyFilter, + EmptyPropertyFilter, + ] + ] + ] = Field(default=None, description="Properties configurable in the interface") + response: Optional[Dict[str, Any]] = Field(default=None, description="Cached query response") + + +class FunnelExclusionEventsNode(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + custom_name: Optional[str] = None + event: Optional[str] = Field(default=None, description="The event or `null` for all events.") + fixedProperties: Optional[ + List[ + Union[ + EventPropertyFilter, + PersonPropertyFilter, + ElementPropertyFilter, + SessionPropertyFilter, + CohortPropertyFilter, + RecordingDurationFilter, + GroupPropertyFilter, + FeaturePropertyFilter, + HogQLPropertyFilter, + EmptyPropertyFilter, + ] + ] + ] = Field( + default=None, + description="Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person)", + ) + funnelFromStep: Optional[int] = None + funnelToStep: Optional[int] = None + kind: Literal["EventsNode"] = "EventsNode" + limit: Optional[int] = None + math: Optional[ + Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + ] = None + math_group_type_index: Optional[MathGroupTypeIndex] = None + math_hogql: Optional[str] = None + math_property: Optional[str] = None + name: Optional[str] = None + orderBy: Optional[List[str]] = Field(default=None, description="Columns to order by") + properties: Optional[ + List[ + Union[ + EventPropertyFilter, + PersonPropertyFilter, + ElementPropertyFilter, + SessionPropertyFilter, + CohortPropertyFilter, + RecordingDurationFilter, + GroupPropertyFilter, + FeaturePropertyFilter, + HogQLPropertyFilter, + EmptyPropertyFilter, + ] + ] + ] = Field(default=None, description="Properties configurable in the interface") + response: Optional[Response] = Field(default=None, description="Return a limited set of data") + + class HogQLFilters(BaseModel): model_config = ConfigDict( extra="forbid", @@ -1746,12 +1830,6 @@ class HogQLQuery(BaseModel): ) -class InsightFilter( - RootModel[Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter]] -): - root: Union[TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, LifecycleFilter] - - class PersonsNode(BaseModel): model_config = ConfigDict( extra="forbid", @@ -1963,10 +2041,36 @@ class DataVisualizationNode(BaseModel): source: HogQLQuery +class FunnelsFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + binCount: Optional[Union[float, str]] = None + breakdownAttributionType: Optional[BreakdownAttributionType] = None + breakdownAttributionValue: Optional[float] = None + exclusions: Optional[List[Union[FunnelExclusionEventsNode, FunnelExclusionActionsNode]]] = None + funnelAggregateByHogQL: Optional[str] = None + funnelFromStep: Optional[float] = None + funnelOrderType: Optional[StepOrderValue] = None + funnelStepReference: Optional[FunnelStepReference] = None + funnelToStep: Optional[float] = None + funnelVizType: Optional[FunnelVizType] = None + funnelWindowInterval: Optional[float] = None + funnelWindowIntervalUnit: Optional[FunnelConversionWindowTimeUnit] = None + hidden_legend_breakdowns: Optional[List[str]] = None + layout: Optional[FunnelLayout] = None + + class HasPropertiesNode(RootModel[Union[EventsNode, EventsQuery, PersonsNode]]): root: Union[EventsNode, EventsQuery, PersonsNode] +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",