Make Breakdown limit customizable and Allow empty breakdown value in trends and funnels

ee/clickhouse/queries/funnels/base.py

@@ -233,8 +233,7 @@ def _get_inner_event_query(
                 extra_join = self._get_cohort_breakdown_join()
             else:
                 breakdown_conditions = self._get_breakdown_conditions()
-                extra_conditions = "AND prop != ''" if select_prop else ""
-                extra_conditions += f"AND {breakdown_conditions}" if breakdown_conditions and select_prop else ""
+                extra_conditions = f" AND {breakdown_conditions}" if breakdown_conditions and select_prop else ""
 
         return FUNNEL_INNER_EVENT_STEPS_QUERY.format(
             steps=steps,
@@ -309,7 +308,7 @@ def _get_funnel_person_step_condition(self):
             self.params.update({"step_num": abs(step_num) - 1})
             conditions.append("steps = %(step_num)s")
 
-        if self._filter.funnel_step_breakdown:
+        if self._filter.funnel_step_breakdown is not None:
             prop_vals = self._parse_breakdown_prop_value()
             self.params.update({"breakdown_prop_value": prop_vals})
             conditions.append("prop IN %(breakdown_prop_value)s")
@@ -402,7 +401,7 @@ def _get_breakdown_conditions(self) -> str:
             if self._filter.funnel_step_breakdown:
                 values = self._parse_breakdown_prop_value()
             else:
-                limit = 5
+                limit = self._filter.breakdown_limit or 5
                 first_entity = next(x for x in self._filter.entities if x.order == 0)
                 if not first_entity:
                     ValidationError("An entity with order 0 was not provided")

ee/clickhouse/queries/funnels/test/breakdown_cases.py

@@ -493,6 +493,154 @@ def test_funnel_step_breakdown_limit(self):
             breakdown_vals = sorted([res[0]["breakdown"] for res in result])
             self.assertEqual(["5", "6", "7", "8", "9"], breakdown_vals)
 
+        def test_funnel_step_custom_breakdown_limit_with_nulls(self):
+
+            filters = {
+                "events": [{"id": "sign up", "order": 0}, {"id": "play movie", "order": 1}, {"id": "buy", "order": 2},],
+                "insight": INSIGHT_FUNNELS,
+                "date_from": "2020-01-01",
+                "date_to": "2020-01-08",
+                "funnel_window_days": 7,
+                "breakdown_type": "event",
+                "breakdown_limit": 3,
+                "breakdown": "some_breakdown_val",
+            }
+
+            filter = Filter(data=filters)
+            funnel = Funnel(filter, self.team)
+
+            for num in range(5):
+                for i in range(num):
+                    _create_person(distinct_ids=[f"person_{num}_{i}"], team_id=self.team.pk)
+                    _create_event(
+                        team=self.team,
+                        event="sign up",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T12:00:00Z",
+                    )
+                    _create_event(
+                        team=self.team,
+                        event="play movie",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T13:00:00Z",
+                    )
+                    _create_event(
+                        team=self.team,
+                        event="buy",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T15:00:00Z",
+                    )
+
+            # no breakdown value for this guy
+            _create_person(distinct_ids=[f"person_null"], team_id=self.team.pk)
+            _create_event(
+                team=self.team,
+                event="sign up",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T12:00:00Z",
+            )
+            _create_event(
+                team=self.team,
+                event="play movie",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T13:00:00Z",
+            )
+            _create_event(
+                team=self.team,
+                event="buy",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T15:00:00Z",
+            )
+
+            result = funnel.run()
+
+            breakdown_vals = sorted([res[0]["breakdown"] for res in result])
+            self.assertEqual(["2", "3", "4"], breakdown_vals)
+            # skipped 1 and '' because the limit was 3.
+
+        def test_funnel_step_custom_breakdown_limit_with_nulls_included(self):
+
+            filters = {
+                "events": [{"id": "sign up", "order": 0}, {"id": "play movie", "order": 1}, {"id": "buy", "order": 2},],
+                "insight": INSIGHT_FUNNELS,
+                "date_from": "2020-01-01",
+                "date_to": "2020-01-08",
+                "funnel_window_days": 7,
+                "breakdown_type": "event",
+                "breakdown_limit": 6,
+                "breakdown": "some_breakdown_val",
+            }
+
+            filter = Filter(data=filters)
+            funnel = Funnel(filter, self.team)
+
+            for num in range(5):
+                for i in range(num):
+                    _create_person(distinct_ids=[f"person_{num}_{i}"], team_id=self.team.pk)
+                    _create_event(
+                        team=self.team,
+                        event="sign up",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T12:00:00Z",
+                    )
+                    _create_event(
+                        team=self.team,
+                        event="play movie",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T13:00:00Z",
+                    )
+                    _create_event(
+                        team=self.team,
+                        event="buy",
+                        distinct_id=f"person_{num}_{i}",
+                        properties={"key": "val", "some_breakdown_val": num},
+                        timestamp="2020-01-01T15:00:00Z",
+                    )
+
+            # no breakdown value for this guy
+            p_null = _create_person(distinct_ids=[f"person_null"], team_id=self.team.pk)
+            _create_event(
+                team=self.team,
+                event="sign up",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T12:00:00Z",
+            )
+            _create_event(
+                team=self.team,
+                event="play movie",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T13:00:00Z",
+            )
+            _create_event(
+                team=self.team,
+                event="buy",
+                distinct_id=f"person_null",
+                properties={"key": "val"},
+                timestamp="2020-01-01T15:00:00Z",
+            )
+
+            result = funnel.run()
+
+            breakdown_vals = sorted([res[0]["breakdown"] for res in result])
+            self.assertEqual(["", "1", "2", "3", "4"], breakdown_vals)
+            # included 1 and '' because the limit was 6.
+
+            for i in range(1, 5):
+                self.assertEqual(len(self._get_people_at_step(filter, 3, str(i))), i)
+
+            self.assertEqual([p_null.uuid], self._get_people_at_step(filter, 1, ""))
+            self.assertEqual([p_null.uuid], self._get_people_at_step(filter, 3, ""))
+
         def test_funnel_step_breakdown_event_single_person_multiple_breakdowns(self):
 
             filters = {

ee/clickhouse/queries/test/test_trends.py

@@ -15,6 +15,7 @@
 from posthog.models.filters import Filter
 from posthog.models.person import Person
 from posthog.queries.test.test_trends import trend_test_factory
+from posthog.settings import SHELL_PLUS_PRINT_SQL
 
 
 def _create_action(**kwargs):
@@ -60,8 +61,9 @@ def test_breakdown_with_filter(self):
             ),
             self.team,
         )
-        self.assertEqual(len(response), 2)
-        self.assertEqual(response[1]["breakdown_value"], "val")
+        self.assertEqual(len(response), 1)
+        # don't return none option when empty
+        self.assertEqual(response[0]["breakdown_value"], "val")
 
     def test_breakdown_filtering_limit(self):
         self._create_breakdown_events()
@@ -76,7 +78,7 @@ def test_breakdown_filtering_limit(self):
                 ),
                 self.team,
             )
