From 17d7ab5ad4ce3d0fbc1251572c22687c237a30b1 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Tue, 19 Mar 2024 06:28:43 +0100 Subject: [PATCH] Notify reviewers added via CODEOWNERS (#29842) --- services/issue/assignee.go | 23 +++++++++++++++--- services/issue/issue.go | 24 +++++++++++++------ services/issue/pull.go | 49 +++++++++++++++++++++++++++----------- services/pull/pull.go | 7 ++++-- 4 files changed, 77 insertions(+), 26 deletions(-) diff --git a/services/issue/assignee.go b/services/issue/assignee.go index b5f472ba5..8740a6664 100644 --- a/services/issue/assignee.go +++ b/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 diff --git a/services/issue/issue.go b/services/issue/issue.go index 7662c9d9e..94b0ee6f6 100644 --- a/services/issue/issue.go +++ b/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 { + 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 } - if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) { - if err := PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest); 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 698e2622f..8e85c11e9 100644 --- a/services/issue/pull.go +++ b/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 } diff --git a/services/pull/pull.go b/services/pull/pull.go index 57873b63b..8a9c6db91 100644 --- a/services/pull/pull.go +++ b/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)