mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-17 03:38:32 +02:00
feat(notifications): only bubble status_changed from sub-issue to parent subscribers (MUL-1189) (#1481)
* feat(notifications): only bubble status_changed from sub-issue to parent subscribers (MUL-1189) Subscribing to a parent issue used to surface every event from every sub-issue in the inbox — comments, priority/due-date tweaks, assignee shuffles, the lot — which drowned out the signal that actually matters to a parent watcher: "did the sub-task move forward?". notifySubscribers now consults a small allowlist (parentBubbleNotifTypes) before walking up to the parent's subscriber list. Only status_changed bubbles today; sub-issue subscribers themselves still get every event. Direct notifications (issue_assigned, mentioned, task_failed targeted at specific recipients) are unaffected — they go through notifyDirect, not the parent-bubble path. Tests cover the three behaviors that matter: - status_changed on a sub-issue reaches the parent's subscriber, with the inbox item still pointing at the sub-issue (so the user lands on the actual change). - new_comment on a sub-issue does NOT bubble. - priority_changed on a sub-issue does NOT bubble. * fix(test): pick next per-workspace issue number in test helpers Both createTestIssue and createTestSubIssue inserted with the default number=0, which collides with the uq_issue_workspace_number unique constraint as soon as a single test creates two issues in the same workspace (e.g. parent + sub-issue). The first failure also leaked the parent row because t.Cleanup hadn't been registered yet, breaking every subsequent test in the package. Both helpers now compute number as MAX(number)+1 for the workspace, and the parent-bubble tests register cleanup right after each insert so a mid-test failure can't leave orphans.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user