From 3953ee26ba56c56a695d8619656919b5255e8275 Mon Sep 17 00:00:00 2001 From: oppiliappan Date: Mon, 19 May 2025 15:48:40 +0000 Subject: [PATCH] appview: pulls: display abandoned pulls --- flake.nix | 2 +- appview/db/db.go | 7 ------- appview/db/pulls.go | 39 +++++++++++++++++++++++++++++++++++++-- appview/pages/pages.go | 18 ++++++++++-------- appview/state/middleware.go | 6 ++++++ appview/state/pull.go | 69 +++++++++++++++++++++++++-------------------------------------------- appview/pages/templates/repo/pulls/pull.html | 12 +++++++++--- appview/pages/templates/repo/pulls/fragments/pullActions.html | 13 ++++++++++++- appview/pages/templates/repo/pulls/fragments/pullHeader.html | 5 ++++- appview/pages/templates/repo/pulls/fragments/pullStack.html | 61 ++++++++++++++++++++++++++++++++++++++++--------------------- 10 file(s) changed, 144 insertion(s)(+), 88 deletion(s)(-) diff --git a/flake.nix b/flake.nix --- a/flake.nix +++ b/flake.nix @@ -435,7 +435,7 @@ g = config.services.tangled-knotserver.gitUser; in [ "d /var/lib/knotserver 0770 ${u} ${g} - -" # Create the directory first - "f+ /var/lib/knotserver/secret 0660 ${u} ${g} - KNOT_SERVER_SECRET=679f15000084699abc6a20d3ef449efa3656583f38e456a08f0638250688ff2e" + "f+ /var/lib/knotserver/secret 0660 ${u} ${g} - KNOT_SERVER_SECRET=38a7c3237c2a585807e06a5bcfac92eb39442063f3da306b7acb15cfdc51d19d" ]; services.tangled-knotserver = { enable = true; diff --git a/appview/db/db.go b/appview/db/db.go --- a/appview/db/db.go +++ b/appview/db/db.go @@ -393,9 +393,6 @@ db.Exec("pragma foreign_keys = off;") runMigration(db, "recreate-pulls-column-for-stacking-support", func(tx *sql.Tx) error { _, err := tx.Exec(` - -- disable fk to not delete submissions table - pragma foreign_keys = off; - create table pulls_new ( -- identifiers id integer primary key autoincrement, @@ -446,15 +443,11 @@ drop table pulls; alter table pulls_new rename to pulls; - - -- reenable fk - pragma foreign_keys = on; `) return err }) db.Exec("pragma foreign_keys = on;") ->>>>>>> Conflict 1 of 1 ends return &DB{db}, nil } diff --git a/appview/db/pulls.go b/appview/db/pulls.go --- a/appview/db/pulls.go +++ b/appview/db/pulls.go @@ -49,7 +49,7 @@ func (p PullState) IsClosed() bool { return p == PullClosed } -func (p PullState) IsDelete() bool { +func (p PullState) IsDeleted() bool { return p == PullDeleted } @@ -885,11 +885,12 @@ func SetPullState(e Execer, repoAt syntax.ATURI, pullId int, pullState PullState) error { _, err := e.Exec( - `update pulls set state = ? where repo_at = ? and pull_id = ? and state <> ?`, + `update pulls set state = ? where repo_at = ? and pull_id = ? and (state <> ? or state <> ?)`, pullState, repoAt, pullId, PullDeleted, // only update state of non-deleted pulls + PullMerged, // only update state of non-merged pulls ) return err } @@ -1032,6 +1033,19 @@ return pulls, nil } +func GetAbandonedPulls(e Execer, stackId string) ([]*Pull, error) { + pulls, err := GetPulls( + e, + FilterEq("stack_id", stackId), + FilterEq("state", PullDeleted), + ) + if err != nil { + return nil, err + } + + return pulls, nil +} + // position of this pull in the stack func (stack Stack) Position(pull *Pull) int { return slices.IndexFunc(stack, func(p *Pull) bool { @@ -1095,4 +1109,25 @@ combined.WriteString("\n") } return combined.String() +} + +// filter out PRs that are "active" +// +// PRs that are still open are active +func (stack Stack) Mergeable() Stack { + var mergeable Stack + + for _, p := range stack { + // stop at the first merged PR + if p.State == PullMerged || p.State == PullClosed { + break + } + + // skip over deleted PRs + if p.State != PullDeleted { + mergeable = append(mergeable, p) + } + } + + return mergeable } diff --git a/appview/pages/pages.go b/appview/pages/pages.go --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -738,14 +738,15 @@ } type RepoSinglePullParams struct { - LoggedInUser *oauth.User - RepoInfo repoinfo.RepoInfo - Active string - DidHandleMap map[string]string - Pull *db.Pull - Stack db.Stack - MergeCheck types.MergeCheckResponse - ResubmitCheck ResubmitResult + LoggedInUser *oauth.User + RepoInfo repoinfo.RepoInfo + Active string + DidHandleMap map[string]string + Pull *db.Pull + Stack db.Stack + AbandonedPulls []*db.Pull + MergeCheck types.MergeCheckResponse + ResubmitCheck ResubmitResult } func (p *Pages) RepoSinglePull(w io.Writer, params RepoSinglePullParams) error { @@ -837,6 +838,7 @@ RoundNumber int MergeCheck types.MergeCheckResponse ResubmitCheck ResubmitResult + Stack db.Stack } func (p *Pages) PullActionsFragment(w io.Writer, params PullActionsParams) error { diff --git a/appview/state/middleware.go b/appview/state/middleware.go --- a/appview/state/middleware.go +++ b/appview/state/middleware.go @@ -178,8 +178,14 @@ log.Println("failed to get stack", err) return } + abandonedPulls, err := db.GetAbandonedPulls(s.db, pr.StackId) + if err != nil { + log.Println("failed to get abandoned pulls", err) + return + } ctx = context.WithValue(ctx, "stack", stack) + ctx = context.WithValue(ctx, "abandonedPulls", abandonedPulls) } next.ServeHTTP(w, r.WithContext(ctx)) diff --git a/appview/state/pull.go b/appview/state/pull.go --- a/appview/state/pull.go +++ b/appview/state/pull.go @@ -75,6 +75,7 @@ RoundNumber: roundNumber, MergeCheck: mergeCheckResponse, ResubmitCheck: resubmitResult, + Stack: stack, }) return } @@ -97,6 +98,7 @@ // can be nil if this pull is not stacked stack, _ := r.Context().Value("stack").(db.Stack) + abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*db.Pull) totalIdents := 1 for _, submission := range pull.Submissions { @@ -132,13 +134,14 @@ } s.pages.RepoSinglePull(w, pages.RepoSinglePullParams{ - LoggedInUser: user, - RepoInfo: f.RepoInfo(s, user), - DidHandleMap: didHandleMap, - Pull: pull, - Stack: stack, - MergeCheck: mergeCheckResponse, - ResubmitCheck: resubmitResult, + LoggedInUser: user, + RepoInfo: f.RepoInfo(s, user), + DidHandleMap: didHandleMap, + Pull: pull, + Stack: stack, + AbandonedPulls: abandonedPulls, + MergeCheck: mergeCheckResponse, + ResubmitCheck: resubmitResult, }) } @@ -167,21 +170,9 @@ if pull.IsStacked() { // combine patches of substack subStack := stack.Below(pull) - // collect the portion of the stack that is mergeable - var mergeable db.Stack - for _, p := range subStack { - // stop at the first merged PR - if p.State == db.PullMerged || p.State == db.PullClosed { - break - } - - // skip over deleted PRs - if p.State != db.PullDeleted { - mergeable = append(mergeable, p) - } - } - + mergeable := subStack.Mergeable() + // combine each patch patch = mergeable.CombinedPatch() } @@ -225,7 +216,7 @@ } func (s *State) resubmitCheck(f *FullyResolvedRepo, pull *db.Pull, stack db.Stack) pages.ResubmitResult { - if pull.State == db.PullMerged || pull.PullSource == nil { + if pull.State == db.PullMerged || pull.State == db.PullDeleted || pull.PullSource == nil { return pages.Unknown } @@ -903,6 +894,13 @@ return } + client, err := s.oauth.AuthorizedClient(r) + if err != nil { + log.Println("failed to get authorized client", err) + s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") + return + } + tx, err := s.db.BeginTx(r.Context(), nil) if err != nil { log.Println("failed to start tx") @@ -947,12 +945,6 @@ }) if err != nil { log.Println("failed to create pull request", err) - s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") - return - } - client, err := s.oauth.AuthorizedClient(r) - if err != nil { - log.Println("failed to get authorized client", err) s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") return } @@ -1820,21 +1812,10 @@ // combine patches of substack subStack := stack.Below(pull) - // collect the portion of the stack that is mergeable - for _, p := range subStack { - // stop at the first merged/closed PR - if p.State == db.PullMerged || p.State == db.PullClosed { - break - } - - // skip over deleted PRs - if p.State == db.PullDeleted { - continue - } - - pullsToMerge = append(pullsToMerge, p) - } + mergeable := subStack.Mergeable() + // add to total patch + pullsToMerge = append(pullsToMerge, mergeable...) } patch := pullsToMerge.CombinedPatch() @@ -2014,10 +1995,10 @@ var pullsToReopen []*db.Pull pullsToReopen = append(pullsToReopen, pull) - // if this PR is stacked, then we want to reopen all PRs below this one on the stack + // if this PR is stacked, then we want to reopen all PRs above this one on the stack if pull.IsStacked() { stack := r.Context().Value("stack").(db.Stack) - subStack := stack.StrictlyBelow(pull) + subStack := stack.StrictlyAbove(pull) pullsToReopen = append(pullsToReopen, subStack...) } diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html --- a/appview/pages/templates/repo/pulls/pull.html +++ b/appview/pages/templates/repo/pulls/pull.html @@ -15,7 +15,6 @@ {{ if .Pull.IsStacked }}
-

STACK

{{ template "repo/pulls/fragments/pullStack" . }}
{{ end }} @@ -85,7 +84,7 @@ {{ end }} - + {{ if .IsFormatPatch }} {{ $patches := .AsFormatPatch }} {{ $round := .RoundNumber }} @@ -169,7 +168,7 @@ {{ end }} {{ if $.LoggedInUser }} - {{ template "repo/pulls/fragments/pullActions" (dict "LoggedInUser" $.LoggedInUser "Pull" $.Pull "RepoInfo" $.RepoInfo "RoundNumber" .RoundNumber "MergeCheck" $.MergeCheck "ResubmitCheck" $.ResubmitCheck) }} + {{ template "repo/pulls/fragments/pullActions" (dict "LoggedInUser" $.LoggedInUser "Pull" $.Pull "RepoInfo" $.RepoInfo "RoundNumber" .RoundNumber "MergeCheck" $.MergeCheck "ResubmitCheck" $.ResubmitCheck "Stack" $.Stack) }} {{ else }}
@@ -198,6 +197,13 @@ {{ i "git-merge" "w-4 h-4" }} pull request successfully merged +
+ + {{ else if .Pull.State.IsDeleted }} +
+
+ {{ i "git-pull-request-closed" "w-4 h-4" }} + This pull has been deleted (possibly by jj abandon or jj squash)
{{ else if and .MergeCheck .MergeCheck.Error }} diff --git a/appview/pages/templates/repo/pulls/fragments/pullActions.html b/appview/pages/templates/repo/pulls/fragments/pullActions.html --- a/appview/pages/templates/repo/pulls/fragments/pullActions.html +++ b/appview/pages/templates/repo/pulls/fragments/pullActions.html @@ -1,6 +1,17 @@ {{ define "repo/pulls/fragments/pullActions" }} {{ $lastIdx := sub (len .Pull.Submissions) 1 }} {{ $roundNumber := .RoundNumber }} + {{ $stack := .Stack }} + + {{ $totalPulls := sub 0 1 }} + {{ $below := sub 0 1 }} + {{ $stackCount := "" }} + {{ if .Pull.IsStacked }} + {{ $totalPulls = len $stack }} + {{ $below = $stack.Below .Pull }} + {{ $mergeable := len $below.Mergeable }} + {{ $stackCount = printf "%d/%d" $mergeable $totalPulls }} + {{ end }} {{ $isPushAllowed := .RepoInfo.Roles.IsPushAllowed }} {{ $isMerged := .Pull.State.IsMerged }} @@ -33,7 +44,7 @@ hx-confirm="Are you sure you want to merge pull #{{ .Pull.PullId }} into the `{{ .Pull.TargetBranch }}` branch?" class="btn p-2 flex items-center gap-2 group" {{ $disabled }}> {{ i "git-merge" "w-4 h-4" }} - merge + merge{{if $stackCount}} {{$stackCount}}{{end}} {{ i "loader-circle" "w-4 h-4 animate-spin hidden group-[.htmx-request]:inline" }} {{ end }} diff --git a/appview/pages/templates/repo/pulls/fragments/pullHeader.html b/appview/pages/templates/repo/pulls/fragments/pullHeader.html --- a/appview/pages/templates/repo/pulls/fragments/pullHeader.html +++ b/appview/pages/templates/repo/pulls/fragments/pullHeader.html @@ -2,8 +2,8 @@
{{ block "pullState" .Pull }} {{ end }}

- #{{ .Pull.PullId }} {{ .Pull.Title }} + #{{ .Pull.PullId }}

@@ -96,6 +96,9 @@ {{ else if .State.IsMerged }} {{ $bgColor = "bg-purple-600 dark:bg-purple-700" }} {{ $icon = "git-merge" }} + {{ else if .State.IsDeleted }} + {{ $bgColor = "bg-red-600 dark:bg-red-700" }} + {{ $icon = "git-pull-request-closed" }} {{ end }}
diff --git a/appview/pages/templates/repo/pulls/fragments/pullStack.html b/appview/pages/templates/repo/pulls/fragments/pullStack.html --- a/appview/pages/templates/repo/pulls/fragments/pullStack.html +++ b/appview/pages/templates/repo/pulls/fragments/pullStack.html @@ -1,27 +1,16 @@ {{ define "repo/pulls/fragments/pullStack" }} -
- {{ range $pull := .Stack }} - {{ $isCurrent := false }} - {{ with $.Pull }} - {{ $isCurrent = eq $pull.PullId $.Pull.PullId }} - {{ end }} - -
- {{ if $isCurrent }} - {{ i "arrow-right" "w-4 h-4" }} - {{ end }} -
- {{ block "summarizedHeader" $pull }} {{ end }} -
-
- - {{ end }} -
+

STACK

+ {{ block "pullList" (list .Stack $) }} {{ end }} + + {{ if gt (len .AbandonedPulls) 0 }} +

ABANDONED PULLS

+ {{ block "pullList" (list .AbandonedPulls $) }} {{ end }} + {{ end }} {{ end }} {{ define "summarizedHeader" }}
-
+
{{ block "summarizedPullState" . }} {{ end }} #{{ .PullId }} @@ -35,12 +24,16 @@ {{ $commentCount := len $lastSubmission.Comments }}
+ {{ i "message-square" "w-3 h-3 md:hidden" }} {{ $commentCount }} - comment{{if ne $commentCount 1}}s{{end}} +
- round #{{ $latestRound }} + + + #{{ $latestRound }} +
{{ end }} @@ -55,9 +48,35 @@ {{ else if .State.IsMerged }} {{ $fgColor = "text-purple-600 dark:text-purple-500" }} {{ $icon = "git-merge" }} + {{ else if .State.IsDeleted }} + {{ $fgColor = "text-red-600 dark:text-red-500" }} + {{ $icon = "git-pull-request-closed" }} {{ end }} {{ $style := printf "w-4 h-4 %s" $fgColor }} {{ i $icon $style }} +{{ end }} + +{{ define "pullList" }} + {{ $list := index . 0 }} + {{ $root := index . 1 }} +
+ {{ range $pull := $list }} + {{ $isCurrent := false }} + {{ with $root.Pull }} + {{ $isCurrent = eq $pull.PullId $root.Pull.PullId }} + {{ end }} + +
+ {{ if $isCurrent }} + {{ i "arrow-right" "w-4 h-4" }} + {{ end }} +
+ {{ block "summarizedHeader" $pull }} {{ end }} +
+
+ + {{ end }} +
{{ end }} -- tangled.sh