From fc7d42046c31a01ca315f1b15bddc9bde7f50d82 Mon Sep 17 00:00:00 2001 From: Robbie Date: Fri, 12 Apr 2024 09:41:51 +0100 Subject: [PATCH 01/32] Handle hogql session properties --- posthog/hogql/database/schema/channel_type.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/posthog/hogql/database/schema/channel_type.py b/posthog/hogql/database/schema/channel_type.py index 39c9b31d36918..c45c71458d7d1 100644 --- a/posthog/hogql/database/schema/channel_type.py +++ b/posthog/hogql/database/schema/channel_type.py @@ -139,3 +139,23 @@ def wrap_with_null_if_empty(expr: ast.Expr) -> ast.Expr: "gad_source": wrap_with_null_if_empty(gad_source), }, ) + + +POSSIBLE_CHANNEL_TYPES = [ + "Cross Network", + "Paid Search", + "Paid Video", + "Paid Shopping", + "Paid Other", + "Direct", + "Organic Search", + "Organic Video", + "Organic Shopping", + "Push", + "SMS", + "Audio", + "Email", + "Referral", + "Affiliate", + "Other", +] From cd473483761d27075aa8d7ece2269f25c5edb7a3 Mon Sep 17 00:00:00 2001 From: Robbie Date: Fri, 12 Apr 2024 09:54:57 +0100 Subject: [PATCH 02/32] Add test for two lazy joins in one query (session & person) --- .../database/schema/test/test_sessions.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/posthog/hogql/database/schema/test/test_sessions.py b/posthog/hogql/database/schema/test/test_sessions.py index 2f4728fb9b558..1e751cfefe0d6 100644 --- a/posthog/hogql/database/schema/test/test_sessions.py +++ b/posthog/hogql/database/schema/test/test_sessions.py @@ -5,6 +5,7 @@ APIBaseTest, ClickhouseTestMixin, _create_event, + _create_person, ) @@ -103,3 +104,35 @@ def test_events_session_dot_channel_type(self): result[0], "Paid Search", ) + + def test_persons_and_sessions_on_events(self): + p1 = _create_person(distinct_ids=["d1"], team=self.team) + p2 = _create_person(distinct_ids=["d2"], team=self.team) + + s1 = "session_test_persons_and_sessions_on_events_1" + s2 = "session_test_persons_and_sessions_on_events_2" + + _create_event( + event="$pageview", + team=self.team, + distinct_id="d1", + properties={"$session_id": s1, "utm_source": "source1"}, + ) + _create_event( + event="$pageview", + team=self.team, + distinct_id="d2", + properties={"$session_id": s2, "utm_source": "source2"}, + ) + + response = execute_hogql_query( + parse_select( + "select events.person_id, session.initial_utm_source from events where $session_id = {session_id} or $session_id = {session_id2} order by 2 asc", + placeholders={"session_id": ast.Constant(value=s1), "session_id2": ast.Constant(value=s2)}, + ), + self.team, + ) + + [row1, row2] = response.results + self.assertEqual(row1, (p1.uuid, "source1")) + self.assertEqual(row2, (p2.uuid, "source2")) From 394a3090dd141755998548ef4a75a9f4b1b2001d Mon Sep 17 00:00:00 2001 From: Robbie Date: Fri, 12 Apr 2024 12:26:28 +0100 Subject: [PATCH 03/32] Support passing session properties in web analytics queries --- frontend/src/lib/components/PropertyFilters/utils.ts | 12 +++++++++--- frontend/src/queries/schema.json | 3 +++ frontend/src/queries/schema.ts | 3 ++- .../src/scenes/web-analytics/WebPropertyFilters.tsx | 4 ++-- frontend/src/types.ts | 1 + posthog/schema.py | 8 ++++---- 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/components/PropertyFilters/utils.ts b/frontend/src/lib/components/PropertyFilters/utils.ts index ad135c0525b0e..d6745a9d62b75 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.ts @@ -183,10 +183,14 @@ export function isEventPropertyFilter(filter?: AnyFilterLike | null): filter is export function isPersonPropertyFilter(filter?: AnyFilterLike | null): filter is PersonPropertyFilter { return filter?.type === PropertyFilterType.Person } -export function isEventPropertyOrPersonPropertyFilter( +export function isEventPersonOrSessionPropertyFilter( filter?: AnyFilterLike | null -): filter is EventPropertyFilter | PersonPropertyFilter { - return filter?.type === PropertyFilterType.Event || filter?.type === PropertyFilterType.Person +): filter is EventPropertyFilter | PersonPropertyFilter | SessionPropertyFilter { + return ( + filter?.type === PropertyFilterType.Event || + filter?.type === PropertyFilterType.Person || + filter?.type === PropertyFilterType.Session + ) } export function isElementPropertyFilter(filter?: AnyFilterLike | null): filter is ElementPropertyFilter { return filter?.type === PropertyFilterType.Element @@ -308,6 +312,8 @@ export function propertyFilterTypeToPropertyDefinitionType( ? PropertyDefinitionType.Person : filterType === PropertyFilterType.Group ? PropertyDefinitionType.Group + : filterType === PropertyFilterType.Session + ? PropertyDefinitionType.Session : PropertyDefinitionType.Event } diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 868156528ece8..1ef498660d74a 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -5667,6 +5667,9 @@ }, { "$ref": "#/definitions/PersonPropertyFilter" + }, + { + "$ref": "#/definitions/SessionPropertyFilter" } ] }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 01708078b175b..a923dfaec77cf 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -23,6 +23,7 @@ import { PropertyGroupFilter, PropertyMathType, RetentionFilterType, + SessionPropertyFilter, StickinessFilterType, TrendsFilterType, } from '~/types' @@ -985,7 +986,7 @@ export interface SessionsTimelineQuery extends DataNode { before?: string response?: SessionsTimelineQueryResponse } -export type WebAnalyticsPropertyFilter = EventPropertyFilter | PersonPropertyFilter +export type WebAnalyticsPropertyFilter = EventPropertyFilter | PersonPropertyFilter | SessionPropertyFilter export type WebAnalyticsPropertyFilters = WebAnalyticsPropertyFilter[] export interface WebAnalyticsQueryBase { diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index 0f65abec6f0c9..8e223626caeeb 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -1,5 +1,5 @@ import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters' -import { isEventPropertyOrPersonPropertyFilter } from 'lib/components/PropertyFilters/utils' +import { isEventPersonOrSessionPropertyFilter } from 'lib/components/PropertyFilters/utils' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { WebAnalyticsPropertyFilters } from '~/queries/schema' @@ -14,7 +14,7 @@ export const WebPropertyFilters = ({ return ( setWebAnalyticsFilters(filters.filter(isEventPropertyOrPersonPropertyFilter))} + onChange={(filters) => setWebAnalyticsFilters(filters.filter(isEventPersonOrSessionPropertyFilter))} propertyFilters={webAnalyticsFilters} pageKey="web-analytics" eventNames={['$pageview', '$pageleave', '$autocapture']} diff --git a/frontend/src/types.ts b/frontend/src/types.ts index b67305814e7aa..2a0c7151ca111 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2814,6 +2814,7 @@ export enum PropertyDefinitionType { Event = 'event', Person = 'person', Group = 'group', + Session = 'session', } export interface PropertyDefinition { diff --git a/posthog/schema.py b/posthog/schema.py index 46ad0beb9a11a..281cd8ef3a039 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -1699,7 +1699,7 @@ class WebAnalyticsQueryBase(BaseModel): ) dateRange: Optional[DateRange] = None modifiers: Optional[HogQLQueryModifiers] = None - properties: list[Union[EventPropertyFilter, PersonPropertyFilter]] + properties: list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]] sampling: Optional[Sampling] = None useSessionsTable: Optional[bool] = None @@ -1712,7 +1712,7 @@ class WebOverviewQuery(BaseModel): dateRange: Optional[DateRange] = None kind: Literal["WebOverviewQuery"] = "WebOverviewQuery" modifiers: Optional[HogQLQueryModifiers] = None - properties: list[Union[EventPropertyFilter, PersonPropertyFilter]] + properties: list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]] response: Optional[WebOverviewQueryResponse] = None sampling: Optional[Sampling] = None useSessionsTable: Optional[bool] = None @@ -1730,7 +1730,7 @@ class WebStatsTableQuery(BaseModel): kind: Literal["WebStatsTableQuery"] = "WebStatsTableQuery" limit: Optional[int] = None modifiers: Optional[HogQLQueryModifiers] = None - properties: list[Union[EventPropertyFilter, PersonPropertyFilter]] + properties: list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]] response: Optional[WebStatsTableQueryResponse] = None sampling: Optional[Sampling] = None useSessionsTable: Optional[bool] = None @@ -1743,7 +1743,7 @@ class WebTopClicksQuery(BaseModel): dateRange: Optional[DateRange] = None kind: Literal["WebTopClicksQuery"] = "WebTopClicksQuery" modifiers: Optional[HogQLQueryModifiers] = None - properties: list[Union[EventPropertyFilter, PersonPropertyFilter]] + properties: list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]] response: Optional[WebTopClicksQueryResponse] = None sampling: Optional[Sampling] = None useSessionsTable: Optional[bool] = None From 1dec023fe0113f9885a3e5f331899e9e57d92228 Mon Sep 17 00:00:00 2001 From: Robbie Date: Fri, 12 Apr 2024 15:03:15 +0100 Subject: [PATCH 04/32] Fix typings --- posthog/hogql/database/schema/test/test_sessions.py | 2 +- .../web_analytics/web_analytics_query_runner.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/posthog/hogql/database/schema/test/test_sessions.py b/posthog/hogql/database/schema/test/test_sessions.py index 1e751cfefe0d6..6c71df3130dde 100644 --- a/posthog/hogql/database/schema/test/test_sessions.py +++ b/posthog/hogql/database/schema/test/test_sessions.py @@ -133,6 +133,6 @@ def test_persons_and_sessions_on_events(self): self.team, ) - [row1, row2] = response.results + [row1, row2] = response.results or [] self.assertEqual(row1, (p1.uuid, "source1")) self.assertEqual(row2, (p2.uuid, "source2")) diff --git a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py index fb1288ac1bd26..de154e4c46163 100644 --- a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py +++ b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py @@ -24,6 +24,7 @@ WebStatsTableQuery, PersonPropertyFilter, SamplingRate, + SessionPropertyFilter, ) from posthog.utils import generate_cache_key, get_safe_cache @@ -51,7 +52,9 @@ def pathname_property_filter(self) -> Optional[EventPropertyFilter]: return None @cached_property - def property_filters_without_pathname(self) -> list[Union[EventPropertyFilter, PersonPropertyFilter]]: + def property_filters_without_pathname( + self + ) -> list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]]: return [p for p in self.query.properties if p.key != "$pathname"] def session_where(self, include_previous_period: Optional[bool] = None): From 7eadc34ca0579b8390f1ca654d260ad358f639da Mon Sep 17 00:00:00 2001 From: Robbie Date: Mon, 15 Apr 2024 14:17:27 +0100 Subject: [PATCH 05/32] Working session definition fetching --- .../DefinitionPopover/DefinitionPopover.tsx | 3 +- .../TaxonomicFilter/InfiniteList.tsx | 2 + .../TaxonomicFilter/taxonomicFilterLogic.tsx | 13 +-- .../lib/components/TaxonomicFilter/types.ts | 2 +- frontend/src/lib/taxonomy.tsx | 19 ++-- .../src/models/propertyDefinitionsModel.ts | 81 ++++++++++------- .../web-analytics/WebPropertyFilters.tsx | 6 +- posthog/api/__init__.py | 2 + posthog/api/property_definition.py | 12 ++- posthog/api/session.py | 91 +++++++++++++++++++ posthog/hogql/database/schema/sessions.py | 29 ++++++ .../database/schema/test/test_sessions.py | 2 +- .../util/session_where_clause_extractor.py | 6 ++ posthog/models/sessions/sql.py | 64 +++++++++++++ posthog/queries/property_values.py | 32 +++++++ 15 files changed, 308 insertions(+), 56 deletions(-) create mode 100644 posthog/api/session.py diff --git a/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx b/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx index 4d93fd38f2ae9..b9a692f3df411 100644 --- a/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx +++ b/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx @@ -124,7 +124,8 @@ function Example({ value }: { value?: string }): JSX.Element { type === TaxonomicFilterGroupType.EventFeatureFlags || type === TaxonomicFilterGroupType.PersonProperties || type === TaxonomicFilterGroupType.GroupsPrefix || - type === TaxonomicFilterGroupType.Metadata + type === TaxonomicFilterGroupType.Metadata || + type === TaxonomicFilterGroupType.Sessions ) { data = getCoreFilterDefinition(value, type) } else if (type === TaxonomicFilterGroupType.Elements) { diff --git a/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx b/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx index ab03c6f9721c0..08fbb1b47fa72 100644 --- a/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx @@ -106,6 +106,7 @@ const renderItemContents = ({ listGroupType === TaxonomicFilterGroupType.Events || listGroupType === TaxonomicFilterGroupType.CustomEvents || listGroupType === TaxonomicFilterGroupType.Metadata || + listGroupType === TaxonomicFilterGroupType.Sessions || listGroupType.startsWith(TaxonomicFilterGroupType.GroupsPrefix) ? ( <>
@@ -160,6 +161,7 @@ const selectedItemHasPopover = ( TaxonomicFilterGroupType.Cohorts, TaxonomicFilterGroupType.CohortsWithAllUsers, TaxonomicFilterGroupType.Metadata, + TaxonomicFilterGroupType.Sessions, ].includes(listGroupType) || listGroupType.startsWith(TaxonomicFilterGroupType.GroupsPrefix)) ) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index 966b33c5527ed..914718ba5474d 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -486,18 +486,15 @@ export const taxonomicFilterLogic = kea([ getPopoverHeader: () => 'Notebooks', }, { - name: 'Sessions', + name: 'Session Properties', searchPlaceholder: 'sessions', type: TaxonomicFilterGroupType.Sessions, - options: [ - { - name: 'Session duration', - value: '$session_duration', - }, - ], + value: 'sessions', getName: (option: any) => option.name, - getValue: (option: any) => option.value, + getValue: (option) => option.name, getPopoverHeader: () => 'Session', + endpoint: `api/projects/${teamId}/sessions/property_definitions`, + getIcon: getPropertyDefinitionIcon, }, { name: 'HogQL', diff --git a/frontend/src/lib/components/TaxonomicFilter/types.ts b/frontend/src/lib/components/TaxonomicFilter/types.ts index 8b46784a19b7e..fa6b82a0d74d4 100644 --- a/frontend/src/lib/components/TaxonomicFilter/types.ts +++ b/frontend/src/lib/components/TaxonomicFilter/types.ts @@ -105,7 +105,7 @@ export enum TaxonomicFilterGroupType { Plugins = 'plugins', Dashboards = 'dashboards', GroupNamesPrefix = 'name_groups', - Sessions = 'sessions', + Sessions = 'session_properties', HogQLExpression = 'hogql_expression', Notebooks = 'notebooks', } diff --git a/frontend/src/lib/taxonomy.tsx b/frontend/src/lib/taxonomy.tsx index 9b5297cf62955..643dad1ac6202 100644 --- a/frontend/src/lib/taxonomy.tsx +++ b/frontend/src/lib/taxonomy.tsx @@ -993,8 +993,8 @@ export const CORE_FILTER_DEFINITIONS_BY_GROUP = { }, numerical_event_properties: {}, // Same as event properties, see assignment below person_properties: {}, // Currently person properties are the same as event properties, see assignment below - sessions: { - $session_duration: { + session_properties: { + duration: { label: 'Session duration', description: ( @@ -1006,13 +1006,13 @@ export const CORE_FILTER_DEFINITIONS_BY_GROUP = { ), examples: ['01:04:12'], }, - $min_timestamp: { - label: 'First timestamp', + $start_timestamp: { + label: 'Start timestamp', description: The timestamp of the first event from this session., examples: [new Date().toISOString()], }, - $max_timestamp: { - label: 'Last timestamp', + $end_timestamp: { + label: 'End timestamp', description: The timestamp of the last event from this session, examples: [new Date().toISOString()], }, @@ -1036,7 +1036,7 @@ export const CORE_FILTER_DEFINITIONS_BY_GROUP = { description: The number of autocapture events in this session, examples: ['123'], }, - $initial_channel_type: { + $channel_type: { label: 'Channel type', description: What type of acquisition channel this traffic came from., examples: ['Paid Search', 'Organic Video', 'Direct'], @@ -1079,20 +1079,21 @@ for (const [key, value] of Object.entries(CORE_FILTER_DEFINITIONS_BY_GROUP.event CORE_FILTER_DEFINITIONS_BY_GROUP.person_properties[key] = value } if (SESSION_INITIAL_PROPERTIES_ADAPTED_FROM_EVENTS.has(key)) { - CORE_FILTER_DEFINITIONS_BY_GROUP.sessions[`$initial_${key.replace(/^\$/, '')}`] = { + CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties[`$initial_${key.replace(/^\$/, '')}`] = { ...value, label: `Initial ${value.label}`, description: 'description' in value ? `${value.description} Data from the first event in this session.` : 'Data from the first event in this session.', + examples: value.examples, } } } // We treat `$session_duration` as an event property in the context of series `math`, but it's fake in a sense CORE_FILTER_DEFINITIONS_BY_GROUP.event_properties.$session_duration = - CORE_FILTER_DEFINITIONS_BY_GROUP.sessions.$session_duration + CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties.duration export const PROPERTY_KEYS = Object.keys(CORE_FILTER_DEFINITIONS_BY_GROUP.event_properties) diff --git a/frontend/src/models/propertyDefinitionsModel.ts b/frontend/src/models/propertyDefinitionsModel.ts index 338e60a5e956f..a4ca7fc6614d4 100644 --- a/frontend/src/models/propertyDefinitionsModel.ts +++ b/frontend/src/models/propertyDefinitionsModel.ts @@ -1,4 +1,4 @@ -import { actions, kea, listeners, path, reducers, selectors } from 'kea' +import { actions, connect, kea, listeners, path, reducers, selectors } from 'kea' import api, { ApiMethodOptions } from 'lib/api' import { TaxonomicFilterValue } from 'lib/components/TaxonomicFilter/types' import { dayjs } from 'lib/dayjs' @@ -21,19 +21,6 @@ import type { propertyDefinitionsModelType } from './propertyDefinitionsModelTyp export type PropertyDefinitionStorage = Record -// List of property definitions that are calculated on the backend. These -// are valid properties that do not exist on events. -const localProperties: PropertyDefinitionStorage = { - 'event/$session_duration': { - id: '$session_duration', - name: '$session_duration', - description: 'Duration of the session', - is_numerical: true, - is_seen_on_filtered_events: false, - property_type: PropertyType.Duration, - }, -} - export type FormatPropertyValueForDisplayFunction = ( propertyName?: BreakdownKeyType, valueToFormat?: PropertyFilterValue, @@ -95,8 +82,42 @@ const checkOrLoadPropertyDefinition = ( return null } +const getEndpoint = ( + teamId: number, + type: PropertyDefinitionType, + propertyKey: string, + eventNames: string[] | undefined, + newInput: string | undefined +): string => { + let eventParams = '' + for (const eventName of eventNames || []) { + eventParams += `&event_name=${eventName}` + } + + if (type === PropertyDefinitionType.Session) { + return ( + `api/projects/${teamId}/${type}s/values/?key=` + + encodeURIComponent(propertyKey) + + (newInput ? '&value=' + encodeURIComponent(newInput) : '') + + eventParams + ) + } + + return ( + 'api/' + + type + + '/values/?key=' + + encodeURIComponent(propertyKey) + + (newInput ? '&value=' + encodeURIComponent(newInput) : '') + + eventParams + ) +} + export const propertyDefinitionsModel = kea([ path(['models', 'propertyDefinitionsModel']), + connect({ + values: [teamLogic, ['currentTeamId']], + }), actions({ // public loadPropertyDefinitions: ( @@ -123,12 +144,14 @@ export const propertyDefinitionsModel = kea([ }), reducers({ propertyDefinitionStorage: [ - { ...localProperties } as PropertyDefinitionStorage, + {} as PropertyDefinitionStorage, { - updatePropertyDefinitions: (state, { propertyDefinitions }) => ({ - ...state, - ...propertyDefinitions, - }), + updatePropertyDefinitions: (state, { propertyDefinitions }) => { + return { + ...state, + ...propertyDefinitions, + } + }, }, ], options: [ @@ -179,7 +202,7 @@ export const propertyDefinitionsModel = kea([ // take the first 50 pending properties to avoid the 4k query param length limit const allPending = values.pendingProperties.slice(0, 50) const pendingByType: Record< - 'event' | 'person' | 'group/0' | 'group/1' | 'group/2' | 'group/3' | 'group/4', + 'event' | 'person' | 'group/0' | 'group/1' | 'group/2' | 'group/3' | 'group/4' | 'session', string[] > = { event: [], @@ -189,6 +212,7 @@ export const propertyDefinitionsModel = kea([ 'group/2': [], 'group/3': [], 'group/4': [], + session: [], } for (const key of allPending) { let [type, ...rest] = key.split('/') @@ -268,10 +292,10 @@ export const propertyDefinitionsModel = kea([ }, loadPropertyValues: async ({ endpoint, type, newInput, propertyKey, eventNames }, breakpoint) => { - if (['cohort', 'session'].includes(type)) { + if (['cohort'].includes(type)) { return } - if (!propertyKey) { + if (!propertyKey || values.currentTeamId === null) { return } @@ -286,19 +310,8 @@ export const propertyDefinitionsModel = kea([ signal: cache.abortController.signal, } - let eventParams = '' - for (const eventName of eventNames || []) { - eventParams += `&event_name=${eventName}` - } - const propValues: PropValue[] = await api.get( - endpoint || - 'api/' + - type + - '/values/?key=' + - encodeURIComponent(propertyKey) + - (newInput ? '&value=' + encodeURIComponent(newInput) : '') + - eventParams, + endpoint || getEndpoint(values.currentTeamId, type, propertyKey, eventNames, newInput), methodOptions ) breakpoint() diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index 8e223626caeeb..6a3d510db197f 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -13,7 +13,11 @@ export const WebPropertyFilters = ({ }): JSX.Element => { return ( setWebAnalyticsFilters(filters.filter(isEventPersonOrSessionPropertyFilter))} propertyFilters={webAnalyticsFilters} pageKey="web-analytics" diff --git a/posthog/api/__init__.py b/posthog/api/__init__.py index e9828f74e7f1d..453a2c4417d82 100644 --- a/posthog/api/__init__.py +++ b/posthog/api/__init__.py @@ -5,6 +5,7 @@ from posthog.settings import EE_AVAILABLE from posthog.warehouse.api import external_data_source, saved_query, table, view_link, external_data_schema from ..heatmaps.heatmaps_api import LegacyHeatmapViewSet, HeatmapViewSet +from .session import SessionViewSet from ..session_recordings.session_recording_api import SessionRecordingViewSet from . import ( activity_log, @@ -333,6 +334,7 @@ def api_not_found(request): ["team_id"], ) projects_router.register(r"heatmaps", HeatmapViewSet, "project_heatmaps", ["team_id"]) +projects_router.register(r"sessions", SessionViewSet, "project_sessions", ["team_id"]) if EE_AVAILABLE: from ee.clickhouse.views.experiments import ClickhouseExperimentsViewSet diff --git a/posthog/api/property_definition.py b/posthog/api/property_definition.py index 6a87fc6f348d7..7db63497d5a9d 100644 --- a/posthog/api/property_definition.py +++ b/posthog/api/property_definition.py @@ -35,7 +35,7 @@ class PropertyDefinitionQuerySerializer(serializers.Serializer): ) type = serializers.ChoiceField( - choices=["event", "person", "group"], + choices=["event", "person", "group", "session"], help_text="What property definitions to return", default="event", ) @@ -192,6 +192,16 @@ def with_type_filter(self, type: str, group_type_index: Optional[int]): "group_type_index": group_type_index, }, ) + elif type == "session": + return dataclasses.replace( + self, + should_join_event_property=False, + params={ + **self.params, + "type": PropertyDefinition.Type.SESSION, + "group_type_index": -1, + }, + ) def with_event_property_filter( self, event_names: Optional[str], filter_by_event_names: Optional[bool] diff --git a/posthog/api/session.py b/posthog/api/session.py new file mode 100644 index 0000000000000..3ebf103a8a5fa --- /dev/null +++ b/posthog/api/session.py @@ -0,0 +1,91 @@ +import json + +from rest_framework import request, response, viewsets +from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError +from rest_framework.pagination import LimitOffsetPagination +from rest_framework.settings import api_settings +from rest_framework_csv import renderers as csvrenderers + +from posthog.api.routing import TeamAndOrgViewSetMixin +from posthog.hogql.database.schema.sessions import get_lazy_session_table_properties +from posthog.models.event.util import ClickhouseEventSerializer +from posthog.queries.property_values import get_session_column_values_for_key +from posthog.rate_limit import ( + ClickHouseBurstRateThrottle, + ClickHouseSustainedRateThrottle, +) +from posthog.utils import convert_property_value, flatten + +QUERY_DEFAULT_EXPORT_LIMIT = 3_500 + + +class UncountedLimitOffsetPagination(LimitOffsetPagination): + """ + the events api works with the default LimitOffsetPagination, but the + results don't have a count, so we need to override the pagination class + to remove the count from the response schema + """ + + def get_paginated_response_schema(self, schema): + return { + "type": "object", + "properties": { + "next": { + "type": "string", + "nullable": True, + "format": "uri", + "example": "http://api.example.org/accounts/?{offset_param}=400&{limit_param}=100".format( + offset_param=self.offset_query_param, limit_param=self.limit_query_param + ), + }, + "results": schema, + }, + } + + +class SessionViewSet( + TeamAndOrgViewSetMixin, + viewsets.ViewSet, +): + # queryset = Session.objects.none() # not used + scope_object = "query" + renderer_classes = tuple(api_settings.DEFAULT_RENDERER_CLASSES) + (csvrenderers.PaginatedCSVRenderer,) + serializer_class = ClickhouseEventSerializer + throttle_classes = [ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle] + pagination_class = UncountedLimitOffsetPagination + + @action(methods=["GET"], detail=False) + def values(self, request: request.Request, **kwargs) -> response.Response: + team = self.team + + key = request.GET.get("key") + search_term = request.GET.get("value") + + if not key: + raise ValidationError(detail=f"Key not provided") + + result = get_session_column_values_for_key(key, team, search_term=search_term) + + flattened = [] + for value in result: + try: + # Try loading as json for dicts or arrays + flattened.append(json.loads(value[0])) + except json.decoder.JSONDecodeError: + flattened.append(value[0]) + return response.Response([{"name": convert_property_value(value)} for value in flatten(flattened)]) + + @action(methods=["GET"], detail=False) + def property_definitions(self, request: request.Request, **kwargs) -> response.Response: + search = request.GET.get("search") + + # unlike e.g. event properties, there's a very limited number of session properties, + # so we can just return them all + results = get_lazy_session_table_properties(search) + return response.Response( + { + "count": len(results), + "results": results, + } + ) diff --git a/posthog/hogql/database/schema/sessions.py b/posthog/hogql/database/schema/sessions.py index 0bd6bfef09caf..583c8dad2bc7c 100644 --- a/posthog/hogql/database/schema/sessions.py +++ b/posthog/hogql/database/schema/sessions.py @@ -11,10 +11,12 @@ StringArrayDatabaseField, DatabaseField, LazyTable, + FloatDatabaseField, ) from posthog.hogql.database.schema.channel_type import create_channel_type_expr from posthog.hogql.database.schema.util.session_where_clause_extractor import SessionMinTimestampWhereClauseExtractor from posthog.hogql.errors import ResolutionError +from posthog.models.property_definition import PropertyType if TYPE_CHECKING: pass @@ -220,3 +222,30 @@ def join_events_table_to_sessions_table( ) ) return join_expr + + +def get_lazy_session_table_properties(search: Optional[str]): + # some fields shouldn't appear as properties + hidden_fields = {"team_id", "distinct_id", "session_id", "id", "$event_count_map", "$urls"} + + # some fields should have a specific property type which isn't derivable from the type of database field + property_type_overrides = { + "duration": PropertyType.Duration, + } + + results = [ + { + "id": field_name, + "name": field_name, + "is_numerical": isinstance(field_definition, IntegerDatabaseField) + or isinstance(field_definition, FloatDatabaseField), + "property_type": property_type_overrides.get(field_name, None) or PropertyType.Numeric + if isinstance(field_definition, IntegerDatabaseField) or isinstance(field_definition, FloatDatabaseField) + else PropertyType.String, + "is_seen_on_filtered_events": None, + "tags": [], + } + for field_name, field_definition in LAZY_SESSIONS_FIELDS.items() + if (not search or search.lower() in field_name.lower()) and field_name not in hidden_fields + ] + return results diff --git a/posthog/hogql/database/schema/test/test_sessions.py b/posthog/hogql/database/schema/test/test_sessions.py index 6c71df3130dde..230ffc1bc1897 100644 --- a/posthog/hogql/database/schema/test/test_sessions.py +++ b/posthog/hogql/database/schema/test/test_sessions.py @@ -127,7 +127,7 @@ def test_persons_and_sessions_on_events(self): response = execute_hogql_query( parse_select( - "select events.person_id, session.initial_utm_source from events where $session_id = {session_id} or $session_id = {session_id2} order by 2 asc", + "select events.person_id, session.$initial_utm_source from events where $session_id = {session_id} or $session_id = {session_id2} order by 2 asc", placeholders={"session_id": ast.Constant(value=s1), "session_id2": ast.Constant(value=s2)}, ), self.team, diff --git a/posthog/hogql/database/schema/util/session_where_clause_extractor.py b/posthog/hogql/database/schema/util/session_where_clause_extractor.py index 3d94a4a0f691f..139b2724d0d6f 100644 --- a/posthog/hogql/database/schema/util/session_where_clause_extractor.py +++ b/posthog/hogql/database/schema/util/session_where_clause_extractor.py @@ -297,6 +297,9 @@ def visit_placeholder(self, node: ast.Placeholder) -> bool: def visit_alias(self, node: ast.Alias) -> bool: return self.visit(node.expr) + def visit_select_query(self, node: ast.SelectQuery) -> bool: + return False + def is_simple_timestamp_field_expression(expr: ast.Expr, context: HogQLContext) -> bool: return IsSimpleTimestampFieldExpressionVisitor(context).visit(expr) @@ -393,6 +396,9 @@ def visit_alias(self, node: ast.Alias) -> bool: return self.visit(node.expr) + def visit_select_query(self, node: ast.SelectQuery) -> bool: + return False + def rewrite_timestamp_field(expr: ast.Expr, context: HogQLContext) -> ast.Expr: return RewriteTimestampFieldVisitor(context).visit(expr) diff --git a/posthog/models/sessions/sql.py b/posthog/models/sessions/sql.py index 6bebc73e023f4..a68ec2ff97e1e 100644 --- a/posthog/models/sessions/sql.py +++ b/posthog/models/sessions/sql.py @@ -260,3 +260,67 @@ def source_column(column_name: str) -> str: GROUP BY session_id, team_id """ ) + +SELECT_SESSION_PROP_VALUES_SQL = """ +SELECT + value, + count(value) +FROM ( + SELECT + {property_field} as value + FROM + sessions + WHERE + team_id = %(team_id)s AND + {property_field} IS NOT NULL AND + {property_field} != '' + ORDER BY session_id DESC + LIMIT 100000 +) +GROUP BY value +ORDER BY count(value) DESC +LIMIT 20 +""" + +SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER = """ +SELECT + value, + count(value) +FROM ( + SELECT + {property_field} as value + FROM + sessions + WHERE + team_id = %(team_id)s AND + {property_field} ILIKE %(value)s + ORDER BY session_id DESC + LIMIT 100000 +) +GROUP BY value +ORDER BY count(value) DESC +LIMIT 20 +""" + + +SESSION_PROPERTY_TO_COLUMN_MAP = { + "$initial_referring_domain": "initial_referring_domain", + "$initial_utm_source": "initial_utm_source", + "$initial_utm_campaign": "initial_utm_campaign", + "$initial_utm_medium": "initial_utm_medium", + "$initial_utm_term": "initial_utm_term", + "$initial_utm_content": "initial_utm_content", + "$initial_gclid": "initial_gclid", + "$initial_gad_source": "initial_gad_source", + "$initial_gclsrc": "initial_gclsrc", + "$initial_dclid": "initial_dclid", + "$initial_gbraid": "initial_gbraid", + "$initial_wbraid": "initial_wbraid", + "$initial_fbclid": "initial_fbclid", + "$initial_msclkid": "initial_msclkid", + "$initial_twclid": "initial_twclid", + "$initial_li_fat_id": "initial_li_fat_id", + "$initial_mc_cid": "initial_mc_cid", + "$initial_igshid": "initial_igshid", + "$initial_ttclid": "initial_ttclid", +} diff --git a/posthog/queries/property_values.py b/posthog/queries/property_values.py index 0e79d15146af6..fe77dded0ac42 100644 --- a/posthog/queries/property_values.py +++ b/posthog/queries/property_values.py @@ -2,12 +2,18 @@ from django.utils import timezone +from posthog.hogql.database.schema.channel_type import POSSIBLE_CHANNEL_TYPES from posthog.models.event.sql import SELECT_PROP_VALUES_SQL_WITH_FILTER from posthog.models.person.sql import ( SELECT_PERSON_PROP_VALUES_SQL, SELECT_PERSON_PROP_VALUES_SQL_WITH_FILTER, ) from posthog.models.property.util import get_property_string_expr +from posthog.models.sessions.sql import ( + SESSION_PROPERTY_TO_COLUMN_MAP, + SELECT_SESSION_PROP_VALUES_SQL, + SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER, +) from posthog.models.team import Team from posthog.queries.insight import insight_sync_execute from posthog.utils import relative_date_parse @@ -79,3 +85,29 @@ def get_person_property_values_for_key(key: str, team: Team, value: Optional[str query_type="get_person_property_values", team_id=team.pk, ) + + +def get_session_column_values_for_key(key: str, team: Team, search_term: Optional[str] = None): + # the sessions table does not have a properties json object like the events and person tables + + if key == "$channel_type": + return [[name] for name in POSSIBLE_CHANNEL_TYPES if not search_term or search_term.lower() in name.lower()] + + column = SESSION_PROPERTY_TO_COLUMN_MAP.get(key) + + if not column: + return [] + + if search_term: + return insight_sync_execute( + SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER.format(property_field=column), + {"team_id": team.pk, "key": key, "value": "%{}%".format(search_term)}, + query_type="get_session_property_values_with_value", + team_id=team.pk, + ) + return insight_sync_execute( + SELECT_SESSION_PROP_VALUES_SQL.format(property_field=column), + {"team_id": team.pk, "key": key}, + query_type="get_session_property_values", + team_id=team.pk, + ) From d90274a7762ca972309c5072bf7293509d1a98ac Mon Sep 17 00:00:00 2001 From: Robbie Date: Mon, 15 Apr 2024 21:51:29 +0100 Subject: [PATCH 06/32] Working session property definitions --- frontend/src/lib/api.ts | 21 +++++++++++++ .../src/models/propertyDefinitionsModel.ts | 17 +++++++--- posthog/api/session.py | 31 ------------------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 5e3a3aab0604f..09e3924e21deb 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -388,6 +388,10 @@ class ApiRequest { .withQueryString(queryParams) } + public sessionPropertyDefinitions(teamId?: TeamType['id']): ApiRequest { + return this.projectsDetail(teamId).addPathComponent('sessions').addPathComponent('property_definitions') + } + public dataManagementActivity(teamId?: TeamType['id']): ApiRequest { return this.projectsDetail(teamId).addPathComponent('data_management').addPathComponent('activity') } @@ -1212,6 +1216,23 @@ const api = { }, }, + sessions: { + async propertyDefinitions({ + teamId = ApiConfig.getCurrentTeamId(), + search, + properties, + }: { + teamId?: TeamType['id'] + search?: string + properties?: string[] + }): Promise> { + return new ApiRequest() + .sessionPropertyDefinitions(teamId) + .withQueryString(toParams({ search, ...(properties ? { properties: properties.join(',') } : {}) })) + .get() + }, + }, + cohorts: { async get(cohortId: CohortType['id']): Promise { return await new ApiRequest().cohortsDetail(cohortId).get() diff --git a/frontend/src/models/propertyDefinitionsModel.ts b/frontend/src/models/propertyDefinitionsModel.ts index a4ca7fc6614d4..4930b6c2a1796 100644 --- a/frontend/src/models/propertyDefinitionsModel.ts +++ b/frontend/src/models/propertyDefinitionsModel.ts @@ -1,5 +1,5 @@ import { actions, connect, kea, listeners, path, reducers, selectors } from 'kea' -import api, { ApiMethodOptions } from 'lib/api' +import api, { ApiMethodOptions, CountedPaginatedResponse } from 'lib/api' import { TaxonomicFilterValue } from 'lib/components/TaxonomicFilter/types' import { dayjs } from 'lib/dayjs' import { captureTimeToSeeData } from 'lib/internalMetrics' @@ -250,10 +250,17 @@ export const propertyDefinitionsModel = kea([ } // and then fetch them - const propertyDefinitions = await api.propertyDefinitions.list({ - properties: pending, - ...queryParams, - }) + let propertyDefinitions: CountedPaginatedResponse + if (type === 'session') { + propertyDefinitions = await api.sessions.propertyDefinitions({ + properties: pending, + }) + } else { + propertyDefinitions = await api.propertyDefinitions.list({ + properties: pending, + ...queryParams, + }) + } for (const propertyDefinition of propertyDefinitions.results) { newProperties[`${type}/${propertyDefinition.name}`] = propertyDefinition diff --git a/posthog/api/session.py b/posthog/api/session.py index 3ebf103a8a5fa..aaf34399d1e63 100644 --- a/posthog/api/session.py +++ b/posthog/api/session.py @@ -3,13 +3,11 @@ from rest_framework import request, response, viewsets from rest_framework.decorators import action from rest_framework.exceptions import ValidationError -from rest_framework.pagination import LimitOffsetPagination from rest_framework.settings import api_settings from rest_framework_csv import renderers as csvrenderers from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.hogql.database.schema.sessions import get_lazy_session_table_properties -from posthog.models.event.util import ClickhouseEventSerializer from posthog.queries.property_values import get_session_column_values_for_key from posthog.rate_limit import ( ClickHouseBurstRateThrottle, @@ -17,43 +15,14 @@ ) from posthog.utils import convert_property_value, flatten -QUERY_DEFAULT_EXPORT_LIMIT = 3_500 - - -class UncountedLimitOffsetPagination(LimitOffsetPagination): - """ - the events api works with the default LimitOffsetPagination, but the - results don't have a count, so we need to override the pagination class - to remove the count from the response schema - """ - - def get_paginated_response_schema(self, schema): - return { - "type": "object", - "properties": { - "next": { - "type": "string", - "nullable": True, - "format": "uri", - "example": "http://api.example.org/accounts/?{offset_param}=400&{limit_param}=100".format( - offset_param=self.offset_query_param, limit_param=self.limit_query_param - ), - }, - "results": schema, - }, - } - class SessionViewSet( TeamAndOrgViewSetMixin, viewsets.ViewSet, ): - # queryset = Session.objects.none() # not used scope_object = "query" renderer_classes = tuple(api_settings.DEFAULT_RENDERER_CLASSES) + (csvrenderers.PaginatedCSVRenderer,) - serializer_class = ClickhouseEventSerializer throttle_classes = [ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle] - pagination_class = UncountedLimitOffsetPagination @action(methods=["GET"], detail=False) def values(self, request: request.Request, **kwargs) -> response.Response: From 45998497fb7b876f70c3644db1fb68b77563720d Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 10:18:44 +0100 Subject: [PATCH 07/32] Remove property allow list in web analytics --- .../web-analytics/WebPropertyFilters.tsx | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index 6a3d510db197f..f33180f8fc63b 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -22,35 +22,6 @@ export const WebPropertyFilters = ({ propertyFilters={webAnalyticsFilters} pageKey="web-analytics" eventNames={['$pageview', '$pageleave', '$autocapture']} - propertyAllowList={{ - [TaxonomicFilterGroupType.EventProperties]: [ - '$pathname', - '$host', - '$browser', - '$os', - '$device_type', - '$geoip_country_code', - '$geoip_subdivision_1_code', - '$geoip_city_name', - // re-enable after https://github.com/PostHog/posthog-js/pull/875 is merged - // '$client_session_initial_pathname', - // '$client_session_initial_referring_host', - // '$client_session_initial_utm_source', - // '$client_session_initial_utm_campaign', - // '$client_session_initial_utm_medium', - // '$client_session_initial_utm_content', - // '$client_session_initial_utm_term', - ], - [TaxonomicFilterGroupType.PersonProperties]: [ - '$initial_pathname', - '$initial_referring_domain', - '$initial_utm_source', - '$initial_utm_campaign', - '$initial_utm_medium', - '$initial_utm_content', - '$initial_utm_term', - ], - }} /> ) } From 6c2bced5074f0b5bf92d33132c76fc04a1e08a33 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 10:21:17 +0100 Subject: [PATCH 08/32] Put session properties first --- frontend/src/scenes/web-analytics/WebPropertyFilters.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index f33180f8fc63b..6f94d481d687e 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -14,9 +14,9 @@ export const WebPropertyFilters = ({ return ( setWebAnalyticsFilters(filters.filter(isEventPersonOrSessionPropertyFilter))} propertyFilters={webAnalyticsFilters} From a0a07aad7c8396d73794712cec28f1800fc53bc7 Mon Sep 17 00:00:00 2001 From: Robbie Date: Tue, 16 Apr 2024 11:15:12 +0100 Subject: [PATCH 09/32] Add session values --- posthog/api/session.py | 5 +- posthog/hogql/database/schema/sessions.py | 69 ++++++++++++++++++++++- posthog/models/sessions/sql.py | 37 +++--------- posthog/queries/property_values.py | 32 ----------- 4 files changed, 75 insertions(+), 68 deletions(-) diff --git a/posthog/api/session.py b/posthog/api/session.py index aaf34399d1e63..7683187f74d2a 100644 --- a/posthog/api/session.py +++ b/posthog/api/session.py @@ -7,8 +7,7 @@ from rest_framework_csv import renderers as csvrenderers from posthog.api.routing import TeamAndOrgViewSetMixin -from posthog.hogql.database.schema.sessions import get_lazy_session_table_properties -from posthog.queries.property_values import get_session_column_values_for_key +from posthog.hogql.database.schema.sessions import get_lazy_session_table_properties, get_lazy_session_table_values from posthog.rate_limit import ( ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle, @@ -34,7 +33,7 @@ def values(self, request: request.Request, **kwargs) -> response.Response: if not key: raise ValidationError(detail=f"Key not provided") - result = get_session_column_values_for_key(key, team, search_term=search_term) + result = get_lazy_session_table_values(key, search_term=search_term, team=team) flattened = [] for value in result: diff --git a/posthog/hogql/database/schema/sessions.py b/posthog/hogql/database/schema/sessions.py index 583c8dad2bc7c..61730e98a7bda 100644 --- a/posthog/hogql/database/schema/sessions.py +++ b/posthog/hogql/database/schema/sessions.py @@ -1,4 +1,4 @@ -from typing import cast, Any, TYPE_CHECKING +from typing import cast, Any, Optional, TYPE_CHECKING from posthog.hogql import ast from posthog.hogql.context import HogQLContext @@ -13,13 +13,18 @@ LazyTable, FloatDatabaseField, ) -from posthog.hogql.database.schema.channel_type import create_channel_type_expr +from posthog.hogql.database.schema.channel_type import create_channel_type_expr, POSSIBLE_CHANNEL_TYPES from posthog.hogql.database.schema.util.session_where_clause_extractor import SessionMinTimestampWhereClauseExtractor from posthog.hogql.errors import ResolutionError from posthog.models.property_definition import PropertyType +from posthog.models.sessions.sql import ( + SELECT_SESSION_PROP_STRING_VALUES_SQL_WITH_FILTER, + SELECT_SESSION_PROP_STRING_VALUES_SQL, +) +from posthog.queries.insight import insight_sync_execute if TYPE_CHECKING: - pass + from posthog.models.team import Team RAW_SESSIONS_FIELDS: dict[str, FieldOrTable] = { "id": StringDatabaseField(name="session_id"), @@ -249,3 +254,61 @@ def get_lazy_session_table_properties(search: Optional[str]): if (not search or search.lower() in field_name.lower()) and field_name not in hidden_fields ] return results + + +SESSION_PROPERTY_TO_RAW_SESSIONS_EXPR_MAP = { + "$initial_referring_domain": "finalizeAggregation(initial_referring_domain)", + "$initial_utm_source": "finalizeAggregation(initial_utm_source)", + "$initial_utm_campaign": "finalizeAggregation(initial_utm_campaign)", + "$initial_utm_medium": "finalizeAggregation(initial_utm_medium)", + "$initial_utm_term": "finalizeAggregation(initial_utm_term)", + "$initial_utm_content": "finalizeAggregation(initial_utm_content)", + "$initial_gclid": "finalizeAggregation(initial_gclid)", + "$initial_gad_source": "finalizeAggregation(initial_gad_source)", + "$initial_gclsrc": "finalizeAggregation(initial_gclsrc)", + "$initial_dclid": "finalizeAggregation(initial_dclid)", + "$initial_gbraid": "finalizeAggregation(initial_gbraid)", + "$initial_wbraid": "finalizeAggregation(initial_wbraid)", + "$initial_fbclid": "finalizeAggregation(initial_fbclid)", + "$initial_msclkid": "finalizeAggregation(initial_msclkid)", + "$initial_twclid": "finalizeAggregation(initial_twclid)", + "$initial_li_fat_id": "finalizeAggregation(initial_li_fat_id)", + "$initial_mc_cid": "finalizeAggregation(initial_mc_cid)", + "$initial_igshid": "finalizeAggregation(initial_igshid)", + "$initial_ttclid": "finalizeAggregation(initial_ttclid)", + "$entry_url": "finalizeAggregation(entry_url)", + "$exit_url": "finalizeAggregation(exit_url)", +} + + +def get_lazy_session_table_values(key: str, search_term: Optional[str], team: "Team"): + # the sessions table does not have a properties json object like the events and person tables + + if key == "$channel_type": + return [[name] for name in POSSIBLE_CHANNEL_TYPES if not search_term or search_term.lower() in name.lower()] + + expr = SESSION_PROPERTY_TO_RAW_SESSIONS_EXPR_MAP.get(key) + + if not expr: + return [] + + field_definition = LAZY_SESSIONS_FIELDS.get(key) + if not field_definition: + return [] + + if isinstance(field_definition, StringDatabaseField): + if search_term: + return insight_sync_execute( + SELECT_SESSION_PROP_STRING_VALUES_SQL_WITH_FILTER.format(property_expr=expr), + {"team_id": team.pk, "key": key, "value": "%{}%".format(search_term)}, + query_type="get_session_property_values_with_value", + team_id=team.pk, + ) + return insight_sync_execute( + SELECT_SESSION_PROP_STRING_VALUES_SQL.format(property_expr=expr), + {"team_id": team.pk, "key": key}, + query_type="get_session_property_values", + team_id=team.pk, + ) + + return [] diff --git a/posthog/models/sessions/sql.py b/posthog/models/sessions/sql.py index a68ec2ff97e1e..22d3431099f94 100644 --- a/posthog/models/sessions/sql.py +++ b/posthog/models/sessions/sql.py @@ -261,19 +261,19 @@ def source_column(column_name: str) -> str: """ ) -SELECT_SESSION_PROP_VALUES_SQL = """ +SELECT_SESSION_PROP_STRING_VALUES_SQL = """ SELECT value, count(value) FROM ( SELECT - {property_field} as value + {property_expr} as value FROM sessions WHERE team_id = %(team_id)s AND - {property_field} IS NOT NULL AND - {property_field} != '' + {property_expr} IS NOT NULL AND + {property_expr} != '' ORDER BY session_id DESC LIMIT 100000 ) @@ -282,18 +282,18 @@ def source_column(column_name: str) -> str: LIMIT 20 """ -SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER = """ +SELECT_SESSION_PROP_STRING_VALUES_SQL_WITH_FILTER = """ SELECT value, count(value) FROM ( SELECT - {property_field} as value + {property_expr} as value FROM sessions WHERE team_id = %(team_id)s AND - {property_field} ILIKE %(value)s + {property_expr} ILIKE %(value)s ORDER BY session_id DESC LIMIT 100000 ) @@ -301,26 +301,3 @@ def source_column(column_name: str) -> str: ORDER BY count(value) DESC LIMIT 20 """ - - -SESSION_PROPERTY_TO_COLUMN_MAP = { - "$initial_referring_domain": "initial_referring_domain", - "$initial_utm_source": "initial_utm_source", - "$initial_utm_campaign": "initial_utm_campaign", - "$initial_utm_medium": "initial_utm_medium", - "$initial_utm_term": "initial_utm_term", - "$initial_utm_content": "initial_utm_content", - "$initial_gclid": "initial_gclid", - "$initial_gad_source": "initial_gad_source", - "$initial_gclsrc": "initial_gclsrc", - "$initial_dclid": "initial_dclid", - "$initial_gbraid": "initial_gbraid", - "$initial_wbraid": "initial_wbraid", - "$initial_fbclid": "initial_fbclid", - "$initial_msclkid": "initial_msclkid", - "$initial_twclid": "initial_twclid", - "$initial_li_fat_id": "initial_li_fat_id", - "$initial_mc_cid": "initial_mc_cid", - "$initial_igshid": "initial_igshid", - "$initial_ttclid": "initial_ttclid", -} diff --git a/posthog/queries/property_values.py b/posthog/queries/property_values.py index fe77dded0ac42..0e79d15146af6 100644 --- a/posthog/queries/property_values.py +++ b/posthog/queries/property_values.py @@ -2,18 +2,12 @@ from django.utils import timezone -from posthog.hogql.database.schema.channel_type import POSSIBLE_CHANNEL_TYPES from posthog.models.event.sql import SELECT_PROP_VALUES_SQL_WITH_FILTER from posthog.models.person.sql import ( SELECT_PERSON_PROP_VALUES_SQL, SELECT_PERSON_PROP_VALUES_SQL_WITH_FILTER, ) from posthog.models.property.util import get_property_string_expr -from posthog.models.sessions.sql import ( - SESSION_PROPERTY_TO_COLUMN_MAP, - SELECT_SESSION_PROP_VALUES_SQL, - SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER, -) from posthog.models.team import Team from posthog.queries.insight import insight_sync_execute from posthog.utils import relative_date_parse @@ -85,29 +79,3 @@ def get_person_property_values_for_key(key: str, team: Team, value: Optional[str query_type="get_person_property_values", team_id=team.pk, ) - - -def get_session_column_values_for_key(key: str, team: Team, search_term: Optional[str] = None): - # the sessions table does not have a properties json object like the events and person tables - - if key == "$channel_type": - return [[name] for name in POSSIBLE_CHANNEL_TYPES if not search_term or search_term.lower() in name.lower()] - - column = SESSION_PROPERTY_TO_COLUMN_MAP.get(key) - - if not column: - return [] - - if search_term: - return insight_sync_execute( - SELECT_SESSION_PROP_VALUES_SQL_WITH_FILTER.format(property_field=column), - {"team_id": team.pk, "key": key, "value": "%{}%".format(search_term)}, - query_type="get_session_property_values_with_value", - team_id=team.pk, - ) - return insight_sync_execute( - SELECT_SESSION_PROP_VALUES_SQL.format(property_field=column), - {"team_id": team.pk, "key": key}, - query_type="get_session_property_values", - team_id=team.pk, - ) From 7ea52bc681cdcbb03c62d59cc63dc6c5fffb8f19 Mon Sep 17 00:00:00 2001 From: Robbie Date: Tue, 16 Apr 2024 17:20:08 +0100 Subject: [PATCH 10/32] Rename duration back to $session_duration --- frontend/src/lib/taxonomy.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/taxonomy.tsx b/frontend/src/lib/taxonomy.tsx index 643dad1ac6202..d52ee74c583c0 100644 --- a/frontend/src/lib/taxonomy.tsx +++ b/frontend/src/lib/taxonomy.tsx @@ -994,7 +994,7 @@ export const CORE_FILTER_DEFINITIONS_BY_GROUP = { numerical_event_properties: {}, // Same as event properties, see assignment below person_properties: {}, // Currently person properties are the same as event properties, see assignment below session_properties: { - duration: { + $session_duration: { label: 'Session duration', description: ( @@ -1022,7 +1022,7 @@ export const CORE_FILTER_DEFINITIONS_BY_GROUP = { examples: ['https://example.com/interesting-article?parameter=true'], }, $exit_url: { - label: 'Entry URL', + label: 'Exit URL', description: The last URL visited in this session, examples: ['https://example.com/interesting-article?parameter=true'], }, @@ -1086,14 +1086,14 @@ for (const [key, value] of Object.entries(CORE_FILTER_DEFINITIONS_BY_GROUP.event 'description' in value ? `${value.description} Data from the first event in this session.` : 'Data from the first event in this session.', - examples: value.examples, + examples: 'examples' in value ? value.examples : undefined, } } } // We treat `$session_duration` as an event property in the context of series `math`, but it's fake in a sense CORE_FILTER_DEFINITIONS_BY_GROUP.event_properties.$session_duration = - CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties.duration + CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties.$session_duration export const PROPERTY_KEYS = Object.keys(CORE_FILTER_DEFINITIONS_BY_GROUP.event_properties) From e666f6f0955a0410e43cfc6d0fd02cfc5e1742c7 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 17:55:37 +0100 Subject: [PATCH 11/32] Fix "keeps infiniteListCounts in sync" --- .../taxonomicFilterLogic.test.ts | 19 ++++++++++++++++--- frontend/src/test/mocks.ts | 9 +++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts index 2c6f0ff84c2db..12ed88ab4889c 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts @@ -7,7 +7,7 @@ import { useMocks } from '~/mocks/jest' import { actionsModel } from '~/models/actionsModel' import { groupsModel } from '~/models/groupsModel' import { initKeaTests } from '~/test/init' -import { mockEventDefinitions } from '~/test/mocks' +import { mockEventDefinitions, mockSessionPropertyDefinitions } from '~/test/mocks' import { AppContext } from '~/types' import { infiniteListLogic } from './infiniteListLogic' @@ -33,6 +33,19 @@ describe('taxonomicFilterLogic', () => { }, ] }, + '/api/projects/:team/sessions/property_definitions': (res) => { + const search = res.url.searchParams.get('search') + const results = search + ? mockSessionPropertyDefinitions.filter((e) => e.name.includes(search)) + : mockSessionPropertyDefinitions + return [ + 200, + { + results, + count: results.length, + }, + ] + }, }, }) initKeaTests() @@ -76,7 +89,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 1, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.Sessions]: 0, }, }) .toDispatchActions(['infiniteListResultsReceived']) @@ -87,7 +100,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, // not mocked [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.Sessions]: 2, }, }) }) diff --git a/frontend/src/test/mocks.ts b/frontend/src/test/mocks.ts index dcd926d4f1e7a..4a2f75776909f 100644 --- a/frontend/src/test/mocks.ts +++ b/frontend/src/test/mocks.ts @@ -89,6 +89,15 @@ export const mockEventPropertyDefinitions: PropertyDefinition[] = [ is_seen_on_filtered_events: (name || '').includes('$'), })) +export const mockSessionPropertyDefinitions: PropertyDefinition[] = ['$session_duration', '$initial_utm_source'].map( + (name) => ({ + ...mockEventPropertyDefinition, + id: name, + name: name, + description: `${name} is the best!`, + }) +) + export const mockPersonProperty = { name: '$browser_version', count: 1, From eba45b6280e12f81fe212b4d47fcb5679cec1094 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 20:44:06 +0100 Subject: [PATCH 12/32] Fix "setting search query filters events" --- .../TaxonomicFilter/taxonomicFilterLogic.test.ts | 14 +++++++++----- frontend/src/test/mocks.ts | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts index 12ed88ab4889c..f00a600841419 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts @@ -110,17 +110,21 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic).toDispatchActionsInAnyOrder([ 'infiniteListResultsReceived', 'infiniteListResultsReceived', + 'infiniteListResultsReceived', + 'infiniteListResultsReceived', + 'infiniteListResultsReceived', + 'infiniteListResultsReceived', ]) await expectLogic(logic, () => { - logic.actions.setSearchQuery('event') + logic.actions.setSearchQuery('search term') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) .toMatchValues({ - searchQuery: 'event', + searchQuery: 'search term', activeTab: TaxonomicFilterGroupType.Events, infiniteListCounts: { - [TaxonomicFilterGroupType.Events]: 4, + [TaxonomicFilterGroupType.Events]: 1, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 0, [TaxonomicFilterGroupType.Sessions]: 0, @@ -174,7 +178,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.Sessions]: 2, }, }) diff --git a/frontend/src/test/mocks.ts b/frontend/src/test/mocks.ts index 4a2f75776909f..78cba1619bfb8 100644 --- a/frontend/src/test/mocks.ts +++ b/frontend/src/test/mocks.ts @@ -51,7 +51,7 @@ export const mockEventDefinitions: EventDefinition[] = [ 'test event', '$click', '$autocapture', - 'search', + 'search term', 'other event', ...Array(150), ].map((name, index) => ({ From 50afd651a07bf51ddeb1c42f01c7546f5dd9912d Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 21:13:48 +0100 Subject: [PATCH 13/32] Fix taxonomy tests --- frontend/src/lib/taxonomy.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/taxonomy.test.tsx b/frontend/src/lib/taxonomy.test.tsx index 3805f9d7670ad..e8820fbad0c32 100644 --- a/frontend/src/lib/taxonomy.test.tsx +++ b/frontend/src/lib/taxonomy.test.tsx @@ -26,10 +26,10 @@ describe('taxonomy', () => { }) describe('session properties', () => { - const sessionPropertyNames = Object.keys(CORE_FILTER_DEFINITIONS_BY_GROUP.sessions) + const sessionPropertyNames = Object.keys(CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties) it('should have an $initial_referring_domain property', () => { const property: CoreFilterDefinition = - CORE_FILTER_DEFINITIONS_BY_GROUP.sessions['$initial_referring_domain'] + CORE_FILTER_DEFINITIONS_BY_GROUP.session_properties['$initial_referring_domain'] expect(property.label).toEqual('Initial Referring Domain') }) it(`should have every property in SESSION_PROPERTIES_ADAPTED_FROM_PERSON`, () => { From 6ceb302f61eba522cc5541685fb3d7d5ac459a70 Mon Sep 17 00:00:00 2001 From: Robbie Date: Tue, 16 Apr 2024 21:14:26 +0100 Subject: [PATCH 14/32] Change session api to GenericViewSet --- posthog/api/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/api/session.py b/posthog/api/session.py index 7683187f74d2a..5a6778c3fcd0a 100644 --- a/posthog/api/session.py +++ b/posthog/api/session.py @@ -17,7 +17,7 @@ class SessionViewSet( TeamAndOrgViewSetMixin, - viewsets.ViewSet, + viewsets.GenericViewSet, ): scope_object = "query" renderer_classes = tuple(api_settings.DEFAULT_RENDERER_CLASSES) + (csvrenderers.PaginatedCSVRenderer,) From 200b09ab55291e94dab15071f15658799fb167d8 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 21:15:52 +0100 Subject: [PATCH 15/32] Add local properties back --- frontend/src/models/propertyDefinitionsModel.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/frontend/src/models/propertyDefinitionsModel.ts b/frontend/src/models/propertyDefinitionsModel.ts index 4930b6c2a1796..0bc2dfae5c11b 100644 --- a/frontend/src/models/propertyDefinitionsModel.ts +++ b/frontend/src/models/propertyDefinitionsModel.ts @@ -21,6 +21,19 @@ import type { propertyDefinitionsModelType } from './propertyDefinitionsModelTyp export type PropertyDefinitionStorage = Record +// List of property definitions that are calculated on the backend. These +// are valid properties that do not exist on events. +const localProperties: PropertyDefinitionStorage = { + 'event/$session_duration': { + id: '$session_duration', + name: '$session_duration', + description: 'Duration of the session', + is_numerical: true, + is_seen_on_filtered_events: false, + property_type: PropertyType.Duration, + }, +} + export type FormatPropertyValueForDisplayFunction = ( propertyName?: BreakdownKeyType, valueToFormat?: PropertyFilterValue, @@ -144,7 +157,7 @@ export const propertyDefinitionsModel = kea([ }), reducers({ propertyDefinitionStorage: [ - {} as PropertyDefinitionStorage, + { ...localProperties } as PropertyDefinitionStorage, { updatePropertyDefinitions: (state, { propertyDefinitions }) => { return { From 7f77e5f9346c9e1fde76043ca4d5050ec6b8b991 Mon Sep 17 00:00:00 2001 From: Robbie Date: Tue, 16 Apr 2024 21:25:37 +0100 Subject: [PATCH 16/32] Support datetime and boolean session properties --- posthog/hogql/database/schema/sessions.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/posthog/hogql/database/schema/sessions.py b/posthog/hogql/database/schema/sessions.py index 61730e98a7bda..8675d2167c697 100644 --- a/posthog/hogql/database/schema/sessions.py +++ b/posthog/hogql/database/schema/sessions.py @@ -12,6 +12,7 @@ DatabaseField, LazyTable, FloatDatabaseField, + BooleanDatabaseField, ) from posthog.hogql.database.schema.channel_type import create_channel_type_expr, POSSIBLE_CHANNEL_TYPES from posthog.hogql.database.schema.util.session_where_clause_extractor import SessionMinTimestampWhereClauseExtractor @@ -235,18 +236,27 @@ def get_lazy_session_table_properties(search: Optional[str]): # some fields should have a specific property type which isn't derivable from the type of database field property_type_overrides = { - "duration": PropertyType.Duration, + "$session_duration": PropertyType.Duration, } + def get_property_type(field_name: str, field_definition: FieldOrTable): + if field_name in property_type_overrides: + return property_type_overrides[field_name] + if isinstance(field_definition, IntegerDatabaseField) or isinstance(field_definition, FloatDatabaseField): + return PropertyType.Numeric + if isinstance(field_definition, DateTimeDatabaseField): + return PropertyType.Datetime + if isinstance(field_definition, BooleanDatabaseField): + return PropertyType.Boolean + return PropertyType.String + results = [ { "id": field_name, "name": field_name, "is_numerical": isinstance(field_definition, IntegerDatabaseField) or isinstance(field_definition, FloatDatabaseField), - "property_type": property_type_overrides.get(field_name, None) or PropertyType.Numeric - if isinstance(field_definition, IntegerDatabaseField) or isinstance(field_definition, FloatDatabaseField) - else PropertyType.String, + "property_type": get_property_type(field_name, field_definition), "is_seen_on_filtered_events": None, "tags": [], } From 68c1a887ec6e1cc3c4299e40027e8c65cc20713e Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Tue, 16 Apr 2024 22:10:23 +0100 Subject: [PATCH 17/32] Update mypy baseline --- mypy-baseline.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mypy-baseline.txt b/mypy-baseline.txt index 9b607b6222cd3..b1e56d812c2f0 100644 --- a/mypy-baseline.txt +++ b/mypy-baseline.txt @@ -66,11 +66,8 @@ posthog/hogql/database/schema/person_distinct_ids.py:0: error: Argument 1 to "se posthog/hogql/database/schema/person_distinct_id_overrides.py:0: error: Argument 1 to "select_from_person_distinct_id_overrides_table" has incompatible type "dict[str, list[str]]"; expected "dict[str, list[str | int]]" [arg-type] posthog/plugins/utils.py:0: error: Subclass of "str" and "bytes" cannot exist: would have incompatible method signatures [unreachable] posthog/plugins/utils.py:0: error: Statement is unreachable [unreachable] +posthog/clickhouse/kafka_engine.py:0: error: Argument 1 to "join" of "str" has incompatible type "list"; expected "Iterable[str]" [arg-type] posthog/models/filters/base_filter.py:0: error: "HogQLContext" has no attribute "person_on_events_mode" [attr-defined] -posthog/hogql/database/database.py:0: error: "FieldOrTable" has no attribute "fields" [attr-defined] -posthog/hogql/database/database.py:0: error: Incompatible types (expression has type "Literal['view', 'lazy_table']", TypedDict item "type" has type "Literal['integer', 'float', 'string', 'datetime', 'date', 'boolean', 'array', 'json', 'lazy_table', 'virtual_table', 'field_traverser', 'expression']") [typeddict-item] -posthog/warehouse/models/datawarehouse_saved_query.py:0: error: Argument 1 to "create_hogql_database" has incompatible type "int | None"; expected "int" [arg-type] -posthog/warehouse/models/datawarehouse_saved_query.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "SelectQuery | SelectUnionQuery") [assignment] posthog/models/user.py:0: error: Incompatible types in assignment (expression has type "None", base class "AbstractUser" defined the type as "CharField[str | int | Combinable, str]") [assignment] posthog/models/user.py:0: error: Incompatible types in assignment (expression has type "posthog.models.user.UserManager", base class "AbstractUser" defined the type as "django.contrib.auth.models.UserManager[AbstractUser]") [assignment] posthog/models/user.py:0: error: Cannot override writeable attribute with read-only property [override] @@ -82,6 +79,10 @@ posthog/models/user.py:0: note: bool posthog/models/user.py:0: error: "User" has no attribute "social_auth" [attr-defined] posthog/models/user.py:0: error: "User" has no attribute "social_auth" [attr-defined] posthog/models/person/person.py:0: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "ValuesQuerySet[PersonDistinctId, str]") [assignment] +posthog/hogql/database/database.py:0: error: "FieldOrTable" has no attribute "fields" [attr-defined] +posthog/hogql/database/database.py:0: error: Incompatible types (expression has type "Literal['view', 'lazy_table']", TypedDict item "type" has type "Literal['integer', 'float', 'string', 'datetime', 'date', 'boolean', 'array', 'json', 'lazy_table', 'virtual_table', 'field_traverser', 'expression']") [typeddict-item] +posthog/warehouse/models/datawarehouse_saved_query.py:0: error: Argument 1 to "create_hogql_database" has incompatible type "int | None"; expected "int" [arg-type] +posthog/warehouse/models/datawarehouse_saved_query.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "SelectQuery | SelectUnionQuery") [assignment] posthog/models/feature_flag/flag_matching.py:0: error: Statement is unreachable [unreachable] posthog/hogql_queries/utils/query_date_range.py:0: error: Incompatible return value type (got "str", expected "Literal['hour', 'day', 'week', 'month']") [return-value] posthog/hogql_queries/utils/query_date_range.py:0: error: Item "None" of "dict[str, int] | None" has no attribute "get" [union-attr] From 0db62488b5b70ab8933dead12d3415af96477808 Mon Sep 17 00:00:00 2001 From: Robbie Date: Thu, 18 Apr 2024 13:45:11 +0100 Subject: [PATCH 18/32] Hide duration as a property and asterisk field --- posthog/hogql/database/schema/sessions.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/posthog/hogql/database/schema/sessions.py b/posthog/hogql/database/schema/sessions.py index 8675d2167c697..5ff98b9c67b61 100644 --- a/posthog/hogql/database/schema/sessions.py +++ b/posthog/hogql/database/schema/sessions.py @@ -208,6 +208,11 @@ def to_printed_clickhouse(self, context): def to_printed_hogql(self): return "sessions" + def avoid_asterisk_fields(self) -> List[str]: + return [ + "duration", # alias of $session_duration, deprecated but included for backwards compatibility + ] + def join_events_table_to_sessions_table( from_table: str, to_table: str, requested_fields: dict[str, Any], context: HogQLContext, node: ast.SelectQuery @@ -232,7 +237,7 @@ def join_events_table_to_sessions_table( def get_lazy_session_table_properties(search: Optional[str]): # some fields shouldn't appear as properties - hidden_fields = {"team_id", "distinct_id", "session_id", "id", "$event_count_map", "$urls"} + hidden_fields = {"team_id", "distinct_id", "session_id", "id", "$event_count_map", "$urls", "duration"} # some fields should have a specific property type which isn't derivable from the type of database field property_type_overrides = { From dbda7311bd2f4d49ecd35cb7709f2b78d77d8d2e Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Thu, 18 Apr 2024 14:29:30 +0100 Subject: [PATCH 19/32] Add AsyncReturnType --- frontend/src/lib/utils.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/lib/utils.tsx b/frontend/src/lib/utils.tsx index ab32f34b314f3..20b8114375939 100644 --- a/frontend/src/lib/utils.tsx +++ b/frontend/src/lib/utils.tsx @@ -1597,6 +1597,8 @@ export function promiseResolveReject(): { return { resolve: resolve!, reject: reject!, promise } } +export type AsyncReturnType any> = T extends (...args: any) => Promise ? R : any + export function calculateDays(timeValue: number, timeUnit: TimeUnitType): number { if (timeUnit === TimeUnitType.Year) { return timeValue * 365 From cb2fb3b020d6dd3582850574d1cf946d65e91c0a Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Thu, 18 Apr 2024 14:30:45 +0100 Subject: [PATCH 20/32] Make taxonoicFilterLogic tests more reliable (maybe) --- .../taxonomicFilterLogic.test.ts | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts index f00a600841419..ac16be52e55be 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts @@ -92,7 +92,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Sessions]: 0, }, }) - .toDispatchActions(['infiniteListResultsReceived']) + .toDispatchActions(['infiniteListResultsReceived', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -107,19 +107,15 @@ describe('taxonomicFilterLogic', () => { it('setting search query filters events', async () => { // load the initial results - await expectLogic(logic).toDispatchActionsInAnyOrder([ - 'infiniteListResultsReceived', - 'infiniteListResultsReceived', - 'infiniteListResultsReceived', - 'infiniteListResultsReceived', - 'infiniteListResultsReceived', - 'infiniteListResultsReceived', - ]) + await expectLogic(logic) + .toDispatchActions(['infiniteListResultsReceived', 'infiniteListResultsReceived']) + .delay(1) await expectLogic(logic, () => { logic.actions.setSearchQuery('search term') }) .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) + .delay(1) .toMatchValues({ searchQuery: 'search term', activeTab: TaxonomicFilterGroupType.Events, @@ -134,7 +130,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('selector') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -151,7 +147,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('this is not found') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -168,7 +164,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -208,7 +204,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('event') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ From 870334a5aa29d7eb8f209148cfea27accd725538 Mon Sep 17 00:00:00 2001 From: Robbie Date: Thu, 18 Apr 2024 14:57:49 +0100 Subject: [PATCH 21/32] Fix duplicates from bad rebase --- .../database/schema/util/session_where_clause_extractor.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/posthog/hogql/database/schema/util/session_where_clause_extractor.py b/posthog/hogql/database/schema/util/session_where_clause_extractor.py index 139b2724d0d6f..3d94a4a0f691f 100644 --- a/posthog/hogql/database/schema/util/session_where_clause_extractor.py +++ b/posthog/hogql/database/schema/util/session_where_clause_extractor.py @@ -297,9 +297,6 @@ def visit_placeholder(self, node: ast.Placeholder) -> bool: def visit_alias(self, node: ast.Alias) -> bool: return self.visit(node.expr) - def visit_select_query(self, node: ast.SelectQuery) -> bool: - return False - def is_simple_timestamp_field_expression(expr: ast.Expr, context: HogQLContext) -> bool: return IsSimpleTimestampFieldExpressionVisitor(context).visit(expr) @@ -396,9 +393,6 @@ def visit_alias(self, node: ast.Alias) -> bool: return self.visit(node.expr) - def visit_select_query(self, node: ast.SelectQuery) -> bool: - return False - def rewrite_timestamp_field(expr: ast.Expr, context: HogQLContext) -> ast.Expr: return RewriteTimestampFieldVisitor(context).visit(expr) From 5658a57e3cfe1d278067a630139e0d94dc00672b Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Thu, 18 Apr 2024 15:24:19 +0100 Subject: [PATCH 22/32] Only show session table session properties if feature flag enabled --- .../TaxonomicFilter/taxonomicFilterLogic.tsx | 17 ++++++++++++++--- frontend/src/lib/constants.tsx | 1 + .../web-analytics/WebPropertyFilters.tsx | 19 ++++++++++++++----- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index 914718ba5474d..ce3845b4c3eae 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -11,7 +11,9 @@ import { TaxonomicFilterLogicProps, TaxonomicFilterValue, } from 'lib/components/TaxonomicFilter/types' +import { FEATURE_FLAGS } from 'lib/constants' import { IconCohort } from 'lib/lemon-ui/icons' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { CORE_FILTER_DEFINITIONS_BY_GROUP } from 'lib/taxonomy' import { capitalizeFirstLetter, pluralize, toParams } from 'lib/utils' import { getEventDefinitionIcon, getPropertyDefinitionIcon } from 'scenes/data-management/events/DefinitionHeader' @@ -168,6 +170,7 @@ export const taxonomicFilterLogic = kea([ s.metadataSource, s.excludedProperties, s.propertyAllowList, + featureFlagLogic.selectors.featureFlags, ], ( teamId, @@ -177,7 +180,8 @@ export const taxonomicFilterLogic = kea([ schemaColumns, metadataSource, excludedProperties, - propertyAllowList + propertyAllowList, + featureFlags ): TaxonomicFilterGroup[] => { const groups: TaxonomicFilterGroup[] = [ { @@ -489,11 +493,18 @@ export const taxonomicFilterLogic = kea([ name: 'Session Properties', searchPlaceholder: 'sessions', type: TaxonomicFilterGroupType.Sessions, - value: 'sessions', + options: [ + { + name: 'Session duration', + value: '$session_duration', + }, + ], getName: (option: any) => option.name, getValue: (option) => option.name, getPopoverHeader: () => 'Session', - endpoint: `api/projects/${teamId}/sessions/property_definitions`, + endpoint: featureFlags[FEATURE_FLAGS.SESSION_TABLE_PROPERTY_FILTERS] + ? `api/projects/${teamId}/sessions/property_definitions` + : undefined, getIcon: getPropertyDefinitionIcon, }, { diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 0db68836f76d5..86450c67f19e0 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -212,6 +212,7 @@ export const FEATURE_FLAGS = { EMAIL_VERIFICATION_TICKET_SUBMISSION: 'email-verification-ticket-submission', // owner: #team-growth TOOLBAR_HEATMAPS: 'toolbar-heatmaps', // owner: #team-replay THEME: 'theme', // owner: @aprilfools + SESSION_TABLE_PROPERTY_FILTERS: 'session-table-property-filters', // owner: @robbie-c } as const export type FeatureFlagKey = (typeof FEATURE_FLAGS)[keyof typeof FEATURE_FLAGS] diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index 6f94d481d687e..b055cf59d5fa2 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -1,6 +1,9 @@ +import { useValues } from 'kea' import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters' import { isEventPersonOrSessionPropertyFilter } from 'lib/components/PropertyFilters/utils' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' +import { FEATURE_FLAGS } from 'lib/constants' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { WebAnalyticsPropertyFilters } from '~/queries/schema' @@ -11,13 +14,19 @@ export const WebPropertyFilters = ({ webAnalyticsFilters: WebAnalyticsPropertyFilters setWebAnalyticsFilters: (filters: WebAnalyticsPropertyFilters) => void }): JSX.Element => { + const { featureFlags } = useValues(featureFlagLogic) + return ( setWebAnalyticsFilters(filters.filter(isEventPersonOrSessionPropertyFilter))} propertyFilters={webAnalyticsFilters} pageKey="web-analytics" From 888a1da0cdc56ad92d3a6a9c6e6542738376da9f Mon Sep 17 00:00:00 2001 From: Robbie Date: Thu, 18 Apr 2024 15:29:46 +0100 Subject: [PATCH 23/32] Improve error message for non-hogql non-duration session property --- posthog/models/property/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/models/property/util.py b/posthog/models/property/util.py index b1ce9b6087aaa..de2602539e6ec 100644 --- a/posthog/models/property/util.py +++ b/posthog/models/property/util.py @@ -930,7 +930,7 @@ def get_session_property_filter_statement(prop: Property, idx: int, prepend: str ) else: - raise exceptions.ValidationError(f"Property '{prop.key}' is not allowed in session property filters.") + raise exceptions.ValidationError(f"Session property '{prop.key}' is only valid in HogQL queries.") def clear_excess_levels(prop: Union["PropertyGroup", "Property"], skip=False): From 085e150462c54251794aef967e4d3eb028530f53 Mon Sep 17 00:00:00 2001 From: Robbie Date: Thu, 18 Apr 2024 16:15:22 +0100 Subject: [PATCH 24/32] Put session table properties behind FF --- .../DefinitionPopoverContents.tsx | 45 +++++++++++-------- .../definitionPopoverLogic.ts | 5 +++ .../lib/components/DefinitionPopover/utils.ts | 1 + .../TaxonomicFilter/taxonomicFilterLogic.tsx | 16 ++++--- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx b/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx index d6419882b3327..f578e4d37b07f 100644 --- a/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx +++ b/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx @@ -78,6 +78,7 @@ function DefinitionView({ group }: { group: TaxonomicFilterGroup }): JSX.Element isCohort, isDataWarehouse, isProperty, + hasSentAs, } = useValues(definitionPopoverLogic) const { setLocalDefinition } = useActions(definitionPopoverLogic) @@ -142,13 +143,17 @@ function DefinitionView({ group }: { group: TaxonomicFilterGroup }): JSX.Element /> - - - {_definition.name}} - /> - + {hasSentAs ? ( + <> + + + {_definition.name}} + /> + + + ) : null} ) } @@ -176,17 +181,21 @@ function DefinitionView({ group }: { group: TaxonomicFilterGroup }): JSX.Element - - - - {_definition.name !== '' ? _definition.name : (empty string)} - - } - /> - + {hasSentAs ? ( + <> + + + + {_definition.name !== '' ? _definition.name : (empty string)} + + } + /> + + + ) : null} ) } diff --git a/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts b/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts index c3baac7ce76ca..fe4cafcfee27a 100644 --- a/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts +++ b/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts @@ -176,11 +176,16 @@ export const definitionPopoverLogic = kea([ [ TaxonomicFilterGroupType.PersonProperties, TaxonomicFilterGroupType.EventProperties, + TaxonomicFilterGroupType.Sessions, TaxonomicFilterGroupType.EventFeatureFlags, TaxonomicFilterGroupType.NumericalEventProperties, TaxonomicFilterGroupType.Metadata, ].includes(type) || type.startsWith(TaxonomicFilterGroupType.GroupsPrefix), ], + hasSentAs: [ + (s) => [s.type, s.isProperty, s.isEvent], + (type, isProperty, isEvent) => isEvent || (isProperty && type !== TaxonomicFilterGroupType.Sessions), + ], isCohort: [(s) => [s.type], (type) => type === TaxonomicFilterGroupType.Cohorts], isDataWarehouse: [(s) => [s.type], (type) => type === TaxonomicFilterGroupType.DataWarehouse], viewFullDetailUrl: [ diff --git a/frontend/src/lib/components/DefinitionPopover/utils.ts b/frontend/src/lib/components/DefinitionPopover/utils.ts index f828aea981fc2..842ed5e1fa701 100644 --- a/frontend/src/lib/components/DefinitionPopover/utils.ts +++ b/frontend/src/lib/components/DefinitionPopover/utils.ts @@ -48,6 +48,7 @@ export function getSingularType(type: TaxonomicFilterGroupType): string { case TaxonomicFilterGroupType.EventProperties: case TaxonomicFilterGroupType.PersonProperties: case TaxonomicFilterGroupType.GroupsPrefix: // Group properties + case TaxonomicFilterGroupType.Sessions: return 'property' case TaxonomicFilterGroupType.EventFeatureFlags: return 'feature' diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index ce3845b4c3eae..6306b4ceb9644 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -493,12 +493,16 @@ export const taxonomicFilterLogic = kea([ name: 'Session Properties', searchPlaceholder: 'sessions', type: TaxonomicFilterGroupType.Sessions, - options: [ - { - name: 'Session duration', - value: '$session_duration', - }, - ], + options: featureFlags[FEATURE_FLAGS.SESSION_TABLE_PROPERTY_FILTERS] + ? undefined + : [ + { + id: '$session_duration', + name: '$session_duration', + property_type: 'Duration', + is_numerical: true, + }, + ], getName: (option: any) => option.name, getValue: (option) => option.name, getPopoverHeader: () => 'Session', From c7c3a9ab555f44e33103ae9e97550402087cbf34 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Thu, 18 Apr 2024 16:35:36 +0100 Subject: [PATCH 25/32] Revert taxonomicFilterLogic tests --- .../taxonomicFilterLogic.test.ts | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts index ac16be52e55be..2c6f0ff84c2db 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts @@ -7,7 +7,7 @@ import { useMocks } from '~/mocks/jest' import { actionsModel } from '~/models/actionsModel' import { groupsModel } from '~/models/groupsModel' import { initKeaTests } from '~/test/init' -import { mockEventDefinitions, mockSessionPropertyDefinitions } from '~/test/mocks' +import { mockEventDefinitions } from '~/test/mocks' import { AppContext } from '~/types' import { infiniteListLogic } from './infiniteListLogic' @@ -33,19 +33,6 @@ describe('taxonomicFilterLogic', () => { }, ] }, - '/api/projects/:team/sessions/property_definitions': (res) => { - const search = res.url.searchParams.get('search') - const results = search - ? mockSessionPropertyDefinitions.filter((e) => e.name.includes(search)) - : mockSessionPropertyDefinitions - return [ - 200, - { - results, - count: results.length, - }, - ] - }, }, }) initKeaTests() @@ -89,10 +76,10 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 1, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 0, + [TaxonomicFilterGroupType.Sessions]: 1, }, }) - .toDispatchActions(['infiniteListResultsReceived', 'infiniteListResultsReceived']) + .toDispatchActions(['infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -100,27 +87,27 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, // not mocked [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 2, + [TaxonomicFilterGroupType.Sessions]: 1, }, }) }) it('setting search query filters events', async () => { // load the initial results - await expectLogic(logic) - .toDispatchActions(['infiniteListResultsReceived', 'infiniteListResultsReceived']) - .delay(1) + await expectLogic(logic).toDispatchActionsInAnyOrder([ + 'infiniteListResultsReceived', + 'infiniteListResultsReceived', + ]) await expectLogic(logic, () => { - logic.actions.setSearchQuery('search term') + logic.actions.setSearchQuery('event') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) - .delay(1) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) .toMatchValues({ - searchQuery: 'search term', + searchQuery: 'event', activeTab: TaxonomicFilterGroupType.Events, infiniteListCounts: { - [TaxonomicFilterGroupType.Events]: 1, + [TaxonomicFilterGroupType.Events]: 4, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 0, [TaxonomicFilterGroupType.Sessions]: 0, @@ -130,7 +117,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('selector') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -147,7 +134,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('this is not found') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -164,7 +151,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ @@ -174,7 +161,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 2, + [TaxonomicFilterGroupType.Sessions]: 1, }, }) @@ -204,7 +191,7 @@ describe('taxonomicFilterLogic', () => { await expectLogic(logic, () => { logic.actions.setSearchQuery('event') }) - .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived', 'infiniteListResultsReceived']) + .toDispatchActions(['setSearchQuery', 'infiniteListResultsReceived']) .delay(1) .clearHistory() .toMatchValues({ From f5ef830f46cecd9367df7a5e10fd402a73149fd5 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:53:43 +0000 Subject: [PATCH 26/32] Update query snapshots --- posthog/api/test/__snapshots__/test_api_docs.ambr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr index b4a5bb2780673..5e807603e82de 100644 --- a/posthog/api/test/__snapshots__/test_api_docs.ambr +++ b/posthog/api/test/__snapshots__/test_api_docs.ambr @@ -84,6 +84,8 @@ '/home/runner/work/posthog/posthog/posthog/session_recordings/session_recording_api.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer]: could not resolve field on model with path "viewed". This is likely a custom field that does some unknown magic. Maybe consider annotating the field/property? Defaulting to "string". (Exception: SessionRecording has no field named \'viewed\')', '/home/runner/work/posthog/posthog/posthog/api/person.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer > MinimalPersonSerializer]: unable to resolve type hint for function "get_distinct_ids". Consider using a type hint or @extend_schema_field. Defaulting to string.', '/home/runner/work/posthog/posthog/posthog/session_recordings/session_recording_api.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer]: unable to resolve type hint for function "storage". Consider using a type hint or @extend_schema_field. Defaulting to string.', + "/home/runner/work/posthog/posthog/posthog/api/session.py: Error [SessionViewSet]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'SessionViewSet' should either include a `serializer_class` attribute, or override the `get_serializer_class()` method.)", + '/home/runner/work/posthog/posthog/posthog/api/session.py: Warning [SessionViewSet]: could not derive type of path parameter "project_id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. ) or annotating the parameter type with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/api/subscription.py: Warning [SubscriptionViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.subscription.Subscription" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/api/subscription.py: Warning [SubscriptionViewSet > SubscriptionSerializer]: unable to resolve type hint for function "summary". Consider using a type hint or @extend_schema_field. Defaulting to string.', '/home/runner/work/posthog/posthog/posthog/api/survey.py: Warning [SurveyViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.feedback.survey.Survey" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', From 350f046c240c3c9048ace5e95ba613d4ae59ea5d Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Thu, 18 Apr 2024 17:38:50 +0100 Subject: [PATCH 27/32] Rename TaxonomicFilterGroupType.Sessions to TaxonomicFilterGroupType.SessionProperties --- .../DefinitionPopover/DefinitionPopover.tsx | 2 +- .../definitionPopoverLogic.ts | 5 +++-- .../lib/components/DefinitionPopover/utils.ts | 2 +- .../components/PropertyFilters/utils.test.ts | 4 ++-- .../lib/components/PropertyFilters/utils.ts | 4 ++-- .../TaxonomicFilter/InfiniteList.tsx | 4 ++-- .../taxonomicFilterLogic.test.ts | 22 +++++++++---------- .../TaxonomicFilter/taxonomicFilterLogic.tsx | 2 +- .../lib/components/TaxonomicFilter/types.ts | 2 +- .../nodes/InsightViz/GlobalAndOrFilters.tsx | 2 +- .../queries/nodes/InsightViz/TrendsSeries.tsx | 2 +- .../ActionFilterRow/ActionFilterRow.tsx | 2 +- .../TaxonomicBreakdownPopover.tsx | 2 +- .../web-analytics/WebPropertyFilters.tsx | 2 +- 14 files changed, 29 insertions(+), 28 deletions(-) diff --git a/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx b/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx index b9a692f3df411..807f1097e45c2 100644 --- a/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx +++ b/frontend/src/lib/components/DefinitionPopover/DefinitionPopover.tsx @@ -125,7 +125,7 @@ function Example({ value }: { value?: string }): JSX.Element { type === TaxonomicFilterGroupType.PersonProperties || type === TaxonomicFilterGroupType.GroupsPrefix || type === TaxonomicFilterGroupType.Metadata || - type === TaxonomicFilterGroupType.Sessions + type === TaxonomicFilterGroupType.SessionProperties ) { data = getCoreFilterDefinition(value, type) } else if (type === TaxonomicFilterGroupType.Elements) { diff --git a/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts b/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts index fe4cafcfee27a..38d9af8eb5311 100644 --- a/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts +++ b/frontend/src/lib/components/DefinitionPopover/definitionPopoverLogic.ts @@ -176,7 +176,7 @@ export const definitionPopoverLogic = kea([ [ TaxonomicFilterGroupType.PersonProperties, TaxonomicFilterGroupType.EventProperties, - TaxonomicFilterGroupType.Sessions, + TaxonomicFilterGroupType.SessionProperties, TaxonomicFilterGroupType.EventFeatureFlags, TaxonomicFilterGroupType.NumericalEventProperties, TaxonomicFilterGroupType.Metadata, @@ -184,7 +184,8 @@ export const definitionPopoverLogic = kea([ ], hasSentAs: [ (s) => [s.type, s.isProperty, s.isEvent], - (type, isProperty, isEvent) => isEvent || (isProperty && type !== TaxonomicFilterGroupType.Sessions), + (type, isProperty, isEvent) => + isEvent || (isProperty && type !== TaxonomicFilterGroupType.SessionProperties), ], isCohort: [(s) => [s.type], (type) => type === TaxonomicFilterGroupType.Cohorts], isDataWarehouse: [(s) => [s.type], (type) => type === TaxonomicFilterGroupType.DataWarehouse], diff --git a/frontend/src/lib/components/DefinitionPopover/utils.ts b/frontend/src/lib/components/DefinitionPopover/utils.ts index 842ed5e1fa701..a12c76237da42 100644 --- a/frontend/src/lib/components/DefinitionPopover/utils.ts +++ b/frontend/src/lib/components/DefinitionPopover/utils.ts @@ -48,7 +48,7 @@ export function getSingularType(type: TaxonomicFilterGroupType): string { case TaxonomicFilterGroupType.EventProperties: case TaxonomicFilterGroupType.PersonProperties: case TaxonomicFilterGroupType.GroupsPrefix: // Group properties - case TaxonomicFilterGroupType.Sessions: + case TaxonomicFilterGroupType.SessionProperties: return 'property' case TaxonomicFilterGroupType.EventFeatureFlags: return 'feature' diff --git a/frontend/src/lib/components/PropertyFilters/utils.test.ts b/frontend/src/lib/components/PropertyFilters/utils.test.ts index 33ad74f8e35d6..da3f32d7e1b3c 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.test.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.test.ts @@ -83,7 +83,7 @@ describe('propertyFilterTypeToTaxonomicFilterType()', () => { ...baseFilter, type: PropertyFilterType.Session, } as SessionPropertyFilter) - ).toEqual(TaxonomicFilterGroupType.Sessions) + ).toEqual(TaxonomicFilterGroupType.SessionProperties) expect(propertyFilterTypeToTaxonomicFilterType({ ...baseFilter, type: PropertyFilterType.HogQL })).toEqual( TaxonomicFilterGroupType.HogQLExpression ) @@ -122,7 +122,7 @@ describe('breakdownFilterToTaxonomicFilterType()', () => { TaxonomicFilterGroupType.EventProperties ) expect(breakdownFilterToTaxonomicFilterType({ ...baseFilter, breakdown_type: 'session' })).toEqual( - TaxonomicFilterGroupType.Sessions + TaxonomicFilterGroupType.SessionProperties ) expect(breakdownFilterToTaxonomicFilterType({ ...baseFilter, breakdown_type: 'hogql' })).toEqual( TaxonomicFilterGroupType.HogQLExpression diff --git a/frontend/src/lib/components/PropertyFilters/utils.ts b/frontend/src/lib/components/PropertyFilters/utils.ts index d6745a9d62b75..63833cbd7acb5 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.ts @@ -97,7 +97,7 @@ export const PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE: Omit< [PropertyFilterType.Feature]: TaxonomicFilterGroupType.EventFeatureFlags, [PropertyFilterType.Cohort]: TaxonomicFilterGroupType.Cohorts, [PropertyFilterType.Element]: TaxonomicFilterGroupType.Elements, - [PropertyFilterType.Session]: TaxonomicFilterGroupType.Sessions, + [PropertyFilterType.Session]: TaxonomicFilterGroupType.SessionProperties, [PropertyFilterType.HogQL]: TaxonomicFilterGroupType.HogQLExpression, [PropertyFilterType.Group]: TaxonomicFilterGroupType.GroupsPrefix, [PropertyFilterType.DataWarehouse]: TaxonomicFilterGroupType.DataWarehouse, @@ -268,7 +268,7 @@ const propertyFilterMapping: Partial
@@ -161,7 +161,7 @@ const selectedItemHasPopover = ( TaxonomicFilterGroupType.Cohorts, TaxonomicFilterGroupType.CohortsWithAllUsers, TaxonomicFilterGroupType.Metadata, - TaxonomicFilterGroupType.Sessions, + TaxonomicFilterGroupType.SessionProperties, ].includes(listGroupType) || listGroupType.startsWith(TaxonomicFilterGroupType.GroupsPrefix)) ) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts index 2c6f0ff84c2db..0dbbe75ddd38d 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.test.ts @@ -45,7 +45,7 @@ describe('taxonomicFilterLogic', () => { TaxonomicFilterGroupType.Events, TaxonomicFilterGroupType.Actions, TaxonomicFilterGroupType.Elements, - TaxonomicFilterGroupType.Sessions, + TaxonomicFilterGroupType.SessionProperties, ], } logic = taxonomicFilterLogic(logicProps) @@ -62,7 +62,7 @@ describe('taxonomicFilterLogic', () => { infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.Events }), infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.Actions }), infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.Elements }), - infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.Sessions }), + infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.SessionProperties }), ]) expect( infiniteListLogic({ ...logic.props, listGroupType: TaxonomicFilterGroupType.Cohorts }).isMounted() @@ -76,7 +76,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 1, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.SessionProperties]: 1, }, }) .toDispatchActions(['infiniteListResultsReceived']) @@ -87,7 +87,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, // not mocked [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.SessionProperties]: 1, }, }) }) @@ -110,7 +110,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 4, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 0, - [TaxonomicFilterGroupType.Sessions]: 0, + [TaxonomicFilterGroupType.SessionProperties]: 0, }, }) @@ -127,7 +127,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 0, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 1, - [TaxonomicFilterGroupType.Sessions]: 0, + [TaxonomicFilterGroupType.SessionProperties]: 0, }, }) @@ -144,7 +144,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 0, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 0, - [TaxonomicFilterGroupType.Sessions]: 0, + [TaxonomicFilterGroupType.SessionProperties]: 0, }, }) @@ -161,13 +161,13 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 157, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 4, - [TaxonomicFilterGroupType.Sessions]: 1, + [TaxonomicFilterGroupType.SessionProperties]: 1, }, }) // move right, skipping Actions await expectLogic(logic, () => logic.actions.tabRight()).toMatchValues({ - activeTab: TaxonomicFilterGroupType.Sessions, + activeTab: TaxonomicFilterGroupType.SessionProperties, }) await expectLogic(logic, () => logic.actions.tabRight()).toMatchValues({ activeTab: TaxonomicFilterGroupType.Events, @@ -181,7 +181,7 @@ describe('taxonomicFilterLogic', () => { activeTab: TaxonomicFilterGroupType.Events, }) await expectLogic(logic, () => logic.actions.tabLeft()).toMatchValues({ - activeTab: TaxonomicFilterGroupType.Sessions, + activeTab: TaxonomicFilterGroupType.SessionProperties, }) await expectLogic(logic, () => logic.actions.tabLeft()).toMatchValues({ activeTab: TaxonomicFilterGroupType.Elements, @@ -201,7 +201,7 @@ describe('taxonomicFilterLogic', () => { [TaxonomicFilterGroupType.Events]: 4, [TaxonomicFilterGroupType.Actions]: 0, [TaxonomicFilterGroupType.Elements]: 0, - [TaxonomicFilterGroupType.Sessions]: 0, + [TaxonomicFilterGroupType.SessionProperties]: 0, }, }) }) diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index 6306b4ceb9644..f35dbda2f7cf5 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -492,7 +492,7 @@ export const taxonomicFilterLogic = kea([ { name: 'Session Properties', searchPlaceholder: 'sessions', - type: TaxonomicFilterGroupType.Sessions, + type: TaxonomicFilterGroupType.SessionProperties, options: featureFlags[FEATURE_FLAGS.SESSION_TABLE_PROPERTY_FILTERS] ? undefined : [ diff --git a/frontend/src/lib/components/TaxonomicFilter/types.ts b/frontend/src/lib/components/TaxonomicFilter/types.ts index fa6b82a0d74d4..37f1c95b45daa 100644 --- a/frontend/src/lib/components/TaxonomicFilter/types.ts +++ b/frontend/src/lib/components/TaxonomicFilter/types.ts @@ -105,7 +105,7 @@ export enum TaxonomicFilterGroupType { Plugins = 'plugins', Dashboards = 'dashboards', GroupNamesPrefix = 'name_groups', - Sessions = 'session_properties', + SessionProperties = 'session_properties', HogQLExpression = 'hogql_expression', Notebooks = 'notebooks', } diff --git a/frontend/src/queries/nodes/InsightViz/GlobalAndOrFilters.tsx b/frontend/src/queries/nodes/InsightViz/GlobalAndOrFilters.tsx index 8485b9a71ee84..351e82136133a 100644 --- a/frontend/src/queries/nodes/InsightViz/GlobalAndOrFilters.tsx +++ b/frontend/src/queries/nodes/InsightViz/GlobalAndOrFilters.tsx @@ -27,7 +27,7 @@ export function GlobalAndOrFilters({ insightProps }: EditorFilterProps): JSX.Ele ...groupsTaxonomicTypes, TaxonomicFilterGroupType.Cohorts, TaxonomicFilterGroupType.Elements, - ...(isTrends ? [TaxonomicFilterGroupType.Sessions] : []), + ...(isTrends ? [TaxonomicFilterGroupType.SessionProperties] : []), TaxonomicFilterGroupType.HogQLExpression, ...(featureFlags[FEATURE_FLAGS.DATA_WAREHOUSE] && featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS] ? [TaxonomicFilterGroupType.DataWarehousePersonProperties] diff --git a/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx b/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx index 38b96c2162aca..dd75cd96a6f97 100644 --- a/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx +++ b/frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx @@ -34,7 +34,7 @@ export function TrendsSeries(): JSX.Element | null { ...groupsTaxonomicTypes, TaxonomicFilterGroupType.Cohorts, TaxonomicFilterGroupType.Elements, - ...(isTrends ? [TaxonomicFilterGroupType.Sessions] : []), + ...(isTrends ? [TaxonomicFilterGroupType.SessionProperties] : []), TaxonomicFilterGroupType.HogQLExpression, TaxonomicFilterGroupType.DataWarehouseProperties, ...(featureFlags[FEATURE_FLAGS.DATA_WAREHOUSE] && featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS] diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx index 0cb3eaeb086b3..f544e601e506d 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx @@ -390,7 +390,7 @@ export function ActionFilterRow({ groupTypes={[ TaxonomicFilterGroupType.DataWarehouseProperties, TaxonomicFilterGroupType.NumericalEventProperties, - TaxonomicFilterGroupType.Sessions, + TaxonomicFilterGroupType.SessionProperties, ]} schemaColumns={ filter.type == TaxonomicFilterGroupType.DataWarehouse && filter.name diff --git a/frontend/src/scenes/insights/filters/BreakdownFilter/TaxonomicBreakdownPopover.tsx b/frontend/src/scenes/insights/filters/BreakdownFilter/TaxonomicBreakdownPopover.tsx index abbcb7020c116..9a8df4a8c392f 100644 --- a/frontend/src/scenes/insights/filters/BreakdownFilter/TaxonomicBreakdownPopover.tsx +++ b/frontend/src/scenes/insights/filters/BreakdownFilter/TaxonomicBreakdownPopover.tsx @@ -28,7 +28,7 @@ export const TaxonomicBreakdownPopover = ({ open, setOpen, children }: Taxonomic TaxonomicFilterGroupType.EventFeatureFlags, ...groupsTaxonomicTypes, TaxonomicFilterGroupType.CohortsWithAllUsers, - ...(includeSessions ? [TaxonomicFilterGroupType.Sessions] : []), + ...(includeSessions ? [TaxonomicFilterGroupType.SessionProperties] : []), TaxonomicFilterGroupType.HogQLExpression, TaxonomicFilterGroupType.DataWarehouseProperties, ] diff --git a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx index b055cf59d5fa2..5237a78e3cb8b 100644 --- a/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx +++ b/frontend/src/scenes/web-analytics/WebPropertyFilters.tsx @@ -21,7 +21,7 @@ export const WebPropertyFilters = ({ taxonomicGroupTypes={ featureFlags[FEATURE_FLAGS.SESSION_TABLE_PROPERTY_FILTERS] ? [ - TaxonomicFilterGroupType.Sessions, + TaxonomicFilterGroupType.SessionProperties, TaxonomicFilterGroupType.EventProperties, TaxonomicFilterGroupType.PersonProperties, ] From 569b873b3cf3eb87e26489ebf59bd1fee6f4d1e2 Mon Sep 17 00:00:00 2001 From: Robbie Date: Wed, 24 Apr 2024 12:33:37 +0100 Subject: [PATCH 28/32] Use ViewSet instead of GenericViewSet --- posthog/api/session.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/posthog/api/session.py b/posthog/api/session.py index 5a6778c3fcd0a..b4c79600d1999 100644 --- a/posthog/api/session.py +++ b/posthog/api/session.py @@ -3,8 +3,6 @@ from rest_framework import request, response, viewsets from rest_framework.decorators import action from rest_framework.exceptions import ValidationError -from rest_framework.settings import api_settings -from rest_framework_csv import renderers as csvrenderers from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.hogql.database.schema.sessions import get_lazy_session_table_properties, get_lazy_session_table_values @@ -17,10 +15,9 @@ class SessionViewSet( TeamAndOrgViewSetMixin, - viewsets.GenericViewSet, + viewsets.ViewSet, ): scope_object = "query" - renderer_classes = tuple(api_settings.DEFAULT_RENDERER_CLASSES) + (csvrenderers.PaginatedCSVRenderer,) throttle_classes = [ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle] @action(methods=["GET"], detail=False) From 0332190e778eec6548c66ff6a6bcee23f644af55 Mon Sep 17 00:00:00 2001 From: Robbie Date: Wed, 24 Apr 2024 13:48:31 +0100 Subject: [PATCH 29/32] Add tests for session properties and session property values api --- posthog/api/test/test_session.py | 137 +++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 posthog/api/test/test_session.py diff --git a/posthog/api/test/test_session.py b/posthog/api/test/test_session.py new file mode 100644 index 0000000000000..46fcafabd7c13 --- /dev/null +++ b/posthog/api/test/test_session.py @@ -0,0 +1,137 @@ +import uuid + +from rest_framework import status + +from posthog.models.event.util import create_event +from posthog.test.base import APIBaseTest + + +class TestSessionsAPI(APIBaseTest): + def setUp(self) -> None: + super().setUp() + + create_event( + team=self.team, + event="$pageview", + distinct_id="d1", + properties={"$session_id": "s1", "utm_source": "google"}, + event_uuid=(uuid.uuid4()), + ) + create_event( + team=self.team, + event="$pageview", + distinct_id="d1", + properties={"$session_id": "s1", "utm_source": "youtube"}, + event_uuid=(uuid.uuid4()), + ) + + def test_expected_session_properties(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/property_definitions/") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_properties = {entry["name"] for entry in response.json()["results"]} + expected_properties = { + "$autocapture_count", + "$channel_type", + "$end_timestamp", + "$entry_url", + "$exit_url", + "$initial_gad_source", + "$initial_gclid", + "$initial_referring_domain", + "$initial_utm_campaign", + "$initial_utm_content", + "$initial_utm_medium", + "$initial_utm_source", + "$initial_utm_term", + "$pageview_count", + "$session_duration", + "$start_timestamp", + } + assert actual_properties == expected_properties + + def test_search_session_properties(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/property_definitions/?search=utm") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_properties = {entry["name"] for entry in response.json()["results"]} + expected_properties = { + "$initial_utm_campaign", + "$initial_utm_content", + "$initial_utm_medium", + "$initial_utm_source", + "$initial_utm_term", + } + assert actual_properties == expected_properties + + def test_empty_search_session_properties(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/property_definitions/?search=doesnotexist") + self.assertEqual(response.status_code, status.HTTP_200_OK) + assert len(response.json()["results"]) == 0 + + def test_list_channel_type_values(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/values/?key=$channel_type") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_values = {entry["name"] for entry in response.json()} + expected_values = { + "Affiliate", + "Audio", + "Cross Network", + "Direct", + "Email", + "Organic Search", + "Organic Shopping", + "Organic Video", + "Other", + "Paid Other", + "Paid Search", + "Paid Shopping", + "Paid Video", + "Push", + "Referral", + "SMS", + } + assert actual_values == expected_values + + def test_search_channel_type_values(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/values/?key=$channel_type&value=paid") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_values = {entry["name"] for entry in response.json()} + expected_values = { + "Paid Other", + "Paid Search", + "Paid Shopping", + "Paid Video", + } + assert actual_values == expected_values + + def test_list_session_property_values(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/values/?key=$initial_utm_source") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_values = {entry["name"] for entry in response.json()} + expected_values = { + "google", + "youtube", + } + assert actual_values == expected_values + + def test_search_session_property_values(self): + response = self.client.get(f"/api/projects/{self.team.pk}/sessions/values/?key=$initial_utm_source&value=tub") + self.assertEqual(response.status_code, status.HTTP_200_OK) + actual_values = {entry["name"] for entry in response.json()} + expected_values = { + "youtube", + } + assert actual_values == expected_values + + def test_search_session_property_no_matching_values(self): + response = self.client.get( + f"/api/projects/{self.team.pk}/sessions/values/?key=$initial_utm_source&value=doesnotexist" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + assert len(response.json()) == 0 + + def test_search_missing_session_property_values(self): + response = self.client.get( + f"/api/projects/{self.team.pk}/sessions/values/?key=$initial_utm_source&value=doesnotexist" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + assert len(response.json()) == 0 From 0ec3bf11bff9b6c2eb3dc2646847fe38d9530bb7 Mon Sep 17 00:00:00 2001 From: Robbie Date: Wed, 24 Apr 2024 13:57:49 +0100 Subject: [PATCH 30/32] Formatting --- .../hogql_queries/web_analytics/web_analytics_query_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py index de154e4c46163..cd6a218e7ea0f 100644 --- a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py +++ b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py @@ -53,7 +53,7 @@ def pathname_property_filter(self) -> Optional[EventPropertyFilter]: @cached_property def property_filters_without_pathname( - self + self, ) -> list[Union[EventPropertyFilter, PersonPropertyFilter, SessionPropertyFilter]]: return [p for p in self.query.properties if p.key != "$pathname"] From 5ab36e1a33136f44f61d9456d9678f7c455537d7 Mon Sep 17 00:00:00 2001 From: Robbie Date: Thu, 25 Apr 2024 10:03:37 +0100 Subject: [PATCH 31/32] New typing --- posthog/hogql/database/schema/sessions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/hogql/database/schema/sessions.py b/posthog/hogql/database/schema/sessions.py index 5ff98b9c67b61..63f0e4f98e79f 100644 --- a/posthog/hogql/database/schema/sessions.py +++ b/posthog/hogql/database/schema/sessions.py @@ -208,7 +208,7 @@ def to_printed_clickhouse(self, context): def to_printed_hogql(self): return "sessions" - def avoid_asterisk_fields(self) -> List[str]: + def avoid_asterisk_fields(self) -> list[str]: return [ "duration", # alias of $session_duration, deprecated but included for backwards compatibility ] From 6cd2c855ce17d167d95be1f1aac754791979b8ec Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 09:13:41 +0000 Subject: [PATCH 32/32] Update query snapshots --- posthog/api/test/__snapshots__/test_api_docs.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr index 5e807603e82de..37bb741043ef9 100644 --- a/posthog/api/test/__snapshots__/test_api_docs.ambr +++ b/posthog/api/test/__snapshots__/test_api_docs.ambr @@ -84,7 +84,7 @@ '/home/runner/work/posthog/posthog/posthog/session_recordings/session_recording_api.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer]: could not resolve field on model with path "viewed". This is likely a custom field that does some unknown magic. Maybe consider annotating the field/property? Defaulting to "string". (Exception: SessionRecording has no field named \'viewed\')', '/home/runner/work/posthog/posthog/posthog/api/person.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer > MinimalPersonSerializer]: unable to resolve type hint for function "get_distinct_ids". Consider using a type hint or @extend_schema_field. Defaulting to string.', '/home/runner/work/posthog/posthog/posthog/session_recordings/session_recording_api.py: Warning [SessionRecordingViewSet > SessionRecordingSerializer]: unable to resolve type hint for function "storage". Consider using a type hint or @extend_schema_field. Defaulting to string.', - "/home/runner/work/posthog/posthog/posthog/api/session.py: Error [SessionViewSet]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'SessionViewSet' should either include a `serializer_class` attribute, or override the `get_serializer_class()` method.)", + '/home/runner/work/posthog/posthog/posthog/api/session.py: Error [SessionViewSet]: unable to guess serializer. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. Either way you may want to add a serializer_class (or method). Ignoring view for now.', '/home/runner/work/posthog/posthog/posthog/api/session.py: Warning [SessionViewSet]: could not derive type of path parameter "project_id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. ) or annotating the parameter type with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/api/subscription.py: Warning [SubscriptionViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.subscription.Subscription" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/api/subscription.py: Warning [SubscriptionViewSet > SubscriptionSerializer]: unable to resolve type hint for function "summary". Consider using a type hint or @extend_schema_field. Defaulting to string.',