Skip to content

fix: add fallback icons for notifications #17013

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 11 commits into from
Mar 28, 2025
Merged

fix: add fallback icons for notifications #17013

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f9662a8
work on fallback icons
defelmnq Mar 20, 2025
f2c57f4
merge with main
defelmnq Mar 26, 2025
033d78a
add tests for fallback
defelmnq Mar 26, 2025
a633e8e
Merge remote-tracking branch 'origin/main' into notif-inbox/internal-522
defelmnq Mar 27, 2025
0469e57
add tests and improve fallback logic
defelmnq Mar 27, 2025
d22f0b5
improve tests
defelmnq Mar 27, 2025
4c4527c
improve testings on icons
defelmnq Mar 28, 2025
783b04b
improve testings on icons
defelmnq Mar 28, 2025
3ac9d92
nit changes
defelmnq Mar 28, 2025
0b2496e
nit changes
defelmnq Mar 28, 2025
dc38924
Merge branch 'main' into notif-inbox/internal-522
defelmnq Mar 28, 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
57 changes: 55 additions & 2 deletions coderd/inboxnotifications.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ 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"
markdown "github.com/coder/coder/v2/coderd/render"
"github.com/coder/coder/v2/codersdk"
Expand All @@ -28,16 +29,63 @@ const (
notificationFormatPlaintext = "plaintext"
)

const (
FallbackIconWorkspace = "DEFAULT_WORKSPACE_ICON"
FallbackIconAccount = "DEFAULT_ACCOUNT_ICON"
FallbackIconTemplate = "DEFAULT_TEMPLATE_ICON"
FallbackIconOther = "DEFAULT_OTHER_ICON"
Copy link
Member

Choose a reason for hiding this comment

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

If these are values processed by "site", you should place them in codersdk as Enum, so they will be generated to TS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a really good idea, thanks for that - I moved it.

cc @BrunoQuaresma you would be able to use it now.

)

var fallbackIcons = map[uuid.UUID]string{
Copy link
Member

Choose a reason for hiding this comment

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

alternative data structure:

fallbackIcons = map[NotificationIcon][]uuid.UUID

not a must, just a preference

// 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,

// 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,
}

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: notif.Icon,
Actions: func() []codersdk.InboxNotificationAction {
var actionsList []codersdk.InboxNotificationAction
err := json.Unmarshal([]byte(notif.Actions), &actionsList)
Expand All @@ -54,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.
Expand Down Expand Up @@ -145,6 +196,8 @@ 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:
Expand Down
71 changes: 71 additions & 0 deletions coderd/inboxnotifications_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ 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"
Expand Down Expand Up @@ -135,6 +137,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) {
Expand Down Expand Up @@ -862,3 +867,69 @@ 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(&notif)
require.Equal(t, tt.expectedIcon, notif.Icon)
})
}
}
1 change: 1 addition & 0 deletions coderd/notifications/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Loading