Skip to content

Commit

Permalink
fix: toolbar include params (#21182)
Browse files Browse the repository at this point in the history
* fix: toolbar include params

* Update query snapshots

* default to both autpcapture and rageclicks is simpler

* Update query snapshots

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
pauldambra and github-actions[bot] authored Mar 27, 2024
1 parent 553c8d2 commit 0d11b57
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 29 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&include=$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
5 changes: 5 additions & 0 deletions posthog/api/element.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +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", [])

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
2 changes: 1 addition & 1 deletion posthog/api/test/__snapshots__/test_feature_flag.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@
# ---
# name: TestFeatureFlag.test_creating_static_cohort.14
'''
/* user_id:200 celery:posthog.tasks.calculate_cohort.insert_cohort_from_feature_flag */
/* user_id:201 celery:posthog.tasks.calculate_cohort.insert_cohort_from_feature_flag */
SELECT count(DISTINCT person_id)
FROM person_static_cohort
WHERE team_id = 2
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/test/__snapshots__/test_query.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
# ---
# name: TestQuery.test_full_hogql_query_async
'''
/* user_id:467 celery:posthog.tasks.tasks.process_query_task */
/* user_id:468 celery:posthog.tasks.tasks.process_query_task */
SELECT events.uuid AS uuid,
events.event AS event,
events.properties AS properties,
Expand Down
79 changes: 57 additions & 22 deletions posthog/api/test/test_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from django.test import override_settings
from freezegun import freeze_time
from parameterized import parameterized
from rest_framework import status

from posthog.models import Element, ElementGroup, Organization
Expand All @@ -16,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 @@ -158,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 @@ -183,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 @@ -244,7 +245,7 @@ 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

def test_element_stats_can_load_only_rageclick_data(self) -> None:
self._setup_events()
Expand All @@ -258,38 +259,52 @@ def test_element_stats_can_load_only_rageclick_data(self) -> None:

assert results == expected_rage_click_data_response_results

def test_element_stats_can_load_rageclick_and_autocapture_data(self) -> None:
# 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()

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

response_json = response.json()
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 @@ -351,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 0d11b57

Please sign in to comment.