From dabeac97ec679c6a77c29185b6feb1ddf09b529b Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 28 Oct 2024 01:19:06 +0000 Subject: [PATCH 1/4] fix(notifications): return default parameters for empty values --- coderd/notifications/fetcher.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/coderd/notifications/fetcher.go b/coderd/notifications/fetcher.go index a579275d127bf..0688b88907981 100644 --- a/coderd/notifications/fetcher.go +++ b/coderd/notifications/fetcher.go @@ -38,6 +38,10 @@ func (n *notifier) fetchAppName(ctx context.Context) (string, error) { } return "", xerrors.Errorf("get application name: %w", err) } + + if appName == "" { + appName = notificationsDefaultAppName + } return appName, nil } @@ -49,5 +53,9 @@ func (n *notifier) fetchLogoURL(ctx context.Context) (string, error) { } return "", xerrors.Errorf("get logo URL: %w", err) } + + if logoURL == "" { + logoURL = notificationsDefaultLogoURL + } return logoURL, nil } From a5f4625eb7184e739d3881a980c1df5af5654c79 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 28 Oct 2024 03:00:17 +0100 Subject: [PATCH 2/4] test(notifications): add test for fetchers --- coderd/notifications/fetcher_test.go | 232 +++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 coderd/notifications/fetcher_test.go diff --git a/coderd/notifications/fetcher_test.go b/coderd/notifications/fetcher_test.go new file mode 100644 index 0000000000000..a0037705666bd --- /dev/null +++ b/coderd/notifications/fetcher_test.go @@ -0,0 +1,232 @@ +package notifications + +import ( + "context" + "database/sql" + "testing" + "text/template" + + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database/dbmock" +) + +func TestNotifier_FetchHelpers(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + helpers: template.FuncMap{}, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("ACME Inc.", nil) + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("https://example.com/logo.png", nil) + + ctx := context.Background() + helpers, err := n.fetchHelpers(ctx) + require.NoError(t, err) + + appName, ok := helpers["app_name"].(func() string) + require.True(t, ok) + require.Equal(t, "ACME Inc.", appName()) + + logoURL, ok := helpers["logo_url"].(func() string) + require.True(t, ok) + require.Equal(t, "https://example.com/logo.png", logoURL()) + }) + + t.Run("failed to fetch app name", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + helpers: template.FuncMap{}, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("", xerrors.New("internal error")) + + ctx := context.Background() + _, err := n.fetchHelpers(ctx) + require.Error(t, err) + require.ErrorContains(t, err, "get application name") + }) + + t.Run("failed to fetch logo URL", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + helpers: template.FuncMap{}, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("ACME Inc.", nil) + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("", xerrors.New("internal error")) + + ctx := context.Background() + _, err := n.fetchHelpers(ctx) + require.Error(t, err) + require.ErrorContains(t, err, "get logo URL") + }) +} + +func TestNotifier_FetchAppName(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("ACME Inc.", nil) + + ctx := context.Background() + appName, err := n.fetchAppName(ctx) + require.NoError(t, err) + require.Equal(t, "ACME Inc.", appName) + }) + + t.Run("No rows", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("", sql.ErrNoRows) + + ctx := context.Background() + appName, err := n.fetchAppName(ctx) + require.NoError(t, err) + require.Equal(t, notificationsDefaultAppName, appName) + }) + + t.Run("Empty string", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("", nil) + + ctx := context.Background() + appName, err := n.fetchAppName(ctx) + require.NoError(t, err) + require.Equal(t, notificationsDefaultAppName, appName) + }) + + t.Run("internal error", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetApplicationName(gomock.Any()).Return("", xerrors.New("internal error")) + + ctx := context.Background() + _, err := n.fetchAppName(ctx) + require.Error(t, err) + }) +} + +func TestNotifier_FetchLogoURL(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("https://example.com/logo.png", nil) + + ctx := context.Background() + logoURL, err := n.fetchLogoURL(ctx) + require.NoError(t, err) + require.Equal(t, "https://example.com/logo.png", logoURL) + }) + + t.Run("No rows", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("", sql.ErrNoRows) + + ctx := context.Background() + logoURL, err := n.fetchLogoURL(ctx) + require.NoError(t, err) + require.Equal(t, notificationsDefaultLogoURL, logoURL) + }) + + t.Run("Empty string", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("", nil) + + ctx := context.Background() + logoURL, err := n.fetchLogoURL(ctx) + require.NoError(t, err) + require.Equal(t, notificationsDefaultLogoURL, logoURL) + }) + + t.Run("internal error", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + dbmock := dbmock.NewMockStore(ctrl) + + n := ¬ifier{ + store: dbmock, + } + + dbmock.EXPECT().GetLogoURL(gomock.Any()).Return("", xerrors.New("internal error")) + + ctx := context.Background() + _, err := n.fetchLogoURL(ctx) + require.Error(t, err) + }) +} From bff79e4c0b83414ee0a4a2f353ab6d803034d5e4 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 28 Oct 2024 02:24:36 +0000 Subject: [PATCH 3/4] fix(notifications): fix tests file to be internal --- .../notifications/{fetcher_test.go => fetcher_internal_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename coderd/notifications/{fetcher_test.go => fetcher_internal_test.go} (100%) diff --git a/coderd/notifications/fetcher_test.go b/coderd/notifications/fetcher_internal_test.go similarity index 100% rename from coderd/notifications/fetcher_test.go rename to coderd/notifications/fetcher_internal_test.go From e9caa12a17e6ce35640ea2a1ef9c3479d0049326 Mon Sep 17 00:00:00 2001 From: Vincent Vielle Date: Mon, 28 Oct 2024 16:38:48 +0100 Subject: [PATCH 4/4] Remove useless verification in error --- coderd/notifications/fetcher_internal_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/notifications/fetcher_internal_test.go b/coderd/notifications/fetcher_internal_test.go index a0037705666bd..a8d0149c883b8 100644 --- a/coderd/notifications/fetcher_internal_test.go +++ b/coderd/notifications/fetcher_internal_test.go @@ -78,7 +78,6 @@ func TestNotifier_FetchHelpers(t *testing.T) { ctx := context.Background() _, err := n.fetchHelpers(ctx) - require.Error(t, err) require.ErrorContains(t, err, "get logo URL") }) }