From 306b7b5d1dba0fc13299c2b018369c1b9d09a205 Mon Sep 17 00:00:00 2001
From: Giteabot <teabot@gitea.io>
Date: Mon, 8 May 2023 06:48:06 -0400
Subject: [PATCH] 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>
---
models/issues/pull.go | 4 +++
models/issues/pull_list.go | 11 ++------
models/issues/pull_test.go | 2 +-
routers/web/repo/issue.go | 56 +++++++++++++++++++++++++++++++++++++-
services/pull/pull.go | 16 ++++-------
5 files changed, 68 insertions(+), 21 deletions(-)
diff --git a/models/issues/pull.go b/models/issues/pull.go
index 6a1dc31556..88b6bba7f0 100644
--- a/models/issues/pull.go
+++ b/models/issues/pull.go
@@ -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
diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go
index 9d361c5c5d..f2adac0701 100644
--- a/models/issues/pull_list.go
+++ b/models/issues/pull_list.go
@@ -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
}
diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go
index bcd33329eb..8ce8eecc4a 100644
--- a/models/issues/pull_test.go
+++ b/models/issues/pull_test.go
@@ -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 {
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 4f370c610d..9c9bf9ad1e 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -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.
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 73d7c57148..d5abcec222 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -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
}