Backport #24231 by @sillyguodong
Close #24213
Replace #23830
#### Cause
- Before, in order to making PR can get latest commit after reopening,
the `ref`(${REPO_PATH}/refs/pull/${PR_INDEX}/head) of evrey closed PR
will be updated when pushing commits to the `head branch` of the closed
PR.
#### Changes
- For closed PR , won't perform these behavior: insert`comment`, push
`notification` (UI and email), exectue
[pushToBaseRepo](7422503341/services/pull/pull.go (L409))
function and trigger `action` any more when pushing to the `head branch`
of the closed PR.
- Refresh the reference of the PR when reopening the closed PR (**even
if the head branch has been deleted before**). Make the reference of PR
consistent with the `head branch`.
Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com>
tags/v1.19.4
@@ -433,6 +433,10 @@ func (pr *PullRequest) GetGitRefName() string { | |||
return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index) | |||
} | |||
func (pr *PullRequest) GetGitHeadBranchRefName() string { | |||
return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch) | |||
} | |||
// IsChecking returns true if this pull request is still checking conflict. | |||
func (pr *PullRequest) IsChecking() bool { | |||
return pr.Status == PullRequestStatusChecking |
@@ -51,16 +51,11 @@ func listPullRequestStatement(baseRepoID int64, opts *PullRequestsOptions) (*xor | |||
} | |||
// GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged | |||
// by given head information (repo and branch). | |||
// arg `includeClosed` controls whether the SQL returns closed PRs | |||
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string, includeClosed bool) ([]*PullRequest, error) { | |||
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string) ([]*PullRequest, error) { | |||
prs := make([]*PullRequest, 0, 2) | |||
sess := db.GetEngine(db.DefaultContext). | |||
Join("INNER", "issue", "issue.id = pull_request.issue_id"). | |||
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND flow = ?", repoID, branch, false, PullRequestFlowGithub) | |||
if !includeClosed { | |||
sess.Where("issue.is_closed = ?", false) | |||
} | |||
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?", repoID, branch, false, false, PullRequestFlowGithub) | |||
return prs, sess.Find(&prs) | |||
} | |||
@@ -74,7 +69,7 @@ func CanMaintainerWriteToBranch(p access_model.Permission, branch string, user * | |||
return false | |||
} | |||
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch, false) | |||
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch) | |||
if err != nil { | |||
return false | |||
} |
@@ -118,7 +118,7 @@ func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) { | |||
func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) { | |||
assert.NoError(t, unittest.PrepareTestDatabase()) | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2", false) | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2") | |||
assert.NoError(t, err) | |||
assert.Len(t, prs, 1) | |||
for _, pr := range prs { |
@@ -37,6 +37,7 @@ import ( | |||
"code.gitea.io/gitea/modules/log" | |||
"code.gitea.io/gitea/modules/markup" | |||
"code.gitea.io/gitea/modules/markup/markdown" | |||
repo_module "code.gitea.io/gitea/modules/repository" | |||
"code.gitea.io/gitea/modules/setting" | |||
api "code.gitea.io/gitea/modules/structs" | |||
"code.gitea.io/gitea/modules/templates/vars" | |||
@@ -2735,7 +2736,8 @@ func NewComment(ctx *context.Context) { | |||
pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow) | |||
if err != nil { | |||
if !issues_model.IsErrPullRequestNotExist(err) { | |||
ctx.ServerError("GetUnmergedPullRequest", err) | |||
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) | |||
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index)) | |||
return | |||
} | |||
} | |||
@@ -2745,6 +2747,57 @@ func NewComment(ctx *context.Context) { | |||
issue.PullRequest.HeadCommitID = "" | |||
pull_service.AddToTaskQueue(issue.PullRequest) | |||
} | |||
// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo | |||
// get head commit of PR | |||
prHeadRef := pull.GetGitRefName() | |||
if err := pull.LoadBaseRepo(ctx); err != nil { | |||
ctx.ServerError("Unable to load base repo", err) | |||
return | |||
} | |||
prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef) | |||
if err != nil { | |||
ctx.ServerError("Get head commit Id of pr fail", err) | |||
return | |||
} | |||
// get head commit of branch in the head repo | |||
if err := pull.LoadHeadRepo(ctx); err != nil { | |||
ctx.ServerError("Unable to load head repo", err) | |||
return | |||
} | |||
if ok := git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.BaseBranch); !ok { | |||
// todo localize | |||
ctx.Flash.Error("The origin branch is delete, cannot reopen.") | |||
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index)) | |||
return | |||
} | |||
headBranchRef := pull.GetGitHeadBranchRefName() | |||
headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef) | |||
if err != nil { | |||
ctx.ServerError("Get head commit Id of head branch fail", err) | |||
return | |||
} | |||
err = pull.LoadIssue(ctx) | |||
if err != nil { | |||
ctx.ServerError("load the issue of pull request error", err) | |||
return | |||
} | |||
if prHeadCommitID != headBranchCommitID { | |||
// force push to base repo | |||
err := git.Push(ctx, pull.HeadRepo.RepoPath(), git.PushOptions{ | |||
Remote: pull.BaseRepo.RepoPath(), | |||
Branch: pull.HeadBranch + ":" + prHeadRef, | |||
Force: true, | |||
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo), | |||
}) | |||
if err != nil { | |||
ctx.ServerError("force push error", err) | |||
return | |||
} | |||
} | |||
} | |||
if pr != nil { | |||
@@ -2773,6 +2826,7 @@ func NewComment(ctx *context.Context) { | |||
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) | |||
} | |||
} | |||
} | |||
// Redirect to comment hashtag if there is any actual content. |
@@ -257,7 +257,8 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, | |||
// If you don't let it run all the way then you will lose data | |||
// TODO: graceful: AddTestPullRequestTask needs to become a queue! | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, true) | |||
// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR. | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch) | |||
if err != nil { | |||
log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err) | |||
return | |||
@@ -274,12 +275,9 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, | |||
continue | |||
} | |||
// If the PR is closed, someone still push some commits to the PR, | |||
// 1. We will insert comments of commits, but hidden until the PR is reopened. | |||
// 2. We won't send any notification. | |||
AddToTaskQueue(pr) | |||
comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID) | |||
if err == nil && comment != nil && !pr.Issue.IsClosed { | |||
if err == nil && comment != nil { | |||
notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment) | |||
} | |||
} | |||
@@ -294,10 +292,6 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, | |||
} | |||
if err == nil { | |||
for _, pr := range prs { | |||
if pr.Issue.IsClosed { | |||
// The closed PR never trigger action or webhook | |||
continue | |||
} | |||
if newCommitID != "" && newCommitID != git.EmptySHA { | |||
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID) | |||
if err != nil { | |||
@@ -503,7 +497,7 @@ func (errs errlist) Error() string { | |||
// CloseBranchPulls close all the pull requests who's head branch is the branch | |||
func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error { | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, false) | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch) | |||
if err != nil { | |||
return err | |||
} | |||
@@ -539,7 +533,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re | |||
var errs errlist | |||
for _, branch := range branches { | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name, false) | |||
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name) | |||
if err != nil { | |||
return err | |||
} |