8000 feat(coderd): add inbox notifications endpoints by defelmnq · Pull Request #16889 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

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.

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 18 commits into from
Mar 17, 2025
Merged

feat(coderd): add inbox notifications endpoints #16889

Show file tree
Hide file tree
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 Mar 12, 2025
c8ccc60
cleanup comments and errors
defelmnq Mar 12, 2025
18b694b
skip tests on windows
defelmnq Mar 12, 2025
0e8ac4c
work on pr comments - cleanup endpoints and isolate logic
defelmnq Mar 13, 2025
d72d1f2
work on PR comments
defelmnq Mar 13, 2025
796bcd0
fix parameters validation
defelmnq Mar 13, 2025
75c310d
improve parameters validation
defelmnq Mar 13, 2025
07ab7c4
fmt and lint
defelmnq Mar 13, 2025
cb41d1a
websocket testing wip
defelmnq Mar 14, 2025
6ff4c7e
improve tests
defelmnq Mar 14, 2025
1ebc7f4
make fmt and lint
defelmnq Mar 14, 2025
736a2d7
make fmt and lint
defelmnq Mar 14, 2025
c28002e
remove windows from watch handler tests
defelmnq Mar 17, 2025
2637d86
fix comments
defelmnq Mar 17, 2025
4d0a561
fix tests
defelmnq Mar 17, 2025
1f18868
fix ci lint
defelmnq Mar 17, 2025
4f01a86
fix ci lint
defelmnq Mar 17, 2025
cf6af1b
fix ci lint
defelmnq Mar 17, 2025
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
cleanup comments and errors
  • Loading branch information
@defelmnq
defelmnq committed Mar 12, 2025
commit c8ccc6098456746426c2b62d6ce01c0ced9ea583
24 changes: 12 additions & 12 deletions coderd/inboxnotifications.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request)
case notif := <-notificationCh:
unreadCount, err := api.Database.CountUnreadInboxNotificationsByUserID(ctx, apikey.UserID)
if err != nil {
api.Logger.Error(ctx, "count unread inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to count unread inbox notifications", slog.Error(err))
return
}
if err := encoder.Encode(codersdk.GetInboxNotificationResponse{
Expand Down Expand Up @@ -261,7 +261,7 @@ func (api *API) listInboxNotifications(rw http.ResponseWriter, r *http.Request)
lastNotif, err := api.Database.GetInboxNotificationByID(ctx, lastNotifID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid starting before.",
Message: "Failed to get notification by id.",
})
return
}
Expand All @@ -276,16 +276,16 @@ func (api *API) listInboxNotifications(rw http.ResponseWriter, r *http.Request)
CreatedAtOpt: startingBefore,
})
if err != nil {
api.Logger.Error(ctx, "get filtered inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to get filtered inbox notifications", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to get inbox notifications.",
Message: "Failed to get filtered inbox notifications.",
})
return
}

unreadCount, err := api.Database.CountUnreadInboxNotificationsByUserID(ctx, apikey.UserID)
if err != nil {
api.Logger.Error(ctx, "count unread inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to count unread inbox notifications", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to count unread inbox notifications.",
})
Expand Down Expand Up @@ -351,7 +351,7 @@ func (api *API) updateInboxNotificationReadStatus(rw http.ResponseWriter, r *htt

parsedNotifID, err := uuid.Parse(notifID)
if err != nil {
api.Logger.Error(ctx, "failed to parse uuid", slog.Error(err))
api.Logger.Error(ctx, "failed to parse notification uuid", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Copy link
Member

Choose a reason for hiding this comment

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

This is probably more of a 4xx error (bad request?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah you're right ✅
Changed to 400 Bad Request to align with the other endpoints.

Message: "Failed to parse notification uuid.",
})
Expand All @@ -372,27 +372,27 @@ func (api *API) updateInboxNotificationReadStatus(rw http.ResponseWriter, r *htt
}(),
})
if err != nil {
api.Logger.Error(ctx, "get filtered inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to update inbox notification read status", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to get inbox notifications.",
Message: "Failed to update inbox notification read status.",
})
return
}

