Notify reviewers added via CODEOWNERS (#29842)

Author: jpraet

services/issue/assignee.go

@@ -226,16 +226,33 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use
 		return nil, nil
 	}
 
+	return comment, teamReviewRequestNotify(ctx, issue, doer, reviewer, isAdd, comment)
+}
+
+func ReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewNotifers []*ReviewRequestNotifier) {
+	for _, reviewNotifer := range reviewNotifers {
+		if reviewNotifer.Reviwer != nil {
+			notify_service.PullRequestReviewRequest(ctx, issue.Poster, issue, reviewNotifer.Reviwer, reviewNotifer.IsAdd, reviewNotifer.Comment)
+		} else if reviewNotifer.ReviewTeam != nil {
+			if err := teamReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifer.ReviewTeam, reviewNotifer.IsAdd, reviewNotifer.Comment); err != nil {
+				log.Error("teamReviewRequestNotify: %v", err)
+			}
+		}
+	}
+}
+
+// teamReviewRequestNotify notify all user in this team
+func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool, comment *issues_model.Comment) error {
 	// notify all user in this team
 	if err := comment.LoadIssue(ctx); err != nil {
-		return nil, err
+		return err
 	}
 
 	members, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{
 		TeamID: reviewer.ID,
 	})
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	for _, member := range members {
@@ -246,7 +263,7 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use
 		notify_service.PullRequestReviewRequest(ctx, doer, issue, member, isAdd, comment)
 	}
 
-	return comment, err
+	return err
 }
 
 // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR

services/issue/issue.go

@@ -85,17 +85,27 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode
 		}
 	}
 
-	if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
-		return err
-	}
+	var reviewNotifers []*ReviewRequestNotifier
 
-	if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
-		if err := PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest); err != nil {
+	if err := db.WithTx(ctx, func(ctx context.Context) error {
+		if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
 			return err
 		}
+
+		if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
+			var err error
+			reviewNotifers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	}); err != nil {
+		return err
 	}
 
 	notify_service.IssueChangeTitle(ctx, doer, issue, oldTitle)
+	ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifers)
 
 	return nil
 }

services/issue/pull.go

@@ -33,34 +33,41 @@ func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch
 	return mergeBase, err
 }
 
-func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue, pr *issues_model.PullRequest) error {
+type ReviewRequestNotifier struct {
+	Comment    *issues_model.Comment
+	IsAdd      bool
+	Reviwer    *user_model.User
+	ReviewTeam *org_model.Team
+}
+
+func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
 	files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}
 
 	if pr.IsWorkInProgress(ctx) {
-		return nil
+		return nil, nil
 	}
 
 	if err := pr.LoadHeadRepo(ctx); err != nil {
-		return err
+		return nil, err
 	}
 
 	if pr.HeadRepo.IsFork {
-		return nil
+		return nil, nil
 	}
 
 	if err := pr.LoadBaseRepo(ctx); err != nil {
-		return err
+		return nil, err
 	}
 
 	repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	defer repo.Close()
 
 	commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	var data string
@@ -78,14 +85,14 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
 	// get the mergebase
 	mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed
 	// between the merge base and the head commit but not the base branch and the head commit
 	changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.HeadCommitID)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	uniqUsers := make(map[int64]*user_model.User)
@@ -103,20 +110,34 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
 		}
 	}
 
+	notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams))
+
 	for _, u := range uniqUsers {
 		if u.ID != pull.Poster.ID {
-			if _, err := issues_model.AddReviewRequest(ctx, pull, u, pull.Poster); err != nil {
+			comment, err := issues_model.AddReviewRequest(ctx, pull, u, pull.Poster)
+			if err != nil {
 				log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err)
-				return err
+				return nil, err
 			}
+			notifiers = append(notifiers, &ReviewRequestNotifier{
+				Comment: comment,
+				IsAdd:   true,
+				Reviwer: pull.Poster,
+			})
 		}
 	}
 	for _, t := range uniqTeams {
-		if _, err := issues_model.AddTeamReviewRequest(ctx, pull, t, pull.Poster); err != nil {
+		comment, err := issues_model.AddTeamReviewRequest(ctx, pull, t, pull.Poster)
+		if err != nil {
 			log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err)
-			return err
+			return nil, err
 		}
+		notifiers = append(notifiers, &ReviewRequestNotifier{
+			Comment:    comment,
+			IsAdd:      true,
+			ReviewTeam: t,
+		})
 	}
 
-	return nil
+	return notifiers, nil
 }

services/pull/pull.go

@@ -77,6 +77,7 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 	}
 	defer baseGitRepo.Close()
 
+	var reviewNotifers []*issue_service.ReviewRequestNotifier
 	if err := db.WithTx(ctx, func(ctx context.Context) error {
 		if err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr); err != nil {
 			return err
@@ -136,7 +137,8 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 		}
 
 		if !pr.IsWorkInProgress(ctx) {
-			if err := issue_service.PullRequestCodeOwnersReview(ctx, issue, pr); err != nil {
+			reviewNotifers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr)
+			if err != nil {
 				return err
 			}
 		}
@@ -150,11 +152,12 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 	}
 	baseGitRepo.Close() // close immediately to avoid notifications will open the repository again
 
+	issue_service.ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifers)
+
 	mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, issue.Poster, issue.Content)
 	if err != nil {
 		return err
 	}
-
 	notify_service.NewPullRequest(ctx, pr, mentions)
 	if len(issue.Labels) > 0 {
 		notify_service.IssueChangeLabels(ctx, issue.Poster, issue, issue.Labels, nil)