Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh the refernce of the closed PR when reopening #24231

Merged
merged 3 commits into from
May 8, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
@@ -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
11 changes: 3 additions & 8 deletions models/issues/pull_list.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion models/issues/pull_test.go
Original file line number Diff line number Diff line change
@@ -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 {
56 changes: 55 additions & 1 deletion routers/web/repo/issue.go
Original file line number Diff line number Diff line change
@@ -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"
@@ -2784,7 +2785,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
}
}
@@ -2794,6 +2796,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 {
@@ -2822,6 +2875,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.
16 changes: 5 additions & 11 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
@@ -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
}