Skip to content

Commit

Permalink
Remove Autocapture functionality from Paths (#5895)
Browse files Browse the repository at this point in the history
Co-authored-by: eric <[email protected]>
  • Loading branch information
neilkakkar and EDsCODE authored Sep 20, 2021
1 parent 06f3f3a commit d6e410b
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 158 deletions.
21 changes: 1 addition & 20 deletions ee/clickhouse/queries/paths/path_event_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from ee.clickhouse.models.property import get_property_string_expr
from ee.clickhouse.queries.event_query import ClickhouseEventQuery
from posthog.constants import (
AUTOCAPTURE_EVENT,
FUNNEL_PATH_AFTER_STEP,
FUNNEL_PATH_BEFORE_STEP,
FUNNEL_PATH_BETWEEN_STEPS,
Expand Down Expand Up @@ -55,12 +54,7 @@ def get_query(self) -> Tuple[str, Dict[str, Any]]:
if self._should_query_url()
else "if(0, '', "
)
event_conditional += (
f"if({self.EVENT_TABLE_ALIAS}.event = '{AUTOCAPTURE_EVENT}', concat('autocapture:', {self.EVENT_TABLE_ALIAS}.elements_chain), "
if self._should_query_elements_chain()
else "if(0, '', "
)
event_conditional += f"{self.EVENT_TABLE_ALIAS}.event))) AS path_item"
event_conditional += f"{self.EVENT_TABLE_ALIAS}.event)) AS path_item"

_fields.append(event_conditional)

Expand Down Expand Up @@ -113,9 +107,6 @@ def _get_event_query(self) -> Tuple[str, Dict[str, Any]]:
if self._filter.include_screenviews:
or_conditions.append(f"event = '{SCREEN_EVENT}'")

if self._filter.include_autocaptures:
or_conditions.append(f"event = '{AUTOCAPTURE_EVENT}'")

if self._filter.include_all_custom_events:
or_conditions.append(f"NOT event LIKE '$%%'")

Expand Down Expand Up @@ -154,13 +145,3 @@ def _should_query_screen(self) -> bool:
return True

return False

def _should_query_elements_chain(self) -> bool:
if (
self._filter.target_events == [] and self._filter.custom_events == []
) and AUTOCAPTURE_EVENT not in self._filter.exclude_events:
return True
elif self._filter.include_autocaptures:
return True

return False
2 changes: 0 additions & 2 deletions ee/clickhouse/queries/paths/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,13 @@ def __init__(self, filter: PathFilter, team: Team, funnel_filter: Optional[Filte
"events": [], # purely a speed optimization, don't need this for filtering
"event_in_session_limit": self._filter.step_limit or EVENT_IN_SESSION_LIMIT_DEFAULT,
"session_time_threshold": SESSION_TIME_THRESHOLD_DEFAULT,
"autocapture_match": "%autocapture:%",
}
self._funnel_filter = funnel_filter

if self._filter.include_all_custom_events and self._filter.custom_events:
raise ValidationError("Cannot include all custom events and specific custom events in the same query")

# TODO: don't allow including $pageview and excluding $pageview at the same time
# TODO: Filter on specific autocapture / page URLs

def run(self, *args, **kwargs):

Expand Down
27 changes: 9 additions & 18 deletions ee/clickhouse/queries/test/test_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -1106,30 +1106,26 @@ def test_paths_start_and_end(self):
)

def test_properties_queried_using_path_filter(self):
def should_query_list(filter) -> Tuple[bool, bool, bool]:
def should_query_list(filter) -> Tuple[bool, bool]:
path_query = PathEventQuery(filter, self.team.id)
return (
path_query._should_query_url(),
path_query._should_query_screen(),
path_query._should_query_elements_chain(),
)
return (path_query._should_query_url(), path_query._should_query_screen())

filter = PathFilter()
self.assertEqual(should_query_list(filter), (True, True, True))
self.assertEqual(should_query_list(filter), (True, True))

filter = PathFilter({"include_event_types": ["$pageview"]})
self.assertEqual(should_query_list(filter), (True, False, False))
self.assertEqual(should_query_list(filter), (True, False))

filter = PathFilter({"include_event_types": ["$screen"]})
self.assertEqual(should_query_list(filter), (False, True, False))
self.assertEqual(should_query_list(filter), (False, True))

filter = filter.with_data({"include_event_types": [], "include_custom_events": ["/custom1", "/custom2"]})
self.assertEqual(should_query_list(filter), (False, False, False))
self.assertEqual(should_query_list(filter), (False, False))

filter = filter.with_data(
{"include_event_types": ["$pageview", "$screen", "custom_event"], "include_custom_events": []}
)
self.assertEqual(should_query_list(filter), (True, True, False))
self.assertEqual(should_query_list(filter), (True, True))

filter = filter.with_data(
{
Expand All @@ -1138,14 +1134,9 @@ def should_query_list(filter) -> Tuple[bool, bool, bool]:
"exclude_events": ["/custom1"],
}
)
self.assertEqual(should_query_list(filter), (True, True, False))
self.assertEqual(should_query_list(filter), (True, True))

filter = filter.with_data(
{"include_event_types": [], "include_custom_events": [], "exclude_events": ["$pageview"],}
)
self.assertEqual(should_query_list(filter), (False, True, True))

filter = filter.with_data(
{"include_event_types": [], "include_custom_events": [], "exclude_events": ["$autocapture"],}
)
self.assertEqual(should_query_list(filter), (True, True, False))
self.assertEqual(should_query_list(filter), (False, True))
12 changes: 1 addition & 11 deletions ee/clickhouse/sql/paths/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,7 @@


PATH_ARRAY_QUERY = """
SELECT if(target_event LIKE %(autocapture_match)s, concat(arrayElement(splitByString('autocapture:', assumeNotNull(source_event)), 1), final_source_element), source_event) final_source_event,
if(target_event LIKE %(autocapture_match)s, concat(arrayElement(splitByString('autocapture:', assumeNotNull(target_event)), 1), final_target_element), target_event) final_target_event,
event_count,
average_conversion_time,
if(target_event LIKE %(autocapture_match)s, arrayElement(splitByString('autocapture:', assumeNotNull(source_event)), 2), NULL) source_event_elements_chain,
concat('<', extract(source_event_elements_chain, '^(.*?)[.|:]'), '> ', extract(source_event_elements_chain, 'text="(.*?)"')) final_source_element,
if(target_event LIKE %(autocapture_match)s, arrayElement(splitByString('autocapture:', assumeNotNull(target_event)), 2), NULL) target_event_elements_chain,
concat('<', extract(target_event_elements_chain, '^(.*?)[.|:]'), '> ', extract(target_event_elements_chain, 'text="(.*?)"')) final_target_element
FROM (
SELECT last_path_key as source_event,
SELECT last_path_key as source_event,
path_key as target_event,
COUNT(*) AS event_count,
avg(conversion_time) AS average_conversion_time
Expand Down Expand Up @@ -201,6 +192,5 @@
source_event,
target_event
LIMIT 20
)
"""
2 changes: 0 additions & 2 deletions frontend/src/scenes/insights/InsightTabs/PathTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export function PathTab(): JSX.Element {
<Col>starting at</Col>
<Col>
<PropertyValue
endpoint={filter.path_type === PathType.AutoCapture ? 'api/paths/elements' : undefined}
outerOptions={
filter.path_type === PathType.CustomEvent
? customEventNames.map((name) => ({
Expand All @@ -58,7 +57,6 @@ export function PathTab(): JSX.Element {
value={filter.start_point}
placeholder={'Select start element'}
autoFocus={false}
allowCustom={filter.path_type !== PathType.AutoCapture}
/>
</Col>
</Row>
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/scenes/paths/pathsLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ import { dashboardsModel } from '~/models/dashboardsModel'
export const pathOptionsToLabels = {
[PathType.PageView]: 'Page views (Web)',
[PathType.Screen]: 'Screen views (Mobile)',
[PathType.AutoCapture]: 'Autocaptured events',
[PathType.CustomEvent]: 'Custom events',
}

export const pathOptionsToProperty = {
[PathType.PageView]: '$current_url',
[PathType.Screen]: '$screen_name',
[PathType.AutoCapture]: 'autocaptured_event',
[PathType.CustomEvent]: 'custom_event',
}

Expand Down
1 change: 0 additions & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,6 @@ export enum ViewType {

export enum PathType {
PageView = '$pageview',
AutoCapture = '$autocapture',
Screen = '$screen',
CustomEvent = 'custom_event',
}
Expand Down
13 changes: 1 addition & 12 deletions posthog/models/filters/mixins/paths.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from typing import Dict, List, Literal, Optional, Tuple, cast

from posthog.constants import (
AUTOCAPTURE_EVENT,
CUSTOM_EVENT,
END_POINT,
FUNNEL_PATHS,
Expand All @@ -17,7 +16,7 @@
from posthog.models.filters.mixins.common import BaseParamMixin
from posthog.models.filters.mixins.utils import cached_property, include_dict, process_bool

PathType = Literal["$pageview", "$autocapture", "$screen", "custom_event"]
PathType = Literal["$pageview", "$screen", "custom_event"]

FunnelPathsType = Literal["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"]

Expand Down Expand Up @@ -57,8 +56,6 @@ class PropTypeDerivedMixin(PathTypeMixin):
def prop_type(self) -> str:
if self.path_type == SCREEN_EVENT:
return "properties->> '$screen_name'"
elif self.path_type == AUTOCAPTURE_EVENT:
return "tag_name_source"
elif self.path_type == CUSTOM_EVENT:
return "event"
else:
Expand All @@ -70,8 +67,6 @@ class ComparatorDerivedMixin(PropTypeDerivedMixin):
def comparator(self) -> str:
if self.path_type == SCREEN_EVENT:
return "{} =".format(self.prop_type)
elif self.path_type == AUTOCAPTURE_EVENT:
return "group_id ="
elif self.path_type == CUSTOM_EVENT:
return "event ="
else:
Expand All @@ -83,8 +78,6 @@ class TargetEventDerivedMixin(PropTypeDerivedMixin):
def target_event(self) -> Tuple[Optional[PathType], Dict[str, str]]:
if self.path_type == SCREEN_EVENT:
return cast(PathType, SCREEN_EVENT), {"event": SCREEN_EVENT}
elif self.path_type == AUTOCAPTURE_EVENT:
return cast(PathType, AUTOCAPTURE_EVENT), {"event": AUTOCAPTURE_EVENT}
elif self.path_type == CUSTOM_EVENT:
return None, {}
else:
Expand Down Expand Up @@ -112,10 +105,6 @@ def include_pageviews(self) -> bool:
def include_screenviews(self) -> bool:
return SCREEN_EVENT in self.target_events

@property
def include_autocaptures(self) -> bool:
return AUTOCAPTURE_EVENT in self.target_events

@property
def include_all_custom_events(self) -> bool:
return CUSTOM_EVENT in self.target_events
Expand Down
3 changes: 0 additions & 3 deletions posthog/queries/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ def calculate_paths(self, filter: PathFilter, team: Team):

sessions_sql, sessions_sql_params = sessions.query.sql_with_params()

if event == "$autocapture":
sessions_sql = self._add_elements(query_string=sessions_sql)

events_notated = "\
SELECT *, CASE WHEN EXTRACT('EPOCH' FROM (timestamp - previous_timestamp)) >= (60 * 30) OR previous_timestamp IS NULL THEN 1 ELSE 0 END AS new_session\
FROM ({}) AS inner_sessions\
Expand Down
87 changes: 0 additions & 87 deletions posthog/queries/test/test_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,93 +228,6 @@ def test_screen_paths(self):
self.assertEqual(response[3]["target"], "3_/about")
self.assertEqual(response[3]["value"], 1)

def test_autocapture_paths(self):
person_factory(team_id=self.team.pk, distinct_ids=["person_1"])

event_factory(
event="$autocapture",
team=self.team,
distinct_id="person_1",
elements=[
Element(tag_name="a", text="hello", href="/a-url", nth_child=1, nth_of_type=0),
Element(tag_name="button", nth_child=0, nth_of_type=0),
Element(tag_name="div", nth_child=0, nth_of_type=0),
Element(tag_name="div", nth_child=0, nth_of_type=0, attr_id="nested",),
],
)

event_factory(
event="$autocapture",
team=self.team,
distinct_id="person_1",
elements=[
Element(tag_name="a", text="goodbye", nth_child=2, nth_of_type=0, attr_id="someId",),
Element(tag_name="div", nth_child=0, nth_of_type=0),
# make sure elements don't get double counted if they're part of the same event
Element(href="/a-url-2", nth_child=0, nth_of_type=0),
],
)

person_factory(team_id=self.team.pk, distinct_ids=["person_2"])
event_factory(
event="$autocapture",
team=self.team,
distinct_id="person_2",
elements=[
Element(tag_name="a", text="hello1", href="/a-url", nth_child=1, nth_of_type=0,),
Element(tag_name="button", nth_child=0, nth_of_type=0),
Element(tag_name="div", nth_child=0, nth_of_type=0),
Element(tag_name="div", nth_child=0, nth_of_type=0, attr_id="nested",),
],
)

event_factory(
event="$autocapture",
team=self.team,
distinct_id="person_2",
elements=[
Element(tag_name="a", text="goodbye1", nth_child=2, nth_of_type=0, attr_id="someId",),
Element(tag_name="div", nth_child=0, nth_of_type=0),
# make sure elements don't get double counted if they're part of the same event
Element(href="/a-url-2", nth_child=0, nth_of_type=0),
],
)

event_factory(
event="$autocapture",
team=self.team,
distinct_id="person_2",
elements=[
Element(tag_name="div", text="goodbye1", nth_child=2, nth_of_type=0, attr_id="someId",),
Element(tag_name="div", nth_child=0, nth_of_type=0),
# make sure elements don't get double counted if they're part of the same event
Element(href="/a-url-2", nth_child=0, nth_of_type=0),
],
)
filter = PathFilter(data={"path_type": "$autocapture"})

response = paths(team=self.team, filter=filter).run(team=self.team, filter=filter)

self.assertEqual(response[0]["source"], "1_<a> hello")
self.assertEqual(response[0]["target"], "2_<a> goodbye")
self.assertEqual(response[0]["value"], 1)

self.assertEqual(response[1]["source"], "1_<a> hello1")
self.assertEqual(response[1]["target"], "2_<a> goodbye1")
self.assertEqual(response[1]["value"], 1)

self.assertEqual(response[2]["source"], "2_<a> goodbye1")
self.assertEqual(response[2]["target"], "3_<div> goodbye1")
self.assertEqual(response[2]["value"], 1)

elements = self.client.get("/api/paths/elements/").json()
self.assertEqual(elements[0]["name"], "<a> goodbye") # first since captured twice
self.assertEqual(elements[1]["name"], "<a> goodbye1")
self.assertEqual(elements[2]["name"], "<a> hello")
self.assertEqual(elements[3]["name"], "<a> hello1")
self.assertEqual(elements[4]["name"], "<div> goodbye1")
self.assertEqual(len(elements), 5)

def test_paths_properties_filter(self):
person_factory(team_id=self.team.pk, distinct_ids=["person_1"])
event_factory(
Expand Down

0 comments on commit d6e410b

Please sign in to comment.