Skip to content

Commit

Permalink
[Port] gitea#29842: Notify reviewers added via CODEOWNERS
Browse files Browse the repository at this point in the history
Fixes go-gitea/gitea#28297

This PR also fixed a problem that it needs a database transaction when
removing the WIP title.

---

Resolves #2771
Also partially ports gitea#29783

(cherry picked from commit 17d7ab5ad4ce3d0fbc1251572c22687c237a30b1)
  • Loading branch information
jpraet authored and Gusted committed Mar 28, 2024
1 parent 33d6b5b commit 003881b
Show file tree
Hide file tree
Showing 6 changed files with 308 additions and 102 deletions.
87 changes: 0 additions & 87 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
Expand Down Expand Up @@ -884,92 +883,6 @@ func MergeBlockedByOutdatedBranch(protectBranch *git_model.ProtectedBranch, pr *
return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0
}

func PullRequestCodeOwnersReview(ctx context.Context, pull *Issue, pr *PullRequest) error {
files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}

if pr.IsWorkInProgress(ctx) {
return nil
}

if err := pull.LoadRepo(ctx); err != nil {
return err
}

if pull.Repo.IsFork {
return nil
}

if err := pr.LoadBaseRepo(ctx); err != nil {
return err
}

repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
if err != nil {
return err
}
defer repo.Close()

commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch)
if err != nil {
return err
}

var data string
for _, file := range files {
if blob, err := commit.GetBlobByPath(file); err == nil {
data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize)
if err == nil {
break
}
}
}

rules, _ := GetCodeOwnersFromContent(ctx, data)

prInfo, err := repo.GetCompareInfo(repo.Path, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false, false)
if err != nil {
return err
}
// Use the merge base as the base instead of the main branch to avoid problems
// if the pull request is out of date with the base branch.
changedFiles, err := repo.GetFilesChangedBetween(prInfo.MergeBase, prInfo.HeadCommitID)
if err != nil {
return err
}

uniqUsers := make(map[int64]*user_model.User)
uniqTeams := make(map[string]*org_model.Team)
for _, rule := range rules {
for _, f := range changedFiles {
if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) {
for _, u := range rule.Users {
uniqUsers[u.ID] = u
}
for _, t := range rule.Teams {
uniqTeams[fmt.Sprintf("%d/%d", t.OrgID, t.ID)] = t
}
}
}
}

for _, u := range uniqUsers {
if u.ID != pull.Poster.ID {
if _, err := AddReviewRequest(ctx, pull, u, pull.Poster); 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
}
}
}
for _, t := range uniqTeams {
if _, err := AddTeamReviewRequest(ctx, pull, t, pull.Poster); 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
}

// GetCodeOwnersFromContent returns the code owners configuration
// Return empty slice if files missing
// Return warning messages on parsing errors
Expand Down
23 changes: 20 additions & 3 deletions services/issue/assignee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
20 changes: 15 additions & 5 deletions services/issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,27 @@ 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 {
return err
}
var reviewNotifers []*ReviewRequestNotifier

if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
if err := issues_model.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
}
Expand Down
143 changes: 143 additions & 0 deletions services/issue/pull.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package issue

import (
"context"
"fmt"
"time"

issues_model "code.gitea.io/gitea/models/issues"
org_model "code.gitea.io/gitea/models/organization"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)

func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) {
// Add a temporary remote
tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano())
if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil {
return "", fmt.Errorf("AddRemote: %w", err)
}
defer func() {
if err := repo.RemoveRemote(tmpRemote); err != nil {
log.Error("getMergeBase: RemoveRemote: %v", err)
}
}()

mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch)
return mergeBase, err
}

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, nil
}

if err := pr.LoadHeadRepo(ctx); err != nil {
return nil, err
}

if pr.HeadRepo.IsFork {
return nil, nil
}

if err := pr.LoadBaseRepo(ctx); err != nil {
return nil, err
}

repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
if err != nil {
return nil, err
}
defer repo.Close()

commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch)
if err != nil {
return nil, err
}

var data string
for _, file := range files {
if blob, err := commit.GetBlobByPath(file); err == nil {
data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize)
if err == nil {
break
}
}
}

rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data)

// get the mergebase
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
if err != nil {
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 nil, err
}

uniqUsers := make(map[int64]*user_model.User)
uniqTeams := make(map[string]*org_model.Team)
for _, rule := range rules {
for _, f := range changedFiles {
if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) {
for _, u := range rule.Users {
uniqUsers[u.ID] = u
}
for _, t := range rule.Teams {
uniqTeams[fmt.Sprintf("%d/%d", t.OrgID, t.ID)] = t
}
}
}
}

notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams))

for _, u := range uniqUsers {
if u.ID != pull.Poster.ID {
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 nil, err
}
notifiers = append(notifiers, &ReviewRequestNotifier{
Comment: comment,
IsAdd: true,
Reviwer: pull.Poster,
})
}
}
for _, t := range uniqTeams {
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 nil, err
}
notifiers = append(notifiers, &ReviewRequestNotifier{
Comment: comment,
IsAdd: true,
ReviewTeam: t,
})
}

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 @@ -74,6 +74,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 @@ -133,7 +134,8 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
}

if !pr.IsWorkInProgress(ctx) {
if err := issues_model.PullRequestCodeOwnersReview(ctx, issue, pr); err != nil {
reviewNotifers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr)
if err != nil {
return err
}
}
Expand All @@ -147,11 +149,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

0 comments on commit 003881b

Please sign in to comment.