Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow hogql property queries in replay filtering #26176

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
f1d1551
feat: allow hogql property queries in replay filtering
pauldambra Nov 13, 2024
416c61b
Schema change so that recordingsquery can be consider
pauldambra Nov 13, 2024
055f2c3
Merge branch 'master' into feat/allow-hogql-property-queries-in-repla…
pauldambra Nov 23, 2024
d3d8c2f
flag the new filter types
pauldambra Nov 23, 2024
7b35837
add failing test
pauldambra Nov 23, 2024
b630acd
first pass removing flag filtering
pauldambra Nov 23, 2024
3b82878
flag the new query route
pauldambra Nov 23, 2024
9f5b54c
more
pauldambra Nov 23, 2024
647faa9
like this?
pauldambra Nov 23, 2024
1454b13
Discard changes to posthog/hogql/printer.py
pauldambra Nov 23, 2024
00794a3
ee tests
pauldambra Nov 23, 2024
438c075
a little more
pauldambra Nov 23, 2024
29de030
a little more
pauldambra Nov 23, 2024
54a0650
almost
pauldambra Nov 23, 2024
57b6cda
Update query snapshots
github-actions[bot] Nov 24, 2024
1a77a19
test should not be passing
pauldambra Nov 24, 2024
efcfad2
Update UI snapshots for `chromium` (2)
github-actions[bot] Nov 24, 2024
62f2a63
fiddling
pauldambra Nov 24, 2024
3c49ecc
Merge branch 'main' into feat/allow-hogql-property-queries-in-replay-…
pauldambra Nov 30, 2024
8b5c8ea
maybe progress
pauldambra Dec 1, 2024
baff95e
maybe progress
pauldambra Dec 1, 2024
4ef52fd
fixes
pauldambra Dec 1, 2024
cde6c10
Merge branch 'master' into feat/allow-hogql-property-queries-in-repla…
pauldambra Dec 2, 2024
d0ddf24
snaps
pauldambra Dec 2, 2024
8ca0f80
Merge branch 'master' into feat/allow-hogql-property-queries-in-repla…
pauldambra Dec 2, 2024
d773d9a
wat
pauldambra Dec 2, 2024
65e2688
fix
pauldambra Dec 2, 2024
d249602
Merge branch 'master' into feat/allow-hogql-property-queries-in-repla…
pauldambra Dec 2, 2024
ff64984
fix
pauldambra Dec 2, 2024
54cc559
Update query snapshots
github-actions[bot] Dec 2, 2024
bdc85dc
obey mypy
pauldambra Dec 2, 2024
143dc6b
obey mypy
pauldambra Dec 2, 2024
1f35120
obeymypy
pauldambra Dec 2, 2024
f892a49
obeymypy
pauldambra Dec 2, 2024
7b12a52
obeymypy
pauldambra Dec 2, 2024
2b6a2c2
obeymypy
pauldambra Dec 2, 2024
6d9eae2
Update query snapshots
github-actions[bot] Dec 2, 2024
e809757
obeymypy
pauldambra Dec 2, 2024
0653b9f
obeymypy
pauldambra Dec 2, 2024
d8db5df
obeymypy
pauldambra Dec 2, 2024
d48679b
schema
pauldambra Dec 3, 2024
e478f7a
mypy
pauldambra Dec 3, 2024
e26c8be
obeymypy
pauldambra Dec 3, 2024
763f7e5
Merge branch 'master' into feat/allow-hogql-property-queries-in-repla…
pauldambra Dec 3, 2024
3a75e57
Discard changes to mypy-baseline.txt
pauldambra Dec 3, 2024
3b6c293
Update query snapshots
github-actions[bot] Dec 3, 2024
07d4ac7
rage
pauldambra Dec 3, 2024
9db36ed
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 3, 2024
48788e3
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 3, 2024
9e45158
fix
pauldambra Dec 3, 2024
ee74de6
Update query snapshots
github-actions[bot] Dec 3, 2024
32f0ef1
fix
pauldambra Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { LemonButton, LemonButtonProps, LemonDropdown, Popover } from '@posthog/
import { BindLogic, useActions, useValues } from 'kea'
import { useState } from 'react'

import { AnyDataNode } from '~/queries/schema'
import { UniversalFiltersGroup, UniversalFilterValue } from '~/types'

