Commit f7a4bb91fd

f7a4bb91fdd6909f1b3f483eab2e0f0fac1f5f76

parent: fcf2ea4dd4

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-28 22:10 UTC

web: new-issue form takes milestone and assignee, one dispatch

Ref #271

Layout: unified · split

internal/httpd/accounts.go +17 −7
@@ -420,8 +420,9 @@ func (s *Server) issueCreateForm(w http.ResponseWriter, r *http.Request, u store
420420 Template string
421421 Templates []control.IssueTemplate
422422 Draft *draft
423 CanWrite bool
423424 }{p, d.Body, d.Format, r.FormValue("title"), r.FormValue("labels"),
424 "", control.IssueTemplates(p.Dir, p.Repo.DefaultBranch), d})
425 "", control.IssueTemplates(p.Dir, p.Repo.DefaultBranch), d, s.canWriteRepoAs(u, p.Repo)})
425426 return
426427 }
427428 templates := control.IssueTemplates(p.Dir, p.Repo.DefaultBranch)
@@ -455,7 +456,8 @@ func (s *Server) issueCreateForm(w http.ResponseWriter, r *http.Request, u store
455456 Template string
456457 Templates []control.IssueTemplate
457458 Draft *draft
458 }{p, body, format, "", "", tplName, templates, nil})
459 CanWrite bool
460 }{p, body, format, "", "", tplName, templates, nil, s.canWriteRepoAs(u, p.Repo)})
459461}
460462
461463// Issue and merge request writes run the command the CLI runs, so the
@@ -472,17 +474,25 @@ func (s *Server) issueCreateSubmit(w http.ResponseWriter, r *http.Request, u sto
472474 }
473475 var created control.Created
474476 argv := []string{"issue", "create", repoPath, "--title", title, "--format", format, "--file", "-"}
477 // Labels, milestone and assignee go on the same dispatch issue create
478 // itself resolves and applies: a typo in any of them creates nothing,
479 // and the label/milestone/assign code paths run so notifications and
480 // events happen (#271). issue create refuses the whole create when any
481 // of them is set without write access, so a reader's hand-crafted POST
482 // carrying one is dropped here rather than failing the create.
483 if repo, err := s.st.RepoByPath(repoPath); err == nil && s.canWriteRepoAs(u, repo) {
484 argv = append(argv, fieldArgs("--label", r.FormValue("labels"))...)
485 if milestone := strings.TrimSpace(r.FormValue("milestone")); milestone != "" {
486 argv = append(argv, "--milestone", milestone)
487 }
488 argv = append(argv, fieldArgs("--assignee", r.FormValue("assignee"))...)
489 }
475490 code, msg := s.dispatchIntoStdin(u, argv, r.FormValue("body"), &created)
476491 if code != protocol.ExitOK {
477492 http.Error(w, msg, statusForExit(code))
478493 return
479494 }
480495 n := created.Number
481 // Labels need write access, matching the SSH rule; the command refuses
482 // otherwise and the issue stands without them.
483 if args := fieldArgs("--add", r.FormValue("labels")); len(args) > 0 {
484 s.runControl(u, append([]string{"issue", "label", repoPath, fmt.Sprint(n)}, args...))
485 }
486496 http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repoPath, n), http.StatusSeeOther)
487497}
488498
internal/httpd/issuecreate_test.go added +306
@@ -0,0 +1,306 @@
1package httpd
2
3import (
4 "net/http"
5 "net/http/httptest"
6 "net/url"
7 "strings"
8 "testing"
9 "time"
10
11 "gitbay.org/gitbay/internal/config"
12 "gitbay.org/gitbay/internal/store"
13)
14
15// sessionCookieFor gives uid a real web session, the way canWriteRepo's
16// call to s.viewer(r) needs (internal/httpd/accounts.go:37-47), since
17// issueCreateForm gates the milestone/assignee fields on it rather than
18// on the handler's own user parameter.
19func sessionCookieFor(t *testing.T, s *Server, st *store.Store, uid int64) *http.Cookie {
20 t.Helper()
21 tok, hash, err := store.NewToken()
22 if err != nil {
23 t.Fatal(err)
24 }
25 if err := st.CreateWebSession(hash, uid, time.Hour); err != nil {
26 t.Fatal(err)
27 }
28 return s.sessionCookieFor(tok)
29}
30
31// The new-issue form takes milestone and assignee, resolved on the same
32// issue create dispatch as the title and labels, so a typo in either
33// creates nothing and the label/milestone/assign code paths still run
34// notifications and events (#271).
35func TestIssueCreateFormHasMilestoneAndAssigneeForWriter(t *testing.T) {
36 st, err := store.Open(":memory:")
37 if err != nil {
38 t.Fatal(err)
39 }
40 defer st.Close()
41 if err := st.MigrateUp(); err != nil {
42 t.Fatal(err)
43 }
44 uid, err := st.CreateUser("alice", false)
45 if err != nil {
46 t.Fatal(err)
47 }
48 u := store.User{ID: uid, Username: "alice"}
49 if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil {
50 t.Fatal(err)
51 }
52
53 cfg := config.Default()
54 cfg.Web.Mode = "accounts"
55 s := New(cfg, st)
56 req := httptest.NewRequest("GET", "/alice/app/issues/new", nil)
57 req.SetPathValue("owner", "alice")
58 req.SetPathValue("repo", "app")
59 req.AddCookie(sessionCookieFor(t, s, st, uid))
60 rr := httptest.NewRecorder()
61 s.issueCreateForm(rr, req, u)
62
63 body := rr.Body.String()
64 if !strings.Contains(body, `name="labels"`) {
65 t.Error("no labels field for a writer")
66 }
67 if !strings.Contains(body, `name="milestone"`) {
68 t.Error("no milestone field for a writer")
69 }
70 if !strings.Contains(body, `name="assignee"`) {
71 t.Error("no assignee field for a writer")
72 }
73}
74
75// A reader (no write access) sees no milestone/assignee fields, and can
76// still create an issue with title and body alone.
77func TestIssueCreateFormHidesMilestoneAndAssigneeForReader(t *testing.T) {
78 st, err := store.Open(":memory:")
79 if err != nil {
80 t.Fatal(err)
81 }
82 defer st.Close()
83 if err := st.MigrateUp(); err != nil {
84 t.Fatal(err)
85 }
86 ownerID, err := st.CreateUser("alice", false)
87 if err != nil {
88 t.Fatal(err)
89 }
90 readerID, err := st.CreateUser("bob", false)
91 if err != nil {
92 t.Fatal(err)
93 }
94 reader := store.User{ID: readerID, Username: "bob"}
95 if _, err := st.CreateRepo("user", ownerID, "app", "public"); err != nil {
96 t.Fatal(err)
97 }
98
99 cfg := config.Default()
100 cfg.Web.Mode = "accounts"
101 s := New(cfg, st)
102 req := httptest.NewRequest("GET", "/alice/app/issues/new", nil)
103 req.SetPathValue("owner", "alice")
104 req.SetPathValue("repo", "app")
105 req.AddCookie(sessionCookieFor(t, s, st, readerID))
106 rr := httptest.NewRecorder()
107 s.issueCreateForm(rr, req, reader)
108
109 body := rr.Body.String()
110 if strings.Contains(body, `name="labels"`) {
111 t.Error("reader should not see a labels field")
112 }
113 if strings.Contains(body, `name="milestone"`) {
114 t.Error("reader should not see a milestone field")
115 }
116 if strings.Contains(body, `name="assignee"`) {
117 t.Error("reader should not see an assignee field")
118 }
119
120 form := url.Values{"title": {"a bug"}, "body": {"steps"}}
121 submit := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode()))
122 submit.Header.Set("Content-Type", "application/x-www-form-urlencoded")
123 submit.SetPathValue("owner", "alice")
124 submit.SetPathValue("repo", "app")
125 rr2 := httptest.NewRecorder()
126 s.issueCreateSubmit(rr2, submit, reader)
127 if rr2.Code != http.StatusSeeOther {
128 t.Fatalf("reader create: status %d, body %q", rr2.Code, rr2.Body.String())
129 }
130
131 repo, err := st.RepoByPath("alice/app")
132 if err != nil {
133 t.Fatal(err)
134 }
135 issue, err := st.IssueByNumber(repo.ID, 1)
136 if err != nil {
137 t.Fatalf("issue not created: %v", err)
138 }
139 if issue.Title != "a bug" {
140 t.Fatalf("got title %q", issue.Title)
141 }
142}
143
144// A reader's hand-crafted POST carrying a labels value still creates a
145// plain issue: issue create requires write access for --label, so
146// issueCreateSubmit drops labels/milestone/assignee from the argv for a
147// non-writer rather than sending them and failing the whole create.
148func TestIssueCreateSubmitReaderLabelIsDropped(t *testing.T) {
149 st, err := store.Open(":memory:")
150 if err != nil {
151 t.Fatal(err)
152 }
153 defer st.Close()
154 if err := st.MigrateUp(); err != nil {
155 t.Fatal(err)
156 }
157 ownerID, err := st.CreateUser("alice", false)
158 if err != nil {
159 t.Fatal(err)
160 }
161 readerID, err := st.CreateUser("bob", false)
162 if err != nil {
163 t.Fatal(err)
164 }
165 reader := store.User{ID: readerID, Username: "bob"}
166 if _, err := st.CreateRepo("user", ownerID, "app", "public"); err != nil {
167 t.Fatal(err)
168 }
169 repo, err := st.RepoByPath("alice/app")
170 if err != nil {
171 t.Fatal(err)
172 }
173
174 s := New(config.Default(), st)
175 form := url.Values{
176 "title": {"a bug"},
177 "body": {"steps"},
178 "labels": {"bug"},
179 }
180 req := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode()))
181 req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
182 req.SetPathValue("owner", "alice")
183 req.SetPathValue("repo", "app")
184 rr := httptest.NewRecorder()
185 s.issueCreateSubmit(rr, req, reader)
186 if rr.Code != http.StatusSeeOther {
187 t.Fatalf("reader create: status %d, body %q", rr.Code, rr.Body.String())
188 }
189
190 issue, err := st.IssueByNumber(repo.ID, 1)
191 if err != nil {
192 t.Fatalf("issue not created: %v", err)
193 }
194 if len(issue.Labels) != 0 {
195 t.Errorf("labels = %v, want none", issue.Labels)
196 }
197}
198
199// A writer creates an issue with a milestone and an assignee in one
200// request; both land on the issue because they go through the same
201// dispatch as the create.
202func TestIssueCreateSubmitSetsMilestoneAndAssignee(t *testing.T) {
203 st, err := store.Open(":memory:")
204 if err != nil {
205 t.Fatal(err)
206 }
207 defer st.Close()
208 if err := st.MigrateUp(); err != nil {
209 t.Fatal(err)
210 }
211 uid, err := st.CreateUser("alice", false)
212 if err != nil {
213 t.Fatal(err)
214 }
215 u := store.User{ID: uid, Username: "alice"}
216 if _, err := st.CreateUser("bob", false); err != nil {
217 t.Fatal(err)
218 }
219 if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil {
220 t.Fatal(err)
221 }
222 repo, err := st.RepoByPath("alice/app")
223 if err != nil {
224 t.Fatal(err)
225 }
226 if _, err := st.CreateMilestone(repo, "v1", "", ""); err != nil {
227 t.Fatal(err)
228 }
229
230 s := New(config.Default(), st)
231 form := url.Values{
232 "title": {"needs a fix"},
233 "body": {"details"},
234 "milestone": {"v1"},
235 "assignee": {"bob"},
236 }
237 req := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode()))
238 req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
239 req.SetPathValue("owner", "alice")
240 req.SetPathValue("repo", "app")
241 rr := httptest.NewRecorder()
242 s.issueCreateSubmit(rr, req, u)
243 if rr.Code != http.StatusSeeOther {
244 t.Fatalf("status %d, body %q", rr.Code, rr.Body.String())
245 }
246
247 issue, err := st.IssueByNumber(repo.ID, 1)
248 if err != nil {
249 t.Fatalf("issue not created: %v", err)
250 }
251 if issue.Milestone != "v1" {
252 t.Errorf("milestone = %q, want v1", issue.Milestone)
253 }
254 if len(issue.Assignees) != 1 || issue.Assignees[0] != "bob" {
255 t.Errorf("assignees = %v, want [bob]", issue.Assignees)
256 }
257}
258
259// A bad assignee creates nothing: issue create resolves the assignee
260// before writing the issue, so a typo leaves the repo without a
261// half-created issue (#271).
262func TestIssueCreateSubmitBadAssigneeCreatesNothing(t *testing.T) {
263 st, err := store.Open(":memory:")
264 if err != nil {
265 t.Fatal(err)
266 }
267 defer st.Close()
268 if err := st.MigrateUp(); err != nil {
269 t.Fatal(err)
270 }
271 uid, err := st.CreateUser("alice", false)
272 if err != nil {
273 t.Fatal(err)
274 }
275 u := store.User{ID: uid, Username: "alice"}
276 if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil {
277 t.Fatal(err)
278 }
279 repo, err := st.RepoByPath("alice/app")
280 if err != nil {
281 t.Fatal(err)
282 }
283
284 s := New(config.Default(), st)
285 form := url.Values{
286 "title": {"needs a fix"},
287 "body": {"details"},
288 "assignee": {"nobody-such-user"},
289 }
290 req := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode()))
291 req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
292 req.SetPathValue("owner", "alice")
293 req.SetPathValue("repo", "app")
294 rr := httptest.NewRecorder()
295 s.issueCreateSubmit(rr, req, u)
296 if rr.Code == http.StatusSeeOther {
297 t.Fatalf("expected a failure status, got redirect")
298 }
299 if !strings.Contains(rr.Body.String(), "nobody-such-user") {
300 t.Errorf("error body %q does not name the bad assignee", rr.Body.String())
301 }
302
303 if _, err := st.IssueByNumber(repo.ID, 1); err == nil {
304 t.Fatal("issue was created despite the bad assignee")
305 }
306}
internal/httpd/web.go +7
@@ -2003,6 +2003,13 @@ func (s *Server) canWriteRepo(r *http.Request, repo store.Repo) bool {
20032003 if u.ID == 0 {
20042004 return false
20052005 }
2006 return s.canWriteRepoAs(u, repo)
2007}
2008
2009// canWriteRepoAs is canWriteRepo for a handler that already has its
2010// viewer as a parameter (behind requireUser) rather than needing to
2011// resolve one from the request's session cookie.
2012func (s *Server) canWriteRepoAs(u store.User, repo store.Repo) bool {
20062013 grant, _ := s.st.AccessRole(repo.ID, u.ID)
20072014 return policy.CanWrite(u, repo, grant)
20082015}
internal/web/templates/issuenew.html +5 −1
@@ -9,7 +9,11 @@
99<p><input type="text" name="title" aria-label="Title" placeholder="title" value="{{.Title}}" required></p>
1010<p><textarea name="body" aria-label="Description" rows="12">{{.Body}}</textarea></p>
1111{{template "formatpicker" .Format}}
12<p><input type="text" name="labels" aria-label="Labels" placeholder="labels, space-separated (write access)" value="{{.Labels}}"></p>
12{{if .CanWrite}}
13<p><input type="text" name="labels" aria-label="Labels" placeholder="labels, space-separated (optional)" value="{{.Labels}}"></p>
14<p><input type="text" name="milestone" aria-label="Milestone" placeholder="milestone (optional)"></p>
15<p><input type="text" name="assignee" aria-label="Assignee" placeholder="assignees, space-separated (optional)"></p>
16{{end}}
1317<p><button type="submit">Open issue</button> {{template "previewbtn"}}</p>
1418</form>
1519{{end}}