From a1003bd62956f0c8c46371c40ab1c4303486a58b Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Mon, 12 Feb 2024 11:17:46 +0100 Subject: [PATCH 1/5] Disallow merge when not all required contexts are matched --- services/pull/commit_status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index b73816c7eb225..3282f4f379a01 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -51,6 +51,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 { From 110b60a9ee2ddb34a23dc29256725a94397ee1b2 Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Mon, 12 Feb 2024 11:44:17 +0100 Subject: [PATCH 2/5] Render missing checks in UI --- routers/web/repo/pull.go | 18 ++++++++++++++++++ templates/repo/issue/view_content/pull.tmpl | 1 + templates/repo/pulls/status.tmpl | 10 +++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index b265cf47548bc..b328eb4de736b 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -652,6 +652,24 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } if pb != nil && pb.EnableStatusCheck { + + missingRequiredChecks := make([]string, 0) + for _, requiredContext := range pb.StatusCheckContexts { + contextFound := false + for _, presentStatus := range commitStatuses { + if presentStatus.Context == requiredContext { + contextFound = true + break + } + } + + if !contextFound { + missingRequiredChecks = append(missingRequiredChecks, requiredContext) + } + } + ctx.Data["MissingRequiredChecks"] = missingRequiredChecks + + ctx.Data["RequiredStatusChecks"] = pb.StatusCheckContexts ctx.Data["is_context_required"] = func(context string) bool { for _, c := range pb.StatusCheckContexts { if c == context { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 2b5776ea03a52..32406b3149b50 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 ae508b8fa4fa2..e8636ba1b81ba 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}} From 2e87c39ff030737782fd95d0c04533015e8e540b Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Fri, 16 Feb 2024 11:19:17 +0100 Subject: [PATCH 3/5] Resolve review Co-authored-by: wxiaoguang --- routers/web/repo/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index b328eb4de736b..1a6dc7df2d060 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -653,7 +653,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C if pb != nil && pb.EnableStatusCheck { - missingRequiredChecks := make([]string, 0) + var missingRequiredChecks []string for _, requiredContext := range pb.StatusCheckContexts { contextFound := false for _, presentStatus := range commitStatuses { From 9ea36348c3d39dca1464db42303960ead3cfb9be Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Sat, 17 Feb 2024 16:51:20 +0100 Subject: [PATCH 4/5] Compare by globbing if required context is a valid glob pattern --- routers/web/repo/pull.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 1a6dc7df2d060..f8535671b17f8 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -656,8 +656,9 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C var missingRequiredChecks []string for _, requiredContext := range pb.StatusCheckContexts { contextFound := false + matchesRequiredContext := createRequiredContextMatcher(requiredContext) for _, presentStatus := range commitStatuses { - if presentStatus.Context == requiredContext { + if matchesRequiredContext(presentStatus.Context) { contextFound = true break } @@ -738,6 +739,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"` From 0f3895cd0938b8634b436feebbcf89aa10f00f4b Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Sat, 17 Feb 2024 16:52:54 +0100 Subject: [PATCH 5/5] Remove unused assignment --- routers/web/repo/pull.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index f8535671b17f8..616a8a409d44d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -670,7 +670,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } ctx.Data["MissingRequiredChecks"] = missingRequiredChecks - ctx.Data["RequiredStatusChecks"] = pb.StatusCheckContexts ctx.Data["is_context_required"] = func(context string) bool { for _, c := range pb.StatusCheckContexts { if c == context {