Skip to content

feat: notify about created user account #14010

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.

Sign up for GitHub

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

Merged
merged 26 commits into from
Jul 30, 2024
Merged

feat: notify about created user account #14010

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c18ada0
feat: notify about created user account
mtojek Jul 25, 2024
1cc3adb
Merge branch 'main' into 17-account-notifs
mtojek Jul 26, 2024
ca2bdde
migrations
mtojek Jul 26, 2024
d6e8964
Username
mtojek Jul 26, 2024
2d74c4e
events
mtojek Jul 26, 2024
3c3b5af
fix
mtojek Jul 26, 2024
312d9fa
WIP
mtojek Jul 26, 2024
82ec37f
fix version
mtojek Jul 26, 2024
d7b2c73
simplify
mtojek Jul 29, 2024
b81bb6a
WIP
mtojek Jul 29, 2024
00275dd
fix: versions
mtojek Jul 29, 2024
e356ba8
test
mtojek Jul 29, 2024
6bc1d2d
fix test
mtojek Jul 29, 2024
67a5137
users notified
mtojek Jul 29, 2024
6431a0e
Merge branch 'main' into 17-account-notifs
mtojek Jul 29, 2024
42d9ba1
post merge
mtojek Jul 29, 2024
ecc7d30
fmt
mtojek Jul 29, 2024
b2dcb3b
skip notif
mtojek Jul 29, 2024
70e2d2c
Danny's feedback
mtojek Jul 30, 2024
8696b70
given when then
mtojek Jul 30, 2024
a92d059
Fix tests
mtojek Jul 30, 2024
b5b0d90
Skip notifications
mtojek Jul 30, 2024
2de1786
Merge branch 'main' into 17-account-notifs
mtojek Jul 30, 2024
b843232
fix: created_account_name
mtojek Jul 30, 2024
2a53a0b
api.Database -> tx
mtojek Jul 30, 2024
77c0c28
pq issue
mtojek Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix tests
  • Loading branch information
@mtojek
mtojek committed Jul 30, 2024
commit a92d059fe57f62bee59dc521594f282cedded980
15 changes: 8 additions & 7 deletions coderd/autobuild/lifecycle_executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,13 +1115,14 @@ func TestNotifications(t *testing.T) {
require.NotNil(t, workspace.DormantAt)

// Check that a notification was enqueued
require.Len(t, notifyEnq.Sent, 1)
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
require.Len(t, notifyEnq.Sent, 2)
// notifyEnq.Sent[0] is an event for created user account
require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID)
require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant)
require.Contains(t, notifyEnq.Sent[1].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID)
})
}

Expand Down
15 changes: 8 additions & 7 deletions coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3476,13 +3476,14 @@ func TestNotifications(t *testing.T) {

// Then
require.NoError(t, err, "mark workspace as dormant")
require.Len(t, notifyEnq.Sent, 1)
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
require.Len(t, notifyEnq.Sent, 2)
// notifyEnq.Sent[0] is an event for created user account
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: would be safer to validate this

Copy link
Member Author

@mtojek mtojek Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it like this on purpose, this is not the aim of this test. Less chance that it will require refining if we alter something in Notifications Enqueuer logic

require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant)
require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID)
require.Contains(t, notifyEnq.Sent[1].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID)
})

t.Run("InitiatorIsOwner", func(t *testing.T) {
Expand Down