Fix detecting PR for current branch pushed to fork

mislav · mislav · commit 201217d88ab0 · 2020-03-02T11:35:20.000+01:00
Now passing base repo as reference to `prSelectorForCurrentBranch()` so
that it doesn't have to (inexactly) determine the base repo itself and
risk it being different than the base repo determined during `pr
status/view`.
diff --git a/api/fake_http.go b/api/fake_http.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"strings"
 )
 
 // FakeHTTP provides a mechanism by which to stub HTTP responses through
@@ -56,3 +57,35 @@ func (f *FakeHTTP) StubRepoResponse(owner, repo string) {
 	}
 	f.responseStubs = append(f.responseStubs, resp)
 }
+
+func (f *FakeHTTP) StubForkedRepoResponse(forkFullName, parentFullName string) {
+	forkRepo := strings.SplitN(forkFullName, "/", 2)
+	parentRepo := strings.SplitN(parentFullName, "/", 2)
+	body := bytes.NewBufferString(fmt.Sprintf(`
+		{ "data": { "repo_000": {
+			"id": "REPOID2",
+			"name": "%s",
+			"owner": {"login": "%s"},
+			"defaultBranchRef": {
+				"name": "master",
+				"target": {"oid": "deadbeef"}
+			},
+			"viewerPermission": "ADMIN",
+			"parent": {
+				"id": "REPOID1",
+				"name": "%s",
+				"owner": {"login": "%s"},
+				"defaultBranchRef": {
+					"name": "master",
+					"target": {"oid": "deadbeef"}
+				},
+				"viewerPermission": "READ"
+			}
+		} } }
+	`, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0]))
+	resp := &http.Response{
+		StatusCode: 200,
+		Body:       ioutil.NopCloser(body),
+	}
+	f.responseStubs = append(f.responseStubs, resp)
+}
diff --git a/command/pr.go b/command/pr.go
@@ -71,16 +71,17 @@ func prStatus(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx)
+	currentUser, err := ctx.AuthLogin()
 	if err != nil {
 		return err
 	}
-	currentUser, err := ctx.AuthLogin()
+
+	baseRepo, err := determineBaseRepo(cmd, ctx)
 	if err != nil {
 		return err
 	}
 
-	baseRepo, err := determineBaseRepo(cmd, ctx)
+	currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo)
 	if err != nil {
 		return err
 	}
@@ -275,7 +276,7 @@ func prView(cmd *cobra.Command, args []string) error {
 		}
 		openURL = pr.URL
 	} else {
-		prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx)
+		prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo)
 		if err != nil {
 			return err
 		}
@@ -345,11 +346,7 @@ func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*a
 	return api.PullRequestForBranch(apiClient, baseRepo, arg)
 }
 
-func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef string, err error) {
-	baseRepo, err := ctx.BaseRepo()
-	if err != nil {
-		return
-	}
+func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) {
 	prHeadRef, err = ctx.Branch()
 	if err != nil {
 		return
diff --git a/command/pr_test.go b/command/pr_test.go
@@ -98,6 +98,36 @@ func TestPRStatus(t *testing.T) {
 	}
 }
 
+func TestPRStatus_fork(t *testing.T) {
+	initBlankContext("OWNER/REPO", "blueberries")
+	http := initFakeHTTP()
+	http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO")
+
+	jsonFile, _ := os.Open("../test/fixtures/prStatusFork.json")
+	defer jsonFile.Close()
+	http.StubResponse(200, jsonFile)
+
+	defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
+		switch strings.Join(cmd.Args, " ") {
+		case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
+			return &outputStub{[]byte(`branch.blueberries.remote origin
+branch.blueberries.merge refs/heads/blueberries`)}
+		default:
+			panic("not implemented")
+		}
+	})()
+
+	output, err := RunCommand(prStatusCmd, "pr status")
+	if err != nil {
+		t.Fatalf("error running command `pr status`: %v", err)
+	}
+
+	branchRE := regexp.MustCompile(`#10.*\[OWNER:blueberries\]`)
+	if !branchRE.MatchString(output.String()) {
+		t.Errorf("did not match current branch:\n%v", output.String())
+	}
+}
+
 func TestPRStatus_reviewsAndChecks(t *testing.T) {
 	initBlankContext("OWNER/REPO", "blueberries")
 	http := initFakeHTTP()
diff --git a/test/fixtures/prStatusFork.json b/test/fixtures/prStatusFork.json
@@ -0,0 +1,31 @@
+{
+  "data": {
+    "repository": {
+      "pullRequests": {
+        "totalCount": 1,
+        "edges": [
+          {
+            "node": {
+              "number": 10,
+              "title": "Blueberries are a good fruit",
+              "url": "https://github.com/PARENT/REPO/pull/10",
+              "headRefName": "blueberries",
+              "headRepositoryOwner": {
+                "login": "OWNER"
+              },
+              "isCrossRepository": true
+            }
+          }
+        ]
+      }
+    },
+    "viewerCreated": {
+      "totalCount": 0,
+      "edges": []
+    },
+    "reviewRequested": {
+      "totalCount": 0,
+      "edges": []
+    }
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -71,16 +71,17 @@ func prStatus(cmd *cobra.Command, args []string) error {`
`71`	`71`	`return err`
`72`	`72`	`}`
`73`	`73`
`74`		`- currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx)`
	`74`	`+ currentUser, err := ctx.AuthLogin()`
`75`	`75`	`if err != nil {`
`76`	`76`	`return err`
`77`	`77`	`}`
`78`		`- currentUser, err := ctx.AuthLogin()`
	`78`	`+`
	`79`	`+ baseRepo, err := determineBaseRepo(cmd, ctx)`
`79`	`80`	`if err != nil {`
`80`	`81`	`return err`
`81`	`82`	`}`
`82`	`83`
`83`		`- baseRepo, err := determineBaseRepo(cmd, ctx)`
	`84`	`+ currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo)`
`84`	`85`	`if err != nil {`
`85`	`86`	`return err`
`86`	`87`	`}`
`@@ -275,7 +276,7 @@ func prView(cmd *cobra.Command, args []string) error {`
`275`	`276`	`}`
`276`	`277`	`openURL = pr.URL`
`277`	`278`	`} else {`
`278`		`- prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx)`
	`279`	`+ prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo)`
`279`	`280`	`if err != nil {`
`280`	`281`	`return err`
`281`	`282`	`}`
`@@ -345,11 +346,7 @@ func prFromArg(apiClient api.Client, baseRepo ghrepo.Interface, arg string) (a`
`345`	`346`	`return api.PullRequestForBranch(apiClient, baseRepo, arg)`
`346`	`347`	`}`
`347`	`348`
`348`		`-func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef string, err error) {`
`349`		`- baseRepo, err := ctx.BaseRepo()`
`350`		`- if err != nil {`
`351`		`- return`
`352`		`- }`
	`349`	`+func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) {`
`353`	`350`	`prHeadRef, err = ctx.Branch()`
`354`	`351`	`if err != nil {`
`355`	`352`	`return`