Skip to content

Commit

Permalink
fix: funnel first_time_for_user logic change (#25448)
Browse files Browse the repository at this point in the history
  • Loading branch information
aspicer authored Oct 9, 2024
1 parent c2c660f commit 37e2fcc
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
DataWarehouseFilter,
FilterType,
FunnelExclusionLegacy,
FunnelMathType,
FunnelsFilterType,
GroupMathType,
HogQLMathType,
Expand Down Expand Up @@ -84,7 +85,7 @@ const actorsOnlyMathTypes = [
HogQLMathType.HogQL,
]

const funnelsMathTypes = [BaseMathType.FirstTimeForUser]
const funnelsMathTypes = [FunnelMathType.FirstTimeForUser, FunnelMathType.FirstTimeForUserWithFilters]

type FilterTypeActionsAndEvents = {
events?: ActionFilter[]
Expand Down Expand Up @@ -118,7 +119,7 @@ export const legacyEntityToNode = (
}

if (mathAvailability !== MathAvailability.None) {
// only trends and stickiness insights support math.
// only trends, funnels, 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 = {
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -5373,6 +5373,10 @@
"enum": ["horizontal", "vertical"],
"type": "string"
},
"FunnelMathType": {
"enum": ["total", "first_time_for_user", "first_time_for_user_with_filters"],
"type": "string"
},
"FunnelPathType": {
"enum": ["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"],
"type": "string"
Expand Down Expand Up @@ -7132,6 +7136,9 @@
{
"$ref": "#/definitions/BaseMathType"
},
{
"$ref": "#/definitions/FunnelMathType"
},
{
"$ref": "#/definitions/PropertyMathType"
},
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
FeaturePropertyFilter,
FilterLogicalOperator,
FilterType,
FunnelMathType,
FunnelsFilterType,
GroupMathType,
GroupPropertyFilter,
Expand Down Expand Up @@ -460,7 +461,13 @@ export interface HogQLAutocomplete extends DataNode<HogQLAutocompleteResponse> {
endPosition: integer
}

export type MathType = BaseMathType | PropertyMathType | CountPerActorMathType | GroupMathType | HogQLMathType
export type MathType =
| BaseMathType
| FunnelMathType
| PropertyMathType
| CountPerActorMathType
| GroupMathType
| HogQLMathType

export interface EntityNode extends Node {
name?: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { CSS } from '@dnd-kit/utilities'
import { IconCopy, IconEllipsis, IconFilter, IconPencil, IconTrash, IconWarning } from '@posthog/icons'
import {
LemonBadge,
LemonCheckbox,
LemonDivider,
LemonMenu,
LemonSelect,
Expand All @@ -21,8 +20,7 @@ import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
import { SeriesGlyph, SeriesLetter } from 'lib/components/SeriesGlyph'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { TaxonomicPopover, TaxonomicStringPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover'
import { IconWithCount } from 'lib/lemon-ui/icons'
import { SortableDragIcon } from 'lib/lemon-ui/icons'
import { IconWithCount, SortableDragIcon } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonDropdown } from 'lib/lemon-ui/LemonDropdown'
import { Tooltip } from 'lib/lemon-ui/Tooltip'
Expand Down Expand Up @@ -529,20 +527,15 @@ export function ActionFilterRow({
{
label: () => (
<>
<LemonCheckbox
className="py-1 px-2 flex-row-reverse [&_svg]:ml-1 [&>label]:gap-2.5"
checked={math === BaseMathType.FirstTimeForUser}
onChange={(checked) => {
onMathSelect(
index,
checked
? BaseMathType.FirstTimeForUser
: undefined
)
}}
data-attr={`math-first-time-for-user-${index}`}
label="Count by first time for user"
fullWidth
<MathSelector
math={math}
mathGroupTypeIndex={mathGroupTypeIndex}
index={index}
onMathSelect={onMathSelect}
disabled={readOnly}
style={{ maxWidth: '100%', width: 'initial' }}
mathAvailability={mathAvailability}
trendsDisplayCategory={trendsDisplayCategory}
/>
<LemonDivider />
</>
Expand Down Expand Up @@ -570,7 +563,7 @@ export function ActionFilterRow({
<LemonBadge
position="top-right"
size="small"
visible={math === BaseMathType.FirstTimeForUser}
visible={math !== undefined}
/>
</div>
</>
Expand Down Expand Up @@ -649,8 +642,13 @@ function useMathSelectorOptions({

const isStickiness = query && isInsightVizNode(query) && isStickinessQuery(query.source)

const { needsUpgradeForGroups, canStartUsingGroups, staticMathDefinitions, staticActorsOnlyMathDefinitions } =
useValues(mathsLogic)
const {
needsUpgradeForGroups,
canStartUsingGroups,
staticMathDefinitions,
funnelMathDefinitions,
staticActorsOnlyMathDefinitions,
} = useValues(mathsLogic)

const [propertyMathTypeShown, setPropertyMathTypeShown] = useState<PropertyMathType>(
isPropertyValueMath(math) ? math : PropertyMathType.Average
Expand All @@ -659,9 +657,14 @@ function useMathSelectorOptions({
isCountPerActorMath(math) ? math : CountPerActorMathType.Average
)

const options: LemonSelectOption<string>[] = Object.entries(
mathAvailability != MathAvailability.ActorsOnly ? staticMathDefinitions : staticActorsOnlyMathDefinitions
)
let definitions = staticMathDefinitions
if (mathAvailability === MathAvailability.FunnelsOnly) {
definitions = funnelMathDefinitions
} else if (mathAvailability === MathAvailability.ActorsOnly) {
definitions = staticActorsOnlyMathDefinitions
}

const options: LemonSelectOption<string>[] = Object.entries(definitions)
.filter(([key]) => {
if (isStickiness) {
// Remove WAU and MAU from stickiness insights
Expand Down Expand Up @@ -691,7 +694,7 @@ function useMathSelectorOptions({
}
})

if (mathAvailability !== MathAvailability.ActorsOnly) {
if (mathAvailability !== MathAvailability.ActorsOnly && mathAvailability !== MathAvailability.FunnelsOnly) {
options.splice(1, 0, {
value: countPerActorMathTypeShown,
label: `Count per user ${COUNT_PER_ACTOR_MATH_DEFINITIONS[countPerActorMathTypeShown].shortName}`,
Expand Down Expand Up @@ -749,12 +752,14 @@ function useMathSelectorOptions({
})
}

options.push({
value: HogQLMathType.HogQL,
label: 'HogQL expression',
tooltip: 'Aggregate events by custom SQL expression.',
'data-attr': `math-node-hogql-expression-${index}`,
})
if (mathAvailability !== MathAvailability.FunnelsOnly) {
options.push({
value: HogQLMathType.HogQL,
label: 'HogQL expression',
tooltip: 'Aggregate events by custom SQL expression.',
'data-attr': `math-node-hogql-expression-${index}`,
})
}

return [
{
Expand Down
55 changes: 54 additions & 1 deletion frontend/src/scenes/trends/mathsLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { groupsAccessLogic } from 'lib/introductions/groupsAccessLogic'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'

import { groupsModel } from '~/models/groupsModel'
import { BaseMathType, CountPerActorMathType, HogQLMathType, PropertyMathType } from '~/types'
import { BaseMathType, CountPerActorMathType, FunnelMathType, HogQLMathType, PropertyMathType } from '~/types'

import type { mathsLogicType } from './mathsLogicType'

Expand All @@ -25,6 +25,50 @@ export interface MathDefinition {
category: MathCategory
}

export const FUNNEL_MATH_DEFINITIONS: Record<FunnelMathType, MathDefinition> = {
[FunnelMathType.AnyMatch]: {
name: 'Any events match',
shortName: 'any event',
description: <>Any event of this type that matches the filter will count towards the funnel</>,
category: MathCategory.EventCount,
},
[FunnelMathType.FirstTimeForUser]: {
name: 'First event for user',
shortName: 'first event',
description: (
<>
Only the first time the user performed this event will count towards the funnel, and only if it matches
the event filters.
<br />
<br />
<i>
Example: If the we are looking for pageview events to posthog.com/about, but the user's first
pageview was on posthog.com, it will not match, even if they went to posthog.com/about afterwards.
</i>
</>
),
category: MathCategory.EventCount,
},
[FunnelMathType.FirstTimeForUserWithFilters]: {
name: 'First matching event for user',
shortName: 'first matching event',
description: (
<>
The first time the user performed this event that matches the event filters will count towards the
funnel.
<br />
<br />
<i>
Example: If the we are looking for pageview events to posthog.com/about, and the user's first
pageview was on posthog.com but then they navigated to posthog.com/about, it will match the pageview
event from posthog.com/about
</i>
</>
),
category: MathCategory.EventCount,
},
}

export const BASE_MATH_DEFINITIONS: Record<BaseMathType, MathDefinition> = {
[BaseMathType.TotalCount]: {
name: 'Total count',
Expand Down Expand Up @@ -320,6 +364,15 @@ export const mathsLogic = kea<mathsLogicType>([
return filterMathTypesUnderFeatureFlags(allMathDefinitions, featureFlags)
},
],
funnelMathDefinitions: [
(s) => [s.featureFlags],
(featureFlags): Record<string, MathDefinition> => {
const funnelMathDefinitions: Record<string, MathDefinition> = {
...FUNNEL_MATH_DEFINITIONS,
}
return filterMathTypesUnderFeatureFlags(funnelMathDefinitions, featureFlags)
},
],
// Static means the options do not have nested selectors (like math function)
staticMathDefinitions: [
(s) => [s.groupsMathDefinitions, s.needsUpgradeForGroups, s.featureFlags],
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3514,6 +3514,12 @@ export interface InstanceSetting {
is_secret: boolean
}

export enum FunnelMathType {
AnyMatch = 'total',
FirstTimeForUser = 'first_time_for_user',
FirstTimeForUserWithFilters = 'first_time_for_user_with_filters',
}

export enum BaseMathType {
TotalCount = 'total',
UniqueUsers = 'dau',
Expand Down
8 changes: 6 additions & 2 deletions posthog/hogql_queries/insights/funnels/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from posthog.queries.util import correct_result_for_sampling
from posthog.schema import (
ActionsNode,
BaseMathType,
BreakdownAttributionType,
BreakdownType,
DataWarehouseNode,
Expand All @@ -34,6 +33,7 @@
FunnelTimeToConvertResults,
FunnelVizType,
FunnelExclusionEventsNode,
FunnelMathType,
)
from posthog.types import EntityNode, ExclusionEntityNode

Expand Down Expand Up @@ -697,10 +697,14 @@ def _build_step_query(
filter_expr = property_to_expr(entity.properties, self.context.team)
filters.append(filter_expr)

if entity.math == BaseMathType.FIRST_TIME_FOR_USER:
if entity.math == FunnelMathType.FIRST_TIME_FOR_USER:
subquery = FirstTimeForUserAggregationQuery(self.context, filter_expr, event_expr).to_query()
first_time_filter = parse_expr("e.uuid IN {subquery}", placeholders={"subquery": subquery})
return ast.And(exprs=[*filters, first_time_filter])
elif entity.math == FunnelMathType.FIRST_TIME_FOR_USER_WITH_FILTERS:
subquery = FirstTimeForUserAggregationQuery(self.context, ast.Constant(value=1), filter_expr).to_query()
first_time_filter = parse_expr("e.uuid IN {subquery}", placeholders={"subquery": subquery})
return ast.And(exprs=[*filters, first_time_filter])
elif len(filters) > 1:
return ast.And(exprs=filters)
return filters[0]
Expand Down
61 changes: 61 additions & 0 deletions posthog/hogql_queries/insights/funnels/test/test_funnel.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
InsightDateRange,
PersonsOnEventsMode,
PropertyOperator,
FunnelMathType,
)
from posthog.test.base import (
APIBaseTest,
Expand Down Expand Up @@ -4189,6 +4190,66 @@ def test_first_time_for_user_funnel_multiple_ids(self):
self.assertEqual(results[0]["count"], 3)
self.assertEqual(results[1]["count"], 1)

def test_first_time_for_user_funnel_person_properties(self):
_create_person(distinct_ids=["user_1"], team=self.team, properties={"email": "[email protected]"})
_create_person(
distinct_ids=["user_2"],
properties={"email": "[email protected]"},
team=self.team,
)

_create_event(
team=self.team,
event="event1",
distinct_id="user_1",
timestamp="2024-03-20T13:00:00Z",
)
_create_event(
team=self.team,
event="event1",
distinct_id="user_1",
properties={"property": "woah"},
timestamp="2024-03-21T13:00:00Z",
)
_create_event(
team=self.team,
event="event1",
distinct_id="user_2",
timestamp="2024-03-22T14:00:00Z",
)
_create_event(
team=self.team,
event="event2",
distinct_id="user_1",
timestamp="2024-03-23T13:00:00Z",
)

query = FunnelsQuery(
series=[
EventsNode(
event="event1",
math=FunnelMathType.FIRST_TIME_FOR_USER_WITH_FILTERS,
properties=[
EventPropertyFilter(key="property", value="woah", operator=PropertyOperator.EXACT),
],
),
EventsNode(event="event2"),
],
dateRange=InsightDateRange(
date_from="2024-03-20",
date_to="2024-03-24",
),
)
results = FunnelsQueryRunner(query=query, team=self.team).calculate().results

self.assertEqual(results[0]["count"], 1)
self.assertEqual(results[1]["count"], 1)

query.series[0].math = FunnelMathType.FIRST_TIME_FOR_USER
results = FunnelsQueryRunner(query=query, team=self.team).calculate().results
# classic and udf funnels handle no events differently
assert len(results) == 0 or results[0]["count"] == 0

def test_funnel_personless_events_are_supported(self):
user_id = uuid.uuid4()
_create_event(
Expand Down
Loading

0 comments on commit 37e2fcc

Please sign in to comment.