fix(pr): prefer an open pull request over a merged one recorded by checkout

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-12 20:25:40 +02:00
1 parent 12972e6327
commit badd934d64
2 files changed
+27 -6

No files matched your search

+15 -5
View File
@@ -223,9 +223,10 @@ func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, i
return t, n, nil return t, n, nil
} }
// currentPull finds the pull request of the checked-out branch: the one pr checkout recorded, // currentPull finds the pull request of the checked-out branch: the one pr checkout recorded
// else the open pull request whose head is the branch's upstream (and the upstream remote's // while it is open, else the open pull request whose head is the branch's upstream (and the
// owner), else one whose head has the local branch's name, else N from a pr/N branch. // upstream remote's owner), else one whose head has the local branch's name, else N from a pr/N
// branch, else the recorded pull request even though it is no longer open.
func (a *application) currentPull(ctx context.Context) (target, int64, error) { func (a *application) currentPull(ctx context.Context) (target, int64, error) {
branch, err := git.CurrentBranchConfig(ctx, a.gitProbe(), a.options.cwd) branch, err := git.CurrentBranchConfig(ctx, a.gitProbe(), a.options.cwd)
if err != nil { if err != nil {
@@ -235,10 +236,16 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) {
if err != nil { if err != nil {
return t, 0, err return t, 0, err
} }
service := services.Pulls{Client: t.client, Repository: t.Repo}
// A recorded pull request that was merged or closed only counts when no open one has the branch.
recorded := int64(0)
if branch.PullNumber > 0 && (branch.BaseRemote == "" || a.remoteOwns(ctx, t, branch.BaseRemote)) { if branch.PullNumber > 0 && (branch.BaseRemote == "" || a.remoteOwns(ctx, t, branch.BaseRemote)) {
return t, branch.PullNumber, nil if pull, err := service.View(ctx, branch.PullNumber); err != nil || pull.State == "open" {
return t, branch.PullNumber, nil
}
recorded = branch.PullNumber
} }
pulls, err := (services.Pulls{Client: t.client, Repository: t.Repo}).ListOpen(ctx) pulls, err := service.ListOpen(ctx)
if err != nil { if err != nil {
return t, 0, err return t, 0, err
} }
@@ -261,6 +268,9 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) {
return t, n, nil return t, n, nil
} }
} }
if recorded > 0 {
return t, recorded, nil
}
return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", branch.Name, t.Repo) return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", branch.Name, t.Repo)
} }
+12 -1
View File
@@ -6,6 +6,7 @@ package app
import ( import (
"context" "context"
"net/http" "net/http"
"strings"
"testing" "testing"
"gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/clierr"
@@ -59,10 +60,18 @@ func TestParseRefAcceptsNumbersQualifiedNumbersAndURLs(t *testing.T) {
// refApplication returns an application authenticated against a server listing the open pull // refApplication returns an application authenticated against a server listing the open pull
// requests #3 (feat/one in alice/project), #4 (feat/two), #17 (fix in bob/project) and two // requests #3 (feat/one in alice/project), #4 (feat/two), #17 (fix in bob/project) and two
// pull requests from branch dup. // pull requests from branch dup. Viewing one pull request finds it open, except the merged #30.
func refApplication(t *testing.T, gitResults map[string]string) (*application, string) { func refApplication(t *testing.T, gitResults map[string]string) (*application, string) {
t.Helper() t.Helper()
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if number, ok := strings.CutPrefix(r.URL.Path, "/api/v1/repos/alice/project/pulls/"); ok {
state := "open"
if number == "30" {
state = "closed"
}
_, _ = w.Write([]byte(`{"number":` + number + `,"state":"` + state + `","merged":true}`))
return
}
require.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path) require.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path)
writeFirstPage(w, r, `[ writeFirstPage(w, r, `[
{"number":3,"head":{"ref":"feat/one","repo":{"full_name":"alice/project"}}}, {"number":3,"head":{"ref":"feat/one","repo":{"full_name":"alice/project"}}},
@@ -111,6 +120,8 @@ func TestPullSelectorDefaultsToTheCheckedOutBranch(t *testing.T) {
}{ }{
{"recorded by pr checkout", branch("pr/17", map[string]string{"tea-pr": "17", "tea-base-remote": "origin", "remote": "pulls/bob/project", "merge": "refs/heads/fix"}), 17, ""}, {"recorded by pr checkout", branch("pr/17", map[string]string{"tea-pr": "17", "tea-base-remote": "origin", "remote": "pulls/bob/project", "merge": "refs/heads/fix"}), 17, ""},
{"head branch name kept by pr checkout", branch("fix", map[string]string{"tea-pr": "17", "tea-base-remote": "origin"}), 17, ""}, {"head branch name kept by pr checkout", branch("fix", map[string]string{"tea-pr": "17", "tea-base-remote": "origin"}), 17, ""},
{"recorded but merged, the head branch has a newer one", branch("fix", map[string]string{"tea-pr": "30", "tea-base-remote": "origin"}), 17, ""},
{"recorded but merged, no open one", branch("gone", map[string]string{"tea-pr": "30", "tea-base-remote": "origin"}), 30, ""},
{"recorded for another repository", branch("pr/9", map[string]string{"tea-pr": "9", "tea-base-remote": "fork", "remote": "fork", "merge": "refs/heads/fix"}), 17, ""}, {"recorded for another repository", branch("pr/9", map[string]string{"tea-pr": "9", "tea-base-remote": "fork", "remote": "fork", "merge": "refs/heads/fix"}), 17, ""},
{"upstream branch and owner", branch("local", map[string]string{"remote": "origin", "merge": "refs/heads/feat/one"}), 3, ""}, {"upstream branch and owner", branch("local", map[string]string{"remote": "origin", "merge": "refs/heads/feat/one"}), 3, ""},
{"upstream owner tells forks apart", branch("dup", map[string]string{"remote": "pulls/carol/project", "merge": "refs/heads/dup"}), 21, ""}, {"upstream owner tells forks apart", branch("dup", map[string]string{"remote": "pulls/carol/project", "merge": "refs/heads/dup"}), 21, ""},