From 167d12ec89db89281fa3f9d3b9fb0dc11210af74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Mon, 5 Dec 2016 10:15:39 +0100 Subject: [PATCH 1/3] Fix #344 (regression in #248) --- models/pull.go | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/models/pull.go b/models/pull.go index 5b54058b8df02..292989a98daab 100644 --- a/models/pull.go +++ b/models/pull.go @@ -121,12 +121,30 @@ func (pr *PullRequest) LoadIssue() (err error) { // Required - Issue // Optional - Merger func (pr *PullRequest) APIFormat() *api.PullRequest { - + var ( + baseBranch *Branch + headBranch *Branch + baseCommit *git.Commit + headCommit *git.Commit + err error + ) apiIssue := pr.Issue.APIFormat() - baseBranch, _ := pr.BaseRepo.GetBranch(pr.BaseBranch) - baseCommit, _ := baseBranch.GetCommit() - headBranch, _ := pr.HeadRepo.GetBranch(pr.HeadBranch) - headCommit, _ := headBranch.GetCommit() + if pr.BaseRepo == nil || pr.HeadRepo == nil { + log.Critical(log.CRITICAL, "Base/HeadRepo not set for PR!!!") + return nil + } + if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { + return nil + } + if baseCommit, err = baseBranch.GetCommit(); err != nil { + return nil + } + if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + return nil + } + if headCommit, err = headBranch.GetCommit(); err != nil { + return nil + } apiBaseBranchInfo := &api.PRBranchInfo{ Name: pr.BaseBranch, Ref: pr.BaseBranch, @@ -590,8 +608,12 @@ func GetPullRequestByIndex(repoID int64, index int64) (*PullRequest, error) { return nil, ErrPullRequestNotExist{0, repoID, index, 0, "", ""} } - pr.LoadAttributes() - pr.LoadIssue() + if err = pr.LoadAttributes(); err != nil { + return nil, err + } + if err = pr.LoadIssue(); err != nil { + return nil, err + } return pr, nil } From ee27c23fa040de31b4c0c819a0019d1a5177c916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Mon, 5 Dec 2016 11:47:28 +0100 Subject: [PATCH 2/3] Actually fix the error :laughing: --- models/pull.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/models/pull.go b/models/pull.go index 292989a98daab..4c860a7bb09b8 100644 --- a/models/pull.go +++ b/models/pull.go @@ -129,9 +129,19 @@ func (pr *PullRequest) APIFormat() *api.PullRequest { err error ) apiIssue := pr.Issue.APIFormat() - if pr.BaseRepo == nil || pr.HeadRepo == nil { - log.Critical(log.CRITICAL, "Base/HeadRepo not set for PR!!!") - return nil + if pr.BaseRepo == nil { + pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID) + if err != nil { + log.Error(log.ERROR, "BaseRepo not set for PR %d", pr.ID) + return nil + } + } + if pr.HeadRepo == nil { + pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID) + if err != nil { + log.Error(log.ERROR, "HeadRepo not set for PR %d", pr.ID) + return nil + } } if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { return nil From a7485b49dcfab54f5280c63adb1e57fb4cffdb4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Mon, 5 Dec 2016 11:51:14 +0100 Subject: [PATCH 3/3] Change log-msg for #349 --- models/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/pull.go b/models/pull.go index 4c860a7bb09b8..d149a142d0cc6 100644 --- a/models/pull.go +++ b/models/pull.go @@ -132,14 +132,14 @@ func (pr *PullRequest) APIFormat() *api.PullRequest { if pr.BaseRepo == nil { pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID) if err != nil { - log.Error(log.ERROR, "BaseRepo not set for PR %d", pr.ID) + log.Error(log.ERROR, "GetRepositoryById[%d]: %v", pr.ID, err) return nil } } if pr.HeadRepo == nil { pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID) if err != nil { - log.Error(log.ERROR, "HeadRepo not set for PR %d", pr.ID) + log.Error(log.ERROR, "GetRepositoryById[%d]: %v", pr.ID, err) return nil } }