Commit d906ddff2e
Verified · cmc
Layout: unified · split
internal/httpd/accounts.go +40 −26
| @@ -399,6 +399,24 @@ func (s *Server) signupSubmit(w http.ResponseWriter, r *http.Request) { | ||
| 399 | 399 | }{basePage{Site: s.siteName(), Host: s.cfg.SiteHost()}, username, msg, s.cfg.SiteHost()}) |
| 400 | 400 | } |
| 401 | 401 | |
| 402 | // issueNewPage is what the new-issue form renders with, whether that is a | |
| 403 | // fresh form, a Preview round trip, or a refused create — each keeps | |
| 404 | // whatever the visitor typed (#271). | |
| 405 | type issueNewPage struct { | |
| 406 | repoPage | |
| 407 | Body string | |
| 408 | Format string | |
| 409 | Title string | |
| 410 | Labels string | |
| 411 | Milestone string | |
| 412 | Assignee string | |
| 413 | Template string | |
| 414 | Templates []control.IssueTemplate | |
| 415 | Draft *draft | |
| 416 | CanWrite bool | |
| 417 | Notice string | |
| 418 | } | |
| 419 | ||
| 402 | 420 | // issueCreateForm renders the new-issue form, prefilled from the repo's |
| 403 | 421 | // default issue template when one exists. A Preview submit comes back |
| 404 | 422 | // here with the draft in the form, so the page returns with everything |
| @@ -411,18 +429,11 @@ func (s *Server) issueCreateForm(w http.ResponseWriter, r *http.Request, u store | ||
| 411 | 429 | p.Tab = "issues" |
| 412 | 430 | if wantsPreview(r) { |
| 413 | 431 | d := s.draftFor(r, p.Repo, "body", "body", bodyFormat(r)) |
| 414 | s.render(w, "issuenew.html", struct { | |
| 415 | repoPage | |
| 416 | Body string | |
| 417 | Format string | |
| 418 | Title string | |
| 419 | Labels string | |
| 420 | Template string | |
| 421 | Templates []control.IssueTemplate | |
| 422 | Draft *draft | |
| 423 | CanWrite bool | |
| 424 | }{p, d.Body, d.Format, r.FormValue("title"), r.FormValue("labels"), | |
| 425 | "", control.IssueTemplates(p.Dir, p.Repo.DefaultBranch), d, s.canWriteRepoAs(u, p.Repo)}) | |
| 432 | s.render(w, "issuenew.html", issueNewPage{ | |
| 433 | repoPage: p, Body: d.Body, Format: d.Format, Title: r.FormValue("title"), | |
| 434 | Labels: r.FormValue("labels"), Milestone: r.FormValue("milestone"), Assignee: r.FormValue("assignee"), | |
| 435 | Templates: control.IssueTemplates(p.Dir, p.Repo.DefaultBranch), Draft: d, CanWrite: s.canWriteRepoAs(u, p.Repo), | |
| 436 | }) | |
| 426 | 437 | return |
| 427 | 438 | } |
| 428 | 439 | templates := control.IssueTemplates(p.Dir, p.Repo.DefaultBranch) |
| @@ -447,17 +458,10 @@ func (s *Server) issueCreateForm(w http.ResponseWriter, r *http.Request, u store | ||
| 447 | 458 | if format != "org" { |
| 448 | 459 | format = "md" |
| 449 | 460 | } |
| 450 | s.render(w, "issuenew.html", struct { | |
| 451 | repoPage | |
| 452 | Body string | |
| 453 | Format string | |
| 454 | Title string | |
| 455 | Labels string | |
| 456 | Template string | |
| 457 | Templates []control.IssueTemplate | |
| 458 | Draft *draft | |
| 459 | CanWrite bool | |
| 460 | }{p, body, format, "", "", tplName, templates, nil, s.canWriteRepoAs(u, p.Repo)}) | |
| 461 | s.render(w, "issuenew.html", issueNewPage{ | |
| 462 | repoPage: p, Body: body, Format: format, Template: tplName, Templates: templates, | |
| 463 | CanWrite: s.canWriteRepoAs(u, p.Repo), | |
| 464 | }) | |
| 461 | 465 | } |
| 462 | 466 | |
| 463 | 467 | // Issue and merge request writes run the command the CLI runs, so the |
| @@ -465,13 +469,18 @@ func (s *Server) issueCreateForm(w http.ResponseWriter, r *http.Request, u store | ||
| 465 | 469 | // implementation. Bodies travel on stdin, the way --file - does. |
| 466 | 470 | |
| 467 | 471 | func (s *Server) issueCreateSubmit(w http.ResponseWriter, r *http.Request, u store.User) { |
| 468 | repoPath := r.PathValue("owner") + "/" + r.PathValue("repo") | |
| 472 | p, ok := s.repoFor(w, r, "") | |
| 473 | if !ok { | |
| 474 | return | |
| 475 | } | |
| 476 | repoPath := p.Repo.Path() | |
| 469 | 477 | title := strings.TrimSpace(r.FormValue("title")) |
| 470 | 478 | format := bodyFormat(r) |
| 471 | 479 | if wantsPreview(r) { |
| 472 | 480 | s.issueCreateForm(w, r, u) |
| 473 | 481 | return |
| 474 | 482 | } |
| 483 | canWrite := s.canWriteRepoAs(u, p.Repo) | |
| 475 | 484 | var created control.Created |
| 476 | 485 | argv := []string{"issue", "create", repoPath, "--title", title, "--format", format, "--file", "-"} |
| 477 | 486 | // Labels, milestone and assignee go on the same dispatch issue create |
| @@ -480,7 +489,7 @@ func (s *Server) issueCreateSubmit(w http.ResponseWriter, r *http.Request, u sto | ||
| 480 | 489 | // events happen (#271). issue create refuses the whole create when any |
| 481 | 490 | // of them is set without write access, so a reader's hand-crafted POST |
| 482 | 491 | // carrying one is dropped here rather than failing the create. |
| 483 | if repo, err := s.st.RepoByPath(repoPath); err == nil && s.canWriteRepoAs(u, repo) { | |
| 492 | if canWrite { | |
| 484 | 493 | argv = append(argv, fieldArgs("--label", r.FormValue("labels"))...) |
| 485 | 494 | if milestone := strings.TrimSpace(r.FormValue("milestone")); milestone != "" { |
| 486 | 495 | argv = append(argv, "--milestone", milestone) |
| @@ -489,7 +498,12 @@ func (s *Server) issueCreateSubmit(w http.ResponseWriter, r *http.Request, u sto | ||
| 489 | 498 | } |
| 490 | 499 | code, msg := s.dispatchIntoStdin(u, argv, r.FormValue("body"), &created) |
| 491 | 500 | if code != protocol.ExitOK { |
| 492 | http.Error(w, msg, statusForExit(code)) | |
| 501 | p.Tab = "issues" | |
| 502 | s.render(w, "issuenew.html", issueNewPage{ | |
| 503 | repoPage: p, Body: r.FormValue("body"), Format: format, Title: title, | |
| 504 | Labels: r.FormValue("labels"), Milestone: r.FormValue("milestone"), Assignee: r.FormValue("assignee"), | |
| 505 | Templates: control.IssueTemplates(p.Dir, p.Repo.DefaultBranch), CanWrite: canWrite, Notice: msg, | |
| 506 | }) | |
| 493 | 507 | return |
| 494 | 508 | } |
| 495 | 509 | n := created.Number |
internal/httpd/issuecreate_test.go +105
| @@ -283,6 +283,111 @@ func TestIssueCreateSubmitSetsMilestoneAndAssignee(t *testing.T) { | ||
| 283 | 283 | } |
| 284 | 284 | } |
| 285 | 285 | |
| 286 | // Preview carries the milestone and assignee back into the form, the same | |
| 287 | // way it already carries title and labels, so a writer previewing the | |
| 288 | // body does not lose what they picked (#271). | |
| 289 | func TestIssueCreateFormPreviewKeepsMilestoneAndAssignee(t *testing.T) { | |
| 290 | st, err := store.Open(":memory:") | |
| 291 | if err != nil { | |
| 292 | t.Fatal(err) | |
| 293 | } | |
| 294 | defer st.Close() | |
| 295 | if err := st.MigrateUp(); err != nil { | |
| 296 | t.Fatal(err) | |
| 297 | } | |
| 298 | uid, err := st.CreateUser("alice", false) | |
| 299 | if err != nil { | |
| 300 | t.Fatal(err) | |
| 301 | } | |
| 302 | u := store.User{ID: uid, Username: "alice"} | |
| 303 | if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil { | |
| 304 | t.Fatal(err) | |
| 305 | } | |
| 306 | ||
| 307 | cfg := config.Default() | |
| 308 | cfg.Web.Mode = "accounts" | |
| 309 | s := New(cfg, st) | |
| 310 | form := url.Values{ | |
| 311 | "title": {"a bug"}, | |
| 312 | "body": {"**steps**"}, | |
| 313 | "milestone": {"v1"}, | |
| 314 | "assignee": {"bob"}, | |
| 315 | "preview": {"1"}, | |
| 316 | } | |
| 317 | req := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode())) | |
| 318 | req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| 319 | req.SetPathValue("owner", "alice") | |
| 320 | req.SetPathValue("repo", "app") | |
| 321 | req.AddCookie(sessionCookieFor(t, s, st, uid)) | |
| 322 | rr := httptest.NewRecorder() | |
| 323 | s.issueCreateSubmit(rr, req, u) | |
| 324 | ||
| 325 | body := rr.Body.String() | |
| 326 | if !strings.Contains(body, `value="v1"`) { | |
| 327 | t.Errorf("preview lost the milestone:\n%s", body) | |
| 328 | } | |
| 329 | if !strings.Contains(body, `value="bob"`) { | |
| 330 | t.Errorf("preview lost the assignee:\n%s", body) | |
| 331 | } | |
| 332 | } | |
| 333 | ||
| 334 | // A refused create — here a bad milestone — re-renders the new-issue form | |
| 335 | // with the draft and a notice, rather than an http.Error page that drops | |
| 336 | // everything the visitor typed (#271). | |
| 337 | func TestIssueCreateSubmitRefusedKeepsDraft(t *testing.T) { | |
| 338 | st, err := store.Open(":memory:") | |
| 339 | if err != nil { | |
| 340 | t.Fatal(err) | |
| 341 | } | |
| 342 | defer st.Close() | |
| 343 | if err := st.MigrateUp(); err != nil { | |
| 344 | t.Fatal(err) | |
| 345 | } | |
| 346 | uid, err := st.CreateUser("alice", false) | |
| 347 | if err != nil { | |
| 348 | t.Fatal(err) | |
| 349 | } | |
| 350 | u := store.User{ID: uid, Username: "alice"} | |
| 351 | if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil { | |
| 352 | t.Fatal(err) | |
| 353 | } | |
| 354 | repo, err := st.RepoByPath("alice/app") | |
| 355 | if err != nil { | |
| 356 | t.Fatal(err) | |
| 357 | } | |
| 358 | ||
| 359 | s := New(config.Default(), st) | |
| 360 | form := url.Values{ | |
| 361 | "title": {"needs a fix"}, | |
| 362 | "body": {"details"}, | |
| 363 | "milestone": {"no-such-milestone"}, | |
| 364 | } | |
| 365 | req := httptest.NewRequest("POST", "/alice/app/issues/new", strings.NewReader(form.Encode())) | |
| 366 | req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| 367 | req.SetPathValue("owner", "alice") | |
| 368 | req.SetPathValue("repo", "app") | |
| 369 | rr := httptest.NewRecorder() | |
| 370 | s.issueCreateSubmit(rr, req, u) | |
| 371 | ||
| 372 | if rr.Code == http.StatusSeeOther { | |
| 373 | t.Fatalf("expected a failure status, got redirect") | |
| 374 | } | |
| 375 | body := rr.Body.String() | |
| 376 | if !strings.Contains(body, `value="needs a fix"`) { | |
| 377 | t.Errorf("refused create lost the title:\n%s", body) | |
| 378 | } | |
| 379 | if !strings.Contains(body, "details") { | |
| 380 | t.Errorf("refused create lost the body:\n%s", body) | |
| 381 | } | |
| 382 | if !strings.Contains(body, `class="error"`) { | |
| 383 | t.Errorf("refused create has no notice:\n%s", body) | |
| 384 | } | |
| 385 | ||
| 386 | if _, err := st.IssueByNumber(repo.ID, 1); err == nil { | |
| 387 | t.Fatal("issue was created despite the bad milestone") | |
| 388 | } | |
| 389 | } | |
| 390 | ||
| 286 | 391 | // A bad assignee creates nothing: issue create resolves the assignee |
| 287 | 392 | // before writing the issue, so a typo leaves the repo without a |
| 288 | 393 | // half-created issue (#271). |
internal/web/templates/issuenew.html +3 −2
| @@ -2,6 +2,7 @@ | ||
| 2 | 2 | {{define "title"}}new issue · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}} |
| 3 | 3 | {{define "content"}} |
| 4 | 4 | <h1>New issue</h1> |
| 5 | {{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}} | |
| 5 | 6 | {{if gt (len .Templates) 1}}<p class="meta">template: |
| 6 | 7 | {{range .Templates}}{{if eq .Name $.Template}}<strong>{{.Name}}</strong>{{else}}<a href="?template={{.Name}}">{{.Name}}</a>{{end}} {{end}}</p>{{end}} |
| 7 | 8 | {{if .Draft.Is "body"}}{{template "previewblock" .Draft.HTML}}{{end}} |
| @@ -11,8 +12,8 @@ | ||
| 11 | 12 | {{template "formatpicker" .Format}} |
| 12 | 13 | {{if .CanWrite}} |
| 13 | 14 | <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> | |
| 15 | <p><input type="text" name="milestone" aria-label="Milestone" placeholder="milestone (optional)" value="{{.Milestone}}"></p> | |
| 16 | <p><input type="text" name="assignee" aria-label="Assignee" placeholder="assignees, space-separated (optional)" value="{{.Assignee}}"></p> | |
| 16 | 17 | {{end}} |
| 17 | 18 | <p><button type="submit">Open issue</button> {{template "previewbtn"}}</p> |
| 18 | 19 | </form> |