Branches not at ref commit ID should not be listed as Merged (#9614)

integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head

@@ -0,0 +1 @@
+4a357436d925b5c974181ff12a994538ddc5a269

models/pull.go

@@ -136,7 +136,7 @@ func (pr *PullRequest) LoadHeadRepo() error {
 		if has, err := x.ID(pr.HeadRepoID).Get(&repo); err != nil {
 			return err
 		} else if !has {
-			return ErrRepoNotExist{ID: pr.BaseRepoID}
+			return ErrRepoNotExist{ID: pr.HeadRepoID}
 		}
 		pr.HeadRepo = &repo
 	}

routers/repo/branch.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/repofiles"
 	"code.gitea.io/gitea/modules/util"
+	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 const (
@@ -32,6 +33,7 @@ type Branch struct {
 	CommitsAhead      int
 	CommitsBehind     int
 	LatestPullRequest *models.PullRequest
+	MergeMovedOn      bool
 }
 
 // Branches render repository branch page
@@ -168,6 +170,12 @@ func loadBranches(ctx *context.Context) []*Branch {
 		return nil
 	}
 
+	repoIDToRepo := map[int64]*models.Repository{}
+	repoIDToRepo[ctx.Repo.Repository.ID] = ctx.Repo.Repository
+
+	repoIDToGitRepo := map[int64]*git.Repository{}
+	repoIDToGitRepo[ctx.Repo.Repository.ID] = ctx.Repo.GitRepo
+
 	branches := make([]*Branch, len(rawBranches))
 	for i := range rawBranches {
 		commit, err := rawBranches[i].GetCommit()
@@ -196,11 +204,46 @@ func loadBranches(ctx *context.Context) []*Branch {
 			ctx.ServerError("GetLatestPullRequestByHeadInfo", err)
 			return nil
 		}
+		headCommit := commit.ID.String()
+
+		mergeMovedOn := false
 		if pr != nil {
+			pr.HeadRepo = ctx.Repo.Repository
 			if err := pr.LoadIssue(); err != nil {
 				ctx.ServerError("pr.LoadIssue", err)
 				return nil
 			}
+			if repo, ok := repoIDToRepo[pr.BaseRepoID]; ok {
+				pr.BaseRepo = repo
+			} else if err := pr.LoadBaseRepo(); err != nil {
+				ctx.ServerError("pr.LoadBaseRepo", err)
+				return nil
+			} else {
+				repoIDToRepo[pr.BaseRepoID] = pr.BaseRepo
+			}
+
+			if pr.HasMerged {
+				baseGitRepo, ok := repoIDToGitRepo[pr.BaseRepoID]
+				if !ok {
+					baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath())
+					if err != nil {
+						ctx.ServerError("OpenRepository", err)
+						return nil
+					}
+					defer baseGitRepo.Close()
+					repoIDToGitRepo[pr.BaseRepoID] = baseGitRepo
+				}
+				pullCommit, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
+				if err != nil && err != plumbing.ErrReferenceNotFound {
+					ctx.ServerError("GetBranchCommitID", err)
+					return nil
+				}
+				if err == nil && headCommit != pullCommit {
+					// the head has moved on from the merge - we shouldn't delete
+					mergeMovedOn = true
+				}
+			}
+
 		}
 
 		branches[i] = &Branch{
@@ -210,6 +253,7 @@ func loadBranches(ctx *context.Context) []*Branch {
 			CommitsAhead:      divergence.Ahead,
 			CommitsBehind:     divergence.Behind,
 			LatestPullRequest: pr,
+			MergeMovedOn:      mergeMovedOn,
 		}
 	}
 

routers/repo/issue.go

@@ -931,7 +931,10 @@ func ViewIssue(ctx *context.Context) {
 			ctx.Data["IsBlockedByApprovals"] = pull.ProtectedBranch.RequiredApprovals > 0 && cnt < pull.ProtectedBranch.RequiredApprovals
 			ctx.Data["GrantedApprovals"] = cnt
 		}
-		ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch)
+		ctx.Data["IsPullBranchDeletable"] = canDelete &&
+			pull.HeadRepo != nil &&
+			git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) &&
+			(!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"])
 
 		ctx.Data["PullReviewersWithType"], err = models.GetReviewersByPullID(issue.ID)
 		if err != nil {

routers/repo/pull.go

@@ -315,25 +315,37 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 	repo := ctx.Repo.Repository
 	pull := issue.PullRequest
 
-	var err error
-	if err = pull.GetHeadRepo(); err != nil {
+	if err := pull.GetHeadRepo(); err != nil {
 		ctx.ServerError("GetHeadRepo", err)
 		return nil
 	}
 
+	if err := pull.GetBaseRepo(); err != nil {
+		ctx.ServerError("GetBaseRepo", err)
+		return nil
+	}
+
 	setMergeTarget(ctx, pull)
 
-	if err = pull.LoadProtectedBranch(); err != nil {
+	if err := pull.LoadProtectedBranch(); err != nil {
 		ctx.ServerError("GetLatestCommitStatus", err)
 		return nil
 	}
 	ctx.Data["EnableStatusCheck"] = pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck
 
-	var headGitRepo *git.Repository
+	baseGitRepo, err := git.OpenRepository(pull.BaseRepo.RepoPath())
+	if err != nil {
+		ctx.ServerError("OpenRepository", err)
+		return nil
+	}
+	defer baseGitRepo.Close()
 	var headBranchExist bool
+	var headBranchSha string
 	// HeadRepo may be missing
 	if pull.HeadRepo != nil {
-		headGitRepo, err = git.OpenRepository(pull.HeadRepo.RepoPath())
+		var err error
+
+		headGitRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath())
 		if err != nil {
 			ctx.ServerError("OpenRepository", err)
 			return nil
@@ -343,46 +355,53 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 		headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch)
 
 		if headBranchExist {
-			sha, err := headGitRepo.GetBranchCommitID(pull.HeadBranch)
+			headBranchSha, err = headGitRepo.GetBranchCommitID(pull.HeadBranch)
 			if err != nil {
 				ctx.ServerError("GetBranchCommitID", err)
 				return nil
 			}
+		}
+	}
 
-			commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
-			if err != nil {
-				ctx.ServerError("GetLatestCommitStatus", err)
-				return nil
-			}
-			if len(commitStatuses) > 0 {
-				ctx.Data["LatestCommitStatuses"] = commitStatuses
-				ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
-			}
+	sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
+	if err != nil {
+		ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
+		return nil
+	}
 
-			if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
-				ctx.Data["is_context_required"] = func(context string) bool {
-					for _, c := range pull.ProtectedBranch.StatusCheckContexts {
-						if c == context {
-							return true
-						}
-					}
-					return false
+	commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
+	if err != nil {
+		ctx.ServerError("GetLatestCommitStatus", err)
+		return nil
+	}
+	if len(commitStatuses) > 0 {
+		ctx.Data["LatestCommitStatuses"] = commitStatuses
+		ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
+	}
+
+	if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
+		ctx.Data["is_context_required"] = func(context string) bool {
+			for _, c := range pull.ProtectedBranch.StatusCheckContexts {
+				if c == context {
+					return true
 				}
-				ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
 			}
+			return false
 		}
+		ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
 	}
 
-	if pull.HeadRepo == nil || !headBranchExist {
+	ctx.Data["HeadBranchMovedOn"] = headBranchSha != sha
+	ctx.Data["HeadBranchCommitID"] = headBranchSha
+	ctx.Data["PullHeadCommitID"] = sha
+
+	if pull.HeadRepo == nil || !headBranchExist || headBranchSha != sha {
 		ctx.Data["IsPullRequestBroken"] = true
 		ctx.Data["HeadTarget"] = "deleted"
-		ctx.Data["NumCommits"] = 0
-		ctx.Data["NumFiles"] = 0
-		return nil
 	}
 
-	compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(repo.Owner.Name, repo.Name),
-		pull.BaseBranch, pull.HeadBranch)
+	compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
+		pull.BaseBranch, pull.GetGitRefName())
 	if err != nil {
 		if strings.Contains(err.Error(), "fatal: Not a valid object name") {
 			ctx.Data["IsPullRequestBroken"] = true

templates/repo/branch/list.tmpl

@@ -80,6 +80,12 @@
 												<button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
 											</a>
 											{{end}}
+										{{else if and .LatestPullRequest.HasMerged .MergeMovedOn}}
+											{{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
+											<a href="{{$.RepoLink}}/compare/{{$.DefaultBranch | EscapePound}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{$.Owner.Name}}:{{end}}{{.Name | EscapePound}}">
+												<button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
+											</a>
+											{{end}}
 										{{else}}
 											<a href="{{$.RepoLink}}/pulls/{{.LatestPullRequest.Issue.Index}}">#{{.LatestPullRequest.Issue.Index}}</a>
 											{{if .LatestPullRequest.HasMerged}}

templates/repo/issue/view_content/pull.tmpl

@@ -71,7 +71,7 @@
 						{{$.i18n.Tr "repo.pulls.reopen_to_merge"}}
 					{{end}}
 				</div>
-				{{if .IsPullBranchDeletable}}
+				{{if and .IsPullBranchDeletable ( not .IsPullRequestBroken )}}
 					<div class="ui divider"></div>
 					<div>
 						<a class="delete-button ui red button" href="" data-url="{{.DeleteBranchLink}}">{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</a>