Skip to content

Commit 7d42f46

Browse files
doi-tdoi-t
committed
Merge branch 'master' into view-the-current-state
2 parents 0ba0a07 + bc3f964 commit 7d42f46

File tree

13 files changed

+177
-39
lines changed

13 files changed

+177
-39
lines changed

‎api/queries_pr.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
342342
return &resp.Repository.PullRequest, nil
343343
}
344344

345-
func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) (*PullRequest, error) {
345+
func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, headBranch string) (*PullRequest, error) {
346346
type response struct {
347347
Repository struct {
348348
PullRequests struct {
@@ -379,9 +379,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string)
379379
}
380380
}`
381381

382-
branchWithoutOwner := branch
383-
if idx := strings.Index(branch, ":"); idx >= 0 {
384-
branchWithoutOwner = branch[idx+1:]
382+
branchWithoutOwner := headBranch
383+
if idx := strings.Index(headBranch, ":"); idx >= 0 {
384+
branchWithoutOwner = headBranch[idx+1:]
385385
}
386386

387387
variables := map[string]interface{}{
@@ -397,12 +397,17 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string)
397397
}
398398

399399
for _, pr := range resp.Repository.PullRequests.Nodes {
400-
if pr.HeadLabel() == branch {
400+
if pr.HeadLabel() == headBranch {
401+
if baseBranch != "" {
402+
if pr.BaseRefName != baseBranch {
403+
continue
404+
}
405+
}
401406
return &pr, nil
402407
}
403408
}
404409

405-
return nil, &NotFoundError{fmt.Errorf("no open pull requests found for branch %q", branch)}
410+
return nil, &NotFoundError{fmt.Errorf("no open pull requests found for branch %q", headBranch)}
406411
}
407412

408413
// CreatePullRequest creates a pull request in a GitHub repository

‎api/queries_repo.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e
154154
keys = append(keys, key)
155155
}
156156
// sort keys to ensure `repo_{N}` entries are processed in order
157-
sort.Sort(sort.StringSlice(keys))
157+
sort.Strings(keys)
158158

159159
// Iterate over keys of GraphQL response data and, based on its name,
160160
// dynamically allocate the target struct an individual message gets decoded to.

‎command/pr.go‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,21 @@ func prView(cmd *cobra.Command, args []string) error {
249249
return err
250250
}
251251

252-
baseRepo, err := determineBaseRepo(cmd, ctx)
253-
if err != nil {
254-
return err
252+
var baseRepo ghrepo.Interface
253+
var prArg string
254+
if len(args) > 0 {
255+
prArg = args[0]
256+
if prNum, repo := prFromURL(prArg); repo != nil {
257+
prArg = prNum
258+
baseRepo = repo
259+
}
260+
}
261+
262+
if baseRepo == nil {
263+
baseRepo, err = determineBaseRepo(cmd, ctx)
264+
if err != nil {
265+
return err
266+
}
255267
}
256268

257269
preview, err := cmd.Flags().GetBool("preview")
@@ -262,7 +274,7 @@ func prView(cmd *cobra.Command, args []string) error {
262274
var openURL string
263275
var pr *api.PullRequest
264276
if len(args) > 0 {
265-
pr, err = prFromArg(apiClient, baseRepo, args[0])
277+
pr, err = prFromArg(apiClient, baseRepo, prArg)
266278
if err != nil {
267279
return err
268280
}
@@ -282,7 +294,7 @@ func prView(cmd *cobra.Command, args []string) error {
282294
}
283295
}
284296
} else {
285-
pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner)
297+
pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
286298
if err != nil {
287299
return err
288300
}
@@ -326,17 +338,19 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error {
326338

327339
var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`)
328340

