Skip to content

Commit

Permalink
feat(hogql): sequential steps funnel (#19951)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Feb 7, 2024
1 parent 69ab4dc commit cbf09b5
Show file tree
Hide file tree
Showing 30 changed files with 6,354 additions and 127 deletions.
35 changes: 25 additions & 10 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@
"type": "object"
}
},
"required": ["id", "kind"],
"required": ["funnelFromStep", "funnelToStep", "id", "kind"],
"type": "object"
},
"FunnelExclusionEventsNode": {
Expand Down Expand Up @@ -1509,7 +1509,7 @@
"type": "object"
}
},
"required": ["kind"],
"required": ["funnelFromStep", "funnelToStep", "kind"],
"type": "object"
},
"FunnelExclusionLegacy": {
Expand Down Expand Up @@ -1540,6 +1540,7 @@
"$ref": "#/definitions/EntityType"
}
},
"required": ["funnel_from_step", "funnel_to_step"],
"type": "object"
},
"FunnelExclusionSteps": {
Expand All @@ -1552,6 +1553,7 @@
"type": "integer"
}
},
"required": ["funnelFromStep", "funnelToStep"],
"type": "object"
},
"FunnelLayout": {
Expand Down Expand Up @@ -1580,7 +1582,7 @@
"$ref": "#/definitions/BreakdownAttributionType"
},
"breakdownAttributionValue": {
"type": "number"
"type": "integer"
},
"exclusions": {
"items": {
Expand All @@ -1592,7 +1594,7 @@
"type": "string"
},
"funnelFromStep": {
"type": "number"
"type": "integer"
},
"funnelOrderType": {
"$ref": "#/definitions/StepOrderValue"
Expand All @@ -1601,13 +1603,13 @@
"$ref": "#/definitions/FunnelStepReference"
},
"funnelToStep": {
"type": "number"
"type": "integer"
},
"funnelVizType": {
"$ref": "#/definitions/FunnelVizType"
},
"funnelWindowInterval": {
"type": "number"
"type": "integer"
},
"funnelWindowIntervalUnit": {
"$ref": "#/definitions/FunnelConversionWindowTimeUnit"
Expand Down Expand Up @@ -1755,10 +1757,23 @@
"type": "string"
},
"results": {
"items": {
"type": "object"
},
"type": "array"
"anyOf": [
{
"items": {
"type": "object"
},
"type": "array"
},
{
"items": {
"items": {
"type": "object"
},
"type": "array"
},
"type": "array"
}
]
},
"timings": {
"items": {
Expand Down
10 changes: 7 additions & 3 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,9 +579,9 @@ export type FunnelsFilterLegacy = Omit<

export interface FunnelExclusionSteps {
/** @asType integer */
funnelFromStep?: number
funnelFromStep: number
/** @asType integer */
funnelToStep?: number
funnelToStep: number
}
export interface FunnelExclusionEventsNode extends EventsNode, FunnelExclusionSteps {}
export interface FunnelExclusionActionsNode extends ActionsNode, FunnelExclusionSteps {}
Expand All @@ -592,12 +592,16 @@ export type FunnelsFilter = {
layout?: FunnelsFilterLegacy['layout']
binCount?: FunnelsFilterLegacy['bin_count']
breakdownAttributionType?: FunnelsFilterLegacy['breakdown_attribution_type']
/** @asType integer */
breakdownAttributionValue?: FunnelsFilterLegacy['breakdown_attribution_value']
funnelAggregateByHogQL?: FunnelsFilterLegacy['funnel_aggregate_by_hogql']
/** @asType integer */
funnelToStep?: FunnelsFilterLegacy['funnel_to_step']
/** @asType integer */
funnelFromStep?: FunnelsFilterLegacy['funnel_from_step']
funnelOrderType?: FunnelsFilterLegacy['funnel_order_type']
funnelVizType?: FunnelsFilterLegacy['funnel_viz_type']
/** @asType integer */
funnelWindowInterval?: FunnelsFilterLegacy['funnel_window_interval']
funnelWindowIntervalUnit?: FunnelsFilterLegacy['funnel_window_interval_unit']
hidden_legend_breakdowns?: FunnelsFilterLegacy['hidden_legend_breakdowns']
Expand All @@ -617,7 +621,7 @@ export interface FunnelsQuery extends InsightsQueryBase {
}

export interface FunnelsQueryResponse extends QueryResponse {
results: Record<string, any>[]
results: Record<string, any>[] | Record<string, any>[][]
}

/** `RetentionFilterType` minus everything inherited from `FilterType` */
Expand Down
31 changes: 5 additions & 26 deletions frontend/src/scenes/funnels/funnelUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import {
EMPTY_BREAKDOWN_VALUES,
getBreakdownStepValues,
getClampedStepRange,
getClampedExclusionStepRange,
getIncompleteConversionWindowStartDate,
getMeanAndStandardDeviation,
getVisibilityKey,
Expand Down Expand Up @@ -171,7 +171,7 @@ describe('getIncompleteConversionWindowStartDate()', () => {
})
})

describe('getClampedStepRange', () => {
describe('getClampedExclusionStepRange', () => {
it('prefers step range to existing filters', () => {
const stepRange: FunnelExclusionSteps = {
funnelFromStep: 0,
Expand All @@ -185,7 +185,7 @@ describe('getClampedStepRange', () => {
},
series: [{}, {}] as EventsNode[],
}
const clampedStepRange = getClampedStepRange({
const clampedStepRange = getClampedExclusionStepRange({
stepRange,
query,
})
Expand All @@ -196,17 +196,16 @@ describe('getClampedStepRange', () => {
})

it('ensures step range is clamped to step range', () => {
const stepRange: FunnelExclusionSteps = {}
const stepRange = {} as FunnelExclusionSteps
const query: FunnelsQuery = {
kind: NodeKind.FunnelsQuery,
funnelsFilter: {
funnelFromStep: -1,

funnelToStep: 12,
},
series: [{}, {}, {}] as EventsNode[],
}
const clampedStepRange = getClampedStepRange({
const clampedStepRange = getClampedExclusionStepRange({
stepRange,
query,
})
Expand All @@ -215,26 +214,6 @@ describe('getClampedStepRange', () => {
funnelToStep: 2,
})
})

it('returns undefined if the incoming filters are undefined', () => {
const stepRange: FunnelExclusionSteps = {}
const query: FunnelsQuery = {
kind: NodeKind.FunnelsQuery,
funnelsFilter: {
funnelFromStep: undefined,
funnelToStep: undefined,
},
series: [{}, {}] as EventsNode[],
}
const clampedStepRange = getClampedStepRange({
stepRange,
query,
})
expect(clampedStepRange).toEqual({
funnelFromStep: undefined,
funnelToStep: undefined,
})
})
})

describe('parseEventAndProperty', () => {
Expand Down
19 changes: 5 additions & 14 deletions frontend/src/scenes/funnels/funnelUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,28 +218,19 @@ export const getBreakdownStepValues = (
return EMPTY_BREAKDOWN_VALUES
}

const findFirstNumber = (candidates: (number | undefined)[]): number | undefined =>
candidates.find((s) => typeof s === 'number')

export const getClampedStepRange = ({
export const getClampedExclusionStepRange = ({
stepRange,
query,
}: {
stepRange?: FunnelExclusionSteps
stepRange: FunnelExclusionSteps
query: FunnelsQuery
}): FunnelExclusionSteps => {
const maxStepIndex = Math.max((query.series.length || 0) - 1, 1)

let funnelFromStep = findFirstNumber([stepRange?.funnelFromStep, query.funnelsFilter?.funnelFromStep])
let funnelToStep = findFirstNumber([stepRange?.funnelToStep, query.funnelsFilter?.funnelToStep])

const funnelFromStepIsSet = typeof funnelFromStep === 'number'
const funnelToStepIsSet = typeof funnelToStep === 'number'
let { funnelFromStep, funnelToStep } = stepRange

if (funnelFromStepIsSet && funnelToStepIsSet) {
funnelFromStep = clamp(funnelFromStep ?? 0, 0, maxStepIndex)
funnelToStep = clamp(funnelToStep ?? maxStepIndex, funnelFromStep + 1, maxStepIndex)
}
funnelFromStep = clamp(funnelFromStep ?? 0, 0, maxStepIndex)
funnelToStep = clamp(funnelToStep ?? maxStepIndex, funnelFromStep + 1, maxStepIndex)

return {
...(stepRange || {}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { LemonButton, LemonSelect } from '@posthog/lemon-ui'
import clsx from 'clsx'
import { useActions, useValues } from 'kea'
import { IconDelete } from 'lib/lemon-ui/icons'
import { getClampedStepRange } from 'scenes/funnels/funnelUtils'
import { getClampedExclusionStepRange } from 'scenes/funnels/funnelUtils'
import { insightLogic } from 'scenes/insights/insightLogic'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

Expand Down Expand Up @@ -33,11 +33,8 @@ export function ExclusionRowSuffix({
funnelToStep: exclusions?.[index]?.funnelToStep ?? exclusionDefaultStepRange.funnelToStep,
}

const onChange = (
funnelFromStep: number | undefined = stepRange.funnelFromStep,
funnelToStep: number | undefined = stepRange.funnelToStep
): void => {
const newStepRange = getClampedStepRange({
const onChange = (funnelFromStep = stepRange.funnelFromStep, funnelToStep = stepRange.funnelToStep): void => {
const newStepRange = getClampedExclusionStepRange({
stepRange: { funnelFromStep, funnelToStep },
query: querySource as FunnelsQuery,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'

import { legacyEntityToNode } from '~/queries/nodes/InsightQuery/utils/filtersToQueryNode'
import { ActionFilter as ActionFilterType, EntityTypes, FilterType, FunnelExclusionLegacy } from '~/types'
import { ActionFilter as ActionFilterType, EntityTypes, FilterType } from '~/types'

import { ExclusionRow } from './ExclusionRow'
import { ExclusionRowSuffix } from './ExclusionRowSuffix'
Expand All @@ -26,7 +26,7 @@ export function FunnelExclusionsFilter(): JSX.Element {
const isVerticalLayout = !!width && width < 450 // If filter container shrinks below 500px, initiate verticality

const setFilters = (filters: Partial<FilterType>): void => {
const exclusions = filters.events?.map((entity: FunnelExclusionLegacy) => {
const exclusions = filters.events?.map((entity) => {
const baseEntity = legacyEntityToNode(entity as ActionFilterType, false, MathAvailability.None)
return { ...baseEntity, funnelFromStep: entity.funnel_from_step, funnelToStep: entity.funnel_to_step }
})
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/scenes/insights/utils/cleanFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ export const getClampedStepRangeFilter = ({
stepRange,
filters,
}: {
stepRange?: FunnelExclusionLegacy
stepRange?: FunnelExclusionLegacy | { funnel_from_step?: number; funnel_to_step?: number }
filters: FunnelsFilterType
}): FunnelExclusionLegacy => {
}): FunnelExclusionLegacy | { funnel_from_step?: number; funnel_to_step?: number } => {
const maxStepIndex = Math.max((filters.events?.length || 0) + (filters.actions?.length || 0) - 1, 1)

let funnel_from_step = findFirstNumber([stepRange?.funnel_from_step, filters.funnel_from_step])
Expand Down Expand Up @@ -368,7 +368,7 @@ export function cleanFilters(
stepRange: e,
filters: funnelsFilter,
})
),
) as FunnelExclusionLegacy[],
}
return returnedParams
} else if (isPathsFilter(filters)) {
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
isLegacyTrendsFilter,
} from '~/queries/nodes/InsightQuery/utils/legacy'
import { InsightVizNode, NodeKind } from '~/queries/schema'
import { NotebookNodeType, NotebookType } from '~/types'
import { FunnelExclusionLegacy, NotebookNodeType, NotebookType } from '~/types'

// NOTE: Increment this number when you add a new content migration
// It will bust the cache on the localContent in the notebookLogic
Expand Down Expand Up @@ -111,7 +111,9 @@ function convertInsightQueriesToNewSchema(content: JSONContent[]): JSONContent[]
} else if (isLegacyFunnelsExclusion(query.funnelsFilter as any)) {
query.funnelsFilter = {
...query.funnelsFilter,
exclusions: query.funnelsFilter!.exclusions!.map((entity) => exlusionEntityToNode(entity)),
exclusions: query.funnelsFilter!.exclusions!.map((entity) =>
exlusionEntityToNode(entity as unknown as FunnelExclusionLegacy)
),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -901,8 +901,8 @@ export interface ActionFilter extends EntityFilter {
}

export interface FunnelExclusionLegacy extends Partial<EntityFilter> {
funnel_from_step?: number
funnel_to_step?: number
funnel_from_step: number
funnel_to_step: number
}

export type EntityFilterTypes = EntityFilter | ActionFilter | null
Expand Down
3 changes: 3 additions & 0 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,9 @@ posthog/hogql/query.py:0: error: "SelectQuery" has no attribute "select_queries"
posthog/hogql/query.py:0: error: Subclass of "SelectQuery" and "SelectUnionQuery" cannot exist: would have incompatible method signatures [unreachable]
posthog/hogql_queries/insights/trends/breakdown_values.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select" [union-attr]
posthog/hogql_queries/insights/trends/breakdown_values.py:0: error: Value of type "list[Any] | None" is not indexable [index]
posthog/hogql_queries/insights/funnels/base.py:0: error: Incompatible types in assignment (expression has type "FunnelExclusionEventsNode | FunnelExclusionActionsNode", variable has type "EventsNode | ActionsNode") [assignment]
posthog/hogql_queries/insights/funnels/base.py:0: error: Item "EventsNode" of "EventsNode | ActionsNode" has no attribute "funnelFromStep" [union-attr]
posthog/hogql_queries/insights/funnels/base.py:0: error: Item "ActionsNode" of "EventsNode | ActionsNode" has no attribute "funnelFromStep" [union-attr]
posthog/hogql_queries/sessions_timeline_query_runner.py:0: error: Statement is unreachable [unreachable]
posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type" [union-attr]
posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_histogram_bin_count" [union-attr]
Expand Down
Loading

0 comments on commit cbf09b5

Please sign in to comment.