-        self.assertEqual(len(response), 26)  # We fetch 25 to see if there are more ethan 20 values
+        self.assertEqual(len(response), 25)  # We fetch 25 to see if there are more ethan 20 values
 
     def test_breakdown_by_person_property(self):
         person1, person2, person3, person4 = self._create_multiple_people()
@@ -107,8 +109,7 @@ def test_breakdown_by_person_property(self):
             )
 
         self.assertListEqual(
-            sorted([res["breakdown_value"] for res in event_response]),
-            sorted(["none", "person1", "person2", "person3"]),
+            sorted([res["breakdown_value"] for res in event_response]), sorted(["person1", "person2", "person3"]),
         )
 
         for response in event_response:
@@ -263,17 +264,13 @@ def test_breakdown_filtering_with_properties(self):
 
         response = sorted(response, key=lambda x: x["label"])
         self.assertEqual(response[0]["label"], "sign up - first url")
-        self.assertEqual(response[1]["label"], "sign up - none")
-        self.assertEqual(response[2]["label"], "sign up - second url")
+        self.assertEqual(response[1]["label"], "sign up - second url")
 
         self.assertEqual(sum(response[0]["data"]), 1)
         self.assertEqual(response[0]["breakdown_value"], "first url")
 
-        self.assertEqual(sum(response[1]["data"]), 0)
-        self.assertEqual(response[1]["breakdown_value"], "none")
-
-        self.assertEqual(sum(response[2]["data"]), 1)
-        self.assertEqual(response[2]["breakdown_value"], "second url")
+        self.assertEqual(sum(response[1]["data"]), 1)
+        self.assertEqual(response[1]["breakdown_value"], "second url")
 
     def test_dau_with_breakdown_filtering(self):
         sign_up_action, _ = self._create_events()
