-
Notifications
You must be signed in to change notification settings - Fork 901
feat(coderd): add inbox notifications endpoints #16889
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 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
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
3c6512b
add endpoints for inbox notifications
defelmnq c8ccc60
cleanup comments and errors
defelmnq 18b694b
skip tests on windows
defelmnq 0e8ac4c
work on pr comments - cleanup endpoints and isolate logic
defelmnq d72d1f2
work on PR comments
defelmnq 796bcd0
fix parameters validation
defelmnq 75c310d
improve parameters validation
defelmnq 07ab7c4
fmt and lint
defelmnq cb41d1a
websocket testing wip
defelmnq 6ff4c7e
improve tests
defelmnq 1ebc7f4
make fmt and lint
defelmnq 736a2d7
make fmt and lint
defelmnq c28002e
remove windows from watch handler tests
defelmnq 2637d86
fix comments
defelmnq 4d0a561
fix tests
defelmnq 1f18868
fix ci lint
defelmnq 4f01a86
fix ci lint
defelmnq cf6af1b
fix ci lint
defelmnq File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
work on pr comments - cleanup endpoints and isolate logic
- Loading branch information
commit 0e8ac4c0823bcc7f3672b2af2bd7143db481ff68
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import ( | |
|
||
"github.com/go-chi/chi/v5" | ||
"github.com/google/uuid" | ||
"golang.org/x/xerrors" | ||
|
||
"cdr.dev/slog" | ||
|
||
|
@@ -24,39 +25,16 @@ import ( | |
"github.com/coder/websocket" | ||
) | ||
|
||
// watchInboxNotifications watches for new inbox notifications and sends them to the client. | ||
// The client can specify a list of target IDs to filter the notifications. | ||
// @Summary Watch for new inbox notifications | ||
// @ID watch-for-new-inbox-notifications | ||
// @Security CoderSessionToken | ||
// @Produce json | ||
// @Tags Notifications | ||
// @Param targets query string false "Comma-separated list of target IDs to filter notifications" | ||
// @Param templates query string false "Comma-separated list of template IDs to filter notifications" | ||
// @Param read_status query string false "Filter notifications by read status. Possible values: read, unread, all" | ||
// @Success 200 {object} codersdk.GetInboxNotificationResponse | ||
// @Router /notifications/inbox/watch [get] | ||
func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) { | ||
ctx := r.Context() | ||
|
||
var ( | ||
apikey = httpmw.APIKey(r) | ||
targetsParam = r.URL.Query().Get("targets") | ||
templatesParam = r.URL.Query().Get("templates") | ||
readStatusParam = r.URL.Query().Get("read_status") | ||
) | ||
|
||
// convertInboxNotificationParameters parses and validates the common parameters used in get and list endpoints for inbox notifications | ||
func convertInboxNotificationParameters(ctx context.Context, logger slog.Logger, targetsParam string, templatesParam string, readStatusParam string) ([]uuid.UUID, []uuid.UUID, string, error) { | ||
var targets []uuid.UUID | ||
if targetsParam != "" { | ||
splitTargets := strings.Split(targetsParam, ",") | ||
for _, target := range splitTargets { | ||
id, err := uuid.Parse(target) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid target ID.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
logger.Error(ctx, "unable to parse target id", slog.Error(err)) | ||
return nil, nil, "", xerrors.New("unable to parse target id") | ||
} | ||
targets = append(targets, id) | ||
} | ||
|
@@ -68,11 +46,8 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) | |
for _, template := range splitTemplates { | ||
id, err := uuid.Parse(template) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid template ID.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
logger.Error(ctx, "unable to parse template id", slog.Error(err)) | ||
return nil, nil, "", xerrors.New("unable to parse template id") | ||
} | ||
templates = append(templates, id) | ||
} | ||
|
@@ -86,13 +61,73 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) | |
} | ||
|
||
if !slices.Contains(readOptions, readStatusParam) { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid read status.", | ||
}) | ||
return | ||
logger.Error(ctx, "unable to parse read status") | ||
return nil, nil, "", xerrors.New("unable to parse read status") | ||
} | ||
} | ||
|
||
return targets, templates, readStatusParam, nil | ||
} | ||
|
||
// convertInboxNotificationResponse works as a util function to transform a database.InboxNotification to codersdk.InboxNotification | ||
func convertInboxNotificationResponse(ctx context.Context, logger slog.Logger, notif database.InboxNotification) codersdk.InboxNotification { | ||
return codersdk.InboxNotification{ | ||
ID: notif.ID, | ||
UserID: notif.UserID, | ||
TemplateID: notif.TemplateID, | ||
Targets: notif.Targets, | ||
Title: notif.Title, | ||
Content: notif.Content, | ||
Icon: notif.Icon, | ||
Actions: func() []codersdk.InboxNotificationAction { | ||
var actionsList []codersdk.InboxNotificationAction | ||
err := json.Unmarshal([]byte(notif.Actions), &actionsList) | ||
if err != nil { | ||
logger.Error(ctx, "unmarshal inbox notification actions", slog.Error(err)) | ||
dannykopping marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
return actionsList | ||
}(), | ||
ReadAt: func() *time.Time { | ||
if !notif.ReadAt.Valid { | ||
return nil | ||
} | ||
return ¬if.ReadAt.Time | ||
}(), | ||
CreatedAt: notif.CreatedAt, | ||
} | ||
} | ||
|
||
// watchInboxNotifications watches for new inbox notifications and sends them to the client. | ||
// The client can specify a list of target IDs to filter the notifications. | ||
// @Summary Watch for new inbox notifications | ||
// @ID watch-for-new-inbox-notifications | ||
// @Security CoderSessionToken | ||
// @Produce json | ||
// @Tags Notifications | ||
// @Param targets query string false "Comma-separated list of target IDs to filter notifications" | ||
// @Param templates query string false "Comma-separated list of template IDs to filter notifications" | ||
// @Param read_status query string false "Filter notifications by read status. Possible values: read, unread, all" | ||
// @Success 200 {object} codersdk.GetInboxNotificationResponse | ||
// @Router /notifications/inbox/watch [get] | ||
func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I bet you want to write some tests for this API endpoint 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some tests to cover most of the endpoint. ✅ |
||
ctx := r.Context() | ||
|
||
var ( | ||
apikey = httpmw.APIKey(r) | ||
targetsParam = r.URL.Query().Get("targets") | ||
templatesParam = r.URL.Query().Get("templates") | ||
readStatusParam = r.URL.Query().Get("read_status") | ||
) | ||
|
||
targets, templates, readStatusParam, err := convertInboxNotificationParameters(ctx, api.Logger, targetsParam, templatesParam, readStatusParam) | ||
dannykopping marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid query parameter.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
} | ||
|
||
conn, err := websocket.Accept(rw, r, nil) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ | ||
|
@@ -200,53 +235,13 @@ func (api *API) listInboxNotifications(rw http.ResponseWriter, r *http.Request) | |
startingBeforeParam = r.URL.Query().Get("starting_before") | ||
) | ||
|
||
var targets []uuid.UUID | ||
if targetsParam != "" { | ||
splitTargets := strings.Split(targetsParam, ",") | ||
for _, target := range splitTargets { | ||
id, err := uuid.Parse(target) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid target ID.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
} | ||
targets = append(targets, id) | ||
} | ||
} | ||
|
||
var templates []uuid.UUID | ||
if templatesParam != "" { | ||
splitTemplates := strings.Split(templatesParam, ",") | ||
for _, template := range splitTemplates { | ||
id, err := uuid.Parse(template) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid template ID.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
} | ||
templates = append(templates, id) | ||
} | ||
} | ||
|
||
readStatus := database.InboxNotificationReadStatusAll | ||
if readStatusParam != "" { | ||
readOptions := []string{ | ||
string(database.InboxNotificationReadStatusRead), | ||
string(database.InboxNotificationReadStatusUnread), | ||
string(database.InboxNotificationReadStatusAll), | ||
} | ||
|
||
if !slices.Contains(readOptions, readStatusParam) { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid read status.", | ||
}) | ||
return | ||
} | ||
readStatus = database.InboxNotificationReadStatus(readStatusParam) | ||
targets, templates, readStatus, err := convertInboxNotificationParameters(ctx, api.Logger, targetsParam, templatesParam, readStatusParam) | ||
if err != nil { | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Invalid query parameter.", | ||
Detail: err.Error(), | ||
}) | ||
return | ||
} | ||
|
||
startingBefore := dbtime.Now() | ||
|
@@ -272,7 +267,7 @@ func (api *API) listInboxNotifications(rw http.ResponseWriter, r *http.Request) | |
UserID: apikey.UserID, | ||
Templates: templates, | ||
Targets: targets, | ||
ReadStatus: readStatus, | ||
ReadStatus: database.InboxNotificationReadStatus(readStatus), | ||
CreatedAtOpt: startingBefore, | ||
}) | ||
if err != nil { | ||
|
@@ -296,30 +291,7 @@ func (api *API) listInboxNotifications(rw http.ResponseWriter, r *http.Request) | |
Notifications: func() []codersdk.InboxNotification { | ||
notificationsList := make([]codersdk.InboxNotification, 0, len(notifs)) | ||
for _, notification := range notifs { | ||
notificationsList = append(notificationsList, codersdk.InboxNotification{ | ||
ID: notification.ID, | ||
UserID: notification.UserID, | ||
TemplateID: notification.TemplateID, | ||
Targets: notification.Targets, | ||
Title: notification.Title, | ||
Content: notification.Content, | ||
Icon: notification.Icon, | ||
Actions: func() []codersdk.InboxNotificationAction { | ||
var actionsList []codersdk.InboxNotificationAction | ||
err := json.Unmarshal([]byte(notification.Actions), &actionsList) | ||
if err != nil { | ||
api.Logger.Error(ctx, "unmarshal inbox notification actions", slog.Error(err)) | ||
} | ||
return actionsList | ||
}(), | ||
ReadAt: func() *time.Time { | ||
if !notification.ReadAt.Valid { | ||
return nil | ||
} | ||
return ¬ification.ReadAt.Time | ||
}(), | ||
CreatedAt: notification.CreatedAt, | ||
}) | ||
notificationsList = append(notificationsList, convertInboxNotificationResponse(ctx, api.Logger, notification)) | ||
} | ||
return notificationsList | ||
}(), | ||
|
@@ -352,7 +324,7 @@ func (api *API) updateInboxNotificationReadStatus(rw http.ResponseWriter, r *htt | |
parsedNotifID, err := uuid.Parse(notifID) | ||
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
api.Logger.Error(ctx, "failed to parse notification uuid", slog.Error(err)) | ||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ | ||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
Message: "Failed to parse notification uuid.", | ||
}) | ||
return | ||
|
@@ -398,30 +370,7 @@ func (api *API) updateInboxNotificationReadStatus(rw http.ResponseWriter, r *htt | |
} | ||
|
||
httpapi.Write(ctx, rw, http.StatusOK, codersdk.UpdateInboxNotificationReadStatusResponse{ | ||
Notification: codersdk.InboxNotification{ | ||
ID: updatedNotification.ID, | ||
UserID: updatedNotification.UserID, | ||
TemplateID: updatedNotification.TemplateID, | ||
Targets: updatedNotification.Targets, | ||
Title: updatedNotification.Title, | ||
Content: updatedNotification.Content, | ||
Icon: updatedNotification.Icon, | ||
Actions: func() []codersdk.InboxNotificationAction { | ||
var actionsList []codersdk.InboxNotificationAction | ||
err := json.Unmarshal([]byte(updatedNotification.Actions), &actionsList) | ||
if err != nil { | ||
api.Logger.Error(ctx, "unmarshal inbox notification actions", slog.Error(err)) | ||
} | ||
return actionsList | ||
}(), | ||
ReadAt: func() *time.Time { | ||
if !updatedNotification.ReadAt.Valid { | ||
return nil | ||
} | ||
return &updatedNotification.ReadAt.Time | ||
}(), | ||
CreatedAt: updatedNotification.CreatedAt, | ||
}, | ||
UnreadCount: int(unreadCount), | ||
Notification: convertInboxNotificationResponse(ctx, api.Logger, updatedNotification), | ||
UnreadCount: int(unreadCount), | ||
}) | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a general comment - if we want to use inbox just for notifications, we should adjust filenames and function names to indicate the relationship:
notificationsinbox.go
orconvertNotificationsInboxParameters
.