import { TaxonomicPropertyFilter } from '../PropertyFilters/components/TaxonomicPropertyFilter'
Expand Down Expand Up @@ -75,12 +76,14 @@ const Value = ({
onChange,
onRemove,
initiallyOpen = false,
metadataSource,
}: {
index: number
filter: UniversalFilterValue
onChange: (property: UniversalFilterValue) => void
onRemove: () => void
initiallyOpen?: boolean
metadataSource?: AnyDataNode
}): JSX.Element => {
const { rootKey, taxonomicPropertyFilterGroupTypes } = useValues(universalFiltersLogic)

Expand All @@ -103,6 +106,7 @@ const Value = ({
onChange={(properties) => onChange({ ...filter, properties })}
disablePopover
taxonomicGroupTypes={[TaxonomicFilterGroupType.EventProperties]}
metadataSource={metadataSource}
/>
) : isEditable ? (
<TaxonomicPropertyFilter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const universalFiltersLogic = kea<universalFiltersLogicType>([
newValues.push(newFeatureFlagFilter)
} else {
const propertyType =
item.propertyFilterType ?? taxonomicFilterTypeToPropertyFilterType(taxonomicGroup.type)
item?.propertyFilterType ?? taxonomicFilterTypeToPropertyFilterType(taxonomicGroup.type)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hogql property filters don't have an item so this needs to be slightly safer

if (propertyKey && propertyType) {
const newPropertyFilter = createDefaultPropertyFilter(
{},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { actionsModel } from '~/models/actionsModel'
import { cohortsModel } from '~/models/cohortsModel'
import { AndOrFilterSelect } from '~/queries/nodes/InsightViz/PropertyGroupFilters/AndOrFilterSelect'
import { NodeKind } from '~/queries/schema'
import { RecordingUniversalFilters, UniversalFiltersGroup } from '~/types'

import { DurationFilter } from './DurationFilter'
Expand Down Expand Up @@ -109,6 +110,8 @@
TaxonomicFilterGroupType.Cohorts,
TaxonomicFilterGroupType.PersonProperties,
TaxonomicFilterGroupType.SessionProperties,
TaxonomicFilterGroupType.FeatureFlags,
TaxonomicFilterGroupType.HogQLExpression,
]}
onChange={(filterGroup) => setFilters({ filter_group: filterGroup })}
>
Expand Down Expand Up @@ -144,6 +147,7 @@
onRemove={() => removeGroupValue(index)}
onChange={(value) => replaceGroupValue(index, value)}
initiallyOpen={allowInitiallyOpen}
metadataSource={{ kind: NodeKind.RecordingsQuery }}

Check failure on line 150 in frontend/src/scenes/session-recordings/filters/RecordingsUniversalFilters.tsx

View workflow job for this annotation

GitHub Actions / Code quality checks

Type 'NodeKind.RecordingsQuery' is not assignable to type 'NodeKind.ExperimentTrendsQuery'.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need something here to correct the autocomplete

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a query runner in python for the recordings query

/>
)
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { loaders } from 'kea-loaders'
import { actionToUrl, router, urlToAction } from 'kea-router'
import { subscriptions } from 'kea-subscriptions'
import api from 'lib/api'
import { isAnyPropertyfilter } from 'lib/components/PropertyFilters/utils'
import { isAnyPropertyfilter, isHogQLPropertyFilter } from 'lib/components/PropertyFilters/utils'
import { DEFAULT_UNIVERSAL_GROUP_FILTER } from 'lib/components/UniversalFilters/universalFiltersLogic'
import {
isActionFilter,
Expand Down Expand Up @@ -120,6 +120,8 @@ export function convertUniversalFiltersToRecordingsQuery(universalFilters: Recor
actions.push(f)
} else if (isLogEntryPropertyFilter(f)) {
console_log_filters.push(f)
} else if (isHogQLPropertyFilter(f)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this "just works"™

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work for all property types e.g person & event properties? Wondering if the subqueries in the runner affect things here in any way

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works for hogql... am going to ignore everything else for now 🤣
(but noted)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be more worried about it not working for folks without PoE enabled (but maybe that's everyone now)

properties.push(f)
} else if (isAnyPropertyfilter(f)) {
if (isRecordingPropertyFilter(f)) {
if (f.key === 'visited_page') {
Expand Down
Loading