Skip to content

Commit

Permalink
default to both autpcapture and rageclicks is simpler
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra committed Mar 27, 2024
1 parent 86b151c commit b042f2f
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 41 deletions.
7 changes: 2 additions & 5 deletions frontend/src/toolbar/elements/heatmapLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,8 @@ export const heatmapLogic = kea<heatmapLogicType>([
],
...values.heatmapFilter,
}
const includeEventsParams = '&include=["$autocapture", "$rageclick"]'
defaultUrl = `/api/element/stats/${encodeParams(
{ ...params, paginate_response: true },
'?'
)}${includeEventsParams}`

defaultUrl = `/api/element/stats/${encodeParams({ ...params, paginate_response: true }, '?')}`
}

// toolbar fetch collapses queryparams but this URL has multiple with the same name
Expand Down
18 changes: 4 additions & 14 deletions posthog/api/element.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
from typing import Literal, Tuple

from rest_framework import request, response, serializers, viewsets
Expand Down Expand Up @@ -131,23 +130,14 @@ def stats(self, request: request.Request, **kwargs) -> response.Response:

def _events_filter(self, request) -> Tuple[Literal["$autocapture", "$rageclick"], ...]:
event_to_filter: Tuple[Literal["$autocapture", "$rageclick"], ...] = ()
# when multiple includes are sent expects them as separate parameters
# e.g. ?include=a&include=b
events_to_include = request.query_params.getlist("include", [])

# we are migrating from sending multiple include parameters
# ?include=a&include=b to a single include parameter
# ?include=["a", "b"]
# some people run old versions of the toolbar code, so we need to support both
if events_to_include and len(events_to_include) == 1:
if events_to_include[0].startswith("["):
# once we are getting no hits on this counter we can stop processing separate include params
statsd.incr(
"toolbar_element_stats_separate_include_params_tombstone",
tags={"team_id": self.team_id},
)
events_to_include = json.loads(events_to_include[0])

if not events_to_include:
# sensible default when not provided
event_to_filter += ("$autocapture",)
event_to_filter += ("$rageclick",)
else:
if "$rageclick" in events_to_include:
events_to_include.remove("$rageclick")
Expand Down
78 changes: 56 additions & 22 deletions posthog/api/test/test_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
snapshot_postgres_queries,
)

