diff --git a/routers/web/repo/view_file.go b/routers/web/repo/view_file.go
index 5784297918..4ce7a8e3a4 100644
--- a/routers/web/repo/view_file.go
+++ b/routers/web/repo/view_file.go
@@ -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 {
diff --git a/services/issue/issue.go b/services/issue/issue.go
index 586b6031c8..455a1ec297 100644
--- a/services/issue/issue.go
+++ b/services/issue/issue.go
@@ -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)
}
diff --git a/services/issue/pull.go b/services/issue/pull.go
index 896802108d..bd19c25436 100644
--- a/services/issue/pull.go
+++ b/services/issue/pull.go
@@ -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
}
diff --git a/services/pull/merge.go b/services/pull/merge.go
index 665909e99a..1e1ca55bc1 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -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)
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 1f06d6b182..21f31e16cf 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -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 {
diff --git a/services/pull/update.go b/services/pull/update.go
index abf7ad4509..3e00dd4e65 100644
--- a/services/pull/update.go
+++ b/services/pull/update.go
@@ -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
diff --git a/services/repository/push.go b/services/repository/push.go
index 0ea51f9c07..00d4fb11af 100644
--- a/services/repository/push.go
+++ b/services/repository/push.go
@@ -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)
diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go
index 68de421413..cd354e1b6c 100644
--- a/tests/integration/pull_review_test.go
+++ b/tests/integration/pull_review_test.go
@@ -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) {