Skip to content

Commit

Permalink
feat(hogql): add backend side filter to query conversion method (#17542)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Sep 26, 2023
1 parent 00e4477 commit 16a0cf7
Show file tree
Hide file tree
Showing 22 changed files with 1,366 additions and 87 deletions.
34 changes: 10 additions & 24 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -528,20 +528,6 @@
},
"EmptyPropertyFilter": {
"additionalProperties": false,
"properties": {
"key": {
"not": {}
},
"operator": {
"not": {}
},
"type": {
"not": {}
},
"value": {
"not": {}
}
},
"type": "object"
},
"EntityType": {
Expand Down Expand Up @@ -860,15 +846,7 @@
"enum": ["second", "minute", "hour", "day", "week", "month"],
"type": "string"
},
"FunnelLayout": {
"enum": ["horizontal", "vertical"],
"type": "string"
},
"FunnelPathType": {
"enum": ["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"],
"type": "string"
},
"FunnelStepRangeEntityFilter": {
"FunnelExclusion": {
"additionalProperties": false,
"properties": {
"custom_name": {
Expand Down Expand Up @@ -898,6 +876,14 @@
},
"type": "object"
},
"FunnelLayout": {
"enum": ["horizontal", "vertical"],
"type": "string"
},
"FunnelPathType": {
"enum": ["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"],
"type": "string"
},
"FunnelStepReference": {
"enum": ["total", "previous"],
"type": "string"
Expand Down Expand Up @@ -927,7 +913,7 @@
},
"exclusions": {
"items": {
"$ref": "#/definitions/FunnelStepRangeEntityFilter"
"$ref": "#/definitions/FunnelExclusion"
},
"type": "array"
},
Expand Down
1 change: 1 addition & 0 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ export interface ActionsNode extends EntityNode {
kind: NodeKind.ActionsNode
id: number
}

export interface QueryTiming {
/** Key. Shortened to 'k' to save on data. */
k: string
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/funnels/funnelDataLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
FunnelResultType,
FunnelVizType,
FunnelStep,
FunnelStepRangeEntityFilter,
FunnelExclusion,
FunnelStepReference,
FunnelStepWithNestedBreakdown,
InsightLogicProps,
Expand Down Expand Up @@ -381,7 +381,7 @@ export const funnelDataLogic = kea<funnelDataLogicType>([
// Exclusion filters
exclusionDefaultStepRange: [
(s) => [s.querySource],
(querySource: FunnelsQuery): Omit<FunnelStepRangeEntityFilter, 'id' | 'name'> => ({
(querySource: FunnelsQuery): Omit<FunnelExclusion, 'id' | 'name'> => ({
funnel_from_step: 0,
funnel_to_step: (querySource.series || []).length > 1 ? querySource.series.length - 1 : 1,
}),
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/scenes/funnels/funnelUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
FunnelCorrelation,
FunnelCorrelationResultsType,
FunnelCorrelationType,
FunnelStepRangeEntityFilter,
FunnelExclusion,
} from '~/types'
import { dayjs } from 'lib/dayjs'

Expand Down Expand Up @@ -175,7 +175,7 @@ describe('getClampedStepRangeFilter', () => {
const stepRange = {
funnel_from_step: 0,
funnel_to_step: 1,
} as FunnelStepRangeEntityFilter
} as FunnelExclusion
const filters = {
funnel_from_step: 1,
funnel_to_step: 2,
Expand All @@ -193,7 +193,7 @@ describe('getClampedStepRangeFilter', () => {
})

it('ensures step range is clamped to step range', () => {
const stepRange = {} as FunnelStepRangeEntityFilter
const stepRange = {} as FunnelExclusion
const filters = {
funnel_from_step: -1,
funnel_to_step: 12,
Expand All @@ -211,7 +211,7 @@ describe('getClampedStepRangeFilter', () => {
})

it('returns undefined if the incoming filters are undefined', () => {
const stepRange = {} as FunnelStepRangeEntityFilter
const stepRange = {} as FunnelExclusion
const filters = {
funnel_from_step: undefined,
funnel_to_step: undefined,
Expand Down
14 changes: 6 additions & 8 deletions frontend/src/scenes/funnels/funnelUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { autoCaptureEventToDescription, clamp } from 'lib/utils'
import {
FunnelStepRangeEntityFilter,
FunnelExclusion,
FunnelStep,
FunnelStepWithNestedBreakdown,
BreakdownKeyType,
Expand Down Expand Up @@ -225,9 +225,7 @@ export const isStepsEmpty = (filters: FunnelsFilterType): boolean =>
export const isStepsUndefined = (filters: FunnelsFilterType): boolean =>
typeof filters.events === 'undefined' && (typeof filters.actions === 'undefined' || filters.actions.length === 0)

export const deepCleanFunnelExclusionEvents = (
filters: FunnelsFilterType
): FunnelStepRangeEntityFilter[] | undefined => {
export const deepCleanFunnelExclusionEvents = (filters: FunnelsFilterType): FunnelExclusion[] | undefined => {
if (!filters.exclusions) {
return undefined
}
Expand Down Expand Up @@ -255,9 +253,9 @@ export const getClampedStepRangeFilter = ({
stepRange,
filters,
}: {
stepRange?: FunnelStepRangeEntityFilter
stepRange?: FunnelExclusion
filters: FunnelsFilterType
}): FunnelStepRangeEntityFilter => {
}): FunnelExclusion => {
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 All @@ -282,9 +280,9 @@ export const getClampedStepRangeFilterDataExploration = ({
stepRange,
query,
}: {
stepRange?: FunnelStepRangeEntityFilter
stepRange?: FunnelExclusion
query: FunnelsQuery
}): FunnelStepRangeEntityFilter => {
}): FunnelExclusion => {
const maxStepIndex = Math.max(query.series.length || 0 - 1, 1)

let funnel_from_step = findFirstNumber([stepRange?.funnel_from_step, query.funnelsFilter?.funnel_from_step])
Expand Down
14 changes: 2 additions & 12 deletions frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,7 @@ import React, { useEffect } from 'react'
import { BindLogic, useActions, useValues } from 'kea'
import { entityFilterLogic, toFilters, LocalFilter } from './entityFilterLogic'
import { ActionFilterRow, MathAvailability } from './ActionFilterRow/ActionFilterRow'
import {
ActionFilter as ActionFilterType,
FilterType,
FunnelStepRangeEntityFilter,
InsightType,
Optional,
} from '~/types'
import { ActionFilter as ActionFilterType, FilterType, FunnelExclusion, InsightType, Optional } from '~/types'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { RenameModal } from 'scenes/insights/filters/ActionFilter/RenameModal'
import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
Expand Down Expand Up @@ -55,11 +49,7 @@ export interface ActionFilterProps {
customRowSuffix?:
| string
| JSX.Element
| ((props: {
filter: ActionFilterType | FunnelStepRangeEntityFilter
index: number
onClose: () => void
}) => JSX.Element)
| ((props: { filter: ActionFilterType | FunnelExclusion; 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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
ActionFilter,
EntityType,
EntityTypes,
FunnelStepRangeEntityFilter,
FunnelExclusion,
PropertyFilterValue,
BaseMathType,
PropertyMathType,
Expand Down Expand Up @@ -89,11 +89,7 @@ export interface ActionFilterRowProps {
customRowSuffix?:
| string
| JSX.Element
| ((props: {
filter: ActionFilterType | FunnelStepRangeEntityFilter
index: number
onClose: () => void
}) => JSX.Element) // Custom suffix element to show in each row
| ((props: { filter: ActionFilterType | FunnelExclusion; 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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Row, Select } from 'antd'
import { useActions, useValues } from 'kea'
import { ANTD_TOOLTIP_PLACEMENTS } from 'lib/utils'
import { FunnelStepRangeEntityFilter, ActionFilter as ActionFilterType, FunnelsFilterType } from '~/types'
import { FunnelExclusion, ActionFilter as ActionFilterType, FunnelsFilterType } from '~/types'
import { insightLogic } from 'scenes/insights/insightLogic'
import { LemonButton } from '@posthog/lemon-ui'
import { IconDelete } from 'lib/lemon-ui/icons'
Expand All @@ -10,7 +10,7 @@ import { FunnelsQuery } from '~/queries/schema'
import { getClampedStepRangeFilterDataExploration } from 'scenes/funnels/funnelUtils'

type ExclusionRowSuffixComponentBaseProps = {
filter: ActionFilterType | FunnelStepRangeEntityFilter
filter: ActionFilterType | FunnelExclusion
index: number
onClose?: () => void
isVertical: boolean
Expand All @@ -28,7 +28,7 @@ export function ExclusionRowSuffix({
)
const { updateInsightFilter } = useActions(funnelDataLogic(insightProps))

const setOneEventExclusionFilter = (eventFilter: FunnelStepRangeEntityFilter, index: number): void => {
const setOneEventExclusionFilter = (eventFilter: FunnelExclusion, index: number): void => {
const exclusions = ((insightFilter as FunnelsFilterType)?.exclusions || []).map((e, e_i) =>
e_i === index
? getClampedStepRangeFilterDataExploration({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useActions, useValues } from 'kea'
import useSize from '@react-hook/size'
import { ActionFilter } from 'scenes/insights/filters/ActionFilter/ActionFilter'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { FunnelStepRangeEntityFilter, EntityTypes, FilterType } from '~/types'
import { FunnelExclusion, EntityTypes, FilterType } from '~/types'
import { insightLogic } from 'scenes/insights/insightLogic'
import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow'
import { funnelDataLogic } from 'scenes/funnels/funnelDataLogic'
Expand All @@ -22,7 +22,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 as FunnelStepRangeEntityFilter[]).map((e) => ({
const exclusions = (filters.events as FunnelExclusion[]).map((e) => ({
...e,
funnel_from_step: e.funnel_from_step || exclusionDefaultStepRange.funnel_from_step,
funnel_to_step: e.funnel_to_step || exclusionDefaultStepRange.funnel_to_step,
Expand Down
15 changes: 8 additions & 7 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,10 @@ export interface HogQLPropertyFilter extends BasePropertyFilter {
}

export interface EmptyPropertyFilter {
type?: undefined
value?: undefined
operator?: undefined
key?: undefined
type?: never
value?: never
operator?: never
key?: never
}

export type AnyPropertyFilter =
Expand Down Expand Up @@ -786,8 +786,7 @@ export type EntityFilter = {
order?: number
}

// TODO: Separate FunnelStepRange and FunnelStepRangeEntity filter types
export interface FunnelStepRangeEntityFilter extends Partial<EntityFilter> {
export interface FunnelExclusion extends Partial<EntityFilter> {
funnel_from_step?: number
funnel_to_step?: number
}
Expand Down Expand Up @@ -1682,6 +1681,7 @@ export interface TrendsFilterType extends FilterType {
show_percent_stack_view?: boolean
breakdown_histogram_bin_count?: number // trends breakdown histogram bin count
}

export interface StickinessFilterType extends FilterType {
compare?: boolean
show_legend?: boolean // used to show/hide legend next to insights graph
Expand All @@ -1691,6 +1691,7 @@ export interface StickinessFilterType extends FilterType {
display?: ChartDisplayType
show_values_on_series?: boolean
}

export interface FunnelsFilterType extends FilterType {
funnel_viz_type?: FunnelVizType // parameter sent to funnels API for time conversion code path
funnel_from_step?: number // used in time to convert: initial step index to compute time to convert
Expand All @@ -1703,7 +1704,7 @@ export interface FunnelsFilterType extends FilterType {
funnel_window_interval_unit?: FunnelConversionWindowTimeUnit // minutes, days, weeks, etc. for conversion window
funnel_window_interval?: number | undefined // length of conversion window
funnel_order_type?: StepOrderValue
exclusions?: FunnelStepRangeEntityFilter[] // used in funnel exclusion filters
exclusions?: FunnelExclusion[] // used in funnel exclusion filters
funnel_correlation_person_entity?: Record<string, any> // Funnel Correlation Persons Filter
funnel_correlation_person_converted?: 'true' | 'false' // Funnel Correlation Persons Converted - success or failure counts
funnel_custom_steps?: number[] // used to provide custom steps for which to get people in a funnel - primarily for correlation use
Expand Down
2 changes: 2 additions & 0 deletions playwright/e2e-vrt/layout/Navigation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ test.describe('Navigation', () => {
test('App Page With Side Bar Hidden (Mobile)', async ({ storyPage }) => {
await storyPage.resizeToMobile()
await storyPage.goto(toId('Layout/Navigation', 'App Page With Side Bar Hidden'))
await storyPage.mainAppContent.waitFor()
await storyPage.expectFullPageScreenshot()
})

test('App Page With Side Bar Shown (Mobile)', async ({ storyPage }) => {
await storyPage.resizeToMobile()
await storyPage.goto(toId('Layout/Navigation', 'App Page With Side Bar Shown'))
await storyPage.mainAppContent.waitFor()
await storyPage.expectFullPageScreenshot()
})
})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 16a0cf7

Please sign in to comment.