From e962ade99cfd0471273f3dcf956c7cd222472758 Mon Sep 17 00:00:00 2001 From: sillyguodong <33891828+sillyguodong@users.noreply.github.com> Date: Mon, 8 May 2023 14:39:32 +0800 Subject: [PATCH] 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`. --- 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 a15ebec0b..855d37867 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 9d361c5c5..f2adac070 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 8d99b2667..0b0609521 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 5c96d326a..4efac5c38 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" @@ -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 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. diff --git a/services/pull/pull.go b/services/pull/pull.go index 55dfd3c18..8f2befa8f 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 }