diff --git a/frontend/src/toolbar/elements/heatmapLogic.ts b/frontend/src/toolbar/elements/heatmapLogic.ts index 9e4ad28706076..45ec141630420 100644 --- a/frontend/src/toolbar/elements/heatmapLogic.ts +++ b/frontend/src/toolbar/elements/heatmapLogic.ts @@ -112,11 +112,8 @@ export const heatmapLogic = kea([ ], ...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 diff --git a/posthog/api/element.py b/posthog/api/element.py index f05adb666b748..d7b721dee8195 100644 --- a/posthog/api/element.py +++ b/posthog/api/element.py @@ -1,4 +1,3 @@ -import json from typing import Literal, Tuple from rest_framework import request, response, serializers, viewsets @@ -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") diff --git a/posthog/api/test/test_element.py b/posthog/api/test/test_element.py index 7772c0b844d07..72a97ea2b9b43 100644 --- a/posthog/api/test/test_element.py +++ b/posthog/api/test/test_element.py @@ -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": [ @@ -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() @@ -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" @@ -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() @@ -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() @@ -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") @@ -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=[