329-
func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.PullRequest, error) {
330-
if prNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil {
331-
return api.PullRequestByNumber(apiClient, baseRepo, prNumber)
341+
func prFromURL(arg string) (string, ghrepo.Interface) {
342+
if m := prURLRE.FindStringSubmatch(arg); m != nil {
343+
return m[3], ghrepo.New(m[1], m[2])
332344
}
345+
return "", nil
346+
}
333347

334-
if m := prURLRE.FindStringSubmatch(arg); m != nil {
335-
prNumber, _ := strconv.Atoi(m[3])
348+
func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.PullRequest, error) {
349+
if prNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil {
336350
return api.PullRequestByNumber(apiClient, baseRepo, prNumber)
337351
}
338352

339-
return api.PullRequestForBranch(apiClient, baseRepo, arg)
353+
return api.PullRequestForBranch(apiClient, baseRepo, "", arg)
340354
}
341355

342356
func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) {

‎command/pr_checkout.go‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os/exec"
88

99
"github.com/cli/cli/git"
10+
"github.com/cli/cli/internal/ghrepo"
1011
"github.com/cli/cli/utils"
1112
"github.com/spf13/cobra"
1213
)
@@ -18,21 +19,38 @@ func prCheckout(cmd *cobra.Command, args []string) error {
1819
if err != nil {
1920
return err
2021
}
21-
// FIXME: duplicates logic from fsContext.BaseRepo
22-
baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*")
23-
if err != nil {
24-
return err
25-
}
22+
2623
apiClient, err := apiClientForContext(ctx)
2724
if err != nil {
2825
return err
2926
}
3027

31-
pr, err := prFromArg(apiClient, baseRemote, args[0])
28+
var baseRepo ghrepo.Interface
29+
prArg := args[0]
30+
if prNum, repo := prFromURL(prArg); repo != nil {
31+
prArg = prNum
32+
baseRepo = repo
33+
}
34+
35+
if baseRepo == nil {
36+
baseRepo, err = determineBaseRepo(cmd, ctx)
37+
if err != nil {
38+
return err
39+
}
40+
}
41+
42+
pr, err := prFromArg(apiClient, baseRepo, prArg)
3243
if err != nil {
3344
return err
3445
}
3546

47+
baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
48+
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
49+
baseURLOrName := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(baseRepo))
50+
if baseRemote != nil {
51+
baseURLOrName = baseRemote.Name
52+
}
53+
3654
headRemote := baseRemote
3755
if pr.IsCrossRepository {
3856
headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)
@@ -67,15 +85,15 @@ func prCheckout(cmd *cobra.Command, args []string) error {
6785
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)
6886
if newBranchName == currentBranch {
6987
// PR head matches currently checked out branch
70-
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref})
88+
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, ref})
7189
cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"})
7290
} else {
7391
// create a new branch
74-
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)})
92+
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName)})
7593
cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName})
7694
}
7795

78-
remote := baseRemote.Name
96+
remote := baseURLOrName
7997
mergeRef := ref
8098
if pr.MaintainerCanModify {
8199
remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)

‎command/pr_checkout_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package command
22

33
import (
44
"bytes"
5+
"encoding/json"
6+
"io/ioutil"
57
"os/exec"
68
"strings"
79
"testing"
@@ -21,6 +23,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
2123
return ctx
2224
}
2325
http := initFakeHTTP()
26+
http.StubRepoResponse("OWNER", "REPO")
2427

2528
http.StubResponse(200, bytes.NewBufferString(`
2629
{ "data": { "repository": { "pullRequest": {
@@ -112,6 +115,68 @@ func TestPRCheckout_urlArg(t *testing.T) {
112115
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
113116
}
114117

118+
func TestPRCheckout_urlArg_differentBase(t *testing.T) {
119+
ctx := context.NewBlank()
120+
ctx.SetBranch("master")
121+
ctx.SetRemotes(map[string]string{
122+
"origin": "OWNER/REPO",
123+
})
124+
initContext = func() context.Context {
125+
return ctx
126+
}
127+
http := initFakeHTTP()
128+
129+
http.StubResponse(200, bytes.NewBufferString(`
130+
{ "data": { "repository": { "pullRequest": {
131+
"number": 123,
132+
"headRefName": "feature",
133+
"headRepositoryOwner": {
134+
"login": "hubot"
135+
},
136+
"headRepository": {
137+
"name": "POE",
138+
"defaultBranchRef": {
139+
"name": "master"
140+
}
141+
},
142+
"isCrossRepository": false,
143+
"maintainerCanModify": false
144+
} } } }
145+
`))
146+
147+
ranCommands := [][]string{}
148+
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
149+
switch strings.Join(cmd.Args, " ") {
150+
case "git show-ref --verify --quiet refs/heads/feature":
151+
return &errorStub{"exit status: 1"}
152+
default:
153+
ranCommands = append(ranCommands, cmd.Args)
154+
return &test.OutputStub{}
155+
}
156+
})
157+
defer restoreCmd()
158+
159+
output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OTHER/POE/pull/123/files`)
160+
eq(t, err, nil)
161+
eq(t, output.String(), "")
162+
163+
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
164+
reqBody := struct {
165+
Variables struct {
166+
Owner string
167+
Repo string
168+
}
169+
}{}
170+
json.Unmarshal(bodyBytes, &reqBody)
171+
172+
eq(t, reqBody.Variables.Owner, "OTHER")
173+
eq(t, reqBody.Variables.Repo, "POE")
174+
175+
eq(t, len(ranCommands), 5)
176+
eq(t, strings.Join(ranCommands[1], " "), "git fetch https://github.com/OTHER/POE.git refs/pull/123/head:feature")
177+
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote https://github.com/OTHER/POE.git")
178+
}
179+
115180
func TestPRCheckout_branchArg(t *testing.T) {
116181
ctx := context.NewBlank()
117182
ctx.SetBranch("master")
@@ -122,6 +187,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
122187
return ctx
123188
}
124189
http := initFakeHTTP()
190+
http.StubRepoResponse("OWNER", "REPO")
125191

