From 1c4c3d8ddf71d97bb8faac4a569d93588b631e5e Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Thu, 23 Oct 2025 23:44:11 +0900 Subject: [PATCH] appview/notify: merge `NewPullClosed/Merged/Reopen` to `NewPullState` Change-Id: xxrpmyysunoooxrvmpzwozszsumqoxuv same to `NewIssueState`, we can determine the detailed event type from latest pull state Signed-off-by: Seongmin Lee --- appview/indexer/notifier.go | 11 +-- appview/notify/db/db.go | 105 ++++------------------------- appview/notify/merged_notifier.go | 12 +--- appview/notify/notifier.go | 8 +-- appview/notify/posthog/notifier.go | 16 ++++- appview/pulls/pulls.go | 6 +- 6 files changed, 34 insertions(+), 124 deletions(-) diff --git a/appview/indexer/notifier.go b/appview/indexer/notifier.go index 4447d9f1..db722f12 100644 --- a/appview/indexer/notifier.go +++ b/appview/indexer/notifier.go @@ -46,16 +46,7 @@ func (ix *Indexer) NewPull(ctx context.Context, pull *models.Pull) { } } -func (ix *Indexer) NewPullMerged(ctx context.Context, pull *models.Pull) { - l := log.FromContext(ctx).With("notifier", "indexer", "pull", pull) - l.Debug("updating a pr") - err := ix.Pulls.Index(ctx, pull) - if err != nil { - l.Error("failed to index a pr", "err", err) - } -} - -func (ix *Indexer) NewPullClosed(ctx context.Context, pull *models.Pull) { +func (ix *Indexer) NewPullState(ctx context.Context, pull *models.Pull) { l := log.FromContext(ctx).With("notifier", "indexer", "pull", pull) l.Debug("updating a pr") err := ix.Pulls.Index(ctx, pull) diff --git a/appview/notify/db/db.go b/appview/notify/db/db.go index 2e94069d..9c34baea 100644 --- a/appview/notify/db/db.go +++ b/appview/notify/db/db.go @@ -328,11 +328,11 @@ func (n *databaseNotifier) NewIssueState(ctx context.Context, issue *models.Issu ) } -func (n *databaseNotifier) NewPullMerged(ctx context.Context, pull *models.Pull) { +func (n *databaseNotifier) NewPullState(ctx context.Context, pull *models.Pull) { // Get repo details repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(pull.RepoAt))) if err != nil { - log.Printf("NewPullMerged: failed to get repos: %v", err) + log.Printf("NewPullState: failed to get repos: %v", err) return } @@ -354,103 +354,22 @@ func (n *databaseNotifier) NewPullMerged(ctx context.Context, pull *models.Pull) } actorDid := syntax.DID(repo.Did) - eventType := models.NotificationTypePullMerged entityType := "pull" entityId := pull.PullAt().String() repoId := &repo.Id var issueId *int64 - p := int64(pull.ID) - pullId := &p - - n.notifyEvent( - actorDid, - recipients, - eventType, - entityType, - entityId, - repoId, - issueId, - pullId, - ) -} - -func (n *databaseNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) { - // Get repo details - repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(pull.RepoAt))) - if err != nil { - log.Printf("NewPullMerged: 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)) - collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", repo.RepoAt())) - if err != nil { - log.Printf("failed to fetch collaborators: %v", err) - return - } - for _, c := range collaborators { - recipients = append(recipients, c.SubjectDid) - } - for _, p := range pull.Participants() { - recipients = append(recipients, syntax.DID(p)) - } - - actorDid := syntax.DID(repo.Did) - eventType := models.NotificationTypePullClosed - entityType := "pull" - entityId := pull.PullAt().String() - repoId := &repo.Id - var issueId *int64 - p := int64(pull.ID) - pullId := &p - - n.notifyEvent( - actorDid, - recipients, - eventType, - entityType, - entityId, - repoId, - issueId, - pullId, - ) -} - -func (n *databaseNotifier) NewPullReopen(ctx context.Context, pull *models.Pull) { - // Get repo details - repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(pull.RepoAt))) - if err != nil { - log.Printf("NewPullMerged: 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)) - collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", repo.RepoAt())) - if err != nil { - log.Printf("failed to fetch collaborators: %v", err) + var eventType models.NotificationType + switch pull.State { + case models.PullClosed: + eventType = models.NotificationTypePullClosed + case models.PullOpen: + eventType = models.NotificationTypePullReopen + case models.PullMerged: + eventType = models.NotificationTypePullMerged + default: + log.Println("NewPullState: unexpected new PR state:", pull.State) return } - for _, c := range collaborators { - recipients = append(recipients, c.SubjectDid) - } - for _, p := range pull.Participants() { - recipients = append(recipients, syntax.DID(p)) - } - - actorDid := syntax.DID(repo.Did) - eventType := models.NotificationTypePullReopen - entityType := "pull" - entityId := pull.PullAt().String() - repoId := &repo.Id - var issueId *int64 p := int64(pull.ID) pullId := &p diff --git a/appview/notify/merged_notifier.go b/appview/notify/merged_notifier.go index d473c0b9..ae4e94ef 100644 --- a/appview/notify/merged_notifier.go +++ b/appview/notify/merged_notifier.go @@ -85,16 +85,8 @@ func (m *mergedNotifier) NewPullComment(ctx context.Context, comment *models.Pul m.fanout("NewPullComment", ctx, comment) } -func (m *mergedNotifier) NewPullMerged(ctx context.Context, pull *models.Pull) { - m.fanout("NewPullMerged", ctx, pull) -} - -func (m *mergedNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) { - m.fanout("NewPullClosed", ctx, pull) -} - -func (m *mergedNotifier) NewPullReopen(ctx context.Context, pull *models.Pull) { - m.fanout("NewPullReopen", ctx, pull) +func (m *mergedNotifier) NewPullState(ctx context.Context, pull *models.Pull) { + m.fanout("NewPullState", ctx, pull) } func (m *mergedNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) { diff --git a/appview/notify/notifier.go b/appview/notify/notifier.go index eb704f34..f7bcfc37 100644 --- a/appview/notify/notifier.go +++ b/appview/notify/notifier.go @@ -22,9 +22,7 @@ type Notifier interface { NewPull(ctx context.Context, pull *models.Pull) NewPullComment(ctx context.Context, comment *models.PullComment) - NewPullMerged(ctx context.Context, pull *models.Pull) - NewPullClosed(ctx context.Context, pull *models.Pull) - NewPullReopen(ctx context.Context, pull *models.Pull) + NewPullState(ctx context.Context, pull *models.Pull) UpdateProfile(ctx context.Context, profile *models.Profile) @@ -53,9 +51,7 @@ 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.PullComment) {} -func (m *BaseNotifier) NewPullMerged(ctx context.Context, pull *models.Pull) {} -func (m *BaseNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) {} -func (m *BaseNotifier) NewPullReopen(ctx context.Context, pull *models.Pull) {} +func (m *BaseNotifier) NewPullState(ctx context.Context, 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 528cef00..ea62d903 100644 --- a/appview/notify/posthog/notifier.go +++ b/appview/notify/posthog/notifier.go @@ -210,10 +210,22 @@ func (n *posthogNotifier) NewIssueState(ctx context.Context, issue *models.Issue } } -func (n *posthogNotifier) NewPullMerged(ctx context.Context, pull *models.Pull) { +func (n *posthogNotifier) NewPullState(ctx context.Context, pull *models.Pull) { + var event string + switch pull.State { + case models.PullClosed: + event = "pull_closed" + case models.PullOpen: + event = "pull_reopen" + case models.PullMerged: + event = "pull_merged" + default: + log.Println("posthog: unexpected new PR state:", pull.State) + return + } err := n.client.Enqueue(posthog.Capture{ DistinctId: pull.OwnerDid, - Event: "pull_merged", + Event: event, Properties: posthog.Properties{ "repo_at": pull.RepoAt, "pull_id": pull.PullId, diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index 2e1c9631..6c4cb38e 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -2216,7 +2216,7 @@ func (s *Pulls) MergePull(w http.ResponseWriter, r *http.Request) { // notify about the pull merge for _, p := range pullsToMerge { - s.notifier.NewPullMerged(r.Context(), p) + s.notifier.NewPullState(r.Context(), p) } s.pages.HxLocation(w, fmt.Sprintf("/@%s/%s/pulls/%d", f.OwnerHandle(), f.Name, pull.PullId)) @@ -2288,7 +2288,7 @@ func (s *Pulls) ClosePull(w http.ResponseWriter, r *http.Request) { } for _, p := range pullsToClose { - s.notifier.NewPullClosed(r.Context(), p) + s.notifier.NewPullState(r.Context(), p) } s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) @@ -2361,7 +2361,7 @@ func (s *Pulls) ReopenPull(w http.ResponseWriter, r *http.Request) { } for _, p := range pullsToReopen { - s.notifier.NewPullReopen(r.Context(), p) + s.notifier.NewPullState(r.Context(), p) } s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) -- 2.43.0