Skip to content

Commit 306b7b5

Browse files
Refresh the refernce of the closed PR when reopening (#24231) (#24587)
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](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`. Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com>
1 parent 4498a26 commit 306b7b5

File tree

5 files changed

+68
-21
lines changed

5 files changed

+68
-21
lines changed

models/issues/pull.go

+4
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

+3-8
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

+1-1
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

+55-1
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"
@@ -2735,7 +2736,8 @@ func NewComment(ctx *context.Context) {
27352736
pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
27362737
if err != nil {
27372738
if !issues_model.IsErrPullRequestNotExist(err) {
2738-
ctx.ServerError("GetUnmergedPullRequest", err)
2739+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
2740+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index))
27392741
return
27402742
}
27412743
}
@@ -2745,6 +2747,57 @@ func NewComment(ctx *context.Context) {
27452747
issue.PullRequest.HeadCommitID = ""
27462748
pull_service.AddToTaskQueue(issue.PullRequest)
27472749
}
2750+
2751+
// 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
2752+
// get head commit of PR
2753+
prHeadRef := pull.GetGitRefName()
2754+
if err := pull.LoadBaseRepo(ctx); err != nil {
2755+
ctx.ServerError("Unable to load base repo", err)
2756+
return
2757+
}
2758+
prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef)
2759+
if err != nil {
2760+
ctx.ServerError("Get head commit Id of pr fail", err)
2761+
return
2762+
}
2763+
2764+
// get head commit of branch in the head repo
2765+
if err := pull.LoadHeadRepo(ctx); err != nil {
2766+
ctx.ServerError("Unable to load head repo", err)
2767+
return
2768+
}
2769+
if ok := git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.BaseBranch); !ok {
2770+
// todo localize
2771+
ctx.Flash.Error("The origin branch is delete, cannot reopen.")
2772+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, pull.Index))
2773+
return
2774+
}
2775+
headBranchRef := pull.GetGitHeadBranchRefName()
2776+
headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef)
2777+
if err != nil {
2778+
ctx.ServerError("Get head commit Id of head branch fail", err)
2779+
return
2780+
}
2781+
2782+
err = pull.LoadIssue(ctx)
2783+
if err != nil {
2784+
ctx.ServerError("load the issue of pull request error", err)
2785+
return
2786+
}
2787+
2788+
if prHeadCommitID != headBranchCommitID {
2789+
// force push to base repo
2790+
err := git.Push(ctx, pull.HeadRepo.RepoPath(), git.PushOptions{
2791+
Remote: pull.BaseRepo.RepoPath(),
2792+
Branch: pull.HeadBranch + ":" + prHeadRef,
2793+
Force: true,
2794+
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
2795+
})
2796+
if err != nil {
2797+
ctx.ServerError("force push error", err)
2798+
return
2799+
}
2800+
}
27482801
}
27492802

27502803
if pr != nil {
@@ -2773,6 +2826,7 @@ func NewComment(ctx *context.Context) {
27732826
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
27742827
}
27752828
}
2829+
27762830
}
27772831

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

services/pull/pull.go

+5-11
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)