@@ -331,10 +328,10 @@ def test_dau_with_breakdown_filtering_with_prop_filter(self):
                 self.team,
             )
 
-        self.assertEqual(event_response[1]["label"], "sign up - other_value")
+        self.assertEqual(event_response[0]["label"], "sign up - other_value")
 
-        self.assertEqual(sum(event_response[1]["data"]), 1)
-        self.assertEqual(event_response[1]["data"][5], 1)  # property not defined
+        self.assertEqual(sum(event_response[0]["data"]), 1)
+        self.assertEqual(event_response[0]["data"][5], 1)  # property not defined
 
         self.assertEntityResponseEqual(action_response, event_response)
 
@@ -412,8 +409,8 @@ def test_breakdown_user_props_with_filter(self):
             ),
             self.team,
         )
-        self.assertEqual(len(response), 2)
-        self.assertEqual(response[1]["breakdown_value"], "test@gmail.com")
+        self.assertEqual(len(response), 1)
+        self.assertEqual(response[0]["breakdown_value"], "test@gmail.com")
 
     def _create_active_user_events(self):
         p0 = Person.objects.create(team_id=self.team.pk, distinct_ids=["p0"], properties={"name": "p1"})
@@ -541,7 +538,7 @@ def test_breakdown_active_user_math(self):
 
         filter = Filter(data=data)
         result = ClickhouseTrends().run(filter, self.team,)
-        self.assertEqual(result[1]["data"], [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 2.0, 2.0, 2.0, 0.0])
+        self.assertEqual(result[0]["data"], [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 2.0, 2.0, 2.0, 0.0])
 
     def test_filter_test_accounts(self):
         p1 = Person.objects.create(team_id=self.team.pk, distinct_ids=["p1"], properties={"name": "p1"})
@@ -577,7 +574,7 @@ def test_filter_test_accounts(self):
         result = ClickhouseTrends().run(filter_2, self.team,)
         self.assertEqual(result[0]["count"], 2)
         result = ClickhouseTrends().run(filter_3, self.team,)
-        self.assertEqual(result[1]["count"], 1)
+        self.assertEqual(result[0]["count"], 1)
 
     def test_breakdown_filtering_bar_chart_by_value(self):
         self._create_events()
@@ -596,8 +593,9 @@ def test_breakdown_filtering_bar_chart_by_value(self):
                 self.team,
             )
 
-        self.assertEqual(response[0]["aggregated_value"], 1)
+        self.assertEqual(response[0]["aggregated_value"], 2)  # the events without breakdown value
         self.assertEqual(response[1]["aggregated_value"], 1)
+        self.assertEqual(response[2]["aggregated_value"], 1)
         self.assertEqual(
             response[0]["days"],
             [