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
Show file tree
Hide file tree
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
Expand Up @@ -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
Expand Down
11 changes: 3 additions & 8 deletions models/issues/pull_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion models/issues/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
56 changes: 55 additions & 1 deletion routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
sillyguodong marked this conversation as resolved.
Show resolved Hide resolved
return
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 5 additions & 11 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down