Commit 4dbe61f2db
Verified · cmc ci/build: success
e2e/mrweb_test.go +59
| @@ -155,3 +155,62 @@ func TestMRWebReviewLoop(t *testing.T) { | ||
| 155 | 155 | t.Fatalf("reader was not refused:\n%s", denied) |
| 156 | 156 | } |
| 157 | 157 | } |
| 158 | ||
| 159 | // TestMRWebCreate opens a merge request from the browser and checks the | |
| 160 | // form survives a refusal with the draft intact. | |
| 161 | func TestMRWebCreate(t *testing.T) { | |
| 162 | inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n") | |
| 163 | aliceKey := inst.newKey(t, "alice") | |
| 164 | inst.admin(t, "admin", "user", "create", "alice", | |
| 165 | "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified") | |
| 166 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 { | |
| 167 | t.Fatalf("repo create: %s", errOut) | |
| 168 | } | |
| 169 | env := inst.gitEnv(aliceKey) | |
| 170 | work := t.TempDir() | |
| 171 | mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w") | |
| 172 | dir := filepath.Join(work, "w") | |
| 173 | os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644) | |
| 174 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | |
| 175 | mustGit(t, dir, env, "add", ".") | |
| 176 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | |
| 177 | mustGit(t, dir, env, "push", "-q", "origin", "main") | |
| 178 | mustGit(t, dir, env, "checkout", "-q", "-b", "topic") | |
| 179 | os.WriteFile(filepath.Join(dir, "b.txt"), []byte("b\n"), 0o644) | |
| 180 | mustGit(t, dir, env, "add", ".") | |
| 181 | mustGit(t, dir, env, "commit", "-q", "-m", "topic work") | |
| 182 | mustGit(t, dir, env, "push", "-q", "origin", "topic") | |
| 183 | ||
| 184 | alice := inst.login(t, aliceKey) | |
| 185 | base := inst.base() + "/alice/lib" | |
| 186 | ||
| 187 | // The list links to the form, and the form offers the pushed branches. | |
| 188 | if _, body := browserGet(t, alice, base+"/mrs"); !strings.Contains(body, "/alice/lib/mrs/new") { | |
| 189 | t.Fatalf("no create link on the list:\n%s", body) | |
| 190 | } | |
| 191 | _, form := browserGet(t, alice, base+"/mrs/new") | |
| 192 | for _, want := range []string{`name="source"`, `value="topic"`, `value="main"`} { | |
| 193 | if !strings.Contains(form, want) { | |
| 194 | t.Fatalf("form missing %q:\n%s", want, form) | |
| 195 | } | |
| 196 | } | |
| 197 | ||
| 198 | // A refusal keeps the draft: the branch does not exist. | |
| 199 | _, retry := browserPost(t, alice, base+"/mrs/new", url.Values{ | |
| 200 | "source": {"nope"}, "target": {"main"}, "title": {"my title"}, "body": {"my body"}}) | |
| 201 | if !strings.Contains(retry, `class="error"`) || !strings.Contains(retry, "my title") || | |
| 202 | !strings.Contains(retry, "my body") { | |
| 203 | t.Fatalf("refusal lost the draft:\n%s", retry) | |
| 204 | } | |
| 205 | ||
| 206 | // A real one lands on the merge request it created. | |
| 207 | status, created := browserPost(t, alice, base+"/mrs/new", url.Values{ | |
| 208 | "source": {"topic"}, "target": {"main"}, "title": {"topic into main"}, "body": {"please review"}}) | |
| 209 | if status != 200 || !strings.Contains(created, "topic into main") { | |
| 210 | t.Fatalf("create failed: %d\n%s", status, created) | |
| 211 | } | |
| 212 | show := inst.mrShow(t, aliceKey, "alice/lib", "1") | |
| 213 | if show.State != "open" || show.Source != "topic" { | |
| 214 | t.Fatalf("created MR wrong: %+v", show) | |
| 215 | } | |
| 216 | } | |
internal/httpd/control.go +37
| @@ -2,6 +2,7 @@ package httpd | ||
| 2 | 2 | |
| 3 | 3 | import ( |
| 4 | 4 | "bytes" |
| 5 | "encoding/json" | |
| 5 | 6 | "strings" |
| 6 | 7 | |
| 7 | 8 | "gitbay.org/gitbay/internal/control" |
| @@ -36,3 +37,39 @@ func (s *Server) runControl(u store.User, argv []string) (out string, msg string | ||
| 36 | 37 | } |
| 37 | 38 | return stdout.String(), m, code == protocol.ExitOK |
| 38 | 39 | } |
| 40 | ||
| 41 | // runControlJSON runs a command in JSON mode and returns its data object. | |
| 42 | // In JSON mode a failure is an envelope carrying the message rather than | |
| 43 | // stderr text, so both paths are read from the same envelope. | |
| 44 | func (s *Server) runControlJSON(u store.User, argv []string) (data map[string]any, msg string, ok bool) { | |
| 45 | var stdout, stderr bytes.Buffer | |
| 46 | ctx := &control.Ctx{ | |
| 47 | User: u, | |
| 48 | Source: "web", | |
| 49 | Scope: "full", | |
| 50 | Store: s.st, | |
| 51 | Cfg: s.cfg, | |
| 52 | Stdin: strings.NewReader(""), | |
| 53 | Stdout: &stdout, | |
| 54 | Stderr: &stderr, | |
| 55 | JSON: true, | |
| 56 | ViaAPI: true, | |
| 57 | } | |
| 58 | code := control.Dispatch(ctx, argv) | |
| 59 | var env struct { | |
| 60 | Data map[string]any `json:"data"` | |
| 61 | Error string `json:"error"` | |
| 62 | } | |
| 63 | json.Unmarshal(stdout.Bytes(), &env) | |
| 64 | if code != protocol.ExitOK { | |
| 65 | m := env.Error | |
| 66 | if m == "" { | |
| 67 | m = strings.TrimSpace(stderr.String()) | |
| 68 | } | |
| 69 | if m == "" { | |
| 70 | m = "the command failed" | |
| 71 | } | |
| 72 | return nil, m, false | |
| 73 | } | |
| 74 | return env.Data, "", true | |
| 75 | } | |
internal/httpd/mractions.go +66
| @@ -7,6 +7,7 @@ import ( | ||
| 7 | 7 | "strconv" |
| 8 | 8 | "strings" |
| 9 | 9 | |
| 10 | "gitbay.org/gitbay/internal/gitutil" | |
| 10 | 11 | "gitbay.org/gitbay/internal/store" |
| 11 | 12 | ) |
| 12 | 13 | |
| @@ -89,3 +90,68 @@ func (s *Server) mrThreadSubmit(w http.ResponseWriter, r *http.Request, u store. | ||
| 89 | 90 | } |
| 90 | 91 | s.mrRedirect(w, r, msg) |
| 91 | 92 | } |
| 93 | ||
| 94 | // mrNewPage is the create form: branches to choose from, plus whatever | |
| 95 | // the last attempt had in it so a refusal does not lose the draft. | |
| 96 | type mrNewPage struct { | |
| 97 | repoPage | |
| 98 | Branches []gitutil.Ref | |
| 99 | Source string | |
| 100 | Target string | |
| 101 | Title string | |
| 102 | Body string | |
| 103 | Notice string | |
| 104 | } | |
| 105 | ||
| 106 | func (s *Server) mrCreateForm(w http.ResponseWriter, r *http.Request, u store.User) { | |
| 107 | p, ok := s.repoFor(w, r, "") | |
| 108 | if !ok { | |
| 109 | return | |
| 110 | } | |
| 111 | p.Tab = "merge requests" | |
| 112 | branches, _ := gitutil.Refs(p.Dir, "heads") | |
| 113 | q := r.URL.Query() | |
| 114 | target := q.Get("target") | |
| 115 | if target == "" { | |
| 116 | target = p.Repo.DefaultBranch | |
| 117 | } | |
| 118 | s.render(w, "mrnew.html", mrNewPage{ | |
| 119 | repoPage: p, Branches: branches, | |
| 120 | Source: q.Get("source"), Target: target, | |
| 121 | Title: q.Get("title"), Body: q.Get("body"), Notice: q.Get("e"), | |
| 122 | }) | |
| 123 | } | |
| 124 | ||
| 125 | func (s *Server) mrCreateSubmit(w http.ResponseWriter, r *http.Request, u store.User) { | |
| 126 | p, ok := s.repoFor(w, r, "") | |
| 127 | if !ok { | |
| 128 | return | |
| 129 | } | |
| 130 | source := strings.TrimSpace(r.FormValue("source")) | |
| 131 | target := strings.TrimSpace(r.FormValue("target")) | |
| 132 | title := strings.TrimSpace(r.FormValue("title")) | |
| 133 | body := strings.TrimSpace(r.FormValue("body")) | |
| 134 | ||
| 135 | back := func(msg string) { | |
| 136 | q := url.Values{"source": {source}, "target": {target}, "title": {title}, "body": {body}, "e": {msg}} | |
| 137 | http.Redirect(w, r, fmt.Sprintf("/%s/mrs/new?%s", p.Repo.Path(), q.Encode()), http.StatusSeeOther) | |
| 138 | } | |
| 139 | if source == "" || title == "" { | |
| 140 | back("pick a source branch and give the merge request a title") | |
| 141 | return | |
| 142 | } | |
| 143 | argv := []string{"mr", "create", p.Repo.Path(), "--source", source, "--title", title} | |
| 144 | if target != "" { | |
| 145 | argv = append(argv, "--target", target) | |
| 146 | } | |
| 147 | if body != "" { | |
| 148 | argv = append(argv, "--body", body) | |
| 149 | } | |
| 150 | data, msg, ok := s.runControlJSON(u, argv) | |
| 151 | if !ok { | |
| 152 | back(msg) | |
| 153 | return | |
| 154 | } | |
| 155 | n, _ := data["number"].(float64) | |
| 156 | http.Redirect(w, r, fmt.Sprintf("/%s/mrs/%d", p.Repo.Path(), int64(n)), http.StatusSeeOther) | |
| 157 | } | |
internal/httpd/routes.go +4
| @@ -110,6 +110,10 @@ func (s *Server) Routes() []Route { | ||
| 110 | 110 | Handler: s.checkOrigin(s.requireUser(s.issueCommentSubmit))}, |
| 111 | 111 | Route{Method: "POST", Pattern: "/{owner}/{repo}/issues/{n}/edit", Mutating: true, |
| 112 | 112 | Handler: s.checkOrigin(s.requireUser(s.issueEditSubmit))}, |
| 113 | Route{Method: "GET", Pattern: "/{owner}/{repo}/mrs/new", | |
| 114 | Handler: s.requireUser(s.mrCreateForm)}, | |
| 115 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/new", Mutating: true, | |
| 116 | Handler: s.checkOrigin(s.requireUser(s.mrCreateSubmit))}, | |
| 113 | 117 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/edit", Mutating: true, |
| 114 | 118 | Handler: s.checkOrigin(s.requireUser(s.mrEditSubmit))}, |
| 115 | 119 | Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/comment", Mutating: true, |
internal/web/static/style.css +10
| @@ -540,6 +540,16 @@ code.fullsha { color: var(--muted); overflow-wrap: anywhere; } | ||
| 540 | 540 | padding: var(--sp-2) var(--sp-3); |
| 541 | 541 | margin-bottom: var(--sp-4); |
| 542 | 542 | } |
| 543 | /* branch picker on the new merge request form */ | |
| 544 | p.branchpick { | |
| 545 | display: flex; | |
| 546 | flex-wrap: wrap; | |
| 547 | align-items: center; | |
| 548 | gap: var(--sp-2); | |
| 549 | } | |
| 550 | p.branchpick label { color: var(--muted); font-size: var(--fs-2); } | |
| 551 | p.branchpick select { flex: 1 1 12rem; } | |
| 552 | ||
| 543 | 553 | /* merge request actions in the aside: stacked controls, full width */ |
| 544 | 554 | .aside form.actions { |
| 545 | 555 | display: flex; |
internal/web/templates/mrnew.html added +23
| @@ -0,0 +1,23 @@ | ||
| 1 | {{define "title"}}new merge request · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}} | |
| 2 | {{define "content"}} | |
| 3 | <h1>New merge request</h1> | |
| 4 | {{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}} | |
| 5 | <form method="post" action="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new" class="commentform"> | |
| 6 | <p class="branchpick"> | |
| 7 | <label for="source">Merge</label> | |
| 8 | <select id="source" name="source" required> | |
| 9 | <option value="">Choose a branch…</option> | |
| 10 | {{range .Branches}}<option value="{{.Name}}"{{if eq .Name $.Source}} selected{{end}}>{{.Name}}</option>{{end}} | |
| 11 | </select> | |
| 12 | <label for="target">into</label> | |
| 13 | <select id="target" name="target"> | |
| 14 | {{range .Branches}}<option value="{{.Name}}"{{if eq .Name $.Target}} selected{{end}}>{{.Name}}</option>{{end}} | |
| 15 | </select> | |
| 16 | </p> | |
| 17 | <p><input type="text" name="title" aria-label="Title" placeholder="title" value="{{.Title}}" required></p> | |
| 18 | <p><textarea name="body" aria-label="Description" rows="10" placeholder="what changes, and why">{{.Body}}</textarea></p> | |
| 19 | <p><button type="submit" class="primary">Open merge request</button></p> | |
| 20 | </form> | |
| 21 | <p class="meta">A branch in a fork opens from the CLI: | |
| 22 | <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source owner/fork:branch --target {{.Repo.DefaultBranch}} --title "…"</code></p> | |
| 23 | {{end}} | |
internal/web/templates/mrs.html +1
| @@ -9,6 +9,7 @@ | ||
| 9 | 9 | <a {{if eq .State "all"}}class="active" {{end}}href="?state=all">all</a> |
| 10 | 10 | </nav> |
| 11 | 11 | </div> |
| 12 | {{if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p>{{end}} | |
| 12 | 13 | <ul class="issuelist"> |
| 13 | 14 | {{range .MRs}}<li> |
| 14 | 15 | <div class="issuemain"> |