Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 37 additions & 2 deletions cmd/beebuzz-server/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion cmd/beebuzz-server/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 8 additions & 1 deletion docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
66 changes: 54 additions & 12 deletions internal/system/notifications/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
49 changes: 49 additions & 0 deletions internal/system/notifications/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,29 @@ 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())
svc := NewService(
NewRepository(db),
&testTopicProvider{svc: topicSvc},
nil,
subs,
slog.Default(),
)

Expand Down Expand Up @@ -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")
}
}
41 changes: 29 additions & 12 deletions internal/system/notifications/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -41,23 +49,31 @@ 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"`
TopicID string `db:"topic_id"`
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.
Expand All @@ -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(),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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'
Expand All @@ -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');
Expand Down Expand Up @@ -143,6 +155,15 @@
</div>
{/if}

{#if persistedEnabled && persistedTopicID !== '' && persistedTopicID === selectedTopicID && !recipientHasActiveDeviceForTopic}
<div class="alert alert-warning">
<span>
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.
</span>
</div>
{/if}

<div class="card-actions justify-end">
<button type="button" class="btn btn-primary" disabled={!canSave} onclick={saveSettings}>
{#if saving}
Expand Down
7 changes: 7 additions & 0 deletions web/packages/shared/src/api/admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ export interface SystemNotificationSettings {
recipient_user_id?: string;
topic_id?: string;
signup_created_enabled: boolean;
/**
* 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 (TotalSent=0).
*/
recipient_has_active_device_for_topic: boolean;
created_at?: string;
updated_at?: string;
}
Expand Down
Loading