From d6eaecd52e4b709c2f1261f881dbf1ff06f04db5 Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Mon, 8 Dec 2025 23:34:14 +0900 Subject: [PATCH] appview/notify: merge new comment events into one Change-Id: wnrvrwyvrlzozvsxuswtsnzolrprqlpt Signed-off-by: Seongmin Lee --- appview/issues/issues.go | 2 +- appview/notify/db/db.go | 229 ++++++++++++++--------------- appview/notify/merged_notifier.go | 16 +- appview/notify/notifier.go | 14 +- appview/notify/posthog/notifier.go | 22 +-- appview/pulls/pulls.go | 2 +- 6 files changed, 132 insertions(+), 153 deletions(-) diff --git a/appview/issues/issues.go b/appview/issues/issues.go index 6f361030..b4a4ab19 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -489,7 +489,7 @@ func (rp *Issues) NewIssueComment(w http.ResponseWriter, r *http.Request) { // reset atUri to make rollback a no-op atUri = "" - rp.notifier.NewIssueComment(r.Context(), &comment, mentions) + rp.notifier.NewComment(r.Context(), &comment) ownerSlashRepo := reporesolver.GetBaseRepoPath(r, f) rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d#comment-%d", ownerSlashRepo, issue.IssueId, comment.Id)) diff --git a/appview/notify/db/db.go b/appview/notify/db/db.go index ac0c0ac5..dc72f44f 100644 --- a/appview/notify/db/db.go +++ b/appview/notify/db/db.go @@ -74,36 +74,110 @@ func (n *databaseNotifier) DeleteStar(ctx context.Context, star *models.Star) { // no-op } -func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) { - collaborators, err := db.GetCollaborators(n.db, orm.FilterEq("repo_at", issue.Repo.RepoAt())) +func (n *databaseNotifier) NewComment(ctx context.Context, comment *models.Comment) { + var ( + // built the recipients list: + // - the owner of the repo + // - | if the comment is a reply -> everybody on that thread + // | if the comment is a top level -> just the issue owner + // - remove mentioned users from the recipients list + recipients = sets.New[syntax.DID]() + entityType string + entityId string + repoId *int64 + issueId *int64 + pullId *int64 + ) + + subjectDid, err := comment.Subject.Authority().AsDID() if err != nil { - log.Printf("failed to fetch collaborators: %v", err) + log.Printf("NewComment: expected did based at-uri for comment.subject") return } + switch comment.Subject.Collection() { + case tangled.RepoIssueNSID: + issues, err := db.GetIssues( + n.db, + orm.FilterEq("did", subjectDid), + orm.FilterEq("rkey", comment.Subject.RecordKey()), + ) + if err != nil { + log.Printf("NewComment: failed to get issues: %v", err) + return + } + if len(issues) == 0 { + log.Printf("NewComment: no issue found for %s", comment.Subject) + return + } + issue := issues[0] + + recipients.Insert(syntax.DID(issue.Repo.Did)) + if comment.IsReply() { + // if this comment is a reply, then notify everybody in that thread + parentAtUri := *comment.ReplyTo + + // find the parent thread, and add all DIDs from here to the recipient list + for _, t := range issue.CommentList() { + if t.Self.AtUri() == parentAtUri { + for _, p := range t.Participants() { + recipients.Insert(p) + } + } + } + } else { + // not a reply, notify just the issue author + recipients.Insert(syntax.DID(issue.Did)) + } - // build the recipients list - // - owner of the repo - // - collaborators in the repo - // - remove users already mentioned - recipients := sets.Singleton(syntax.DID(issue.Repo.Did)) - for _, c := range collaborators { - recipients.Insert(c.SubjectDid) + entityType = "issue" + entityId = issue.AtUri().String() + repoId = &issue.Repo.Id + issueId = &issue.Id + case tangled.RepoPullNSID: + pulls, err := db.GetPullsWithLimit( + n.db, + 1, + orm.FilterEq("owner_did", subjectDid), + orm.FilterEq("rkey", comment.Subject.RecordKey()), + ) + if err != nil { + log.Printf("NewComment: failed to get pulls: %v", err) + return + } + if len(pulls) == 0 { + log.Printf("NewComment: no pull found for %s", comment.Subject) + return + } + pull := pulls[0] + + pull.Repo, err = db.GetRepo(n.db, orm.FilterEq("at_uri", pull.RepoAt)) + if err != nil { + log.Printf("NewComment: failed to get repos: %v", err) + return + } + + recipients.Insert(syntax.DID(pull.Repo.Did)) + for _, p := range pull.Participants() { + recipients.Insert(syntax.DID(p)) + } + + entityType = "pull" + entityId = pull.AtUri().String() + repoId = &pull.Repo.Id + p := int64(pull.ID) + pullId = &p + default: + return // no-op } - for _, m := range mentions { + + for _, m := range comment.Mentions { recipients.Remove(m) } - actorDid := syntax.DID(issue.Did) - entityType := "issue" - entityId := issue.AtUri().String() - repoId := &issue.Repo.Id - issueId := &issue.Id - var pullId *int64 - n.notifyEvent( - actorDid, + comment.Did, recipients, - models.NotificationTypeIssueCreated, + models.NotificationTypeIssueCommented, entityType, entityId, repoId, @@ -111,8 +185,8 @@ func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, me pullId, ) n.notifyEvent( - actorDid, - sets.Collect(slices.Values(mentions)), + comment.Did, + sets.Collect(slices.Values(comment.Mentions)), models.NotificationTypeUserMentioned, entityType, entityId, @@ -122,47 +196,30 @@ func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, me ) } -func (n *databaseNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { - issues, err := db.GetIssues(n.db, orm.FilterEq("at_uri", comment.Subject)) +func (n *databaseNotifier) DeleteComment(ctx context.Context, comment *models.Comment) { + // no-op +} + +func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) { + collaborators, err := db.GetCollaborators(n.db, orm.FilterEq("repo_at", issue.Repo.RepoAt())) if err != nil { - log.Printf("NewIssueComment: failed to get issues: %v", err) - return - } - if len(issues) == 0 { - log.Printf("NewIssueComment: no issue found for %s", comment.Subject) + log.Printf("failed to fetch collaborators: %v", err) return } - issue := issues[0] - // built the recipients list: - // - the owner of the repo - // - | if the comment is a reply -> everybody on that thread - // | if the comment is a top level -> just the issue owner - // - remove mentioned users from the recipients list + // build the recipients list + // - owner of the repo + // - collaborators in the repo + // - remove users already mentioned recipients := sets.Singleton(syntax.DID(issue.Repo.Did)) - - if comment.IsReply() { - // if this comment is a reply, then notify everybody in that thread - parentAtUri := *comment.ReplyTo - - // find the parent thread, and add all DIDs from here to the recipient list - for _, t := range issue.CommentList() { - if t.Self.AtUri() == parentAtUri { - for _, p := range t.Participants() { - recipients.Insert(p) - } - } - } - } else { - // not a reply, notify just the issue author - recipients.Insert(syntax.DID(issue.Did)) + for _, c := range collaborators { + recipients.Insert(c.SubjectDid) } - for _, m := range mentions { recipients.Remove(m) } - actorDid := syntax.DID(comment.Did) + actorDid := syntax.DID(issue.Did) entityType := "issue" entityId := issue.AtUri().String() repoId := &issue.Repo.Id @@ -172,7 +229,7 @@ func (n *databaseNotifier) NewIssueComment(ctx context.Context, comment *models. n.notifyEvent( actorDid, recipients, - models.NotificationTypeIssueCommented, + models.NotificationTypeIssueCreated, entityType, entityId, repoId, @@ -260,70 +317,6 @@ func (n *databaseNotifier) NewPull(ctx context.Context, pull *models.Pull) { ) } -func (n *databaseNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { - pulls, err := db.GetPulls(n.db, - orm.FilterEq("owner_did", comment.Subject.Authority()), - orm.FilterEq("rkey", comment.Subject.RecordKey()), - ) - if err != nil { - log.Printf("NewPullComment: failed to get pulls: %v", err) - return - } - if len(pulls) == 0 { - log.Printf("NewPullComment: no pull found for %s", comment.Subject) - return - } - pull := pulls[0] - - repo, err := db.GetRepo(n.db, orm.FilterEq("at_uri", pull.RepoAt)) - if err != nil { - log.Printf("NewPullComment: failed to get repos: %v", err) - return - } - - // build up the recipients list: - // - repo owner - // - all pull participants - // - remove those already mentioned - recipients := sets.Singleton(syntax.DID(repo.Did)) - for _, p := range pull.Participants() { - recipients.Insert(syntax.DID(p)) - } - for _, m := range mentions { - recipients.Remove(m) - } - - actorDid := comment.Did - eventType := models.NotificationTypePullCommented - entityType := "pull" - entityId := pull.AtUri().String() - repoId := &repo.Id - var issueId *int64 - p := int64(pull.ID) - pullId := &p - - n.notifyEvent( - actorDid, - recipients, - eventType, - entityType, - entityId, - repoId, - issueId, - pullId, - ) - n.notifyEvent( - actorDid, - sets.Collect(slices.Values(mentions)), - models.NotificationTypeUserMentioned, - entityType, - entityId, - repoId, - issueId, - pullId, - ) -} - func (n *databaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) { // no-op } diff --git a/appview/notify/merged_notifier.go b/appview/notify/merged_notifier.go index a4446cd8..22332685 100644 --- a/appview/notify/merged_notifier.go +++ b/appview/notify/merged_notifier.go @@ -53,12 +53,16 @@ func (m *mergedNotifier) DeleteStar(ctx context.Context, star *models.Star) { m.fanout("DeleteStar", ctx, star) } -func (m *mergedNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) { - m.fanout("NewIssue", ctx, issue, mentions) +func (m *mergedNotifier) NewComment(ctx context.Context, comment *models.Comment) { + m.fanout("NewComment", ctx, comment) +} + +func (m *mergedNotifier) DeleteComment(ctx context.Context, comment *models.Comment) { + m.fanout("DeleteComment", ctx, comment) } -func (m *mergedNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { - m.fanout("NewIssueComment", ctx, comment, mentions) +func (m *mergedNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) { + m.fanout("NewIssue", ctx, issue, mentions) } func (m *mergedNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) { @@ -81,10 +85,6 @@ func (m *mergedNotifier) NewPull(ctx context.Context, pull *models.Pull) { m.fanout("NewPull", ctx, pull) } -func (m *mergedNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { - m.fanout("NewPullComment", ctx, comment, mentions) -} - func (m *mergedNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) { m.fanout("NewPullState", ctx, actor, pull) } diff --git a/appview/notify/notifier.go b/appview/notify/notifier.go index 685b71c7..a8aac9e4 100644 --- a/appview/notify/notifier.go +++ b/appview/notify/notifier.go @@ -13,8 +13,10 @@ type Notifier interface { NewStar(ctx context.Context, star *models.Star) DeleteStar(ctx context.Context, star *models.Star) + NewComment(ctx context.Context, comment *models.Comment) + DeleteComment(ctx context.Context, comment *models.Comment) + NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) - NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) DeleteIssue(ctx context.Context, issue *models.Issue) @@ -22,7 +24,6 @@ type Notifier interface { DeleteFollow(ctx context.Context, follow *models.Follow) NewPull(ctx context.Context, pull *models.Pull) - NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) UpdateProfile(ctx context.Context, profile *models.Profile) @@ -42,18 +43,17 @@ func (m *BaseNotifier) NewRepo(ctx context.Context, repo *models.Repo) {} func (m *BaseNotifier) NewStar(ctx context.Context, star *models.Star) {} func (m *BaseNotifier) DeleteStar(ctx context.Context, star *models.Star) {} +func (m *BaseNotifier) NewComment(ctx context.Context, comment *models.Comment) {} +func (m *BaseNotifier) DeleteComment(ctx context.Context, comment *models.Comment) {} + func (m *BaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {} -func (m *BaseNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { -} func (m *BaseNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) {} func (m *BaseNotifier) DeleteIssue(ctx context.Context, issue *models.Issue) {} func (m *BaseNotifier) NewFollow(ctx context.Context, follow *models.Follow) {} func (m *BaseNotifier) DeleteFollow(ctx context.Context, follow *models.Follow) {} -func (m *BaseNotifier) NewPull(ctx context.Context, pull *models.Pull) {} -func (m *BaseNotifier) NewPullComment(ctx context.Context, models *models.Comment, mentions []syntax.DID) { -} +func (m *BaseNotifier) NewPull(ctx context.Context, pull *models.Pull) {} func (m *BaseNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {} func (m *BaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {} diff --git a/appview/notify/posthog/notifier.go b/appview/notify/posthog/notifier.go index aa11523b..0562fcd1 100644 --- a/appview/notify/posthog/notifier.go +++ b/appview/notify/posthog/notifier.go @@ -86,20 +86,6 @@ func (n *posthogNotifier) NewPull(ctx context.Context, pull *models.Pull) { } } -func (n *posthogNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { - err := n.client.Enqueue(posthog.Capture{ - DistinctId: comment.Did.String(), - Event: "new_pull_comment", - Properties: posthog.Properties{ - "pull_at": comment.Subject, - "mentions": mentions, - }, - }) - if err != nil { - log.Println("failed to enqueue posthog event:", err) - } -} - func (n *posthogNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) { err := n.client.Enqueue(posthog.Capture{ DistinctId: pull.OwnerDid, @@ -179,13 +165,13 @@ func (n *posthogNotifier) NewString(ctx context.Context, string *models.String) } } -func (n *posthogNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) { +func (n *posthogNotifier) NewComment(ctx context.Context, comment *models.Comment) { err := n.client.Enqueue(posthog.Capture{ DistinctId: comment.Did.String(), - Event: "new_issue_comment", + Event: "new_comment", Properties: posthog.Properties{ - "issue_at": comment.Subject, - "mentions": mentions, + "subject_at": comment.Subject, + "mentions": comment.Mentions, }, }) if err != nil { diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index ef398823..37747943 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -794,7 +794,7 @@ func (s *Pulls) PullComment(w http.ResponseWriter, r *http.Request) { return } - s.notifier.NewPullComment(r.Context(), &comment, mentions) + s.notifier.NewComment(r.Context(), &comment) ownerSlashRepo := reporesolver.GetBaseRepoPath(r, f) s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d#comment-%d", ownerSlashRepo, pull.PullId, comment.Id)) -- 2.43.0