From 0cf45c3f0eac9c864b1c67d5726fa660501b214b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 1 Jul 2025 18:15:32 -0700 Subject: [PATCH 1/2] Don't send trigger for a pending review's comment create/update/delete --- models/issues/comment.go | 10 +++------- services/notify/notify.go | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 9bef96d0ddfcc..4fdb0c1808fb4 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -715,7 +715,8 @@ func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository return nil } -func (c *Comment) loadReview(ctx context.Context) (err error) { +// LoadReview loads the associated review +func (c *Comment) LoadReview(ctx context.Context) (err error) { if c.ReviewID == 0 { return nil } @@ -732,11 +733,6 @@ func (c *Comment) loadReview(ctx context.Context) (err error) { return nil } -// LoadReview loads the associated review -func (c *Comment) LoadReview(ctx context.Context) error { - return c.loadReview(ctx) -} - // DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes. func (c *Comment) DiffSide() string { if c.Line < 0 { @@ -856,7 +852,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } if comment.ReviewID != 0 { if comment.Review == nil { - if err := comment.loadReview(ctx); err != nil { + if err := comment.LoadReview(ctx); err != nil { return err } } diff --git a/services/notify/notify.go b/services/notify/notify.go index 0c6fdf9cef9df..4442cc3136d1d 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -50,6 +50,14 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, ) { + if err := comment.LoadReview(ctx); err != nil { + log.Error("LoadReview: %v", err) + return + } else if comment.Review != nil && comment.Review.Type == issues_model.ReviewTypePending { + // Pending review comments updating should not triggered + return + } + for _, notifier := range notifiers { notifier.CreateIssueComment(ctx, doer, repo, issue, comment, mentions) } @@ -156,6 +164,14 @@ func PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issue // UpdateComment notifies update comment to notifiers func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment, oldContent string) { + if err := c.LoadReview(ctx); err != nil { + log.Error("LoadReview: %v", err) + return + } else if c.Review != nil && c.Review.Type == issues_model.ReviewTypePending { + // Pending review comments updating should not triggered + return + } + for _, notifier := range notifiers { notifier.UpdateComment(ctx, doer, c, oldContent) } @@ -163,6 +179,14 @@ func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.C // DeleteComment notifies delete comment to notifiers func DeleteComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment) { + if err := c.LoadReview(ctx); err != nil { + log.Error("LoadReview: %v", err) + return + } else if c.Review != nil && c.Review.Type == issues_model.ReviewTypePending { + // Pending review comments updating should not triggered + return + } + for _, notifier := range notifiers { notifier.DeleteComment(ctx, doer, c) } From 681ffa6ff1b87c9a2f4086aa1dbc46110647fee1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 2 Jul 2025 10:59:33 -0700 Subject: [PATCH 2/2] update --- services/notify/notify.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/services/notify/notify.go b/services/notify/notify.go index 4442cc3136d1d..2416cbd2e0830 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -46,15 +46,22 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model } } -// CreateIssueComment notifies issue comment related message to notifiers -func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, - issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, -) { +func shouldSendCommentChangeNotification(ctx context.Context, comment *issues_model.Comment) bool { if err := comment.LoadReview(ctx); err != nil { log.Error("LoadReview: %v", err) - return + return false } else if comment.Review != nil && comment.Review.Type == issues_model.ReviewTypePending { // Pending review comments updating should not triggered + return false + } + return true +} + +// CreateIssueComment notifies issue comment related message to notifiers +func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, + issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, +) { + if !shouldSendCommentChangeNotification(ctx, comment) { return } @@ -164,11 +171,7 @@ func PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issue // UpdateComment notifies update comment to notifiers func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment, oldContent string) { - if err := c.LoadReview(ctx); err != nil { - log.Error("LoadReview: %v", err) - return - } else if c.Review != nil && c.Review.Type == issues_model.ReviewTypePending { - // Pending review comments updating should not triggered + if !shouldSendCommentChangeNotification(ctx, c) { return } @@ -179,11 +182,7 @@ func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.C // DeleteComment notifies delete comment to notifiers func DeleteComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment) { - if err := c.LoadReview(ctx); err != nil { - log.Error("LoadReview: %v", err) - return - } else if c.Review != nil && c.Review.Type == issues_model.ReviewTypePending { - // Pending review comments updating should not triggered + if !shouldSendCommentChangeNotification(ctx, c) { return }