Commit 6f184c2936
Verified · cmc ci/build: success ci/test: success ci/vuln: success
Layout: unified · split
cmd/gitbay/main.go +2
| @@ -507,6 +507,8 @@ func mrCmd() *cobra.Command { | |||
| 507 | pass("review", "review: --approve|--request-changes|--comment", passOpts{server: []string{"mr", "review"}, needsRepo: true}), | 507 | pass("review", "review: --approve|--request-changes|--comment", passOpts{server: []string{"mr", "review"}, needsRepo: true}), |
| 508 | pass("merge", "merge: [--strategy ff|merge|squash|rebase]", passOpts{server: []string{"mr", "merge"}, needsRepo: true}), | 508 | pass("merge", "merge: [--strategy ff|merge|squash|rebase]", passOpts{server: []string{"mr", "merge"}, needsRepo: true}), |
| 509 | pass("close", "close without merging", passOpts{server: []string{"mr", "close"}, needsRepo: true}), | 509 | pass("close", "close without merging", passOpts{server: []string{"mr", "close"}, needsRepo: true}), |
| 510 | pass("draft", "mark as work in progress", passOpts{server: []string{"mr", "draft"}, needsRepo: true}), | ||
| 511 | pass("ready", "take the draft mark off, so it can merge", passOpts{server: []string{"mr", "ready"}, needsRepo: true}), | ||
| 510 | pass("edit", "edit title or body: <n> [--title <t>] [--body <b>|--file -]", passOpts{server: []string{"mr", "edit"}, needsRepo: true, stdinOK: true}), | 512 | pass("edit", "edit title or body: <n> [--title <t>] [--body <b>|--file -]", passOpts{server: []string{"mr", "edit"}, needsRepo: true, stdinOK: true}), |
| 511 | pass("milestone", "set or clear the milestone: <n> <title|none>", passOpts{server: []string{"mr", "milestone"}, needsRepo: true}), | 513 | pass("milestone", "set or clear the milestone: <n> <title|none>", passOpts{server: []string{"mr", "milestone"}, needsRepo: true}), |
| 512 | pass("retarget", "retarget onto another branch: <n> <branch>", passOpts{server: []string{"mr", "retarget"}, needsRepo: true}), | 514 | pass("retarget", "retarget onto another branch: <n> <branch>", passOpts{server: []string{"mr", "retarget"}, needsRepo: true}), |
e2e/mrdraft_test.go added +99
| @@ -0,0 +1,99 @@ | |||
| 1 | package e2e | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "encoding/json" | ||
| 5 | "os" | ||
| 6 | "path/filepath" | ||
| 7 | "strings" | ||
| 8 | "testing" | ||
| 9 | ) | ||
| 10 | |||
| 11 | // TestMRDraft covers the first stage of #111: a merge request opened to | ||
| 12 | // show work rather than to ask for a merge. | ||
| 13 | func TestMRDraft(t *testing.T) { | ||
| 14 | inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n") | ||
| 15 | aliceKey := inst.newKey(t, "alice") | ||
| 16 | bobKey := inst.newKey(t, "bob") | ||
| 17 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | ||
| 18 | inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub") | ||
| 19 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { | ||
| 20 | t.Fatalf("repo create: %s", errOut) | ||
| 21 | } | ||
| 22 | if _, _, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/app", "bob", "write"); code != 0 { | ||
| 23 | t.Fatal("grant failed") | ||
| 24 | } | ||
| 25 | |||
| 26 | env := inst.gitEnv(aliceKey) | ||
| 27 | work := t.TempDir() | ||
| 28 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 29 | dir := filepath.Join(work, "w") | ||
| 30 | os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644) | ||
| 31 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 32 | mustGit(t, dir, env, "add", ".") | ||
| 33 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 34 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 35 | mustGit(t, dir, env, "checkout", "-q", "-b", "feat") | ||
| 36 | mustGit(t, dir, env, "commit", "-q", "--allow-empty", "-m", "work") | ||
| 37 | mustGit(t, dir, env, "push", "-q", "origin", "feat") | ||
| 38 | |||
| 39 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", | ||
| 40 | "--source", "feat", "--target", "main", "--title", "'wip'", "--draft"); code != 0 { | ||
| 41 | t.Fatalf("mr create --draft: %s", errOut) | ||
| 42 | } | ||
| 43 | |||
| 44 | draft := func() bool { | ||
| 45 | t.Helper() | ||
| 46 | out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "1", "--json") | ||
| 47 | if code != 0 { | ||
| 48 | t.Fatalf("mr show: %s", errOut) | ||
| 49 | } | ||
| 50 | var env struct { | ||
| 51 | Data struct { | ||
| 52 | State string `json:"state"` | ||
| 53 | Draft bool `json:"draft"` | ||
| 54 | } `json:"data"` | ||
| 55 | } | ||
| 56 | json.Unmarshal([]byte(out), &env) | ||
| 57 | // A draft is open, not a fifth state. | ||
| 58 | if env.Data.State != "open" { | ||
| 59 | t.Fatalf("draft changed the state to %q", env.Data.State) | ||
| 60 | } | ||
| 61 | return env.Data.Draft | ||
| 62 | } | ||
| 63 | |||
| 64 | if !draft() { | ||
| 65 | t.Fatal("--draft did not mark it") | ||
| 66 | } | ||
| 67 | |||
| 68 | // A draft does not merge, and no repository setting is involved. | ||
| 69 | _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1") | ||
| 70 | if code != 4 || !strings.Contains(errOut, "is a draft") { | ||
| 71 | t.Fatalf("draft merged: exit %d, %s", code, errOut) | ||
| 72 | } | ||
| 73 | |||
| 74 | // Nor does it sit in anyone's review queue. | ||
| 75 | out, _, _ := inst.ssh(t, bobKey, "", "dashboard", "--json") | ||
| 76 | if strings.Contains(out, `"review_queue":[{`) { | ||
| 77 | t.Fatalf("draft is in the review queue: %s", out) | ||
| 78 | } | ||
| 79 | |||
| 80 | // Ready, and now it does both. | ||
| 81 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "ready", "alice/app", "1"); code != 0 { | ||
| 82 | t.Fatalf("mr ready: %s", errOut) | ||
| 83 | } | ||
| 84 | if draft() { | ||
| 85 | t.Fatal("mr ready did not clear the mark") | ||
| 86 | } | ||
| 87 | out, _, _ = inst.ssh(t, bobKey, "", "dashboard", "--json") | ||
| 88 | if !strings.Contains(out, `"number":1`) { | ||
| 89 | t.Fatalf("ready MR is not in the review queue: %s", out) | ||
| 90 | } | ||
| 91 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1"); code != 0 { | ||
| 92 | t.Fatalf("ready MR refused: %s", errOut) | ||
| 93 | } | ||
| 94 | |||
| 95 | // Ready twice is a usage error rather than a silent no-op. | ||
| 96 | if _, _, code := inst.ssh(t, aliceKey, "", "mr", "ready", "alice/app", "1"); code == 0 { | ||
| 97 | t.Fatal("mr ready on a merged MR succeeded") | ||
| 98 | } | ||
| 99 | } | ||
internal/control/events.go +1
| @@ -31,6 +31,7 @@ var EventKinds = []string{ | |||
| 31 | "mr.closed", | 31 | "mr.closed", |
| 32 | "mr.commented", | 32 | "mr.commented", |
| 33 | "mr.created", | 33 | "mr.created", |
| 34 | "mr.draft", | ||
| 34 | "mr.edited", | 35 | "mr.edited", |
| 35 | "mr.merged", | 36 | "mr.merged", |
| 36 | "mr.milestoned", | 37 | "mr.milestoned", |
internal/control/ghimport.go +1 −1
| @@ -169,7 +169,7 @@ func runImportIssues(c *Ctx, args []string) int { | |||
| 169 | return c.fail(protocol.ExitFailure, "%v", err) | 169 | return c.fail(protocol.ExitFailure, "%v", err) |
| 170 | } | 170 | } |
| 171 | body := attribution(src, it.Number, "pull request", it.User.Login, it.CreatedAt) + it.Body | 171 | body := attribution(src, it.Number, "pull request", it.User.Login, it.CreatedAt) + it.Body |
| 172 | localN, err = c.Store.CreateMR(repo.ID, c.User.ID, repo.ID, pr.Head.Ref, pr.Base.Ref, it.Title, body, pr.Head.SHA, "md") | 172 | localN, err = c.Store.CreateMR(repo.ID, c.User.ID, repo.ID, pr.Head.Ref, pr.Base.Ref, it.Title, body, pr.Head.SHA, "md", false) |
| 173 | if err != nil { | 173 | if err != nil { |
| 174 | return c.fail(protocol.ExitFailure, "%v", err) | 174 | return c.fail(protocol.ExitFailure, "%v", err) |
| 175 | } | 175 | } |
internal/control/migrate.go +1 −1
| @@ -256,7 +256,7 @@ func runAccountImportBundle(c *Ctx, args []string) int { | |||
| 256 | continue | 256 | continue |
| 257 | } | 257 | } |
| 258 | body := migAttribution(src, "merge request", bm.Author, bm.CreatedAt, bm.Number) + bm.Body | 258 | body := migAttribution(src, "merge request", bm.Author, bm.CreatedAt, bm.Number) + bm.Body |
| 259 | n, err := c.Store.CreateMR(repo.ID, c.User.ID, repo.ID, bm.SourceRef, bm.TargetRef, bm.Title, body, "", "md") | 259 | n, err := c.Store.CreateMR(repo.ID, c.User.ID, repo.ID, bm.SourceRef, bm.TargetRef, bm.Title, body, "", "md", false) |
| 260 | if err != nil { | 260 | if err != nil { |
| 261 | return c.fail(protocol.ExitFailure, "%v", err) | 261 | return c.fail(protocol.ExitFailure, "%v", err) |
| 262 | } | 262 | } |
internal/control/mr.go +82 −8
| @@ -37,8 +37,14 @@ func init() { | |||
| 37 | Usage: "repo settings require-signed <owner/name> on|off", Run: runRequireSigned}) | 37 | Usage: "repo settings require-signed <owner/name> on|off", Run: runRequireSigned}) |
| 38 | register(Command{Path: []string{"mr", "create"}, | 38 | register(Command{Path: []string{"mr", "create"}, |
| 39 | Summary: "open a merge request", | 39 | Summary: "open a merge request", |
| 40 | Usage: "mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t> [--body <b> | --file -] [--format md|org]", | 40 | Usage: "mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t> [--body <b> | --file -] [--format md|org] [--draft]", |
| 41 | ReadsStdin: true, Run: runMRCreate}) | 41 | ReadsStdin: true, Run: runMRCreate}) |
| 42 | register(Command{Path: []string{"mr", "draft"}, | ||
| 43 | Summary: "mark a merge request as work in progress", | ||
| 44 | Usage: "mr draft <owner/name> <n>", Run: runMRDraft}) | ||
| 45 | register(Command{Path: []string{"mr", "ready"}, | ||
| 46 | Summary: "take the draft mark off, so it can merge", | ||
| 47 | Usage: "mr ready <owner/name> <n>", Run: runMRReady}) | ||
| 42 | register(Command{Path: []string{"mr", "list"}, | 48 | register(Command{Path: []string{"mr", "list"}, |
| 43 | Summary: "list merge requests", | 49 | Summary: "list merge requests", |
| 44 | Usage: "mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runMRList}) | 50 | Usage: "mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runMRList}) |
| @@ -229,15 +235,16 @@ func mrRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool | |||
| 229 | func mrHeadRef(n int64) string { return fmt.Sprintf("refs/merge-requests/%d/head", n) } | 235 | func mrHeadRef(n int64) string { return fmt.Sprintf("refs/merge-requests/%d/head", n) } |
| 230 | 236 | ||
| 231 | func runMRCreate(c *Ctx, args []string) int { | 237 | func runMRCreate(c *Ctx, args []string) int { |
| 232 | f, err := parseFlags(args, flagSpec{Values: []string{"--source", "--target", "--title", "--body", "--file", "--format"}, MaxPos: 1, | 238 | f, err := parseFlags(args, flagSpec{Values: []string{"--source", "--target", "--title", "--body", "--file", "--format"}, |
| 233 | Usage: "mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t>"}) | 239 | Bools: []string{"--draft"}, MaxPos: 1, |
| 240 | Usage: "mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t> [--draft]"}) | ||
| 234 | if err != nil { | 241 | if err != nil { |
| 235 | return c.fail(protocol.ExitUsage, "%v", err) | 242 | return c.fail(protocol.ExitUsage, "%v", err) |
| 236 | } | 243 | } |
| 237 | path, source, target := f.pos(0), f.Value("--source"), f.Value("--target") | 244 | path, source, target := f.pos(0), f.Value("--source"), f.Value("--target") |
| 238 | title, body, file, format := f.Value("--title"), f.Value("--body"), f.Value("--file"), f.Value("--format") | 245 | title, body, file, format := f.Value("--title"), f.Value("--body"), f.Value("--file"), f.Value("--format") |
| 239 | if path == "" || source == "" || title == "" { | 246 | if path == "" || source == "" || title == "" { |
| 240 | return c.fail(protocol.ExitUsage, "usage: mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t>") | 247 | return c.fail(protocol.ExitUsage, "usage: mr create <target owner/name> --source [owner/name:]<branch> --target <branch> --title <t> [--draft]") |
| 241 | } | 248 | } |
| 242 | fmtName, err := markupFormat(format) | 249 | fmtName, err := markupFormat(format) |
| 243 | if err != nil { | 250 | if err != nil { |
| @@ -280,7 +287,7 @@ func runMRCreate(c *Ctx, args []string) int { | |||
| 280 | if err != nil { | 287 | if err != nil { |
| 281 | return c.failErr(err) | 288 | return c.failErr(err) |
| 282 | } | 289 | } |
| 283 | n, err := c.Store.CreateMR(repo.ID, c.User.ID, srcRepo.ID, srcBranch, target, title, b, headSHA, fmtName) | 290 | n, err := c.Store.CreateMR(repo.ID, c.User.ID, srcRepo.ID, srcBranch, target, title, b, headSHA, fmtName, f.Has("--draft")) |
| 284 | if err != nil { | 291 | if err != nil { |
| 285 | return c.fail(protocol.ExitFailure, "%v", err) | 292 | return c.fail(protocol.ExitFailure, "%v", err) |
| 286 | } | 293 | } |
| @@ -315,6 +322,8 @@ type mrOut struct { | |||
| 315 | Number int64 `json:"number"` | 322 | Number int64 `json:"number"` |
| 316 | Title string `json:"title"` | 323 | Title string `json:"title"` |
| 317 | State string `json:"state"` | 324 | State string `json:"state"` |
| 325 | // Draft is an open merge request not asking to be merged yet. | ||
| 326 | Draft bool `json:"draft,omitempty"` | ||
| 318 | Author string `json:"author"` | 327 | Author string `json:"author"` |
| 319 | Source string `json:"source"` // owner/name:branch, or branch, "" if gone | 328 | Source string `json:"source"` // owner/name:branch, or branch, "" if gone |
| 320 | TargetRef string `json:"target_ref"` | 329 | TargetRef string `json:"target_ref"` |
| @@ -372,7 +381,7 @@ func mrToOut(repo store.Repo, m store.MR, withBody bool) mrOut { | |||
| 372 | src = m.SourcePath + ":" + m.SourceRef | 381 | src = m.SourcePath + ":" + m.SourceRef |
| 373 | } | 382 | } |
| 374 | } | 383 | } |
| 375 | o := mrOut{Number: m.Number, Title: m.Title, State: m.State, Author: m.Author, | 384 | o := mrOut{Number: m.Number, Title: m.Title, State: m.State, Draft: m.Draft, Author: m.Author, |
| 376 | Source: src, TargetRef: m.TargetRef, HeadSHA: m.HeadSHA, Milestone: m.Milestone, | 385 | Source: src, TargetRef: m.TargetRef, HeadSHA: m.HeadSHA, Milestone: m.Milestone, |
| 377 | CreatedAt: m.CreatedAt, MergedAt: m.MergedAt, MergedBy: m.MergedBy, | 386 | CreatedAt: m.CreatedAt, MergedAt: m.MergedAt, MergedBy: m.MergedBy, |
| 378 | ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy} | 387 | ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy} |
| @@ -433,7 +442,11 @@ func runMRList(c *Ctx, args []string) int { | |||
| 433 | if d.StackedOn != nil { | 442 | if d.StackedOn != nil { |
| 434 | stacked = fmt.Sprintf("\tstacked on !%d", d.StackedOn.Number) | 443 | stacked = fmt.Sprintf("\tstacked on !%d", d.StackedOn.Number) |
| 435 | } | 444 | } |
| 436 | fmt.Fprintf(w, "!%d\t%s\t%s\t%s -> %s%s\n", d.Number, d.State, d.Title, d.Source, d.TargetRef, stacked) | 445 | state := d.State |
| 446 | if d.Draft { | ||
| 447 | state = "draft" | ||
| 448 | } | ||
| 449 | fmt.Fprintf(w, "!%d\t%s\t%s\t%s -> %s%s\n", d.Number, state, d.Title, d.Source, d.TargetRef, stacked) | ||
| 437 | } | 450 | } |
| 438 | }) | 451 | }) |
| 439 | } | 452 | } |
| @@ -513,7 +526,11 @@ func runMRShow(c *Ctx, args []string) int { | |||
| 513 | UnresolvedThreads: unresolved, Commits: commits, Comments: cs, Reviews: rs} | 526 | UnresolvedThreads: unresolved, Commits: commits, Comments: cs, Reviews: rs} |
| 514 | d.StackedOn, d.Stacked = stackOf(c, repo, mr) | 527 | d.StackedOn, d.Stacked = stackOf(c, repo, mr) |
| 515 | return c.emit(d, func(w io.Writer) { | 528 | return c.emit(d, func(w io.Writer) { |
| 516 | fmt.Fprintf(w, "!%d %s [%s] by %s\n%s -> %s @ %.10s\n", d.Number, d.Title, d.State, d.Author, d.Source, d.TargetRef, d.HeadSHA) | 529 | state := d.State |
| 530 | if d.Draft { | ||
| 531 | state = "draft" | ||
| 532 | } | ||
| 533 | fmt.Fprintf(w, "!%d %s [%s] by %s\n%s -> %s @ %.10s\n", d.Number, d.Title, state, d.Author, d.Source, d.TargetRef, d.HeadSHA) | ||
| 517 | if d.StackedOn != nil { | 534 | if d.StackedOn != nil { |
| 518 | fmt.Fprintf(w, "stacked on !%d %s\n", d.StackedOn.Number, d.StackedOn.Title) | 535 | fmt.Fprintf(w, "stacked on !%d %s\n", d.StackedOn.Number, d.StackedOn.Title) |
| 519 | } | 536 | } |
| @@ -1049,6 +1066,12 @@ func runMRMerge(c *Ctx, args []string) int { | |||
| 1049 | // per reviewer; a fresh request-changes blocks), require_codeowners, and | 1066 | // per reviewer; a fresh request-changes blocks), require_codeowners, and |
| 1050 | // require_resolved. Returns -1 to proceed. | 1067 | // require_resolved. Returns -1 to proceed. |
| 1051 | func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int { | 1068 | func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int { |
| 1069 | // A draft is open but not asking. This gate is unconditional — no | ||
| 1070 | // setting turns it off — because the author said so themselves. | ||
| 1071 | if mr.Draft { | ||
| 1072 | return c.fail(protocol.ExitDenied, | ||
| 1073 | "!%d is a draft; `gitbay mr ready %s %d` first", mr.Number, repo.Path(), mr.Number) | ||
| 1074 | } | ||
| 1052 | set := repo.Settings | 1075 | set := repo.Settings |
| 1053 | reviews, err := c.Store.ListMRReviews(mr.ID) | 1076 | reviews, err := c.Store.ListMRReviews(mr.ID) |
| 1054 | if err != nil { | 1077 | if err != nil { |
| @@ -1156,6 +1179,57 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA | |||
| 1156 | return -1 | 1179 | return -1 |
| 1157 | } | 1180 | } |
| 1158 | 1181 | ||
| 1182 | func runMRDraft(c *Ctx, args []string) int { return setMRDraft(c, args, true) } | ||
| 1183 | func runMRReady(c *Ctx, args []string) int { return setMRDraft(c, args, false) } | ||
| 1184 | |||
| 1185 | func setMRDraft(c *Ctx, args []string, draft bool) int { | ||
| 1186 | verb := "ready" | ||
| 1187 | if draft { | ||
| 1188 | verb = "draft" | ||
| 1189 | } | ||
| 1190 | repo, mr, code := mrRef(c, args, policy.CanRead) | ||
| 1191 | if code >= 0 { | ||
| 1192 | return code | ||
| 1193 | } | ||
| 1194 | if code := refuseArchived(c, repo); code >= 0 { | ||
| 1195 | return code | ||
| 1196 | } | ||
| 1197 | if len(args) != 2 { | ||
| 1198 | return c.fail(protocol.ExitUsage, "usage: mr %s <owner/name> <n>", verb) | ||
| 1199 | } | ||
| 1200 | if code := authorOrWrite(c, repo, mr.Author, "change this merge request"); code >= 0 { | ||
| 1201 | return code | ||
| 1202 | } | ||
| 1203 | if mr.State != "open" && mr.State != "source_gone" { | ||
| 1204 | return c.fail(protocol.ExitUsage, "MR !%d is %s", mr.Number, mr.State) | ||
| 1205 | } | ||
| 1206 | if mr.Draft == draft { | ||
| 1207 | state := "already ready" | ||
| 1208 | if draft { | ||
| 1209 | state = "already a draft" | ||
| 1210 | } | ||
| 1211 | return c.fail(protocol.ExitUsage, "MR !%d is %s", mr.Number, state) | ||
| 1212 | } | ||
| 1213 | if err := c.Store.SetMRDraft(mr.ID, draft); err != nil { | ||
| 1214 | return c.fail(protocol.ExitFailure, "%v", err) | ||
| 1215 | } | ||
| 1216 | c.Store.RecordEvent(repo.ID, c.User.ID, "mr.draft", | ||
| 1217 | fmt.Sprintf(`{"number":%d,"draft":%t}`, mr.Number, draft)) | ||
| 1218 | // Marking ready is the request for review; going back to draft | ||
| 1219 | // withdraws it and is not worth anyone's inbox. | ||
| 1220 | if !draft { | ||
| 1221 | if parts, err := c.Store.MRParticipants(mr.ID); err == nil { | ||
| 1222 | notify(c, parts, notice{repo: repo, kind: "mr", | ||
| 1223 | subject: mrSubject(repo, mr.Number, mr.Title), | ||
| 1224 | action: fmt.Sprintf("marked !%d ready for review", mr.Number), | ||
| 1225 | path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) | ||
| 1226 | } | ||
| 1227 | } | ||
| 1228 | return c.emit(map[string]any{"number": mr.Number, "draft": draft}, func(w io.Writer) { | ||
| 1229 | fmt.Fprintf(w, "%s!%d is %s\n", repo.Path(), mr.Number, map[bool]string{true: "a draft", false: "ready"}[draft]) | ||
| 1230 | }) | ||
| 1231 | } | ||
| 1232 | |||
| 1159 | func runMRClose(c *Ctx, args []string) int { | 1233 | func runMRClose(c *Ctx, args []string) int { |
| 1160 | repo, mr, code := mrRef(c, args, policy.CanRead) | 1234 | repo, mr, code := mrRef(c, args, policy.CanRead) |
| 1161 | if code >= 0 { | 1235 | if code >= 0 { |
internal/httpd/mractions.go +11
| @@ -68,6 +68,17 @@ func (s *Server) mrCloseSubmit(w http.ResponseWriter, r *http.Request, u store.U | |||
| 68 | s.done(w, r, code, msg, s.mrRedirect) | 68 | s.done(w, r, code, msg, s.mrRedirect) |
| 69 | } | 69 | } |
| 70 | 70 | ||
| 71 | // mrDraftSubmit toggles the draft mark. The form says which way it is | ||
| 72 | // going, so a stale page cannot flip the wrong one. | ||
| 73 | func (s *Server) mrDraftSubmit(w http.ResponseWriter, r *http.Request, u store.User) { | ||
| 74 | verb := "ready" | ||
| 75 | if r.FormValue("draft") == "on" { | ||
| 76 | verb = "draft" | ||
| 77 | } | ||
| 78 | _, msg, code := s.runControlCode(u, mrArgs(r, verb)) | ||
| 79 | s.done(w, r, code, msg, s.mrRedirect) | ||
| 80 | } | ||
| 81 | |||
| 71 | // mrDiffCommentSubmit opens a review thread on a diff line, or replies to | 82 | // mrDiffCommentSubmit opens a review thread on a diff line, or replies to |
| 72 | // one. The body goes in on stdin: it is user prose, and argv is visible in | 83 | // one. The body goes in on stdin: it is user prose, and argv is visible in |
| 73 | // /proc. | 84 | // /proc. |
internal/httpd/routes.go +2
| @@ -167,6 +167,8 @@ func (s *Server) Routes() []Route { | |||
| 167 | Handler: s.checkOrigin(s.requireUser(s.mrMergeSubmit))}, | 167 | Handler: s.checkOrigin(s.requireUser(s.mrMergeSubmit))}, |
| 168 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/close", Mutating: true, | 168 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/close", Mutating: true, |
| 169 | Handler: s.checkOrigin(s.requireUser(s.mrCloseSubmit))}, | 169 | Handler: s.checkOrigin(s.requireUser(s.mrCloseSubmit))}, |
| 170 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/draft", Mutating: true, | ||
| 171 | Handler: s.checkOrigin(s.requireUser(s.mrDraftSubmit))}, | ||
| 170 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/retarget", Mutating: true, | 172 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/retarget", Mutating: true, |
| 171 | Handler: s.checkOrigin(s.requireUser(s.mrRetargetSubmit))}, | 173 | Handler: s.checkOrigin(s.requireUser(s.mrRetargetSubmit))}, |
| 172 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/thread", Mutating: true, | 174 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/thread", Mutating: true, |
internal/store/dashboard.go +1
| @@ -139,6 +139,7 @@ const reviewQueueQuery = ` | |||
| 139 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id | 139 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id |
| 140 | JOIN users au ON au.id = x.author_id | 140 | JOIN users au ON au.id = x.author_id |
| 141 | WHERE x.state IN ('open', 'source_gone') | 141 | WHERE x.state IN ('open', 'source_gone') |
| 142 | AND x.draft = 0 | ||
| 142 | AND x.author_id <> ?1 | 143 | AND x.author_id <> ?1 |
| 143 | AND NOT EXISTS (SELECT 1 FROM mr_reviews rv | 144 | AND NOT EXISTS (SELECT 1 FROM mr_reviews rv |
| 144 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 | 145 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 |
internal/store/migrations/0037_mr_draft.down.sql added +1
| @@ -0,0 +1 @@ | |||
| 1 | ALTER TABLE merge_requests DROP COLUMN draft; | ||
internal/store/migrations/0037_mr_draft.up.sql added +6
| @@ -0,0 +1,6 @@ | |||
| 1 | -- A merge request opened to show work in progress rather than to ask for | ||
| 2 | -- a merge. Deliberately a flag, not a state: state is open | merged | | ||
| 3 | -- closed | source_gone, and adding a fifth would change what every | ||
| 4 | -- `state = 'open'` query means, including the merge gates and the review | ||
| 5 | -- queue. A draft is open; it is just not asking yet (#111). | ||
| 6 | ALTER TABLE merge_requests ADD COLUMN draft INTEGER NOT NULL DEFAULT 0; | ||
internal/store/mrs.go +27 −15
| @@ -19,15 +19,18 @@ type MR struct { | |||
| 19 | Body string | 19 | Body string |
| 20 | BodyFormat string // md | org | 20 | BodyFormat string // md | org |
| 21 | State string // open | merged | closed | source_gone | 21 | State string // open | merged | closed | source_gone |
| 22 | Milestone string | 22 | // Draft marks an open merge request that is not asking to be merged |
| 23 | HeadSHA string | 23 | // yet. Not a state: see migration 0037. |
| 24 | MergedBase string // target tip at merge time; base for historical diffs | 24 | Draft bool |
| 25 | MergedAt string // "" unless merged | 25 | Milestone string |
| 26 | MergedBy string // "" when unknown (imports) or the account is gone | 26 | HeadSHA string |
| 27 | ClosedAt string // "" unless closed without merging | 27 | MergedBase string // target tip at merge time; base for historical diffs |
| 28 | ClosedBy string | 28 | MergedAt string // "" unless merged |
| 29 | CreatedAt string | 29 | MergedBy string // "" when unknown (imports) or the account is gone |
| 30 | UpdatedAt string | 30 | ClosedAt string // "" unless closed without merging |
| 31 | ClosedBy string | ||
| 32 | CreatedAt string | ||
| 33 | UpdatedAt string | ||
| 31 | } | 34 | } |
| 32 | 35 | ||
| 33 | type MRReview struct { | 36 | type MRReview struct { |
| @@ -38,7 +41,7 @@ type MRReview struct { | |||
| 38 | CreatedAt string | 41 | CreatedAt string |
| 39 | } | 42 | } |
| 40 | 43 | ||
| 41 | func (s *Store) CreateMR(repoID, authorID, sourceRepoID int64, sourceRef, targetRef, title, body, headSHA, format string) (int64, error) { | 44 | func (s *Store) CreateMR(repoID, authorID, sourceRepoID int64, sourceRef, targetRef, title, body, headSHA, format string, draft bool) (int64, error) { |
| 42 | tx, err := s.DB.Begin() | 45 | tx, err := s.DB.Begin() |
| 43 | if err != nil { | 46 | if err != nil { |
| 44 | return 0, err | 47 | return 0, err |
| @@ -52,19 +55,28 @@ func (s *Store) CreateMR(repoID, authorID, sourceRepoID int64, sourceRef, target | |||
| 52 | return 0, err | 55 | return 0, err |
| 53 | } | 56 | } |
| 54 | if _, err := tx.Exec(` | 57 | if _, err := tx.Exec(` |
| 55 | INSERT INTO merge_requests (repo_id, number, author_id, source_repo_id, source_ref, target_ref, title, body, head_sha, body_format) | 58 | INSERT INTO merge_requests (repo_id, number, author_id, source_repo_id, source_ref, target_ref, title, body, head_sha, body_format, draft) |
| 56 | VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | 59 | VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, |
| 57 | repoID, n, authorID, sourceRepoID, sourceRef, targetRef, title, body, headSHA, format); err != nil { | 60 | repoID, n, authorID, sourceRepoID, sourceRef, targetRef, title, body, headSHA, format, draft); err != nil { |
| 58 | return 0, err | 61 | return 0, err |
| 59 | } | 62 | } |
| 60 | return n, tx.Commit() | 63 | return n, tx.Commit() |
| 61 | } | 64 | } |
| 62 | 65 | ||
| 66 | // SetMRDraft marks an open merge request as a draft, or takes the mark | ||
| 67 | // off. Merging is refused while it is set. | ||
| 68 | func (s *Store) SetMRDraft(mrID int64, draft bool) error { | ||
| 69 | _, err := s.DB.Exec( | ||
| 70 | "UPDATE merge_requests SET draft = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?", | ||
| 71 | draft, mrID) | ||
| 72 | return err | ||
| 73 | } | ||
| 74 | |||
| 63 | const mrSelect = ` | 75 | const mrSelect = ` |
| 64 | SELECT m.id, m.repo_id, m.number, u.username, | 76 | SELECT m.id, m.repo_id, m.number, u.username, |
| 65 | COALESCE(m.source_repo_id, 0), | 77 | COALESCE(m.source_repo_id, 0), |
| 66 | COALESCE(COALESCE(su.username, so.name) || '/' || sr.name, ''), | 78 | COALESCE(COALESCE(su.username, so.name) || '/' || sr.name, ''), |
| 67 | m.source_ref, m.target_ref, m.title, m.body, m.body_format, m.state, | 79 | m.source_ref, m.target_ref, m.title, m.body, m.body_format, m.state, m.draft, |
| 68 | COALESCE(ms.title, ''), m.head_sha, | 80 | COALESCE(ms.title, ''), m.head_sha, |
| 69 | m.merged_base, m.merged_at, COALESCE(mu.username, ''), | 81 | m.merged_base, m.merged_at, COALESCE(mu.username, ''), |
| 70 | m.closed_at, COALESCE(cu.username, ''), m.created_at, m.updated_at | 82 | m.closed_at, COALESCE(cu.username, ''), m.created_at, m.updated_at |
| @@ -80,7 +92,7 @@ const mrSelect = ` | |||
| 80 | func scanMR(row interface{ Scan(...any) error }) (MR, error) { | 92 | func scanMR(row interface{ Scan(...any) error }) (MR, error) { |
| 81 | var m MR | 93 | var m MR |
| 82 | err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath, | 94 | err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath, |
| 83 | &m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.BodyFormat, &m.State, &m.Milestone, &m.HeadSHA, &m.MergedBase, | 95 | &m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.BodyFormat, &m.State, &m.Draft, &m.Milestone, &m.HeadSHA, &m.MergedBase, |
| 84 | &m.MergedAt, &m.MergedBy, &m.ClosedAt, &m.ClosedBy, &m.CreatedAt, &m.UpdatedAt) | 96 | &m.MergedAt, &m.MergedBy, &m.ClosedAt, &m.ClosedBy, &m.CreatedAt, &m.UpdatedAt) |
| 85 | return m, err | 97 | return m, err |
| 86 | } | 98 | } |
internal/store/mrs_test.go +1 −1
| @@ -16,7 +16,7 @@ func mrFixture(t *testing.T) (*Store, int64, int64) { | |||
| 16 | if err != nil { | 16 | if err != nil { |
| 17 | t.Fatal(err) | 17 | t.Fatal(err) |
| 18 | } | 18 | } |
| 19 | if _, err := s.CreateMR(repoID, uid, repoID, "feature", "main", "t", "", "abc123", "md"); err != nil { | 19 | if _, err := s.CreateMR(repoID, uid, repoID, "feature", "main", "t", "", "abc123", "md", false); err != nil { |
| 20 | t.Fatal(err) | 20 | t.Fatal(err) |
| 21 | } | 21 | } |
| 22 | return s, repoID, uid | 22 | return s, repoID, uid |
internal/web/templates/mr.html +5 −1
| @@ -3,7 +3,7 @@ | |||
| 3 | {{define "content"}} | 3 | {{define "content"}} |
| 4 | {{$base := printf "/%s/%s/mrs/%d" .Repo.OwnerName .Repo.Name .MR.Number}} | 4 | {{$base := printf "/%s/%s/mrs/%d" .Repo.OwnerName .Repo.Name .MR.Number}} |
| 5 | <h1 class="issuetitle">{{.MR.Title}} <span class="issuenumber">!{{.MR.Number}}</span></h1> | 5 | <h1 class="issuetitle">{{.MR.Title}} <span class="issuenumber">!{{.MR.Number}}</span></h1> |
| 6 | <p class="issuemeta"><span class="chip chip-{{.MR.State}}">{{.MR.State}}</span> | 6 | <p class="issuemeta">{{if .MR.Draft}}<span class="chip chip-neutral">draft</span> {{end}}<span class="chip chip-{{.MR.State}}">{{.MR.State}}</span> |
| 7 | {{if and (eq .MR.State "merged") .MR.MergedAt}} | 7 | {{if and (eq .MR.State "merged") .MR.MergedAt}} |
| 8 | {{if .MR.MergedBy}}<a href="/{{.MR.MergedBy}}">{{.MR.MergedBy}}</a> merged{{else}}Merged{{end}} | 8 | {{if .MR.MergedBy}}<a href="/{{.MR.MergedBy}}">{{.MR.MergedBy}}</a> merged{{else}}Merged{{end}} |
| 9 | {{template "mrrange" .MR}} on {{when .MR.MergedAt}} | 9 | {{template "mrrange" .MR}} on {{when .MR.MergedAt}} |
| @@ -107,6 +107,10 @@ | |||
| 107 | <form method="post" action="{{$base}}/close" class="actions"> | 107 | <form method="post" action="{{$base}}/close" class="actions"> |
| 108 | <button type="submit">Close without merging</button> | 108 | <button type="submit">Close without merging</button> |
| 109 | </form> | 109 | </form> |
| 110 | <form method="post" action="{{$base}}/draft" class="actions"> | ||
| 111 | {{if .MR.Draft}}<button type="submit">Ready for review</button> | ||
| 112 | {{else}}<input type="hidden" name="draft" value="on"><button type="submit">Convert to draft</button>{{end}} | ||
| 113 | </form> | ||
| 110 | </div> | 114 | </div> |
| 111 | {{end}} | 115 | {{end}} |
| 112 | {{if and .CanEdit (or (eq .MR.State "open") (eq .MR.State "source_gone"))}} | 116 | {{if and .CanEdit (or (eq .MR.State "open") (eq .MR.State "source_gone"))}} |
internal/web/templates/mrs.html +1 −1
| @@ -21,7 +21,7 @@ | |||
| 21 | <p class="title"><a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{.Number}}">{{.Title}}</a></p> | 21 | <p class="title"><a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{.Number}}">{{.Title}}</a></p> |
| 22 | <p class="meta">!{{.Number}} by <a href="/{{.Author}}">{{.Author}}</a> · {{if .SourcePath}}{{.SourcePath}}:{{end}}{{.SourceRef}} → {{.TargetRef}}</p> | 22 | <p class="meta">!{{.Number}} by <a href="/{{.Author}}">{{.Author}}</a> · {{if .SourcePath}}{{.SourcePath}}:{{end}}{{.SourceRef}} → {{.TargetRef}}</p> |
| 23 | </div> | 23 | </div> |
| 24 | <span class="chip chip-{{.State}}">{{.State}}</span> | 24 | {{if .Draft}}<span class="chip chip-neutral">draft</span> {{end}}<span class="chip chip-{{.State}}">{{.State}}</span> |
| 25 | </li> | 25 | </li> |
| 26 | {{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li> | 26 | {{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li> |
| 27 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}}{{end}} | 27 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}}{{end}} |