From 1a41a36655cd7bfb6a1d6a543b0e19dee304446c 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 | 215 ++++++++++++++--------------- appview/notify/merged_notifier.go | 16 +-- appview/notify/notifier.go | 12 +- appview/notify/posthog/notifier.go | 22 +-- appview/pulls/pulls.go | 2 +- 6 files changed, 125 insertions(+), 144 deletions(-) diff --git a/appview/issues/issues.go b/appview/issues/issues.go index a230ff6a..cd275768 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -488,7 +488,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 6d7df6a7..713a5eba 100644 --- a/appview/notify/db/db.go +++ b/appview/notify/db/db.go @@ -73,33 +73,100 @@ 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) { +func (n *databaseNotifier) NewComment(ctx context.Context, comment *models.Comment) { + var ( + recipients []syntax.DID + entityType string + entityId string + repoId *int64 + issueId *int64 + pullId *int64 + ) - // build the recipients list - // - owner of the repo - // - collaborators in the repo - var recipients []syntax.DID - recipients = append(recipients, syntax.DID(issue.Repo.Did)) - collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", issue.Repo.RepoAt())) + 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 } - for _, c := range collaborators { - recipients = append(recipients, c.SubjectDid) - } + switch comment.Subject.Collection() { + case tangled.RepoIssueNSID: + issues, err := db.GetIssues( + n.db, + db.FilterEq("did", subjectDid), + db.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 = append(recipients, syntax.DID(issue.Repo.Did)) + if comment.IsReply() { + // if this comment is a reply, then notify everybody in that thread + parentAtUri := *comment.ReplyTo + allThreads := issue.CommentList() + + // find the parent thread, and add all DIDs from here to the recipient list + for _, t := range allThreads { + if t.Self.AtUri() == parentAtUri { + recipients = append(recipients, t.Participants()...) + } + } + } else { + // not a reply, notify just the issue author + recipients = append(recipients, syntax.DID(issue.Did)) + } - actorDid := syntax.DID(issue.Did) - entityType := "issue" - entityId := issue.AtUri().String() - repoId := &issue.Repo.Id - issueId := &issue.Id - var pullId *int64 + entityType = "issue" + entityId = issue.AtUri().String() + repoId = &issue.Repo.Id + issueId = &issue.Id + case tangled.RepoPullNSID: + pulls, err := db.GetPullsWithLimit( + n.db, + 1, + db.FilterEq("owner_did", subjectDid), + db.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, db.FilterEq("at_uri", pull.RepoAt)) + if err != nil { + log.Printf("NewComment: failed to get repos: %v", err) + return + } + + recipients = append(recipients, syntax.DID(pull.Repo.Did)) + for _, p := range pull.Participants() { + recipients = append(recipients, syntax.DID(p)) + } + + entityType = "pull" + entityId = pull.AtUri().String() + repoId = &pull.Repo.Id + p := int64(pull.ID) + pullId = &p + default: + return // no-op + } n.notifyEvent( - actorDid, + comment.Did, recipients, - models.NotificationTypeIssueCreated, + models.NotificationTypeIssueCommented, entityType, entityId, repoId, @@ -107,8 +174,8 @@ func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, me pullId, ) n.notifyEvent( - actorDid, - mentions, + comment.Did, + comment.Mentions, models.NotificationTypeUserMentioned, entityType, entityId, @@ -118,38 +185,27 @@ 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, db.FilterEq("at_uri", comment.Subject)) - 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) - return - } - issue := issues[0] +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) { + // build the recipients list + // - owner of the repo + // - collaborators in the repo var recipients []syntax.DID recipients = append(recipients, syntax.DID(issue.Repo.Did)) - - if comment.IsReply() { - // if this comment is a reply, then notify everybody in that thread - parentAtUri := *comment.ReplyTo - allThreads := issue.CommentList() - - // find the parent thread, and add all DIDs from here to the recipient list - for _, t := range allThreads { - if t.Self.AtUri() == parentAtUri { - recipients = append(recipients, t.Participants()...) - } - } - } else { - // not a reply, notify just the issue author - recipients = append(recipients, syntax.DID(issue.Did)) + collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", issue.Repo.RepoAt())) + if err != nil { + log.Printf("failed to fetch collaborators: %v", err) + return + } + for _, c := range collaborators { + recipients = append(recipients, c.SubjectDid) } - actorDid := syntax.DID(comment.Did) + actorDid := syntax.DID(issue.Did) entityType := "issue" entityId := issue.AtUri().String() repoId := &issue.Repo.Id @@ -159,7 +215,7 @@ func (n *databaseNotifier) NewIssueComment(ctx context.Context, comment *models. n.notifyEvent( actorDid, recipients, - models.NotificationTypeIssueCommented, + models.NotificationTypeIssueCreated, entityType, entityId, repoId, @@ -248,67 +304,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, - db.FilterEq("owner_did", comment.Subject.Authority()), - db.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, db.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 - var recipients []syntax.DID - recipients = append(recipients, syntax.DID(repo.Did)) - for _, p := range pull.Participants() { - recipients = append(recipients, syntax.DID(p)) - } - - 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, - 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 5b29d49b..13bcdbd4 100644 --- a/appview/notify/merged_notifier.go +++ b/appview/notify/merged_notifier.go @@ -54,12 +54,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) { @@ -82,10 +86,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..046601b1 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,9 +43,10 @@ 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) {} @@ -52,8 +54,6 @@ 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) 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 ce33af69..38739190 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -793,7 +793,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