126192
http.StubResponse(200, bytes.NewBufferString(`
127193
{ "data": { "repository": { "pullRequests": { "nodes": [
@@ -171,6 +237,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
171237
return ctx
172238
}
173239
http := initFakeHTTP()
240+
http.StubRepoResponse("OWNER", "REPO")
174241

175242
http.StubResponse(200, bytes.NewBufferString(`
176243
{ "data": { "repository": { "pullRequest": {
@@ -223,6 +290,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
223290
return ctx
224291
}
225292
http := initFakeHTTP()
293+
http.StubRepoResponse("OWNER", "REPO")
226294

227295
http.StubResponse(200, bytes.NewBufferString(`
228296
{ "data": { "repository": { "pullRequest": {
@@ -275,6 +343,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
275343
return ctx
276344
}
277345
http := initFakeHTTP()
346+
http.StubRepoResponse("OWNER", "REPO")
278347

279348
http.StubResponse(200, bytes.NewBufferString(`
280349
{ "data": { "repository": { "pullRequest": {
@@ -327,6 +396,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
327396
return ctx
328397
}
329398
http := initFakeHTTP()
399+
http.StubRepoResponse("OWNER", "REPO")
330400

331401
http.StubResponse(200, bytes.NewBufferString(`
332402
{ "data": { "repository": { "pullRequest": {
@@ -377,6 +447,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
377447
return ctx
378448
}
379449
http := initFakeHTTP()
450+
http.StubRepoResponse("OWNER", "REPO")
380451

381452
http.StubResponse(200, bytes.NewBufferString(`
382453
{ "data": { "repository": { "pullRequest": {
@@ -427,6 +498,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
427498
return ctx
428499
}
429500
http := initFakeHTTP()
501+
http.StubRepoResponse("OWNER", "REPO")
430502

431503
http.StubResponse(200, bytes.NewBufferString(`
432504
{ "data": { "repository": { "pullRequest": {

‎command/pr_create.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,13 @@ func prCreate(cmd *cobra.Command, _ []string) error {
132132
if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) {
133133
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch)
134134
}
135-
existingPR, err := api.PullRequestForBranch(client, baseRepo, headBranchLabel)
135+
existingPR, err := api.PullRequestForBranch(client, baseRepo, baseBranch, headBranchLabel)
136136
var notFound *api.NotFoundError
137137
if err != nil && !errors.As(err, &notFound) {
138138
return fmt.Errorf("error checking for existing pull request: %w", err)
139139
}
140140
if err == nil {
141-
return fmt.Errorf("a pull request for branch %q already exists:\n%s", headBranchLabel, existingPR.URL)
141+
return fmt.Errorf("a pull request for branch %q into branch %q already exists:\n%s", headBranchLabel, baseBranch, existingPR.URL)
142142
}
143143
}
144144

‎command/pr_create_test.go‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ func TestPRCreate_alreadyExists(t *testing.T) {
6464
http.StubResponse(200, bytes.NewBufferString(`
6565
{ "data": { "repository": { "pullRequests": { "nodes": [
6666
{ "url": "https://github.com/OWNER/REPO/pull/123",
67-
"headRefName": "feature" }
67+
"headRefName": "feature",
68+
"baseRefName": "master" }
6869
] } } } }
6970
`))
7071

@@ -78,11 +79,37 @@ func TestPRCreate_alreadyExists(t *testing.T) {
7879
if err == nil {
7980
t.Fatal("error expected, got nil")
8081
}
81-
if err.Error() != "a pull request for branch \"feature\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" {
82+
if err.Error() != "a pull request for branch \"feature\" into branch \"master\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" {
8283
t.Errorf("got error %q", err)
8384
}
8485
}
8586

87+
func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) {
88+
initBlankContext("OWNER/REPO", "feature")
89+
http := initFakeHTTP()
90+
http.StubRepoResponse("OWNER", "REPO")
91+
http.StubResponse(200, bytes.NewBufferString(`
92+
{ "data": { "repository": { "pullRequests": { "nodes": [
93+
{ "url": "https://github.com/OWNER/REPO/pull/123",
94+
"headRefName": "feature",
95+
"baseRefName": "master" }
96+
] } } } }
97+
`))
98+
http.StubResponse(200, bytes.NewBufferString("{}"))
99+
100+
cs, cmdTeardown := initCmdStubber()
101+
defer cmdTeardown()
102+
103+
cs.Stub("") // git status
104+
cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log
105+
cs.Stub("") // git rev-parse
106+
107+
_, err := RunCommand(prCreateCmd, `pr create -BanotherBase -t"cool" -b"nah"`)
108+
if err != nil {
109+
t.Errorf("got unexpected error %q", err)
110+
}
111+
}
112+
86113
func TestPRCreate_web(t *testing.T) {
87114
initBlankContext("OWNER/REPO", "feature")
88115
http := initFakeHTTP()

0 commit comments

Comments
 (0)