Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify reviewers added via CODEOWNERS (#29842) #29902

Merged
merged 2 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions services/issue/assignee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
20 changes: 17 additions & 3 deletions services/issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
49 changes: 35 additions & 14 deletions services/issue/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
}
7 changes: 5 additions & 2 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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)
Expand Down
Loading