diff --git a/cmd/beebuzz-server/adapter.go b/cmd/beebuzz-server/adapter.go index 6a1e119..40fd10b 100644 --- a/cmd/beebuzz-server/adapter.go +++ b/cmd/beebuzz-server/adapter.go @@ -154,6 +154,26 @@ func (a *systemNotificationTopicProviderAdapter) GetTopicByID(ctx context.Contex var _ systemnotifications.TopicProvider = (*systemNotificationTopicProviderAdapter)(nil) +// systemNotificationDeviceSubscriptionAdapter adapts device.Service to the +// system notifications device subscription checker. It is used by the admin +// settings response to flag the misconfiguration where notifications are +// enabled but the recipient has no paired device on the chosen topic. +type systemNotificationDeviceSubscriptionAdapter struct { + deviceSvc *device.Service +} + +// HasActiveDeviceForTopic reports whether the user has at least one paired +// device subscribed to topicName. +func (a *systemNotificationDeviceSubscriptionAdapter) HasActiveDeviceForTopic(ctx context.Context, userID, topicName string) (bool, error) { + subs, err := a.deviceSvc.GetSubscribedDevices(ctx, userID, topicName) + if err != nil { + return false, err + } + return len(subs) > 0, nil +} + +var _ systemnotifications.DeviceSubscriptionChecker = (*systemNotificationDeviceSubscriptionAdapter)(nil) + // pushAuthorizerAdapter adapts token.Service to notification.PushAuthorizer. type pushAuthorizerAdapter struct { tokenSvc *token.Service @@ -242,15 +262,30 @@ type systemNotificationDeliveryAdapter struct { } // SendSystemNotification sends a server-trusted notification generated by BeeBuzz itself. +// +// A zero TotalSent is logged at warn level: the call succeeded but no paired +// device on the configured topic received the push, which usually means the +// admin enabled system notifications without pairing a device on that topic. +// Without this signal the misconfiguration is silent in production. func (a *systemNotificationDeliveryAdapter) SendSystemNotification(ctx context.Context, input systemnotifications.DeliveryInput) error { - _, err := a.notifSvc.Send(ctx, input.RecipientUserID, input.TopicID, notification.SendInput{ + report, err := a.notifSvc.Send(ctx, input.RecipientUserID, input.TopicID, notification.SendInput{ TopicName: input.TopicName, Title: input.Title, Body: input.Body, Source: event.SourceInternal, DeliveryMode: notification.DeliveryModeServerTrusted, }, a.log) - return err + if err != nil { + return err + } + if report != nil && report.TotalSent == 0 { + a.log.Warn("system notification delivered to zero devices", + "recipient_user_id", input.RecipientUserID, + "topic_id", input.TopicID, + "topic", input.TopicName, + ) + } + return nil } var _ systemnotifications.Delivery = (*systemNotificationDeliveryAdapter)(nil) diff --git a/cmd/beebuzz-server/serve.go b/cmd/beebuzz-server/serve.go index 0f85552..46e7658 100644 --- a/cmd/beebuzz-server/serve.go +++ b/cmd/beebuzz-server/serve.go @@ -174,7 +174,8 @@ func buildServices(db *sqlx.DB, cfg *config.Config, log *slog.Logger, m mailer.M systemNotifRepo := systemnotifications.NewRepository(db) systemNotifTopics := &systemNotificationTopicProviderAdapter{topicSvc: topicSvc} systemNotifDelivery := &systemNotificationDeliveryAdapter{notifSvc: notifSvc, log: log} - systemNotifSvc := systemnotifications.NewService(systemNotifRepo, systemNotifTopics, systemNotifDelivery, log) + systemNotifSubscriptions := &systemNotificationDeviceSubscriptionAdapter{deviceSvc: deviceSvc} + systemNotifSvc := systemnotifications.NewService(systemNotifRepo, systemNotifTopics, systemNotifDelivery, systemNotifSubscriptions, log) authSvc.SetSignupNotifier(systemNotifSvc) webhookRepo := webhook.NewRepository(db) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index b8e7c69..9e47fed 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2219,7 +2219,7 @@ components: $ref: "#/components/schemas/AdminUserResponse" SystemNotificationSettingsResponse: type: object - required: [enabled, signup_created_enabled] + required: [enabled, signup_created_enabled, recipient_has_active_device_for_topic] properties: enabled: type: boolean @@ -2229,6 +2229,13 @@ components: type: string signup_created_enabled: type: boolean + recipient_has_active_device_for_topic: + type: boolean + description: | + Best-effort flag computed at read time. True when the recipient + admin has at least one paired device subscribed to the configured + topic. Used by the admin UI to warn about a misconfiguration that + would silently drop deliveries. created_at: type: string format: date-time diff --git a/internal/system/notifications/service.go b/internal/system/notifications/service.go index 7ea787e..f86a2b2 100644 --- a/internal/system/notifications/service.go +++ b/internal/system/notifications/service.go @@ -12,25 +12,33 @@ const signupCreatedTitle = "New BeeBuzz signup" // Service owns system notification policy and dispatch decisions. type Service struct { - repo *Repository - topics TopicProvider - delivery Delivery - log *slog.Logger + repo *Repository + topics TopicProvider + delivery Delivery + subscriptions DeviceSubscriptionChecker + log *slog.Logger } // NewService creates a system notifications service. -func NewService(repo *Repository, topics TopicProvider, delivery Delivery, log *slog.Logger) *Service { +func NewService(repo *Repository, topics TopicProvider, delivery Delivery, subscriptions DeviceSubscriptionChecker, log *slog.Logger) *Service { return &Service{ - repo: repo, - topics: topics, - delivery: delivery, - log: log, + repo: repo, + topics: topics, + delivery: delivery, + subscriptions: subscriptions, + log: log, } } -// GetSettings returns the current singleton settings. +// GetSettings returns the current singleton settings, enriched with the +// best-effort RecipientHasActiveDeviceForTopic flag for the admin UI. func (s *Service) GetSettings(ctx context.Context) (*Settings, error) { - return s.repo.GetSettings(ctx) + settings, err := s.repo.GetSettings(ctx) + if err != nil { + return nil, err + } + s.fillRecipientDeviceFlag(ctx, settings) + return settings, nil } // UpdateSettings validates and stores settings for the current admin user. @@ -49,12 +57,46 @@ func (s *Service) UpdateSettings(ctx context.Context, adminUserID string, input } } - return s.repo.UpsertSettings(ctx, Settings{ + settings, err := s.repo.UpsertSettings(ctx, Settings{ Enabled: input.Enabled, RecipientUserID: adminUserID, TopicID: input.TopicID, SignupCreatedEnabled: input.SignupCreatedEnabled, }) + if err != nil { + return nil, err + } + s.fillRecipientDeviceFlag(ctx, settings) + return settings, nil +} + +// fillRecipientDeviceFlag sets RecipientHasActiveDeviceForTopic on the given +// settings. The check is skipped (flag stays false) when there is no topic +// configured. Lookup failures are logged but not propagated: this flag is a +// UI hint, never a gate. +func (s *Service) fillRecipientDeviceFlag(ctx context.Context, settings *Settings) { + if settings == nil || s.subscriptions == nil { + return + } + if settings.RecipientUserID == "" || settings.TopicID == "" { + return + } + + topic, err := s.topics.GetTopicByID(ctx, settings.RecipientUserID, settings.TopicID) + if err != nil { + s.log.Warn("failed to resolve topic for system notification device check", "error", err) + return + } + if topic == nil { + return + } + + hasDevice, err := s.subscriptions.HasActiveDeviceForTopic(ctx, settings.RecipientUserID, topic.Name) + if err != nil { + s.log.Warn("failed to check active device for system notification topic", "error", err) + return + } + settings.RecipientHasActiveDeviceForTopic = hasDevice } // NotifySignupCreated sends the configured notification for a newly created account. diff --git a/internal/system/notifications/service_test.go b/internal/system/notifications/service_test.go index 9f544ae..0d95498 100644 --- a/internal/system/notifications/service_test.go +++ b/internal/system/notifications/service_test.go @@ -21,8 +21,21 @@ func (p *testTopicProvider) GetTopicByID(ctx context.Context, userID, topicID st return &Topic{ID: t.ID, UserID: t.UserID, Name: t.Name}, nil } +type testDeviceSubscriptions struct { + withDevice map[string]bool // key: userID + "|" + topicName +} + +func (c *testDeviceSubscriptions) HasActiveDeviceForTopic(_ context.Context, userID, topicName string) (bool, error) { + return c.withDevice[userID+"|"+topicName], nil +} + func newTestService(t *testing.T) (*Service, *topic.Service, context.Context) { t.Helper() + return newTestServiceWith(t, nil) +} + +func newTestServiceWith(t *testing.T, subs DeviceSubscriptionChecker) (*Service, *topic.Service, context.Context) { + t.Helper() db := testutil.NewDBWithUsers(t, "admin-1", "other-1") topicSvc := topic.NewService(topic.NewRepository(db), slog.Default()) @@ -30,6 +43,7 @@ func newTestService(t *testing.T) (*Service, *topic.Service, context.Context) { NewRepository(db), &testTopicProvider{svc: topicSvc}, nil, + subs, slog.Default(), ) @@ -99,3 +113,38 @@ func TestUpdateSettingsRequiresTopicWhenEnabled(t *testing.T) { t.Fatalf("UpdateSettings() error = %v, want %v", err, ErrTopicRequired) } } + +func TestGetSettingsReportsRecipientDevicePresence(t *testing.T) { + subs := &testDeviceSubscriptions{withDevice: map[string]bool{}} + svc, topicSvc, ctx := newTestServiceWith(t, subs) + topicRow, err := topicSvc.CreateTopic(ctx, "admin-1", "ops", "Operational alerts") + if err != nil { + t.Fatalf("CreateTopic() error = %v", err) + } + if _, err := svc.UpdateSettings(ctx, "admin-1", UpdateSettingsRequest{ + Enabled: true, + TopicID: topicRow.ID, + SignupCreatedEnabled: true, + }); err != nil { + t.Fatalf("UpdateSettings() error = %v", err) + } + + // Recipient has no paired device on the topic: flag must be false. + got, err := svc.GetSettings(ctx) + if err != nil { + t.Fatalf("GetSettings() error = %v", err) + } + if got.RecipientHasActiveDeviceForTopic { + t.Fatalf("RecipientHasActiveDeviceForTopic = true, want false when no device is paired") + } + + // Once a device is registered for the recipient on that topic, the flag flips. + subs.withDevice["admin-1|ops"] = true + got, err = svc.GetSettings(ctx) + if err != nil { + t.Fatalf("GetSettings() error = %v", err) + } + if !got.RecipientHasActiveDeviceForTopic { + t.Fatalf("RecipientHasActiveDeviceForTopic = false, want true once a device is paired") + } +} diff --git a/internal/system/notifications/types.go b/internal/system/notifications/types.go index 8fc7584..333250e 100644 --- a/internal/system/notifications/types.go +++ b/internal/system/notifications/types.go @@ -31,6 +31,14 @@ type Delivery interface { SendSystemNotification(ctx context.Context, input DeliveryInput) error } +// DeviceSubscriptionChecker reports whether a user has at least one active +// paired device subscribed to the given topic. It is used to surface a +// configuration warning when system notifications are enabled but would have +// no destination at delivery time. +type DeviceSubscriptionChecker interface { + HasActiveDeviceForTopic(ctx context.Context, userID, topicName string) (bool, error) +} + // DeliveryInput carries a resolved system notification delivery request. type DeliveryInput struct { RecipientUserID string @@ -41,6 +49,11 @@ type DeliveryInput struct { } // Settings is the persisted system notifications configuration. +// +// RecipientHasActiveDeviceForTopic is a derived (non-persisted) flag computed +// at read time so the admin UI can warn when delivery would have no +// destination. It is best-effort: callers may leave it false when the check +// is not applicable or fails. type Settings struct { Enabled bool `db:"enabled"` RecipientUserID string `db:"recipient_user_id"` @@ -48,16 +61,19 @@ type Settings struct { SignupCreatedEnabled bool `db:"signup_created_enabled"` CreatedAt int64 `db:"created_at"` UpdatedAt int64 `db:"updated_at"` + + RecipientHasActiveDeviceForTopic bool `db:"-"` } // SettingsResponse is the admin API response for system notification settings. type SettingsResponse struct { - Enabled bool `json:"enabled"` - RecipientUserID string `json:"recipient_user_id,omitempty"` - TopicID string `json:"topic_id,omitempty"` - SignupCreatedEnabled bool `json:"signup_created_enabled"` - CreatedAt time.Time `json:"created_at,omitempty"` - UpdatedAt time.Time `json:"updated_at,omitempty"` + Enabled bool `json:"enabled"` + RecipientUserID string `json:"recipient_user_id,omitempty"` + TopicID string `json:"topic_id,omitempty"` + SignupCreatedEnabled bool `json:"signup_created_enabled"` + RecipientHasActiveDeviceForTopic bool `json:"recipient_has_active_device_for_topic"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` } // UpdateSettingsRequest is the admin API request for system notification settings. @@ -74,12 +90,13 @@ func ToSettingsResponse(settings *Settings) SettingsResponse { } return SettingsResponse{ - Enabled: settings.Enabled, - RecipientUserID: settings.RecipientUserID, - TopicID: settings.TopicID, - SignupCreatedEnabled: settings.SignupCreatedEnabled, - CreatedAt: time.UnixMilli(settings.CreatedAt).UTC(), - UpdatedAt: time.UnixMilli(settings.UpdatedAt).UTC(), + Enabled: settings.Enabled, + RecipientUserID: settings.RecipientUserID, + TopicID: settings.TopicID, + SignupCreatedEnabled: settings.SignupCreatedEnabled, + RecipientHasActiveDeviceForTopic: settings.RecipientHasActiveDeviceForTopic, + CreatedAt: time.UnixMilli(settings.CreatedAt).UTC(), + UpdatedAt: time.UnixMilli(settings.UpdatedAt).UTC(), } } diff --git a/web/apps/site/src/routes/(protected)/admin/system/notifications/+page.svelte b/web/apps/site/src/routes/(protected)/admin/system/notifications/+page.svelte index ede6267..915a69c 100644 --- a/web/apps/site/src/routes/(protected)/admin/system/notifications/+page.svelte +++ b/web/apps/site/src/routes/(protected)/admin/system/notifications/+page.svelte @@ -12,6 +12,12 @@ let signupCreatedEnabled = $state(false); let selectedTopicID = $state(''); let recipientUserID = $state(''); + let recipientHasActiveDeviceForTopic = $state(false); + // Tracks the topic the server actually computed the device flag against, so + // we only show the "no paired device" warning when the dropdown still + // matches the persisted selection. + let persistedTopicID = $state(''); + let persistedEnabled = $state(false); onMount(async () => { await loadSettings(); @@ -30,6 +36,9 @@ signupCreatedEnabled = settings.signup_created_enabled; selectedTopicID = settings.topic_id ?? ''; recipientUserID = settings.recipient_user_id ?? ''; + recipientHasActiveDeviceForTopic = settings.recipient_has_active_device_for_topic; + persistedTopicID = settings.topic_id ?? ''; + persistedEnabled = settings.enabled; } catch (err) { toast.error( err instanceof ApiError ? err.userMessage : 'Failed to load system notifications' @@ -53,6 +62,9 @@ signup_created_enabled: signupCreatedEnabled }); recipientUserID = settings.recipient_user_id ?? ''; + recipientHasActiveDeviceForTopic = settings.recipient_has_active_device_for_topic; + persistedTopicID = settings.topic_id ?? ''; + persistedEnabled = settings.enabled; toast.success('System notification settings saved'); } catch (err) { toast.error(err instanceof ApiError ? err.userMessage : 'Failed to save settings'); @@ -143,6 +155,15 @@ {/if} + {#if persistedEnabled && persistedTopicID !== '' && persistedTopicID === selectedTopicID && !recipientHasActiveDeviceForTopic} +
+ + No paired device is subscribed to this topic for the recipient admin. System + notifications will be silently dropped until a device is paired and subscribed. + +
+ {/if} +