fix: send workspace create/update notifications to template admins only #16071
Conversation
There was a problem hiding this comment.
I'm nervous about this notification being enabled by default.
It will get very noisy in large environments, and will be fairly low signal. Only if an admin explicitly wants to add an integration should they enable this, and likely with webhooks only.
WDYT @bpmct @DanielleMaywood think about disabling this notification by default?
We don't really have a precedent for this (i.e. disabling a notification by default), but now might be the time to introduce it.
Blowing up the scope is better than blowing up admins' mailboxes, IMHO.
This PR largely LGTM modulo some minor points, but I'm blocking its merging until we come to a conclusion on the above.
I'm in two minds about this:
|
|
I'm a big fan of Principle of Least Surprise. I think for existing customers who upgrade to a more recent version and now start getting hundreds of emails they didn't expect violates that principle. |
|
Hey folks - apologies for the confusion here. Let's disable this notification by default both for users and admins. In fact, we should probably not make it available for users to configure. The second point is less important to me, but I just can't think of a good use case for it. Some of these notifications are really interesting from a webhook POV (fails, creates/updates to an external system) but emails themselves with no filtering would create a lot of spam. |
Sounds good 👍 thanks @bpmct - and no need to apologise!
Fortunately, regular (unprivileged) users will only be able to modify their own workspaces, and we're now inhibiting these notifications if the initiator is owner of the workspace; so we won't have to build in this out. |
Change as part of #16071 It has been decided that we want to be able to have some notification templates be disabled _by default_ #16071 (comment). This adds a new column (`enabled_by_default`) to `notification_templates` that defaults to `TRUE`. It also modifies the `inhibit_enqueue_if_disabled` function to reject notifications for templates that have `enabled_by_default = FALSE` with the user not explicitly enabling it.
…ns-to-template-admins-only
Relates to #15845
Rather than sending the notification to the user, we send it to the template admins. We also do not send it to the person that created the request.