diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2020-03-03 06:31:55 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-03-02 22:31:55 +0000 |
commit | 5abe1c52de533b52725b1e584db9fea6fb1b39ab (patch) | |
tree | 1cd49bc1a609bb919c70d6bc30727c524c3e6105 /modules/convert | |
parent | 22b750702408a39a524e011b729d0e0befb30e16 (diff) | |
download | gitea-5abe1c52de533b52725b1e584db9fea6fb1b39ab.tar.gz gitea-5abe1c52de533b52725b1e584db9fea6fb1b39ab.zip |
Display pull request head branch even the branch deleted or repository deleted (#10413)
* Display pull request head branch even the branch deleted or repository deleted
* Merge getHeadRepo/loadHeadRepo and getBaseRepo/loadBaseRepo on pull and fill repo when pr.Issue.Repo is available
* retrieve sha from pull head when pull request branch deleted and fix tests
* Fix test
* Ensure MustHeadRepoName returns empty string if no head repo
Co-authored-by: zeripath <art27@cantab.net>
Diffstat (limited to 'modules/convert')
-rw-r--r-- | modules/convert/pull.go | 121 | ||||
-rw-r--r-- | modules/convert/pull_test.go | 10 |
2 files changed, 68 insertions, 63 deletions
diff --git a/modules/convert/pull.go b/modules/convert/pull.go index cf2bf88aa8..ccf64ef3ea 100644 --- a/modules/convert/pull.go +++ b/modules/convert/pull.go @@ -22,7 +22,6 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { baseBranch *git.Branch headBranch *git.Branch baseCommit *git.Commit - headCommit *git.Commit err error ) @@ -32,20 +31,14 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { } apiIssue := ToAPIIssue(pr.Issue) - if pr.BaseRepo == nil { - pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID) - if err != nil { - log.Error("GetRepositoryById[%d]: %v", pr.ID, err) - return nil - } + if err := pr.LoadBaseRepo(); err != nil { + log.Error("GetRepositoryById[%d]: %v", pr.ID, err) + return nil } - if pr.HeadRepoID != 0 && pr.HeadRepo == nil { - pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID) - if err != nil && !models.IsErrRepoNotExist(err) { - log.Error("GetRepositoryById[%d]: %v", pr.ID, err) - return nil - } + if err := pr.LoadHeadRepo(); err != nil { + log.Error("GetRepositoryById[%d]: %v", pr.ID, err) + return nil } apiPullRequest := &api.PullRequest{ @@ -69,70 +62,74 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { Deadline: apiIssue.Deadline, Created: pr.Issue.CreatedUnix.AsTimePtr(), Updated: pr.Issue.UpdatedUnix.AsTimePtr(), - } - baseBranch, err = repo_module.GetBranch(pr.BaseRepo, 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{ + + Base: &api.PRBranchInfo{ Name: pr.BaseBranch, Ref: pr.BaseBranch, RepoID: pr.BaseRepoID, Repository: pr.BaseRepo.APIFormat(models.AccessModeNone), - } + }, + Head: &api.PRBranchInfo{ + Name: pr.HeadBranch, + Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index), + RepoID: -1, + }, + } + + baseBranch, err = repo_module.GetBranch(pr.BaseRepo, pr.BaseBranch) + if err != nil && !git.IsErrBranchNotExist(err) { + log.Error("GetBranch[%s]: %v", pr.BaseBranch, err) + return nil + } + + if err == nil { 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() + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", baseBranch.Name, err) + return nil + } + + if err == nil { + apiPullRequest.Base.Sha = baseCommit.ID.String() } - apiPullRequest.Base = apiBaseBranchInfo } if pr.HeadRepo != nil { - headBranch, err = repo_module.GetBranch(pr.HeadRepo, pr.HeadBranch) + apiPullRequest.Head.RepoID = pr.HeadRepo.ID + apiPullRequest.Head.Repository = pr.HeadRepo.APIFormat(models.AccessModeNone) + + headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) if err != nil { - if git.IsErrBranchNotExist(err) { - apiPullRequest.Head = nil - } else { - log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) + log.Error("OpenRepository[%s]: %v", pr.HeadRepo.RepoPath(), err) + return nil + } + defer headGitRepo.Close() + + headBranch, err = headGitRepo.GetBranch(pr.HeadBranch) + if err != nil && !git.IsErrBranchNotExist(err) { + log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) + return nil + } + + if git.IsErrBranchNotExist(err) { + headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", headBranch.Name, err) return nil } + if err == nil { + apiPullRequest.Head.Sha = headCommitID + } } else { - apiHeadBranchInfo := &api.PRBranchInfo{ - Name: pr.HeadBranch, - Ref: pr.HeadBranch, - RepoID: pr.HeadRepoID, - Repository: pr.HeadRepo.APIFormat(models.AccessModeNone), + commit, err := headBranch.GetCommit() + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", headBranch.Name, err) + return nil } - 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() + if err == nil { + apiPullRequest.Head.Ref = pr.HeadBranch + apiPullRequest.Head.Sha = commit.ID.String() } - apiPullRequest.Head = apiHeadBranchInfo - } - } else { - apiPullRequest.Head = &api.PRBranchInfo{ - Name: pr.HeadBranch, - Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index), - RepoID: -1, } } diff --git a/modules/convert/pull_test.go b/modules/convert/pull_test.go index fe82a61bd4..b78bbe87c9 100644 --- a/modules/convert/pull_test.go +++ b/modules/convert/pull_test.go @@ -8,6 +8,7 @@ import ( "testing" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" ) @@ -15,12 +16,19 @@ import ( func TestPullRequest_APIFormat(t *testing.T) { //with HeadRepo assert.NoError(t, models.PrepareTestDatabase()) + headRepo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest) assert.NoError(t, pr.LoadAttributes()) assert.NoError(t, pr.LoadIssue()) apiPullRequest := ToAPIPullRequest(pr) assert.NotNil(t, apiPullRequest) - assert.Nil(t, apiPullRequest.Head) + assert.EqualValues(t, &structs.PRBranchInfo{ + Name: "branch1", + Ref: "refs/pull/2/head", + Sha: "4a357436d925b5c974181ff12a994538ddc5a269", + RepoID: 1, + Repository: headRepo.APIFormat(models.AccessModeNone), + }, apiPullRequest.Head) //withOut HeadRepo pr = models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest) |