feat: allow notification templates to be disabled by default (#16093)

Change as part of https://github.com/coder/coder/pull/16071

It has been decided that we want to be able to have some notification
templates be disabled _by default_
https://github.com/coder/coder/pull/16071#issuecomment-2580757061.

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.
This commit is contained in:
Danielle Maywood
2025-01-13 15:01:47 +00:00
committed by GitHub
parent 22236f2988
commit 009069cd47
20 changed files with 231 additions and 62 deletions
+3
View File
@@ -11712,6 +11712,9 @@ const docTemplate = `{
"body_template": {
"type": "string"
},
"enabled_by_default": {
"type": "boolean"
},
"group": {
"type": "string"
},
+3
View File
@@ -10506,6 +10506,9 @@
"body_template": {
"type": "string"
},
"enabled_by_default": {
"type": "boolean"
},
"group": {
"type": "string"
},
+20 -8
View File
@@ -346,13 +346,24 @@ CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
-- Fail the insertion if the user has disabled this notification.
IF EXISTS (SELECT 1
FROM notification_preferences
WHERE disabled = TRUE
AND user_id = NEW.user_id
AND notification_template_id = NEW.notification_template_id) THEN
RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification';
-- Fail the insertion if one of the following:
-- * the user has disabled this notification.
-- * the notification template is disabled by default and hasn't
-- been explicitly enabled by the user.
IF EXISTS (
SELECT 1 FROM notification_templates
LEFT JOIN notification_preferences
ON notification_preferences.notification_template_id = notification_templates.id
AND notification_preferences.user_id = NEW.user_id
WHERE notification_templates.id = NEW.notification_template_id AND (
-- Case 1: The user has explicitly disabled this template
notification_preferences.disabled = TRUE
OR
-- Case 2: The template is disabled by default AND the user hasn't enabled it
(notification_templates.enabled_by_default = FALSE AND notification_preferences.notification_template_id IS NULL)
)
) THEN
RAISE EXCEPTION 'cannot enqueue message: notification is not enabled';
END IF;
RETURN NEW;
@@ -874,7 +885,8 @@ CREATE TABLE notification_templates (
actions jsonb,
"group" text,
method notification_method,
kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL
kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL,
enabled_by_default boolean DEFAULT true NOT NULL
);
COMMENT ON TABLE notification_templates IS 'Templates from which to create notification messages.';
@@ -0,0 +1,18 @@
ALTER TABLE notification_templates DROP COLUMN enabled_by_default;
CREATE OR REPLACE FUNCTION inhibit_enqueue_if_disabled()
RETURNS TRIGGER AS
$$
BEGIN
-- Fail the insertion if the user has disabled this notification.
IF EXISTS (SELECT 1
FROM notification_preferences
WHERE disabled = TRUE
AND user_id = NEW.user_id
AND notification_template_id = NEW.notification_template_id) THEN
RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification';
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
@@ -0,0 +1,29 @@
ALTER TABLE notification_templates ADD COLUMN enabled_by_default boolean DEFAULT TRUE NOT NULL;
CREATE OR REPLACE FUNCTION inhibit_enqueue_if_disabled()
RETURNS TRIGGER AS
$$
BEGIN
-- Fail the insertion if one of the following:
-- * the user has disabled this notification.
-- * the notification template is disabled by default and hasn't
-- been explicitly enabled by the user.
IF EXISTS (
SELECT 1 FROM notification_templates
LEFT JOIN notification_preferences
ON notification_preferences.notification_template_id = notification_templates.id
AND notification_preferences.user_id = NEW.user_id
WHERE notification_templates.id = NEW.notification_template_id AND (
-- Case 1: The user has explicitly disabled this template
notification_preferences.disabled = TRUE
OR
-- Case 2: The template is disabled by default AND the user hasn't enabled it
(notification_templates.enabled_by_default = FALSE AND notification_preferences.notification_template_id IS NULL)
)
) THEN
RAISE EXCEPTION 'cannot enqueue message: notification is not enabled';
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
@@ -0,0 +1,9 @@
-- Enable 'workspace created' notification by default
UPDATE notification_templates
SET enabled_by_default = TRUE
WHERE id = '281fdf73-c6d6-4cbb-8ff5-888baf8a2fff';
-- Enable 'workspace manually updated' notification by default
UPDATE notification_templates
SET enabled_by_default = TRUE
WHERE id = 'd089fe7b-d5c5-4c0c-aaf5-689859f7d392';
@@ -0,0 +1,9 @@
-- Disable 'workspace created' notification by default
UPDATE notification_templates
SET enabled_by_default = FALSE
WHERE id = '281fdf73-c6d6-4cbb-8ff5-888baf8a2fff';
-- Disable 'workspace manually updated' notification by default
UPDATE notification_templates
SET enabled_by_default = FALSE
WHERE id = 'd089fe7b-d5c5-4c0c-aaf5-689859f7d392';
+3 -2
View File
@@ -2480,8 +2480,9 @@ type NotificationTemplate struct {
Actions []byte `db:"actions" json:"actions"`
Group sql.NullString `db:"group" json:"group"`
// NULL defers to the deployment-level method
Method NullNotificationMethod `db:"method" json:"method"`
Kind NotificationTemplateKind `db:"kind" json:"kind"`
Method NullNotificationMethod `db:"method" json:"method"`
Kind NotificationTemplateKind `db:"kind" json:"kind"`
EnabledByDefault bool `db:"enabled_by_default" json:"enabled_by_default"`
}
// A table used to configure apps that can use Coder as an OAuth2 provider, the reverse of what we are calling external authentication.
+6 -3
View File
@@ -4134,7 +4134,7 @@ func (q *sqlQuerier) GetNotificationReportGeneratorLogByTemplate(ctx context.Con
}
const getNotificationTemplateByID = `-- name: GetNotificationTemplateByID :one
SELECT id, name, title_template, body_template, actions, "group", method, kind
SELECT id, name, title_template, body_template, actions, "group", method, kind, enabled_by_default
FROM notification_templates
WHERE id = $1::uuid
`
@@ -4151,12 +4151,13 @@ func (q *sqlQuerier) GetNotificationTemplateByID(ctx context.Context, id uuid.UU
&i.Group,
&i.Method,
&i.Kind,
&i.EnabledByDefault,
)
return i, err
}
const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind :many
SELECT id, name, title_template, body_template, actions, "group", method, kind
SELECT id, name, title_template, body_template, actions, "group", method, kind, enabled_by_default
FROM notification_templates
WHERE kind = $1::notification_template_kind
ORDER BY name ASC
@@ -4180,6 +4181,7 @@ func (q *sqlQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind No
&i.Group,
&i.Method,
&i.Kind,
&i.EnabledByDefault,
); err != nil {
return nil, err
}
@@ -4233,7 +4235,7 @@ const updateNotificationTemplateMethodByID = `-- name: UpdateNotificationTemplat
UPDATE notification_templates
SET method = $1::notification_method
WHERE id = $2::uuid
RETURNING id, name, title_template, body_template, actions, "group", method, kind
RETURNING id, name, title_template, body_template, actions, "group", method, kind, enabled_by_default
`
type UpdateNotificationTemplateMethodByIDParams struct {
@@ -4253,6 +4255,7 @@ func (q *sqlQuerier) UpdateNotificationTemplateMethodByID(ctx context.Context, a
&i.Group,
&i.Method,
&i.Kind,
&i.EnabledByDefault,
)
return i, err
}
+9 -8
View File
@@ -271,14 +271,15 @@ func (api *API) putUserNotificationPreferences(rw http.ResponseWriter, r *http.R
func convertNotificationTemplates(in []database.NotificationTemplate) (out []codersdk.NotificationTemplate) {
for _, tmpl := range in {
out = append(out, codersdk.NotificationTemplate{
ID: tmpl.ID,
Name: tmpl.Name,
TitleTemplate: tmpl.TitleTemplate,
BodyTemplate: tmpl.BodyTemplate,
Actions: string(tmpl.Actions),
Group: tmpl.Group.String,
Method: string(tmpl.Method.NotificationMethod),
Kind: string(tmpl.Kind),
ID: tmpl.ID,
Name: tmpl.Name,
TitleTemplate: tmpl.TitleTemplate,
BodyTemplate: tmpl.BodyTemplate,
Actions: string(tmpl.Actions),
Group: tmpl.Group.String,
Method: string(tmpl.Method.NotificationMethod),
Kind: string(tmpl.Kind),
EnabledByDefault: tmpl.EnabledByDefault,
})
}
+1 -1
View File
@@ -20,7 +20,7 @@ import (
)
var (
ErrCannotEnqueueDisabledNotification = xerrors.New("user has disabled this notification")
ErrCannotEnqueueDisabledNotification = xerrors.New("notification is not enabled")
ErrDuplicate = xerrors.New("duplicate notification")
)
@@ -1106,6 +1106,20 @@ func TestNotificationTemplates_Golden(t *testing.T) {
r.Name = tc.payload.UserName
},
)
// With the introduction of notifications that can be disabled
// by default, we want to make sure the user preferences have
// the notification enabled.
_, err := adminClient.UpdateUserNotificationPreferences(
context.Background(),
user.ID,
codersdk.UpdateUserNotificationPreferences{
TemplateDisabledMap: map[string]bool{
tc.id.String(): false,
},
})
require.NoError(t, err)
return &db, &api.Logger, &user
}()
@@ -1275,6 +1289,20 @@ func TestNotificationTemplates_Golden(t *testing.T) {
r.Name = tc.payload.UserName
},
)
// With the introduction of notifications that can be disabled
// by default, we want to make sure the user preferences have
// the notification enabled.
_, err := adminClient.UpdateUserNotificationPreferences(
context.Background(),
user.ID,
codersdk.UpdateUserNotificationPreferences{
TemplateDisabledMap: map[string]bool{
tc.id.String(): false,
},
})
require.NoError(t, err)
return &db, &api.Logger, &user
}()
@@ -1410,6 +1438,30 @@ func normalizeGoldenWebhook(content []byte) []byte {
return content
}
func TestDisabledByDefaultBeforeEnqueue(t *testing.T) {
t.Parallel()
if !dbtestutil.WillUsePostgres() {
t.Skip("This test requires postgres; it is testing business-logic implemented in the database")
}
// nolint:gocritic // Unit test.
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
store, _ := dbtestutil.NewDB(t)
logger := testutil.Logger(t)
cfg := defaultNotificationsConfig(database.NotificationMethodSmtp)
enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal())
require.NoError(t, err)
user := createSampleUser(t, store)
// We want to try enqueuing a notification on a template that is disabled
// by default. We expect this to fail.
templateID := notifications.TemplateWorkspaceManuallyUpdated
_, err = enq.Enqueue(ctx, user.ID, templateID, map[string]string{}, "test")
require.ErrorIs(t, err, notifications.ErrCannotEnqueueDisabledNotification, "enqueuing did not fail with expected error")
}
// TestDisabledBeforeEnqueue ensures that notifications cannot be enqueued once a user has disabled that notification template
func TestDisabledBeforeEnqueue(t *testing.T) {
t.Parallel()