Skip to content

Commit 438bcba

Browse files
dannykoppingdannykopping
committed
Review comments
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent f53a8aa commit 438bcba

File tree

10 files changed

+13
-103
lines changed

10 files changed

+13
-103
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,13 +2501,6 @@ func (q *querier) InsertMissingGroups(ctx context.Context, arg database.InsertMi
25012501
return q.db.InsertMissingGroups(ctx, arg)
25022502
}
25032503

2504-
func (q *querier) InsertNotificationTemplate(ctx context.Context, arg database.InsertNotificationTemplateParams) (database.NotificationTemplate, error) {
2505-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
2506-
return database.NotificationTemplate{}, err
2507-
}
2508-
return q.db.InsertNotificationTemplate(ctx, arg)
2509-
}
2510-
25112504
func (q *querier) InsertOAuth2ProviderApp(ctx context.Context, arg database.InsertOAuth2ProviderAppParams) (database.OAuth2ProviderApp, error) {
25122505
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceOauth2App); err != nil {
25132506
return database.OAuth2ProviderApp{}, err

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2461,10 +2461,6 @@ func (s *MethodTestSuite) TestSystemFunctions() {
24612461
// TODO: update this test once we have a specific role for notifications
24622462
check.Args(database.FetchNewMessageMetadataParams{}).Asserts(rbac.ResourceSystem, policy.ActionRead)
24632463
}))
2464-
s.Run("InsertNotificationTemplate", s.Subtest(func(db database.Store, check *expects) {
2465-
// TODO: update this test once we have a specific role for notifications
2466-
check.Args(database.InsertNotificationTemplateParams{}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
2467-
}))
24682464
}
24692465

24702466
func (s *MethodTestSuite) TestOAuth2ProviderApps() {

‎coderd/database/dbmem/dbmem.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6108,14 +6108,6 @@ func (q *FakeQuerier) InsertMissingGroups(_ context.Context, arg database.Insert
61086108
return newGroups, nil
61096109
}
61106110

6111-
func (*FakeQuerier) InsertNotificationTemplate(_ context.Context, arg database.InsertNotificationTemplateParams) (database.NotificationTemplate, error) {
6112-
err := validateDatabaseType(arg)
6113-
if err != nil {
6114-
return database.NotificationTemplate{}, err
6115-
}
6116-
return database.NotificationTemplate{}, nil
6117-
}
6118-
61196111
func (q *FakeQuerier) InsertOAuth2ProviderApp(_ context.Context, arg database.InsertOAuth2ProviderAppParams) (database.OAuth2ProviderApp, error) {
61206112
err := validateDatabaseType(arg)
61216113
if err != nil {

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 0 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000219_notifications.down.sql

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
DROP TABLE IF EXISTS notification_messages;
22
DROP TABLE IF EXISTS notification_templates;
3-
DROP TYPE IF EXISTS notification_receiver;
43
DROP TYPE IF EXISTS notification_method;
54
DROP TYPE IF EXISTS notification_message_status;

‎coderd/database/migrations/testdata/fixtures/000219_notifications.up.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ $$
1010
('b0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12', 'B', template, template, 'Group 1'),
1111
('c0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13', 'C', template, template, 'Group 2');
1212

13-
INSERT INTO public.users(id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, deleted)
13+
INSERT INTO users(id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, deleted)
1414
VALUES ('fc1511ef-4fcf-4a3b-98a1-8df64160e35a', 'githubuser@coder.com', 'githubuser', '\x', '2022-11-02 13:05:21.445455+02', '2022-11-02 13:05:21.445455+02', 'active', '{}', false) ON CONFLICT DO NOTHING;
1515

1616
INSERT INTO notification_messages (id, notification_template_id, user_id, method, created_by, payload)

‎coderd/database/querier.go

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 6 additions & 44 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/notifications.sql

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
-- name: InsertNotificationTemplate :one
2-
INSERT INTO notification_templates (id, name, title_template, body_template, "group")
3-
VALUES ($1, $2, $3, $4, $5)
4-
RETURNING *;
5-
61
-- name: FetchNewMessageMetadata :one
72
-- This is used to build up the notification_message's JSON payload.
83
SELECT nt.name AS notification_name,
@@ -47,7 +42,6 @@ WITH acquired AS (
4742
leased_until = NOW() + CONCAT(sqlc.arg('lease_seconds')::int, ' seconds')::interval
4843
WHERE id IN (SELECT nm.id
4944
FROM notification_messages AS nm
50-
LEFT JOIN notification_templates AS nt ON (nm.notification_template_id = nt.id)
5145
WHERE (
5246
(
5347
-- message is in acquirable states
@@ -113,18 +107,17 @@ FROM (SELECT id, status, status_reason, failed_at
113107
WHERE notification_messages.id = subquery.id;
114108

115109
-- name: BulkMarkNotificationMessagesSent :execrows
116-
WITH new_values AS (SELECT UNNEST(@ids::uuid[]) AS id,
117-
UNNEST(@sent_ats::timestamptz[]) AS sent_at)
118110
UPDATE notification_messages
119-
SET updated_at = subquery.sent_at,
111+
SET updated_at = new_values.sent_at,
120112
attempt_count = attempt_count + 1,
121113
status = 'sent'::notification_message_status,
122114
status_reason = NULL,
123115
leased_until = NULL,
124116
next_retry_after = NULL
125-
FROM (SELECT id, sent_at
126-
FROM new_values) AS subquery
127-
WHERE notification_messages.id = subquery.id;
117+
FROM (SELECT UNNEST(@ids::uuid[]) AS id,
118+
UNNEST(@sent_ats::timestamptz[]) AS sent_at)
119+
AS new_values
120+
WHERE notification_messages.id = new_values.id;
128121

129122
-- Delete all notification messages which have not been updated for over a week.
130123
-- name: DeleteOldNotificationMessages :exec
@@ -133,7 +126,5 @@ FROM notification_messages
133126
WHERE id IN
134127
(SELECT id
135128
FROM notification_messages AS nested
136-
WHERE nested.updated_at < NOW() - INTERVAL '7 days'
137-
-- ensure we don't clash with the notifier
138-
FOR UPDATE SKIP LOCKED);
129+
WHERE nested.updated_at < NOW() - INTERVAL '7 days');
139130

0 commit comments

Comments
 (0)