Fix pull view when head repository or head branch missed and close related pull requests when delete head repository or head branch (#9927)

* fix pull view when head repository or head branch missed and close related pull requests when delete branch

* fix pull view broken when head repository deleted

* close pull requests when head repositories deleted

* Add tests for broken pull request head repository or branch

* fix typo

* ignore special error when close pull request

Co-authored-by: Lauris BH <lauris@nix.lv>

integrations/pull_create_test.go

@@ -106,3 +106,57 @@ func TestPullCreate_TitleEscape(t *testing.T) {
 		assert.Equal(t, "<u>XSS PR</u>", titleHTML)
 	})
 }
+
+func testUIDeleteBranch(t *testing.T, session *TestSession, ownerName, repoName, branchName string) {
+	relURL := "/" + path.Join(ownerName, repoName, "branches")
+	req := NewRequest(t, "GET", relURL)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+
+	req = NewRequestWithValues(t, "POST", relURL+"/delete", map[string]string{
+		"_csrf": getCsrf(t, htmlDoc.doc),
+		"name":  branchName,
+	})
+	session.MakeRequest(t, req, http.StatusOK)
+}
+
+func testDeleteRepository(t *testing.T, session *TestSession, ownerName, repoName string) {
+	relURL := "/" + path.Join(ownerName, repoName, "settings")
+	req := NewRequest(t, "GET", relURL)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+
+	req = NewRequestWithValues(t, "POST", relURL+"?action=delete", map[string]string{
+		"_csrf":     getCsrf(t, htmlDoc.doc),
+		"repo_name": repoName,
+	})
+	session.MakeRequest(t, req, http.StatusFound)
+}
+
+func TestPullBranchDelete(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		defer prepareTestEnv(t)()
+
+		session := loginUser(t, "user1")
+		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+		testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusFound)
+		testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n")
+		resp := testPullCreate(t, session, "user1", "repo1", "master1", "This is a pull title")
+
+		// check the redirected URL
+		url := resp.HeaderMap.Get("Location")
+		assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
+		req := NewRequest(t, "GET", url)
+		session.MakeRequest(t, req, http.StatusOK)
+
+		// delete head branch and confirm pull page is ok
+		testUIDeleteBranch(t, session, "user1", "repo1", "master1")
+		req = NewRequest(t, "GET", url)
+		session.MakeRequest(t, req, http.StatusOK)
+
+		// delete head repository and confirm pull page is ok
+		testDeleteRepository(t, session, "user1", "repo1")
+		req = NewRequest(t, "GET", url)
+		session.MakeRequest(t, req, http.StatusOK)
+	})
+}

models/pull.go

@@ -67,7 +67,11 @@ type PullRequest struct {
 // MustHeadUserName returns the HeadRepo's username if failed return blank
 func (pr *PullRequest) MustHeadUserName() string {
 	if err := pr.LoadHeadRepo(); err != nil {
-		log.Error("LoadHeadRepo: %v", err)
+		if !IsErrRepoNotExist(err) {
+			log.Error("LoadHeadRepo: %v", err)
+		} else {
+			log.Warn("LoadHeadRepo %d but repository does not exist: %v", pr.HeadRepoID, err)
+		}
 		return ""
 	}
 	return pr.HeadRepo.OwnerName

modules/repofiles/update.go

@@ -495,9 +495,18 @@ func PushUpdate(repo *models.Repository, branch string, opts PushUpdateOptions)
 		return err
 	}
 
-	log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
+	if !isDelRef {
+		if err = models.RemoveDeletedBranch(repo.ID, opts.Branch); err != nil {
+			log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, opts.Branch, err)
+		}
 
-	go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, opts.OldCommitID, opts.NewCommitID)
+		log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
+
+		go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, opts.OldCommitID, opts.NewCommitID)
+		// close all related pulls
+	} else if err = pull_service.CloseBranchPulls(pusher, repo.ID, branch); err != nil {
+		log.Error("close related pull request failed: %v", err)
+	}
 
 	if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil {
 		log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err)
@@ -544,12 +553,15 @@ func PushUpdates(repo *models.Repository, optsList []*PushUpdateOptions) error {
 			if err = models.RemoveDeletedBranch(repo.ID, opts.Branch); err != nil {
 				log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, opts.Branch, err)
 			}
