Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add paths persons to the backend #6035

Merged
merged 4 commits into from
Sep 22, 2021
Merged

Add paths persons to the backend #6035

merged 4 commits into from
Sep 22, 2021

Conversation

neilkakkar
Copy link
Collaborator

Changes

API pending, part of #5901

Will go in after resolving merge conflicts with: #5895

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests
  • Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious
  • New/changed UI is decent on smartphones (viewport width around 360px)

@timgl timgl temporarily deployed to posthog-pr-6035 September 20, 2021 15:11 Inactive
@timgl timgl temporarily deployed to posthog-pr-6035 September 20, 2021 15:18 Inactive
@neilkakkar neilkakkar marked this pull request as ready for review September 20, 2021 15:18
@timgl timgl temporarily deployed to posthog-pr-6035 September 20, 2021 15:30 Inactive
@neilkakkar neilkakkar mentioned this pull request Sep 21, 2021
Merged
6 tasks
EDsCODE
EDsCODE reviewed Sep 21, 2021
View reviewed changes
Copy link
Member

@EDsCODE EDsCODE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Before merging, can you add detailed PR descriptions about which parameters have been added and what's been exposed? Might be worth putting this into the query classes too

ee/clickhouse/queries/paths/path_event_query.py Outdated Show resolved Hide resolved
@neilkakkar
Copy link
Collaborator Author

Looks good. Before merging, can you add detailed PR descriptions about which parameters have been added and what's been exposed? Might be worth putting this into the query classes too

Will do so in a follow up PR, which introduces the api/person/paths endpoint

@timgl timgl temporarily deployed to posthog-pr-6035 September 22, 2021 09:31 Inactive
@neilkakkar neilkakkar merged commit 0d5ac10 into master Sep 22, 2021
@neilkakkar neilkakkar deleted the path_persons branch September 22, 2021 09:53
@EDsCODE EDsCODE mentioned this pull request Sep 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EDsCODE EDsCODE EDsCODE left review comments

@liyiy liyiy Awaiting requested review from liyiy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neilkakkar @EDsCODE @timgl