Notify reviewers added via CODEOWNERS (#29842) (#29902)

backport #29842

Co-authored-by: Jimmy Praet <jimmy.praet@telenet.be>

diff --git a/services/issue/assignee.go b/services/issue/assignee.go
index 27fc695533e3a867525dfc6e3b3d088b1a4f9a2a..f991473d7db7ff4dde3ee4e39b7e353c77ca7e7f 100644 (file)
--- a/services/issue/assignee.go
+++ b/services/issue/assignee.go
@@ -242,16 +242,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 {
@@ -262,5 +279,5 @@ 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
 }

diff --git a/services/issue/issue.go b/services/issue/issue.go
index 10aec1288f1017f6f0876cc96dcdbf8d7d1de2cf..81fa980b6b1cb8ca04b711434d58e417b53ea99c 100644 (file)
--- a/services/issue/issue.go
+++ b/services/issue/issue.go
@@ -66,17 +66,31 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode
                return nil
        }
 
-       if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
+       if err := issue.LoadRepo(ctx); err != nil {
                return err
        }
 
-       if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
-               if err := PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest); err != nil {
+       var reviewNotifers []*ReviewRequestNotifier
+
+       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
 }

diff --git a/services/issue/pull.go b/services/issue/pull.go
index 94020de42da6d33defb37700cb8f1831cc95afb1..2bb8cbbc9ebd3af65d5a8b1c21a3ce5a9a4c96de 100644 (file)
--- a/services/issue/pull.go
+++ b/services/issue/pull.go
@@ -32,34 +32,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 := git.OpenRepository(ctx, pr.BaseRepo.RepoPath())
        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
@@ -77,14 +84,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)
@@ -102,20 +109,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
 }

diff --git a/services/pull/pull.go b/services/pull/pull.go
index 020c80add36a7f109b8fd7dbd5940e8b16275a38..756e542c4d0174b65cfa5ee484745440ef7dfb3f 100644 (file)
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -68,6 +68,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
@@ -127,7 +128,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
                        }
                }
@@ -141,11 +143,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)