expected_all_data_response_results: List[Dict] = [
expected_autocapture_data_response_results: List[Dict] = [
{
"count": 2,
"count": 3,
"hash": None,
"type": "$autocapture",
"elements": [
Expand Down Expand Up @@ -159,7 +159,7 @@ def test_element_stats_can_filter_by_properties(self) -> None:
self._setup_events()

response = self.client.get("/api/element/stats/?paginate_response=true").json()
assert len(response["results"]) == 2
assert len(response["results"]) == 3

properties_filter = json.dumps([{"key": "$current_url", "value": "http://example.com/another_page"}])
response = self.client.get(f"/api/element/stats/?paginate_response=true&properties={properties_filter}").json()
Expand All @@ -184,7 +184,7 @@ def test_element_stats_without_pagination(self) -> None:

response = self.client.get("/api/element/stats").json()
# not nested into a results property
assert response == expected_all_data_response_results
assert response == expected_autocapture_data_response_results + expected_rage_click_data_response_results

def test_element_stats_clamps_date_from_to_start_of_day(self) -> None:
event_start = "2012-01-14T03:21:34.000Z"
Expand Down Expand Up @@ -245,13 +245,12 @@ def test_element_stats_can_load_all_the_data(self) -> None:
assert response_json["next"] is None # loaded all the data, so no next link
results = response_json["results"]

assert results == expected_all_data_response_results
assert results == expected_autocapture_data_response_results + expected_rage_click_data_response_results

@parameterized.expand(["&include=$rageclick", '&include=["$rageclick"]'])
def test_element_stats_can_load_only_rageclick_data(self, include_params) -> None:
def test_element_stats_can_load_only_rageclick_data(self) -> None:
self._setup_events()

response = self.client.get(f"/api/element/stats/?paginate_response=true{include_params}")
response = self.client.get(f"/api/element/stats/?paginate_response=true&include=$rageclick")
self.assertEqual(response.status_code, status.HTTP_200_OK)

response_json = response.json()
Expand All @@ -260,7 +259,8 @@ def test_element_stats_can_load_only_rageclick_data(self, include_params) -> Non

assert results == expected_rage_click_data_response_results

@parameterized.expand(["&include=$rageclick&include=$autocapture", '&include=["$rageclick", "$autocapture"]'])
# no include params is equivalent to autocapture and rageclick
@parameterized.expand(["&include=$rageclick&include=$autocapture", ""])
def test_element_stats_can_load_rageclick_and_autocapture_data(self, include_params) -> None:
self._setup_events()

Expand All @@ -271,26 +271,40 @@ def test_element_stats_can_load_rageclick_and_autocapture_data(self, include_par
assert response_json["next"] is None # loaded all the data, so no next link
results = response_json["results"]

assert results == expected_all_data_response_results + expected_rage_click_data_response_results
assert results == expected_autocapture_data_response_results + expected_rage_click_data_response_results

def test_element_stats_obeys_limit_parameter(self) -> None:
self._setup_events()

response = self.client.get(f"/api/element/stats/?paginate_response=true&limit=1")
self.assertEqual(response.status_code, status.HTTP_200_OK)
page_one_response = self.client.get(f"/api/element/stats/?paginate_response=true&limit=1")
self.assertEqual(page_one_response.status_code, status.HTTP_200_OK)

response_json = response.json()
assert response_json["next"] == "http://testserver/api/element/stats/?paginate_response=true&limit=1&offset=1"
limit_to_one_results = response_json["results"]
assert limit_to_one_results == [expected_all_data_response_results[0]]
page_one_response_json = page_one_response.json()
assert (
page_one_response_json["next"]
== "http://testserver/api/element/stats/?paginate_response=true&limit=1&offset=1"
)
limit_to_one_results = page_one_response_json["results"]
assert limit_to_one_results == [expected_autocapture_data_response_results[0]]

response = self.client.get(f"/api/element/stats/?paginate_response=true&limit=1&offset=1")
self.assertEqual(response.status_code, status.HTTP_200_OK)
page_two_response = self.client.get(f"/api/element/stats/?paginate_response=true&limit=1&offset=1")
self.assertEqual(page_two_response.status_code, status.HTTP_200_OK)

response_json = response.json()
assert response_json["next"] is None
limit_to_one_results = response_json["results"]
assert limit_to_one_results == [expected_all_data_response_results[1]]
page_two_response_json = page_two_response.json()
assert (
page_two_response_json["next"]
== "http://testserver/api/element/stats/?paginate_response=true&limit=1&offset=2"
)
limit_to_one_results_page_two = page_two_response_json["results"]
assert limit_to_one_results_page_two == [expected_autocapture_data_response_results[1]]

page_three_response = self.client.get(f"/api/element/stats/?paginate_response=true&limit=1&offset=2")
self.assertEqual(page_three_response.status_code, status.HTTP_200_OK)

page_three_response_json = page_three_response.json()
assert page_three_response_json["next"] is None
limit_to_one_results_page_three = page_three_response_json["results"]
assert limit_to_one_results_page_three == [expected_rage_click_data_response_results[0]]

def test_element_stats_does_not_allow_non_numeric_limit(self) -> None:
response = self.client.get(f"/api/element/stats/?limit=not-a-number")
Expand Down Expand Up @@ -352,6 +366,26 @@ def _setup_events(self):
distinct_id="one",
properties={"$current_url": "http://example.com/demo"},
)
_create_event(
team=self.team,
elements=[
Element(
tag_name="a",
href="https://posthog.com/event-1",
text="event 1",
order=0,
),
Element(
tag_name="div",
href="https://posthog.com/event-1",
text="event 1",
order=1,
),
],
event="$autocapture",
distinct_id="one",
properties={"$current_url": "http://example.com/demo"},
)
_create_event(
team=self.team,
elements=[
Expand Down

0 comments on commit b042f2f

Please sign in to comment.