NotificationsYou must be signed in to change notification settings
Fork928
Star10.1k

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mtojek merged 26 commits intomainfrom17-account-notifs

Jul 30, 2024

Merged

feat: notify about created user account#14010

mtojek merged 26 commits intomainfrom17-account-notifs

Jul 30, 2024

Conversation

Copy link

Member

mtojek commentedJul 25, 2024•
edited
Loading

Related:coder/internal#17

This PR extendsapi.CreateUser to notify owners and user admins when somebody manually creates a new user.

feat: notify about created user account

c18ada0

mtojek self-assigned this

Jul 25, 2024

mtojek added16 commits

July 26, 2024 10:12

Merge branch 'main' into 17-account-notifs

1cc3adb

migrations

ca2bdde

Username

d6e8964

events

2d74c4e

fix

3c3b5af

WIP

312d9fa

fix version

82ec37f

simplify

d7b2c73

WIP

b81bb6a

fix: versions

00275dd

test

e356ba8

fix test

6bc1d2d

users notified

67a5137

Merge branch 'main' into 17-account-notifs

6431a0e

post merge

42d9ba1

fmt

ecc7d30

mtojek marked this pull request as ready for review

July 29, 2024 14:35

mtojek requested a review fromdannykopping

July 29, 2024 14:35

skip notif

b2dcb3b

dannykopping reviewed

Jul 29, 2024

View reviewed changes

Copy link

Contributor

dannykopping left a comment

Choose a reason for hiding this comment

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

Looking good! Mostly small comments and nits.

coderd/notifications/events.go OutdatedShow resolvedHide resolved

coderd/users.go OutdatedShow resolvedHide resolved

coderd/users_test.goShow resolvedHide resolved

enterprise/coderd/scim.goShow resolvedHide resolved

Danny's feedback

70e2d2c

Copy link

alwaysmeticulousbot commentedJul 30, 2024•
edited
Loading

🤖 Meticulous spotted visual differences in 11 of 1069 screens tested:view and approve differences detected.

_{Last updated for commit 77c0c28. This comment will update as new commits are pushed.}

mtojek added3 commits

July 30, 2024 11:41

given when then

8696b70

Fix tests

a92d059

Skip notifications

b5b0d90

mtojek mentioned this pull request

Jul 30, 2024

feat: account notificationscoder/internal#17

Closed

5 tasks

mtojek added2 commits

July 30, 2024 13:10

Merge branch 'main' into 17-account-notifs

2de1786

fix: created_account_name

b843232

mtojek requested a review fromdannykopping

July 30, 2024 12:06

dannykopping approved these changes

Jul 30, 2024

View reviewed changes

Copy link

Contributor

dannykopping left a comment

Choose a reason for hiding this comment

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

LGTM, with non-blocking nit 👍

coderd/workspaces_test.go

		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

dannykoppingJul 30, 2024

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

MemberAuthor

mtojekJul 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

mtojek added2 commits

July 30, 2024 15:16

api.Database -> tx

2a53a0b

pq issue

77c0c28

mtojek merged commitcf1fcab intomain

Jul 30, 2024

29 of 30 checks passed

mtojek deleted the 17-account-notifs branch

July 30, 2024 13:37

github-actionsbot locked and limited conversation to collaborators

Jul 30, 2024

Labels

None yet

2 participants

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: notify about created user account#14010

feat: notify about created user account#14010

Uh oh!

Conversation

mtojek commentedJul 25, 2024•
edited
Loading

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alwaysmeticulousbot commentedJul 30, 2024•
edited
Loading

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

dannykoppingJul 30, 2024

Choose a reason for hiding this comment

Uh oh!

mtojekJul 30, 2024•
edited
Loading

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Movatterモバイル変換

feat: notify about created user account#14010

feat: notify about created user account#14010

Uh oh!

Conversation

mtojek commentedJul 25, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alwaysmeticulousbot commentedJul 30, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

dannykoppingJul 30, 2024

Choose a reason for hiding this comment

Uh oh!

mtojekJul 30, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mtojek commentedJul 25, 2024•
edited
Loading

alwaysmeticulousbot commentedJul 30, 2024•
edited
Loading

mtojekJul 30, 2024•
edited
Loading