Commit 2d65be8962

2d65be8962980c4c1c966517f54c1e8ff5d62b72

parent: a5ee6a83a1

Verified · cmc ci/build: success ci/test: success

cmc <hello@cleberg.net> · 2026-09-07 22:40 UTC

control, web, wiki: merge gates on mr show and the page, every unmet gate at once

mr merge was refused one gate at a time and nothing said beforehand
what the repository required or which owners were outstanding. The
gates are now one computation, MergeGates, shared by mr show (a gates
block: approvals counted and required, owners outstanding per file,
open threads, checks, fast-forward), the merge request page (the same
block), and mr merge, which names every unmet gate in one refusal. The
check gate moves in from the merge path. mr review reports counts,
and says when a verdict is advisory because the reviewer cannot write.

Closes #199
.gitbay/wiki/Users.org +8
@@ -408,6 +408,14 @@ to ask without it gating merges; with it on and no file on the target
408408branch, the merge is refused and says so. It does not wait on
409409=require-approvals=.
410410
411=mr show= carries a =gates= block with where the merge request stands
412against all of them — approvals counted and required, owners still
413outstanding per file, open threads, checks, and whether a fast-forward
414is possible — and the merge request page shows the same block. =mr
415merge= names every unmet gate at once rather than the first. =mr
416review= says when a verdict is advisory, which it is from anyone
417without write access: the gates do not count it.
418
411419Those gates apply to =mr merge=. A direct push to a protected branch
412420passes none of them until =require-mr on=: then an existing protected
413421branch refuses every push, including =repo commit-file= and the web
e2e/gates_test.go added +99
@@ -0,0 +1,99 @@
1package e2e
2
3import (
4 "os"
5 "path/filepath"
6 "strings"
7 "testing"
8)
9
10// mr show reports the gates before the merge fails, mr merge names every
11// unmet gate at once, and mr review says when a verdict is advisory
12// (#199).
13func TestMergeGatesVisible(t *testing.T) {
14 inst := startInstance(t)
15 aliceKey := inst.newKey(t, "alice")
16 bobKey := inst.newKey(t, "bob")
17 carolKey := inst.newKey(t, "carol")
18 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
19 inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
20 inst.admin(t, "admin", "user", "create", "carol", "--key", carolKey+".pub")
21 for _, args := range [][]string{
22 {"repo", "create", "alice/svc"},
23 {"repo", "access", "grant", "alice/svc", "bob", "write"},
24 {"repo", "settings", "require-approvals", "alice/svc", "1"},
25 {"repo", "settings", "require-codeowners", "alice/svc", "on"},
26 {"repo", "settings", "require-resolved", "alice/svc", "on"},
27 } {
28 if _, errOut, code := inst.ssh(t, aliceKey, "", args...); code != 0 {
29 t.Fatalf("%v: %s", args, errOut)
30 }
31 }
32 env := inst.gitEnv(aliceKey)
33 work := t.TempDir()
34 mustGit(t, work, env, "clone", "-q", inst.sshURL("alice/svc"), "w")
35 dir := filepath.Join(work, "w")
36 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
37 os.WriteFile(filepath.Join(dir, "CODEOWNERS"), []byte("*.go @bob\n"), 0o644)
38 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n"), 0o644)
39 mustGit(t, dir, env, "add", ".")
40 mustGit(t, dir, env, "commit", "-q", "-m", "base")
41 mustGit(t, dir, env, "push", "-q", "origin", "main")
42 mustGit(t, dir, env, "checkout", "-q", "-b", "feat")
43 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n\nvar V = 1\n"), 0o644)
44 mustGit(t, dir, env, "commit", "-q", "-am", "change")
45 mustGit(t, dir, env, "push", "-q", "origin", "feat")
46 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/svc",
47 "--source", "feat", "--target", "main", "--title", "'change'"); code != 0 {
48 t.Fatalf("mr create: %s", errOut)
49 }
50
51 // Before anyone reviews: two unmet gates, visible on mr show.
52 out, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/svc", "1", "--json")
53 for _, want := range []string{
54 `"approvals_required":1`, `"codeowners_required":true`, `"resolved_required":true`,
55 `"owners_outstanding":[{"files":["svc.go"],"owners":["bob"]}]`, `"fast_forward":true`,
56 `requires 1 fresh approval(s)`, `CODEOWNERS approval missing for: svc.go (owned by bob)`,
57 } {
58 if !strings.Contains(out, want) {
59 t.Errorf("mr show gates missing %s:\n%s", want, out)
60 }
61 }
62 // The merge names both at once.
63 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 4 ||
64 !strings.Contains(errOut, "fresh approval") || !strings.Contains(errOut, "CODEOWNERS approval missing") {
65 t.Fatalf("merge refusal does not name every gate: %d %s", code, errOut)
66 }
67
68 // A reader's approval is advisory, and says so when made.
69 out, _, code := inst.ssh(t, carolKey, "", "mr", "review", "alice/svc", "1", "--approve", "--json")
70 if code != 0 || !strings.Contains(out, `"counts":false`) {
71 t.Fatalf("reader review: %d %s", code, out)
72 }
73 if out, _, _ := inst.ssh(t, carolKey, "", "mr", "review", "alice/svc", "1", "--comment"); !strings.Contains(out, "advisory") {
74 t.Fatalf("reader review does not say it is advisory: %s", out)
75 }
76 out, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/svc", "1", "--json")
77 if !strings.Contains(out, `requires 1 fresh approval(s)`) {
78 t.Fatalf("advisory approval counted in the gates:\n%s", out)
79 }
80
81 // The owner's approval meets both; the block says so and the merge lands.
82 out, _, _ = inst.ssh(t, bobKey, "", "mr", "review", "alice/svc", "1", "--approve", "--json")
83 if !strings.Contains(out, `"counts":true`) {
84 t.Fatalf("writer review: %s", out)
85 }
86 out, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/svc", "1", "--json")
87 if strings.Contains(out, `"unmet"`) || !strings.Contains(out, `"approvals":["bob"]`) {
88 t.Fatalf("gates after approval:\n%s", out)
89 }
90 if out, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/svc", "1"); !strings.Contains(out, "gates: met; fast-forward possible") {
91 t.Fatalf("text gates line: %s", out)
92 }
93 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 0 {
94 t.Fatalf("merge: %s", errOut)
95 }
96 if out, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/svc", "1", "--json"); strings.Contains(out, `"gates"`) {
97 t.Fatalf("gates reported on a merged request:\n%s", out)
98 }
99}
internal/control/mr.go +145 −96
@@ -567,6 +567,13 @@ func runMRShow(c *Ctx, args []string) int {
567567 d := MRShow{mrOut: mrToOut(repo, mr, true), Checks: checks, Combined: combined,
568568 UnresolvedThreads: unresolved, Commits: commits, Comments: cs, Reviews: rs}
569569 d.StackedOn, d.Stacked = stackOf(c, repo, mr)
570 if mr.State == "open" || mr.State == "source_gone" {
571 if targetSHA, err := gitutil.ResolveRef(dir, "refs/heads/"+mr.TargetRef); err == nil {
572 if g, err := MergeGates(c.Store, repo, mr, dir, targetSHA, mr.HeadSHA); err == nil {
573 d.Gates = &g
574 }
575 }
576 }
570577 return c.emit(d, func(w io.Writer) {
571578 state := d.State
572579 if d.Draft {
@@ -604,6 +611,20 @@ func runMRShow(c *Ctx, args []string) int {
604611 if d.UnresolvedThreads > 0 {
605612 fmt.Fprintf(w, "unresolved threads: %d\n", d.UnresolvedThreads)
606613 }
614 if g := d.Gates; g != nil {
615 ff := "fast-forward possible"
616 if !g.FastForward {
617 ff = "fast-forward not possible"
618 }
619 if len(g.Unmet) == 0 {
620 fmt.Fprintf(w, "gates: met; %s\n", ff)
621 } else {
622 fmt.Fprintf(w, "gates: %d unmet; %s\n", len(g.Unmet), ff)
623 for _, u := range g.Unmet {
624 fmt.Fprintf(w, "gate: %s\n", u)
625 }
626 }
627 }
607628 for _, r := range rs {
608629 stale := ""
609630 if r.Stale {
@@ -800,11 +821,17 @@ func runMRReview(c *Ctx, args []string) int {
800821 action: reviewAction(mr.Number, verdict, published),
801822 path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)})
802823 }
803 return c.emit(map[string]any{"number": mr.Number, "verdict": verdict, "published": published}, func(w io.Writer) {
824 // Whether the merge gates will count this verdict, said now rather
825 // than at the refusal (#199).
826 counts := ReviewersWhoCount(c.Store, repo, []store.MRReview{{Reviewer: c.User.Username}})[c.User.Username]
827 return c.emit(map[string]any{"number": mr.Number, "verdict": verdict, "published": published, "counts": counts}, func(w io.Writer) {
804828 fmt.Fprintf(w, "reviewed %s!%d: %s", repo.Path(), mr.Number, verdict)
805829 if published > 0 {
806830 fmt.Fprintf(w, " (%d comment(s))", published)
807831 }
832 if !counts {
833 fmt.Fprintf(w, " (advisory: no write access on %s, so the merge gates do not count it)", repo.Path())
834 }
808835 fmt.Fprintln(w)
809836 })
810837}
@@ -928,31 +955,8 @@ func runMRMerge(c *Ctx, args []string) int {
928955 return c.fail(protocol.ExitFailure, "MR head ref: %v", err)
929956 }
930957
931 // Check gate: with require_checks, the MR head must carry statuses
932 // and every one of them must be green.
933 if repo.Settings.RequireChecks {
934 statuses, err := c.Store.ListCommitStatuses(repo.ID, headSHA)
935 if err != nil {
936 return c.fail(protocol.ExitFailure, "%v", err)
937 }
938 switch store.CombinedStatus(statuses) {
939 case "success":
940 case "":
941 return c.fail(protocol.ExitDenied,
942 "%s requires green checks and none were reported on %.10s", repo.Path(), headSHA)
943 default:
944 var bad []string
945 for _, st := range statuses {
946 if st.State != "success" {
947 bad = append(bad, st.Context+"="+st.State)
948 }
949 }
950 return c.fail(protocol.ExitDenied,
951 "%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", "))
952 }
953 }
954
955 // Review gates: approvals, CODEOWNERS, resolved threads.
958 // Merge gates: draft, checks, approvals, CODEOWNERS, resolved threads,
959 // all reported at once.
956960 if code := c.reviewGates(repo, mr, dir, targetSHA, headSHA); code >= 0 {
957961 return code
958962 }
@@ -1225,20 +1229,62 @@ func runMRMerge(c *Ctx, args []string) int {
12251229 })
12261230}
12271231
1228// reviewGates enforces require_approvals (fresh, non-author, latest review
1229// per reviewer; a fresh request-changes blocks), require_codeowners, and
1230// require_resolved. Returns -1 to proceed.
1232// reviewGates refuses a merge whose gates are not all met, naming every
1233// unmet one. Returns -1 to proceed.
12311234func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int {
1235 g, err := MergeGates(c.Store, repo, mr, dir, targetSHA, headSHA)
1236 if err != nil {
1237 return c.fail(protocol.ExitFailure, "%v", err)
1238 }
1239 if len(g.Unmet) > 0 {
1240 return c.fail(protocol.ExitDenied, "%s", strings.Join(g.Unmet, "; "))
1241 }
1242 return -1
1243}
1244
1245// MergeGates computes where a merge request stands against its
1246// repository's gates: draft, require_checks, require_approvals (fresh,
1247// non-author, latest review per reviewer from someone who can write; a
1248// fresh request-changes blocks), require_codeowners and require_resolved.
1249// Unmet carries one sentence per gate not passed. Fast-forward is
1250// reported, not gated: whether it matters depends on the strategy.
1251func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) (GatesOut, error) {
1252 set := repo.Settings
1253 g := GatesOut{Draft: mr.Draft, ApprovalsRequired: set.RequireApprovals,
1254 CodeownersRequired: set.RequireCodeowners, ResolvedRequired: set.RequireResolved,
1255 ChecksRequired: set.RequireChecks}
12321256 // A draft is open but not asking. This gate is unconditional — no
12331257 // setting turns it off — because the author said so themselves.
12341258 if mr.Draft {
1235 return c.fail(protocol.ExitDenied,
1236 "!%d is a draft; `gitbay mr ready %s %d` first", mr.Number, repo.Path(), mr.Number)
1259 g.Unmet = append(g.Unmet, fmt.Sprintf("!%d is a draft; `gitbay mr ready %s %d` first", mr.Number, repo.Path(), mr.Number))
12371260 }
1238 set := repo.Settings
1239 reviews, err := c.Store.ListMRReviews(mr.ID)
1261
1262 // Checks: with require_checks, the head must carry statuses and every
1263 // one of them must be green.
1264 statuses, err := st.ListCommitStatuses(repo.ID, headSHA)
12401265 if err != nil {
1241 return c.fail(protocol.ExitFailure, "%v", err)
1266 return g, err
1267 }
1268 g.Checks = store.CombinedStatus(statuses)
1269 if set.RequireChecks {
1270 switch g.Checks {
1271 case "success":
1272 case "":
1273 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks and none were reported on %.10s", repo.Path(), headSHA))
1274 default:
1275 var bad []string
1276 for _, st := range statuses {
1277 if st.State != "success" {
1278 bad = append(bad, st.Context+"="+st.State)
1279 }
1280 }
1281 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", ")))
1282 }
1283 }
1284
1285 reviews, err := st.ListMRReviews(mr.ID)
1286 if err != nil {
1287 return g, err
12421288 }
12431289 // Latest fresh review per reviewer decides their stance — but only
12441290 // from someone the repository trusts to write to it. Reviewing is
@@ -1246,7 +1292,7 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA
12461292 // public change possible; deciding a merge gate is not the same
12471293 // thing, and counting every verdict let anyone with an account
12481294 // satisfy require_approvals or block a merge indefinitely (#147).
1249 counts := ReviewersWhoCount(c.Store, repo, reviews)
1295 counts := ReviewersWhoCount(st, repo, reviews)
12501296 latest := map[string]string{}
12511297 for _, r := range reviews {
12521298 if r.Stale || r.Reviewer == mr.Author || !counts[r.Reviewer] {
@@ -1254,26 +1300,22 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA
12541300 }
12551301 latest[r.Reviewer] = r.Verdict
12561302 }
1257 var approvers []string
1258 var blockers []string
12591303 for who, verdict := range latest {
12601304 switch verdict {
12611305 case "approve":
1262 approvers = append(approvers, who)
1306 g.Approvals = append(g.Approvals, who)
12631307 case "request_changes":
1264 blockers = append(blockers, who)
1308 g.ChangesRequested = append(g.ChangesRequested, who)
12651309 }
12661310 }
1267
1311 slices.Sort(g.Approvals)
1312 slices.Sort(g.ChangesRequested)
12681313 if set.RequireApprovals > 0 {
1269 if len(blockers) > 0 {
1270 slices.Sort(blockers)
1271 return c.fail(protocol.ExitDenied,
1272 "%s requested changes on !%d; resolve their review before merging", strings.Join(blockers, ", "), mr.Number)
1314 if len(g.ChangesRequested) > 0 {
1315 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requested changes on !%d; resolve their review before merging", strings.Join(g.ChangesRequested, ", "), mr.Number))
12731316 }
1274 if len(approvers) < set.RequireApprovals {
1275 return c.fail(protocol.ExitDenied,
1276 "%s requires %d fresh approval(s); !%d has %d", repo.Path(), set.RequireApprovals, mr.Number, len(approvers))
1317 if len(g.Approvals) < set.RequireApprovals {
1318 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires %d fresh approval(s); !%d has %d", repo.Path(), set.RequireApprovals, mr.Number, len(g.Approvals)))
12771319 }
12781320 }
12791321
@@ -1287,65 +1329,72 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA
12871329 content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20)
12881330 }
12891331 if err != nil || len(content) == 0 {
1290 return c.fail(protocol.ExitDenied,
1291 "%s requires CODEOWNERS approval but %s carries no CODEOWNERS file",
1292 repo.Path(), mr.TargetRef)
1293 }
1294 rules := policy.ParseCodeowners(string(content))
1295 base, err := gitutil.MergeBase(dir, targetSHA, headSHA)
1296 if err != nil {
1297 return c.fail(protocol.ExitFailure, "%v", err)
1298 }
1299 files, err := gitutil.DiffFiles(dir, base, headSHA)
1300 if err != nil {
1301 return c.fail(protocol.ExitFailure, "%v", err)
1302 }
1303 approved := map[string]bool{}
1304 for _, a := range approvers {
1305 approved[a] = true
1306 }
1307 missing := map[string][]string{} // owner-set key -> example paths
1308 for _, f := range files {
1309 owners := policy.OwnersFor(rules, f)
1310 if owners == nil {
1311 continue
1332 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires CODEOWNERS approval but %s carries no CODEOWNERS file", repo.Path(), mr.TargetRef))
1333 } else {
1334 rules := policy.ParseCodeowners(string(content))
1335 base, err := gitutil.MergeBase(dir, targetSHA, headSHA)
1336 if err != nil {
1337 return g, err
13121338 }
1313 ok := false
1314 for _, o := range owners {
1315 if approved[o] {
1316 ok = true
1317 break
1318 }
1339 files, err := gitutil.DiffFiles(dir, base, headSHA)
1340 if err != nil {
1341 return g, err
1342 }
1343 approved := map[string]bool{}
1344 for _, a := range g.Approvals {
1345 approved[a] = true
13191346 }
1320 if !ok {
1321 key := strings.Join(owners, ",")
1322 if len(missing[key]) < 3 {
1347 missing := map[string][]string{} // owner-set key -> paths
1348 var keys []string
1349 for _, f := range files {
1350 owners := policy.OwnersFor(rules, f)
1351 if owners == nil {
1352 continue
1353 }
1354 ok := false
1355 for _, o := range owners {
1356 if approved[o] {
1357 ok = true
1358 break
1359 }
1360 }
1361 if !ok {
1362 key := strings.Join(owners, ",")
1363 if _, seen := missing[key]; !seen {
1364 keys = append(keys, key)
1365 }
13231366 missing[key] = append(missing[key], f)
13241367 }
13251368 }
1326 }
1327 if len(missing) > 0 {
1328 var parts []string
1329 for owners, paths := range missing {
1330 parts = append(parts, fmt.Sprintf("%s (owned by %s)", strings.Join(paths, ", "), owners))
1369 if len(missing) > 0 {
1370 slices.Sort(keys)
1371 var parts []string
1372 for _, key := range keys {
1373 paths := missing[key]
1374 g.OwnersOutstanding = append(g.OwnersOutstanding, OwnersOut{Files: paths, Owners: strings.Split(key, ",")})
1375 if len(paths) > 3 {
1376 paths = paths[:3]
1377 }
1378 parts = append(parts, fmt.Sprintf("%s (owned by %s)", strings.Join(paths, ", "), key))
1379 }
1380 g.Unmet = append(g.Unmet, "CODEOWNERS approval missing for: "+strings.Join(parts, "; "))
13311381 }
1332 slices.Sort(parts)
1333 return c.fail(protocol.ExitDenied,
1334 "CODEOWNERS approval missing for: %s", strings.Join(parts, "; "))
13351382 }
13361383 }
13371384
1338 if set.RequireResolved {
1339 n, err := c.Store.UnresolvedThreadCount(mr.ID)
1340 if err != nil {
1341 return c.fail(protocol.ExitFailure, "%v", err)
1342 }
1343 if n > 0 {
1344 return c.fail(protocol.ExitDenied,
1345 "%s requires review threads resolved; !%d has %d open (mr threads %s %d)", repo.Path(), mr.Number, n, repo.Path(), mr.Number)
1346 }
1385 n, err := st.UnresolvedThreadCount(mr.ID)
1386 if err != nil {
1387 return g, err
13471388 }
1348 return -1
1389 g.OpenThreads = n
1390 if set.RequireResolved && n > 0 {
1391 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires review threads resolved; !%d has %d open (mr threads %s %d)", repo.Path(), mr.Number, n, repo.Path(), mr.Number))
1392 }
1393
1394 if ff, err := gitutil.IsAncestor(dir, targetSHA, headSHA); err == nil {
1395 g.FastForward = ff
1396 }
1397 return g, nil
13491398}
13501399
13511400func runMRDraft(c *Ctx, args []string) int { return setMRDraft(c, args, true) }
internal/control/output.go +27
@@ -33,6 +33,33 @@ type MRShow struct {
3333 Commits []CommitOut `json:"commits,omitempty"`
3434 Comments []commentOut `json:"comments,omitempty"`
3535 Reviews []ReviewOut `json:"reviews,omitempty"`
36 // Gates is set while the merge request is open.
37 Gates *GatesOut `json:"gates,omitempty"`
38}
39
40// OwnersOut is a set of changed files still waiting on an approval from
41// one of their CODEOWNERS.
42type OwnersOut struct {
43 Files []string `json:"files"`
44 Owners []string `json:"owners"`
45}
46
47// GatesOut is what a merge request has to pass to merge, and where it
48// stands: the same computation mr merge refuses on, so nothing is
49// learned at the refusal that mr show did not say (#199).
50type GatesOut struct {
51 Draft bool `json:"draft,omitempty"`
52 ApprovalsRequired int `json:"approvals_required"`
53 Approvals []string `json:"approvals,omitempty"` // fresh, from reviewers who count
54 ChangesRequested []string `json:"changes_requested,omitempty"`
55 CodeownersRequired bool `json:"codeowners_required"`
56 OwnersOutstanding []OwnersOut `json:"owners_outstanding,omitempty"`
57 ResolvedRequired bool `json:"resolved_required"`
58 OpenThreads int `json:"open_threads"`
59 ChecksRequired bool `json:"checks_required"`
60 Checks string `json:"checks,omitempty"` // combined status; "" when none reported
61 FastForward bool `json:"fast_forward"`
62 Unmet []string `json:"unmet,omitempty"`
3663}
3764
3865// ReviewOut is one review on a merge request.
internal/httpd/mrpage_test.go +32
@@ -6,6 +6,7 @@ import (
66 "testing"
77 "time"
88
9 "gitbay.org/gitbay/internal/control"
910 "gitbay.org/gitbay/internal/gitutil"
1011 "gitbay.org/gitbay/internal/store"
1112 "gitbay.org/gitbay/internal/web"
@@ -31,6 +32,7 @@ type mrPageData struct {
3132 Revisions []store.MRHead
3233 Notice string
3334 DetachedThreads []diffThread
35 Gates *control.GatesOut
3436}
3537
3638func renderMR(t *testing.T, m store.MR, reviews []store.MRReview, checks []store.Check) string {
@@ -131,3 +133,33 @@ func TestMRChecksRenderSkippedWithoutBuildLink(t *testing.T) {
131133 t.Errorf("skipped check with no build linked anyway: %s", row)
132134 }
133135}
136
137// The gates block says what the merge is waiting on before a merge is
138// refused (#199): every unmet gate, the approval count, the outstanding
139// owners, and whether a fast-forward is possible.
140func TestMRGatesRender(t *testing.T) {
141 render := func(g *control.GatesOut) string {
142 t.Helper()
143 var sb strings.Builder
144 if err := web.Render(&sb, "mr.html", mrPageData{
145 repoPage: testRepoPage(), MR: testMR("open"), View: "conversation", Gates: g,
146 }); err != nil {
147 t.Fatalf("render: %v", err)
148 }
149 return sb.String()
150 }
151 out := render(&control.GatesOut{ApprovalsRequired: 2, Approvals: []string{"bob"},
152 OwnersOutstanding: []control.OwnersOut{{Files: []string{"svc.go"}, Owners: []string{"carol"}}},
153 Unmet: []string{"krz/hutch requires 2 fresh approval(s); !42 has 1", "CODEOWNERS approval missing for: svc.go (owned by carol)"}})
154 for _, want := range []string{"Merge gates", "requires 2 fresh approval(s)", "approvals: 1 of 2 (bob)", `waiting on <a href="/carol">carol</a>`, "not a fast-forward"} {
155 if !strings.Contains(out, want) {
156 t.Errorf("gates block missing %q:\n%s", want, out)
157 }
158 }
159 if out := render(&control.GatesOut{FastForward: true}); !strings.Contains(out, "All gates met") || !strings.Contains(out, "fast-forward possible") {
160 t.Errorf("met gates not rendered:\n%s", out)
161 }
162 if out := render(nil); strings.Contains(out, "Merge gates") {
163 t.Errorf("gates block on a merge request without gates:\n%s", out)
164 }
165}
internal/httpd/web.go +12 −1
@@ -1828,6 +1828,16 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) {
18281828 if view != "commits" && view != "diff" {
18291829 view = "conversation"
18301830 }
1831 // Where the merge request stands against the gates, the same
1832 // computation mr merge refuses on (#199).
1833 var gates *control.GatesOut
1834 if m.State == "open" || m.State == "source_gone" {
1835 if targetSHA, err := gitutil.ResolveRef(p.Dir, "refs/heads/"+m.TargetRef); err == nil {
1836 if g, err := control.MergeGates(s.st, p.Repo, m, p.Dir, targetSHA, m.HeadSHA); err == nil {
1837 gates = &g
1838 }
1839 }
1840 }
18311841 // The stack around an open merge request, for the header.
18321842 var stackedOn *store.MR
18331843 var stacked []store.MR
@@ -1862,9 +1872,10 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) {
18621872 DetachedThreads []diffThread
18631873 StackedOn *store.MR
18641874 Stacked []store.MR
1875 Gates *control.GatesOut
18651876 }{p, m, view, md(m.Body, m.BodyFormat), checks, combined, renderComments(comments, md),
18661877 reviewRows, files, diffTruncated, stat, commits, commitsTotal, branches, s.canEditItem(r, p.Repo, m.Author),
1867 canWrite, unresolved, revisions, s.takeFlash(w, r), detachedThreads, stackedOn, stacked})
1878 canWrite, unresolved, revisions, s.takeFlash(w, r), detachedThreads, stackedOn, stacked, gates})
18681879}
18691880
18701881func (s *Server) refs(w http.ResponseWriter, r *http.Request) {
internal/web/templates/mr.html +8
@@ -145,6 +145,14 @@
145145 {{if gt (len .Revisions) 1}}<p class="row none">{{len .Revisions}} revisions pushed. What changed between the last two:
146146 <code>gitbay mr range-diff {{.Repo.OwnerName}}/{{.Repo.Name}} {{.MR.Number}}</code></p>{{end}}
147147 </div>
148 {{with .Gates}}<div class="grp">
149 <h2>Merge gates</h2>
150 {{if .Unmet}}{{range .Unmet}}<p class="row"><span class="dot bad"></span>{{.}}</p>{{end}}
151 {{else}}<p class="row"><span class="dot ok"></span>All gates met</p>{{end}}
152 {{if .ApprovalsRequired}}<p class="row none">approvals: {{len .Approvals}} of {{.ApprovalsRequired}}{{if .Approvals}} ({{range $i, $a := .Approvals}}{{if $i}}, {{end}}{{$a}}{{end}}){{end}}</p>{{end}}
153 {{range .OwnersOutstanding}}<p class="row none">waiting on {{range $i, $o := .Owners}}{{if $i}} or {{end}}<a href="/{{$o}}">{{$o}}</a>{{end}} for {{range $i, $f := .Files}}{{if $i}}, {{end}}<code>{{$f}}</code>{{end}}</p>{{end}}
154 <p class="row none">{{if .FastForward}}fast-forward possible{{else}}not a fast-forward: rebase, or merge with a merge commit{{end}}</p>
155 </div>{{end}}
148156 <div class="grp">
149157 <h2>Checks</h2>
150158 {{if .Checks}}<p class="row"><span class="badge check-{{.Combined}}">{{.Combined}}</span></p>