shaba/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-03-22 14:50:13 +03:00
Code Issues Projects Releases Wiki Activity

Pull request updates will also trigger code owners review requests (#33744)

Browse Source 
Fix #33490 

It will only read the changed file on the pushed commits but not all the
files of this PR.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao 2025-03-13 19:36:14 -07:00 committed by GitHub
parent de2d472d90
commit 1e7248047c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 157 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
routers/web/repo/view_file.go
View File

@ -9,7 +9,6 @@ import (
"image"
"io"
"path"
"slices"
"strings"
git_model "code.gitea.io/gitea/models/git"
@ -79,7 +78,7 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) {
if workFlowErr != nil {
ctx.Data["FileError"] = ctx.Locale.Tr("actions.runs.invalid_workflow_helper", workFlowErr.Error())
}
} else if slices.Contains([]string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}, ctx.Repo.TreePath) {
} else if issue_service.IsCodeOwnerFile(ctx.Repo.TreePath) {
if data, err := blob.GetBlobContent(setting.UI.MaxDisplayFileSize); err == nil {
_, warnings := issue_model.GetCodeOwnersFromContent(ctx, data)
if len(warnings) > 0 {

6
services/issue/issue.go
View File

@ -92,8 +92,12 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode
var reviewNotifiers []*ReviewRequestNotifier
if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
if err := issue.LoadPullRequest(ctx); err != nil {
return err
}
var err error
reviewNotifiers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest)
reviewNotifiers, err = PullRequestCodeOwnersReview(ctx, issue.PullRequest)
if err != nil {
log.Error("PullRequestCodeOwnersReview: %v", err)
}

42
services/issue/pull.go
View File

@ -6,6 +6,7 @@ package issue
import (
"context"
"fmt"
"slices"
"time"
issues_model "code.gitea.io/gitea/models/issues"
@ -40,20 +41,31 @@ type ReviewRequestNotifier struct {
ReviewTeam *org_model.Team
}
func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}
var codeOwnerFiles = []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}
func IsCodeOwnerFile(f string) bool {
return slices.Contains(codeOwnerFiles, f)
}
func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
return PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, "", "") // no commit is provided, then it uses PR's base&head branch
}
func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_model.PullRequest, startCommitID, endCommitID string) ([]*ReviewRequestNotifier, error) {
if err := pr.LoadIssue(ctx); err != nil {
return nil, err
}
issue := pr.Issue
if pr.IsWorkInProgress(ctx) {
return nil, nil
}
if err := pr.LoadHeadRepo(ctx); err != nil {
return nil, err
}
if err := pr.LoadBaseRepo(ctx); err != nil {
return nil, err
}
pr.Issue.Repo = pr.BaseRepo
if pr.BaseRepo.IsFork {
return nil, nil
@ -71,7 +83,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue,
}
var data string
for _, file := range files {
for _, file := range codeOwnerFiles {
if blob, err := commit.GetBlobByPath(file); err == nil {
data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize)
if err == nil {
@ -79,18 +91,28 @@ func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue,
}
}
}
if data == "" {
return nil, nil
}
rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data)
if len(rules) == 0 {
return nil, nil
}
// get the mergebase
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
if err != nil {
return nil, err
if startCommitID == "" && endCommitID == "" {
// get the mergebase
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
if err != nil {
return nil, err
}
startCommitID = mergeBase
endCommitID = pr.GetGitRefName()
}
// 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.GetGitRefName())
changedFiles, err := repo.GetFilesChangedBetween(startCommitID, endCommitID)
if err != nil {
return nil, err
}

10
services/pull/merge.go
View File

@ -211,7 +211,15 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
}
defer releaser()
defer func() {
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
go AddTestPullRequestTask(TestPullRequestOptions{
RepoID: pr.BaseRepo.ID,
Doer: doer,
Branch: pr.BaseBranch,
IsSync: false,
IsForcePush: false,
OldCommitID: "",
NewCommitID: "",
})
}()
_, err = doMergeAndPush(ctx, pr, doer, mergeStyle, expectedHeadCommitID, message, repo_module.PushTriggerPRMergeToBase)

