Skip to content

Commit e962ade

Browse files
sillyguodongsillyguodong
authored
Refresh the refernce of the closed PR when reopening (#24231)
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](https://github.com/go-gitea/gitea/blob/74225033413dc0f2b308bbe069f6d185b551e364/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`.
1 parent 80765aa commit e962ade

File tree

5 files changed

+68
-21
lines changed

5 files changed

+68
-21
lines changed

‎models/issues/pull.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,10 @@ func (pr *PullRequest) GetGitRefName() string {
433433
return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index)
434434
}
435435

436+
func (pr *PullRequest) GetGitHeadBranchRefName() string {
437+
return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch)
438+
}
439+
436440
// IsChecking returns true if this pull request is still checking conflict.
437441
func (pr *PullRequest) IsChecking() bool {
438442
return pr.Status == PullRequestStatusChecking

‎models/issues/pull_list.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,11 @@ func listPullRequestStatement(baseRepoID int64, opts *PullRequestsOptions) (*xor
5151
}
5252

5353
// GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged
54-
// by given head information (repo and branch).
55-
// arg `includeClosed` controls whether the SQL returns closed PRs
56-
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string, includeClosed bool) ([]*PullRequest, error) {
54+
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string) ([]*PullRequest, error) {
5755
prs := make([]*PullRequest, 0, 2)
5856
sess := db.GetEngine(db.DefaultContext).
5957
Join("INNER", "issue", "issue.id = pull_request.issue_id").
60-
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND flow = ?", repoID, branch, false, PullRequestFlowGithub)
61-
if !includeClosed {
62-
sess.Where("issue.is_closed = ?", false)
63-
}
58+
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?", repoID, branch, false, false, PullRequestFlowGithub)
6459
return prs, sess.Find(&prs)
6560
}
6661

@@ -74,7 +69,7 @@ func CanMaintainerWriteToBranch(p access_model.Permission, branch string, user *
7469
return false
7570
}
7671

77-
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch, false)
72+
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch)
7873
if err != nil {
7974
return false
8075
}

‎models/issues/pull_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
118118

119119
func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
120120
assert.NoError(t, unittest.PrepareTestDatabase())
121-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2", false)
121+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2")
122122
assert.NoError(t, err)
123123
assert.Len(t, prs, 1)
124124
for _, pr := range prs {

‎routers/web/repo/issue.go‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"code.gitea.io/gitea/modules/log"
3838
"code.gitea.io/gitea/modules/markup"
3939
"code.gitea.io/gitea/modules/markup/markdown"
40+
repo_module "code.gitea.io/gitea/modules/repository"
4041
"code.gitea.io/gitea/modules/setting"
4142
api "code.gitea.io/gitea/modules/structs"
4243
"code.gitea.io/gitea/modules/templates/vars"
@@ -2784,7 +2785,8 @@ func NewComment(ctx *context.Context) {
27842785
pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
27852786
if err != nil {
27862787
if !issues_model.IsErrPullRequestNotExist(err) {
2787-
ctx.ServerError("GetUnmergedPullRequest", err)
2788+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
2789+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index))
27882790
return
27892791
}
27902792
}
@@ -2794,6 +2796,57 @@ func NewComment(ctx *context.Context) {
27942796
issue.PullRequest.HeadCommitID = ""
27952797
pull_service.AddToTaskQueue(issue.PullRequest)
27962798
}
2799+
2800+
// 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
2801+
// get head commit of PR
2802+
prHeadRef := pull.GetGitRefName()
2803+
if err := pull.LoadBaseRepo(ctx); err != nil {
2804+
ctx.ServerError("Unable to load base repo", err)
2805+
return
2806+
}
2807+
prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef)
2808+
if err != nil {
2809+
ctx.ServerError("Get head commit Id of pr fail", err)
2810+
return
2811+
}
2812+
2813+
// get head commit of branch in the head repo
2814+
if err := pull.LoadHeadRepo(ctx); err != nil {
2815+
ctx.ServerError("Unable to load head repo", err)
2816+
return
2817+
}
2818+
if ok := git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.BaseBranch); !ok {
2819+
// todo localize
2820+
ctx.Flash.Error("The origin branch is delete, cannot reopen.")
2821+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index))
2822+
return
2823+
}
2824+
headBranchRef := pull.GetGitHeadBranchRefName()
2825+
headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef)
2826+
if err != nil {
2827+
ctx.ServerError("Get head commit Id of head branch fail", err)
2828+
return
2829+
}
2830+
2831+
err = pull.LoadIssue(ctx)
2832+
if err != nil {
2833+
ctx.ServerError("load the issue of pull request error", err)
2834+
return
2835+
}
2836+
2837+
if prHeadCommitID != headBranchCommitID {
2838+
// force push to base repo
2839+
err := git.Push(ctx, pull.HeadRepo.RepoPath(), git.PushOptions{
2840+
Remote: pull.BaseRepo.RepoPath(),
2841+
Branch: pull.HeadBranch + ":" + prHeadRef,
2842+
Force: true,
2843+
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
2844+
})
2845+
if err != nil {
2846+
ctx.ServerError("force push error", err)
2847+
return
2848+
}
2849+
}
27972850
}
27982851

27992852
if pr != nil {
@@ -2822,6 +2875,7 @@ func NewComment(ctx *context.Context) {
28222875
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
28232876
}
28242877
}
2878+
28252879
}
28262880

28272881
// Redirect to comment hashtag if there is any actual content.

‎services/pull/pull.go‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,8 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
257257
// If you don't let it run all the way then you will lose data
258258
// TODO: graceful: AddTestPullRequestTask needs to become a queue!
259259

260-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, true)
260+
// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR.
261+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
261262
if err != nil {
262263
log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err)
263264
return
@@ -274,12 +275,9 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
274275
continue
275276
}
276277

277-
// If the PR is closed, someone still push some commits to the PR,
278-
// 1. We will insert comments of commits, but hidden until the PR is reopened.
279-
// 2. We won't send any notification.
280278
AddToTaskQueue(pr)
281279
comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
282-
if err == nil && comment != nil && !pr.Issue.IsClosed {
280+
if err == nil && comment != nil {
283281
notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
284282
}
285283
}
@@ -294,10 +292,6 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
294292
}
295293
if err == nil {
296294
for _, pr := range prs {
297-
if pr.Issue.IsClosed {
298-
// The closed PR never trigger action or webhook
299-
continue
300-
}
301295
if newCommitID != "" && newCommitID != git.EmptySHA {
302296
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
303297
if err != nil {
@@ -503,7 +497,7 @@ func (errs errlist) Error() string {
503497

504498
// CloseBranchPulls close all the pull requests who's head branch is the branch
505499
func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error {
506-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, false)
500+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
507501
if err != nil {
508502
return err
509503
}
@@ -539,7 +533,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
539533

540534
var errs errlist
541535
for _, branch := range branches {
542-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name, false)
536+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name)
543537
if err != nil {
544538
return err
545539
}

0 commit comments

Comments
 (0)