Skip to content

Commit feb1649

Browse files
authored
feat(system-notifications): warn when system notifications have no recipient device (#7)
Notifications were silently dropped when the admin had no paired device on the topic and TotalSent was zero without raising any error The server now logs a warning when a system notification reaches zero devices so the misconfiguration is visible in logs The admin UI also shows a warning when the configured recipient has no active device on the chosen topic so the issue is caught at setup time instead of through missing events later Save remains enabled intentionally so admins can enable notifications now and pair a device right after without being blocked
1 parent 9e4771c commit feb1649

9 files changed

Lines changed: 259 additions & 28 deletions

File tree

cmd/beebuzz-server/adapter.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,26 @@ func (a *systemNotificationTopicProviderAdapter) GetTopicByID(ctx context.Contex
154154

155155
var _ systemnotifications.TopicProvider = (*systemNotificationTopicProviderAdapter)(nil)
156156

157+
// systemNotificationDeviceSubscriptionAdapter adapts device.Service to the
158+
// system notifications device subscription checker. It is used by the admin
159+
// settings response to flag the misconfiguration where notifications are
160+
// enabled but the recipient has no paired device on the chosen topic.
161+
type systemNotificationDeviceSubscriptionAdapter struct {
162+
deviceSvc *device.Service
163+
}
164+
165+
// HasActiveDeviceForTopic reports whether the user has at least one paired
166+
// device subscribed to topicName.
167+
func (a *systemNotificationDeviceSubscriptionAdapter) HasActiveDeviceForTopic(ctx context.Context, userID, topicName string) (bool, error) {
168+
subs, err := a.deviceSvc.GetSubscribedDevices(ctx, userID, topicName)
169+
if err != nil {
170+
return false, err
171+
}
172+
return len(subs) > 0, nil
173+
}
174+
175+
var _ systemnotifications.DeviceSubscriptionChecker = (*systemNotificationDeviceSubscriptionAdapter)(nil)
176+
157177
// pushAuthorizerAdapter adapts token.Service to notification.PushAuthorizer.
158178
type pushAuthorizerAdapter struct {
159179
tokenSvc *token.Service
@@ -242,15 +262,30 @@ type systemNotificationDeliveryAdapter struct {
242262
}
243263

244264
// SendSystemNotification sends a server-trusted notification generated by BeeBuzz itself.
265+
//
266+
// A zero TotalSent is logged at warn level: the call succeeded but no paired
267+
// device on the configured topic received the push, which usually means the
268+
// admin enabled system notifications without pairing a device on that topic.
269+
// Without this signal the misconfiguration is silent in production.
245270
func (a *systemNotificationDeliveryAdapter) SendSystemNotification(ctx context.Context, input systemnotifications.DeliveryInput) error {
246-
_, err := a.notifSvc.Send(ctx, input.RecipientUserID, input.TopicID, notification.SendInput{
271+
report, err := a.notifSvc.Send(ctx, input.RecipientUserID, input.TopicID, notification.SendInput{
247272
TopicName: input.TopicName,
248273
Title: input.Title,
249274
Body: input.Body,
250275
Source: event.SourceInternal,
251276
DeliveryMode: notification.DeliveryModeServerTrusted,
252277
}, a.log)
253-
return err
278+
if err != nil {
279+
return err
280+
}
281+
if report != nil && report.TotalSent == 0 {
282+
a.log.Warn("system notification delivered to zero devices",
283+
"recipient_user_id", input.RecipientUserID,
284+
"topic_id", input.TopicID,
285+
"topic", input.TopicName,
286+
)
287+
}
288+
return nil
254289
}
255290

256291
var _ systemnotifications.Delivery = (*systemNotificationDeliveryAdapter)(nil)

cmd/beebuzz-server/serve.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ func buildServices(db *sqlx.DB, cfg *config.Config, log *slog.Logger, m mailer.M
174174
systemNotifRepo := systemnotifications.NewRepository(db)
175175
systemNotifTopics := &systemNotificationTopicProviderAdapter{topicSvc: topicSvc}
176176
systemNotifDelivery := &systemNotificationDeliveryAdapter{notifSvc: notifSvc, log: log}
177-
systemNotifSvc := systemnotifications.NewService(systemNotifRepo, systemNotifTopics, systemNotifDelivery, log)
177+
systemNotifSubscriptions := &systemNotificationDeviceSubscriptionAdapter{deviceSvc: deviceSvc}
178+
systemNotifSvc := systemnotifications.NewService(systemNotifRepo, systemNotifTopics, systemNotifDelivery, systemNotifSubscriptions, log)
178179
authSvc.SetSignupNotifier(systemNotifSvc)
179180

180181
webhookRepo := webhook.NewRepository(db)

docs/openapi.yaml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2219,7 +2219,7 @@ components:
22192219
$ref: "#/components/schemas/AdminUserResponse"
22202220
SystemNotificationSettingsResponse:
22212221
type: object
2222-
required: [enabled, signup_created_enabled]
2222+
required: [enabled, signup_created_enabled, recipient_has_active_device_for_topic]
22232223
properties:
22242224
enabled:
22252225
type: boolean
@@ -2229,6 +2229,13 @@ components:
22292229
type: string
22302230
signup_created_enabled:
22312231
type: boolean
2232+
recipient_has_active_device_for_topic:
2233+
type: boolean
2234+
description: |
2235+
Best-effort flag computed at read time. True when the recipient
2236+
admin has at least one paired device subscribed to the configured
2237+
topic. Used by the admin UI to warn about a misconfiguration that
2238+
would silently drop deliveries.
22322239
created_at:
22332240
type: string
22342241
format: date-time

internal/system/notifications/service.go

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,33 @@ const signupCreatedTitle = "New BeeBuzz signup"
1212

1313
// Service owns system notification policy and dispatch decisions.
1414
type Service struct {
15-
repo *Repository
16-
topics TopicProvider
17-
delivery Delivery
18-
log *slog.Logger
15+
repo *Repository
16+
topics TopicProvider
17+
delivery Delivery
18+
subscriptions DeviceSubscriptionChecker
19+
log *slog.Logger
1920
}
2021

2122
// NewService creates a system notifications service.
22-
func NewService(repo *Repository, topics TopicProvider, delivery Delivery, log *slog.Logger) *Service {
23+
func NewService(repo *Repository, topics TopicProvider, delivery Delivery, subscriptions DeviceSubscriptionChecker, log *slog.Logger) *Service {
2324
return &Service{
24-
repo: repo,
25-
topics: topics,
26-
delivery: delivery,
27-
log: log,
25+
repo: repo,
26+
topics: topics,
27+
delivery: delivery,
28+
subscriptions: subscriptions,
29+
log: log,
2830
}
2931
}
3032

31-
// GetSettings returns the current singleton settings.
33+
// GetSettings returns the current singleton settings, enriched with the
34+
// best-effort RecipientHasActiveDeviceForTopic flag for the admin UI.
3235
func (s *Service) GetSettings(ctx context.Context) (*Settings, error) {
33-
return s.repo.GetSettings(ctx)
36+
settings, err := s.repo.GetSettings(ctx)
37+
if err != nil {
38+
return nil, err
39+
}
40+
s.fillRecipientDeviceFlag(ctx, settings)
41+
return settings, nil
3442
}
3543

3644
// UpdateSettings validates and stores settings for the current admin user.
@@ -49,12 +57,46 @@ func (s *Service) UpdateSettings(ctx context.Context, adminUserID string, input
4957
}
5058
}
5159

52-
return s.repo.UpsertSettings(ctx, Settings{
60+
settings, err := s.repo.UpsertSettings(ctx, Settings{
5361
Enabled: input.Enabled,
5462
RecipientUserID: adminUserID,
5563
TopicID: input.TopicID,
5664
SignupCreatedEnabled: input.SignupCreatedEnabled,
5765
})
66+
if err != nil {
67+
return nil, err
68+
}
69+
s.fillRecipientDeviceFlag(ctx, settings)
70+
return settings, nil
71+
}
72+
73+
// fillRecipientDeviceFlag sets RecipientHasActiveDeviceForTopic on the given
74+
// settings. The check is skipped (flag stays false) when there is no topic
75+
// configured. Lookup failures are logged but not propagated: this flag is a
76+
// UI hint, never a gate.
77+
func (s *Service) fillRecipientDeviceFlag(ctx context.Context, settings *Settings) {
78+
if settings == nil || s.subscriptions == nil {
79+
return
80+
}
81+
if settings.RecipientUserID == "" || settings.TopicID == "" {
82+
return
83+
}
84+
85+
topic, err := s.topics.GetTopicByID(ctx, settings.RecipientUserID, settings.TopicID)
86+
if err != nil {
87+
s.log.Warn("failed to resolve topic for system notification device check", "error", err)
88+
return
89+
}
90+
if topic == nil {
91+
return
92+
}
93+
94+
hasDevice, err := s.subscriptions.HasActiveDeviceForTopic(ctx, settings.RecipientUserID, topic.Name)
95+
if err != nil {
96+
s.log.Warn("failed to check active device for system notification topic", "error", err)
97+
return
98+
}
99+
settings.RecipientHasActiveDeviceForTopic = hasDevice
58100
}
59101

60102
// NotifySignupCreated sends the configured notification for a newly created account.

internal/system/notifications/service_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,29 @@ func (p *testTopicProvider) GetTopicByID(ctx context.Context, userID, topicID st
2121
return &Topic{ID: t.ID, UserID: t.UserID, Name: t.Name}, nil
2222
}
2323

24+
type testDeviceSubscriptions struct {
25+
withDevice map[string]bool // key: userID + "|" + topicName
26+
}
27+
28+
func (c *testDeviceSubscriptions) HasActiveDeviceForTopic(_ context.Context, userID, topicName string) (bool, error) {
29+
return c.withDevice[userID+"|"+topicName], nil
30+
}
31+
2432
func newTestService(t *testing.T) (*Service, *topic.Service, context.Context) {
2533
t.Helper()
34+
return newTestServiceWith(t, nil)
35+
}
36+
37+
func newTestServiceWith(t *testing.T, subs DeviceSubscriptionChecker) (*Service, *topic.Service, context.Context) {
38+
t.Helper()
2639

2740
db := testutil.NewDBWithUsers(t, "admin-1", "other-1")
2841
topicSvc := topic.NewService(topic.NewRepository(db), slog.Default())
2942
svc := NewService(
3043
NewRepository(db),
3144
&testTopicProvider{svc: topicSvc},
3245
nil,
46+
subs,
3347
slog.Default(),
3448
)
3549

@@ -99,3 +113,38 @@ func TestUpdateSettingsRequiresTopicWhenEnabled(t *testing.T) {
99113
t.Fatalf("UpdateSettings() error = %v, want %v", err, ErrTopicRequired)
100114
}
101115
}
116+
117+
func TestGetSettingsReportsRecipientDevicePresence(t *testing.T) {
118+
subs := &testDeviceSubscriptions{withDevice: map[string]bool{}}
119+
svc, topicSvc, ctx := newTestServiceWith(t, subs)
120+
topicRow, err := topicSvc.CreateTopic(ctx, "admin-1", "ops", "Operational alerts")
121+
if err != nil {
122+
t.Fatalf("CreateTopic() error = %v", err)
123+
}
124+
if _, err := svc.UpdateSettings(ctx, "admin-1", UpdateSettingsRequest{
125+
Enabled: true,
126+
TopicID: topicRow.ID,
127+
SignupCreatedEnabled: true,
128+
}); err != nil {
129+
t.Fatalf("UpdateSettings() error = %v", err)
130+
}
131+
132+
// Recipient has no paired device on the topic: flag must be false.
133+
got, err := svc.GetSettings(ctx)
134+
if err != nil {
135+
t.Fatalf("GetSettings() error = %v", err)
136+
}
137+
if got.RecipientHasActiveDeviceForTopic {
138+
t.Fatalf("RecipientHasActiveDeviceForTopic = true, want false when no device is paired")
139+
}
140+
141+
// Once a device is registered for the recipient on that topic, the flag flips.
142+
subs.withDevice["admin-1|ops"] = true
143+
got, err = svc.GetSettings(ctx)
144+
if err != nil {
145+
t.Fatalf("GetSettings() error = %v", err)
146+
}
147+
if !got.RecipientHasActiveDeviceForTopic {
148+
t.Fatalf("RecipientHasActiveDeviceForTopic = false, want true once a device is paired")
149+
}
150+
}

internal/system/notifications/types.go

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ type Delivery interface {
3131
SendSystemNotification(ctx context.Context, input DeliveryInput) error
3232
}
3333

34+
// DeviceSubscriptionChecker reports whether a user has at least one active
35+
// paired device subscribed to the given topic. It is used to surface a
36+
// configuration warning when system notifications are enabled but would have
37+
// no destination at delivery time.
38+
type DeviceSubscriptionChecker interface {
39+
HasActiveDeviceForTopic(ctx context.Context, userID, topicName string) (bool, error)
40+
}
41+
3442
// DeliveryInput carries a resolved system notification delivery request.
3543
type DeliveryInput struct {
3644
RecipientUserID string
@@ -41,23 +49,31 @@ type DeliveryInput struct {
4149
}
4250

4351
// Settings is the persisted system notifications configuration.
52+
//
53+
// RecipientHasActiveDeviceForTopic is a derived (non-persisted) flag computed
54+
// at read time so the admin UI can warn when delivery would have no
55+
// destination. It is best-effort: callers may leave it false when the check
56+
// is not applicable or fails.
4457
type Settings struct {
4558
Enabled bool `db:"enabled"`
4659
RecipientUserID string `db:"recipient_user_id"`
4760
TopicID string `db:"topic_id"`
4861
SignupCreatedEnabled bool `db:"signup_created_enabled"`
4962
CreatedAt int64 `db:"created_at"`
5063
UpdatedAt int64 `db:"updated_at"`
64+
65+
RecipientHasActiveDeviceForTopic bool `db:"-"`
5166
}
5267

5368
// SettingsResponse is the admin API response for system notification settings.
5469
type SettingsResponse struct {
55-
Enabled bool `json:"enabled"`
56-
RecipientUserID string `json:"recipient_user_id,omitempty"`
57-
TopicID string `json:"topic_id,omitempty"`
58-
SignupCreatedEnabled bool `json:"signup_created_enabled"`
59-
CreatedAt time.Time `json:"created_at,omitempty"`
60-
UpdatedAt time.Time `json:"updated_at,omitempty"`
70+
Enabled bool `json:"enabled"`
71+
RecipientUserID string `json:"recipient_user_id,omitempty"`
72+
TopicID string `json:"topic_id,omitempty"`
73+
SignupCreatedEnabled bool `json:"signup_created_enabled"`
74+
RecipientHasActiveDeviceForTopic bool `json:"recipient_has_active_device_for_topic"`
75+
CreatedAt time.Time `json:"created_at,omitempty"`
76+
UpdatedAt time.Time `json:"updated_at,omitempty"`
6177
}
6278

6379
// UpdateSettingsRequest is the admin API request for system notification settings.
@@ -74,12 +90,13 @@ func ToSettingsResponse(settings *Settings) SettingsResponse {
7490
}
7591

7692
return SettingsResponse{
77-
Enabled: settings.Enabled,
78-
RecipientUserID: settings.RecipientUserID,
79-
TopicID: settings.TopicID,
80-
SignupCreatedEnabled: settings.SignupCreatedEnabled,
81-
CreatedAt: time.UnixMilli(settings.CreatedAt).UTC(),
82-
UpdatedAt: time.UnixMilli(settings.UpdatedAt).UTC(),
93+
Enabled: settings.Enabled,
94+
RecipientUserID: settings.RecipientUserID,
95+
TopicID: settings.TopicID,
96+
SignupCreatedEnabled: settings.SignupCreatedEnabled,
97+
RecipientHasActiveDeviceForTopic: settings.RecipientHasActiveDeviceForTopic,
98+
CreatedAt: time.UnixMilli(settings.CreatedAt).UTC(),
99+
UpdatedAt: time.UnixMilli(settings.UpdatedAt).UTC(),
83100
}
84101
}
85102

web/apps/site/src/routes/(protected)/admin/system/notifications/+page.svelte

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@
1212
let signupCreatedEnabled = $state(false);
1313
let selectedTopicID = $state('');
1414
let recipientUserID = $state('');
15+
let recipientHasActiveDeviceForTopic = $state(false);
16+
// Tracks the topic the server actually computed the device flag against, so
17+
// we only show the "no paired device" warning when the dropdown still
18+
// matches the persisted selection.
19+
let persistedTopicID = $state('');
20+
let persistedEnabled = $state(false);
1521
1622
onMount(async () => {
1723
await loadSettings();
@@ -30,6 +36,9 @@
3036
signupCreatedEnabled = settings.signup_created_enabled;
3137
selectedTopicID = settings.topic_id ?? '';
3238
recipientUserID = settings.recipient_user_id ?? '';
39+
recipientHasActiveDeviceForTopic = settings.recipient_has_active_device_for_topic;
40+
persistedTopicID = settings.topic_id ?? '';
41+
persistedEnabled = settings.enabled;
3342
} catch (err) {
3443
toast.error(
3544
err instanceof ApiError ? err.userMessage : 'Failed to load system notifications'
@@ -53,6 +62,9 @@
5362
signup_created_enabled: signupCreatedEnabled
5463
});
5564
recipientUserID = settings.recipient_user_id ?? '';
65+
recipientHasActiveDeviceForTopic = settings.recipient_has_active_device_for_topic;
66+
persistedTopicID = settings.topic_id ?? '';
67+
persistedEnabled = settings.enabled;
5668
toast.success('System notification settings saved');
5769
} catch (err) {
5870
toast.error(err instanceof ApiError ? err.userMessage : 'Failed to save settings');
@@ -143,6 +155,15 @@
143155
</div>
144156
{/if}
145157

158+
{#if persistedEnabled && persistedTopicID !== '' && persistedTopicID === selectedTopicID && !recipientHasActiveDeviceForTopic}
159+
<div class="alert alert-warning">
160+
<span>
161+
No paired device is subscribed to this topic for the recipient admin. System
162+
notifications will be silently dropped until a device is paired and subscribed.
163+
</span>
164+
</div>
165+
{/if}
166+
146167
<div class="card-actions justify-end">
147168
<button type="button" class="btn btn-primary" disabled={!canSave} onclick={saveSettings}>
148169
{#if saving}

web/packages/shared/src/api/admin.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ export interface SystemNotificationSettings {
5959
recipient_user_id?: string;
6060
topic_id?: string;
6161
signup_created_enabled: boolean;
62+
/**
63+
* Best-effort flag computed at read time: true when the recipient admin
64+
* has at least one paired device subscribed to the configured topic.
65+
* Used by the admin UI to warn about a misconfiguration that would
66+
* silently drop deliveries (TotalSent=0).
67+
*/
68+
recipient_has_active_device_for_topic: boolean;
6269
created_at?: string;
6370
updated_at?: string;
6471
}

0 commit comments

Comments
 (0)