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

Events/definitions deletion #4673

Closed
liyiy opened this issue Jun 9, 2021 · 8 comments
Closed

Events/definitions deletion #4673

liyiy opened this issue Jun 9, 2021 · 8 comments
Labels
enhancement New feature or request

Comments

@liyiy
Copy link
Contributor

liyiy commented Jun 9, 2021

Is your feature request related to a problem?

As part of #4267 , we were planning to introduce a "delete event" button

Screen Shot 2021-06-09 at 2 58 02 PM

How should we handle this?

An option could be to introduce event deletion as users have requested for a while now. Looking for thoughts and opinions

Describe the solution you'd like

Describe alternatives you've considered

Additional context

Thank you for your feature request – we love each and every one!

@liyiy liyiy added the enhancement New feature or request label Jun 9, 2021
@paolodamico
Copy link
Contributor

  1. So for context, this is helpful to delete the event definition (description, tags, owner, ...). It's also helpful when you want to remove an event you know you'll no longer send so it doesn't show up in the action/event selector anymore.

  2. The reason we decided to table this for some discussion first is that intuitively when clicking here you would expect the underlying actual event data to be deleted too (which by the way is something that has been requested by users). So an initial question here is if we're able to consider now the possibility of deleting some events in Clickhouse. CC @fuziontech @macobo @tiina303

  3. If the above is still unfeasible, we could consider patching this with some UX handling where we clarify that the underlying events will not be deleted, and that if the event comes in again (ingested), it'll get added again.

  4. One final consideration is that we need to consider what happens if the event is registered in an action, dashboard or saved insight (we'd probably need to clear those too).

@mariusandra
Copy link
Collaborator

We can't delete events from Clickhouse, and definitely shouldn't scrub data with the interface we use to edit descriptions :).

Thus events in this list are permanent. We can however hide them (instead of deleting) if the user wants a cleaner interface. Renaming "delete" to "hide" and perhaps having a "show 3 hidden events" button to show them again.

@paolodamico
Copy link
Contributor

Perhaps the event deletion topic is a conversation for a separate issue, but I do find it worrying that we can't delete events from CH. It has significant privacy concerns as events may contain PII, not to mention probably non-compliant with a bunch of privacy laws. Deleting an event doesn't necessarily have to be deleting the actual object from the DB, but we do need to have a way to scrub data.

In terms of deleting the event data here, agreed it might not be the best location here right now. Hiding event definitions could be useful as well as deleting the event definition data you've set (owner, description, ...). Should we do both here?

@mariusandra
Copy link
Collaborator

I share your concerns re: not deleting, though there's not much I can do there... now. It's definitely for a separate topic. I assume if anything, we can make some "event scrubber" tool that includes 3 different "are you really sure?" confirmation screens. :)

Regarding deleting vs hiding, I don't have a super good intuition on what's the best way to describe it. The fact will remain that the definition will come back if deleted from here, when the event is next seen. Thus my suggestion for "hide" is to remove it from the list, even if it comes back later.

@paolodamico
Copy link
Contributor

Alright, let's keep the event deletion/scrubbing for a separate conversation. For the scope of this issue, how about calling this "Clear definition & hide event" and in the confirmation prompt we explain that the definition stuff will be permanently deleted and the event will be hidden, but old events will not be deleted and the event will show up again if new events are received. Thoughts @liyiy ?

@liyiy
Copy link
Contributor Author

liyiy commented Jun 14, 2021

That makes sense! Some questions:

  • Doesn't the definition pop up again if the "old" event for it is newly received, or are we keeping track of it in a is_deleted key?
  • Also should we implement this along with a mechanism for bringing back that definition?

No rush in answering this though, I don't think it's a priority at the moment to have deletion set up, but would be a nice to have down the line?

@paolodamico
Copy link
Contributor

  1. Well we would delete the definition record (i.e. owner, description, tags, ...), it's fine if the name comes back again. If we're still receiving events with the same name, it seems like expected behavior.
  2. Well we talked about having a way to create a definition too before an event comes in, so I think this would be the mechanism.
  3. Agreed, it's not a priority right now.

@paolodamico
Copy link
Contributor

Duplicate of #6099

@paolodamico paolodamico marked this as a duplicate of #6099 Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants