From fe899be289893f43c56ff29ac3ffbcad4121996f Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Sun, 13 Dec 2020 20:11:49 +0100 Subject: [PATCH 1/8] Show status check for merged PRs --- routers/repo/pull.go | 13 ++++++++++++ services/pull/pull.go | 26 ++++++++++++------------ templates/repo/pulls/status.tmpl | 34 +++++++++++++++++--------------- 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 0ea6ec33deb7c..220b720943c03 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -325,6 +325,19 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C } ctx.Data["NumCommits"] = compareInfo.Commits.Len() ctx.Data["NumFiles"] = compareInfo.NumFiles + + // get last commit hash for this PR + sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() + commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) + if err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return nil + } + if len(commitStatuses) > 0 { + ctx.Data["LatestCommitStatuses"] = commitStatuses + ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) + } + return compareInfo } diff --git a/services/pull/pull.go b/services/pull/pull.go index 9cf4abfd54218..e54de1719c4b9 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -641,31 +641,31 @@ func GetCommitMessages(pr *models.PullRequest) string { // GetLastCommitStatus returns the last commit status for this pull request. func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, err error) { - if err = pr.LoadHeadRepo(); err != nil { + // load base repo + if err := pr.LoadBaseRepo(); err != nil { return nil, err } - if pr.HeadRepo == nil { - return nil, models.ErrPullRequestHeadRepoMissing{ID: pr.ID, HeadRepoID: pr.HeadRepoID} - } - - headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + // open repo with git + gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { return nil, err } - defer headGitRepo.Close() - lastCommitID, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) + // get list of commits + compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), + pr.MergeBase, pr.GetGitRefName()) if err != nil { - return nil, err - } + if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { + return nil, err + } - err = pr.LoadBaseRepo() - if err != nil { return nil, err } - statusList, err := models.GetLatestCommitStatus(pr.BaseRepo, lastCommitID, 0) + // get last commit hash for this PR + sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() + statusList, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0) if err != nil { return nil, err } diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 2847903a52a97..e48f5f3fe3100 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -1,19 +1,21 @@ {{if $.LatestCommitStatus}} -
- {{if eq .LatestCommitStatus.State "pending"}} - {{$.i18n.Tr "repo.pulls.status_checking"}} - {{else if eq .LatestCommitStatus.State "success"}} - {{$.i18n.Tr "repo.pulls.status_checks_success"}} - {{else if eq .LatestCommitStatus.State "warning"}} - {{$.i18n.Tr "repo.pulls.status_checks_warning"}} - {{else if eq .LatestCommitStatus.State "failure"}} - {{$.i18n.Tr "repo.pulls.status_checks_failure"}} - {{else if eq .LatestCommitStatus.State "error"}} - {{$.i18n.Tr "repo.pulls.status_checks_error"}} - {{else}} - {{$.i18n.Tr "repo.pulls.status_checking"}} - {{end}} -
+ {{if not $.Issue.PullRequest.HasMerged}} +
+ {{if eq .LatestCommitStatus.State "pending"}} + {{$.i18n.Tr "repo.pulls.status_checking"}} + {{else if eq .LatestCommitStatus.State "success"}} + {{$.i18n.Tr "repo.pulls.status_checks_success"}} + {{else if eq .LatestCommitStatus.State "warning"}} + {{$.i18n.Tr "repo.pulls.status_checks_warning"}} + {{else if eq .LatestCommitStatus.State "failure"}} + {{$.i18n.Tr "repo.pulls.status_checks_failure"}} + {{else if eq .LatestCommitStatus.State "error"}} + {{$.i18n.Tr "repo.pulls.status_checks_error"}} + {{else}} + {{$.i18n.Tr "repo.pulls.status_checking"}} + {{end}} +
+ {{end}} {{range $.LatestCommitStatuses}}
@@ -21,7 +23,7 @@ {{.Context}} {{.Description}}
{{if $.is_context_required}} - {{if (call $.is_context_required .Context)}}
{{$.i18n.Tr "repo.pulls.status_checks_requested"}}
{{end}} + {{if (call $.is_context_required .Context)}}
{{$.i18n.Tr "repo.pulls.status_checks_requested"}}
{{end}} {{end}} {{if .TargetURL}}{{$.i18n.Tr "repo.pulls.status_checks_details"}}{{end}}
From 59878dc647b854c069978170687c8f50ead96f15 Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Sun, 13 Dec 2020 22:11:04 +0100 Subject: [PATCH 2/8] Handle PRs with no commits --- routers/repo/pull.go | 21 +++++++++++---------- services/pull/pull.go | 6 +++++- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 220b720943c03..1d5e0a1dfb34d 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -326,16 +326,17 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C ctx.Data["NumCommits"] = compareInfo.Commits.Len() ctx.Data["NumFiles"] = compareInfo.NumFiles - // get last commit hash for this PR - sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() - commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) - if err != nil { - ctx.ServerError("GetLatestCommitStatus", err) - return nil - } - if len(commitStatuses) > 0 { - ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) + if compareInfo.Commits.Len() > 0 { + sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() + commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) + if err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return nil + } + if len(commitStatuses) > 0 { + ctx.Data["LatestCommitStatuses"] = commitStatuses + ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) + } } return compareInfo diff --git a/services/pull/pull.go b/services/pull/pull.go index e54de1719c4b9..7be7169ee6f39 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "strings" "time" @@ -663,7 +664,10 @@ func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, e return nil, err } - // get last commit hash for this PR + if compareInfo.Commits.Len() == 0 { + return nil, errors.New("pull request has no commits") + } + sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() statusList, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0) if err != nil { From 4048ba6e92249ef8f9ba9a61ee45403685c6d6ef Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Mon, 14 Dec 2020 02:44:34 +0100 Subject: [PATCH 3/8] Styling --- services/pull/pull.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 7be7169ee6f39..bbd677c77479c 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -642,25 +642,20 @@ func GetCommitMessages(pr *models.PullRequest) string { // GetLastCommitStatus returns the last commit status for this pull request. func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, err error) { - // load base repo if err := pr.LoadBaseRepo(); err != nil { return nil, err } - // open repo with git gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { return nil, err } - // get list of commits - compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), - pr.MergeBase, pr.GetGitRefName()) + compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName()) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { return nil, err } - return nil, err } From 1464d3d3ab8a640ce3de5f0ccf23547a655487d7 Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Mon, 14 Dec 2020 02:47:03 +0100 Subject: [PATCH 4/8] := --- services/pull/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index bbd677c77479c..fe72191144dd6 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -642,7 +642,7 @@ func GetCommitMessages(pr *models.PullRequest) string { // GetLastCommitStatus returns the last commit status for this pull request. func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, err error) { - if err := pr.LoadBaseRepo(); err != nil { + if err = pr.LoadBaseRepo(); err != nil { return nil, err } From f193f61e7f206c496693337e1222796b24abc6f2 Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Mon, 14 Dec 2020 02:49:33 +0100 Subject: [PATCH 5/8] defer close --- services/pull/pull.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/pull/pull.go b/services/pull/pull.go index fe72191144dd6..454a5d3958e84 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -650,6 +650,7 @@ func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, e if err != nil { return nil, err } + defer gitRepo.Close() compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName()) if err != nil { From 976737cc9e185710b7d7e4ac767eba67121de44f Mon Sep 17 00:00:00 2001 From: CirnoT <1447794+CirnoT@users.noreply.github.com> Date: Mon, 14 Dec 2020 02:50:51 +0100 Subject: [PATCH 6/8] useless code --- services/pull/pull.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 454a5d3958e84..3a9ae04d9e9f0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -654,9 +654,6 @@ func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, e compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName()) if err != nil { - if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { - return nil, err - } return nil, err } From fbd1f4785ab566b02883b503cb3dc788c5698cce Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 18 Dec 2020 01:38:22 +0000 Subject: [PATCH 7/8] nits --- routers/repo/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 1d5e0a1dfb34d..fc267104452b3 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -326,14 +326,14 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C ctx.Data["NumCommits"] = compareInfo.Commits.Len() ctx.Data["NumFiles"] = compareInfo.NumFiles - if compareInfo.Commits.Len() > 0 { + if compareInfo.Commits.Len() != 0 { sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) if err != nil { ctx.ServerError("GetLatestCommitStatus", err) return nil } - if len(commitStatuses) > 0 { + if len(commitStatuses) != 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) } From 1bbdcca5ffe68741838629722f83557f12a6adb1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 18 Dec 2020 10:22:15 +0000 Subject: [PATCH 8/8] fix merge conflict relict --- routers/repo/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index c397bbdd4b12e..901a6686323c8 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -328,7 +328,7 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C if compareInfo.Commits.Len() != 0 { sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() - commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) + commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, sha, models.ListOptions{}) if err != nil { ctx.ServerError("GetLatestCommitStatus", err) return nil