Skip to content

Commit

Permalink
feat(insights): improve series and exclusion entity handling (#20089)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Feb 5, 2024
1 parent 07cfe91 commit e634a5c
Show file tree
Hide file tree
Showing 27 changed files with 811 additions and 275 deletions.
3 changes: 1 addition & 2 deletions frontend/src/queries/nodes/InsightQuery/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const stickinessQueryDefault: StickinessQuery = {
kind: NodeKind.EventsNode,
name: '$pageview',
event: '$pageview',
math: BaseMathType.TotalCount,
math: BaseMathType.UniqueUsers,
},
],
stickinessFilter: {},
Expand All @@ -84,7 +84,6 @@ const lifecycleQueryDefault: LifecycleQuery = {
kind: NodeKind.EventsNode,
name: '$pageview',
event: '$pageview',
math: BaseMathType.TotalCount,
},
],
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { FunnelLayout, ShownAsValue } from 'lib/constants'
import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow'

import {
FunnelsQuery,
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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)
})
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -749,7 +772,6 @@ describe('filtersToQueryNode', () => {
kind: NodeKind.ActionsNode,
id: 1,
name: 'Interacted with file',
math: BaseMathType.TotalCount,
},
],
interval: 'day',
Expand Down Expand Up @@ -1074,7 +1096,6 @@ describe('filtersToQueryNode', () => {
},
],
custom_name: 'Viewed homepage',
math: BaseMathType.TotalCount,
},
{
kind: NodeKind.EventsNode,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
126 changes: 90 additions & 36 deletions frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -14,6 +15,8 @@ import {
ActionsNode,
BreakdownFilter,
EventsNode,
FunnelExclusionActionsNode,
FunnelExclusionEventsNode,
FunnelsFilter,
InsightNodeKind,
InsightQueryNode,
Expand All @@ -38,9 +41,13 @@ import {
import {
ActionFilter,
AnyPropertyFilter,
BaseMathType,
FilterLogicalOperator,
FilterType,
FunnelExclusionLegacy,
FunnelsFilterType,
GroupMathType,
HogQLMathType,
InsightType,
PathsFilterType,
PropertyFilterType,
Expand All @@ -60,39 +67,83 @@ const reverseInsightMap: Record<Exclude<InsightType, InsightType.JSON | InsightT
[InsightType.LIFECYCLE]: NodeKind.LifecycleQuery,
}

const actorsOnlyMathTypes = [
BaseMathType.UniqueUsers,
BaseMathType.WeeklyActiveUsers,
BaseMathType.MonthlyActiveUsers,
GroupMathType.UniqueGroup,
HogQLMathType.HogQL,
]

type FilterTypeActionsAndEvents = { events?: ActionFilter[]; actions?: ActionFilter[]; new_entity?: ActionFilter[] }

export const actionsAndEventsToSeries = ({
actions,
events,
new_entity,
}: FilterTypeActionsAndEvents): (EventsNode | ActionsNode)[] => {
export const legacyEntityToNode = (
entity: ActionFilter,
includeProperties: boolean,
mathAvailability: MathAvailability
): EventsNode | ActionsNode => {
let shared: Partial<EventsNode | ActionsNode> = {
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
}
Expand Down Expand Up @@ -235,9 +286,16 @@ export const filtersToQueryNode = (filters: Partial<FilterType>): 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
}

Expand Down Expand Up @@ -336,11 +394,7 @@ export const funnelsFilterToQuery = (filters: Partial<FunnelsFilterType>): 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),
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/queries/nodes/InsightQuery/utils/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ export const isLegacyFunnelsFilter = (filters: Record<string, any> | undefined):
return legacyKeys.some((key) => key in filters)
}

export const isLegacyFunnelsExclusion = (filters: Record<string, any> | undefined): boolean => {
if (filters == null) {
return false
}

const exclusions = filters.exclusions || []
return exclusions.some((exclusion: Record<string, any>) => 'type' in exclusion)
}

export const isLegacyRetentionFilter = (filters: Record<string, any> | undefined): boolean => {
if (filters == null) {
return false
Expand Down
Loading

0 comments on commit e634a5c

Please sign in to comment.