From d5ab102ad54ad5d22b3d3cf637cfc52d35cec990 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 28 Jan 2020 02:04:02 +0100 Subject: [PATCH 1/5] only try to get HeadBranch if HeadRepo exist --- modules/convert/pull.go | 55 ++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/modules/convert/pull.go b/modules/convert/pull.go index 18b4bc8de5e91..0ad7aa54c4f98 100644 --- a/modules/convert/pull.go +++ b/modules/convert/pull.go @@ -35,11 +35,12 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { return nil } } - if pr.HeadRepo == nil { + if pr.HeadRepoID != 0 && pr.HeadRepo == nil { pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID) - if err != nil { + if err != nil && !models.IsErrRepoNotExist(err) { log.Error("GetRepositoryById[%d]: %v", pr.ID, err) return nil + } } @@ -99,33 +100,41 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { apiPullRequest.Base = apiBaseBranchInfo } - headBranch, err = repo_module.GetBranch(pr.HeadRepo, 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.APIFormat(models.AccessModeNone), - } - headCommit, err = headBranch.GetCommit() + if pr.HeadRepo != nil { + headBranch, err = repo_module.GetBranch(pr.HeadRepo, pr.HeadBranch) if err != nil { - if git.IsErrNotExist(err) { - apiHeadBranchInfo.Sha = "" + if git.IsErrBranchNotExist(err) { + apiPullRequest.Head = nil } else { - log.Error("GetCommit[%s]: %v", headBranch.Name, err) + log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) return nil } } else { - apiHeadBranchInfo.Sha = headCommit.ID.String() + apiHeadBranchInfo := &api.PRBranchInfo{ + Name: pr.HeadBranch, + Ref: pr.HeadBranch, + RepoID: pr.HeadRepoID, + Repository: pr.HeadRepo.APIFormat(models.AccessModeNone), + } + 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 + } + } else { + apiPullRequest.Head = &api.PRBranchInfo{ + Name: pr.HeadBranch, + Ref: pr.HeadBranch, + RepoID: -1, } - apiPullRequest.Head = apiHeadBranchInfo } if pr.Status != models.PullRequestStatusChecking { From d3e8387780ae7858a12becd802954469878669ba Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 30 Jan 2020 05:59:14 +0100 Subject: [PATCH 2/5] impruve --- modules/convert/pull.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/modules/convert/pull.go b/modules/convert/pull.go index 0ad7aa54c4f98..446fc50e46104 100644 --- a/modules/convert/pull.go +++ b/modules/convert/pull.go @@ -5,6 +5,8 @@ package convert import ( + "fmt" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -23,10 +25,12 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { headCommit *git.Commit err error ) + if err = pr.Issue.LoadRepo(); err != nil { - log.Error("loadRepo[%d]: %v", pr.ID, err) + log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err) return nil } + apiIssue := pr.Issue.APIFormat() if pr.BaseRepo == nil { pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID) @@ -44,11 +48,6 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { } } - if err = pr.Issue.LoadRepo(); err != nil { - log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err) - return nil - } - apiPullRequest := &api.PullRequest{ ID: pr.ID, URL: pr.Issue.HTMLURL(), @@ -132,7 +131,7 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { } else { apiPullRequest.Head = &api.PRBranchInfo{ Name: pr.HeadBranch, - Ref: pr.HeadBranch, + Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index), RepoID: -1, } } From c216a3b249f6d9fc6c54879071d3480b849b6853 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 30 Jan 2020 05:59:39 +0100 Subject: [PATCH 3/5] no nil error --- models/user.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/user.go b/models/user.go index d10d2cbff8baa..ee66acf1a6bbf 100644 --- a/models/user.go +++ b/models/user.go @@ -234,6 +234,9 @@ func (u *User) GetEmail() string { // APIFormat converts a User to api.User func (u *User) APIFormat() *api.User { + if u == nil { + return nil + } return &api.User{ ID: u.ID, UserName: u.Name, From 52c9eb278a51a2de038846b4f196a21002e89259 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 30 Jan 2020 06:00:40 +0100 Subject: [PATCH 4/5] add TEST --- modules/convert/pull_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/modules/convert/pull_test.go b/modules/convert/pull_test.go index 9055d555e8871..fe82a61bd4e8e 100644 --- a/modules/convert/pull_test.go +++ b/modules/convert/pull_test.go @@ -13,6 +13,7 @@ import ( ) func TestPullRequest_APIFormat(t *testing.T) { + //with HeadRepo assert.NoError(t, models.PrepareTestDatabase()) pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest) assert.NoError(t, pr.LoadAttributes()) @@ -20,4 +21,16 @@ func TestPullRequest_APIFormat(t *testing.T) { apiPullRequest := ToAPIPullRequest(pr) assert.NotNil(t, apiPullRequest) assert.Nil(t, apiPullRequest.Head) + + //withOut HeadRepo + pr = models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest) + assert.NoError(t, pr.LoadIssue()) + assert.NoError(t, pr.LoadAttributes()) + // simulate fork deletion + pr.HeadRepo = nil + pr.HeadRepoID = 100000 + apiPullRequest = ToAPIPullRequest(pr) + assert.NotNil(t, apiPullRequest) + assert.Nil(t, apiPullRequest.Head.Repository) + assert.EqualValues(t, -1, apiPullRequest.Head.RepoID) } From 71c9391e489ecef9bf6a7e4685805268bbaa2f95 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 31 Jan 2020 06:55:14 +0100 Subject: [PATCH 5/5] correct error msg --- modules/convert/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/convert/pull.go b/modules/convert/pull.go index 446fc50e46104..fa22977d027e0 100644 --- a/modules/convert/pull.go +++ b/modules/convert/pull.go @@ -27,7 +27,7 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { ) if err = pr.Issue.LoadRepo(); err != nil { - log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err) + log.Error("pr.Issue.LoadRepo[%d]: %v", pr.ID, err) return nil }