Don't send trigger for a pending review's comment create/update/delete (#34928) (#34939)

Backport #34928 by lunny

Fix #18846 
Fix #34924

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/issues/comment.go

@@ -719,7 +719,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
 	}
@@ -736,11 +737,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 {
@@ -860,7 +856,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
 				}
 			}

services/notify/notify.go

@@ -46,10 +46,25 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 	}
 }
 
+func shouldSendCommentChangeNotification(ctx context.Context, comment *issues_model.Comment) bool {
+	if err := comment.LoadReview(ctx); err != nil {
+		log.Error("LoadReview: %v", err)
+		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
+	}
+
 	for _, notifier := range notifiers {
 		notifier.CreateIssueComment(ctx, doer, repo, issue, comment, mentions)
 	}
@@ -156,6 +171,10 @@ 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 !shouldSendCommentChangeNotification(ctx, c) {
+		return
+	}
+
 	for _, notifier := range notifiers {
 		notifier.UpdateComment(ctx, doer, c, oldContent)
 	}
@@ -163,6 +182,10 @@ 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 !shouldSendCommentChangeNotification(ctx, c) {
+		return
+	}
+
 	for _, notifier := range notifiers {
 		notifier.DeleteComment(ctx, doer, c)
 	}

templates/repo/issue/filter_item_label.tmpl

@@ -22,7 +22,7 @@
 		<span class="label-filter-exclude-info">{{ctx.Locale.Tr "repo.issues.filter_label_exclude"}}</span>
 		<div class="divider"></div>
 		<a class="item label-filter-query-default" href="{{QueryBuild $queryLink "labels" NIL}}">{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}}</a>
-		<a class="item label-filter-query-not-set" href="{{QueryBuild $queryLink "labels" 0}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
+		<a class="item label-filter-query-not-set" href="{{QueryBuild $queryLink "labels" "0"}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
 		{{/* The logic here is not the same as the label selector in the issue sidebar.
 		The one in the issue sidebar renders "repo labels | divider | org labels".
 		Maybe the logic should be updated to be consistent.*/}}

templates/repo/issue/filter_list.tmpl

@@ -15,7 +15,7 @@
 			<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_milestone"}}">
 		</div>
 		<div class="divider"></div>
-		<a class="{{if not $.MilestoneID}}active selected {{end}}item" href="{{QueryBuild $queryLink "milestone" 0}}">{{ctx.Locale.Tr "repo.issues.filter_milestone_all"}}</a>
+		<a class="{{if not $.MilestoneID}}active selected {{end}}item" href="{{QueryBuild $queryLink "milestone" NIL}}">{{ctx.Locale.Tr "repo.issues.filter_milestone_all"}}</a>
 		<a class="{{if $.MilestoneID}}{{if eq $.MilestoneID -1}}active selected {{end}}{{end}}item" href="{{QueryBuild $queryLink "milestone" -1}}">{{ctx.Locale.Tr "repo.issues.filter_milestone_none"}}</a>
 		{{if .OpenMilestones}}
 			<div class="divider"></div>