unreadCount, err := api.Database.CountUnreadInboxNotificationsByUserID(ctx, apikey.UserID)
if err != nil {
api.Logger.Error(ctx, "count unread inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to call count unread inbox notifications", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to count unread inbox notifications.",
Message: "Failed to call count unread inbox notifications.",
})
return
}

updatedNotification, err := api.Database.GetInboxNotificationByID(ctx, parsedNotifID)
if err != nil {
api.Logger.Error(ctx, "count unread inbox notifications", slog.Error(err))
api.Logger.Error(ctx, "failed to get notification by id", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to count unread inbox notifications.",
Message: "Failed to get notification by id.",
})
return
}
Expand Down
47 changes: 20 additions & 27 deletions coderd/inboxnotifications_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ package coderd_test

import (
"context"
"database/sql"
"encoding/json"
"fmt"
"runtime"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -43,6 +42,13 @@ func TestInboxNotifications_List(t *testing.T) {
t.Run("OK with pagination", func(t *testing.T) {
t.Parallel()

// I skip these tests specifically on windows as for now they are flaky - only on Windows.
// For now the idea is that the runner takes too long to insert the entries, could be worth
// investigating a manual Tx.
if runtime.GOOS == "windows" {
t.Skip("our runners are randomly taking too long to insert entries")
}

client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{})
firstUser := coderdtest.CreateFirstUser(t, client)

Expand All @@ -58,23 +64,18 @@ func TestInboxNotifications_List(t *testing.T) {
// nolint:gocritic // used only to seed database
notifierCtx := dbauthz.AsNotifier(ctx)

api.Database.InTx(func(tx database.Store) error {
for i := range 40 {
_, err = api.Database.InsertInboxNotification(notifierCtx, database.InsertInboxNotificationParams{
ID: uuid.New(),
UserID: firstUser.UserID,
TemplateID: notifications.TemplateWorkspaceOutOfMemory,
Title: fmt.Sprintf("Notification %d", i),
Actions: json.RawMessage("[]"),
Content: fmt.Sprintf("Content of the notif %d", i),
CreatedAt: dbtime.Now(),
})
require.NoError(t, err)
}
return nil
}, &database.TxOptions{
Isolation: sql.LevelReadCommitted,
})
for i := range 40 {
_, err = api.Database.InsertInboxNotification(notifierCtx, database.InsertInboxNotificationParams{
ID: uuid.New(),
UserID: firstUser.UserID,
TemplateID: notifications.TemplateWorkspaceOutOfMemory,
Title: fmt.Sprintf("Notification %d", i),
Actions: json.RawMessage("[]"),
Content: fmt.Sprintf("Content of the notif %d", i),
CreatedAt: dbtime.Now(),
})
require.NoError(t, err)
}
Copy link
Member

Choose a reason for hiding this comment

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

This could be a good candidate for a dbgen helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch - even more that the function was already there for other testing.. :')

I migrated all tests to it.


notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{})
require.NoError(t, err)
Expand Down Expand Up @@ -132,8 +133,6 @@ func TestInboxNotifications_List(t *testing.T) {
require.NoError(t, err)
}

time.Sleep(5 * time.Second)

notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{
Templates: []uuid.UUID{notifications.TemplateWorkspaceOutOfMemory},
})
Expand Down Expand Up @@ -185,8 +184,6 @@ func TestInboxNotifications_List(t *testing.T) {
require.NoError(t, err)
}

time.Sleep(5 * time.Second)

notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{
Targets: []uuid.UUID{filteredTarget},
})
Expand Down Expand Up @@ -244,8 +241,6 @@ func TestInboxNotifications_List(t *testing.T) {
require.NoError(t, err)
}

time.Sleep(5 * time.Second)

notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{
Targets: []uuid.UUID{filteredTarget},
Templates: []uuid.UUID{notifications.TemplateWorkspaceOutOfDisk},
Expand Down Expand Up @@ -290,8 +285,6 @@ func TestInboxNotifications_ReadStatus(t *testing.T) {
require.NoError(t, err)
}

time.Sleep(5 * time.Second)

notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{})
require.NoError(t, err)
require.NotNil(t, notifs)
Expand Down
Loading
0