diff --git a/server/cmd/server/notification_listeners.go b/server/cmd/server/notification_listeners.go index 8f5271019..e29d77724 100644 --- a/server/cmd/server/notification_listeners.go +++ b/server/cmd/server/notification_listeners.go @@ -66,11 +66,20 @@ func parseMentions(content string) []mention { return result } +// parentBubbleNotifTypes is the allowlist of inbox notification types that +// bubble up from a sub-issue to subscribers of its parent. Other event types +// only notify subscribers of the sub-issue itself, to keep parent watchers' +// inboxes focused on the signal that matters most: status transitions. +var parentBubbleNotifTypes = map[string]bool{ + "status_changed": true, +} + // notifySubscribers queries the subscriber table for an issue, excludes the // actor and any extra IDs, and creates inbox items for each remaining member // subscriber. Publishes an inbox:new event for each notification. -// If the issue has a parent, parent issue subscribers are also notified -// (deduplicated against direct subscribers). +// If the issue has a parent and the notification type is in the bubble +// allowlist, parent issue subscribers are also notified (deduplicated +// against direct subscribers). func notifySubscribers( ctx context.Context, queries *db.Queries, @@ -90,6 +99,11 @@ func notifySubscribers( issueID, issueID, issueStatus, workspaceID, e, exclude, notifType, severity, title, body, details) + // Only a small allowlist of event types bubbles to parent subscribers. + if !parentBubbleNotifTypes[notifType] { + return + } + // Also notify parent issue subscribers if this is a sub-issue. issue, err := queries.GetIssue(ctx, parseUUID(issueID)) if err != nil { diff --git a/server/cmd/server/notification_listeners_test.go b/server/cmd/server/notification_listeners_test.go index 638ca813e..031cba85b 100644 --- a/server/cmd/server/notification_listeners_test.go +++ b/server/cmd/server/notification_listeners_test.go @@ -47,6 +47,26 @@ func addTestSubscriber(t *testing.T, issueID, userType, userID, reason string) { } } +// createTestSubIssue inserts an issue with parent_issue_id set and returns its UUID. +// Picks the next per-workspace number to avoid colliding with the +// uq_issue_workspace_number unique constraint (parent + sub created in the +// same test would otherwise both default to number=0). +func createTestSubIssue(t *testing.T, workspaceID, creatorID, parentIssueID string) string { + t.Helper() + ctx := context.Background() + var issueID string + err := testPool.QueryRow(ctx, ` + INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, position, parent_issue_id, number) + VALUES ($1, 'sub-issue test', 'todo', 'medium', 'member', $2, 0, $3, + (SELECT COALESCE(MAX(number), 0) + 1 FROM issue WHERE workspace_id = $1)) + RETURNING id + `, workspaceID, creatorID, parentIssueID).Scan(&issueID) + if err != nil { + t.Fatalf("createTestSubIssue: %v", err) + } + return issueID +} + // newNotificationBus creates a bus with subscriber + notification listeners registered. func newNotificationBus(t *testing.T, queries *db.Queries) *events.Bus { t.Helper() @@ -675,3 +695,168 @@ func TestNotification_DueDateChanged(t *testing.T) { t.Fatalf("expected severity 'info', got %q", sub1Items[0].Severity) } } + +// TestNotification_ParentBubble_StatusChanged verifies that a status_changed +// event on a sub-issue bubbles to subscribers of the parent issue. +func TestNotification_ParentBubble_StatusChanged(t *testing.T) { + queries := db.New(testPool) + bus := newNotificationBus(t, queries) + + parentSubEmail := "notif-parent-sub-status@multica.ai" + parentSubID := createTestUser(t, parentSubEmail) + t.Cleanup(func() { cleanupTestUser(t, parentSubEmail) }) + + parentID := createTestIssue(t, testWorkspaceID, testUserID) + t.Cleanup(func() { + cleanupInboxForIssue(t, parentID) + cleanupTestIssue(t, parentID) + }) + subID := createTestSubIssue(t, testWorkspaceID, testUserID, parentID) + t.Cleanup(func() { + cleanupInboxForIssue(t, subID) + cleanupTestIssue(t, subID) + }) + + // Subscribe a watcher to the parent only — they should hear about + // status changes on the sub-issue. + addTestSubscriber(t, parentID, "member", parentSubID, "manual") + + bus.Publish(events.Event{ + Type: protocol.EventIssueUpdated, + WorkspaceID: testWorkspaceID, + ActorType: "member", + ActorID: testUserID, + Payload: map[string]any{ + "issue": handler.IssueResponse{ + ID: subID, + WorkspaceID: testWorkspaceID, + Title: "sub-issue status bubble", + Status: "done", + Priority: "medium", + CreatorType: "member", + CreatorID: testUserID, + }, + "assignee_changed": false, + "status_changed": true, + "prev_status": "in_progress", + }, + }) + + items := inboxItemsForRecipient(t, queries, parentSubID) + if len(items) != 1 { + t.Fatalf("expected 1 inbox item bubbled to parent subscriber, got %d", len(items)) + } + if items[0].Type != "status_changed" { + t.Fatalf("expected type 'status_changed', got %q", items[0].Type) + } + // The inbox item should point to the sub-issue, not the parent. + if util.UUIDToString(items[0].IssueID) != subID { + t.Fatalf("expected inbox item issue_id=%s (sub-issue), got %s", + subID, util.UUIDToString(items[0].IssueID)) + } +} + +// TestNotification_ParentBubble_NewCommentSuppressed verifies that comments +// on a sub-issue do NOT bubble to subscribers of the parent issue. Comments +// are the loudest signal and we explicitly want to keep them off the parent +// watcher's inbox. +func TestNotification_ParentBubble_NewCommentSuppressed(t *testing.T) { + queries := db.New(testPool) + bus := newNotificationBus(t, queries) + + commenterEmail := "notif-parent-bubble-commenter@multica.ai" + commenterID := createTestUser(t, commenterEmail) + t.Cleanup(func() { cleanupTestUser(t, commenterEmail) }) + + parentSubEmail := "notif-parent-sub-comment@multica.ai" + parentSubID := createTestUser(t, parentSubEmail) + t.Cleanup(func() { cleanupTestUser(t, parentSubEmail) }) + + parentID := createTestIssue(t, testWorkspaceID, testUserID) + t.Cleanup(func() { + cleanupInboxForIssue(t, parentID) + cleanupTestIssue(t, parentID) + }) + subID := createTestSubIssue(t, testWorkspaceID, testUserID, parentID) + t.Cleanup(func() { + cleanupInboxForIssue(t, subID) + cleanupTestIssue(t, subID) + }) + + addTestSubscriber(t, parentID, "member", parentSubID, "manual") + + bus.Publish(events.Event{ + Type: protocol.EventCommentCreated, + WorkspaceID: testWorkspaceID, + ActorType: "member", + ActorID: commenterID, + Payload: map[string]any{ + "comment": handler.CommentResponse{ + ID: "00000000-0000-0000-0000-000000000000", + IssueID: subID, + AuthorType: "member", + AuthorID: commenterID, + Content: "comment on sub-issue", + Type: "comment", + }, + "issue_title": "sub-issue comment bubble", + "issue_status": "todo", + }, + }) + + items := inboxItemsForRecipient(t, queries, parentSubID) + if len(items) != 0 { + t.Fatalf("expected 0 inbox items bubbled to parent subscriber for new_comment, got %d", len(items)) + } +} + +// TestNotification_ParentBubble_PriorityChangeSuppressed verifies that a +// priority change on a sub-issue does NOT bubble to parent subscribers. +func TestNotification_ParentBubble_PriorityChangeSuppressed(t *testing.T) { + queries := db.New(testPool) + bus := newNotificationBus(t, queries) + + parentSubEmail := "notif-parent-sub-priority@multica.ai" + parentSubID := createTestUser(t, parentSubEmail) + t.Cleanup(func() { cleanupTestUser(t, parentSubEmail) }) + + parentID := createTestIssue(t, testWorkspaceID, testUserID) + t.Cleanup(func() { + cleanupInboxForIssue(t, parentID) + cleanupTestIssue(t, parentID) + }) + subID := createTestSubIssue(t, testWorkspaceID, testUserID, parentID) + t.Cleanup(func() { + cleanupInboxForIssue(t, subID) + cleanupTestIssue(t, subID) + }) + + addTestSubscriber(t, parentID, "member", parentSubID, "manual") + + bus.Publish(events.Event{ + Type: protocol.EventIssueUpdated, + WorkspaceID: testWorkspaceID, + ActorType: "member", + ActorID: testUserID, + Payload: map[string]any{ + "issue": handler.IssueResponse{ + ID: subID, + WorkspaceID: testWorkspaceID, + Title: "sub-issue priority bubble", + Status: "todo", + Priority: "high", + CreatorType: "member", + CreatorID: testUserID, + }, + "assignee_changed": false, + "status_changed": false, + "priority_changed": true, + "prev_priority": "medium", + }, + }) + + items := inboxItemsForRecipient(t, queries, parentSubID) + if len(items) != 0 { + t.Fatalf("expected 0 inbox items bubbled to parent subscriber for priority_changed, got %d", len(items)) + } +} diff --git a/server/cmd/server/subscriber_listeners_test.go b/server/cmd/server/subscriber_listeners_test.go index 31fedab0d..64565315a 100644 --- a/server/cmd/server/subscriber_listeners_test.go +++ b/server/cmd/server/subscriber_listeners_test.go @@ -16,13 +16,17 @@ import ( // (testPool, testUserID, testWorkspaceID are set in integration_test.go). // createTestIssue inserts a minimal issue and returns its UUID string. +// Picks the next per-workspace number to avoid colliding with the +// uq_issue_workspace_number unique constraint when a single test creates +// multiple issues. func createTestIssue(t *testing.T, workspaceID, creatorID string) string { t.Helper() ctx := context.Background() var issueID string err := testPool.QueryRow(ctx, ` - INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, position) - VALUES ($1, 'subscriber test issue', 'todo', 'medium', 'member', $2, 0) + INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, position, number) + VALUES ($1, 'subscriber test issue', 'todo', 'medium', 'member', $2, 0, + (SELECT COALESCE(MAX(number), 0) + 1 FROM issue WHERE workspace_id = $1)) RETURNING id `, workspaceID, creatorID).Scan(&issueID) if err != nil {