From c1d3084f908d7b4f98158c4eb0e815a8121ece31 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 Jul 2024 11:24:46 +0200 Subject: [PATCH 1/4] fix: notifications: use username in workspace URLs --- coderd/database/dbmem/dbmem.go | 9 ++------- coderd/database/queries.sql.go | 11 +++++++---- coderd/database/queries/notifications.sql | 3 ++- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index cf1773f637a02..09a3be6e877a4 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1915,12 +1915,6 @@ func (q *FakeQuerier) FetchNewMessageMetadata(_ context.Context, arg database.Fe return database.FetchNewMessageMetadataRow{}, xerrors.Errorf("fetch user: %w", err) } - // Mimic COALESCE in query - userName := user.Name - if userName == "" { - userName = user.Username - } - actions, err := json.Marshal([]types.TemplateAction{{URL: "http://xyz.com", Label: "XYZ"}}) if err != nil { return database.FetchNewMessageMetadataRow{}, err @@ -1928,7 +1922,8 @@ func (q *FakeQuerier) FetchNewMessageMetadata(_ context.Context, arg database.Fe return database.FetchNewMessageMetadataRow{ UserEmail: user.Email, - UserName: userName, + UserName: user.Name, + UserUsername: user.Username, NotificationName: "Some notification", Actions: actions, UserID: arg.UserID, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d67448fa09b47..bef3336a3e25b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3089,7 +3089,7 @@ func (q *sqlQuerier) GetJFrogXrayScanByWorkspaceAndAgentID(ctx context.Context, } const upsertJFrogXrayScanByWorkspaceAndAgentID = `-- name: UpsertJFrogXrayScanByWorkspaceAndAgentID :exec -INSERT INTO +INSERT INTO jfrog_xray_scans ( agent_id, workspace_id, @@ -3098,7 +3098,7 @@ INSERT INTO medium, results_url ) -VALUES +VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (agent_id, workspace_id) DO UPDATE SET critical = $3, high = $4, medium = $5, results_url = $6 @@ -3546,7 +3546,8 @@ SELECT nt.name AS notificatio nt.actions AS actions, u.id AS user_id, u.email AS user_email, - COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name + COALESCE(u.name, '') AS user_name, + COALESCE(u.username, '') AS user_username FROM notification_templates nt, users u WHERE nt.id = $1 @@ -3564,6 +3565,7 @@ type FetchNewMessageMetadataRow struct { UserID uuid.UUID `db:"user_id" json:"user_id"` UserEmail string `db:"user_email" json:"user_email"` UserName string `db:"user_name" json:"user_name"` + UserUsername string `db:"user_username" json:"user_username"` } // This is used to build up the notification_message's JSON payload. @@ -3576,6 +3578,7 @@ func (q *sqlQuerier) FetchNewMessageMetadata(ctx context.Context, arg FetchNewMe &i.UserID, &i.UserEmail, &i.UserName, + &i.UserUsername, ) return i, err } @@ -5557,7 +5560,7 @@ FROM provisioner_keys WHERE organization_id = $1 -AND +AND lower(name) = lower($2) ` diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index edbb0fd6f5d58..b4bc255caa4b1 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -4,7 +4,8 @@ SELECT nt.name AS notificatio nt.actions AS actions, u.id AS user_id, u.email AS user_email, - COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name + COALESCE(u.name, '') AS user_name, + COALESCE(u.username, '') AS user_username FROM notification_templates nt, users u WHERE nt.id = @notification_template_id From ba9cd2fd1b0b481bafeb93ef03af8179afb43d87 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 Jul 2024 11:31:03 +0200 Subject: [PATCH 2/4] migrations --- .../migrations/000230_notifications_fix_username.down.sql | 3 +++ .../migrations/000230_notifications_fix_username.up.sql | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 coderd/database/migrations/000230_notifications_fix_username.down.sql create mode 100644 coderd/database/migrations/000230_notifications_fix_username.up.sql diff --git a/coderd/database/migrations/000230_notifications_fix_username.down.sql b/coderd/database/migrations/000230_notifications_fix_username.down.sql new file mode 100644 index 0000000000000..4c3e7dda9b03d --- /dev/null +++ b/coderd/database/migrations/000230_notifications_fix_username.down.sql @@ -0,0 +1,3 @@ +UPDATE notification_templates +SET + actions = REPLACE(actions::text, '@{{.UserUsername}}', '@{{.UserName}}')::jsonb; diff --git a/coderd/database/migrations/000230_notifications_fix_username.up.sql b/coderd/database/migrations/000230_notifications_fix_username.up.sql new file mode 100644 index 0000000000000..bfd01ae3c8637 --- /dev/null +++ b/coderd/database/migrations/000230_notifications_fix_username.up.sql @@ -0,0 +1,3 @@ +UPDATE notification_templates +SET + actions = REPLACE(actions::text, '@{{.UserName}}', '@{{.UserUsername}}')::jsonb; From 0813f69d8d7e59a7ff8eed662ba1a20a68407c52 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 Jul 2024 11:44:54 +0200 Subject: [PATCH 3/4] fix: nullif --- coderd/database/dbmem/dbmem.go | 8 +++++++- coderd/database/queries.sql.go | 8 ++++---- coderd/database/queries/notifications.sql | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 09a3be6e877a4..f74197e234fa8 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1915,6 +1915,12 @@ func (q *FakeQuerier) FetchNewMessageMetadata(_ context.Context, arg database.Fe return database.FetchNewMessageMetadataRow{}, xerrors.Errorf("fetch user: %w", err) } + // Mimic COALESCE in query + userName := user.Name + if userName == "" { + userName = user.Username + } + actions, err := json.Marshal([]types.TemplateAction{{URL: "http://xyz.com", Label: "XYZ"}}) if err != nil { return database.FetchNewMessageMetadataRow{}, err @@ -1922,7 +1928,7 @@ func (q *FakeQuerier) FetchNewMessageMetadata(_ context.Context, arg database.Fe return database.FetchNewMessageMetadataRow{ UserEmail: user.Email, - UserName: user.Name, + UserName: userName, UserUsername: user.Username, NotificationName: "Some notification", Actions: actions, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index bef3336a3e25b..b761e451b3041 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3089,7 +3089,7 @@ func (q *sqlQuerier) GetJFrogXrayScanByWorkspaceAndAgentID(ctx context.Context, } const upsertJFrogXrayScanByWorkspaceAndAgentID = `-- name: UpsertJFrogXrayScanByWorkspaceAndAgentID :exec -INSERT INTO +INSERT INTO jfrog_xray_scans ( agent_id, workspace_id, @@ -3098,7 +3098,7 @@ INSERT INTO medium, results_url ) -VALUES +VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (agent_id, workspace_id) DO UPDATE SET critical = $3, high = $4, medium = $5, results_url = $6 @@ -3546,7 +3546,7 @@ SELECT nt.name AS notificatio nt.actions AS actions, u.id AS user_id, u.email AS user_email, - COALESCE(u.name, '') AS user_name, + COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name, COALESCE(u.username, '') AS user_username FROM notification_templates nt, users u @@ -5560,7 +5560,7 @@ FROM provisioner_keys WHERE organization_id = $1 -AND +AND lower(name) = lower($2) ` diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index b4bc255caa4b1..2fd372e9df029 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -4,7 +4,7 @@ SELECT nt.name AS notificatio nt.actions AS actions, u.id AS user_id, u.email AS user_email, - COALESCE(u.name, '') AS user_name, + COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name, COALESCE(u.username, '') AS user_username FROM notification_templates nt, users u From 909c03e6be6c36fa7485a55bcda349e68df84c52 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 Jul 2024 11:50:34 +0200 Subject: [PATCH 4/4] fix: message payload --- coderd/notifications/enqueuer.go | 7 ++++--- coderd/notifications/notifications_test.go | 8 +++++--- coderd/notifications/render/gotmpl_test.go | 5 +++-- coderd/notifications/types/payload.go | 7 ++++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/coderd/notifications/enqueuer.go b/coderd/notifications/enqueuer.go index d73826142f7ca..32822dd6ab9d7 100644 --- a/coderd/notifications/enqueuer.go +++ b/coderd/notifications/enqueuer.go @@ -94,9 +94,10 @@ func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID, templateID uui NotificationName: metadata.NotificationName, - UserID: metadata.UserID.String(), - UserEmail: metadata.UserEmail, - UserName: metadata.UserName, + UserID: metadata.UserID.String(), + UserEmail: metadata.UserEmail, + UserName: metadata.UserName, + UserUsername: metadata.UserUsername, Labels: labels, // No actions yet diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index 481244bf21f2a..7d55ac01c5b52 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -201,12 +201,13 @@ func TestWebhookDispatch(t *testing.T) { require.NoError(t, err) const ( - email = "bob@coder.com" - name = "Robert McBobbington" + email = "bob@coder.com" + name = "Robert McBobbington" + username = "bob" ) user := dbgen.User(t, db, database.User{ Email: email, - Username: "bob", + Username: username, Name: name, }) @@ -229,6 +230,7 @@ func TestWebhookDispatch(t *testing.T) { // UserName is coalesced from `name` and `username`; in this case `name` wins. // This is not strictly necessary for this test, but it's testing some side logic which is too small for its own test. require.Equal(t, payload.Payload.UserName, name) + require.Equal(t, payload.Payload.UserUsername, username) // Right now we don't have a way to query notification templates by ID in dbmem, and it's not necessary to add this // just to satisfy this test. We can safely assume that as long as this value is not empty that the given value was delivered. require.NotEmpty(t, payload.Payload.NotificationName) diff --git a/coderd/notifications/render/gotmpl_test.go b/coderd/notifications/render/gotmpl_test.go index 0cb95bccfcb43..ec2ec7ffe6237 100644 --- a/coderd/notifications/render/gotmpl_test.go +++ b/coderd/notifications/render/gotmpl_test.go @@ -42,10 +42,11 @@ func TestGoTemplate(t *testing.T) { name: "render workspace URL", in: `[{ "label": "View workspace", - "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + "url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}" }]`, payload: types.MessagePayload{ - UserName: "johndoe", + UserName: "John Doe", + UserUsername: "johndoe", Labels: map[string]string{ "name": "my-workspace", }, diff --git a/coderd/notifications/types/payload.go b/coderd/notifications/types/payload.go index f6b18215e5357..ba666219af654 100644 --- a/coderd/notifications/types/payload.go +++ b/coderd/notifications/types/payload.go @@ -9,9 +9,10 @@ type MessagePayload struct { NotificationName string `json:"notification_name"` - UserID string `json:"user_id"` - UserEmail string `json:"user_email"` - UserName string `json:"user_name"` + UserID string `json:"user_id"` + UserEmail string `json:"user_email"` + UserName string `json:"user_name"` + UserUsername string `json:"user_username"` Actions []TemplateAction `json:"actions"` Labels map[string]string `json:"labels"`