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 <wxiaoguang@gmail.com>
This commit is contained in:
Markus Amshove 2024-02-19 10:57:08 +01:00 committed by GitHub
parent 0ea8de2d07
commit a11ccc9fcd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 44 additions and 1 deletions

View File

@ -652,6 +652,24 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
} }
if pb != nil && pb.EnableStatusCheck { 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 { ctx.Data["is_context_required"] = func(context string) bool {
for _, c := range pb.StatusCheckContexts { for _, c := range pb.StatusCheckContexts {
if c == context { if c == context {
@ -720,6 +738,18 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
return compareInfo 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 { type pullCommitList struct {
Commits []pull_service.CommitInfo `json:"commits"` Commits []pull_service.CommitInfo `json:"commits"`
LastReviewCommitSha string `json:"last_review_commit_sha"` LastReviewCommitSha string `json:"last_review_commit_sha"`

View File

@ -51,6 +51,10 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus,
} }
} }
if matchedCount != len(requiredContexts) {
return structs.CommitStatusPending
}
if matchedCount == 0 { if matchedCount == 0 {
status := git_model.CalcCommitStatus(commitStatuses) status := git_model.CalcCommitStatus(commitStatuses)
if status != nil { if status != nil {

View File

@ -24,6 +24,7 @@
{{template "repo/pulls/status" (dict {{template "repo/pulls/status" (dict
"CommitStatus" .LatestCommitStatus "CommitStatus" .LatestCommitStatus
"CommitStatuses" .LatestCommitStatuses "CommitStatuses" .LatestCommitStatuses
"MissingRequiredChecks" .MissingRequiredChecks
"ShowHideChecks" true "ShowHideChecks" true
"is_context_required" .is_context_required "is_context_required" .is_context_required
)}} )}}

View File

@ -2,6 +2,7 @@
Template Attributes: Template Attributes:
* CommitStatus: summary of all commit status state * CommitStatus: summary of all commit status state
* CommitStatuses: all commit status elements * 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 * ShowHideChecks: whether use a button to show/hide the checks
* is_context_required: Used in pull request commit status check table * is_context_required: Used in pull request commit status check table
*/}} */}}
@ -9,7 +10,7 @@ Template Attributes:
{{if .CommitStatus}} {{if .CommitStatus}}
<div class="commit-status-panel"> <div class="commit-status-panel">
<div class="ui top attached header commit-status-header"> <div class="ui top attached header commit-status-header">
{{if eq .CommitStatus.State "pending"}} {{if or (eq .CommitStatus.State "pending") (.MissingRequiredChecks)}}
{{ctx.Locale.Tr "repo.pulls.status_checking"}} {{ctx.Locale.Tr "repo.pulls.status_checking"}}
{{else if eq .CommitStatus.State "success"}} {{else if eq .CommitStatus.State "success"}}
{{ctx.Locale.Tr "repo.pulls.status_checks_success"}} {{ctx.Locale.Tr "repo.pulls.status_checks_success"}}
@ -46,6 +47,13 @@ Template Attributes:
</div> </div>
</div> </div>
{{end}} {{end}}
{{range .MissingRequiredChecks}}
<div class="commit-status-item">
{{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}}
<div class="status-context gt-ellipsis">{{.}}</div>
<div class="ui label">{{ctx.Locale.Tr "repo.pulls.status_checks_requested"}}</div>
</div>
{{end}}
</div> </div>
</div> </div>
{{end}} {{end}}