-
-
-
-
-
+
+
+
+ {{ i "loader-circle" "size-4 animate-spin hidden peer-[.htmx-request]:inline text-gray-500 dark:text-gray-400" }}
+
-
+
+ {{ if and .Fork .ForkBranches }}
+ {{ template "repo/pulls/fragments/pullCompareForksBranches" (dict "SourceBranches" .ForkBranches "SourceBranch" .SourceBranch "RepoInfo" .RepoInfo) }}
+ {{ else }}
Select a fork first to view available branches
-
+ {{ end }}
-
-
-
-
-
-
-
- Title and description are optional; if left out, they will be extracted
- from the first commit.
-
{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullCompareForksBranches.html b/appview/pages/templates/repo/pulls/fragments/pullCompareForksBranches.html
index cc426f5f..7ada1950 100644
--- a/appview/pages/templates/repo/pulls/fragments/pullCompareForksBranches.html
+++ b/appview/pages/templates/repo/pulls/fragments/pullCompareForksBranches.html
@@ -2,18 +2,31 @@
+ {{ i "loader-circle" "size-4 animate-spin hidden peer-[.htmx-request]:inline text-gray-500 dark:text-gray-400" }}
{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullPatchUpload.html b/appview/pages/templates/repo/pulls/fragments/pullPatchUpload.html
index 9f330a11..7748d814 100644
--- a/appview/pages/templates/repo/pulls/fragments/pullPatchUpload.html
+++ b/appview/pages/templates/repo/pulls/fragments/pullPatchUpload.html
@@ -1,13 +1,21 @@
{{ define "repo/pulls/fragments/pullPatchUpload" }}
-
- You can paste a git diff or a
- git format-patch patch series here.
-
+
+
+ You can paste a git diff or a
+ git format-patch patch series here.
+
+
+ {{ i "loader-circle" "size-4 animate-spin hidden group-[.htmx-request]:inline text-gray-500 dark:text-gray-400" }}
+
+
+ >{{ .Patch }}
{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullStepDetails.html b/appview/pages/templates/repo/pulls/fragments/pullStepDetails.html
new file mode 100644
index 00000000..389fba8a
--- /dev/null
+++ b/appview/pages/templates/repo/pulls/fragments/pullStepDetails.html
@@ -0,0 +1,146 @@
+{{ define "repo/pulls/fragments/pullStepDetails" }}
+ {{ $hasSidePanel := and .LabelDefs .RepoInfo.Roles.IsPushAllowed }}
+ {{ $previewUrl := printf "/%s/pulls/new/preview" .RepoInfo.FullName }}
+ {{ $labelCtx := dict "Defs" .LabelDefs "State" .LabelState "RepoInfo" .RepoInfo "Subject" "" "LoggedInUser" .LoggedInUser }}
+
+
+
+ {{ template "pullStepDetailsSingle" (dict "Root" . "PreviewUrl" $previewUrl) }}
+ {{ template "pullSubmitRow" . }}
+
+
+ {{ if $hasSidePanel }}
+
+ {{ end }}
+
+
+ {{ template "markdownEditorScript" }}
+{{ end }}
+
+{{ define "pullStepDetailsSingle" }}
+ {{ $root := .Root }}
+ {{ $previewUrl := .PreviewUrl }}
+
+
+
+
+
+ {{ template "markdownEditor" (dict
+ "Id" "pull-body"
+ "Name" "body"
+ "Value" $root.Body
+ "Rows" 6
+ "Placeholder" "Describe your change. Markdown is supported."
+ "PreviewUrl" $previewUrl
+ ) }}
+{{ end }}
+
+{{ define "markdownEditor" }}
+ {{ $id := .Id }}
+ {{ $name := .Name }}
+ {{ $value := .Value }}
+ {{ $rows := .Rows }}
+ {{ $placeholder := .Placeholder }}
+ {{ $previewUrl := .PreviewUrl }}
+
+ {{ $tabClasses := "group flex items-center gap-2 px-3 py-1.5 text-sm whitespace-nowrap hover:no-underline data-[active=true]:bg-white data-[active=true]:dark:bg-gray-700 data-[active=true]:shadow-sm data-[active=true]:cursor-default data-[active=false]:bg-gray-100 data-[active=false]:dark:bg-gray-800 data-[active=false]:shadow-inner" }}
+
+
+
+
+
+
+
+
+
+ Loading preview...
+
+
+
+{{ end }}
+
+{{ define "pullSubmitRow" }}
+
+ {{ if and .MergeCheck .MergeCheck.IsConflicted }}
+
+
+ {{ i "x" "w-4 h-4" }}
+ Can't automatically merge
+
+ You can still create the pull request
+
+ {{ else if and .MergeCheck .MergeCheck.Error }}
+
+ {{ i "triangle-alert" "w-4 h-4" }}
+ Merge check failed
+
+ {{ end }}
+
+
+
+{{ end }}
+
+{{ define "markdownEditorScript" }}
+
+{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullStepReview.html b/appview/pages/templates/repo/pulls/fragments/pullStepReview.html
new file mode 100644
index 00000000..9101fcc8
--- /dev/null
+++ b/appview/pages/templates/repo/pulls/fragments/pullStepReview.html
@@ -0,0 +1,435 @@
+{{ define "repo/pulls/fragments/pullStepReview" }}
+
+ {{ if not .Comparison }}
+
+ {{ if eq .Source "patch" }}
+ Paste a patch above to see a comparison.
+ {{ else }}
+ Pick a source and target above to see a comparison.
+ {{ end }}
+
+ {{ else }}
+ {{ $commits := .Comparison.FormatPatch }}
+ {{ if $commits }}
+
+
+ {{ len $commits }} commit{{ if ne (len $commits) 1 }}s{{ end }}
+ {{ if and .SourceBranch .TargetBranch }}
+
+ {{ .TargetBranch }}
+ {{ i "arrow-left-right" "w-4 h-4 flex-shrink-0" }}
+ {{ .SourceBranch }}
+
+ {{ end }}
+
+ {{ if .IsStacked }}
+ {{ template "pullReviewStackedCommits" . }}
+ {{ else }}
+ {{ template "pullReviewFlatCommits" . }}
+ {{ end }}
+
+ {{ else if ne .Source "patch" }}
+
+ {{ if and .SourceBranch .TargetBranch (eq .SourceBranch .TargetBranch) }}
+ Source and target are the same branch, nothing to merge.
+ {{ else }}
+ No commits between target and source. Make sure your source branch has commits not on the target.
+ {{ end }}
+
+ {{ end }}
+
+ {{ if and .Diff (not .IsStacked) }}
+ {{ template "repo/fragments/diff" (list .Diff .DiffOpts) }}
+ {{ end }}
+
+ {{ if and .IsStacked $commits }}
+ {{ template "pullSubmitRow" . }}
+ {{ template "pullStackApplyAllScript" }}
+ {{ end }}
+ {{ end }}
+
+{{ end }}
+
+{{ define "pullStackApplyAllScript" }}
+
+{{ end }}
+
+{{ define "pullReviewFlatCommits" }}
+ {{ $commits := .Comparison.FormatPatch }}
+
+ {{ range $commits }}
+ {{ $email := "" }}
+ {{ if .Author }}{{ $email = .Author.Email }}{{ end }}
+ {{ $did := "" }}
+ {{ if $.EmailToDid }}{{ $did = index $.EmailToDid $email }}{{ end }}
+ -
+ {{ template "pullReviewCommitAuthor" (dict "Did" $did "Patch" .) }}
+ {{ .Title }}
+ {{ template "pullReviewCommitMeta" (dict "Patch" . "RepoInfo" $.RepoInfo) }}
+
+ {{ end }}
+
+{{ end }}
+
+{{ define "pullReviewStackedCommits" }}
+ {{ $root := . }}
+ {{ $commits := .Comparison.FormatPatch }}
+ {{ $previewUrl := printf "/%s/pulls/new/preview" .RepoInfo.FullName }}
+
+ {{ range $idx, $p := $commits }}
+ {{ $cid := $p.ChangeIdOrEmpty }}
+ {{ $email := "" }}
+ {{ if $p.Author }}{{ $email = $p.Author.Email }}{{ end }}
+ {{ $did := "" }}
+ {{ if $root.EmailToDid }}{{ $did = index $root.EmailToDid $email }}{{ end }}
+ {{ $titleOverride := index $root.StackTitles $cid }}
+ {{ $bodyOverride := index $root.StackBodies $cid }}
+ {{ $displayTitle := $p.Title }}
+ {{ if $titleOverride }}{{ $displayTitle = $titleOverride }}{{ end }}
+ {{ $bodyValue := $p.Body }}
+ {{ if $bodyOverride }}{{ $bodyValue = $bodyOverride }}{{ end }}
+ {{ $perDiff := "" }}
+ {{ if lt $idx (len $root.PerCommitDiffs) }}
+ {{ $perDiff = index $root.PerCommitDiffs $idx }}
+ {{ end }}
+ {{ $perOpts := dict }}
+ {{ if lt $idx (len $root.StackDiffOpts) }}
+ {{ $perOpts = index $root.StackDiffOpts $idx }}
+ {{ end }}
+ -
+
+
+
+ {{ i "chevron-right" "w-4 h-4 group-open/stacked:hidden inline" }}
+ {{ i "chevron-down" "w-4 h-4 hidden group-open/stacked:inline" }}
+
+ {{ template "pullReviewCommitAuthor" (dict "Did" $did "Patch" $p) }}
+ {{ $displayTitle }}
+ {{ template "pullReviewCommitMeta" (dict "Patch" $p "RepoInfo" $root.RepoInfo) }}
+
+ {{ if $cid }}
+
+ {{ if $perDiff }}
+
+
+ {{ template "pullStackedDiffArea" (dict "Diff" $perDiff "DiffOpts" $perOpts "Cid" $cid) }}
+
+ {{ end }}
+ {{ $titleName := printf "stackTitle[%s]" $cid }}
+ {{ $bodyName := printf "stackBody[%s]" $cid }}
+ {{ $hasSidePanel := and $root.LabelDefs $root.RepoInfo.Roles.IsPushAllowed }}
+
+
+
+
+
+
+ {{ template "markdownEditor" (dict
+ "Id" (printf "stack-body-%s" $cid)
+ "Name" $bodyName
+ "Value" $bodyValue
+ "Rows" 4
+ "Placeholder" "Describe this pull request. Markdown is supported."
+ "LabelText" "description"
+ "PreviewUrl" $previewUrl
+ ) }}
+
+ {{ if $hasSidePanel }}
+
+ {{ end }}
+
+
+ {{ else }}
+
+ This commit has no Change-Id header and can't be stacked. Set one on the commit and re-push.
+
+ {{ end }}
+
+
+ {{ end }}
+
+{{ end }}
+
+{{ define "pullReviewCommitAuthor" }}
+ {{ $did := .Did }}
+ {{ $p := .Patch }}
+ {{ if $did }}
+
+
+ {{ resolve $did }}
+
+ {{ else }}
+
+ {{ placeholderAvatar "tiny" }}
+ {{ if $p.Author }}
+ {{ $p.Author.Name }}
+ {{ end }}
+
+ {{ end }}
+{{ end }}
+
+{{ define "pullReviewCommitMeta" }}
+ {{ $p := .Patch }}
+ {{ $repoInfo := .RepoInfo }}
+
+ {{ if not $p.AuthorDate.IsZero }}
+ {{ template "repo/fragments/shortTimeAgo" $p.AuthorDate }}
+ {{ end }}
+ {{ if $p.SHA }}
+ {{ slice $p.SHA 0 8 }}
+
+
+ {{ i "folder-code" "w-4 h-4" }}
+
+ {{ end }}
+
+{{ end }}
+
+{{ define "pullStackedDiffArea" }}
+ {{ $diff := .Diff }}
+ {{ $opts := .DiffOpts }}
+ {{ $cid := .Cid }}
+ {{ $togId := printf "stack-%s-filesToggle" $cid }}
+ {{ $colId := printf "stack-%s-collapseToggle" $cid }}
+ {{ $filesId := printf "stack-%s-files" $cid }}
+ {{ $diffAreaId := printf "stack-%s-diff-area" $cid }}
+ {{ $filePrefix := printf "stack-%s-file-" $cid }}
+ {{ $stat := $diff.Stats }}
+ {{ $count := len $diff.ChangedFiles }}
+
+
+
+
+
+
+
+
+
+ {{ template "repo/fragments/diffStatPill" $stat }}
+
{{ $count }} changed file{{ if ne $count 1 }}s{{ end }}
+
+
+
+
+
+
+ {{ template "repo/fragments/diffOpts" $opts }}
+
+
+
+
+
+
+ {{ template "repo/fragments/fileTreePrefixed" (dict "Tree" $diff.FileTree "Prefix" $filePrefix) }}
+
+
+
+
+
+ {{ if eq $count 0 }}
+
+
No differences found.
+
+ {{ else }}
+ {{ range $idx, $file := $diff.ChangedFiles }}
+ {{ template "stackedDiffFile" (dict "Idx" $idx "File" $file "IsSplit" $opts.Split "Prefix" $filePrefix) }}
+ {{ end }}
+ {{ end }}
+
+
+
+
+
+
+{{ end }}
+
+{{ define "stackedDiffFile" }}
+ {{ $idx := .Idx }}
+ {{ $file := .File }}
+ {{ $isSplit := .IsSplit }}
+ {{ $prefix := .Prefix }}
+ {{ $isGenerated := false }}
+ {{ $isDeleted := false }}
+ {{ with $file }}
+ {{ $n := .Names }}
+ {{ $isDeleted = and (eq $n.New "") (ne $n.Old "") }}
+ {{ if $n.New }}
+ {{ $isGenerated = isGenerated $n.New }}
+ {{ else if $n.Old }}
+ {{ $isGenerated = isGenerated $n.Old }}
+ {{ end }}
+
+
+
+
+
{{ i "chevron-right" "w-4 h-4" }}
+
{{ i "chevron-down" "w-4 h-4" }}
+ {{ template "repo/fragments/diffStatPill" .Stats }}
+
+ {{ if and $n.New $n.Old (ne $n.New $n.Old)}}
+ {{ $n.Old }} {{ i "arrow-right" "w-4 h-4" }} {{ $n.New }}
+ {{ else if $n.New }}
+ {{ $n.New }}
+ {{ else }}
+ {{ $n.Old }}
+ {{ end }}
+ {{ if $isDeleted }}
+
+ {{ i "circle-question-mark" "size-4" }}
+
+ {{ else if $isGenerated }}
+
+ {{ i "circle-question-mark" "size-4" }}
+
+ {{ end }}
+
+
+
+
+
+
+ {{ $reason := .CanRender }}
+ {{ if $reason }}
+
{{ $reason }}
+ {{ else }}
+ {{ if $isSplit }}
+ {{- template "repo/fragments/splitDiff" .Split -}}
+ {{ else }}
+ {{- template "repo/fragments/unifiedDiff" . -}}
+ {{ end }}
+ {{- end -}}
+
+
+ {{ end }}
+{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullStepSource.html b/appview/pages/templates/repo/pulls/fragments/pullStepSource.html
new file mode 100644
index 00000000..b956bbdd
--- /dev/null
+++ b/appview/pages/templates/repo/pulls/fragments/pullStepSource.html
@@ -0,0 +1,156 @@
+{{ define "repo/pulls/fragments/pullStepSource" }}
+
+
+
+ {{ template "pullSourceTabs" . }}
+
+ {{ if eq .Source "patch" }}
+
+ {{ template "repo/fragments/labelSectionHeaderText" "Merge into" }}
+ {{ template "pullTargetBranchSelect" . }}
+
+ {{ template "repo/pulls/fragments/pullPatchUpload" . }}
+ {{ else }}
+
+
+ {{ template "repo/fragments/labelSectionHeaderText" "Merge into" }}
+ {{ template "pullTargetBranchSelect" . }}
+
+
+ {{ template "repo/fragments/labelSectionHeaderText" "Pull from" }}
+ {{ if eq .Source "fork" }}
+ {{ template "repo/pulls/fragments/pullCompareForks" . }}
+ {{ else }}
+ {{ template "repo/pulls/fragments/pullCompareBranches" . }}
+ {{ end }}
+
+
+ {{ end }}
+
+
+
+ {{ if ne .Source "patch" }}
+
+
+
+
+ {{ i "loader-circle" "size-4 animate-spin hidden peer-[.htmx-request]:inline text-gray-500 dark:text-gray-400" }}
+
+ {{ template "repo/pulls/fragments/stackedExplainer" . }}
+ {{ end }}
+
+{{ end }}
+
+{{ define "pullTargetBranchSelect" }}
+
+
+ {{ i "loader-circle" "size-4 animate-spin hidden peer-[.htmx-request]:inline text-gray-500 dark:text-gray-400" }}
+
+{{ end }}
+
+{{ define "pullSourceTabs" }}
+ {{ $active := "bg-white dark:bg-gray-700 shadow-sm cursor-default" }}
+ {{ $inactive := "bg-gray-100 dark:bg-gray-800 shadow-inner" }}
+ {{ $shared := "group flex-1 p-3 text-left hover:no-underline flex flex-col gap-1" }}
+ {{ $titleCls := "font-medium text-sm dark:text-white flex items-center gap-2" }}
+ {{ $descCls := "text-xs text-gray-500 dark:text-gray-400" }}
+ {{ $fullName := .RepoInfo.FullName }}
+
+ {{ if .RepoInfo.Roles.IsPushAllowed }}
+
+ {{ end }}
+
+
+
+{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullWizardHost.html b/appview/pages/templates/repo/pulls/fragments/pullWizardHost.html
new file mode 100644
index 00000000..96b1ae9a
--- /dev/null
+++ b/appview/pages/templates/repo/pulls/fragments/pullWizardHost.html
@@ -0,0 +1,74 @@
+{{ define "repo/pulls/fragments/pullWizardHost" }}
+
+ {{ if .PrefillError }}
+
+ {{ i "triangle-alert" "w-4 h-4 flex-shrink-0" }}
+ {{ .PrefillError }}
+
+ {{ end }}
+
+ {{ $hasCommits := and .Comparison .Comparison.FormatPatch }}
+ {{ $hasDiff := false }}
+ {{ if .Diff }}{{ if .Diff.Diff }}{{ $hasDiff = true }}{{ end }}{{ end }}
+ {{ $showDetails := and (or $hasCommits $hasDiff) (not .IsStacked) }}
+
+
+
+
+
+{{ end }}
+
+{{ define "pullWizardSectionNumber" }}
+
{{ . }}
+{{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/stackedExplainer.html b/appview/pages/templates/repo/pulls/fragments/stackedExplainer.html
new file mode 100644
index 00000000..020a0c1d
--- /dev/null
+++ b/appview/pages/templates/repo/pulls/fragments/stackedExplainer.html
@@ -0,0 +1,51 @@
+{{ define "repo/pulls/fragments/stackedExplainer" }}
+
+
+
+ Stacked PRs
+
+
+
+
+
+ Each commit on your branch becomes its own pull request. Reviewers can
+ comment per commit, and Tangled tracks each PR across rewrites using
+ jujutsu change-ids. You can jj edit an old commit,
+ push, and the right PR advances to the next round.
+
+
+
+ Without stacking, all changes land as one PR. Force-pushes turn opaque
+ and git blame clobbers across rounds.
+
+
+
+ With stacking, edit/split/squash old commits freely, and descendants
+ will auto-rebase! Each PR shows an "interdiff" between rounds so
+ reviewers will see exactly what changed.
+
+
+
+ Full write-up
+
+
+{{ end }}
diff --git a/appview/pages/templates/repo/pulls/new.html b/appview/pages/templates/repo/pulls/new.html
index cc0b8d92..b9c42886 100644
--- a/appview/pages/templates/repo/pulls/new.html
+++ b/appview/pages/templates/repo/pulls/new.html
@@ -1,156 +1,5 @@
{{ define "title" }}new pull · {{ .RepoInfo.FullName }}{{ end }}
{{ define "repoContent" }}
-
- Create new pull request
-
-
-
+ {{ template "repo/pulls/fragments/pullWizardHost" . }}
{{ end }}
diff --git a/appview/pages/wizard_parse_test.go b/appview/pages/wizard_parse_test.go
new file mode 100644
index 00000000..a5141ef4
--- /dev/null
+++ b/appview/pages/wizard_parse_test.go
@@ -0,0 +1,315 @@
+package pages
+
+import (
+ "bytes"
+ "io"
+ "log/slog"
+ "strings"
+ "testing"
+
+ "tangled.org/core/appview/config"
+ "tangled.org/core/appview/models"
+ "tangled.org/core/appview/pages/repoinfo"
+ "tangled.org/core/patchutil"
+ "tangled.org/core/types"
+)
+
+func TestPullWizardTemplatesParse(t *testing.T) {
+ cfg := &config.Config{}
+ p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil)))
+
+ cases := []struct {
+ name string
+ stack []string
+ }{
+ {"new.html via repo base", []string{"layouts/base", "layouts/repobase", "repo/pulls/new"}},
+ {"pullWizardHost", []string{"repo/pulls/fragments/pullWizardHost"}},
+ {"pullStepSource", []string{"repo/pulls/fragments/pullStepSource"}},
+ {"pullStepReview", []string{"repo/pulls/fragments/pullStepReview"}},
+ {"pullStepDetails", []string{"repo/pulls/fragments/pullStepDetails"}},
+ {"pullCompareForks", []string{"repo/pulls/fragments/pullCompareForks"}},
+ {"pullCompareBranches", []string{"repo/pulls/fragments/pullCompareBranches"}},
+ {"pullCompareForksBranches", []string{"repo/pulls/fragments/pullCompareForksBranches"}},
+ {"stackedExplainer", []string{"repo/pulls/fragments/stackedExplainer"}},
+ }
+
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ if _, err := p.rawParse(c.stack...); err != nil {
+ t.Fatalf("parse %v: %v", c.stack, err)
+ }
+ })
+ }
+}
+
+func TestPullWizardHostRender(t *testing.T) {
+ cfg := &config.Config{}
+ p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil)))
+
+ base := RepoNewPullParams{
+ RepoInfo: repoinfo.RepoInfo{
+ OwnerDid: "did:plc:test",
+ Name: "test-repo",
+ },
+ }
+
+ for _, source := range []Source{"", SourceBranch, SourceFork, SourcePatch} {
+ for _, stacked := range []bool{false, true} {
+ if source == SourcePatch && stacked {
+ continue
+ }
+ params := base
+ params.Source = source
+ params.IsStacked = stacked
+ name := string(source)
+ if name == "" {
+ name = "default"
+ }
+ if stacked {
+ name += "-stacked"
+ }
+ t.Run(name, func(t *testing.T) {
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render source=%q stacked=%v: %v", source, stacked, err)
+ }
+ })
+ }
+ }
+}
+
+func TestPullWizardHostRenderWithData(t *testing.T) {
+ cfg := &config.Config{}
+ p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil)))
+
+ sampleBranches := []types.Branch{
+ {Reference: types.Reference{Name: "feature"}},
+ {Reference: types.Reference{Name: "main"}, IsDefault: true},
+ }
+
+ formatPatch := `From 1111111111111111111111111111111111111111 Mon Sep 11 00:00:00 2001
+From: Test
+Date: Tue, 1 Jan 2020 00:00:00 +0000
+Subject: [PATCH] example commit
+
+---
+ a.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/a.txt b/a.txt
+index 0000000..1111111 100644
+--- a/a.txt
++++ b/a.txt
+@@ -0,0 +1 @@
++hello
+`
+ patches, err := patchutil.ExtractPatches(formatPatch)
+ if err != nil {
+ t.Fatalf("extract patches: %v", err)
+ }
+ comparison := &types.RepoFormatPatchResponse{
+ FormatPatchRaw: formatPatch,
+ FormatPatch: patches,
+ }
+ diff := patchutil.AsNiceDiff(formatPatch, "main")
+
+ params := RepoNewPullParams{
+ RepoInfo: repoinfo.RepoInfo{
+ OwnerDid: "did:plc:test",
+ Name: "test-repo",
+ },
+ Branches: sampleBranches,
+ SourceBranches: []types.Branch{sampleBranches[0]},
+ ForkBranches: []types.Branch{sampleBranches[0]},
+ Source: SourceBranch,
+ SourceBranch: "feature",
+ TargetBranch: "main",
+ Comparison: comparison,
+ Diff: &diff,
+ }
+
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render with data: %v", err)
+ }
+
+ params.IsStacked = true
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render stacked: %v", err)
+ }
+
+ params.PrefillError = "branch not found"
+ params.Comparison = nil
+ params.Diff = nil
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render with prefill error: %v", err)
+ }
+
+ bugDef := &models.LabelDefinition{
+ Did: "did:plc:test",
+ Rkey: "bug",
+ Name: "bug",
+ ValueType: models.ValueType{Type: models.ConcreteTypeNull},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ priorityDef := &models.LabelDefinition{
+ Did: "did:plc:test",
+ Rkey: "priority",
+ Name: "priority",
+ ValueType: models.ValueType{Type: models.ConcreteTypeString, Enum: []string{"low", "med", "high"}},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ assigneeDef := &models.LabelDefinition{
+ Did: "did:plc:test",
+ Rkey: "assignee",
+ Name: "assignee",
+ ValueType: models.ValueType{Type: models.ConcreteTypeString, Format: models.ValueTypeFormatDid},
+ Scope: []string{"sh.tangled.repo.pull"},
+ Multiple: true,
+ }
+ labelDefs := map[string]*models.LabelDefinition{
+ bugDef.AtUri().String(): bugDef,
+ priorityDef.AtUri().String(): priorityDef,
+ assigneeDef.AtUri().String(): assigneeDef,
+ }
+
+ pushRepoInfo := repoinfo.RepoInfo{
+ OwnerDid: "did:plc:test",
+ Name: "test-repo",
+ Roles: repoinfo.RolesInRepo{Roles: []string{"repo:push"}},
+ }
+ params = RepoNewPullParams{
+ RepoInfo: pushRepoInfo,
+ Branches: sampleBranches,
+ SourceBranches: []types.Branch{sampleBranches[0]},
+ Source: SourceBranch,
+ SourceBranch: "feature",
+ TargetBranch: "main",
+ Comparison: comparison,
+ Diff: &diff,
+ LabelDefs: labelDefs,
+ LabelState: models.NewLabelState(),
+ }
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render with labels: %v", err)
+ }
+
+ params.IsStacked = true
+ if err := p.PullWizardHostFragment(io.Discard, params); err != nil {
+ t.Fatalf("render stacked with labels: %v", err)
+ }
+}
+
+func TestPullWizardLabelStateRoundTrip(t *testing.T) {
+ cfg := &config.Config{}
+ p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil)))
+
+ sampleBranches := []types.Branch{
+ {Reference: types.Reference{Name: "feature"}},
+ {Reference: types.Reference{Name: "main"}, IsDefault: true},
+ }
+
+ bugDef := &models.LabelDefinition{
+ Did: "did:plc:test", Rkey: "bug", Name: "bug",
+ ValueType: models.ValueType{Type: models.ConcreteTypeNull},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ priorityDef := &models.LabelDefinition{
+ Did: "did:plc:test", Rkey: "priority", Name: "priority",
+ ValueType: models.ValueType{Type: models.ConcreteTypeString, Enum: []string{"low", "med", "high"}},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ bugKey := bugDef.AtUri().String()
+ priorityKey := priorityDef.AtUri().String()
+ labelDefs := map[string]*models.LabelDefinition{
+ bugKey: bugDef,
+ priorityKey: priorityDef,
+ }
+
+ state := models.NewLabelState()
+ actx := &models.LabelApplicationCtx{Defs: labelDefs}
+ for _, op := range []models.LabelOp{
+ {OperandKey: bugKey, OperandValue: "null", Operation: models.LabelOperationAdd},
+ {OperandKey: priorityKey, OperandValue: "high", Operation: models.LabelOperationAdd},
+ } {
+ if err := actx.ApplyLabelOp(state, op); err != nil {
+ t.Fatalf("seed state: %v", err)
+ }
+ }
+
+ formatPatch := `From 1111111111111111111111111111111111111111 Mon Sep 11 00:00:00 2001
+From: Test
+Date: Tue, 1 Jan 2020 00:00:00 +0000
+Subject: [PATCH] example commit
+
+---
+ a.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/a.txt b/a.txt
+index 0000000..1111111 100644
+--- a/a.txt
++++ b/a.txt
+@@ -0,0 +1 @@
++hello
+`
+ patches, err := patchutil.ExtractPatches(formatPatch)
+ if err != nil {
+ t.Fatalf("extract patches: %v", err)
+ }
+ comparison := &types.RepoFormatPatchResponse{
+ FormatPatchRaw: formatPatch,
+ FormatPatch: patches,
+ }
+
+ params := RepoNewPullParams{
+ RepoInfo: repoinfo.RepoInfo{
+ OwnerDid: "did:plc:test",
+ Name: "test-repo",
+ Roles: repoinfo.RolesInRepo{Roles: []string{"repo:push"}},
+ },
+ Branches: sampleBranches,
+ SourceBranches: []types.Branch{sampleBranches[0]},
+ Source: SourceBranch,
+ SourceBranch: "feature",
+ TargetBranch: "main",
+ Comparison: comparison,
+ LabelDefs: labelDefs,
+ LabelState: state,
+ }
+
+ var buf bytes.Buffer
+ if err := p.PullWizardHostFragment(&buf, params); err != nil {
+ t.Fatalf("render: %v", err)
+ }
+ out := buf.String()
+ for _, want := range []string{
+ `value="null" checked`,
+ `value="high" checked`,
+ } {
+ if !strings.Contains(out, want) {
+ t.Errorf("missing pre-selection %q", want)
+ }
+ }
+}
+
+func TestParseSource(t *testing.T) {
+ cases := []struct {
+ in string
+ want Source
+ wantOk bool
+ }{
+ {"branch", SourceBranch, true},
+ {"BRANCH", SourceBranch, true},
+ {"fork", SourceFork, true},
+ {"patch", SourcePatch, true},
+ {"", "", false},
+ {"method", "", false},
+ {"strategy", "", false},
+ {"unknown", "", false},
+ }
+ for _, c := range cases {
+ got, ok := ParseSource(c.in)
+ if got != c.want || ok != c.wantOk {
+ t.Errorf("ParseSource(%q) = %q, %v; want %q, %v", c.in, got, ok, c.want, c.wantOk)
+ }
+ }
+}
diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go
index 3faf8495..8082bcf7 100644
--- a/appview/pulls/pulls.go
+++ b/appview/pulls/pulls.go
@@ -9,8 +9,11 @@ import (
"errors"
"fmt"
"io"
+ "iter"
"log/slog"
+ "maps"
"net/http"
+ "net/url"
"slices"
"sort"
"strconv"
@@ -43,6 +46,7 @@ import (
"tangled.org/core/xrpc"
comatproto "github.com/bluesky-social/indigo/api/atproto"
+ "github.com/bluesky-social/indigo/atproto/atclient"
"github.com/bluesky-social/indigo/atproto/syntax"
lexutil "github.com/bluesky-social/indigo/lex/util"
indigoxrpc "github.com/bluesky-social/indigo/xrpc"
@@ -98,6 +102,14 @@ func New(
}
}
+func (s *Pulls) knotClient(host string) *indigoxrpc.Client {
+ scheme := "https"
+ if s.config.Core.Dev {
+ scheme = "http"
+ }
+ return &indigoxrpc.Client{Host: fmt.Sprintf("%s://%s", scheme, host)}
+}
+
// htmx fragment
func (s *Pulls) PullActions(w http.ResponseWriter, r *http.Request) {
l := s.logger.With("handler", "PullActions")
@@ -328,15 +340,7 @@ func (s *Pulls) mergeCheck(r *http.Request, f *models.Repo, pull *models.Pull, s
return types.MergeCheckResponse{}
}
- scheme := "https"
- if s.config.Core.Dev {
- scheme = "http"
- }
- host := fmt.Sprintf("%s://%s", scheme, f.Knot)
-
- xrpcc := indigoxrpc.Client{
- Host: host,
- }
+ xrpcc := s.knotClient(f.Knot)
// combine patches of substack
subStack := stack.Below(pull)
@@ -347,7 +351,7 @@ func (s *Pulls) mergeCheck(r *http.Request, f *models.Repo, pull *models.Pull, s
resp, err := tangled.RepoMergeCheck(
r.Context(),
- &xrpcc,
+ xrpcc,
&tangled.RepoMergeCheck_Input{
Did: f.Did,
Name: f.Name,
@@ -362,29 +366,25 @@ func (s *Pulls) mergeCheck(r *http.Request, f *models.Repo, pull *models.Pull, s
}
}
- // convert xrpc response to internal types
+ return mergeCheckResponseFrom(resp)
+}
+
+func mergeCheckResponseFrom(resp *tangled.RepoMergeCheck_Output) types.MergeCheckResponse {
conflicts := make([]types.ConflictInfo, len(resp.Conflicts))
- for i, conflict := range resp.Conflicts {
- conflicts[i] = types.ConflictInfo{
- Filename: conflict.Filename,
- Reason: conflict.Reason,
- }
+ for i, c := range resp.Conflicts {
+ conflicts[i] = types.ConflictInfo{Filename: c.Filename, Reason: c.Reason}
}
-
- result := types.MergeCheckResponse{
+ out := types.MergeCheckResponse{
IsConflicted: resp.Is_conflicted,
Conflicts: conflicts,
}
-
if resp.Message != nil {
- result.Message = *resp.Message
+ out.Message = *resp.Message
}
-
if resp.Error != nil {
- result.Error = *resp.Error
+ out.Error = *resp.Error
}
-
- return result
+ return out
}
func (s *Pulls) branchDeleteStatus(r *http.Request, repo *models.Repo, pull *models.Pull) *models.BranchDeleteStatus {
@@ -930,42 +930,13 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodGet:
- xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url}
-
- xrpcBytes, err := tangled.GitTempListBranches(r.Context(), xrpcc, "", 0, f.RepoAt().String())
+ params, err := s.wizardParams(r, f)
if err != nil {
- if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
- l.Error("failed to call XRPC repo.branches", "xrpcerr", xrpcerr, "err", err)
- s.pages.Error503(w)
- return
- }
- l.Error("failed to fetch branches", "err", err)
- return
- }
-
- var result types.RepoBranchesResponse
- if err := json.Unmarshal(xrpcBytes, &result); err != nil {
- l.Error("failed to decode XRPC response", "err", err)
+ l.Error("failed to build wizard params", "err", err)
s.pages.Error503(w)
return
}
-
- // can be one of "patch", "branch" or "fork"
- strategy := r.URL.Query().Get("strategy")
- // ignored if strategy is "patch"
- sourceBranch := r.URL.Query().Get("sourceBranch")
- targetBranch := r.URL.Query().Get("targetBranch")
-
- s.pages.RepoNewPull(w, pages.RepoNewPullParams{
- LoggedInUser: user,
- RepoInfo: s.repoResolver.GetRepoInfo(r, user),
- Branches: result.Branches,
- Strategy: strategy,
- SourceBranch: sourceBranch,
- TargetBranch: targetBranch,
- Title: r.URL.Query().Get("title"),
- Body: r.URL.Query().Get("body"),
- })
+ s.pages.RepoNewPull(w, params)
case http.MethodPost:
title := r.FormValue("title")
@@ -987,7 +958,7 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) {
isBranchBased := isPushAllowed && sourceBranch != "" && fromFork == ""
isForkBased := fromFork != "" && sourceBranch != ""
isPatchBased := patch != "" && !isBranchBased && !isForkBased
- isStacked := r.FormValue("isStacked") == "on"
+ isStacked := r.FormValue("mode") == "stack" && !isPatchBased
if isPatchBased && !patchutil.IsFormatPatch(patch) {
if title == "" {
@@ -1013,6 +984,11 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) {
return
}
+ if isBranchBased && sourceBranch == targetBranch {
+ s.pages.Notice(w, "pull", "Source and target branch must be different.")
+ return
+ }
+
// us, err := knotclient.NewUnsignedClient(f.Knot, s.config.Core.Dev)
// if err != nil {
// log.Printf("failed to create unsigned client to %s: %v", f.Knot, err)
@@ -1054,25 +1030,28 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) {
return
}
+ stackTitles := parseBracketedForm(r.Form, "stackTitle")
+ stackBodies := parseBracketedForm(r.Form, "stackBody")
+
// Handle the PR creation based on the type
if isBranchBased {
if !caps.PullRequests.BranchSubmissions {
s.pages.Notice(w, "pull", "This knot doesn't support branch-based pull requests. Try another way?")
return
}
- s.handleBranchBasedPull(w, r, f, userDid, title, body, targetBranch, sourceBranch, isStacked)
+ s.handleBranchBasedPull(w, r, f, userDid, title, body, targetBranch, sourceBranch, isStacked, stackTitles, stackBodies)
} else if isForkBased {
if !caps.PullRequests.ForkSubmissions {
s.pages.Notice(w, "pull", "This knot doesn't support fork-based pull requests. Try another way?")
return
}
- s.handleForkBasedPull(w, r, f, userDid, fromFork, title, body, targetBranch, sourceBranch, isStacked)
+ s.handleForkBasedPull(w, r, f, userDid, fromFork, title, body, targetBranch, sourceBranch, isStacked, stackTitles, stackBodies)
} else if isPatchBased {
if !caps.PullRequests.PatchSubmissions {
s.pages.Notice(w, "pull", "This knot doesn't support patch-based pull requests. Send your patch over email.")
return
}
- s.handlePatchBasedPull(w, r, f, userDid, title, body, targetBranch, patch, isStacked)
+ s.handlePatchBasedPull(w, r, f, userDid, title, body, targetBranch, patch, isStacked, stackTitles, stackBodies)
}
return
}
@@ -1088,17 +1067,11 @@ func (s *Pulls) handleBranchBasedPull(
targetBranch,
sourceBranch string,
isStacked bool,
+ stackTitles, stackBodies map[string]string,
) {
l := s.logger.With("handler", "handleBranchBasedPull", "user", userDid, "target_branch", targetBranch, "source_branch", sourceBranch, "is_stacked", isStacked)
- scheme := "http"
- if !s.config.Core.Dev {
- scheme = "https"
- }
- host := fmt.Sprintf("%s://%s", scheme, repo.Knot)
- xrpcc := &indigoxrpc.Client{
- Host: host,
- }
+ xrpcc := s.knotClient(repo.Knot)
xrpcBytes, err := tangled.RepoCompare(r.Context(), xrpcc, repo.RepoIdentifier(), targetBranch, sourceBranch)
if err != nil {
@@ -1119,6 +1092,11 @@ func (s *Pulls) handleBranchBasedPull(
return
}
+ if len(comparison.FormatPatch) == 0 {
+ s.pages.Notice(w, "pull", "No commits between target and source.")
+ return
+ }
+
sourceRev := comparison.Rev2
patch := comparison.FormatPatchRaw
combined := comparison.CombinedPatchRaw
@@ -1133,20 +1111,20 @@ func (s *Pulls) handleBranchBasedPull(
Branch: sourceBranch,
}
- s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked)
+ s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked, stackTitles, stackBodies)
}
-func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, title, body, targetBranch, patch string, isStacked bool) {
+func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, title, body, targetBranch, patch string, isStacked bool, stackTitles, stackBodies map[string]string) {
if err := s.validator.ValidatePatch(&patch); err != nil {
s.logger.Error("patch validation failed", "err", err)
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
return
}
- s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, "", "", nil, isStacked)
+ s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, "", "", nil, isStacked, stackTitles, stackBodies)
}
-func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, forkRepo string, title, body, targetBranch, sourceBranch string, isStacked bool) {
+func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, forkRepo string, title, body, targetBranch, sourceBranch string, isStacked bool, stackTitles, stackBodies map[string]string) {
l := s.logger.With("handler", "handleForkBasedPull", "user", userDid, "fork_repo", forkRepo, "target_branch", targetBranch, "source_branch", sourceBranch, "is_stacked", isStacked)
repoString := strings.SplitN(forkRepo, "/", 2)
@@ -1199,14 +1177,7 @@ func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo
// hiddenRef: hidden/feature-1/main (on repo-fork)
// targetBranch: main (on repo-1)
// sourceBranch: feature-1 (on repo-fork)
- forkScheme := "http"
- if !s.config.Core.Dev {
- forkScheme = "https"
- }
- forkHost := fmt.Sprintf("%s://%s", forkScheme, fork.Knot)
- forkXrpcc := &indigoxrpc.Client{
- Host: forkHost,
- }
+ forkXrpcc := s.knotClient(fork.Knot)
forkXrpcBytes, err := tangled.RepoCompare(r.Context(), forkXrpcc, fork.RepoIdentifier(), hiddenRef, sourceBranch)
if err != nil {
@@ -1227,6 +1198,11 @@ func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo
return
}
+ if len(comparison.FormatPatch) == 0 {
+ s.pages.Notice(w, "pull", "No commits between target and source.")
+ return
+ }
+
sourceRev := comparison.Rev2
patch := comparison.FormatPatchRaw
combined := comparison.CombinedPatchRaw
@@ -1250,7 +1226,7 @@ func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo
RepoDid: forkDid,
}
- s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked)
+ s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked, stackTitles, stackBodies)
}
func (s *Pulls) createPullRequest(
@@ -1264,6 +1240,7 @@ func (s *Pulls) createPullRequest(
sourceRev string,
pullSource *models.PullSource,
isStacked bool,
+ stackTitles, stackBodies map[string]string,
) {
l := s.logger.With("handler", "createPullRequest", "user", userDid, "target_branch", targetBranch, "is_stacked", isStacked)
@@ -1278,6 +1255,8 @@ func (s *Pulls) createPullRequest(
patch,
sourceRev,
pullSource,
+ stackTitles,
+ stackBodies,
)
return
}
@@ -1390,6 +1369,8 @@ func (s *Pulls) createPullRequest(
s.notifier.NewPull(r.Context(), pull)
+ s.applyCreationLabels(r.Context(), client, userDid, []*models.Pull{pull}, r.Form, repo)
+
ownerSlashRepo := reporesolver.GetBaseRepoPath(r, repo)
s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pullId))
}
@@ -1403,18 +1384,12 @@ func (s *Pulls) createStackedPullRequest(
patch string,
sourceRev string,
pullSource *models.PullSource,
+ stackTitles, stackBodies map[string]string,
) {
l := s.logger.With("handler", "createStackedPullRequest", "user", userDid, "target_branch", targetBranch, "source_rev", sourceRev)
// run some necessary checks for stacked-prs first
- // must be branch or fork based
- if sourceRev == "" {
- l.Error("stacked PR from patch-based pull")
- s.pages.Notice(w, "pull", "Stacking is only supported on branch and fork based pull-requests.")
- return
- }
-
formatPatches, err := patchutil.ExtractPatches(patch)
if err != nil {
l.Error("failed to extract patches", "err", err)
@@ -1450,7 +1425,7 @@ func (s *Pulls) createStackedPullRequest(
}
// build a stack out of this patch
- stack, err := s.newStack(r.Context(), repo, userDid, targetBranch, pullSource, formatPatches, blobs)
+ stack, err := s.newStack(r.Context(), repo, userDid, targetBranch, pullSource, formatPatches, blobs, stackTitles, stackBodies)
if err != nil {
l.Error("failed to create stack", "err", err)
s.pages.Notice(w, "pull", fmt.Sprintf("Failed to create stack: %v", err))
@@ -1513,6 +1488,8 @@ func (s *Pulls) createStackedPullRequest(
s.notifier.NewPull(r.Context(), p)
}
+ s.applyCreationLabels(r.Context(), client, userDid, stack, r.Form, repo)
+
ownerSlashRepo := reporesolver.GetBaseRepoPath(r, repo)
s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls", ownerSlashRepo))
}
@@ -1545,158 +1522,682 @@ func (s *Pulls) ValidatePatch(w http.ResponseWriter, r *http.Request) {
}
}
-func (s *Pulls) PatchUploadFragment(w http.ResponseWriter, r *http.Request) {
- user := s.oauth.GetMultiAccountUser(r)
-
- s.pages.PullPatchUploadFragment(w, pages.PullPatchUploadParams{
- RepoInfo: s.repoResolver.GetRepoInfo(r, user),
- })
+func (s *Pulls) MarkdownPreview(w http.ResponseWriter, r *http.Request) {
+ body := r.FormValue("body")
+ s.pages.MarkdownPreviewFragment(w, body)
}
-func (s *Pulls) CompareBranchesFragment(w http.ResponseWriter, r *http.Request) {
- l := s.logger.With("handler", "CompareBranchesFragment")
+func (s *Pulls) RefreshWizard(w http.ResponseWriter, r *http.Request) {
+ l := s.logger.With("handler", "RefreshWizard")
- user := s.oauth.GetMultiAccountUser(r)
f, err := s.repoResolver.Resolve(r)
if err != nil {
- l.Error("failed to get repo and knot", "err", err)
+ l.Error("failed to resolve repo", "err", err)
+ s.pages.Error503(w)
return
}
- xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url}
-
- xrpcBytes, err := tangled.GitTempListBranches(r.Context(), xrpcc, "", 0, f.RepoAt().String())
+ params, err := s.wizardParams(r, f)
if err != nil {
- l.Error("failed to fetch branches", "err", err)
+ l.Error("failed to build wizard params", "err", err)
s.pages.Error503(w)
return
}
+ w.Header().Set("HX-Replace-Url", wizardCanonicalURL(params))
+ s.pages.PullWizardHostFragment(w, params)
+}
- var result types.RepoBranchesResponse
- if err := json.Unmarshal(xrpcBytes, &result); err != nil {
- l.Error("failed to decode XRPC response", "err", err)
- s.pages.Error503(w)
- return
+func wizardCanonicalURL(params pages.RepoNewPullParams) string {
+ base := fmt.Sprintf("/%s/pulls/new", params.RepoInfo.FullName())
+ q := url.Values{}
+ if params.IsStacked {
+ q.Set("mode", "stack")
+ }
+ if params.Source != "" && params.Source != pages.SourceBranch {
+ q.Set("source", string(params.Source))
+ }
+ if params.SourceBranch != "" {
+ q.Set("sourceBranch", params.SourceBranch)
+ }
+ if params.TargetBranch != "" {
+ q.Set("targetBranch", params.TargetBranch)
+ }
+ if params.Source == pages.SourceFork && params.Fork != "" {
+ q.Set("fork", params.Fork)
}
+ if len(q) == 0 {
+ return base
+ }
+ return base + "?" + q.Encode()
+}
- branches := result.Branches
- sort.Slice(branches, func(i int, j int) bool {
- return branches[i].Commit.Committer.When.After(branches[j].Commit.Committer.When)
- })
+func (s *Pulls) wizardParams(r *http.Request, repo *models.Repo) (pages.RepoNewPullParams, error) {
+ l := s.logger.With("handler", "wizardParams")
+ user := s.oauth.GetMultiAccountUser(r)
+
+ branches, err := s.listBranches(r.Context(), repo)
+ if err != nil {
+ return pages.RepoNewPullParams{}, err
+ }
+
+ var forks []models.Repo
+ if user != nil {
+ forks, err = db.GetForksByDid(s.db, user.Did)
+ if err != nil {
+ l.Warn("failed to list user forks", "err", err, "user", user.Did)
+ }
+ }
+
+ repoInfo := s.repoResolver.GetRepoInfo(r, user)
+ source, ok := pages.ParseSource(r.FormValue("source"))
+ if !ok {
+ source = pages.SourceBranch
+ if !repoInfo.Roles.IsPushAllowed() {
+ source = pages.SourceFork
+ }
+ }
+
+ sourceBranch := r.FormValue("sourceBranch")
+ targetBranch := r.FormValue("targetBranch")
+ fork := r.FormValue("fork")
+ patch := r.FormValue("patch")
+
+ if source == pages.SourceFork && fork == "" && len(forks) == 1 {
+ fork = fmt.Sprintf("%s/%s", forks[0].Did, forks[0].Name)
+ }
+
+ var forkBranches []types.Branch
+ if source == pages.SourceFork && fork != "" {
+ forkBranches, err = s.listForkBranches(r.Context(), fork)
+ if err != nil {
+ l.Warn("failed to list fork branches", "err", err, "fork", fork)
+ }
+ }
+
+ sourceBranchList := sourceBranchChoices(branches)
+ targetBranch = defaultTargetBranch(branches, targetBranch)
+ sourceBranch = defaultSourceBranch(source, sourceBranch, sourceBranchList, forkBranches)
+
+ comparison, diff, prefetchErr := s.prefetchComparison(r, repo, source, fork, targetBranch, sourceBranch, patch)
+ var prefillErr string
+ if prefetchErr != nil {
+ prefillErr = prefetchErr.Error()
+ }
+
+ emailToDid, err := s.comparisonEmailToDid(comparison)
+ if err != nil {
+ l.Warn("failed to map commit emails to dids", "err", err)
+ emailToDid = make(map[string]string)
+ }
+
+ mergeCheck := s.wizardMergeCheck(r.Context(), repo, targetBranch, comparison)
+
+ refreshUrl := fmt.Sprintf("/%s/pulls/new/refresh", repoInfo.FullName())
+ var diffOpts types.DiffOpts
+ if r.FormValue("diff") == "split" {
+ diffOpts.Split = true
+ }
+ diffOpts.RefreshUrl = refreshUrl
- withoutDefault := []types.Branch{}
- for _, b := range branches {
- if b.IsDefault {
+ labelDefs, err := s.pullLabelDefs(repo)
+ if err != nil {
+ l.Warn("failed to load label definitions", "err", err)
+ }
+ labelState := labelStateFromForm(r.Form, labelDefs)
+ perCidLabelForms := parseStackLabelForms(r.Form)
+ stackLabelStates := make(map[string]models.LabelState, len(perCidLabelForms))
+ for cid, perForm := range perCidLabelForms {
+ stackLabelStates[cid] = labelStateFromForm(perForm, labelDefs)
+ }
+
+ stackTitles := parseBracketedForm(r.Form, "stackTitle")
+ stackBodies := parseBracketedForm(r.Form, "stackBody")
+ stackSplits := parseBracketedForm(r.Form, "stackSplit")
+
+ title := r.FormValue("title")
+ body := r.FormValue("body")
+ if comparison != nil && len(comparison.FormatPatch) > 0 {
+ first := comparison.FormatPatch[0]
+ if title == "" && first.PatchHeader != nil {
+ title = first.Title
+ }
+ if body == "" && first.PatchHeader != nil {
+ body = first.Body
+ }
+ }
+
+ isStacked := r.FormValue("mode") == "stack" && source != pages.SourcePatch
+ var perCommitDiffs []*types.NiceDiff
+ var stackDiffOpts []types.DiffOpts
+ if isStacked {
+ perCommitDiffs, stackDiffOpts = stackPerCommitDiffs(comparison, targetBranch, refreshUrl, stackSplits)
+ }
+
+ return pages.RepoNewPullParams{
+ LoggedInUser: user,
+ RepoInfo: repoInfo,
+ Branches: branches,
+ SourceBranches: sourceBranchList,
+ ForkBranches: forkBranches,
+ Forks: forks,
+ Source: source,
+ SourceBranch: sourceBranch,
+ TargetBranch: targetBranch,
+ Fork: fork,
+ Patch: patch,
+ Title: title,
+ Body: body,
+ IsStacked: isStacked,
+ Comparison: comparison,
+ Diff: diff,
+ PerCommitDiffs: perCommitDiffs,
+ DiffOpts: diffOpts,
+ StackDiffOpts: stackDiffOpts,
+ EmailToDid: emailToDid,
+ MergeCheck: mergeCheck,
+ StackTitles: stackTitles,
+ StackBodies: stackBodies,
+ PrefillError: prefillErr,
+ LabelDefs: labelDefs,
+ LabelState: labelState,
+ StackLabelStates: stackLabelStates,
+ }, nil
+}
+
+func (s *Pulls) pullLabelDefs(repo *models.Repo) (map[string]*models.LabelDefinition, error) {
+ defs, err := db.GetLabelDefinitions(
+ s.db,
+ orm.FilterIn("at_uri", repo.Labels),
+ orm.FilterContains("scope", tangled.RepoPullNSID),
+ )
+ if err != nil {
+ return nil, err
+ }
+
+ out := make(map[string]*models.LabelDefinition, len(defs))
+ for i := range defs {
+ d := defs[i]
+ if !slices.Contains(d.Scope, tangled.RepoPullNSID) {
continue
}
- withoutDefault = append(withoutDefault, b)
+ out[d.AtUri().String()] = &d
}
+ return out, nil
+}
- s.pages.PullCompareBranchesFragment(w, pages.PullCompareBranchesParams{
- RepoInfo: s.repoResolver.GetRepoInfo(r, user),
- Branches: withoutDefault,
- })
+func formLabelEntries(form url.Values, defs map[string]*models.LabelDefinition) iter.Seq2[string, string] {
+ return func(yield func(string, string) bool) {
+ for key := range defs {
+ for _, v := range form[key] {
+ if v == "" {
+ continue
+ }
+ if !yield(key, v) {
+ return
+ }
+ }
+ }
+ }
}
-func (s *Pulls) CompareForksFragment(w http.ResponseWriter, r *http.Request) {
- l := s.logger.With("handler", "CompareForksFragment")
+func labelStateFromForm(form url.Values, defs map[string]*models.LabelDefinition) models.LabelState {
+ state := models.NewLabelState()
+ actx := &models.LabelApplicationCtx{Defs: defs}
+ for key, val := range formLabelEntries(form, defs) {
+ _ = actx.ApplyLabelOp(state, models.LabelOp{
+ Operation: models.LabelOperationAdd,
+ OperandKey: key,
+ OperandValue: val,
+ })
+ }
+ return state
+}
- user := s.oauth.GetMultiAccountUser(r)
- if user != nil {
- l = l.With("user", user.Did)
+func buildCreationLabelOps(
+ userDid syntax.DID,
+ subject syntax.ATURI,
+ rkey string,
+ form url.Values,
+ defs map[string]*models.LabelDefinition,
+ performedAt time.Time,
+) []models.LabelOp {
+ var ops []models.LabelOp
+ for key, val := range formLabelEntries(form, defs) {
+ ops = append(ops, models.LabelOp{
+ Did: userDid.String(),
+ Rkey: rkey,
+ Subject: subject,
+ Operation: models.LabelOperationAdd,
+ OperandKey: key,
+ OperandValue: val,
+ PerformedAt: performedAt,
+ })
}
+ return ops
+}
+
+func (s *Pulls) applyCreationLabels(
+ ctx context.Context,
+ client *atclient.APIClient,
+ userDid syntax.DID,
+ pulls []*models.Pull,
+ form url.Values,
+ repo *models.Repo,
+) {
+ l := s.logger.With("handler", "applyCreationLabels", "user", userDid)
- forks, err := db.GetForksByDid(s.db, user.Did)
+ defs, err := s.pullLabelDefs(repo)
if err != nil {
- l.Error("failed to get forks", "err", err)
+ l.Warn("failed to fetch label defs", "err", err)
+ return
+ }
+ if len(defs) == 0 {
return
}
- s.pages.PullCompareForkFragment(w, pages.PullCompareForkParams{
- RepoInfo: s.repoResolver.GetRepoInfo(r, user),
- Forks: forks,
- Selected: r.URL.Query().Get("fork"),
- })
-}
+ perCidForms := parseStackLabelForms(form)
-func (s *Pulls) CompareForksBranchesFragment(w http.ResponseWriter, r *http.Request) {
- l := s.logger.With("handler", "CompareForksBranchesFragment")
+ performedAt := time.Now()
+ for _, pull := range pulls {
+ labelForm := form
+ if len(perCidForms) > 0 && len(pull.Submissions) > 0 {
+ if cid := pull.Submissions[0].ChangeId(); cid != "" {
+ if perForm, ok := perCidForms[cid]; ok {
+ labelForm = perForm
+ }
+ }
+ }
+ rkey := tid.TID()
+ raw := buildCreationLabelOps(userDid, pull.AtUri(), rkey, labelForm, defs, performedAt)
- user := s.oauth.GetMultiAccountUser(r)
- if user != nil {
- l = l.With("user", user.Did)
+ valid := make([]models.LabelOp, 0, len(raw))
+ for _, op := range raw {
+ def := defs[op.OperandKey]
+ if err := s.validator.ValidateLabelOp(def, repo, &op); err != nil {
+ l.Warn("invalid label op", "err", err, "subject", op.Subject, "key", op.OperandKey)
+ continue
+ }
+ valid = append(valid, op)
+ }
+ if len(valid) == 0 {
+ continue
+ }
+
+ record := models.LabelOpsAsRecord(valid)
+ if _, err := comatproto.RepoPutRecord(ctx, client, &comatproto.RepoPutRecord_Input{
+ Collection: tangled.LabelOpNSID,
+ Repo: userDid.String(),
+ Rkey: rkey,
+ Record: &lexutil.LexiconTypeDecoder{Val: &record},
+ }); err != nil {
+ l.Warn("failed to write label ops to PDS", "err", err, "subject", pull.AtUri())
+ continue
+ }
+
+ if err := s.indexLabelOps(ctx, valid); err != nil {
+ l.Warn("failed to index label ops", "err", err, "subject", pull.AtUri())
+ if _, err := comatproto.RepoDeleteRecord(context.Background(), client, &comatproto.RepoDeleteRecord_Input{
+ Collection: tangled.LabelOpNSID,
+ Repo: userDid.String(),
+ Rkey: rkey,
+ }); err != nil {
+ l.Warn("failed to rollback label ops record from PDS", "err", err, "subject", pull.AtUri())
+ }
+ continue
+ }
+
+ s.notifier.NewPullLabelOp(ctx, pull)
}
+}
- f, err := s.repoResolver.Resolve(r)
+func (s *Pulls) indexLabelOps(ctx context.Context, ops []models.LabelOp) error {
+ tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
- l.Error("failed to get repo and knot", "err", err)
- return
+ return err
}
+ defer tx.Rollback()
+ for _, op := range ops {
+ if _, err := db.AddLabelOp(tx, &op); err != nil {
+ return err
+ }
+ }
+ return tx.Commit()
+}
+func (s *Pulls) listBranches(ctx context.Context, repo *models.Repo) ([]types.Branch, error) {
xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url}
+ xrpcBytes, err := tangled.GitTempListBranches(ctx, xrpcc, "", 0, repo.RepoAt().String())
+ if err != nil {
+ return nil, err
+ }
+ var result types.RepoBranchesResponse
+ if err := json.Unmarshal(xrpcBytes, &result); err != nil {
+ return nil, err
+ }
+ return result.Branches, nil
+}
- forkVal := r.URL.Query().Get("fork")
- repoString := strings.SplitN(forkVal, "/", 2)
- forkOwnerDid := repoString[0]
- forkName := repoString[1]
- // fork repo
- repo, err := db.GetRepo(
- s.db,
- orm.FilterEq("did", forkOwnerDid),
- orm.FilterEq("name", forkName),
- )
+func (s *Pulls) listForkBranches(ctx context.Context, forkIdent string) ([]types.Branch, error) {
+ parts := strings.SplitN(forkIdent, "/", 2)
+ if len(parts) != 2 {
+ return nil, fmt.Errorf("invalid fork identifier: %s", forkIdent)
+ }
+ forkRepo, err := db.GetRepo(s.db, orm.FilterEq("did", parts[0]), orm.FilterEq("name", parts[1]))
if err != nil {
- l.Error("failed to get repo", "fork_owner_did", forkOwnerDid, "fork_name", forkName, "err", err)
- return
+ return nil, err
+ }
+ branches, err := s.listBranches(ctx, forkRepo)
+ if err != nil {
+ return nil, err
}
+ return sortBranchesByRecency(branches), nil
+}
+
+func sourceBranchChoices(branches []types.Branch) []types.Branch {
+ withoutDefault := slices.DeleteFunc(slices.Clone(branches), func(b types.Branch) bool {
+ return b.IsDefault
+ })
+ return sortBranchesByRecency(withoutDefault)
+}
+
+func defaultTargetBranch(branches []types.Branch, current string) string {
+ if slices.ContainsFunc(branches, func(b types.Branch) bool { return b.Reference.Name == current }) {
+ return current
+ }
+ if idx := slices.IndexFunc(branches, func(b types.Branch) bool { return b.IsDefault }); idx >= 0 {
+ return branches[idx].Reference.Name
+ }
+ return ""
+}
- sourceXrpcBytes, err := tangled.GitTempListBranches(r.Context(), xrpcc, "", 0, repo.RepoAt().String())
+func defaultSourceBranch(source pages.Source, current string, branchChoices, forkBranches []types.Branch) string {
+ var candidates []types.Branch
+ switch source {
+ case pages.SourceFork:
+ candidates = forkBranches
+ case pages.SourceBranch:
+ candidates = branchChoices
+ default:
+ return current
+ }
+ if slices.ContainsFunc(candidates, func(b types.Branch) bool { return b.Reference.Name == current }) {
+ return current
+ }
+ if len(candidates) == 0 {
+ return ""
+ }
+ return candidates[0].Reference.Name
+}
+
+func sortBranchesByRecency(branches []types.Branch) []types.Branch {
+ out := slices.Clone(branches)
+ sort.SliceStable(out, func(i, j int) bool {
+ if out[i].Commit == nil || out[j].Commit == nil {
+ return out[i].Commit != nil
+ }
+ return out[i].Commit.Committer.When.After(out[j].Commit.Committer.When)
+ })
+ return out
+}
+
+func (s *Pulls) prefetchComparison(r *http.Request, repo *models.Repo, source pages.Source, fork, targetBranch, sourceBranch, patch string) (*types.RepoFormatPatchResponse, *types.NiceDiff, error) {
+ var (
+ comparison *types.RepoFormatPatchResponse
+ err error
+ )
+ switch source {
+ case pages.SourcePatch:
+ if strings.TrimSpace(patch) == "" {
+ return nil, nil, nil
+ }
+ if verr := s.validator.ValidatePatch(&patch); verr != nil {
+ return nil, nil, fmt.Errorf("invalid patch: paste a valid git diff or format-patch")
+ }
+ comparison = parsePastedPatch(patch)
+ case pages.SourceBranch:
+ if targetBranch == "" || sourceBranch == "" {
+ return nil, nil, nil
+ }
+ comparison, err = s.fetchBranchComparison(r.Context(), repo, targetBranch, sourceBranch)
+ case pages.SourceFork:
+ if fork == "" || targetBranch == "" || sourceBranch == "" {
+ return nil, nil, nil
+ }
+ comparison, err = s.fetchForkComparison(r, fork, targetBranch, sourceBranch)
+ default:
+ return nil, nil, nil
+ }
if err != nil {
- if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
- l.Error("failed to call XRPC repo.branches for source", "xrpcerr", xrpcerr, "err", err)
- s.pages.Error503(w)
- return
+ s.logger.With("handler", "prefetchComparison").Warn("failed to pre-fetch comparison", "err", err, "source", source)
+ return nil, nil, err
+ }
+
+ return comparison, deriveDiff(comparison, targetBranch), nil
+}
+
+func (s *Pulls) wizardMergeCheck(ctx context.Context, repo *models.Repo, targetBranch string, comparison *types.RepoFormatPatchResponse) *types.MergeCheckResponse {
+ if comparison == nil || targetBranch == "" {
+ return nil
+ }
+ patch := comparison.CombinedPatchRaw
+ if patch == "" {
+ patch = comparison.FormatPatchRaw
+ }
+ if patch == "" {
+ return nil
+ }
+
+ xrpcc := s.knotClient(repo.Knot)
+
+ resp, err := tangled.RepoMergeCheck(ctx, xrpcc, &tangled.RepoMergeCheck_Input{
+ Did: repo.Did,
+ Name: repo.Name,
+ Branch: targetBranch,
+ Patch: patch,
+ })
+ if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
+ s.logger.With("handler", "wizardMergeCheck").Warn("failed to check mergeability", "xrpcerr", xrpcerr, "err", err, "target_branch", targetBranch)
+ return &types.MergeCheckResponse{Error: xrpcerr.Error()}
+ }
+
+ out := mergeCheckResponseFrom(resp)
+ return &out
+}
+
+func bracketComponents(key, prefix string) ([]string, bool) {
+ if !strings.HasPrefix(key, prefix) {
+ return nil, false
+ }
+ rest := key[len(prefix):]
+ var parts []string
+ for len(rest) > 0 {
+ if !strings.HasPrefix(rest, "[") {
+ return nil, false
}
- l.Error("failed to fetch source branches", "err", err)
- return
+ end := strings.Index(rest, "]")
+ if end <= 0 {
+ return nil, false
+ }
+ parts = append(parts, rest[1:end])
+ rest = rest[end+1:]
+ }
+ if len(parts) == 0 {
+ return nil, false
}
+ return parts, true
+}
- // Decode source branches
- var sourceBranches types.RepoBranchesResponse
- if err := json.Unmarshal(sourceXrpcBytes, &sourceBranches); err != nil {
- l.Error("failed to decode source branches XRPC response", "err", err)
- s.pages.Error503(w)
- return
+func parseBracketedForm(form url.Values, prefix string) map[string]string {
+ out := make(map[string]string)
+ for key, vals := range form {
+ parts, ok := bracketComponents(key, prefix)
+ if !ok || len(parts) != 1 || parts[0] == "" || len(vals) == 0 {
+ continue
+ }
+ out[parts[0]] = vals[0]
+ }
+ return out
+}
+
+func parseStackLabelForms(form url.Values) map[string]url.Values {
+ out := make(map[string]url.Values)
+ for key, vals := range form {
+ parts, ok := bracketComponents(key, "stackLabel")
+ if !ok || len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+ continue
+ }
+ cid, atUri := parts[0], parts[1]
+ if _, ok := out[cid]; !ok {
+ out[cid] = make(url.Values)
+ }
+ out[cid][atUri] = append(out[cid][atUri], vals...)
+ }
+ return out
+}
+
+func (s *Pulls) comparisonEmailToDid(comparison *types.RepoFormatPatchResponse) (map[string]string, error) {
+ if comparison == nil {
+ return make(map[string]string), nil
}
+ seen := make(map[string]struct{})
+ for _, p := range comparison.FormatPatch {
+ if p.PatchHeader == nil {
+ continue
+ }
+ if p.Author != nil && p.Author.Email != "" {
+ seen[p.Author.Email] = struct{}{}
+ }
+ if p.Committer != nil && p.Committer.Email != "" {
+ seen[p.Committer.Email] = struct{}{}
+ }
+ }
+ if len(seen) == 0 {
+ return make(map[string]string), nil
+ }
+ emails := slices.Collect(maps.Keys(seen))
+ return db.GetEmailToDid(s.db, emails, true)
+}
- targetXrpcBytes, err := tangled.GitTempListBranches(r.Context(), xrpcc, "", 0, f.RepoAt().String())
+func parsePastedPatch(patch string) *types.RepoFormatPatchResponse {
+ if patch == "" {
+ return nil
+ }
+ response := &types.RepoFormatPatchResponse{FormatPatchRaw: patch}
+ if patchutil.IsFormatPatch(patch) {
+ if patches, err := patchutil.ExtractPatches(patch); err == nil {
+ response.FormatPatch = patches
+ }
+ }
+ return response
+}
+
+func (s *Pulls) fetchBranchComparison(ctx context.Context, repo *models.Repo, targetBranch, sourceBranch string) (*types.RepoFormatPatchResponse, error) {
+ xrpcc := s.knotClient(repo.Knot)
+
+ xrpcBytes, err := tangled.RepoCompare(ctx, xrpcc, repo.RepoIdentifier(), targetBranch, sourceBranch)
if err != nil {
- if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
- l.Error("failed to call XRPC repo.branches for target", "xrpcerr", xrpcerr, "err", err)
- s.pages.Error503(w)
- return
+ return nil, err
+ }
+
+ var comparison types.RepoFormatPatchResponse
+ if err := json.Unmarshal(xrpcBytes, &comparison); err != nil {
+ return nil, err
+ }
+ return &comparison, nil
+}
+
+func (s *Pulls) fetchForkComparison(r *http.Request, forkIdent, targetBranch, sourceBranch string) (*types.RepoFormatPatchResponse, error) {
+ parts := strings.SplitN(forkIdent, "/", 2)
+ if len(parts) != 2 {
+ return nil, fmt.Errorf("invalid fork identifier: %s", forkIdent)
+ }
+ fork, err := db.GetForkByDid(s.db, parts[0], parts[1])
+ if err != nil {
+ return nil, err
+ }
+
+ client, err := s.oauth.ServiceClient(
+ r,
+ oauth.WithService(fork.Knot),
+ oauth.WithLxm(tangled.RepoHiddenRefNSID),
+ oauth.WithDev(s.config.Core.Dev),
+ )
+ if err != nil {
+ return nil, err
+ }
+
+ resp, err := tangled.RepoHiddenRef(
+ r.Context(),
+ client,
+ &tangled.RepoHiddenRef_Input{
+ ForkRef: sourceBranch,
+ RemoteRef: targetBranch,
+ Repo: fork.RepoAt().String(),
+ },
+ )
+ if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
+ return nil, xrpcerr
+ }
+ if !resp.Success {
+ if resp.Error != nil {
+ return nil, fmt.Errorf("hidden ref failed: %s", *resp.Error)
}
- l.Error("failed to fetch target branches", "err", err)
- return
+ return nil, fmt.Errorf("hidden ref failed")
}
- // Decode target branches
- var targetBranches types.RepoBranchesResponse
- if err := json.Unmarshal(targetXrpcBytes, &targetBranches); err != nil {
- l.Error("failed to decode target branches XRPC response", "err", err)
- s.pages.Error503(w)
- return
+ hiddenRef := fmt.Sprintf("hidden/%s/%s", sourceBranch, targetBranch)
+ forkXrpcc := s.knotClient(fork.Knot)
+
+ forkXrpcBytes, err := tangled.RepoCompare(r.Context(), forkXrpcc, fork.RepoIdentifier(), hiddenRef, sourceBranch)
+ if err != nil {
+ return nil, err
}
- sort.Slice(sourceBranches.Branches, func(i int, j int) bool {
- return sourceBranches.Branches[i].Commit.Committer.When.After(sourceBranches.Branches[j].Commit.Committer.When)
- })
+ var comparison types.RepoFormatPatchResponse
+ if err := json.Unmarshal(forkXrpcBytes, &comparison); err != nil {
+ return nil, err
+ }
+ return &comparison, nil
+}
- s.pages.PullCompareForkBranchesFragment(w, pages.PullCompareForkBranchesParams{
- RepoInfo: s.repoResolver.GetRepoInfo(r, user),
- SourceBranches: sourceBranches.Branches,
- TargetBranches: targetBranches.Branches,
- })
+func stackPerCommitDiffs(
+ comparison *types.RepoFormatPatchResponse,
+ targetBranch, refreshUrl string,
+ stackSplits map[string]string,
+) ([]*types.NiceDiff, []types.DiffOpts) {
+ if comparison == nil {
+ return nil, nil
+ }
+ n := len(comparison.FormatPatch)
+ diffs := make([]*types.NiceDiff, n)
+ opts := make([]types.DiffOpts, n)
+ for i, p := range comparison.FormatPatch {
+ nd := patchutil.AsNiceDiff(p.Raw, targetBranch)
+ diffs[i] = &nd
+ cid := p.ChangeIdOrEmpty()
+ if cid == "" {
+ continue
+ }
+ opts[i] = types.DiffOpts{
+ Split: stackSplits[cid] == "split",
+ RefreshUrl: refreshUrl,
+ Target: fmt.Sprintf("#stack-diff-%s", cid),
+ Field: fmt.Sprintf("stackSplit[%s]", cid),
+ }
+ }
+ return diffs, opts
+}
+
+func deriveDiff(comparison *types.RepoFormatPatchResponse, targetBranch string) *types.NiceDiff {
+ if comparison == nil {
+ return nil
+ }
+ raw := comparison.CombinedPatchRaw
+ if raw == "" {
+ raw = comparison.FormatPatchRaw
+ }
+ d := patchutil.AsNiceDiff(raw, targetBranch)
+ return &d
}
func (s *Pulls) ResubmitPull(w http.ResponseWriter, r *http.Request) {
@@ -1804,14 +2305,7 @@ func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) {
return
}
- scheme := "http"
- if !s.config.Core.Dev {
- scheme = "https"
- }
- host := fmt.Sprintf("%s://%s", scheme, f.Knot)
- xrpcc := &indigoxrpc.Client{
- Host: host,
- }
+ xrpcc := s.knotClient(f.Knot)
xrpcBytes, err := tangled.RepoCompare(r.Context(), xrpcc, f.RepoIdentifier(), pull.TargetBranch, pull.PullSource.Branch)
if err != nil {
@@ -1908,12 +2402,7 @@ func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) {
hiddenRef := fmt.Sprintf("hidden/%s/%s", pull.PullSource.Branch, pull.TargetBranch)
// extract patch by performing compare
- forkScheme := "http"
- if !s.config.Core.Dev {
- forkScheme = "https"
- }
- forkHost := fmt.Sprintf("%s://%s", forkScheme, forkRepo.Knot)
- forkXrpcBytes, err := tangled.RepoCompare(r.Context(), &indigoxrpc.Client{Host: forkHost}, forkRepo.RepoIdentifier(), hiddenRef, pull.PullSource.Branch)
+ forkXrpcBytes, err := tangled.RepoCompare(r.Context(), s.knotClient(forkRepo.Knot), forkRepo.RepoIdentifier(), hiddenRef, pull.PullSource.Branch)
if err != nil {
if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil {
l.Error("failed to call XRPC repo.compare for fork", "xrpcerr", xrpcerr, "err", err, "hidden_ref", hiddenRef, "source_branch", pull.PullSource.Branch)
@@ -2086,7 +2575,7 @@ func (s *Pulls) resubmitStackedPullHelper(
blobs[i] = blob.Blob
}
- newStack, err := s.newStack(r.Context(), repo, userDid, targetBranch, pull.PullSource, formatPatches, blobs)
+ newStack, err := s.newStack(r.Context(), repo, userDid, targetBranch, pull.PullSource, formatPatches, blobs, nil, nil)
if err != nil {
l.Error("failed to create resubmitted stack", "err", err)
s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.")
@@ -2583,18 +3072,25 @@ func (s *Pulls) newStack(
pullSource *models.PullSource,
formatPatches []types.FormatPatch,
blobs []*lexutil.LexBlob,
+ stackTitles, stackBodies map[string]string,
) (models.Stack, error) {
var stack models.Stack
var parentAtUri *syntax.ATURI
for i, fp := range formatPatches {
// all patches must have a jj change-id
- _, err := fp.ChangeId()
+ cid, err := fp.ChangeId()
if err != nil {
return nil, fmt.Errorf("Stacking is only supported if all patches contain a change-id commit header.")
}
title := fp.Title
body := fp.Body
+ if override, ok := stackTitles[cid]; ok && strings.TrimSpace(override) != "" {
+ title = override
+ }
+ if override, ok := stackBodies[cid]; ok {
+ body = override
+ }
rkey := tid.TID()
mentions, references := s.mentionsResolver.Resolve(ctx, body)
diff --git a/appview/pulls/router.go b/appview/pulls/router.go
index c1a0fce4..cb782cb5 100644
--- a/appview/pulls/router.go
+++ b/appview/pulls/router.go
@@ -12,11 +12,10 @@ func (s *Pulls) Router(mw *middleware.Middleware) http.Handler {
r.With(middleware.Paginate).Get("/", s.RepoPulls)
r.With(middleware.AuthMiddleware(s.oauth)).Route("/new", func(r chi.Router) {
r.Get("/", s.NewPull)
- r.Get("/patch-upload", s.PatchUploadFragment)
r.Post("/validate-patch", s.ValidatePatch)
- r.Get("/compare-branches", s.CompareBranchesFragment)
- r.Get("/compare-forks", s.CompareForksFragment)
- r.Get("/fork-branches", s.CompareForksBranchesFragment)
+ r.Get("/refresh", s.RefreshWizard)
+ r.Post("/refresh", s.RefreshWizard)
+ r.Post("/preview", s.MarkdownPreview)
r.Post("/", s.NewPull)
})
diff --git a/appview/pulls/wizard_helpers_test.go b/appview/pulls/wizard_helpers_test.go
new file mode 100644
index 00000000..dc3c5e99
--- /dev/null
+++ b/appview/pulls/wizard_helpers_test.go
@@ -0,0 +1,269 @@
+package pulls
+
+import (
+ "net/url"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/go-git/go-git/v5/plumbing/object"
+ "tangled.org/core/appview/models"
+ "tangled.org/core/appview/pages"
+ "tangled.org/core/appview/pages/repoinfo"
+ "tangled.org/core/types"
+)
+
+func TestBracketComponents(t *testing.T) {
+ cases := []struct {
+ key, prefix string
+ want []string
+ ok bool
+ }{
+ {"foo[a]", "foo", []string{"a"}, true},
+ {"foo[a][b]", "foo", []string{"a", "b"}, true},
+ {"foo[a][b][c]", "foo", []string{"a", "b", "c"}, true},
+ {"foo[]", "foo", []string{""}, true},
+ {"foo[a][]", "foo", []string{"a", ""}, true},
+ {"foo", "foo", nil, false},
+ {"bar[a]", "foo", nil, false},
+ {"foo[a", "foo", nil, false},
+ {"fooa]", "foo", nil, false},
+ {"foo[a]extra", "foo", nil, false},
+ {"", "foo", nil, false},
+ }
+ for _, c := range cases {
+ got, ok := bracketComponents(c.key, c.prefix)
+ if ok != c.ok || !reflect.DeepEqual(got, c.want) {
+ t.Errorf("bracketComponents(%q, %q) = %v, %v; want %v, %v", c.key, c.prefix, got, ok, c.want, c.ok)
+ }
+ }
+}
+
+func TestParseBracketedForm(t *testing.T) {
+ form := url.Values{
+ "stackTitle[abc]": {"hello"},
+ "stackTitle[xyz]": {"world", "ignored"},
+ "stackTitle[]": {"empty-id"},
+ "stackTitle[a][b]": {"too-deep"},
+ "stackTitle": {"no-bracket"},
+ "unrelated[abc]": {"skip"},
+ "stackTitle[noval]": {},
+ }
+ got := parseBracketedForm(form, "stackTitle")
+ want := map[string]string{
+ "abc": "hello",
+ "xyz": "world",
+ }
+ if !reflect.DeepEqual(got, want) {
+ t.Errorf("parseBracketedForm = %v; want %v", got, want)
+ }
+}
+
+func TestParseStackLabelForms(t *testing.T) {
+ form := url.Values{
+ "stackLabel[c1][at://uri/a]": {"v1"},
+ "stackLabel[c1][at://uri/b]": {"v2"},
+ "stackLabel[c2][at://uri/a]": {"v3", "v4"},
+ "stackLabel[c1][]": {"empty-uri"},
+ "stackLabel[][at://uri/a]": {"empty-cid"},
+ "stackLabel[c1]": {"missing-second-bracket"},
+ "stackLabel[c1][a][b]": {"too-deep"},
+ "stackTitle[c1]": {"wrong-prefix"},
+ }
+ got := parseStackLabelForms(form)
+ want := map[string]url.Values{
+ "c1": {
+ "at://uri/a": {"v1"},
+ "at://uri/b": {"v2"},
+ },
+ "c2": {
+ "at://uri/a": {"v3", "v4"},
+ },
+ }
+ if !reflect.DeepEqual(got, want) {
+ t.Errorf("parseStackLabelForms = %v; want %v", got, want)
+ }
+}
+
+func TestDefaultTargetBranch(t *testing.T) {
+ branches := []types.Branch{
+ {Reference: types.Reference{Name: "feature"}},
+ {Reference: types.Reference{Name: "main"}, IsDefault: true},
+ }
+ cases := []struct {
+ name string
+ branches []types.Branch
+ current string
+ want string
+ }{
+ {"current is valid", branches, "feature", "feature"},
+ {"current is default", branches, "main", "main"},
+ {"current invalid, falls to default", branches, "ghost", "main"},
+ {"current empty, falls to default", branches, "", "main"},
+ {"no default, no match returns empty", []types.Branch{{Reference: types.Reference{Name: "only"}}}, "ghost", ""},
+ {"empty branches returns empty", nil, "anything", ""},
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ if got := defaultTargetBranch(c.branches, c.current); got != c.want {
+ t.Errorf("defaultTargetBranch = %q; want %q", got, c.want)
+ }
+ })
+ }
+}
+
+func TestDefaultSourceBranch(t *testing.T) {
+ choices := []types.Branch{
+ {Reference: types.Reference{Name: "feature"}},
+ {Reference: types.Reference{Name: "wip"}},
+ }
+ forks := []types.Branch{
+ {Reference: types.Reference{Name: "fork-feature"}},
+ }
+ cases := []struct {
+ name string
+ source pages.Source
+ current string
+ want string
+ }{
+ {"branch source, valid current", pages.SourceBranch, "feature", "feature"},
+ {"branch source, invalid falls to first", pages.SourceBranch, "ghost", "feature"},
+ {"branch source, empty falls to first", pages.SourceBranch, "", "feature"},
+ {"fork source, valid current", pages.SourceFork, "fork-feature", "fork-feature"},
+ {"fork source, invalid falls to first fork", pages.SourceFork, "ghost", "fork-feature"},
+ {"patch source preserves current", pages.SourcePatch, "anything", "anything"},
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ if got := defaultSourceBranch(c.source, c.current, choices, forks); got != c.want {
+ t.Errorf("defaultSourceBranch = %q; want %q", got, c.want)
+ }
+ })
+ }
+ if got := defaultSourceBranch(pages.SourceBranch, "", nil, nil); got != "" {
+ t.Errorf("empty choices should return empty, got %q", got)
+ }
+}
+
+func TestSortBranchesByRecency(t *testing.T) {
+ mk := func(name string, when *time.Time) types.Branch {
+ b := types.Branch{Reference: types.Reference{Name: name}}
+ if when != nil {
+ b.Commit = &object.Commit{Committer: object.Signature{When: *when}}
+ }
+ return b
+ }
+ t1 := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+ t2 := time.Date(2026, 2, 1, 0, 0, 0, 0, time.UTC)
+ t3 := time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC)
+
+ in := []types.Branch{
+ mk("oldest", &t1),
+ mk("newest", &t3),
+ mk("nil-commit", nil),
+ mk("middle", &t2),
+ }
+ got := sortBranchesByRecency(in)
+ wantNames := []string{"newest", "middle", "oldest", "nil-commit"}
+ for i, want := range wantNames {
+ if got[i].Reference.Name != want {
+ t.Errorf("position %d: got %q, want %q", i, got[i].Reference.Name, want)
+ }
+ }
+
+ if &got[0] == &in[0] {
+ t.Error("expected new slice, got aliased input")
+ }
+}
+
+func TestWizardCanonicalURL(t *testing.T) {
+ repo := repoinfo.RepoInfo{OwnerDid: "did:plc:abc", Name: "demo"}
+ cases := []struct {
+ name string
+ p pages.RepoNewPullParams
+ want string
+ }{
+ {
+ "defaults",
+ pages.RepoNewPullParams{RepoInfo: repo, Source: pages.SourceBranch},
+ "/did:plc:abc/demo/pulls/new",
+ },
+ {
+ "stacked",
+ pages.RepoNewPullParams{RepoInfo: repo, Source: pages.SourceBranch, IsStacked: true},
+ "/did:plc:abc/demo/pulls/new?mode=stack",
+ },
+ {
+ "fork with selection",
+ pages.RepoNewPullParams{
+ RepoInfo: repo,
+ Source: pages.SourceFork,
+ Fork: "did:plc:other/repo",
+ SourceBranch: "feature",
+ TargetBranch: "main",
+ },
+ "/did:plc:abc/demo/pulls/new?fork=did%3Aplc%3Aother%2Frepo&source=fork&sourceBranch=feature&targetBranch=main",
+ },
+ {
+ "branch with selection drops source param",
+ pages.RepoNewPullParams{
+ RepoInfo: repo,
+ Source: pages.SourceBranch,
+ SourceBranch: "feature",
+ TargetBranch: "main",
+ },
+ "/did:plc:abc/demo/pulls/new?sourceBranch=feature&targetBranch=main",
+ },
+ {
+ "fork field skipped when source != fork",
+ pages.RepoNewPullParams{
+ RepoInfo: repo,
+ Source: pages.SourceBranch,
+ Fork: "stale",
+ },
+ "/did:plc:abc/demo/pulls/new",
+ },
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ if got := wizardCanonicalURL(c.p); got != c.want {
+ t.Errorf("wizardCanonicalURL = %q; want %q", got, c.want)
+ }
+ })
+ }
+}
+
+func TestLabelStateFromForm(t *testing.T) {
+ bug := &models.LabelDefinition{
+ Did: "did:plc:test", Rkey: "bug", Name: "bug",
+ ValueType: models.ValueType{Type: models.ConcreteTypeNull},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ priority := &models.LabelDefinition{
+ Did: "did:plc:test", Rkey: "priority", Name: "priority",
+ ValueType: models.ValueType{Type: models.ConcreteTypeString, Enum: []string{"low", "med", "high"}},
+ Scope: []string{"sh.tangled.repo.pull"},
+ }
+ defs := map[string]*models.LabelDefinition{
+ bug.AtUri().String(): bug,
+ priority.AtUri().String(): priority,
+ }
+
+ form := url.Values{
+ bug.AtUri().String(): {"null"},
+ priority.AtUri().String(): {"high", ""},
+ "unrelated": {"ignored"},
+ }
+ state := labelStateFromForm(form, defs)
+ if !state.ContainsLabel(bug.AtUri().String()) {
+ t.Error("expected bug label in state")
+ }
+ if !state.ContainsLabel(priority.AtUri().String()) {
+ t.Error("expected priority label in state")
+ }
+
+ emptyState := labelStateFromForm(url.Values{}, defs)
+ if emptyState.ContainsLabel(bug.AtUri().String()) {
+ t.Error("empty form should produce empty state")
+ }
+}
diff --git a/knotserver/internal.go b/knotserver/internal.go
index 6a1a8170..1a374c4b 100644
--- a/knotserver/internal.go
+++ b/knotserver/internal.go
@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"net/http"
+ "net/url"
"os"
"path/filepath"
"strings"
@@ -257,9 +258,9 @@ func (h *InternalHandle) PostReceiveHook(w http.ResponseWriter, r *http.Request)
l.Error("failed to insert op", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir)
}
- err = h.emitCompareLink(&resp.Messages, line, ownerDid, repoName, repoDid)
+ err = h.emitPullRequestLink(&resp.Messages, line, ownerDid, repoName, repoDid)
if err != nil {
- l.Error("failed to reply with compare link", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir)
+ l.Error("failed to reply with pull request link", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir)
}
err = h.triggerPipeline(&resp.Messages, line, gitUserDid, ownerDid, repoName, repoDid, pushOptions)
@@ -415,31 +416,31 @@ func (h *InternalHandle) triggerPipeline(
return h.db.InsertEvent(event, h.n)
}
-func (h *InternalHandle) emitCompareLink(
+func (h *InternalHandle) emitPullRequestLink(
clientMsgs *[]string,
line git.PostReceiveLine,
ownerDid string,
repoName string,
repoDid string,
) error {
- // this is a second push to a branch, don't reply with the link again
- if !line.OldSha.IsZero() {
+ if line.NewSha.IsZero() {
return nil
}
// the ref was not updated to a new hash, don't reply with the link
//
// NOTE: do we need this?
- if line.NewSha.String() == line.OldSha.String() {
+ if line.NewSha == line.OldSha {
return nil
}
pushedRef := plumbing.ReferenceName(line.Ref)
+ if !pushedRef.IsBranch() {
+ return nil
+ }
- userIdent, err := h.res.ResolveIdent(context.Background(), ownerDid)
- user := ownerDid
- if err == nil {
- user = userIdent.Handle.String()
+ if !line.OldSha.IsZero() {
+ return nil
}
repoPath, _, _, resolveErr := h.db.ResolveRepoDIDOnDisk(h.c.Repo.ScanPath, repoDid)
@@ -457,20 +458,34 @@ func (h *InternalHandle) emitCompareLink(
return err
}
+ pushedBranch := pushedRef.Short()
+
// pushing to default branch
- if pushedRef == plumbing.NewBranchReferenceName(defaultBranch) {
+ if pushedBranch == defaultBranch {
return nil
}
- // pushing a tag, don't prompt the user the open a PR
- if pushedRef.IsTag() {
- return nil
+ userIdent, err := h.res.ResolveIdent(context.Background(), ownerDid)
+ user := ownerDid
+ if err == nil {
+ user = userIdent.Handle.String()
+ }
+
+ query := url.Values{}
+ query.Set("source", "branch")
+ query.Set("sourceBranch", pushedBranch)
+ query.Set("targetBranch", defaultBranch)
+
+ basePath, err := url.JoinPath(h.c.AppViewEndpoint, user, repoName, "pulls", "new")
+ if err != nil {
+ return err
}
+ pullURL := basePath + "?" + query.Encode()
ZWS := "\u200B"
*clientMsgs = append(*clientMsgs, ZWS)
- *clientMsgs = append(*clientMsgs, fmt.Sprintf("Create a PR pointing to %s", defaultBranch))
- *clientMsgs = append(*clientMsgs, fmt.Sprintf("\t%s/%s/%s/compare/%s...%s", h.c.AppViewEndpoint, user, repoName, defaultBranch, strings.TrimPrefix(line.Ref, "refs/heads/")))
+ *clientMsgs = append(*clientMsgs, "→ Open pull request:")
+ *clientMsgs = append(*clientMsgs, " "+pullURL)
*clientMsgs = append(*clientMsgs, ZWS)
return nil
}
diff --git a/types/diff.go b/types/diff.go
index 5204581b..96509ae0 100644
--- a/types/diff.go
+++ b/types/diff.go
@@ -8,7 +8,10 @@ import (
)
type DiffOpts struct {
- Split bool `json:"split"`
+ Split bool `json:"split"`
+ RefreshUrl string `json:"refresh_url,omitempty"`
+ Target string `json:"target,omitempty"`
+ Field string `json:"field,omitempty"`
}
func (d DiffOpts) Encode() string {
diff --git a/types/patch.go b/types/patch.go
index ce9bfae0..1c725f4b 100644
--- a/types/patch.go
+++ b/types/patch.go
@@ -18,3 +18,11 @@ func (f FormatPatch) ChangeId() (string, error) {
}
return "", fmt.Errorf("no change-id found")
}
+
+func (f FormatPatch) ChangeIdOrEmpty() string {
+ id, err := f.ChangeId()
+ if err != nil {
+ return ""
+ }
+ return id
+}