Fix pull view when head repository or head branch missed and close related pull reque...

* 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>

diff --git a/integrations/pull_create_test.go b/integrations/pull_create_test.go
index 84bcbff0dc6b8f2225066e1b906a2c92c7d937b6..d0c78a046c2e9db4f4b3d274a20e01456aade8c6 100644 (file)
--- a/integrations/pull_create_test.go
+++ b/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)
+       })
+}

diff --git a/models/pull.go b/models/pull.go
index 42d93bd54158df438b5f853ffed7e73956e1aa7d..1895030f754219155108e918a13236721073e24d 100644 (file)
--- a/models/pull.go
+++ b/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

diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go
index 430a83093d49d64c454448d8737bc51574c3ab02..5188529d1687ae5656c25415ff101cfa37a1d714 100644 (file)
--- a/modules/repofiles/update.go
+++ b/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)
+               }
+
+               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)
+               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,11 +553,14 @@ 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)
+                       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)
+                       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)
+               }
 
                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)

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index c84174783a11170bdaca8e3d61e028ea1287fabc..655be2e82e9930d67005d03317dc6d3c97c6675d 100644 (file)
--- a/routers/repo/pull.go
+++ b/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)

diff --git a/services/pull/pull.go b/services/pull/pull.go
index bc71e52213589b2c7cdeda66c6cf59f60666fe12..705eab06a2bed507b0f6cb02e7323a920979e6b1 100644 (file)
--- a/services/pull/pull.go
+++ b/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
+}

diff --git a/services/repository/repository.go b/services/repository/repository.go
index eea8b352b4b51da5cf1e6bf5ad69fadb1178894f..f50b98b64099ad61456589dc8270d49ae23a542b 100644 (file)
--- a/services/repository/repository.go
+++ b/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
        }