Fix the handling of closed PR when push commits

options/locale/locale_en-US.ini

@@ -1648,6 +1648,7 @@ pulls.push_rejected = Merge Failed: The push was rejected. Review the Git Hooks
 pulls.push_rejected_summary = Full Rejection Message
 pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the Git Hooks for this repository
 pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
+pulls.open_unmerged_pull_merged = `You cannot perform a reopen operation because the commits of this pull request have been merged into the base branch by another pull request.`
 pulls.status_checking = Some checks are pending
 pulls.status_checks_success = All checks were successful
 pulls.status_checks_warning = Some checks reported warnings

routers/web/repo/issue.go

@@ -2768,6 +2768,36 @@ func NewComment(ctx *context.Context) {
 			if form.Status == "reopen" && issue.IsPull {
 				pull := issue.PullRequest
 				var err error
+				// Check whether the head commit of PR exists in base branch
+				// Get head commit ID 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 fail", err)
+					return
+				}
+				// Open base repo
+				baseRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pull.BaseRepo.RepoPath())
+				if err != nil {
+					ctx.ServerError("Unable to open base repo", err)
+					return
+				}
+				defer closer.Close()
+				ok, err := baseRepo.IsCommitInBranch(prHeadCommitID, pull.BaseBranch)
+				if err != nil {
+					ctx.ServerError("", err)
+					return
+				}
+				if ok {
+					ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_merged"))
+					ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index))
+					return
+				}
+
 				pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
 				if err != nil {
 					if !issues_model.IsErrPullRequestNotExist(err) {

services/pull/pull.go

@@ -265,22 +265,48 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
 
 		for _, pr := range prs {
 			log.Trace("Updating PR[%d]: composing new test task", pr.ID)
-			if pr.Flow == issues_model.PullRequestFlowGithub {
-				if err := PushToBaseRepo(ctx, pr); err != nil {
-					log.Error("PushToBaseRepo: %v", err)
+
+			if err = pr.LoadIssue(ctx); err != nil {
+				log.Error("Load PR[%d]'s issue fail, error: %v", pr.ID, err)
+				return
+			}
+			if !pr.Issue.IsClosed {
+				// PR is unmerged but open.
+				// need do these: trigger action, create comment, notification(ui and email), execute `pushToBaseRepo()`
+				pushPRToBaseRepo(ctx, pr)
+				commentAndNotifyPullRequestPushComments(ctx, doer, pr, oldCommitID, newCommitID)
+			} else {
+				// Get head commit ID of PR
+				prHeadRef := pr.GetGitRefName()
+				if err := pr.LoadBaseRepo(ctx); err != nil {
+					log.Error("LoadBaseRepo(%d), error: %v", pr.ID, err)
+					return
+				}
+				prHeadCommitID, err := git.GetFullCommitID(ctx, pr.BaseRepo.RepoPath(), prHeadRef)
+				if err != nil {
+					log.Error("GetFullCommitID(%s) in %s: %v", prHeadRef, pr.BaseRepo.FullName(), err)
+					return
+				}
+				// Open the base repo of PR
+				baseRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo.RepoPath())
+				if err != nil {
+					log.Error("Unable to open base repo, Error: %v", err)
+					return
+				}
+				defer closer.Close()
+				// If we can find it that the `prHeadCommitID` already exists in PR's `base_branch`.
+				// It means that all changes of the PR have been merged into PR's base_branch`.
+				// So we don't need to create action task, comment, notification, execute `pushToBaseRepo()` in this case.
+				ok, err := baseRepo.IsCommitInBranch(prHeadCommitID, pr.BaseBranch)
+				if err != nil {
+					log.Error("Check It that whether prHeadRef is in the baseBranch fail, Error: %v", err)
+					return
+				}
+				if ok {
 					continue
 				}
-			} else {
-				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 {
-				notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
+				pushPRToBaseRepo(ctx, pr)
+				commentAndNotifyPullRequestPushComments(ctx, doer, pr, oldCommitID, newCommitID)
 			}
 		}
 
@@ -294,6 +320,10 @@ 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 {
@@ -349,6 +379,22 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
 	})
 }
 
+func commentAndNotifyPullRequestPushComments(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string) {
+	comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
+	if err == nil && comment != nil {
+		notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
+	}
+	AddToTaskQueue(pr)
+}
+
+func pushPRToBaseRepo(ctx context.Context, pr *issues_model.PullRequest) {
+	if pr.Flow == issues_model.PullRequestFlowGithub {
+		if err := PushToBaseRepo(ctx, pr); err != nil {
+			log.Error("PushToBaseRepo: %v", err)
+		}
+	}
+}
+
 // checkIfPRContentChanged checks if diff to target branch has changed by push
 // A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged
 func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) {