-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Add group support to the lifecycle insight #21876
Merged
Merged
Changes from 14 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
b99156f
frontend display dropdown
aspicer 470928e
lifecycle
aspicer c106cb5
update title
aspicer 50c4aa3
fe tests
aspicer 2d74b4f
schema
aspicer fa7832c
Merge remote-tracking branch 'origin/HEAD' into aspicer/lifecycle_groups
aspicer 8082e9e
fixing test
aspicer 2e54125
working basic test
aspicer 1de9e0e
group 1
aspicer f123280
need to remove references to person_id
aspicer ee6989c
Update UI snapshots for `webkit` (2)
github-actions[bot] dc4ef82
Update UI snapshots for `chromium` (2)
github-actions[bot] 7ac06c0
Update query snapshots
github-actions[bot] 16570e9
Update query snapshots
github-actions[bot] 11566c1
merge from master
aspicer 57215cd
refactor for events
aspicer 282a167
Update UI snapshots for `chromium` (1)
github-actions[bot] ce53791
Update UI snapshots for `chromium` (1)
github-actions[bot] 6318dc5
fix none
aspicer d4945ae
Merge branch 'aspicer/lifecycle_groups' of github.com:PostHog/posthog…
aspicer 67e8b9a
lifecycle query
aspicer 7c3d2b7
actors
aspicer 640eb7f
trends query logic
aspicer ff4fdca
Merge remote-tracking branch 'origin/master' into aspicer/lifecycle_g…
aspicer e1649d5
types
aspicer 32c4fab
test update
aspicer 9db2ff4
remove comments
aspicer 07ee6b1
Update query snapshots
github-actions[bot] cc008c1
Update UI snapshots for `chromium` (2)
github-actions[bot] 7c042ff
Update UI snapshots for `chromium` (2)
github-actions[bot] b15b5f0
Merge branch 'master' into aspicer/lifecycle_groups
aspicer ecf8d5e
merge master
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
Binary file modified
BIN
+986 Bytes
(100%)
frontend/__snapshots__/scenes-app-insights--lifecycle-edit--dark--webkit.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+1.21 KB
(100%)
frontend/__snapshots__/scenes-app-insights--lifecycle-edit--dark.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+1.01 KB
(100%)
frontend/__snapshots__/scenes-app-insights--lifecycle-edit--light--webkit.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+1.04 KB
(100%)
frontend/__snapshots__/scenes-app-insights--lifecycle-edit--light.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,7 +76,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: | |
ORDER BY status, start_of_period | ||
UNION ALL | ||
SELECT | ||
start_of_period, count(DISTINCT person_id) AS counts, status | ||
start_of_period, count(DISTINCT target) AS counts, status | ||
FROM {events_query} | ||
GROUP BY start_of_period, status | ||
) | ||
|
@@ -95,6 +95,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: | |
def to_actors_query( | ||
self, day: Optional[str] = None, status: Optional[str] = None | ||
) -> ast.SelectQuery | ast.SelectUnionQuery: | ||
# should I update this? | ||
with self.timings.measure("persons_query"): | ||
exprs = [] | ||
if day is not None: | ||
|
@@ -278,13 +279,39 @@ def event_filter(self) -> ast.Expr: | |
else: | ||
return ast.And(exprs=event_filters) | ||
|
||
@property | ||
def group_type_index(self) -> int | None: | ||
return self.query.aggregation_group_type_index | ||
|
||
@cached_property | ||
def events_query(self): | ||
with self.timings.measure("events_query"): | ||
target_field = "person_id" | ||
|
||
event_filters = [self.event_filter] | ||
|
||
if self.group_type_index is not None: | ||
group_index = int(self.group_type_index) | ||
if 0 <= group_index <= 4: | ||
target_field = f"$group_{group_index}" | ||
|
||
event_filters.append( | ||
ast.Not( | ||
expr=ast.Call( | ||
name="has", | ||
args=[ | ||
ast.Array(exprs=[ast.Constant(value="")]), | ||
ast.Field(chain=["events", f"$group_{self.group_type_index}"]), | ||
], | ||
), | ||
), | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should go directly into the |
||
# events.person_id as person_id, | ||
# removed the above line from the select ( we need to replace person_id with target everywhere ) | ||
events_query = parse_select( | ||
""" | ||
SELECT | ||
events.person_id as person_id, | ||
min(events.person.created_at) AS created_at, | ||
arraySort(groupUniqArray({trunc_timestamp})) AS all_activity, | ||
arrayPopBack(arrayPushFront(all_activity, {trunc_created_at})) as previous_activity, | ||
|
@@ -296,14 +323,16 @@ def events_query(self): | |
arrayConcat(arrayZip(all_activity, initial_status), arrayZip(dormant_periods, dormant_label)) as temp_concat, | ||
arrayJoin(temp_concat) as period_status_pairs, | ||
period_status_pairs.1 as start_of_period, | ||
period_status_pairs.2 as status | ||
period_status_pairs.2 as status, | ||
{target} | ||
FROM events | ||
WHERE {event_filter} | ||
GROUP BY person_id | ||
GROUP BY target | ||
""", | ||
placeholders={ | ||
**self.query_date_range.to_placeholders(), | ||
"event_filter": self.event_filter, | ||
"target": ast.Alias(alias="target", expr=ast.Field(chain=["events", target_field])), | ||
"event_filter": ast.And(exprs=event_filters), | ||
"trunc_timestamp": self.query_date_range.date_to_start_of_interval_hogql( | ||
ast.Field(chain=["events", "timestamp"]) | ||
), | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I could see this being a followup PR, however yes, somebody one day should. This is the query that opens when you click on one of the bars on the chart. I don't think it's that many extra steps to get this working.