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.

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

#477 filter per entity #671

Merged
merged 21 commits into from
May 4, 2020
Merged

#477 filter per entity #671

merged 21 commits into from
May 4, 2020

Conversation

EDsCODE
Copy link
Member

@EDsCODE EDsCODE commented Apr 27, 2020

Addresses #477

Changes

  • Minor: Remove unused showMaths property
  • Add prop-filter component to entity filters
  • add properties dict to entity object and filter by these properties by entity within _filter_events
  • add symbols to prop filters

TODO:

  • integrate with funnel
  • update tests
  • adjust UI? somewhat hectic right now

Checklist

  • All querysets/queries filter by Team
  • Code reviewed
  • QA'd

@EDsCODE EDsCODE marked this pull request as draft April 27, 2020 18:26
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 27, 2020 18:27 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 13:53 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 14:20 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 14:29 Inactive
@EDsCODE EDsCODE marked this pull request as ready for review April 28, 2020 14:30
@EDsCODE EDsCODE requested a review from timgl April 28, 2020 14:33
@EDsCODE EDsCODE changed the title #654 filter per entity #477 filter per entity Apr 28, 2020
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 17:13 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 17:19 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 28, 2020 17:36 Inactive
@EDsCODE
Copy link
Member Author

EDsCODE commented Apr 28, 2020

@timgl How do you think we should start displaying the entity specific information (DAU, properties)? The graphs and tables can get confusing if only the action/event is noted. The dau is easy as we can just add an extra bit to the labels, but the properties would be a lot of text to try to push into the label

@timgl
Copy link
Collaborator

timgl commented Apr 30, 2020

@EDsCODE Reviewed, functionally works great!

Re: how to display this in the label, I think the key is actually not that important in this scenario.

  • $pageview
    • $browser = firefox
    • utm_source = twitter

Should be displayed as $pageview (firefox; twitter), which I think conveys all the information you need.

image
Also this looks a little strange (partly b/c of super subtle border-radius that makes it look misaligned), but can leave for now.

@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba April 30, 2020 14:18 Inactive
@EDsCODE
Copy link
Member Author

EDsCODE commented Apr 30, 2020

Don't want this to overextend so I added the symbols to all property filters and did an abbreviated property addition to the label. One concern is that adding a bunch of prop filters could result in a really long label. I opted for this anyway because for the moment I think it's sufficient. Can add a more comprehensive legend as needed. lmk!

@timgl
Copy link
Collaborator

timgl commented Apr 30, 2020

I like the symbols!

image
If I select "does not equal" first, then go back to "= equal" it adds __null to the key which means nothing gets returned

@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 1, 2020 00:37 Inactive
@EDsCODE
Copy link
Member Author

EDsCODE commented May 1, 2020

oof I checked out the problem because it seemed strange that any of my changes would affect how the filters are being processed and realized it was a bug on master too. Changed the null to 'exact' so the backend will pick it up directly in parsing and so we have consistent value for equality filter

@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 1, 2020 01:12 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 4, 2020 04:08 Inactive
@EDsCODE
Copy link
Member Author

EDsCODE commented May 4, 2020

Ready for a look again! The failing test is the common one regarding paginate_query unrelated to this PR

@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 4, 2020 04:28 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 4, 2020 13:41 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 4, 2020 14:09 Inactive
@timgl timgl temporarily deployed to posthog-654-filter-per--d1kiba May 4, 2020 14:10 Inactive
@timgl timgl merged commit 350ba84 into master May 4, 2020
@timgl timgl deleted the #654-filter-per-entity branch May 4, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants