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

feat(events): show stale events in separate section #4811

Closed
wants to merge 1 commit into from

Conversation

hazzadous
Copy link
Contributor

@hazzadous hazzadous commented Jun 19, 2021

Changes

Previously all events would be listed in one list, irrespective of if
they have recent activity. This makes it difficult to find relevent
events if you have lots of stale events.

This change:

  • adds a group "Stale events" based on volume_30_days being falsy
  • filters the main "Events" group by volume_30_days being truthy

Closes #4791

References #4502

Note that this also references #4502 as it achieves the functionality of
highlighting which events have had recent activity.

Using this issue as a test bed to get setup with the repo.

Outstanding questions

  • What does it mean for volume_30_days to be null? I'm just going on the typescript here but is this a special case that needs to be considered
  • Handling of preflight?.is_event_property_usage_enabled
  • Do we need a different icon for this grouping?
  • Should we add an information hover to clarify that "Stale" means, or be explicit with the grouping name eg "Events not active in 30 days"
  • Is there a specific place that a test should be added

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
  • Frontend/CSS is usable at 320px (iPhone SE) and decent at 360px (most phones)
  • Breaking changes are backwards-compatible. Ensure old/new frontend requests work with new/old backends, and vice versa.

Previously all events would ve listed in one list, irrespective of if
they have recent activity. This makes it difficult to find relevent
events if you have lots of stale events.

This change:

 * adds a group "Stale events" based on `volume_30_days` being falsy
 * filters the main "Events" group by `volume_30_days` being truthy

Closes PostHog#4791

References PostHog#4502

Note that this also references PostHog#4502 as it achieves the functionality of
highlighting which events have had recent activity.
@timgl
Copy link
Collaborator

timgl commented Jun 21, 2021

Thanks for grabbing this Harry!!

What does it mean for volume_30_days to be null? I'm just going on the typescript here but is this a special case that needs to be considered

This is a good point. I'm pretty sure this means that we haven't run the query yet to get the volume, in which case it should not be marked as stale.

Do we need a different icon for this grouping?

Maybe this one?
image

Should we add an information hover to clarify that "Stale" means, or be explicit with the grouping name eg "Events not active in 30 days"

Yep good idea, can use the <Tooltip component. You could also add it to the EventInfo component in ActionFilterDropdown.tsx as an extra warning

Is there a specific place that a test should be added

we don't have an amazing story for tests in the frontend yet, especially with this type of more fiddly stuff. You could add an e2e test but you'd have to change some of the env variables so that usage is enabled

@hazzadous
Copy link
Contributor Author

Closing for now to avoid cluttering the PR list, I'll reopen when I get a chance to make the suggested updates.

@hazzadous hazzadous closed this Jun 27, 2021
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.

Hide events in event select box with no usage data in last 30 days
2 participants