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

Always show event stats and add warnings #3908

Merged
merged 8 commits into from Apr 9, 2021

Conversation

timgl
Copy link
Collaborator

@timgl timgl commented Apr 8, 2021

Changes

  • Always show the list of events/properties, even if ASYNC_EVENT_PROPERTY_USAGE isn't set (so we won't have any stats to show)
  • Add a search box
  • Add warnings (specifically whether an event/key has leading/trailing spaces)
    • I think we can build these out to have a really good system for ensuring data consistency etc. This is just a start.

image

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl requested a review from paolodamico April 8, 2021 12:49
@timgl timgl temporarily deployed to posthog-pr-3908 April 8, 2021 12:52 Inactive
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

lgtm!

a.usage_count == b.usage_count ? a.volume - b.volume : a.usage_count - b.usage_count,
},
]
const { user } = useValues(userLogic)
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

As a small improvement for later, I think this logic can be moved to a selector.

@timgl timgl temporarily deployed to posthog-pr-3908 April 9, 2021 17:09 Inactive
@paolodamico
Copy link
Contributor

FYI did a few tweaks,

  • Changed the UI of the notice when this is disabled to be a bit more clear.
  • Removed the warnings column in favor of just showing them next to the name if applicable (plus adjusted styling a bit).
  • You can filter events/properties on whether they have any warnings.

@timgl timgl added the automerge Merge this PR automatically when REQUIRED checks pass label Apr 9, 2021
@posthog-bot posthog-bot merged commit 6b09a7c into master Apr 9, 2021
@posthog-bot posthog-bot deleted the always-show-event-stats-and-warnings branch April 9, 2021 17:14
fuziontech pushed a commit that referenced this pull request Apr 19, 2021
…o 3765-cohort-by-trend

* '3765-cohort-by-trend' of github.com:PostHog/posthog: (39 commits)
  'string, parsable as datetime' (#3942)
  Update plugin server to 0.16.3 (#3944)
  Resizable table columns in Sessions (#3927)
  bump cryptography==3.4.7 and add macosx_arm64 install script (#3935)
  🤖: Add jeduden as a contributor 🎉 (#3938)
  Fix feature flags default rollout (#3745)
  Less dancing in dashboards (#3824)
  Always show event stats and add warnings (#3908)
  Update plugin server to 0.16.2 (#3932)
  Minimum PostHog version in plugins (#3916)
  Renames Active users to Unique users (#3930)
  Fix action with same name (#3909)
  Fix navigation to insights from dashboards (#3928)
  User V2 Part II - Frontend changes (#3866)
  update autocapture label to be more descriptive (#3925)
  Log to sentry when migrations are out of date (#3924)
  Revert "Increase Element model varchar limits (#3912)" (#3923)
  Update plugin server to 0.16.1 (#3920)
  Are migrations safe to run on cloud?  (#3917)
  Run Automerge as posthog-bot (#3919)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR automatically when REQUIRED checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants