From b1d66f50fbd1af1db8aa66b0c6393e57f8d08353 Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Mon, 19 Feb 2024 10:57:08 +0100 Subject: [PATCH] Disallow merge when required checked are missing (#29143) fixes #21892 This PR disallows merging a PR when not all commit status contexts configured in the branch protection are met. Previously, the PR was happy to merge when one commit status was successful and the other contexts weren't reported. Any feedback is welcome, first time Go :-) I'm also not sure if the changes in the template break something else Given the following branch protection: ![branch_protection](https://github.com/go-gitea/gitea/assets/2401875/f871b4e4-138b-435a-b496-f9ad432e3dec) This was shown before the change: ![before](https://github.com/go-gitea/gitea/assets/2401875/60424ff0-ee09-4fa0-856e-64e6e3fb0612) With the change, it is now shown as this: ![after](https://github.com/go-gitea/gitea/assets/2401875/4e464142-efb1-4889-8166-eb3be26c8f3d) --------- Co-authored-by: wxiaoguang (cherry picked from commit a11ccc9fcd61fb25ffb1c37b87a0df4ee9efd84e) --- routers/web/repo/pull.go | 30 +++++++++++++++++++++ services/pull/commit_status.go | 4 +++ templates/repo/issue/view_content/pull.tmpl | 1 + templates/repo/pulls/status.tmpl | 10 ++++++- 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ca854a35f2..ab821f8884 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -662,6 +662,24 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } if pb != nil && pb.EnableStatusCheck { + + var missingRequiredChecks []string + for _, requiredContext := range pb.StatusCheckContexts { + contextFound := false + matchesRequiredContext := createRequiredContextMatcher(requiredContext) + for _, presentStatus := range commitStatuses { + if matchesRequiredContext(presentStatus.Context) { + contextFound = true + break + } + } + + if !contextFound { + missingRequiredChecks = append(missingRequiredChecks, requiredContext) + } + } + ctx.Data["MissingRequiredChecks"] = missingRequiredChecks + ctx.Data["is_context_required"] = func(context string) bool { for _, c := range pb.StatusCheckContexts { if c == context { @@ -730,6 +748,18 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C return compareInfo } +func createRequiredContextMatcher(requiredContext string) func(string) bool { + if gp, err := glob.Compile(requiredContext); err == nil { + return func(contextToCheck string) bool { + return gp.Match(contextToCheck) + } + } + + return func(contextToCheck string) bool { + return requiredContext == contextToCheck + } +} + type pullCommitList struct { Commits []pull_service.CommitInfo `json:"commits"` LastReviewCommitSha string `json:"last_review_commit_sha"` diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 06e66fad77..27ee572640 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -52,6 +52,10 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, } } + if matchedCount != len(requiredContexts) { + return structs.CommitStatusPending + } + if matchedCount == 0 { status := git_model.CalcCommitStatus(commitStatuses) if status != nil { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index a28b849f98..e86deb8915 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -24,6 +24,7 @@ {{template "repo/pulls/status" (dict "CommitStatus" .LatestCommitStatus "CommitStatuses" .LatestCommitStatuses + "MissingRequiredChecks" .MissingRequiredChecks "ShowHideChecks" true "is_context_required" .is_context_required )}} diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index ae508b8fa4..e8636ba1b8 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -2,6 +2,7 @@ Template Attributes: * CommitStatus: summary of all commit status state * CommitStatuses: all commit status elements +* MissingRequiredChecks: commit check contexts that are required by branch protection but not present * ShowHideChecks: whether use a button to show/hide the checks * is_context_required: Used in pull request commit status check table */}} @@ -9,7 +10,7 @@ Template Attributes: {{if .CommitStatus}}
- {{if eq .CommitStatus.State "pending"}} + {{if or (eq .CommitStatus.State "pending") (.MissingRequiredChecks)}} {{ctx.Locale.Tr "repo.pulls.status_checking"}} {{else if eq .CommitStatus.State "success"}} {{ctx.Locale.Tr "repo.pulls.status_checks_success"}} @@ -46,6 +47,13 @@ Template Attributes:
{{end}} + {{range .MissingRequiredChecks}} +
+ {{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}} +
{{.}}
+
{{ctx.Locale.Tr "repo.pulls.status_checks_requested"}}
+
+ {{end}} {{end}}