+
+			log.Trace("TriggerTask '%s/%s' by %s", repo.Name, opts.Branch, pusher.Name)
+
+			go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true, opts.OldCommitID, opts.NewCommitID)
+			// close all related pulls
+		} else if err = pull_service.CloseBranchPulls(pusher, repo.ID, opts.Branch); err != nil {
+			log.Error("close related pull request failed: %v", err)
 		}
 
-		log.Trace("TriggerTask '%s/%s' by %s", repo.Name, opts.Branch, pusher.Name)
-
-		go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true, opts.OldCommitID, opts.NewCommitID)
-
 		if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil {
 			log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err)
 		}

routers/repo/pull.go

@@ -343,19 +343,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 
 	setMergeTarget(ctx, pull)
 
-	divergence, err := pull_service.GetDiverging(pull)
-	if err != nil {
-		ctx.ServerError("GetDiverging", err)
-		return nil
-	}
-	ctx.Data["Divergence"] = divergence
-	allowUpdate, err := pull_service.IsUserAllowedToUpdate(pull, ctx.User)
-	if err != nil {
-		ctx.ServerError("IsUserAllowedToUpdate", err)
-		return nil
-	}
-	ctx.Data["UpdateAllowed"] = allowUpdate
-
 	if err := pull.LoadProtectedBranch(); err != nil {
 		ctx.ServerError("LoadProtectedBranch", err)
 		return nil
@@ -392,6 +379,22 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 		}
 	}
 
+	if headBranchExist {
+		allowUpdate, err := pull_service.IsUserAllowedToUpdate(pull, ctx.User)
+		if err != nil {
+			ctx.ServerError("IsUserAllowedToUpdate", err)
+			return nil
+		}
+		ctx.Data["UpdateAllowed"] = allowUpdate
+
+		divergence, err := pull_service.GetDiverging(pull)
+		if err != nil {
+			ctx.ServerError("GetDiverging", err)
+			return nil
+		}
+		ctx.Data["Divergence"] = divergence
+	}
+
 	sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
 	if err != nil {
 		ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)

services/pull/pull.go

@@ -355,3 +355,79 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {
 
 	return nil
 }
+
+type errlist []error
+
+func (errs errlist) Error() string {
+	if len(errs) > 0 {
+		var buf strings.Builder
+		for i, err := range errs {
+			if i > 0 {
+				buf.WriteString(", ")
+			}
+			buf.WriteString(err.Error())
+		}
+		return buf.String()
+	}
+	return ""
+}
+
+// CloseBranchPulls close all the pull requests who's head branch is the branch
+func CloseBranchPulls(doer *models.User, repoID int64, branch string) error {
+	prs, err := models.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
+	if err != nil {
+		return err
+	}
+
+	prs2, err := models.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
+	if err != nil {
+		return err
+	}
+
+	prs = append(prs, prs2...)
+	if err := models.PullRequestList(prs).LoadAttributes(); err != nil {
+		return err
+	}
+
+	var errs errlist
+	for _, pr := range prs {
+		if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !models.IsErrIssueWasClosed(err) {
+			errs = append(errs, err)
+		}
+	}
+	if len(errs) > 0 {
+		return errs
+	}
+	return nil
+}
+
+// CloseRepoBranchesPulls close all pull requests which head branches are in the given repository
+func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error {
+	branches, err := git.GetBranchesByPath(repo.RepoPath())
+	if err != nil {
+		return err
+	}
+
+	var errs errlist
+	for _, branch := range branches {
+		prs, err := models.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name)
+		if err != nil {
+			return err
+		}
+
+		if err = models.PullRequestList(prs).LoadAttributes(); err != nil {
+			return err
+		}
+
+		for _, pr := range prs {
+			if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !models.IsErrIssueWasClosed(err) {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	if len(errs) > 0 {
+		return errs
+	}
+	return nil
+}

services/repository/repository.go

@@ -11,6 +11,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification"
 	repo_module "code.gitea.io/gitea/modules/repository"
+	pull_service "code.gitea.io/gitea/services/pull"
 )
 
 // CreateRepository creates a repository for the user/organization.
@@ -49,6 +50,10 @@ func ForkRepository(doer, u *models.User, oldRepo *models.Repository, name, desc
 
 // DeleteRepository deletes a repository for a user or organization.
 func DeleteRepository(doer *models.User, repo *models.Repository) error {
+	if err := pull_service.CloseRepoBranchesPulls(doer, repo); err != nil {
+		log.Error("CloseRepoBranchesPulls failed: %v", err)
+	}
+
 	if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil {
 		return err
 	}