Skip to content

Commit 3270cd5

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Simplify passing data
1 parent ec17ad5 commit 3270cd5

File tree

10 files changed

+35
-64
lines changed

10 files changed

+35
-64
lines changed

‎cli/server.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
992992
)
993993
if experiments.Enabled(codersdk.ExperimentNotifications) {
994994
cfg := options.DeploymentValues.Notifications
995-
accessUrl := options.DeploymentValues.AccessURL.Value().String()
996995
metrics := notifications.NewMetrics(options.PrometheusRegistry)
997996

998997
// The enqueuer is responsible for enqueueing notifications to the given store.
@@ -1005,7 +1004,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
10051004
// The notification manager is responsible for:
10061005
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
10071006
// - keeping the store updated with status updates
1008-
notificationsManager, err = notifications.NewManager(cfg, options.Database, accessUrl, metrics, logger.Named("notifications.manager"))
1007+
notificationsManager, err = notifications.NewManager(cfg, options.Database, metrics, logger.Named("notifications.manager"))
10091008
if err != nil {
10101009
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
10111010
}

‎coderd/notifications/dispatch/smtp.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,14 @@ var (
4949
// NOTE: auth and TLS is currently *not* enabled in this initial thin slice.
5050
// TODO: implement DKIM/SPF/DMARC? https://github.com/emersion/go-msgauth
5151
type SMTPHandler struct {
52-
cfg codersdk.NotificationsEmailConfig
53-
log slog.Logger
54-
accessURL string
52+
cfg codersdk.NotificationsEmailConfig
53+
log slog.Logger
5554

5655
loginWarnOnce sync.Once
5756
}
5857

59-
func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, accessURL string, log slog.Logger) *SMTPHandler {
60-
return &SMTPHandler{cfg: cfg, accessURL: accessURL, log: log}
58+
func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, log slog.Logger) *SMTPHandler {
59+
return &SMTPHandler{cfg: cfg, log: log}
6160
}
6261

6362
func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
@@ -76,8 +75,6 @@ func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTm
7675
// Then, reuse these strings in the HTML & plain body templates.
7776
payload.Labels["_subject"] = subject
7877
payload.Labels["_body"] = htmlBody
79-
payload.Labels["_year"] = time.Now().Format("2006")
80-
payload.Labels["_accessURL"] = s.accessURL
8178
htmlBody, err = render.GoTemplate(htmlTemplate, payload, nil)
8279
if err != nil {
8380
return nil, xerrors.Errorf("render full html template: %w", err)

‎coderd/notifications/dispatch/smtp/html.gotmpl

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,11 @@
137137
<div class="footer">
138138
<!-- TODO: dynamic copyright -->
139139
<p>
140-
&copy; {{ .Labels._year }} Coder. All rights reserved -
141-
<a
142-
href="{{ .Labels._accessURL }}"
143-
class="link"
144-
>{{ .Labels._accessURL }}</a
145-
>
140+
&copy; {{ current_year }} Coder. All rights reserved -
141+
<a href="{{ base_url }}" class="link">{{ base_url }}</a>
146142
</p>
147143
<p>
148-
<a href="{{ .Labels._accessURL }}/settings/notifications" class="link"
144+
<a href="{{ base_url }}/settings/notifications" class="link"
149145
>Click here to manage your notification settings</a
150146
>
151147
</p>

‎coderd/notifications/dispatch/smtp_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,9 +416,8 @@ func TestSMTP(t *testing.T) {
416416
var hp serpent.HostPort
417417
require.NoError(t, hp.Set(listen.Addr().String()))
418418
tc.cfg.Smarthost = hp
419-
accessURL := "http://localhost:8080"
420419

421-
handler := dispatch.NewSMTPHandler(tc.cfg, accessURL, logger.Named("smtp"))
420+
handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp"))
422421

423422
// Start mock SMTP server in the background.
424423
var wg sync.WaitGroup

‎coderd/notifications/manager.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type Manager struct {
5959
//
6060
// helpers is a map of template helpers which are used to customize notification messages to use global settings like
6161
// access URL etc.
62-
func NewManager(cfg codersdk.NotificationsConfig, store Store, host string, metrics *Metrics, log slog.Logger) (*Manager, error) {
62+
func NewManager(cfg codersdk.NotificationsConfig, store Store, metrics *Metrics, log slog.Logger) (*Manager, error) {
6363
// TODO(dannyk): add the ability to use multiple notification methods.
6464
var method database.NotificationMethod
6565
if err := method.Scan(cfg.Method.String()); err != nil {
@@ -93,14 +93,14 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, host string, metr
9393
stop: make(chan any),
9494
done: make(chan any),
9595

96-
handlers: defaultHandlers(cfg, host, log),
96+
handlers: defaultHandlers(cfg, log),
9797
}, nil
9898
}
9999

100100
// defaultHandlers builds a set of known handlers; panics if any error occurs as these handlers should be valid at compile time.
101-
func defaultHandlers(cfg codersdk.NotificationsConfig, accessURL string, log slog.Logger) map[database.NotificationMethod]Handler {
101+
func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler {
102102
return map[database.NotificationMethod]Handler{
103-
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, accessURL, log.Named("dispatcher.smtp")),
103+
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
104104
database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
105105
}
106106
}

‎coderd/notifications/manager_test.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ func TestBufferedUpdates(t *testing.T) {
3434
cfg.StoreSyncInterval = serpent.Duration(time.Hour) // Ensure we don't sync the store automatically.
3535

3636
// GIVEN: a manager which will pass or fail notifications based on their "nice" labels
37-
accessURL := "http://localhost:8080"
38-
mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("notifications-manager"))
37+
mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("notifications-manager"))
3938
require.NoError(t, err)
4039
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{
4140
database.NotificationMethodSmtp: santa,
@@ -151,14 +150,7 @@ func TestStopBeforeRun(t *testing.T) {
151150
ctx, logger, db := setupInMemory(t)
152151

153152
// GIVEN: a standard manager
154-
accessURL := "http://localhost:8080"
155-
mgr, err := notifications.NewManager(
156-
defaultNotificationsConfig(database.NotificationMethodSmtp),
157-
db,
158-
accessURL,
159-
createMetrics(),
160-
logger.Named("notifications-manager"),
161-
)
153+
mgr, err := notifications.NewManager(defaultNotificationsConfig(database.NotificationMethodSmtp), db, createMetrics(), logger.Named("notifications-manager"))
162154
require.NoError(t, err)
163155

164156
// THEN: validate that the manager can be stopped safely without Run() having been called yet

‎coderd/notifications/metrics_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ func TestMetrics(t *testing.T) {
5151
cfg.RetryInterval = serpent.Duration(time.Millisecond * 50)
5252
cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100) // Twice as long as fetch interval to ensure we catch pending updates.
5353

54-
accessURL := "http://localhost:8080"
55-
mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager"))
54+
mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
5655
require.NoError(t, err)
5756
t.Cleanup(func() {
5857
assert.NoError(t, mgr.Stop(ctx))
@@ -219,8 +218,7 @@ func TestPendingUpdatesMetric(t *testing.T) {
219218

220219
syncer := &syncInterceptor{Store: store}
221220
interceptor := newUpdateSignallingInterceptor(syncer)
222-
accessURL := "http://localhost:8080"
223-
mgr, err := notifications.NewManager(cfg, interceptor, accessURL, metrics, logger.Named("manager"))
221+
mgr, err := notifications.NewManager(cfg, interceptor, metrics, logger.Named("manager"))
224222
require.NoError(t, err)
225223
t.Cleanup(func() {
226224
assert.NoError(t, mgr.Stop(ctx))
@@ -294,8 +292,7 @@ func TestInflightDispatchesMetric(t *testing.T) {
294292
cfg.RetryInterval = serpent.Duration(time.Hour) // Delay retries so they don't interfere.
295293
cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100)
296294

297-
accessURL := "http://localhost:8080"
298-
mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager"))
295+
mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
299296
require.NoError(t, err)
300297
t.Cleanup(func() {
301298
assert.NoError(t, mgr.Stop(ctx))
@@ -374,8 +371,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {
374371

375372
// WHEN: two notifications (each with different templates) are enqueued.
376373
cfg := defaultNotificationsConfig(defaultMethod)
377-
accessURL := "http://localhost:8080"
378-
mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager"))
374+
mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
379375
require.NoError(t, err)
380376
t.Cleanup(func() {
381377
assert.NoError(t, mgr.Stop(ctx))

‎coderd/notifications/notifications_test.go

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ func TestBasicNotificationRoundtrip(t *testing.T) {
6565
interceptor := &syncInterceptor{Store: db}
6666
cfg := defaultNotificationsConfig(method)
6767
cfg.RetryInterval = serpent.Duration(time.Hour) // Ensure retries don't interfere with the test
68-
accessURL := "http://localhost:8080"
69-
mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("manager"))
68+
mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("manager"))
7069
require.NoError(t, err)
7170
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
7271
t.Cleanup(func() {
@@ -139,9 +138,8 @@ func TestSMTPDispatch(t *testing.T) {
139138
Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
140139
Hello: "localhost",
141140
}
142-
accessURL := "http://localhost:8080"
143-
handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, accessURL, logger.Named("smtp")))
144-
mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
141+
handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp")))
142+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
145143
require.NoError(t, err)
146144
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
147145
t.Cleanup(func() {
@@ -202,8 +200,7 @@ func TestWebhookDispatch(t *testing.T) {
202200
cfg.Webhook = codersdk.NotificationsWebhookConfig{
203201
Endpoint: *serpent.URLOf(endpoint),
204202
}
205-
accessURL := "http://localhost:8080"
206-
mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
203+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
207204
require.NoError(t, err)
208205
t.Cleanup(func() {
209206
assert.NoError(t, mgr.Stop(ctx))
@@ -301,8 +298,7 @@ func TestBackpressure(t *testing.T) {
301298
storeInterceptor := &syncInterceptor{Store: db}
302299

303300
// GIVEN: a notification manager whose updates will be intercepted
304-
accessURL := "http://localhost:8080"
305-
mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager"))
301+
mgr, err := notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
306302
require.NoError(t, err)
307303
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
308304
enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
@@ -397,8 +393,7 @@ func TestRetries(t *testing.T) {
397393
// Intercept calls to submit the buffered updates to the store.
398394
storeInterceptor := &syncInterceptor{Store: db}
399395

400-
accessURL := "http://localhost:8080"
401-
mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager"))
396+
mgr, err := notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
402397
require.NoError(t, err)
403398
t.Cleanup(func() {
404399
assert.NoError(t, mgr.Stop(ctx))
@@ -459,8 +454,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
459454
mgrCtx, cancelManagerCtx := context.WithCancel(context.Background())
460455
t.Cleanup(cancelManagerCtx)
461456

462-
accessURL := "http://localhost:8080"
463-
mgr, err := notifications.NewManager(cfg, noopInterceptor, accessURL, createMetrics(), logger.Named("manager"))
457+
mgr, err := notifications.NewManager(cfg, noopInterceptor, createMetrics(), logger.Named("manager"))
464458
require.NoError(t, err)
465459
enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
466460
require.NoError(t, err)
@@ -507,7 +501,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
507501
// Intercept calls to submit the buffered updates to the store.
508502
storeInterceptor := &syncInterceptor{Store: db}
509503
handler := newDispatchInterceptor(&fakeHandler{})
510-
mgr, err = notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager"))
504+
mgr, err = notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
511505
require.NoError(t, err)
512506
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
513507

@@ -548,8 +542,7 @@ func TestInvalidConfig(t *testing.T) {
548542
cfg.DispatchTimeout = serpent.Duration(leasePeriod)
549543

550544
// WHEN: the manager is created with invalid config
551-
accessURL := "http://localhost:8080"
552-
_, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
545+
_, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
553546

554547
// THEN: the manager will fail to be created, citing invalid config as error
555548
require.ErrorIs(t, err, notifications.ErrInvalidDispatchTimeout)
@@ -567,8 +560,7 @@ func TestNotifierPaused(t *testing.T) {
567560
user := createSampleUser(t, db)
568561

569562
cfg := defaultNotificationsConfig(method)
570-
accessURL := "http://localhost:8080"
571-
mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
563+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
572564
require.NoError(t, err)
573565
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
574566
t.Cleanup(func() {
@@ -838,9 +830,8 @@ func TestDisabledAfterEnqueue(t *testing.T) {
838830

839831
method := database.NotificationMethodSmtp
840832
cfg := defaultNotificationsConfig(method)
841-
accessURL := "http://localhost:8080"
842833

843-
mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
834+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
844835
require.NoError(t, err)
845836
t.Cleanup(func() {
846837
assert.NoError(t, mgr.Stop(ctx))
@@ -946,8 +937,7 @@ func TestCustomNotificationMethod(t *testing.T) {
946937
Endpoint: *serpent.URLOf(endpoint),
947938
}
948939

949-
accessURL := "http://localhost:8080"
950-
mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager"))
940+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
951941
require.NoError(t, err)
952942
t.Cleanup(func() {
953943
_ = mgr.Stop(ctx)

‎coderd/notifications/render/gotmpl_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ func TestGoTemplate(t *testing.T) {
6565
t.Parallel()
6666

6767
out, err := render.GoTemplate(tc.in, tc.payload, map[string]any{
68-
"base_url": func() string { return "https://mocked-server-address" },
68+
"base_url": func() string { return "https://mocked-server-address" },
69+
"current_year": func() string { return "2024" },
6970
})
7071
if tc.expectedErr == nil {
7172
require.NoError(t, err)

‎coderd/notifications/utils_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ func defaultNotificationsConfig(method database.NotificationMethod) codersdk.Not
7777

7878
func defaultHelpers() map[string]any {
7979
return map[string]any{
80-
"base_url": func() string { return "http://test.com" },
80+
"base_url": func() string { return "http://test.com" },
81+
"current_year": func() int { return time.Now().Year() },
8182
}
8283
}
8384

0 commit comments

Comments
 (0)