59
services/pull/pull.go
View File

@ -176,7 +176,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
}
if !pr.IsWorkInProgress(ctx) {
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr)
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr)
if err != nil {
return err
}
@ -372,19 +372,29 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest
return nil
}
type TestPullRequestOptions struct {
RepoID int64
Doer *user_model.User
Branch string
IsSync bool // True means it's a pull request synchronization, false means it's triggered for pull request merging or updating
IsForcePush bool
OldCommitID string
NewCommitID string
}
// AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch,
// and generate new patch for testing as needed.
func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, isSync bool, oldCommitID, newCommitID string) {
log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", repoID, branch)
func AddTestPullRequestTask(opts TestPullRequestOptions) {
log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", opts.RepoID, opts.Branch)
graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) {
// There is no sensible way to shut this down ":-("
// If you don't let it run all the way then you will lose data
// TODO: graceful: AddTestPullRequestTask needs to become a queue!
// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR.
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repoID, branch)
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, opts.RepoID, opts.Branch)
if err != nil {
log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err)
log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", opts.RepoID, opts.Branch, err)
return
}
@ -400,24 +410,24 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
}
AddToTaskQueue(ctx, pr)
comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID)
if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, doer, pr, comment)
notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
}
}
if isSync {
if opts.IsSync {
if err = prs.LoadAttributes(ctx); err != nil {
log.Error("PullRequestList.LoadAttributes: %v", err)
}
if invalidationErr := checkForInvalidation(ctx, prs, repoID, doer, branch); invalidationErr != nil {
if invalidationErr := checkForInvalidation(ctx, prs, opts.RepoID, opts.Doer, opts.Branch); invalidationErr != nil {
log.Error("checkForInvalidation: %v", invalidationErr)
}
if err == nil {
for _, pr := range prs {
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
if opts.NewCommitID != "" && opts.NewCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, opts.OldCommitID, opts.NewCommitID)
if err != nil {
log.Error("checkIfPRContentChanged: %v", err)
}
@ -433,12 +443,12 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
log.Error("GetFirstMatchProtectedBranchRule: %v", err)
}
if pb != nil && pb.DismissStaleApprovals {
if err := DismissApprovalReviews(ctx, doer, pr); err != nil {
if err := DismissApprovalReviews(ctx, opts.Doer, pr); err != nil {
log.Error("DismissApprovalReviews: %v", err)
}
}
}
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil {
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, opts.NewCommitID); err != nil {
log.Error("MarkReviewsAsNotStale: %v", err)
}
divergence, err := GetDiverging(ctx, pr)
@ -452,15 +462,30 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
}
}
notify_service.PullRequestSynchronized(ctx, doer, pr)
if !pr.IsWorkInProgress(ctx) {
var reviewNotifiers []*issue_service.ReviewRequestNotifier
if opts.IsForcePush {
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr)
} else {
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, opts.OldCommitID, opts.NewCommitID)
}
if err != nil {
log.Error("PullRequestCodeOwnersReview: %v", err)
}
if len(reviewNotifiers) > 0 {
issue_service.ReviewRequestNotify(ctx, pr.Issue, opts.Doer, reviewNotifiers)
}
}
notify_service.PullRequestSynchronized(ctx, opts.Doer, pr)
}
}
}
log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch)
prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", opts.RepoID, opts.Branch)
prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, opts.RepoID, opts.Branch)
if err != nil {
log.Error("Find pull requests [base_repo_id: %d, base_branch: %s]: %v", repoID, branch, err)
log.Error("Find pull requests [base_repo_id: %d, base_branch: %s]: %v", opts.RepoID, opts.Branch, err)
return
}
for _, pr := range prs {

20
services/pull/update.go
View File

@ -42,7 +42,15 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
if rebase {
defer func() {
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
go AddTestPullRequestTask(TestPullRequestOptions{
RepoID: pr.BaseRepo.ID,
Doer: doer,
Branch: pr.BaseBranch,
IsSync: false,
IsForcePush: false,
OldCommitID: "",
NewCommitID: "",
})
}()
return updateHeadByRebaseOnToBase(ctx, pr, doer)
@ -83,7 +91,15 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase)
defer func() {
go AddTestPullRequestTask(doer, reversePR.HeadRepo.ID, reversePR.HeadBranch, false, "", "")
go AddTestPullRequestTask(TestPullRequestOptions{
RepoID: reversePR.HeadRepo.ID,
Doer: doer,
Branch: reversePR.HeadBranch,
IsSync: false,
IsForcePush: false,
OldCommitID: "",
NewCommitID: "",
})
}()
return err

12
services/repository/push.go
View File

@ -166,7 +166,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
branch := opts.RefFullName.BranchName()
if !opts.IsDelRef() {
log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, opts.OldCommitID, opts.NewCommitID)
newCommit, err := gitRepo.GetCommit(opts.NewCommitID)
if err != nil {
@ -208,6 +207,17 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
log.Error("IsForcePush %s:%s failed: %v", repo.FullName(), branch, err)
}
// only update branch can trigger pull request task because the pull request hasn't been created yet when creaing a branch
go pull_service.AddTestPullRequestTask(pull_service.TestPullRequestOptions{
RepoID: repo.ID,
Doer: pusher,
Branch: branch,
IsSync: true,
IsForcePush: isForcePush,
OldCommitID: opts.OldCommitID,
NewCommitID: opts.NewCommitID,
})
if isForcePush {
log.Trace("Push %s is a force push", opts.NewCommitID)

42
tests/integration/pull_review_test.go
View File

@ -8,6 +8,7 @@ import (
"net/http/httptest"
"net/url"
"path"
"sort"
"strings"
"testing"
@ -67,7 +68,7 @@ func TestPullView_CodeOwner(t *testing.T) {
{
Operation: "create",
TreePath: "CODEOWNERS",
ContentReader: strings.NewReader("README.md @user5\n"),
ContentReader: strings.NewReader("README.md @user5\nuser8-file.md @user8\n"),
},
},
})
@ -75,7 +76,7 @@ func TestPullView_CodeOwner(t *testing.T) {
t.Run("First Pull Request", func(t *testing.T) {
// create a new branch to prepare for pull request
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
resp1, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
NewBranch: "codeowner-basebranch",
Files: []*files_service.ChangeRepoFile{
{
@ -95,7 +96,37 @@ func TestPullView_CodeOwner(t *testing.T) {
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
assert.NoError(t, pr.LoadIssue(db.DefaultContext))
err := issue_service.ChangeTitle(db.DefaultContext, pr.Issue, user2, "[WIP] Test Pull Request")
reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 5, reviewNotifiers[0].Reviewer.ID)
// update the file on the pr branch
resp2, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
OldBranch: "codeowner-basebranch",
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
TreePath: "user8-file.md",
ContentReader: strings.NewReader("# This is a new project2\n"),
},
},
})
assert.NoError(t, err)
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 2)
reviewerIDs := []int64{reviewNotifiers[0].Reviewer.ID, reviewNotifiers[1].Reviewer.ID}
sort.Slice(reviewerIDs, func(i, j int) bool { return reviewerIDs[i] < reviewerIDs[j] })
assert.EqualValues(t, []int64{5, 8}, reviewerIDs)
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(db.DefaultContext, pr, resp1.Commit.SHA, resp2.Commit.SHA)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 8, reviewNotifiers[0].Reviewer.ID)
err = issue_service.ChangeTitle(db.DefaultContext, pr.Issue, user2, "[WIP] Test Pull Request")
assert.NoError(t, err)
prUpdated1 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
assert.NoError(t, prUpdated1.LoadIssue(db.DefaultContext))
@ -140,6 +171,11 @@ func TestPullView_CodeOwner(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 8, reviewNotifiers[0].Reviewer.ID)
})
t.Run("Forked Repo Pull Request", func(t *testing.T) {