-
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
Make Breakdown limit customizable and Allow empty breakdown value in trends and funnels #5357
Conversation
Labels still broken for Funnels, fixing that |
@@ -51,14 +51,16 @@ export function FunnelStepTable({}: FunnelStepTableProps): JSX.Element | null { | |||
render: function RenderLabel({}, step: FlattenedFunnelStep): JSX.Element { | |||
const isBreakdownChild = !!filters.breakdown && !step.isBreakdownParent | |||
const color = getStepColor(step, !!filters.breakdown) | |||
console.log(step) |
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.
oops, rm!
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.
backend changes look good
@@ -6,7 +6,6 @@ | |||
FROM events e | |||
WHERE | |||
team_id = %(team_id)s {entity_query} {parsed_date_from} {parsed_date_to} {prop_filters} | |||
AND JSONHas(properties, %(key)s) |
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.
Oh, was this what was preventing none results from returning? I think that may have led me to implement the workaround for unioning the events with no property
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.
correct!
Changes
Adds a customizable limit to the number of breakdown values. Resolves #5341
Also does the same for trends, and changes how
none
breakdown used to work: Now, none isn't returned if it's empty, and is treated just like any other breakdown value (so works with limit/offset as well)For the frontend: Since now
''
is a valid breakdown value, I update the labels to work with this (and added tests for this)Checklist