From f9662a816eeca068df06baa3016e376eb57eace9 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Thu, 20 Mar 2025 01:39:35 +0000 Subject: [PATCH 1/8] work on fallback icons --- coderd/inboxnotifications.go | 48 +++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/coderd/inboxnotifications.go b/coderd/inboxnotifications.go index ebb2a08dfe7eb..b266201ecafc3 100644 --- a/coderd/inboxnotifications.go +++ b/coderd/inboxnotifications.go @@ -16,12 +16,52 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/pubsub" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/wsjson" "github.com/coder/websocket" ) +const ( + fallbackIconWorkspace = "" + fallbackIconAccount = "" + fallbackIconTemplate = "" + fallbackIconOther = "" +) + +var fallbackIcons = map[uuid.UUID]string{ + // workspace related notifications + notifications.TemplateWorkspaceCreated: fallbackIconWorkspace, + notifications.TemplateWorkspaceCreated: fallbackIconWorkspace, + notifications.TemplateWorkspaceManuallyUpdated: fallbackIconWorkspace, + notifications.TemplateWorkspaceDeleted: fallbackIconWorkspace, + notifications.TemplateWorkspaceAutobuildFailed: fallbackIconWorkspace, + notifications.TemplateWorkspaceDormant: fallbackIconWorkspace, + notifications.TemplateWorkspaceAutoUpdated: fallbackIconWorkspace, + notifications.TemplateWorkspaceMarkedForDeletion: fallbackIconWorkspace, + notifications.TemplateWorkspaceManualBuildFailed: fallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfMemory: fallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfDisk: fallbackIconWorkspace, + + // account related notifications + notifications.TemplateUserAccountCreated: fallbackIconAccount, + notifications.TemplateUserAccountDeleted: fallbackIconAccount, + notifications.TemplateUserAccountSuspended: fallbackIconAccount, + notifications.TemplateUserAccountActivated: fallbackIconAccount, + notifications.TemplateYourAccountSuspended: fallbackIconAccount, + notifications.TemplateYourAccountActivated: fallbackIconAccount, + notifications.TemplateUserRequestedOneTimePasscode: fallbackIconAccount, + + // template related notifications + notifications.TemplateTemplateDeleted: fallbackIconTemplate, + notifications.TemplateTemplateDeprecated: fallbackIconTemplate, + notifications.TemplateWorkspaceBuildsFailedReport: fallbackIconTemplate, + + // other related notifications + notifications.TemplateTestNotification: fallbackIconOther, +} + // 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{ @@ -31,7 +71,13 @@ func convertInboxNotificationResponse(ctx context.Context, logger slog.Logger, n Targets: notif.Targets, Title: notif.Title, Content: notif.Content, - Icon: notif.Icon, + Icon: func() string { + if notif.Icon != "" { + return notif.Icon + } + + return fallbackIcons[notif.TemplateID] + }(), Actions: func() []codersdk.InboxNotificationAction { var actionsList []codersdk.InboxNotificationAction err := json.Unmarshal([]byte(notif.Actions), &actionsList) From 033d78a4c0d242ebe78255d9f6d1ea023d18b047 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Wed, 26 Mar 2025 01:43:51 +0000 Subject: [PATCH 2/8] add tests for fallback --- coderd/inboxnotifications.go | 61 +++++++++++++++++-------------- coderd/inboxnotifications_test.go | 4 ++ coderd/notifications/events.go | 1 + 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/coderd/inboxnotifications.go b/coderd/inboxnotifications.go index 2fa4e77d5adcc..df43527bdf78e 100644 --- a/coderd/inboxnotifications.go +++ b/coderd/inboxnotifications.go @@ -25,47 +25,47 @@ import ( ) const ( - fallbackIconWorkspace = "" - fallbackIconAccount = "" - fallbackIconTemplate = "" - fallbackIconOther = "" + notificationFormatMarkdown = "markdown" + notificationFormatPlaintext = "plaintext" ) const ( - notificationFormatMarkdown = "markdown" - notificationFormatPlaintext = "plaintext" + FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON" + FallbackIconAccount = "DEFAULT_ACCOUNT_ICON" + FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON" + FallbackIconOther = "DEFAULT_OTHER_ICON" ) var fallbackIcons = map[uuid.UUID]string{ // workspace related notifications - notifications.TemplateWorkspaceCreated: fallbackIconWorkspace, - notifications.TemplateWorkspaceCreated: fallbackIconWorkspace, - notifications.TemplateWorkspaceManuallyUpdated: fallbackIconWorkspace, - notifications.TemplateWorkspaceDeleted: fallbackIconWorkspace, - notifications.TemplateWorkspaceAutobuildFailed: fallbackIconWorkspace, - notifications.TemplateWorkspaceDormant: fallbackIconWorkspace, - notifications.TemplateWorkspaceAutoUpdated: fallbackIconWorkspace, - notifications.TemplateWorkspaceMarkedForDeletion: fallbackIconWorkspace, - notifications.TemplateWorkspaceManualBuildFailed: fallbackIconWorkspace, - notifications.TemplateWorkspaceOutOfMemory: fallbackIconWorkspace, - notifications.TemplateWorkspaceOutOfDisk: fallbackIconWorkspace, + notifications.TemplateWorkspaceCreated: FallbackIconWorkspace, + notifications.TemplateWorkspaceCreated: FallbackIconWorkspace, + notifications.TemplateWorkspaceManuallyUpdated: FallbackIconWorkspace, + notifications.TemplateWorkspaceDeleted: FallbackIconWorkspace, + notifications.TemplateWorkspaceAutobuildFailed: FallbackIconWorkspace, + notifications.TemplateWorkspaceDormant: FallbackIconWorkspace, + notifications.TemplateWorkspaceAutoUpdated: FallbackIconWorkspace, + notifications.TemplateWorkspaceMarkedForDeletion: FallbackIconWorkspace, + notifications.TemplateWorkspaceManualBuildFailed: FallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfMemory: FallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfDisk: FallbackIconWorkspace, // account related notifications - notifications.TemplateUserAccountCreated: fallbackIconAccount, - notifications.TemplateUserAccountDeleted: fallbackIconAccount, - notifications.TemplateUserAccountSuspended: fallbackIconAccount, - notifications.TemplateUserAccountActivated: fallbackIconAccount, - notifications.TemplateYourAccountSuspended: fallbackIconAccount, - notifications.TemplateYourAccountActivated: fallbackIconAccount, - notifications.TemplateUserRequestedOneTimePasscode: fallbackIconAccount, + notifications.TemplateUserAccountCreated: FallbackIconAccount, + notifications.TemplateUserAccountDeleted: FallbackIconAccount, + notifications.TemplateUserAccountSuspended: FallbackIconAccount, + notifications.TemplateUserAccountActivated: FallbackIconAccount, + notifications.TemplateYourAccountSuspended: FallbackIconAccount, + notifications.TemplateYourAccountActivated: FallbackIconAccount, + notifications.TemplateUserRequestedOneTimePasscode: FallbackIconAccount, // template related notifications - notifications.TemplateTemplateDeleted: fallbackIconTemplate, - notifications.TemplateTemplateDeprecated: fallbackIconTemplate, - notifications.TemplateWorkspaceBuildsFailedReport: fallbackIconTemplate, + notifications.TemplateTemplateDeleted: FallbackIconTemplate, + notifications.TemplateTemplateDeprecated: FallbackIconTemplate, + notifications.TemplateWorkspaceBuildsFailedReport: FallbackIconTemplate, // other related notifications - notifications.TemplateTestNotification: fallbackIconOther, + notifications.TemplateTestNotification: FallbackIconOther, } // convertInboxNotificationResponse works as a util function to transform a database.InboxNotification to codersdk.InboxNotification @@ -191,6 +191,11 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) } } + // convert icon to fallback if required + if payload.InboxNotification.Icon == "" { + payload.InboxNotification.Icon = fallbackIcons[payload.InboxNotification.TemplateID] + } + // keep a safe guard in case of latency to push notifications through websocket select { case notificationCh <- payload.InboxNotification: diff --git a/coderd/inboxnotifications_test.go b/coderd/inboxnotifications_test.go index ed0696195cb60..524915698e5d6 100644 --- a/coderd/inboxnotifications_test.go +++ b/coderd/inboxnotifications_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" @@ -135,6 +136,9 @@ func TestInboxNotification_Watch(t *testing.T) { require.Equal(t, 1, notif.UnreadCount) require.Equal(t, memberClient.ID, notif.Notification.UserID) + + // check for the fallback icon logic + require.Equal(t, coderd.FallbackIconWorkspace, notif.Notification.Icon) }) t.Run("OK - change format", func(t *testing.T) { diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go index 3399da96cf28a..2f45205bf33ec 100644 --- a/coderd/notifications/events.go +++ b/coderd/notifications/events.go @@ -4,6 +4,7 @@ import "github.com/google/uuid" // These vars are mapped to UUIDs in the notification_templates table. // TODO: autogenerate these: https://github.com/coder/team-coconut/issues/36 +// TODO(defelmnq): add fallback icon to coderd/inboxnofication.go when adding a new template // Workspace-related events. var ( From 0469e574df9822841fae439098cda07c59008722 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Thu, 27 Mar 2025 14:53:44 +0000 Subject: [PATCH 3/8] add tests and improve fallback logic --- coderd/inboxnotifications.go | 32 ++++++++-------- coderd/inboxnotifications_test.go | 61 +++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/coderd/inboxnotifications.go b/coderd/inboxnotifications.go index df43527bdf78e..4e1079528cc2d 100644 --- a/coderd/inboxnotifications.go +++ b/coderd/inboxnotifications.go @@ -39,7 +39,6 @@ const ( var fallbackIcons = map[uuid.UUID]string{ // workspace related notifications notifications.TemplateWorkspaceCreated: FallbackIconWorkspace, - notifications.TemplateWorkspaceCreated: FallbackIconWorkspace, notifications.TemplateWorkspaceManuallyUpdated: FallbackIconWorkspace, notifications.TemplateWorkspaceDeleted: FallbackIconWorkspace, notifications.TemplateWorkspaceAutobuildFailed: FallbackIconWorkspace, @@ -63,27 +62,30 @@ var fallbackIcons = map[uuid.UUID]string{ notifications.TemplateTemplateDeleted: FallbackIconTemplate, notifications.TemplateTemplateDeprecated: FallbackIconTemplate, notifications.TemplateWorkspaceBuildsFailedReport: FallbackIconTemplate, +} - // other related notifications - notifications.TemplateTestNotification: FallbackIconOther, +func SetInboxNotificationIcon(notif *codersdk.InboxNotification) { + if notif.Icon != "" { + return + } + + fallbackIcon, ok := fallbackIcons[notif.TemplateID] + if !ok { + fallbackIcon = FallbackIconOther + } + + notif.Icon = fallbackIcon } // 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{ + convertedNotif := codersdk.InboxNotification{ ID: notif.ID, UserID: notif.UserID, TemplateID: notif.TemplateID, Targets: notif.Targets, Title: notif.Title, Content: notif.Content, - Icon: func() string { - if notif.Icon != "" { - return notif.Icon - } - - return fallbackIcons[notif.TemplateID] - }(), Actions: func() []codersdk.InboxNotificationAction { var actionsList []codersdk.InboxNotificationAction err := json.Unmarshal([]byte(notif.Actions), &actionsList) @@ -100,6 +102,9 @@ func convertInboxNotificationResponse(ctx context.Context, logger slog.Logger, n }(), CreatedAt: notif.CreatedAt, } + + SetInboxNotificationIcon(&convertedNotif) + return convertedNotif } // watchInboxNotifications watches for new inbox notifications and sends them to the client. @@ -191,10 +196,7 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) } } - // convert icon to fallback if required - if payload.InboxNotification.Icon == "" { - payload.InboxNotification.Icon = fallbackIcons[payload.InboxNotification.TemplateID] - } + SetInboxNotificationIcon(&payload.InboxNotification) // keep a safe guard in case of latency to push notifications through websocket select { diff --git a/coderd/inboxnotifications_test.go b/coderd/inboxnotifications_test.go index 524915698e5d6..3d7701808b8f3 100644 --- a/coderd/inboxnotifications_test.go +++ b/coderd/inboxnotifications_test.go @@ -7,6 +7,7 @@ import ( "net/http" "runtime" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/require" @@ -866,3 +867,63 @@ func TestInboxNotifications_MarkAllAsRead(t *testing.T) { require.Len(t, notifs.Notifications, 25) }) } + +func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { + tests := []struct { + name string + icon string + templateID uuid.UUID + expectedIcon string + }{ + {"WorkspaceCreated", "", notifications.TemplateWorkspaceCreated, coderd.FallbackIconWorkspace}, + {"WorkspaceManuallyUpdated", "", notifications.TemplateWorkspaceManuallyUpdated, coderd.FallbackIconWorkspace}, + {"WorkspaceDeleted", "", notifications.TemplateWorkspaceDeleted, coderd.FallbackIconWorkspace}, + {"WorkspaceAutobuildFailed", "", notifications.TemplateWorkspaceAutobuildFailed, coderd.FallbackIconWorkspace}, + {"WorkspaceDormant", "", notifications.TemplateWorkspaceDormant, coderd.FallbackIconWorkspace}, + {"WorkspaceAutoUpdated", "", notifications.TemplateWorkspaceAutoUpdated, coderd.FallbackIconWorkspace}, + {"WorkspaceMarkedForDeletion", "", notifications.TemplateWorkspaceMarkedForDeletion, coderd.FallbackIconWorkspace}, + {"WorkspaceManualBuildFailed", "", notifications.TemplateWorkspaceManualBuildFailed, coderd.FallbackIconWorkspace}, + {"WorkspaceOutOfMemory", "", notifications.TemplateWorkspaceOutOfMemory, coderd.FallbackIconWorkspace}, + {"WorkspaceOutOfDisk", "", notifications.TemplateWorkspaceOutOfDisk, coderd.FallbackIconWorkspace}, + + // Account-related events + {"UserAccountCreated", "", notifications.TemplateUserAccountCreated, coderd.FallbackIconAccount}, + {"UserAccountDeleted", "", notifications.TemplateUserAccountDeleted, coderd.FallbackIconAccount}, + {"UserAccountSuspended", "", notifications.TemplateUserAccountSuspended, coderd.FallbackIconAccount}, + {"UserAccountActivated", "", notifications.TemplateUserAccountActivated, coderd.FallbackIconAccount}, + {"YourAccountSuspended", "", notifications.TemplateYourAccountSuspended, coderd.FallbackIconAccount}, + {"YourAccountActivated", "", notifications.TemplateYourAccountActivated, coderd.FallbackIconAccount}, + {"UserRequestedOneTimePasscode", "", notifications.TemplateUserRequestedOneTimePasscode, coderd.FallbackIconAccount}, + + // Template-related events + {"TemplateDeleted", "", notifications.TemplateTemplateDeleted, coderd.FallbackIconTemplate}, + {"TemplateDeprecated", "", notifications.TemplateTemplateDeprecated, coderd.FallbackIconTemplate}, + {"WorkspaceBuildsFailedReport", "", notifications.TemplateWorkspaceBuildsFailedReport, coderd.FallbackIconTemplate}, + + // Notification-related events + {"TestNotification", "", notifications.TemplateTestNotification, coderd.FallbackIconOther}, + + // Testing out that we dont erase existing icon + {"TestExistingIcon", "https://cdn.coder.com/icon_notif.png", notifications.TemplateTemplateDeleted, "https://cdn.coder.com/icon_notif.png"}, + + // Unknown template + {"UnknownTemplate", "", uuid.New(), coderd.FallbackIconOther}, + } + + for _, tt := range tests { + tt := tt + + notif := codersdk.InboxNotification{ + ID: uuid.New(), + UserID: uuid.New(), + TemplateID: tt.templateID, + Title: "notification title", + Content: "notification content", + Icon: tt.icon, + CreatedAt: time.Now(), + } + + coderd.SetInboxNotificationIcon(¬if) + require.Equal(t, tt.expectedIcon, notif.Icon) + } +} From d22f0b56e23a88ef4ad285e33987e1a563f89cdc Mon Sep 17 00:00:00 2001 From: defelmnq Date: Thu, 27 Mar 2025 14:58:25 +0000 Subject: [PATCH 4/8] improve tests --- coderd/inboxnotifications_test.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/coderd/inboxnotifications_test.go b/coderd/inboxnotifications_test.go index 3d7701808b8f3..68eb89e1207b2 100644 --- a/coderd/inboxnotifications_test.go +++ b/coderd/inboxnotifications_test.go @@ -869,6 +869,8 @@ func TestInboxNotifications_MarkAllAsRead(t *testing.T) { } func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { + t.Parallel() + tests := []struct { name string icon string @@ -913,17 +915,21 @@ func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { for _, tt := range tests { tt := tt - notif := codersdk.InboxNotification{ - ID: uuid.New(), - UserID: uuid.New(), - TemplateID: tt.templateID, - Title: "notification title", - Content: "notification content", - Icon: tt.icon, - CreatedAt: time.Now(), - } + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - coderd.SetInboxNotificationIcon(¬if) - require.Equal(t, tt.expectedIcon, notif.Icon) + notif := codersdk.InboxNotification{ + ID: uuid.New(), + UserID: uuid.New(), + TemplateID: tt.templateID, + Title: "notification title", + Content: "notification content", + Icon: tt.icon, + CreatedAt: time.Now(), + } + + coderd.SetInboxNotificationIcon(¬if) + require.Equal(t, tt.expectedIcon, notif.Icon) + }) } } From 4c4527c4df4f2df5bad7031ea0bdbd0aa2df08fd Mon Sep 17 00:00:00 2001 From: defelmnq Date: Fri, 28 Mar 2025 09:16:32 +0000 Subject: [PATCH 5/8] improve testings on icons --- coderd/inboxnotifications.go | 62 ++++----- coderd/inboxnotifications_internal_test.go | 51 ++++++++ coderd/inboxnotifications_test.go | 141 ++++++++++----------- codersdk/inboxnotification.go | 7 + site/src/api/typesGenerated.ts | 12 ++ 5 files changed, 167 insertions(+), 106 deletions(-) create mode 100644 coderd/inboxnotifications_internal_test.go diff --git a/coderd/inboxnotifications.go b/coderd/inboxnotifications.go index 4e1079528cc2d..df6ebe9d25aaf 100644 --- a/coderd/inboxnotifications.go +++ b/coderd/inboxnotifications.go @@ -29,52 +29,46 @@ const ( notificationFormatPlaintext = "plaintext" ) -const ( - FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON" - FallbackIconAccount = "DEFAULT_ACCOUNT_ICON" - FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON" - FallbackIconOther = "DEFAULT_OTHER_ICON" -) - var fallbackIcons = map[uuid.UUID]string{ // workspace related notifications - notifications.TemplateWorkspaceCreated: FallbackIconWorkspace, - notifications.TemplateWorkspaceManuallyUpdated: FallbackIconWorkspace, - notifications.TemplateWorkspaceDeleted: FallbackIconWorkspace, - notifications.TemplateWorkspaceAutobuildFailed: FallbackIconWorkspace, - notifications.TemplateWorkspaceDormant: FallbackIconWorkspace, - notifications.TemplateWorkspaceAutoUpdated: FallbackIconWorkspace, - notifications.TemplateWorkspaceMarkedForDeletion: FallbackIconWorkspace, - notifications.TemplateWorkspaceManualBuildFailed: FallbackIconWorkspace, - notifications.TemplateWorkspaceOutOfMemory: FallbackIconWorkspace, - notifications.TemplateWorkspaceOutOfDisk: FallbackIconWorkspace, + notifications.TemplateWorkspaceCreated: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceManuallyUpdated: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceDeleted: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceAutobuildFailed: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceDormant: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceAutoUpdated: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceMarkedForDeletion: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceManualBuildFailed: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfMemory: codersdk.FallbackIconWorkspace, + notifications.TemplateWorkspaceOutOfDisk: codersdk.FallbackIconWorkspace, // account related notifications - notifications.TemplateUserAccountCreated: FallbackIconAccount, - notifications.TemplateUserAccountDeleted: FallbackIconAccount, - notifications.TemplateUserAccountSuspended: FallbackIconAccount, - notifications.TemplateUserAccountActivated: FallbackIconAccount, - notifications.TemplateYourAccountSuspended: FallbackIconAccount, - notifications.TemplateYourAccountActivated: FallbackIconAccount, - notifications.TemplateUserRequestedOneTimePasscode: FallbackIconAccount, + notifications.TemplateUserAccountCreated: codersdk.FallbackIconAccount, + notifications.TemplateUserAccountDeleted: codersdk.FallbackIconAccount, + notifications.TemplateUserAccountSuspended: codersdk.FallbackIconAccount, + notifications.TemplateUserAccountActivated: codersdk.FallbackIconAccount, + notifications.TemplateYourAccountSuspended: codersdk.FallbackIconAccount, + notifications.TemplateYourAccountActivated: codersdk.FallbackIconAccount, + notifications.TemplateUserRequestedOneTimePasscode: codersdk.FallbackIconAccount, // template related notifications - notifications.TemplateTemplateDeleted: FallbackIconTemplate, - notifications.TemplateTemplateDeprecated: FallbackIconTemplate, - notifications.TemplateWorkspaceBuildsFailedReport: FallbackIconTemplate, + notifications.TemplateTemplateDeleted: codersdk.FallbackIconTemplate, + notifications.TemplateTemplateDeprecated: codersdk.FallbackIconTemplate, + notifications.TemplateWorkspaceBuildsFailedReport: codersdk.FallbackIconTemplate, } -func SetInboxNotificationIcon(notif *codersdk.InboxNotification) { +func ensureNotificationIcon(notif codersdk.InboxNotification) codersdk.InboxNotification { if notif.Icon != "" { - return + return notif } fallbackIcon, ok := fallbackIcons[notif.TemplateID] if !ok { - fallbackIcon = FallbackIconOther + fallbackIcon = codersdk.FallbackIconOther } notif.Icon = fallbackIcon + return notif } // convertInboxNotificationResponse works as a util function to transform a database.InboxNotification to codersdk.InboxNotification @@ -86,6 +80,7 @@ func convertInboxNotificationResponse(ctx context.Context, logger slog.Logger, n 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) @@ -103,8 +98,7 @@ func convertInboxNotificationResponse(ctx context.Context, logger slog.Logger, n CreatedAt: notif.CreatedAt, } - SetInboxNotificationIcon(&convertedNotif) - return convertedNotif + return ensureNotificationIcon(convertedNotif) } // watchInboxNotifications watches for new inbox notifications and sends them to the client. @@ -196,11 +190,9 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) } } - SetInboxNotificationIcon(&payload.InboxNotification) - // keep a safe guard in case of latency to push notifications through websocket select { - case notificationCh <- payload.InboxNotification: + case notificationCh <- ensureNotificationIcon(payload.InboxNotification): default: api.Logger.Error(ctx, "failed to push consumed notification into websocket handler, check latency") } diff --git a/coderd/inboxnotifications_internal_test.go b/coderd/inboxnotifications_internal_test.go new file mode 100644 index 0000000000000..06f19d30f61a0 --- /dev/null +++ b/coderd/inboxnotifications_internal_test.go @@ -0,0 +1,51 @@ +package coderd + +import ( + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/codersdk" +) + +func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + icon string + templateID uuid.UUID + expectedIcon string + }{ + {"WorkspaceCreated", "", notifications.TemplateWorkspaceCreated, codersdk.FallbackIconWorkspace}, + {"UserAccountCreated", "", notifications.TemplateUserAccountCreated, codersdk.FallbackIconAccount}, + {"TemplateDeleted", "", notifications.TemplateTemplateDeleted, codersdk.FallbackIconTemplate}, + {"TestNotification", "", notifications.TemplateTestNotification, codersdk.FallbackIconOther}, + {"TestExistingIcon", "https://cdn.coder.com/icon_notif.png", notifications.TemplateTemplateDeleted, "https://cdn.coder.com/icon_notif.png"}, + {"UnknownTemplate", "", uuid.New(), codersdk.FallbackIconOther}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + notif := codersdk.InboxNotification{ + ID: uuid.New(), + UserID: uuid.New(), + TemplateID: tt.templateID, + Title: "notification title", + Content: "notification content", + Icon: tt.icon, + CreatedAt: time.Now(), + } + + notif = ensureNotificationIcon(notif) + require.Equal(t, tt.expectedIcon, notif.Icon) + }) + } +} diff --git a/coderd/inboxnotifications_test.go b/coderd/inboxnotifications_test.go index 68eb89e1207b2..71c6159cc5a26 100644 --- a/coderd/inboxnotifications_test.go +++ b/coderd/inboxnotifications_test.go @@ -7,12 +7,10 @@ import ( "net/http" "runtime" "testing" - "time" "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" @@ -139,7 +137,7 @@ func TestInboxNotification_Watch(t *testing.T) { require.Equal(t, memberClient.ID, notif.Notification.UserID) // check for the fallback icon logic - require.Equal(t, coderd.FallbackIconWorkspace, notif.Notification.Icon) + require.Equal(t, codersdk.FallbackIconWorkspace, notif.Notification.Icon) }) t.Run("OK - change format", func(t *testing.T) { @@ -479,8 +477,9 @@ func TestInboxNotifications_List(t *testing.T) { TemplateID: notifications.TemplateWorkspaceOutOfMemory, Title: fmt.Sprintf("Notification %d", i), Actions: json.RawMessage("[]"), - Content: fmt.Sprintf("Content of the notif %d", i), - CreatedAt: dbtime.Now(), + + Content: fmt.Sprintf("Content of the notif %d", i), + CreatedAt: dbtime.Now(), }) } @@ -503,6 +502,71 @@ func TestInboxNotifications_List(t *testing.T) { require.Equal(t, "Notification 14", notifs.Notifications[0].Title) }) + t.Run("OK check icons", func(t *testing.T) { + t.Parallel() + + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{}) + firstUser := coderdtest.CreateFirstUser(t, client) + client, member := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + notifs, err := client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{}) + require.NoError(t, err) + require.NotNil(t, notifs) + require.Equal(t, 0, notifs.UnreadCount) + require.Empty(t, notifs.Notifications) + + for i := range 10 { + dbgen.NotificationInbox(t, api.Database, database.InsertInboxNotificationParams{ + ID: uuid.New(), + UserID: member.ID, + TemplateID: func() uuid.UUID { + switch i { + case 0: + return notifications.TemplateWorkspaceCreated + case 1: + return notifications.TemplateWorkspaceMarkedForDeletion + case 2: + return notifications.TemplateUserAccountActivated + case 3: + return notifications.TemplateTemplateDeprecated + case 4: + return notifications.TemplateTestNotification + default: + return uuid.New() + } + }(), + Title: fmt.Sprintf("Notification %d", i), + Actions: json.RawMessage("[]"), + Icon: func() string { + if i == 9 { + return "https://dev.coder.com/icon.png" + } + + return "" + }(), + Content: fmt.Sprintf("Content of the notif %d", i), + CreatedAt: dbtime.Now(), + }) + } + + notifs, err = client.ListInboxNotifications(ctx, codersdk.ListInboxNotificationsRequest{}) + require.NoError(t, err) + require.NotNil(t, notifs) + require.Equal(t, 10, notifs.UnreadCount) + require.Len(t, notifs.Notifications, 10) + + require.Equal(t, "https://dev.coder.com/icon.png", notifs.Notifications[0].Icon) + require.Equal(t, codersdk.FallbackIconWorkspace, notifs.Notifications[9].Icon) + require.Equal(t, codersdk.FallbackIconWorkspace, notifs.Notifications[8].Icon) + require.Equal(t, codersdk.FallbackIconAccount, notifs.Notifications[7].Icon) + require.Equal(t, codersdk.FallbackIconTemplate, notifs.Notifications[6].Icon) + require.Equal(t, codersdk.FallbackIconOther, notifs.Notifications[5].Icon) + require.Equal(t, codersdk.FallbackIconOther, notifs.Notifications[4].Icon) + }) + t.Run("OK with template filter", func(t *testing.T) { t.Parallel() @@ -546,6 +610,7 @@ func TestInboxNotifications_List(t *testing.T) { require.Len(t, notifs.Notifications, 5) require.Equal(t, "Notification 8", notifs.Notifications[0].Title) + require.Equal(t, codersdk.FallbackIconWorkspace, notifs.Notifications[0].Icon) }) t.Run("OK with target filter", func(t *testing.T) { @@ -867,69 +932,3 @@ func TestInboxNotifications_MarkAllAsRead(t *testing.T) { require.Len(t, notifs.Notifications, 25) }) } - -func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - icon string - templateID uuid.UUID - expectedIcon string - }{ - {"WorkspaceCreated", "", notifications.TemplateWorkspaceCreated, coderd.FallbackIconWorkspace}, - {"WorkspaceManuallyUpdated", "", notifications.TemplateWorkspaceManuallyUpdated, coderd.FallbackIconWorkspace}, - {"WorkspaceDeleted", "", notifications.TemplateWorkspaceDeleted, coderd.FallbackIconWorkspace}, - {"WorkspaceAutobuildFailed", "", notifications.TemplateWorkspaceAutobuildFailed, coderd.FallbackIconWorkspace}, - {"WorkspaceDormant", "", notifications.TemplateWorkspaceDormant, coderd.FallbackIconWorkspace}, - {"WorkspaceAutoUpdated", "", notifications.TemplateWorkspaceAutoUpdated, coderd.FallbackIconWorkspace}, - {"WorkspaceMarkedForDeletion", "", notifications.TemplateWorkspaceMarkedForDeletion, coderd.FallbackIconWorkspace}, - {"WorkspaceManualBuildFailed", "", notifications.TemplateWorkspaceManualBuildFailed, coderd.FallbackIconWorkspace}, - {"WorkspaceOutOfMemory", "", notifications.TemplateWorkspaceOutOfMemory, coderd.FallbackIconWorkspace}, - {"WorkspaceOutOfDisk", "", notifications.TemplateWorkspaceOutOfDisk, coderd.FallbackIconWorkspace}, - - // Account-related events - {"UserAccountCreated", "", notifications.TemplateUserAccountCreated, coderd.FallbackIconAccount}, - {"UserAccountDeleted", "", notifications.TemplateUserAccountDeleted, coderd.FallbackIconAccount}, - {"UserAccountSuspended", "", notifications.TemplateUserAccountSuspended, coderd.FallbackIconAccount}, - {"UserAccountActivated", "", notifications.TemplateUserAccountActivated, coderd.FallbackIconAccount}, - {"YourAccountSuspended", "", notifications.TemplateYourAccountSuspended, coderd.FallbackIconAccount}, - {"YourAccountActivated", "", notifications.TemplateYourAccountActivated, coderd.FallbackIconAccount}, - {"UserRequestedOneTimePasscode", "", notifications.TemplateUserRequestedOneTimePasscode, coderd.FallbackIconAccount}, - - // Template-related events - {"TemplateDeleted", "", notifications.TemplateTemplateDeleted, coderd.FallbackIconTemplate}, - {"TemplateDeprecated", "", notifications.TemplateTemplateDeprecated, coderd.FallbackIconTemplate}, - {"WorkspaceBuildsFailedReport", "", notifications.TemplateWorkspaceBuildsFailedReport, coderd.FallbackIconTemplate}, - - // Notification-related events - {"TestNotification", "", notifications.TemplateTestNotification, coderd.FallbackIconOther}, - - // Testing out that we dont erase existing icon - {"TestExistingIcon", "https://cdn.coder.com/icon_notif.png", notifications.TemplateTemplateDeleted, "https://cdn.coder.com/icon_notif.png"}, - - // Unknown template - {"UnknownTemplate", "", uuid.New(), coderd.FallbackIconOther}, - } - - for _, tt := range tests { - tt := tt - - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - notif := codersdk.InboxNotification{ - ID: uuid.New(), - UserID: uuid.New(), - TemplateID: tt.templateID, - Title: "notification title", - Content: "notification content", - Icon: tt.icon, - CreatedAt: time.Now(), - } - - coderd.SetInboxNotificationIcon(¬if) - require.Equal(t, tt.expectedIcon, notif.Icon) - }) - } -} diff --git a/codersdk/inboxnotification.go b/codersdk/inboxnotification.go index 056584d6cf359..2ec0c3306342c 100644 --- a/codersdk/inboxnotification.go +++ b/codersdk/inboxnotification.go @@ -10,6 +10,13 @@ import ( "github.com/google/uuid" ) +const ( + FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON" + FallbackIconAccount = "DEFAULT_ACCOUNT_ICON" + FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON" + FallbackIconOther = "DEFAULT_OTHER_ICON" +) + type InboxNotification struct { ID uuid.UUID `json:"id" format:"uuid"` UserID uuid.UUID `json:"user_id" format:"uuid"` diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index fe966d7b5ddd2..883e04646b64c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -832,6 +832,18 @@ export interface ExternalAuthUser { readonly name: string; } +// From codersdk/inboxnotification.go +export const FallbackIconAccount = "DEFAULT_ACCOUNT_ICON"; + +// From codersdk/inboxnotification.go +export const FallbackIconOther = "DEFAULT_OTHER_ICON"; + +// From codersdk/inboxnotification.go +export const FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON"; + +// From codersdk/inboxnotification.go +export const FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON"; + // From codersdk/deployment.go export interface Feature { readonly entitlement: Entitlement; From 783b04be6acf228ec0eefd5f7c3fa3873426bed6 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Fri, 28 Mar 2025 09:29:44 +0000 Subject: [PATCH 6/8] improve testings on icons --- coderd/inboxnotifications_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/coderd/inboxnotifications_test.go b/coderd/inboxnotifications_test.go index 71c6159cc5a26..d9ee0ee936a94 100644 --- a/coderd/inboxnotifications_test.go +++ b/coderd/inboxnotifications_test.go @@ -532,10 +532,8 @@ func TestInboxNotifications_List(t *testing.T) { return notifications.TemplateUserAccountActivated case 3: return notifications.TemplateTemplateDeprecated - case 4: - return notifications.TemplateTestNotification default: - return uuid.New() + return notifications.TemplateTestNotification } }(), Title: fmt.Sprintf("Notification %d", i), @@ -563,7 +561,6 @@ func TestInboxNotifications_List(t *testing.T) { require.Equal(t, codersdk.FallbackIconWorkspace, notifs.Notifications[8].Icon) require.Equal(t, codersdk.FallbackIconAccount, notifs.Notifications[7].Icon) require.Equal(t, codersdk.FallbackIconTemplate, notifs.Notifications[6].Icon) - require.Equal(t, codersdk.FallbackIconOther, notifs.Notifications[5].Icon) require.Equal(t, codersdk.FallbackIconOther, notifs.Notifications[4].Icon) }) From 3ac9d92172515f4ae59531214f2f0506187ca26d Mon Sep 17 00:00:00 2001 From: defelmnq Date: Fri, 28 Mar 2025 09:51:22 +0000 Subject: [PATCH 7/8] nit changes --- coderd/inboxnotifications_internal_test.go | 2 +- codersdk/inboxnotification.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/coderd/inboxnotifications_internal_test.go b/coderd/inboxnotifications_internal_test.go index 06f19d30f61a0..6dd36fcffe145 100644 --- a/coderd/inboxnotifications_internal_test.go +++ b/coderd/inboxnotifications_internal_test.go @@ -11,7 +11,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func TestInboxNotifications_SetInboxNotificationIcon(t *testing.T) { +func TestInboxNotifications_ensureNotificationIcon(t *testing.T) { t.Parallel() tests := []struct { diff --git a/codersdk/inboxnotification.go b/codersdk/inboxnotification.go index 2ec0c3306342c..ba68351c39bfe 100644 --- a/codersdk/inboxnotification.go +++ b/codersdk/inboxnotification.go @@ -11,10 +11,10 @@ import ( ) const ( - FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON" - FallbackIconAccount = "DEFAULT_ACCOUNT_ICON" - FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON" - FallbackIconOther = "DEFAULT_OTHER_ICON" + FallbackIconWorkspace = "DEFAULT_ICON_WORKSPACE" + FallbackIconAccount = "DEFAULT_ICON_ACCOUNT" + FallbackIconTemplate = "DEFAULT_ICON_TEMPLATE" + FallbackIconOther = "DEFAULT_ICON_OTHER" ) type InboxNotification struct { From 0b2496e926af3cf8c7eae97c7f7d465e5a0861c1 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Fri, 28 Mar 2025 10:00:37 +0000 Subject: [PATCH 8/8] nit changes --- site/src/api/typesGenerated.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 883e04646b64c..87a6836a7d26f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -833,16 +833,16 @@ export interface ExternalAuthUser { } // From codersdk/inboxnotification.go -export const FallbackIconAccount = "DEFAULT_ACCOUNT_ICON"; +export const FallbackIconAccount = "DEFAULT_ICON_ACCOUNT"; // From codersdk/inboxnotification.go -export const FallbackIconOther = "DEFAULT_OTHER_ICON"; +export const FallbackIconOther = "DEFAULT_ICON_OTHER"; // From codersdk/inboxnotification.go -export const FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON"; +export const FallbackIconTemplate = "DEFAULT_ICON_TEMPLATE"; // From codersdk/inboxnotification.go -export const FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON"; +export const FallbackIconWorkspace = "DEFAULT_ICON_WORKSPACE"; // From codersdk/deployment.go export interface Feature {