Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Compact the change request target controls

41d1fee900568b1bff5c7cff3b4c52942f47fabd · Keenan Nemetz · 13d ago · parent e050e14a

Compact the change request target controls

Replace the separate base and head forms and the duplicated labels with one
row: base picker, head picker, update. The pickers preselect the current
targets, and a pinned head appears as the current commit. Collapse the help
text to one line.

Post both targets to one /target handler and write them in one metadata
commit. Either change invalidates approvals; keeping the current value is
allowed even if its branch has gone missing so the other side can still be
repaired.

server/auth_test.go +23−29

@@ -425,11 +425,11 @@ func TestContributorNamespaceAndRequestUseTheAccountName(t *testing.T) {
425 425 }
426 426 }
427 427
428 // TestContributorMovesTheirOwnRequestHead: a request opened from a branch follows
429 // it, so a fixup push moves the head without the contributor writing metadata. The
430 // browser's head control is how a request is repointed at another branch, under the
428 // TestContributorRepointsRequestTarget: a request opened from a branch follows it,
429 // so a fixup push moves the head without the contributor writing metadata. The
430 // browser's single target control changes the base and head together, under the
431 431 // same namespace rule the create form uses, and only while the request is open.
432 func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
432 func TestContributorRepointsRequestTarget(t *testing.T) {
433 433 t.Parallel()
434 434 f := browserFixture(t)
435 435
@@ -502,15 +502,15 @@ func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
502 502 if resp.StatusCode != http.StatusOK {
503 503 t.Fatalf("request page: status=%d body=%s", resp.StatusCode, body)
504 504 }
505 if !strings.Contains(body, `action="/r/`+f.repo+`/requests/`+id+`/head"`) || !strings.Contains(body, headRef) {
506 t.Fatalf("the request page does not offer to move the head:\n%s", body)
505 if !strings.Contains(body, `action="/r/`+f.repo+`/requests/`+id+`/target"`) || !strings.Contains(body, headRef) {
506 t.Fatalf("the request page does not offer the target control:\n%s", body)
507 507 }
508 508
509 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
510 "csrf": {sess.CSRF}, "head": {headRef},
509 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/target", cookie, url.Values{
510 "csrf": {sess.CSRF}, "base": {"main"}, "head": {headRef},
511 511 })
512 512 if resp.StatusCode != http.StatusSeeOther {
513 t.Fatalf("move head: status=%d body=%s", resp.StatusCode, body)
513 t.Fatalf("repoint target: status=%d body=%s", resp.StatusCode, body)
514 514 }
515 515 got, err := metadata.GetRequest(m.Git(), id)
516 516 if err != nil {
@@ -522,8 +522,8 @@ func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
522 522
523 523 // The namespace rule is the one the create form applies: a contributor cannot
524 524 // point their request at a branch they could not have pushed.
525 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
526 "csrf": {sess.CSRF}, "head": {"main"},
525 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/target", cookie, url.Values{
526 "csrf": {sess.CSRF}, "base": {"main"}, "head": {"main"},
527 527 })
528 528 if resp.StatusCode != http.StatusForbidden {
529 529 t.Fatalf("contributor moved the head to main: status=%d", resp.StatusCode)
@@ -535,31 +535,31 @@ func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
535 535 // A maintainer may move any open request, including a contributor's.
536 536 ownerSess, ownerCookie := f.browserSession(t, f.ownerID)
537 537 mainTip, _ := m.Git().RevParse("refs/heads/main")
538 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", ownerCookie, url.Values{
539 "csrf": {ownerSess.CSRF}, "head": {"main"},
538 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/target", ownerCookie, url.Values{
539 "csrf": {ownerSess.CSRF}, "base": {"main"}, "head": {"main"},
540 540 })
541 541 if resp.StatusCode != http.StatusSeeOther {
542 t.Fatalf("maintainer move head: status=%d body=%s", resp.StatusCode, body)
542 t.Fatalf("maintainer repoint: status=%d body=%s", resp.StatusCode, body)
543 543 }
544 544 if got, _ := metadata.GetRequest(m.Git(), id); got.Head != mainTip || got.HeadRef != "main" {
545 545 t.Fatalf("head = %s on %q, want the maintainer's choice %s on main", got.Head, got.HeadRef, mainTip)
546 546 }
547 547
548 548 // A stacked request can be retargeted at the branch it should land on now. The
549 // page offers the base control, and the change moves the diff and starts
549 // same control carries the base change, and the change moves the diff and starts
550 550 // approvals over.
551 551 if _, errs, err := f.git(base, "push", f.sshURL(f.repo), "main:refs/heads/parent"); err != nil {
552 552 t.Fatalf("parent push: %v\n%s", err, errs)
553 553 }
554 554 resp, body = f.browserGet(t, "/r/"+f.repo+"/requests/"+id, cookie)
555 if resp.StatusCode != http.StatusOK || !strings.Contains(body, `action="/r/`+f.repo+`/requests/`+id+`/base"`) {
556 t.Fatalf("the request page does not offer to change the base:\n%s", body)
555 if resp.StatusCode != http.StatusOK || !strings.Contains(body, `action="/r/`+f.repo+`/requests/`+id+`/target"`) {
556 t.Fatalf("the request page does not offer the target control:\n%s", body)
557 557 }
558 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/base", cookie, url.Values{
559 "csrf": {sess.CSRF}, "base": {"parent"},
558 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/target", cookie, url.Values{
559 "csrf": {sess.CSRF}, "base": {"parent"}, "head": {"main"},
560 560 })
561 561 if resp.StatusCode != http.StatusSeeOther {
562 t.Fatalf("change base: status=%d body=%s", resp.StatusCode, body)
562 t.Fatalf("retarget: status=%d body=%s", resp.StatusCode, body)
563 563 }
564 564 if got, _ := metadata.GetRequest(m.Git(), id); got.Base != "parent" {
565 565 t.Fatalf("base = %s, want parent", got.Base)
@@ -572,17 +572,11 @@ func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
572 572 if resp.StatusCode != http.StatusSeeOther {
573 573 t.Fatalf("close: status=%d body=%s", resp.StatusCode, body)
574 574 }
575 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
576 "csrf": {sess.CSRF}, "head": {headRef},
575 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/target", cookie, url.Values{
576 "csrf": {sess.CSRF}, "base": {"main"}, "head": {headRef},
577 577 })
578 578 if resp.StatusCode != http.StatusForbidden {
579 t.Fatalf("a closed request moved its head: status=%d", resp.StatusCode)
580 }
581 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/base", cookie, url.Values{
582 "csrf": {sess.CSRF}, "base": {"main"},
583 })
584 if resp.StatusCode != http.StatusForbidden {
585 t.Fatalf("a closed request changed its base: status=%d", resp.StatusCode)
579 t.Fatalf("a closed request changed its target: status=%d", resp.StatusCode)
586 580 }
587 581 }
588 582