-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(trends): use the bespoke other and null value in trends breakdowns #19620
Conversation
Hey @Gilbert09! 👋 |
@@ -4745,7 +4753,7 @@ def test_breakdown_filtering(self): | |||
self.team, | |||
) | |||
|
|||
self.assertEqual(response[0]["label"], "sign up - none") | |||
self.assertEqual(response[0]["label"], "sign up - $$_posthog_breakdown_other_$$") | |||
self.assertEqual(response[1]["label"], "sign up - value") | |||
self.assertEqual(response[2]["label"], "sign up - other_value") | |||
self.assertEqual(response[3]["label"], "no events - none") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to stay as none
. Because there's no events, there are no breakdown values, and therefore we don't know the type of the values, therefore we dont know which version of the "other" value to use. Using the wrong one causes a clickhouse error with mismatched types with no supertype
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks about right 👍
It seems that one test is legit failing though.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
…ns (PostHog#19620) * Use the bespoke other and null value in trends breakdowns * Fixed tests * Update query snapshots --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Problem
Changes
None
in the code, allowing for theOther
breakdown to work as expected with strings, ints, and floatsHow did you test this code?