only return head: null if source branch was deleted (#6705)

* only return head: null if source branch was deleted

* add URL into GetPullRequest

* TestPullRequest_APIFormat

* log error if it is not Err(Branch)NotExist

diff --git a/models/pull.go b/models/pull.go
index 6ee2ec555d9897b2a8b382c90e316ff4b73a09e5..7dd6050c67600e2b6ee85fa15e4d54420f223683 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -189,36 +189,6 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
                        return nil
                }
        }
-       if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil {
-               log.Error("pr.BaseRepo.GetBranch[%d]: %v", pr.BaseBranch, err)
-               return nil
-       }
-       if baseCommit, err = baseBranch.GetCommit(); err != nil {
-               log.Error("baseBranch.GetCommit[%d]: %v", pr.ID, err)
-               return nil
-       }
-       if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil {
-               log.Error("pr.HeadRepo.GetBranch[%d]: %v", pr.HeadBranch, err)
-               return nil
-       }
-       if headCommit, err = headBranch.GetCommit(); err != nil {
-               log.Error("headBranch.GetCommit[%d]: %v", pr.ID, err)
-               return nil
-       }
-       apiBaseBranchInfo := &api.PRBranchInfo{
-               Name:       pr.BaseBranch,
-               Ref:        pr.BaseBranch,
-               Sha:        baseCommit.ID.String(),
-               RepoID:     pr.BaseRepoID,
-               Repository: pr.BaseRepo.innerAPIFormat(e, AccessModeNone, false),
-       }
-       apiHeadBranchInfo := &api.PRBranchInfo{
-               Name:       pr.HeadBranch,
-               Ref:        pr.HeadBranch,
-               Sha:        headCommit.ID.String(),
-               RepoID:     pr.HeadRepoID,
-               Repository: pr.HeadRepo.innerAPIFormat(e, AccessModeNone, false),
-       }
 
        if err = pr.Issue.loadRepo(e); err != nil {
                log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err)
@@ -227,6 +197,7 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
 
        apiPullRequest := &api.PullRequest{
                ID:        pr.ID,
+               URL:       pr.Issue.HTMLURL(),
                Index:     pr.Index,
                Poster:    apiIssue.Poster,
                Title:     apiIssue.Title,
@@ -241,13 +212,68 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
                DiffURL:   pr.Issue.DiffURL(),
                PatchURL:  pr.Issue.PatchURL(),
                HasMerged: pr.HasMerged,
-               Base:      apiBaseBranchInfo,
-               Head:      apiHeadBranchInfo,
                MergeBase: pr.MergeBase,
                Deadline:  apiIssue.Deadline,
                Created:   pr.Issue.CreatedUnix.AsTimePtr(),
                Updated:   pr.Issue.UpdatedUnix.AsTimePtr(),
        }
+       baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch)
+       if err != nil {
+               if git.IsErrBranchNotExist(err) {
+                       apiPullRequest.Base = nil
+               } else {
+                       log.Error("GetBranch[%s]: %v", pr.BaseBranch, err)
+                       return nil
+               }
+       } else {
+               apiBaseBranchInfo := &api.PRBranchInfo{
+                       Name:       pr.BaseBranch,
+                       Ref:        pr.BaseBranch,
+                       RepoID:     pr.BaseRepoID,
+                       Repository: pr.BaseRepo.innerAPIFormat(e, AccessModeNone, false),
+               }
+               baseCommit, err = baseBranch.GetCommit()
+               if err != nil {
+                       if git.IsErrNotExist(err) {
+                               apiBaseBranchInfo.Sha = ""
+                       } else {
+                               log.Error("GetCommit[%s]: %v", baseBranch.Name, err)
+                               return nil
+                       }
+               } else {
+                       apiBaseBranchInfo.Sha = baseCommit.ID.String()
+               }
+               apiPullRequest.Base = apiBaseBranchInfo
+       }
+
+       headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch)
+       if err != nil {
+               if git.IsErrBranchNotExist(err) {
+                       apiPullRequest.Head = nil
+               } else {
+                       log.Error("GetBranch[%s]: %v", pr.HeadBranch, err)
+                       return nil
+               }
+       } else {
+               apiHeadBranchInfo := &api.PRBranchInfo{
+                       Name:       pr.HeadBranch,
+                       Ref:        pr.HeadBranch,
+                       RepoID:     pr.HeadRepoID,
+                       Repository: pr.HeadRepo.innerAPIFormat(e, AccessModeNone, false),
+               }
+               headCommit, err = headBranch.GetCommit()
+               if err != nil {
+                       if git.IsErrNotExist(err) {
+                               apiHeadBranchInfo.Sha = ""
+                       } else {
+                               log.Error("GetCommit[%s]: %v", headBranch.Name, err)
+                               return nil
+                       }
+               } else {
+                       apiHeadBranchInfo.Sha = headCommit.ID.String()
+               }
+               apiPullRequest.Head = apiHeadBranchInfo
+       }
 
        if pr.Status != PullRequestStatusChecking {
                mergeable := pr.Status != PullRequestStatusConflict && !pr.IsWorkInProgress()

diff --git a/models/pull_test.go b/models/pull_test.go
index 5a53474ac4fc7851bbcd4a8697652ab45727e58f..df051d51bceac135c2bb74f4b8dffbf65ebca2b4 100644 (file)
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -31,7 +31,15 @@ func TestPullRequest_LoadIssue(t *testing.T) {
        assert.Equal(t, int64(2), pr.Issue.ID)
 }
 
-// TODO TestPullRequest_APIFormat
+func TestPullRequest_APIFormat(t *testing.T) {
+       assert.NoError(t, PrepareTestDatabase())
+       pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)
+       assert.NoError(t, pr.LoadAttributes())
+       assert.NoError(t, pr.LoadIssue())
+       apiPullRequest := pr.APIFormat()
+       assert.NotNil(t, apiPullRequest)
+       assert.Nil(t, apiPullRequest.Head)
+}
 
 func TestPullRequest_GetBaseRepo(t *testing.T) {
        assert.NoError(t, PrepareTestDatabase())